diff mbox series

venus: replace arrary index with enum for supported formats

Message ID 1684736229-30567-1-git-send-email-quic_dikshita@quicinc.com
State New
Headers
Series venus: replace arrary index with enum for supported formats |

Commit Message

Dikshita Agarwal May 22, 2023, 6:17 a.m. UTC 
  Use enums to list supported formats for encoder and decoder
instead of array index which was a error prone design.

Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
---
 drivers/media/platform/qcom/venus/core.h | 16 ++++++++
 drivers/media/platform/qcom/venus/vdec.c | 63 +++++++++++++++++++-------------
 drivers/media/platform/qcom/venus/venc.c | 31 +++++++++-------
 3 files changed, 72 insertions(+), 38 deletions(-)

Comments

Konrad Dybcio May 23, 2023, 8:02 a.m. UTC | #1 
On 22.05.2023 08:17, Dikshita Agarwal wrote:
> Use enums to list supported formats for encoder and decoder
> instead of array index which was a error prone design.
> 
> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> ---
Thanks a lot.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

>  drivers/media/platform/qcom/venus/core.h | 16 ++++++++
>  drivers/media/platform/qcom/venus/vdec.c | 63 +++++++++++++++++++-------------
>  drivers/media/platform/qcom/venus/venc.c | 31 +++++++++-------
>  3 files changed, 72 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index 12a42fb..e988ed4 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -83,6 +83,22 @@ struct venus_resources {
>  	const char *fwname;
>  };
>  
> +enum venus_fmt {
> +	VENUS_FMT_NV12			= 0,
> +	VENUS_FMT_QC08C			= 1,
> +	VENUS_FMT_QC10C			= 2,
> +	VENUS_FMT_H264			= 3,
> +	VENUS_FMT_VP8			= 4,
> +	VENUS_FMT_VP9			= 5,
> +	VENUS_FMT_HEVC			= 6,
> +	VENUS_FMT_VC1_ANNEX_G		= 7,
> +	VENUS_FMT_VC1_ANNEX_L		= 8,
> +	VENUS_FMT_MPEG4			= 9,
> +	VENUS_FMT_MPEG2			= 10,
> +	VENUS_FMT_H263			= 11,
> +	VENUS_FMT_XVID			= 12,
Nit: I don't think the '= n' is necessary here, as it doesn't
map to anything in hw/fw (or does it?)

Konrad

> +};
> +
>  struct venus_format {
>  	u32 pixfmt;
>  	unsigned int num_planes;
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index c6f0fd08..bab985b 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -30,69 +30,82 @@
>   * - future firmware versions could add support for >1 planes
>   */
>  static const struct venus_format vdec_formats[] = {
> -	{
> +	[VENUS_FMT_NV12] = {
>  		.pixfmt = V4L2_PIX_FMT_NV12,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> -	}, {
> +	},
> +	[VENUS_FMT_QC08C] = {
>  		.pixfmt = V4L2_PIX_FMT_QC08C,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> -	}, {
> +	},
> +	[VENUS_FMT_QC10C] = {
>  		.pixfmt = V4L2_PIX_FMT_QC10C,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_MPEG4,
> +	},
> +	[VENUS_FMT_H264] = {
> +		.pixfmt = V4L2_PIX_FMT_H264,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_MPEG2,
> +	},
> +	[VENUS_FMT_VP8] = {
> +		.pixfmt = V4L2_PIX_FMT_VP8,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_H263,
> +	},
> +	[VENUS_FMT_VP9] = {
> +		.pixfmt = V4L2_PIX_FMT_VP9,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
> +	},
> +	[VENUS_FMT_HEVC] = {
> +		.pixfmt = V4L2_PIX_FMT_HEVC,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
> +	},
> +	[VENUS_FMT_VC1_ANNEX_G] = {
> +		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_H264,
> +	},
> +	[VENUS_FMT_VC1_ANNEX_L] = {
> +		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_VP8,
> +	},
> +	[VENUS_FMT_MPEG4] = {
> +		.pixfmt = V4L2_PIX_FMT_MPEG4,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_VP9,
> +	},
> +	[VENUS_FMT_MPEG2] = {
> +		.pixfmt = V4L2_PIX_FMT_MPEG2,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_XVID,
> +	},
> +	[VENUS_FMT_H263] = {
> +		.pixfmt = V4L2_PIX_FMT_H263,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_HEVC,
> +	},
> +	[VENUS_FMT_XVID] = {
> +		.pixfmt = V4L2_PIX_FMT_XVID,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
>  	},
> +
>  };
>  
>  static const struct venus_format *
> @@ -1575,8 +1588,8 @@ static const struct hfi_inst_ops vdec_hfi_ops = {
>  static void vdec_inst_init(struct venus_inst *inst)
>  {
>  	inst->hfi_codec = HFI_VIDEO_CODEC_H264;
> -	inst->fmt_out = &vdec_formats[8];
> -	inst->fmt_cap = &vdec_formats[0];
> +	inst->fmt_out = &vdec_formats[VENUS_FMT_H264];
> +	inst->fmt_cap = &vdec_formats[VENUS_FMT_NV12];
>  	inst->width = frame_width_min(inst);
>  	inst->height = ALIGN(frame_height_min(inst), 32);
>  	inst->crop.left = 0;
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index 4666f42..b60772c 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -32,28 +32,33 @@
>   * - future firmware versions could add support for >1 planes
>   */
>  static const struct venus_format venc_formats[] = {
> -	{
> +	[VENUS_FMT_NV12] = {
>  		.pixfmt = V4L2_PIX_FMT_NV12,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_MPEG4,
> +	},
> +	[VENUS_FMT_H264] = {
> +		.pixfmt = V4L2_PIX_FMT_H264,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_H263,
> +	},
> +	[VENUS_FMT_VP8] = {
> +		.pixfmt = V4L2_PIX_FMT_VP8,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_H264,
> +	},
> +	[VENUS_FMT_HEVC] = {
> +		.pixfmt = V4L2_PIX_FMT_HEVC,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_VP8,
> +	},
> +	[VENUS_FMT_MPEG4] = {
> +		.pixfmt = V4L2_PIX_FMT_MPEG4,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> -	}, {
> -		.pixfmt = V4L2_PIX_FMT_HEVC,
> +	},
> +	[VENUS_FMT_H263] = {
> +		.pixfmt = V4L2_PIX_FMT_H263,
>  		.num_planes = 1,
>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>  	},
> @@ -1416,8 +1421,8 @@ static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
>  
>  static void venc_inst_init(struct venus_inst *inst)
>  {
> -	inst->fmt_cap = &venc_formats[3];
> -	inst->fmt_out = &venc_formats[0];
> +	inst->fmt_cap = &venc_formats[VENUS_FMT_H264];
> +	inst->fmt_out = &venc_formats[VENUS_FMT_NV12];
>  	inst->width = 1280;
>  	inst->height = ALIGN(720, 32);
>  	inst->out_width = 1280;
Dikshita Agarwal May 24, 2023, 4:53 a.m. UTC | #2 
On 5/23/2023 1:32 PM, Konrad Dybcio wrote:
> 
> 
> On 22.05.2023 08:17, Dikshita Agarwal wrote:
>> Use enums to list supported formats for encoder and decoder
>> instead of array index which was a error prone design.
>>
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
>> ---
> Thanks a lot.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> 
>>  drivers/media/platform/qcom/venus/core.h | 16 ++++++++
>>  drivers/media/platform/qcom/venus/vdec.c | 63 +++++++++++++++++++-------------
>>  drivers/media/platform/qcom/venus/venc.c | 31 +++++++++-------
>>  3 files changed, 72 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
>> index 12a42fb..e988ed4 100644
>> --- a/drivers/media/platform/qcom/venus/core.h
>> +++ b/drivers/media/platform/qcom/venus/core.h
>> @@ -83,6 +83,22 @@ struct venus_resources {
>>  	const char *fwname;
>>  };
>>  
>> +enum venus_fmt {
>> +	VENUS_FMT_NV12			= 0,
>> +	VENUS_FMT_QC08C			= 1,
>> +	VENUS_FMT_QC10C			= 2,
>> +	VENUS_FMT_H264			= 3,
>> +	VENUS_FMT_VP8			= 4,
>> +	VENUS_FMT_VP9			= 5,
>> +	VENUS_FMT_HEVC			= 6,
>> +	VENUS_FMT_VC1_ANNEX_G		= 7,
>> +	VENUS_FMT_VC1_ANNEX_L		= 8,
>> +	VENUS_FMT_MPEG4			= 9,
>> +	VENUS_FMT_MPEG2			= 10,
>> +	VENUS_FMT_H263			= 11,
>> +	VENUS_FMT_XVID			= 12,
> Nit: I don't think the '= n' is necessary here, as it doesn't
> map to anything in hw/fw (or does it?)
> 
> Konrad
> 
Yes, It doesn't. Will remove in next patch.

Thanks,
Dikshita

>> +};
>> +
>>  struct venus_format {
>>  	u32 pixfmt;
>>  	unsigned int num_planes;
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>> index c6f0fd08..bab985b 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -30,69 +30,82 @@
>>   * - future firmware versions could add support for >1 planes
>>   */
>>  static const struct venus_format vdec_formats[] = {
>> -	{
>> +	[VENUS_FMT_NV12] = {
>>  		.pixfmt = V4L2_PIX_FMT_NV12,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> -	}, {
>> +	},
>> +	[VENUS_FMT_QC08C] = {
>>  		.pixfmt = V4L2_PIX_FMT_QC08C,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> -	}, {
>> +	},
>> +	[VENUS_FMT_QC10C] = {
>>  		.pixfmt = V4L2_PIX_FMT_QC10C,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_MPEG4,
>> +	},
>> +	[VENUS_FMT_H264] = {
>> +		.pixfmt = V4L2_PIX_FMT_H264,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_MPEG2,
>> +	},
>> +	[VENUS_FMT_VP8] = {
>> +		.pixfmt = V4L2_PIX_FMT_VP8,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_H263,
>> +	},
>> +	[VENUS_FMT_VP9] = {
>> +		.pixfmt = V4L2_PIX_FMT_VP9,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
>> +	},
>> +	[VENUS_FMT_HEVC] = {
>> +		.pixfmt = V4L2_PIX_FMT_HEVC,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
>> +	},
>> +	[VENUS_FMT_VC1_ANNEX_G] = {
>> +		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_H264,
>> +	},
>> +	[VENUS_FMT_VC1_ANNEX_L] = {
>> +		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_VP8,
>> +	},
>> +	[VENUS_FMT_MPEG4] = {
>> +		.pixfmt = V4L2_PIX_FMT_MPEG4,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_VP9,
>> +	},
>> +	[VENUS_FMT_MPEG2] = {
>> +		.pixfmt = V4L2_PIX_FMT_MPEG2,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_XVID,
>> +	},
>> +	[VENUS_FMT_H263] = {
>> +		.pixfmt = V4L2_PIX_FMT_H263,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_HEVC,
>> +	},
>> +	[VENUS_FMT_XVID] = {
>> +		.pixfmt = V4L2_PIX_FMT_XVID,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>>  		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
>>  	},
>> +
>>  };
>>  
>>  static const struct venus_format *
>> @@ -1575,8 +1588,8 @@ static const struct hfi_inst_ops vdec_hfi_ops = {
>>  static void vdec_inst_init(struct venus_inst *inst)
>>  {
>>  	inst->hfi_codec = HFI_VIDEO_CODEC_H264;
>> -	inst->fmt_out = &vdec_formats[8];
>> -	inst->fmt_cap = &vdec_formats[0];
>> +	inst->fmt_out = &vdec_formats[VENUS_FMT_H264];
>> +	inst->fmt_cap = &vdec_formats[VENUS_FMT_NV12];
>>  	inst->width = frame_width_min(inst);
>>  	inst->height = ALIGN(frame_height_min(inst), 32);
>>  	inst->crop.left = 0;
>> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
>> index 4666f42..b60772c 100644
>> --- a/drivers/media/platform/qcom/venus/venc.c
>> +++ b/drivers/media/platform/qcom/venus/venc.c
>> @@ -32,28 +32,33 @@
>>   * - future firmware versions could add support for >1 planes
>>   */
>>  static const struct venus_format venc_formats[] = {
>> -	{
>> +	[VENUS_FMT_NV12] = {
>>  		.pixfmt = V4L2_PIX_FMT_NV12,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_MPEG4,
>> +	},
>> +	[VENUS_FMT_H264] = {
>> +		.pixfmt = V4L2_PIX_FMT_H264,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_H263,
>> +	},
>> +	[VENUS_FMT_VP8] = {
>> +		.pixfmt = V4L2_PIX_FMT_VP8,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_H264,
>> +	},
>> +	[VENUS_FMT_HEVC] = {
>> +		.pixfmt = V4L2_PIX_FMT_HEVC,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_VP8,
>> +	},
>> +	[VENUS_FMT_MPEG4] = {
>> +		.pixfmt = V4L2_PIX_FMT_MPEG4,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>> -	}, {
>> -		.pixfmt = V4L2_PIX_FMT_HEVC,
>> +	},
>> +	[VENUS_FMT_H263] = {
>> +		.pixfmt = V4L2_PIX_FMT_H263,
>>  		.num_planes = 1,
>>  		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
>>  	},
>> @@ -1416,8 +1421,8 @@ static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
>>  
>>  static void venc_inst_init(struct venus_inst *inst)
>>  {
>> -	inst->fmt_cap = &venc_formats[3];
>> -	inst->fmt_out = &venc_formats[0];
>> +	inst->fmt_cap = &venc_formats[VENUS_FMT_H264];
>> +	inst->fmt_out = &venc_formats[VENUS_FMT_NV12];
>>  	inst->width = 1280;
>>  	inst->height = ALIGN(720, 32);
>>  	inst->out_width = 1280;
Stanimir Varbanov May 25, 2023, 10:12 p.m. UTC | #3 
Hi Konrad,

On 23.05.23 г. 11:02 ч., Konrad Dybcio wrote:
> 
> 
> On 22.05.2023 08:17, Dikshita Agarwal wrote:
>> Use enums to list supported formats for encoder and decoder
>> instead of array index which was a error prone design.
>>
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
>> ---
> Thanks a lot.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> 
>>   drivers/media/platform/qcom/venus/core.h | 16 ++++++++
>>   drivers/media/platform/qcom/venus/vdec.c | 63 +++++++++++++++++++-------------
>>   drivers/media/platform/qcom/venus/venc.c | 31 +++++++++-------
>>   3 files changed, 72 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
>> index 12a42fb..e988ed4 100644
>> --- a/drivers/media/platform/qcom/venus/core.h
>> +++ b/drivers/media/platform/qcom/venus/core.h
>> @@ -83,6 +83,22 @@ struct venus_resources {
>>   	const char *fwname;
>>   };
>>   
>> +enum venus_fmt {
>> +	VENUS_FMT_NV12			= 0,
>> +	VENUS_FMT_QC08C			= 1,
>> +	VENUS_FMT_QC10C			= 2,
>> +	VENUS_FMT_H264			= 3,
>> +	VENUS_FMT_VP8			= 4,
>> +	VENUS_FMT_VP9			= 5,
>> +	VENUS_FMT_HEVC			= 6,
>> +	VENUS_FMT_VC1_ANNEX_G		= 7,
>> +	VENUS_FMT_VC1_ANNEX_L		= 8,
>> +	VENUS_FMT_MPEG4			= 9,
>> +	VENUS_FMT_MPEG2			= 10,
>> +	VENUS_FMT_H263			= 11,
>> +	VENUS_FMT_XVID			= 12,
> Nit: I don't think the '= n' is necessary here, as it doesn't
> map to anything in hw/fw (or does it?)

IMO, the numbers could help when debugging to avoid counting.

> 
> Konrad
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 12a42fb..e988ed4 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -83,6 +83,22 @@  struct venus_resources {
 	const char *fwname;
 };
 
+enum venus_fmt {
+	VENUS_FMT_NV12			= 0,
+	VENUS_FMT_QC08C			= 1,
+	VENUS_FMT_QC10C			= 2,
+	VENUS_FMT_H264			= 3,
+	VENUS_FMT_VP8			= 4,
+	VENUS_FMT_VP9			= 5,
+	VENUS_FMT_HEVC			= 6,
+	VENUS_FMT_VC1_ANNEX_G		= 7,
+	VENUS_FMT_VC1_ANNEX_L		= 8,
+	VENUS_FMT_MPEG4			= 9,
+	VENUS_FMT_MPEG2			= 10,
+	VENUS_FMT_H263			= 11,
+	VENUS_FMT_XVID			= 12,
+};
+
 struct venus_format {
 	u32 pixfmt;
 	unsigned int num_planes;
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index c6f0fd08..bab985b 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -30,69 +30,82 @@ 
  * - future firmware versions could add support for >1 planes
  */
 static const struct venus_format vdec_formats[] = {
-	{
+	[VENUS_FMT_NV12] = {
 		.pixfmt = V4L2_PIX_FMT_NV12,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
-	}, {
+	},
+	[VENUS_FMT_QC08C] = {
 		.pixfmt = V4L2_PIX_FMT_QC08C,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
-	}, {
+	},
+	[VENUS_FMT_QC10C] = {
 		.pixfmt = V4L2_PIX_FMT_QC10C,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_MPEG4,
+	},
+	[VENUS_FMT_H264] = {
+		.pixfmt = V4L2_PIX_FMT_H264,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
 		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_MPEG2,
+	},
+	[VENUS_FMT_VP8] = {
+		.pixfmt = V4L2_PIX_FMT_VP8,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
 		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_H263,
+	},
+	[VENUS_FMT_VP9] = {
+		.pixfmt = V4L2_PIX_FMT_VP9,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
 		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
+	},
+	[VENUS_FMT_HEVC] = {
+		.pixfmt = V4L2_PIX_FMT_HEVC,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
 		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
+	},
+	[VENUS_FMT_VC1_ANNEX_G] = {
+		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
 		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_H264,
+	},
+	[VENUS_FMT_VC1_ANNEX_L] = {
+		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
 		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_VP8,
+	},
+	[VENUS_FMT_MPEG4] = {
+		.pixfmt = V4L2_PIX_FMT_MPEG4,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
 		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_VP9,
+	},
+	[VENUS_FMT_MPEG2] = {
+		.pixfmt = V4L2_PIX_FMT_MPEG2,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
 		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_XVID,
+	},
+	[VENUS_FMT_H263] = {
+		.pixfmt = V4L2_PIX_FMT_H263,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
 		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_HEVC,
+	},
+	[VENUS_FMT_XVID] = {
+		.pixfmt = V4L2_PIX_FMT_XVID,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
 		.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
 	},
+
 };
 
 static const struct venus_format *
@@ -1575,8 +1588,8 @@  static const struct hfi_inst_ops vdec_hfi_ops = {
 static void vdec_inst_init(struct venus_inst *inst)
 {
 	inst->hfi_codec = HFI_VIDEO_CODEC_H264;
-	inst->fmt_out = &vdec_formats[8];
-	inst->fmt_cap = &vdec_formats[0];
+	inst->fmt_out = &vdec_formats[VENUS_FMT_H264];
+	inst->fmt_cap = &vdec_formats[VENUS_FMT_NV12];
 	inst->width = frame_width_min(inst);
 	inst->height = ALIGN(frame_height_min(inst), 32);
 	inst->crop.left = 0;
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 4666f42..b60772c 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -32,28 +32,33 @@ 
  * - future firmware versions could add support for >1 planes
  */
 static const struct venus_format venc_formats[] = {
-	{
+	[VENUS_FMT_NV12] = {
 		.pixfmt = V4L2_PIX_FMT_NV12,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_MPEG4,
+	},
+	[VENUS_FMT_H264] = {
+		.pixfmt = V4L2_PIX_FMT_H264,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_H263,
+	},
+	[VENUS_FMT_VP8] = {
+		.pixfmt = V4L2_PIX_FMT_VP8,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_H264,
+	},
+	[VENUS_FMT_HEVC] = {
+		.pixfmt = V4L2_PIX_FMT_HEVC,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_VP8,
+	},
+	[VENUS_FMT_MPEG4] = {
+		.pixfmt = V4L2_PIX_FMT_MPEG4,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
-	}, {
-		.pixfmt = V4L2_PIX_FMT_HEVC,
+	},
+	[VENUS_FMT_H263] = {
+		.pixfmt = V4L2_PIX_FMT_H263,
 		.num_planes = 1,
 		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
 	},
@@ -1416,8 +1421,8 @@  static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
 
 static void venc_inst_init(struct venus_inst *inst)
 {
-	inst->fmt_cap = &venc_formats[3];
-	inst->fmt_out = &venc_formats[0];
+	inst->fmt_cap = &venc_formats[VENUS_FMT_H264];
+	inst->fmt_out = &venc_formats[VENUS_FMT_NV12];
 	inst->width = 1280;
 	inst->height = ALIGN(720, 32);
 	inst->out_width = 1280;