* [PATCH] venus: replace arrary index with enum for supported formats
@ 2023-05-22 6:17 Dikshita Agarwal
2023-05-23 8:02 ` Konrad Dybcio
0 siblings, 1 reply; 4+ messages in thread
From: Dikshita Agarwal @ 2023-05-22 6:17 UTC (permalink / raw)
To: linux-media, stanimir.k.varbanov, quic_vgarodia, agross,
andersson, konrad.dybcio, mchehab
Cc: linux-kernel, linux-arm-msm, Dikshita Agarwal
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(-)
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;
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] venus: replace arrary index with enum for supported formats
2023-05-22 6:17 [PATCH] venus: replace arrary index with enum for supported formats Dikshita Agarwal
@ 2023-05-23 8:02 ` Konrad Dybcio
2023-05-24 4:53 ` Dikshita Agarwal
2023-05-25 22:12 ` Stanimir Varbanov
0 siblings, 2 replies; 4+ messages in thread
From: Konrad Dybcio @ 2023-05-23 8:02 UTC (permalink / raw)
To: Dikshita Agarwal, linux-media, stanimir.k.varbanov,
quic_vgarodia, agross, andersson, mchehab
Cc: linux-kernel, linux-arm-msm
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;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] venus: replace arrary index with enum for supported formats
2023-05-23 8:02 ` Konrad Dybcio
@ 2023-05-24 4:53 ` Dikshita Agarwal
2023-05-25 22:12 ` Stanimir Varbanov
1 sibling, 0 replies; 4+ messages in thread
From: Dikshita Agarwal @ 2023-05-24 4:53 UTC (permalink / raw)
To: Konrad Dybcio, linux-media, stanimir.k.varbanov, quic_vgarodia,
agross, andersson, mchehab
Cc: linux-kernel, linux-arm-msm
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;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] venus: replace arrary index with enum for supported formats
2023-05-23 8:02 ` Konrad Dybcio
2023-05-24 4:53 ` Dikshita Agarwal
@ 2023-05-25 22:12 ` Stanimir Varbanov
1 sibling, 0 replies; 4+ messages in thread
From: Stanimir Varbanov @ 2023-05-25 22:12 UTC (permalink / raw)
To: Konrad Dybcio, Dikshita Agarwal, linux-media, quic_vgarodia,
agross, andersson, mchehab
Cc: linux-kernel, linux-arm-msm
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
--
regards,
Stan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-25 22:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-22 6:17 [PATCH] venus: replace arrary index with enum for supported formats Dikshita Agarwal
2023-05-23 8:02 ` Konrad Dybcio
2023-05-24 4:53 ` Dikshita Agarwal
2023-05-25 22:12 ` Stanimir Varbanov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.