On Wed, May 28, 2014 at 5:57 AM, Daniel Vetter 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 > > --- > > 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