On Thu, Mar 01, 2018 at 11:00:40AM -0800, Rodrigo Vivi wrote: > On Thu, Mar 01, 2018 at 08:43:05PM +0200, Ville Syrjälä wrote: > > On Thu, Mar 01, 2018 at 10:35:48AM -0800, Rodrigo Vivi wrote: > > > On Thu, Mar 01, 2018 at 01:22:50PM +0200, Ville Syrjälä wrote: > > > > On Thu, Mar 01, 2018 at 12:51:45PM +0200, Ville Syrjälä wrote: > > > > > On Wed, Feb 28, 2018 at 11:38:56PM +0000, Pandiyan, Dhinakaran wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Wed, 2018-02-28 at 22:38 +0200, Ville Syrjälä wrote: > > > > > > > On Wed, Feb 28, 2018 at 10:28:13PM +0200, Ville Syrjälä wrote: > > > > > > > > On Sat, Feb 24, 2018 at 03:24:55AM +0000, Pandiyan, Dhinakaran > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, 2018-02-19 at 10:07 +0100, Maarten Lankhorst wrote: > > > > > > > > > > Op 16-02-18 om 20:27 schreef Pandiyan, Dhinakaran: > > > > > > > > > > > On Fri, 2018-02-16 at 08:55 +0000, Chris Wilson wrote: > > > > > > > > > > >> Quoting Dhinakaran Pandiyan (2018-02-16 04:33:21) > > > > > > > > > > >>> Preparing a framebuffer should not require a flush. > > > > > > > > > > >>> _post_plane_update() > > > > > > > > > > >>> takes care of flushing when a flip is scheduled, this > > > > > > > > > > >>> should be > > > > > > > > > > >>> sufficient for PSR and FBC. > > > > > > > > > > >> Makes sense. > > > > > > > > > > >> > > > > > > > > > > > I also think this might speed up the flips a bit by > > > > > > > > > > > avoiding flushes. > > > > > > > > > > > > > > > > > > > > > >>> Cc: Paulo Zanoni <paulo.r.zan...@intel.com> > > > > > > > > > > >>> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com> > > > > > > > > > > >>> Cc: Chris Wilson <ch...@chris-wilson.co.uk> > > > > > > > > > > >>> Signed-off-by: Dhinakaran Pandiyan > > > > > > > > > > >>> <dhinakaran.pandi...@intel.com> > > > > > > > > > > >> Also > > > > > > > > > > >> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com> > > > > > > > > > > >> to validate the flow through atomic. > > > > > > > > > > >> -Chris > > > > > > > > > > >> > > > > > > > > > > Page flips used to do intel_frontbuffer_flip_prepare here, > > > > > > > > > > followed by intel_frontbuffer_flip_complete. I think it > > > > > > > > > > would make sense to change the patch to do that? > > > > > > > > > > > > > > > > > > > > > > > > > > > > I have no context why it was removed, I'll have to understand > > > > > > > > > that > > > > > > > > > change and get back to you. > > > > > > > > > > > > > > > > Since we supposedly have hw nuke for both fbc and psr there > > > > > > > > doesn't seem > > > > > > > > to be much need to do anything for flips. I guess DRRS is the > > > > > > > > only > > > > > > > > thing that kinda needs it (not really, just avoids flipping > > > > > > > > with the > > > > > > > > slow timings). But I think DRRS should really be tied into the > > > > > > > > vblank > > > > > > > > stuff somehow so that we switch to the fast timings whenever a > > > > > > > > vblank > > > > > > > > interrupts are enabled. > > > > > > > > > > > > > > Oh, I guess VLV/CHV PSR is what would need this. To do that > > > > > > > properly > > > > > > > (ie. main link off) I think we'd basically need to do a full > > > > > > > modeset > > > > > > > when exiting PSR, so it should probably handled somewhere higher > > > > > > > up > > > > > > > during modeset, and for other uses the frontbuffer tracking > > > > > > > should perhaps just schedule a work to do the full modeset. > > > > > > > > > > > > > The mention of "full modeset" got me thinking. I believe you said > > > > > > full > > > > > > modeset because the link needs to be trained on PSR exit if it was > > > > > > off. > > > > > > But, link off isn't supported on VLV/CHV > > > > > > > > > > > > else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > > > > > > /* On VLV and CHV only standby mode is supported. */ > > > > > > dev_priv->psr.link_standby = true; > > > > > > > > > > I think that's just because we've been lazy and done it. I think even > > > > > with the link off we'd need to reprogram all planes at least. > > > > > > > > Not had coffee yet today, and it shows. Let me rewrite that entire > > > > thing: > > > > > > > > I think that's just because we've been lazy and not done it (link off > > > > mode). > > > > > > I agree with you here. This was probably exactly what was missing. > > > But, how to do this without flickering (blinking?!) the screen? > > > > > > > I think even without the link off we'd need to reprogram all planes at > > > > least. > > > > > > on VLV/CHV or everywhere? and why do you think that? > > > > VLV/CHV. I believe the hw implements PSR by simplty turning off the > > planes (and pipes+dplls for link off), but it doesn't have any automagic > > stuff for recovering the original state. It's all up to the driver. > > > > IIRC Rodrigo even confirmed this at some point, or at least he had to > > trigger a plane update to get something visible on the screen. > > My memory is terrible. But this makes so much sense and in sync > with some vague memory flashes here :) > > And probably what blocked me for looking there besides the > other bugs on core platforms was my disbelieve that would > be possible without seeing any blink of a blank screen when > exiting PSR. :/
I suppose we should start the pipe/planes back up, and only then we can tell the sink to stop using the old stored frame. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
