Граф коммитов

15 Коммитов

Автор SHA1 Сообщение Дата
Sakari Ailus 3c8c153914 media: v4l: async: Rename async nf functions, clean up long lines
Rename V4L2 async notifier functions, replacing "notifier" with "nf" and
removing "_subdev" at the end of the function names adding subdevs as you
can only add subdevs to a notifier. Also wrap and otherwise clean up long
lines.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com> (imx7)
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-09-30 10:07:35 +02:00
Jacopo Mondi 4ff5278dce media: i2c: max9286: Rework comments in .bound()
Rephrase a comment in .bound() callback to make it clear we register
a subdev notifier and remove a redundant comment about disabling i2c
auto-ack.

No functional changes intended.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-06-17 11:29:27 +02:00
Jacopo Mondi 731c24ffa2 media: i2c: max9286: Define high channel amplitude
Provide a macro to define the reverse channel amplitude to
be used to compensate the remote serializer noise immunity.

While at it, update a comment.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-06-17 11:28:24 +02:00
Jacopo Mondi 902edc2a1c media: i2c: max9286: Cache channel amplitude
Cache the current channel amplitude in a driver variable
to skip updating it if the newly requested value is the same
as the currently configured one.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-06-17 11:28:00 +02:00
Jacopo Mondi f78723eb62 media: i2c: max9286: Rename reverse_channel_mv
Rename the reverse_channel_mv variable to init_rev_chan_mv as
the next patch will cache the reverse channel amplitude in
a new driver variable.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-06-17 11:27:41 +02:00
Jacopo Mondi 3de09c7ae7 media: i2c: max9286: Adjust parameters indent
The parameters to max9286_i2c_mux_configure() fits on the previous
line. Adjust it.

Cosmetic change only.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-06-17 11:27:18 +02:00
Tomi Valkeinen 0d346d2a6f media: v4l2-subdev: add subdev-wide state struct
We have 'struct v4l2_subdev_pad_config' which contains configuration for
a single pad used for the TRY functionality, and an array of those
structs is passed to various v4l2_subdev_pad_ops.

I was working on subdev internal routing between pads, and realized that
there's no way to add TRY functionality for routes, which is not pad
specific configuration. Adding a separate struct for try-route config
wouldn't work either, as e.g. set-fmt needs to know the try-route
configuration to propagate the settings.

This patch adds a new struct, 'struct v4l2_subdev_state' (which at the
moment only contains the v4l2_subdev_pad_config array) and the new
struct is used in most of the places where v4l2_subdev_pad_config was
used. All v4l2_subdev_pad_ops functions taking v4l2_subdev_pad_config
are changed to instead take v4l2_subdev_state.

The changes to drivers/media/v4l2-core/v4l2-subdev.c and
include/media/v4l2-subdev.h were written by hand, and all the driver
changes were done with the semantic patch below. The spatch needs to be
applied to a select list of directories. I used the following shell
commands to apply the spatch:

dirs="drivers/media/i2c drivers/media/platform drivers/media/usb drivers/media/test-drivers/vimc drivers/media/pci drivers/staging/media"
for dir in $dirs; do spatch -j8 --dir --include-headers --no-show-diff --in-place --sp-file v4l2-subdev-state.cocci $dir; done

Note that Coccinelle chokes on a few drivers (gcc extensions?). With
minor changes we can make Coccinelle run fine, and these changes can be
reverted after spatch. The diff for these changes is:

For drivers/media/i2c/s5k5baf.c:

	@@ -1481,7 +1481,7 @@ static int s5k5baf_set_selection(struct v4l2_subdev *sd,
	 				&s5k5baf_cis_rect,
	 				v4l2_subdev_get_try_crop(sd, cfg, PAD_CIS),
	 				v4l2_subdev_get_try_compose(sd, cfg, PAD_CIS),
	-				v4l2_subdev_get_try_crop(sd, cfg, PAD_OUT)
	+				v4l2_subdev_get_try_crop(sd, cfg, PAD_OUT),
	 			};
	 		s5k5baf_set_rect_and_adjust(rects, rtype, &sel->r);
	 		return 0;

For drivers/media/platform/s3c-camif/camif-capture.c:

	@@ -1230,7 +1230,7 @@ static int s3c_camif_subdev_get_fmt(struct v4l2_subdev *sd,
	 		*mf = camif->mbus_fmt;
	 		break;

	-	case CAMIF_SD_PAD_SOURCE_C...CAMIF_SD_PAD_SOURCE_P:
	+	case CAMIF_SD_PAD_SOURCE_C:
	 		/* crop rectangle at camera interface input */
	 		mf->width = camif->camif_crop.width;
	 		mf->height = camif->camif_crop.height;
	@@ -1332,7 +1332,7 @@ static int s3c_camif_subdev_set_fmt(struct v4l2_subdev *sd,
	 		}
	 		break;

	-	case CAMIF_SD_PAD_SOURCE_C...CAMIF_SD_PAD_SOURCE_P:
	+	case CAMIF_SD_PAD_SOURCE_C:
	 		/* Pixel format can be only changed on the sink pad. */
	 		mf->code = camif->mbus_fmt.code;
	 		mf->width = crop->width;

The semantic patch is:

// <smpl>

// Change function parameter

@@
identifier func;
identifier cfg;
@@

 func(...,
-   struct v4l2_subdev_pad_config *cfg
+   struct v4l2_subdev_state *sd_state
    , ...)
 {
 <...
- cfg
+ sd_state
 ...>
 }

// Change function declaration parameter

@@
identifier func;
identifier cfg;
type T;
@@
T func(...,
-   struct v4l2_subdev_pad_config *cfg
+   struct v4l2_subdev_state *sd_state
    , ...);

// Change function return value

@@
identifier func;
@@
- struct v4l2_subdev_pad_config
+ struct v4l2_subdev_state
 *func(...)
 {
    ...
 }

// Change function declaration return value

@@
identifier func;
@@
- struct v4l2_subdev_pad_config
+ struct v4l2_subdev_state
 *func(...);

// Some drivers pass a local pad_cfg for a single pad to a called function. Wrap it
// inside a pad_state.

@@
identifier func;
identifier pad_cfg;
@@
func(...)
{
    ...
    struct v4l2_subdev_pad_config pad_cfg;
+   struct v4l2_subdev_state pad_state = { .pads = &pad_cfg };

    <+...

(
    v4l2_subdev_call
|
    sensor_call
|
    isi_try_fse
|
    isc_try_fse
|
    saa_call_all
)
    (...,
-   &pad_cfg
+   &pad_state
    ,...)

    ...+>
}

// If the function uses fields from pad_config, access via state->pads

@@
identifier func;
identifier state;
@@
 func(...,
    struct v4l2_subdev_state *state
    , ...)
 {
    <...
(
-   state->try_fmt
+   state->pads->try_fmt
|
-   state->try_crop
+   state->pads->try_crop
|
-   state->try_compose
+   state->pads->try_compose
)
    ...>
}

// If the function accesses the filehandle, use fh->state instead

@@
struct v4l2_subdev_fh *fh;
@@
-    fh->pad
+    fh->state

@@
struct v4l2_subdev_fh fh;
@@
-    fh.pad
+    fh.state

// Start of vsp1 specific

@@
@@
struct vsp1_entity {
    ...
-    struct v4l2_subdev_pad_config *config;
+    struct v4l2_subdev_state *config;
    ...
};

@@
symbol entity;
@@
vsp1_entity_init(...)
{
    ...
    entity->config =
-    v4l2_subdev_alloc_pad_config
+    v4l2_subdev_alloc_state
    (&entity->subdev);
    ...
}

@@
symbol entity;
@@
vsp1_entity_destroy(...)
{
    ...
-   v4l2_subdev_free_pad_config
+   v4l2_subdev_free_state
    (entity->config);
    ...
}

@exists@
identifier func =~ "(^vsp1.*)|(hsit_set_format)|(sru_enum_frame_size)|(sru_set_format)|(uif_get_selection)|(uif_set_selection)|(uds_enum_frame_size)|(uds_set_format)|(brx_set_format)|(brx_get_selection)|(histo_get_selection)|(histo_set_selection)|(brx_set_selection)";
symbol config;
@@
func(...) {
    ...
-    struct v4l2_subdev_pad_config *config;
+    struct v4l2_subdev_state *config;
    ...
}

// End of vsp1 specific

// Start of rcar specific

@@
identifier sd;
identifier pad_cfg;
@@
 rvin_try_format(...)
 {
    ...
-   struct v4l2_subdev_pad_config *pad_cfg;
+   struct v4l2_subdev_state *sd_state;
    ...
-   pad_cfg = v4l2_subdev_alloc_pad_config(sd);
+   sd_state = v4l2_subdev_alloc_state(sd);
    <...
-   pad_cfg
+   sd_state
    ...>
-   v4l2_subdev_free_pad_config(pad_cfg);
+   v4l2_subdev_free_state(sd_state);
    ...
 }

// End of rcar specific

// Start of rockchip specific

@@
identifier func =~ "(rkisp1_rsz_get_pad_fmt)|(rkisp1_rsz_get_pad_crop)|(rkisp1_rsz_register)";
symbol rsz;
symbol pad_cfg;
@@

 func(...)
 {
+   struct v4l2_subdev_state state = { .pads = rsz->pad_cfg };
    ...
-   rsz->pad_cfg
+   &state
    ...
 }

@@
identifier func =~ "(rkisp1_isp_get_pad_fmt)|(rkisp1_isp_get_pad_crop)";
symbol isp;
symbol pad_cfg;
@@

 func(...)
 {
+   struct v4l2_subdev_state state = { .pads = isp->pad_cfg };
    ...
-   isp->pad_cfg
+   &state
    ...
 }

@@
symbol rkisp1;
symbol isp;
symbol pad_cfg;
@@

 rkisp1_isp_register(...)
 {
+   struct v4l2_subdev_state state = { .pads = rkisp1->isp.pad_cfg };
    ...
-   rkisp1->isp.pad_cfg
+   &state
    ...
 }

// End of rockchip specific

// Start of tegra-video specific

@@
identifier sd;
identifier pad_cfg;
@@
 __tegra_channel_try_format(...)
 {
    ...
-   struct v4l2_subdev_pad_config *pad_cfg;
+   struct v4l2_subdev_state *sd_state;
    ...
-   pad_cfg = v4l2_subdev_alloc_pad_config(sd);
+   sd_state = v4l2_subdev_alloc_state(sd);
    <...
-   pad_cfg
+   sd_state
    ...>
-   v4l2_subdev_free_pad_config(pad_cfg);
+   v4l2_subdev_free_state(sd_state);
    ...
 }

@@
identifier sd_state;
@@
 __tegra_channel_try_format(...)
 {
    ...
    struct v4l2_subdev_state *sd_state;
    <...
-   sd_state->try_crop
+   sd_state->pads->try_crop
    ...>
 }

// End of tegra-video specific

// </smpl>

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-06-17 10:01:27 +02:00
Jacopo Mondi 85cb767cbf media: i2c: max9286: Configure reverse channel amplitude
Adjust the initial reverse channel amplitude parsing from
firmware interface the 'maxim,reverse-channel-microvolt'
property.

This change is required for both rdacm20 and rdacm21 camera
modules to be correctly probed when used in combination with
the max9286 deserializer.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-02-06 09:34:31 +01:00
Jacopo Mondi 5a386b1ff7 media: i2c: max9286: Make channel amplitude programmable
Instrument the function that configures the reverse channel with a
programmable amplitude value.

This change serves to prepare to adjust the reverse channel amplitude
depending on the remote end high-threshold configuration.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-02-06 09:33:45 +01:00
Jacopo Mondi 02b57eb3b5 media: i2c: max9286: Break-out reverse channel setup
Break out the reverse channel setup configuration procedure to its own
function.

This change prepares for configuring the reverse channel conditionally
to the remote side high threshold configuration.

No functional changes intended.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesasa@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-02-06 09:33:22 +01:00
Laurent Pinchart b01edcbd40 media: v4l2-async: Improve v4l2_async_notifier_add_*_subdev() API
The functions that add an async subdev to an async subdev notifier take
as an argument the size of the container structure they need to
allocate. This is error prone, as passing an invalid size will not be
caught by the compiler. Wrap those functions in macros that take a
container type instead of a size, and cast the returned pointer to the
desired type. The compiler will catch mistakes if the incorrect type is
passed to the macro, as the assignment types won't match.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Helen Koike <helen.koike@collabora.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> (core+ti-cal)
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-02-06 09:18:53 +01:00
Tomi Valkeinen e88ccf09e7 media: i2c: max9286: fix access to unallocated memory
The asd allocated with v4l2_async_notifier_add_fwnode_subdev() must be
of size max9286_asd, otherwise access to max9286_asd->source will go to
unallocated memory.

Fixes: 86d37bf31a ("media: i2c: max9286: Allocate v4l2_async_subdev dynamically")
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: stable@vger.kernel.org # v5.10+
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2021-02-06 08:38:06 +01:00
Laurent Pinchart 86d37bf31a media: i2c: max9286: Allocate v4l2_async_subdev dynamically
v4l2_async_notifier_add_subdev() requires the asd to be allocated
dynamically, but the max9286 driver embeds it in the max9286_source
structure. This causes memory corruption when the notifier is destroyed
at remove time with v4l2_async_notifier_cleanup().

Fix this issue by registering the asd with
v4l2_async_notifier_add_fwnode_subdev(), which allocates it dynamically
internally. A new max9286_asd structure is introduced, to store a
pointer to the corresonding max9286_source that needs to be accessed
from bound and unbind callbacks. There's no need to take an extra
explicit reference to the fwnode anymore as
v4l2_async_notifier_add_fwnode_subdev() does so internally.

While at it, use %u instead of %d to print the unsigned index in the
error message from the v4l2_async_notifier_add_fwnode_subdev() error
path.

Fixes: 66d8c9d242 ("media: i2c: Add MAX9286 driver")
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2020-09-10 14:27:46 +02:00
Colin Ian King e5b95c8feb media: i2c: fix error check on max9286_read call
Currently the error return from the call to max9286_read is masked
with 0xf0 so the following check for a negative error return is
never true.  Fix this by checking for an error first, then masking
the return value for subsequent conflink_mask checking.

Addresses-Coverity: ("Logically dead code")

Fixes: 66d8c9d242 ("media: i2c: Add MAX9286 driver")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2020-09-01 14:13:27 +02:00
Kieran Bingham 66d8c9d242 media: i2c: Add MAX9286 driver
The MAX9286 is a 4-channel GMSL deserializer with coax or STP input and
CSI-2 output. The device supports multicamera streaming applications,
and features the ability to synchronise the attached cameras.

CSI-2 output can be configured with 1 to 4 lanes, and a control channel
is supported over I2C, which implements an I2C mux to facilitate
communications with connected cameras across the reverse control
channel.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
2020-07-19 14:13:51 +02:00