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

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6fdfd1a..190bae5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -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;
-- 
1.8.1.2

Re: [PATCH] drm/i915: Haswell FBC supports up to 4096x4096

[Daniel Vetter]

On Thu, May 23, 2013 at 11:30 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> 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.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

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

Re: [PATCH] drm/i915: Haswell FBC supports up to 4096x4096

[Ville Syrjälä]

On Fri, May 24, 2013 at 01:41:22AM +0200, Daniel Vetter wrote:
> On Thu, May 23, 2013 at 11:30 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > 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.
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> 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.

> 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
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 49a1887..cf123c1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -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;
-- 
1.8.1.2

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

[Daniel Vetter]

On Mon, Jun 3, 2013 at 11:15 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> 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.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 49a1887..cf123c1 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -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) {

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

> +               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
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

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

[Paulo Zanoni]

2013/6/4 Daniel Vetter <daniel@ffwll.ch>:
> On Mon, Jun 3, 2013 at 11:15 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> 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.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 49a1887..cf123c1 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -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) {
>
> 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?" ...

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.


>
> Cheers, Daniel
>
>> +               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
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Paulo Zanoni

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

[Ville Syrjälä]

On Tue, Jun 04, 2013 at 02:46:14PM -0300, Paulo Zanoni wrote:
> 2013/6/4 Daniel Vetter <daniel@ffwll.ch>:
> > On Mon, Jun 3, 2013 at 11:15 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> 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.
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++---
> >>  1 file changed, 12 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index 49a1887..cf123c1 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -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) {
> >
> > 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?" ...
> 
> 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.

-- 
Ville Syrjälä
Intel OTC

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

[Paulo Zanoni]

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4126fb1..1db2a57 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -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;
-- 
1.8.1.2

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

[Daniel Vetter]

On Tue, Jun 04, 2013 at 04:53:39PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> 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).
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

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