| Message ID | 20180823132544.521-22-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | v4l: add support for multiplexed streams | expand |
Hi Niklas, Sakari On 23/08/2018 14:25, Niklas Söderlund wrote: > From: Sakari Ailus <sakari.ailus@linux.intel.com> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > include/media/v4l2-subdev.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 5acaeeb9b3cacefa..ac1f7ee4cdb978ad 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -349,12 +349,21 @@ struct v4l2_mbus_frame_desc_entry { > > #define V4L2_FRAME_DESC_ENTRY_MAX 4 > > +enum { > + V4L2_MBUS_FRAME_DESC_TYPE_PLATFORM, > + V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL, > + V4L2_MBUS_FRAME_DESC_TYPE_CCP2, > + V4L2_MBUS_FRAME_DESC_TYPE_CSI2, Does this need to be extended to differentiate CSI2 DPHY/CPHY as has been done in the v4l2_mbus_config structures? > +}; > + > /** > * struct v4l2_mbus_frame_desc - media bus data frame description > + * @type: type of the bus (V4L2_MBUS_FRAME_DESC_TYPE_*) > * @entry: frame descriptors array > * @num_entries: number of entries in @entry array > */ > struct v4l2_mbus_frame_desc { > + u32 type; > struct v4l2_mbus_frame_desc_entry entry[V4L2_FRAME_DESC_ENTRY_MAX]; > unsigned short num_entries; > }; >
Hi Kieran, On Fri, Nov 02, 2018 at 12:27:11PM +0000, Kieran Bingham wrote: > Hi Niklas, Sakari > > On 23/08/2018 14:25, Niklas Söderlund wrote: > > From: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > include/media/v4l2-subdev.h | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > > index 5acaeeb9b3cacefa..ac1f7ee4cdb978ad 100644 > > --- a/include/media/v4l2-subdev.h > > +++ b/include/media/v4l2-subdev.h > > @@ -349,12 +349,21 @@ struct v4l2_mbus_frame_desc_entry { > > > > #define V4L2_FRAME_DESC_ENTRY_MAX 4 > > > > +enum { > > + V4L2_MBUS_FRAME_DESC_TYPE_PLATFORM, > > + V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL, > > + V4L2_MBUS_FRAME_DESC_TYPE_CCP2, > > + V4L2_MBUS_FRAME_DESC_TYPE_CSI2, > > Does this need to be extended to differentiate CSI2 DPHY/CPHY as has > been done in the v4l2_mbus_config structures? I'd say no; the PHY isn't really relevant at this level. The configuration from fwnode should suffice.
Hi Sakari, Niklas, On 02/11/2018 13:15, Sakari Ailus wrote: > Hi Kieran, > > On Fri, Nov 02, 2018 at 12:27:11PM +0000, Kieran Bingham wrote: >> Hi Niklas, Sakari >> >> On 23/08/2018 14:25, Niklas Söderlund wrote: >>> From: Sakari Ailus <sakari.ailus@linux.intel.com> >>> >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> >>> --- >>> include/media/v4l2-subdev.h | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h >>> index 5acaeeb9b3cacefa..ac1f7ee4cdb978ad 100644 >>> --- a/include/media/v4l2-subdev.h >>> +++ b/include/media/v4l2-subdev.h >>> @@ -349,12 +349,21 @@ struct v4l2_mbus_frame_desc_entry { >>> >>> #define V4L2_FRAME_DESC_ENTRY_MAX 4 >>> >>> +enum { >>> + V4L2_MBUS_FRAME_DESC_TYPE_PLATFORM, >>> + V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL, >>> + V4L2_MBUS_FRAME_DESC_TYPE_CCP2, >>> + V4L2_MBUS_FRAME_DESC_TYPE_CSI2, >> >> Does this need to be extended to differentiate CSI2 DPHY/CPHY as has >> been done in the v4l2_mbus_config structures? > > I'd say no; the PHY isn't really relevant at this level. The configuration > from fwnode should suffice. Great - Thanks for the feedback. Well then - now that I've gone through the patch - and the PHY type naming is cleared up, I can add: Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> (I guess Niklas can pick up that tag currently) Although - we're missing any commit message other than the commit title. Should something be added? There's not much to describe above the title really.
On Fri, Nov 02, 2018 at 01:35:02PM +0000, Kieran Bingham wrote: > Hi Sakari, Niklas, > > On 02/11/2018 13:15, Sakari Ailus wrote: > > Hi Kieran, > > > > On Fri, Nov 02, 2018 at 12:27:11PM +0000, Kieran Bingham wrote: > >> Hi Niklas, Sakari > >> > >> On 23/08/2018 14:25, Niklas Söderlund wrote: > >>> From: Sakari Ailus <sakari.ailus@linux.intel.com> > >>> > >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > >>> --- > >>> include/media/v4l2-subdev.h | 9 +++++++++ > >>> 1 file changed, 9 insertions(+) > >>> > >>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > >>> index 5acaeeb9b3cacefa..ac1f7ee4cdb978ad 100644 > >>> --- a/include/media/v4l2-subdev.h > >>> +++ b/include/media/v4l2-subdev.h > >>> @@ -349,12 +349,21 @@ struct v4l2_mbus_frame_desc_entry { > >>> > >>> #define V4L2_FRAME_DESC_ENTRY_MAX 4 > >>> > >>> +enum { > >>> + V4L2_MBUS_FRAME_DESC_TYPE_PLATFORM, > >>> + V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL, > >>> + V4L2_MBUS_FRAME_DESC_TYPE_CCP2, > >>> + V4L2_MBUS_FRAME_DESC_TYPE_CSI2, > >> > >> Does this need to be extended to differentiate CSI2 DPHY/CPHY as has > >> been done in the v4l2_mbus_config structures? > > > > I'd say no; the PHY isn't really relevant at this level. The configuration > > from fwnode should suffice. > > Great - Thanks for the feedback. > > > Well then - now that I've gone through the patch - and the PHY type > naming is cleared up, I can add: > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > (I guess Niklas can pick up that tag currently) > > Although - we're missing any commit message other than the commit title. > Should something be added? > > There's not much to describe above the title really. Oh, indeed. How about this: The type will be used to determine which bus specific frame descriptor struct is applicable to a given frame descriptor.
Hi Sakari, On 02/11/2018 14:18, Sakari Ailus wrote: > On Fri, Nov 02, 2018 at 01:35:02PM +0000, Kieran Bingham wrote: >> Hi Sakari, Niklas, >> >> On 02/11/2018 13:15, Sakari Ailus wrote: >>> Hi Kieran, >>> >>> On Fri, Nov 02, 2018 at 12:27:11PM +0000, Kieran Bingham wrote: >>>> Hi Niklas, Sakari >>>> >>>> On 23/08/2018 14:25, Niklas Söderlund wrote: >>>>> From: Sakari Ailus <sakari.ailus@linux.intel.com> >>>>> >>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> >>>>> --- >>>>> include/media/v4l2-subdev.h | 9 +++++++++ >>>>> 1 file changed, 9 insertions(+) >>>>> >>>>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h >>>>> index 5acaeeb9b3cacefa..ac1f7ee4cdb978ad 100644 >>>>> --- a/include/media/v4l2-subdev.h >>>>> +++ b/include/media/v4l2-subdev.h >>>>> @@ -349,12 +349,21 @@ struct v4l2_mbus_frame_desc_entry { >>>>> >>>>> #define V4L2_FRAME_DESC_ENTRY_MAX 4 >>>>> >>>>> +enum { >>>>> + V4L2_MBUS_FRAME_DESC_TYPE_PLATFORM, >>>>> + V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL, >>>>> + V4L2_MBUS_FRAME_DESC_TYPE_CCP2, >>>>> + V4L2_MBUS_FRAME_DESC_TYPE_CSI2, >>>> >>>> Does this need to be extended to differentiate CSI2 DPHY/CPHY as has >>>> been done in the v4l2_mbus_config structures? >>> >>> I'd say no; the PHY isn't really relevant at this level. The configuration >>> from fwnode should suffice. >> >> Great - Thanks for the feedback. >> >> >> Well then - now that I've gone through the patch - and the PHY type >> naming is cleared up, I can add: >> >> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> >> (I guess Niklas can pick up that tag currently) >> >> Although - we're missing any commit message other than the commit title. >> Should something be added? >> >> There's not much to describe above the title really. > > Oh, indeed. > > How about this: > > The type will be used to determine which bus specific frame descriptor > struct is applicable to a given frame descriptor. Looks good to me!
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 5acaeeb9b3cacefa..ac1f7ee4cdb978ad 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -349,12 +349,21 @@ struct v4l2_mbus_frame_desc_entry { #define V4L2_FRAME_DESC_ENTRY_MAX 4 +enum { + V4L2_MBUS_FRAME_DESC_TYPE_PLATFORM, + V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL, + V4L2_MBUS_FRAME_DESC_TYPE_CCP2, + V4L2_MBUS_FRAME_DESC_TYPE_CSI2, +}; + /** * struct v4l2_mbus_frame_desc - media bus data frame description + * @type: type of the bus (V4L2_MBUS_FRAME_DESC_TYPE_*) * @entry: frame descriptors array * @num_entries: number of entries in @entry array */ struct v4l2_mbus_frame_desc { + u32 type; struct v4l2_mbus_frame_desc_entry entry[V4L2_FRAME_DESC_ENTRY_MAX]; unsigned short num_entries; };