Re: [PATCH v3 07/16] media: mtk-vcodec: venc: specify supported formats per-chip
On Sun, Jul 26, 2020 at 11:29 PM Ezequiel Garcia
wrote:
>
> Hi Alexandre,
>
> Last review on my side, this series looks mostly good.
Thanks for taking the time to look at it! I know it's not the best
looking code out there. ^_^;
>
> On Mon, 13 Jul 2020 at 03:09, Alexandre Courbot wrote:
> >
> > Different chips have different supported bitrate ranges. Move the list
>
> s/bitrate ranges/formats
Ack.
>
> > of supported formats to the platform data, and split the output and
> > capture formats into two lists to make it easier to find the default
> > format for each queue.
> >
> > Signed-off-by: Alexandre Courbot
> > Acked-by: Tiffany Lin
> > ---
> > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 8 ++
> > .../platform/mtk-vcodec/mtk_vcodec_enc.c | 122 +++---
> > .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 40 ++
> > 3 files changed, 95 insertions(+), 75 deletions(-)
> >
> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> > b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> > index b8f913de8d80..59b4b750666b 100644
> > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> > @@ -313,6 +313,10 @@ enum mtk_chip {
> > * @has_lt_irq: whether the encoder uses the LT irq
> > * @min_birate: minimum supported encoding bitrate
> > * @max_bitrate: maximum supported encoding bitrate
> > + * @capture_formats: array of supported capture formats
> > + * @num_capture_formats: number of entries in capture_formats
> > + * @output_formats: array of supported output formats
> > + * @num_output_formats: number of entries in output_formats
> > */
> > struct mtk_vcodec_enc_pdata {
> > enum mtk_chip chip;
> > @@ -321,6 +325,10 @@ struct mtk_vcodec_enc_pdata {
> > bool has_lt_irq;
> > unsigned long min_bitrate;
> > unsigned long max_bitrate;
> > + const struct mtk_video_fmt *capture_formats;
> > + size_t num_capture_formats;
> > + const struct mtk_video_fmt *output_formats;
> > + size_t num_output_formats;
> > };
> >
> > /**
> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> > b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> > index 50ba9da59153..05743a745a11 100644
> > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> > @@ -23,47 +23,9 @@
> > #define DFT_CFG_WIDTH MTK_VENC_MIN_W
> > #define DFT_CFG_HEIGHT MTK_VENC_MIN_H
> > #define MTK_MAX_CTRLS_HINT 20
> > -#define OUT_FMT_IDX0
> > -#define CAP_FMT_IDX4
> > -
> >
> > static void mtk_venc_worker(struct work_struct *work);
> >
> > -static const struct mtk_video_fmt mtk_video_formats[] = {
> > - {
> > - .fourcc = V4L2_PIX_FMT_NV12M,
> > - .type = MTK_FMT_FRAME,
> > - .num_planes = 2,
>
> Again, not an issue with your patch, so feel free to ignore this.
>
> You may avoid keeping track of num_planes (or any other
> property of the pixel format) and use v4l2_fill_pixfmt_mp
> to get all the information you need.
That's a good idea. Let's see if I can come with an extra patch in a
way that does not prevent the already acked patches from being merged.
:)
Re: [PATCH v3 07/16] media: mtk-vcodec: venc: specify supported formats per-chip
Hi Alexandre,
Last review on my side, this series looks mostly good.
On Mon, 13 Jul 2020 at 03:09, Alexandre Courbot wrote:
>
> Different chips have different supported bitrate ranges. Move the list
s/bitrate ranges/formats
> of supported formats to the platform data, and split the output and
> capture formats into two lists to make it easier to find the default
> format for each queue.
>
> Signed-off-by: Alexandre Courbot
> Acked-by: Tiffany Lin
> ---
> .../platform/mtk-vcodec/mtk_vcodec_drv.h | 8 ++
> .../platform/mtk-vcodec/mtk_vcodec_enc.c | 122 +++---
> .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 40 ++
> 3 files changed, 95 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> index b8f913de8d80..59b4b750666b 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> @@ -313,6 +313,10 @@ enum mtk_chip {
> * @has_lt_irq: whether the encoder uses the LT irq
> * @min_birate: minimum supported encoding bitrate
> * @max_bitrate: maximum supported encoding bitrate
> + * @capture_formats: array of supported capture formats
> + * @num_capture_formats: number of entries in capture_formats
> + * @output_formats: array of supported output formats
> + * @num_output_formats: number of entries in output_formats
> */
> struct mtk_vcodec_enc_pdata {
> enum mtk_chip chip;
> @@ -321,6 +325,10 @@ struct mtk_vcodec_enc_pdata {
> bool has_lt_irq;
> unsigned long min_bitrate;
> unsigned long max_bitrate;
> + const struct mtk_video_fmt *capture_formats;
> + size_t num_capture_formats;
> + const struct mtk_video_fmt *output_formats;
> + size_t num_output_formats;
> };
>
> /**
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> index 50ba9da59153..05743a745a11 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> @@ -23,47 +23,9 @@
> #define DFT_CFG_WIDTH MTK_VENC_MIN_W
> #define DFT_CFG_HEIGHT MTK_VENC_MIN_H
> #define MTK_MAX_CTRLS_HINT 20
> -#define OUT_FMT_IDX0
> -#define CAP_FMT_IDX4
> -
>
> static void mtk_venc_worker(struct work_struct *work);
>
> -static const struct mtk_video_fmt mtk_video_formats[] = {
> - {
> - .fourcc = V4L2_PIX_FMT_NV12M,
> - .type = MTK_FMT_FRAME,
> - .num_planes = 2,
Again, not an issue with your patch, so feel free to ignore this.
You may avoid keeping track of num_planes (or any other
property of the pixel format) and use v4l2_fill_pixfmt_mp
to get all the information you need.
Thanks,
Ezequiel
[PATCH v3 07/16] media: mtk-vcodec: venc: specify supported formats per-chip
Different chips have different supported bitrate ranges. Move the list
of supported formats to the platform data, and split the output and
capture formats into two lists to make it easier to find the default
format for each queue.
Signed-off-by: Alexandre Courbot
Acked-by: Tiffany Lin
---
.../platform/mtk-vcodec/mtk_vcodec_drv.h | 8 ++
.../platform/mtk-vcodec/mtk_vcodec_enc.c | 122 +++---
.../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 40 ++
3 files changed, 95 insertions(+), 75 deletions(-)
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
index b8f913de8d80..59b4b750666b 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
@@ -313,6 +313,10 @@ enum mtk_chip {
* @has_lt_irq: whether the encoder uses the LT irq
* @min_birate: minimum supported encoding bitrate
* @max_bitrate: maximum supported encoding bitrate
+ * @capture_formats: array of supported capture formats
+ * @num_capture_formats: number of entries in capture_formats
+ * @output_formats: array of supported output formats
+ * @num_output_formats: number of entries in output_formats
*/
struct mtk_vcodec_enc_pdata {
enum mtk_chip chip;
@@ -321,6 +325,10 @@ struct mtk_vcodec_enc_pdata {
bool has_lt_irq;
unsigned long min_bitrate;
unsigned long max_bitrate;
+ const struct mtk_video_fmt *capture_formats;
+ size_t num_capture_formats;
+ const struct mtk_video_fmt *output_formats;
+ size_t num_output_formats;
};
/**
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
index 50ba9da59153..05743a745a11 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
@@ -23,47 +23,9 @@
#define DFT_CFG_WIDTH MTK_VENC_MIN_W
#define DFT_CFG_HEIGHT MTK_VENC_MIN_H
#define MTK_MAX_CTRLS_HINT 20
-#define OUT_FMT_IDX0
-#define CAP_FMT_IDX4
-
static void mtk_venc_worker(struct work_struct *work);
-static const struct mtk_video_fmt mtk_video_formats[] = {
- {
- .fourcc = V4L2_PIX_FMT_NV12M,
- .type = MTK_FMT_FRAME,
- .num_planes = 2,
- },
- {
- .fourcc = V4L2_PIX_FMT_NV21M,
- .type = MTK_FMT_FRAME,
- .num_planes = 2,
- },
- {
- .fourcc = V4L2_PIX_FMT_YUV420M,
- .type = MTK_FMT_FRAME,
- .num_planes = 3,
- },
- {
- .fourcc = V4L2_PIX_FMT_YVU420M,
- .type = MTK_FMT_FRAME,
- .num_planes = 3,
- },
- {
- .fourcc = V4L2_PIX_FMT_H264,
- .type = MTK_FMT_ENC,
- .num_planes = 1,
- },
- {
- .fourcc = V4L2_PIX_FMT_VP8,
- .type = MTK_FMT_ENC,
- .num_planes = 1,
- },
-};
-
-#define NUM_FORMATS ARRAY_SIZE(mtk_video_formats)
-
static const struct mtk_codec_framesizes mtk_venc_framesizes[] = {
{
.fourcc = V4L2_PIX_FMT_H264,
@@ -156,27 +118,17 @@ static const struct v4l2_ctrl_ops mtk_vcodec_enc_ctrl_ops
= {
.s_ctrl = vidioc_venc_s_ctrl,
};
-static int vidioc_enum_fmt(struct v4l2_fmtdesc *f, bool output_queue)
+static int vidioc_enum_fmt(struct v4l2_fmtdesc *f,
+ const struct mtk_video_fmt *formats,
+ size_t num_formats)
{
- const struct mtk_video_fmt *fmt;
- int i, j = 0;
+ if (f->index >= num_formats)
+ return -EINVAL;
- for (i = 0; i < NUM_FORMATS; ++i) {
- if (output_queue && mtk_video_formats[i].type != MTK_FMT_FRAME)
- continue;
- if (!output_queue && mtk_video_formats[i].type != MTK_FMT_ENC)
- continue;
+ f->pixelformat = formats[f->index].fourcc;
+ memset(f->reserved, 0, sizeof(f->reserved));
- if (j == f->index) {
- fmt = _video_formats[i];
- f->pixelformat = fmt->fourcc;
- memset(f->reserved, 0, sizeof(f->reserved));
- return 0;
- }
- ++j;
- }
-
- return -EINVAL;
+ return 0;
}
static int vidioc_enum_framesizes(struct file *file, void *fh,
@@ -202,13 +154,21 @@ static int vidioc_enum_framesizes(struct file *file, void
*fh,
static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
struct v4l2_fmtdesc *f)
{
- return vidioc_enum_fmt(f, false);
+ const struct mtk_vcodec_enc_pdata *pdata =
+ fh_to_ctx(priv)->dev->venc_pdata;
+
+ return vidioc_enum_fmt(f, pdata->capture_formats,
+
