[Intel-gfx] [PATCH 4/5] drm/i915/frontbuffer: Remove early frontbuffer flush in prepare_plane_fb()

[Ville Syrjälä]

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.zanoni at intel.com>
> > > > > >>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > >>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > > > >>> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > > > >> Also
> > > > > >> Cc: Maarten Lankhorst <maarten.lankhorst at 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.

-- 
Ville Syrjälä
Intel OTC