Re: [Intel-gfx] [PATCH 1/2] drm/i915/display/fbc: Make fences a nice-to-have for GEN9+

["Souza, Jose" <jose.souza@intel.com>]

On Wed, 2020-01-29 at 13:58 +0200, Ville Syrjälä wrote:
> On Wed, Jan 29, 2020 at 01:44:49PM +0200, Ville Syrjälä wrote:
> > On Tue, Jan 28, 2020 at 03:52:40PM -0800, José Roberto de Souza
> > wrote:
> > > dGFX have local memory so it do not have aperture and do not
> > > support
> > > CPU fences but even for iGFX it have a small number of fences.
> > > 
> > > As replacement for fences to track frontbuffer modifications by
> > > CPU
> > > we have a software tracking that is already in used by FBC and
> > > PSR.
> > > PSR don't support fences so it shows that this tracking is
> > > reliable.
> > > 
> > > So lets make fences a nice-to-have to activate FBC for GEN9+(as
> > > we
> > > only have a good CI coverage for GEN9+), this will allow us to
> > > enable
> > > FBC for dGFXs and iGFXs even when there is no available fence.
> > > 
> > > intel_fbc_hw_tracking_covers_screen() maybe can also have the
> > > same
> > > treatment as fences but BSpec is not clear if the size limitation
> > > is
> > > for hardware tracking or general use of FBC and I don't have a 5K
> > > display to test it, so keeping as is for safety.
> > > 
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_fbc.c | 12 ++++++++----
> > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> > > b/drivers/gpu/drm/i915/display/intel_fbc.c
> > > index 2a3f1333c8ff..1f0d24a1dec1 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > > @@ -717,11 +717,15 @@ static bool intel_fbc_can_activate(struct
> > > intel_crtc *crtc)
> > >  		return false;
> > >  	}
> > >  
> > > -	/* The use of a CPU fence is mandatory in order to detect
> > > writes
> > > -	 * by the CPU to the scanout and trigger updates to the FBC.
> > > +	/* The use of a CPU fence is one of two ways to detect writes
> > > by the
> > > +	 * CPU to the scanout and trigger updates to the FBC.
> > > +	 *
> > > +	 * The other method is by software tracking(see
> > > +	 * intel_fbc_invalidate/flush()), it will manually notify FBC
> > > and nuke
> > > +	 * the current compressed buffer and recompress it.
> > >  	 *
> > >  	 * Note that is possible for a tiled surface to be unmappable
> > > (and
> > > -	 * so have no fence associated with it) due to aperture
> > > constaints
> > > +	 * so have no fence associated with it) due to aperture
> > > constraints
> > >  	 * at the time of pinning.
> > >  	 *
> > >  	 * FIXME with 90/270 degree rotation we should use the fence on
> > > @@ -730,7 +734,7 @@ static bool intel_fbc_can_activate(struct
> > > intel_crtc *crtc)
> > >  	 * For now this will effecively disable FBC with 90/270 degree
> > >  	 * rotation.
> > >  	 */
> > > -	if (cache->fence_id < 0) {
> > > +	if (cache->fence_id < 0 && INTEL_GEN(dev_priv) < 9) {
> > 
> > Not enough. We need to check that the tiling format is actually
> > supported.
> 
> Actually not sure if all of them are or not. IIRC some docs just said
> X/Y tile is supported (and in some other place linear was also
> listed).

Did some testing with kms_frontbuffer_tracking and linear works at
least in GEN12 with fences complete disabled, will do some testing in
GEN9 and GEN11 too and with the other types of tiling.

Any tips to find this documents? It was relevant for GEN9+?
Only thing that BSpec says is that FBC is not supported in RGB 16bpp
with planes rotated by 90 or 270.	

> 
> > Also the tracking stuff is busted in intel ddx so need to get
> > https://patchwork.freedesktop.org/series/70636/ merged somehow.

Ouch, hopefully no one using Intel DDX have PSR panels otherwise we
would have bug reports.

Well maybe until that is figure out, the check above could be
INTEL_GEN(dev_priv) < 11, if I remember correctly ICL is the first
platform not supported by DDX.

> > 
> > >  		fbc->no_fbc_reason = "framebuffer not tiled or fenced";
> > >  		return false;
> > >  	}
> > > -- 
> > > 2.25.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx