linux-kernel - Re: [PATCH v3 16/29] media: iris: implement iris v4l2_ctrl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <15703542-1b70-4042-86b9-7b3f3a675e3e@quicinc.com>
Date: Tue, 1 Oct 2024 18:31:16 +0530
From: Vedang Nagar <quic_vnagar@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        <quic_dikshita@...cinc.com>
CC: Vikash Garodia <quic_vgarodia@...cinc.com>,
        Abhinav Kumar
	<quic_abhinavk@...cinc.com>,
        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>, <linux-media@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 16/29] media: iris: implement iris v4l2_ctrl_ops and
 prepare capabilities

Hi Dmitry,

On 8/29/2024 3:03 PM, Dmitry Baryshkov wrote:
> On Tue, Aug 27, 2024 at 03:35:41PM GMT, Dikshita Agarwal via B4 Relay wrote:
>> From: Vedang Nagar <quic_vnagar@...cinc.com>
>>
>> Implement s_ctrl and g_volatile_ctrl ctrl ops.
>> Introduce platform specific driver and firmware capabilities.
>> Capabilities are set of video specifications
>> and features supported by a specific platform (SOC).
>> Each capability is defined with min, max, range, default
>> value and corresponding HFI.
>>
>> Signed-off-by: Vedang Nagar <quic_vnagar@...cinc.com>
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@...cinc.com>
>> ---
>>  drivers/media/platform/qcom/iris/Makefile          |   1 +
>>  drivers/media/platform/qcom/iris/iris_buffer.c     |   3 +-
>>  drivers/media/platform/qcom/iris/iris_core.h       |   2 +
>>  drivers/media/platform/qcom/iris/iris_ctrls.c      | 194 +++++++++++++++++++++
>>  drivers/media/platform/qcom/iris/iris_ctrls.h      |  15 ++
>>  .../platform/qcom/iris/iris_hfi_gen1_defines.h     |   4 +
>>  .../platform/qcom/iris/iris_hfi_gen2_command.c     |   1 +
>>  .../platform/qcom/iris/iris_hfi_gen2_defines.h     |   9 +
>>  drivers/media/platform/qcom/iris/iris_instance.h   |   6 +
>>  .../platform/qcom/iris/iris_platform_common.h      |  71 ++++++++
>>  .../platform/qcom/iris/iris_platform_sm8250.c      |  56 ++++++
>>  .../platform/qcom/iris/iris_platform_sm8550.c      | 138 +++++++++++++++
>>  drivers/media/platform/qcom/iris/iris_probe.c      |   7 +
>>  drivers/media/platform/qcom/iris/iris_vdec.c       |  24 ++-
>>  drivers/media/platform/qcom/iris/iris_vdec.h       |   2 +-
>>  drivers/media/platform/qcom/iris/iris_vidc.c       |  16 +-
>>  16 files changed, 536 insertions(+), 13 deletions(-)

[Skipped]

>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> index a74114b0761a..6ad2ca7be0f0 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> @@ -108,6 +108,7 @@ static int iris_hfi_gen2_session_set_default_header(struct iris_inst *inst)
>>  	struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
>>  	u32 default_header = false;
>>  
>> +	default_header = inst->fw_cap[DEFAULT_HEADER].value;
> 
> This isn't related to the s_ctrl and g_volatile_ctrl. Please split this
> commit into the chunk that is actually related to that API and the rest
> of the changes.
Could you please help to provide more details on how are you expecting the
split of the patches?

Do you expect to split V4L2 ctrls_init/s_ctrl/g_ctrl in one patch and the
introduction of all the capabilities into another patch? We are not finding
it feasible to split the patch that way as in ctrl_init we read the
capability from platform data to initialize the respective control.

Regards,
Vedang Nagar
> 
>>  	iris_hfi_gen2_packet_session_property(inst,
>>  					      HFI_PROP_DEC_DEFAULT_HEADER,
>>  					      HFI_HOST_FLAGS_NONE,
> 
>