On Tue, Jul 07, 2015 at 09:08:20AM +0200, Maarten Lankhorst wrote: > Perform a full readout of the state by making sure the mode is set > up correctly atomically. > > Also there was a small memory leak by doing the memset, fix this > by calling __drm_atomic_helper_crtc_destroy_state first. > > Changes since v1: > - Moved after reworking primary plane and updating mode members. > - Force a modeset calculation by changing mode->type from what the > final mode will be. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> tbh I don't really like mode_from_pipe_config since it goes in reverse. With the adjust mode of config_compare we can compare different final hw states and make a call whether they match enough for modeset avoidance or not. mode_from_pipe_config otoh is a lie on panels where we can do upscaling, hence I'd like to remove it to avoid confusion. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 35 ++++++++++++++++------------------- > drivers/gpu/drm/i915/intel_fbdev.c | 9 ++------- > 2 files changed, 18 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index ee543e1515f1..dc4bdb91ad4d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15191,6 +15191,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) > { > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->base.state); > struct intel_encoder *encoder; > u32 reg; > bool enable; > @@ -15202,6 +15203,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) > /* restore vblank interrupts to correct state */ > drm_crtc_vblank_reset(&crtc->base); > if (crtc->active) { > + drm_calc_timestamping_constants(&crtc->base, &crtc->base.hwmode); > update_scanline_offset(crtc); > drm_crtc_vblank_on(&crtc->base); > } > @@ -15254,9 +15256,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) > crtc->base.state->enable ? "enabled" : "disabled", > crtc->active ? "enabled" : "disabled"); > > - crtc->base.state->enable = crtc->active; > - crtc->base.state->active = crtc->active; > - crtc->base.enabled = crtc->active; > + crtc->base.enabled = crtc->base.state->active = crtc->active; > + WARN_ON(drm_atomic_set_mode_for_crtc(&pipe_config->base, NULL) < 0); > > /* Because we only establish the connector -> encoder -> > * crtc links if something is active, this means the > @@ -15412,6 +15413,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > int i; > > for_each_intel_crtc(dev, crtc) { > + __drm_atomic_helper_crtc_destroy_state(&crtc->base, crtc->base.state); > memset(crtc->config, 0, sizeof(*crtc->config)); > crtc->config->base.crtc = &crtc->base; > > @@ -15420,11 +15422,21 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > crtc->active = dev_priv->display.get_pipe_config(crtc, > crtc->config); > > - crtc->base.state->enable = crtc->active; > crtc->base.state->active = crtc->active; > crtc->base.enabled = crtc->active; > crtc->base.hwmode = crtc->config->base.adjusted_mode; > > + if (crtc->base.enabled) { > + memset(&crtc->base.mode, 0, sizeof(crtc->base.mode)); > + intel_mode_from_pipe_config(&crtc->base.mode, crtc->config); > + WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, &crtc->base.mode) < 0); > + > + /* make sure a initial modeset happens by making sure > + * mode->type will be different from the final mode > + */ > + crtc->base.state->mode.type = 0; > + } > + > readout_plane_state(crtc, to_intel_crtc_state(crtc->base.state)); > > DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n", > @@ -15501,21 +15513,6 @@ void intel_modeset_setup_hw_state(struct drm_device *dev, > > intel_modeset_readout_hw_state(dev); > > - /* > - * Now that we have the config, copy it to each CRTC struct > - * Note that this could go away if we move to using crtc_config > - * checking everywhere. > - */ > - for_each_intel_crtc(dev, crtc) { > - if (crtc->active && i915.fastboot) { > - intel_mode_from_pipe_config(&crtc->base.mode, > - crtc->config); > - DRM_DEBUG_KMS("[CRTC:%d] found active mode: ", > - crtc->base.base.id); > - drm_mode_debug_printmodeline(&crtc->base.mode); > - } > - } > - > /* HW state is read out, now we need to sanitize this mess. */ > for_each_intel_encoder(dev, encoder) { > intel_sanitize_encoder(encoder); > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index 3d5bb56477ab..52d20cea182c 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -486,16 +486,11 @@ retry: > * config, not the input mode, which is what crtc->mode > * usually contains. But since our current fastboot > * code puts a mode derived from the post-pfit timings > - * into crtc->mode this works out correctly. We don't > - * use hwmode anywhere right now, so use it for this > - * since the fb helper layer wants a pointer to > - * something we own. > + * into crtc->mode this works out correctly. > */ > DRM_DEBUG_KMS("looking for current mode on connector %s\n", > connector->name); > - intel_mode_from_pipe_config(&encoder->crtc->hwmode, > - to_intel_crtc(encoder->crtc)->config); > - modes[i] = &encoder->crtc->hwmode; > + modes[i] = &encoder->crtc->mode; > } > crtcs[i] = new_crtc; > > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx