1. Patchew
  2. linux
  3. View series

[PATCH next] drm: zynqmp_dp: Unlock on error in zynqmp_dp_bridge_atomic_enable()

Dan Carpenter posted 1 patch 1 week, 5 days ago 
drivers/gpu/drm/xlnx/zynqmp_dp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH next] drm: zynqmp_dp: Unlock on error in zynqmp_dp_bridge_atomic_enable()
Posted by Dan Carpenter 1 week, 5 days ago 
We added some locking to this function, but accidentally forgot to unlock
if zynqmp_dp_mode_configure() failed.  Use a guard lock to fix it.

Fixes: a7d5eeaa57d7 ("drm: zynqmp_dp: Add locking")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/gpu/drm/xlnx/zynqmp_dp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 25c5dc61ee88..0bea908b281e 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1537,7 +1537,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
 
 	pm_runtime_get_sync(dp->dev);
 
-	mutex_lock(&dp->lock);
+	guard(mutex)(&dp->lock);
 	zynqmp_dp_disp_enable(dp, old_bridge_state);
 
 	/*
@@ -1598,7 +1598,6 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
 	zynqmp_dp_write(dp, ZYNQMP_DP_SOFTWARE_RESET,
 			ZYNQMP_DP_SOFTWARE_RESET_ALL);
 	zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 1);
-	mutex_unlock(&dp->lock);
 }
 
 static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
-- 
2.45.2
Re: [PATCH next] drm: zynqmp_dp: Unlock on error in zynqmp_dp_bridge_atomic_enable()
Posted by Laurent Pinchart 1 week, 4 days ago 
Hi Dan,

Thank you for the patch.

On Mon, Nov 11, 2024 at 12:06:10PM +0300, Dan Carpenter wrote:
> We added some locking to this function, but accidentally forgot to unlock
> if zynqmp_dp_mode_configure() failed.  Use a guard lock to fix it.
> 
> Fixes: a7d5eeaa57d7 ("drm: zynqmp_dp: Add locking")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Sean, how about replacing all the mutex_lock()/mutex_unlock() calls
you've added in a7d5eeaa57d7 with guards ?

> ---
>  drivers/gpu/drm/xlnx/zynqmp_dp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 25c5dc61ee88..0bea908b281e 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1537,7 +1537,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>  
>  	pm_runtime_get_sync(dp->dev);
>  
> -	mutex_lock(&dp->lock);
> +	guard(mutex)(&dp->lock);
>  	zynqmp_dp_disp_enable(dp, old_bridge_state);
>  
>  	/*
> @@ -1598,7 +1598,6 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>  	zynqmp_dp_write(dp, ZYNQMP_DP_SOFTWARE_RESET,
>  			ZYNQMP_DP_SOFTWARE_RESET_ALL);
>  	zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 1);
> -	mutex_unlock(&dp->lock);
>  }
>  
>  static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,

-- 
Regards,

Laurent Pinchart
Re: [PATCH next] drm: zynqmp_dp: Unlock on error in zynqmp_dp_bridge_atomic_enable()
Posted by Sean Anderson 1 week, 4 days ago 
On 11/12/24 00:27, Laurent Pinchart wrote:
> Hi Dan,
> 
> Thank you for the patch.
> 
> On Mon, Nov 11, 2024 at 12:06:10PM +0300, Dan Carpenter wrote:
>> We added some locking to this function, but accidentally forgot to unlock
>> if zynqmp_dp_mode_configure() failed.  Use a guard lock to fix it.
>> 
>> Fixes: a7d5eeaa57d7 ("drm: zynqmp_dp: Add locking")
>> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Sean, how about replacing all the mutex_lock()/mutex_unlock() calls
> you've added in a7d5eeaa57d7 with guards ?

I have no objection to that.

--Sean
Re: [PATCH next] drm: zynqmp_dp: Unlock on error in zynqmp_dp_bridge_atomic_enable()
Posted by Laurent Pinchart 1 week, 4 days ago 
On Tue, Nov 12, 2024 at 09:41:58AM -0500, Sean Anderson wrote:
> On 11/12/24 00:27, Laurent Pinchart wrote:
> > Hi Dan,
> > 
> > Thank you for the patch.
> > 
> > On Mon, Nov 11, 2024 at 12:06:10PM +0300, Dan Carpenter wrote:
> >> We added some locking to this function, but accidentally forgot to unlock
> >> if zynqmp_dp_mode_configure() failed.  Use a guard lock to fix it.
> >> 
> >> Fixes: a7d5eeaa57d7 ("drm: zynqmp_dp: Add locking")
> >> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Sean, how about replacing all the mutex_lock()/mutex_unlock() calls
> > you've added in a7d5eeaa57d7 with guards ?
> 
> I have no objection to that.

Would you send a patch ? Otherwise I can do it.

-- 
Regards,

Laurent Pinchart
Re: [PATCH next] drm: zynqmp_dp: Unlock on error in zynqmp_dp_bridge_atomic_enable()
Posted by Sean Anderson 1 week, 4 days ago 
On 11/12/24 11:43, Laurent Pinchart wrote:
> On Tue, Nov 12, 2024 at 09:41:58AM -0500, Sean Anderson wrote:
>> On 11/12/24 00:27, Laurent Pinchart wrote:
>> > Hi Dan,
>> > 
>> > Thank you for the patch.
>> > 
>> > On Mon, Nov 11, 2024 at 12:06:10PM +0300, Dan Carpenter wrote:
>> >> We added some locking to this function, but accidentally forgot to unlock
>> >> if zynqmp_dp_mode_configure() failed.  Use a guard lock to fix it.
>> >> 
>> >> Fixes: a7d5eeaa57d7 ("drm: zynqmp_dp: Add locking")
>> >> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
>> > 
>> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> > 
>> > Sean, how about replacing all the mutex_lock()/mutex_unlock() calls
>> > you've added in a7d5eeaa57d7 with guards ?
>> 
>> I have no objection to that.
> 
> Would you send a patch ? Otherwise I can do it.
> 

I can send a patch, but it will not be for a week or two.

--Sean
Re: [PATCH next] drm: zynqmp_dp: Unlock on error in zynqmp_dp_bridge_atomic_enable()
Posted by Sean Anderson 1 week, 5 days ago 
On 11/11/24 04:06, Dan Carpenter wrote:
> We added some locking to this function, but accidentally forgot to unlock
> if zynqmp_dp_mode_configure() failed.  Use a guard lock to fix it.
> 
> Fixes: a7d5eeaa57d7 ("drm: zynqmp_dp: Add locking")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  drivers/gpu/drm/xlnx/zynqmp_dp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 25c5dc61ee88..0bea908b281e 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1537,7 +1537,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>  
>  	pm_runtime_get_sync(dp->dev);
>  
> -	mutex_lock(&dp->lock);
> +	guard(mutex)(&dp->lock);
>  	zynqmp_dp_disp_enable(dp, old_bridge_state);
>  
>  	/*
> @@ -1598,7 +1598,6 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>  	zynqmp_dp_write(dp, ZYNQMP_DP_SOFTWARE_RESET,
>  			ZYNQMP_DP_SOFTWARE_RESET_ALL);
>  	zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 1);
> -	mutex_unlock(&dp->lock);
>  }
>  
>  static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>

Although this reverses the order of pm_runtime_put and mutex_unlock in
the error case, I don't think it matters.

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.