On Wed, May 28, 2014 at 5:57 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
On Tue, May 27, 2014 at 04:50:14PM -0700, Rodrigo Vivi wrote:
> Some registers set during setup might not be persistent after suspend/resume.
> This was causing bugs for some people that was unable to get PSR entry state
> after resume cycle.
>
> v2: Adding some comments and better commit message explaining why this is needed.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_suspend.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index 56785e8..1923708 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -288,6 +288,12 @@ static void i915_restore_display(struct drm_device *dev)
>               I915_WRITE(PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
>       }
>
> +     /* Forcing a full init sequence after resume to make sure all
> +     * registers are properly set. Some might not be persistent after
> +     * suspend/resume cycle. */
> +     dev_priv->psr.setup_done = false;
> +     intel_edp_psr_update(dev);

No, crtc_enable should take care of this. There's more places (like after
runtime pm) where the hw has potentially lost all register contents, so
crtc_enabl is the right place for this.
-Daniel

crtc_enable takes care of the update, but not the setup part that is done only once...
I do believe that only the setup_done = false is really needed here, but doesn't heart to trigger the update right here
and fixes the bug...
 

> +
>       /* only restore FBC info on the platform that supports FBC*/
>       intel_disable_fbc(dev);
>
> --
> 1.9.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch



--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br