| Message ID | 20240429190852.1008003-3-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use V4L2 mbus config for conveying MEI CSI link frequency | expand |
Hi Sakari On Mon, Apr 29, 2024 at 10:08:50PM +0300, Sakari Ailus wrote: > Add link_freq field to struct v4l2_mbus_config in order to pass the link > frequency to the reciving sub-device. 'receiving' > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > drivers/media/v4l2-core/v4l2-common.c | 13 +++++++++---- > include/media/v4l2-mediabus.h | 2 ++ > 2 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > index 7f69b5a025fa..09b26ce612e9 100644 > --- a/drivers/media/v4l2-core/v4l2-common.c > +++ b/drivers/media/v4l2-core/v4l2-common.c > @@ -503,15 +503,20 @@ s64 __v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler, > EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_ctrl); > > s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul, > - unsigned int div) > + unsigned int div) Goes in the previous patch that introduces the function ? > { > + struct v4l2_mbus_config mbus_config = {}; > struct v4l2_subdev *sd; > + int ret; > > sd = media_entity_to_v4l2_subdev(pad->entity); > - if (!sd) > - return -ENODEV; > + ret = v4l2_subdev_call(sd, pad, get_mbus_config, pad->index, > + &mbus_config); > + if (ret < 0 && ret != -ENOIOCTLCMD) > + return ret; > > - return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div); > + return mbus_config.link_freq ?: > + __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div); > } > EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad); > > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h > index 5bce6e423e94..2f39b52bb4d4 100644 > --- a/include/media/v4l2-mediabus.h > +++ b/include/media/v4l2-mediabus.h > @@ -159,6 +159,7 @@ enum v4l2_mbus_type { > * @bus.mipi_csi2: embedded &struct v4l2_mbus_config_mipi_csi2. > * Used if the bus is MIPI Alliance's Camera Serial > * Interface version 2 (MIPI CSI2). > + * @link_freq: The link frequency. See also V4L2_CID_LINK_FREQ control. > */ > struct v4l2_mbus_config { > enum v4l2_mbus_type type; > @@ -167,6 +168,7 @@ struct v4l2_mbus_config { > struct v4l2_mbus_config_mipi_csi1 mipi_csi1; > struct v4l2_mbus_config_mipi_csi2 mipi_csi2; > } bus; > + u64 link_freq; Does this apply to all the supported busses ? I count: struct v4l2_mbus_config_parallel parallel; struct v4l2_mbus_config_mipi_csi1 mipi_csi1; struct v4l2_mbus_config_mipi_csi2 mipi_csi2; not sure about csi1, but I would say "yes" ? However it would feel more natural to fetch the 'link_freq' from the bus-specific structure like in 'bus.mipi_csi2.link_freq' (and 'bus' is an union, so we're not consuming any additional space if we move it to the per-bus structure). > }; > > /** > -- > 2.39.2 > >
Hi Jacopo, On Tue, Apr 30, 2024 at 09:19:32AM +0200, Jacopo Mondi wrote: > Hi Sakari > > On Mon, Apr 29, 2024 at 10:08:50PM +0300, Sakari Ailus wrote: > > Add link_freq field to struct v4l2_mbus_config in order to pass the link > > frequency to the reciving sub-device. > > 'receiving' > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > drivers/media/v4l2-core/v4l2-common.c | 13 +++++++++---- > > include/media/v4l2-mediabus.h | 2 ++ > > 2 files changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > > index 7f69b5a025fa..09b26ce612e9 100644 > > --- a/drivers/media/v4l2-core/v4l2-common.c > > +++ b/drivers/media/v4l2-core/v4l2-common.c > > @@ -503,15 +503,20 @@ s64 __v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler, > > EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_ctrl); > > > > s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul, > > - unsigned int div) > > + unsigned int div) > > Goes in the previous patch that introduces the function ? Yes. > > > { > > + struct v4l2_mbus_config mbus_config = {}; > > struct v4l2_subdev *sd; > > + int ret; > > > > sd = media_entity_to_v4l2_subdev(pad->entity); > > - if (!sd) > > - return -ENODEV; > > + ret = v4l2_subdev_call(sd, pad, get_mbus_config, pad->index, > > + &mbus_config); > > + if (ret < 0 && ret != -ENOIOCTLCMD) > > + return ret; > > > > - return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div); > > + return mbus_config.link_freq ?: > > + __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div); > > } > > EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad); > > > > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h > > index 5bce6e423e94..2f39b52bb4d4 100644 > > --- a/include/media/v4l2-mediabus.h > > +++ b/include/media/v4l2-mediabus.h > > @@ -159,6 +159,7 @@ enum v4l2_mbus_type { > > * @bus.mipi_csi2: embedded &struct v4l2_mbus_config_mipi_csi2. > > * Used if the bus is MIPI Alliance's Camera Serial > > * Interface version 2 (MIPI CSI2). > > + * @link_freq: The link frequency. See also V4L2_CID_LINK_FREQ control. > > */ > > struct v4l2_mbus_config { > > enum v4l2_mbus_type type; > > @@ -167,6 +168,7 @@ struct v4l2_mbus_config { > > struct v4l2_mbus_config_mipi_csi1 mipi_csi1; > > struct v4l2_mbus_config_mipi_csi2 mipi_csi2; > > } bus; > > + u64 link_freq; > > Does this apply to all the supported busses ? Just like the LINK_FREQ control, it does, albeit the current user (IVSC driver) uses CSI-2 only. I'm fine moving it after "type" though, that may be a better location for it. > > I count: > > struct v4l2_mbus_config_parallel parallel; > struct v4l2_mbus_config_mipi_csi1 mipi_csi1; > struct v4l2_mbus_config_mipi_csi2 mipi_csi2; > > not sure about csi1, but I would say "yes" ? > > However it would feel more natural to fetch the 'link_freq' from the > bus-specific structure like in 'bus.mipi_csi2.link_freq' (and 'bus' is > an union, so we're not consuming any additional space if we move it to > the per-bus structure). > > > > }; > > > > /**
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index 7f69b5a025fa..09b26ce612e9 100644 --- a/drivers/media/v4l2-core/v4l2-common.c +++ b/drivers/media/v4l2-core/v4l2-common.c @@ -503,15 +503,20 @@ s64 __v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler, EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_ctrl); s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul, - unsigned int div) + unsigned int div) { + struct v4l2_mbus_config mbus_config = {}; struct v4l2_subdev *sd; + int ret; sd = media_entity_to_v4l2_subdev(pad->entity); - if (!sd) - return -ENODEV; + ret = v4l2_subdev_call(sd, pad, get_mbus_config, pad->index, + &mbus_config); + if (ret < 0 && ret != -ENOIOCTLCMD) + return ret; - return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div); + return mbus_config.link_freq ?: + __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div); } EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad); diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 5bce6e423e94..2f39b52bb4d4 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -159,6 +159,7 @@ enum v4l2_mbus_type { * @bus.mipi_csi2: embedded &struct v4l2_mbus_config_mipi_csi2. * Used if the bus is MIPI Alliance's Camera Serial * Interface version 2 (MIPI CSI2). + * @link_freq: The link frequency. See also V4L2_CID_LINK_FREQ control. */ struct v4l2_mbus_config { enum v4l2_mbus_type type; @@ -167,6 +168,7 @@ struct v4l2_mbus_config { struct v4l2_mbus_config_mipi_csi1 mipi_csi1; struct v4l2_mbus_config_mipi_csi2 mipi_csi2; } bus; + u64 link_freq; }; /**
Add link_freq field to struct v4l2_mbus_config in order to pass the link frequency to the reciving sub-device. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/media/v4l2-core/v4l2-common.c | 13 +++++++++---- include/media/v4l2-mediabus.h | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-)