Re: [PATCH 00/10] drm/panthor: minor AS_CONTROL clean up

[Steven Price <steven.price@arm.com>]

On 16/09/2025 22:08, Chia-I Wu wrote:
> This series performs minor AS_CONTROL clean up.
> 
> Patch 1 to 5 rename and document AS_CONTROL config functions. There is
> no functional change. All functions are now prefixed by mmu_hw_ for
> consistency. All of them also expect locking. I choose not to suffix
> them by _locked, but I can be convinced.
> 
> Patch 6 to 7 eliminiate redundant mmu_hw_wait_ready. This is the main
> functional change of the series. panthor_vm_flush_range no longer waits
> for UNLOCK to complete.
> 
> Patch 8 to 10 give mmu_hw_flush_caches final touches, to improve error
> handling, simplifying code, etc.

I think you need to provide better justification for these changes. Some
of them might make some sense, but in general most of the "cleanup"
patches by themselves seem to make the code harder to read. Which can be
fine if they are a precursor to achieving an improvement in a following
patch, but as things stand I'm having a hard time to figure out what the
benefit is.

The cover letter implies that we have redundant mmu_hw_wait_ready calls
(which I can believe). But we need a proper justification on why they
are redundant, and proper patch descriptions for the precursor patches
so that anyone coming to them in the future can understand why they were
applied (without having to hunt through mail archives for the cover
letter, or guess from the later patches).

Having said the above, I do appreciate the time you took to write the
documentation blocks - we do have a bunch of fairly confusing functions.

Thanks,
Steve

> Chia-I Wu (10):
>   drm/panthor: rename and document wait_ready
>   drm/panthor: rename and document lock_region
>   drm/panthor: add mmu_hw_cmd_unlock
>   drm/panthor: add mmu_hw_cmd_update
>   drm/panthor: rename and document mmu_hw_do_operation_locked
>   drm/panthor: remove write_cmd
>   drm/panthor: remove unnecessary mmu_hw_wait_ready calls
>   drm/panthor: improve error handling for mmu_hw_flush_caches
>   drm/panthor: move size check to mmu_hw_flush_caches
>   drm/panthor: simplify mmu_hw_flush_caches
> 
>  drivers/gpu/drm/panthor/panthor_mmu.c | 157 +++++++++++++++-----------
>  1 file changed, 94 insertions(+), 63 deletions(-)
>