[CI] Revert "drm/i915/fbc: Allow on unfenced surfaces, for recent gen"

[CI] Revert "drm/i915/fbc: Allow on unfenced surfaces, for recent gen"

[Chris Wilson]

This reverts commit 8678fdaf396c ("drm/i915/fbc: Allow on unfenced surfaces,
for recent gen") as Skylake has issues with unfenced FBC tracking (and
yes Skylake doesn't even enable FBC yet). Paulo would like to do a full
review of all existing workarounds to see if any more are missing prior
to allowing FBC on unfenced surfaces. In the meantime lets hope that all
framebuffers are idle and naturally fit within the mappable aperture.

Requested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Fixes: 8678fdaf396c ("drm/i915/fbc: Allow on unfenced surfaces...");
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index bf8b22ad9aed..faa67624e1ed 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -799,10 +799,8 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 	 */
 	if (cache->fb.tiling_mode != I915_TILING_X ||
 	    cache->fb.fence_reg == I915_FENCE_REG_NONE) {
-		if (INTEL_GEN(dev_priv) < 5) {
-			fbc->no_fbc_reason = "framebuffer not tiled or fenced";
-			return false;
-		}
+		fbc->no_fbc_reason = "framebuffer not tiled or fenced";
+		return false;
 	}
 	if (INTEL_INFO(dev_priv)->gen <= 4 && !IS_G4X(dev_priv) &&
 	    cache->plane.rotation != DRM_ROTATE_0) {
-- 
2.9.3

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

✗ Fi.CI.BAT: failure for Revert "drm/i915/fbc: Allow on unfenced surfaces, for recent gen"

[Patchwork]

== Series Details ==

Series: Revert "drm/i915/fbc: Allow on unfenced surfaces, for recent gen"
URL   : https://patchwork.freedesktop.org/series/11529/
State : failure

== Summary ==

Series 11529v1 Revert "drm/i915/fbc: Allow on unfenced surfaces, for recent gen"
http://patchwork.freedesktop.org/api/1.0/series/11529/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-flip-vs-cursor-legacy:
                fail       -> PASS       (fi-hsw-4770k)
        Subgroup basic-flip-vs-cursor-varying-size:
                pass       -> FAIL       (fi-hsw-4770r)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-c:
                skip       -> PASS       (fi-hsw-4770r)

fi-bdw-5557u     total:252  pass:235  dwarn:0   dfail:0   fail:2   skip:15 
fi-bsw-n3050     total:252  pass:203  dwarn:0   dfail:0   fail:3   skip:46 
fi-hsw-4770k     total:252  pass:222  dwarn:6   dfail:1   fail:1   skip:22 
fi-hsw-4770r     total:252  pass:223  dwarn:0   dfail:0   fail:3   skip:26 
fi-ivb-3520m     total:252  pass:220  dwarn:0   dfail:0   fail:1   skip:31 
fi-skl-6260u     total:252  pass:236  dwarn:0   dfail:0   fail:2   skip:14 
fi-skl-6700k     total:252  pass:216  dwarn:4   dfail:0   fail:4   skip:28 
fi-snb-2520m     total:252  pass:203  dwarn:4   dfail:0   fail:2   skip:43 
fi-snb-2600      total:252  pass:203  dwarn:4   dfail:0   fail:2   skip:43 

Results at /archive/results/CI_IGT_test/Patchwork_2423/

6737eeadd55aa09ac998698461138309ab623dbb drm-intel-nightly: 2016y-08m-24d-15h-53m-53s UTC integration manifest
a9aac28 Revert "drm/i915/fbc: Allow on unfenced surfaces, for recent gen"

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

Re: [CI] Revert "drm/i915/fbc: Allow on unfenced surfaces, for recent gen"

[Zanoni, Paulo R]

Em Qua, 2016-08-24 às 19:00 +0100, Chris Wilson escreveu:
> This reverts commit 8678fdaf396c ("drm/i915/fbc: Allow on unfenced
> surfaces,
> for recent gen") as Skylake has issues with unfenced FBC tracking
> (and
> yes Skylake doesn't even enable FBC yet)

But it used to work if you had i915.enable_fbc=1 after your latest
fixes :)

> . Paulo would like to do a full
> review of all existing workarounds to see if any more are missing
> prior
> to allowing FBC on unfenced surfaces

We are missing at least one workaround for this, and it's described in
the FBC_CTL page of BSpec for SKL, bit 31 description: we need to
calculate the compressed buffer stride and program it to a register.

I didin't say I would do the review of all existing workarounds, just
that someone needs to do this before enabling FBC on non-X-tiled
buffers since I recall skipping the implementation of workarounds that
didn't apply to X-tiling. Maybe the one I mentioned was the only one,
maybe not.

Also, our test suite only tests X tiling, and the
kms_frontbuffer_tracking/fbc-tilingchange will need to be updated.

> . In the meantime lets hope that all
> framebuffers are idle and naturally fit within the mappable aperture.

What exactly do you mean with the sentence above? Is there some other
bug you spotted? Please share the information.

> 
> Requested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Fixes: 8678fdaf396c ("drm/i915/fbc: Allow on unfenced surfaces...");
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> b/drivers/gpu/drm/i915/intel_fbc.c
> index bf8b22ad9aed..faa67624e1ed 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -799,10 +799,8 @@ static bool intel_fbc_can_activate(struct
> intel_crtc *crtc)
>  	 */
>  	if (cache->fb.tiling_mode != I915_TILING_X ||
>  	    cache->fb.fence_reg == I915_FENCE_REG_NONE) {
> -		if (INTEL_GEN(dev_priv) < 5) {
> -			fbc->no_fbc_reason = "framebuffer not tiled
> or fenced";
> -			return false;
> -		}
> +		fbc->no_fbc_reason = "framebuffer not tiled or
> fenced";
> +		return false;
>  	}
>  	if (INTEL_INFO(dev_priv)->gen <= 4 && !IS_G4X(dev_priv) &&
>  	    cache->plane.rotation != DRM_ROTATE_0) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI] Revert "drm/i915/fbc: Allow on unfenced surfaces, for recent gen"

[chris]

On Wed, Aug 24, 2016 at 09:14:59PM +0000, Zanoni, Paulo R wrote:
> Em Qua, 2016-08-24 às 19:00 +0100, Chris Wilson escreveu:
> > . In the meantime lets hope that all
> > framebuffers are idle and naturally fit within the mappable aperture.
> 
> What exactly do you mean with the sentence above? Is there some other
> bug you spotted? Please share the information.

Not all framebuffers will be fenced, especially if they are being
rendered to before being flipped to the first time. The pressure is less
for full-ppgtt systems (now that we have independent activity tracking
on the vma) but that is not sufficient to guarrantee the object will be
assigned a map_and_fenceable VMA for display.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI] Revert "drm/i915/fbc: Allow on unfenced surfaces, for recent gen"

[Daniel Vetter]

On Wed, Aug 24, 2016 at 10:48:13PM +0100, chris@chris-wilson.co.uk wrote:
> On Wed, Aug 24, 2016 at 09:14:59PM +0000, Zanoni, Paulo R wrote:
> > Em Qua, 2016-08-24 às 19:00 +0100, Chris Wilson escreveu:
> > > . In the meantime lets hope that all
> > > framebuffers are idle and naturally fit within the mappable aperture.
> > 
> > What exactly do you mean with the sentence above? Is there some other
> > bug you spotted? Please share the information.
> 
> Not all framebuffers will be fenced, especially if they are being
> rendered to before being flipped to the first time. The pressure is less
> for full-ppgtt systems (now that we have independent activity tracking
> on the vma) but that is not sufficient to guarrantee the object will be
> assigned a map_and_fenceable VMA for display.

Yeah, with unmappable scanout fbc might luck out a lot.

Anyway, need to get the w/a and testing in place, on this revert
meanwhile:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx