diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index e901c1e8f77e7e..0e9d5d9222490a 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3089,23 +3089,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, ctx, 0, ret); + 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(ctx, ret); } EXPORT_SYMBOL(drm_atomic_helper_shutdown); @@ -3140,14 +3130,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, ctx, 0, err); state = drm_atomic_helper_duplicate_state(dev, &ctx); if (IS_ERR(state)) @@ -3161,13 +3144,10 @@ struct drm_atomic_state *drm_atomic_helper_suspend(struct drm_device *dev) } unlock: - if (PTR_ERR(state) == -EDEADLK) { - drm_modeset_backoff(&ctx); - goto retry; - } + DRM_MODESET_LOCK_ALL_END(ctx, err); + if (err) + return ERR_PTR(err); - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); return state; } EXPORT_SYMBOL(drm_atomic_helper_suspend); @@ -3241,22 +3221,11 @@ 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, ctx, 0, err); - 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); - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); + DRM_MODESET_LOCK_ALL_END(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 581cc378822309..07dcf47daafe2b 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, ctx, 0, ret); 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_MODESET_LOCK_ALL_END(ctx, ret); return ret; } diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index af4b94ce8e9428..42cdb418164388 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -599,11 +599,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, plane = crtc->primary; mutex_lock(&crtc->dev->mode_config.mutex); - drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE); -retry: - ret = drm_modeset_lock_all_ctx(crtc->dev, &ctx); - if (ret) - goto out; + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, + DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret); if (crtc_req->mode_valid) { /* If we have a mode we need a framebuffer. */ @@ -768,13 +765,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, fb = NULL; mode = NULL; - if (ret == -EDEADLK) { - ret = drm_modeset_backoff(&ctx); - if (!ret) - goto retry; - } - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); + DRM_MODESET_LOCK_ALL_END(ctx, ret); mutex_unlock(&crtc->dev->mode_config.mutex); return ret; diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index 8a5100685875a3..51f534db910707 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -56,6 +56,10 @@ * drm_modeset_drop_locks(ctx); * drm_modeset_acquire_fini(ctx); * + * For convenience this control flow is implemented in + * DRM_MODESET_LOCK_ALL_BEGIN() and DRM_MODESET_LOCK_ALL_END() for the case + * where all modeset locks need to be taken through drm_modeset_lock_all_ctx(). + * * If all that is needed is a single modeset lock, then the &struct * drm_modeset_acquire_ctx is not needed and the locking can be simplified * by passing a NULL instead of ctx in the drm_modeset_lock() call or @@ -383,6 +387,8 @@ EXPORT_SYMBOL(drm_modeset_unlock); * Locks acquired with this function should be released by calling the * drm_modeset_drop_locks() function on @ctx. * + * See also: DRM_MODESET_LOCK_ALL_BEGIN() and DRM_MODESET_LOCK_ALL_END() + * * Returns: 0 on success or a negative error-code on failure. */ int drm_modeset_lock_all_ctx(struct drm_device *dev, diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 1fa98bd120030e..dbac05f0418dd1 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -744,11 +744,8 @@ static int setplane_internal(struct drm_plane *plane, struct drm_modeset_acquire_ctx ctx; int ret; - drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE); -retry: - ret = drm_modeset_lock_all_ctx(plane->dev, &ctx); - if (ret) - goto fail; + DRM_MODESET_LOCK_ALL_BEGIN(plane->dev, ctx, + DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret); if (drm_drv_uses_atomic_modeset(plane->dev)) ret = __setplane_atomic(plane, crtc, fb, @@ -759,14 +756,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); -fail: - if (ret == -EDEADLK) { - ret = drm_modeset_backoff(&ctx); - if (!ret) - goto retry; - } - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); + DRM_MODESET_LOCK_ALL_END(ctx, ret); return ret; } diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h index a685d1bb21f262..a308f2d6496f02 100644 --- a/include/drm/drm_modeset_lock.h +++ b/include/drm/drm_modeset_lock.h @@ -130,4 +130,63 @@ 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 + * @ctx: local modeset acquire context, will be dereferenced + * @flags: DRM_MODESET_ACQUIRE_* flags to pass to drm_modeset_acquire_init() + * @ret: local ret/err/etc variable to track error status + * + * Use these macros to simplify grabbing all modeset locks using a local + * context. This has the advantage of reducing boilerplate, but also properly + * checking return values where appropriate. + * + * Any code run between BEGIN and END will be holding the modeset locks. + * + * This must be paired with DRM_MODESET_LOCK_ALL_END(). We will jump back and + * forth between the labels on deadlock and error conditions. + * + * Drivers can acquire additional modeset locks. If any lock acquisition + * fails, the control flow needs to jump to DRM_MODESET_LOCK_ALL_END() with + * the @ret parameter containing the return value of drm_modeset_lock(). + * + * Returns: + * The only possible value of ret immediately after DRM_MODESET_LOCK_ALL_BEGIN() + * is 0, so no error checking is necessary + */ +#define DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, flags, ret) \ + drm_modeset_acquire_init(&ctx, flags); \ +modeset_lock_retry: \ + ret = drm_modeset_lock_all_ctx(dev, &ctx); \ + if (ret) \ + goto modeset_lock_fail; + +/** + * DRM_MODESET_LOCK_ALL_END - Helper to release and cleanup modeset locks + * @ctx: local modeset acquire context, will be dereferenced + * @ret: local ret/err/etc variable to track error status + * + * The other side of DRM_MODESET_LOCK_ALL_BEGIN(). It will bounce back to BEGIN + * if ret is -EDEADLK. + * + * It's important that you use the same ret variable for begin and end so + * deadlock conditions are properly handled. + * + * Returns: + * ret will be untouched unless it is -EDEADLK on entry. That means that if you + * successfully acquire the locks, ret will be whatever your code sets it to. If + * there is a deadlock or other failure with acquire or backoff, ret will be set + * 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) \ +modeset_lock_fail: \ + if (ret == -EDEADLK) { \ + ret = drm_modeset_backoff(&ctx); \ + if (!ret) \ + goto modeset_lock_retry; \ + } \ + drm_modeset_drop_locks(&ctx); \ + drm_modeset_acquire_fini(&ctx); + #endif /* DRM_MODESET_LOCK_H_ */