linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/8] Switch to sync registration for IPU subdevs
@ 2019-04-30 22:50 Steve Longerbeam
  2019-04-30 22:50 ` [PATCH v3 1/8] media: staging/imx: " Steve Longerbeam
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Steve Longerbeam @ 2019-04-30 22:50 UTC (permalink / raw)
  To: linux-media; +Cc: Steve Longerbeam

Switch to sync registration for the IPU internal sub-devices, re-organize
modules, and a few other miscellaneous cleanups.

History:
v3:
- A couple patches did not compile/link. All patches now build so the
  series is fully bisectable. No functional changes.
v2:
- Added a patch that improves the pipeline upstream/downstream search
  functions, which no longer require the media device.
- Add a patch to remove getting media device from v4l2_dev driver data.

Steve Longerbeam (8):
  media: staging/imx: Switch to sync registration for IPU subdevs
  media: staging/imx: Pass device to alloc/free_dma_buf
  media: staging/imx: Move add_video_device into capture_device_register
  Revert "media: imx: Set capture compose rectangle in
    capture_device_set_format"
  media: staging/imx: Remove capture_device_set_format
  media: staging/imx: Re-organize modules
  media: staging/imx: Improve pipeline searching
  media: staging/imx: Don't set driver data for v4l2_dev

 drivers/staging/media/imx/Makefile            |  18 +-
 drivers/staging/media/imx/imx-ic-common.c     |  68 +--
 drivers/staging/media/imx/imx-ic-prp.c        |  36 +-
 drivers/staging/media/imx/imx-ic-prpencvf.c   |  88 ++--
 drivers/staging/media/imx/imx-ic.h            |   6 +-
 drivers/staging/media/imx/imx-media-capture.c |  75 ++-
 drivers/staging/media/imx/imx-media-csi.c     |  45 +-
 .../staging/media/imx/imx-media-dev-common.c  | 346 +++++++++++++-
 drivers/staging/media/imx/imx-media-dev.c     | 449 +-----------------
 drivers/staging/media/imx/imx-media-fim.c     |   9 -
 .../staging/media/imx/imx-media-internal-sd.c | 357 ++++++--------
 drivers/staging/media/imx/imx-media-of.c      |  41 +-
 drivers/staging/media/imx/imx-media-utils.c   | 170 +++----
 drivers/staging/media/imx/imx-media-vdic.c    |  84 +---
 drivers/staging/media/imx/imx-media.h         | 113 +++--
 drivers/staging/media/imx/imx7-media-csi.c    |  43 +-
 16 files changed, 848 insertions(+), 1100 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/8] media: staging/imx: Switch to sync registration for IPU subdevs
  2019-04-30 22:50 [PATCH v3 0/8] Switch to sync registration for IPU subdevs Steve Longerbeam
@ 2019-04-30 22:50 ` Steve Longerbeam
  2019-04-30 22:50 ` [PATCH v3 2/8] media: staging/imx: Pass device to alloc/free_dma_buf Steve Longerbeam
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Steve Longerbeam @ 2019-04-30 22:50 UTC (permalink / raw)
  To: linux-media
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Rui Miguel Silva, open list:STAGING SUBSYSTEM,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list

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.

Since the VDIC and IC task drivers are no longer platform drivers,
they are now statically linked to imx-media module.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
Changes in v3:
- statically link VDIC and IC task objects to imx-media module in
  Makefile.
---
 drivers/staging/media/imx/Makefile            |   6 +-
 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 +-
 13 files changed, 327 insertions(+), 539 deletions(-)

diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
index d2d909a36239..86f0c81b6a3b 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -1,14 +1,12 @@
 # SPDX-License-Identifier: GPL-2.0
-imx-media-objs := imx-media-dev.o imx-media-internal-sd.o imx-media-of.o
+imx-media-objs := imx-media-dev.o imx-media-internal-sd.o imx-media-of.o \
+	imx-ic-common.o imx-ic-prp.o imx-ic-prpencvf.o imx-media-vdic.o
 imx-media-objs += imx-media-dev-common.o
 imx-media-common-objs := imx-media-utils.o imx-media-fim.o
-imx-media-ic-objs := imx-ic-common.o imx-ic-prp.o imx-ic-prpencvf.o
 
 obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media.o
 obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-common.o
 obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-capture.o
-obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-vdic.o
-obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-ic.o
 
 obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o
 obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
diff --git a/drivers/staging/media/imx/imx-ic-common.c b/drivers/staging/media/imx/imx-ic-common.c
index 1addb0893c57..ad0c291db03c 100644
--- a/drivers/staging/media/imx/imx-ic-common.c
+++ b/drivers/staging/media/imx/imx-ic-common.c
@@ -8,8 +8,6 @@
  * the Free Software Foundation; either version 2 of the License, or
  * (at your option) any later version.
  */
-#include <linux/module.h>
-#include <linux/platform_device.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-subdev.h>
 #include "imx-media.h"
@@ -24,23 +22,25 @@ static struct imx_ic_ops *ic_ops[IC_NUM_OPS] = {
 	[IC_TASK_VIEWFINDER]     = &imx_ic_prpencvf_ops,
 };
 
-static int imx_ic_probe(struct platform_device *pdev)
+struct v4l2_subdev *imx_media_ic_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 imx_ic_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;
+	priv->ipu_dev = ipu_dev;
+	priv->ipu = ipu;
+	priv->md = imxmd;
 
-	/* get our ipu_id, grp_id and IC task id */
-	pdata = priv->dev->platform_data;
-	priv->ipu_id = pdata->ipu_id;
-	switch (pdata->grp_id) {
+	/* get our IC task id */
+	switch (grp_id) {
 	case IMX_MEDIA_GRP_ID_IPU_IC_PRP:
 		priv->task_id = IC_TASK_PRP;
 		break;
@@ -51,7 +51,7 @@ static int imx_ic_probe(struct platform_device *pdev)
 		priv->task_id = IC_TASK_VIEWFINDER;
 		break;
 	default:
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	v4l2_subdev_init(&priv->sd, ic_ops[priv->task_id]->subdev_ops);
@@ -59,55 +59,35 @@ static int imx_ic_probe(struct platform_device *pdev)
 	priv->sd.internal_ops = ic_ops[priv->task_id]->internal_ops;
 	priv->sd.entity.ops = ic_ops[priv->task_id]->entity_ops;
 	priv->sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER;
-	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 | V4L2_SUBDEV_FL_HAS_EVENTS;
-	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));
 
 	ret = ic_ops[priv->task_id]->init(priv);
 	if (ret)
-		return ret;
+		return ERR_PTR(ret);
 
-	ret = v4l2_async_register_subdev(&priv->sd);
-	if (ret)
+	ret = v4l2_device_register_subdev(v4l2_dev, &priv->sd);
+	if (ret) {
 		ic_ops[priv->task_id]->remove(priv);
+		return ERR_PTR(ret);
+	}
 
-	return ret;
+	return &priv->sd;
 }
 
-static int imx_ic_remove(struct platform_device *pdev)
+int imx_media_ic_unregister(struct v4l2_subdev *sd)
 {
-	struct v4l2_subdev *sd = platform_get_drvdata(pdev);
 	struct imx_ic_priv *priv = container_of(sd, struct imx_ic_priv, sd);
 
 	v4l2_info(sd, "Removing\n");
 
 	ic_ops[priv->task_id]->remove(priv);
 
-	v4l2_async_unregister_subdev(sd);
+	v4l2_device_unregister_subdev(sd);
 	media_entity_cleanup(&sd->entity);
 
 	return 0;
 }
-
-static const struct platform_device_id imx_ic_ids[] = {
-	{ .name = "imx-ipuv3-ic" },
-	{ },
-};
-MODULE_DEVICE_TABLE(platform, imx_ic_ids);
-
-static struct platform_driver imx_ic_driver = {
-	.probe = imx_ic_probe,
-	.remove = imx_ic_remove,
-	.id_table = imx_ic_ids,
-	.driver = {
-		.name = "imx-ipuv3-ic",
-	},
-};
-module_platform_driver(imx_ic_driver);
-
-MODULE_DESCRIPTION("i.MX IC subdev driver");
-MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@mentor.com>");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:imx-ipuv3-ic");
diff --git a/drivers/staging/media/imx/imx-ic-prp.c b/drivers/staging/media/imx/imx-ic-prp.c
index 3d43cdcb4bb9..663db200e594 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -39,16 +39,12 @@
 #define S_ALIGN    1 /* multiple of 2 */
 
 struct prp_priv {
-	struct imx_media_dev *md;
 	struct imx_ic_priv *ic_priv;
 	struct media_pad pad[PRP_NUM_PADS];
 
 	/* lock to protect all members below */
 	struct mutex lock;
 
-	/* IPU units we require */
-	struct ipu_soc *ipu;
-
 	struct v4l2_subdev *src_sd;
 	struct v4l2_subdev *sink_sd_prpenc;
 	struct v4l2_subdev *sink_sd_prpvf;
@@ -66,7 +62,7 @@ static inline struct prp_priv *sd_to_priv(struct v4l2_subdev *sd)
 {
 	struct imx_ic_priv *ic_priv = v4l2_get_subdevdata(sd);
 
-	return ic_priv->prp_priv;
+	return ic_priv->task_priv;
 }
 
 static int prp_start(struct prp_priv *priv)
@@ -74,12 +70,10 @@ static int prp_start(struct prp_priv *priv)
 	struct imx_ic_priv *ic_priv = priv->ic_priv;
 	bool src_is_vdic;
 
-	priv->ipu = priv->md->ipu[ic_priv->ipu_id];
-
 	/* set IC to receive from CSI or VDI depending on source */
 	src_is_vdic = !!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_IPU_VDIC);
 
-	ipu_set_ic_src_mux(priv->ipu, priv->csi_id, src_is_vdic);
+	ipu_set_ic_src_mux(ic_priv->ipu, priv->csi_id, src_is_vdic);
 
 	return 0;
 }
@@ -220,12 +214,12 @@ static int prp_link_setup(struct media_entity *entity,
 {
 	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
 	struct imx_ic_priv *ic_priv = v4l2_get_subdevdata(sd);
-	struct prp_priv *priv = ic_priv->prp_priv;
+	struct prp_priv *priv = ic_priv->task_priv;
 	struct v4l2_subdev *remote_sd;
 	int ret = 0;
 
-	dev_dbg(ic_priv->dev, "link setup %s -> %s", remote->entity->name,
-		local->entity->name);
+	dev_dbg(ic_priv->ipu_dev, "%s: link setup %s -> %s",
+		ic_priv->sd.name, remote->entity->name, local->entity->name);
 
 	remote_sd = media_entity_to_v4l2_subdev(remote->entity);
 
@@ -299,7 +293,7 @@ static int prp_link_validate(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_format *sink_fmt)
 {
 	struct imx_ic_priv *ic_priv = v4l2_get_subdevdata(sd);
-	struct prp_priv *priv = ic_priv->prp_priv;
+	struct prp_priv *priv = ic_priv->task_priv;
 	struct v4l2_subdev *csi;
 	int ret;
 
@@ -308,7 +302,7 @@ static int prp_link_validate(struct v4l2_subdev *sd,
 	if (ret)
 		return ret;
 
-	csi = imx_media_find_upstream_subdev(priv->md, &ic_priv->sd.entity,
+	csi = imx_media_find_upstream_subdev(ic_priv->md, &ic_priv->sd.entity,
 					     IMX_MEDIA_GRP_ID_IPU_CSI);
 	if (IS_ERR(csi))
 		csi = NULL;
@@ -355,7 +349,7 @@ static int prp_link_validate(struct v4l2_subdev *sd,
 static int prp_s_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct imx_ic_priv *ic_priv = v4l2_get_subdevdata(sd);
-	struct prp_priv *priv = ic_priv->prp_priv;
+	struct prp_priv *priv = ic_priv->task_priv;
 	int ret = 0;
 
 	mutex_lock(&priv->lock);
@@ -372,7 +366,8 @@ static int prp_s_stream(struct v4l2_subdev *sd, int enable)
 	if (priv->stream_count != !enable)
 		goto update_count;
 
-	dev_dbg(ic_priv->dev, "stream %s\n", enable ? "ON" : "OFF");
+	dev_dbg(ic_priv->ipu_dev, "%s: stream %s\n", sd->name,
+		enable ? "ON" : "OFF");
 
 	if (enable)
 		ret = prp_start(priv);
@@ -444,9 +439,6 @@ static int prp_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 < PRP_NUM_PADS; i++) {
 		priv->pad[i].flags = (i == PRP_SINK_PAD) ?
 			MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
@@ -498,12 +490,12 @@ static int prp_init(struct imx_ic_priv *ic_priv)
 {
 	struct prp_priv *priv;
 
-	priv = devm_kzalloc(ic_priv->dev, sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(ic_priv->ipu_dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
 	mutex_init(&priv->lock);
-	ic_priv->prp_priv = priv;
+	ic_priv->task_priv = priv;
 	priv->ic_priv = ic_priv;
 
 	return 0;
@@ -511,7 +503,7 @@ static int prp_init(struct imx_ic_priv *ic_priv)
 
 static void prp_remove(struct imx_ic_priv *ic_priv)
 {
-	struct prp_priv *priv = ic_priv->prp_priv;
+	struct prp_priv *priv = ic_priv->task_priv;
 
 	mutex_destroy(&priv->lock);
 }
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 5c8e6ad8c025..069cce512280 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -54,7 +54,6 @@
 #define S_ALIGN       1 /* multiple of 2 */
 
 struct prp_priv {
-	struct imx_media_dev *md;
 	struct imx_ic_priv *ic_priv;
 	struct media_pad pad[PRPENCVF_NUM_PADS];
 	/* the video device at output pad */
@@ -64,7 +63,6 @@ struct prp_priv {
 	struct mutex lock;
 
 	/* IPU units we require */
-	struct ipu_soc *ipu;
 	struct ipu_ic *ic;
 	struct ipuv3_channel *out_ch;
 	struct ipuv3_channel *rot_in_ch;
@@ -160,9 +158,7 @@ static int prp_get_ipu_resources(struct prp_priv *priv)
 	struct ipuv3_channel *out_ch, *rot_in_ch, *rot_out_ch;
 	int ret, task = ic_priv->task_id;
 
-	priv->ipu = priv->md->ipu[ic_priv->ipu_id];
-
-	ic = ipu_ic_get(priv->ipu, task);
+	ic = ipu_ic_get(ic_priv->ipu, task);
 	if (IS_ERR(ic)) {
 		v4l2_err(&ic_priv->sd, "failed to get IC\n");
 		ret = PTR_ERR(ic);
@@ -170,7 +166,7 @@ static int prp_get_ipu_resources(struct prp_priv *priv)
 	}
 	priv->ic = ic;
 
-	out_ch = ipu_idmac_get(priv->ipu, prp_channel[task].out_ch);
+	out_ch = ipu_idmac_get(ic_priv->ipu, prp_channel[task].out_ch);
 	if (IS_ERR(out_ch)) {
 		v4l2_err(&ic_priv->sd, "could not get IDMAC channel %u\n",
 			 prp_channel[task].out_ch);
@@ -179,7 +175,7 @@ static int prp_get_ipu_resources(struct prp_priv *priv)
 	}
 	priv->out_ch = out_ch;
 
-	rot_in_ch = ipu_idmac_get(priv->ipu, prp_channel[task].rot_in_ch);
+	rot_in_ch = ipu_idmac_get(ic_priv->ipu, prp_channel[task].rot_in_ch);
 	if (IS_ERR(rot_in_ch)) {
 		v4l2_err(&ic_priv->sd, "could not get IDMAC channel %u\n",
 			 prp_channel[task].rot_in_ch);
@@ -188,7 +184,7 @@ static int prp_get_ipu_resources(struct prp_priv *priv)
 	}
 	priv->rot_in_ch = rot_in_ch;
 
-	rot_out_ch = ipu_idmac_get(priv->ipu, prp_channel[task].rot_out_ch);
+	rot_out_ch = ipu_idmac_get(ic_priv->ipu, prp_channel[task].rot_out_ch);
 	if (IS_ERR(rot_out_ch)) {
 		v4l2_err(&ic_priv->sd, "could not get IDMAC channel %u\n",
 			 prp_channel[task].rot_out_ch);
@@ -468,13 +464,13 @@ static int prp_setup_rotation(struct prp_priv *priv)
 	incc = priv->cc[PRPENCVF_SINK_PAD];
 	outcc = vdev->cc;
 
-	ret = imx_media_alloc_dma_buf(priv->md, &priv->rot_buf[0],
+	ret = imx_media_alloc_dma_buf(ic_priv->md, &priv->rot_buf[0],
 				      outfmt->sizeimage);
 	if (ret) {
 		v4l2_err(&ic_priv->sd, "failed to alloc rot_buf[0], %d\n", ret);
 		return ret;
 	}
-	ret = imx_media_alloc_dma_buf(priv->md, &priv->rot_buf[1],
+	ret = imx_media_alloc_dma_buf(ic_priv->md, &priv->rot_buf[1],
 				      outfmt->sizeimage);
 	if (ret) {
 		v4l2_err(&ic_priv->sd, "failed to alloc rot_buf[1], %d\n", ret);
@@ -547,14 +543,16 @@ static int prp_setup_rotation(struct prp_priv *priv)
 unsetup_vb2:
 	prp_unsetup_vb2_buf(priv, VB2_BUF_STATE_QUEUED);
 free_rot1:
-	imx_media_free_dma_buf(priv->md, &priv->rot_buf[1]);
+	imx_media_free_dma_buf(ic_priv->md, &priv->rot_buf[1]);
 free_rot0:
-	imx_media_free_dma_buf(priv->md, &priv->rot_buf[0]);
+	imx_media_free_dma_buf(ic_priv->md, &priv->rot_buf[0]);
 	return ret;
 }
 
 static void prp_unsetup_rotation(struct prp_priv *priv)
 {
+	struct imx_ic_priv *ic_priv = priv->ic_priv;
+
 	ipu_ic_task_disable(priv->ic);
 
 	ipu_idmac_disable_channel(priv->out_ch);
@@ -565,8 +563,8 @@ static void prp_unsetup_rotation(struct prp_priv *priv)
 
 	ipu_ic_disable(priv->ic);
 
-	imx_media_free_dma_buf(priv->md, &priv->rot_buf[0]);
-	imx_media_free_dma_buf(priv->md, &priv->rot_buf[1]);
+	imx_media_free_dma_buf(ic_priv->md, &priv->rot_buf[0]);
+	imx_media_free_dma_buf(ic_priv->md, &priv->rot_buf[1]);
 }
 
 static int prp_setup_norotation(struct prp_priv *priv)
@@ -606,7 +604,7 @@ static int prp_setup_norotation(struct prp_priv *priv)
 
 	ipu_cpmem_dump(priv->out_ch);
 	ipu_ic_dump(priv->ic);
-	ipu_dump(priv->ipu);
+	ipu_dump(ic_priv->ipu);
 
 	ipu_ic_enable(priv->ic);
 
@@ -658,7 +656,7 @@ static int prp_start(struct prp_priv *priv)
 
 	outfmt = &vdev->fmt.fmt.pix;
 
-	ret = imx_media_alloc_dma_buf(priv->md, &priv->underrun_buf,
+	ret = imx_media_alloc_dma_buf(ic_priv->md, &priv->underrun_buf,
 				      outfmt->sizeimage);
 	if (ret)
 		goto out_put_ipu;
@@ -678,10 +676,10 @@ static int prp_start(struct prp_priv *priv)
 	if (ret)
 		goto out_free_underrun;
 
-	priv->nfb4eof_irq = ipu_idmac_channel_irq(priv->ipu,
+	priv->nfb4eof_irq = ipu_idmac_channel_irq(ic_priv->ipu,
 						  priv->out_ch,
 						  IPU_IRQ_NFB4EOF);
-	ret = devm_request_irq(ic_priv->dev, priv->nfb4eof_irq,
+	ret = devm_request_irq(ic_priv->ipu_dev, priv->nfb4eof_irq,
 			       prp_nfb4eof_interrupt, 0,
 			       "imx-ic-prp-nfb4eof", priv);
 	if (ret) {
@@ -692,12 +690,12 @@ static int prp_start(struct prp_priv *priv)
 
 	if (ipu_rot_mode_is_irt(priv->rot_mode))
 		priv->eof_irq = ipu_idmac_channel_irq(
-			priv->ipu, priv->rot_out_ch, IPU_IRQ_EOF);
+			ic_priv->ipu, priv->rot_out_ch, IPU_IRQ_EOF);
 	else
 		priv->eof_irq = ipu_idmac_channel_irq(
-			priv->ipu, priv->out_ch, IPU_IRQ_EOF);
+			ic_priv->ipu, priv->out_ch, IPU_IRQ_EOF);
 
-	ret = devm_request_irq(ic_priv->dev, priv->eof_irq,
+	ret = devm_request_irq(ic_priv->ipu_dev, priv->eof_irq,
 			       prp_eof_interrupt, 0,
 			       "imx-ic-prp-eof", priv);
 	if (ret) {
@@ -722,13 +720,13 @@ static int prp_start(struct prp_priv *priv)
 	return 0;
 
 out_free_eof_irq:
-	devm_free_irq(ic_priv->dev, priv->eof_irq, priv);
+	devm_free_irq(ic_priv->ipu_dev, priv->eof_irq, priv);
 out_free_nfb4eof_irq:
-	devm_free_irq(ic_priv->dev, priv->nfb4eof_irq, priv);
+	devm_free_irq(ic_priv->ipu_dev, priv->nfb4eof_irq, priv);
 out_unsetup:
 	prp_unsetup(priv, VB2_BUF_STATE_QUEUED);
 out_free_underrun:
-	imx_media_free_dma_buf(priv->md, &priv->underrun_buf);
+	imx_media_free_dma_buf(ic_priv->md, &priv->underrun_buf);
 out_put_ipu:
 	prp_put_ipu_resources(priv);
 	return ret;
@@ -760,12 +758,12 @@ static void prp_stop(struct prp_priv *priv)
 		v4l2_warn(&ic_priv->sd,
 			  "upstream stream off failed: %d\n", ret);
 
-	devm_free_irq(ic_priv->dev, priv->eof_irq, priv);
-	devm_free_irq(ic_priv->dev, priv->nfb4eof_irq, priv);
+	devm_free_irq(ic_priv->ipu_dev, priv->eof_irq, priv);
+	devm_free_irq(ic_priv->ipu_dev, priv->nfb4eof_irq, priv);
 
 	prp_unsetup(priv, VB2_BUF_STATE_ERROR);
 
-	imx_media_free_dma_buf(priv->md, &priv->underrun_buf);
+	imx_media_free_dma_buf(ic_priv->md, &priv->underrun_buf);
 
 	/* cancel the EOF timeout timer */
 	del_timer_sync(&priv->eof_timeout_timer);
@@ -1015,8 +1013,8 @@ static int prp_link_setup(struct media_entity *entity,
 	struct v4l2_subdev *remote_sd;
 	int ret = 0;
 
-	dev_dbg(ic_priv->dev, "link setup %s -> %s", remote->entity->name,
-		local->entity->name);
+	dev_dbg(ic_priv->ipu_dev, "%s: link setup %s -> %s",
+		ic_priv->sd.name, remote->entity->name, local->entity->name);
 
 	mutex_lock(&priv->lock);
 
@@ -1182,7 +1180,8 @@ static int prp_s_stream(struct v4l2_subdev *sd, int enable)
 	if (priv->stream_count != !enable)
 		goto update_count;
 
-	dev_dbg(ic_priv->dev, "stream %s\n", enable ? "ON" : "OFF");
+	dev_dbg(ic_priv->ipu_dev, "%s: stream %s\n", sd->name,
+		enable ? "ON" : "OFF");
 
 	if (enable)
 		ret = prp_start(priv);
@@ -1242,12 +1241,10 @@ static int prp_s_frame_interval(struct v4l2_subdev *sd,
 static int prp_registered(struct v4l2_subdev *sd)
 {
 	struct prp_priv *priv = sd_to_priv(sd);
+	struct imx_ic_priv *ic_priv = priv->ic_priv;
 	int i, ret;
 	u32 code;
 
-	/* get media device */
-	priv->md = dev_get_drvdata(sd->v4l2_dev->dev);
-
 	for (i = 0; i < PRPENCVF_NUM_PADS; i++) {
 		priv->pad[i].flags = (i == PRPENCVF_SINK_PAD) ?
 			MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
@@ -1274,7 +1271,7 @@ static int prp_registered(struct v4l2_subdev *sd)
 	if (ret)
 		return ret;
 
-	ret = imx_media_add_video_device(priv->md, priv->vdev);
+	ret = imx_media_add_video_device(ic_priv->md, priv->vdev);
 	if (ret)
 		goto unreg;
 
@@ -1329,7 +1326,7 @@ static int prp_init(struct imx_ic_priv *ic_priv)
 {
 	struct prp_priv *priv;
 
-	priv = devm_kzalloc(ic_priv->dev, sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(ic_priv->ipu_dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
@@ -1339,7 +1336,8 @@ static int prp_init(struct imx_ic_priv *ic_priv)
 	spin_lock_init(&priv->irqlock);
 	timer_setup(&priv->eof_timeout_timer, prp_eof_timeout, 0);
 
-	priv->vdev = imx_media_capture_device_init(&ic_priv->sd,
+	priv->vdev = imx_media_capture_device_init(ic_priv->ipu_dev,
+						   &ic_priv->sd,
 						   PRPENCVF_SRC_PAD);
 	if (IS_ERR(priv->vdev))
 		return PTR_ERR(priv->vdev);
diff --git a/drivers/staging/media/imx/imx-ic.h b/drivers/staging/media/imx/imx-ic.h
index 6b2267bda8ab..1dcbb37aeada 100644
--- a/drivers/staging/media/imx/imx-ic.h
+++ b/drivers/staging/media/imx/imx-ic.h
@@ -14,11 +14,11 @@
 #include <media/v4l2-subdev.h>
 
 struct imx_ic_priv {
-	struct device *dev;
+	struct device *ipu_dev;
+	struct ipu_soc *ipu;
+	struct imx_media_dev *md;
 	struct v4l2_subdev sd;
-	int    ipu_id;
 	int    task_id;
-	void   *prp_priv;
 	void   *task_priv;
 };
 
@@ -33,6 +33,5 @@ struct imx_ic_ops {
 
 extern struct imx_ic_ops imx_ic_prp_ops;
 extern struct imx_ic_ops imx_ic_prpencvf_ops;
-extern struct imx_ic_ops imx_ic_pp_ops;
 
 #endif
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 9703c85b19c4..211ec4df2066 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -804,18 +804,19 @@ void imx_media_capture_device_unregister(struct imx_media_video_dev *vdev)
 EXPORT_SYMBOL_GPL(imx_media_capture_device_unregister);
 
 struct imx_media_video_dev *
-imx_media_capture_device_init(struct v4l2_subdev *src_sd, int pad)
+imx_media_capture_device_init(struct device *dev, struct v4l2_subdev *src_sd,
+			      int pad)
 {
 	struct capture_priv *priv;
 	struct video_device *vfd;
 
-	priv = devm_kzalloc(src_sd->dev, sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return ERR_PTR(-ENOMEM);
 
 	priv->src_sd = src_sd;
 	priv->src_sd_pad = pad;
-	priv->dev = src_sd->dev;
+	priv->dev = dev;
 
 	mutex_init(&priv->mutex);
 	spin_lock_init(&priv->q_lock);
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 41965d8b56c4..93b107eab5f5 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1987,7 +1987,7 @@ static int imx_csi_probe(struct platform_device *pdev)
 	imx_media_grp_id_to_sd_name(priv->sd.name, sizeof(priv->sd.name),
 				    priv->sd.grp_id, ipu_get_num(priv->ipu));
 
-	priv->vdev = imx_media_capture_device_init(&priv->sd,
+	priv->vdev = imx_media_capture_device_init(priv->sd.dev, &priv->sd,
 						   CSI_SRC_PAD_IDMAC);
 	if (IS_ERR(priv->vdev))
 		return PTR_ERR(priv->vdev);
diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
index 0a7d1d183141..55181756a940 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -32,111 +32,24 @@ static inline struct imx_media_dev *notifier2dev(struct v4l2_async_notifier *n)
 	return container_of(n, struct imx_media_dev, notifier);
 }
 
-/*
- * Adds a subdev to the root notifier's async subdev list. If fwnode is
- * non-NULL, adds the async as a V4L2_ASYNC_MATCH_FWNODE match type,
- * otherwise as a V4L2_ASYNC_MATCH_DEVNAME match type using the dev_name
- * of the given platform_device. This is called during driver load when
- * forming the async subdev list.
- */
-int imx_media_add_async_subdev(struct imx_media_dev *imxmd,
-			       struct fwnode_handle *fwnode,
-			       struct platform_device *pdev)
-{
-	struct device_node *np = to_of_node(fwnode);
-	struct imx_media_async_subdev *imxasd;
-	struct v4l2_async_subdev *asd;
-	const char *devname = NULL;
-	int ret;
-
-	if (fwnode) {
-		asd = v4l2_async_notifier_add_fwnode_subdev(&imxmd->notifier,
-							    fwnode,
-							    sizeof(*imxasd));
-	} else {
-		devname = dev_name(&pdev->dev);
-		asd = v4l2_async_notifier_add_devname_subdev(&imxmd->notifier,
-							     devname,
-							     sizeof(*imxasd));
-	}
-
-	if (IS_ERR(asd)) {
-		ret = PTR_ERR(asd);
-		if (ret == -EEXIST) {
-			if (np)
-				dev_dbg(imxmd->md.dev, "%s: already added %pOFn\n",
-					__func__, np);
-			else
-				dev_dbg(imxmd->md.dev, "%s: already added %s\n",
-					__func__, devname);
-		}
-		return ret;
-	}
-
-	imxasd = to_imx_media_asd(asd);
-
-	if (devname)
-		imxasd->pdev = pdev;
-
-	if (np)
-		dev_dbg(imxmd->md.dev, "%s: added %pOFn, match type FWNODE\n",
-			__func__, np);
-	else
-		dev_dbg(imxmd->md.dev, "%s: added %s, match type DEVNAME\n",
-			__func__, devname);
-
-	return 0;
-}
-
-/*
- * get IPU from this CSI and add it to the list of IPUs
- * the media driver will control.
- */
-static int imx_media_get_ipu(struct imx_media_dev *imxmd,
-			     struct v4l2_subdev *csi_sd)
-{
-	struct ipu_soc *ipu;
-	int ipu_id;
-
-	ipu = dev_get_drvdata(csi_sd->dev->parent);
-	if (!ipu) {
-		v4l2_err(&imxmd->v4l2_dev,
-			 "CSI %s has no parent IPU!\n", csi_sd->name);
-		return -ENODEV;
-	}
-
-	ipu_id = ipu_get_num(ipu);
-	if (ipu_id > 1) {
-		v4l2_err(&imxmd->v4l2_dev, "invalid IPU id %d!\n", ipu_id);
-		return -ENODEV;
-	}
-
-	if (!imxmd->ipu[ipu_id])
-		imxmd->ipu[ipu_id] = ipu;
-
-	return 0;
-}
-
 /* async subdev bound notifier */
 int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
 			   struct v4l2_subdev *sd,
 			   struct v4l2_async_subdev *asd)
 {
 	struct imx_media_dev *imxmd = notifier2dev(notifier);
-	int ret = 0;
-
-	mutex_lock(&imxmd->mutex);
+	int ret;
 
 	if (sd->grp_id & IMX_MEDIA_GRP_ID_IPU_CSI) {
-		ret = imx_media_get_ipu(imxmd, sd);
+		/* register the IPU internal subdevs */
+		ret = imx_media_register_ipu_internal_subdevs(imxmd, sd);
 		if (ret)
-			goto out;
+			return ret;
 	}
 
 	v4l2_info(&imxmd->v4l2_dev, "subdev %s bound\n", sd->name);
-out:
-	mutex_unlock(&imxmd->mutex);
-	return ret;
+
+	return 0;
 }
 
 /*
@@ -147,7 +60,6 @@ static int imx_media_create_links(struct v4l2_async_notifier *notifier)
 {
 	struct imx_media_dev *imxmd = notifier2dev(notifier);
 	struct v4l2_subdev *sd;
-	int ret;
 
 	list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
 		switch (sd->grp_id) {
@@ -155,22 +67,15 @@ static int imx_media_create_links(struct v4l2_async_notifier *notifier)
 		case IMX_MEDIA_GRP_ID_IPU_IC_PRP:
 		case IMX_MEDIA_GRP_ID_IPU_IC_PRPENC:
 		case IMX_MEDIA_GRP_ID_IPU_IC_PRPVF:
-		case IMX_MEDIA_GRP_ID_IPU_CSI0:
-		case IMX_MEDIA_GRP_ID_IPU_CSI1:
-			ret = imx_media_create_ipu_internal_links(imxmd, sd);
-			if (ret)
-				return ret;
 			/*
-			 * the CSIs straddle between the external and the IPU
-			 * internal entities, so create the external links
-			 * to the CSI sink pads.
+			 * links have already been created for the
+			 * sync-registered subdevs.
 			 */
-			if (sd->grp_id & IMX_MEDIA_GRP_ID_IPU_CSI)
-				imx_media_create_csi_of_links(imxmd, sd);
 			break;
+		case IMX_MEDIA_GRP_ID_IPU_CSI0:
+		case IMX_MEDIA_GRP_ID_IPU_CSI1:
 		case IMX_MEDIA_GRP_ID_CSI:
 			imx_media_create_csi_of_links(imxmd, sd);
-
 			break;
 		default:
 			/*
@@ -480,12 +385,10 @@ static int imx_media_probe(struct platform_device *pdev)
 
 	ret = imx_media_dev_notifier_register(imxmd);
 	if (ret)
-		goto del_int;
+		goto cleanup;
 
 	return 0;
 
-del_int:
-	imx_media_remove_ipu_internal_subdevs(imxmd);
 cleanup:
 	v4l2_async_notifier_cleanup(&imxmd->notifier);
 	v4l2_device_unregister(&imxmd->v4l2_dev);
@@ -502,7 +405,7 @@ static int imx_media_remove(struct platform_device *pdev)
 	v4l2_info(&imxmd->v4l2_dev, "Removing imx-media\n");
 
 	v4l2_async_notifier_unregister(&imxmd->notifier);
-	imx_media_remove_ipu_internal_subdevs(imxmd);
+	imx_media_unregister_ipu_internal_subdevs(imxmd);
 	v4l2_async_notifier_cleanup(&imxmd->notifier);
 	media_device_unregister(&imxmd->md);
 	v4l2_device_unregister(&imxmd->v4l2_dev);
diff --git a/drivers/staging/media/imx/imx-media-internal-sd.c b/drivers/staging/media/imx/imx-media-internal-sd.c
index dc510dcfe160..618a930da37c 100644
--- a/drivers/staging/media/imx/imx-media-internal-sd.c
+++ b/drivers/staging/media/imx/imx-media-internal-sd.c
@@ -13,208 +13,138 @@
 #include <linux/platform_device.h>
 #include "imx-media.h"
 
-enum isd_enum {
-	isd_csi0 = 0,
-	isd_csi1,
-	isd_vdic,
-	isd_ic_prp,
-	isd_ic_prpenc,
-	isd_ic_prpvf,
-	num_isd,
-};
-
-static const struct internal_subdev_id {
-	enum isd_enum index;
-	const char *name;
-	u32 grp_id;
-} isd_id[num_isd] = {
-	[isd_csi0] = {
-		.index = isd_csi0,
-		.grp_id = IMX_MEDIA_GRP_ID_IPU_CSI0,
-		.name = "imx-ipuv3-csi",
-	},
-	[isd_csi1] = {
-		.index = isd_csi1,
-		.grp_id = IMX_MEDIA_GRP_ID_IPU_CSI1,
-		.name = "imx-ipuv3-csi",
-	},
-	[isd_vdic] = {
-		.index = isd_vdic,
-		.grp_id = IMX_MEDIA_GRP_ID_IPU_VDIC,
-		.name = "imx-ipuv3-vdic",
-	},
-	[isd_ic_prp] = {
-		.index = isd_ic_prp,
-		.grp_id = IMX_MEDIA_GRP_ID_IPU_IC_PRP,
-		.name = "imx-ipuv3-ic",
-	},
-	[isd_ic_prpenc] = {
-		.index = isd_ic_prpenc,
-		.grp_id = IMX_MEDIA_GRP_ID_IPU_IC_PRPENC,
-		.name = "imx-ipuv3-ic",
-	},
-	[isd_ic_prpvf] = {
-		.index = isd_ic_prpvf,
-		.grp_id = IMX_MEDIA_GRP_ID_IPU_IC_PRPVF,
-		.name = "imx-ipuv3-ic",
-	},
-};
+/* max pads per internal-sd */
+#define MAX_INTERNAL_PADS   8
+/* max links per internal-sd pad */
+#define MAX_INTERNAL_LINKS  8
 
 struct internal_subdev;
 
 struct internal_link {
-	const struct internal_subdev *remote;
+	int remote;
 	int local_pad;
 	int remote_pad;
 };
 
-/* max pads per internal-sd */
-#define MAX_INTERNAL_PADS   8
-/* max links per internal-sd pad */
-#define MAX_INTERNAL_LINKS  8
-
 struct internal_pad {
+	int num_links;
 	struct internal_link link[MAX_INTERNAL_LINKS];
 };
 
-static const struct internal_subdev {
-	const struct internal_subdev_id *id;
+struct internal_subdev {
+	u32 grp_id;
 	struct internal_pad pad[MAX_INTERNAL_PADS];
-} int_subdev[num_isd] = {
-	[isd_csi0] = {
-		.id = &isd_id[isd_csi0],
+
+	struct v4l2_subdev * (*sync_register)(struct imx_media_dev *imxmd,
+					      struct device *ipu_dev,
+					      struct ipu_soc *ipu,
+					      u32 grp_id);
+	int (*sync_unregister)(struct v4l2_subdev *sd);
+};
+
+static const struct internal_subdev int_subdev[NUM_IPU_SUBDEVS] = {
+	[IPU_CSI0] = {
+		.grp_id = IMX_MEDIA_GRP_ID_IPU_CSI0,
 		.pad[CSI_SRC_PAD_DIRECT] = {
+			.num_links = 2,
 			.link = {
 				{
 					.local_pad = CSI_SRC_PAD_DIRECT,
-					.remote = &int_subdev[isd_ic_prp],
+					.remote = IPU_IC_PRP,
 					.remote_pad = PRP_SINK_PAD,
 				}, {
 					.local_pad = CSI_SRC_PAD_DIRECT,
-					.remote = &int_subdev[isd_vdic],
+					.remote = IPU_VDIC,
 					.remote_pad = VDIC_SINK_PAD_DIRECT,
 				},
 			},
 		},
 	},
 
-	[isd_csi1] = {
-		.id = &isd_id[isd_csi1],
+	[IPU_CSI1] = {
+		.grp_id = IMX_MEDIA_GRP_ID_IPU_CSI1,
 		.pad[CSI_SRC_PAD_DIRECT] = {
+			.num_links = 2,
 			.link = {
 				{
 					.local_pad = CSI_SRC_PAD_DIRECT,
-					.remote = &int_subdev[isd_ic_prp],
+					.remote = IPU_IC_PRP,
 					.remote_pad = PRP_SINK_PAD,
 				}, {
 					.local_pad = CSI_SRC_PAD_DIRECT,
-					.remote = &int_subdev[isd_vdic],
+					.remote = IPU_VDIC,
 					.remote_pad = VDIC_SINK_PAD_DIRECT,
 				},
 			},
 		},
 	},
 
-	[isd_vdic] = {
-		.id = &isd_id[isd_vdic],
+	[IPU_VDIC] = {
+		.grp_id = IMX_MEDIA_GRP_ID_IPU_VDIC,
+		.sync_register = imx_media_vdic_register,
+		.sync_unregister = imx_media_vdic_unregister,
 		.pad[VDIC_SRC_PAD_DIRECT] = {
+			.num_links = 1,
 			.link = {
 				{
 					.local_pad = VDIC_SRC_PAD_DIRECT,
-					.remote = &int_subdev[isd_ic_prp],
+					.remote = IPU_IC_PRP,
 					.remote_pad = PRP_SINK_PAD,
 				},
 			},
 		},
 	},
 
-	[isd_ic_prp] = {
-		.id = &isd_id[isd_ic_prp],
+	[IPU_IC_PRP] = {
+		.grp_id = IMX_MEDIA_GRP_ID_IPU_IC_PRP,
+		.sync_register = imx_media_ic_register,
+		.sync_unregister = imx_media_ic_unregister,
 		.pad[PRP_SRC_PAD_PRPENC] = {
+			.num_links = 1,
 			.link = {
 				{
 					.local_pad = PRP_SRC_PAD_PRPENC,
-					.remote = &int_subdev[isd_ic_prpenc],
-					.remote_pad = 0,
+					.remote = IPU_IC_PRPENC,
+					.remote_pad = PRPENCVF_SINK_PAD,
 				},
 			},
 		},
 		.pad[PRP_SRC_PAD_PRPVF] = {
+			.num_links = 1,
 			.link = {
 				{
 					.local_pad = PRP_SRC_PAD_PRPVF,
-					.remote = &int_subdev[isd_ic_prpvf],
-					.remote_pad = 0,
+					.remote = IPU_IC_PRPVF,
+					.remote_pad = PRPENCVF_SINK_PAD,
 				},
 			},
 		},
 	},
 
-	[isd_ic_prpenc] = {
-		.id = &isd_id[isd_ic_prpenc],
+	[IPU_IC_PRPENC] = {
+		.grp_id = IMX_MEDIA_GRP_ID_IPU_IC_PRPENC,
+		.sync_register = imx_media_ic_register,
+		.sync_unregister = imx_media_ic_unregister,
 	},
 
-	[isd_ic_prpvf] = {
-		.id = &isd_id[isd_ic_prpvf],
+	[IPU_IC_PRPVF] = {
+		.grp_id = IMX_MEDIA_GRP_ID_IPU_IC_PRPVF,
+		.sync_register = imx_media_ic_register,
+		.sync_unregister = imx_media_ic_unregister,
 	},
 };
 
-/* form a device name given an internal subdev and ipu id */
-static inline void isd_to_devname(char *devname, int sz,
-				  const struct internal_subdev *isd,
-				  int ipu_id)
-{
-	int pdev_id = ipu_id * num_isd + isd->id->index;
-
-	snprintf(devname, sz, "%s.%d", isd->id->name, pdev_id);
-}
-
-static const struct internal_subdev *find_intsd_by_grp_id(u32 grp_id)
-{
-	enum isd_enum i;
-
-	for (i = 0; i < num_isd; i++) {
-		const struct internal_subdev *isd = &int_subdev[i];
-
-		if (isd->id->grp_id == grp_id)
-			return isd;
-	}
-
-	return NULL;
-}
-
-static struct v4l2_subdev *find_sink(struct imx_media_dev *imxmd,
-				     struct v4l2_subdev *src,
-				     const struct internal_link *link)
-{
-	char sink_devname[32];
-	int ipu_id;
-
-	/*
-	 * retrieve IPU id from subdev name, note: can't get this from
-	 * struct imx_media_ipu_internal_sd_pdata because if src is
-	 * a CSI, it has different struct ipu_client_platformdata which
-	 * does not contain IPU id.
-	 */
-	if (sscanf(src->name, "ipu%d", &ipu_id) != 1)
-		return NULL;
-
-	isd_to_devname(sink_devname, sizeof(sink_devname),
-		       link->remote, ipu_id - 1);
-
-	return imx_media_find_subdev_by_devname(imxmd, sink_devname);
-}
-
-static int create_ipu_internal_link(struct imx_media_dev *imxmd,
-				    struct v4l2_subdev *src,
-				    const struct internal_link *link)
+static int create_internal_link(struct imx_media_dev *imxmd,
+				struct v4l2_subdev *src,
+				struct v4l2_subdev *sink,
+				const struct internal_link *link)
 {
-	struct v4l2_subdev *sink;
 	int ret;
 
-	sink = find_sink(imxmd, src, link);
-	if (!sink)
-		return -ENODEV;
+	/* skip if this link already created */
+	if (media_entity_find_link(&src->entity.pads[link->local_pad],
+				   &sink->entity.pads[link->remote_pad]))
+		return 0;
 
 	v4l2_info(&imxmd->v4l2_dev, "%s:%d -> %s:%d\n",
 		  src->name, link->local_pad,
@@ -223,25 +153,21 @@ static int create_ipu_internal_link(struct imx_media_dev *imxmd,
 	ret = media_create_pad_link(&src->entity, link->local_pad,
 				    &sink->entity, link->remote_pad, 0);
 	if (ret)
-		v4l2_err(&imxmd->v4l2_dev,
-			 "create_pad_link failed: %d\n", ret);
+		v4l2_err(&imxmd->v4l2_dev, "%s failed: %d\n", __func__, ret);
 
 	return ret;
 }
 
-int imx_media_create_ipu_internal_links(struct imx_media_dev *imxmd,
-					struct v4l2_subdev *sd)
+static int create_ipu_internal_links(struct imx_media_dev *imxmd,
+				     const struct internal_subdev *intsd,
+				     struct v4l2_subdev *sd,
+				     int ipu_id)
 {
-	const struct internal_subdev *intsd;
 	const struct internal_pad *intpad;
 	const struct internal_link *link;
 	struct media_pad *pad;
 	int i, j, ret;
 
-	intsd = find_intsd_by_grp_id(sd->grp_id);
-	if (!intsd)
-		return -ENODEV;
-
 	/* create the source->sink links */
 	for (i = 0; i < sd->entity.num_pads; i++) {
 		intpad = &intsd->pad[i];
@@ -250,13 +176,13 @@ int imx_media_create_ipu_internal_links(struct imx_media_dev *imxmd,
 		if (!(pad->flags & MEDIA_PAD_FL_SOURCE))
 			continue;
 
-		for (j = 0; ; j++) {
-			link = &intpad->link[j];
+		for (j = 0; j < intpad->num_links; j++) {
+			struct v4l2_subdev *sink;
 
-			if (!link->remote)
-				break;
+			link = &intpad->link[j];
+			sink = imxmd->sync_sd[ipu_id][link->remote];
 
-			ret = create_ipu_internal_link(imxmd, sd, link);
+			ret = create_internal_link(imxmd, sd, sink, link);
 			if (ret)
 				return ret;
 		}
@@ -265,85 +191,115 @@ int imx_media_create_ipu_internal_links(struct imx_media_dev *imxmd,
 	return 0;
 }
 
-/* register an internal subdev as a platform device */
-static int add_internal_subdev(struct imx_media_dev *imxmd,
-			       const struct internal_subdev *isd,
-			       int ipu_id)
+int imx_media_register_ipu_internal_subdevs(struct imx_media_dev *imxmd,
+					    struct v4l2_subdev *csi)
 {
-	struct imx_media_ipu_internal_sd_pdata pdata;
-	struct platform_device_info pdevinfo = {};
-	struct platform_device *pdev;
+	struct device *ipu_dev = csi->dev->parent;
+	const struct internal_subdev *intsd;
+	struct v4l2_subdev *sd;
+	struct ipu_soc *ipu;
+	int i, ipu_id, ret;
 
-	pdata.grp_id = isd->id->grp_id;
+	ipu = dev_get_drvdata(ipu_dev);
+	if (!ipu) {
+		v4l2_err(&imxmd->v4l2_dev, "invalid IPU device!\n");
+		return -ENODEV;
+	}
 
-	/* the id of IPU this subdev will control */
-	pdata.ipu_id = ipu_id;
+	ipu_id = ipu_get_num(ipu);
+	if (ipu_id > 1) {
+		v4l2_err(&imxmd->v4l2_dev, "invalid IPU id %d!\n", ipu_id);
+		return -ENODEV;
+	}
 
-	/* create subdev name */
-	imx_media_grp_id_to_sd_name(pdata.sd_name, sizeof(pdata.sd_name),
-				    pdata.grp_id, ipu_id);
+	mutex_lock(&imxmd->mutex);
 
-	pdevinfo.name = isd->id->name;
-	pdevinfo.id = ipu_id * num_isd + isd->id->index;
-	pdevinfo.parent = imxmd->md.dev;
-	pdevinfo.data = &pdata;
-	pdevinfo.size_data = sizeof(pdata);
-	pdevinfo.dma_mask = DMA_BIT_MASK(32);
+	/* register the synchronous subdevs */
+	for (i = 0; i < NUM_IPU_SUBDEVS; i++) {
+		intsd = &int_subdev[i];
 
-	pdev = platform_device_register_full(&pdevinfo);
-	if (IS_ERR(pdev))
-		return PTR_ERR(pdev);
+		sd = imxmd->sync_sd[ipu_id][i];
 
-	return imx_media_add_async_subdev(imxmd, NULL, pdev);
-}
+		/*
+		 * skip if this sync subdev already registered or its
+		 * not a sync subdev (one of the CSIs)
+		 */
+		if (sd || !intsd->sync_register)
+			continue;
 
-/* adds the internal subdevs in one ipu */
-int imx_media_add_ipu_internal_subdevs(struct imx_media_dev *imxmd,
-				       int ipu_id)
-{
-	enum isd_enum i;
-	int ret;
+		mutex_unlock(&imxmd->mutex);
+		sd = intsd->sync_register(imxmd, ipu_dev, ipu, intsd->grp_id);
+		mutex_lock(&imxmd->mutex);
+		if (IS_ERR(sd)) {
+			ret = PTR_ERR(sd);
+			goto err_unwind;
+		}
 
-	for (i = 0; i < num_isd; i++) {
-		const struct internal_subdev *isd = &int_subdev[i];
+		imxmd->sync_sd[ipu_id][i] = sd;
+	}
 
-		/*
-		 * the CSIs are represented in the device-tree, so those
-		 * devices are already added to the async subdev list by
-		 * of_parse_subdev().
-		 */
-		switch (isd->id->grp_id) {
-		case IMX_MEDIA_GRP_ID_IPU_CSI0:
-		case IMX_MEDIA_GRP_ID_IPU_CSI1:
-			ret = 0;
-			break;
-		default:
-			ret = add_internal_subdev(imxmd, isd, ipu_id);
-			break;
+	/*
+	 * all the sync subdevs are registered, create the media links
+	 * between them.
+	 */
+	for (i = 0; i < NUM_IPU_SUBDEVS; i++) {
+		intsd = &int_subdev[i];
+
+		if (intsd->grp_id == csi->grp_id) {
+			sd = csi;
+		} else {
+			sd = imxmd->sync_sd[ipu_id][i];
+			if (!sd)
+				continue;
 		}
 
-		if (ret)
-			goto remove;
+		ret = create_ipu_internal_links(imxmd, intsd, sd, ipu_id);
+		if (ret) {
+			mutex_unlock(&imxmd->mutex);
+			imx_media_unregister_ipu_internal_subdevs(imxmd);
+			return ret;
+		}
 	}
 
+	mutex_unlock(&imxmd->mutex);
 	return 0;
 
-remove:
-	imx_media_remove_ipu_internal_subdevs(imxmd);
+err_unwind:
+	while (--i >= 0) {
+		intsd = &int_subdev[i];
+		sd = imxmd->sync_sd[ipu_id][i];
+		if (!sd || !intsd->sync_unregister)
+			continue;
+		mutex_unlock(&imxmd->mutex);
+		intsd->sync_unregister(sd);
+		mutex_lock(&imxmd->mutex);
+	}
+
+	mutex_unlock(&imxmd->mutex);
 	return ret;
 }
 
-void imx_media_remove_ipu_internal_subdevs(struct imx_media_dev *imxmd)
+void imx_media_unregister_ipu_internal_subdevs(struct imx_media_dev *imxmd)
 {
-	struct imx_media_async_subdev *imxasd;
-	struct v4l2_async_subdev *asd;
+	const struct internal_subdev *intsd;
+	struct v4l2_subdev *sd;
+	int i, j;
 
-	list_for_each_entry(asd, &imxmd->notifier.asd_list, asd_list) {
-		imxasd = to_imx_media_asd(asd);
+	mutex_lock(&imxmd->mutex);
 
-		if (!imxasd->pdev)
-			continue;
+	for (i = 0; i < 2; i++) {
+		for (j = 0; j < NUM_IPU_SUBDEVS; j++) {
+			intsd = &int_subdev[j];
+			sd = imxmd->sync_sd[i][j];
+
+			if (!sd || !intsd->sync_unregister)
+				continue;
 
-		platform_device_unregister(imxasd->pdev);
+			mutex_unlock(&imxmd->mutex);
+			intsd->sync_unregister(sd);
+			mutex_lock(&imxmd->mutex);
+		}
 	}
+
+	mutex_unlock(&imxmd->mutex);
 }
diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
index 12383f4785ad..b7034699391d 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -23,6 +23,9 @@
 int imx_media_of_add_csi(struct imx_media_dev *imxmd,
 			 struct device_node *csi_np)
 {
+	struct v4l2_async_subdev *asd;
+	int ret = 0;
+
 	if (!of_device_is_available(csi_np)) {
 		dev_dbg(imxmd->md.dev, "%s: %pOFn not enabled\n", __func__,
 			csi_np);
@@ -30,18 +33,25 @@ int imx_media_of_add_csi(struct imx_media_dev *imxmd,
 	}
 
 	/* add CSI fwnode to async notifier */
-	return imx_media_add_async_subdev(imxmd, of_fwnode_handle(csi_np),
-					  NULL);
+	asd = v4l2_async_notifier_add_fwnode_subdev(&imxmd->notifier,
+						    of_fwnode_handle(csi_np),
+						    sizeof(*asd));
+	if (IS_ERR(asd)) {
+		ret = PTR_ERR(asd);
+		if (ret == -EEXIST)
+			dev_dbg(imxmd->md.dev, "%s: already added %pOFn\n",
+				__func__, csi_np);
+	}
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(imx_media_of_add_csi);
 
 int imx_media_add_of_subdevs(struct imx_media_dev *imxmd,
 			     struct device_node *np)
 {
-	bool ipu_found[2] = {false, false};
 	struct device_node *csi_np;
 	int i, ret;
-	u32 ipu_id;
 
 	for (i = 0; ; i++) {
 		csi_np = of_parse_phandle(np, "ports", i);
@@ -59,31 +69,11 @@ int imx_media_add_of_subdevs(struct imx_media_dev *imxmd,
 			/* other error, can't continue */
 			goto err_out;
 		}
-
-		ret = of_alias_get_id(csi_np->parent, "ipu");
-		if (ret < 0)
-			goto err_out;
-		if (ret > 1) {
-			ret = -EINVAL;
-			goto err_out;
-		}
-
-		ipu_id = ret;
-
-		if (!ipu_found[ipu_id]) {
-			ret = imx_media_add_ipu_internal_subdevs(imxmd,
-								 ipu_id);
-			if (ret)
-				goto err_out;
-		}
-
-		ipu_found[ipu_id] = true;
 	}
 
 	return 0;
 
 err_out:
-	imx_media_remove_ipu_internal_subdevs(imxmd);
 	of_node_put(csi_np);
 	return ret;
 }
diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
index 5439b88dba20..5dd4c733de97 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -8,13 +8,6 @@
  * the Free Software Foundation; either version 2 of the License, or
  * (at your option) any later version.
  */
-#include <linux/delay.h>
-#include <linux/interrupt.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/sched.h>
-#include <linux/slab.h>
-#include <linux/timer.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ioctl.h>
@@ -69,12 +62,12 @@ struct vdic_pipeline_ops {
 #define S_ALIGN    1 /* multiple of 2 */
 
 struct vdic_priv {
-	struct device        *dev;
-	struct ipu_soc       *ipu;
+	struct device *ipu_dev;
+	struct ipu_soc *ipu;
+
 	struct imx_media_dev *md;
 	struct v4l2_subdev   sd;
 	struct media_pad pad[VDIC_NUM_PADS];
-	int ipu_id;
 
 	/* lock to protect all members below */
 	struct mutex lock;
@@ -149,8 +142,6 @@ static int vdic_get_ipu_resources(struct vdic_priv *priv)
 	struct ipuv3_channel *ch;
 	struct ipu_vdi *vdi;
 
-	priv->ipu = priv->md->ipu[priv->ipu_id];
-
 	vdi = ipu_vdi_get(priv->ipu);
 	if (IS_ERR(vdi)) {
 		v4l2_err(&priv->sd, "failed to get VDIC\n");
@@ -515,7 +506,8 @@ static int vdic_s_stream(struct v4l2_subdev *sd, int enable)
 	if (priv->stream_count != !enable)
 		goto update_count;
 
-	dev_dbg(priv->dev, "stream %s\n", enable ? "ON" : "OFF");
+	dev_dbg(priv->ipu_dev, "%s: stream %s\n", sd->name,
+		enable ? "ON" : "OFF");
 
 	if (enable)
 		ret = vdic_start(priv);
@@ -690,8 +682,8 @@ static int vdic_link_setup(struct media_entity *entity,
 	struct v4l2_subdev *remote_sd;
 	int ret = 0;
 
-	dev_dbg(priv->dev, "link setup %s -> %s", remote->entity->name,
-		local->entity->name);
+	dev_dbg(priv->ipu_dev, "%s: link setup %s -> %s",
+		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@mentor.com>");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:imx-ipuv3-vdic");
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index dd603a6b3a70..fadde3435cb7 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -19,6 +19,19 @@
 #include <media/videobuf2-dma-contig.h>
 #include <video/imx-ipu-v3.h>
 
+/*
+ * Enumeration of the IPU internal sub-devices
+ */
+enum {
+	IPU_CSI0 = 0,
+	IPU_CSI1,
+	IPU_VDIC,
+	IPU_IC_PRP,
+	IPU_IC_PRPENC,
+	IPU_IC_PRPVF,
+	NUM_IPU_SUBDEVS,
+};
+
 /*
  * Pad definitions for the subdevs with multiple source or
  * sink pads
@@ -115,25 +128,6 @@ struct imx_media_pad_vdev {
 	struct list_head list;
 };
 
-struct imx_media_ipu_internal_sd_pdata {
-	char sd_name[V4L2_SUBDEV_NAME_SIZE];
-	u32 grp_id;
-	int ipu_id;
-};
-
-struct imx_media_async_subdev {
-	/* the base asd - must be first in this struct */
-	struct v4l2_async_subdev asd;
-	/* the platform device of IPU-internal subdevs */
-	struct platform_device *pdev;
-};
-
-static inline struct imx_media_async_subdev *
-to_imx_media_asd(struct v4l2_async_subdev *asd)
-{
-	return container_of(asd, struct imx_media_async_subdev, asd);
-}
-
 struct imx_media_dev {
 	struct media_device md;
 	struct v4l2_device  v4l2_dev;
@@ -146,11 +140,11 @@ struct imx_media_dev {
 	/* master video device list */
 	struct list_head vdev_list;
 
-	/* IPUs this media driver control, valid after subdevs bound */
-	struct ipu_soc *ipu[2];
-
 	/* for async subdev registration */
 	struct v4l2_async_notifier notifier;
+
+	/* the IPU internal subdev's registered synchronously */
+	struct v4l2_subdev *sync_sd[2][NUM_IPU_SUBDEVS];
 };
 
 enum codespace_sel {
@@ -225,10 +219,6 @@ int imx_media_pipeline_set_stream(struct imx_media_dev *imxmd,
 				  bool on);
 
 /* imx-media-dev.c */
-int imx_media_add_async_subdev(struct imx_media_dev *imxmd,
-			       struct fwnode_handle *fwnode,
-			       struct platform_device *pdev);
-
 int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
 			   struct v4l2_subdev *sd,
 			   struct v4l2_async_subdev *asd);
@@ -252,11 +242,9 @@ struct imx_media_fim *imx_media_fim_init(struct v4l2_subdev *sd);
 void imx_media_fim_free(struct imx_media_fim *fim);
 
 /* imx-media-internal-sd.c */
-int imx_media_add_ipu_internal_subdevs(struct imx_media_dev *imxmd,
-				       int ipu_id);
-int imx_media_create_ipu_internal_links(struct imx_media_dev *imxmd,
-					struct v4l2_subdev *sd);
-void imx_media_remove_ipu_internal_subdevs(struct imx_media_dev *imxmd);
+int imx_media_register_ipu_internal_subdevs(struct imx_media_dev *imxmd,
+					    struct v4l2_subdev *csi);
+void imx_media_unregister_ipu_internal_subdevs(struct imx_media_dev *imxmd);
 
 /* imx-media-of.c */
 int imx_media_add_of_subdevs(struct imx_media_dev *dev,
@@ -268,9 +256,24 @@ int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
 int imx_media_of_add_csi(struct imx_media_dev *imxmd,
 			 struct device_node *csi_np);
 
+/* imx-media-vdic.c */
+struct v4l2_subdev *imx_media_vdic_register(struct imx_media_dev *imxmd,
+					    struct device *ipu_dev,
+					    struct ipu_soc *ipu,
+					    u32 grp_id);
+int imx_media_vdic_unregister(struct v4l2_subdev *sd);
+
+/* imx-ic-common.c */
+struct v4l2_subdev *imx_media_ic_register(struct imx_media_dev *imxmd,
+					  struct device *ipu_dev,
+					  struct ipu_soc *ipu,
+					  u32 grp_id);
+int imx_media_ic_unregister(struct v4l2_subdev *sd);
+
 /* imx-media-capture.c */
 struct imx_media_video_dev *
-imx_media_capture_device_init(struct v4l2_subdev *src_sd, int pad);
+imx_media_capture_device_init(struct device *dev, 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);
 void imx_media_capture_device_unregister(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 18eb5d3ecf10..95f3808762b4 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -1292,7 +1292,8 @@ static int imx7_csi_probe(struct platform_device *pdev)
 	csi->sd.grp_id = IMX_MEDIA_GRP_ID_CSI;
 	snprintf(csi->sd.name, sizeof(csi->sd.name), "csi");
 
-	csi->vdev = imx_media_capture_device_init(&csi->sd, IMX7_CSI_PAD_SRC);
+	csi->vdev = imx_media_capture_device_init(csi->sd.dev, &csi->sd,
+						  IMX7_CSI_PAD_SRC);
 	if (IS_ERR(csi->vdev))
 		return PTR_ERR(csi->vdev);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/8] media: staging/imx: Pass device to alloc/free_dma_buf
  2019-04-30 22:50 [PATCH v3 0/8] Switch to sync registration for IPU subdevs Steve Longerbeam
  2019-04-30 22:50 ` [PATCH v3 1/8] media: staging/imx: " Steve Longerbeam
@ 2019-04-30 22:50 ` Steve Longerbeam
  2019-04-30 22:50 ` [PATCH v3 3/8] media: staging/imx: Move add_video_device into capture_device_register Steve Longerbeam
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Steve Longerbeam @ 2019-04-30 22:50 UTC (permalink / raw)
  To: linux-media
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Rui Miguel Silva, open list:STAGING SUBSYSTEM,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list

Allocate and free a DMA coherent buffer in imx_media_alloc/free_dma_buf()
from the given device. This allows DMA alloc and free using a device
that is backed by real hardware, which for the imx5/6/7 CSI is the CSI
unit, and for the internal IPU sub-devices, is the parent IPU.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-ic-prpencvf.c | 18 +++++++++---------
 drivers/staging/media/imx/imx-media-csi.c   |  6 +++---
 drivers/staging/media/imx/imx-media-utils.c | 13 ++++++-------
 drivers/staging/media/imx/imx-media.h       |  4 ++--
 drivers/staging/media/imx/imx7-media-csi.c  |  4 ++--
 5 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 069cce512280..ddcd87a17c71 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -464,13 +464,13 @@ static int prp_setup_rotation(struct prp_priv *priv)
 	incc = priv->cc[PRPENCVF_SINK_PAD];
 	outcc = vdev->cc;
 
-	ret = imx_media_alloc_dma_buf(ic_priv->md, &priv->rot_buf[0],
+	ret = imx_media_alloc_dma_buf(ic_priv->ipu_dev, &priv->rot_buf[0],
 				      outfmt->sizeimage);
 	if (ret) {
 		v4l2_err(&ic_priv->sd, "failed to alloc rot_buf[0], %d\n", ret);
 		return ret;
 	}
-	ret = imx_media_alloc_dma_buf(ic_priv->md, &priv->rot_buf[1],
+	ret = imx_media_alloc_dma_buf(ic_priv->ipu_dev, &priv->rot_buf[1],
 				      outfmt->sizeimage);
 	if (ret) {
 		v4l2_err(&ic_priv->sd, "failed to alloc rot_buf[1], %d\n", ret);
@@ -543,9 +543,9 @@ static int prp_setup_rotation(struct prp_priv *priv)
 unsetup_vb2:
 	prp_unsetup_vb2_buf(priv, VB2_BUF_STATE_QUEUED);
 free_rot1:
-	imx_media_free_dma_buf(ic_priv->md, &priv->rot_buf[1]);
+	imx_media_free_dma_buf(ic_priv->ipu_dev, &priv->rot_buf[1]);
 free_rot0:
-	imx_media_free_dma_buf(ic_priv->md, &priv->rot_buf[0]);
+	imx_media_free_dma_buf(ic_priv->ipu_dev, &priv->rot_buf[0]);
 	return ret;
 }
 
@@ -563,8 +563,8 @@ static void prp_unsetup_rotation(struct prp_priv *priv)
 
 	ipu_ic_disable(priv->ic);
 
-	imx_media_free_dma_buf(ic_priv->md, &priv->rot_buf[0]);
-	imx_media_free_dma_buf(ic_priv->md, &priv->rot_buf[1]);
+	imx_media_free_dma_buf(ic_priv->ipu_dev, &priv->rot_buf[0]);
+	imx_media_free_dma_buf(ic_priv->ipu_dev, &priv->rot_buf[1]);
 }
 
 static int prp_setup_norotation(struct prp_priv *priv)
@@ -656,7 +656,7 @@ static int prp_start(struct prp_priv *priv)
 
 	outfmt = &vdev->fmt.fmt.pix;
 
-	ret = imx_media_alloc_dma_buf(ic_priv->md, &priv->underrun_buf,
+	ret = imx_media_alloc_dma_buf(ic_priv->ipu_dev, &priv->underrun_buf,
 				      outfmt->sizeimage);
 	if (ret)
 		goto out_put_ipu;
@@ -726,7 +726,7 @@ static int prp_start(struct prp_priv *priv)
 out_unsetup:
 	prp_unsetup(priv, VB2_BUF_STATE_QUEUED);
 out_free_underrun:
-	imx_media_free_dma_buf(ic_priv->md, &priv->underrun_buf);
+	imx_media_free_dma_buf(ic_priv->ipu_dev, &priv->underrun_buf);
 out_put_ipu:
 	prp_put_ipu_resources(priv);
 	return ret;
@@ -763,7 +763,7 @@ static void prp_stop(struct prp_priv *priv)
 
 	prp_unsetup(priv, VB2_BUF_STATE_ERROR);
 
-	imx_media_free_dma_buf(ic_priv->md, &priv->underrun_buf);
+	imx_media_free_dma_buf(ic_priv->ipu_dev, &priv->underrun_buf);
 
 	/* cancel the EOF timeout timer */
 	del_timer_sync(&priv->eof_timeout_timer);
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 93b107eab5f5..ea3d13103c91 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -612,7 +612,7 @@ static int csi_idmac_start(struct csi_priv *priv)
 
 	outfmt = &vdev->fmt.fmt.pix;
 
-	ret = imx_media_alloc_dma_buf(priv->md, &priv->underrun_buf,
+	ret = imx_media_alloc_dma_buf(priv->dev, &priv->underrun_buf,
 				      outfmt->sizeimage);
 	if (ret)
 		goto out_put_ipu;
@@ -666,7 +666,7 @@ static int csi_idmac_start(struct csi_priv *priv)
 out_unsetup:
 	csi_idmac_unsetup(priv, VB2_BUF_STATE_QUEUED);
 out_free_dma_buf:
-	imx_media_free_dma_buf(priv->md, &priv->underrun_buf);
+	imx_media_free_dma_buf(priv->dev, &priv->underrun_buf);
 out_put_ipu:
 	csi_idmac_put_ipu_resources(priv);
 	return ret;
@@ -698,7 +698,7 @@ static void csi_idmac_stop(struct csi_priv *priv)
 
 	csi_idmac_unsetup(priv, VB2_BUF_STATE_ERROR);
 
-	imx_media_free_dma_buf(priv->md, &priv->underrun_buf);
+	imx_media_free_dma_buf(priv->dev, &priv->underrun_buf);
 
 	/* cancel the EOF timeout timer */
 	del_timer_sync(&priv->eof_timeout_timer);
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 1c63a2765a81..c52aa59acd05 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -679,29 +679,28 @@ int imx_media_ipu_image_to_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
 }
 EXPORT_SYMBOL_GPL(imx_media_ipu_image_to_mbus_fmt);
 
-void imx_media_free_dma_buf(struct imx_media_dev *imxmd,
+void imx_media_free_dma_buf(struct device *dev,
 			    struct imx_media_dma_buf *buf)
 {
 	if (buf->virt)
-		dma_free_coherent(imxmd->md.dev, buf->len,
-				  buf->virt, buf->phys);
+		dma_free_coherent(dev, buf->len, buf->virt, buf->phys);
 
 	buf->virt = NULL;
 	buf->phys = 0;
 }
 EXPORT_SYMBOL_GPL(imx_media_free_dma_buf);
 
-int imx_media_alloc_dma_buf(struct imx_media_dev *imxmd,
+int imx_media_alloc_dma_buf(struct device *dev,
 			    struct imx_media_dma_buf *buf,
 			    int size)
 {
-	imx_media_free_dma_buf(imxmd, buf);
+	imx_media_free_dma_buf(dev, buf);
 
 	buf->len = PAGE_ALIGN(size);
-	buf->virt = dma_alloc_coherent(imxmd->md.dev, buf->len, &buf->phys,
+	buf->virt = dma_alloc_coherent(dev, buf->len, &buf->phys,
 				       GFP_DMA | GFP_KERNEL);
 	if (!buf->virt) {
-		dev_err(imxmd->md.dev, "failed to alloc dma buffer\n");
+		dev_err(dev, "%s: failed\n", __func__);
 		return -ENOMEM;
 	}
 
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index fadde3435cb7..ba2d75bcc4c9 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -208,9 +208,9 @@ struct imx_media_dma_buf {
 	unsigned long  len;
 };
 
-void imx_media_free_dma_buf(struct imx_media_dev *imxmd,
+void imx_media_free_dma_buf(struct device *dev,
 			    struct imx_media_dma_buf *buf);
-int imx_media_alloc_dma_buf(struct imx_media_dev *imxmd,
+int imx_media_alloc_dma_buf(struct device *dev,
 			    struct imx_media_dma_buf *buf,
 			    int size);
 
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 95f3808762b4..96d01d8af874 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -714,7 +714,7 @@ static int imx7_csi_dma_start(struct imx7_csi *csi)
 	struct v4l2_pix_format *out_pix = &vdev->fmt.fmt.pix;
 	int ret;
 
-	ret = imx_media_alloc_dma_buf(csi->imxmd, &csi->underrun_buf,
+	ret = imx_media_alloc_dma_buf(csi->dev, &csi->underrun_buf,
 				      out_pix->sizeimage);
 	if (ret < 0) {
 		v4l2_warn(&csi->sd, "consider increasing the CMA area\n");
@@ -754,7 +754,7 @@ static void imx7_csi_dma_stop(struct imx7_csi *csi)
 
 	imx7_csi_dma_unsetup_vb2_buf(csi, VB2_BUF_STATE_ERROR);
 
-	imx_media_free_dma_buf(csi->imxmd, &csi->underrun_buf);
+	imx_media_free_dma_buf(csi->dev, &csi->underrun_buf);
 }
 
 static int imx7_csi_configure(struct imx7_csi *csi)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 3/8] media: staging/imx: Move add_video_device into capture_device_register
  2019-04-30 22:50 [PATCH v3 0/8] Switch to sync registration for IPU subdevs Steve Longerbeam
  2019-04-30 22:50 ` [PATCH v3 1/8] media: staging/imx: " Steve Longerbeam
  2019-04-30 22:50 ` [PATCH v3 2/8] media: staging/imx: Pass device to alloc/free_dma_buf Steve Longerbeam
@ 2019-04-30 22:50 ` Steve Longerbeam
  2019-04-30 22:50 ` [PATCH v3 4/8] Revert "media: imx: Set capture compose rectangle in capture_device_set_format" Steve Longerbeam
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Steve Longerbeam @ 2019-04-30 22:50 UTC (permalink / raw)
  To: linux-media
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Rui Miguel Silva, open list:STAGING SUBSYSTEM,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list

Move imx_media_add_video_device() into imx_media_capture_device_register().
Also the former has no error conditions to convert to void.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-ic-prpencvf.c   |  5 -----
 drivers/staging/media/imx/imx-media-capture.c |  3 +++
 drivers/staging/media/imx/imx-media-csi.c     |  7 +------
 drivers/staging/media/imx/imx-media-utils.c   |  9 ++++-----
 drivers/staging/media/imx/imx-media.h         |  4 ++--
 drivers/staging/media/imx/imx7-media-csi.c    | 12 +-----------
 6 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index ddcd87a17c71..8242d88dfb82 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -1241,7 +1241,6 @@ static int prp_s_frame_interval(struct v4l2_subdev *sd,
 static int prp_registered(struct v4l2_subdev *sd)
 {
 	struct prp_priv *priv = sd_to_priv(sd);
-	struct imx_ic_priv *ic_priv = priv->ic_priv;
 	int i, ret;
 	u32 code;
 
@@ -1271,10 +1270,6 @@ static int prp_registered(struct v4l2_subdev *sd)
 	if (ret)
 		return ret;
 
-	ret = imx_media_add_video_device(ic_priv->md, priv->vdev);
-	if (ret)
-		goto unreg;
-
 	ret = prp_init_controls(priv);
 	if (ret)
 		goto unreg;
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 211ec4df2066..335084a6b0cd 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -780,6 +780,9 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
 
 	vfd->ctrl_handler = &priv->ctrl_hdlr;
 
+	/* add vdev to the video device list */
+	imx_media_add_video_device(priv->md, vdev);
+
 	return 0;
 unreg:
 	video_unregister_device(vfd);
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index ea3d13103c91..c70fa6b509ae 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1820,13 +1820,8 @@ static int csi_registered(struct v4l2_subdev *sd)
 	if (ret)
 		goto free_fim;
 
-	ret = imx_media_add_video_device(priv->md, priv->vdev);
-	if (ret)
-		goto unreg;
-
 	return 0;
-unreg:
-	imx_media_capture_device_unregister(priv->vdev);
+
 free_fim:
 	if (priv->fim)
 		imx_media_fim_free(priv->fim);
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index c52aa59acd05..8a6e57652402 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -767,18 +767,17 @@ imx_media_find_subdev_by_devname(struct imx_media_dev *imxmd,
 EXPORT_SYMBOL_GPL(imx_media_find_subdev_by_devname);
 
 /*
- * Adds a video device to the master video device list. This is called by
- * an async subdev that owns a video device when it is registered.
+ * Adds a video device to the master video device list. This is called
+ * when a video device is registered.
  */
-int imx_media_add_video_device(struct imx_media_dev *imxmd,
-			       struct imx_media_video_dev *vdev)
+void imx_media_add_video_device(struct imx_media_dev *imxmd,
+				struct imx_media_video_dev *vdev)
 {
 	mutex_lock(&imxmd->mutex);
 
 	list_add_tail(&vdev->list, &imxmd->vdev_list);
 
 	mutex_unlock(&imxmd->mutex);
-	return 0;
 }
 EXPORT_SYMBOL_GPL(imx_media_add_video_device);
 
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index ba2d75bcc4c9..71e20f53ed7b 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -189,8 +189,8 @@ imx_media_find_subdev_by_fwnode(struct imx_media_dev *imxmd,
 struct v4l2_subdev *
 imx_media_find_subdev_by_devname(struct imx_media_dev *imxmd,
 				 const char *devname);
-int imx_media_add_video_device(struct imx_media_dev *imxmd,
-			       struct imx_media_video_dev *vdev);
+void imx_media_add_video_device(struct imx_media_dev *imxmd,
+				struct imx_media_video_dev *vdev);
 int imx_media_find_mipi_csi2_channel(struct imx_media_dev *imxmd,
 				     struct media_entity *start_entity);
 struct media_pad *
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 96d01d8af874..f2037aba6e0e 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -1126,17 +1126,7 @@ static int imx7_csi_registered(struct v4l2_subdev *sd)
 	if (ret < 0)
 		return ret;
 
-	ret = imx_media_capture_device_register(csi->vdev);
-	if (ret < 0)
-		return ret;
-
-	ret = imx_media_add_video_device(csi->imxmd, csi->vdev);
-	if (ret < 0) {
-		imx_media_capture_device_unregister(csi->vdev);
-		return ret;
-	}
-
-	return 0;
+	return imx_media_capture_device_register(csi->vdev);
 }
 
 static void imx7_csi_unregistered(struct v4l2_subdev *sd)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 4/8] Revert "media: imx: Set capture compose rectangle in capture_device_set_format"
  2019-04-30 22:50 [PATCH v3 0/8] Switch to sync registration for IPU subdevs Steve Longerbeam
                   ` (2 preceding siblings ...)
  2019-04-30 22:50 ` [PATCH v3 3/8] media: staging/imx: Move add_video_device into capture_device_register Steve Longerbeam
@ 2019-04-30 22:50 ` Steve Longerbeam
  2019-04-30 22:50 ` [PATCH v3 5/8] media: staging/imx: Remove capture_device_set_format Steve Longerbeam
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Steve Longerbeam @ 2019-04-30 22:50 UTC (permalink / raw)
  To: linux-media
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Rui Miguel Silva, open list:STAGING SUBSYSTEM,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list

Rvert this commit, as imx_media_capture_device_set_format() will be
removed. The arguments to mx_media_mbus_fmt_to_pix_fmt() and
imx_media_capture_device_set_format() in imx7_csi_set_fmt() are also
reverted.

This reverts commit 5964cbd8692252615370b77eb96764dd70c2f837.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
Chnges in v3:
- revert to previous args in imx7_csi_set_fmt().
---
 drivers/staging/media/imx/imx-ic-prpencvf.c   |  5 ++--
 drivers/staging/media/imx/imx-media-capture.c | 24 +++++++++----------
 drivers/staging/media/imx/imx-media-csi.c     |  5 ++--
 drivers/staging/media/imx/imx-media-utils.c   | 20 ++++------------
 drivers/staging/media/imx/imx-media.h         |  6 ++---
 drivers/staging/media/imx/imx7-media-csi.c    |  5 ++--
 6 files changed, 25 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 8242d88dfb82..afaa3a8b15e9 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -910,7 +910,6 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
 	const struct imx_media_pixfmt *cc;
 	struct v4l2_pix_format vdev_fmt;
 	struct v4l2_mbus_framefmt *fmt;
-	struct v4l2_rect vdev_compose;
 	int ret = 0;
 
 	if (sdformat->pad >= PRPENCVF_NUM_PADS)
@@ -952,11 +951,11 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
 	priv->cc[sdformat->pad] = cc;
 
 	/* propagate output pad format to capture device */
-	imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
+	imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
 				      &priv->format_mbus[PRPENCVF_SRC_PAD],
 				      priv->cc[PRPENCVF_SRC_PAD]);
 	mutex_unlock(&priv->lock);
-	imx_media_capture_device_set_format(vdev, &vdev_fmt, &vdev_compose);
+	imx_media_capture_device_set_format(vdev, &vdev_fmt);
 
 	return 0;
 out:
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 335084a6b0cd..555f6204660b 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -205,8 +205,7 @@ static int capture_g_fmt_vid_cap(struct file *file, void *fh,
 
 static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
 				     struct v4l2_subdev_format *fmt_src,
-				     struct v4l2_format *f,
-				     struct v4l2_rect *compose)
+				     struct v4l2_format *f)
 {
 	const struct imx_media_pixfmt *cc, *cc_src;
 
@@ -246,8 +245,7 @@ static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
 		}
 	}
 
-	imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, compose,
-				      &fmt_src->format, cc);
+	imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src->format, cc);
 
 	return 0;
 }
@@ -265,7 +263,7 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
 	if (ret)
 		return ret;
 
-	return __capture_try_fmt_vid_cap(priv, &fmt_src, f, NULL);
+	return __capture_try_fmt_vid_cap(priv, &fmt_src, f);
 }
 
 static int capture_s_fmt_vid_cap(struct file *file, void *fh,
@@ -273,7 +271,6 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
 {
 	struct capture_priv *priv = video_drvdata(file);
 	struct v4l2_subdev_format fmt_src;
-	struct v4l2_rect compose;
 	int ret;
 
 	if (vb2_is_busy(&priv->q)) {
@@ -287,14 +284,17 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
 	if (ret)
 		return ret;
 
-	ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f, &compose);
+	ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f);
 	if (ret)
 		return ret;
 
 	priv->vdev.fmt.fmt.pix = f->fmt.pix;
 	priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
 					      CS_SEL_ANY, true);
-	priv->vdev.compose = compose;
+	priv->vdev.compose.left = 0;
+	priv->vdev.compose.top = 0;
+	priv->vdev.compose.width = fmt_src.format.width;
+	priv->vdev.compose.height = fmt_src.format.height;
 
 	return 0;
 }
@@ -655,8 +655,7 @@ static struct video_device capture_videodev = {
 };
 
 void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
-					 const struct v4l2_pix_format *pix,
-					 const struct v4l2_rect *compose)
+					 struct v4l2_pix_format *pix)
 {
 	struct capture_priv *priv = to_capture_priv(vdev);
 
@@ -664,7 +663,6 @@ void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
 	priv->vdev.fmt.fmt.pix = *pix;
 	priv->vdev.cc = imx_media_find_format(pix->pixelformat, CS_SEL_ANY,
 					      true);
-	priv->vdev.compose = *compose;
 	mutex_unlock(&priv->mutex);
 }
 EXPORT_SYMBOL_GPL(imx_media_capture_device_set_format);
@@ -770,8 +768,10 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
 	}
 
 	vdev->fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-	imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix, &vdev->compose,
+	imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix,
 				      &fmt_src.format, NULL);
+	vdev->compose.width = fmt_src.format.width;
+	vdev->compose.height = fmt_src.format.height;
 	vdev->cc = imx_media_find_format(vdev->fmt.fmt.pix.pixelformat,
 					 CS_SEL_ANY, false);
 
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index c70fa6b509ae..09b89a72eaa2 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1512,7 +1512,6 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
 	struct v4l2_pix_format vdev_fmt;
 	struct v4l2_mbus_framefmt *fmt;
 	struct v4l2_rect *crop, *compose;
-	struct v4l2_rect vdev_compose;
 	int ret;
 
 	if (sdformat->pad >= CSI_NUM_PADS)
@@ -1568,11 +1567,11 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
 	priv->cc[sdformat->pad] = cc;
 
 	/* propagate IDMAC output pad format to capture device */
-	imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
+	imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
 				      &priv->format_mbus[CSI_SRC_PAD_IDMAC],
 				      priv->cc[CSI_SRC_PAD_IDMAC]);
 	mutex_unlock(&priv->lock);
-	imx_media_capture_device_set_format(vdev, &vdev_fmt, &vdev_compose);
+	imx_media_capture_device_set_format(vdev, &vdev_fmt);
 
 	return 0;
 out:
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 8a6e57652402..a80ef0b087ad 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -577,8 +577,7 @@ void imx_media_fill_default_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
 EXPORT_SYMBOL_GPL(imx_media_fill_default_mbus_fields);
 
 int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
-				  struct v4l2_rect *compose,
-				  const struct v4l2_mbus_framefmt *mbus,
+				  struct v4l2_mbus_framefmt *mbus,
 				  const struct imx_media_pixfmt *cc)
 {
 	u32 width;
@@ -625,17 +624,6 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
 	pix->sizeimage = cc->planar ? ((stride * pix->height * cc->bpp) >> 3) :
 			 stride * pix->height;
 
-	/*
-	 * set capture compose rectangle, which is fixed to the
-	 * source subdevice mbus format.
-	 */
-	if (compose) {
-		compose->left = 0;
-		compose->top = 0;
-		compose->width = mbus->width;
-		compose->height = mbus->height;
-	}
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(imx_media_mbus_fmt_to_pix_fmt);
@@ -647,11 +635,13 @@ int imx_media_mbus_fmt_to_ipu_image(struct ipu_image *image,
 
 	memset(image, 0, sizeof(*image));
 
-	ret = imx_media_mbus_fmt_to_pix_fmt(&image->pix, &image->rect,
-					    mbus, NULL);
+	ret = imx_media_mbus_fmt_to_pix_fmt(&image->pix, mbus, NULL);
 	if (ret)
 		return ret;
 
+	image->rect.width = mbus->width;
+	image->rect.height = mbus->height;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(imx_media_mbus_fmt_to_ipu_image);
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 71e20f53ed7b..ddea972b7bc4 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -174,8 +174,7 @@ void imx_media_fill_default_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
 					struct v4l2_mbus_framefmt *fmt,
 					bool ic_route);
 int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
-				  struct v4l2_rect *compose,
-				  const struct v4l2_mbus_framefmt *mbus,
+				  struct v4l2_mbus_framefmt *mbus,
 				  const struct imx_media_pixfmt *cc);
 int imx_media_mbus_fmt_to_ipu_image(struct ipu_image *image,
 				    struct v4l2_mbus_framefmt *mbus);
@@ -280,8 +279,7 @@ 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);
 void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
-					 const struct v4l2_pix_format *pix,
-					 const struct v4l2_rect *compose);
+					 struct v4l2_pix_format *pix);
 void imx_media_capture_device_error(struct imx_media_video_dev *vdev);
 
 /* subdev group ids */
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index f2037aba6e0e..e2622c05a793 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -1035,7 +1035,6 @@ static int imx7_csi_set_fmt(struct v4l2_subdev *sd,
 	const struct imx_media_pixfmt *outcc;
 	struct v4l2_mbus_framefmt *outfmt;
 	struct v4l2_pix_format vdev_fmt;
-	struct v4l2_rect vdev_compose;
 	const struct imx_media_pixfmt *cc;
 	struct v4l2_mbus_framefmt *fmt;
 	struct v4l2_subdev_format format;
@@ -1086,11 +1085,11 @@ static int imx7_csi_set_fmt(struct v4l2_subdev *sd,
 	csi->cc[sdformat->pad] = cc;
 
 	/* propagate output pad format to capture device */
-	imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
+	imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
 				      &csi->format_mbus[IMX7_CSI_PAD_SRC],
 				      csi->cc[IMX7_CSI_PAD_SRC]);
 	mutex_unlock(&csi->lock);
-	imx_media_capture_device_set_format(vdev, &vdev_fmt, &vdev_compose);
+	imx_media_capture_device_set_format(vdev, &vdev_fmt);
 
 	return 0;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 5/8] media: staging/imx: Remove capture_device_set_format
  2019-04-30 22:50 [PATCH v3 0/8] Switch to sync registration for IPU subdevs Steve Longerbeam
                   ` (3 preceding siblings ...)
  2019-04-30 22:50 ` [PATCH v3 4/8] Revert "media: imx: Set capture compose rectangle in capture_device_set_format" Steve Longerbeam
@ 2019-04-30 22:50 ` Steve Longerbeam
  2019-05-02 10:01   ` Rui Miguel Silva
  2019-04-30 22:50 ` [PATCH v3 6/8] media: staging/imx: Re-organize modules Steve Longerbeam
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Steve Longerbeam @ 2019-04-30 22:50 UTC (permalink / raw)
  To: linux-media
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Rui Miguel Silva, open list:STAGING SUBSYSTEM,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list

Don't propagate the source pad format to the connected capture device.
It's now the responsibility of userspace to call VIDIOC_S_FMT on the
capture device to ensure the capture format and compose rectangle
are compatible with the connected source. To check this, validate
the capture format with the source before streaming starts.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-ic-prpencvf.c   | 16 +----
 drivers/staging/media/imx/imx-media-capture.c | 64 +++++++++++++------
 drivers/staging/media/imx/imx-media-csi.c     | 16 +----
 drivers/staging/media/imx/imx-media.h         |  2 -
 drivers/staging/media/imx/imx7-media-csi.c    | 17 +----
 5 files changed, 50 insertions(+), 65 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index afaa3a8b15e9..63334fd61492 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -906,9 +906,7 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
 		       struct v4l2_subdev_format *sdformat)
 {
 	struct prp_priv *priv = sd_to_priv(sd);
-	struct imx_media_video_dev *vdev = priv->vdev;
 	const struct imx_media_pixfmt *cc;
-	struct v4l2_pix_format vdev_fmt;
 	struct v4l2_mbus_framefmt *fmt;
 	int ret = 0;
 
@@ -945,19 +943,9 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
 			priv->cc[PRPENCVF_SRC_PAD] = outcc;
 	}
 
-	if (sdformat->which == V4L2_SUBDEV_FORMAT_TRY)
-		goto out;
-
-	priv->cc[sdformat->pad] = cc;
+	if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+		priv->cc[sdformat->pad] = cc;
 
-	/* propagate output pad format to capture device */
-	imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
-				      &priv->format_mbus[PRPENCVF_SRC_PAD],
-				      priv->cc[PRPENCVF_SRC_PAD]);
-	mutex_unlock(&priv->lock);
-	imx_media_capture_device_set_format(vdev, &vdev_fmt);
-
-	return 0;
 out:
 	mutex_unlock(&priv->lock);
 	return ret;
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 555f6204660b..b77a67bda47c 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -205,7 +205,8 @@ static int capture_g_fmt_vid_cap(struct file *file, void *fh,
 
 static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
 				     struct v4l2_subdev_format *fmt_src,
-				     struct v4l2_format *f)
+				     struct v4l2_format *f,
+				     struct v4l2_rect *compose)
 {
 	const struct imx_media_pixfmt *cc, *cc_src;
 
@@ -247,6 +248,13 @@ static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
 
 	imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src->format, cc);
 
+	if (compose) {
+		compose->left = 0;
+		compose->top = 0;
+		compose->width = fmt_src->format.width;
+		compose->height = fmt_src->format.height;
+	}
+
 	return 0;
 }
 
@@ -263,7 +271,7 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
 	if (ret)
 		return ret;
 
-	return __capture_try_fmt_vid_cap(priv, &fmt_src, f);
+	return __capture_try_fmt_vid_cap(priv, &fmt_src, f, NULL);
 }
 
 static int capture_s_fmt_vid_cap(struct file *file, void *fh,
@@ -271,6 +279,7 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
 {
 	struct capture_priv *priv = video_drvdata(file);
 	struct v4l2_subdev_format fmt_src;
+	struct v4l2_rect compose;
 	int ret;
 
 	if (vb2_is_busy(&priv->q)) {
@@ -284,17 +293,14 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
 	if (ret)
 		return ret;
 
-	ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f);
+	ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f, &compose);
 	if (ret)
 		return ret;
 
 	priv->vdev.fmt.fmt.pix = f->fmt.pix;
 	priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
 					      CS_SEL_ANY, true);
-	priv->vdev.compose.left = 0;
-	priv->vdev.compose.top = 0;
-	priv->vdev.compose.width = fmt_src.format.width;
-	priv->vdev.compose.height = fmt_src.format.height;
+	priv->vdev.compose = compose;
 
 	return 0;
 }
@@ -524,6 +530,33 @@ static void capture_buf_queue(struct vb2_buffer *vb)
 	spin_unlock_irqrestore(&priv->q_lock, flags);
 }
 
+static int capture_validate_fmt(struct capture_priv *priv)
+{
+	struct v4l2_subdev_format fmt_src;
+	struct v4l2_rect compose;
+	struct v4l2_format f;
+	int ret;
+
+	fmt_src.pad = priv->src_sd_pad;
+	fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+	ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
+	if (ret)
+		return ret;
+
+	v4l2_fill_pix_format(&f.fmt.pix, &fmt_src.format);
+
+	ret = __capture_try_fmt_vid_cap(priv, &fmt_src, &f, &compose);
+	if (ret)
+		return ret;
+
+	return (priv->vdev.fmt.fmt.pix.width != f.fmt.pix.width ||
+		priv->vdev.fmt.fmt.pix.height != f.fmt.pix.height ||
+		priv->vdev.cc->cs !=
+		ipu_pixelformat_to_colorspace(f.fmt.pix.pixelformat) ||
+		priv->vdev.compose.width != compose.width ||
+		priv->vdev.compose.height != compose.height) ? -EINVAL : 0;
+}
+
 static int capture_start_streaming(struct vb2_queue *vq, unsigned int count)
 {
 	struct capture_priv *priv = vb2_get_drv_priv(vq);
@@ -531,6 +564,10 @@ static int capture_start_streaming(struct vb2_queue *vq, unsigned int count)
 	unsigned long flags;
 	int ret;
 
+	ret = capture_validate_fmt(priv);
+	if (ret)
+		goto return_bufs;
+
 	ret = imx_media_pipeline_set_stream(priv->md, &priv->src_sd->entity,
 					    true);
 	if (ret) {
@@ -654,19 +691,6 @@ static struct video_device capture_videodev = {
 	.device_caps	= V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING,
 };
 
-void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
-					 struct v4l2_pix_format *pix)
-{
-	struct capture_priv *priv = to_capture_priv(vdev);
-
-	mutex_lock(&priv->mutex);
-	priv->vdev.fmt.fmt.pix = *pix;
-	priv->vdev.cc = imx_media_find_format(pix->pixelformat, CS_SEL_ANY,
-					      true);
-	mutex_unlock(&priv->mutex);
-}
-EXPORT_SYMBOL_GPL(imx_media_capture_device_set_format);
-
 struct imx_media_buffer *
 imx_media_capture_device_next_buf(struct imx_media_video_dev *vdev)
 {
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 09b89a72eaa2..68c2b1a3066a 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1506,10 +1506,8 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
 		       struct v4l2_subdev_format *sdformat)
 {
 	struct csi_priv *priv = v4l2_get_subdevdata(sd);
-	struct imx_media_video_dev *vdev = priv->vdev;
 	struct v4l2_fwnode_endpoint upstream_ep = { .bus_type = 0 };
 	const struct imx_media_pixfmt *cc;
-	struct v4l2_pix_format vdev_fmt;
 	struct v4l2_mbus_framefmt *fmt;
 	struct v4l2_rect *crop, *compose;
 	int ret;
@@ -1561,19 +1559,9 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
 		}
 	}
 
-	if (sdformat->which == V4L2_SUBDEV_FORMAT_TRY)
-		goto out;
-
-	priv->cc[sdformat->pad] = cc;
+	if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+		priv->cc[sdformat->pad] = cc;
 
-	/* propagate IDMAC output pad format to capture device */
-	imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
-				      &priv->format_mbus[CSI_SRC_PAD_IDMAC],
-				      priv->cc[CSI_SRC_PAD_IDMAC]);
-	mutex_unlock(&priv->lock);
-	imx_media_capture_device_set_format(vdev, &vdev_fmt);
-
-	return 0;
 out:
 	mutex_unlock(&priv->lock);
 	return ret;
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index ddea972b7bc4..f928697b0491 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -278,8 +278,6 @@ int imx_media_capture_device_register(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);
-void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
-					 struct v4l2_pix_format *pix);
 void imx_media_capture_device_error(struct imx_media_video_dev *vdev);
 
 /* subdev group ids */
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index e2622c05a793..0ec4c57259f9 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -1031,10 +1031,8 @@ static int imx7_csi_set_fmt(struct v4l2_subdev *sd,
 			    struct v4l2_subdev_format *sdformat)
 {
 	struct imx7_csi *csi = v4l2_get_subdevdata(sd);
-	struct imx_media_video_dev *vdev = csi->vdev;
 	const struct imx_media_pixfmt *outcc;
 	struct v4l2_mbus_framefmt *outfmt;
-	struct v4l2_pix_format vdev_fmt;
 	const struct imx_media_pixfmt *cc;
 	struct v4l2_mbus_framefmt *fmt;
 	struct v4l2_subdev_format format;
@@ -1079,19 +1077,8 @@ static int imx7_csi_set_fmt(struct v4l2_subdev *sd,
 			csi->cc[IMX7_CSI_PAD_SRC] = outcc;
 	}
 
-	if (sdformat->which == V4L2_SUBDEV_FORMAT_TRY)
-		goto out_unlock;
-
-	csi->cc[sdformat->pad] = cc;
-
-	/* propagate output pad format to capture device */
-	imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
-				      &csi->format_mbus[IMX7_CSI_PAD_SRC],
-				      csi->cc[IMX7_CSI_PAD_SRC]);
-	mutex_unlock(&csi->lock);
-	imx_media_capture_device_set_format(vdev, &vdev_fmt);
-
-	return 0;
+	if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+		csi->cc[sdformat->pad] = cc;
 
 out_unlock:
 	mutex_unlock(&csi->lock);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 6/8] media: staging/imx: Re-organize modules
  2019-04-30 22:50 [PATCH v3 0/8] Switch to sync registration for IPU subdevs Steve Longerbeam
                   ` (4 preceding siblings ...)
  2019-04-30 22:50 ` [PATCH v3 5/8] media: staging/imx: Remove capture_device_set_format Steve Longerbeam
@ 2019-04-30 22:50 ` Steve Longerbeam
  2019-04-30 22:50 ` [PATCH v3 7/8] media: staging/imx: Improve pipeline searching Steve Longerbeam
  2019-04-30 22:50 ` [PATCH v3 8/8] media: staging/imx: Don't set driver data for v4l2_dev Steve Longerbeam
  7 siblings, 0 replies; 11+ messages in thread
From: Steve Longerbeam @ 2019-04-30 22:50 UTC (permalink / raw)
  To: linux-media
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Rui Miguel Silva, open list:STAGING SUBSYSTEM,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list

Re-organize modules, and which objects are linked into those modules, so
that:

- imx6-media (renamed from imx-media) is the media driver module for
  imx5/6 only, and has no symbol exports.

- imx6-media-csi (renamed from imx-media-csi) is the subdev driver
  module for imx5/6 CSI. It is now linked direcly with imx-media-fim,
  since only the imx5/6 CSI makes use of the frame interval monitor.

- imx-media-common now only contains common code between imx5/6 and imx7
  media drivers. It contains imx-media-utils, imx-media-of,
  imx-media-dev-common, and imx-media-capture. In order to acheive that,
  some functions common to imx5/6 and imx7 have been moved out of
  imx-media-dev.c and into imx-media-dev-common.c.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/Makefile            |  14 +-
 .../staging/media/imx/imx-media-dev-common.c  | 345 +++++++++++++++++-
 drivers/staging/media/imx/imx-media-dev.c     | 340 +----------------
 drivers/staging/media/imx/imx-media-fim.c     |   5 -
 drivers/staging/media/imx/imx-media-of.c      |   3 +
 drivers/staging/media/imx/imx-media.h         |  16 +-
 drivers/staging/media/imx/imx7-media-csi.c    |   4 +-
 7 files changed, 369 insertions(+), 358 deletions(-)

diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
index 86f0c81b6a3b..aa6c4b4ad37e 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -1,14 +1,16 @@
 # SPDX-License-Identifier: GPL-2.0
-imx-media-objs := imx-media-dev.o imx-media-internal-sd.o imx-media-of.o \
+imx6-media-objs := imx-media-dev.o imx-media-internal-sd.o \
 	imx-ic-common.o imx-ic-prp.o imx-ic-prpencvf.o imx-media-vdic.o
-imx-media-objs += imx-media-dev-common.o
-imx-media-common-objs := imx-media-utils.o imx-media-fim.o
 
-obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media.o
+imx-media-common-objs := imx-media-capture.o imx-media-dev-common.o \
+	imx-media-of.o imx-media-utils.o
+
+imx6-media-csi-objs := imx-media-csi.o imx-media-fim.o
+
+obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx6-media.o
 obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-common.o
-obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-capture.o
 
-obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o
+obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-media-csi.o
 obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
 
 obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
diff --git a/drivers/staging/media/imx/imx-media-dev-common.c b/drivers/staging/media/imx/imx-media-dev-common.c
index 6cd93419b81d..89dc4ec8dadb 100644
--- a/drivers/staging/media/imx/imx-media-dev-common.c
+++ b/drivers/staging/media/imx/imx-media-dev-common.c
@@ -8,9 +8,342 @@
 
 #include <linux/of_graph.h>
 #include <linux/of_platform.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-mc.h>
 #include "imx-media.h"
 
-static const struct v4l2_async_notifier_operations imx_media_subdev_ops = {
+static inline struct imx_media_dev *notifier2dev(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct imx_media_dev, notifier);
+}
+
+/* async subdev bound notifier */
+static int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
+				  struct v4l2_subdev *sd,
+				  struct v4l2_async_subdev *asd)
+{
+	v4l2_info(sd->v4l2_dev, "subdev %s bound\n", sd->name);
+
+	return 0;
+}
+
+/*
+ * Create the media links for all subdevs that registered.
+ * Called after all async subdevs have bound.
+ */
+static int imx_media_create_links(struct v4l2_async_notifier *notifier)
+{
+	struct imx_media_dev *imxmd = notifier2dev(notifier);
+	struct v4l2_subdev *sd;
+
+	list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
+		switch (sd->grp_id) {
+		case IMX_MEDIA_GRP_ID_IPU_VDIC:
+		case IMX_MEDIA_GRP_ID_IPU_IC_PRP:
+		case IMX_MEDIA_GRP_ID_IPU_IC_PRPENC:
+		case IMX_MEDIA_GRP_ID_IPU_IC_PRPVF:
+			/*
+			 * links have already been created for the
+			 * sync-registered subdevs.
+			 */
+			break;
+		case IMX_MEDIA_GRP_ID_IPU_CSI0:
+		case IMX_MEDIA_GRP_ID_IPU_CSI1:
+		case IMX_MEDIA_GRP_ID_CSI:
+			imx_media_create_csi_of_links(imxmd, sd);
+			break;
+		default:
+			/*
+			 * if this subdev has fwnode links, create media
+			 * links for them.
+			 */
+			imx_media_create_of_links(imxmd, sd);
+			break;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * adds given video device to given imx-media source pad vdev list.
+ * Continues upstream from the pad entity's sink pads.
+ */
+static int imx_media_add_vdev_to_pad(struct imx_media_dev *imxmd,
+				     struct imx_media_video_dev *vdev,
+				     struct media_pad *srcpad)
+{
+	struct media_entity *entity = srcpad->entity;
+	struct imx_media_pad_vdev *pad_vdev;
+	struct list_head *pad_vdev_list;
+	struct media_link *link;
+	struct v4l2_subdev *sd;
+	int i, ret;
+
+	/* skip this entity if not a v4l2_subdev */
+	if (!is_media_entity_v4l2_subdev(entity))
+		return 0;
+
+	sd = media_entity_to_v4l2_subdev(entity);
+
+	pad_vdev_list = to_pad_vdev_list(sd, srcpad->index);
+	if (!pad_vdev_list) {
+		v4l2_warn(&imxmd->v4l2_dev, "%s:%u has no vdev list!\n",
+			  entity->name, srcpad->index);
+		/*
+		 * shouldn't happen, but no reason to fail driver load,
+		 * just skip this entity.
+		 */
+		return 0;
+	}
+
+	/* just return if we've been here before */
+	list_for_each_entry(pad_vdev, pad_vdev_list, list) {
+		if (pad_vdev->vdev == vdev)
+			return 0;
+	}
+
+	dev_dbg(imxmd->md.dev, "adding %s to pad %s:%u\n",
+		vdev->vfd->entity.name, entity->name, srcpad->index);
+
+	pad_vdev = devm_kzalloc(imxmd->md.dev, sizeof(*pad_vdev), GFP_KERNEL);
+	if (!pad_vdev)
+		return -ENOMEM;
+
+	/* attach this vdev to this pad */
+	pad_vdev->vdev = vdev;
+	list_add_tail(&pad_vdev->list, pad_vdev_list);
+
+	/* move upstream from this entity's sink pads */
+	for (i = 0; i < entity->num_pads; i++) {
+		struct media_pad *pad = &entity->pads[i];
+
+		if (!(pad->flags & MEDIA_PAD_FL_SINK))
+			continue;
+
+		list_for_each_entry(link, &entity->links, list) {
+			if (link->sink != pad)
+				continue;
+			ret = imx_media_add_vdev_to_pad(imxmd, vdev,
+							link->source);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * For every subdevice, allocate an array of list_head's, one list_head
+ * for each pad, to hold the list of video devices reachable from that
+ * pad.
+ */
+static int imx_media_alloc_pad_vdev_lists(struct imx_media_dev *imxmd)
+{
+	struct list_head *vdev_lists;
+	struct media_entity *entity;
+	struct v4l2_subdev *sd;
+	int i;
+
+	list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
+		entity = &sd->entity;
+		vdev_lists = devm_kcalloc(imxmd->md.dev,
+					  entity->num_pads, sizeof(*vdev_lists),
+					  GFP_KERNEL);
+		if (!vdev_lists)
+			return -ENOMEM;
+
+		/* attach to the subdev's host private pointer */
+		sd->host_priv = vdev_lists;
+
+		for (i = 0; i < entity->num_pads; i++)
+			INIT_LIST_HEAD(to_pad_vdev_list(sd, i));
+	}
+
+	return 0;
+}
+
+/* form the vdev lists in all imx-media source pads */
+static int imx_media_create_pad_vdev_lists(struct imx_media_dev *imxmd)
+{
+	struct imx_media_video_dev *vdev;
+	struct media_link *link;
+	int ret;
+
+	ret = imx_media_alloc_pad_vdev_lists(imxmd);
+	if (ret)
+		return ret;
+
+	list_for_each_entry(vdev, &imxmd->vdev_list, list) {
+		link = list_first_entry(&vdev->vfd->entity.links,
+					struct media_link, list);
+		ret = imx_media_add_vdev_to_pad(imxmd, vdev, link->source);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/* async subdev complete notifier */
+int imx_media_probe_complete(struct v4l2_async_notifier *notifier)
+{
+	struct imx_media_dev *imxmd = notifier2dev(notifier);
+	int ret;
+
+	mutex_lock(&imxmd->mutex);
+
+	ret = imx_media_create_links(notifier);
+	if (ret)
+		goto unlock;
+
+	ret = imx_media_create_pad_vdev_lists(imxmd);
+	if (ret)
+		goto unlock;
+
+	ret = v4l2_device_register_subdev_nodes(&imxmd->v4l2_dev);
+unlock:
+	mutex_unlock(&imxmd->mutex);
+	if (ret)
+		return ret;
+
+	return media_device_register(&imxmd->md);
+}
+EXPORT_SYMBOL_GPL(imx_media_probe_complete);
+
+/*
+ * adds controls to a video device from an entity subdevice.
+ * Continues upstream from the entity's sink pads.
+ */
+static int imx_media_inherit_controls(struct imx_media_dev *imxmd,
+				      struct video_device *vfd,
+				      struct media_entity *entity)
+{
+	int i, ret = 0;
+
+	if (is_media_entity_v4l2_subdev(entity)) {
+		struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+
+		dev_dbg(imxmd->md.dev,
+			"adding controls to %s from %s\n",
+			vfd->entity.name, sd->entity.name);
+
+		ret = v4l2_ctrl_add_handler(vfd->ctrl_handler,
+					    sd->ctrl_handler,
+					    NULL, true);
+		if (ret)
+			return ret;
+	}
+
+	/* move upstream */
+	for (i = 0; i < entity->num_pads; i++) {
+		struct media_pad *pad, *spad = &entity->pads[i];
+
+		if (!(spad->flags & MEDIA_PAD_FL_SINK))
+			continue;
+
+		pad = media_entity_remote_pad(spad);
+		if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
+			continue;
+
+		ret = imx_media_inherit_controls(imxmd, vfd, pad->entity);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+static int imx_media_link_notify(struct media_link *link, u32 flags,
+				 unsigned int notification)
+{
+	struct media_entity *source = link->source->entity;
+	struct imx_media_pad_vdev *pad_vdev;
+	struct list_head *pad_vdev_list;
+	struct imx_media_dev *imxmd;
+	struct video_device *vfd;
+	struct v4l2_subdev *sd;
+	int pad_idx, ret;
+
+	ret = v4l2_pipeline_link_notify(link, flags, notification);
+	if (ret)
+		return ret;
+
+	/* don't bother if source is not a subdev */
+	if (!is_media_entity_v4l2_subdev(source))
+		return 0;
+
+	sd = media_entity_to_v4l2_subdev(source);
+	pad_idx = link->source->index;
+
+	imxmd = dev_get_drvdata(sd->v4l2_dev->dev);
+
+	pad_vdev_list = to_pad_vdev_list(sd, pad_idx);
+	if (!pad_vdev_list) {
+		/* nothing to do if source sd has no pad vdev list */
+		return 0;
+	}
+
+	/*
+	 * Before disabling a link, reset controls for all video
+	 * devices reachable from this link.
+	 *
+	 * After enabling a link, refresh controls for all video
+	 * devices reachable from this link.
+	 */
+	if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH &&
+	    !(flags & MEDIA_LNK_FL_ENABLED)) {
+		list_for_each_entry(pad_vdev, pad_vdev_list, list) {
+			vfd = pad_vdev->vdev->vfd;
+			dev_dbg(imxmd->md.dev,
+				"reset controls for %s\n",
+				vfd->entity.name);
+			v4l2_ctrl_handler_free(vfd->ctrl_handler);
+			v4l2_ctrl_handler_init(vfd->ctrl_handler, 0);
+		}
+	} else if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
+		   (link->flags & MEDIA_LNK_FL_ENABLED)) {
+		list_for_each_entry(pad_vdev, pad_vdev_list, list) {
+			vfd = pad_vdev->vdev->vfd;
+			dev_dbg(imxmd->md.dev,
+				"refresh controls for %s\n",
+				vfd->entity.name);
+			ret = imx_media_inherit_controls(imxmd, vfd,
+							 &vfd->entity);
+			if (ret)
+				break;
+		}
+	}
+
+	return ret;
+}
+
+static void imx_media_notify(struct v4l2_subdev *sd, unsigned int notification,
+			     void *arg)
+{
+	struct media_entity *entity = &sd->entity;
+	int i;
+
+	if (notification != V4L2_DEVICE_NOTIFY_EVENT)
+		return;
+
+	for (i = 0; i < entity->num_pads; i++) {
+		struct media_pad *pad = &entity->pads[i];
+		struct imx_media_pad_vdev *pad_vdev;
+		struct list_head *pad_vdev_list;
+
+		pad_vdev_list = to_pad_vdev_list(sd, pad->index);
+		if (!pad_vdev_list)
+			continue;
+		list_for_each_entry(pad_vdev, pad_vdev_list, list)
+			v4l2_event_queue(pad_vdev->vdev->vfd, arg);
+	}
+}
+
+static const struct v4l2_async_notifier_operations imx_media_notifier_ops = {
 	.bound = imx_media_subdev_bound,
 	.complete = imx_media_probe_complete,
 };
@@ -19,7 +352,8 @@ static const struct media_device_ops imx_media_md_ops = {
 	.link_notify = imx_media_link_notify,
 };
 
-struct imx_media_dev *imx_media_dev_init(struct device *dev)
+struct imx_media_dev *imx_media_dev_init(struct device *dev,
+					 const struct media_device_ops *ops)
 {
 	struct imx_media_dev *imxmd;
 	int ret;
@@ -31,7 +365,7 @@ struct imx_media_dev *imx_media_dev_init(struct device *dev)
 	dev_set_drvdata(dev, imxmd);
 
 	strscpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model));
-	imxmd->md.ops = &imx_media_md_ops;
+	imxmd->md.ops = ops ? ops : &imx_media_md_ops;
 	imxmd->md.dev = dev;
 
 	mutex_init(&imxmd->mutex);
@@ -65,7 +399,8 @@ struct imx_media_dev *imx_media_dev_init(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(imx_media_dev_init);
 
-int imx_media_dev_notifier_register(struct imx_media_dev *imxmd)
+int imx_media_dev_notifier_register(struct imx_media_dev *imxmd,
+			    const struct v4l2_async_notifier_operations *ops)
 {
 	int ret;
 
@@ -76,7 +411,7 @@ int imx_media_dev_notifier_register(struct imx_media_dev *imxmd)
 	}
 
 	/* prepare the async subdev notifier and register it */
-	imxmd->notifier.ops = &imx_media_subdev_ops;
+	imxmd->notifier.ops = ops ? ops : &imx_media_notifier_ops;
 	ret = v4l2_async_notifier_register(&imxmd->v4l2_dev,
 					   &imxmd->notifier);
 	if (ret) {
diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
index 55181756a940..422afb31f4ca 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -1,29 +1,18 @@
 /*
  * V4L2 Media Controller Driver for Freescale i.MX5/6 SOC
  *
- * Copyright (c) 2016 Mentor Graphics Inc.
+ * Copyright (c) 2016-2019 Mentor Graphics Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
  * (at your option) any later version.
  */
-#include <linux/delay.h>
 #include <linux/fs.h>
 #include <linux/module.h>
-#include <linux/of_graph.h>
-#include <linux/of_platform.h>
-#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
-#include <linux/sched.h>
-#include <linux/slab.h>
-#include <linux/spinlock.h>
-#include <linux/timer.h>
-#include <media/v4l2-ctrls.h>
+#include <media/v4l2-async.h>
 #include <media/v4l2-event.h>
-#include <media/v4l2-ioctl.h>
-#include <media/v4l2-mc.h>
-#include <video/imx-ipu-v3.h>
 #include <media/imx.h>
 #include "imx-media.h"
 
@@ -33,9 +22,9 @@ static inline struct imx_media_dev *notifier2dev(struct v4l2_async_notifier *n)
 }
 
 /* async subdev bound notifier */
-int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
-			   struct v4l2_subdev *sd,
-			   struct v4l2_async_subdev *asd)
+static int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
+				  struct v4l2_subdev *sd,
+				  struct v4l2_async_subdev *asd)
 {
 	struct imx_media_dev *imxmd = notifier2dev(notifier);
 	int ret;
@@ -52,318 +41,11 @@ int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
 	return 0;
 }
 
-/*
- * Create the media links for all subdevs that registered.
- * Called after all async subdevs have bound.
- */
-static int imx_media_create_links(struct v4l2_async_notifier *notifier)
-{
-	struct imx_media_dev *imxmd = notifier2dev(notifier);
-	struct v4l2_subdev *sd;
-
-	list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
-		switch (sd->grp_id) {
-		case IMX_MEDIA_GRP_ID_IPU_VDIC:
-		case IMX_MEDIA_GRP_ID_IPU_IC_PRP:
-		case IMX_MEDIA_GRP_ID_IPU_IC_PRPENC:
-		case IMX_MEDIA_GRP_ID_IPU_IC_PRPVF:
-			/*
-			 * links have already been created for the
-			 * sync-registered subdevs.
-			 */
-			break;
-		case IMX_MEDIA_GRP_ID_IPU_CSI0:
-		case IMX_MEDIA_GRP_ID_IPU_CSI1:
-		case IMX_MEDIA_GRP_ID_CSI:
-			imx_media_create_csi_of_links(imxmd, sd);
-			break;
-		default:
-			/*
-			 * if this subdev has fwnode links, create media
-			 * links for them.
-			 */
-			imx_media_create_of_links(imxmd, sd);
-			break;
-		}
-	}
-
-	return 0;
-}
-
-/*
- * adds given video device to given imx-media source pad vdev list.
- * Continues upstream from the pad entity's sink pads.
- */
-static int imx_media_add_vdev_to_pad(struct imx_media_dev *imxmd,
-				     struct imx_media_video_dev *vdev,
-				     struct media_pad *srcpad)
-{
-	struct media_entity *entity = srcpad->entity;
-	struct imx_media_pad_vdev *pad_vdev;
-	struct list_head *pad_vdev_list;
-	struct media_link *link;
-	struct v4l2_subdev *sd;
-	int i, ret;
-
-	/* skip this entity if not a v4l2_subdev */
-	if (!is_media_entity_v4l2_subdev(entity))
-		return 0;
-
-	sd = media_entity_to_v4l2_subdev(entity);
-
-	pad_vdev_list = to_pad_vdev_list(sd, srcpad->index);
-	if (!pad_vdev_list) {
-		v4l2_warn(&imxmd->v4l2_dev, "%s:%u has no vdev list!\n",
-			  entity->name, srcpad->index);
-		/*
-		 * shouldn't happen, but no reason to fail driver load,
-		 * just skip this entity.
-		 */
-		return 0;
-	}
-
-	/* just return if we've been here before */
-	list_for_each_entry(pad_vdev, pad_vdev_list, list) {
-		if (pad_vdev->vdev == vdev)
-			return 0;
-	}
-
-	dev_dbg(imxmd->md.dev, "adding %s to pad %s:%u\n",
-		vdev->vfd->entity.name, entity->name, srcpad->index);
-
-	pad_vdev = devm_kzalloc(imxmd->md.dev, sizeof(*pad_vdev), GFP_KERNEL);
-	if (!pad_vdev)
-		return -ENOMEM;
-
-	/* attach this vdev to this pad */
-	pad_vdev->vdev = vdev;
-	list_add_tail(&pad_vdev->list, pad_vdev_list);
-
-	/* move upstream from this entity's sink pads */
-	for (i = 0; i < entity->num_pads; i++) {
-		struct media_pad *pad = &entity->pads[i];
-
-		if (!(pad->flags & MEDIA_PAD_FL_SINK))
-			continue;
-
-		list_for_each_entry(link, &entity->links, list) {
-			if (link->sink != pad)
-				continue;
-			ret = imx_media_add_vdev_to_pad(imxmd, vdev,
-							link->source);
-			if (ret)
-				return ret;
-		}
-	}
-
-	return 0;
-}
-
-/*
- * For every subdevice, allocate an array of list_head's, one list_head
- * for each pad, to hold the list of video devices reachable from that
- * pad.
- */
-static int imx_media_alloc_pad_vdev_lists(struct imx_media_dev *imxmd)
-{
-	struct list_head *vdev_lists;
-	struct media_entity *entity;
-	struct v4l2_subdev *sd;
-	int i;
-
-	list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
-		entity = &sd->entity;
-		vdev_lists = devm_kcalloc(imxmd->md.dev,
-					  entity->num_pads, sizeof(*vdev_lists),
-					  GFP_KERNEL);
-		if (!vdev_lists)
-			return -ENOMEM;
-
-		/* attach to the subdev's host private pointer */
-		sd->host_priv = vdev_lists;
-
-		for (i = 0; i < entity->num_pads; i++)
-			INIT_LIST_HEAD(to_pad_vdev_list(sd, i));
-	}
-
-	return 0;
-}
-
-/* form the vdev lists in all imx-media source pads */
-static int imx_media_create_pad_vdev_lists(struct imx_media_dev *imxmd)
-{
-	struct imx_media_video_dev *vdev;
-	struct media_link *link;
-	int ret;
-
-	ret = imx_media_alloc_pad_vdev_lists(imxmd);
-	if (ret)
-		return ret;
-
-	list_for_each_entry(vdev, &imxmd->vdev_list, list) {
-		link = list_first_entry(&vdev->vfd->entity.links,
-					struct media_link, list);
-		ret = imx_media_add_vdev_to_pad(imxmd, vdev, link->source);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
 /* async subdev complete notifier */
-int imx_media_probe_complete(struct v4l2_async_notifier *notifier)
-{
-	struct imx_media_dev *imxmd = notifier2dev(notifier);
-	int ret;
-
-	mutex_lock(&imxmd->mutex);
-
-	ret = imx_media_create_links(notifier);
-	if (ret)
-		goto unlock;
-
-	ret = imx_media_create_pad_vdev_lists(imxmd);
-	if (ret)
-		goto unlock;
-
-	ret = v4l2_device_register_subdev_nodes(&imxmd->v4l2_dev);
-unlock:
-	mutex_unlock(&imxmd->mutex);
-	if (ret)
-		return ret;
-
-	return media_device_register(&imxmd->md);
-}
-
-/*
- * adds controls to a video device from an entity subdevice.
- * Continues upstream from the entity's sink pads.
- */
-static int imx_media_inherit_controls(struct imx_media_dev *imxmd,
-				      struct video_device *vfd,
-				      struct media_entity *entity)
-{
-	int i, ret = 0;
-
-	if (is_media_entity_v4l2_subdev(entity)) {
-		struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
-
-		dev_dbg(imxmd->md.dev,
-			"adding controls to %s from %s\n",
-			vfd->entity.name, sd->entity.name);
-
-		ret = v4l2_ctrl_add_handler(vfd->ctrl_handler,
-					    sd->ctrl_handler,
-					    NULL, true);
-		if (ret)
-			return ret;
-	}
-
-	/* move upstream */
-	for (i = 0; i < entity->num_pads; i++) {
-		struct media_pad *pad, *spad = &entity->pads[i];
-
-		if (!(spad->flags & MEDIA_PAD_FL_SINK))
-			continue;
-
-		pad = media_entity_remote_pad(spad);
-		if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
-			continue;
-
-		ret = imx_media_inherit_controls(imxmd, vfd, pad->entity);
-		if (ret)
-			break;
-	}
-
-	return ret;
-}
-
-int imx_media_link_notify(struct media_link *link, u32 flags,
-			  unsigned int notification)
-{
-	struct media_entity *source = link->source->entity;
-	struct imx_media_pad_vdev *pad_vdev;
-	struct list_head *pad_vdev_list;
-	struct imx_media_dev *imxmd;
-	struct video_device *vfd;
-	struct v4l2_subdev *sd;
-	int pad_idx, ret;
-
-	ret = v4l2_pipeline_link_notify(link, flags, notification);
-	if (ret)
-		return ret;
-
-	/* don't bother if source is not a subdev */
-	if (!is_media_entity_v4l2_subdev(source))
-		return 0;
-
-	sd = media_entity_to_v4l2_subdev(source);
-	pad_idx = link->source->index;
-
-	imxmd = dev_get_drvdata(sd->v4l2_dev->dev);
-
-	pad_vdev_list = to_pad_vdev_list(sd, pad_idx);
-	if (!pad_vdev_list) {
-		/* shouldn't happen, but no reason to fail link setup */
-		return 0;
-	}
-
-	/*
-	 * Before disabling a link, reset controls for all video
-	 * devices reachable from this link.
-	 *
-	 * After enabling a link, refresh controls for all video
-	 * devices reachable from this link.
-	 */
-	if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH &&
-	    !(flags & MEDIA_LNK_FL_ENABLED)) {
-		list_for_each_entry(pad_vdev, pad_vdev_list, list) {
-			vfd = pad_vdev->vdev->vfd;
-			dev_dbg(imxmd->md.dev,
-				"reset controls for %s\n",
-				vfd->entity.name);
-			v4l2_ctrl_handler_free(vfd->ctrl_handler);
-			v4l2_ctrl_handler_init(vfd->ctrl_handler, 0);
-		}
-	} else if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
-		   (link->flags & MEDIA_LNK_FL_ENABLED)) {
-		list_for_each_entry(pad_vdev, pad_vdev_list, list) {
-			vfd = pad_vdev->vdev->vfd;
-			dev_dbg(imxmd->md.dev,
-				"refresh controls for %s\n",
-				vfd->entity.name);
-			ret = imx_media_inherit_controls(imxmd, vfd,
-							 &vfd->entity);
-			if (ret)
-				break;
-		}
-	}
-
-	return ret;
-}
-
-void imx_media_notify(struct v4l2_subdev *sd, unsigned int notification,
-		      void *arg)
-{
-	struct media_entity *entity = &sd->entity;
-	int i;
-
-	if (notification != V4L2_DEVICE_NOTIFY_EVENT)
-		return;
-
-	for (i = 0; i < entity->num_pads; i++) {
-		struct media_pad *pad = &entity->pads[i];
-		struct imx_media_pad_vdev *pad_vdev;
-		struct list_head *pad_vdev_list;
-
-		pad_vdev_list = to_pad_vdev_list(sd, pad->index);
-		if (!pad_vdev_list)
-			continue;
-		list_for_each_entry(pad_vdev, pad_vdev_list, list)
-			v4l2_event_queue(pad_vdev->vdev->vfd, arg);
-	}
-}
+static const struct v4l2_async_notifier_operations imx_media_notifier_ops = {
+	.bound = imx_media_subdev_bound,
+	.complete = imx_media_probe_complete,
+};
 
 static int imx_media_probe(struct platform_device *pdev)
 {
@@ -372,7 +54,7 @@ static int imx_media_probe(struct platform_device *pdev)
 	struct imx_media_dev *imxmd;
 	int ret;
 
-	imxmd = imx_media_dev_init(dev);
+	imxmd = imx_media_dev_init(dev, NULL);
 	if (IS_ERR(imxmd))
 		return PTR_ERR(imxmd);
 
@@ -383,7 +65,7 @@ static int imx_media_probe(struct platform_device *pdev)
 		goto cleanup;
 	}
 
-	ret = imx_media_dev_notifier_register(imxmd);
+	ret = imx_media_dev_notifier_register(imxmd, &imx_media_notifier_ops);
 	if (ret)
 		goto cleanup;
 
diff --git a/drivers/staging/media/imx/imx-media-fim.c b/drivers/staging/media/imx/imx-media-fim.c
index 8cf773eef9da..6d283f4ea4d1 100644
--- a/drivers/staging/media/imx/imx-media-fim.c
+++ b/drivers/staging/media/imx/imx-media-fim.c
@@ -420,7 +420,6 @@ void imx_media_fim_eof_monitor(struct imx_media_fim *fim, ktime_t timestamp)
 
 	spin_unlock_irqrestore(&fim->lock, flags);
 }
-EXPORT_SYMBOL_GPL(imx_media_fim_eof_monitor);
 
 /* Called by the subdev in its s_stream callback */
 int imx_media_fim_set_stream(struct imx_media_fim *fim,
@@ -457,7 +456,6 @@ int imx_media_fim_set_stream(struct imx_media_fim *fim,
 	v4l2_ctrl_unlock(fim->ctrl[FIM_CL_ENABLE]);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(imx_media_fim_set_stream);
 
 int imx_media_fim_add_controls(struct imx_media_fim *fim)
 {
@@ -465,7 +463,6 @@ int imx_media_fim_add_controls(struct imx_media_fim *fim)
 	return v4l2_ctrl_add_handler(fim->sd->ctrl_handler,
 				     &fim->ctrl_handler, NULL, false);
 }
-EXPORT_SYMBOL_GPL(imx_media_fim_add_controls);
 
 /* Called by the subdev in its subdev registered callback */
 struct imx_media_fim *imx_media_fim_init(struct v4l2_subdev *sd)
@@ -489,10 +486,8 @@ struct imx_media_fim *imx_media_fim_init(struct v4l2_subdev *sd)
 
 	return fim;
 }
-EXPORT_SYMBOL_GPL(imx_media_fim_init);
 
 void imx_media_fim_free(struct imx_media_fim *fim)
 {
 	v4l2_ctrl_handler_free(&fim->ctrl_handler);
 }
-EXPORT_SYMBOL_GPL(imx_media_fim_free);
diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
index b7034699391d..1b4b5a5aff38 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -77,6 +77,7 @@ int imx_media_add_of_subdevs(struct imx_media_dev *imxmd,
 	of_node_put(csi_np);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(imx_media_add_of_subdevs);
 
 /*
  * Create a single media link to/from sd using a fwnode link.
@@ -146,6 +147,7 @@ int imx_media_create_of_links(struct imx_media_dev *imxmd,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(imx_media_create_of_links);
 
 /*
  * Create media links to the given CSI subdevice's sink pads,
@@ -189,3 +191,4 @@ int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(imx_media_create_csi_of_links);
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index f928697b0491..c0e58d4f2dfb 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -217,18 +217,12 @@ int imx_media_pipeline_set_stream(struct imx_media_dev *imxmd,
 				  struct media_entity *entity,
 				  bool on);
 
-/* imx-media-dev.c */
-int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
-			   struct v4l2_subdev *sd,
-			   struct v4l2_async_subdev *asd);
-int imx_media_link_notify(struct media_link *link, u32 flags,
-			  unsigned int notification);
-void imx_media_notify(struct v4l2_subdev *sd, unsigned int notification,
-		      void *arg);
+/* imx-media-dev-common.c */
 int imx_media_probe_complete(struct v4l2_async_notifier *notifier);
-
-struct imx_media_dev *imx_media_dev_init(struct device *dev);
-int imx_media_dev_notifier_register(struct imx_media_dev *imxmd);
+struct imx_media_dev *imx_media_dev_init(struct device *dev,
+					 const struct media_device_ops *ops);
+int imx_media_dev_notifier_register(struct imx_media_dev *imxmd,
+			    const struct v4l2_async_notifier_operations *ops);
 
 /* imx-media-fim.c */
 struct imx_media_fim;
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 0ec4c57259f9..75514618d021 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -1241,7 +1241,7 @@ static int imx7_csi_probe(struct platform_device *pdev)
 	}
 
 	/* add media device */
-	imxmd = imx_media_dev_init(dev);
+	imxmd = imx_media_dev_init(dev, NULL);
 	if (IS_ERR(imxmd)) {
 		ret = PTR_ERR(imxmd);
 		goto destroy_mutex;
@@ -1252,7 +1252,7 @@ static int imx7_csi_probe(struct platform_device *pdev)
 	if (ret < 0 && ret != -ENODEV && ret != -EEXIST)
 		goto cleanup;
 
-	ret = imx_media_dev_notifier_register(imxmd);
+	ret = imx_media_dev_notifier_register(imxmd, NULL);
 	if (ret < 0)
 		goto cleanup;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 7/8] media: staging/imx: Improve pipeline searching
  2019-04-30 22:50 [PATCH v3 0/8] Switch to sync registration for IPU subdevs Steve Longerbeam
                   ` (5 preceding siblings ...)
  2019-04-30 22:50 ` [PATCH v3 6/8] media: staging/imx: Re-organize modules Steve Longerbeam
@ 2019-04-30 22:50 ` Steve Longerbeam
  2019-04-30 22:50 ` [PATCH v3 8/8] media: staging/imx: Don't set driver data for v4l2_dev Steve Longerbeam
  7 siblings, 0 replies; 11+ messages in thread
From: Steve Longerbeam @ 2019-04-30 22:50 UTC (permalink / raw)
  To: linux-media
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Rui Miguel Silva, open list:STAGING SUBSYSTEM,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list

Export find_pipeline_pad(), renaming to imx_media_pipeline_pad(), and
extend its functionality to allow searching for video devices in the
enabled pipeline in addition to sub-devices.

As part of this:

- Rename imx_media_find_mipi_csi2_channel() to
  imx_media_pipeline_csi2_channel().

- Remove imx_media_find_upstream_pad(), it is redundant now.

- Rename imx_media_find_upstream_subdev() to imx_media_pipeline_subdev()
  with an additional boolean argument for searching upstream or downstream.

- Add imx_media_pipeline_video_device() which is analogous to
  imx_media_pipeline_subdev() but searches for video devices.

- Remove imxmd pointer arg from all of the functions above, it was
  never used in those functions. With that change the i.MX5/6 CSI,
  VDIC, and IC sub-devices no longer require the media_device.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-ic-common.c     |   4 +-
 drivers/staging/media/imx/imx-ic-prp.c        |   4 +-
 drivers/staging/media/imx/imx-ic.h            |   1 -
 drivers/staging/media/imx/imx-media-csi.c     |  13 +-
 drivers/staging/media/imx/imx-media-fim.c     |   4 -
 .../staging/media/imx/imx-media-internal-sd.c |   5 +-
 drivers/staging/media/imx/imx-media-utils.c   | 128 ++++++++++--------
 drivers/staging/media/imx/imx-media-vdic.c    |   5 +-
 drivers/staging/media/imx/imx-media.h         |  20 +--
 drivers/staging/media/imx/imx7-media-csi.c    |   2 +-
 10 files changed, 93 insertions(+), 93 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-common.c b/drivers/staging/media/imx/imx-ic-common.c
index ad0c291db03c..37734984beb4 100644
--- a/drivers/staging/media/imx/imx-ic-common.c
+++ b/drivers/staging/media/imx/imx-ic-common.c
@@ -22,12 +22,11 @@ static struct imx_ic_ops *ic_ops[IC_NUM_OPS] = {
 	[IC_TASK_VIEWFINDER]     = &imx_ic_prpencvf_ops,
 };
 
-struct v4l2_subdev *imx_media_ic_register(struct imx_media_dev *imxmd,
+struct v4l2_subdev *imx_media_ic_register(struct v4l2_device *v4l2_dev,
 					  struct device *ipu_dev,
 					  struct ipu_soc *ipu,
 					  u32 grp_id)
 {
-	struct v4l2_device *v4l2_dev = &imxmd->v4l2_dev;
 	struct imx_ic_priv *priv;
 	int ret;
 
@@ -37,7 +36,6 @@ struct v4l2_subdev *imx_media_ic_register(struct imx_media_dev *imxmd,
 
 	priv->ipu_dev = ipu_dev;
 	priv->ipu = ipu;
-	priv->md = imxmd;
 
 	/* get our IC task id */
 	switch (grp_id) {
diff --git a/drivers/staging/media/imx/imx-ic-prp.c b/drivers/staging/media/imx/imx-ic-prp.c
index 663db200e594..1432776a33f9 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -302,8 +302,8 @@ static int prp_link_validate(struct v4l2_subdev *sd,
 	if (ret)
 		return ret;
 
-	csi = imx_media_find_upstream_subdev(ic_priv->md, &ic_priv->sd.entity,
-					     IMX_MEDIA_GRP_ID_IPU_CSI);
+	csi = imx_media_pipeline_subdev(&ic_priv->sd.entity,
+					IMX_MEDIA_GRP_ID_IPU_CSI, true);
 	if (IS_ERR(csi))
 		csi = NULL;
 
diff --git a/drivers/staging/media/imx/imx-ic.h b/drivers/staging/media/imx/imx-ic.h
index 1dcbb37aeada..ff2f66f11982 100644
--- a/drivers/staging/media/imx/imx-ic.h
+++ b/drivers/staging/media/imx/imx-ic.h
@@ -16,7 +16,6 @@
 struct imx_ic_priv {
 	struct device *ipu_dev;
 	struct ipu_soc *ipu;
-	struct imx_media_dev *md;
 	struct v4l2_subdev sd;
 	int    task_id;
 	void   *task_priv;
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 68c2b1a3066a..555904759078 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -60,7 +60,6 @@ struct csi_skip_desc {
 struct csi_priv {
 	struct device *dev;
 	struct ipu_soc *ipu;
-	struct imx_media_dev *md;
 	struct v4l2_subdev sd;
 	struct media_pad pad[CSI_NUM_PADS];
 	/* the video device at IDMAC output pad */
@@ -182,8 +181,8 @@ static int csi_get_upstream_endpoint(struct csi_priv *priv,
 		 * CSI-2 receiver if it is in the path, otherwise stay
 		 * with video mux.
 		 */
-		sd = imx_media_find_upstream_subdev(priv->md, src,
-						    IMX_MEDIA_GRP_ID_CSI2);
+		sd = imx_media_pipeline_subdev(src, IMX_MEDIA_GRP_ID_CSI2,
+					       true);
 		if (!IS_ERR(sd))
 			src = &sd->entity;
 	}
@@ -197,7 +196,7 @@ static int csi_get_upstream_endpoint(struct csi_priv *priv,
 		src = &priv->sd.entity;
 
 	/* get source pad of entity directly upstream from src */
-	pad = imx_media_find_upstream_pad(priv->md, src, 0);
+	pad = imx_media_pipeline_pad(src, 0, 0, true);
 	if (IS_ERR(pad))
 		return PTR_ERR(pad);
 
@@ -1138,8 +1137,7 @@ static int csi_link_validate(struct v4l2_subdev *sd,
 		 */
 #if 0
 		mutex_unlock(&priv->lock);
-		vc_num = imx_media_find_mipi_csi2_channel(priv->md,
-							  &priv->sd.entity);
+		vc_num = imx_media_find_mipi_csi2_channel(&priv->sd.entity);
 		if (vc_num < 0)
 			return vc_num;
 		mutex_lock(&priv->lock);
@@ -1753,9 +1751,6 @@ static int csi_registered(struct v4l2_subdev *sd)
 	int i, ret;
 	u32 code;
 
-	/* get media device */
-	priv->md = dev_get_drvdata(sd->v4l2_dev->dev);
-
 	/* get handle to IPU CSI */
 	csi = ipu_csi_get(priv->ipu, priv->csi_id);
 	if (IS_ERR(csi)) {
diff --git a/drivers/staging/media/imx/imx-media-fim.c b/drivers/staging/media/imx/imx-media-fim.c
index 6d283f4ea4d1..750682fcd48d 100644
--- a/drivers/staging/media/imx/imx-media-fim.c
+++ b/drivers/staging/media/imx/imx-media-fim.c
@@ -41,8 +41,6 @@ enum {
 #define FIM_CL_TOLERANCE_MAX_DEF   0 /* no max tolerance (unbounded) */
 
 struct imx_media_fim {
-	struct imx_media_dev *md;
-
 	/* the owning subdev of this fim instance */
 	struct v4l2_subdev *sd;
 
@@ -474,8 +472,6 @@ struct imx_media_fim *imx_media_fim_init(struct v4l2_subdev *sd)
 	if (!fim)
 		return ERR_PTR(-ENOMEM);
 
-	/* get media device */
-	fim->md = dev_get_drvdata(sd->v4l2_dev->dev);
 	fim->sd = sd;
 
 	spin_lock_init(&fim->lock);
diff --git a/drivers/staging/media/imx/imx-media-internal-sd.c b/drivers/staging/media/imx/imx-media-internal-sd.c
index 618a930da37c..f74e2ab4f37f 100644
--- a/drivers/staging/media/imx/imx-media-internal-sd.c
+++ b/drivers/staging/media/imx/imx-media-internal-sd.c
@@ -35,7 +35,7 @@ struct internal_subdev {
 	u32 grp_id;
 	struct internal_pad pad[MAX_INTERNAL_PADS];
 
-	struct v4l2_subdev * (*sync_register)(struct imx_media_dev *imxmd,
+	struct v4l2_subdev * (*sync_register)(struct v4l2_device *v4l2_dev,
 					      struct device *ipu_dev,
 					      struct ipu_soc *ipu,
 					      u32 grp_id);
@@ -228,7 +228,8 @@ int imx_media_register_ipu_internal_subdevs(struct imx_media_dev *imxmd,
 			continue;
 
 		mutex_unlock(&imxmd->mutex);
-		sd = intsd->sync_register(imxmd, ipu_dev, ipu, intsd->grp_id);
+		sd = intsd->sync_register(&imxmd->v4l2_dev, ipu_dev, ipu,
+					  intsd->grp_id);
 		mutex_lock(&imxmd->mutex);
 		if (IS_ERR(sd)) {
 			ret = PTR_ERR(sd);
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index a80ef0b087ad..862508d5d30c 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -772,19 +772,22 @@ void imx_media_add_video_device(struct imx_media_dev *imxmd,
 EXPORT_SYMBOL_GPL(imx_media_add_video_device);
 
 /*
- * Search upstream/downstream for a subdevice in the current pipeline
- * with given grp_id, starting from start_entity. Returns the subdev's
- * source/sink pad that it was reached from. If grp_id is zero, just
- * returns the nearest source/sink pad to start_entity. Must be called
- * with mdev->graph_mutex held.
+ * Search upstream/downstream for a subdevice or video device pad in the
+ * current pipeline, starting from start_entity. Returns the device's
+ * source/sink pad that it was reached from. Must be called with
+ * mdev->graph_mutex held.
+ *
+ * If grp_id != 0, finds a subdevice's pad of given grp_id.
+ * Else If buftype != 0, finds a video device's pad of given buffer type.
+ * Else, returns the nearest source/sink pad to start_entity.
  */
-static struct media_pad *
-find_pipeline_pad(struct imx_media_dev *imxmd,
-		  struct media_entity *start_entity,
-		  u32 grp_id, bool upstream)
+struct media_pad *
+imx_media_pipeline_pad(struct media_entity *start_entity, u32 grp_id,
+		       enum v4l2_buf_type buftype, bool upstream)
 {
 	struct media_entity *me = start_entity;
 	struct media_pad *pad = NULL;
+	struct video_device *vfd;
 	struct v4l2_subdev *sd;
 	int i;
 
@@ -796,16 +799,27 @@ find_pipeline_pad(struct imx_media_dev *imxmd,
 			continue;
 
 		pad = media_entity_remote_pad(spad);
-		if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
+		if (!pad)
 			continue;
 
-		if (grp_id != 0) {
-			sd = media_entity_to_v4l2_subdev(pad->entity);
-			if (sd->grp_id & grp_id)
-				return pad;
+		if (grp_id) {
+			if (is_media_entity_v4l2_subdev(pad->entity)) {
+				sd = media_entity_to_v4l2_subdev(pad->entity);
+				if (sd->grp_id & grp_id)
+					return pad;
+			}
+
+			return imx_media_pipeline_pad(pad->entity, grp_id,
+						      buftype, upstream);
+		} else if (buftype) {
+			if (is_media_entity_v4l2_video_device(pad->entity)) {
+				vfd = media_entity_to_video_device(pad->entity);
+				if (buftype == vfd->queue->type)
+					return pad;
+			}
 
-			return find_pipeline_pad(imxmd, pad->entity,
-						 grp_id, upstream);
+			return imx_media_pipeline_pad(pad->entity, grp_id,
+						      buftype, upstream);
 		} else {
 			return pad;
 		}
@@ -813,28 +827,33 @@ find_pipeline_pad(struct imx_media_dev *imxmd,
 
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(imx_media_pipeline_pad);
 
 /*
- * Search upstream for a subdev in the current pipeline with
- * given grp_id. Must be called with mdev->graph_mutex held.
+ * Search upstream/downstream for a subdev or video device in the current
+ * pipeline. Must be called with mdev->graph_mutex held.
  */
-static struct v4l2_subdev *
-find_upstream_subdev(struct imx_media_dev *imxmd,
-		     struct media_entity *start_entity,
-		     u32 grp_id)
+static struct media_entity *
+find_pipeline_entity(struct media_entity *start, u32 grp_id,
+		     enum v4l2_buf_type buftype, bool upstream)
 {
+	struct media_pad *pad = NULL;
+	struct video_device *vfd;
 	struct v4l2_subdev *sd;
-	struct media_pad *pad;
 
-	if (is_media_entity_v4l2_subdev(start_entity)) {
-		sd = media_entity_to_v4l2_subdev(start_entity);
+	if (grp_id && is_media_entity_v4l2_subdev(start)) {
+		sd = media_entity_to_v4l2_subdev(start);
 		if (sd->grp_id & grp_id)
-			return sd;
+			return &sd->entity;
+	} else if (buftype && is_media_entity_v4l2_video_device(start)) {
+		vfd = media_entity_to_video_device(pad->entity);
+		if (buftype == vfd->queue->type)
+			return &vfd->entity;
 	}
 
-	pad = find_pipeline_pad(imxmd, start_entity, grp_id, true);
+	pad = imx_media_pipeline_pad(start, grp_id, buftype, upstream);
 
-	return pad ? media_entity_to_v4l2_subdev(pad->entity) : NULL;
+	return pad ? pad->entity : NULL;
 }
 
 /*
@@ -842,62 +861,57 @@ find_upstream_subdev(struct imx_media_dev *imxmd,
  * start entity in the current pipeline.
  * Must be called with mdev->graph_mutex held.
  */
-int imx_media_find_mipi_csi2_channel(struct imx_media_dev *imxmd,
-				     struct media_entity *start_entity)
+int imx_media_pipeline_csi2_channel(struct media_entity *start_entity)
 {
 	struct media_pad *pad;
 	int ret = -EPIPE;
 
-	pad = find_pipeline_pad(imxmd, start_entity, IMX_MEDIA_GRP_ID_CSI2,
-				true);
-	if (pad) {
+	pad = imx_media_pipeline_pad(start_entity, IMX_MEDIA_GRP_ID_CSI2,
+				     0, true);
+	if (pad)
 		ret = pad->index - 1;
-		dev_dbg(imxmd->md.dev, "found vc%d from %s\n",
-			ret, start_entity->name);
-	}
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(imx_media_find_mipi_csi2_channel);
+EXPORT_SYMBOL_GPL(imx_media_pipeline_csi2_channel);
 
 /*
- * Find a source pad reached upstream from the given start entity in
- * the current pipeline. Must be called with mdev->graph_mutex held.
+ * Find a subdev reached upstream from the given start entity in
+ * the current pipeline.
+ * Must be called with mdev->graph_mutex held.
  */
-struct media_pad *
-imx_media_find_upstream_pad(struct imx_media_dev *imxmd,
-			    struct media_entity *start_entity,
-			    u32 grp_id)
+struct v4l2_subdev *
+imx_media_pipeline_subdev(struct media_entity *start_entity, u32 grp_id,
+			  bool upstream)
 {
-	struct media_pad *pad;
+	struct media_entity *me;
 
-	pad = find_pipeline_pad(imxmd, start_entity, grp_id, true);
-	if (!pad)
+	me = find_pipeline_entity(start_entity, grp_id, 0, upstream);
+	if (!me)
 		return ERR_PTR(-ENODEV);
 
-	return pad;
+	return media_entity_to_v4l2_subdev(me);
 }
-EXPORT_SYMBOL_GPL(imx_media_find_upstream_pad);
+EXPORT_SYMBOL_GPL(imx_media_pipeline_subdev);
 
 /*
  * Find a subdev reached upstream from the given start entity in
  * the current pipeline.
  * Must be called with mdev->graph_mutex held.
  */
-struct v4l2_subdev *
-imx_media_find_upstream_subdev(struct imx_media_dev *imxmd,
-			       struct media_entity *start_entity,
-			       u32 grp_id)
+struct video_device *
+imx_media_pipeline_video_device(struct media_entity *start_entity,
+				enum v4l2_buf_type buftype, bool upstream)
 {
-	struct v4l2_subdev *sd;
+	struct media_entity *me;
 
-	sd = find_upstream_subdev(imxmd, start_entity, grp_id);
-	if (!sd)
+	me = find_pipeline_entity(start_entity, 0, buftype, upstream);
+	if (!me)
 		return ERR_PTR(-ENODEV);
 
-	return sd;
+	return media_entity_to_video_device(me);
 }
-EXPORT_SYMBOL_GPL(imx_media_find_upstream_subdev);
+EXPORT_SYMBOL_GPL(imx_media_pipeline_video_device);
 
 /*
  * Turn current pipeline streaming on/off starting from entity.
diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
index 5dd4c733de97..5f6a1d5853f3 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -65,7 +65,6 @@ struct vdic_priv {
 	struct device *ipu_dev;
 	struct ipu_soc *ipu;
 
-	struct imx_media_dev *md;
 	struct v4l2_subdev   sd;
 	struct media_pad pad[VDIC_NUM_PADS];
 
@@ -927,12 +926,11 @@ static const struct v4l2_subdev_internal_ops vdic_internal_ops = {
 	.unregistered = vdic_unregistered,
 };
 
-struct v4l2_subdev *imx_media_vdic_register(struct imx_media_dev *imxmd,
+struct v4l2_subdev *imx_media_vdic_register(struct v4l2_device *v4l2_dev,
 					    struct device *ipu_dev,
 					    struct ipu_soc *ipu,
 					    u32 grp_id)
 {
-	struct v4l2_device *v4l2_dev = &imxmd->v4l2_dev;
 	struct vdic_priv *priv;
 	int ret;
 
@@ -942,7 +940,6 @@ struct v4l2_subdev *imx_media_vdic_register(struct imx_media_dev *imxmd,
 
 	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);
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index c0e58d4f2dfb..a3b60f99a7d8 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -190,16 +190,16 @@ imx_media_find_subdev_by_devname(struct imx_media_dev *imxmd,
 				 const char *devname);
 void imx_media_add_video_device(struct imx_media_dev *imxmd,
 				struct imx_media_video_dev *vdev);
-int imx_media_find_mipi_csi2_channel(struct imx_media_dev *imxmd,
-				     struct media_entity *start_entity);
+int imx_media_pipeline_csi2_channel(struct media_entity *start_entity);
 struct media_pad *
-imx_media_find_upstream_pad(struct imx_media_dev *imxmd,
-			    struct media_entity *start_entity,
-			    u32 grp_id);
+imx_media_pipeline_pad(struct media_entity *start_entity, u32 grp_id,
+		       enum v4l2_buf_type buftype, bool upstream);
 struct v4l2_subdev *
-imx_media_find_upstream_subdev(struct imx_media_dev *imxmd,
-			       struct media_entity *start_entity,
-			       u32 grp_id);
+imx_media_pipeline_subdev(struct media_entity *start_entity, u32 grp_id,
+			  bool upstream);
+struct video_device *
+imx_media_pipeline_video_device(struct media_entity *start_entity,
+				enum v4l2_buf_type buftype, bool upstream);
 
 struct imx_media_dma_buf {
 	void          *virt;
@@ -250,14 +250,14 @@ int imx_media_of_add_csi(struct imx_media_dev *imxmd,
 			 struct device_node *csi_np);
 
 /* imx-media-vdic.c */
-struct v4l2_subdev *imx_media_vdic_register(struct imx_media_dev *imxmd,
+struct v4l2_subdev *imx_media_vdic_register(struct v4l2_device *v4l2_dev,
 					    struct device *ipu_dev,
 					    struct ipu_soc *ipu,
 					    u32 grp_id);
 int imx_media_vdic_unregister(struct v4l2_subdev *sd);
 
 /* imx-ic-common.c */
-struct v4l2_subdev *imx_media_ic_register(struct imx_media_dev *imxmd,
+struct v4l2_subdev *imx_media_ic_register(struct v4l2_device *v4l2_dev,
 					  struct device *ipu_dev,
 					  struct ipu_soc *ipu,
 					  u32 grp_id);
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 75514618d021..7eda67ed1bd7 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -450,7 +450,7 @@ static int imx7_csi_get_upstream_endpoint(struct imx7_csi *csi,
 
 skip_video_mux:
 	/* get source pad of entity directly upstream from src */
-	pad = imx_media_find_upstream_pad(csi->imxmd, src, 0);
+	pad = imx_media_pipeline_pad(src, 0, 0, true);
 	if (IS_ERR(pad))
 		return PTR_ERR(pad);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 8/8] media: staging/imx: Don't set driver data for v4l2_dev
  2019-04-30 22:50 [PATCH v3 0/8] Switch to sync registration for IPU subdevs Steve Longerbeam
                   ` (6 preceding siblings ...)
  2019-04-30 22:50 ` [PATCH v3 7/8] media: staging/imx: Improve pipeline searching Steve Longerbeam
@ 2019-04-30 22:50 ` Steve Longerbeam
  7 siblings, 0 replies; 11+ messages in thread
From: Steve Longerbeam @ 2019-04-30 22:50 UTC (permalink / raw)
  To: linux-media
  Cc: Steve Longerbeam, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	open list:STAGING SUBSYSTEM,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list

The media device is already available via multiple methods, there is no
need to set driver data for v4l2_dev to the media device.

In imx_media_link_notify(), get media device from link->graph_obj.mdev.

In imx_media_capture_device_register(), get media device from
v4l2_dev->mdev.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-media-capture.c    | 5 +++--
 drivers/staging/media/imx/imx-media-dev-common.c | 7 ++-----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index b77a67bda47c..565d21f169d8 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -732,15 +732,16 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
 {
 	struct capture_priv *priv = to_capture_priv(vdev);
 	struct v4l2_subdev *sd = priv->src_sd;
+	struct v4l2_device *v4l2_dev = sd->v4l2_dev;
 	struct video_device *vfd = vdev->vfd;
 	struct vb2_queue *vq = &priv->q;
 	struct v4l2_subdev_format fmt_src;
 	int ret;
 
 	/* get media device */
-	priv->md = dev_get_drvdata(sd->v4l2_dev->dev);
+	priv->md = container_of(v4l2_dev->mdev, struct imx_media_dev, md);
 
-	vfd->v4l2_dev = sd->v4l2_dev;
+	vfd->v4l2_dev = v4l2_dev;
 
 	ret = video_register_device(vfd, VFL_TYPE_GRABBER, -1);
 	if (ret) {
diff --git a/drivers/staging/media/imx/imx-media-dev-common.c b/drivers/staging/media/imx/imx-media-dev-common.c
index 89dc4ec8dadb..66b505f7e8df 100644
--- a/drivers/staging/media/imx/imx-media-dev-common.c
+++ b/drivers/staging/media/imx/imx-media-dev-common.c
@@ -260,10 +260,11 @@ static int imx_media_inherit_controls(struct imx_media_dev *imxmd,
 static int imx_media_link_notify(struct media_link *link, u32 flags,
 				 unsigned int notification)
 {
+	struct imx_media_dev *imxmd = container_of(link->graph_obj.mdev,
+						   struct imx_media_dev, md);
 	struct media_entity *source = link->source->entity;
 	struct imx_media_pad_vdev *pad_vdev;
 	struct list_head *pad_vdev_list;
-	struct imx_media_dev *imxmd;
 	struct video_device *vfd;
 	struct v4l2_subdev *sd;
 	int pad_idx, ret;
@@ -279,8 +280,6 @@ static int imx_media_link_notify(struct media_link *link, u32 flags,
 	sd = media_entity_to_v4l2_subdev(source);
 	pad_idx = link->source->index;
 
-	imxmd = dev_get_drvdata(sd->v4l2_dev->dev);
-
 	pad_vdev_list = to_pad_vdev_list(sd, pad_idx);
 	if (!pad_vdev_list) {
 		/* nothing to do if source sd has no pad vdev list */
@@ -384,8 +383,6 @@ struct imx_media_dev *imx_media_dev_init(struct device *dev,
 		goto cleanup;
 	}
 
-	dev_set_drvdata(imxmd->v4l2_dev.dev, imxmd);
-
 	INIT_LIST_HEAD(&imxmd->vdev_list);
 
 	v4l2_async_notifier_init(&imxmd->notifier);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 5/8] media: staging/imx: Remove capture_device_set_format
  2019-04-30 22:50 ` [PATCH v3 5/8] media: staging/imx: Remove capture_device_set_format Steve Longerbeam
@ 2019-05-02 10:01   ` Rui Miguel Silva
  2019-05-02 17:02     ` Steve Longerbeam
  0 siblings, 1 reply; 11+ messages in thread
From: Rui Miguel Silva @ 2019-05-02 10:01 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: linux-media, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	open list:STAGING SUBSYSTEM,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list

Hi Steve,
Thanks for v3 with bisect fixed.

On Tue 30 Apr 2019 at 23:50, Steve Longerbeam wrote:
> Don't propagate the source pad format to the connected capture device.
> It's now the responsibility of userspace to call VIDIOC_S_FMT on the
> capture device to ensure the capture format and compose rectangle
> are compatible with the connected source. To check this, validate
> the capture format with the source before streaming starts.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
>  drivers/staging/media/imx/imx-ic-prpencvf.c   | 16 +----
>  drivers/staging/media/imx/imx-media-capture.c | 64 +++++++++++++------
>  drivers/staging/media/imx/imx-media-csi.c     | 16 +----
>  drivers/staging/media/imx/imx-media.h         |  2 -
>  drivers/staging/media/imx/imx7-media-csi.c    | 17 +----
>  5 files changed, 50 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index afaa3a8b15e9..63334fd61492 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -906,9 +906,7 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
>  		       struct v4l2_subdev_format *sdformat)
>  {
>  	struct prp_priv *priv = sd_to_priv(sd);
> -	struct imx_media_video_dev *vdev = priv->vdev;
>  	const struct imx_media_pixfmt *cc;
> -	struct v4l2_pix_format vdev_fmt;
>  	struct v4l2_mbus_framefmt *fmt;
>  	int ret = 0;
>  
> @@ -945,19 +943,9 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
>  			priv->cc[PRPENCVF_SRC_PAD] = outcc;
>  	}
>  
> -	if (sdformat->which == V4L2_SUBDEV_FORMAT_TRY)
> -		goto out;
> -
> -	priv->cc[sdformat->pad] = cc;
> +	if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> +		priv->cc[sdformat->pad] = cc;
>  
> -	/* propagate output pad format to capture device */
> -	imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
> -				      &priv->format_mbus[PRPENCVF_SRC_PAD],
> -				      priv->cc[PRPENCVF_SRC_PAD]);
> -	mutex_unlock(&priv->lock);
> -	imx_media_capture_device_set_format(vdev, &vdev_fmt);
> -
> -	return 0;
>  out:
>  	mutex_unlock(&priv->lock);
>  	return ret;
> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> index 555f6204660b..b77a67bda47c 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -205,7 +205,8 @@ static int capture_g_fmt_vid_cap(struct file *file, void *fh,
>  
>  static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
>  				     struct v4l2_subdev_format *fmt_src,
> -				     struct v4l2_format *f)
> +				     struct v4l2_format *f,
> +				     struct v4l2_rect *compose)
>  {
>  	const struct imx_media_pixfmt *cc, *cc_src;
>  
> @@ -247,6 +248,13 @@ static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
>  
>  	imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src->format, cc);
>  
> +	if (compose) {
> +		compose->left = 0;
> +		compose->top = 0;
> +		compose->width = fmt_src->format.width;
> +		compose->height = fmt_src->format.height;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -263,7 +271,7 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
>  	if (ret)
>  		return ret;
>  
> -	return __capture_try_fmt_vid_cap(priv, &fmt_src, f);
> +	return __capture_try_fmt_vid_cap(priv, &fmt_src, f, NULL);
>  }
>  
>  static int capture_s_fmt_vid_cap(struct file *file, void *fh,
> @@ -271,6 +279,7 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
>  {
>  	struct capture_priv *priv = video_drvdata(file);
>  	struct v4l2_subdev_format fmt_src;
> +	struct v4l2_rect compose;
>  	int ret;
>  
>  	if (vb2_is_busy(&priv->q)) {
> @@ -284,17 +293,14 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
>  	if (ret)
>  		return ret;
>  
> -	ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f);
> +	ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f, &compose);
>  	if (ret)
>  		return ret;
>  
>  	priv->vdev.fmt.fmt.pix = f->fmt.pix;
>  	priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
>  					      CS_SEL_ANY, true);
> -	priv->vdev.compose.left = 0;
> -	priv->vdev.compose.top = 0;
> -	priv->vdev.compose.width = fmt_src.format.width;
> -	priv->vdev.compose.height = fmt_src.format.height;
> +	priv->vdev.compose = compose;
>  
>  	return 0;
>  }
> @@ -524,6 +530,33 @@ static void capture_buf_queue(struct vb2_buffer *vb)
>  	spin_unlock_irqrestore(&priv->q_lock, flags);
>  }
>  
> +static int capture_validate_fmt(struct capture_priv *priv)
> +{
> +	struct v4l2_subdev_format fmt_src;
> +	struct v4l2_rect compose;
> +	struct v4l2_format f;
> +	int ret;
> +
> +	fmt_src.pad = priv->src_sd_pad;
> +	fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> +	ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
> +	if (ret)
> +		return ret;
> +
> +	v4l2_fill_pix_format(&f.fmt.pix, &fmt_src.format);
> +
> +	ret = __capture_try_fmt_vid_cap(priv, &fmt_src, &f, &compose);
> +	if (ret)
> +		return ret;
> +
> +	return (priv->vdev.fmt.fmt.pix.width != f.fmt.pix.width ||
> +		priv->vdev.fmt.fmt.pix.height != f.fmt.pix.height ||
> +		priv->vdev.cc->cs !=
> +		ipu_pixelformat_to_colorspace(f.fmt.pix.pixelformat) ||

This fails on imx7, no ipu, it returns unknown colorspace.
Removing this check everything works ok on my setup with this
series.
Do not know the best way to fix this but you may have? :)

> +		priv->vdev.compose.width != compose.width ||
> +		priv->vdev.compose.height != compose.height) ? -EINVAL : 0;
> +}
> +
>  static int capture_start_streaming(struct vb2_queue *vq, unsigned int count)
>  {
>  	struct capture_priv *priv = vb2_get_drv_priv(vq);
> @@ -531,6 +564,10 @@ static int capture_start_streaming(struct vb2_queue *vq, unsigned int count)
>  	unsigned long flags;
>  	int ret;
>  
> +	ret = capture_validate_fmt(priv);
> +	if (ret)

Maybe some verbose output here to let know userland what failed.

> +		goto return_bufs;
> +
>  	ret = imx_media_pipeline_set_stream(priv->md, &priv->src_sd->entity,
>  					    true);
>  	if (ret) {
> @@ -654,19 +691,6 @@ static struct video_device capture_videodev = {
>  	.device_caps	= V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING,
>  };

---
Cheers,
	Rui


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 5/8] media: staging/imx: Remove capture_device_set_format
  2019-05-02 10:01   ` Rui Miguel Silva
@ 2019-05-02 17:02     ` Steve Longerbeam
  0 siblings, 0 replies; 11+ messages in thread
From: Steve Longerbeam @ 2019-05-02 17:02 UTC (permalink / raw)
  To: Rui Miguel Silva
  Cc: linux-media, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	open list:STAGING SUBSYSTEM,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list



On 5/2/19 3:01 AM, Rui Miguel Silva wrote:
> Hi Steve,
> Thanks for v3 with bisect fixed.
>
> On Tue 30 Apr 2019 at 23:50, Steve Longerbeam wrote:
>> Don't propagate the source pad format to the connected capture device.
>> It's now the responsibility of userspace to call VIDIOC_S_FMT on the
>> capture device to ensure the capture format and compose rectangle
>> are compatible with the connected source. To check this, validate
>> the capture format with the source before streaming starts.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>>   drivers/staging/media/imx/imx-ic-prpencvf.c   | 16 +----
>>   drivers/staging/media/imx/imx-media-capture.c | 64 +++++++++++++------
>>   drivers/staging/media/imx/imx-media-csi.c     | 16 +----
>>   drivers/staging/media/imx/imx-media.h         |  2 -
>>   drivers/staging/media/imx/imx7-media-csi.c    | 17 +----
>>   5 files changed, 50 insertions(+), 65 deletions(-)
>>
>> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
>> index afaa3a8b15e9..63334fd61492 100644
>> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
>> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
>> @@ -906,9 +906,7 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
>>   		       struct v4l2_subdev_format *sdformat)
>>   {
>>   	struct prp_priv *priv = sd_to_priv(sd);
>> -	struct imx_media_video_dev *vdev = priv->vdev;
>>   	const struct imx_media_pixfmt *cc;
>> -	struct v4l2_pix_format vdev_fmt;
>>   	struct v4l2_mbus_framefmt *fmt;
>>   	int ret = 0;
>>   
>> @@ -945,19 +943,9 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
>>   			priv->cc[PRPENCVF_SRC_PAD] = outcc;
>>   	}
>>   
>> -	if (sdformat->which == V4L2_SUBDEV_FORMAT_TRY)
>> -		goto out;
>> -
>> -	priv->cc[sdformat->pad] = cc;
>> +	if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>> +		priv->cc[sdformat->pad] = cc;
>>   
>> -	/* propagate output pad format to capture device */
>> -	imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
>> -				      &priv->format_mbus[PRPENCVF_SRC_PAD],
>> -				      priv->cc[PRPENCVF_SRC_PAD]);
>> -	mutex_unlock(&priv->lock);
>> -	imx_media_capture_device_set_format(vdev, &vdev_fmt);
>> -
>> -	return 0;
>>   out:
>>   	mutex_unlock(&priv->lock);
>>   	return ret;
>> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
>> index 555f6204660b..b77a67bda47c 100644
>> --- a/drivers/staging/media/imx/imx-media-capture.c
>> +++ b/drivers/staging/media/imx/imx-media-capture.c
>> @@ -205,7 +205,8 @@ static int capture_g_fmt_vid_cap(struct file *file, void *fh,
>>   
>>   static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
>>   				     struct v4l2_subdev_format *fmt_src,
>> -				     struct v4l2_format *f)
>> +				     struct v4l2_format *f,
>> +				     struct v4l2_rect *compose)
>>   {
>>   	const struct imx_media_pixfmt *cc, *cc_src;
>>   
>> @@ -247,6 +248,13 @@ static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
>>   
>>   	imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src->format, cc);
>>   
>> +	if (compose) {
>> +		compose->left = 0;
>> +		compose->top = 0;
>> +		compose->width = fmt_src->format.width;
>> +		compose->height = fmt_src->format.height;
>> +	}
>> +
>>   	return 0;
>>   }
>>   
>> @@ -263,7 +271,7 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
>>   	if (ret)
>>   		return ret;
>>   
>> -	return __capture_try_fmt_vid_cap(priv, &fmt_src, f);
>> +	return __capture_try_fmt_vid_cap(priv, &fmt_src, f, NULL);
>>   }
>>   
>>   static int capture_s_fmt_vid_cap(struct file *file, void *fh,
>> @@ -271,6 +279,7 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
>>   {
>>   	struct capture_priv *priv = video_drvdata(file);
>>   	struct v4l2_subdev_format fmt_src;
>> +	struct v4l2_rect compose;
>>   	int ret;
>>   
>>   	if (vb2_is_busy(&priv->q)) {
>> @@ -284,17 +293,14 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
>>   	if (ret)
>>   		return ret;
>>   
>> -	ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f);
>> +	ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f, &compose);
>>   	if (ret)
>>   		return ret;
>>   
>>   	priv->vdev.fmt.fmt.pix = f->fmt.pix;
>>   	priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
>>   					      CS_SEL_ANY, true);
>> -	priv->vdev.compose.left = 0;
>> -	priv->vdev.compose.top = 0;
>> -	priv->vdev.compose.width = fmt_src.format.width;
>> -	priv->vdev.compose.height = fmt_src.format.height;
>> +	priv->vdev.compose = compose;
>>   
>>   	return 0;
>>   }
>> @@ -524,6 +530,33 @@ static void capture_buf_queue(struct vb2_buffer *vb)
>>   	spin_unlock_irqrestore(&priv->q_lock, flags);
>>   }
>>   
>> +static int capture_validate_fmt(struct capture_priv *priv)
>> +{
>> +	struct v4l2_subdev_format fmt_src;
>> +	struct v4l2_rect compose;
>> +	struct v4l2_format f;
>> +	int ret;
>> +
>> +	fmt_src.pad = priv->src_sd_pad;
>> +	fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>> +	ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
>> +	if (ret)
>> +		return ret;
>> +
>> +	v4l2_fill_pix_format(&f.fmt.pix, &fmt_src.format);
>> +
>> +	ret = __capture_try_fmt_vid_cap(priv, &fmt_src, &f, &compose);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return (priv->vdev.fmt.fmt.pix.width != f.fmt.pix.width ||
>> +		priv->vdev.fmt.fmt.pix.height != f.fmt.pix.height ||
>> +		priv->vdev.cc->cs !=
>> +		ipu_pixelformat_to_colorspace(f.fmt.pix.pixelformat) ||
> This fails on imx7, no ipu, it returns unknown colorspace.

Oops, thanks for catching.

> Removing this check everything works ok on my setup with this
> series.
> Do not know the best way to fix this but you may have? :)

Maybe the best fix is simply to remove the check for same colorspace, 
verifying the rectangles is probably good enough.


>
>> +		priv->vdev.compose.width != compose.width ||
>> +		priv->vdev.compose.height != compose.height) ? -EINVAL : 0;
>> +}
>> +
>>   static int capture_start_streaming(struct vb2_queue *vq, unsigned int count)
>>   {
>>   	struct capture_priv *priv = vb2_get_drv_priv(vq);
>> @@ -531,6 +564,10 @@ static int capture_start_streaming(struct vb2_queue *vq, unsigned int count)
>>   	unsigned long flags;
>>   	int ret;
>>   
>> +	ret = capture_validate_fmt(priv);
>> +	if (ret)
> Maybe some verbose output here to let know userland what failed.

Ok.

Steve

>> +		goto return_bufs;
>> +
>>   	ret = imx_media_pipeline_set_stream(priv->md, &priv->src_sd->entity,
>>   					    true);
>>   	if (ret) {
>> @@ -654,19 +691,6 @@ static struct video_device capture_videodev = {
>>   	.device_caps	= V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING,
>>   };
> ---
> Cheers,
> 	Rui
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-05-02 17:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-30 22:50 [PATCH v3 0/8] Switch to sync registration for IPU subdevs Steve Longerbeam
2019-04-30 22:50 ` [PATCH v3 1/8] media: staging/imx: " Steve Longerbeam
2019-04-30 22:50 ` [PATCH v3 2/8] media: staging/imx: Pass device to alloc/free_dma_buf Steve Longerbeam
2019-04-30 22:50 ` [PATCH v3 3/8] media: staging/imx: Move add_video_device into capture_device_register Steve Longerbeam
2019-04-30 22:50 ` [PATCH v3 4/8] Revert "media: imx: Set capture compose rectangle in capture_device_set_format" Steve Longerbeam
2019-04-30 22:50 ` [PATCH v3 5/8] media: staging/imx: Remove capture_device_set_format Steve Longerbeam
2019-05-02 10:01   ` Rui Miguel Silva
2019-05-02 17:02     ` Steve Longerbeam
2019-04-30 22:50 ` [PATCH v3 6/8] media: staging/imx: Re-organize modules Steve Longerbeam
2019-04-30 22:50 ` [PATCH v3 7/8] media: staging/imx: Improve pipeline searching Steve Longerbeam
2019-04-30 22:50 ` [PATCH v3 8/8] media: staging/imx: Don't set driver data for v4l2_dev Steve Longerbeam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).