Hi Sakari,
+Maxime because of it's D-PHY work in the phy framework.
On Sat, Mar 16, 2019 at 07:51:21PM +0200, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Sat, Mar 16, 2019 at 04:47:57PM +0100, Jacopo Mondi wrote:
> > Add PHY-specific parameters to MIPI CSI-2 frame descriptor.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> > ---
> > include/media/v4l2-subdev.h | 42 +++++++++++++++++++++++++++++++------
> > 1 file changed, 36 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index 6311f670de3c..eca9633c83bf 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -317,11 +317,33 @@ struct v4l2_subdev_audio_ops {
> > int (*s_stream)(struct v4l2_subdev *sd, int enable);
> > };
> >
> > +#define V4L2_FRAME_DESC_ENTRY_DPHY_DATA_LANES 4
> > +
> > +/**
> > + * struct v4l2_mbus_frame_desc_entry_csi2_dphy - MIPI D-PHY bus configuration
> > + *
> > + * @clock_lane: physical lane index of the clock lane
> > + * @data_lanes: an array of physical data lane indexes
> > + * @num_data_lanes: number of data lanes
> > + */
> > +struct v4l2_mbus_frame_desc_entry_csi2_dphy {
> > + u8 clock_lane;
> > + u8 data_lanes[V4L2_FRAME_DESC_ENTRY_DPHY_DATA_LANES];
> > + u8 num_data_lanes;
>
> Do you need more than the number of the data lanes? I'd expect few devices
> to be able to do more than that. The PHY type comes already from the
> firmware but I guess it's good to do the separation here as well.
Indeed lane reordering at run time seems like a quite unusual
operation. I would say I could drop that, but then, a structure and a
new field in v4l2_mbus_frame_desc for just an u8, isn't it an
overkill (unless we know it could be expanded, with say, D-PHY timings
as in Maxime's D-PHY phy support implementation. Again, not sure they
should be run-time negotiated, but...)
>
> Could you use V4L2_FWNODE_CSI2_MAX_DATA_LANES? Or we could rename it. But I
> think it'd be good to stick to a single definition.
>
I initially moved and renamed that define, then went back and added a
new one as I was not sure where to put this new global and D-PHY
specific define. I'll look into unifying them.
Thanks
j
> > +};
> > +
> > +/**
> > + * struct v4l2_mbus_frame_desc_entry_csi2_cphy - MIPI C-PHY bus configuration
> > + */
> > +struct v4l2_mbus_frame_desc_entry_csi2_cphy {
> > + /* TODO */
> > +};
> > +
> > /**
> > * struct v4l2_mbus_frame_desc_entry_csi2
> > *
> > - * @channel: CSI-2 virtual channel
> > - * @data_type: CSI-2 data type ID
> > + * @channel: CSI-2 virtual channel
> > + * @data_type: CSI-2 data type ID
> > */
> > struct v4l2_mbus_frame_desc_entry_csi2 {
> > u8 channel;
> > @@ -371,18 +393,26 @@ enum v4l2_mbus_frame_desc_type {
> > V4L2_MBUS_FRAME_DESC_TYPE_PLATFORM,
> > V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL,
> > V4L2_MBUS_FRAME_DESC_TYPE_CCP2,
> > - V4L2_MBUS_FRAME_DESC_TYPE_CSI2,
> > + V4L2_MBUS_FRAME_DESC_TYPE_CSI2_DPHY,
> > + V4L2_MBUS_FRAME_DESC_TYPE_CSI2_CPHY,
> > };
> >
> > /**
> > * struct v4l2_mbus_frame_desc - media bus data frame description
> > - * @type: type of the bus (enum v4l2_mbus_frame_desc_type)
> > - * @entry: frame descriptors array
> > - * @num_entries: number of entries in @entry array
> > + * @type: type of the bus (enum v4l2_mbus_frame_desc_type)
> > + * @entry: frame descriptors array
> > + * @phy: PHY specific parameters
> > + * @phy.dphy: MIPI D-PHY specific bus configurations
> > + * @phy.cphy: MIPI C-PHY specific bus configurations
> > + * @num_entries: number of entries in @entry array
> > */
> > struct v4l2_mbus_frame_desc {
> > enum v4l2_mbus_frame_desc_type type;
> > struct v4l2_mbus_frame_desc_entry entry[V4L2_FRAME_DESC_ENTRY_MAX];
> > + union {
> > + struct v4l2_mbus_frame_desc_entry_csi2_dphy csi2_dphy;
> > + struct v4l2_mbus_frame_desc_entry_csi2_cphy csi2_cphy;
> > + } phy;
> > unsigned short num_entries;
> > };
> >
>
> --
> Regards,
>
> Sakari Ailus
> sakari.ailus@xxxxxxxxxxxxxxx
Attachment:
signature.asc
Description: PGP signature
