Re: [Intel-gfx] [PATCH 2/9] drm/i915: fix the FBC CFB size tracking

2014-12-26 Thread Chris Wilson 
On Fri, Dec 26, 2014 at 11:46:34AM -0200, Paulo Zanoni wrote:
 2014-12-25 8:16 GMT-02:00 Chris Wilson ch...@chris-wilson.co.uk:
  On Tue, Dec 23, 2014 at 10:35:38AM -0200, Paulo Zanoni wrote:
  From: Paulo Zanoni paulo.r.zan...@intel.com
 
  We have dev_priv-fbc.size which is supposed to contain the compressed
  FB size, but it is not: at find_compression_threshold() we try to
  overallocate the CFB, but we don't consider this when we assign a
  value to dev_priv-fbc.size. Since the correct CFB size should already
  be stored at dev_priv-fbc.compressed_fb.size, just kill
  dev_priv-fbc.size and use the correct value isntead.
 
  They should not be equivalent though. We actually want fbc.size to
  compensate for the compression in the allocation so that the simple
  check for enough space succeeds even if we are compressing.
 
 Can you please elaborate more on that? Are you talking about the
 threshold? As far as I can see, we're failing to properly take it into
 consideration both before and after this patch, so it wouldn't be a
 valid reason.

We have both identified the problem, and the fix to the current code
looks rather easy as it seems a simple bug in

commit 5e59f7175f96550ede91f58d267d2b551cb6fbba
Author: Ben Widawsky benjamin.widaw...@intel.com
Date:   Mon Jun 30 10:41:24 2014 -0700

drm/i915: Try harder to get FBC

that fluffs the compare of uncompressed request sizes.

Your subject line claims to be a fix, so fix something.
-Chris

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

Re: [Intel-gfx] [PATCH 2/9] drm/i915: fix the FBC CFB size tracking

2014-12-25 Thread Chris Wilson 
On Tue, Dec 23, 2014 at 10:35:38AM -0200, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 We have dev_priv-fbc.size which is supposed to contain the compressed
 FB size, but it is not: at find_compression_threshold() we try to
 overallocate the CFB, but we don't consider this when we assign a
 value to dev_priv-fbc.size. Since the correct CFB size should already
 be stored at dev_priv-fbc.compressed_fb.size, just kill
 dev_priv-fbc.size and use the correct value isntead.

They should not be equivalent though. We actually want fbc.size to
compensate for the compression in the allocation so that the simple
check for enough space succeeds even if we are compressing.
-Chris

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

[Intel-gfx] [PATCH 2/9] drm/i915: fix the FBC CFB size tracking

2014-12-23 Thread Paulo Zanoni 
From: Paulo Zanoni paulo.r.zan...@intel.com

We have dev_priv-fbc.size which is supposed to contain the compressed
FB size, but it is not: at find_compression_threshold() we try to
overallocate the CFB, but we don't consider this when we assign a
value to dev_priv-fbc.size. Since the correct CFB size should already
be stored at dev_priv-fbc.compressed_fb.size, just kill
dev_priv-fbc.size and use the correct value isntead.

Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h|  1 -
 drivers/gpu/drm/i915/i915_gem_stolen.c | 13 +
 drivers/gpu/drm/i915/intel_fbc.c   |  2 +-
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3752040..f0419c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -734,7 +734,6 @@ enum fb_op_origin {
 };
 
 struct i915_fbc {
-   unsigned long size;
unsigned threshold;
unsigned int fb_id;
unsigned int possible_framebuffer_bits;
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 4797138..d02c102 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -231,10 +231,8 @@ static int i915_setup_compression(struct drm_device *dev, 
int size, int fb_cpp)
   dev_priv-mm.stolen_base + compressed_llb-start);
}
 
-   dev_priv-fbc.size = size / dev_priv-fbc.threshold;
-
-   DRM_DEBUG_KMS(reserved %d bytes of contiguous stolen space for FBC\n,
- size);
+   DRM_DEBUG_KMS(reserved %lu bytes of contiguous stolen space for FBC\n,
+ dev_priv-fbc.compressed_fb.size);
 
return 0;
 
@@ -253,7 +251,8 @@ int i915_gem_stolen_setup_compression(struct drm_device 
*dev, int size, int fb_c
if (!drm_mm_initialized(dev_priv-mm.stolen))
return -ENODEV;
 
-   if (size = dev_priv-fbc.size)
+   if (dev_priv-fbc.compressed_fb.allocated 
+   size = dev_priv-fbc.compressed_fb.size)
return 0;
 
/* Release any current block */
@@ -266,7 +265,7 @@ void i915_gem_stolen_cleanup_compression(struct drm_device 
*dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
 
-   if (dev_priv-fbc.size == 0)
+   if (dev_priv-fbc.compressed_fb.allocated == 0)
return;
 
drm_mm_remove_node(dev_priv-fbc.compressed_fb);
@@ -275,8 +274,6 @@ void i915_gem_stolen_cleanup_compression(struct drm_device 
*dev)
drm_mm_remove_node(dev_priv-fbc.compressed_llb);
kfree(dev_priv-fbc.compressed_llb);
}
-
-   dev_priv-fbc.size = 0;
 }
 
 void i915_gem_cleanup_stolen(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index d3ff2c1..5270dc4 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -78,7 +78,7 @@ static void i8xx_fbc_enable(struct drm_crtc *crtc)
 
dev_priv-fbc.enabled = true;
 
-   cfb_pitch = dev_priv-fbc.size / FBC_LL_SIZE;
+   cfb_pitch = dev_priv-fbc.compressed_fb.size / FBC_LL_SIZE;
if (fb-pitches[0]  cfb_pitch)
cfb_pitch = fb-pitches[0];
 
-- 
2.1.3

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