[PATCH v7 00/19] Small fixes and cleanups (ov2740 and ccs)

[PATCH v7 00/19] Small fixes and cleanups (ov2740 and ccs)

[Sakari Ailus]

Hi folks,

This small set contains fixes and cleanups, mainly for the ccs and ov2740
drivers. I wrote these while working on the metadata set, but these could
and should be merged earlier.

since v6:

- New patch: In scope of the CCS driver, revert the patch moving CCS to
  use pm_runtime_get_and_resume().

- New patch: Drop s_stream re-entrancy in CCS driver.

- New patch: Rename ccs_create_subdev() as ccs_init_subdev.

- New patch: CCS driver rework: move sub-device initialisation earlier in
  probe to address initialisation ordering issues later on in embedded
  data support. This introduces minor changes to the patch adding
  sub-device state support.

since v5:

- Send right patches (was v3 + an additional patch)!

since v4:

- Fix CCS driver active state patch --- media entity was initialised too
  late.

- Rebase on Laurent's ov2740 cleanups.

- Add a new patch for MIPI CSI-2 long packet types.

since v3:

- Don't print frame descriptor entry flags as strings but in a numeric
  form.

- Add a WARN_ON() for string truncation in printing the frame descriptor.

- Use 0 flag in printing hexadecimal values in frame descriptor instead of
  specifying precision.

- Add curly braces around a loop (11th patch).

since v2:

- Wrap init_cfg callback long line.

- Remove "pad_" from variable names in ccs_init_cfg.

- Fix media_entity_pads_init() error handling bug (was introduced in the
  last patch).

- Print frame descriptor in less verbose way.

since v1:

- Add a comment on ov2740 active state patch on serialising sensor access.

- Improved commit message of ov2740 patch enabling runtime PM earlier.

- Added patches for printing and zeroing frame descriptor, (debug)
  printing of frame descriptor, switching ccs to init_cfg and sub-device
  state and checking pad flag validity.

Sakari Ailus (19):
  media: Documentation: Align numbered list, make it a proper ReST
  media: ccs: Fix driver quirk struct documentation
  media: ccs: Correctly initialise try compose rectangle
  media: ccs: Correct error handling in ccs_register_subdev
  media: ccs: Switch to init_cfg
  media: ccs: Rename ccs_create_subdev as ccs_init_subdev
  media: ccs: Move media_entity_pads_init to init from register
  media: ccs: Obtain media bus formats before initialising up
    sub-devices
  media: ccs: Use sub-device active state
  media: ccs: Partially revert "media: i2c: Use
    pm_runtime_resume_and_get()"
  media: ccs: Drop re-entrant s_stream support
  media: ov2740: Enable runtime PM before registering the async subdev
  media: ov2740: Use sub-device active state
  media: ov2740: Return -EPROBE_DEFER if no endpoint is found
  media: v4l: subdev: Clear frame descriptor before get_frame_desc
  media: v4l: subdev: Print debug information on frame descriptor
  media: mc: Check pad flag validity
  media: Add MIPI CSI-2 generic long packet type definition
  media: Documentation: Split camera sensor documentation

 .../driver-api/media/camera-sensor.rst        | 131 ++----
 .../media/drivers/camera-sensor.rst           | 104 +++++
 .../userspace-api/media/drivers/index.rst     |   1 +
 .../userspace-api/media/v4l/control.rst       |   4 +
 .../userspace-api/media/v4l/dev-subdev.rst    |  49 ++-
 drivers/media/i2c/ccs/ccs-core.c              | 374 +++++++-----------
 drivers/media/i2c/ccs/ccs-quirk.h             |   4 +-
 drivers/media/i2c/ccs/ccs.h                   |   4 +-
 drivers/media/i2c/ds90ub913.c                 |   2 -
 drivers/media/i2c/ds90ub953.c                 |   2 -
 drivers/media/i2c/ds90ub960.c                 |   2 -
 drivers/media/i2c/ov2740.c                    | 125 +++---
 drivers/media/mc/mc-entity.c                  |  15 +-
 drivers/media/platform/nxp/imx-mipi-csis.c    |   2 -
 drivers/media/v4l2-core/v4l2-subdev.c         |  38 ++
 include/media/mipi-csi2.h                     |   1 +
 16 files changed, 422 insertions(+), 436 deletions(-)
 create mode 100644 Documentation/userspace-api/media/drivers/camera-sensor.rst

-- 
2.39.2

[PATCH v7 01/19] media: Documentation: Align numbered list, make it a proper ReST

[Sakari Ailus]

Align lines for numbered list so that Sphinx produces an uniform output
for all list entries. Also indent paragraphs of such list entries for
consistency.

Also use ReST numbered list syntax for the entries.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../userspace-api/media/v4l/dev-subdev.rst    | 49 +++++++++----------
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
index a4f1df7093e8..43988516acdd 100644
--- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
+++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
@@ -579,20 +579,19 @@ is started.
 
 There are three steps in configuring the streams:
 
-1) Set up links. Connect the pads between sub-devices using the :ref:`Media
-Controller API <media_controller>`
+1. Set up links. Connect the pads between sub-devices using the
+   :ref:`Media Controller API <media_controller>`
 
-2) Streams. Streams are declared and their routing is configured by
-setting the routing table for the sub-device using
-:ref:`VIDIOC_SUBDEV_S_ROUTING <VIDIOC_SUBDEV_G_ROUTING>` ioctl. Note that
-setting the routing table will reset formats and selections in the
-sub-device to default values.
+2. Streams. Streams are declared and their routing is configured by setting the
+   routing table for the sub-device using :ref:`VIDIOC_SUBDEV_S_ROUTING
+   <VIDIOC_SUBDEV_G_ROUTING>` ioctl. Note that setting the routing table will
+   reset formats and selections in the sub-device to default values.
 
-3) Configure formats and selections. Formats and selections of each stream
-are configured separately as documented for plain sub-devices in
-:ref:`format-propagation`. The stream ID is set to the same stream ID
-associated with either sink or source pads of routes configured using the
-:ref:`VIDIOC_SUBDEV_S_ROUTING <VIDIOC_SUBDEV_G_ROUTING>` ioctl.
+3. Configure formats and selections. Formats and selections of each stream are
+   configured separately as documented for plain sub-devices in
+   :ref:`format-propagation`. The stream ID is set to the same stream ID
+   associated with either sink or source pads of routes configured using the
+   :ref:`VIDIOC_SUBDEV_S_ROUTING <VIDIOC_SUBDEV_G_ROUTING>` ioctl.
 
 Multiplexed streams setup example
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -618,11 +617,11 @@ modeled as V4L2 devices, exposed to userspace via /dev/videoX nodes.
 
 To configure this pipeline, the userspace must take the following steps:
 
-1) Set up media links between entities: connect the sensors to the bridge,
-bridge to the receiver, and the receiver to the DMA engines. This step does
-not differ from normal non-multiplexed media controller setup.
+1. Set up media links between entities: connect the sensors to the bridge,
+   bridge to the receiver, and the receiver to the DMA engines. This step does
+   not differ from normal non-multiplexed media controller setup.
 
-2) Configure routing
+2. Configure routing
 
 .. flat-table:: Bridge routing table
     :header-rows:  1
@@ -656,14 +655,14 @@ not differ from normal non-multiplexed media controller setup.
       - V4L2_SUBDEV_ROUTE_FL_ACTIVE
       - Pixel data stream from Sensor B
 
-3) Configure formats and selections
+3. Configure formats and selections
 
-After configuring routing, the next step is configuring the formats and
-selections for the streams. This is similar to performing this step without
-streams, with just one exception: the ``stream`` field needs to be assigned
-to the value of the stream ID.
+   After configuring routing, the next step is configuring the formats and
+   selections for the streams. This is similar to performing this step without
+   streams, with just one exception: the ``stream`` field needs to be assigned
+   to the value of the stream ID.
 
-A common way to accomplish this is to start from the sensors and propagate the
-configurations along the stream towards the receiver,
-using :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>` ioctls to configure each
-stream endpoint in each sub-device.
+   A common way to accomplish this is to start from the sensors and propagate
+   the configurations along the stream towards the receiver, using
+   :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>` ioctls to configure each
+   stream endpoint in each sub-device.
-- 
2.39.2

[PATCH v7 02/19] media: ccs: Fix driver quirk struct documentation

[Sakari Ailus]

Fix documentation for struct ccs_quirk, a device specific struct for
managing deviations from the standard. The flags field was drifted away
from where it should have been.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs/ccs-quirk.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-quirk.h b/drivers/media/i2c/ccs/ccs-quirk.h
index 5838fcda92fd..0b1a64958d71 100644
--- a/drivers/media/i2c/ccs/ccs-quirk.h
+++ b/drivers/media/i2c/ccs/ccs-quirk.h
@@ -32,12 +32,10 @@ struct ccs_sensor;
  *		@reg: Pointer to the register to access
  *		@value: Register value, set by the caller on write, or
  *			by the quirk on read
- *
- * @flags: Quirk flags
- *
  *		@return: 0 on success, -ENOIOCTLCMD if no register
  *			 access may be done by the caller (default read
  *			 value is zero), else negative error code on error
