| Message ID | 20240516122539.30787-2-sakari.ailus@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Use V4L2 mbus config for conveying MEI CSI link frequency | expand |
Hi Sakari On Thu, May 16, 2024 at 03:25:36PM GMT, Sakari Ailus wrote: > v4l2_get_link_freq() accepts a V4L2 control handler for now, but it needs > to take struct media_pad argument in order to obtain the link frequency > using get_mbus_config() pad op. Prepare for this by allowing struct > media_pad as well. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > drivers/media/v4l2-core/v4l2-common.c | 21 ++++++++++++++++++--- > include/media/v4l2-common.h | 19 ++++++++++++++++--- > 2 files changed, 34 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > index 4165c815faef..01650aed7c30 100644 > --- a/drivers/media/v4l2-core/v4l2-common.c > +++ b/drivers/media/v4l2-core/v4l2-common.c > @@ -464,8 +464,8 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat, > } > EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt); > > -s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul, > - unsigned int div) > +s64 __v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler, > + unsigned int mul, unsigned int div) > { > struct v4l2_ctrl *ctrl; > s64 freq; > @@ -500,7 +500,22 @@ s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul, > > return freq > 0 ? freq : -EINVAL; > } > -EXPORT_SYMBOL_GPL(v4l2_get_link_freq); > +EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_ctrl); > + > +#ifdef CONFIG_MEDIA_CONTROLLER > +s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul, > + unsigned int div) > +{ > + struct v4l2_subdev *sd; > + > + sd = media_entity_to_v4l2_subdev(pad->entity); > + if (!sd) > + return -ENODEV; > + > + return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div); > +} > +EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad); > +#endif /* CONFIG_MEDIA_CONTROLLER */ > > /* > * Simplify a fraction using a simple continued fraction decomposition. The > diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h > index 63ad36f04f72..f29dc8bb8f09 100644 > --- a/include/media/v4l2-common.h > +++ b/include/media/v4l2-common.h > @@ -525,7 +525,8 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat, > /** > * v4l2_get_link_freq - Get link rate from transmitter > * > - * @handler: The transmitter's control handler > + * @pad: The transmitter's media pad (or control handler for non-MC users or The users of get_link_freq we have in mainline all select MEDIA_CONTROLLER if I checked correctly v4l2_get_link_freq drivers/media/i2c/st-mipid02.c drivers/media/i2c/tc358746.c drivers/media/pci/intel/ipu3/ipu3-cio2.c drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c drivers/media/pci/intel/ivsc/mei_csi.c drivers/media/platform/cadence/cdns-csi2rx.c drivers/media/platform/nxp/imx-mipi-csis.c drivers/media/platform/nxp/imx8mq-mipi-csi2.c drivers/media/platform/qcom/camss/camss.c drivers/media/platform/ti/cal/cal-camerarx.c is it even worth having a compatibility fallback or can they be switched to use the remote's pad ? > + * compatibility reasons, don't use in new code) > * @mul: The multiplier between pixel rate and link frequency. Bits per pixel on > * D-PHY, samples per clock on parallel. 0 otherwise. > * @div: The divisor between pixel rate and link frequency. Number of data lanes > @@ -541,8 +542,20 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat, > * * %-ENOENT: Link frequency or pixel rate control not found > * * %-EINVAL: Invalid link frequency value > */ > -s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul, > - unsigned int div); > +#ifdef CONFIG_MEDIA_CONTROLLER > +#define v4l2_get_link_freq(pad, mul, div) \ > + _Generic(pad, \ > + struct media_pad *: __v4l2_get_link_freq_pad, \ > + struct v4l2_ctrl_handler *: __v4l2_get_link_freq_ctrl) \ > + (pad, mul, div) > +s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul, > + unsigned int div); > +#else /* !CONFIG_MEDIA_CONTROLLER */ > +#define v4l2_get_link_freq(handler, mul, div) \ > + __v4l2_get_link_freq_ctrl(handler, mul, div) > +#endif /* !CONFIG_MEDIA_CONTROLLER */ nit: no ! in !CONFIG_MEDIA_CONTROLLER nit: maybe an empty line here ? > +s64 __v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler, > + unsigned int mul, unsigned int div); > > void v4l2_simplify_fraction(u32 *numerator, u32 *denominator, > unsigned int n_terms, unsigned int threshold); > -- > 2.39.2 >
Hi Jacopo, On Fri, May 17, 2024 at 12:17:35PM +0200, Jacopo Mondi wrote: > Hi Sakari > > On Thu, May 16, 2024 at 03:25:36PM GMT, Sakari Ailus wrote: > > v4l2_get_link_freq() accepts a V4L2 control handler for now, but it needs > > to take struct media_pad argument in order to obtain the link frequency > > using get_mbus_config() pad op. Prepare for this by allowing struct > > media_pad as well. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > drivers/media/v4l2-core/v4l2-common.c | 21 ++++++++++++++++++--- > > include/media/v4l2-common.h | 19 ++++++++++++++++--- > > 2 files changed, 34 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > > index 4165c815faef..01650aed7c30 100644 > > --- a/drivers/media/v4l2-core/v4l2-common.c > > +++ b/drivers/media/v4l2-core/v4l2-common.c > > @@ -464,8 +464,8 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat, > > } > > EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt); > > > > -s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul, > > - unsigned int div) > > +s64 __v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler, > > + unsigned int mul, unsigned int div) > > { > > struct v4l2_ctrl *ctrl; > > s64 freq; > > @@ -500,7 +500,22 @@ s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul, > > > > return freq > 0 ? freq : -EINVAL; > > } > > -EXPORT_SYMBOL_GPL(v4l2_get_link_freq); > > +EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_ctrl); > > + > > +#ifdef CONFIG_MEDIA_CONTROLLER > > +s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul, > > + unsigned int div) > > +{ > > + struct v4l2_subdev *sd; > > + > > + sd = media_entity_to_v4l2_subdev(pad->entity); > > + if (!sd) > > + return -ENODEV; > > + > > + return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div); > > +} > > +EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad); > > +#endif /* CONFIG_MEDIA_CONTROLLER */ > > > > /* > > * Simplify a fraction using a simple continued fraction decomposition. The > > diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h > > index 63ad36f04f72..f29dc8bb8f09 100644 > > --- a/include/media/v4l2-common.h > > +++ b/include/media/v4l2-common.h > > @@ -525,7 +525,8 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat, > > /** > > * v4l2_get_link_freq - Get link rate from transmitter > > * > > - * @handler: The transmitter's control handler > > + * @pad: The transmitter's media pad (or control handler for non-MC users or > > The users of get_link_freq we have in mainline all select > MEDIA_CONTROLLER if I checked correctly > v4l2_get_link_freq > > drivers/media/i2c/st-mipid02.c > drivers/media/i2c/tc358746.c > drivers/media/pci/intel/ipu3/ipu3-cio2.c > drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > drivers/media/pci/intel/ivsc/mei_csi.c > drivers/media/platform/cadence/cdns-csi2rx.c > drivers/media/platform/nxp/imx-mipi-csis.c > drivers/media/platform/nxp/imx8mq-mipi-csi2.c > drivers/media/platform/qcom/camss/camss.c > drivers/media/platform/ti/cal/cal-camerarx.c > > is it even worth having a compatibility fallback or can they be > switched to use the remote's pad ? Good question. Now that we have this patch, I'd merge it and remove the non-pad variant after all drivers have been converted. I can do the conversion soonish. > > > > + * compatibility reasons, don't use in new code) > > * @mul: The multiplier between pixel rate and link frequency. Bits per pixel on > > * D-PHY, samples per clock on parallel. 0 otherwise. > > * @div: The divisor between pixel rate and link frequency. Number of data lanes > > @@ -541,8 +542,20 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat, > > * * %-ENOENT: Link frequency or pixel rate control not found > > * * %-EINVAL: Invalid link frequency value > > */ > > -s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul, > > - unsigned int div); > > +#ifdef CONFIG_MEDIA_CONTROLLER > > +#define v4l2_get_link_freq(pad, mul, div) \ > > + _Generic(pad, \ > > + struct media_pad *: __v4l2_get_link_freq_pad, \ > > + struct v4l2_ctrl_handler *: __v4l2_get_link_freq_ctrl) \ > > + (pad, mul, div) > > +s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul, > > + unsigned int div); > > +#else /* !CONFIG_MEDIA_CONTROLLER */ > > +#define v4l2_get_link_freq(handler, mul, div) \ > > + __v4l2_get_link_freq_ctrl(handler, mul, div) > > +#endif /* !CONFIG_MEDIA_CONTROLLER */ > > nit: no ! in !CONFIG_MEDIA_CONTROLLER I can also remove the comments, these sections are short enough so it's not very useful. > nit: maybe an empty line here ? It's very much related to what's above, I'd prefer it this way but I don't have a strong opinion about it either. :-) > > > +s64 __v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler, > > + unsigned int mul, unsigned int div); > > > > void v4l2_simplify_fraction(u32 *numerator, u32 *denominator, > > unsigned int n_terms, unsigned int threshold);
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index 4165c815faef..01650aed7c30 100644 --- a/drivers/media/v4l2-core/v4l2-common.c +++ b/drivers/media/v4l2-core/v4l2-common.c @@ -464,8 +464,8 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat, } EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt); -s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul, - unsigned int div) +s64 __v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler, + unsigned int mul, unsigned int div) { struct v4l2_ctrl *ctrl; s64 freq; @@ -500,7 +500,22 @@ s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul, return freq > 0 ? freq : -EINVAL; } -EXPORT_SYMBOL_GPL(v4l2_get_link_freq); +EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_ctrl); + +#ifdef CONFIG_MEDIA_CONTROLLER +s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul, + unsigned int div) +{ + struct v4l2_subdev *sd; + + sd = media_entity_to_v4l2_subdev(pad->entity); + if (!sd) + return -ENODEV; + + return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div); +} +EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad); +#endif /* CONFIG_MEDIA_CONTROLLER */ /* * Simplify a fraction using a simple continued fraction decomposition. The diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h index 63ad36f04f72..f29dc8bb8f09 100644 --- a/include/media/v4l2-common.h +++ b/include/media/v4l2-common.h @@ -525,7 +525,8 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat, /** * v4l2_get_link_freq - Get link rate from transmitter * - * @handler: The transmitter's control handler + * @pad: The transmitter's media pad (or control handler for non-MC users or + * compatibility reasons, don't use in new code) * @mul: The multiplier between pixel rate and link frequency. Bits per pixel on * D-PHY, samples per clock on parallel. 0 otherwise. * @div: The divisor between pixel rate and link frequency. Number of data lanes @@ -541,8 +542,20 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat, * * %-ENOENT: Link frequency or pixel rate control not found * * %-EINVAL: Invalid link frequency value */ -s64 v4l2_get_link_freq(struct v4l2_ctrl_handler *handler, unsigned int mul, - unsigned int div); +#ifdef CONFIG_MEDIA_CONTROLLER +#define v4l2_get_link_freq(pad, mul, div) \ + _Generic(pad, \ + struct media_pad *: __v4l2_get_link_freq_pad, \ + struct v4l2_ctrl_handler *: __v4l2_get_link_freq_ctrl) \ + (pad, mul, div) +s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul, + unsigned int div); +#else /* !CONFIG_MEDIA_CONTROLLER */ +#define v4l2_get_link_freq(handler, mul, div) \ + __v4l2_get_link_freq_ctrl(handler, mul, div) +#endif /* !CONFIG_MEDIA_CONTROLLER */ +s64 __v4l2_get_link_freq_ctrl(struct v4l2_ctrl_handler *handler, + unsigned int mul, unsigned int div); void v4l2_simplify_fraction(u32 *numerator, u32 *denominator, unsigned int n_terms, unsigned int threshold);
v4l2_get_link_freq() accepts a V4L2 control handler for now, but it needs to take struct media_pad argument in order to obtain the link frequency using get_mbus_config() pad op. Prepare for this by allowing struct media_pad as well. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/media/v4l2-core/v4l2-common.c | 21 ++++++++++++++++++--- include/media/v4l2-common.h | 19 ++++++++++++++++--- 2 files changed, 34 insertions(+), 6 deletions(-)