Hi Laurent,
On Mon, Nov 27, 2023 at 01:33:14PM +0200, Laurent Pinchart wrote:
> Hi Tommaso,
>
> On Mon, Nov 27, 2023 at 12:27:37PM +0100, Tommaso Merciai wrote:
> > Hi Laurent,
> > Patch looks good to me.
> >
> > Some clarification:
> >
> > I'm working as you know on alvium driver actually v14 sent.
> > Need to wait for a feedback on this or I can send v15 with the following
> > missing stuff:
> >
> > diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
> > index 1c6450c9b994..3f4104ab3094 100644
> > --- a/drivers/media/i2c/alvium-csi2.c
> > +++ b/drivers/media/i2c/alvium-csi2.c
> > @@ -1850,8 +1850,8 @@ static int alvium_s_stream(struct v4l2_subdev *sd, int enable)
> > return ret;
> > }
> >
> > -static int alvium_init_cfg(struct v4l2_subdev *sd,
> > - struct v4l2_subdev_state *state)
> > +static int alvium_init_state(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state)
> > {
> > struct alvium_dev *alvium = sd_to_alvium(sd);
> > struct alvium_mode *mode = &alvium->mode;
> > @@ -2233,7 +2233,6 @@ static const struct v4l2_subdev_video_ops alvium_video_ops = {
> > };
> >
> > static const struct v4l2_subdev_pad_ops alvium_pad_ops = {
> > - .init_cfg = alvium_init_cfg,
> > .enum_mbus_code = alvium_enum_mbus_code,
> > .get_fmt = v4l2_subdev_get_fmt,
> > .set_fmt = alvium_set_fmt,
> > @@ -2241,6 +2240,10 @@ static const struct v4l2_subdev_pad_ops alvium_pad_ops = {
> > .set_selection = alvium_set_selection,
> > };
> >
> > +static const struct v4l2_subdev_internal_ops alvium_internal_ops = {
> > + .init_state = alvium_init_state,
> > +};
> > +
> > static const struct v4l2_subdev_ops alvium_subdev_ops = {
> > .core = &alvium_core_ops,
> > .pad = &alvium_pad_ops,
> > @@ -2271,6 +2274,7 @@ static int alvium_subdev_init(struct alvium_dev *alvium)
> > /* init alvium sd */
> > v4l2_i2c_subdev_init(sd, client, &alvium_subdev_ops);
> >
> > + sd->internal_ops = &alvium_internal_ops;
> > sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_HAS_DEVNODE;
> > alvium->pad.flags = MEDIA_PAD_FL_SOURCE;
> > sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
> >
> > Rebased this morning on top of Sakari master branch :)
>
> As far as I understand, Sakari will send a pull request that will
> include this patch in a few days. I think you can already base the next
> version of the Alvium driver on top of his master branch.
Done: https://github.com/avs-sas/linux/blob/tm/media_stage/v6.7.0-rc2/alvium_drv/v15/drivers/media/i2c/alvium-csi2.c
> Sakari, please let us know if this is not correct.
>
> By the way, I forgot to CC you, but I have posted the next version of
> the .[gs]_frame_interval rework ([1]). I don't mind rebasing on top of
> your driver in case it gets merged first.
>
> [1] https://lore.kernel.org/linux-media/20231127111359.30315-1-laurent.pinchart@xxxxxxxxxxxxxxxx/T/#t
No problem.
Thanks for sharing.
Regards,
Tommaso
> > On Mon, Nov 27, 2023 at 11:07:44AM +0200, Laurent Pinchart wrote:
> > > The subdev .init_cfg() operation is affected by two issues:
> > >
> > > - It has long been extended to initialize a whole v4l2_subdev_state
> > > instead of just a v4l2_subdev_pad_config, but its name has stuck
> > > around.
> > >
> > > - Despite operating on a whole subdev state and not being directly
> > > exposed to the subdev users (either in-kernel or through the userspace
> > > API), .init_cfg() is categorized as a subdev pad operation.
> > >
> > > This participates in making the subdev API confusing for new developers.
> > > Fix it by renaming the operation to .init_state(), and make it a subdev
> > > internal operation.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > Acked-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx> # for imx415
> > > Acked-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> # for vimc
> > > ---
> > > Changes since v3:
> > >
> > > - Rebase on top of the new stm32-dcmipp driver
> > > - Fix uninitialized variable in __v4l2_subdev_state_alloc() due to bad
> > > rebase
> > >
> > > Changes since v2:
> > >
> > > - Rebase on top of the latest media tree
> > >
> > > Changes since v1:
> > >
> > > - Fix compilation of the vsp1 driver
> > > ---
> > > drivers/media/i2c/adv7180.c | 10 ++--
> > > drivers/media/i2c/ccs/ccs-core.c | 6 +--
> > > drivers/media/i2c/ds90ub913.c | 10 ++--
> > > drivers/media/i2c/ds90ub953.c | 10 ++--
> > > drivers/media/i2c/ds90ub960.c | 11 ++--
> > > drivers/media/i2c/gc2145.c | 10 ++--
> > > drivers/media/i2c/hi846.c | 10 ++--
> > > drivers/media/i2c/imx214.c | 12 +++--
> > > drivers/media/i2c/imx219.c | 9 ++--
> > > drivers/media/i2c/imx290.c | 10 ++--
> > > drivers/media/i2c/imx296.c | 10 ++--
> > > drivers/media/i2c/imx334.c | 12 +++--
> > > drivers/media/i2c/imx335.c | 12 +++--
> > > drivers/media/i2c/imx412.c | 12 +++--
> > > drivers/media/i2c/imx415.c | 10 ++--
> > > drivers/media/i2c/mt9m001.c | 10 ++--
> > > drivers/media/i2c/mt9m111.c | 10 ++--
> > > drivers/media/i2c/mt9m114.c | 16 +++---
> > > drivers/media/i2c/mt9p031.c | 8 +--
> > > drivers/media/i2c/mt9v111.c | 10 ++--
> > > drivers/media/i2c/ov01a10.c | 10 ++--
> > > drivers/media/i2c/ov02a10.c | 10 ++--
> > > drivers/media/i2c/ov2640.c | 10 ++--
> > > drivers/media/i2c/ov2680.c | 10 ++--
> > > drivers/media/i2c/ov2740.c | 10 ++--
> > > drivers/media/i2c/ov5640.c | 10 ++--
> > > drivers/media/i2c/ov5645.c | 12 +++--
> > > drivers/media/i2c/ov5670.c | 10 ++--
> > > drivers/media/i2c/ov7251.c | 12 +++--
> > > drivers/media/i2c/ov8858.c | 10 ++--
> > > drivers/media/i2c/ov9282.c | 12 +++--
> > > drivers/media/i2c/st-vgxy61.c | 10 ++--
> > > drivers/media/i2c/tc358746.c | 10 ++--
> > > drivers/media/i2c/tda1997x.c | 10 ++--
> > > drivers/media/i2c/thp7312.c | 10 ++--
> > > drivers/media/i2c/tvp5150.c | 6 +--
> > > drivers/media/pci/intel/ivsc/mei_csi.c | 10 ++--
> > > drivers/media/platform/cadence/cdns-csi2rx.c | 10 ++--
> > > .../platform/microchip/microchip-csi2dc.c | 10 ++--
> > > .../platform/microchip/microchip-isc-scaler.c | 10 ++--
> > > drivers/media/platform/nxp/imx-mipi-csis.c | 10 ++--
> > > drivers/media/platform/nxp/imx7-media-csi.c | 6 +--
> > > .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 10 ++--
> > > .../platform/nxp/imx8-isi/imx8-isi-pipe.c | 10 ++--
> > > drivers/media/platform/nxp/imx8mq-mipi-csi2.c | 10 ++--
> > > .../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 10 ++--
> > > .../platform/renesas/rzg2l-cru/rzg2l-ip.c | 10 ++--
> > > .../media/platform/renesas/vsp1/vsp1_brx.c | 1 -
> > > .../media/platform/renesas/vsp1/vsp1_clu.c | 1 -
> > > .../media/platform/renesas/vsp1/vsp1_entity.c | 53 +++++++++----------
> > > .../media/platform/renesas/vsp1/vsp1_entity.h | 2 -
> > > .../media/platform/renesas/vsp1/vsp1_hsit.c | 1 -
> > > .../media/platform/renesas/vsp1/vsp1_lif.c | 1 -
> > > .../media/platform/renesas/vsp1/vsp1_lut.c | 1 -
> > > .../media/platform/renesas/vsp1/vsp1_rwpf.c | 1 -
> > > .../media/platform/renesas/vsp1/vsp1_sru.c | 1 -
> > > .../media/platform/renesas/vsp1/vsp1_uds.c | 1 -
> > > .../media/platform/renesas/vsp1/vsp1_uif.c | 1 -
> > > .../platform/rockchip/rkisp1/rkisp1-csi.c | 10 ++--
> > > .../platform/rockchip/rkisp1/rkisp1-isp.c | 10 ++--
> > > .../platform/rockchip/rkisp1/rkisp1-resizer.c | 10 ++--
> > > .../st/stm32/stm32-dcmipp/dcmipp-byteproc.c | 6 +--
> > > .../st/stm32/stm32-dcmipp/dcmipp-parallel.c | 6 +--
> > > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 1 +
> > > .../platform/sunxi/sun4i-csi/sun4i_csi.h | 1 +
> > > .../platform/sunxi/sun4i-csi/sun4i_v4l2.c | 9 ++--
> > > .../sunxi/sun6i-csi/sun6i_csi_bridge.c | 10 ++--
> > > .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c | 10 ++--
> > > .../sun8i_a83t_mipi_csi2.c | 10 ++--
> > > drivers/media/platform/ti/cal/cal-camerarx.c | 10 ++--
> > > drivers/media/platform/video-mux.c | 10 ++--
> > > .../media/platform/xilinx/xilinx-csi2rxss.c | 10 ++--
> > > .../media/test-drivers/vimc/vimc-debayer.c | 11 ++--
> > > drivers/media/test-drivers/vimc/vimc-scaler.c | 11 ++--
> > > drivers/media/test-drivers/vimc/vimc-sensor.c | 11 ++--
> > > drivers/media/v4l2-core/v4l2-subdev.c | 20 +++----
> > > drivers/staging/media/imx/imx-ic-prp.c | 2 +-
> > > drivers/staging/media/imx/imx-ic-prpencvf.c | 2 +-
> > > drivers/staging/media/imx/imx-media-csi.c | 2 +-
> > > drivers/staging/media/imx/imx-media-utils.c | 8 +--
> > > drivers/staging/media/imx/imx-media-vdic.c | 2 +-
> > > drivers/staging/media/imx/imx-media.h | 4 +-
> > > drivers/staging/media/imx/imx6-mipi-csi2.c | 2 +-
> > > .../staging/media/starfive/camss/stf-isp.c | 6 ++-
> > > .../media/sunxi/sun6i-isp/sun6i_isp_proc.c | 10 ++--
> > > include/media/v4l2-subdev.h | 7 +--
> > > 86 files changed, 487 insertions(+), 264 deletions(-)
>
> [snip]
>
> --
> Regards,
>
> Laurent Pinchart
