drm/atomic-helper: Drop plane->fb references only for drm_atomic_helper_shutdown()

drm_atomic_helper_shutdown() needs to release the reference held by
plane->fb. Since commit 49d70aeaeca8 ("drm/atomic-helper: Fix leak in
disable_all") we're doing that by calling drm_atomic_clean_old_fb() in
drm_atomic_helper_disable_all(). This also leaves plane->fb == NULL
afterwards. However, since drm_atomic_helper_disable_all() is also
used by the i915 gpu reset code
drm_atomic_helper_commit_duplicated_state() then has to undo the
damage and put the correct plane->fb pointers back in (and also
adjust the ref counts to match again as well).

That approach doesn't work so well for load detection as nothing
sets up the plane->old_fb pointers for us. This causes us to
leak an extra reference for each plane->fb when
drm_atomic_helper_commit_duplicated_state() calls
drm_atomic_clean_old_fb() after load detection.

To fix this let's call drm_atomic_clean_old_fb() only for
drm_atomic_helper_shutdown() as that's the only time we need to
actually drop the plane->fb references. In all the other cases
(load detection, gpu reset) we want to leave plane->fb alone.

v2: Don't inflict the clean_old_fbs bool to drivers (Daniel)
v3: Squash in the revert and rewrite the commit msg (Daniel)

Cc: martin.peres@free.fr
Cc: chris@chris-wilson.co.uk
Cc: Dave Airlie <airlied@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180322152313.6561-3-ville.syrjala@linux.intel.com
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> #pre-squash
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index d63c806e7d38c2dc4dc38b9d485c73dadb2f448a..ef4ddfecc4a4b9a31cd560ed9c3aec830eac25aa 100644 (file)
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2892,31 +2892,9 @@ commit:
        return 0;
 }
 
-/**
- * drm_atomic_helper_disable_all - disable all currently active outputs
- * @dev: DRM device
- * @ctx: lock acquisition context
- *
- * Loops through all connectors, finding those that aren't turned off and then
- * turns them off by setting their DPMS mode to OFF and deactivating the CRTC
- * that they are connected to.
- *
- * This is used for example in suspend/resume to disable all currently active
- * functions when suspending. If you just want to shut down everything at e.g.
- * driver unload, look at drm_atomic_helper_shutdown().
- *
- * Note that if callers haven't already acquired all modeset locks this might
- * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
- *
- * Returns:
- * 0 on success or a negative error code on failure.
- *
- * See also:
- * drm_atomic_helper_suspend(), drm_atomic_helper_resume() and
- * drm_atomic_helper_shutdown().
- */
-int drm_atomic_helper_disable_all(struct drm_device *dev,
-                                 struct drm_modeset_acquire_ctx *ctx)
+static int __drm_atomic_helper_disable_all(struct drm_device *dev,
+                                          struct drm_modeset_acquire_ctx *ctx,
+                                          bool clean_old_fbs)
 {
        struct drm_atomic_state *state;
        struct drm_connector_state *conn_state;
@@ -2968,8 +2946,11 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
                        goto free;
 
                drm_atomic_set_fb_for_plane(plane_state, NULL);
-               plane_mask |= BIT(drm_plane_index(plane));
-               plane->old_fb = plane->fb;
+
+               if (clean_old_fbs) {
+                       plane->old_fb = plane->fb;
+                       plane_mask |= BIT(drm_plane_index(plane));
+               }
        }
 
        ret = drm_atomic_commit(state);
@@ -2980,6 +2961,34 @@ free:
        return ret;
 }
 
+/**
+ * drm_atomic_helper_disable_all - disable all currently active outputs
+ * @dev: DRM device
+ * @ctx: lock acquisition context
+ *
+ * Loops through all connectors, finding those that aren't turned off and then
+ * turns them off by setting their DPMS mode to OFF and deactivating the CRTC
+ * that they are connected to.
+ *
+ * This is used for example in suspend/resume to disable all currently active
+ * functions when suspending. If you just want to shut down everything at e.g.
+ * driver unload, look at drm_atomic_helper_shutdown().
+ *
+ * Note that if callers haven't already acquired all modeset locks this might
+ * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ *
+ * See also:
+ * drm_atomic_helper_suspend(), drm_atomic_helper_resume() and
+ * drm_atomic_helper_shutdown().
+ */
+int drm_atomic_helper_disable_all(struct drm_device *dev,
+                                 struct drm_modeset_acquire_ctx *ctx)
+{
+       return __drm_atomic_helper_disable_all(dev, ctx, false);
+}
 EXPORT_SYMBOL(drm_atomic_helper_disable_all);
 
 /**
@@ -3002,7 +3011,7 @@ void drm_atomic_helper_shutdown(struct drm_device *dev)
        while (1) {
                ret = drm_modeset_lock_all_ctx(dev, &ctx);
                if (!ret)
-                       ret = drm_atomic_helper_disable_all(dev, &ctx);
+                       ret = __drm_atomic_helper_disable_all(dev, &ctx, true);
 
                if (ret != -EDEADLK)
                        break;
@@ -3106,16 +3115,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
        struct drm_connector_state *new_conn_state;
        struct drm_crtc *crtc;
        struct drm_crtc_state *new_crtc_state;
-       unsigned plane_mask = 0;
-       struct drm_device *dev = state->dev;
-       int ret;
 
        state->acquire_ctx = ctx;
 
-       for_each_new_plane_in_state(state, plane, new_plane_state, i) {
-               plane_mask |= BIT(drm_plane_index(plane));
+       for_each_new_plane_in_state(state, plane, new_plane_state, i)
                state->planes[i].old_state = plane->state;
-       }
 
        for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
                state->crtcs[i].old_state = crtc->state;
@@ -3123,11 +3127,7 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state,
        for_each_new_connector_in_state(state, connector, new_conn_state, i)
                state->connectors[i].old_state = connector->state;
 
-       ret = drm_atomic_commit(state);
-       if (plane_mask)
-               drm_atomic_clean_old_fb(dev, plane_mask, ret);
-
-       return ret;
+       return drm_atomic_commit(state);
 }
 EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state);