[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <96aca644-8fdc-8076-c94b-ed655ac526d2@quicinc.com>
Date: Mon, 29 Sep 2025 11:14:53 +0530
From: Vishnu Reddy <quic_bvisredd@...cinc.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
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>
Subject: Re: [PATCH 5/8] media: iris: Move vpu register defines to common
header file
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.
>
> 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?
Thanks and regards,
Vishnu Reddy
Powered by blists - more mailing lists