Re: [PATCH 3/3] drm: Add DRM_MODESET_LOCK_BEGIN/END helpers

2018-11-28 Thread Daniel Vetter
On Wed, Nov 28, 2018 at 09:31:11AM -0500, Sean Paul wrote:
> On Wed, Nov 28, 2018 at 10:01:07AM +0100, Daniel Vetter wrote:
> > On Tue, Nov 27, 2018 at 05:46:40PM -0500, Sean Paul wrote:
> > > diff --git a/include/drm/drm_modeset_lock.h 
> > > b/include/drm/drm_modeset_lock.h
> > > index a685d1bb21f26..6213a11445633 100644
> > > --- a/include/drm/drm_modeset_lock.h
> > > +++ b/include/drm/drm_modeset_lock.h
> > > @@ -130,4 +130,58 @@ void drm_warn_on_modeset_not_all_locked(struct 
> > > drm_device *dev);
> > >  int drm_modeset_lock_all_ctx(struct drm_device *dev,
> > >struct drm_modeset_acquire_ctx *ctx);
> > >  
> > > +/**
> > > + * DRM_MODESET_LOCK_ALL_BEGIN - Helper to acquire modeset locks
> > > + * @dev: drm device
> > > + * @ret: local ret/err/etc variable to track error status
> > > + * @ctx: local modeset acquire context, will be dereferenced
> > > + * @flags: DRM_MODESET_ACQUIRE_* flags to pass to acquire_init()
> > 
> > Full function name for the nice hyperlink. Needs a continuation line,
> > which just needs to be indentend.
> > 
> > And a bikeshed: I'd put ret last in both macros, I think that's where
> > usually the cursors/output variables are.
> 
> For _BEGIN is effectively a void, since it can't return with anything but
> ret==0. I agonized a little over doing this for _END, but figured since it was
> setting the value of ret, it might be misleading to put it at the end since
> folks might not realize that if they ignore it ret can still change (if that
> makes sense). In other words, I don't want people to think that:
> 
> ret = DRM_MODESET_LOCAL_ALL_END(ret, ctx);
> 
> behaves differently than 
> 
> DRM_MODESET_LOCAL_ALL_END(ret, ctx);
> 
> By not allowing the assignment, it might poke people to think more about ret 
> on
> END.
> 
> I'm happy to have my mind changed on this, but figured context would be 
> useful.
> 
> All other bikesheds LGTM.

I think I wasn't clear enough: I meant to put ret last in the parameter
list of each. Especially the (ret, ctx) ordering looks very strange to me.

Definitely agreed that these macros shouldn't have some kind of contrived
return value.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 3/3] drm: Add DRM_MODESET_LOCK_BEGIN/END helpers

2018-11-28 Thread Daniel Vetter
On Wed, Nov 28, 2018 at 11:59:48AM -0500, Sean Paul wrote:
> On Wed, Nov 28, 2018 at 10:01:07AM +0100, Daniel Vetter wrote:
> > On Tue, Nov 27, 2018 at 05:46:40PM -0500, Sean Paul wrote:
 > >  }
> > > diff --git a/include/drm/drm_modeset_lock.h 
> > > b/include/drm/drm_modeset_lock.h
> > > index a685d1bb21f26..6213a11445633 100644
> > > --- a/include/drm/drm_modeset_lock.h
> > > +++ b/include/drm/drm_modeset_lock.h
> > > @@ -130,4 +130,58 @@ void drm_warn_on_modeset_not_all_locked(struct 
> > > drm_device *dev);
> > >  int drm_modeset_lock_all_ctx(struct drm_device *dev,
> > >struct drm_modeset_acquire_ctx *ctx);
> > >  
> > > +/**
> > > + * DRM_MODESET_LOCK_ALL_BEGIN - Helper to acquire modeset locks
> > > + * @dev: drm device
> > > + * @ret: local ret/err/etc variable to track error status
> > > + * @ctx: local modeset acquire context, will be dereferenced
> > > + * @flags: DRM_MODESET_ACQUIRE_* flags to pass to acquire_init()
> > 
> > Full function name for the nice hyperlink. Needs a continuation line,
> > which just needs to be indentend.
> 
> This isn't a function, but a series of flags prefixed with 
> DRM_MODESET_ACQUIRE_
> (currently only one there, but perhaps more could come).

I meant you should spell out drm_modeset_acquire_init() in full. For the
hyperlink.

Wrt DRM_MODESET_LOCAL_ALL_END() just being a #define: I think kerneldoc
treats that as a function too, and you can link to it like a normal
function. Good to double-check with

$ make htmldocs

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 3/3] drm: Add DRM_MODESET_LOCK_BEGIN/END helpers

2018-11-28 Thread Sean Paul
On Wed, Nov 28, 2018 at 10:01:07AM +0100, Daniel Vetter wrote:
> On Tue, Nov 27, 2018 at 05:46:40PM -0500, Sean Paul wrote:
> > From: Sean Paul 
> > 
> > This patch adds a couple of helpers to remove the boilerplate involved
> > in grabbing all of the modeset locks.
> > 
> > I've also converted the obvious cases in drm core to use the helpers.
> > 
> > The only remaining instance of drm_modeset_lock_all_ctx() is in
> > drm_framebuffer. It's complicated by the state clear that occurs on
> > deadlock. ATM, there's no way to inject code in the deadlock path with
> > the helpers, so it's unfit for conversion.
> > 
> > Cc: Daniel Vetter 
> > Signed-off-by: Sean Paul 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 52 ++-
> >  drivers/gpu/drm/drm_color_mgmt.c| 14 ++--
> >  drivers/gpu/drm/drm_crtc.c  | 15 ++--
> >  drivers/gpu/drm/drm_plane.c | 16 ++---
> >  include/drm/drm_modeset_lock.h  | 54 +
> >  5 files changed, 72 insertions(+), 79 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 15a75b9f185fa..997735eea9abc 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3124,23 +3124,13 @@ void drm_atomic_helper_shutdown(struct drm_device 
> > *dev)
> > struct drm_modeset_acquire_ctx ctx;
> > int ret;
> >  
> > -   drm_modeset_acquire_init(&ctx, 0);
> > -   while (1) {
> > -   ret = drm_modeset_lock_all_ctx(dev, &ctx);
> > -   if (!ret)
> > -   ret = __drm_atomic_helper_disable_all(dev, &ctx, true);
> > -
> > -   if (ret != -EDEADLK)
> > -   break;
> > -
> > -   drm_modeset_backoff(&ctx);
> > -   }
> > +   DRM_MODESET_LOCK_ALL_BEGIN(dev, ret, ctx, 0);
> >  
> > +   ret = __drm_atomic_helper_disable_all(dev, &ctx, true);
> > if (ret)
> > DRM_ERROR("Disabling all crtc's during unload failed with 
> > %i\n", ret);
> >  
> > -   drm_modeset_drop_locks(&ctx);
> > -   drm_modeset_acquire_fini(&ctx);
> > +   DRM_MODESET_LOCK_ALL_END(ret, ctx);
> >  }
> >  EXPORT_SYMBOL(drm_atomic_helper_shutdown);
> >  
> > @@ -3175,14 +3165,7 @@ struct drm_atomic_state 
> > *drm_atomic_helper_suspend(struct drm_device *dev)
> > struct drm_atomic_state *state;
> > int err;
> >  
> > -   drm_modeset_acquire_init(&ctx, 0);
> > -
> > -retry:
> > -   err = drm_modeset_lock_all_ctx(dev, &ctx);
> > -   if (err < 0) {
> > -   state = ERR_PTR(err);
> > -   goto unlock;
> > -   }
> > +   DRM_MODESET_LOCK_ALL_BEGIN(dev, err, ctx, 0);
> >  
> > state = drm_atomic_helper_duplicate_state(dev, &ctx);
> > if (IS_ERR(state))
> > @@ -3191,18 +3174,14 @@ struct drm_atomic_state 
> > *drm_atomic_helper_suspend(struct drm_device *dev)
> > err = drm_atomic_helper_disable_all(dev, &ctx);
> > if (err < 0) {
> > drm_atomic_state_put(state);
> > -   state = ERR_PTR(err);
> > goto unlock;
> > }
> >  
> >  unlock:
> > -   if (PTR_ERR(state) == -EDEADLK) {
> > -   drm_modeset_backoff(&ctx);
> > -   goto retry;
> > -   }
> > +   DRM_MODESET_LOCK_ALL_END(err, ctx);
> > +   if (err)
> > +   return ERR_PTR(err);
> >  
> > -   drm_modeset_drop_locks(&ctx);
> > -   drm_modeset_acquire_fini(&ctx);
> > return state;
> >  }
> >  EXPORT_SYMBOL(drm_atomic_helper_suspend);
> > @@ -3272,23 +3251,12 @@ int drm_atomic_helper_resume(struct drm_device *dev,
> >  
> > drm_mode_config_reset(dev);
> >  
> > -   drm_modeset_acquire_init(&ctx, 0);
> > -   while (1) {
> > -   err = drm_modeset_lock_all_ctx(dev, &ctx);
> > -   if (err)
> > -   goto out;
> > +   DRM_MODESET_LOCK_ALL_BEGIN(dev, err, ctx, 0);
> >  
> > -   err = drm_atomic_helper_commit_duplicated_state(state, &ctx);
> > -out:
> > -   if (err != -EDEADLK)
> > -   break;
> > -
> > -   drm_modeset_backoff(&ctx);
> > -   }
> > +   err = drm_atomic_helper_commit_duplicated_state(state, &ctx);
> >  
> > state->acquire_ctx = NULL;
> > -   drm_modeset_drop_locks(&ctx);
> > -   drm_modeset_acquire_fini(&ctx);
> > +   DRM_MODESET_LOCK_ALL_END(err, ctx);
> > drm_atomic_state_put(state);
> >  
> > return err;
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c 
> > b/drivers/gpu/drm/drm_color_mgmt.c
> > index 581cc37882230..9c6827171d795 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -255,11 +255,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
> > if (crtc_lut->gamma_size != crtc->gamma_size)
> > return -EINVAL;
> >  
> > -   drm_modeset_acquire_init(&ctx, 0);
> > -retry:
> > -   ret = drm_modeset_lock_all_ctx(dev, &ctx);
> > -   if (ret)
> > -   goto out;
> > +   DRM_MODESET_LOCK_ALL_BEGIN(dev, ret, ctx, 0);
> >  
> > size = crtc_lut->gamma_size * (sizeof(ui

Re: [PATCH 3/3] drm: Add DRM_MODESET_LOCK_BEGIN/END helpers

2018-11-28 Thread Sean Paul
On Wed, Nov 28, 2018 at 10:01:07AM +0100, Daniel Vetter wrote:
> On Tue, Nov 27, 2018 at 05:46:40PM -0500, Sean Paul wrote:
> > From: Sean Paul 
> > 
> > This patch adds a couple of helpers to remove the boilerplate involved
> > in grabbing all of the modeset locks.
> > 
> > I've also converted the obvious cases in drm core to use the helpers.
> > 
> > The only remaining instance of drm_modeset_lock_all_ctx() is in
> > drm_framebuffer. It's complicated by the state clear that occurs on
> > deadlock. ATM, there's no way to inject code in the deadlock path with
> > the helpers, so it's unfit for conversion.
> > 
> > Cc: Daniel Vetter 
> > Signed-off-by: Sean Paul 
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 52 ++-
> >  drivers/gpu/drm/drm_color_mgmt.c| 14 ++--
> >  drivers/gpu/drm/drm_crtc.c  | 15 ++--
> >  drivers/gpu/drm/drm_plane.c | 16 ++---
> >  include/drm/drm_modeset_lock.h  | 54 +
> >  5 files changed, 72 insertions(+), 79 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 15a75b9f185fa..997735eea9abc 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3124,23 +3124,13 @@ void drm_atomic_helper_shutdown(struct drm_device 
> > *dev)
> > struct drm_modeset_acquire_ctx ctx;
> > int ret;
> >  
> > -   drm_modeset_acquire_init(&ctx, 0);
> > -   while (1) {
> > -   ret = drm_modeset_lock_all_ctx(dev, &ctx);
> > -   if (!ret)
> > -   ret = __drm_atomic_helper_disable_all(dev, &ctx, true);
> > -
> > -   if (ret != -EDEADLK)
> > -   break;
> > -
> > -   drm_modeset_backoff(&ctx);
> > -   }
> > +   DRM_MODESET_LOCK_ALL_BEGIN(dev, ret, ctx, 0);
> >  
> > +   ret = __drm_atomic_helper_disable_all(dev, &ctx, true);
> > if (ret)
> > DRM_ERROR("Disabling all crtc's during unload failed with 
> > %i\n", ret);
> >  
> > -   drm_modeset_drop_locks(&ctx);
> > -   drm_modeset_acquire_fini(&ctx);
> > +   DRM_MODESET_LOCK_ALL_END(ret, ctx);
> >  }
> >  EXPORT_SYMBOL(drm_atomic_helper_shutdown);
> >  
> > @@ -3175,14 +3165,7 @@ struct drm_atomic_state 
> > *drm_atomic_helper_suspend(struct drm_device *dev)
> > struct drm_atomic_state *state;
> > int err;
> >  
> > -   drm_modeset_acquire_init(&ctx, 0);
> > -
> > -retry:
> > -   err = drm_modeset_lock_all_ctx(dev, &ctx);
> > -   if (err < 0) {
> > -   state = ERR_PTR(err);
> > -   goto unlock;
> > -   }
> > +   DRM_MODESET_LOCK_ALL_BEGIN(dev, err, ctx, 0);
> >  
> > state = drm_atomic_helper_duplicate_state(dev, &ctx);
> > if (IS_ERR(state))
> > @@ -3191,18 +3174,14 @@ struct drm_atomic_state 
> > *drm_atomic_helper_suspend(struct drm_device *dev)
> > err = drm_atomic_helper_disable_all(dev, &ctx);
> > if (err < 0) {
> > drm_atomic_state_put(state);
> > -   state = ERR_PTR(err);
> > goto unlock;
> > }
> >  
> >  unlock:
> > -   if (PTR_ERR(state) == -EDEADLK) {
> > -   drm_modeset_backoff(&ctx);
> > -   goto retry;
> > -   }
> > +   DRM_MODESET_LOCK_ALL_END(err, ctx);
> > +   if (err)
> > +   return ERR_PTR(err);
> >  
> > -   drm_modeset_drop_locks(&ctx);
> > -   drm_modeset_acquire_fini(&ctx);
> > return state;
> >  }
> >  EXPORT_SYMBOL(drm_atomic_helper_suspend);
> > @@ -3272,23 +3251,12 @@ int drm_atomic_helper_resume(struct drm_device *dev,
> >  
> > drm_mode_config_reset(dev);
> >  
> > -   drm_modeset_acquire_init(&ctx, 0);
> > -   while (1) {
> > -   err = drm_modeset_lock_all_ctx(dev, &ctx);
> > -   if (err)
> > -   goto out;
> > +   DRM_MODESET_LOCK_ALL_BEGIN(dev, err, ctx, 0);
> >  
> > -   err = drm_atomic_helper_commit_duplicated_state(state, &ctx);
> > -out:
> > -   if (err != -EDEADLK)
> > -   break;
> > -
> > -   drm_modeset_backoff(&ctx);
> > -   }
> > +   err = drm_atomic_helper_commit_duplicated_state(state, &ctx);
> >  
> > state->acquire_ctx = NULL;
> > -   drm_modeset_drop_locks(&ctx);
> > -   drm_modeset_acquire_fini(&ctx);
> > +   DRM_MODESET_LOCK_ALL_END(err, ctx);
> > drm_atomic_state_put(state);
> >  
> > return err;
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c 
> > b/drivers/gpu/drm/drm_color_mgmt.c
> > index 581cc37882230..9c6827171d795 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -255,11 +255,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
> > if (crtc_lut->gamma_size != crtc->gamma_size)
> > return -EINVAL;
> >  
> > -   drm_modeset_acquire_init(&ctx, 0);
> > -retry:
> > -   ret = drm_modeset_lock_all_ctx(dev, &ctx);
> > -   if (ret)
> > -   goto out;
> > +   DRM_MODESET_LOCK_ALL_BEGIN(dev, ret, ctx, 0);
> >  
> > size = crtc_lut->gamma_size * (sizeof(ui

Re: [PATCH 3/3] drm: Add DRM_MODESET_LOCK_BEGIN/END helpers

2018-11-28 Thread Daniel Vetter
On Tue, Nov 27, 2018 at 05:46:40PM -0500, Sean Paul wrote:
> From: Sean Paul 
> 
> This patch adds a couple of helpers to remove the boilerplate involved
> in grabbing all of the modeset locks.
> 
> I've also converted the obvious cases in drm core to use the helpers.
> 
> The only remaining instance of drm_modeset_lock_all_ctx() is in
> drm_framebuffer. It's complicated by the state clear that occurs on
> deadlock. ATM, there's no way to inject code in the deadlock path with
> the helpers, so it's unfit for conversion.
> 
> Cc: Daniel Vetter 
> Signed-off-by: Sean Paul 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 52 ++-
>  drivers/gpu/drm/drm_color_mgmt.c| 14 ++--
>  drivers/gpu/drm/drm_crtc.c  | 15 ++--
>  drivers/gpu/drm/drm_plane.c | 16 ++---
>  include/drm/drm_modeset_lock.h  | 54 +
>  5 files changed, 72 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 15a75b9f185fa..997735eea9abc 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3124,23 +3124,13 @@ void drm_atomic_helper_shutdown(struct drm_device 
> *dev)
>   struct drm_modeset_acquire_ctx ctx;
>   int ret;
>  
> - drm_modeset_acquire_init(&ctx, 0);
> - while (1) {
> - ret = drm_modeset_lock_all_ctx(dev, &ctx);
> - if (!ret)
> - ret = __drm_atomic_helper_disable_all(dev, &ctx, true);
> -
> - if (ret != -EDEADLK)
> - break;
> -
> - drm_modeset_backoff(&ctx);
> - }
> + DRM_MODESET_LOCK_ALL_BEGIN(dev, ret, ctx, 0);
>  
> + ret = __drm_atomic_helper_disable_all(dev, &ctx, true);
>   if (ret)
>   DRM_ERROR("Disabling all crtc's during unload failed with 
> %i\n", ret);
>  
> - drm_modeset_drop_locks(&ctx);
> - drm_modeset_acquire_fini(&ctx);
> + DRM_MODESET_LOCK_ALL_END(ret, ctx);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_shutdown);
>  
> @@ -3175,14 +3165,7 @@ struct drm_atomic_state 
> *drm_atomic_helper_suspend(struct drm_device *dev)
>   struct drm_atomic_state *state;
>   int err;
>  
> - drm_modeset_acquire_init(&ctx, 0);
> -
> -retry:
> - err = drm_modeset_lock_all_ctx(dev, &ctx);
> - if (err < 0) {
> - state = ERR_PTR(err);
> - goto unlock;
> - }
> + DRM_MODESET_LOCK_ALL_BEGIN(dev, err, ctx, 0);
>  
>   state = drm_atomic_helper_duplicate_state(dev, &ctx);
>   if (IS_ERR(state))
> @@ -3191,18 +3174,14 @@ struct drm_atomic_state 
> *drm_atomic_helper_suspend(struct drm_device *dev)
>   err = drm_atomic_helper_disable_all(dev, &ctx);
>   if (err < 0) {
>   drm_atomic_state_put(state);
> - state = ERR_PTR(err);
>   goto unlock;
>   }
>  
>  unlock:
> - if (PTR_ERR(state) == -EDEADLK) {
> - drm_modeset_backoff(&ctx);
> - goto retry;
> - }
> + DRM_MODESET_LOCK_ALL_END(err, ctx);
> + if (err)
> + return ERR_PTR(err);
>  
> - drm_modeset_drop_locks(&ctx);
> - drm_modeset_acquire_fini(&ctx);
>   return state;
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_suspend);
> @@ -3272,23 +3251,12 @@ int drm_atomic_helper_resume(struct drm_device *dev,
>  
>   drm_mode_config_reset(dev);
>  
> - drm_modeset_acquire_init(&ctx, 0);
> - while (1) {
> - err = drm_modeset_lock_all_ctx(dev, &ctx);
> - if (err)
> - goto out;
> + DRM_MODESET_LOCK_ALL_BEGIN(dev, err, ctx, 0);
>  
> - err = drm_atomic_helper_commit_duplicated_state(state, &ctx);
> -out:
> - if (err != -EDEADLK)
> - break;
> -
> - drm_modeset_backoff(&ctx);
> - }
> + err = drm_atomic_helper_commit_duplicated_state(state, &ctx);
>  
>   state->acquire_ctx = NULL;
> - drm_modeset_drop_locks(&ctx);
> - drm_modeset_acquire_fini(&ctx);
> + DRM_MODESET_LOCK_ALL_END(err, ctx);
>   drm_atomic_state_put(state);
>  
>   return err;
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c 
> b/drivers/gpu/drm/drm_color_mgmt.c
> index 581cc37882230..9c6827171d795 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -255,11 +255,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
>   if (crtc_lut->gamma_size != crtc->gamma_size)
>   return -EINVAL;
>  
> - drm_modeset_acquire_init(&ctx, 0);
> -retry:
> - ret = drm_modeset_lock_all_ctx(dev, &ctx);
> - if (ret)
> - goto out;
> + DRM_MODESET_LOCK_ALL_BEGIN(dev, ret, ctx, 0);
>  
>   size = crtc_lut->gamma_size * (sizeof(uint16_t));
>   r_base = crtc->gamma_store;
> @@ -284,13 +280,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
>crtc->gamma_size, &ctx);
>  
>  out:
> -  

[PATCH 3/3] drm: Add DRM_MODESET_LOCK_BEGIN/END helpers

2018-11-27 Thread Sean Paul
From: Sean Paul 

This patch adds a couple of helpers to remove the boilerplate involved
in grabbing all of the modeset locks.

I've also converted the obvious cases in drm core to use the helpers.

The only remaining instance of drm_modeset_lock_all_ctx() is in
drm_framebuffer. It's complicated by the state clear that occurs on
deadlock. ATM, there's no way to inject code in the deadlock path with
the helpers, so it's unfit for conversion.

Cc: Daniel Vetter 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/drm_atomic_helper.c | 52 ++-
 drivers/gpu/drm/drm_color_mgmt.c| 14 ++--
 drivers/gpu/drm/drm_crtc.c  | 15 ++--
 drivers/gpu/drm/drm_plane.c | 16 ++---
 include/drm/drm_modeset_lock.h  | 54 +
 5 files changed, 72 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 15a75b9f185fa..997735eea9abc 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3124,23 +3124,13 @@ void drm_atomic_helper_shutdown(struct drm_device *dev)
struct drm_modeset_acquire_ctx ctx;
int ret;
 
-   drm_modeset_acquire_init(&ctx, 0);
-   while (1) {
-   ret = drm_modeset_lock_all_ctx(dev, &ctx);
-   if (!ret)
-   ret = __drm_atomic_helper_disable_all(dev, &ctx, true);
-
-   if (ret != -EDEADLK)
-   break;
-
-   drm_modeset_backoff(&ctx);
-   }
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ret, ctx, 0);
 
+   ret = __drm_atomic_helper_disable_all(dev, &ctx, true);
if (ret)
DRM_ERROR("Disabling all crtc's during unload failed with 
%i\n", ret);
 
-   drm_modeset_drop_locks(&ctx);
-   drm_modeset_acquire_fini(&ctx);
+   DRM_MODESET_LOCK_ALL_END(ret, ctx);
 }
 EXPORT_SYMBOL(drm_atomic_helper_shutdown);
 
@@ -3175,14 +3165,7 @@ struct drm_atomic_state 
*drm_atomic_helper_suspend(struct drm_device *dev)
struct drm_atomic_state *state;
int err;
 
-   drm_modeset_acquire_init(&ctx, 0);
-
-retry:
-   err = drm_modeset_lock_all_ctx(dev, &ctx);
-   if (err < 0) {
-   state = ERR_PTR(err);
-   goto unlock;
-   }
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, err, ctx, 0);
 
state = drm_atomic_helper_duplicate_state(dev, &ctx);
if (IS_ERR(state))
@@ -3191,18 +3174,14 @@ struct drm_atomic_state 
*drm_atomic_helper_suspend(struct drm_device *dev)
err = drm_atomic_helper_disable_all(dev, &ctx);
if (err < 0) {
drm_atomic_state_put(state);
-   state = ERR_PTR(err);
goto unlock;
}
 
 unlock:
-   if (PTR_ERR(state) == -EDEADLK) {
-   drm_modeset_backoff(&ctx);
-   goto retry;
-   }
+   DRM_MODESET_LOCK_ALL_END(err, ctx);
+   if (err)
+   return ERR_PTR(err);
 
-   drm_modeset_drop_locks(&ctx);
-   drm_modeset_acquire_fini(&ctx);
return state;
 }
 EXPORT_SYMBOL(drm_atomic_helper_suspend);
@@ -3272,23 +3251,12 @@ int drm_atomic_helper_resume(struct drm_device *dev,
 
drm_mode_config_reset(dev);
 
-   drm_modeset_acquire_init(&ctx, 0);
-   while (1) {
-   err = drm_modeset_lock_all_ctx(dev, &ctx);
-   if (err)
-   goto out;
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, err, ctx, 0);
 
-   err = drm_atomic_helper_commit_duplicated_state(state, &ctx);
-out:
-   if (err != -EDEADLK)
-   break;
-
-   drm_modeset_backoff(&ctx);
-   }
+   err = drm_atomic_helper_commit_duplicated_state(state, &ctx);
 
state->acquire_ctx = NULL;
-   drm_modeset_drop_locks(&ctx);
-   drm_modeset_acquire_fini(&ctx);
+   DRM_MODESET_LOCK_ALL_END(err, ctx);
drm_atomic_state_put(state);
 
return err;
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 581cc37882230..9c6827171d795 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -255,11 +255,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
if (crtc_lut->gamma_size != crtc->gamma_size)
return -EINVAL;
 
-   drm_modeset_acquire_init(&ctx, 0);
-retry:
-   ret = drm_modeset_lock_all_ctx(dev, &ctx);
-   if (ret)
-   goto out;
+   DRM_MODESET_LOCK_ALL_BEGIN(dev, ret, ctx, 0);
 
size = crtc_lut->gamma_size * (sizeof(uint16_t));
r_base = crtc->gamma_store;
@@ -284,13 +280,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 crtc->gamma_size, &ctx);
 
 out:
-   if (ret == -EDEADLK) {
-   drm_modeset_backoff(&ctx);
-   goto retry;
-   }
-   drm_modeset_drop_locks(&ctx);
-   drm_modeset_acquire_fini(&ctx);
-
+   DRM_MODE