2022-07-08 10:47:10

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v2 0/2] media: mediatek-vcodec: Fix capability fields again

Hi everyone,

This is v2 of my mtk-vcodec capabilities fixes series.

Changes since v1:
- Squashed 3 patches into one, as Hans recommended

The previous round of changes to the mtk-vcodec driver's returned
capabilities caused some issues for ChromeOS. In particular, the
ChromeOS stateless video decoder uses the "Driver Name" field to
match a video device to its media device. As the field was only
changed for the video device and not the media device, a match
could no longer be found.

While fixing this, I found that the current info used for the fields
don't make a lot of sense, and tried to fix them in this series.

Patch 1 fixes the capabilities for the decoder. The driver name field
change is reverted and made explicit that the field really should match
the driver name. The card name is made a user readable string that
includes the SoC model. The bus_info field is dropped and the default
value from the V4L2 core is used.

Patch 2 does the same, but for the encoder size. And since the last
reference to MTK_VCODEC_DRV_NAME is removed, the macro is removed as
well.

This series is based on next-20220708, and was tested on mainline on
MT8183 Juniper, and on ChromeOS v5.10, on which we have a whole bunch
of backports pending, on MT8195 Tomato.

Please have a look.


Thanks
ChenYu

Chen-Yu Tsai (2):
media: mediatek: vcodec: Make decoder capability fields fit
requirements
media: mediatek: vcodec: Make encoder capability fields fit
requirements

drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 7 ++++---
drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h | 1 -
drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 7 ++++---
3 files changed, 8 insertions(+), 7 deletions(-)

--
2.37.0.rc0.161.g10f37bed90-goog


2022-07-08 10:47:31

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v2 1/2] media: mediatek: vcodec: Make decoder capability fields fit requirements

This partially reverts commit a8a7a278c56ad3b4ddd4db9a960e0537d032b0b3,
and changes things so that the capability string fields of the decoder
conform to their requirements.

This recent change caused ChromeOS's decoder to no longer function. This
is due to ChromeOS using the driver name field to match the video device
with its accompanying media device. After the change, they no longer
matched.

The driver name field should contain the actual driver name, not some
otherwise unused string macro from the driver. To make this clear,
copy the name from the driver's name field.

The card name for the video decoder previously held a static platform
name that was fixed to match MT8173. This obviously doesn't make sense
for newer chips. Since commit a8a7a278c56a ("media: mediatek: vcodec:
Change decoder v4l2 capability value"), this field was changed to hold
the driver's name, or "mtk-vcodec-dec". This doesn't make much sense
either, since this still doesn't reflect what chip this is.

Instead, fill in the card name with "MTxxxx video decoder" with the
proper chip number.

Since commit f2d8b6917f3b ("media: v4l: ioctl: Set bus_info in
v4l_querycap()"), the V4L2 core provides a default value for the
bus_info field for platform and PCI devices. This value will match
the default value for media devices added by commit cef699749f37
("media: mc: Set bus_info in media_device_init()"). These defaults
are stable and device-specific.

Drop the custom capability bus_info from the mtk-vcodec decoder
driver, and use the defaults. This also fixes the long standing
issue where the media device used for the stateless decoder didn't
have its bus_info set, and would never match its accompanying video
device.

Fixes: a8a7a278c56a ("media: mediatek: vcodec: Change decoder v4l2 capability value")
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 7 ++++---
1 file changed, 4 insertions(+), 3 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..209de1ec02e4 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -243,11 +243,12 @@ static int mtk_vcodec_dec_get_chip_name(void *priv)
static int vidioc_vdec_querycap(struct file *file, void *priv,
struct v4l2_capability *cap)
{
+ struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
+ struct device *dev = &ctx->dev->plat_dev->dev;
int platform_name = mtk_vcodec_dec_get_chip_name(priv);

- strscpy(cap->driver, MTK_VCODEC_DRV_NAME, sizeof(cap->driver));
- strscpy(cap->card, MTK_VCODEC_DEC_NAME, sizeof(cap->card));
- snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:mt%d-dec", platform_name);
+ strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
+ snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", platform_name);

return 0;
}
--
2.37.0.rc0.161.g10f37bed90-goog

2022-07-08 11:13:50

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v2 2/2] media: mediatek: vcodec: Make encoder capability fields fit requirements

This partially reverts commit fd9f8050e355d7fd1e126cd207b06c96cde7f783,
and changes things so that the capability string fields of the encoder
conform to their requirements.

The driver name field should contain the actual driver name, not some
otherwise unused string macro from the driver. To make this clear,
copy the name from the driver's name field.

The card name for the video encoder previously held a static platform
name that was fixed to match MT8173. This obviously doesn't make sense
for newer chips. Since commit fd9f8050e355 ("media: mediatek: vcodec:
Change encoder v4l2 capability value"), this field was changed to hold
the driver's name, or "mtk-vcodec-dec". This doesn't make much sense
either, since this still doesn't reflect what chip this is.

Instead, fill in the card name with "MTxxxx video encoder" with the
proper chip number.

Since commit f2d8b6917f3b ("media: v4l: ioctl: Set bus_info in
v4l_querycap()"), the V4L2 core provides a default value for the
bus_info field for platform and PCI devices. This value will match
the default value for media devices added by commit cef699749f37
("media: mc: Set bus_info in media_device_init()"). These defaults
are stable and device-specific.

Drop the custom capability bus_info from the mtk-vcodec encoder
driver, and use the defaults.

As this patch removes the last usage of MTK_VCODEC_DRV_NAME, remove
the macro as well.

Fixes: fd9f8050e355 ("media: mediatek: vcodec: Change encoder v4l2 capability value")
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h | 1 -
drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 7 ++++---
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
index 4140b4dd85bf..a8570a654561 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
@@ -19,7 +19,6 @@
#include "mtk_vcodec_util.h"
#include "vdec_msg_queue.h"

-#define MTK_VCODEC_DRV_NAME "mtk_vcodec_drv"
#define MTK_VCODEC_DEC_NAME "mtk-vcodec-dec"
#define MTK_VCODEC_ENC_NAME "mtk-vcodec-enc"

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
index ccc753074816..25e816863597 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
@@ -232,11 +232,12 @@ static int mtk_vcodec_enc_get_chip_name(void *priv)
static int vidioc_venc_querycap(struct file *file, void *priv,
struct v4l2_capability *cap)
{
+ struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
+ struct device *dev = &ctx->dev->plat_dev->dev;
int platform_name = mtk_vcodec_enc_get_chip_name(priv);

- strscpy(cap->driver, MTK_VCODEC_DRV_NAME, sizeof(cap->driver));
- strscpy(cap->card, MTK_VCODEC_ENC_NAME, sizeof(cap->card));
- snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:mt%d-enc", platform_name);
+ strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
+ snprintf(cap->card, sizeof(cap->card), "MT%d video encoder", platform_name);

return 0;
}
--
2.37.0.rc0.161.g10f37bed90-goog