linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] rcar-vin: Fix issues with partial probed media graphs
@ 2020-08-13 21:06 Niklas Söderlund
  2020-08-13 21:06 ` [PATCH 1/2] rcar-vin: Unconditionally unregister notifier on remove Niklas Söderlund
  2020-08-13 21:06 ` [PATCH 2/2] rcar-vin: Register media device when all sub-devices bound Niklas Söderlund
  0 siblings, 2 replies; 3+ messages in thread
From: Niklas Söderlund @ 2020-08-13 21:06 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund

Hi,

The R-Car VIN driver have since it gain Gen3 support tried to be 
functional with a partial probed media graph. This was done to allow for 
situations where parts of the graph failed to probe, the hardware 
contains two separate CSI-2 receivers one of them could fail while still 
allow capturing from the other.

This design have been proven to be impractical as there are some issues 
in the V4L2 framework that needs to be worked on before a driver could 
be made to support this. And interacting with the media device or the 
video devices exposed by the driver when it's in a partially probed 
state have been proven to cause user-space issues as the driver gained 
new features.

Instead of playing wack-a-mole trying to make the driver work in a 
partially probed mode within a uncooperative framework. This series 
disallows any user-space interactions until the media graph is fully 
probed by withholding all video and media devices from user-space while 
the graph is not complete.

This series makes no attempt to improve on the object life-time 
management issues pointed out about V4L2 in general when a video device 
is unbound. Those problems should be worked on a framework level and can 
not be fixed in drivers alone.

This series is based on the media-tree and tested on R-Car M3-N.

Niklas Söderlund (2):
  rcar-vin: Unconditionally unregister notifier on remove
  rcar-vin: Register media device when all sub-devices bound

 drivers/media/platform/rcar-vin/rcar-core.c | 22 +++++++++------------
 1 file changed, 9 insertions(+), 13 deletions(-)

-- 
2.28.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/2] rcar-vin: Unconditionally unregister notifier on remove
  2020-08-13 21:06 [PATCH 0/2] rcar-vin: Fix issues with partial probed media graphs Niklas Söderlund
@ 2020-08-13 21:06 ` Niklas Söderlund
  2020-08-13 21:06 ` [PATCH 2/2] rcar-vin: Register media device when all sub-devices bound Niklas Söderlund
  1 sibling, 0 replies; 3+ messages in thread
From: Niklas Söderlund @ 2020-08-13 21:06 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund

If the VIN device is part of a group of VIN devices (all Gen3 boards)
there is no reason to only unregister the group notifier if the VIN that
registers the notifier is removed. The VIN that registers the notifier
is always the last VIN device to be bound, so keeping the notifier
around after any VIN is unbound creates an unbalanced state where no VIN
in the group is operational.

Fix this by unconditionally unregistering the notifier when any VIN
device is unbound. Unregistering the notifier will lead to unbound()
being called and all video devices exposed by any VIN instance to be
removed.

The lock was only needed to protect the check which VIN registers the
notifier and is no longer needed.

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

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 7440c8965d27e64f..1cbbcc9008e39627 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -1370,12 +1370,8 @@ static int rcar_vin_remove(struct platform_device *pdev)
 	v4l2_async_notifier_cleanup(&vin->notifier);
 
 	if (vin->info->use_mc) {
-		mutex_lock(&vin->group->lock);
-		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
-			v4l2_async_notifier_unregister(&vin->group->notifier);
-			v4l2_async_notifier_cleanup(&vin->group->notifier);
-		}
-		mutex_unlock(&vin->group->lock);
+		v4l2_async_notifier_unregister(&vin->group->notifier);
+		v4l2_async_notifier_cleanup(&vin->group->notifier);
 		rvin_group_put(vin);
 	}
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] rcar-vin: Register media device when all sub-devices bound
  2020-08-13 21:06 [PATCH 0/2] rcar-vin: Fix issues with partial probed media graphs Niklas Söderlund
  2020-08-13 21:06 ` [PATCH 1/2] rcar-vin: Unconditionally unregister notifier on remove Niklas Söderlund
@ 2020-08-13 21:06 ` Niklas Söderlund
  1 sibling, 0 replies; 3+ messages in thread
From: Niklas Söderlund @ 2020-08-13 21:06 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund

The media device is not usable by userspace before all devices involved
in capture are present in the system. Move registering of the media
device to the async complete callback.

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

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 1cbbcc9008e39627..04c9b33fde68f5a3 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -243,7 +243,6 @@ static struct rvin_group *rvin_group_data;
 
 static void rvin_group_cleanup(struct rvin_group *group)
 {
-	media_device_unregister(&group->mdev);
 	media_device_cleanup(&group->mdev);
 	mutex_destroy(&group->lock);
 }
@@ -253,7 +252,6 @@ static int rvin_group_init(struct rvin_group *group, struct rvin_dev *vin)
 	struct media_device *mdev = &group->mdev;
 	const struct of_device_id *match;
 	struct device_node *np;
-	int ret;
 
 	mutex_init(&group->lock);
 
@@ -278,11 +276,7 @@ static int rvin_group_init(struct rvin_group *group, struct rvin_dev *vin)
 
 	media_device_init(mdev);
 
-	ret = media_device_register(&group->mdev);
-	if (ret)
-		rvin_group_cleanup(group);
-
-	return ret;
+	return 0;
 }
 
 static void rvin_group_release(struct kref *kref)
@@ -682,6 +676,10 @@ static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
 	unsigned int i;
 	int ret;
 
+	ret = media_device_register(&vin->group->mdev);
+	if (ret)
+		return ret;
+
 	ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
 	if (ret) {
 		vin_err(vin, "Failed to register subdev nodes\n");
@@ -762,6 +760,8 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
 	}
 
 	mutex_unlock(&vin->group->lock);
+
+	media_device_unregister(&vin->group->mdev);
 }
 
 static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-08-13 21:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-13 21:06 [PATCH 0/2] rcar-vin: Fix issues with partial probed media graphs Niklas Söderlund
2020-08-13 21:06 ` [PATCH 1/2] rcar-vin: Unconditionally unregister notifier on remove Niklas Söderlund
2020-08-13 21:06 ` [PATCH 2/2] rcar-vin: Register media device when all sub-devices bound Niklas Söderlund

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).