diff mbox series

[RFC,1/5] uapi/linux/media.h: add flag field to struct media_device_info

Message ID 20200318213051.3200981-2-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State New, archived
Headers show
Series media-device: Report if graph is complete or not | expand

Commit Message

Niklas Söderlund March 18, 2020, 9:30 p.m. UTC
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(-)

Comments

Laurent Pinchart March 19, 2020, 2:37 a.m. UTC | #1
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
>   *
Laurent Pinchart March 19, 2020, 2:38 a.m. UTC | #2
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
> >   *
Kieran Bingham June 9, 2020, 3:16 p.m. UTC | #3
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 mbox series

Patch

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
  *