linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] v4l: rcar-vin: Simplify rvin_group_notify_{bound,unbind}
@ 2017-07-21  9:55 Kieran Bingham
  2017-07-21 10:15 ` Niklas Söderlund
  2017-07-28 11:28 ` Hans Verkuil
  0 siblings, 2 replies; 3+ messages in thread
From: Kieran Bingham @ 2017-07-21  9:55 UTC (permalink / raw)
  To: niklas.soderlund
  Cc: laurent.pinchart, linux-media, linux-renesas-soc, kieran.bingham,
	Kieran Bingham

Use a container_of macro to obtain the graph entity object from the ASD
This removes the error conditions, and reduces the lock contention.

(The locking may even be potentially removed)

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---

Hi Niklas,

While working through the Multi camera setup, we came across this improvement.

If this code isn't yet upstream, feel free to squash this change into your
existing branch if you wish.

Regards

Kieran

 drivers/media/platform/rcar-vin/rcar-core.c | 28 ++++++----------------------
 drivers/media/platform/rcar-vin/rcar-vin.h  |  3 +++
 2 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 8393a1598660..b4fc7b56c8a1 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -688,20 +688,11 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
 				     struct v4l2_async_subdev *asd)
 {
 	struct rvin_dev *vin = notifier_to_vin(notifier);
-	unsigned int i;
+	struct rvin_graph_entity *csi = to_rvin_graph_entity(asd);
 
 	mutex_lock(&vin->group->lock);
-	for (i = 0; i < RVIN_CSI_MAX; i++) {
-		if (&vin->group->csi[i].asd == asd) {
-			vin_dbg(vin, "Unbind CSI-2 %s\n", subdev->name);
-			vin->group->csi[i].subdev = NULL;
-			mutex_unlock(&vin->group->lock);
-			return;
-		}
-	}
+	csi->subdev = NULL;
 	mutex_unlock(&vin->group->lock);
-
-	vin_err(vin, "No entity for subdev %s to unbind\n", subdev->name);
 }
 
 static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
@@ -709,23 +700,16 @@ static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
 				   struct v4l2_async_subdev *asd)
 {
 	struct rvin_dev *vin = notifier_to_vin(notifier);
-	unsigned int i;
+	struct rvin_graph_entity *csi = to_rvin_graph_entity(asd);
 
 	v4l2_set_subdev_hostdata(subdev, vin);
 
 	mutex_lock(&vin->group->lock);
-	for (i = 0; i < RVIN_CSI_MAX; i++) {
-		if (&vin->group->csi[i].asd == asd) {
-			vin_dbg(vin, "Bound CSI-2 %s\n", subdev->name);
-			vin->group->csi[i].subdev = subdev;
-			mutex_unlock(&vin->group->lock);
-			return 0;
-		}
-	}
+	vin_dbg(vin, "Bound CSI-2 %s\n", subdev->name);
+	csi->subdev = subdev;
 	mutex_unlock(&vin->group->lock);
 
-	vin_err(vin, "No entity for subdev %s to bind\n", subdev->name);
-	return -EINVAL;
+	return 0;
 }
 
 static struct device_node *rvin_group_get_csi(struct rvin_dev *vin,
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index e7e600fdf566..900c473c3d15 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -92,6 +92,9 @@ struct rvin_graph_entity {
 	unsigned int sink_pad;
 };
 
+#define to_rvin_graph_entity(asd) \
+	container_of(asd, struct rvin_graph_entity, asd)
+
 struct rvin_group;
 
 
-- 
2.7.4

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

* Re: [PATCH] v4l: rcar-vin: Simplify rvin_group_notify_{bound,unbind}
  2017-07-21  9:55 [PATCH] v4l: rcar-vin: Simplify rvin_group_notify_{bound,unbind} Kieran Bingham
@ 2017-07-21 10:15 ` Niklas Söderlund
  2017-07-28 11:28 ` Hans Verkuil
  1 sibling, 0 replies; 3+ messages in thread
From: Niklas Söderlund @ 2017-07-21 10:15 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: laurent.pinchart, linux-media, linux-renesas-soc, kieran.bingham

Hi Kieran,

Thanks for your patch.

On 2017-07-21 10:55:50 +0100, Kieran Bingham wrote:
> Use a container_of macro to obtain the graph entity object from the ASD
> This removes the error conditions, and reduces the lock contention.
> 
> (The locking may even be potentially removed)
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
> 
> Hi Niklas,
> 
> While working through the Multi camera setup, we came across this improvement.

Nice :-)

> 
> If this code isn't yet upstream, feel free to squash this change into your
> existing branch if you wish.

Patch is not yet upstream and I'm pending to post the next version of 
the series so will squash this in, thanks for allowing me to do so!

> 
> Regards
> 
> Kieran
> 
>  drivers/media/platform/rcar-vin/rcar-core.c | 28 ++++++----------------------
>  drivers/media/platform/rcar-vin/rcar-vin.h  |  3 +++
>  2 files changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 8393a1598660..b4fc7b56c8a1 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -688,20 +688,11 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
>  				     struct v4l2_async_subdev *asd)
>  {
>  	struct rvin_dev *vin = notifier_to_vin(notifier);
> -	unsigned int i;
> +	struct rvin_graph_entity *csi = to_rvin_graph_entity(asd);
>  
>  	mutex_lock(&vin->group->lock);
> -	for (i = 0; i < RVIN_CSI_MAX; i++) {
> -		if (&vin->group->csi[i].asd == asd) {
> -			vin_dbg(vin, "Unbind CSI-2 %s\n", subdev->name);
> -			vin->group->csi[i].subdev = NULL;
> -			mutex_unlock(&vin->group->lock);
> -			return;
> -		}
> -	}
> +	csi->subdev = NULL;
>  	mutex_unlock(&vin->group->lock);
> -
> -	vin_err(vin, "No entity for subdev %s to unbind\n", subdev->name);
>  }
>  
>  static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
> @@ -709,23 +700,16 @@ static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
>  				   struct v4l2_async_subdev *asd)
>  {
>  	struct rvin_dev *vin = notifier_to_vin(notifier);
> -	unsigned int i;
> +	struct rvin_graph_entity *csi = to_rvin_graph_entity(asd);
>  
>  	v4l2_set_subdev_hostdata(subdev, vin);
>  
>  	mutex_lock(&vin->group->lock);
> -	for (i = 0; i < RVIN_CSI_MAX; i++) {
> -		if (&vin->group->csi[i].asd == asd) {
> -			vin_dbg(vin, "Bound CSI-2 %s\n", subdev->name);
> -			vin->group->csi[i].subdev = subdev;
> -			mutex_unlock(&vin->group->lock);
> -			return 0;
> -		}
> -	}
> +	vin_dbg(vin, "Bound CSI-2 %s\n", subdev->name);
> +	csi->subdev = subdev;
>  	mutex_unlock(&vin->group->lock);
>  
> -	vin_err(vin, "No entity for subdev %s to bind\n", subdev->name);
> -	return -EINVAL;
> +	return 0;
>  }
>  
>  static struct device_node *rvin_group_get_csi(struct rvin_dev *vin,
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index e7e600fdf566..900c473c3d15 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -92,6 +92,9 @@ struct rvin_graph_entity {
>  	unsigned int sink_pad;
>  };
>  
> +#define to_rvin_graph_entity(asd) \
> +	container_of(asd, struct rvin_graph_entity, asd)
> +
>  struct rvin_group;
>  
>  
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

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

* Re: [PATCH] v4l: rcar-vin: Simplify rvin_group_notify_{bound,unbind}
  2017-07-21  9:55 [PATCH] v4l: rcar-vin: Simplify rvin_group_notify_{bound,unbind} Kieran Bingham
  2017-07-21 10:15 ` Niklas Söderlund
@ 2017-07-28 11:28 ` Hans Verkuil
  1 sibling, 0 replies; 3+ messages in thread
From: Hans Verkuil @ 2017-07-28 11:28 UTC (permalink / raw)
  To: Kieran Bingham, niklas.soderlund
  Cc: laurent.pinchart, linux-media, linux-renesas-soc, kieran.bingham

On 07/21/2017 11:55 AM, Kieran Bingham wrote:
> Use a container_of macro to obtain the graph entity object from the ASD
> This removes the error conditions, and reduces the lock contention.
> 
> (The locking may even be potentially removed)

I've set the status in patchwork to 'Not Applicable' since the patch isn't
against the mainline code. I leave it to Niklas to decide what to do with this.

Regards,

	Hans

> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
> 
> Hi Niklas,
> 
> While working through the Multi camera setup, we came across this improvement.
> 
> If this code isn't yet upstream, feel free to squash this change into your
> existing branch if you wish.
> 
> Regards
> 
> Kieran
> 
>  drivers/media/platform/rcar-vin/rcar-core.c | 28 ++++++----------------------
>  drivers/media/platform/rcar-vin/rcar-vin.h  |  3 +++
>  2 files changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 8393a1598660..b4fc7b56c8a1 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -688,20 +688,11 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
>  				     struct v4l2_async_subdev *asd)
>  {
>  	struct rvin_dev *vin = notifier_to_vin(notifier);
> -	unsigned int i;
> +	struct rvin_graph_entity *csi = to_rvin_graph_entity(asd);
>  
>  	mutex_lock(&vin->group->lock);
> -	for (i = 0; i < RVIN_CSI_MAX; i++) {
> -		if (&vin->group->csi[i].asd == asd) {
> -			vin_dbg(vin, "Unbind CSI-2 %s\n", subdev->name);
> -			vin->group->csi[i].subdev = NULL;
> -			mutex_unlock(&vin->group->lock);
> -			return;
> -		}
> -	}
> +	csi->subdev = NULL;
>  	mutex_unlock(&vin->group->lock);
> -
> -	vin_err(vin, "No entity for subdev %s to unbind\n", subdev->name);
>  }
>  
>  static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
> @@ -709,23 +700,16 @@ static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
>  				   struct v4l2_async_subdev *asd)
>  {
>  	struct rvin_dev *vin = notifier_to_vin(notifier);
> -	unsigned int i;
> +	struct rvin_graph_entity *csi = to_rvin_graph_entity(asd);
>  
>  	v4l2_set_subdev_hostdata(subdev, vin);
>  
>  	mutex_lock(&vin->group->lock);
> -	for (i = 0; i < RVIN_CSI_MAX; i++) {
> -		if (&vin->group->csi[i].asd == asd) {
> -			vin_dbg(vin, "Bound CSI-2 %s\n", subdev->name);
> -			vin->group->csi[i].subdev = subdev;
> -			mutex_unlock(&vin->group->lock);
> -			return 0;
> -		}
> -	}
> +	vin_dbg(vin, "Bound CSI-2 %s\n", subdev->name);
> +	csi->subdev = subdev;
>  	mutex_unlock(&vin->group->lock);
>  
> -	vin_err(vin, "No entity for subdev %s to bind\n", subdev->name);
> -	return -EINVAL;
> +	return 0;
>  }
>  
>  static struct device_node *rvin_group_get_csi(struct rvin_dev *vin,
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index e7e600fdf566..900c473c3d15 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -92,6 +92,9 @@ struct rvin_graph_entity {
>  	unsigned int sink_pad;
>  };
>  
> +#define to_rvin_graph_entity(asd) \
> +	container_of(asd, struct rvin_graph_entity, asd)
> +
>  struct rvin_group;
>  
>  
> 

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

end of thread, other threads:[~2017-07-28 11:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-21  9:55 [PATCH] v4l: rcar-vin: Simplify rvin_group_notify_{bound,unbind} Kieran Bingham
2017-07-21 10:15 ` Niklas Söderlund
2017-07-28 11:28 ` Hans Verkuil

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).