[PATCH 1/2] drm/i915/fbc: fix CFB size calculation for gen8+

[PATCH 1/2] drm/i915/fbc: fix CFB size calculation for gen8+

[Paulo Zanoni]

Broadwell and newer actually compress up to 2560 lines instead of 2048
(as documented in the FBC_CTL page). If we don't take this into
consideration we end up reserving too little stolen memory for the
CFB, so we may allocate something else (such as a ring) right after
what we reserved, and the hardware will overwrite it with the contents
of the CFB when FBC is active, causing GPU hangs. Another possibility
is that the CFB may be allocated at the very end of the available
space, so the CFB will overlap the reserved stolen area, leading to
FIFO underruns.

This bug has always been a problem on BDW (the only affected platform
where FBC is enabled by default), but it's much easier to reproduce
since the following commit:
    commit c58b735fc762e891481e92af7124b85cb0a51fce
    Author: Chris Wilson <chris@chris-wilson.co.uk>
    Date:   Thu Aug 18 17:16:57 2016 +0100
        drm/i915: Allocate rings from stolen

Of course, you can only reproduce the bug if your screen is taller
than 2048 lines.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98213
Fixes: a98ee79317b4 ("drm/i915/fbc: enable FBC by default on HSW and BDW")
Cc: <stable@vger.kernel.org> # v4.6+
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 617189a..6345cb8 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -104,8 +104,10 @@ static int intel_fbc_calculate_cfb_size(struct drm_i915_private *dev_priv,
 	int lines;
 
 	intel_fbc_get_plane_source_size(cache, NULL, &lines);
-	if (INTEL_INFO(dev_priv)->gen >= 7)
+	if (INTEL_GEN(dev_priv) == 7)
 		lines = min(lines, 2048);
+	else if (INTEL_GEN(dev_priv) >= 8)
+		lines = min(lines, 2560);
 
 	/* Hardware needs the full buffer stride, not just the active area. */
 	return lines * cache->fb.stride;
-- 
2.7.4

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

[PATCH 2/2] drm/i915/fbc: fix FBC_COMPRESSION_MASK on BDW+

[Paulo Zanoni]

Its size is 11:0 instead of 10:0. Found by inspecting the spec. I'm
not aware of any real-world IGT failures caused by this.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++----
 drivers/gpu/drm/i915/i915_reg.h     |  5 +++--
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index dc057c7..b54ff40 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1674,11 +1674,13 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 		seq_printf(m, "FBC disabled: %s\n",
 			   dev_priv->fbc.no_fbc_reason);
 
-	if (intel_fbc_is_active(dev_priv) &&
-	    INTEL_GEN(dev_priv) >= 7)
+	if (intel_fbc_is_active(dev_priv) && INTEL_GEN(dev_priv) >= 7) {
+		uint32_t mask = INTEL_GEN(dev_priv) >= 8 ?
+				BDW_FBC_COMPRESSION_MASK :
+				IVB_FBC_COMPRESSION_MASK;
 		seq_printf(m, "Compressing: %s\n",
-			   yesno(I915_READ(FBC_STATUS2) &
-				 FBC_COMPRESSION_MASK));
+			   yesno(I915_READ(FBC_STATUS2) & mask));
+	}
 
 	mutex_unlock(&dev_priv->fbc.lock);
 	intel_runtime_pm_put(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 00efaa1..a9be3f0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2188,8 +2188,9 @@ enum skl_disp_power_wells {
 #define FBC_FENCE_OFF		_MMIO(0x3218) /* BSpec typo has 321Bh */
 #define FBC_TAG(i)		_MMIO(0x3300 + (i) * 4)
 
-#define FBC_STATUS2		_MMIO(0x43214)
-#define  FBC_COMPRESSION_MASK	0x7ff
+#define FBC_STATUS2			_MMIO(0x43214)
+#define  IVB_FBC_COMPRESSION_MASK	0x7ff
+#define  BDW_FBC_COMPRESSION_MASK	0xfff
 
 #define FBC_LL_SIZE		(1536)
 
-- 
2.7.4

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

Re: [PATCH 1/2] drm/i915/fbc: fix CFB size calculation for gen8+

[Ville Syrjälä]

On Fri, Oct 21, 2016 at 01:55:45PM -0200, Paulo Zanoni wrote:
> Broadwell and newer actually compress up to 2560 lines instead of 2048
> (as documented in the FBC_CTL page). If we don't take this into
> consideration we end up reserving too little stolen memory for the
> CFB, so we may allocate something else (such as a ring) right after
> what we reserved, and the hardware will overwrite it with the contents
> of the CFB when FBC is active, causing GPU hangs. Another possibility
> is that the CFB may be allocated at the very end of the available
> space, so the CFB will overlap the reserved stolen area, leading to
> FIFO underruns.
> 
> This bug has always been a problem on BDW (the only affected platform
> where FBC is enabled by default), but it's much easier to reproduce
> since the following commit:
>     commit c58b735fc762e891481e92af7124b85cb0a51fce
>     Author: Chris Wilson <chris@chris-wilson.co.uk>
>     Date:   Thu Aug 18 17:16:57 2016 +0100
>         drm/i915: Allocate rings from stolen
> 
> Of course, you can only reproduce the bug if your screen is taller
> than 2048 lines.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98213
> Fixes: a98ee79317b4 ("drm/i915/fbc: enable FBC by default on HSW and BDW")
> Cc: <stable@vger.kernel.org> # v4.6+
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_fbc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 617189a..6345cb8 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -104,8 +104,10 @@ static int intel_fbc_calculate_cfb_size(struct drm_i915_private *dev_priv,
>  	int lines;
>  
>  	intel_fbc_get_plane_source_size(cache, NULL, &lines);
> -	if (INTEL_INFO(dev_priv)->gen >= 7)
> +	if (INTEL_GEN(dev_priv) == 7)
>  		lines = min(lines, 2048);
> +	else if (INTEL_GEN(dev_priv) >= 8)
> +		lines = min(lines, 2560);
>  
>  	/* Hardware needs the full buffer stride, not just the active area. */
>  	return lines * cache->fb.stride;
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/fbc: fix FBC_COMPRESSION_MASK on BDW+

[Ville Syrjälä]

On Fri, Oct 21, 2016 at 01:55:46PM -0200, Paulo Zanoni wrote:
> Its size is 11:0 instead of 10:0. Found by inspecting the spec. I'm
> not aware of any real-world IGT failures caused by this.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++----
>  drivers/gpu/drm/i915/i915_reg.h     |  5 +++--
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index dc057c7..b54ff40 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1674,11 +1674,13 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
>  		seq_printf(m, "FBC disabled: %s\n",
>  			   dev_priv->fbc.no_fbc_reason);
>  
> -	if (intel_fbc_is_active(dev_priv) &&
> -	    INTEL_GEN(dev_priv) >= 7)
> +	if (intel_fbc_is_active(dev_priv) && INTEL_GEN(dev_priv) >= 7) {
> +		uint32_t mask = INTEL_GEN(dev_priv) >= 8 ?
> +				BDW_FBC_COMPRESSION_MASK :
> +				IVB_FBC_COMPRESSION_MASK;
>  		seq_printf(m, "Compressing: %s\n",
> -			   yesno(I915_READ(FBC_STATUS2) &
> -				 FBC_COMPRESSION_MASK));
> +			   yesno(I915_READ(FBC_STATUS2) & mask));
> +	}
>  
>  	mutex_unlock(&dev_priv->fbc.lock);
>  	intel_runtime_pm_put(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00efaa1..a9be3f0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2188,8 +2188,9 @@ enum skl_disp_power_wells {
>  #define FBC_FENCE_OFF		_MMIO(0x3218) /* BSpec typo has 321Bh */
>  #define FBC_TAG(i)		_MMIO(0x3300 + (i) * 4)
>  
> -#define FBC_STATUS2		_MMIO(0x43214)
> -#define  FBC_COMPRESSION_MASK	0x7ff
> +#define FBC_STATUS2			_MMIO(0x43214)
> +#define  IVB_FBC_COMPRESSION_MASK	0x7ff
> +#define  BDW_FBC_COMPRESSION_MASK	0xfff

I'm not sure this mask is doing us any good since the high bits
are all MBZ anyway.

But this matches the spec so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  #define FBC_LL_SIZE		(1536)
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/fbc: fix CFB size calculation for gen8+

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/fbc: fix CFB size calculation for gen8+
URL   : https://patchwork.freedesktop.org/series/14173/
State : warning

== Summary ==

Series 14173v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/14173/revisions/1/mbox/

Test drv_module_reload_basic:
                dmesg-warn -> PASS       (fi-skl-6770hq)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-snb-2600)

fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m     total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700k     total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6770hq    total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:210  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:246  pass:208  dwarn:1   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2792/

45ce7992e2d3211009c035db41745e1f12bbf6e8 drm-intel-nightly: 2016y-10m-21d-14h-52m-49s UTC integration manifest
2a7b40b drm/i915/fbc: fix FBC_COMPRESSION_MASK on BDW+
7e6c4b7 drm/i915/fbc: fix CFB size calculation for gen8+

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