> > int i; > > + int ret; > > Please move up with i Done! > > + DRM_MODESET_LOCK_ALL_END((&dev_priv->drm), ctx, ret); > > > > return 0; > > Return ret here Done! > > + struct drm_modeset_acquire_ctx ctx; > > int i; > > + int ret; > > Please move up with i Done! > > - drm_modeset_unlock_all(dev); > > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > > > > return 0; > > Return ret Done! > > - drm_modeset_unlock_all(dev); > > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > > > > return 0; > > Return ret Done! > > - drm_modeset_unlock_all(dev); > > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > > > > Check ret here and return an error if it's != 0 Done! > > @@ -1194,14 +1195,11 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data, > > if (ret != 0) > > goto out_unlock; > > > > - drm_modeset_unlock_all(dev); > > - i915_gem_object_put(new_bo); > > - > > - return 0; > > - > > out_unlock: > > - drm_modeset_unlock_all(dev); > > - i915_gem_object_put(new_bo); > > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > > + > > + if (params->flags & I915_OVERLAY_ENABLE) > > + i915_gem_object_put(new_bo); > > This function refactor is a bit more involved than the > s/drm_modeset_lock_all/DRM_MODESET_LOCK_ALL_*/ changes in the rest of the patch. > Could you split it out into a separate patch so it's not hidden away? Sure, no problem.