Re: [PATCH 5/8] media: iris: Move vpu register defines to common header file

[Konrad Dybcio]

On 9/29/25 7:44 AM, Vishnu Reddy wrote:
> 
> 
> On 9/25/2025 2:40 PM, Konrad Dybcio wrote:
>> On 9/25/25 1:14 AM, Vikash Garodia wrote:
>>> Some of vpu4 register defines are common with vpu3x. Move those into the
>>> common register defines header. This is done to reuse the defines for
>>> vpu4 in subsequent patch which enables the power sequence for vpu4.
>>>
>>> Co-developed-by: Vishnu Reddy <quic_bvisredd@xxxxxxxxxxx>
>>> Signed-off-by: Vishnu Reddy <quic_bvisredd@xxxxxxxxxxx>
>>> Signed-off-by: Vikash Garodia <vikash.garodia@xxxxxxxxxxxxxxxx>
>>> ---
>>>   drivers/media/platform/qcom/iris/iris_vpu3x.c      | 36 ----------------------
>>>   drivers/media/platform/qcom/iris/iris_vpu_common.c | 23 --------------
>>>   .../platform/qcom/iris/iris_vpu_register_defines.h | 29 +++++++++++++++++
>>
>> This is a slippery slope. I think it's better if you explicitly say
>> the header file contains the register map of VPU3 instead, as let's say
>> VPU5 may add a random register in the middle (pushing some existing ones
>> +0x4 down). Such changes are annoying to debug, and we've unfortunately
>> been there on Adreno..
>>
>> Because you're using this for a single common function that is both acting
>> upon the same registers and performing the same operations on them across
>> VPU35 and VPU4, it's okay to de-static-ize the function from iris_vpu3.c and
>> refer to it from vpu4 ops, keeping the register map private to the former
>> file which I think will end up less error-prone for the future.
>>
>> Konrad
> 
>  Just to confirm
>  1. You’re saying it’s better to keep the register definitions for each
>  VPU generation in their own source files, instead of keeping them all
>  in a shared header. Is that right?
> 
>  2. The vpu functions (power on controller, power off controller and
>  etc.) which are common for vpu3x and vpu4x are moved to
>  iris_vpu_common.c and de-static-ize to use for both vpu3x and vpu4x.
>  (This code changes are there in [PATCH 6/8] media: iris: Move vpu35
>  specific api to common to use for vpu4)
> 
>  Will this 2nd point is fine or Do I need the keep the functions also
>  in the platform specific file and reuse for vpu4x by de-static-ize the
>  function in iris_vpu3x.c?

I think we can drop this since Vikash said it's not going to change
much for the forseeable future

Konrad