LinuxTV Patchwork media: staging/imx: add media device to capture register

login
register
mail settings
Submitter Rui Miguel Silva
Date April 12, 2019, 4:44 p.m.
Message ID <20190412164400.1270-1-rui.silva@linaro.org>
Download mbox | patch
Permalink /patch/55657/
State New
Headers show

Comments

Rui Miguel Silva - April 12, 2019, 4:44 p.m.
When register the capture media device it is assumed that the device
data is the media device. In the imx6 case is but in the imx7 is not
case. The device data is the csi structure.

Add the explicit argument of the media device that we want to
associate with the capture device.

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 drivers/staging/media/imx/imx-ic-prpencvf.c   | 2 +-
 drivers/staging/media/imx/imx-media-capture.c | 6 +++---
 drivers/staging/media/imx/imx-media-csi.c     | 2 +-
 drivers/staging/media/imx/imx-media.h         | 3 ++-
 drivers/staging/media/imx/imx7-media-csi.c    | 2 +-
 5 files changed, 8 insertions(+), 7 deletions(-)
Laurent Pinchart - April 18, 2019, 9:57 p.m.
Hi Rui,

Thank you for the patch.

On Fri, Apr 12, 2019 at 05:44:00PM +0100, Rui Miguel Silva wrote:
> When register the capture media device it is assumed that the device
> data is the media device. In the imx6 case is but in the imx7 is not
> case. The device data is the csi structure.
> 
> Add the explicit argument of the media device that we want to
> associate with the capture device.

I've tested this, and the driver no longer deadlocks. I can't capture
images though, but that may well be due to my platform, the sensor
driver hasn't been validated yet.

Thank you for your help, I will report any progress. In any case there's
certainly room for improvement in the i.MX6/7 camera driver if we want
to get it out of staging, the way the multiple components are backed by
separate drivers that access each other's device data is a big hack at
best. If time permits, after getting capture working, I'll see if I can
start cleaning it up.

> Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
> ---
>  drivers/staging/media/imx/imx-ic-prpencvf.c   | 2 +-
>  drivers/staging/media/imx/imx-media-capture.c | 6 +++---
>  drivers/staging/media/imx/imx-media-csi.c     | 2 +-
>  drivers/staging/media/imx/imx-media.h         | 3 ++-
>  drivers/staging/media/imx/imx7-media-csi.c    | 2 +-
>  5 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index 5c8e6ad8c025..3ca1422f6154 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -1270,7 +1270,7 @@ static int prp_registered(struct v4l2_subdev *sd)
>  	if (ret)
>  		return ret;
>  
> -	ret = imx_media_capture_device_register(priv->vdev);
> +	ret = imx_media_capture_device_register(priv->md, priv->vdev);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> index 9703c85b19c4..7688238a3396 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -706,7 +706,8 @@ void imx_media_capture_device_error(struct imx_media_video_dev *vdev)
>  }
>  EXPORT_SYMBOL_GPL(imx_media_capture_device_error);
>  
> -int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
> +int imx_media_capture_device_register(struct imx_media_dev *md,
> +				      struct imx_media_video_dev *vdev)
>  {
>  	struct capture_priv *priv = to_capture_priv(vdev);
>  	struct v4l2_subdev *sd = priv->src_sd;
> @@ -715,8 +716,7 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
>  	struct v4l2_subdev_format fmt_src;
>  	int ret;
>  
> -	/* get media device */
> -	priv->md = dev_get_drvdata(sd->v4l2_dev->dev);
> +	priv->md = md;
>  
>  	vfd->v4l2_dev = sd->v4l2_dev;
>  
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 3b7517348666..3408ec023d29 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -1806,7 +1806,7 @@ static int csi_registered(struct v4l2_subdev *sd)
>  	if (ret)
>  		goto free_fim;
>  
> -	ret = imx_media_capture_device_register(priv->vdev);
> +	ret = imx_media_capture_device_register(priv->md, priv->vdev);
>  	if (ret)
>  		goto free_fim;
>  
> diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
> index ae964c8d5be1..c3a8512bd10f 100644
> --- a/drivers/staging/media/imx/imx-media.h
> +++ b/drivers/staging/media/imx/imx-media.h
> @@ -271,7 +271,8 @@ int imx_media_of_add_csi(struct imx_media_dev *imxmd,
>  struct imx_media_video_dev *
>  imx_media_capture_device_init(struct v4l2_subdev *src_sd, int pad);
>  void imx_media_capture_device_remove(struct imx_media_video_dev *vdev);
> -int imx_media_capture_device_register(struct imx_media_video_dev *vdev);
> +int imx_media_capture_device_register(struct imx_media_dev *md,
> +				       struct imx_media_video_dev *vdev);
>  void imx_media_capture_device_unregister(struct imx_media_video_dev *vdev);
>  struct imx_media_buffer *
>  imx_media_capture_device_next_buf(struct imx_media_video_dev *vdev);
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 3fba7c27c0ec..a907c5feb3eb 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -1124,7 +1124,7 @@ static int imx7_csi_registered(struct v4l2_subdev *sd)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = imx_media_capture_device_register(csi->vdev);
> +	ret = imx_media_capture_device_register(csi->imxmd, csi->vdev);
>  	if (ret < 0)
>  		return ret;
>
Steve Longerbeam - April 18, 2019, 10:13 p.m.
Acked-by: Steve Longerbeam <slongerbeam@gmail.com>

On 4/12/19 9:44 AM, Rui Miguel Silva wrote:
> When register the capture media device it is assumed that the device
> data is the media device. In the imx6 case is but in the imx7 is not
> case. The device data is the csi structure.
>
> Add the explicit argument of the media device that we want to
> associate with the capture device.
>
> Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
> ---
>   drivers/staging/media/imx/imx-ic-prpencvf.c   | 2 +-
>   drivers/staging/media/imx/imx-media-capture.c | 6 +++---
>   drivers/staging/media/imx/imx-media-csi.c     | 2 +-
>   drivers/staging/media/imx/imx-media.h         | 3 ++-
>   drivers/staging/media/imx/imx7-media-csi.c    | 2 +-
>   5 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index 5c8e6ad8c025..3ca1422f6154 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -1270,7 +1270,7 @@ static int prp_registered(struct v4l2_subdev *sd)
>   	if (ret)
>   		return ret;
>   
> -	ret = imx_media_capture_device_register(priv->vdev);
> +	ret = imx_media_capture_device_register(priv->md, priv->vdev);
>   	if (ret)
>   		return ret;
>   
> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> index 9703c85b19c4..7688238a3396 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -706,7 +706,8 @@ void imx_media_capture_device_error(struct imx_media_video_dev *vdev)
>   }
>   EXPORT_SYMBOL_GPL(imx_media_capture_device_error);
>   
> -int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
> +int imx_media_capture_device_register(struct imx_media_dev *md,
> +				      struct imx_media_video_dev *vdev)
>   {
>   	struct capture_priv *priv = to_capture_priv(vdev);
>   	struct v4l2_subdev *sd = priv->src_sd;
> @@ -715,8 +716,7 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
>   	struct v4l2_subdev_format fmt_src;
>   	int ret;
>   
> -	/* get media device */
> -	priv->md = dev_get_drvdata(sd->v4l2_dev->dev);
> +	priv->md = md;
>   
>   	vfd->v4l2_dev = sd->v4l2_dev;
>   
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index 3b7517348666..3408ec023d29 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -1806,7 +1806,7 @@ static int csi_registered(struct v4l2_subdev *sd)
>   	if (ret)
>   		goto free_fim;
>   
> -	ret = imx_media_capture_device_register(priv->vdev);
> +	ret = imx_media_capture_device_register(priv->md, priv->vdev);
>   	if (ret)
>   		goto free_fim;
>   
> diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
> index ae964c8d5be1..c3a8512bd10f 100644
> --- a/drivers/staging/media/imx/imx-media.h
> +++ b/drivers/staging/media/imx/imx-media.h
> @@ -271,7 +271,8 @@ int imx_media_of_add_csi(struct imx_media_dev *imxmd,
>   struct imx_media_video_dev *
>   imx_media_capture_device_init(struct v4l2_subdev *src_sd, int pad);
>   void imx_media_capture_device_remove(struct imx_media_video_dev *vdev);
> -int imx_media_capture_device_register(struct imx_media_video_dev *vdev);
> +int imx_media_capture_device_register(struct imx_media_dev *md,
> +				       struct imx_media_video_dev *vdev);
>   void imx_media_capture_device_unregister(struct imx_media_video_dev *vdev);
>   struct imx_media_buffer *
>   imx_media_capture_device_next_buf(struct imx_media_video_dev *vdev);
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 3fba7c27c0ec..a907c5feb3eb 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -1124,7 +1124,7 @@ static int imx7_csi_registered(struct v4l2_subdev *sd)
>   	if (ret < 0)
>   		return ret;
>   
> -	ret = imx_media_capture_device_register(csi->vdev);
> +	ret = imx_media_capture_device_register(csi->imxmd, csi->vdev);
>   	if (ret < 0)
>   		return ret;
>

Patch

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 5c8e6ad8c025..3ca1422f6154 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -1270,7 +1270,7 @@  static int prp_registered(struct v4l2_subdev *sd)
 	if (ret)
 		return ret;
 
-	ret = imx_media_capture_device_register(priv->vdev);
+	ret = imx_media_capture_device_register(priv->md, priv->vdev);
 	if (ret)
 		return ret;
 
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 9703c85b19c4..7688238a3396 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -706,7 +706,8 @@  void imx_media_capture_device_error(struct imx_media_video_dev *vdev)
 }
 EXPORT_SYMBOL_GPL(imx_media_capture_device_error);
 
-int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
+int imx_media_capture_device_register(struct imx_media_dev *md,
+				      struct imx_media_video_dev *vdev)
 {
 	struct capture_priv *priv = to_capture_priv(vdev);
 	struct v4l2_subdev *sd = priv->src_sd;
@@ -715,8 +716,7 @@  int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
 	struct v4l2_subdev_format fmt_src;
 	int ret;
 
-	/* get media device */
-	priv->md = dev_get_drvdata(sd->v4l2_dev->dev);
+	priv->md = md;
 
 	vfd->v4l2_dev = sd->v4l2_dev;
 
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 3b7517348666..3408ec023d29 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1806,7 +1806,7 @@  static int csi_registered(struct v4l2_subdev *sd)
 	if (ret)
 		goto free_fim;
 
-	ret = imx_media_capture_device_register(priv->vdev);
+	ret = imx_media_capture_device_register(priv->md, priv->vdev);
 	if (ret)
 		goto free_fim;
 
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index ae964c8d5be1..c3a8512bd10f 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -271,7 +271,8 @@  int imx_media_of_add_csi(struct imx_media_dev *imxmd,
 struct imx_media_video_dev *
 imx_media_capture_device_init(struct v4l2_subdev *src_sd, int pad);
 void imx_media_capture_device_remove(struct imx_media_video_dev *vdev);
-int imx_media_capture_device_register(struct imx_media_video_dev *vdev);
+int imx_media_capture_device_register(struct imx_media_dev *md,
+				       struct imx_media_video_dev *vdev);
 void imx_media_capture_device_unregister(struct imx_media_video_dev *vdev);
 struct imx_media_buffer *
 imx_media_capture_device_next_buf(struct imx_media_video_dev *vdev);
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 3fba7c27c0ec..a907c5feb3eb 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -1124,7 +1124,7 @@  static int imx7_csi_registered(struct v4l2_subdev *sd)
 	if (ret < 0)
 		return ret;
 
-	ret = imx_media_capture_device_register(csi->vdev);
+	ret = imx_media_capture_device_register(csi->imxmd, csi->vdev);
 	if (ret < 0)
 		return ret;
 

Privacy Policy