2016-11-07 12:42:20

by Minghsiu Tsai

[permalink] [raw]
Subject: [PATCH] media: mtk-mdp: allocate video_device dynamically

v4l2-compliance test output:
v4l2-compliance SHA : abc1453dfe89f244dccd3460d8e1a2e3091cbadb

Driver Info:
Driver name : mtk-mdp
Card type : soc:mdp
Bus info : platform:mt8173
Driver version: 4.9.0
Capabilities : 0x84204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Device Capabilities
Device Caps : 0x04204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format

Compliance test for device /dev/image-proc0 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0

Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
test VIDIOC_QUERYCTRL: OK
test VIDIOC_G/S_CTRL: OK
test VIDIOC_G/S/TRY_EXT_CTRLS: OK
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 5 Private Controls: 0

Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK (Not Supported)
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK
test Composing: OK
test Scaling: OK

Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK

Test input 0:


Total: 43, Succeeded: 43, Failed: 0, Warnings: 0

Minghsiu Tsai (1):
[media] mtk-mdp: allocate video_device dynamically

drivers/media/platform/mtk-mdp/mtk_mdp_core.h | 2 +-
drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 33 ++++++++++++++++-----------
2 files changed, 21 insertions(+), 14 deletions(-)

--
1.9.1




2016-11-07 12:42:24

by Minghsiu Tsai

[permalink] [raw]
Subject: [PATCH] [media] mtk-mdp: allocate video_device dynamically

It can fix known problems with embedded video_device structs.

Signed-off-by: Minghsiu Tsai <[email protected]>
---
drivers/media/platform/mtk-mdp/mtk_mdp_core.h | 2 +-
drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 33 ++++++++++++++++-----------
2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
index 848569d..ad1cff3 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
@@ -167,7 +167,7 @@ struct mtk_mdp_dev {
struct mtk_mdp_comp *comp[MTK_MDP_COMP_ID_MAX];
struct v4l2_m2m_dev *m2m_dev;
struct list_head ctx_list;
- struct video_device vdev;
+ struct video_device *vdev;
struct v4l2_device v4l2_dev;
struct workqueue_struct *job_wq;
struct platform_device *vpu_dev;
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
index 9a747e7..b8dee1c 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
@@ -1236,16 +1236,22 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
int ret;

mdp->variant = &mtk_mdp_default_variant;
- mdp->vdev.device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
- mdp->vdev.fops = &mtk_mdp_m2m_fops;
- mdp->vdev.ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
- mdp->vdev.release = video_device_release_empty;
- mdp->vdev.lock = &mdp->lock;
- mdp->vdev.vfl_dir = VFL_DIR_M2M;
- mdp->vdev.v4l2_dev = &mdp->v4l2_dev;
- snprintf(mdp->vdev.name, sizeof(mdp->vdev.name), "%s:m2m",
+ mdp->vdev = video_device_alloc();
+ if (!mdp->vdev) {
+ dev_err(dev, "failed to allocate video device\n");
+ ret = -ENOMEM;
+ goto err_video_alloc;
+ }
+ mdp->vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
+ mdp->vdev->fops = &mtk_mdp_m2m_fops;
+ mdp->vdev->ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
+ mdp->vdev->release = video_device_release;
+ mdp->vdev->lock = &mdp->lock;
+ mdp->vdev->vfl_dir = VFL_DIR_M2M;
+ mdp->vdev->v4l2_dev = &mdp->v4l2_dev;
+ snprintf(mdp->vdev->name, sizeof(mdp->vdev->name), "%s:m2m",
MTK_MDP_MODULE_NAME);
- video_set_drvdata(&mdp->vdev, mdp);
+ video_set_drvdata(mdp->vdev, mdp);

mdp->m2m_dev = v4l2_m2m_init(&mtk_mdp_m2m_ops);
if (IS_ERR(mdp->m2m_dev)) {
@@ -1254,26 +1260,27 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
goto err_m2m_init;
}

- ret = video_register_device(&mdp->vdev, VFL_TYPE_GRABBER, 2);
+ ret = video_register_device(mdp->vdev, VFL_TYPE_GRABBER, 2);
if (ret) {
dev_err(dev, "failed to register video device\n");
goto err_vdev_register;
}

v4l2_info(&mdp->v4l2_dev, "driver registered as /dev/video%d",
- mdp->vdev.num);
+ mdp->vdev->num);
return 0;

err_vdev_register:
v4l2_m2m_release(mdp->m2m_dev);
err_m2m_init:
- video_device_release(&mdp->vdev);
+ video_unregister_device(mdp->vdev);
+err_video_alloc:

return ret;
}

