| Message ID | 20210415130450.421168-18-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | v4l: subdev internal routing | expand |
Hi Tomi and Sakari, Thank you for the patch. On Thu, Apr 15, 2021 at 04:04:43PM +0300, Tomi Valkeinen wrote: > From: Sakari Ailus <sakari.ailus@linux.intel.com> > > The stream field identifies the stream this frame descriptor applies to in > routing configuration across a multiplexed link. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > --- > include/media/v4l2-subdev.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 30ec011d31e3..436d0445aafd 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -338,6 +338,7 @@ enum v4l2_mbus_frame_desc_flags { > * struct v4l2_mbus_frame_desc_entry - media bus frame description structure > * > * @flags: bitmask flags, as defined by &enum v4l2_mbus_frame_desc_flags. > + * @stream: stream in routing configuration > * @pixelcode: media bus pixel code, valid if @flags > * %FRAME_DESC_FL_BLOB is not set. > * @length: number of octets per frame, valid if @flags > @@ -347,6 +348,7 @@ enum v4l2_mbus_frame_desc_flags { > */ > struct v4l2_mbus_frame_desc_entry { > enum v4l2_mbus_frame_desc_flags flags; > + u32 stream; As this isn't used in this series, I can't really tell how it will be used, so it's hard to review the patch. Should we postpone it to the next patch series ? > u32 pixelcode; > u32 length; > union { -- Regards, Laurent Pinchart
On 18/04/2021 22:27, Laurent Pinchart wrote: > Hi Tomi and Sakari, > > Thank you for the patch. > > On Thu, Apr 15, 2021 at 04:04:43PM +0300, Tomi Valkeinen wrote: >> From: Sakari Ailus <sakari.ailus@linux.intel.com> >> >> The stream field identifies the stream this frame descriptor applies to in >> routing configuration across a multiplexed link. >> >> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> >> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> >> Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org> >> --- >> include/media/v4l2-subdev.h | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h >> index 30ec011d31e3..436d0445aafd 100644 >> --- a/include/media/v4l2-subdev.h >> +++ b/include/media/v4l2-subdev.h >> @@ -338,6 +338,7 @@ enum v4l2_mbus_frame_desc_flags { >> * struct v4l2_mbus_frame_desc_entry - media bus frame description structure >> * >> * @flags: bitmask flags, as defined by &enum v4l2_mbus_frame_desc_flags. >> + * @stream: stream in routing configuration >> * @pixelcode: media bus pixel code, valid if @flags >> * %FRAME_DESC_FL_BLOB is not set. >> * @length: number of octets per frame, valid if @flags >> @@ -347,6 +348,7 @@ enum v4l2_mbus_frame_desc_flags { >> */ >> struct v4l2_mbus_frame_desc_entry { >> enum v4l2_mbus_frame_desc_flags flags; >> + u32 stream; > > As this isn't used in this series, I can't really tell how it will be > used, so it's hard to review the patch. Should we postpone it to the > next patch series ? It's not used here, but it is needed allow the drivers use the features introduced in this series. It is used to match routes to frame_descs. Its usage is quite simple: e.g. when a driver has a route (v4l2_subdev_route), it can get the frame_descs from the entity behind v4l2_subdev_route.source_pad, and then find the matching frame_desc by searching for v4l2_subdev_route.source_stream == v4l2_mbus_frame_desc_entry.stream Tomi
Hi Tomi, On Thu, Apr 22, 2021 at 03:47:05PM +0300, Tomi Valkeinen wrote: > On 18/04/2021 22:27, Laurent Pinchart wrote: > > On Thu, Apr 15, 2021 at 04:04:43PM +0300, Tomi Valkeinen wrote: > >> From: Sakari Ailus <sakari.ailus@linux.intel.com> > >> > >> The stream field identifies the stream this frame descriptor applies to in > >> routing configuration across a multiplexed link. > >> > >> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > >> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > >> Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > >> --- > >> include/media/v4l2-subdev.h | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > >> index 30ec011d31e3..436d0445aafd 100644 > >> --- a/include/media/v4l2-subdev.h > >> +++ b/include/media/v4l2-subdev.h > >> @@ -338,6 +338,7 @@ enum v4l2_mbus_frame_desc_flags { > >> * struct v4l2_mbus_frame_desc_entry - media bus frame description structure > >> * > >> * @flags: bitmask flags, as defined by &enum v4l2_mbus_frame_desc_flags. > >> + * @stream: stream in routing configuration > >> * @pixelcode: media bus pixel code, valid if @flags > >> * %FRAME_DESC_FL_BLOB is not set. > >> * @length: number of octets per frame, valid if @flags > >> @@ -347,6 +348,7 @@ enum v4l2_mbus_frame_desc_flags { > >> */ > >> struct v4l2_mbus_frame_desc_entry { > >> enum v4l2_mbus_frame_desc_flags flags; > >> + u32 stream; > > > > As this isn't used in this series, I can't really tell how it will be > > used, so it's hard to review the patch. Should we postpone it to the > > next patch series ? > > It's not used here, but it is needed allow the drivers use the features > introduced in this series. It is used to match routes to frame_descs. Sure, I'm not saying it should be dropped, but it would be easier to review it in the context of the next series, while the rest of this series bring a self-contained feature that is already useful without streams support. > Its usage is quite simple: e.g. when a driver has a route > (v4l2_subdev_route), it can get the frame_descs from the entity behind > v4l2_subdev_route.source_pad, and then find the matching frame_desc by > searching for > > v4l2_subdev_route.source_stream == v4l2_mbus_frame_desc_entry.stream -- Regards, Laurent Pinchart
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 30ec011d31e3..436d0445aafd 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -338,6 +338,7 @@ enum v4l2_mbus_frame_desc_flags { * struct v4l2_mbus_frame_desc_entry - media bus frame description structure * * @flags: bitmask flags, as defined by &enum v4l2_mbus_frame_desc_flags. + * @stream: stream in routing configuration * @pixelcode: media bus pixel code, valid if @flags * %FRAME_DESC_FL_BLOB is not set. * @length: number of octets per frame, valid if @flags @@ -347,6 +348,7 @@ enum v4l2_mbus_frame_desc_flags { */ struct v4l2_mbus_frame_desc_entry { enum v4l2_mbus_frame_desc_flags flags; + u32 stream; u32 pixelcode; u32 length; union {