| Message ID | 1454334242-23354-2-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 01, 2016 at 02:43:57PM +0100, Maarten Lankhorst wrote: > Instead of restoring dpms and a flag for whether a temp fb is allocated duplicate > the old plane_state and crtc_state, and restore the members we potentially touched. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 128 ++++++++++++++++++++--------------- > drivers/gpu/drm/i915/intel_drv.h | 4 +- > 2 files changed, 76 insertions(+), 56 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4d8c9f7857db..0702ce8ec36a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10361,6 +10361,7 @@ mode_fits_in_fbdev(struct drm_device *dev, > if (obj->base.size < mode->vdisplay * fb->pitches[0]) > return NULL; > > + drm_framebuffer_reference(fb); > return fb; > #else > return NULL; > @@ -10426,6 +10427,9 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector, > encoder->base.id, encoder->name); > > retry: > + old->old_pipe_config = NULL; > + old->old_plane_state = NULL; > + > ret = drm_modeset_lock(&config->connection_mutex, ctx); > if (ret) > goto fail; > @@ -10441,24 +10445,15 @@ retry: > */ > > /* See if we already have a CRTC for this connector */ > - if (encoder->crtc) { > - crtc = encoder->crtc; > + if (connector->state->crtc) { > + crtc = connector->state->crtc; > > ret = drm_modeset_lock(&crtc->mutex, ctx); > if (ret) > goto fail; > - ret = drm_modeset_lock(&crtc->primary->mutex, ctx); > - if (ret) > - goto fail; > - > - old->dpms_mode = connector->dpms; > - old->load_detect_temp = false; > > /* Make sure the crtc and connector are running */ > - if (connector->dpms != DRM_MODE_DPMS_ON) > - connector->funcs->dpms(connector, DRM_MODE_DPMS_ON); > - > - return true; > + goto found; > } > > /* Find an unused one (if possible) */ > @@ -10466,8 +10461,15 @@ retry: > i++; > if (!(encoder->possible_crtcs & (1 << i))) > continue; > - if (possible_crtc->state->enable) > + > + ret = drm_modeset_lock(&possible_crtc->mutex, ctx); > + if (ret) > + goto fail; > + > + if (possible_crtc->state->enable) { > + drm_modeset_unlock(&possible_crtc->mutex); > continue; > + } > > crtc = possible_crtc; > break; > @@ -10481,17 +10483,19 @@ retry: > goto fail; > } > > - ret = drm_modeset_lock(&crtc->mutex, ctx); > - if (ret) > - goto fail; > +found: > + intel_crtc = to_intel_crtc(crtc); > + > ret = drm_modeset_lock(&crtc->primary->mutex, ctx); > if (ret) > goto fail; > > - intel_crtc = to_intel_crtc(crtc); > - old->dpms_mode = connector->dpms; > - old->load_detect_temp = true; > - old->release_fb = NULL; > + old->old_pipe_config = intel_crtc_duplicate_state(crtc); > + old->old_plane_state = intel_plane_duplicate_state(crtc->primary); > + if (!old->old_pipe_config || !old->old_plane_state) { > + ret = -ENOMEM; > + goto fail; > + } > > state = drm_atomic_state_alloc(dev); > if (!state) > @@ -10505,7 +10509,9 @@ retry: > goto fail; > } > > - connector_state->crtc = crtc; > + ret = drm_atomic_set_crtc_for_connector(connector_state, crtc); > + if (ret) > + goto fail; > > crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > if (IS_ERR(crtc_state)) { > @@ -10529,7 +10535,6 @@ retry: > if (fb == NULL) { > DRM_DEBUG_KMS("creating tmp fb for load-detection\n"); > fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32); > - old->release_fb = fb; > } else > DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n"); > if (IS_ERR(fb)) { > @@ -10541,15 +10546,16 @@ retry: > if (ret) > goto fail; > > - drm_mode_copy(&crtc_state->base.mode, mode); > + drm_framebuffer_unreference(fb); > + > + ret = drm_atomic_set_mode_for_crtc(&crtc_state->base, mode); > + if (ret) > + goto fail; > > if (drm_atomic_commit(state)) { > DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n"); > - if (old->release_fb) > - old->release_fb->funcs->destroy(old->release_fb); > goto fail; > } > - crtc->primary->crtc = crtc; > > /* let the connector get through one full cycle before testing */ > intel_wait_for_vblank(dev, intel_crtc->pipe); > @@ -10559,6 +10565,12 @@ fail: > drm_atomic_state_free(state); > state = NULL; > > + if (old->old_pipe_config) > + intel_crtc_destroy_state(crtc, old->old_pipe_config); > + > + if (old->old_plane_state) > + intel_plane_destroy_state(crtc->primary, old->old_plane_state); > + > if (ret == -EDEADLK) { > drm_modeset_backoff(ctx); > goto retry; > @@ -10575,61 +10587,69 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, > struct intel_encoder *intel_encoder = > intel_attached_encoder(connector); > struct drm_encoder *encoder = &intel_encoder->base; > - struct drm_crtc *crtc = encoder->crtc; > + struct drm_crtc *crtc = connector->state->crtc; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct drm_atomic_state *state; > struct drm_connector_state *connector_state; > struct intel_crtc_state *crtc_state; > + struct drm_plane_state *plane_state; > int ret; > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", > connector->base.id, connector->name, > encoder->base.id, encoder->name); > > - if (old->load_detect_temp) { > - state = drm_atomic_state_alloc(dev); > - if (!state) > - goto fail; > + if (!old->old_pipe_config) > + return; > > - state->acquire_ctx = ctx; > + state = drm_atomic_state_alloc(dev); > + if (!state) > + goto fail; > > - connector_state = drm_atomic_get_connector_state(state, connector); > - if (IS_ERR(connector_state)) > - goto fail; > + state->acquire_ctx = ctx; > > - crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > - if (IS_ERR(crtc_state)) > - goto fail; > + connector_state = drm_atomic_get_connector_state(state, connector); > + if (IS_ERR(connector_state)) > + goto fail; > > - connector_state->crtc = NULL; > + crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > + if (IS_ERR(crtc_state)) > + goto fail; > > - crtc_state->base.enable = crtc_state->base.active = false; > + plane_state = drm_atomic_get_plane_state(state, crtc->primary); > + if (IS_ERR(plane_state)) > + goto fail; > > - ret = intel_modeset_setup_plane_state(state, crtc, NULL, NULL, > - 0, 0); > + if (!old->old_pipe_config->enable) { > + ret = drm_atomic_set_crtc_for_connector(connector_state, NULL); > if (ret) > goto fail; > + } > > - ret = drm_atomic_commit(state); > - if (ret) > - goto fail; > + crtc_state->base.enable = old->old_pipe_config->enable; > + crtc_state->base.active = old->old_pipe_config->active; > > - if (old->release_fb) { > - drm_framebuffer_unregister_private(old->release_fb); > - drm_framebuffer_unreference(old->release_fb); > - } > + drm_atomic_set_fb_for_plane(plane_state, old->old_plane_state->fb); > + ret = drm_atomic_set_crtc_for_plane(plane_state, old->old_plane_state->crtc); > + if (ret) > + goto fail; > > - return; > - } > + old->old_plane_state->state = plane_state->state; > + *plane_state = *old->old_plane_state; This part looks messy. I don't think there's any reason anyone interested in load detection should need to do these sort of gymnastics manually. So there really should be some kind of nice thing to just tell atomic that: "i have this old plane/crtc/connector/etc. state here, please just restore it" > > - /* Switch crtc and encoder back off if necessary */ > - if (old->dpms_mode != DRM_MODE_DPMS_ON) > - connector->funcs->dpms(connector, old->dpms_mode); > + ret = drm_atomic_commit(state); > + if (ret) > + goto fail; > > + intel_plane_destroy_state(crtc->primary, old->old_plane_state); > + intel_crtc_destroy_state(crtc, old->old_pipe_config); > return; > + > fail: > DRM_DEBUG_KMS("Couldn't release load detect pipe.\n"); > drm_atomic_state_free(state); > + intel_plane_destroy_state(crtc->primary, old->old_plane_state); > + intel_crtc_destroy_state(crtc, old->old_pipe_config); > } > > static int i9xx_pll_refclk(struct drm_device *dev, > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 93ba14a3bb76..eeda7d02a4f7 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -913,8 +913,8 @@ struct intel_unpin_work { > > struct intel_load_detect_pipe { > struct drm_framebuffer *release_fb; > - bool load_detect_temp; > - int dpms_mode; > + struct drm_crtc_state *old_pipe_config; > + struct drm_plane_state *old_plane_state; > }; > > static inline struct intel_encoder * > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Feb 01, 2016 at 02:43:57PM +0100, Maarten Lankhorst wrote: > Instead of restoring dpms and a flag for whether a temp fb is allocated duplicate > the old plane_state and crtc_state, and restore the members we potentially touched. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 128 ++++++++++++++++++++--------------- > drivers/gpu/drm/i915/intel_drv.h | 4 +- > 2 files changed, 76 insertions(+), 56 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4d8c9f7857db..0702ce8ec36a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10361,6 +10361,7 @@ mode_fits_in_fbdev(struct drm_device *dev, > if (obj->base.size < mode->vdisplay * fb->pitches[0]) > return NULL; > > + drm_framebuffer_reference(fb); > return fb; > #else > return NULL; > @@ -10426,6 +10427,9 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector, > encoder->base.id, encoder->name); > > retry: > + old->old_pipe_config = NULL; > + old->old_plane_state = NULL; > + > ret = drm_modeset_lock(&config->connection_mutex, ctx); > if (ret) > goto fail; > @@ -10441,24 +10445,15 @@ retry: > */ > > /* See if we already have a CRTC for this connector */ > - if (encoder->crtc) { > - crtc = encoder->crtc; > + if (connector->state->crtc) { All these connector->state accesses made me a bit uneasy, but we did indeed grab connection_mutex already so it should be fine. It's even more troubling seeing connector->state accessed outside intel_get_load_detect_pipe() in the later patches, but it seems it's only done if intel_get_load_detect_pipe() succeeded which means we should be holding the right lock. Dunno, maybe there should be some comments explaining this stuff. Or maybe maybe we should have a helper to return the current connector state that also asserts that the right lock is held?
Op 01-02-16 om 17:08 schreef Ville Syrjälä: > On Mon, Feb 01, 2016 at 02:43:57PM +0100, Maarten Lankhorst wrote: >> Instead of restoring dpms and a flag for whether a temp fb is allocated duplicate >> the old plane_state and crtc_state, and restore the members we potentially touched. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 128 ++++++++++++++++++++--------------- >> drivers/gpu/drm/i915/intel_drv.h | 4 +- >> 2 files changed, 76 insertions(+), 56 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 4d8c9f7857db..0702ce8ec36a 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -10361,6 +10361,7 @@ mode_fits_in_fbdev(struct drm_device *dev, >> if (obj->base.size < mode->vdisplay * fb->pitches[0]) >> return NULL; >> >> + drm_framebuffer_reference(fb); >> return fb; >> #else >> return NULL; >> @@ -10426,6 +10427,9 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector, >> encoder->base.id, encoder->name); >> >> retry: >> + old->old_pipe_config = NULL; >> + old->old_plane_state = NULL; >> + >> ret = drm_modeset_lock(&config->connection_mutex, ctx); >> if (ret) >> goto fail; >> @@ -10441,24 +10445,15 @@ retry: >> */ >> >> /* See if we already have a CRTC for this connector */ >> - if (encoder->crtc) { >> - crtc = encoder->crtc; >> + if (connector->state->crtc) { >> + crtc = connector->state->crtc; >> >> ret = drm_modeset_lock(&crtc->mutex, ctx); >> if (ret) >> goto fail; >> - ret = drm_modeset_lock(&crtc->primary->mutex, ctx); >> - if (ret) >> - goto fail; >> - >> - old->dpms_mode = connector->dpms; >> - old->load_detect_temp = false; >> >> /* Make sure the crtc and connector are running */ >> - if (connector->dpms != DRM_MODE_DPMS_ON) >> - connector->funcs->dpms(connector, DRM_MODE_DPMS_ON); >> - >> - return true; >> + goto found; >> } >> >> /* Find an unused one (if possible) */ >> @@ -10466,8 +10461,15 @@ retry: >> i++; >> if (!(encoder->possible_crtcs & (1 << i))) >> continue; >> - if (possible_crtc->state->enable) >> + >> + ret = drm_modeset_lock(&possible_crtc->mutex, ctx); >> + if (ret) >> + goto fail; >> + >> + if (possible_crtc->state->enable) { >> + drm_modeset_unlock(&possible_crtc->mutex); >> continue; >> + } >> >> crtc = possible_crtc; >> break; >> @@ -10481,17 +10483,19 @@ retry: >> goto fail; >> } >> >> - ret = drm_modeset_lock(&crtc->mutex, ctx); >> - if (ret) >> - goto fail; >> +found: >> + intel_crtc = to_intel_crtc(crtc); >> + >> ret = drm_modeset_lock(&crtc->primary->mutex, ctx); >> if (ret) >> goto fail; >> >> - intel_crtc = to_intel_crtc(crtc); >> - old->dpms_mode = connector->dpms; >> - old->load_detect_temp = true; >> - old->release_fb = NULL; >> + old->old_pipe_config = intel_crtc_duplicate_state(crtc); >> + old->old_plane_state = intel_plane_duplicate_state(crtc->primary); >> + if (!old->old_pipe_config || !old->old_plane_state) { >> + ret = -ENOMEM; >> + goto fail; >> + } >> >> state = drm_atomic_state_alloc(dev); >> if (!state) >> @@ -10505,7 +10509,9 @@ retry: >> goto fail; >> } >> >> - connector_state->crtc = crtc; >> + ret = drm_atomic_set_crtc_for_connector(connector_state, crtc); >> + if (ret) >> + goto fail; >> >> crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); >> if (IS_ERR(crtc_state)) { >> @@ -10529,7 +10535,6 @@ retry: >> if (fb == NULL) { >> DRM_DEBUG_KMS("creating tmp fb for load-detection\n"); >> fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32); >> - old->release_fb = fb; >> } else >> DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n"); >> if (IS_ERR(fb)) { >> @@ -10541,15 +10546,16 @@ retry: >> if (ret) >> goto fail; >> >> - drm_mode_copy(&crtc_state->base.mode, mode); >> + drm_framebuffer_unreference(fb); >> + >> + ret = drm_atomic_set_mode_for_crtc(&crtc_state->base, mode); >> + if (ret) >> + goto fail; >> >> if (drm_atomic_commit(state)) { >> DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n"); >> - if (old->release_fb) >> - old->release_fb->funcs->destroy(old->release_fb); >> goto fail; >> } >> - crtc->primary->crtc = crtc; >> >> /* let the connector get through one full cycle before testing */ >> intel_wait_for_vblank(dev, intel_crtc->pipe); >> @@ -10559,6 +10565,12 @@ fail: >> drm_atomic_state_free(state); >> state = NULL; >> >> + if (old->old_pipe_config) >> + intel_crtc_destroy_state(crtc, old->old_pipe_config); >> + >> + if (old->old_plane_state) >> + intel_plane_destroy_state(crtc->primary, old->old_plane_state); >> + >> if (ret == -EDEADLK) { >> drm_modeset_backoff(ctx); >> goto retry; >> @@ -10575,61 +10587,69 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, >> struct intel_encoder *intel_encoder = >> intel_attached_encoder(connector); >> struct drm_encoder *encoder = &intel_encoder->base; >> - struct drm_crtc *crtc = encoder->crtc; >> + struct drm_crtc *crtc = connector->state->crtc; >> struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> struct drm_atomic_state *state; >> struct drm_connector_state *connector_state; >> struct intel_crtc_state *crtc_state; >> + struct drm_plane_state *plane_state; >> int ret; >> >> DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", >> connector->base.id, connector->name, >> encoder->base.id, encoder->name); >> >> - if (old->load_detect_temp) { >> - state = drm_atomic_state_alloc(dev); >> - if (!state) >> - goto fail; >> + if (!old->old_pipe_config) >> + return; >> >> - state->acquire_ctx = ctx; >> + state = drm_atomic_state_alloc(dev); >> + if (!state) >> + goto fail; >> >> - connector_state = drm_atomic_get_connector_state(state, connector); >> - if (IS_ERR(connector_state)) >> - goto fail; >> + state->acquire_ctx = ctx; >> >> - crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); >> - if (IS_ERR(crtc_state)) >> - goto fail; >> + connector_state = drm_atomic_get_connector_state(state, connector); >> + if (IS_ERR(connector_state)) >> + goto fail; >> >> - connector_state->crtc = NULL; >> + crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); >> + if (IS_ERR(crtc_state)) >> + goto fail; >> >> - crtc_state->base.enable = crtc_state->base.active = false; >> + plane_state = drm_atomic_get_plane_state(state, crtc->primary); >> + if (IS_ERR(plane_state)) >> + goto fail; >> >> - ret = intel_modeset_setup_plane_state(state, crtc, NULL, NULL, >> - 0, 0); >> + if (!old->old_pipe_config->enable) { >> + ret = drm_atomic_set_crtc_for_connector(connector_state, NULL); >> if (ret) >> goto fail; >> + } >> >> - ret = drm_atomic_commit(state); >> - if (ret) >> - goto fail; >> + crtc_state->base.enable = old->old_pipe_config->enable; >> + crtc_state->base.active = old->old_pipe_config->active; >> >> - if (old->release_fb) { >> - drm_framebuffer_unregister_private(old->release_fb); >> - drm_framebuffer_unreference(old->release_fb); >> - } >> + drm_atomic_set_fb_for_plane(plane_state, old->old_plane_state->fb); >> + ret = drm_atomic_set_crtc_for_plane(plane_state, old->old_plane_state->crtc); >> + if (ret) >> + goto fail; >> >> - return; >> - } >> + old->old_plane_state->state = plane_state->state; >> + *plane_state = *old->old_plane_state; > This part looks messy. I don't think there's any reason anyone > interested in load detection should need to do these sort of > gymnastics manually. So there really should be some kind of nice > thing to just tell atomic that: > "i have this old plane/crtc/connector/etc. state here, please > just restore it" We could create 2 atomic states, update the first one, and commit the second one in release_load_detect_pipe. ~Maarten
Op 01-02-16 om 18:09 schreef Ville Syrjälä: > On Mon, Feb 01, 2016 at 02:43:57PM +0100, Maarten Lankhorst wrote: >> Instead of restoring dpms and a flag for whether a temp fb is allocated duplicate >> the old plane_state and crtc_state, and restore the members we potentially touched. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 128 ++++++++++++++++++++--------------- >> drivers/gpu/drm/i915/intel_drv.h | 4 +- >> 2 files changed, 76 insertions(+), 56 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 4d8c9f7857db..0702ce8ec36a 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -10361,6 +10361,7 @@ mode_fits_in_fbdev(struct drm_device *dev, >> if (obj->base.size < mode->vdisplay * fb->pitches[0]) >> return NULL; >> >> + drm_framebuffer_reference(fb); >> return fb; >> #else >> return NULL; >> @@ -10426,6 +10427,9 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector, >> encoder->base.id, encoder->name); >> >> retry: >> + old->old_pipe_config = NULL; >> + old->old_plane_state = NULL; >> + >> ret = drm_modeset_lock(&config->connection_mutex, ctx); >> if (ret) >> goto fail; >> @@ -10441,24 +10445,15 @@ retry: >> */ >> >> /* See if we already have a CRTC for this connector */ >> - if (encoder->crtc) { >> - crtc = encoder->crtc; >> + if (connector->state->crtc) { > All these connector->state accesses made me a bit uneasy, but we did > indeed grab connection_mutex already so it should be fine. It's even > more troubling seeing connector->state accessed outside > intel_get_load_detect_pipe() in the later patches, but it seems it's > only done if intel_get_load_detect_pipe() succeeded which means we > should be holding the right lock. > > Dunno, maybe there should be some comments explaining this stuff. > Or maybe maybe we should have a helper to return the current > connector state that also asserts that the right lock is held? I'm not sure how to proceed on that, I do think all accesses should be cleaned up, but I'm not sure yet what the path forward is, and so I want to leave it for a different series.
On Mon, Feb 01, 2016 at 02:43:57PM +0100, Maarten Lankhorst wrote: > Instead of restoring dpms and a flag for whether a temp fb is allocated duplicate > the old plane_state and crtc_state, and restore the members we potentially touched. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Do we have the load-detect igt testcase now in the BAT set? We've broken this way too many times to not do that. Imo that should be done before merging this fix. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 128 ++++++++++++++++++++--------------- > drivers/gpu/drm/i915/intel_drv.h | 4 +- > 2 files changed, 76 insertions(+), 56 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4d8c9f7857db..0702ce8ec36a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10361,6 +10361,7 @@ mode_fits_in_fbdev(struct drm_device *dev, > if (obj->base.size < mode->vdisplay * fb->pitches[0]) > return NULL; > > + drm_framebuffer_reference(fb); > return fb; > #else > return NULL; > @@ -10426,6 +10427,9 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector, > encoder->base.id, encoder->name); > > retry: > + old->old_pipe_config = NULL; > + old->old_plane_state = NULL; > + > ret = drm_modeset_lock(&config->connection_mutex, ctx); > if (ret) > goto fail; > @@ -10441,24 +10445,15 @@ retry: > */ > > /* See if we already have a CRTC for this connector */ > - if (encoder->crtc) { > - crtc = encoder->crtc; > + if (connector->state->crtc) { > + crtc = connector->state->crtc; > > ret = drm_modeset_lock(&crtc->mutex, ctx); > if (ret) > goto fail; > - ret = drm_modeset_lock(&crtc->primary->mutex, ctx); > - if (ret) > - goto fail; > - > - old->dpms_mode = connector->dpms; > - old->load_detect_temp = false; > > /* Make sure the crtc and connector are running */ > - if (connector->dpms != DRM_MODE_DPMS_ON) > - connector->funcs->dpms(connector, DRM_MODE_DPMS_ON); > - > - return true; > + goto found; > } > > /* Find an unused one (if possible) */ > @@ -10466,8 +10461,15 @@ retry: > i++; > if (!(encoder->possible_crtcs & (1 << i))) > continue; > - if (possible_crtc->state->enable) > + > + ret = drm_modeset_lock(&possible_crtc->mutex, ctx); > + if (ret) > + goto fail; > + > + if (possible_crtc->state->enable) { > + drm_modeset_unlock(&possible_crtc->mutex); > continue; > + } > > crtc = possible_crtc; > break; > @@ -10481,17 +10483,19 @@ retry: > goto fail; > } > > - ret = drm_modeset_lock(&crtc->mutex, ctx); > - if (ret) > - goto fail; > +found: > + intel_crtc = to_intel_crtc(crtc); > + > ret = drm_modeset_lock(&crtc->primary->mutex, ctx); > if (ret) > goto fail; > > - intel_crtc = to_intel_crtc(crtc); > - old->dpms_mode = connector->dpms; > - old->load_detect_temp = true; > - old->release_fb = NULL; > + old->old_pipe_config = intel_crtc_duplicate_state(crtc); > + old->old_plane_state = intel_plane_duplicate_state(crtc->primary); > + if (!old->old_pipe_config || !old->old_plane_state) { > + ret = -ENOMEM; > + goto fail; > + } > > state = drm_atomic_state_alloc(dev); > if (!state) > @@ -10505,7 +10509,9 @@ retry: > goto fail; > } > > - connector_state->crtc = crtc; > + ret = drm_atomic_set_crtc_for_connector(connector_state, crtc); > + if (ret) > + goto fail; > > crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > if (IS_ERR(crtc_state)) { > @@ -10529,7 +10535,6 @@ retry: > if (fb == NULL) { > DRM_DEBUG_KMS("creating tmp fb for load-detection\n"); > fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32); > - old->release_fb = fb; > } else > DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n"); > if (IS_ERR(fb)) { > @@ -10541,15 +10546,16 @@ retry: > if (ret) > goto fail; > > - drm_mode_copy(&crtc_state->base.mode, mode); > + drm_framebuffer_unreference(fb); > + > + ret = drm_atomic_set_mode_for_crtc(&crtc_state->base, mode); > + if (ret) > + goto fail; > > if (drm_atomic_commit(state)) { > DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n"); > - if (old->release_fb) > - old->release_fb->funcs->destroy(old->release_fb); > goto fail; > } > - crtc->primary->crtc = crtc; > > /* let the connector get through one full cycle before testing */ > intel_wait_for_vblank(dev, intel_crtc->pipe); > @@ -10559,6 +10565,12 @@ fail: > drm_atomic_state_free(state); > state = NULL; > > + if (old->old_pipe_config) > + intel_crtc_destroy_state(crtc, old->old_pipe_config); > + > + if (old->old_plane_state) > + intel_plane_destroy_state(crtc->primary, old->old_plane_state); > + > if (ret == -EDEADLK) { > drm_modeset_backoff(ctx); > goto retry; > @@ -10575,61 +10587,69 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, > struct intel_encoder *intel_encoder = > intel_attached_encoder(connector); > struct drm_encoder *encoder = &intel_encoder->base; > - struct drm_crtc *crtc = encoder->crtc; > + struct drm_crtc *crtc = connector->state->crtc; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct drm_atomic_state *state; > struct drm_connector_state *connector_state; > struct intel_crtc_state *crtc_state; > + struct drm_plane_state *plane_state; > int ret; > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", > connector->base.id, connector->name, > encoder->base.id, encoder->name); > > - if (old->load_detect_temp) { > - state = drm_atomic_state_alloc(dev); > - if (!state) > - goto fail; > + if (!old->old_pipe_config) > + return; > > - state->acquire_ctx = ctx; > + state = drm_atomic_state_alloc(dev); > + if (!state) > + goto fail; > > - connector_state = drm_atomic_get_connector_state(state, connector); > - if (IS_ERR(connector_state)) > - goto fail; > + state->acquire_ctx = ctx; > > - crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > - if (IS_ERR(crtc_state)) > - goto fail; > + connector_state = drm_atomic_get_connector_state(state, connector); > + if (IS_ERR(connector_state)) > + goto fail; > > - connector_state->crtc = NULL; > + crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > + if (IS_ERR(crtc_state)) > + goto fail; > > - crtc_state->base.enable = crtc_state->base.active = false; > + plane_state = drm_atomic_get_plane_state(state, crtc->primary); > + if (IS_ERR(plane_state)) > + goto fail; > > - ret = intel_modeset_setup_plane_state(state, crtc, NULL, NULL, > - 0, 0); > + if (!old->old_pipe_config->enable) { > + ret = drm_atomic_set_crtc_for_connector(connector_state, NULL); > if (ret) > goto fail; > + } > > - ret = drm_atomic_commit(state); > - if (ret) > - goto fail; > + crtc_state->base.enable = old->old_pipe_config->enable; > + crtc_state->base.active = old->old_pipe_config->active; > > - if (old->release_fb) { > - drm_framebuffer_unregister_private(old->release_fb); > - drm_framebuffer_unreference(old->release_fb); > - } > + drm_atomic_set_fb_for_plane(plane_state, old->old_plane_state->fb); > + ret = drm_atomic_set_crtc_for_plane(plane_state, old->old_plane_state->crtc); > + if (ret) > + goto fail; > > - return; > - } > + old->old_plane_state->state = plane_state->state; > + *plane_state = *old->old_plane_state; > > - /* Switch crtc and encoder back off if necessary */ > - if (old->dpms_mode != DRM_MODE_DPMS_ON) > - connector->funcs->dpms(connector, old->dpms_mode); > + ret = drm_atomic_commit(state); > + if (ret) > + goto fail; > > + intel_plane_destroy_state(crtc->primary, old->old_plane_state); > + intel_crtc_destroy_state(crtc, old->old_pipe_config); > return; > + > fail: > DRM_DEBUG_KMS("Couldn't release load detect pipe.\n"); > drm_atomic_state_free(state); > + intel_plane_destroy_state(crtc->primary, old->old_plane_state); > + intel_crtc_destroy_state(crtc, old->old_pipe_config); > } > > static int i9xx_pll_refclk(struct drm_device *dev, > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 93ba14a3bb76..eeda7d02a4f7 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -913,8 +913,8 @@ struct intel_unpin_work { > > struct intel_load_detect_pipe { > struct drm_framebuffer *release_fb; > - bool load_detect_temp; > - int dpms_mode; > + struct drm_crtc_state *old_pipe_config; > + struct drm_plane_state *old_plane_state; > }; > > static inline struct intel_encoder * > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4d8c9f7857db..0702ce8ec36a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10361,6 +10361,7 @@ mode_fits_in_fbdev(struct drm_device *dev, if (obj->base.size < mode->vdisplay * fb->pitches[0]) return NULL; + drm_framebuffer_reference(fb); return fb; #else return NULL; @@ -10426,6 +10427,9 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector, encoder->base.id, encoder->name); retry: + old->old_pipe_config = NULL; + old->old_plane_state = NULL; + ret = drm_modeset_lock(&config->connection_mutex, ctx); if (ret) goto fail; @@ -10441,24 +10445,15 @@ retry: */ /* See if we already have a CRTC for this connector */ - if (encoder->crtc) { - crtc = encoder->crtc; + if (connector->state->crtc) { + crtc = connector->state->crtc; ret = drm_modeset_lock(&crtc->mutex, ctx); if (ret) goto fail; - ret = drm_modeset_lock(&crtc->primary->mutex, ctx); - if (ret) - goto fail; - - old->dpms_mode = connector->dpms; - old->load_detect_temp = false; /* Make sure the crtc and connector are running */ - if (connector->dpms != DRM_MODE_DPMS_ON) - connector->funcs->dpms(connector, DRM_MODE_DPMS_ON); - - return true; + goto found; } /* Find an unused one (if possible) */ @@ -10466,8 +10461,15 @@ retry: i++; if (!(encoder->possible_crtcs & (1 << i))) continue; - if (possible_crtc->state->enable) + + ret = drm_modeset_lock(&possible_crtc->mutex, ctx); + if (ret) + goto fail; + + if (possible_crtc->state->enable) { + drm_modeset_unlock(&possible_crtc->mutex); continue; + } crtc = possible_crtc; break; @@ -10481,17 +10483,19 @@ retry: goto fail; } - ret = drm_modeset_lock(&crtc->mutex, ctx); - if (ret) - goto fail; +found: + intel_crtc = to_intel_crtc(crtc); + ret = drm_modeset_lock(&crtc->primary->mutex, ctx); if (ret) goto fail; - intel_crtc = to_intel_crtc(crtc); - old->dpms_mode = connector->dpms; - old->load_detect_temp = true; - old->release_fb = NULL; + old->old_pipe_config = intel_crtc_duplicate_state(crtc); + old->old_plane_state = intel_plane_duplicate_state(crtc->primary); + if (!old->old_pipe_config || !old->old_plane_state) { + ret = -ENOMEM; + goto fail; + } state = drm_atomic_state_alloc(dev); if (!state) @@ -10505,7 +10509,9 @@ retry: goto fail; } - connector_state->crtc = crtc; + ret = drm_atomic_set_crtc_for_connector(connector_state, crtc); + if (ret) + goto fail; crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); if (IS_ERR(crtc_state)) { @@ -10529,7 +10535,6 @@ retry: if (fb == NULL) { DRM_DEBUG_KMS("creating tmp fb for load-detection\n"); fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32); - old->release_fb = fb; } else DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n"); if (IS_ERR(fb)) { @@ -10541,15 +10546,16 @@ retry: if (ret) goto fail; - drm_mode_copy(&crtc_state->base.mode, mode); + drm_framebuffer_unreference(fb); + + ret = drm_atomic_set_mode_for_crtc(&crtc_state->base, mode); + if (ret) + goto fail; if (drm_atomic_commit(state)) { DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n"); - if (old->release_fb) - old->release_fb->funcs->destroy(old->release_fb); goto fail; } - crtc->primary->crtc = crtc; /* let the connector get through one full cycle before testing */ intel_wait_for_vblank(dev, intel_crtc->pipe); @@ -10559,6 +10565,12 @@ fail: drm_atomic_state_free(state); state = NULL; + if (old->old_pipe_config) + intel_crtc_destroy_state(crtc, old->old_pipe_config); + + if (old->old_plane_state) + intel_plane_destroy_state(crtc->primary, old->old_plane_state); + if (ret == -EDEADLK) { drm_modeset_backoff(ctx); goto retry; @@ -10575,61 +10587,69 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, struct intel_encoder *intel_encoder = intel_attached_encoder(connector); struct drm_encoder *encoder = &intel_encoder->base; - struct drm_crtc *crtc = encoder->crtc; + struct drm_crtc *crtc = connector->state->crtc; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct drm_atomic_state *state; struct drm_connector_state *connector_state; struct intel_crtc_state *crtc_state; + struct drm_plane_state *plane_state; int ret; DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", connector->base.id, connector->name, encoder->base.id, encoder->name); - if (old->load_detect_temp) { - state = drm_atomic_state_alloc(dev); - if (!state) - goto fail; + if (!old->old_pipe_config) + return; - state->acquire_ctx = ctx; + state = drm_atomic_state_alloc(dev); + if (!state) + goto fail; - connector_state = drm_atomic_get_connector_state(state, connector); - if (IS_ERR(connector_state)) - goto fail; + state->acquire_ctx = ctx; - crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); - if (IS_ERR(crtc_state)) - goto fail; + connector_state = drm_atomic_get_connector_state(state, connector); + if (IS_ERR(connector_state)) + goto fail; - connector_state->crtc = NULL; + crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); + if (IS_ERR(crtc_state)) + goto fail; - crtc_state->base.enable = crtc_state->base.active = false; + plane_state = drm_atomic_get_plane_state(state, crtc->primary); + if (IS_ERR(plane_state)) + goto fail; - ret = intel_modeset_setup_plane_state(state, crtc, NULL, NULL, - 0, 0); + if (!old->old_pipe_config->enable) { + ret = drm_atomic_set_crtc_for_connector(connector_state, NULL); if (ret) goto fail; + } - ret = drm_atomic_commit(state); - if (ret) - goto fail; + crtc_state->base.enable = old->old_pipe_config->enable; + crtc_state->base.active = old->old_pipe_config->active; - if (old->release_fb) { - drm_framebuffer_unregister_private(old->release_fb); - drm_framebuffer_unreference(old->release_fb); - } + drm_atomic_set_fb_for_plane(plane_state, old->old_plane_state->fb); + ret = drm_atomic_set_crtc_for_plane(plane_state, old->old_plane_state->crtc); + if (ret) + goto fail; - return; - } + old->old_plane_state->state = plane_state->state; + *plane_state = *old->old_plane_state; - /* Switch crtc and encoder back off if necessary */ - if (old->dpms_mode != DRM_MODE_DPMS_ON) - connector->funcs->dpms(connector, old->dpms_mode); + ret = drm_atomic_commit(state); + if (ret) + goto fail; + intel_plane_destroy_state(crtc->primary, old->old_plane_state); + intel_crtc_destroy_state(crtc, old->old_pipe_config); return; + fail: DRM_DEBUG_KMS("Couldn't release load detect pipe.\n"); drm_atomic_state_free(state); + intel_plane_destroy_state(crtc->primary, old->old_plane_state); + intel_crtc_destroy_state(crtc, old->old_pipe_config); } static int i9xx_pll_refclk(struct drm_device *dev, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 93ba14a3bb76..eeda7d02a4f7 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -913,8 +913,8 @@ struct intel_unpin_work { struct intel_load_detect_pipe { struct drm_framebuffer *release_fb; - bool load_detect_temp; - int dpms_mode; + struct drm_crtc_state *old_pipe_config; + struct drm_plane_state *old_plane_state; }; static inline struct intel_encoder *
Instead of restoring dpms and a flag for whether a temp fb is allocated duplicate the old plane_state and crtc_state, and restore the members we potentially touched. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 128 ++++++++++++++++++++--------------- drivers/gpu/drm/i915/intel_drv.h | 4 +- 2 files changed, 76 insertions(+), 56 deletions(-)