void mtk_mdp_unregister_m2m_device(struct mtk_mdp_dev *mdp)
{
- video_device_release(&mdp->vdev);
+ video_unregister_device(mdp->vdev);
v4l2_m2m_release(mdp->m2m_dev);
}
--
1.9.1

2016-11-11 10:52:10

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH] [media] mtk-mdp: allocate video_device dynamically

Almost correct:

On 11/07/2016 01:42 PM, Minghsiu Tsai wrote:
> It can fix known problems with embedded video_device structs.
>
> Signed-off-by: Minghsiu Tsai <[email protected]>
> ---
> drivers/media/platform/mtk-mdp/mtk_mdp_core.h | 2 +-
> drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 33 ++++++++++++++++-----------
> 2 files changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> index 848569d..ad1cff3 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> @@ -167,7 +167,7 @@ struct mtk_mdp_dev {
> struct mtk_mdp_comp *comp[MTK_MDP_COMP_ID_MAX];
> struct v4l2_m2m_dev *m2m_dev;
> struct list_head ctx_list;
> - struct video_device vdev;
> + struct video_device *vdev;
> struct v4l2_device v4l2_dev;
> struct workqueue_struct *job_wq;
> struct platform_device *vpu_dev;
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> index 9a747e7..b8dee1c 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> @@ -1236,16 +1236,22 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
> int ret;
>
> mdp->variant = &mtk_mdp_default_variant;
> - mdp->vdev.device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> - mdp->vdev.fops = &mtk_mdp_m2m_fops;
> - mdp->vdev.ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
> - mdp->vdev.release = video_device_release_empty;
> - mdp->vdev.lock = &mdp->lock;
> - mdp->vdev.vfl_dir = VFL_DIR_M2M;
> - mdp->vdev.v4l2_dev = &mdp->v4l2_dev;
> - snprintf(mdp->vdev.name, sizeof(mdp->vdev.name), "%s:m2m",
> + mdp->vdev = video_device_alloc();
> + if (!mdp->vdev) {
> + dev_err(dev, "failed to allocate video device\n");
> + ret = -ENOMEM;
> + goto err_video_alloc;
> + }
> + mdp->vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> + mdp->vdev->fops = &mtk_mdp_m2m_fops;
> + mdp->vdev->ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
> + mdp->vdev->release = video_device_release;
> + mdp->vdev->lock = &mdp->lock;
> + mdp->vdev->vfl_dir = VFL_DIR_M2M;
> + mdp->vdev->v4l2_dev = &mdp->v4l2_dev;
> + snprintf(mdp->vdev->name, sizeof(mdp->vdev->name), "%s:m2m",
> MTK_MDP_MODULE_NAME);
> - video_set_drvdata(&mdp->vdev, mdp);
> + video_set_drvdata(mdp->vdev, mdp);
>
> mdp->m2m_dev = v4l2_m2m_init(&mtk_mdp_m2m_ops);
> if (IS_ERR(mdp->m2m_dev)) {
> @@ -1254,26 +1260,27 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
> goto err_m2m_init;
> }
>
> - ret = video_register_device(&mdp->vdev, VFL_TYPE_GRABBER, 2);
> + ret = video_register_device(mdp->vdev, VFL_TYPE_GRABBER, 2);
> if (ret) {
> dev_err(dev, "failed to register video device\n");
> goto err_vdev_register;
> }
>
> v4l2_info(&mdp->v4l2_dev, "driver registered as /dev/video%d",
> - mdp->vdev.num);
> + mdp->vdev->num);
> return 0;
>
> err_vdev_register:
> v4l2_m2m_release(mdp->m2m_dev);
> err_m2m_init:
> - video_device_release(&mdp->vdev);
> + video_unregister_device(mdp->vdev);

This should remain video_device_release: the video_register_device call failed, so
the device hasn't been registered. In that case just release the device (i.e.
free the allocated memory).

> +err_video_alloc:
>
> return ret;
> }
>
> void mtk_mdp_unregister_m2m_device(struct mtk_mdp_dev *mdp)
> {
> - video_device_release(&mdp->vdev);
> + video_unregister_device(mdp->vdev);
> v4l2_m2m_release(mdp->m2m_dev);
> }
>

Regards,

Hans

2016-11-15 01:37:42

by Minghsiu Tsai

[permalink] [raw]
Subject: Re: [PATCH] [media] mtk-mdp: allocate video_device dynamically

On Fri, 2016-11-11 at 11:51 +0100, Hans Verkuil wrote:
> Almost correct:
>
> On 11/07/2016 01:42 PM, Minghsiu Tsai wrote:
> > It can fix known problems with embedded video_device structs.
> >
> > Signed-off-by: Minghsiu Tsai <[email protected]>
> > ---
> > drivers/media/platform/mtk-mdp/mtk_mdp_core.h | 2 +-
> > drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 33 ++++++++++++++++-----------
> > 2 files changed, 21 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> > index 848569d..ad1cff3 100644
> > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> > @@ -167,7 +167,7 @@ struct mtk_mdp_dev {
> > struct mtk_mdp_comp *comp[MTK_MDP_COMP_ID_MAX];
> > struct v4l2_m2m_dev *m2m_dev;
> > struct list_head ctx_list;
> > - struct video_device vdev;
> > + struct video_device *vdev;
> > struct v4l2_device v4l2_dev;
> > struct workqueue_struct *job_wq;
> > struct platform_device *vpu_dev;
> > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> > index 9a747e7..b8dee1c 100644
> > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> > @@ -1236,16 +1236,22 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
> > int ret;
> >
> > mdp->variant = &mtk_mdp_default_variant;
> > - mdp->vdev.device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> > - mdp->vdev.fops = &mtk_mdp_m2m_fops;
> > - mdp->vdev.ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
> > - mdp->vdev.release = video_device_release_empty;
> > - mdp->vdev.lock = &mdp->lock;
> > - mdp->vdev.vfl_dir = VFL_DIR_M2M;
> > - mdp->vdev.v4l2_dev = &mdp->v4l2_dev;
> > - snprintf(mdp->vdev.name, sizeof(mdp->vdev.name), "%s:m2m",
> > + mdp->vdev = video_device_alloc();
> > + if (!mdp->vdev) {
> > + dev_err(dev, "failed to allocate video device\n");
> > + ret = -ENOMEM;
> > + goto err_video_alloc;
> > + }
> > + mdp->vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> > + mdp->vdev->fops = &mtk_mdp_m2m_fops;
> > + mdp->vdev->ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
> > + mdp->vdev->release = video_device_release;
> > + mdp->vdev->lock = &mdp->lock;
> > + mdp->vdev->vfl_dir = VFL_DIR_M2M;
> > + mdp->vdev->v4l2_dev = &mdp->v4l2_dev;
> > + snprintf(mdp->vdev->name, sizeof(mdp->vdev->name), "%s:m2m",
> > MTK_MDP_MODULE_NAME);
> > - video_set_drvdata(&mdp->vdev, mdp);
> > + video_set_drvdata(mdp->vdev, mdp);
> >
> > mdp->m2m_dev = v4l2_m2m_init(&mtk_mdp_m2m_ops);
> > if (IS_ERR(mdp->m2m_dev)) {
> > @@ -1254,26 +1260,27 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
> > goto err_m2m_init;
> > }
> >
> > - ret = video_register_device(&mdp->vdev, VFL_TYPE_GRABBER, 2);
> > + ret = video_register_device(mdp->vdev, VFL_TYPE_GRABBER, 2);
> > if (ret) {
> > dev_err(dev, "failed to register video device\n");
> > goto err_vdev_register;
> > }
> >
> > v4l2_info(&mdp->v4l2_dev, "driver registered as /dev/video%d",
> > - mdp->vdev.num);
> > + mdp->vdev->num);
> > return 0;
> >
> > err_vdev_register:
> > v4l2_m2m_release(mdp->m2m_dev);
> > err_m2m_init:
> > - video_device_release(&mdp->vdev);
> > + video_unregister_device(mdp->vdev);
>
> This should remain video_device_release: the video_register_device call failed, so
> the device hasn't been registered. In that case just release the device (i.e.
> free the allocated memory).
>
Hi Hans,

I have uploaded patch v2 for this. Thanks for your review.


Best regards,
Ming Hsiu

> > +err_video_alloc:
> >
> > return ret;
> > }
> >
> > void mtk_mdp_unregister_m2m_device(struct mtk_mdp_dev *mdp)
> > {
> > - video_device_release(&mdp->vdev);
> > + video_unregister_device(mdp->vdev);
> > v4l2_m2m_release(mdp->m2m_dev);
> > }
> >
>
> Regards,
>
> Hans