| Message ID | 20210427111526.1772293-8-acourbot@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: mtk-vcodec: support for MT8183 decoder | expand |
On 27/04/2021 13:15, Alexandre Courbot wrote: > From: Yunfei Dong <yunfei.dong@mediatek.com> > > The stateless API requires a media device for issuing requests. Add one > if we are being instantiated as a stateless decoder. Why for the stateless decoder only? Why not create one for all? It's not a blocker, but I would recommend looking at this. Regards, Hans > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > [acourbot: refactor, cleanup and split] > Co-developed-by: Alexandre Courbot <acourbot@chromium.org> > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > --- > drivers/media/platform/Kconfig | 1 + > .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 39 +++++++++++++++++++ > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 2 + > 3 files changed, 42 insertions(+) > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index ae1468aa1b4e..4154fdec2efb 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -315,6 +315,7 @@ config VIDEO_MEDIATEK_VCODEC > select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU > select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP > select V4L2_H264 > + select MEDIA_CONTROLLER > help > Mediatek video codec driver provides HW capability to > encode and decode in a range of video formats on MT8173 > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > index 533781d4680a..e942e28f96fe 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > @@ -14,6 +14,7 @@ > #include <media/v4l2-event.h> > #include <media/v4l2-mem2mem.h> > #include <media/videobuf2-dma-contig.h> > +#include <media/v4l2-device.h> > > #include "mtk_vcodec_drv.h" > #include "mtk_vcodec_dec.h" > @@ -324,6 +325,31 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > goto err_event_workq; > } > > + if (dev->vdec_pdata->uses_stateless_api) { > + dev->mdev_dec.dev = &pdev->dev; > + strscpy(dev->mdev_dec.model, MTK_VCODEC_DEC_NAME, > + sizeof(dev->mdev_dec.model)); > + > + media_device_init(&dev->mdev_dec); > + dev->mdev_dec.ops = &mtk_vcodec_media_ops; > + dev->v4l2_dev.mdev = &dev->mdev_dec; > + > + ret = v4l2_m2m_register_media_controller(dev->m2m_dev_dec, > + dev->vfd_dec, MEDIA_ENT_F_PROC_VIDEO_DECODER); > + if (ret) { > + mtk_v4l2_err("Failed to register media controller"); > + goto err_reg_cont; > + } > + > + ret = media_device_register(&dev->mdev_dec); > + if (ret) { > + mtk_v4l2_err("Failed to register media device"); > + goto err_media_reg; > + } > + > + mtk_v4l2_debug(0, "media registered as /dev/media%d", > + vfd_dec->num); > + } > ret = video_register_device(vfd_dec, VFL_TYPE_VIDEO, 0); > if (ret) { > mtk_v4l2_err("Failed to register video device"); > @@ -336,6 +362,12 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > return 0; > > err_dec_reg: > + if (dev->vdec_pdata->uses_stateless_api) > + media_device_unregister(&dev->mdev_dec); > +err_media_reg: > + if (dev->vdec_pdata->uses_stateless_api) > + v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec); > +err_reg_cont: > destroy_workqueue(dev->decode_workqueue); > err_event_workq: > v4l2_m2m_release(dev->m2m_dev_dec); > @@ -368,6 +400,13 @@ static int mtk_vcodec_dec_remove(struct platform_device *pdev) > > flush_workqueue(dev->decode_workqueue); > destroy_workqueue(dev->decode_workqueue); > + > + if (media_devnode_is_registered(dev->mdev_dec.devnode)) { > + media_device_unregister(&dev->mdev_dec); > + v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec); > + media_device_cleanup(&dev->mdev_dec); > + } > + > if (dev->m2m_dev_dec) > v4l2_m2m_release(dev->m2m_dev_dec); > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > index 78d4a7728ddf..10edd238c939 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > @@ -383,6 +383,7 @@ struct mtk_vcodec_enc_pdata { > * struct mtk_vcodec_dev - driver data > * @v4l2_dev: V4L2 device to register video devices for. > * @vfd_dec: Video device for decoder > + * @mdev_dec: Media device for decoder > * @vfd_enc: Video device for encoder. > * > * @m2m_dev_dec: m2m device for decoder > @@ -418,6 +419,7 @@ struct mtk_vcodec_enc_pdata { > struct mtk_vcodec_dev { > struct v4l2_device v4l2_dev; > struct video_device *vfd_dec; > + struct media_device mdev_dec; > struct video_device *vfd_enc; > > struct v4l2_m2m_dev *m2m_dev_dec; >
Hi Hans, thanks for the review! On Thu, Apr 29, 2021 at 4:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote: > > On 27/04/2021 13:15, Alexandre Courbot wrote: > > From: Yunfei Dong <yunfei.dong@mediatek.com> > > > > The stateless API requires a media device for issuing requests. Add one > > if we are being instantiated as a stateless decoder. > > Why for the stateless decoder only? Why not create one for all? > > It's not a blocker, but I would recommend looking at this. Would there be any use in creating a media device for a stateful decoder that does not need to use requests? Cheers, Alex.
Le jeudi 13 mai 2021 à 17:05 +0900, Alexandre Courbot a écrit : > Hi Hans, thanks for the review! > > On Thu, Apr 29, 2021 at 4:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote: > > > > On 27/04/2021 13:15, Alexandre Courbot wrote: > > > From: Yunfei Dong <yunfei.dong@mediatek.com> > > > > > > The stateless API requires a media device for issuing requests. Add one > > > if we are being instantiated as a stateless decoder. > > > > Why for the stateless decoder only? Why not create one for all? > > > > It's not a blocker, but I would recommend looking at this. > > Would there be any use in creating a media device for a stateful > decoder that does not need to use requests? The only thing I can think of is classification. In GStreamer support for stateless decoders, I use the topology to classify what is a decoder by walking the toplogy and making sure it's what I expect, and that there is no other branches or functions that I may not support. This makes it more strict, so less likely to confuse driver function. Note that v4l2-compliance just use the same old heuristic we have used for stateful, which is to check that one side have only RAW format, and the other side have only encoded formats. It works too, it's just less strict. > > Cheers, > Alex.
On Tue, Apr 27, 2021 at 7:16 PM Alexandre Courbot <acourbot@chromium.org> wrote: > > From: Yunfei Dong <yunfei.dong@mediatek.com> > > The stateless API requires a media device for issuing requests. Add one > if we are being instantiated as a stateless decoder. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > [acourbot: refactor, cleanup and split] > Co-developed-by: Alexandre Courbot <acourbot@chromium.org> > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > --- > drivers/media/platform/Kconfig | 1 + > .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 39 +++++++++++++++++++ > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 2 + > 3 files changed, 42 insertions(+) > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index ae1468aa1b4e..4154fdec2efb 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -315,6 +315,7 @@ config VIDEO_MEDIATEK_VCODEC > select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU > select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP > select V4L2_H264 > + select MEDIA_CONTROLLER Should this also select MEDIA_CONTROLLER_REQUEST_API config? Thanks > help > Mediatek video codec driver provides HW capability to > encode and decode in a range of video formats on MT8173 > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > index 533781d4680a..e942e28f96fe 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > @@ -14,6 +14,7 @@ > #include <media/v4l2-event.h> > #include <media/v4l2-mem2mem.h> > #include <media/videobuf2-dma-contig.h> > +#include <media/v4l2-device.h> > > #include "mtk_vcodec_drv.h" > #include "mtk_vcodec_dec.h" > @@ -324,6 +325,31 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > goto err_event_workq; > } > > + if (dev->vdec_pdata->uses_stateless_api) { > + dev->mdev_dec.dev = &pdev->dev; > + strscpy(dev->mdev_dec.model, MTK_VCODEC_DEC_NAME, > + sizeof(dev->mdev_dec.model)); > + > + media_device_init(&dev->mdev_dec); > + dev->mdev_dec.ops = &mtk_vcodec_media_ops; > + dev->v4l2_dev.mdev = &dev->mdev_dec; > + > + ret = v4l2_m2m_register_media_controller(dev->m2m_dev_dec, > + dev->vfd_dec, MEDIA_ENT_F_PROC_VIDEO_DECODER); > + if (ret) { > + mtk_v4l2_err("Failed to register media controller"); > + goto err_reg_cont; > + } > + > + ret = media_device_register(&dev->mdev_dec); > + if (ret) { > + mtk_v4l2_err("Failed to register media device"); > + goto err_media_reg; > + } > + > + mtk_v4l2_debug(0, "media registered as /dev/media%d", > + vfd_dec->num); > + } > ret = video_register_device(vfd_dec, VFL_TYPE_VIDEO, 0); > if (ret) { > mtk_v4l2_err("Failed to register video device"); > @@ -336,6 +362,12 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > return 0; > > err_dec_reg: > + if (dev->vdec_pdata->uses_stateless_api) > + media_device_unregister(&dev->mdev_dec); > +err_media_reg: > + if (dev->vdec_pdata->uses_stateless_api) > + v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec); > +err_reg_cont: > destroy_workqueue(dev->decode_workqueue); > err_event_workq: > v4l2_m2m_release(dev->m2m_dev_dec); > @@ -368,6 +400,13 @@ static int mtk_vcodec_dec_remove(struct platform_device *pdev) > > flush_workqueue(dev->decode_workqueue); > destroy_workqueue(dev->decode_workqueue); > + > + if (media_devnode_is_registered(dev->mdev_dec.devnode)) { > + media_device_unregister(&dev->mdev_dec); > + v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec); > + media_device_cleanup(&dev->mdev_dec); > + } > + > if (dev->m2m_dev_dec) > v4l2_m2m_release(dev->m2m_dev_dec); > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > index 78d4a7728ddf..10edd238c939 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > @@ -383,6 +383,7 @@ struct mtk_vcodec_enc_pdata { > * struct mtk_vcodec_dev - driver data > * @v4l2_dev: V4L2 device to register video devices for. > * @vfd_dec: Video device for decoder > + * @mdev_dec: Media device for decoder > * @vfd_enc: Video device for encoder. > * > * @m2m_dev_dec: m2m device for decoder > @@ -418,6 +419,7 @@ struct mtk_vcodec_enc_pdata { > struct mtk_vcodec_dev { > struct v4l2_device v4l2_dev; > struct video_device *vfd_dec; > + struct media_device mdev_dec; > struct video_device *vfd_enc; > > struct v4l2_m2m_dev *m2m_dev_dec; > -- > 2.31.1.498.g6c1eba8ee3d-goog > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Mon, May 17, 2021 at 1:35 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > On Tue, Apr 27, 2021 at 7:16 PM Alexandre Courbot <acourbot@chromium.org> wrote: > > > > From: Yunfei Dong <yunfei.dong@mediatek.com> > > > > The stateless API requires a media device for issuing requests. Add one > > if we are being instantiated as a stateless decoder. > > > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > [acourbot: refactor, cleanup and split] > > Co-developed-by: Alexandre Courbot <acourbot@chromium.org> > > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > > --- > > drivers/media/platform/Kconfig | 1 + > > .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 39 +++++++++++++++++++ > > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 2 + > > 3 files changed, 42 insertions(+) > > > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > > index ae1468aa1b4e..4154fdec2efb 100644 > > --- a/drivers/media/platform/Kconfig > > +++ b/drivers/media/platform/Kconfig > > @@ -315,6 +315,7 @@ config VIDEO_MEDIATEK_VCODEC > > select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU > > select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP > > select V4L2_H264 > > + select MEDIA_CONTROLLER > > Should this also select MEDIA_CONTROLLER_REQUEST_API config? Yup, it probably should. hantro and rkvdec also select it, so let's do the same. Thanks! Alex.
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index ae1468aa1b4e..4154fdec2efb 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -315,6 +315,7 @@ config VIDEO_MEDIATEK_VCODEC select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP select V4L2_H264 + select MEDIA_CONTROLLER help Mediatek video codec driver provides HW capability to encode and decode in a range of video formats on MT8173 diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c index 533781d4680a..e942e28f96fe 100644 --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c @@ -14,6 +14,7 @@ #include <media/v4l2-event.h> #include <media/v4l2-mem2mem.h> #include <media/videobuf2-dma-contig.h> +#include <media/v4l2-device.h> #include "mtk_vcodec_drv.h" #include "mtk_vcodec_dec.h" @@ -324,6 +325,31 @@ static int mtk_vcodec_probe(struct platform_device *pdev) goto err_event_workq; } + if (dev->vdec_pdata->uses_stateless_api) { + dev->mdev_dec.dev = &pdev->dev; + strscpy(dev->mdev_dec.model, MTK_VCODEC_DEC_NAME, + sizeof(dev->mdev_dec.model)); + + media_device_init(&dev->mdev_dec); + dev->mdev_dec.ops = &mtk_vcodec_media_ops; + dev->v4l2_dev.mdev = &dev->mdev_dec; + + ret = v4l2_m2m_register_media_controller(dev->m2m_dev_dec, + dev->vfd_dec, MEDIA_ENT_F_PROC_VIDEO_DECODER); + if (ret) { + mtk_v4l2_err("Failed to register media controller"); + goto err_reg_cont; + } + + ret = media_device_register(&dev->mdev_dec); + if (ret) { + mtk_v4l2_err("Failed to register media device"); + goto err_media_reg; + } + + mtk_v4l2_debug(0, "media registered as /dev/media%d", + vfd_dec->num); + } ret = video_register_device(vfd_dec, VFL_TYPE_VIDEO, 0); if (ret) { mtk_v4l2_err("Failed to register video device"); @@ -336,6 +362,12 @@ static int mtk_vcodec_probe(struct platform_device *pdev) return 0; err_dec_reg: + if (dev->vdec_pdata->uses_stateless_api) + media_device_unregister(&dev->mdev_dec); +err_media_reg: + if (dev->vdec_pdata->uses_stateless_api) + v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec); +err_reg_cont: destroy_workqueue(dev->decode_workqueue); err_event_workq: v4l2_m2m_release(dev->m2m_dev_dec); @@ -368,6 +400,13 @@ static int mtk_vcodec_dec_remove(struct platform_device *pdev) flush_workqueue(dev->decode_workqueue); destroy_workqueue(dev->decode_workqueue); + + if (media_devnode_is_registered(dev->mdev_dec.devnode)) { + media_device_unregister(&dev->mdev_dec); + v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec); + media_device_cleanup(&dev->mdev_dec); + } + if (dev->m2m_dev_dec) v4l2_m2m_release(dev->m2m_dev_dec); diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h index 78d4a7728ddf..10edd238c939 100644 --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h @@ -383,6 +383,7 @@ struct mtk_vcodec_enc_pdata { * struct mtk_vcodec_dev - driver data * @v4l2_dev: V4L2 device to register video devices for. * @vfd_dec: Video device for decoder + * @mdev_dec: Media device for decoder * @vfd_enc: Video device for encoder. * * @m2m_dev_dec: m2m device for decoder @@ -418,6 +419,7 @@ struct mtk_vcodec_enc_pdata { struct mtk_vcodec_dev { struct v4l2_device v4l2_dev; struct video_device *vfd_dec; + struct media_device mdev_dec; struct video_device *vfd_enc; struct v4l2_m2m_dev *m2m_dev_dec;