mbox series

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

Message ID 20220701105237.932332-1-wenst@chromium.org (mailing list archive)
Headers show
Series media: mediatek-vcodec: Fix capability fields again | expand

Message

Chen-Yu Tsai July 1, 2022, 10:52 a.m. UTC
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(-)

Comments

Chen-Yu Tsai July 1, 2022, 1:10 p.m. UTC | #1
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
>
>
Nicolas Dufresne July 4, 2022, 7:01 p.m. UTC | #2
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
> > 
> >