Re: [Intel-gfx] [PATCH 4/9] drm/i915: don't free the CFB while FBC is enabled

2014-12-26 Thread Chris Wilson 
On Fri, Dec 26, 2014 at 11:46:49AM -0200, Paulo Zanoni wrote:
 2014-12-25 8:20 GMT-02:00 Chris Wilson ch...@chris-wilson.co.uk:
  On Tue, Dec 23, 2014 at 10:35:40AM -0200, Paulo Zanoni wrote:
  From: Paulo Zanoni paulo.r.zan...@intel.com
 
  Because that is probably not very a good idea: if we used the stolen
  memory for more things, there could be a risk that someone would
  allocate the memory that the HW is still using as the CFB while FBC
  was still enabled.
 
  Not in the code as it currently is due to the struct mutex lock. You
  need to explain how and why you intend to remove that invariant.
 
 Ok, maybe this point is wrong, but I still wee we freeing and then
 reallocating the CFB while FBC is active, which is still a problem,
 even if we end up just reallocating the same chunk of memory later.

No. If you read the current code it will disable FBC using the old
location before it can be reallocated. That it may continue to use the
old address for a frame after we have marked it free isn't clean, but it
is not the bug you describe.
-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 4/9] drm/i915: don't free the CFB while FBC is enabled

2014-12-25 Thread Chris Wilson 
On Tue, Dec 23, 2014 at 10:35:40AM -0200, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 Because that is probably not very a good idea: if we used the stolen
 memory for more things, there could be a risk that someone would
 allocate the memory that the HW is still using as the CFB while FBC
 was still enabled.

Not in the code as it currently is due to the struct mutex lock. You
need to explain how and why you intend to remove that invariant.
-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 4/9] drm/i915: don't free the CFB while FBC is enabled

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

Because that is probably not very a good idea: if we used the stolen
memory for more things, there could be a risk that someone would
allocate the memory that the HW is still using as the CFB while FBC
was still enabled.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index d02c102..f84c5f3 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -198,6 +198,8 @@ static int i915_setup_compression(struct drm_device *dev, 
int size, int fb_cpp)
struct drm_mm_node *uninitialized_var(compressed_llb);
int ret;
 
+   WARN_ON(dev_priv-fbc.enabled);
+
ret = find_compression_threshold(dev, dev_priv-fbc.compressed_fb,
 size, fb_cpp);
if (!ret)
@@ -268,6 +270,7 @@ void i915_gem_stolen_cleanup_compression(struct drm_device 
*dev)
if (dev_priv-fbc.compressed_fb.allocated == 0)
return;
 
+   WARN_ON(dev_priv-fbc.enabled);
drm_mm_remove_node(dev_priv-fbc.compressed_fb);
 
if (dev_priv-fbc.compressed_llb) {
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 1b10b06..83d3c8a 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -593,13 +593,6 @@ void intel_fbc_update(struct drm_device *dev)
if (in_dbg_master())
goto out_disable;
 
-   if (i915_gem_stolen_setup_compression(dev, obj-base.size,
- 
drm_format_plane_cpp(fb-pixel_format, 0))) {
-   if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
-   DRM_DEBUG_KMS(framebuffer too large, disabling 
compression\n);
-   goto out_disable;
-   }
-
/* If the scanout has not changed, don't modify the FBC settings.
 * Note that we make the fundamental assumption that the fb-obj
 * cannot be unpinned (and have its GTT offset and fence revoked)
@@ -638,6 +631,13 @@ void intel_fbc_update(struct drm_device *dev)
intel_fbc_disable(dev);
}
 
+   if (i915_gem_stolen_setup_compression(dev, obj-base.size,
+ 
drm_format_plane_cpp(fb-pixel_format, 0))) {
+   if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
+   DRM_DEBUG_KMS(framebuffer too large, disabling 
compression\n);
+   return;
+   }
+
intel_fbc_enable(crtc);
dev_priv-fbc.no_fbc_reason = FBC_OK;
return;
-- 
2.1.3

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