2022-07-06 08:24:40

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v2 0/6] media: mediatek: vcodec: Fix 4K decoding support

This is v2 of the mtk-vcodec 4K decoder fixes.

While testing a backport of recent mtk-vcodec developments on ChromeOS
v5.10 kernel [1], it was found that 4K decoding support had regressed.
The decoder was not correctly reporting 4K frame sizes when queried,
and ChromeOS then determined that the hardware did not support it.

This turned out to be a mix of different bugs:

1. Frame size enumeration on the output side should not depend on the
currently set format, or any other derived state. This is fixed in
patch 2.

2. TRY_FMT on the output side was incorrectly clamping the resolution
based on the current maximum values. It should not. Fixed in patch
4.

3. The default resolution limit was not set according to the default
output format determined at runtime, but hard-coded to 1080p. An
S_FMT call is needed to override this. The instance resolution limit
is rendered useless after patches 3 and 4, and dropped in patch 5.

Other patches:
- Patch 1 makes stepwise_fhd constant.
- Patch 3 drops redundant aligning of default resolution
- Patch 6 moves framesize inside mtk_video_fmt, making it easier to
access and removes a list search that was added in patch 4.

Changes since v1:
- Added patch to const-ify stepwise_fhd, as Nicolas requested.
- Dropped old patch 2 (media: mediatek: vcodec: dec: Set default
max resolution based on format)
- Dropped old patch 4 (media: mediatek: vcodec: dec: Set maximum
resolution when S_FMT on output only)
- Made max resolution lookup for TRY_FMT return stepwise structure in
patch 4, which helps with the last patch that moves framesize
stepwise into mtk_video_fmt.
- Did some style cleanups in patch 4

This series is based on next-20220705.

This was only tested on the backport kernel [1] on MT8195, which is the
only currently supported SoC that does 4K decoding. Hopefully the folks
at Collabora can give this a test on their mainline MT8195 integration
branch.


Regards
ChenYu

[1] https://crrev.com/c/3713491

Chen-Yu Tsai (6):
media: mediatek: vcodec: decoder: Const-ify stepwise_fhd
media: mediatek: vcodec: decoder: Fix 4K frame size enumeration
media: mediatek: vcodec: decoder: Skip alignment for default
resolution
media: mediatek: vcodec: decoder: Fix resolution clamping in TRY_FMT
media: mediatek: vcodec: decoder: Drop max_{width,height} from
mtk_vcodec_ctx
media: mediatek: vcodec: decoder: Embed framesize inside mtk_video_fmt

.../platform/mediatek/vcodec/mtk_vcodec_dec.c | 54 ++++++++-----------
.../mediatek/vcodec/mtk_vcodec_dec_stateful.c | 29 +++-------
.../vcodec/mtk_vcodec_dec_stateless.c | 30 +++++------
.../platform/mediatek/vcodec/mtk_vcodec_drv.h | 20 +------
4 files changed, 41 insertions(+), 92 deletions(-)

--
2.37.0.rc0.161.g10f37bed90-goog


2022-07-06 08:24:46

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v2 1/6] media: mediatek: vcodec: decoder: Const-ify stepwise_fhd

stepwise_fhd is the reference framesize variable, and should not be
altered. Make it constant.

Fixes: ("76250b48de79 media: mediatek: vcodec: Getting supported decoder format types")
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
.../media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
index 16d55785d84b..f1c0276c9026 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
@@ -119,7 +119,7 @@ static struct mtk_video_fmt default_cap_format;
static unsigned int num_formats;
static unsigned int num_framesizes;

