[RFC 0/5] media-device: Report if graph is complete or not

[RFC 0/5] media-device: Report if graph is complete or not

[Niklas Söderlund]

Hi,

This series is an attempt to scratch an old itch, it's problematic to 
support unbind and then a second call to complete in v4l2-async. When 
the second complete call happens a lot of things can go wrong.

When v4l2-async complete callback is called multiple video devices are 
registered with video_register_device(). Then if a v4l2-async unbind 
happens they are unregistered with video_unregister_device().

Their are multiple problems with this, specially for R-Car VIN.

1. Defending on which subdevice is unbound parts of the video pipeline 
   can still function. There are for example two CSI-2 receivers 
   connected to two different CSI-2 buses in the pipeline. If one of the 
   receivers are unbound the other can still function perfectly well.  
   But with the current setup everything goes away, this is bad for 
   operational safety.

2. The struct video_device contains a static struct device, which in 
   turn contains a static struct kref. When the kref is release by 
   calling video_unregister_device() and then later trying to 
   re-register the video device video_register_device() the kref life 
   time management kicks in and produces warnings in later kernels or 
   OOPS in older ones.

It has been discussed in the past at various conferences that it could 
be OK to not video_unregister_device() if a v4l2-async unbind happens.  
The argument against it was that user-space needed a way to check if a 
pipeline was completely probed or not. And this used to be that the 
video devices where only present if everything was available.

It was agreed in principle that if an alternate way for media controller 
centric devices could be found to inform user-space of this fact could 
be found it would be OK to not unregister video devices in case of an 
unbind or even allow registering the video devices at probe time instead 
of at v4l2-async complete time.

This series aims to provide such a mechanism using the media device 
itself to (optionally) report if the media graph is complete or not.

Patch 1/5, 2/5 and 3/5 adds the plumbing inside the core to carry such 
information from a driver to user-space. Patch 4/5 and 5/5 implements 
the new media device callback to report graph status and stops 
registering the video devices in case an v4l2-async unbind happens.

A complementary series to v4l2-utils is posted as [1] which demonstrates 
the usage of this series from user-space.

1. [PATCH 0/2] v4l-utils: media-ctl: Print media graph completes if 
   available

Niklas Söderlund (5):
  uapi/linux/media.h: add flag field to struct media_device_info
  media-device: Add a graph_complete callback to struct media_device_ops
  mc-device.c: If graph completes status is available report it to
    user-space
  rcar-vin: Report the completeness of the media graph
  rcar-vin: Do not unregister video device when a subdevice is unbound

 drivers/media/mc/mc-device.c                |  4 ++++
 drivers/media/platform/rcar-vin/rcar-core.c | 18 +++++++++++++-----
 drivers/media/platform/rcar-vin/rcar-vin.h  |  4 ++++
 include/media/media-device.h                |  3 +++
 include/uapi/linux/media.h                  |  9 ++++++++-
 5 files changed, 32 insertions(+), 6 deletions(-)

-- 
2.25.1

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

[Niklas Söderlund]

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];
 };
 
+/*
+ * Graph flags
+ */
+#define MEDIA_INFO_FLAG_INCOMPLETE	(1 << 0)
+#define MEDIA_INFO_FLAG_COMPLETE	(1 << 1)
+
 /*
  * Base number ranges for entity functions
  *
-- 
2.25.1

[RFC 2/5] media-device: Add a graph_complete callback to struct media_device_ops

[Niklas Söderlund]

Add a new graph_complete operation to struct media_device_ops. The
callback is optional to implement. If it's implemented it shall return
the status about the media graphs completes. If all entities that the
media device could contain is registered in the graph it shall return
true, otherwise false.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 include/media/media-device.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/media/media-device.h b/include/media/media-device.h
index fa089543072052cf..f637ad2eee38f456 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -61,6 +61,8 @@ struct media_entity_notify {
  *	       request (and thus the buffer) must be available to the driver.
  *	       And once a buffer is queued, then the driver can complete
  *	       or delete objects from the request before req_queue exits.
+ * @graph_complete: Check if the media device graph is complete and all entries
+ *		    have been added to the graph.
  */
 struct media_device_ops {
 	int (*link_notify)(struct media_link *link, u32 flags,
@@ -69,6 +71,7 @@ struct media_device_ops {
 	void (*req_free)(struct media_request *req);
 	int (*req_validate)(struct media_request *req);
 	void (*req_queue)(struct media_request *req);
+	bool (*graph_complete)(struct media_device *mdev);
 };
 
 /**
-- 
2.25.1

[RFC 3/5] mc-device.c: If graph completes status is available report it to user-space

[Niklas Söderlund]

If the media device implements the graph_complete callback utilise it
and fill in the completes of the graph in the struct media_device_info.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/mc/mc-device.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
index da8088351135298a..64c786570b6df129 100644
--- a/drivers/media/mc/mc-device.c
+++ b/drivers/media/mc/mc-device.c
@@ -75,6 +75,10 @@ static long media_device_get_info(struct media_device *dev, void *arg)
 	info->driver_version = info->media_version;
 	info->hw_revision = dev->hw_revision;
 
+	if (dev->ops && dev->ops->graph_complete)
+		info->flags |= dev->ops->graph_complete(dev) ?
+			MEDIA_INFO_FLAG_COMPLETE : MEDIA_INFO_FLAG_INCOMPLETE;
+
 	return 0;
 }
 
-- 
2.25.1

[RFC 4/5] rcar-vin: Report the completeness of the media graph

[Niklas Söderlund]

Implement the graph_complete callback and report if the media graph is
complete or not.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 13 +++++++++++++
 drivers/media/platform/rcar-vin/rcar-vin.h  |  4 ++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 7440c8965d27e64f..21ce3de8168c3224 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -221,8 +221,16 @@ static int rvin_group_link_notify(struct media_link *link, u32 flags,
 	return ret;
 }
 
+static bool rvin_group_graph_complete(struct media_device *mdev)
+{
+	struct rvin_group *group = container_of(mdev, struct rvin_group, mdev);
+
+	return group->complete;
+}
+
 static const struct media_device_ops rvin_media_ops = {
 	.link_notify = rvin_group_link_notify,
+	.graph_complete = rvin_group_graph_complete,
 };
 
 /* -----------------------------------------------------------------------------
@@ -735,6 +743,9 @@ static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
 			break;
 		}
 	}
+
+	vin->group->complete = true;
+
 	mutex_unlock(&vin->group->lock);
 
 	return ret;
@@ -761,6 +772,8 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
 		break;
 	}
 
+	vin->group->complete = false;
+
 	mutex_unlock(&vin->group->lock);
 }
 
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index c19d077ce1cb4f4b..ff04adbb969b07de 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -263,6 +263,8 @@ struct rvin_dev {
  * @vin:		VIN instances which are part of the group
  * @csi:		array of pairs of fwnode and subdev pointers
  *			to all CSI-2 subdevices.
+ * @complete:		True if all devices of the group are in the media graph,
+ *			false otherwise.
  */
 struct rvin_group {
 	struct kref refcount;
@@ -278,6 +280,8 @@ struct rvin_group {
 		struct fwnode_handle *fwnode;
 		struct v4l2_subdev *subdev;
 	} csi[RVIN_CSI_MAX];
+
+	bool complete;
 };
 
 int rvin_dma_register(struct rvin_dev *vin, int irq);
-- 
2.25.1

[RFC 5/5] rcar-vin: Do not unregister video device when a subdevice is unbound

[Niklas Söderlund]

If the v4l2-async notifier have once been complete and the video
device(s) have been register, do not unregister them if one subdevice is
unbound. Depending on which subdevice is unbound other parts of the
pipeline could still be functional. For example if one of multiple
sensors connected to a CSI-2 transmitter is unbound other sensors in
that pipeline are still useable.

This problem is extra critical for R-Car VIN which registers two
independent CSI-2 receivers in the same media graph as they can both be
used by the same dma-engines. If one of the CSI-2 receivers are unbound
the other CSI-2 receiver pipeline is still fully functional.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 21ce3de8168c3224..d51ffe75c34c97c5 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -533,7 +533,6 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
 
 static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
 {
-	rvin_v4l2_unregister(vin);
 	vin->parallel->subdev = NULL;
 
 	if (!vin->info->use_mc) {
@@ -758,10 +757,6 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
 	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
 	unsigned int i;
 
-	for (i = 0; i < RCAR_VIN_NUM; i++)
-		if (vin->group->vin[i])
-			rvin_v4l2_unregister(vin->group->vin[i]);
-
 	mutex_lock(&vin->group->lock);
 
 	for (i = 0; i < RVIN_CSI_MAX; i++) {
-- 
2.25.1

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

[Laurent Pinchart]

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
>   *

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

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
> >   *

-- 
Regards,

Laurent Pinchart

Re: [RFC 2/5] media-device: Add a graph_complete callback to struct media_device_ops

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Wed, Mar 18, 2020 at 10:30:48PM +0100, Niklas Söderlund wrote:
> Add a new graph_complete operation to struct media_device_ops. The
> callback is optional to implement. If it's implemented it shall return
> the status about the media graphs completes. If all entities that the
> media device could contain is registered in the graph it shall return
> true, otherwise false.

I'd rather do it the other way around, implement a function that drivers
can call to signal completion. It will store the flag internally in
media_device, and will also be able to send an event to notify
userspace (once we get MC events).

> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  include/media/media-device.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index fa089543072052cf..f637ad2eee38f456 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -61,6 +61,8 @@ struct media_entity_notify {
>   *	       request (and thus the buffer) must be available to the driver.
>   *	       And once a buffer is queued, then the driver can complete
>   *	       or delete objects from the request before req_queue exits.
> + * @graph_complete: Check if the media device graph is complete and all entries
> + *		    have been added to the graph.
>   */
>  struct media_device_ops {
>  	int (*link_notify)(struct media_link *link, u32 flags,
> @@ -69,6 +71,7 @@ struct media_device_ops {
>  	void (*req_free)(struct media_request *req);
>  	int (*req_validate)(struct media_request *req);
>  	void (*req_queue)(struct media_request *req);
> +	bool (*graph_complete)(struct media_device *mdev);
>  };
>  
>  /**

-- 
Regards,

Laurent Pinchart

Re: [RFC 4/5] rcar-vin: Report the completeness of the media graph

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Wed, Mar 18, 2020 at 10:30:50PM +0100, Niklas Söderlund wrote:
> Implement the graph_complete callback and report if the media graph is
> complete or not.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 13 +++++++++++++
>  drivers/media/platform/rcar-vin/rcar-vin.h  |  4 ++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 7440c8965d27e64f..21ce3de8168c3224 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -221,8 +221,16 @@ static int rvin_group_link_notify(struct media_link *link, u32 flags,
>  	return ret;
>  }
>  
> +static bool rvin_group_graph_complete(struct media_device *mdev)
> +{
> +	struct rvin_group *group = container_of(mdev, struct rvin_group, mdev);
> +
> +	return group->complete;
> +}
> +
>  static const struct media_device_ops rvin_media_ops = {
>  	.link_notify = rvin_group_link_notify,
> +	.graph_complete = rvin_group_graph_complete,
>  };
>  
>  /* -----------------------------------------------------------------------------
> @@ -735,6 +743,9 @@ static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
>  			break;
>  		}
>  	}
> +
> +	vin->group->complete = true;

Going from incomplete to complete is fine...

> +
>  	mutex_unlock(&vin->group->lock);
>  
>  	return ret;
> @@ -761,6 +772,8 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
>  		break;
>  	}
>  
> +	vin->group->complete = false;
> +

... but the other way around is more problematic. We need to define the
exact semantics for userspace, and how it should handle this event.

>  	mutex_unlock(&vin->group->lock);
>  }
>  
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index c19d077ce1cb4f4b..ff04adbb969b07de 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -263,6 +263,8 @@ struct rvin_dev {
>   * @vin:		VIN instances which are part of the group
>   * @csi:		array of pairs of fwnode and subdev pointers
>   *			to all CSI-2 subdevices.
> + * @complete:		True if all devices of the group are in the media graph,
> + *			false otherwise.
>   */
>  struct rvin_group {
>  	struct kref refcount;
> @@ -278,6 +280,8 @@ struct rvin_group {
>  		struct fwnode_handle *fwnode;
>  		struct v4l2_subdev *subdev;
>  	} csi[RVIN_CSI_MAX];
> +
> +	bool complete;
>  };
>  
>  int rvin_dma_register(struct rvin_dev *vin, int irq);

-- 
Regards,

Laurent Pinchart

Re: [RFC 5/5] rcar-vin: Do not unregister video device when a subdevice is unbound

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Wed, Mar 18, 2020 at 10:30:51PM +0100, Niklas Söderlund wrote:
> If the v4l2-async notifier have once been complete and the video
> device(s) have been register, do not unregister them if one subdevice is
> unbound. Depending on which subdevice is unbound other parts of the
> pipeline could still be functional. For example if one of multiple
> sensors connected to a CSI-2 transmitter is unbound other sensors in
> that pipeline are still useable.
> 
> This problem is extra critical for R-Car VIN which registers two
> independent CSI-2 receivers in the same media graph as they can both be
> used by the same dma-engines. If one of the CSI-2 receivers are unbound
> the other CSI-2 receiver pipeline is still fully functional.

I like this, but shouldn't you also move registration of the video nodes
to probe time then ?

> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 21ce3de8168c3224..d51ffe75c34c97c5 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -533,7 +533,6 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
>  
>  static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
>  {
> -	rvin_v4l2_unregister(vin);
>  	vin->parallel->subdev = NULL;
>  
>  	if (!vin->info->use_mc) {
> @@ -758,10 +757,6 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
>  	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
>  	unsigned int i;
>  
> -	for (i = 0; i < RCAR_VIN_NUM; i++)
> -		if (vin->group->vin[i])
> -			rvin_v4l2_unregister(vin->group->vin[i]);
> -
>  	mutex_lock(&vin->group->lock);
>  
>  	for (i = 0; i < RVIN_CSI_MAX; i++) {

-- 
Regards,

Laurent Pinchart

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

[Kieran Bingham]

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
>>>   *
> 

Re: [RFC 3/5] mc-device.c: If graph completes status is available report it to user-space

[Kieran Bingham]

Hi Niklas,

On 18/03/2020 21:30, Niklas Söderlund wrote:> If the media device
implements the graph_complete callback utilise it> and fill in the
completes of the graph in the struct media_device_info.> >
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/mc/mc-device.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
> index da8088351135298a..64c786570b6df129 100644
> --- a/drivers/media/mc/mc-device.c
> +++ b/drivers/media/mc/mc-device.c
> @@ -75,6 +75,10 @@ static long media_device_get_info(struct media_device *dev, void *arg)
>  	info->driver_version = info->media_version;
>  	info->hw_revision = dev->hw_revision;
>  
> +	if (dev->ops && dev->ops->graph_complete)
> +		info->flags |= dev->ops->graph_complete(dev) ?
> +			MEDIA_INFO_FLAG_COMPLETE : MEDIA_INFO_FLAG_INCOMPLETE;

Aha, I see it was because of graph_complete being optional that leads to
having both states of the bool...

And I guess we can't 'default' to either state if graph_complete()
doesn't exist...

--
Kieran



> +
>  	return 0;
>  }
>  
>