[PATCH] media: mtk-mdp: allocate video_device dynamically

[PATCH] media: mtk-mdp: allocate video_device dynamically

[Minghsiu Tsai]

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

[PATCH] [media] mtk-mdp: allocate video_device dynamically

[Minghsiu Tsai]

It can fix known problems with embedded video_device structs.

Signed-off-by: Minghsiu Tsai <minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hans Verkuil]

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 <minghsiu.tsai@mediatek.com>
> ---
>  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

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

[Minghsiu Tsai]

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 <minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  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


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html