| Message ID | 20200128235241.169694-1-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/display/fbc: Make fences a nice-to-have for GEN9+ | expand |
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. Also the tracking stuff is busted in intel ddx so need to get https://patchwork.freedesktop.org/series/70636/ merged somehow. > 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
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). > Also the tracking stuff is busted in intel ddx so need to get > https://patchwork.freedesktop.org/series/70636/ merged somehow. > > > 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
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
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) { fbc->no_fbc_reason = "framebuffer not tiled or fenced"; return false; }
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(-)