| Message ID | 20200318213051.3200981-2-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media-device: Report if graph is complete or not | expand |
Hi Niklas, Thank you for the patch. On Wed, Mar 18, 2020 at 10:30:47PM +0100, Niklas Söderlund wrote: > Add a flags field to the media_device_info structure by taking one > of the reserved u32 fields. The use-case is to have a way to > (optionally) report to user-space if the media graph is complete or not. > > Also define two flags to carry information about if the graph is > complete or not. If neither of the two flags are set the > media device does not support reporting its graph status. The other bits > in the flags field are unused for now, but could be claimed to carry > other type of information in the future. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > include/uapi/linux/media.h | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > index 383ac7b7d8f07eca..9b37ed8b41d0d866 100644 > --- a/include/uapi/linux/media.h > +++ b/include/uapi/linux/media.h > @@ -34,9 +34,16 @@ struct media_device_info { > __u32 media_version; > __u32 hw_revision; > __u32 driver_version; > - __u32 reserved[31]; > + __u32 flags; > + __u32 reserved[30]; I think this information should be added to media_v2_topology, not media_device_info, otherwise you'll have a race condition between retrieving the media device information and the topology. media_device_info is really supposed to be static. > }; > > +/* > + * Graph flags > + */ > +#define MEDIA_INFO_FLAG_INCOMPLETE (1 << 0) > +#define MEDIA_INFO_FLAG_COMPLETE (1 << 1) > + > /* > * Base number ranges for entity functions > *
On Thu, Mar 19, 2020 at 04:37:44AM +0200, Laurent Pinchart wrote: > Hi Niklas, > > Thank you for the patch. > > On Wed, Mar 18, 2020 at 10:30:47PM +0100, Niklas Söderlund wrote: > > Add a flags field to the media_device_info structure by taking one > > of the reserved u32 fields. The use-case is to have a way to > > (optionally) report to user-space if the media graph is complete or not. > > > > Also define two flags to carry information about if the graph is > > complete or not. If neither of the two flags are set the > > media device does not support reporting its graph status. The other bits > > in the flags field are unused for now, but could be claimed to carry > > other type of information in the future. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > --- > > include/uapi/linux/media.h | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > > index 383ac7b7d8f07eca..9b37ed8b41d0d866 100644 > > --- a/include/uapi/linux/media.h > > +++ b/include/uapi/linux/media.h > > @@ -34,9 +34,16 @@ struct media_device_info { > > __u32 media_version; > > __u32 hw_revision; > > __u32 driver_version; > > - __u32 reserved[31]; > > + __u32 flags; > > + __u32 reserved[30]; > > I think this information should be added to media_v2_topology, not > media_device_info, otherwise you'll have a race condition between > retrieving the media device information and the topology. > media_device_info is really supposed to be static. Also, documentation is needed. > > }; > > > > +/* > > + * Graph flags > > + */ > > +#define MEDIA_INFO_FLAG_INCOMPLETE (1 << 0) > > +#define MEDIA_INFO_FLAG_COMPLETE (1 << 1) > > + > > /* > > * Base number ranges for entity functions > > *
On 19/03/2020 02:38, Laurent Pinchart wrote: > On Thu, Mar 19, 2020 at 04:37:44AM +0200, Laurent Pinchart wrote: >> Hi Niklas, >> >> Thank you for the patch. >> >> On Wed, Mar 18, 2020 at 10:30:47PM +0100, Niklas Söderlund wrote: >>> Add a flags field to the media_device_info structure by taking one >>> of the reserved u32 fields. The use-case is to have a way to >>> (optionally) report to user-space if the media graph is complete or not. >>> >>> Also define two flags to carry information about if the graph is >>> complete or not. If neither of the two flags are set the >>> media device does not support reporting its graph status. The other bits >>> in the flags field are unused for now, but could be claimed to carry >>> other type of information in the future. >>> >>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> >>> --- >>> include/uapi/linux/media.h | 9 ++++++++- >>> 1 file changed, 8 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h >>> index 383ac7b7d8f07eca..9b37ed8b41d0d866 100644 >>> --- a/include/uapi/linux/media.h >>> +++ b/include/uapi/linux/media.h >>> @@ -34,9 +34,16 @@ struct media_device_info { >>> __u32 media_version; >>> __u32 hw_revision; >>> __u32 driver_version; >>> - __u32 reserved[31]; >>> + __u32 flags; >>> + __u32 reserved[30]; >> >> I think this information should be added to media_v2_topology, not >> media_device_info, otherwise you'll have a race condition between >> retrieving the media device information and the topology. >> media_device_info is really supposed to be static. > > Also, documentation is needed. > >>> }; >>> >>> +/* >>> + * Graph flags >>> + */ >>> +#define MEDIA_INFO_FLAG_INCOMPLETE (1 << 0) >>> +#define MEDIA_INFO_FLAG_COMPLETE (1 << 1) Isn't this boolean, and therefore wouldn't a single flag be sufficient? or do you expect there to be some in-between state where neither of these flags would be set. -- Kieran >>> + >>> /* >>> * Base number ranges for entity functions >>> * >
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h index 383ac7b7d8f07eca..9b37ed8b41d0d866 100644 --- a/include/uapi/linux/media.h +++ b/include/uapi/linux/media.h @@ -34,9 +34,16 @@ struct media_device_info { __u32 media_version; __u32 hw_revision; __u32 driver_version; - __u32 reserved[31]; + __u32 flags; + __u32 reserved[30]; }; +/* + * Graph flags + */ +#define MEDIA_INFO_FLAG_INCOMPLETE (1 << 0) +#define MEDIA_INFO_FLAG_COMPLETE (1 << 1) + /* * Base number ranges for entity functions *
Add a flags field to the media_device_info structure by taking one of the reserved u32 fields. The use-case is to have a way to (optionally) report to user-space if the media graph is complete or not. Also define two flags to carry information about if the graph is complete or not. If neither of the two flags are set the media device does not support reporting its graph status. The other bits in the flags field are unused for now, but could be claimed to carry other type of information in the future. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- include/uapi/linux/media.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)