[PATCH 0/6] media: mediatek-vcodec: Fix capability fields again

[PATCH 0/6] media: mediatek-vcodec: Fix capability fields again

[Chen-Yu Tsai]

Hi everyone,

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 reverts the driver name field change. It also makes it
explicit that the field really should match the driver name.

Patch 2 changes the value for the card name, making it a free form
string that includes the SoC model.

Patch 3 removes setting the bus_info field, instead using the default
value derived from the underlying |struct device| as set by the V4L2
core.

Patches 4 through 6 do the same as 1 through 3 respectively, but for
the encoder side.

This series is based on next-20220701, 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 (6):
  media: mediatek: vcodec: Revert driver name change in decoder
    capabilities
  media: mediatek: vcodec: Use meaningful decoder card name including
    chip name
  media: mediatek: vcodec: Use default bus_info for decoder capability
  media: mediatek: vcodec: Revert driver name change in encoder
    capabilities
  media: mediatek: vcodec: Use meaningful encoder card name including
    chip name
  media: mediatek: vcodec: Use default bus_info for encoder capability

 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, 9 insertions(+), 6 deletions(-)

-- 
2.37.0.rc0.161.g10f37bed90-goog

[PATCH 1/6] media: mediatek: vcodec: Revert driver name change in decoder capabilities

[Chen-Yu Tsai]

This partially reverts commit a8a7a278c56ad3b4ddd4db9a960e0537d032b0b3.

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.

Fixes: a8a7a278c56a ("media: mediatek: vcodec: Change decoder v4l2 capability value")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
index 5d6fdf18c3a6..e7ea632a3f94 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -243,9 +243,11 @@ 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->driver, dev->driver->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);
 
-- 
2.37.0.rc0.161.g10f37bed90-goog

[PATCH 2/6] media: mediatek: vcodec: Use meaningful decoder card name including chip name

[Chen-Yu Tsai]

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.

Fixes: a8a7a278c56a ("media: mediatek: vcodec: Change decoder v4l2 capability value")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
index e7ea632a3f94..7f03dab518a4 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -248,7 +248,7 @@ static int vidioc_vdec_querycap(struct file *file, void *priv,
 	int platform_name = mtk_vcodec_dec_get_chip_name(priv);
 
 	strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
-	strscpy(cap->card, MTK_VCODEC_DEC_NAME, sizeof(cap->card));
+	snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", platform_name);
 	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:mt%d-dec", platform_name);
 
 	return 0;
-- 
2.37.0.rc0.161.g10f37bed90-goog

[PATCH 3/6] media: mediatek: vcodec: Use default bus_info for decoder capability

[Chen-Yu Tsai]

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.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
index 7f03dab518a4..209de1ec02e4 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec.c
@@ -249,7 +249,6 @@ static int vidioc_vdec_querycap(struct file *file, void *priv,
 
 	strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
 	snprintf(cap->card, sizeof(cap->card), "MT%d video decoder", platform_name);
-	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:mt%d-dec", platform_name);
 
 	return 0;
 }
-- 
2.37.0.rc0.161.g10f37bed90-goog

[PATCH 4/6] media: mediatek: vcodec: Revert driver name change in encoder capabilities

[Chen-Yu Tsai]

This partially reverts commit fd9f8050e355d7fd1e126cd207b06c96cde7f783.

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.

Fixes: fd9f8050e355 ("media: mediatek: vcodec: Change encoder v4l2 capability value")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h | 1 +
 drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 6 ++++--
 2 files changed, 5 insertions(+), 2 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..dc6aada882d9 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
@@ -22,6 +22,7 @@
 #define MTK_VCODEC_DRV_NAME	"mtk_vcodec_drv"
 #define MTK_VCODEC_DEC_NAME	"mtk-vcodec-dec"
 #define MTK_VCODEC_ENC_NAME	"mtk-vcodec-enc"
+#define MTK_PLATFORM_STR	"platform:mt8173"
 
 #define MTK_VCODEC_MAX_PLANES	3
 #define MTK_V4L2_BENCHMARK	0
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
index ccc753074816..30aac54d97fa 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
@@ -232,11 +232,13 @@ 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));
+	strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
 	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:mt%d-enc", platform_name);
+	strscpy(cap->card, MTK_PLATFORM_STR, sizeof(cap->card));
 
 	return 0;
 }
-- 
2.37.0.rc0.161.g10f37bed90-goog

Re: [PATCH 4/6] media: mediatek: vcodec: Revert driver name change in encoder capabilities

[Hans Verkuil]

On 7/1/22 12:52, Chen-Yu Tsai wrote:
> This partially reverts commit fd9f8050e355d7fd1e126cd207b06c96cde7f783.
> 
> 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.
> 
> Fixes: fd9f8050e355 ("media: mediatek: vcodec: Change encoder v4l2 capability value")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>  drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h | 1 +
>  drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 6 ++++--
>  2 files changed, 5 insertions(+), 2 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..dc6aada882d9 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> @@ -22,6 +22,7 @@
>  #define MTK_VCODEC_DRV_NAME	"mtk_vcodec_drv"

Note that this patch removes the last user of this define, so
you can drop that define as well.

>  #define MTK_VCODEC_DEC_NAME	"mtk-vcodec-dec"
>  #define MTK_VCODEC_ENC_NAME	"mtk-vcodec-enc"
> +#define MTK_PLATFORM_STR	"platform:mt8173"

Why add this?

>  
>  #define MTK_VCODEC_MAX_PLANES	3
>  #define MTK_V4L2_BENCHMARK	0
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> index ccc753074816..30aac54d97fa 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> @@ -232,11 +232,13 @@ 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));
> +	strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
>  	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:mt%d-enc", platform_name);
> +	strscpy(cap->card, MTK_PLATFORM_STR, sizeof(cap->card));

The next patch changes cap->card again, and leaves MTK_PLATFORM_STR unused.

>  
>  	return 0;
>  }

I think it makes more sense to combine patches 1-3 and 4-6 into single
patches, one for the decoder, one for the encoder. It's easier to follow
since they all touch on the same querycap function.

Regards,

	Hans

Re: [PATCH 4/6] media: mediatek: vcodec: Revert driver name change in encoder capabilities

[Chen-Yu Tsai]

On Fri, Jul 8, 2022 at 5:19 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
>
>
> On 7/1/22 12:52, Chen-Yu Tsai wrote:
> > This partially reverts commit fd9f8050e355d7fd1e126cd207b06c96cde7f783.
> >
> > 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.
> >
> > Fixes: fd9f8050e355 ("media: mediatek: vcodec: Change encoder v4l2 capability value")
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> >  drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h | 1 +
> >  drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 6 ++++--
> >  2 files changed, 5 insertions(+), 2 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..dc6aada882d9 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> > @@ -22,6 +22,7 @@
> >  #define MTK_VCODEC_DRV_NAME  "mtk_vcodec_drv"
>
> Note that this patch removes the last user of this define, so
> you can drop that define as well.
>
> >  #define MTK_VCODEC_DEC_NAME  "mtk-vcodec-dec"
> >  #define MTK_VCODEC_ENC_NAME  "mtk-vcodec-enc"
> > +#define MTK_PLATFORM_STR     "platform:mt8173"
>
> Why add this?
>
> >
> >  #define MTK_VCODEC_MAX_PLANES        3
> >  #define MTK_V4L2_BENCHMARK   0
> > diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> > index ccc753074816..30aac54d97fa 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> > @@ -232,11 +232,13 @@ 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));
> > +     strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
> >       snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:mt%d-enc", platform_name);
> > +     strscpy(cap->card, MTK_PLATFORM_STR, sizeof(cap->card));
>
> The next patch changes cap->card again, and leaves MTK_PLATFORM_STR unused.
>
> >
> >       return 0;
> >  }
>
> I think it makes more sense to combine patches 1-3 and 4-6 into single
> patches, one for the decoder, one for the encoder. It's easier to follow
> since they all touch on the same querycap function.

I wrote this series as a revert plus additional changes. As you said, it
makes sense to squash three patches into one.

I'll respin.

ChenYu

[PATCH 5/6] media: mediatek: vcodec: Use meaningful encoder card name including chip name

[Chen-Yu Tsai]

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.

Fixes: fd9f8050e355 ("media: mediatek: vcodec: Change encoder v4l2 capability value")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
index 30aac54d97fa..cc286e59021e 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
@@ -238,7 +238,7 @@ static int vidioc_venc_querycap(struct file *file, void *priv,
 
 	strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
 	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:mt%d-enc", platform_name);
-	strscpy(cap->card, MTK_PLATFORM_STR, sizeof(cap->card));
+	snprintf(cap->card, sizeof(cap->card), "MT%d video encoder", platform_name);
 
 	return 0;
 }
-- 
2.37.0.rc0.161.g10f37bed90-goog

[PATCH 6/6] media: mediatek: vcodec: Use default bus_info for encoder capability

[Chen-Yu Tsai]

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.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
index cc286e59021e..25e816863597 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
@@ -237,7 +237,6 @@ static int vidioc_venc_querycap(struct file *file, void *priv,
 	int platform_name = mtk_vcodec_enc_get_chip_name(priv);
 
 	strscpy(cap->driver, dev->driver->name, sizeof(cap->driver));
-	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:mt%d-enc", platform_name);
 	snprintf(cap->card, sizeof(cap->card), "MT%d video encoder", platform_name);
 
 	return 0;
-- 
2.37.0.rc0.161.g10f37bed90-goog

Re: [PATCH 0/6] media: mediatek-vcodec: Fix capability fields again

[Chen-Yu Tsai]

On Fri, Jul 1, 2022 at 8:45 PM Nicolas Dufresne
<nicolas.dufresne@collabora.com> wrote:
>
> Hi Chen,
>
> Le 1 juill. 2022 06 h 52, Chen-Yu Tsai <wenst@chromium.org> a écrit :
>
> Hi everyone,
>
> 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.
>
>
> This match is not specified in the spec. If you feel like it should, perhaps consider specifying it first. To me it's nice if they match, but it's for now just cosmetic.

Right. Even for cosmetic reasons I think my changes make sense though.
Fixing the matching is another thing.

> Though this requires some discussion, as userland is expected to enumerate the media device and find the video device by walking the topology. This is the only specified way to match both.

AFAICT, v4l2-ctl does similar matching, though by bus_info. Maybe it's
out of convenience?

ChenYu

>
>
> 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 reverts the driver name field change. It also makes it
> explicit that the field really should match the driver name.
>
> Patch 2 changes the value for the card name, making it a free form
> string that includes the SoC model.
>
> Patch 3 removes setting the bus_info field, instead using the default
> value derived from the underlying |struct device| as set by the V4L2
> core.
>
> Patches 4 through 6 do the same as 1 through 3 respectively, but for
> the encoder side.
>
> This series is based on next-20220701, 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 (6):
>   media: mediatek: vcodec: Revert driver name change in decoder
>     capabilities
>   media: mediatek: vcodec: Use meaningful decoder card name including
>     chip name
>   media: mediatek: vcodec: Use default bus_info for decoder capability
>   media: mediatek: vcodec: Revert driver name change in encoder
>     capabilities
>   media: mediatek: vcodec: Use meaningful encoder card name including
>     chip name
>   media: mediatek: vcodec: Use default bus_info for encoder capability
>
> 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, 9 insertions(+), 6 deletions(-)
>
> --
> 2.37.0.rc0.161.g10f37bed90-goog
>
>

Re: [PATCH 0/6] media: mediatek-vcodec: Fix capability fields again

[Nicolas Dufresne]

Le vendredi 01 juillet 2022 à 21:10 +0800, Chen-Yu Tsai a écrit :
> On Fri, Jul 1, 2022 at 8:45 PM Nicolas Dufresne
> <nicolas.dufresne@collabora.com> wrote:
> > 
> > Hi Chen,
> > 
> > Le 1 juill. 2022 06 h 52, Chen-Yu Tsai <wenst@chromium.org> a écrit :
> > 
> > Hi everyone,
> > 
> > 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.
> > 
> > 
> > This match is not specified in the spec. If you feel like it should, perhaps consider specifying it first. To me it's nice if they match, but it's for now just cosmetic.
> 
> Right. Even for cosmetic reasons I think my changes make sense though.
> Fixing the matching is another thing.
> 
> > Though this requires some discussion, as userland is expected to enumerate the media device and find the video device by walking the topology. This is the only specified way to match both.
> 
> AFAICT, v4l2-ctl does similar matching, though by bus_info. Maybe it's
> out of convenience?

bus_info has indeed been used a lot in the past, it will be more reliable then
"driver name" matching. Using topology remains the recommended identification
method.

> 
> ChenYu
> 
> > 
> > 
> > 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 reverts the driver name field change. It also makes it
> > explicit that the field really should match the driver name.
> > 
> > Patch 2 changes the value for the card name, making it a free form
> > string that includes the SoC model.
> > 
> > Patch 3 removes setting the bus_info field, instead using the default
> > value derived from the underlying |struct device| as set by the V4L2
> > core.
> > 
> > Patches 4 through 6 do the same as 1 through 3 respectively, but for
> > the encoder side.
> > 
> > This series is based on next-20220701, 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 (6):
> >   media: mediatek: vcodec: Revert driver name change in decoder
> >     capabilities
> >   media: mediatek: vcodec: Use meaningful decoder card name including
> >     chip name
> >   media: mediatek: vcodec: Use default bus_info for decoder capability
> >   media: mediatek: vcodec: Revert driver name change in encoder
> >     capabilities
> >   media: mediatek: vcodec: Use meaningful encoder card name including
> >     chip name
> >   media: mediatek: vcodec: Use default bus_info for encoder capability
> > 
> > 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, 9 insertions(+), 6 deletions(-)
> > 
> > --
> > 2.37.0.rc0.161.g10f37bed90-goog
> > 
> >