Op 12-01-16 om 13:34 schreef Daniel Vetter:

On Tue, Jan 12, 2016 at 12:35:59PM +0100, Maarten Lankhorst wrote:

...

drm_atomic_set_crtc_for_connector should be used, and crtc->primary->crtc is assigned by atomic_commit.

Rely on the helpers for setting this correctly, so connector_mask gets updated too.

Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

After examining the code some more I think this fix is incomplete.

It also needs to do the same on release and if you set i915.nuclear_pageflip you'll get a WARN since mode_blob's not set. Fixing this will break release_load_detect which doesn't unset it. Would the code work?

Cc'ing Ville since he may be able to test it.

--- >8 ---

drm/i915: Use atomic state to obtain load detection crtc.

Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com --- diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bc2ec444925e..9eb1f4e263c6 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10409,6 +10409,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; @@ -10474,6 +10475,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; @@ -10489,24 +10493,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) */ @@ -10514,8 +10509,15 @@ retry: i++; if (!(encoder->possible_crtcs & (1 << i))) continue; - if (possible_crtc->state->enable) + + ret = drm_modeset_lock(&crtc->mutex, ctx); + if (ret) + goto fail; + + if (possible_crtc->state->enable) { + drm_modeset_unlock(&crtc->mutex); continue; + }

crtc = possible_crtc; break; @@ -10529,17 +10531,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) @@ -10553,7 +10557,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)) { @@ -10577,7 +10583,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)) { @@ -10589,15 +10594,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); @@ -10607,6 +10613,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; @@ -10623,61 +10635,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 bdfe4035e074..1361836139a5 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -932,8 +932,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 *