-static struct v4l2_frmsize_stepwise stepwise_fhd = {
+static const struct v4l2_frmsize_stepwise stepwise_fhd = {
.min_width = MTK_VDEC_MIN_W,
.max_width = MTK_VDEC_MAX_W,
.step_width = 16,
--
2.37.0.rc0.161.g10f37bed90-goog

2022-07-06 08:24:57

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v2 2/6] media: mediatek: vcodec: decoder: Fix 4K frame size enumeration

This partially reverts commit b018be06f3c7 ("media: mediatek: vcodec:
Read max resolution from dec_capability"). In this commit, the maximum
resolution ended up being a function of both the firmware capability and
the current set format.

However, frame size enumeration for output (coded) formats should not
depend on the format set, but should return supported resolutions for
the format requested by userspace.

Fix this so that the driver returns the supported resolutions correctly,
even if the instance only has default settings, or if the output format
is currently set to VP8F, which does not support 4K.

This adds an copy of special casing for !VP8 and 4K support. The other
existing copy will be removed when .max_{width,height} are removed from
|struct mtk_vcodec_ctx| in a subsequent patch.

Fixes: b018be06f3c7 ("media: mediatek: vcodec: Read max resolution from dec_capability")
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 2 --
.../platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c | 7 +++++++
2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
index 5d6fdf18c3a6..fcb4b8131c49 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -595,8 +595,6 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
fsize->stepwise = dec_pdata->vdec_framesizes[i].stepwise;

- fsize->stepwise.max_width = ctx->max_width;
- fsize->stepwise.max_height = ctx->max_height;
mtk_v4l2_debug(1, "%x, %d %d %d %d %d %d",
ctx->dev->dec_capability,
fsize->stepwise.min_width,
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
index f1c0276c9026..5da9901e93a3 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
@@ -360,6 +360,13 @@ static void mtk_vcodec_add_formats(unsigned int fourcc,

mtk_vdec_framesizes[count_framesizes].fourcc = fourcc;
mtk_vdec_framesizes[count_framesizes].stepwise = stepwise_fhd;
+ if (!(ctx->dev->dec_capability & VCODEC_CAPABILITY_4K_DISABLED) &&
+ fourcc != V4L2_PIX_FMT_VP8_FRAME) {
+ mtk_vdec_framesizes[count_framesizes].stepwise.max_width =
+ VCODEC_DEC_4K_CODED_WIDTH;
+ mtk_vdec_framesizes[count_framesizes].stepwise.max_height =
+ VCODEC_DEC_4K_CODED_HEIGHT;
+ }
num_framesizes++;
break;
case V4L2_PIX_FMT_MM21:
--
2.37.0.rc0.161.g10f37bed90-goog

2022-07-06 08:25:25

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v2 6/6] media: mediatek: vcodec: decoder: Embed framesize inside mtk_video_fmt

Right now the decoder maintains two separate lists for supported pixel
formats and frame sizes. Getting the supported frame sizes for the
current set format is a bit convoluted, requiring a search through
the separate frame size list. The frame sizes are used to clamp and
align requested resolutions.

Instead, the frame size structure could be embedded inside the pixel
format structure. Getting one also gets the other. And since the
the driver already keeps pointers to the current set format, getting
the frame sizes becomes straightforward.

Do just that. Move v4l2_frmsize_stepwise inside mtk_video_fmt, and get
rid of mtk_codec_framesizes.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
.../platform/mediatek/vcodec/mtk_vcodec_dec.c | 36 +++++--------------
.../mediatek/vcodec/mtk_vcodec_dec_stateful.c | 29 ++++-----------
.../vcodec/mtk_vcodec_dec_stateless.c | 23 ++++--------
.../platform/mediatek/vcodec/mtk_vcodec_drv.h | 16 +--------
4 files changed, 22 insertions(+), 82 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
index 2d370e8041c1..bdcddeaa997c 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -261,42 +261,20 @@ static int vidioc_vdec_subscribe_evt(struct v4l2_fh *fh,
}
}

-static const struct v4l2_frmsize_stepwise *mtk_vdec_get_frmsize(struct mtk_vcodec_ctx *ctx,
- u32 pixfmt)
-{
- const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev->vdec_pdata;
- int i;
-
- for (i = 0; i < *dec_pdata->num_framesizes; ++i)
- if (pixfmt == dec_pdata->vdec_framesizes[i].fourcc)
- return &dec_pdata->vdec_framesizes[i].stepwise;
-
- /*
- * This should never happen since vidioc_try_fmt_vid_out_mplane()
- * always passes through a valid format for the output side, and
- * for the capture side, a valid output format should already have
- * been set.
- */
- WARN_ONCE(1, "Unsupported format requested.\n");
- return &dec_pdata->vdec_framesizes[0].stepwise;
-}
-
static int vidioc_try_fmt(struct mtk_vcodec_ctx *ctx, struct v4l2_format *f,
const struct mtk_video_fmt *fmt)
{
struct v4l2_pix_format_mplane *pix_fmt_mp = &f->fmt.pix_mp;
const struct v4l2_frmsize_stepwise *frmsize;
- u32 fourcc;

pix_fmt_mp->field = V4L2_FIELD_NONE;

/* Always apply frame size constraints from the coded side */
if (V4L2_TYPE_IS_OUTPUT(f->type))
- fourcc = f->fmt.pix_mp.pixelformat;
+ frmsize = &fmt->frmsize;
else
- fourcc = ctx->q_data[MTK_Q_DATA_SRC].fmt->fourcc;
+ frmsize = &ctx->q_data[MTK_Q_DATA_SRC].fmt->frmsize;

- frmsize = mtk_vdec_get_frmsize(ctx, fourcc);
pix_fmt_mp->width = clamp(pix_fmt_mp->width, MTK_VDEC_MIN_W, frmsize->max_width);
pix_fmt_mp->height = clamp(pix_fmt_mp->height, MTK_VDEC_MIN_H, frmsize->max_height);

@@ -596,12 +574,16 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
if (fsize->index != 0)
return -EINVAL;

- for (i = 0; i < *dec_pdata->num_framesizes; ++i) {
- if (fsize->pixel_format != dec_pdata->vdec_framesizes[i].fourcc)
+ for (i = 0; i < *dec_pdata->num_formats; i++) {
+ if (fsize->pixel_format != dec_pdata->vdec_formats[i].fourcc)
continue;

+ /* Only coded formats have frame sizes set */
+ if (!dec_pdata->vdec_formats[i].frmsize.max_width)
+ return -ENOTTY;
+
fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
- fsize->stepwise = dec_pdata->vdec_framesizes[i].stepwise;
+ fsize->stepwise = dec_pdata->vdec_formats[i].frmsize;

mtk_v4l2_debug(1, "%x, %d %d %d %d %d %d",
ctx->dev->dec_capability,
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateful.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateful.c
index 9c7e6145cebb..035c86e7809f 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateful.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateful.c
@@ -17,18 +17,24 @@ static const struct mtk_video_fmt mtk_video_formats[] = {
.type = MTK_FMT_DEC,
.num_planes = 1,
.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
+ .frmsize = { MTK_VDEC_MIN_W, MTK_VDEC_MAX_W, 16,
+ MTK_VDEC_MIN_H, MTK_VDEC_MAX_H, 16 },
},
{
.fourcc = V4L2_PIX_FMT_VP8,
.type = MTK_FMT_DEC,
.num_planes = 1,
.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
+ .frmsize = { MTK_VDEC_MIN_W, MTK_VDEC_MAX_W, 16,
+ MTK_VDEC_MIN_H, MTK_VDEC_MAX_H, 16 },
},
{
.fourcc = V4L2_PIX_FMT_VP9,
.type = MTK_FMT_DEC,
.num_planes = 1,
.flags = V4L2_FMT_FLAG_DYN_RESOLUTION,
+ .frmsize = { MTK_VDEC_MIN_W, MTK_VDEC_MAX_W, 16,
+ MTK_VDEC_MIN_H, MTK_VDEC_MAX_H, 16 },
},
{
.fourcc = V4L2_PIX_FMT_MT21C,
@@ -43,27 +49,6 @@ static const unsigned int num_supported_formats =
#define DEFAULT_OUT_FMT_IDX 0
#define DEFAULT_CAP_FMT_IDX 3

-static const struct mtk_codec_framesizes mtk_vdec_framesizes[] = {
- {
- .fourcc = V4L2_PIX_FMT_H264,
- .stepwise = { MTK_VDEC_MIN_W, MTK_VDEC_MAX_W, 16,
- MTK_VDEC_MIN_H, MTK_VDEC_MAX_H, 16 },
- },
- {
- .fourcc = V4L2_PIX_FMT_VP8,
- .stepwise = { MTK_VDEC_MIN_W, MTK_VDEC_MAX_W, 16,
- MTK_VDEC_MIN_H, MTK_VDEC_MAX_H, 16 },
- },
- {
- .fourcc = V4L2_PIX_FMT_VP9,
- .stepwise = { MTK_VDEC_MIN_W, MTK_VDEC_MAX_W, 16,
- MTK_VDEC_MIN_H, MTK_VDEC_MAX_H, 16 },
- },
-};
-
-static const unsigned int num_supported_framesize =
- ARRAY_SIZE(mtk_vdec_framesizes);
-
/*
* This function tries to clean all display buffers, the buffers will return
* in display order.
@@ -618,8 +603,6 @@ const struct mtk_vcodec_dec_pdata mtk_vdec_8173_pdata = {
.num_formats = &num_supported_formats,
.default_out_fmt = &mtk_video_formats[DEFAULT_OUT_FMT_IDX],
.default_cap_fmt = &mtk_video_formats[DEFAULT_CAP_FMT_IDX],
- .vdec_framesizes = mtk_vdec_framesizes,
- .num_framesizes = &num_supported_framesize,
.worker = mtk_vdec_worker,
.flush_decoder = mtk_vdec_flush_decoder,
.is_subdev_supported = false,
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
index 5da9901e93a3..c45bd2599bb2 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateless.c
@@ -112,12 +112,10 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {
#define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)

static struct mtk_video_fmt mtk_video_formats[5];
-static struct mtk_codec_framesizes mtk_vdec_framesizes[3];

static struct mtk_video_fmt default_out_format;
static struct mtk_video_fmt default_cap_format;
static unsigned int num_formats;
-static unsigned int num_framesizes;

static const struct v4l2_frmsize_stepwise stepwise_fhd = {
.min_width = MTK_VDEC_MIN_W,
@@ -348,7 +346,6 @@ static void mtk_vcodec_add_formats(unsigned int fourcc,
struct mtk_vcodec_dev *dev = ctx->dev;
const struct mtk_vcodec_dec_pdata *pdata = dev->vdec_pdata;
int count_formats = *pdata->num_formats;
- int count_framesizes = *pdata->num_framesizes;

switch (fourcc) {
case V4L2_PIX_FMT_H264_SLICE:
@@ -357,17 +354,15 @@ static void mtk_vcodec_add_formats(unsigned int fourcc,
mtk_video_formats[count_formats].fourcc = fourcc;
mtk_video_formats[count_formats].type = MTK_FMT_DEC;
mtk_video_formats[count_formats].num_planes = 1;
+ mtk_video_formats[count_formats].frmsize = stepwise_fhd;

- mtk_vdec_framesizes[count_framesizes].fourcc = fourcc;
- mtk_vdec_framesizes[count_framesizes].stepwise = stepwise_fhd;
if (!(ctx->dev->dec_capability & VCODEC_CAPABILITY_4K_DISABLED) &&
fourcc != V4L2_PIX_FMT_VP8_FRAME) {
- mtk_vdec_framesizes[count_framesizes].stepwise.max_width =
+ mtk_video_formats[count_formats].frmsize.max_width =
VCODEC_DEC_4K_CODED_WIDTH;
- mtk_vdec_framesizes[count_framesizes].stepwise.max_height =
+ mtk_video_formats[count_formats].frmsize.max_height =
VCODEC_DEC_4K_CODED_HEIGHT;
}
- num_framesizes++;
break;
case V4L2_PIX_FMT_MM21:
case V4L2_PIX_FMT_MT21C:
@@ -381,15 +376,15 @@ static void mtk_vcodec_add_formats(unsigned int fourcc,
}

num_formats++;
- mtk_v4l2_debug(3, "num_formats: %d num_frames:%d dec_capability: 0x%x",
- count_formats, count_framesizes, ctx->dev->dec_capability);
+ mtk_v4l2_debug(3, "num_formats: %d dec_capability: 0x%x",
+ count_formats, ctx->dev->dec_capability);
}

static void mtk_vcodec_get_supported_formats(struct mtk_vcodec_ctx *ctx)
{
int cap_format_count = 0, out_format_count = 0;

- if (num_formats && num_framesizes)
+ if (num_formats)
return;

if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_MM21) {
@@ -468,8 +463,6 @@ const struct mtk_vcodec_dec_pdata mtk_vdec_8183_pdata = {
.num_formats = &num_formats,
.default_out_fmt = &default_out_format,
.default_cap_fmt = &default_cap_format,
- .vdec_framesizes = mtk_vdec_framesizes,
- .num_framesizes = &num_framesizes,
.uses_stateless_api = true,
.worker = mtk_vdec_worker,
.flush_decoder = mtk_vdec_flush_decoder,
@@ -488,8 +481,6 @@ const struct mtk_vcodec_dec_pdata mtk_lat_sig_core_pdata = {
.num_formats = &num_formats,
.default_out_fmt = &default_out_format,
.default_cap_fmt = &default_cap_format,
- .vdec_framesizes = mtk_vdec_framesizes,
- .num_framesizes = &num_framesizes,
.uses_stateless_api = true,
.worker = mtk_vdec_worker,
.flush_decoder = mtk_vdec_flush_decoder,
@@ -507,8 +498,6 @@ const struct mtk_vcodec_dec_pdata mtk_vdec_single_core_pdata = {
.num_formats = &num_formats,
.default_out_fmt = &default_out_format,
.default_cap_fmt = &default_cap_format,
- .vdec_framesizes = mtk_vdec_framesizes,
- .num_framesizes = &num_framesizes,
.uses_stateless_api = true,
.worker = mtk_vdec_worker,
.flush_decoder = mtk_vdec_flush_decoder,
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
index 4f15f5f60e40..9c6ae78c346c 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
@@ -126,15 +126,7 @@ struct mtk_video_fmt {
enum mtk_fmt_type type;
u32 num_planes;
u32 flags;
-};
-
-/*
- * struct mtk_codec_framesizes - Structure used to store information about
- * framesizes
- */
-struct mtk_codec_framesizes {
- u32 fourcc;
- struct v4l2_frmsize_stepwise stepwise;
+ struct v4l2_frmsize_stepwise frmsize;
};

/*
@@ -371,9 +363,6 @@ enum mtk_vdec_format_types {
* @default_out_fmt: default output buffer format
* @default_cap_fmt: default capture buffer format
*
- * @vdec_framesizes: supported video decoder frame sizes
- * @num_framesizes: count of video decoder frame sizes
- *
* @hw_arch: hardware arch is used to separate pure_sin_core and lat_sin_core
*
* @is_subdev_supported: whether support parent-node architecture(subdev)
@@ -396,9 +385,6 @@ struct mtk_vcodec_dec_pdata {
const struct mtk_video_fmt *default_out_fmt;
const struct mtk_video_fmt *default_cap_fmt;

- const struct mtk_codec_framesizes *vdec_framesizes;
- const int *num_framesizes;
-
enum mtk_vdec_hw_arch hw_arch;

bool is_subdev_supported;
--
2.37.0.rc0.161.g10f37bed90-goog

2022-07-06 08:55:13

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v2 5/6] media: mediatek: vcodec: decoder: Drop max_{width,height} from mtk_vcodec_ctx

This partially reverts commit b018be06f3c7 ("media: mediatek: vcodec:
Read max resolution from dec_capability").

After the previous patches:

- media: mediatek: vcodec: decoder: Fix 4K frame size enumeration
- media: mediatek: vcodec: decoder: Skip alignment for default resolution
- media: mediatek: vcodec: decoder: Fix resolution clamping in TRY_FMT

the max_{width,height} fields in |struct mtk_vcodec_ctx| no longer have
any real users. Remove them.

Fixes: b018be06f3c7 ("media: mediatek: vcodec: Read max resolution from dec_capability")
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 9 ---------
drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h | 4 ----
2 files changed, 13 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
index cdd07fa612ee..2d370e8041c1 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -177,8 +177,6 @@ void mtk_vcodec_dec_set_default_params(struct mtk_vcodec_ctx *ctx)
q_data->coded_height = DFT_CFG_HEIGHT;
q_data->fmt = ctx->dev->vdec_pdata->default_cap_fmt;
q_data->field = V4L2_FIELD_NONE;
- ctx->max_width = MTK_VDEC_MAX_W;
- ctx->max_height = MTK_VDEC_MAX_H;

q_data->sizeimage[0] = q_data->coded_width * q_data->coded_height;
q_data->bytesperline[0] = q_data->coded_width;
@@ -514,13 +512,6 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv,
if (fmt == NULL)
return -EINVAL;

- if (!(ctx->dev->dec_capability & VCODEC_CAPABILITY_4K_DISABLED) &&
- fmt->fourcc != V4L2_PIX_FMT_VP8_FRAME) {
- mtk_v4l2_debug(3, "4K is enabled");
- ctx->max_width = VCODEC_DEC_4K_CODED_WIDTH;
- ctx->max_height = VCODEC_DEC_4K_CODED_HEIGHT;
- }
-
q_data->fmt = fmt;
vidioc_try_fmt(ctx, f, q_data->fmt);
if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
index 4140b4dd85bf..4f15f5f60e40 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
@@ -286,8 +286,6 @@ struct vdec_pic_info {
* mtk_video_dec_buf.
* @hw_id: hardware index used to identify different hardware.
*
- * @max_width: hardware supported max width
- * @max_height: hardware supported max height
* @msg_queue: msg queue used to store lat buffer information.
*/
struct mtk_vcodec_ctx {
@@ -334,8 +332,6 @@ struct mtk_vcodec_ctx {
struct mutex lock;
int hw_id;

- unsigned int max_width;
- unsigned int max_height;
struct vdec_msg_queue msg_queue;
};

--
2.37.0.rc0.161.g10f37bed90-goog

2022-07-06 08:56:34

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v2 3/6] media: mediatek: vcodec: decoder: Skip alignment for default resolution

The default resolution of 64x64 is already aligned, according to the
call to v4l_bound_align_image() in mtk_vcodec_dec_set_default_params().

Drop the redundant v4l_bound_align_image() call. This also removes one
usage of ctx->max_{width,height}.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 7 -------
1 file changed, 7 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
index fcb4b8131c49..4a64877bcd8f 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -180,13 +180,6 @@ void mtk_vcodec_dec_set_default_params(struct mtk_vcodec_ctx *ctx)
ctx->max_width = MTK_VDEC_MAX_W;
ctx->max_height = MTK_VDEC_MAX_H;

- v4l_bound_align_image(&q_data->coded_width,
- MTK_VDEC_MIN_W,
- ctx->max_width, 4,
- &q_data->coded_height,
- MTK_VDEC_MIN_H,
- ctx->max_height, 5, 6);
-
q_data->sizeimage[0] = q_data->coded_width * q_data->coded_height;
q_data->bytesperline[0] = q_data->coded_width;
q_data->sizeimage[1] = q_data->sizeimage[0] / 2;
--
2.37.0.rc0.161.g10f37bed90-goog

2022-07-06 09:04:33

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v2 4/6] media: mediatek: vcodec: decoder: Fix resolution clamping in TRY_FMT

In commit b018be06f3c7 ("media: mediatek: vcodec: Read max resolution
from dec_capability"), TRY_FMT clamps the resolution to the maximum
that was previously set either by default 1080p or the limit set by a
previous S_FMT call. This does not make sense when doing TRY_FMT for
the output side, which may have different capabilities.

Instead, for the output side, find the maximum resolution based on the
pixel format requested. For the capture side, find the maximum
resolution based on the currently set output format.

The maximum resolution is found from the list of per-format frame
sizes, so the patch "media: mediatek: vcodec: dec: Fix 4K frame size
enumeration" is needed.

Fixes: b018be06f3c7 ("media: mediatek: vcodec: Read max resolution from dec_capability")
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
.../platform/mediatek/vcodec/mtk_vcodec_dec.c | 48 ++++++++++++++-----
1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
index 4a64877bcd8f..cdd07fa612ee 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -263,17 +263,44 @@ static int vidioc_vdec_subscribe_evt(struct v4l2_fh *fh,
}
}

+static const struct v4l2_frmsize_stepwise *mtk_vdec_get_frmsize(struct mtk_vcodec_ctx *ctx,
+ u32 pixfmt)
+{
+ const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev->vdec_pdata;
+ int i;
+
+ for (i = 0; i < *dec_pdata->num_framesizes; ++i)
+ if (pixfmt == dec_pdata->vdec_framesizes[i].fourcc)
+ return &dec_pdata->vdec_framesizes[i].stepwise;
+
+ /*
+ * This should never happen since vidioc_try_fmt_vid_out_mplane()
+ * always passes through a valid format for the output side, and
+ * for the capture side, a valid output format should already have
+ * been set.
+ */
+ WARN_ONCE(1, "Unsupported format requested.\n");
+ return &dec_pdata->vdec_framesizes[0].stepwise;
+}
+
static int vidioc_try_fmt(struct mtk_vcodec_ctx *ctx, struct v4l2_format *f,
const struct mtk_video_fmt *fmt)
{
struct v4l2_pix_format_mplane *pix_fmt_mp = &f->fmt.pix_mp;
+ const struct v4l2_frmsize_stepwise *frmsize;
+ u32 fourcc;

pix_fmt_mp->field = V4L2_FIELD_NONE;

- pix_fmt_mp->width =
- clamp(pix_fmt_mp->width, MTK_VDEC_MIN_W, ctx->max_width);
- pix_fmt_mp->height =
- clamp(pix_fmt_mp->height, MTK_VDEC_MIN_H, ctx->max_height);
+ /* Always apply frame size constraints from the coded side */
+ if (V4L2_TYPE_IS_OUTPUT(f->type))
+ fourcc = f->fmt.pix_mp.pixelformat;
+ else
+ fourcc = ctx->q_data[MTK_Q_DATA_SRC].fmt->fourcc;
+
+ frmsize = mtk_vdec_get_frmsize(ctx, fourcc);
+ pix_fmt_mp->width = clamp(pix_fmt_mp->width, MTK_VDEC_MIN_W, frmsize->max_width);
+ pix_fmt_mp->height = clamp(pix_fmt_mp->height, MTK_VDEC_MIN_H, frmsize->max_height);

if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
pix_fmt_mp->num_planes = 1;
@@ -289,18 +316,15 @@ static int vidioc_try_fmt(struct mtk_vcodec_ctx *ctx, struct v4l2_format *f,
*/
tmp_w = pix_fmt_mp->width;
tmp_h = pix_fmt_mp->height;
- v4l_bound_align_image(&pix_fmt_mp->width,
- MTK_VDEC_MIN_W,
- ctx->max_width, 6,
- &pix_fmt_mp->height,
- MTK_VDEC_MIN_H,
- ctx->max_height, 6, 9);
+ v4l_bound_align_image(&pix_fmt_mp->width, MTK_VDEC_MIN_W, frmsize->max_width, 6,
+ &pix_fmt_mp->height, MTK_VDEC_MIN_H, frmsize->max_height, 6,
+ 9);

if (pix_fmt_mp->width < tmp_w &&
- (pix_fmt_mp->width + 64) <= ctx->max_width)
+ (pix_fmt_mp->width + 64) <= frmsize->max_width)
pix_fmt_mp->width += 64;
if (pix_fmt_mp->height < tmp_h &&
- (pix_fmt_mp->height + 64) <= ctx->max_height)
+ (pix_fmt_mp->height + 64) <= frmsize->max_height)
pix_fmt_mp->height += 64;

mtk_v4l2_debug(0,
--
2.37.0.rc0.161.g10f37bed90-goog

Subject: Re: [PATCH v2 0/6] media: mediatek: vcodec: Fix 4K decoding support

Il 06/07/22 10:21, Chen-Yu Tsai ha scritto:
> This is v2 of the mtk-vcodec 4K decoder fixes.
>
> While testing a backport of recent mtk-vcodec developments on ChromeOS
> v5.10 kernel [1], it was found that 4K decoding support had regressed.
> The decoder was not correctly reporting 4K frame sizes when queried,
> and ChromeOS then determined that the hardware did not support it.
>
> This turned out to be a mix of different bugs:
>
> 1. Frame size enumeration on the output side should not depend on the
> currently set format, or any other derived state. This is fixed in
> patch 2.
>
> 2. TRY_FMT on the output side was incorrectly clamping the resolution
> based on the current maximum values. It should not. Fixed in patch
> 4.
>
> 3. The default resolution limit was not set according to the default
> output format determined at runtime, but hard-coded to 1080p. An
> S_FMT call is needed to override this. The instance resolution limit
> is rendered useless after patches 3 and 4, and dropped in patch 5.
>
> Other patches:
> - Patch 1 makes stepwise_fhd constant.
> - Patch 3 drops redundant aligning of default resolution
> - Patch 6 moves framesize inside mtk_video_fmt, making it easier to
> access and removes a list search that was added in patch 4.
>
> Changes since v1:
> - Added patch to const-ify stepwise_fhd, as Nicolas requested.
> - Dropped old patch 2 (media: mediatek: vcodec: dec: Set default
> max resolution based on format)
> - Dropped old patch 4 (media: mediatek: vcodec: dec: Set maximum
> resolution when S_FMT on output only)
> - Made max resolution lookup for TRY_FMT return stepwise structure in
> patch 4, which helps with the last patch that moves framesize
> stepwise into mtk_video_fmt.
> - Did some style cleanups in patch 4
>
> This series is based on next-20220705.
>
> This was only tested on the backport kernel [1] on MT8195, which is the
> only currently supported SoC that does 4K decoding. Hopefully the folks
> at Collabora can give this a test on their mainline MT8195 integration
> branch.

Cannot spot any regression after fluster tests.

For the whole series:

Tested-by: AngeloGioacchino Del Regno <[email protected]>

>
>
> Regards
> ChenYu
>
> [1] https://crrev.com/c/3713491
>
> Chen-Yu Tsai (6):
> media: mediatek: vcodec: decoder: Const-ify stepwise_fhd
> media: mediatek: vcodec: decoder: Fix 4K frame size enumeration
> media: mediatek: vcodec: decoder: Skip alignment for default
> resolution
> media: mediatek: vcodec: decoder: Fix resolution clamping in TRY_FMT
> media: mediatek: vcodec: decoder: Drop max_{width,height} from
> mtk_vcodec_ctx
> media: mediatek: vcodec: decoder: Embed framesize inside mtk_video_fmt
>
> .../platform/mediatek/vcodec/mtk_vcodec_dec.c | 54 ++++++++-----------
> .../mediatek/vcodec/mtk_vcodec_dec_stateful.c | 29 +++-------
> .../vcodec/mtk_vcodec_dec_stateless.c | 30 +++++------
> .../platform/mediatek/vcodec/mtk_vcodec_drv.h | 20 +------
> 4 files changed, 41 insertions(+), 92 deletions(-)
>


--
AngeloGioacchino Del Regno
Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718