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