| Message ID | b4042bd9-c943-4738-a2e1-8647259137c6@stanley.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [next] drm: zynqmp_dp: Unlock on error in zynqmp_dp_bridge_atomic_enable() | expand |
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.
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,
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
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.
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
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,
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(-)