Re: [PATCH] drm/i915: Wait for vblank in i9xx_disable_crtc() for gen 2 only

[Ander Conselvan De Oliveira <conselvan2@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2521 bytes --]

On Wed, 2016-02-17 at 11:25 +0200, Jani Nikula wrote:
> On Tue, 16 Feb 2016, Ander Conselvan de Oliveira <
> ander.conselvan.de.oliveira@intel.com> wrote:
> > The wait for other gens was added in commit 564ed191f5d8 ("drm/i915:
> > gmch: fix stuck primary plane due to memory self-refresh mode") since
> > that's necessary when disabling cxsr. However, cxsr disabling was later
> > moved to intel_pre_disable_primary() in commit 87d4300a7dbc ("drm/i915:
> > Move intel_(pre_disable/post_enable)_primary to intel_display.c, and use
> > it there.") and that function got its own vblank wait for cxsr in commit
> > 262cd2e154c2 ("drm/i915: CHV DDR DVFS support and another watermark
> > rewrite"). So remove the extra vblank wait from i9xx_crtc_distable().
> > 
> > Cc: Kalyan Kondapally <kalyan.kondapally@intel.com>
> > Signed-off-by: Ander Conselvan de Oliveira <
> > ander.conselvan.de.oliveira@intel.com>
> 
> Cc: fixes or stable? Fixes which commit exactly of all those?

Fixes: 262cd2e154c2 ("drm/i915: CHV DDR DVFS support and another watermark
rewrite")

Kalyan was seeing some screen corruption with BSW when suspending. He did some
experimenting with moving vblank waits around and it seemed to help. When I dug
through the history of the code I noticed that we now have a double wait for
vblank for cxsr, but I wasn't able to confirm this is actually the fix for that
screen corruption. So this may be a candidate for fixes or even stable, but I'm
not entirely sure.

I attached an image of what the display corruption looks like, in case someone
has any clues.

Thanks,
Ander

> 
> BR,
> Jani.
> 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 568eefc..3cb9383 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6317,10 +6317,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
> >  	/*
> >  	 * On gen2 planes are double buffered but the pipe isn't, so we
> > must
> >  	 * wait for planes to fully turn off before disabling the pipe.
> > -	 * We also need to wait on all gmch platforms because of the
> > -	 * self-refresh mode constraint explained above.
> >  	 */
> > -	intel_wait_for_vblank(dev, pipe);
> > +	if (IS_GEN2(dev))
> > +		intel_wait_for_vblank(dev, pipe);
> >  
> >  	for_each_encoder_on_crtc(dev, crtc, encoder)
> >  		encoder->disable(encoder);
> 

[-- Attachment #2: vlcsnap-2016-02-17-14h25m02s857.png --]
[-- Type: image/png, Size: 1436467 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx