Re: [RFC PATCH 04/11] drm: Add __drm_framebuffer_remove_atomic
On Tue, Oct 11, 2016 at 03:54:01PM +0100, Brian Starkey wrote: > Implement the CRTC/Plane disable functionality of drm_framebuffer_remove > using the atomic API, and use it if possible. > > For atomic drivers, this removes the possibility of several commits when > a framebuffer is in use by more than one CRTC/plane. > > Additionally, this will provide a suitable place to support the removal > of a framebuffer from a writeback connector, in the case that a > writeback connector is still actively using a framebuffer when it is > removed by userspace. > > Signed-off-by: Brian StarkeyJust the small comment here: Last time around I wanted toland an atomic disable function for fb remove code it blew up. Need to check out git history to make sure we've addressed those isses. Caveat emperor ;-) -Daniel > --- > drivers/gpu/drm/drm_framebuffer.c | 154 > - > 1 file changed, 152 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_framebuffer.c > b/drivers/gpu/drm/drm_framebuffer.c > index 528f75d..b02cf73 100644 > --- a/drivers/gpu/drm/drm_framebuffer.c > +++ b/drivers/gpu/drm/drm_framebuffer.c > @@ -22,6 +22,7 @@ > > #include > #include > +#include > #include > #include > > @@ -795,6 +796,148 @@ void drm_framebuffer_cleanup(struct drm_framebuffer *fb) > EXPORT_SYMBOL(drm_framebuffer_cleanup); > > /** > + * __drm_framebuffer_remove_atomic - atomic version of > __drm_framebuffer_remove > + * @dev: drm device > + * @fb: framebuffer to remove > + * > + * If the driver implements the atomic API, we can handle the disabling of > all > + * CRTCs/planes which use a framebuffer which is going away in a single > atomic > + * commit. > + * > + * This scans all CRTCs and planes in @dev's mode_config. If they're using > @fb, > + * it is removed and the CRTC/plane disabled. > + * The legacy references are dropped and the ->fb pointers set to NULL > + * accordingly. > + * > + * Returns: > + * true if the framebuffer was successfully removed from use > + */ > +static bool __drm_framebuffer_remove_atomic(struct drm_device *dev, > + struct drm_framebuffer *fb) > +{ > + struct drm_modeset_acquire_ctx ctx; > + struct drm_atomic_state *state; > + struct drm_connector_state *conn_state; > + struct drm_connector *connector; > + struct drm_plane *plane; > + struct drm_crtc *crtc; > + unsigned plane_mask; > + int i, ret; > + > + drm_modeset_acquire_init(, 0); > + > + state = drm_atomic_state_alloc(dev); > + if (!state) > + return false; > + > + state->acquire_ctx = > + > +retry: > + drm_for_each_crtc(crtc, dev) { > + struct drm_plane_state *primary_state; > + struct drm_crtc_state *crtc_state; > + > + primary_state = drm_atomic_get_plane_state(state, > crtc->primary); > + if (IS_ERR(primary_state)) { > + ret = PTR_ERR(primary_state); > + goto fail; > + } > + > + if (primary_state->fb != fb) > + continue; > + > + crtc_state = drm_atomic_get_crtc_state(state, crtc); > + if (IS_ERR(crtc_state)) { > + ret = PTR_ERR(crtc_state); > + goto fail; > + } > + > + /* Only handle the CRTC itself here, handle the plane later */ > + ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL); > + if (ret != 0) > + goto fail; > + > + crtc_state->active = false; > + > + /* Get the connectors in order to disable them */ > + ret = drm_atomic_add_affected_connectors(state, crtc); > + if (ret) > + goto fail; > + } > + > + plane_mask = 0; > + drm_for_each_plane(plane, dev) { > + struct drm_plane_state *plane_state; > + > + plane_state = drm_atomic_get_plane_state(state, plane); > + if (IS_ERR(plane_state)) { > + ret = PTR_ERR(plane_state); > + goto fail; > + } > + > + if (plane_state->fb != fb) > + continue; > + > + plane->old_fb = plane->fb; > + plane_mask |= 1 << drm_plane_index(plane); > + > + /* > + * Open-coded copy of __drm_atomic_helper_disable_plane to avoid > + * a dependency on atomic-helper > + */ > + ret = drm_atomic_set_crtc_for_plane(plane_state, NULL); > + if (ret != 0) > + goto fail; > + > + drm_atomic_set_fb_for_plane(plane_state, NULL); > + plane_state->crtc_x = 0; > + plane_state->crtc_y = 0; > + plane_state->crtc_w = 0; > + plane_state->crtc_h = 0; > + plane_state->src_x = 0; > + plane_state->src_y = 0; > +
[RFC PATCH 04/11] drm: Add __drm_framebuffer_remove_atomic
Implement the CRTC/Plane disable functionality of drm_framebuffer_remove using the atomic API, and use it if possible. For atomic drivers, this removes the possibility of several commits when a framebuffer is in use by more than one CRTC/plane. Additionally, this will provide a suitable place to support the removal of a framebuffer from a writeback connector, in the case that a writeback connector is still actively using a framebuffer when it is removed by userspace. Signed-off-by: Brian Starkey--- drivers/gpu/drm/drm_framebuffer.c | 154 - 1 file changed, 152 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index 528f75d..b02cf73 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -795,6 +796,148 @@ void drm_framebuffer_cleanup(struct drm_framebuffer *fb) EXPORT_SYMBOL(drm_framebuffer_cleanup); /** + * __drm_framebuffer_remove_atomic - atomic version of __drm_framebuffer_remove + * @dev: drm device + * @fb: framebuffer to remove + * + * If the driver implements the atomic API, we can handle the disabling of all + * CRTCs/planes which use a framebuffer which is going away in a single atomic + * commit. + * + * This scans all CRTCs and planes in @dev's mode_config. If they're using @fb, + * it is removed and the CRTC/plane disabled. + * The legacy references are dropped and the ->fb pointers set to NULL + * accordingly. + * + * Returns: + * true if the framebuffer was successfully removed from use + */ +static bool __drm_framebuffer_remove_atomic(struct drm_device *dev, + struct drm_framebuffer *fb) +{ + struct drm_modeset_acquire_ctx ctx; + struct drm_atomic_state *state; + struct drm_connector_state *conn_state; + struct drm_connector *connector; + struct drm_plane *plane; + struct drm_crtc *crtc; + unsigned plane_mask; + int i, ret; + + drm_modeset_acquire_init(, 0); + + state = drm_atomic_state_alloc(dev); + if (!state) + return false; + + state->acquire_ctx = + +retry: + drm_for_each_crtc(crtc, dev) { + struct drm_plane_state *primary_state; + struct drm_crtc_state *crtc_state; + + primary_state = drm_atomic_get_plane_state(state, crtc->primary); + if (IS_ERR(primary_state)) { + ret = PTR_ERR(primary_state); + goto fail; + } + + if (primary_state->fb != fb) + continue; + + crtc_state = drm_atomic_get_crtc_state(state, crtc); + if (IS_ERR(crtc_state)) { + ret = PTR_ERR(crtc_state); + goto fail; + } + + /* Only handle the CRTC itself here, handle the plane later */ + ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL); + if (ret != 0) + goto fail; + + crtc_state->active = false; + + /* Get the connectors in order to disable them */ + ret = drm_atomic_add_affected_connectors(state, crtc); + if (ret) + goto fail; + } + + plane_mask = 0; + drm_for_each_plane(plane, dev) { + struct drm_plane_state *plane_state; + + plane_state = drm_atomic_get_plane_state(state, plane); + if (IS_ERR(plane_state)) { + ret = PTR_ERR(plane_state); + goto fail; + } + + if (plane_state->fb != fb) + continue; + + plane->old_fb = plane->fb; + plane_mask |= 1 << drm_plane_index(plane); + + /* +* Open-coded copy of __drm_atomic_helper_disable_plane to avoid +* a dependency on atomic-helper +*/ + ret = drm_atomic_set_crtc_for_plane(plane_state, NULL); + if (ret != 0) + goto fail; + + drm_atomic_set_fb_for_plane(plane_state, NULL); + plane_state->crtc_x = 0; + plane_state->crtc_y = 0; + plane_state->crtc_w = 0; + plane_state->crtc_h = 0; + plane_state->src_x = 0; + plane_state->src_y = 0; + plane_state->src_w = 0; + plane_state->src_h = 0; + } + + /* All of the connectors in state need disabling */ + for_each_connector_in_state(state, connector, conn_state, i) { + ret = drm_atomic_set_crtc_for_connector(conn_state, + NULL); + if (ret) + goto fail; + } + + if