+ * @flags: Quirk flags
  */
 struct ccs_quirk {
 	int (*limits)(struct ccs_sensor *sensor);
-- 
2.39.2

[PATCH v7 03/19] media: ccs: Correctly initialise try compose rectangle

[Sakari Ailus]

Initialise the try sink compose rectangle size to the sink compose
rectangle for binner and scaler sub-devices. This was missed due to the
faulty condition that lead to the compose rectangles to be initialised for
the pixel array sub-device where it is not relevant.

Fixes: ccfc97bdb5ae ("[media] smiapp: Add driver")
Cc: stable@vger.kernel.org
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 6a8116454f87..022e8712d48e 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -3097,7 +3097,7 @@ static int ccs_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 		try_fmt->code = sensor->internal_csi_format->code;
 		try_fmt->field = V4L2_FIELD_NONE;
 
-		if (ssd != sensor->pixel_array)
+		if (ssd == sensor->pixel_array)
 			continue;
 
 		try_comp = v4l2_subdev_get_try_compose(sd, fh->state, i);
-- 
2.39.2

[PATCH v7 04/19] media: ccs: Correct error handling in ccs_register_subdev

[Sakari Ailus]

ccs_register_subdev() did not clean up the media entity in error case, do
that now. Also switch to goto based error handling.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 022e8712d48e..fb823c5c3dd3 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -2968,7 +2968,7 @@ static int ccs_register_subdev(struct ccs_sensor *sensor,
 	rval = v4l2_device_register_subdev(sensor->src->sd.v4l2_dev, &ssd->sd);
 	if (rval) {
 		dev_err(&client->dev, "v4l2_device_register_subdev failed\n");
-		return rval;
+		goto out_media_entity_cleanup;
 	}
 
 	rval = media_create_pad_link(&ssd->sd.entity, source_pad,
@@ -2976,11 +2976,18 @@ static int ccs_register_subdev(struct ccs_sensor *sensor,
 				     link_flags);
 	if (rval) {
 		dev_err(&client->dev, "media_create_pad_link failed\n");
-		v4l2_device_unregister_subdev(&ssd->sd);
-		return rval;
+		goto out_v4l2_device_unregister_subdev;
 	}
 
 	return 0;
+
+out_v4l2_device_unregister_subdev:
+	v4l2_device_unregister_subdev(&ssd->sd);
+
+out_media_entity_cleanup:
+	media_entity_cleanup(&ssd->sd.entity);
+
+	return rval;
 }
 
 static void ccs_unregistered(struct v4l2_subdev *subdev)
-- 
2.39.2

[PATCH v7 05/19] media: ccs: Switch to init_cfg

[Sakari Ailus]

Use init_cfg() instead of manually setting up defaults in file handle
open.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index fb823c5c3dd3..9e8769603704 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -2945,7 +2945,6 @@ static int ccs_identify_module(struct ccs_sensor *sensor)
 }
 
 static const struct v4l2_subdev_ops ccs_ops;
-static const struct v4l2_subdev_internal_ops ccs_internal_ops;
 static const struct media_entity_operations ccs_entity_ops;
 
 static int ccs_register_subdev(struct ccs_sensor *sensor,
@@ -3076,13 +3075,13 @@ static void ccs_create_subdev(struct ccs_sensor *sensor,
 	if (ssd == sensor->src)
 		return;
 
-	ssd->sd.internal_ops = &ccs_internal_ops;
 	ssd->sd.owner = THIS_MODULE;
 	ssd->sd.dev = &client->dev;
 	v4l2_set_subdevdata(&ssd->sd, client);
 }
 
-static int ccs_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+static int ccs_init_cfg(struct v4l2_subdev *sd,
+			struct v4l2_subdev_state *sd_state)
 {
 	struct ccs_subdev *ssd = to_ccs_subdev(sd);
 	struct ccs_sensor *sensor = ssd->sensor;
@@ -3092,9 +3091,9 @@ static int ccs_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 
 	for (i = 0; i < ssd->npads; i++) {
 		struct v4l2_mbus_framefmt *try_fmt =
-			v4l2_subdev_get_try_format(sd, fh->state, i);
+			v4l2_subdev_get_try_format(sd, sd_state, i);
 		struct v4l2_rect *try_crop =
-			v4l2_subdev_get_try_crop(sd, fh->state, i);
+			v4l2_subdev_get_try_crop(sd, sd_state, i);
 		struct v4l2_rect *try_comp;
 
 		ccs_get_native_size(ssd, try_crop);
@@ -3107,7 +3106,7 @@ static int ccs_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 		if (ssd == sensor->pixel_array)
 			continue;
 
-		try_comp = v4l2_subdev_get_try_compose(sd, fh->state, i);
+		try_comp = v4l2_subdev_get_try_compose(sd, sd_state, i);
 		*try_comp = *try_crop;
 	}
 
@@ -3123,6 +3122,7 @@ static const struct v4l2_subdev_video_ops ccs_video_ops = {
 };
 
 static const struct v4l2_subdev_pad_ops ccs_pad_ops = {
+	.init_cfg = ccs_init_cfg,
 	.enum_mbus_code = ccs_enum_mbus_code,
 	.get_fmt = ccs_get_format,
 	.set_fmt = ccs_set_format,
@@ -3148,11 +3148,6 @@ static const struct media_entity_operations ccs_entity_ops = {
 static const struct v4l2_subdev_internal_ops ccs_internal_src_ops = {
 	.registered = ccs_registered,
 	.unregistered = ccs_unregistered,
-	.open = ccs_open,
-};
-
-static const struct v4l2_subdev_internal_ops ccs_internal_ops = {
-	.open = ccs_open,
 };
 
 /* -----------------------------------------------------------------------------
-- 
2.39.2

[PATCH v7 06/19] media: ccs: Rename ccs_create_subdev as ccs_init_subdev

[Sakari Ailus]

The ccs_create_subdev() function initialises a sub-device in the CCS
driver, including CCS specific needs. Rename it as ccs_init_subdev() as it
better reflects what the function does.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 9e8769603704..6bcce908f93b 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -3038,9 +3038,9 @@ static void ccs_cleanup(struct ccs_sensor *sensor)
 	ccs_free_controls(sensor);
 }
 
-static void ccs_create_subdev(struct ccs_sensor *sensor,
-			      struct ccs_subdev *ssd, const char *name,
-			      unsigned short num_pads, u32 function)
+static void ccs_init_subdev(struct ccs_sensor *sensor,
+			    struct ccs_subdev *ssd, const char *name,
+			    unsigned short num_pads, u32 function)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
 
@@ -3553,12 +3553,12 @@ static int ccs_probe(struct i2c_client *client)
 	sensor->pll.ext_clk_freq_hz = sensor->hwcfg.ext_clk;
 	sensor->pll.scale_n = CCS_LIM(sensor, SCALER_N_MIN);
 
-	ccs_create_subdev(sensor, sensor->scaler, " scaler", 2,
-			  MEDIA_ENT_F_PROC_VIDEO_SCALER);
-	ccs_create_subdev(sensor, sensor->binner, " binner", 2,
-			  MEDIA_ENT_F_PROC_VIDEO_SCALER);
-	ccs_create_subdev(sensor, sensor->pixel_array, " pixel_array", 1,
-			  MEDIA_ENT_F_CAM_SENSOR);
+	ccs_init_subdev(sensor, sensor->scaler, " scaler", 2,
+			MEDIA_ENT_F_PROC_VIDEO_SCALER);
+	ccs_init_subdev(sensor, sensor->binner, " binner", 2,
+			MEDIA_ENT_F_PROC_VIDEO_SCALER);
+	ccs_init_subdev(sensor, sensor->pixel_array, " pixel_array", 1,
+			MEDIA_ENT_F_CAM_SENSOR);
 
 	rval = ccs_init_controls(sensor);
 	if (rval < 0)
-- 
2.39.2

[PATCH v7 07/19] media: ccs: Move media_entity_pads_init to init from register

[Sakari Ailus]

The media entity will soon need to be initialised before the sub-device
init finalisation that allocates memory for sub-device state. Do that now.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 75 +++++++++++++++-----------------
 1 file changed, 36 insertions(+), 39 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 6bcce908f93b..e2669e9299ab 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -2958,16 +2958,10 @@ static int ccs_register_subdev(struct ccs_sensor *sensor,
 	if (!sink_ssd)
 		return 0;
 
-	rval = media_entity_pads_init(&ssd->sd.entity, ssd->npads, ssd->pads);
-	if (rval) {
-		dev_err(&client->dev, "media_entity_pads_init failed\n");
-		return rval;
-	}
-
 	rval = v4l2_device_register_subdev(sensor->src->sd.v4l2_dev, &ssd->sd);
 	if (rval) {
 		dev_err(&client->dev, "v4l2_device_register_subdev failed\n");
-		goto out_media_entity_cleanup;
+		return rval;
 	}
 
 	rval = media_create_pad_link(&ssd->sd.entity, source_pad,
@@ -2975,18 +2969,11 @@ static int ccs_register_subdev(struct ccs_sensor *sensor,
 				     link_flags);
 	if (rval) {
 		dev_err(&client->dev, "media_create_pad_link failed\n");
-		goto out_v4l2_device_unregister_subdev;
+		v4l2_device_unregister_subdev(&ssd->sd);
+		return rval;
 	}
 
 	return 0;
-
-out_v4l2_device_unregister_subdev:
-	v4l2_device_unregister_subdev(&ssd->sd);
-
-out_media_entity_cleanup:
-	media_entity_cleanup(&ssd->sd.entity);
-
-	return rval;
 }
 
 static void ccs_unregistered(struct v4l2_subdev *subdev)
@@ -3031,6 +3018,10 @@ static int ccs_registered(struct v4l2_subdev *subdev)
 static void ccs_cleanup(struct ccs_sensor *sensor)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
+	unsigned int i;
+
+	for (i = 0; i < sensor->ssds_used; i++)
+		media_entity_cleanup(&sensor->ssds[i].sd.entity);
 
 	device_remove_file(&client->dev, &dev_attr_nvm);
 	device_remove_file(&client->dev, &dev_attr_ident);
@@ -3038,14 +3029,15 @@ static void ccs_cleanup(struct ccs_sensor *sensor)
 	ccs_free_controls(sensor);
 }
 
-static void ccs_init_subdev(struct ccs_sensor *sensor,
-			    struct ccs_subdev *ssd, const char *name,
-			    unsigned short num_pads, u32 function)
+static int ccs_init_subdev(struct ccs_sensor *sensor,
+			   struct ccs_subdev *ssd, const char *name,
+			   unsigned short num_pads, u32 function)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
+	int rval;
 
 	if (!ssd)
-		return;
+		return 0;
 
 	if (ssd != sensor->src)
 		v4l2_subdev_init(&ssd->sd, &ccs_ops);
@@ -3072,12 +3064,20 @@ static void ccs_init_subdev(struct ccs_sensor *sensor,
 
 	ssd->sd.entity.ops = &ccs_entity_ops;
 
+	rval = media_entity_pads_init(&ssd->sd.entity, ssd->npads, ssd->pads);
+	if (rval) {
+		dev_err(&client->dev, "media_entity_pads_init failed\n");
+		return rval;
+	}
+
 	if (ssd == sensor->src)
-		return;
+		return 0;
 
 	ssd->sd.owner = THIS_MODULE;
 	ssd->sd.dev = &client->dev;
 	v4l2_set_subdevdata(&ssd->sd, client);
+
+	return 0;
 }
 
 static int ccs_init_cfg(struct v4l2_subdev *sd,
@@ -3553,12 +3553,18 @@ static int ccs_probe(struct i2c_client *client)
 	sensor->pll.ext_clk_freq_hz = sensor->hwcfg.ext_clk;
 	sensor->pll.scale_n = CCS_LIM(sensor, SCALER_N_MIN);
 
-	ccs_init_subdev(sensor, sensor->scaler, " scaler", 2,
-			MEDIA_ENT_F_PROC_VIDEO_SCALER);
-	ccs_init_subdev(sensor, sensor->binner, " binner", 2,
-			MEDIA_ENT_F_PROC_VIDEO_SCALER);
-	ccs_init_subdev(sensor, sensor->pixel_array, " pixel_array", 1,
-			MEDIA_ENT_F_CAM_SENSOR);
+	rval = ccs_init_subdev(sensor, sensor->scaler, " scaler", 2,
+			       MEDIA_ENT_F_PROC_VIDEO_SCALER);
+	if (rval)
+		goto out_cleanup;
+	rval = ccs_init_subdev(sensor, sensor->binner, " binner", 2,
+			       MEDIA_ENT_F_PROC_VIDEO_SCALER);
+	if (rval)
+		goto out_cleanup;
+	rval = ccs_init_subdev(sensor, sensor->pixel_array, " pixel_array", 1,
+			       MEDIA_ENT_F_CAM_SENSOR);
+	if (rval)
+		goto out_cleanup;
 
 	rval = ccs_init_controls(sensor);
 	if (rval < 0)
@@ -3591,14 +3597,9 @@ static int ccs_probe(struct i2c_client *client)
 	sensor->streaming = false;
 	sensor->dev_init_done = true;
 
-	rval = media_entity_pads_init(&sensor->src->sd.entity, 2,
-				 sensor->src->pads);
-	if (rval < 0)
-		goto out_media_entity_cleanup;
-
 	rval = ccs_write_msr_regs(sensor);
 	if (rval)
-		goto out_media_entity_cleanup;
+		goto out_cleanup;
 
 	pm_runtime_set_active(&client->dev);
 	pm_runtime_get_noresume(&client->dev);
@@ -3618,9 +3619,6 @@ static int ccs_probe(struct i2c_client *client)
 	pm_runtime_put_noidle(&client->dev);
 	pm_runtime_disable(&client->dev);
 
-out_media_entity_cleanup:
-	media_entity_cleanup(&sensor->src->sd.entity);
-
 out_cleanup:
 	ccs_cleanup(sensor);
 
@@ -3653,10 +3651,9 @@ static void ccs_remove(struct i2c_client *client)
 		ccs_power_off(&client->dev);
 	pm_runtime_set_suspended(&client->dev);
 
-	for (i = 0; i < sensor->ssds_used; i++) {
+	for (i = 0; i < sensor->ssds_used; i++)
 		v4l2_device_unregister_subdev(&sensor->ssds[i].sd);
-		media_entity_cleanup(&sensor->ssds[i].sd.entity);
-	}
+
 	ccs_cleanup(sensor);
 	mutex_destroy(&sensor->mutex);
 	kfree(sensor->ccs_limits);
-- 
2.39.2

[PATCH v7 08/19] media: ccs: Obtain media bus formats before initialising up sub-devices

[Sakari Ailus]

The available mbus codes will soon be needed earlier, at the time
sub-devices are initialisaed. This is due to calling init_cfg() op via the
v4l2_subdev_init_finalize().

Move ccs_get_mbus_formats() before ccs_init_subdev() calls.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index e2669e9299ab..422fb6a4a907 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -3553,6 +3553,12 @@ static int ccs_probe(struct i2c_client *client)
 	sensor->pll.ext_clk_freq_hz = sensor->hwcfg.ext_clk;
 	sensor->pll.scale_n = CCS_LIM(sensor, SCALER_N_MIN);
 
+	rval = ccs_get_mbus_formats(sensor);
+	if (rval) {
+		rval = -ENODEV;
+		goto out_cleanup;
+	}
+
 	rval = ccs_init_subdev(sensor, sensor->scaler, " scaler", 2,
 			       MEDIA_ENT_F_PROC_VIDEO_SCALER);
 	if (rval)
@@ -3574,12 +3580,6 @@ static int ccs_probe(struct i2c_client *client)
 	if (rval)
 		goto out_cleanup;
 
-	rval = ccs_get_mbus_formats(sensor);
-	if (rval) {
-		rval = -ENODEV;
-		goto out_cleanup;
-	}
-
 	rval = ccs_init_late_controls(sensor);
 	if (rval) {
 		rval = -ENODEV;
-- 
2.39.2

[PATCH v7 09/19] media: ccs: Use sub-device active state

[Sakari Ailus]

Make use of sub-device active state. In most cases the effect on need for
acquiring the mutex is non-existent as access to the driver's core data
structure still needs to be serialised.

This still removes a lot of code as the code paths for active and try
state are the same in many cases.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 286 +++++++++++--------------------
 drivers/media/i2c/ccs/ccs.h      |   4 +-
 2 files changed, 104 insertions(+), 186 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 422fb6a4a907..c6853622946b 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -508,9 +508,8 @@ static void __ccs_update_exposure_limits(struct ccs_sensor *sensor)
 	struct v4l2_ctrl *ctrl = sensor->exposure;
 	int max;
 
-	max = sensor->pixel_array->crop[CCS_PA_PAD_SRC].height
-		+ sensor->vblank->val
-		- CCS_LIM(sensor, COARSE_INTEGRATION_TIME_MAX_MARGIN);
+	max = sensor->pa_src.height + sensor->vblank->val -
+		CCS_LIM(sensor, COARSE_INTEGRATION_TIME_MAX_MARGIN);
 
 	__v4l2_ctrl_modify_range(ctrl, ctrl->minimum, max, ctrl->step, max);
 }
@@ -728,15 +727,12 @@ static int ccs_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	case V4L2_CID_VBLANK:
 		rval = ccs_write(sensor, FRAME_LENGTH_LINES,
-				 sensor->pixel_array->crop[
-					 CCS_PA_PAD_SRC].height
-				 + ctrl->val);
+				 sensor->pa_src.height + ctrl->val);
 
 		break;
 	case V4L2_CID_HBLANK:
 		rval = ccs_write(sensor, LINE_LENGTH_PCK,
-				 sensor->pixel_array->crop[CCS_PA_PAD_SRC].width
-				 + ctrl->val);
+				 sensor->pa_src.width + ctrl->val);
 
 		break;
 	case V4L2_CID_TEST_PATTERN:
@@ -1214,15 +1210,13 @@ static void ccs_update_blanking(struct ccs_sensor *sensor)
 
 	min = max_t(int,
 		    CCS_LIM(sensor, MIN_FRAME_BLANKING_LINES),
-		    min_fll - sensor->pixel_array->crop[CCS_PA_PAD_SRC].height);
-	max = max_fll -	sensor->pixel_array->crop[CCS_PA_PAD_SRC].height;
+		    min_fll - sensor->pa_src.height);
+	max = max_fll -	sensor->pa_src.height;
 
 	__v4l2_ctrl_modify_range(vblank, min, max, vblank->step, min);
 
-	min = max_t(int,
-		    min_llp - sensor->pixel_array->crop[CCS_PA_PAD_SRC].width,
-		    min_lbp);
-	max = max_llp - sensor->pixel_array->crop[CCS_PA_PAD_SRC].width;
+	min = max_t(int, min_llp - sensor->pa_src.width, min_lbp);
+	max = max_llp - sensor->pa_src.width;
 
 	__v4l2_ctrl_modify_range(hblank, min, max, hblank->step, min);
 
@@ -1246,10 +1240,8 @@ static int ccs_pll_blanking_update(struct ccs_sensor *sensor)
 
 	dev_dbg(&client->dev, "real timeperframe\t100/%d\n",
 		sensor->pll.pixel_rate_pixel_array /
-		((sensor->pixel_array->crop[CCS_PA_PAD_SRC].width
-		  + sensor->hblank->val) *
-		 (sensor->pixel_array->crop[CCS_PA_PAD_SRC].height
-		  + sensor->vblank->val) / 100));
+		((sensor->pa_src.width + sensor->hblank->val) *
+		 (sensor->pa_src.height + sensor->vblank->val) / 100));
 
 	return 0;
 }
@@ -1756,28 +1748,22 @@ static int ccs_start_streaming(struct ccs_sensor *sensor)
 		goto out;
 
 	/* Analog crop start coordinates */
-	rval = ccs_write(sensor, X_ADDR_START,
-			 sensor->pixel_array->crop[CCS_PA_PAD_SRC].left);
+	rval = ccs_write(sensor, X_ADDR_START, sensor->pa_src.left);
 	if (rval < 0)
 		goto out;
 
-	rval = ccs_write(sensor, Y_ADDR_START,
-			 sensor->pixel_array->crop[CCS_PA_PAD_SRC].top);
+	rval = ccs_write(sensor, Y_ADDR_START, sensor->pa_src.top);
 	if (rval < 0)
 		goto out;
 
 	/* Analog crop end coordinates */
-	rval = ccs_write(
-		sensor, X_ADDR_END,
-		sensor->pixel_array->crop[CCS_PA_PAD_SRC].left
-		+ sensor->pixel_array->crop[CCS_PA_PAD_SRC].width - 1);
+	rval = ccs_write(sensor, X_ADDR_END,
+			 sensor->pa_src.left + sensor->pa_src.width - 1);
 	if (rval < 0)
 		goto out;
 
-	rval = ccs_write(
-		sensor, Y_ADDR_END,
-		sensor->pixel_array->crop[CCS_PA_PAD_SRC].top
-		+ sensor->pixel_array->crop[CCS_PA_PAD_SRC].height - 1);
+	rval = ccs_write(sensor, Y_ADDR_END,
+			 sensor->pa_src.top + sensor->pa_src.height - 1);
 	if (rval < 0)
 		goto out;
 
@@ -1789,27 +1775,23 @@ static int ccs_start_streaming(struct ccs_sensor *sensor)
 	/* Digital crop */
 	if (CCS_LIM(sensor, DIGITAL_CROP_CAPABILITY)
 	    == CCS_DIGITAL_CROP_CAPABILITY_INPUT_CROP) {
-		rval = ccs_write(
-			sensor, DIGITAL_CROP_X_OFFSET,
-			sensor->scaler->crop[CCS_PAD_SINK].left);
+		rval = ccs_write(sensor, DIGITAL_CROP_X_OFFSET,
+				 sensor->scaler_sink.left);
 		if (rval < 0)
 			goto out;
 
-		rval = ccs_write(
-			sensor, DIGITAL_CROP_Y_OFFSET,
-			sensor->scaler->crop[CCS_PAD_SINK].top);
+		rval = ccs_write(sensor, DIGITAL_CROP_Y_OFFSET,
+				 sensor->scaler_sink.top);
 		if (rval < 0)
 			goto out;
 
-		rval = ccs_write(
-			sensor, DIGITAL_CROP_IMAGE_WIDTH,
-			sensor->scaler->crop[CCS_PAD_SINK].width);
+		rval = ccs_write(sensor, DIGITAL_CROP_IMAGE_WIDTH,
+				 sensor->scaler_sink.width);
 		if (rval < 0)
 			goto out;
 
-		rval = ccs_write(
-			sensor, DIGITAL_CROP_IMAGE_HEIGHT,
-			sensor->scaler->crop[CCS_PAD_SINK].height);
+		rval = ccs_write(sensor, DIGITAL_CROP_IMAGE_HEIGHT,
+				 sensor->scaler_sink.height);
 		if (rval < 0)
 			goto out;
 	}
@@ -1827,12 +1809,10 @@ static int ccs_start_streaming(struct ccs_sensor *sensor)
 	}
 
 	/* Output size from sensor */
-	rval = ccs_write(sensor, X_OUTPUT_SIZE,
-			 sensor->src->crop[CCS_PAD_SRC].width);
+	rval = ccs_write(sensor, X_OUTPUT_SIZE, sensor->src_src.width);
 	if (rval < 0)
 		goto out;
-	rval = ccs_write(sensor, Y_OUTPUT_SIZE,
-			 sensor->src->crop[CCS_PAD_SRC].height);
+	rval = ccs_write(sensor, Y_OUTPUT_SIZE, sensor->src_src.height);
 	if (rval < 0)
 		goto out;
 
@@ -2053,24 +2033,8 @@ static int __ccs_get_format(struct v4l2_subdev *subdev,
 			    struct v4l2_subdev_state *sd_state,
 			    struct v4l2_subdev_format *fmt)
 {
-	struct ccs_subdev *ssd = to_ccs_subdev(subdev);
-
-	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-		fmt->format = *v4l2_subdev_get_try_format(subdev, sd_state,
-							  fmt->pad);
-	} else {
-		struct v4l2_rect *r;
-
-		if (fmt->pad == ssd->source_pad)
-			r = &ssd->crop[ssd->source_pad];
-		else
-			r = &ssd->sink_fmt;
-
-		fmt->format.code = __ccs_get_mbus_code(subdev, fmt->pad);
-		fmt->format.width = r->width;
-		fmt->format.height = r->height;
-		fmt->format.field = V4L2_FIELD_NONE;
-	}
+	fmt->format = *v4l2_subdev_get_pad_format(subdev, sd_state, fmt->pad);
+	fmt->format.code = __ccs_get_mbus_code(subdev, fmt->pad);
 
 	return 0;
 }
@@ -2092,28 +2056,18 @@ static int ccs_get_format(struct v4l2_subdev *subdev,
 static void ccs_get_crop_compose(struct v4l2_subdev *subdev,
 				 struct v4l2_subdev_state *sd_state,
 				 struct v4l2_rect **crops,
-				 struct v4l2_rect **comps, int which)
+				 struct v4l2_rect **comps)
 {
 	struct ccs_subdev *ssd = to_ccs_subdev(subdev);
 	unsigned int i;
 
-	if (which == V4L2_SUBDEV_FORMAT_ACTIVE) {
-		if (crops)
-			for (i = 0; i < subdev->entity.num_pads; i++)
-				crops[i] = &ssd->crop[i];
-		if (comps)
-			*comps = &ssd->compose;
-	} else {
-		if (crops) {
-			for (i = 0; i < subdev->entity.num_pads; i++)
-				crops[i] = v4l2_subdev_get_try_crop(subdev,
-								    sd_state,
-								    i);
-		}
-		if (comps)
-			*comps = v4l2_subdev_get_try_compose(subdev, sd_state,
-							     CCS_PAD_SINK);
-	}
+	if (crops)
+		for (i = 0; i < subdev->entity.num_pads; i++)
+			crops[i] =
+				v4l2_subdev_get_pad_crop(subdev, sd_state, i);
+	if (comps)
+		*comps = v4l2_subdev_get_pad_compose(subdev, sd_state,
+						     ssd->sink_pad);
 }
 
 /* Changes require propagation only on sink pad. */
@@ -2125,7 +2079,7 @@ static void ccs_propagate(struct v4l2_subdev *subdev,
 	struct ccs_subdev *ssd = to_ccs_subdev(subdev);
 	struct v4l2_rect *comp, *crops[CCS_PADS];
 
-	ccs_get_crop_compose(subdev, sd_state, crops, &comp, which);
+	ccs_get_crop_compose(subdev, sd_state, crops, &comp);
 
 	switch (target) {
 	case V4L2_SEL_TGT_CROP:
@@ -2136,6 +2090,7 @@ static void ccs_propagate(struct v4l2_subdev *subdev,
 				sensor->scale_m = CCS_LIM(sensor, SCALER_N_MIN);
 				sensor->scaling_mode =
 					CCS_SCALING_MODE_NO_SCALING;
+				sensor->scaler_sink = *comp;
 			} else if (ssd == sensor->binner) {
 				sensor->binning_horizontal = 1;
 				sensor->binning_vertical = 1;
@@ -2144,6 +2099,8 @@ static void ccs_propagate(struct v4l2_subdev *subdev,
 		fallthrough;
 	case V4L2_SEL_TGT_COMPOSE:
 		*crops[CCS_PAD_SRC] = *comp;
+		if (which == V4L2_SUBDEV_FORMAT_ACTIVE && ssd == sensor->src)
+			sensor->src_src = *crops[CCS_PAD_SRC];
 		break;
 	default:
 		WARN_ON_ONCE(1);
@@ -2252,14 +2209,12 @@ static int ccs_set_format(struct v4l2_subdev *subdev,
 		      CCS_LIM(sensor, MIN_Y_OUTPUT_SIZE),
 		      CCS_LIM(sensor, MAX_Y_OUTPUT_SIZE));
 
-	ccs_get_crop_compose(subdev, sd_state, crops, NULL, fmt->which);
+	ccs_get_crop_compose(subdev, sd_state, crops, NULL);
 
 	crops[ssd->sink_pad]->left = 0;
 	crops[ssd->sink_pad]->top = 0;
 	crops[ssd->sink_pad]->width = fmt->format.width;
 	crops[ssd->sink_pad]->height = fmt->format.height;
-	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
-		ssd->sink_fmt = *crops[ssd->sink_pad];
 	ccs_propagate(subdev, sd_state, fmt->which, V4L2_SEL_TGT_CROP);
 
 	mutex_unlock(&sensor->mutex);
@@ -2482,7 +2437,7 @@ static int ccs_set_compose(struct v4l2_subdev *subdev,
 	struct ccs_subdev *ssd = to_ccs_subdev(subdev);
 	struct v4l2_rect *comp, *crops[CCS_PADS];
 
-	ccs_get_crop_compose(subdev, sd_state, crops, &comp, sel->which);
+	ccs_get_crop_compose(subdev, sd_state, crops, &comp);
 
 	sel->r.top = 0;
 	sel->r.left = 0;
@@ -2501,8 +2456,8 @@ static int ccs_set_compose(struct v4l2_subdev *subdev,
 	return 0;
 }
 
-static int __ccs_sel_supported(struct v4l2_subdev *subdev,
-			       struct v4l2_subdev_selection *sel)
+static int ccs_sel_supported(struct v4l2_subdev *subdev,
+			     struct v4l2_subdev_selection *sel)
 {
 	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
 	struct ccs_subdev *ssd = to_ccs_subdev(subdev);
@@ -2545,33 +2500,18 @@ static int ccs_set_crop(struct v4l2_subdev *subdev,
 {
 	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
 	struct ccs_subdev *ssd = to_ccs_subdev(subdev);
-	struct v4l2_rect *src_size, *crops[CCS_PADS];
-	struct v4l2_rect _r;
+	struct v4l2_rect src_size = { 0 }, *crops[CCS_PADS], *comp;
 
-	ccs_get_crop_compose(subdev, sd_state, crops, NULL, sel->which);
+	ccs_get_crop_compose(subdev, sd_state, crops, &comp);
 
-	if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
-		if (sel->pad == ssd->sink_pad)
-			src_size = &ssd->sink_fmt;
-		else
-			src_size = &ssd->compose;
+	if (sel->pad == ssd->sink_pad) {
+		struct v4l2_mbus_framefmt *mfmt =
+			v4l2_subdev_get_pad_format(subdev, sd_state, sel->pad);
+
+		src_size.width = mfmt->width;
+		src_size.height = mfmt->height;
 	} else {
-		if (sel->pad == ssd->sink_pad) {
-			_r.left = 0;
-			_r.top = 0;
-			_r.width = v4l2_subdev_get_try_format(subdev,
-							      sd_state,
-							      sel->pad)
-				->width;
-			_r.height = v4l2_subdev_get_try_format(subdev,
-							       sd_state,
-							       sel->pad)
-				->height;
-			src_size = &_r;
-		} else {
-			src_size = v4l2_subdev_get_try_compose(
-				subdev, sd_state, ssd->sink_pad);
-		}
+		src_size = *comp;
 	}
 
 	if (ssd == sensor->src && sel->pad == CCS_PAD_SRC) {
@@ -2579,16 +2519,19 @@ static int ccs_set_crop(struct v4l2_subdev *subdev,
 		sel->r.top = 0;
 	}
 
-	sel->r.width = min(sel->r.width, src_size->width);
-	sel->r.height = min(sel->r.height, src_size->height);
+	sel->r.width = min(sel->r.width, src_size.width);
+	sel->r.height = min(sel->r.height, src_size.height);
 
-	sel->r.left = min_t(int, sel->r.left, src_size->width - sel->r.width);
-	sel->r.top = min_t(int, sel->r.top, src_size->height - sel->r.height);
+	sel->r.left = min_t(int, sel->r.left, src_size.width - sel->r.width);
+	sel->r.top = min_t(int, sel->r.top, src_size.height - sel->r.height);
 
 	*crops[sel->pad] = sel->r;
 
 	if (ssd != sensor->pixel_array && sel->pad == CCS_PAD_SINK)
 		ccs_propagate(subdev, sd_state, sel->which, V4L2_SEL_TGT_CROP);
+	else if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE &&
+		 ssd == sensor->pixel_array)
+		sensor->pa_src = sel->r;
 
 	return 0;
 }
@@ -2601,44 +2544,36 @@ static void ccs_get_native_size(struct ccs_subdev *ssd, struct v4l2_rect *r)
 	r->height = CCS_LIM(ssd->sensor, Y_ADDR_MAX) + 1;
 }
 
-static int __ccs_get_selection(struct v4l2_subdev *subdev,
-			       struct v4l2_subdev_state *sd_state,
-			       struct v4l2_subdev_selection *sel)
+static int ccs_get_selection(struct v4l2_subdev *subdev,
+			     struct v4l2_subdev_state *sd_state,
+			     struct v4l2_subdev_selection *sel)
 {
 	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
 	struct ccs_subdev *ssd = to_ccs_subdev(subdev);
 	struct v4l2_rect *comp, *crops[CCS_PADS];
-	struct v4l2_rect sink_fmt;
 	int ret;
 
-	ret = __ccs_sel_supported(subdev, sel);
+	ret = ccs_sel_supported(subdev, sel);
 	if (ret)
 		return ret;
 
-	ccs_get_crop_compose(subdev, sd_state, crops, &comp, sel->which);
-
-	if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
-		sink_fmt = ssd->sink_fmt;
-	} else {
-		struct v4l2_mbus_framefmt *fmt =
-			v4l2_subdev_get_try_format(subdev, sd_state,
-						   ssd->sink_pad);
-
-		sink_fmt.left = 0;
-		sink_fmt.top = 0;
-		sink_fmt.width = fmt->width;
-		sink_fmt.height = fmt->height;
-	}
+	ccs_get_crop_compose(subdev, sd_state, crops, &comp);
 
 	switch (sel->target) {
 	case V4L2_SEL_TGT_CROP_BOUNDS:
 	case V4L2_SEL_TGT_NATIVE_SIZE:
-		if (ssd == sensor->pixel_array)
+		if (ssd == sensor->pixel_array) {
 			ccs_get_native_size(ssd, &sel->r);
-		else if (sel->pad == ssd->sink_pad)
-			sel->r = sink_fmt;
-		else
+		} else if (sel->pad == ssd->sink_pad) {
+			struct v4l2_mbus_framefmt *sink_fmt =
+				v4l2_subdev_get_pad_format(subdev, sd_state,
+							   ssd->sink_pad);
+			sel->r.top = sel->r.left = 0;
+			sel->r.width = sink_fmt->width;
+			sel->r.height = sink_fmt->height;
+		} else {
 			sel->r = *comp;
+		}
 		break;
 	case V4L2_SEL_TGT_CROP:
 	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
@@ -2652,20 +2587,6 @@ static int __ccs_get_selection(struct v4l2_subdev *subdev,
 	return 0;
 }
 
-static int ccs_get_selection(struct v4l2_subdev *subdev,
-			     struct v4l2_subdev_state *sd_state,
-			     struct v4l2_subdev_selection *sel)
-{
-	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
-	int rval;
-
-	mutex_lock(&sensor->mutex);
-	rval = __ccs_get_selection(subdev, sd_state, sel);
-	mutex_unlock(&sensor->mutex);
-
-	return rval;
-}
-
 static int ccs_set_selection(struct v4l2_subdev *subdev,
 			     struct v4l2_subdev_state *sd_state,
 			     struct v4l2_subdev_selection *sel)
@@ -2673,7 +2594,7 @@ static int ccs_set_selection(struct v4l2_subdev *subdev,
 	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
 	int ret;
 
-	ret = __ccs_sel_supported(subdev, sel);
+	ret = ccs_sel_supported(subdev, sel);
 	if (ret)
 		return ret;
 
@@ -3020,8 +2941,10 @@ static void ccs_cleanup(struct ccs_sensor *sensor)
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
 	unsigned int i;
 
-	for (i = 0; i < sensor->ssds_used; i++)
+	for (i = 0; i < sensor->ssds_used; i++) {
+		v4l2_subdev_cleanup(&sensor->ssds[2].sd);
 		media_entity_cleanup(&sensor->ssds[i].sd.entity);
+	}
 
 	device_remove_file(&client->dev, &dev_attr_nvm);
 	device_remove_file(&client->dev, &dev_attr_ident);
@@ -3051,31 +2974,29 @@ static int ccs_init_subdev(struct ccs_sensor *sensor,
 
 	v4l2_i2c_subdev_set_name(&ssd->sd, client, sensor->minfo.name, name);
 
-	ccs_get_native_size(ssd, &ssd->sink_fmt);
-
-	ssd->compose.width = ssd->sink_fmt.width;
-	ssd->compose.height = ssd->sink_fmt.height;
-	ssd->crop[ssd->source_pad] = ssd->compose;
 	ssd->pads[ssd->source_pad].flags = MEDIA_PAD_FL_SOURCE;
-	if (ssd != sensor->pixel_array) {
-		ssd->crop[ssd->sink_pad] = ssd->compose;
+	if (ssd != sensor->pixel_array)
 		ssd->pads[ssd->sink_pad].flags = MEDIA_PAD_FL_SINK;
-	}
 
 	ssd->sd.entity.ops = &ccs_entity_ops;
 
+	if (ssd != sensor->src) {
+		ssd->sd.owner = THIS_MODULE;
+		ssd->sd.dev = &client->dev;
+		v4l2_set_subdevdata(&ssd->sd, client);
+	}
+
 	rval = media_entity_pads_init(&ssd->sd.entity, ssd->npads, ssd->pads);
 	if (rval) {
 		dev_err(&client->dev, "media_entity_pads_init failed\n");
 		return rval;
 	}
 
-	if (ssd == sensor->src)
-		return 0;
-
-	ssd->sd.owner = THIS_MODULE;
-	ssd->sd.dev = &client->dev;
-	v4l2_set_subdevdata(&ssd->sd, client);
+	rval = v4l2_subdev_init_finalize(&ssd->sd);
+	if (rval) {
+		media_entity_cleanup(&ssd->sd.entity);
+		return rval;
+	}
 
 	return 0;
 }
@@ -3090,24 +3011,24 @@ static int ccs_init_cfg(struct v4l2_subdev *sd,
 	mutex_lock(&sensor->mutex);
 
 	for (i = 0; i < ssd->npads; i++) {
-		struct v4l2_mbus_framefmt *try_fmt =
-			v4l2_subdev_get_try_format(sd, sd_state, i);
-		struct v4l2_rect *try_crop =
-			v4l2_subdev_get_try_crop(sd, sd_state, i);
-		struct v4l2_rect *try_comp;
+		struct v4l2_mbus_framefmt *fmt =
+			v4l2_subdev_get_pad_format(sd, sd_state, i);
+		struct v4l2_rect *crop =
+			v4l2_subdev_get_pad_crop(sd, sd_state, i);
+		struct v4l2_rect *comp;
 
-		ccs_get_native_size(ssd, try_crop);
+		ccs_get_native_size(ssd, crop);
 
-		try_fmt->width = try_crop->width;
-		try_fmt->height = try_crop->height;
-		try_fmt->code = sensor->internal_csi_format->code;
-		try_fmt->field = V4L2_FIELD_NONE;
+		fmt->width = crop->width;
+		fmt->height = crop->height;
+		fmt->code = sensor->internal_csi_format->code;
+		fmt->field = V4L2_FIELD_NONE;
 
 		if (ssd == sensor->pixel_array)
 			continue;
 
-		try_comp = v4l2_subdev_get_try_compose(sd, sd_state, i);
-		*try_comp = *try_crop;
+		comp = v4l2_subdev_get_pad_compose(sd, sd_state, i);
+		*comp = *crop;
 	}
 
 	mutex_unlock(&sensor->mutex);
@@ -3653,7 +3574,6 @@ static void ccs_remove(struct i2c_client *client)
 
 	for (i = 0; i < sensor->ssds_used; i++)
 		v4l2_device_unregister_subdev(&sensor->ssds[i].sd);
-
 	ccs_cleanup(sensor);
 	mutex_destroy(&sensor->mutex);
 	kfree(sensor->ccs_limits);
diff --git a/drivers/media/i2c/ccs/ccs.h b/drivers/media/i2c/ccs/ccs.h
index a94c796cea48..9c3587b2fbe7 100644
--- a/drivers/media/i2c/ccs/ccs.h
+++ b/drivers/media/i2c/ccs/ccs.h
@@ -182,9 +182,6 @@ struct ccs_binning_subtype {
 struct ccs_subdev {
 	struct v4l2_subdev sd;
 	struct media_pad pads[CCS_PADS];
-	struct v4l2_rect sink_fmt;
-	struct v4l2_rect crop[CCS_PADS];
-	struct v4l2_rect compose; /* compose on sink */
 	unsigned short sink_pad;
 	unsigned short source_pad;
 	int npads;
@@ -220,6 +217,7 @@ struct ccs_sensor {
 	u32 mbus_frame_fmts;
 	const struct ccs_csi_data_format *csi_format;
 	const struct ccs_csi_data_format *internal_csi_format;
+	struct v4l2_rect pa_src, scaler_sink, src_src;
 	u32 default_mbus_frame_fmts;
 	int default_pixel_order;
 	struct ccs_data_container sdata, mdata;
-- 
2.39.2

[PATCH v7 10/19] media: ccs: Partially revert "media: i2c: Use pm_runtime_resume_and_get()"

[Sakari Ailus]

ccs_pm_get_init() depends on the return values > 0 of
pm_runtime_get_sync(), thus it can't use pm_runtime_resume_and_get().
There's even a comment in the driver on this, a few lines above the code.

Fixes: aa0adb399d09 ("media: i2c: Use pm_runtime_resume_and_get()")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index c6853622946b..34f4f62a9523 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -1873,9 +1873,9 @@ static int ccs_pm_get_init(struct ccs_sensor *sensor)
 	 * relies at the returned value to detect if the device was already
 	 * active or not.
 	 */
-	rval = pm_runtime_resume_and_get(&client->dev);
-	if (rval)
-		return rval;
+	rval = pm_runtime_get_sync(&client->dev);
+	if (rval < 0)
+		goto error;
 
 	/* Device was already active, so don't set controls */
 	if (rval == 1)
-- 
2.39.2

[PATCH v7 11/19] media: ccs: Drop re-entrant s_stream support

[Sakari Ailus]

The s_stream is called to enable and to disable streaming on a sub-device.
The caller may only call it to change the state, enabling streaming is not
allowed when it is already disabled, and similarly for disabling
streaming. Remove the check from the CCS driver.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 34f4f62a9523..2abfd5932e02 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -1903,9 +1903,6 @@ static int ccs_set_stream(struct v4l2_subdev *subdev, int enable)
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
 	int rval;
 
-	if (sensor->streaming == enable)
-		return 0;
-
 	if (!enable) {
 		ccs_stop_streaming(sensor);
 		sensor->streaming = false;
-- 
2.39.2

[PATCH v7 12/19] media: ov2740: Enable runtime PM before registering the async subdev

[Sakari Ailus]

Enable runtime PM before registering the async subdev as the driver UAPI
may become accessible immediately after the registration. Runtime PM needs
to be enabled by that time.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov2740.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index 3a9700fbbe8c..6d5fb2789dc6 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -1130,6 +1130,12 @@ static int ov2740_probe(struct i2c_client *client)
 		goto probe_error_v4l2_ctrl_handler_free;
 	}
 
+	/* Set the device's state to active if it's in D0 state. */
+	if (full_power)
+		pm_runtime_set_active(&client->dev);
+	pm_runtime_enable(&client->dev);
+	pm_runtime_idle(&client->dev);
+
 	ret = v4l2_async_register_subdev_sensor(&ov2740->sd);
 	if (ret < 0) {
 		dev_err_probe(dev, ret, "failed to register V4L2 subdev\n");
@@ -1140,16 +1146,12 @@ static int ov2740_probe(struct i2c_client *client)
 	if (ret)
 		dev_warn(&client->dev, "register nvmem failed, ret %d\n", ret);
 
-	/* Set the device's state to active if it's in D0 state. */
-	if (full_power)
-		pm_runtime_set_active(&client->dev);
-	pm_runtime_enable(&client->dev);
-	pm_runtime_idle(&client->dev);
-
 	return 0;
 
 probe_error_media_entity_cleanup:
 	media_entity_cleanup(&ov2740->sd.entity);
+	pm_runtime_disable(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
 
 probe_error_v4l2_ctrl_handler_free:
 	v4l2_ctrl_handler_free(ov2740->sd.ctrl_handler);
-- 
2.39.2

[PATCH v7 13/19] media: ov2740: Use sub-device active state

[Sakari Ailus]

Use sub-device active state. Rely on control handler lock to serialise
access to the active state. Also clean up locking on s_stream handler.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov2740.c | 109 ++++++++++++++-----------------------
 1 file changed, 42 insertions(+), 67 deletions(-)

diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index 6d5fb2789dc6..d83ac31efd9c 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -336,9 +336,6 @@ struct ov2740 {
 	/* Current mode */
 	const struct ov2740_mode *cur_mode;
 
-	/* To serialize asynchronus callbacks */
-	struct mutex mutex;
-
 	/* NVM data inforamtion */
 	struct nvm_data *nvm;
 
@@ -579,7 +576,6 @@ static int ov2740_init_controls(struct ov2740 *ov2740)
 	if (ret)
 		return ret;
 
-	ctrl_hdlr->lock = &ov2740->mutex;
 	cur_mode = ov2740->cur_mode;
 	size = ARRAY_SIZE(link_freq_menu_items);
 
@@ -789,15 +785,15 @@ static int ov2740_set_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct ov2740 *ov2740 = to_ov2740(sd);
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct v4l2_subdev_state *sd_state;
 	int ret = 0;
 
-	mutex_lock(&ov2740->mutex);
+	sd_state = v4l2_subdev_lock_and_get_active_state(&ov2740->sd);
+
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-		if (ret < 0) {
-			mutex_unlock(&ov2740->mutex);
-			return ret;
-		}
+		if (ret < 0)
+			goto out_unlock;
 
 		ret = ov2740_start_streaming(ov2740);
 		if (ret) {
@@ -810,7 +806,8 @@ static int ov2740_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	mutex_unlock(&ov2740->mutex);
+out_unlock:
+	v4l2_subdev_unlock_state(sd_state);
 
 	return ret;
 }
@@ -828,48 +825,26 @@ static int ov2740_set_format(struct v4l2_subdev *sd,
 				      height, fmt->format.width,
 				      fmt->format.height);
 
-	mutex_lock(&ov2740->mutex);
 	ov2740_update_pad_format(mode, &fmt->format);
-	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-		*v4l2_subdev_get_try_format(sd, sd_state, fmt->pad) = fmt->format;
-	} else {
-		ov2740->cur_mode = mode;
-		__v4l2_ctrl_s_ctrl(ov2740->link_freq, mode->link_freq_index);
-		__v4l2_ctrl_s_ctrl_int64(ov2740->pixel_rate,
-					 to_pixel_rate(mode->link_freq_index));
-
-		/* Update limits and set FPS to default */
-		vblank_def = mode->vts_def - mode->height;
-		__v4l2_ctrl_modify_range(ov2740->vblank,
-					 mode->vts_min - mode->height,
-					 OV2740_VTS_MAX - mode->height, 1,
-					 vblank_def);
-		__v4l2_ctrl_s_ctrl(ov2740->vblank, vblank_def);
-		h_blank = to_pixels_per_line(mode->hts, mode->link_freq_index) -
-			  mode->width;
-		__v4l2_ctrl_modify_range(ov2740->hblank, h_blank, h_blank, 1,
-					 h_blank);
-	}
-	mutex_unlock(&ov2740->mutex);
+	*v4l2_subdev_get_pad_format(sd, sd_state, fmt->pad) = fmt->format;
 
-	return 0;
-}
-
-static int ov2740_get_format(struct v4l2_subdev *sd,
-			     struct v4l2_subdev_state *sd_state,
-			     struct v4l2_subdev_format *fmt)
-{
-	struct ov2740 *ov2740 = to_ov2740(sd);
-
-	mutex_lock(&ov2740->mutex);
 	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
-		fmt->format = *v4l2_subdev_get_try_format(&ov2740->sd,
-							  sd_state,
-							  fmt->pad);
-	else
-		ov2740_update_pad_format(ov2740->cur_mode, &fmt->format);
+		return 0;
 
-	mutex_unlock(&ov2740->mutex);
+	ov2740->cur_mode = mode;
+	__v4l2_ctrl_s_ctrl(ov2740->link_freq, mode->link_freq_index);
+	__v4l2_ctrl_s_ctrl_int64(ov2740->pixel_rate,
+				 to_pixel_rate(mode->link_freq_index));
+
+	/* Update limits and set FPS to default */
+	vblank_def = mode->vts_def - mode->height;
+	__v4l2_ctrl_modify_range(ov2740->vblank,
+				 mode->vts_min - mode->height,
+				 OV2740_VTS_MAX - mode->height, 1, vblank_def);
+	__v4l2_ctrl_s_ctrl(ov2740->vblank, vblank_def);
+	h_blank = to_pixels_per_line(mode->hts, mode->link_freq_index) -
+		mode->width;
+	__v4l2_ctrl_modify_range(ov2740->hblank, h_blank, h_blank, 1, h_blank);
 
 	return 0;
 }
@@ -904,14 +879,11 @@ static int ov2740_enum_frame_size(struct v4l2_subdev *sd,
 	return 0;
 }
 
-static int ov2740_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+static int ov2740_init_cfg(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_state *sd_state)
 {
-	struct ov2740 *ov2740 = to_ov2740(sd);
-
-	mutex_lock(&ov2740->mutex);
 	ov2740_update_pad_format(&supported_modes[0],
-				 v4l2_subdev_get_try_format(sd, fh->state, 0));
-	mutex_unlock(&ov2740->mutex);
+				 v4l2_subdev_get_pad_format(sd, sd_state, 0));
 
 	return 0;
 }
@@ -921,10 +893,11 @@ static const struct v4l2_subdev_video_ops ov2740_video_ops = {
 };
 
 static const struct v4l2_subdev_pad_ops ov2740_pad_ops = {
+	.get_fmt = v4l2_subdev_get_fmt,
 	.set_fmt = ov2740_set_format,
-	.get_fmt = ov2740_get_format,
 	.enum_mbus_code = ov2740_enum_mbus_code,
 	.enum_frame_size = ov2740_enum_frame_size,
+	.init_cfg = ov2740_init_cfg,
 };
 
 static const struct v4l2_subdev_ops ov2740_subdev_ops = {
@@ -936,10 +909,6 @@ static const struct media_entity_operations ov2740_subdev_entity_ops = {
 	.link_validate = v4l2_subdev_link_validate,
 };
 
-static const struct v4l2_subdev_internal_ops ov2740_internal_ops = {
-	.open = ov2740_open,
-};
-
 static int ov2740_check_hwcfg(struct device *dev)
 {
 	struct fwnode_handle *ep;
@@ -1005,13 +974,12 @@ static int ov2740_check_hwcfg(struct device *dev)
 static void ov2740_remove(struct i2c_client *client)
 {
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct ov2740 *ov2740 = to_ov2740(sd);
 
 	v4l2_async_unregister_subdev(sd);
 	media_entity_cleanup(&sd->entity);
+	v4l2_subdev_cleanup(sd);
 	v4l2_ctrl_handler_free(sd->ctrl_handler);
 	pm_runtime_disable(&client->dev);
-	mutex_destroy(&ov2740->mutex);
 }
 
 static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
@@ -1020,9 +988,11 @@ static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
 	struct nvm_data *nvm = priv;
 	struct device *dev = regmap_get_device(nvm->regmap);
 	struct ov2740 *ov2740 = to_ov2740(dev_get_drvdata(dev));
+	struct v4l2_subdev_state *sd_state;
 	int ret = 0;
 
-	mutex_lock(&ov2740->mutex);
+	/* Serialise sensor access */
+	sd_state = v4l2_subdev_lock_and_get_active_state(&ov2740->sd);
 
 	if (nvm->nvm_buffer) {
 		memcpy(val, nvm->nvm_buffer + off, count);
@@ -1040,7 +1010,7 @@ static int ov2740_nvmem_read(void *priv, unsigned int off, void *val,
 
 	pm_runtime_put(dev);
 exit:
-	mutex_unlock(&ov2740->mutex);
+	v4l2_subdev_unlock_state(sd_state);
 	return ret;
 }
 
@@ -1111,7 +1081,6 @@ static int ov2740_probe(struct i2c_client *client)
 			return dev_err_probe(dev, ret, "failed to find sensor\n");
 	}
 
-	mutex_init(&ov2740->mutex);
 	ov2740->cur_mode = &supported_modes[0];
 	ret = ov2740_init_controls(ov2740);
 	if (ret) {
@@ -1119,7 +1088,7 @@ static int ov2740_probe(struct i2c_client *client)
 		goto probe_error_v4l2_ctrl_handler_free;
 	}
 
-	ov2740->sd.internal_ops = &ov2740_internal_ops;
+	ov2740->sd.state_lock = ov2740->ctrl_handler.lock;
 	ov2740->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	ov2740->sd.entity.ops = &ov2740_subdev_entity_ops;
 	ov2740->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
@@ -1130,6 +1099,10 @@ static int ov2740_probe(struct i2c_client *client)
 		goto probe_error_v4l2_ctrl_handler_free;
 	}
 
+	ret = v4l2_subdev_init_finalize(&ov2740->sd);
+	if (ret)
+		goto probe_error_media_entity_cleanup;
+
 	/* Set the device's state to active if it's in D0 state. */
 	if (full_power)
 		pm_runtime_set_active(&client->dev);
@@ -1139,7 +1112,7 @@ static int ov2740_probe(struct i2c_client *client)
 	ret = v4l2_async_register_subdev_sensor(&ov2740->sd);
 	if (ret < 0) {
 		dev_err_probe(dev, ret, "failed to register V4L2 subdev\n");
-		goto probe_error_media_entity_cleanup;
+		goto probe_error_v4l2_subdev_cleanup;
 	}
 
 	ret = ov2740_register_nvmem(client, ov2740);
@@ -1148,6 +1121,9 @@ static int ov2740_probe(struct i2c_client *client)
 
 	return 0;
 
+probe_error_v4l2_subdev_cleanup:
+	v4l2_subdev_cleanup(&ov2740->sd);
+
 probe_error_media_entity_cleanup:
 	media_entity_cleanup(&ov2740->sd.entity);
 	pm_runtime_disable(&client->dev);
@@ -1155,7 +1131,6 @@ static int ov2740_probe(struct i2c_client *client)
 
 probe_error_v4l2_ctrl_handler_free:
 	v4l2_ctrl_handler_free(ov2740->sd.ctrl_handler);
-	mutex_destroy(&ov2740->mutex);
 
 	return ret;
 }
-- 
2.39.2

[PATCH v7 14/19] media: ov2740: Return -EPROBE_DEFER if no endpoint is found

[Sakari Ailus]

With ipu bridge, endpoints may only be created when ipu bridge has
initialised. This may happen after the sensor driver has first probed.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov2740.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index d83ac31efd9c..24e468485fbf 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -931,7 +931,7 @@ static int ov2740_check_hwcfg(struct device *dev)
 
 	ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
 	if (!ep)
-		return -ENXIO;
+		return -EPROBE_DEFER;
 
 	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
 	fwnode_handle_put(ep);
-- 
2.39.2

[PATCH v7 15/19] media: v4l: subdev: Clear frame descriptor before get_frame_desc

[Sakari Ailus]

Clear frame descriptor before calling transmitter's get_frame_desc() op.
Also remove the corresponding memset() calls from drivers.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/media/i2c/ds90ub913.c              | 2 --
 drivers/media/i2c/ds90ub953.c              | 2 --
 drivers/media/i2c/ds90ub960.c              | 2 --
 drivers/media/platform/nxp/imx-mipi-csis.c | 2 --
 drivers/media/v4l2-core/v4l2-subdev.c      | 9 +++++++++
 5 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ds90ub913.c b/drivers/media/i2c/ds90ub913.c
index 4bfa3b3cf619..8e9ebed09f64 100644
--- a/drivers/media/i2c/ds90ub913.c
+++ b/drivers/media/i2c/ds90ub913.c
@@ -362,8 +362,6 @@ static int ub913_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 	if (ret)
 		return ret;
 
-	memset(fd, 0, sizeof(*fd));
-
 	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL;
 
 	state = v4l2_subdev_lock_and_get_active_state(sd);
diff --git a/drivers/media/i2c/ds90ub953.c b/drivers/media/i2c/ds90ub953.c
index dc394e22a42c..644022312833 100644
--- a/drivers/media/i2c/ds90ub953.c
+++ b/drivers/media/i2c/ds90ub953.c
@@ -499,8 +499,6 @@ static int ub953_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 	if (ret)
 		return ret;
 
-	memset(fd, 0, sizeof(*fd));
-
 	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
 
 	state = v4l2_subdev_lock_and_get_active_state(sd);
diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
index 8ba5750f5a23..b8f3e5ca03ef 100644
--- a/drivers/media/i2c/ds90ub960.c
+++ b/drivers/media/i2c/ds90ub960.c
@@ -2786,8 +2786,6 @@ static int ub960_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 	if (!ub960_pad_is_source(priv, pad))
 		return -EINVAL;
 
-	memset(fd, 0, sizeof(*fd));
-
 	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
 
 	state = v4l2_subdev_lock_and_get_active_state(&priv->sd);
diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 16f19a640130..aac9cffe503c 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -1114,8 +1114,6 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 	fd->type = V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL;
 	fd->num_entries = 1;
 
-	memset(entry, 0, sizeof(*entry));
-
 	entry->flags = 0;
 	entry->pixelcode = csis_fmt->code;
 	entry->bus.csi2.vc = 0;
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 32b7d9cd43e6..9cb04ace86ae 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -306,6 +306,14 @@ static int call_set_selection(struct v4l2_subdev *sd,
 	       sd->ops->pad->set_selection(sd, state, sel);
 }
 
+static int call_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+			       struct v4l2_mbus_frame_desc *fd)
+{
+	memset(fd, 0, sizeof(*fd));
+
+	return sd->ops->pad->get_frame_desc(sd, pad, fd);
+}
+
 static inline int check_edid(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_edid *edid)
 {
@@ -446,6 +454,7 @@ static const struct v4l2_subdev_pad_ops v4l2_subdev_call_pad_wrappers = {
 	.set_edid		= call_set_edid,
 	.dv_timings_cap		= call_dv_timings_cap,
 	.enum_dv_timings	= call_enum_dv_timings,
+	.get_frame_desc		= call_get_frame_desc,
 	.get_mbus_config	= call_get_mbus_config,
 };
 
-- 
2.39.2

[PATCH v7 16/19] media: v4l: subdev: Print debug information on frame descriptor

[Sakari Ailus]

Print debug level information on returned frame descriptors.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 31 ++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 9cb04ace86ae..d295a4e87b66 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -15,6 +15,7 @@
 #include <linux/module.h>
 #include <linux/overflow.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/types.h>
 #include <linux/version.h>
 #include <linux/videodev2.h>
@@ -309,9 +310,37 @@ static int call_set_selection(struct v4l2_subdev *sd,
 static int call_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
 			       struct v4l2_mbus_frame_desc *fd)
 {
+	unsigned int i;
+	int ret;
+
 	memset(fd, 0, sizeof(*fd));
 
-	return sd->ops->pad->get_frame_desc(sd, pad, fd);
+	ret = sd->ops->pad->get_frame_desc(sd, pad, fd);
+	if (ret)
+		return ret;
+
+	dev_dbg(sd->dev, "Frame descriptor on pad %u, type %s\n", pad,
+		fd->type == V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL ? "parallel" :
+		fd->type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2 ? "CSI-2" :
+		"unknown");
+
+	for (i = 0; i < fd->num_entries; i++) {
+		struct v4l2_mbus_frame_desc_entry *entry = &fd->entry[i];
+		char buf[20] = "";
+
+		if (fd->type == V4L2_MBUS_FRAME_DESC_TYPE_CSI2)
+			WARN_ON(snprintf(buf, sizeof(buf),
+					 ", vc %u, dt 0x%02x",
+					 entry->bus.csi2.vc,
+					 entry->bus.csi2.dt) >= sizeof(buf));
+
+		dev_dbg(sd->dev,
+			"\tstream %u, code 0x%04x, length %u, flags 0x%04x%s\n",
+			entry->stream, entry->pixelcode, entry->length,
+			entry->flags, buf);
+	}
+
+	return 0;
 }
 
 static inline int check_edid(struct v4l2_subdev *sd,
-- 
2.39.2

[PATCH v7 17/19] media: mc: Check pad flag validity

[Sakari Ailus]

Check the validity of pad flags on entity init. Exactly one of the flags
must be set.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/mc/mc-entity.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 83468d4a440b..543a392f8635 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -197,6 +197,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
 	struct media_device *mdev = entity->graph_obj.mdev;
 	struct media_pad *iter;
 	unsigned int i = 0;
+	int ret = 0;
 
 	if (num_pads >= MEDIA_ENTITY_MAX_PADS)
 		return -E2BIG;
@@ -210,15 +211,27 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
 	media_entity_for_each_pad(entity, iter) {
 		iter->entity = entity;
 		iter->index = i++;
+
+		if (hweight32(iter->flags & (MEDIA_PAD_FL_SINK |
+					     MEDIA_PAD_FL_SOURCE)) != 1) {
+			ret = -EINVAL;
+			break;
+		}
+
 		if (mdev)
 			media_gobj_create(mdev, MEDIA_GRAPH_PAD,
 					  &iter->graph_obj);
 	}
 
+	if (ret && mdev) {
+		media_entity_for_each_pad(entity, iter)
+			media_gobj_destroy(&iter->graph_obj);
+	}
+
 	if (mdev)
 		mutex_unlock(&mdev->graph_mutex);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(media_entity_pads_init);
 
-- 
2.39.2

[PATCH v7 18/19] media: Add MIPI CSI-2 generic long packet type definition

[Sakari Ailus]

Add a definition for MIPI CSI-2 generic long packet types. The generic
long packet types are numbered from 1 to 4.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/media/mipi-csi2.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/media/mipi-csi2.h b/include/media/mipi-csi2.h
index c3d8f12234b1..40fc0264250d 100644
--- a/include/media/mipi-csi2.h
+++ b/include/media/mipi-csi2.h
@@ -19,6 +19,7 @@
 #define MIPI_CSI2_DT_NULL		0x10
 #define MIPI_CSI2_DT_BLANKING		0x11
 #define MIPI_CSI2_DT_EMBEDDED_8B	0x12
+#define MIPI_CSI2_DT_GENERIC_LONG(n)	(0x13 + (n) - 1)	/* 1..4 */
 #define MIPI_CSI2_DT_YUV420_8B		0x18
 #define MIPI_CSI2_DT_YUV420_10B		0x19
 #define MIPI_CSI2_DT_YUV420_8B_LEGACY	0x1a
-- 
2.39.2

[PATCH v7 19/19] media: Documentation: Split camera sensor documentation

[Sakari Ailus]

Split camera sensor documentation into user and kernel portions. This
should make it easier for the user space developers to find the relevant
documentation.

Also add a list of exemplary drivers and add imx219 driver to it, besides
those that were already mentioned.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 .../driver-api/media/camera-sensor.rst        | 131 +++++-------------
 .../media/drivers/camera-sensor.rst           | 104 ++++++++++++++
 .../userspace-api/media/drivers/index.rst     |   1 +
 .../userspace-api/media/v4l/control.rst       |   4 +
 4 files changed, 147 insertions(+), 93 deletions(-)
 create mode 100644 Documentation/userspace-api/media/drivers/camera-sensor.rst

diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
index 2acc08142a1a..6456145f96ed 100644
--- a/Documentation/driver-api/media/camera-sensor.rst
+++ b/Documentation/driver-api/media/camera-sensor.rst
@@ -1,8 +1,14 @@
 .. SPDX-License-Identifier: GPL-2.0
 
+.. _media_writing_camera_sensor_drivers:
+
 Writing camera sensor drivers
 =============================
 
+This document covers the in-kernel APIs only. For the best practices on
+userspace API implementation in camera sensor drivers, please see
+:ref:`media_using_camera_sensor_drivers`.
+
 CSI-2 and parallel (BT.601 and BT.656) busses
 ---------------------------------------------
 
@@ -34,7 +40,8 @@ Devicetree
 
 The preferred way to achieve this is using ``assigned-clocks``,
 ``assigned-clock-parents`` and ``assigned-clock-rates`` properties. See the
-`clock device tree bindings <https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/clock/clock.yaml>`_
+`clock device tree bindings
+<https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/clock/clock.yaml>`_
 for more information. The driver then gets the frequency using
 ``clk_get_rate()``.
 
@@ -85,9 +92,7 @@ PM instead. If you feel you need to begin calling ``.s_power()`` from an ISP or
 a bridge driver, instead add runtime PM support to the sensor driver you are
 using and drop its ``.s_power()`` handler.
 
-See examples of runtime PM handling in e.g. ``drivers/media/i2c/ov8856.c`` and
-``drivers/media/i2c/ccs/ccs-core.c``. The two drivers work in both ACPI and DT
-based systems.
+Please also see :ref:`examples <media-camera-sensor-examples>`.
 
 Control framework
 ~~~~~~~~~~~~~~~~~
@@ -104,99 +109,39 @@ The function returns a non-zero value if it succeeded getting the power count or
 runtime PM was disabled, in either of which cases the driver may proceed to
 access the device.
 
-Frame size
-----------
-
-There are two distinct ways to configure the frame size produced by camera
-sensors.
-
-Freely configurable camera sensor drivers
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-Freely configurable camera sensor drivers expose the device's internal
-processing pipeline as one or more sub-devices with different cropping and
-scaling configurations. The output size of the device is the result of a series
-of cropping and scaling operations from the device's pixel array's size.
-
-An example of such a driver is the CCS driver (see ``drivers/media/i2c/ccs``).
-
-Register list based drivers
-~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-Register list based drivers generally, instead of able to configure the device
-they control based on user requests, are limited to a number of preset
-configurations that combine a number of different parameters that on hardware
-level are independent. How a driver picks such configuration is based on the
-format set on a source pad at the end of the device's internal pipeline.
-
-Most sensor drivers are implemented this way, see e.g.
-``drivers/media/i2c/imx319.c`` for an example.
-
-Frame interval configuration
-----------------------------
-
-There are two different methods for obtaining possibilities for different frame
-intervals as well as configuring the frame interval. Which one to implement
-depends on the type of the device.
-
-Raw camera sensors
-~~~~~~~~~~~~~~~~~~
-
-Instead of a high level parameter such as frame interval, the frame interval is
-a result of the configuration of a number of camera sensor implementation
-specific parameters. Luckily, these parameters tend to be the same for more or
-less all modern raw camera sensors.
-
-The frame interval is calculated using the following equation::
-
-	frame interval = (analogue crop width + horizontal blanking) *
-			 (analogue crop height + vertical blanking) / pixel rate
-
-The formula is bus independent and is applicable for raw timing parameters on
-large variety of devices beyond camera sensors. Devices that have no analogue
-crop, use the full source image size, i.e. pixel array size.
-
-Horizontal and vertical blanking are specified by ``V4L2_CID_HBLANK`` and
-``V4L2_CID_VBLANK``, respectively. The unit of the ``V4L2_CID_HBLANK`` control
-is pixels and the unit of the ``V4L2_CID_VBLANK`` is lines. The pixel rate in
-the sensor's **pixel array** is specified by ``V4L2_CID_PIXEL_RATE`` in the same
-sub-device. The unit of that control is pixels per second.
-
-Register list based drivers need to implement read-only sub-device nodes for the
-purpose. Devices that are not register list based need these to configure the
-device's internal processing pipeline.
-
-The first entity in the linear pipeline is the pixel array. The pixel array may
-be followed by other entities that are there to allow configuring binning,
-skipping, scaling or digital crop :ref:`v4l2-subdev-selections`.
-
-USB cameras etc. devices
-~~~~~~~~~~~~~~~~~~~~~~~~
-
-USB video class hardware, as well as many cameras offering a similar higher
-level interface natively, generally use the concept of frame interval (or frame
-rate) on device level in firmware or hardware. This means lower level controls
-implemented by raw cameras may not be used on uAPI (or even kAPI) to control the
-frame interval on these devices.
-
 Rotation, orientation and flipping
 ----------------------------------
 
-Some systems have the camera sensor mounted upside down compared to its natural
-mounting rotation. In such cases, drivers shall expose the information to
-userspace with the :ref:`V4L2_CID_CAMERA_SENSOR_ROTATION
-<v4l2-camera-sensor-rotation>` control.
-
-Sensor drivers shall also report the sensor's mounting orientation with the
-:ref:`V4L2_CID_CAMERA_SENSOR_ORIENTATION <v4l2-camera-sensor-orientation>`.
-
 Use ``v4l2_fwnode_device_parse()`` to obtain rotation and orientation
 information from system firmware and ``v4l2_ctrl_new_fwnode_properties()`` to
 register the appropriate controls.
 
-Sensor drivers that have any vertical or horizontal flips embedded in the
-register programming sequences shall initialize the V4L2_CID_HFLIP and
-V4L2_CID_VFLIP controls with the values programmed by the register sequences.
-The default values of these controls shall be 0 (disabled). Especially these
-controls shall not be inverted, independently of the sensor's mounting
-rotation.
+.. _media-camera-sensor-examples:
+
+Example drivers
+---------------
+
+Features implemented by sensor drivers vary, and depending on the set of
+supported features and other qualities, particular sensor drivers better serve
+the purpose of an example. The following drivers are known to be good examples:
+
+.. flat-table:: Example sensor drivers
+    :header-rows: 0
+    :widths:      1 1 1 2
+
+    * - Driver name
+      - File(s)
+      - Driver type
+      - Example topic
+    * - CCS
+      - ``drivers/media/i2c/ccs/``
+      - Freely configurable
+      - Power management (ACPI and DT), UAPI
+    * - imx219
+      - ``drivers/media/i2c/imx219.c``
+      - Register list based
+      - Power management (DT), UAPI, mode selection
+    * - imx319
+      - ``drivers/media/i2c/imx319.c``
+      - Register list based
+      - Power management (ACPI and DT)
diff --git a/Documentation/userspace-api/media/drivers/camera-sensor.rst b/Documentation/userspace-api/media/drivers/camera-sensor.rst
new file mode 100644
index 000000000000..919a50e8b9d9
--- /dev/null
+++ b/Documentation/userspace-api/media/drivers/camera-sensor.rst
@@ -0,0 +1,104 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _media_using_camera_sensor_drivers:
+
+Using camera sensor drivers
+===========================
+
+This section describes common practices for how the V4L2 sub-device interface is
+used to control the camera sensor drivers.
+
+You may also find :ref:`media_writing_camera_sensor_drivers` useful.
+
+Frame size
+----------
+
+There are two distinct ways to configure the frame size produced by camera
+sensors.
+
+Freely configurable camera sensor drivers
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Freely configurable camera sensor drivers expose the device's internal
+processing pipeline as one or more sub-devices with different cropping and
+scaling configurations. The output size of the device is the result of a series
+of cropping and scaling operations from the device's pixel array's size.
+
+An example of such a driver is the CCS driver.
+
+Register list based drivers
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Register list based drivers generally, instead of able to configure the device
+they control based on user requests, are limited to a number of preset
+configurations that combine a number of different parameters that on hardware
+level are independent. How a driver picks such configuration is based on the
+format set on a source pad at the end of the device's internal pipeline.
+
+Most sensor drivers are implemented this way.
+
+Frame interval configuration
+----------------------------
+
+There are two different methods for obtaining possibilities for different frame
+intervals as well as configuring the frame interval. Which one to implement
+depends on the type of the device.
+
+Raw camera sensors
+~~~~~~~~~~~~~~~~~~
+
+Instead of a high level parameter such as frame interval, the frame interval is
+a result of the configuration of a number of camera sensor implementation
+specific parameters. Luckily, these parameters tend to be the same for more or
+less all modern raw camera sensors.
+
+The frame interval is calculated using the following equation::
+
+	frame interval = (analogue crop width + horizontal blanking) *
+			 (analogue crop height + vertical blanking) / pixel rate
+
+The formula is bus independent and is applicable for raw timing parameters on
+large variety of devices beyond camera sensors. Devices that have no analogue
+crop, use the full source image size, i.e. pixel array size.
+
+Horizontal and vertical blanking are specified by ``V4L2_CID_HBLANK`` and
+``V4L2_CID_VBLANK``, respectively. The unit of the ``V4L2_CID_HBLANK`` control
+is pixels and the unit of the ``V4L2_CID_VBLANK`` is lines. The pixel rate in
+the sensor's **pixel array** is specified by ``V4L2_CID_PIXEL_RATE`` in the same
+sub-device. The unit of that control is pixels per second.
+
+Register list based drivers need to implement read-only sub-device nodes for the
+purpose. Devices that are not register list based need these to configure the
+device's internal processing pipeline.
+
+The first entity in the linear pipeline is the pixel array. The pixel array may
+be followed by other entities that are there to allow configuring binning,
+skipping, scaling or digital crop, see :ref:`VIDIOC_SUBDEV_G_SELECTION
+<VIDIOC_SUBDEV_G_SELECTION>`.
+
+USB cameras etc. devices
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+USB video class hardware, as well as many cameras offering a similar higher
+level interface natively, generally use the concept of frame interval (or frame
+rate) on device level in firmware or hardware. This means lower level controls
+implemented by raw cameras may not be used on uAPI (or even kAPI) to control the
+frame interval on these devices.
+
+Rotation, orientation and flipping
+----------------------------------
+
+Some systems have the camera sensor mounted upside down compared to its natural
+mounting rotation. In such cases, drivers shall expose the information to
+userspace with the :ref:`V4L2_CID_CAMERA_SENSOR_ROTATION
+<v4l2-camera-sensor-rotation>` control.
+
+Sensor drivers shall also report the sensor's mounting orientation with the
+:ref:`V4L2_CID_CAMERA_SENSOR_ORIENTATION <v4l2-camera-sensor-orientation>`.
+
+Sensor drivers that have any vertical or horizontal flips embedded in the
+register programming sequences shall initialize the :ref:`V4L2_CID_HFLIP
+<v4l2-cid-hflip>` and :ref:`V4L2_CID_VFLIP <v4l2-cid-vflip>` controls with the
+values programmed by the register sequences. The default values of these
+controls shall be 0 (disabled). Especially these controls shall not be inverted,
+independently of the sensor's mounting rotation.
diff --git a/Documentation/userspace-api/media/drivers/index.rst b/Documentation/userspace-api/media/drivers/index.rst
index 783f92f01a4c..1726f8ec86fa 100644
--- a/Documentation/userspace-api/media/drivers/index.rst
+++ b/Documentation/userspace-api/media/drivers/index.rst
@@ -32,6 +32,7 @@ For more details see the file COPYING in the source distribution of Linux.
 	:numbered:
 
 	aspeed-video
+	camera-sensor
 	ccs
 	cx2341x-uapi
 	dw100
diff --git a/Documentation/userspace-api/media/v4l/control.rst b/Documentation/userspace-api/media/v4l/control.rst
index 4463fce694b0..57893814a1e5 100644
--- a/Documentation/userspace-api/media/v4l/control.rst
+++ b/Documentation/userspace-api/media/v4l/control.rst
@@ -143,9 +143,13 @@ Control IDs
     recognise the difference between digital and analogue gain use
     controls ``V4L2_CID_DIGITAL_GAIN`` and ``V4L2_CID_ANALOGUE_GAIN``.
 
+.. _v4l2-cid-hflip:
+
 ``V4L2_CID_HFLIP`` ``(boolean)``
     Mirror the picture horizontally.
 
+.. _v4l2-cid-vflip:
+
 ``V4L2_CID_VFLIP`` ``(boolean)``
     Mirror the picture vertically.
 
-- 
2.39.2

Re: [PATCH v7 00/19] Small fixes and cleanups (ov2740 and ccs)

[Sakari Ailus]

On Mon, Oct 02, 2023 at 01:55:38PM +0300, Sakari Ailus wrote:
> Hi folks,
> 
> This small set contains fixes and cleanups, mainly for the ccs and ov2740
> drivers. I wrote these while working on the metadata set, but these could
> and should be merged earlier.
> 
> since v6:
> 
> - New patch: In scope of the CCS driver, revert the patch moving CCS to
>   use pm_runtime_get_and_resume().
> 
> - New patch: Drop s_stream re-entrancy in CCS driver.
> 
> - New patch: Rename ccs_create_subdev() as ccs_init_subdev.
> 
> - New patch: CCS driver rework: move sub-device initialisation earlier in
>   probe to address initialisation ordering issues later on in embedded
>   data support. This introduces minor changes to the patch adding
>   sub-device state support.

In addition:

- MIPI CSI-2 long packet DT macro change as suggested in review.

-- 
Sakari Ailus

Re: [PATCH v7 17/19] media: mc: Check pad flag validity

[Sergey Senozhatsky]

On (23/10/02 13:55), Sakari Ailus wrote:
> 
> Check the validity of pad flags on entity init. Exactly one of the flags
> must be set.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/mc/mc-entity.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index 83468d4a440b..543a392f8635 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -197,6 +197,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>  	struct media_device *mdev = entity->graph_obj.mdev;
>  	struct media_pad *iter;
>  	unsigned int i = 0;
> +	int ret = 0;
>  
>  	if (num_pads >= MEDIA_ENTITY_MAX_PADS)
>  		return -E2BIG;
> @@ -210,15 +211,27 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>  	media_entity_for_each_pad(entity, iter) {
>  		iter->entity = entity;
>  		iter->index = i++;
> +
> +		if (hweight32(iter->flags & (MEDIA_PAD_FL_SINK |
> +					     MEDIA_PAD_FL_SOURCE)) != 1) {
> +			ret = -EINVAL;

Can we please have some sort of WARN_ON() or pr_err() here?
This is a pretty big change.

Re: [PATCH v7 17/19] media: mc: Check pad flag validity

[Sakari Ailus]

Hi Sergey,

Thanks for the review.

On Thu, Feb 01, 2024 at 06:17:13PM +0900, Sergey Senozhatsky wrote:
> On (23/10/02 13:55), Sakari Ailus wrote:
> > 
> > Check the validity of pad flags on entity init. Exactly one of the flags
> > must be set.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/mc/mc-entity.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > index 83468d4a440b..543a392f8635 100644
> > --- a/drivers/media/mc/mc-entity.c
> > +++ b/drivers/media/mc/mc-entity.c
> > @@ -197,6 +197,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> >  	struct media_device *mdev = entity->graph_obj.mdev;
> >  	struct media_pad *iter;
> >  	unsigned int i = 0;
> > +	int ret = 0;
> >  
> >  	if (num_pads >= MEDIA_ENTITY_MAX_PADS)
> >  		return -E2BIG;
> > @@ -210,15 +211,27 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> >  	media_entity_for_each_pad(entity, iter) {
> >  		iter->entity = entity;
> >  		iter->index = i++;
> > +
> > +		if (hweight32(iter->flags & (MEDIA_PAD_FL_SINK |
> > +					     MEDIA_PAD_FL_SOURCE)) != 1) {
> > +			ret = -EINVAL;
> 
> Can we please have some sort of WARN_ON() or pr_err() here?
> This is a pretty big change.

Doing proper input validation is hardly anything unusual, is it?

I'm fine with a WARN_ON() though, I'll add that to v8.

-- 
Regards,

Sakari Ailus

Re: [PATCH v7 17/19] media: mc: Check pad flag validity

[Sergey Senozhatsky]

On (24/02/01 09:22), Sakari Ailus wrote:
> Hi Sergey,
> 
> Thanks for the review.

Hi Sakari,

> On Thu, Feb 01, 2024 at 06:17:13PM +0900, Sergey Senozhatsky wrote:
> > On (23/10/02 13:55), Sakari Ailus wrote:
[..]
> > > @@ -197,6 +197,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> > >  	struct media_device *mdev = entity->graph_obj.mdev;
> > >  	struct media_pad *iter;
> > >  	unsigned int i = 0;
> > > +	int ret = 0;
> > >  
> > >  	if (num_pads >= MEDIA_ENTITY_MAX_PADS)
> > >  		return -E2BIG;
> > > @@ -210,15 +211,27 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> > >  	media_entity_for_each_pad(entity, iter) {
> > >  		iter->entity = entity;
> > >  		iter->index = i++;
> > > +
> > > +		if (hweight32(iter->flags & (MEDIA_PAD_FL_SINK |
> > > +					     MEDIA_PAD_FL_SOURCE)) != 1) {
> > > +			ret = -EINVAL;
> > 
> > Can we please have some sort of WARN_ON() or pr_err() here?
> > This is a pretty big change.
> 
> Doing proper input validation is hardly anything unusual, is it?

Well, function requirements change quite significantly, to the point
that drivers that worked before won't work after.

> I'm fine with a WARN_ON() though, I'll add that to v8.

Thanks!

Re: [PATCH v7 17/19] media: mc: Check pad flag validity

[Sakari Ailus]

On Thu, Feb 01, 2024 at 06:33:13PM +0900, Sergey Senozhatsky wrote:
> On (24/02/01 09:22), Sakari Ailus wrote:
> > Hi Sergey,
> > 
> > Thanks for the review.
> 
> Hi Sakari,
> 
> > On Thu, Feb 01, 2024 at 06:17:13PM +0900, Sergey Senozhatsky wrote:
> > > On (23/10/02 13:55), Sakari Ailus wrote:
> [..]
> > > > @@ -197,6 +197,7 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> > > >  	struct media_device *mdev = entity->graph_obj.mdev;
> > > >  	struct media_pad *iter;
> > > >  	unsigned int i = 0;
> > > > +	int ret = 0;
> > > >  
> > > >  	if (num_pads >= MEDIA_ENTITY_MAX_PADS)
> > > >  		return -E2BIG;
> > > > @@ -210,15 +211,27 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> > > >  	media_entity_for_each_pad(entity, iter) {
> > > >  		iter->entity = entity;
> > > >  		iter->index = i++;
> > > > +
> > > > +		if (hweight32(iter->flags & (MEDIA_PAD_FL_SINK |
> > > > +					     MEDIA_PAD_FL_SOURCE)) != 1) {
> > > > +			ret = -EINVAL;
> > > 
> > > Can we please have some sort of WARN_ON() or pr_err() here?
> > > This is a pretty big change.
> > 
> > Doing proper input validation is hardly anything unusual, is it?
> 
> Well, function requirements change quite significantly, to the point
> that drivers that worked before won't work after.
> 
> > I'm fine with a WARN_ON() though, I'll add that to v8.
> 
> Thanks!

Actually this was a patchset that was merged quite some time ago. I'll
post separate patch on this.

-- 
Sakari Ailus

Re: [PATCH v7 17/19] media: mc: Check pad flag validity

[Sergey Senozhatsky]

On (24/02/01 11:05), Sakari Ailus wrote:
[..]
> > > > >  	if (num_pads >= MEDIA_ENTITY_MAX_PADS)
> > > > >  		return -E2BIG;
> > > > > @@ -210,15 +211,27 @@ int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
> > > > >  	media_entity_for_each_pad(entity, iter) {
> > > > >  		iter->entity = entity;
> > > > >  		iter->index = i++;
> > > > > +
> > > > > +		if (hweight32(iter->flags & (MEDIA_PAD_FL_SINK |
> > > > > +					     MEDIA_PAD_FL_SOURCE)) != 1) {
> > > > > +			ret = -EINVAL;
> > > > 
> > > > Can we please have some sort of WARN_ON() or pr_err() here?
> > > > This is a pretty big change.
> > > 
> > > Doing proper input validation is hardly anything unusual, is it?
> > 
> > Well, function requirements change quite significantly, to the point
> > that drivers that worked before won't work after.
> > 
> > > I'm fine with a WARN_ON() though, I'll add that to v8.
> > 
> > Thanks!
> 
> Actually this was a patchset that was merged quite some time ago. I'll
> post separate patch on this.

Ack.

I just debugged a driver that miraculously stopped working, and it
turned out to be because of this media_entity_pads_init() change.
I think I would have benefited from WARN_ON() or pr_err() there.