Re: [PATCH v8 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver

[Sakari Ailus <sakari.ailus@iki.fi>]

On Tue, Apr 17, 2018 at 05:59:47PM +0200, Maxime Ripard wrote:
> On Tue, Apr 17, 2018 at 04:20:10PM +0300, Sakari Ailus wrote:
> > On Tue, Apr 17, 2018 at 03:10:24PM +0200, Maxime Ripard wrote:
> > > Hi Sakari,
> > > 
> > > On Fri, Apr 13, 2018 at 03:14:37PM +0300, Sakari Ailus wrote:
> > > > > +static int csi2tx_set_pad_format(struct v4l2_subdev *subdev,
> > > > > +				 struct v4l2_subdev_pad_config *cfg,
> > > > > +				 struct v4l2_subdev_format *fmt)
> > > > > +{
> > > > > +	struct csi2tx_priv *csi2tx = v4l2_subdev_to_csi2tx(subdev);
> > > > > +
> > > > > +	if (fmt->pad >= CSI2TX_PAD_MAX)
> > > > > +		return -EINVAL;
> > > > > +
> > > > > +	csi2tx->pad_fmts[fmt->pad] = fmt->format;
> > > > 
> > > > Have I asked previously if there are any limitations with this?
> > > > 
> > > > The CSI-2 TX link has multiple formats so I wouldn't support formats on
> > > > that pad in order to be compatible with the planned VC/data type support
> > > > patchset. Or do you see issues with that?
> > > 
> > > It's not just about the CSI-2 link, but more about the input pads as
> > > well, that can be configured (and we need to know the format in order
> > > to configure the IP properly).
> > > 
> > > Maybe we can simply prevent the format change on the CSI-2 pad, but
> > > not the others?
> > 
> > Yes, that was what I wanted to suggest. It's in line with the intended way
> > to support multiplexed pads.
> > 
> > The latest set is here:
> > 
> > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=vc>
> 
> Thanks for the pointer.
> 
> I've looked at the smiapp set_format hook, and especially:
> https://git.linuxtv.org/sailus/media_tree.git/tree/drivers/media/i2c/smiapp/smiapp-core.c?h=vc&id=cb864a1d8e2d19b793d8f550b026dcf8d2f78f11#n1817
> 
> After reading this, I'm not quite sure to get what I should do for the
> CSI-2 pad. Should I ignore all formats change (and thus return 0), or
> should I return EINVAL (which would probably be a bit confusing to the
> userspace)?

-EINVAL, please. The subdev uAPI does currently not (nor it is intended to)
support accessing multiple formats on a single pad, the logical choice is
to return an error when accessing a format here. That is aligned with the
stream support patchset (link above).

If you want to access the format, you need to use the pads on the other
side of the subdev.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi