| Message ID | 20240509161403.111789-1-jacopo.mondi@ideasonboard.com |
|---|---|
| Headers | show |
| Series | media: renesas: rcar-csi2: Use the subdev active state | expand |
Hi Jacopo, Thank you for the patch. On Thu, May 09, 2024 at 06:14:00PM +0200, Jacopo Mondi wrote: > Use the subdev active state in the max9286 driver to store the > image format. > > Replace the .open() function call with the .init_state() one and > simplify the set/get_pad_fmt() operations. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > drivers/media/i2c/max9286.c | 130 ++++++++++++------------------------ > 1 file changed, 44 insertions(+), 86 deletions(-) > > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c > index 5321238cad60..8c75756c945c 100644 > --- a/drivers/media/i2c/max9286.c > +++ b/drivers/media/i2c/max9286.c > @@ -19,7 +19,6 @@ > #include <linux/i2c.h> > #include <linux/i2c-mux.h> > #include <linux/module.h> > -#include <linux/mutex.h> > #include <linux/of_graph.h> > #include <linux/regulator/consumer.h> > #include <linux/slab.h> > @@ -198,12 +197,8 @@ struct max9286_priv { > struct v4l2_ctrl *pixelrate_ctrl; > unsigned int pixelrate; > > - struct v4l2_mbus_framefmt fmt[MAX9286_N_SINKS]; > struct v4l2_fract interval; > > - /* Protects controls and fmt structures */ > - struct mutex mutex; > - > unsigned int nsources; > unsigned int source_mask; > unsigned int route_mask; > @@ -788,19 +783,22 @@ static void max9286_v4l2_notifier_unregister(struct max9286_priv *priv) > static int max9286_s_stream(struct v4l2_subdev *sd, int enable) > { > struct max9286_priv *priv = sd_to_max9286(sd); > + struct v4l2_subdev_state *state; > struct max9286_source *source; > unsigned int i; > bool sync = false; > - int ret; > + int ret = 0; > + > + state = v4l2_subdev_lock_and_get_active_state(sd); > > if (enable) { > const struct v4l2_mbus_framefmt *format; > > /* > - * Get the format from the first used sink pad, as all sink > - * formats must be identical. > + * Get the format from the source pad, as all formats must be > + * identical. > */ > - format = &priv->fmt[__ffs(priv->bound_sources)]; > + format = v4l2_subdev_state_get_format(state, MAX9286_SRC_PAD); > > max9286_set_video_format(priv, format); > max9286_set_fsync_period(priv); > @@ -816,12 +814,12 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) > for_each_source(priv, source) { > ret = v4l2_subdev_call(source->sd, video, s_stream, 1); > if (ret) > - return ret; > + goto unlock; > } > > ret = max9286_check_video_links(priv); > if (ret) > - return ret; > + goto unlock; > > /* > * Wait until frame synchronization is locked. > @@ -842,7 +840,8 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) > if (!sync) { > dev_err(&priv->client->dev, > "Failed to get frame synchronization\n"); > - return -EXDEV; /* Invalid cross-device link */ > + ret = -EXDEV; /* Invalid cross-device link */ > + goto unlock; > } > > /* > @@ -865,7 +864,10 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) > max9286_i2c_mux_close(priv); > } > > - return 0; > +unlock: > + v4l2_subdev_unlock_state(state); > + > + return ret; > } > > static int max9286_get_frame_interval(struct v4l2_subdev *sd, > @@ -922,31 +924,20 @@ static int max9286_enum_mbus_code(struct v4l2_subdev *sd, > return 0; > } > > -static struct v4l2_mbus_framefmt * > -max9286_get_pad_format(struct max9286_priv *priv, > - struct v4l2_subdev_state *sd_state, > - unsigned int pad, u32 which) > -{ > - switch (which) { > - case V4L2_SUBDEV_FORMAT_TRY: > - return v4l2_subdev_state_get_format(sd_state, pad); > - case V4L2_SUBDEV_FORMAT_ACTIVE: > - return &priv->fmt[pad]; > - default: > - return NULL; > - } > -} > - > static int max9286_set_fmt(struct v4l2_subdev *sd, > - struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_state *state, > struct v4l2_subdev_format *format) > { > struct max9286_priv *priv = sd_to_max9286(sd); > - struct v4l2_mbus_framefmt *cfg_fmt; > + struct max9286_source *source; > unsigned int i; > > + /* > + * Disable setting format on the source pad: format is propagated > + * from the sinks. > + */ > if (format->pad == MAX9286_SRC_PAD) > - return -EINVAL; > + return v4l2_subdev_get_fmt(sd, state, format); > > /* Validate the format. */ > for (i = 0; i < ARRAY_SIZE(max9286_formats); ++i) { > @@ -957,42 +948,16 @@ static int max9286_set_fmt(struct v4l2_subdev *sd, > if (i == ARRAY_SIZE(max9286_formats)) > format->format.code = max9286_formats[0].code; > > - cfg_fmt = max9286_get_pad_format(priv, sd_state, format->pad, > - format->which); > - if (!cfg_fmt) > - return -EINVAL; > - > - mutex_lock(&priv->mutex); > - *cfg_fmt = format->format; > - mutex_unlock(&priv->mutex); > - > - return 0; > -} > - > -static int max9286_get_fmt(struct v4l2_subdev *sd, > - struct v4l2_subdev_state *sd_state, > - struct v4l2_subdev_format *format) > -{ > - struct max9286_priv *priv = sd_to_max9286(sd); > - struct v4l2_mbus_framefmt *cfg_fmt; > - unsigned int pad = format->pad; > - > /* > - * Multiplexed Stream Support: Support link validation by returning the > - * format of the first bound link. All links must have the same format, > - * as we do not support mixing and matching of cameras connected to the > - * max9286. > + * Apply the same format on all the other pad as all links must have the > + * same format. > */ > - if (pad == MAX9286_SRC_PAD) > - pad = __ffs(priv->bound_sources); > - > - cfg_fmt = max9286_get_pad_format(priv, sd_state, pad, format->which); > - if (!cfg_fmt) > - return -EINVAL; > + for_each_source(priv, source) { > + unsigned int index = to_index(priv, source); > > - mutex_lock(&priv->mutex); > - format->format = *cfg_fmt; > - mutex_unlock(&priv->mutex); > + *v4l2_subdev_state_get_format(state, index) = format->format; > + } With a blank line here, Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > + *v4l2_subdev_state_get_format(state, MAX9286_SRC_PAD) = format->format; > > return 0; > } > @@ -1003,7 +968,7 @@ static const struct v4l2_subdev_video_ops max9286_video_ops = { > > static const struct v4l2_subdev_pad_ops max9286_pad_ops = { > .enum_mbus_code = max9286_enum_mbus_code, > - .get_fmt = max9286_get_fmt, > + .get_fmt = v4l2_subdev_get_fmt, > .set_fmt = max9286_set_fmt, > .get_frame_interval = max9286_get_frame_interval, > .set_frame_interval = max9286_set_frame_interval, > @@ -1025,26 +990,17 @@ static const struct v4l2_mbus_framefmt max9286_default_format = { > .xfer_func = V4L2_XFER_FUNC_DEFAULT, > }; > > -static void max9286_init_format(struct v4l2_mbus_framefmt *fmt) > +static int max9286_init_state(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *state) > { > - *fmt = max9286_default_format; > -} > - > -static int max9286_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) > -{ > - struct v4l2_mbus_framefmt *format; > - unsigned int i; > - > - for (i = 0; i < MAX9286_N_SINKS; i++) { > - format = v4l2_subdev_state_get_format(fh->state, i); > - max9286_init_format(format); > - } > + for (unsigned int i = 0; i < MAX9286_N_PADS; i++) > + *v4l2_subdev_state_get_format(state, i) = max9286_default_format; > > return 0; > } > > static const struct v4l2_subdev_internal_ops max9286_subdev_internal_ops = { > - .open = max9286_open, > + .init_state = max9286_init_state, > }; > > static const struct media_entity_operations max9286_media_ops = { > @@ -1079,10 +1035,6 @@ static int max9286_v4l2_register(struct max9286_priv *priv) > } > > /* Configure V4L2 for the MAX9286 itself */ > - > - for (i = 0; i < MAX9286_N_SINKS; i++) > - max9286_init_format(&priv->fmt[i]); > - > v4l2_i2c_subdev_init(&priv->sd, priv->client, &max9286_subdev_ops); > priv->sd.internal_ops = &max9286_subdev_internal_ops; > priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > @@ -1109,14 +1061,21 @@ static int max9286_v4l2_register(struct max9286_priv *priv) > if (ret) > goto err_async; > > + priv->sd.state_lock = priv->ctrls.lock; > + ret = v4l2_subdev_init_finalize(&priv->sd); > + if (ret) > + goto err_async; > + > ret = v4l2_async_register_subdev(&priv->sd); > if (ret < 0) { > dev_err(dev, "Unable to register subdevice\n"); > - goto err_async; > + goto err_subdev; > } > > return 0; > > +err_subdev: > + v4l2_subdev_cleanup(&priv->sd); > err_async: > v4l2_ctrl_handler_free(&priv->ctrls); > max9286_v4l2_notifier_unregister(priv); > @@ -1126,6 +1085,7 @@ static int max9286_v4l2_register(struct max9286_priv *priv) > > static void max9286_v4l2_unregister(struct max9286_priv *priv) > { > + v4l2_subdev_cleanup(&priv->sd); > v4l2_ctrl_handler_free(&priv->ctrls); > v4l2_async_unregister_subdev(&priv->sd); > max9286_v4l2_notifier_unregister(priv); > @@ -1629,8 +1589,6 @@ static int max9286_probe(struct i2c_client *client) > if (!priv) > return -ENOMEM; > > - mutex_init(&priv->mutex); > - > priv->client = client; > > /* GPIO values default to high */
Hi Jacopo, Thanks for your work. On 2024-05-09 18:13:51 +0200, Jacopo Mondi wrote: > The YUYV8_1X16 and UYVY8_1X16 formats are treated as 'ITU-R > BT.601/BT.1358 16-bit YCbCr-422 input' (YUV16 - 0x5) in the R-Car VIN > driver and are thus disallowed when capturing frames from the R-Car > CSI-2 interface according to the hardware manual. > > As the 1X16 format variants are meant to be used with serial busses they > have to be treated as 'YCbCr-422 8-bit data input' (0x1) when capturing > from CSI-2, which is a valid setting for CSI-2. > > Commit 78b3f9d75a62 ("media: rcar-vin: Add check that input interface > and format are valid") disallowed capturing YUV16 when using the CSI-2 > interface. Fix this by using YUV8_BT601 for YCbCr422 when CSI-2 is in > use. > > Fixes: 78b3f9d75a62 ("media: rcar-vin: Add check that input interface and format are valid") > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > .../media/platform/renesas/rcar-vin/rcar-dma.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c > index e2c40abc6d3d..21d5b2815e86 100644 > --- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c > @@ -742,12 +742,22 @@ static int rvin_setup(struct rvin_dev *vin) > */ > switch (vin->mbus_code) { > case MEDIA_BUS_FMT_YUYV8_1X16: > - /* BT.601/BT.1358 16bit YCbCr422 */ > - vnmc |= VNMC_INF_YUV16; > + if (vin->is_csi) > + /* YCbCr422 8-bit */ > + vnmc |= VNMC_INF_YUV8_BT601; > + else > + /* BT.601/BT.1358 16bit YCbCr422 */ > + vnmc |= VNMC_INF_YUV16; > input_is_yuv = true; > break; > case MEDIA_BUS_FMT_UYVY8_1X16: > - vnmc |= VNMC_INF_YUV16 | VNMC_YCAL; > + if (vin->is_csi) > + /* YCbCr422 8-bit */ > + vnmc |= VNMC_INF_YUV8_BT601; > + else > + /* BT.601/BT.1358 16bit YCbCr422 */ > + vnmc |= VNMC_INF_YUV16; > + vnmc |= VNMC_YCAL; > input_is_yuv = true; > break; > case MEDIA_BUS_FMT_UYVY8_2X8: > -- > 2.44.0 >
HI Jacopo, Thanks for your work. On 2024-05-09 18:13:53 +0200, Jacopo Mondi wrote: > Cleanup the V4L2 subdevice in the driver's remove function to > ensure its async connection are freed, and guarantee in future that > the subdev active state is cleaned up. > > Fixes: 769afd212b16 ("media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver") > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/media/platform/renesas/rcar-csi2.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c > index 249e58c77176..2d464e43a5be 100644 > --- a/drivers/media/platform/renesas/rcar-csi2.c > +++ b/drivers/media/platform/renesas/rcar-csi2.c > @@ -1938,6 +1938,7 @@ static void rcsi2_remove(struct platform_device *pdev) > v4l2_async_nf_unregister(&priv->notifier); > v4l2_async_nf_cleanup(&priv->notifier); > v4l2_async_unregister_subdev(&priv->subdev); > + v4l2_subdev_cleanup(&priv->subdev); > > pm_runtime_disable(&pdev->dev); > > -- > 2.44.0 >
Hi Jacopo, Thanks for your work. On 2024-05-09 18:13:57 +0200, Jacopo Mondi wrote: > Initialize and use the subdev active state to store the subdevice > format. > > This simplifies the implementation of the get_fmt and set_fmt pad > operations. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/media/i2c/adv748x/adv748x-csi2.c | 107 +++++++++-------------- > drivers/media/i2c/adv748x/adv748x.h | 1 - > 2 files changed, 42 insertions(+), 66 deletions(-) > > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c > index 0cdb397d9e0a..ebe7da8ebed7 100644 > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c > @@ -6,7 +6,6 @@ > */ > > #include <linux/module.h> > -#include <linux/mutex.h> > > #include <media/v4l2-ctrls.h> > #include <media/v4l2-device.h> > @@ -68,7 +67,33 @@ static int adv748x_csi2_register_link(struct adv748x_csi2 *tx, > > /* ----------------------------------------------------------------------------- > * v4l2_subdev_internal_ops > - * > + */ > + > +static int adv748x_csi2_init_state(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *state) > +{ > + static const struct v4l2_mbus_framefmt adv748x_csi2_default_fmt = { > + .width = 1280, > + .height = 720, > + .code = MEDIA_BUS_FMT_UYVY8_1X16, > + .colorspace = V4L2_COLORSPACE_SRGB, > + .field = V4L2_FIELD_NONE, > + .ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT, > + .quantization = V4L2_QUANTIZATION_DEFAULT, > + .xfer_func = V4L2_XFER_FUNC_DEFAULT, > + }; > + struct v4l2_mbus_framefmt *fmt; > + > + fmt = v4l2_subdev_state_get_format(state, ADV748X_CSI2_SINK); > + *fmt = adv748x_csi2_default_fmt; > + > + fmt = v4l2_subdev_state_get_format(state, ADV748X_CSI2_SOURCE); > + *fmt = adv748x_csi2_default_fmt; > + > + return 0; > +} > + > +/* > * We use the internal registered operation to be able to ensure that our > * incremental subdevices (not connected in the forward path) can be registered > * against the resulting video path and media device. > @@ -118,6 +143,7 @@ static int adv748x_csi2_registered(struct v4l2_subdev *sd) > } > > static const struct v4l2_subdev_internal_ops adv748x_csi2_internal_ops = { > + .init_state = adv748x_csi2_init_state, > .registered = adv748x_csi2_registered, > }; > > @@ -183,41 +209,6 @@ static int adv748x_csi2_enum_mbus_code(struct v4l2_subdev *sd, > return 0; > } > > -static struct v4l2_mbus_framefmt * > -adv748x_csi2_get_pad_format(struct v4l2_subdev *sd, > - struct v4l2_subdev_state *sd_state, > - unsigned int pad, u32 which) > -{ > - struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd); > - > - if (which == V4L2_SUBDEV_FORMAT_TRY) > - return v4l2_subdev_state_get_format(sd_state, pad); > - > - return &tx->format; > -} > - > -static int adv748x_csi2_get_format(struct v4l2_subdev *sd, > - struct v4l2_subdev_state *sd_state, > - struct v4l2_subdev_format *sdformat) > -{ > - struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd); > - struct adv748x_state *state = tx->state; > - struct v4l2_mbus_framefmt *mbusformat; > - > - mbusformat = adv748x_csi2_get_pad_format(sd, sd_state, sdformat->pad, > - sdformat->which); > - if (!mbusformat) > - return -EINVAL; > - > - mutex_lock(&state->mutex); > - > - sdformat->format = *mbusformat; > - > - mutex_unlock(&state->mutex); > - > - return 0; > -} > - > static bool adv748x_csi2_is_fmt_supported(struct adv748x_csi2 *tx, u32 code) > { > const unsigned int *codes = is_txa(tx) ? > @@ -239,9 +230,10 @@ static int adv748x_csi2_set_format(struct v4l2_subdev *sd, > struct v4l2_subdev_format *sdformat) > { > struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd); > - struct adv748x_state *state = tx->state; > struct v4l2_mbus_framefmt *mbusformat; > - int ret = 0; > + > + if (sdformat->pad == ADV748X_CSI2_SOURCE) > + return v4l2_subdev_get_fmt(sd, sd_state, sdformat); > > /* > * Make sure the format is supported, if not default it to > @@ -250,34 +242,14 @@ static int adv748x_csi2_set_format(struct v4l2_subdev *sd, > if (!adv748x_csi2_is_fmt_supported(tx, sdformat->format.code)) > sdformat->format.code = MEDIA_BUS_FMT_UYVY8_1X16; > > - mbusformat = adv748x_csi2_get_pad_format(sd, sd_state, sdformat->pad, > - sdformat->which); > - if (!mbusformat) > - return -EINVAL; > - > - mutex_lock(&state->mutex); > - > - if (sdformat->pad == ADV748X_CSI2_SOURCE) { > - const struct v4l2_mbus_framefmt *sink_fmt; > - > - sink_fmt = adv748x_csi2_get_pad_format(sd, sd_state, > - ADV748X_CSI2_SINK, > - sdformat->which); > - > - if (!sink_fmt) { > - ret = -EINVAL; > - goto unlock; > - } > - > - sdformat->format = *sink_fmt; > - } > - > + mbusformat = v4l2_subdev_state_get_format(sd_state, sdformat->pad); > *mbusformat = sdformat->format; > > -unlock: > - mutex_unlock(&state->mutex); > + /* Propagate format to the source pad. */ > + mbusformat = v4l2_subdev_state_get_format(sd_state, ADV748X_CSI2_SOURCE); > + *mbusformat = sdformat->format; > > - return ret; > + return 0; > } > > static int adv748x_csi2_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad, > @@ -296,7 +268,7 @@ static int adv748x_csi2_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad > > static const struct v4l2_subdev_pad_ops adv748x_csi2_pad_ops = { > .enum_mbus_code = adv748x_csi2_enum_mbus_code, > - .get_fmt = adv748x_csi2_get_format, > + .get_fmt = v4l2_subdev_get_fmt, > .set_fmt = adv748x_csi2_set_format, > .get_mbus_config = adv748x_csi2_get_mbus_config, > }; > @@ -388,6 +360,11 @@ int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx) > if (ret) > goto err_cleanup_subdev; > > + tx->sd.state_lock = &state->mutex; > + ret = v4l2_subdev_init_finalize(&tx->sd); > + if (ret) > + goto err_free_ctrl; > + > ret = v4l2_async_register_subdev(&tx->sd); > if (ret) > goto err_free_ctrl; > diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h > index d2b5e722e997..9bc0121d0eff 100644 > --- a/drivers/media/i2c/adv748x/adv748x.h > +++ b/drivers/media/i2c/adv748x/adv748x.h > @@ -75,7 +75,6 @@ enum adv748x_csi2_pads { > > struct adv748x_csi2 { > struct adv748x_state *state; > - struct v4l2_mbus_framefmt format; > unsigned int page; > unsigned int port; > unsigned int num_lanes; > -- > 2.44.0 >
On 2024-05-10 02:06:04 +0300, Laurent Pinchart wrote: > On Fri, May 10, 2024 at 12:48:48AM +0200, Niklas Söderlund wrote: > > On 2024-05-09 23:51:29 +0300, Laurent Pinchart wrote: > > > On Thu, May 09, 2024 at 06:13:50PM +0200, Jacopo Mondi wrote: > > > > v2->v3: > > > > - rcar-csi2: Collect v2.2 of [4/11] > > > > - adv748x: enum_mbus_code: reduce the number of formats to the ones supported > > > > by the HDMI and Analog front ends; > > > > - adv748x: enum_mbus_code: enumerate all formats on sink pad; enumerate the > > > > active format on the source pad > > > > - max9286: Apply the format to all pads to enforce all links to have the same > > > > format > > > > - max9286: Remove max9286_set_fsync_period() from setup > > > > > > > > v2->v1: > > > > - Remove "media: adv748x-csi2: Initialize subdev format" > > > > - Add "media: adv748x-afe: Use 1X16 media bus code" > > > > - Tested with CVBS > > > > - address comments from Laurent and Niklas > > > > > > > > A branch is available at > > > > https://git.kernel.org/pub/scm/linux/kernel/git/jmondi/linux.git/ > > > > jmondi/renesas-drivers-2024-04-23-v6.9-rc5/multistream-prep > > > > > > > > As a follow-up to the recently sent > > > > "media: renesas: rcar-csi2: Support multiple streams" series, this smaller > > > > version collects some fixes and implement usage of the subdev active state > > > > to simplify the R-Car CSI-2, ADV748x and MAX9286 drivers implementations. > > > > > > > > Tested with GMSL on Eagle V3M > > > > Tested with HDMI on Salvator-X > > > > Tested with CVBS on Salvator-X > > > > Boot tested on WhiteHawk V4H > > > > > > Niklas, would you be able to runtime-test this on V4H ? The series is > > > otherwise ready to be merged in my opinion. > > > > I have run-time tested this on Gen4 and it works as expected. I also > > tested on Gen2 and Gen3 and all looks good. > > Can I add your Tested-by ? :-) For patches 1-8, Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> I'm afraid I don't have a turn-key setup to test max9286. > > > Patch 7 needs to be moved earlier in the series to avoid breaking git > > bisect, but that move causes no conflicts so should be easy. With that > > fixed I too think this is ready to be merged. Nice work Jacopo! > > I've reordered the patches in my tree already. > > > > > Jacopo Mondi (11): > > > > media: rcar-vin: Fix YUYV8_1X16 handling for CSI-2 > > > > media: rcar-csi2: Disable runtime_pm in probe error > > > > media: rcar-csi2: Cleanup subdevice in remove() > > > > media: rcar-csi2: Use the subdev active state > > > > media: adv748x-csi2: Implement enum_mbus_codes > > > > media: adv748x-csi2: Validate the image format > > > > media: adv748x-csi2: Use the subdev active state > > > > media: adv748x-afe: Use 1X16 media bus code > > > > media: max9286: Fix enum_mbus_code > > > > media: max9286: Use the subdev active state > > > > media: max9286: Use frame interval from subdev state > > > > > > > > drivers/media/i2c/adv748x/adv748x-afe.c | 4 +- > > > > drivers/media/i2c/adv748x/adv748x-csi2.c | 145 +++++++++----- > > > > drivers/media/i2c/adv748x/adv748x.h | 1 - > > > > drivers/media/i2c/max9286.c | 182 +++++++----------- > > > > drivers/media/platform/renesas/rcar-csi2.c | 155 +++++++++------ > > > > .../platform/renesas/rcar-vin/rcar-dma.c | 16 +- > > > > 6 files changed, 271 insertions(+), 232 deletions(-) > > -- > Regards, > > Laurent Pinchart