Re: [Intel-gfx] [PATCH 3/4] drm/i915: Try harder to get FBC

2014-06-20 Thread Runyan, Arthur J
You give me too much credit.  I just gave you an explanation of what the 
hardware does, then you ran with it.

On Thu, Jun 19, 2014 at 12:06:13PM -0700, Ben Widawsky wrote:
+   DRM_INFO(Reducing the compressed framebuffer size. This may 
lead to increased power. Try to increase stolen memory size if available in 
BIOS.\n);

I prefer This may lead to less power savings than a non-reduced size. since 
FBC is still going to save power.

dpfc_ctl = DPFC_CTL_PLANE(intel_crtc-plane);
if (drm_format_plane_cpp(fb-pixel_format, 0) == 2)
+   dev_priv-fbc.threshold++;
+
+   switch (dev_priv-fbc.threshold) {
+   case 4:
+   dpfc_ctl |= DPFC_CTL_LIMIT_4X;
+   break;
+   case 2:
dpfc_ctl |= DPFC_CTL_LIMIT_2X;
-   else
+   break;
+   case 1:
dpfc_ctl |= DPFC_CTL_LIMIT_1X;
+   break;
+   }

I Am Not A Coder, but at a glance it looks like the ++ could lead to undefined 
case 3 when you want case 4.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 3/4] drm/i915: Try harder to get FBC

2014-06-20 Thread Ben Widawsky
Rodrigo, when you're ready, can you pull in Art's requests?

On Fri, Jun 20, 2014 at 03:56:08PM +, Runyan, Arthur J wrote:
 You give me too much credit.  I just gave you an explanation of what the 
 hardware does, then you ran with it.
 
 On Thu, Jun 19, 2014 at 12:06:13PM -0700, Ben Widawsky wrote:
 + DRM_INFO(Reducing the compressed framebuffer size. This may 
 lead to increased power. Try to increase stolen memory size if available in 
 BIOS.\n);
 
 I prefer This may lead to less power savings than a non-reduced size. since 
 FBC is still going to save power.

Got it.

 
   dpfc_ctl = DPFC_CTL_PLANE(intel_crtc-plane);
   if (drm_format_plane_cpp(fb-pixel_format, 0) == 2)
 + dev_priv-fbc.threshold++;
 +
 + switch (dev_priv-fbc.threshold) {
 + case 4:
 + dpfc_ctl |= DPFC_CTL_LIMIT_4X;
 + break;
 + case 2:
   dpfc_ctl |= DPFC_CTL_LIMIT_2X;
 - else
 + break;
 + case 1:
   dpfc_ctl |= DPFC_CTL_LIMIT_1X;
 + break;
 + }
 
 I Am Not A Coder, but at a glance it looks like the ++ could lead to 
 undefined case 3 when you want case 4.
 
Thanks for your feedback. 3 is an invalid value, but a default case
should be added here. I'd do a BUG_ON, but that's strictly forbidden.

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 3/4] drm/i915: Try harder to get FBC

2014-06-19 Thread Ben Widawsky
The GEN FBC unit provides the ability to set a low pass on frames it
attempts to compress. If a frame is less than a certain amount
compressibility (2:1, 4:1) it will not bother. This allows the driver to
reduce the size it requests out of stolen memory.

Unluckily, a few months ago, Ville actually began using this feature for
framebuffers that are 16bpp (not sure why not 8bpp). In those cases, we
are already using this mechanism for a different purpose, and so we can
only achieve one further level of compression (2:1 - 4:1)

FBC GEN1, ie. pre-G45 is ignored.

The cleverness of the patch is Art's. The bugs are mine.

Cc: Art Runyan arthur.j.run...@intel.com
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_drv.h|  3 +-
 drivers/gpu/drm/i915/i915_gem_stolen.c | 54 +-
 drivers/gpu/drm/i915/intel_pm.c| 28 --
 3 files changed, 67 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0003206..db9dac4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -601,6 +601,7 @@ struct intel_context {
 
 struct i915_fbc {
unsigned long size;
+   unsigned threshold;
unsigned int fb_id;
enum plane plane;
int y;
@@ -2446,7 +2447,7 @@ static inline void i915_gem_chipset_flush(struct 
drm_device *dev)
 
 /* i915_gem_stolen.c */
 int i915_gem_init_stolen(struct drm_device *dev);
-int i915_gem_stolen_setup_compression(struct drm_device *dev, int size);
+int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, int 
fb_cpp);
 void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
 void i915_gem_cleanup_stolen(struct drm_device *dev);
 struct drm_i915_gem_object *
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index a86b331..4b9804e 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -105,35 +105,61 @@ static unsigned long i915_stolen_to_physical(struct 
drm_device *dev)
 
 static int find_compression_threshold(struct drm_device *dev,
  struct drm_mm_node *node,
- int size)
+ int size,
+ int fb_cpp)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
-   const int compression_threshold = 1;
+   int compression_threshold = 1;
int ret;
 
-   /* Try to over-allocate to reduce reallocations and fragmentation */
+   /* HACK: This code depends on what we will do in *_enable_fbc. If that
+* code changes, this code needs to change as well.
+*
+* The enable_fbc code will attempt to use one of our 2 compression
+* thresholds, therefore, in that case, we only have 1 resort.
+*/
+
+   /* Try to over-allocate to reduce reallocations and fragmentation. */
ret = drm_mm_insert_node(dev_priv-mm.stolen, node,
 size = 1, 4096, DRM_MM_SEARCH_DEFAULT);
-   if (ret)
-   ret = drm_mm_insert_node(dev_priv-mm.stolen, node,
-size = 1, 4096,
-DRM_MM_SEARCH_DEFAULT);
-   if (ret)
+   if (ret == 0)
+   return compression_threshold;
+
+again:
+   /* HW's ability to limit the CFB is 1:4 */
+   if (compression_threshold  4 ||
+   (fb_cpp == 2  compression_threshold == 2))
return 0;
-   else
+
+   ret = drm_mm_insert_node(dev_priv-mm.stolen, node,
+size = 1, 4096,
+DRM_MM_SEARCH_DEFAULT);
+   if (ret  INTEL_INFO(dev)-gen = 4) {
+   return 0;
+   } else if (ret) {
+   compression_threshold = 1;
+   goto again;
+   } else {
return compression_threshold;
+   }
 }
 
-static int i915_setup_compression(struct drm_device *dev, int size)
+static int i915_setup_compression(struct drm_device *dev, int size, int fb_cpp)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_mm_node *uninitialized_var(compressed_llb);
int ret;
 
ret = find_compression_threshold(dev, dev_priv-fbc.compressed_fb,
-size);
+size, fb_cpp);
if (!ret)
goto err_llb;
+   else if (ret  1) {
+   DRM_INFO(Reducing the compressed framebuffer size. This may 
lead to increased power. Try to increase stolen memory size if available in 
BIOS.\n);
+
+   }
+
+   dev_priv-fbc.threshold = ret;
 
if (HAS_PCH_SPLIT(dev))
I915_WRITE(ILK_DPFC_CB_BASE, dev_priv-fbc.compressed_fb.start);
@@ -157,7 +183,7 @@ static int i915_setup_compression(struct