Skip to contentSkip to conversation

drm/i915: Haswell FBC supports up to 4096x4096

PATCH In review Paulo Zanoni 3 participants 9 messages May 23, 2013 – Jun 6, 2013

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drm/i915: Haswell FBC supports up to 4096x4096

From: Paulo Zanoni <[email protected]>

But only the first 2048 lines will be compressed. No problem.

With this I can finally see FBC on my 2560x1440 DP monitor, which gives me a boost on the PC7 residency.

---
 drivers/gpu/drm/i915/intel_pm.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +13 -3
index 6fdfd1a..190bae5 100644
@@ -431,7 +431,7 @@ void intel_disable_fbc(struct drm_device *dev)
  *   - no pixel mulitply/line duplication
  *   - no alpha buffer discard
  *   - no dual wide
- *   - framebuffer <= 2048 in width, 1536 in height
+ *   - framebuffer <= max_hdisplay in width, max_vdisplay in height
  *
  * We can't assume that any compression will take place (worst case),
  * so the compressed buffer has to be the same size as the uncompressed
@@ -449,6 +449,7 @@ void intel_update_fbc(struct drm_device *dev)
 	struct intel_framebuffer *intel_fb;
 	struct drm_i915_gem_object *obj;
 	int enable_fbc;
+	unsigned int max_vdisplay, max_hdisplay;
 
 	if (!i915_powersave)
 		return;
@@ -507,8 +508,17 @@ void intel_update_fbc(struct drm_device *dev)
 		dev_priv->no_fbc_reason = FBC_UNSUPPORTED_MODE;
 		goto out_disable;
 	}
