drm/atomic-helper: Fix leak in disable_all
The legacy plane->fb pointer is refcounted by calling drm_atomic_clean_old_fb(). In practice this isn't a real problem because: - The caller in the i915 gpu reset code restores the original state again, which means the plane->fb pointer won't change, hence can't leak. - Drivers using drm_atomic_helper_shutdown call the fbdev cleanup first, and that usually cleans up the fb through drm_remove_framebuffer, which does this correctly. - Without fbdev the only framebuffers are from userspace, and those get cleaned up (again using drm_remove_framebuffer) befor the driver can even be unloaded. But in i915 I've switched the cleanup sequence around so that the _shutdown() calls happens after the drm_remove_framebuffer(), which is how I discovered this issue. v2: My analysis why the current code was ok for gpu reset and suspend/resume was correct, but then I totally failed to realize that we better keep this symmetric. Thanksfully CI noticed that for balance, a refcounting bug must exist at 2 places if previously there was no issue ... v3: Don't be lazy and compute the plane_mask in commit_duplicated_state properly too, instead of just using ~0U. Cc: martin.peres@free.fr Cc: chris@chris-wilson.co.uk Acked-by: Dave Airlie <airlied@gmail.com> (v1) Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20170715093106.19873-1-daniel.vetter@ffwll.ch
This commit is contained in:
parent
1a0c19248a
commit
49d70aeaec
|
@ -2526,6 +2526,7 @@ int drm_atomic_helper_disable_all(struct drm_device *dev,
|
||||||
struct drm_plane *plane;
|
struct drm_plane *plane;
|
||||||
struct drm_crtc_state *crtc_state;
|
struct drm_crtc_state *crtc_state;
|
||||||
struct drm_crtc *crtc;
|
struct drm_crtc *crtc;
|
||||||
|
unsigned plane_mask = 0;
|
||||||
int ret, i;
|
int ret, i;
|
||||||
|
|
||||||
state = drm_atomic_state_alloc(dev);
|
state = drm_atomic_state_alloc(dev);
|
||||||
|
@ -2568,10 +2569,14 @@ 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;
|
||||||
}
|
}
|
||||||
|
|
||||||
ret = drm_atomic_commit(state);
|
ret = drm_atomic_commit(state);
|
||||||
free:
|
free:
|
||||||
|
if (plane_mask)
|
||||||
|
drm_atomic_clean_old_fb(dev, plane_mask, ret);
|
||||||
drm_atomic_state_put(state);
|
drm_atomic_state_put(state);
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
@ -2702,11 +2707,16 @@ 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;
|
||||||
|
@ -2714,7 +2724,11 @@ 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;
|
||||||
|
|
||||||
return drm_atomic_commit(state);
|
ret = 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);
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue