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@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
