[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ac56b74d-de7e-4440-894d-7251b2b893e9@oss.qualcomm.com>
Date: Tue, 7 Oct 2025 15:22:52 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Vikash Garodia <vikash.garodia@....qualcomm.com>,
Dikshita Agarwal <dikshita.agarwal@....qualcomm.com>,
Abhinav Kumar <abhinav.kumar@...ux.dev>,
Bryan O'Donoghue <bod@...nel.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>
Cc: linux-arm-msm@...r.kernel.org, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Vishnu Reddy <quic_bvisredd@...cinc.com>
Subject: Re: [PATCH 5/8] media: iris: Move vpu register defines to common
header file
On 10/2/25 11:35 AM, Vikash Garodia 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@...cinc.com>
>>> Signed-off-by: Vishnu Reddy <quic_bvisredd@...cinc.com>
>>> Signed-off-by: Vikash Garodia <vikash.garodia@....qualcomm.com>
>>> ---
>>> 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.
>
> Appreciate your thoughts on this and trying to bring the design issues faced in
> adreno. I peeked into vpu5 register map, and it follows the offsets from vpu4
> and should reuse them from "iris_vpu_register_defines.h". IMO, we should be good
> in reusing them for vpu4 and atleast for next generation.
If you're confident in that, go ahead with the current approach
Konrad
Powered by blists - more mailing lists