Re: [PATCH v3 16/29] media: iris: implement iris v4l2_ctrl_ops and prepare capabilities

[Vedang Nagar]

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@xxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxx>
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx>
>> ---
>>  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,
> 
>