Re: [PATCH 1/2] media: i2c: cvs: Add driver of Intel Computer Vision Sensing Controller(CVS)

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Kieran,

On Wed, May 06, 2026 at 09:38:49AM +0100, Kieran Bingham wrote:
> > +       case MEDIA_BUS_FMT_SBGGR14_1X14:
> > +       case MEDIA_BUS_FMT_SGBRG14_1X14:
> > +       case MEDIA_BUS_FMT_SGRBG14_1X14:
> > +       case MEDIA_BUS_FMT_SRGGB14_1X14:
> > +       case MEDIA_BUS_FMT_SBGGR16_1X16:
> > +       case MEDIA_BUS_FMT_SGBRG16_1X16:
> > +       case MEDIA_BUS_FMT_SGRBG16_1X16:
> > +       case MEDIA_BUS_FMT_SRGGB16_1X16:
> > +               break;
> 
> I've seen this on other pass through devices, and I wonder if really
> there's nothing to filter here - it could be anything. Or perhaps if we
> did need to ensure the driver only supported a specific bus type - that
> could be factored out.

I revall v4l2-compliance will complain if you don't limit to some set of
formats -- I think it tests for a single value which isn't a valid format.

I'd keep this list here for now but I think we should have such a list in
V4L2 core. Also the mux drivers could use it.

> > +/**
> > + * cvs_csi_get_mbus_config - Provide current CSI-2 bus configuration
> > + * @sd: Sub-device
> > + * @pad: Pad index
> > + * @cfg: Returned bus config
> > + *
> > + * Fills lane ordering and number of lanes; retrieves link frequency from
> > + * remote entity.
> > + *
> > + * Return: 0 on success or negative errno.
> > + */
> > +static int cvs_csi_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad,
> > +                                  struct v4l2_mbus_config *cfg)
> > +{
> > +       struct icvs *ctx = sd_to_csi(sd);
> > +       s64 freq;
> > +
> > +       cfg->type = V4L2_MBUS_CSI2_DPHY;
> > +       for (unsigned int i = 0; i < V4L2_MBUS_CSI2_MAX_DATA_LANES; i++)
> > +               cfg->bus.mipi_csi2.data_lanes[i] = i + 1;
> > +       cfg->bus.mipi_csi2.num_data_lanes = ctx->nr_of_lanes;
> > +
> > +       freq = v4l2_get_link_freq(ctx->remote, 0, 0);
> > +       if (freq < 0)
> > +               return -EINVAL;
> > +
> > +       ctx->link_freq = freq;
> > +       cfg->link_freq = freq;
> > +
> > +       return 0;
> > +}
> 
> So more of a question for linux-media community, I've seen this pattern
> a couple of times - should or could we consider a way to factor out a
> passthrough device which doesn't modify the stream(s) formats and is
> simply representing almost a connector type ?

The CVS in fact does have a CSI-2 receiver and a transmitter so I think it
should have its own sub-device. I understand some implementations also need
to have this configuration in order to operate and there's the privacy
control, too.

-- 
Regards,

Sakari Ailus