Re: [PATCH 4/4] venus: return P010 as preferred format for 10 bit decode

[Dikshita Agarwal <quic_dikshita@quicinc.com>]

On 5/4/2023 10:50 PM, Konrad Dybcio wrote:
>
> On 4.05.2023 12:36, Dikshita Agarwal wrote:
>> If bit depth is detected as 10 bit by firmware, return
>> P010 as preferred decoder format to the client.
>>
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
>> ---
>>   drivers/media/platform/qcom/venus/vdec.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>> index 69f7f6e..ed11dc2 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -1468,8 +1468,13 @@ static void vdec_event_change(struct venus_inst *inst,
>>   	inst->out_width = ev_data->width;
>>   	inst->out_height = ev_data->height;
>>   
>> -	if (inst->bit_depth != ev_data->bit_depth)
>> +	if (inst->bit_depth != ev_data->bit_depth) {
>>   		inst->bit_depth = ev_data->bit_depth;
>> +		if (inst->bit_depth == VIDC_BITDEPTH_10)
>> +			inst->fmt_cap = &vdec_formats[3];
>> +		else
>> +			inst->fmt_cap = &vdec_formats[0];
> This doesn't scale and is very error-prone, please enumerate the
> entries and assign it using the enumerator, like:
>
> enum {
> 	VDEC_FORMAT_FOO,
> 	...
> };
>
> ... vdec_formats[] = {
> 	[VDEC_FORMAT_FOO] = { foo, bar, baz }
> }
>
> Konrad

I agree, this can be improved but I would prefer making that change as 
separate patch.

As this is not only related to HDR 10 decoding, there are other places 
in the code which will require similar change.

Thanks,

Dikshita

>> +	}
>>   
>>   	if (inst->pic_struct != ev_data->pic_struct)
>>   		inst->pic_struct = ev_data->pic_struct;