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
