Commit 5e9cfeba authored by Ville Syrjälä's avatar Ville Syrjälä

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 49d70aea ("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: default avatarVille 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: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent bee330f3
...@@ -2892,31 +2892,9 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, ...@@ -2892,31 +2892,9 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set,
return 0; return 0;
} }
/** static int __drm_atomic_helper_disable_all(struct drm_device *dev,
* drm_atomic_helper_disable_all - disable all currently active outputs struct drm_modeset_acquire_ctx *ctx,
* @dev: DRM device bool clean_old_fbs)
* @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)
{ {
struct drm_atomic_state *state; struct drm_atomic_state *state;
struct drm_connector_state *conn_state; struct drm_connector_state *conn_state;
...@@ -2968,8 +2946,11 @@ int drm_atomic_helper_disable_all(struct drm_device *dev, ...@@ -2968,8 +2946,11 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
goto free; goto free;
drm_atomic_set_fb_for_plane(plane_state, NULL); 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); ret = drm_atomic_commit(state);
...@@ -2980,6 +2961,34 @@ int drm_atomic_helper_disable_all(struct drm_device *dev, ...@@ -2980,6 +2961,34 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
return ret; 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); EXPORT_SYMBOL(drm_atomic_helper_disable_all);
/** /**
...@@ -3002,7 +3011,7 @@ void drm_atomic_helper_shutdown(struct drm_device *dev) ...@@ -3002,7 +3011,7 @@ void drm_atomic_helper_shutdown(struct drm_device *dev)
while (1) { while (1) {
ret = drm_modeset_lock_all_ctx(dev, &ctx); ret = drm_modeset_lock_all_ctx(dev, &ctx);
if (!ret) if (!ret)
ret = drm_atomic_helper_disable_all(dev, &ctx); ret = __drm_atomic_helper_disable_all(dev, &ctx, true);
if (ret != -EDEADLK) if (ret != -EDEADLK)
break; break;
...@@ -3106,16 +3115,11 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state, ...@@ -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_connector_state *new_conn_state;
struct drm_crtc *crtc; struct drm_crtc *crtc;
struct drm_crtc_state *new_crtc_state; struct drm_crtc_state *new_crtc_state;
unsigned plane_mask = 0;
struct drm_device *dev = state->dev;
int ret;
state->acquire_ctx = ctx; state->acquire_ctx = ctx;
for_each_new_plane_in_state(state, plane, new_plane_state, i) { for_each_new_plane_in_state(state, plane, new_plane_state, i)
plane_mask |= BIT(drm_plane_index(plane));
state->planes[i].old_state = plane->state; state->planes[i].old_state = plane->state;
}
for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
state->crtcs[i].old_state = crtc->state; state->crtcs[i].old_state = crtc->state;
...@@ -3123,11 +3127,7 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *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) for_each_new_connector_in_state(state, connector, new_conn_state, i)
state->connectors[i].old_state = connector->state; state->connectors[i].old_state = connector->state;
ret = drm_atomic_commit(state); return drm_atomic_commit(state);
if (plane_mask)
drm_atomic_clean_old_fb(dev, plane_mask, ret);
return ret;
} }
EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state); EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment