On Wed, Nov 01, 2017 at 04:04:33PM +0100, Maarten Lankhorst wrote: > This introduces a slight behavioral change to rmfb. Instead of > disabling a crtc when the primary plane is disabled, we try to > preserve it. > > Apart from old versions of the vmwgfx xorg driver, there is > nothing depending on rmfb disabling a crtc. > > Vmwgfx' and simple kms helper atomic implementation rejects CRTC > enabled without plane, so we can do this safely. > > If the atomic commit is rejected by the driver then we will still > fall back to the old behavior and turn off the crtc. > > Changes since v1: > - Restart completely when rmfb with crtc on fails (Sean Paul). > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > Cc: Sean Paul <seanpaul@xxxxxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > drivers/gpu/drm/drm_framebuffer.c | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c > index 2affe53f3fda..f0679468f421 100644 > --- a/drivers/gpu/drm/drm_framebuffer.c > +++ b/drivers/gpu/drm/drm_framebuffer.c > @@ -765,14 +765,18 @@ static int atomic_remove_fb(struct drm_framebuffer *fb) > struct drm_plane *plane; > struct drm_connector *conn; > struct drm_connector_state *conn_state; > - int i, ret = 0; > + int i, ret; > unsigned plane_mask; > + bool disable_crtcs = false; > > - state = drm_atomic_state_alloc(dev); > - if (!state) > - return -ENOMEM; > - > +retry_disable: > drm_modeset_acquire_init(&ctx, 0); > + > + state = drm_atomic_state_alloc(dev); > + if (!state) { > + ret = -ENOMEM; > + goto out; > + } > state->acquire_ctx = &ctx; > > retry: > @@ -793,7 +797,7 @@ static int atomic_remove_fb(struct drm_framebuffer *fb) > goto unlock; > } > > - if (plane_state->crtc->primary == plane) { > + if (disable_crtcs && plane_state->crtc->primary == plane) { > struct drm_crtc_state *crtc_state; > > crtc_state = drm_atomic_get_existing_crtc_state(state, plane_state->crtc); > @@ -818,6 +822,7 @@ static int atomic_remove_fb(struct drm_framebuffer *fb) > plane->old_fb = plane->fb; > } > > + /* This list is only filled when disable_crtcs is set. */ > for_each_new_connector_in_state(state, conn, conn_state, i) { WARN_ON(!disable_crtcs) maybe? > ret = drm_atomic_set_crtc_for_connector(conn_state, NULL); > > @@ -840,9 +845,15 @@ static int atomic_remove_fb(struct drm_framebuffer *fb) > > drm_atomic_state_put(state); > > +out: > drm_modeset_drop_locks(&ctx); > drm_modeset_acquire_fini(&ctx); > > + if (ret == -EINVAL && !disable_crtcs) { Hmm. -EINVAL seems rather specific. Not sure if we could just check for any error? Or... I'm not sure if we have any central place where we do the "can I disable the primary plane w/o disabling the crtc?" check. If we do then we could also add a comment there informing people that the -EINVAL is important. > + disable_crtcs = true; > + goto retry_disable; > + } > + > return ret; > } > > -- > 2.14.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel