Re: [PATCH v2 1/8] media: staging/imx: Switch to sync registration for IPU subdevs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks Rui for catching the bisect problem, I will fix for v3.

Steve


On 4/30/19 2:56 PM, Rui Miguel Silva wrote:
Hi Steve,
On Sun 28 Apr 2019 at 20:09, Steve Longerbeam wrote:
Because the IPU sub-devices VDIC and IC are not present in the
device-tree, platform devices were created for them instead. This
allowed these sub-devices to be added to the media device's async
notifier and registered asynchronously along with the other
sub-devices that do have a device-tree presence (CSI and devices
external to the IPU and SoC).

But that approach isn't really necessary. The IPU sub-devices don't
actually require a backing device (sd->dev is allowed to be NULL).
And that approach can't get around the fact that the IPU sub-devices
are not part of a device hierarchy, which makes it awkward to retrieve
the parent IPU of these devices.

By registering them synchronously, they can be registered from the CSI
async bound notifier, so the init function for them can be given the CSI
subdev, who's dev->parent is the IPU. That is a somewhat cleaner way
to retrieve the parent IPU.

So convert to synchronous registration for the VDIC and IC task
sub-devices, at the time a CSI sub-device is bound. There is no longer
a backing device for them (sd->dev is NULL), but that's ok. Also
set the VDIC/IC sub-device owner as the IPU, so that a reference can
be taken on the IPU module.

Signed-off-by: Steve Longerbeam <slongerbeam@xxxxxxxxx>
I am trying to bisect when my capture starts to fail to work with
this series, since they are so many changes and reorg that I got
lost on some of them. But... see below.

---
  drivers/staging/media/imx/imx-ic-common.c     |  70 ++--
  drivers/staging/media/imx/imx-ic-prp.c        |  34 +-
  drivers/staging/media/imx/imx-ic-prpencvf.c   |  70 ++--
  drivers/staging/media/imx/imx-ic.h            |   7 +-
  drivers/staging/media/imx/imx-media-capture.c |   7 +-
  drivers/staging/media/imx/imx-media-csi.c     |   2 +-
  drivers/staging/media/imx/imx-media-dev.c     | 121 +-----
  .../staging/media/imx/imx-media-internal-sd.c | 356 ++++++++----------
  drivers/staging/media/imx/imx-media-of.c      |  38 +-
  drivers/staging/media/imx/imx-media-vdic.c    |  85 ++---
  drivers/staging/media/imx/imx-media.h         |  67 ++--
  drivers/staging/media/imx/imx7-media-csi.c    |   3 +-
  12 files changed, 325 insertions(+), 535 deletions(-)

+	dev_dbg(priv->ipu_dev, "%s: link setup %s -> %s",
<snip>

+		sd->name, remote->entity->name, local->entity->name);
mutex_lock(&priv->lock); @@ -864,9 +856,6 @@ static int vdic_registered(struct v4l2_subdev *sd)
  	int i, ret;
  	u32 code;
- /* get media device */
-	priv->md = dev_get_drvdata(sd->v4l2_dev->dev);
-
  	for (i = 0; i < VDIC_NUM_PADS; i++) {
  		priv->pad[i].flags = (i == VDIC_SRC_PAD_DIRECT) ?
  			MEDIA_PAD_FL_SOURCE : MEDIA_PAD_FL_SINK;
@@ -938,77 +927,55 @@ static const struct v4l2_subdev_internal_ops vdic_internal_ops = {
  	.unregistered = vdic_unregistered,
  };
-static int imx_vdic_probe(struct platform_device *pdev)
+struct v4l2_subdev *imx_media_vdic_register(struct imx_media_dev *imxmd,
+					    struct device *ipu_dev,
+					    struct ipu_soc *ipu,
+					    u32 grp_id)
  {
-	struct imx_media_ipu_internal_sd_pdata *pdata;
+	struct v4l2_device *v4l2_dev = &imxmd->v4l2_dev;
  	struct vdic_priv *priv;
  	int ret;
- priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(ipu_dev, sizeof(*priv), GFP_KERNEL);
  	if (!priv)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
- platform_set_drvdata(pdev, &priv->sd);
-	priv->dev = &pdev->dev;
-
-	pdata = priv->dev->platform_data;
-	priv->ipu_id = pdata->ipu_id;
+	priv->ipu_dev = ipu_dev;
+	priv->ipu = ipu;
+	priv->md = imxmd;
v4l2_subdev_init(&priv->sd, &vdic_subdev_ops);
  	v4l2_set_subdevdata(&priv->sd, priv);
  	priv->sd.internal_ops = &vdic_internal_ops;
  	priv->sd.entity.ops = &vdic_entity_ops;
  	priv->sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
-	priv->sd.dev = &pdev->dev;
-	priv->sd.owner = THIS_MODULE;
+	priv->sd.owner = ipu_dev->driver->owner;
  	priv->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
-	/* get our group id */
-	priv->sd.grp_id = pdata->grp_id;
-	strscpy(priv->sd.name, pdata->sd_name, sizeof(priv->sd.name));
+	priv->sd.grp_id = grp_id;
+	imx_media_grp_id_to_sd_name(priv->sd.name, sizeof(priv->sd.name),
+				    priv->sd.grp_id, ipu_get_num(ipu));
mutex_init(&priv->lock); - ret = v4l2_async_register_subdev(&priv->sd);
+	ret = v4l2_device_register_subdev(v4l2_dev, &priv->sd);
  	if (ret)
  		goto free;
- return 0;
+	return &priv->sd;
  free:
  	mutex_destroy(&priv->lock);
-	return ret;
+	return ERR_PTR(ret);
  }
-static int imx_vdic_remove(struct platform_device *pdev)
+int imx_media_vdic_unregister(struct v4l2_subdev *sd)
  {
-	struct v4l2_subdev *sd = platform_get_drvdata(pdev);
  	struct vdic_priv *priv = v4l2_get_subdevdata(sd);
v4l2_info(sd, "Removing\n"); - v4l2_async_unregister_subdev(sd);
+	v4l2_device_unregister_subdev(sd);
  	mutex_destroy(&priv->lock);
  	media_entity_cleanup(&sd->entity);
return 0;
  }
-
-static const struct platform_device_id imx_vdic_ids[] = {
-	{ .name = "imx-ipuv3-vdic" },
-	{ },
-};
-MODULE_DEVICE_TABLE(platform, imx_vdic_ids);
-
-static struct platform_driver imx_vdic_driver = {
-	.probe = imx_vdic_probe,
-	.remove = imx_vdic_remove,
-	.id_table = imx_vdic_ids,
-	.driver = {
-		.name = "imx-ipuv3-vdic",
-	},
-};
-module_platform_driver(imx_vdic_driver);
-
-MODULE_DESCRIPTION("i.MX VDIC subdev driver");
-MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@xxxxxxxxxx>");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:imx-ipuv3-vdic");
This breaks compilation until patch 6/8. I think you need to make
some makefile changes also in this patch.

WARNING: modpost: missing MODULE_LICENSE() in drivers/staging/media/imx/imx-media-ic.o
see include/linux/module.h for more information
WARNING: modpost: missing MODULE_LICENSE() in drivers/staging/media/imx/imx-media-vdic.o
see include/linux/module.h for more information
ERROR: "imx_media_ic_register" [drivers/staging/media/imx/imx-media.ko] undefined!
ERROR: "imx_media_vdic_register" [drivers/staging/media/imx/imx-media.ko] undefined!
ERROR: "imx_media_vdic_unregister" [drivers/staging/media/imx/imx-media.ko] undefined!
ERROR: "imx_media_ic_unregister" [drivers/staging/media/imx/imx-media.ko] undefined!
make[2]: *** [scripts/Makefile.modpost:91: __modpost] Error 1
make[1]: *** [/dev/shm/linux-git/Makefile:1263: modules] Error 2
make: *** [Makefile:170: sub-make] Error 2

---
Cheers,
	Rui


_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux