On Fri, Aug 14, 2020 at 5:38 AM Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: > > This fell off in the conversion in > > commit 9bcaa3fe58ab7559e71df798bcff6e0795158695 > Author: Michal Orzel <michalorzel.eng@xxxxxxxxx> > Date: Tue Apr 28 19:10:04 2020 +0200 > > drm: Replace drm_modeset_lock/unlock_all with DRM_MODESET_LOCK_ALL_* helpers > > but it's caught by the drm_warn_on_modeset_not_all_locked() that the > legacy modeset code uses. Since this is the bkl and it's unclear > what's all protected, play it safe and grab it again for legacy > drivers. > > Unfortunately this means we need to sprinkle a few more #includes > around. > > Also we need to add the drm_device as a parameter to the _END macro. > > Finally remove the mute_lock() from setcrtc, since that's now done by > the macro. > > Cc: Alex Deucher <alexdeucher@xxxxxxxxx> > References: https://gitlab.freedesktop.org/drm/amd/-/issues/1224 > Fixes: 9bcaa3fe58ab ("drm: Replace drm_modeset_lock/unlock_all with DRM_MODESET_LOCK_ALL_* helpers") > Cc: Michal Orzel <michalorzel.eng@xxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > Cc: Maxime Ripard <mripard@xxxxxxxxxx> > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> > Cc: David Airlie <airlied@xxxxxxxx> > Cc: Daniel Vetter <daniel@xxxxxxxx> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > Cc: <stable@xxxxxxxxxxxxxxx> # v5.8+ > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > -- > Patch compiles but otherwise untested, and I'll go on vacations now > for 2 weeks. Alex, can you pls take care of this? Looks good to me. Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx> Also confirmed to fix the issue. I'll push to drm-misc. Thanks! Alex > > Thanks, Daniel > --- > drivers/gpu/drm/drm_atomic_helper.c | 7 ++++--- > drivers/gpu/drm/drm_color_mgmt.c | 2 +- > drivers/gpu/drm/drm_crtc.c | 4 +--- > drivers/gpu/drm/drm_mode_object.c | 4 ++-- > drivers/gpu/drm/drm_plane.c | 2 +- > include/drm/drm_modeset_lock.h | 9 +++++++-- > 6 files changed, 16 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index f67ee513a7cc..7515a40b2056 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -34,6 +34,7 @@ > #include <drm/drm_bridge.h> > #include <drm/drm_damage_helper.h> > #include <drm/drm_device.h> > +#include <drm/drm_drv.h> > #include <drm/drm_plane_helper.h> > #include <drm/drm_print.h> > #include <drm/drm_self_refresh_helper.h> > @@ -3109,7 +3110,7 @@ void drm_atomic_helper_shutdown(struct drm_device *dev) > if (ret) > DRM_ERROR("Disabling all crtc's during unload failed with %i\n", ret); > > - DRM_MODESET_LOCK_ALL_END(ctx, ret); > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > } > EXPORT_SYMBOL(drm_atomic_helper_shutdown); > > @@ -3249,7 +3250,7 @@ struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev) > } > > unlock: > - DRM_MODESET_LOCK_ALL_END(ctx, err); > + DRM_MODESET_LOCK_ALL_END(dev, ctx, err); > if (err) > return ERR_PTR(err); > > @@ -3330,7 +3331,7 @@ int drm_atomic_helper_resume(struct drm_device *dev, > > err = drm_atomic_helper_commit_duplicated_state(state, &ctx); > > - DRM_MODESET_LOCK_ALL_END(ctx, err); > + DRM_MODESET_LOCK_ALL_END(dev, ctx, err); > 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 c93123ff7c21..138ff34b31db 100644 > --- a/drivers/gpu/drm/drm_color_mgmt.c > +++ b/drivers/gpu/drm/drm_color_mgmt.c > @@ -294,7 +294,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev, > crtc->gamma_size, &ctx); > > out: > - DRM_MODESET_LOCK_ALL_END(ctx, ret); > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > return ret; > > } > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 283bcc4362ca..aecdd7ea26dc 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -588,7 +588,6 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, > if (crtc_req->mode_valid && !drm_lease_held(file_priv, plane->base.id)) > return -EACCES; > > - mutex_lock(&crtc->dev->mode_config.mutex); > DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, > DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret); > > @@ -756,8 +755,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, > fb = NULL; > mode = NULL; > > - DRM_MODESET_LOCK_ALL_END(ctx, ret); > - mutex_unlock(&crtc->dev->mode_config.mutex); > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > > return ret; > } > diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c > index 901b078abf40..db05f386a709 100644 > --- a/drivers/gpu/drm/drm_mode_object.c > +++ b/drivers/gpu/drm/drm_mode_object.c > @@ -428,7 +428,7 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data, > out_unref: > drm_mode_object_put(obj); > out: > - DRM_MODESET_LOCK_ALL_END(ctx, ret); > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > return ret; > } > > @@ -470,7 +470,7 @@ static int set_property_legacy(struct drm_mode_object *obj, > break; > } > drm_property_change_valid_put(prop, ref); > - DRM_MODESET_LOCK_ALL_END(ctx, ret); > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > > return ret; > } > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index b7b90b3a2e38..affe1cfed009 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -792,7 +792,7 @@ static int setplane_internal(struct drm_plane *plane, > crtc_x, crtc_y, crtc_w, crtc_h, > src_x, src_y, src_w, src_h, &ctx); > > - DRM_MODESET_LOCK_ALL_END(ctx, ret); > + DRM_MODESET_LOCK_ALL_END(plane->dev, ctx, ret); > > return ret; > } > diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h > index 4fc9a43ac45a..aafd07388eb7 100644 > --- a/include/drm/drm_modeset_lock.h > +++ b/include/drm/drm_modeset_lock.h > @@ -164,6 +164,8 @@ int drm_modeset_lock_all_ctx(struct drm_device *dev, > * is 0, so no error checking is necessary > */ > #define DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, flags, ret) \ > + if (!drm_drv_uses_atomic_modeset(dev)) \ > + mutex_lock(&dev->mode_config.mutex); \ > drm_modeset_acquire_init(&ctx, flags); \ > modeset_lock_retry: \ > ret = drm_modeset_lock_all_ctx(dev, &ctx); \ > @@ -172,6 +174,7 @@ modeset_lock_retry: \ > > /** > * DRM_MODESET_LOCK_ALL_END - Helper to release and cleanup modeset locks > + * @dev: drm device > * @ctx: local modeset acquire context, will be dereferenced > * @ret: local ret/err/etc variable to track error status > * > @@ -188,7 +191,7 @@ modeset_lock_retry: \ > * to that failure. In both of these cases the code between BEGIN/END will not > * be run, so the failure will reflect the inability to grab the locks. > */ > -#define DRM_MODESET_LOCK_ALL_END(ctx, ret) \ > +#define DRM_MODESET_LOCK_ALL_END(dev, ctx, ret) \ > modeset_lock_fail: \ > if (ret == -EDEADLK) { \ > ret = drm_modeset_backoff(&ctx); \ > @@ -196,6 +199,8 @@ modeset_lock_fail: \ > goto modeset_lock_retry; \ > } \ > drm_modeset_drop_locks(&ctx); \ > - drm_modeset_acquire_fini(&ctx); > + drm_modeset_acquire_fini(&ctx); \ > + if (!drm_drv_uses_atomic_modeset(dev)) \ > + mutex_unlock(&dev->mode_config.mutex); > > #endif /* DRM_MODESET_LOCK_H_ */ > -- > 2.28.0 >