-	if ((crtc->mode.hdisplay > 2048) ||
-	    (crtc->mode.vdisplay > 1536)) {
+
+	if (INTEL_INFO(dev)->gen < 7 || IS_IVYBRIDGE(dev) ||
+	    IS_VALLEYVIEW(dev)) {
+		max_hdisplay = 2048;
+		max_vdisplay = 1536;
+	} else {
+		max_hdisplay = 4096;
+		max_vdisplay = 4096;
+	}
+	if ((crtc->mode.hdisplay > max_hdisplay) ||
+	    (crtc->mode.vdisplay > max_vdisplay)) {
 		DRM_DEBUG_KMS("mode too large for compression, disabling\n");
 		dev_priv->no_fbc_reason = FBC_MODE_TOO_LARGE;
 		goto out_disable;
Signature · 1 line
1.8.1.2
#1 lore raw
Quoted 8 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

But only the first 2048 lines will be compressed. No problem.

With this I can finally see FBC on my 2560x1440 DP monitor, which gives me a boost on the PC7 residency.

After some extensive doc hunting I believe that cantiga and ilk+ have a limit of 2048 lines by 4048 pixels wide (this is the fbc2 limit). Original fbc (on gen2/3/crestline) has the limits currently in our code. Can you please amend your patch to include this? -Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

#2 lore raw
Quoted 5 lines · Daniel Vetter
Quoted 4 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

But only the first 2048 lines will be compressed. No problem.

Quoted 7 lines
Quoted 4 lines

With this I can finally see FBC on my 2560x1440 DP monitor, which gives me a boost on the PC7 residency.

After some extensive doc hunting I believe that cantiga and ilk+ have a limit of 2048 lines by 4048 pixels wide (this is the fbc2 limit).

Where does 4048 come from? I only see 4K mentioned in all the docs.

Other mismatches we have:
- fbc2 supports interlaced scan (we disable fbc w/ interlaced modes)
- 16bpp and 32bpp formats (for 16bpp ratio must be configured to 1/2) We don't check the bpp at all currently, so I assume something bad would happend w/ 8bpp and 16bpp.

The ctg docs also has some other things to say:
- No sprite + fbc on the same pipe
- only 32bpp support (it's a bit unclear actually since it still lists the compressions ratio settings for 16bpp).
- "Can only be enabled when output is to a local panel at the native resolution"
Not sure what this means in practice. No pfit?
- pixel multiplier needs to be 1

Additional fbc1 limits we don't seem to check:
- gen3: pitch must be 4K or 8K
gen4: pitch must be 2k-16k
- fb width % 8 = 0, fb height % 2 = 0. I guess we may not have to worry about these since the compression is lossless and thus we can afford to compress in some garbage from outside the viewport.
- The docs also list 640x480 as the minimum size. I have a bit of a hard time believing that this limit would actually be real. Maybe the spec author just thought that 640x480 is the smallest mode anyone would ever use and wrote it down. But I could be wrong.

Quoted 11 lines

Original fbc (on gen2/3/crestline) has the limits currently in our code. Can you please amend your patch to include this? -Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 2 lines
Ville Syrjälä
Intel OTC
#3 lore raw

PATCH 1/1 drm/i915: update FBC maximum fb sizes

In reply to Daniel Vetter

From: Paulo Zanoni <[email protected]>

CTG/ILK/SNB/IVB support 4kx2k surfaces. HSW supports 4kx4k, but without proper front buffer invalidation on the last 2k lines, so don't enable FBC on these cases for now.

---
 drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +12 -3
index 49a1887..cf123c1 100644
@@ -431,7 +431,7 @@ void intel_disable_fbc(struct drm_device *dev)
  *   - no pixel mulitply/line duplication
  *   - no alpha buffer discard
  *   - no dual wide
- *   - framebuffer <= 2048 in width, 1536 in height
+ *   - framebuffer <= max_hdisplay in width, max_vdisplay in height
  *
  * We can't assume that any compression will take place (worst case),
  * so the compressed buffer has to be the same size as the uncompressed
@@ -449,6 +449,7 @@ void intel_update_fbc(struct drm_device *dev)
 	struct intel_framebuffer *intel_fb;
 	struct drm_i915_gem_object *obj;
 	int enable_fbc;
+	unsigned int max_hdisplay, max_vdisplay;
 
 	if (!i915_powersave)
 		return;
@@ -507,8 +508,16 @@ void intel_update_fbc(struct drm_device *dev)
 		dev_priv->no_fbc_reason = FBC_UNSUPPORTED_MODE;
 		goto out_disable;
 	}
-	if ((crtc->mode.hdisplay > 2048) ||
-	    (crtc->mode.vdisplay > 1536)) {
+
+	if (IS_G4X(dev) || INTEL_INFO(dev)->gen > 4) {
+		max_hdisplay = 4096;
+		max_vdisplay = 2048;
+	} else {
+		max_hdisplay = 2048;
+		max_vdisplay = 1536;
+	}
+	if ((crtc->mode.hdisplay > max_hdisplay) ||
+	    (crtc->mode.vdisplay > max_vdisplay)) {
 		DRM_DEBUG_KMS("mode too large for compression, disabling\n");
 		dev_priv->no_fbc_reason = FBC_MODE_TOO_LARGE;
 		goto out_disable;
Signature · 1 line
1.8.1.2
#4 lore raw

Re: [PATCH] drm/i915: update FBC maximum fb sizes

Quoted 33 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

CTG/ILK/SNB/IVB support 4kx2k surfaces. HSW supports 4kx4k, but without proper front buffer invalidation on the last 2k lines, so don't enable FBC on these cases for now.

---
 drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +2 -1
index 49a1887..cf123c1 100644
@@ -431,7 +431,7 @@ void intel_disable_fbc(struct drm_device *dev)
  *   - no pixel mulitply/line duplication
  *   - no alpha buffer discard
  *   - no dual wide
- *   - framebuffer <= 2048 in width, 1536 in height
+ *   - framebuffer <= max_hdisplay in width, max_vdisplay in height
  *
  * We can't assume that any compression will take place (worst case),
  * so the compressed buffer has to be the same size as the uncompressed
@@ -449,6 +449,7 @@ void intel_update_fbc(struct drm_device *dev)
        struct intel_framebuffer *intel_fb;
        struct drm_i915_gem_object *obj;
        int enable_fbc;
+       unsigned int max_hdisplay, max_vdisplay;
 
        if (!i915_powersave)
                return;

drivers/gpu/drm/i915/intel_pm.c void intel_update_fbc(struct drm_device *dev) · line 512

Quoted 7 lines
drivers/gpu/drm/i915/intel_pm.c +2 -2
@@ -507,8 +508,16 @@ void intel_update_fbc(struct drm_device *dev)
                dev_priv->no_fbc_reason = FBC_UNSUPPORTED_MODE;
                goto out_disable;
        }
-       if ((crtc->mode.hdisplay > 2048) ||
-           (crtc->mode.vdisplay > 1536)) {
+
+       if (IS_G4X(dev) || INTEL_INFO(dev)->gen > 4) {

Bikeshed-time: I prefer gen checks to be of the form gen >= x or gen < y because:
- It looks more like C code should look like.
- We we talk about platform ranges we tend to use e.g. ilk+ and pre-gen6, so would fit more naturally with the written word.

Patch looks good otherwise, but for a bit I was thinking "wait, g4x is gen4, what's going on here?" ...

Cheers, Daniel

Quoted 14 lines
drivers/gpu/drm/i915/intel_pm.c +8 -1
+               max_hdisplay = 4096;
+               max_vdisplay = 2048;
+       } else {
+               max_hdisplay = 2048;
+               max_vdisplay = 1536;
+       }
+       if ((crtc->mode.hdisplay > max_hdisplay) ||
+           (crtc->mode.vdisplay > max_vdisplay)) {
                DRM_DEBUG_KMS("mode too large for compression, disabling\n");
                dev_priv->no_fbc_reason = FBC_MODE_TOO_LARGE;
                goto out_disable;
--

1.8.1.2

Quoted 4 lines

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

#5 lore raw

Re: [PATCH] drm/i915: update FBC maximum fb sizes

2013/6/4 Daniel Vetter <[email protected]>:

Quoted 48 lines · Paulo Zanoni
Quoted 33 lines

From: Paulo Zanoni <[email protected]>

CTG/ILK/SNB/IVB support 4kx2k surfaces. HSW supports 4kx4k, but without proper front buffer invalidation on the last 2k lines, so don't enable FBC on these cases for now.

---
 drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +2 -1
index 49a1887..cf123c1 100644
@@ -431,7 +431,7 @@ void intel_disable_fbc(struct drm_device *dev)
  *   - no pixel mulitply/line duplication
  *   - no alpha buffer discard
  *   - no dual wide
- *   - framebuffer <= 2048 in width, 1536 in height
+ *   - framebuffer <= max_hdisplay in width, max_vdisplay in height
  *
  * We can't assume that any compression will take place (worst case),
  * so the compressed buffer has to be the same size as the uncompressed
@@ -449,6 +449,7 @@ void intel_update_fbc(struct drm_device *dev)
        struct intel_framebuffer *intel_fb;
        struct drm_i915_gem_object *obj;
        int enable_fbc;
+       unsigned int max_hdisplay, max_vdisplay;
 
        if (!i915_powersave)
                return;

drivers/gpu/drm/i915/intel_pm.c void intel_update_fbc(struct drm_device *dev) · line 512

Quoted 7 lines
drivers/gpu/drm/i915/intel_pm.c +2 -2
@@ -507,8 +508,16 @@ void intel_update_fbc(struct drm_device *dev)
                dev_priv->no_fbc_reason = FBC_UNSUPPORTED_MODE;
                goto out_disable;
        }
-       if ((crtc->mode.hdisplay > 2048) ||
-           (crtc->mode.vdisplay > 1536)) {
+
+       if (IS_G4X(dev) || INTEL_INFO(dev)->gen > 4) {

Bikeshed-time: I prefer gen checks to be of the form gen >= x or gen < y because:
- It looks more like C code should look like.
- We we talk about platform ranges we tend to use e.g. ilk+ and pre-gen6, so would fit more naturally with the written word.

Quoted 2 lines

Patch looks good otherwise, but for a bit I was thinking "wait, g4x is gen4, what's going on here?" ...

Yeah, you're right. I was trying to find out "how do I check for Cantiga+?" and basically copied the line I found inside intel_crtc_compute_config. We could really have a map between these older codenames and our checks somewhere.

Quoted 28 lines

Cheers, Daniel

Quoted 14 lines
drivers/gpu/drm/i915/intel_pm.c +8 -1
+               max_hdisplay = 4096;
+               max_vdisplay = 2048;
+       } else {
+               max_hdisplay = 2048;
+               max_vdisplay = 1536;
+       }
+       if ((crtc->mode.hdisplay > max_hdisplay) ||
+           (crtc->mode.vdisplay > max_vdisplay)) {
                DRM_DEBUG_KMS("mode too large for compression, disabling\n");
                dev_priv->no_fbc_reason = FBC_MODE_TOO_LARGE;
                goto out_disable;
--

1.8.1.2

Quoted 4 lines

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Signature · 1 line
Paulo Zanoni
#6 lore raw

Re: [PATCH] drm/i915: update FBC maximum fb sizes

Quoted 49 lines · Paulo Zanoni

2013/6/4 Daniel Vetter <[email protected]>:

Quoted 48 lines · Paulo Zanoni
Quoted 33 lines

From: Paulo Zanoni <[email protected]>

CTG/ILK/SNB/IVB support 4kx2k surfaces. HSW supports 4kx4k, but without proper front buffer invalidation on the last 2k lines, so don't enable FBC on these cases for now.

---
 drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +2 -1
index 49a1887..cf123c1 100644
@@ -431,7 +431,7 @@ void intel_disable_fbc(struct drm_device *dev)
  *   - no pixel mulitply/line duplication
  *   - no alpha buffer discard
  *   - no dual wide
- *   - framebuffer <= 2048 in width, 1536 in height
+ *   - framebuffer <= max_hdisplay in width, max_vdisplay in height
  *
  * We can't assume that any compression will take place (worst case),
  * so the compressed buffer has to be the same size as the uncompressed
@@ -449,6 +449,7 @@ void intel_update_fbc(struct drm_device *dev)
        struct intel_framebuffer *intel_fb;
        struct drm_i915_gem_object *obj;
        int enable_fbc;
+       unsigned int max_hdisplay, max_vdisplay;
 
        if (!i915_powersave)
                return;

drivers/gpu/drm/i915/intel_pm.c void intel_update_fbc(struct drm_device *dev) · line 512

Quoted 7 lines
drivers/gpu/drm/i915/intel_pm.c +2 -2
@@ -507,8 +508,16 @@ void intel_update_fbc(struct drm_device *dev)
                dev_priv->no_fbc_reason = FBC_UNSUPPORTED_MODE;
                goto out_disable;
        }
-       if ((crtc->mode.hdisplay > 2048) ||
-           (crtc->mode.vdisplay > 1536)) {
+
+       if (IS_G4X(dev) || INTEL_INFO(dev)->gen > 4) {

Bikeshed-time: I prefer gen checks to be of the form gen >= x or gen < y because:
- It looks more like C code should look like.
- We we talk about platform ranges we tend to use e.g. ilk+ and pre-gen6, so would fit more naturally with the written word.

Quoted 7 lines
Quoted 2 lines

Patch looks good otherwise, but for a bit I was thinking "wait, g4x is gen4, what's going on here?" ...

Yeah, you're right. I was trying to find out "how do I check for Cantiga+?" and basically copied the line I found inside intel_crtc_compute_config. We could really have a map between these older codenames and our checks somewhere.

We should maybe also have some feature macro for ctg+ display. Much of the north display changed already for ctg rather than ilk.

Signature · 2 lines
Ville Syrjälä
Intel OTC
#7 lore raw

PATCH 1/1 drm/i915: update FBC maximum fb sizes

In reply to Daniel Vetter

From: Paulo Zanoni <[email protected]>

CTG/ILK/SNB/IVB support 4kx2k surfaces. HSW supports 4kx4k, but without proper front buffer invalidation on the last 2k lines, so don't enable FBC on these cases for now.

v2: Use gen >= 5, not gen > 4 (Daniel).

---
 drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/intel_pm.c +12 -3
index 4126fb1..1db2a57 100644
@@ -431,7 +431,7 @@ void intel_disable_fbc(struct drm_device *dev)
  *   - no pixel mulitply/line duplication
  *   - no alpha buffer discard
  *   - no dual wide
- *   - framebuffer <= 2048 in width, 1536 in height
+ *   - framebuffer <= max_hdisplay in width, max_vdisplay in height
  *
  * We can't assume that any compression will take place (worst case),
  * so the compressed buffer has to be the same size as the uncompressed
@@ -449,6 +449,7 @@ void intel_update_fbc(struct drm_device *dev)
 	struct intel_framebuffer *intel_fb;
 	struct drm_i915_gem_object *obj;
 	int enable_fbc;
+	unsigned int max_hdisplay, max_vdisplay;
 
 	if (!i915_powersave)
 		return;
@@ -507,8 +508,16 @@ void intel_update_fbc(struct drm_device *dev)
 		dev_priv->no_fbc_reason = FBC_UNSUPPORTED_MODE;
 		goto out_disable;
 	}
-	if ((crtc->mode.hdisplay > 2048) ||
-	    (crtc->mode.vdisplay > 1536)) {
+
+	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
+		max_hdisplay = 4096;
+		max_vdisplay = 2048;
+	} else {
+		max_hdisplay = 2048;
+		max_vdisplay = 1536;
+	}
+	if ((crtc->mode.hdisplay > max_hdisplay) ||
+	    (crtc->mode.vdisplay > max_vdisplay)) {
 		DRM_DEBUG_KMS("mode too large for compression, disabling\n");
 		dev_priv->no_fbc_reason = FBC_MODE_TOO_LARGE;
 		goto out_disable;
Signature · 1 line
1.8.1.2
#8 lore raw

Re: [PATCH] drm/i915: update FBC maximum fb sizes

Quoted 9 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

CTG/ILK/SNB/IVB support 4kx2k surfaces. HSW supports 4kx4k, but without proper front buffer invalidation on the last 2k lines, so don't enable FBC on these cases for now.

v2: Use gen >= 5, not gen > 4 (Daniel).

Queued for -next, thanks for the patch.
-Daniel

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch