Re: [Intel-gfx] [PATCH 08/16] drm/i915: avoid the last 8mb of stolen on BDW/SKL

2015-09-11 Thread Paulo Zanoni 
2015-08-19 5:24 GMT-03:00 ch...@chris-wilson.co.uk :
> On Tue, Aug 18, 2015 at 09:49:34PM +, Zanoni, Paulo R wrote:
>> Em Sáb, 2015-08-15 às 09:29 +0100, Chris Wilson escreveu:
>> > On Fri, Aug 14, 2015 at 06:34:13PM -0300, Paulo Zanoni wrote:
>> > > The FBC hardware for these platforms doesn't have access to the
>> > > bios_reserved range, so it always assumes the maximum (8mb) is
>> > > used.
>> > > So avoid this range while allocating.
>> > >
>> > > This solves a bunch of FIFO underruns that happen if you end up
>> > > putting the CFB in that memory range. On my machine, with 32mb of
>> > > stolen, I need a 2560x1440 mode for that.
>> >
>> > If the restriction applies only to FBC, apply it to only fbc.
>>
>> That's what the patch is doing. The part where we set the unusual
>> start/end is at intel_fbc.c:find_compression_threshold(). Everything
>> else should be behaving just as before.
>>
>> Maybe you're being confused by the addition of the stolen_usable_size
>> variable.
>
> Hmm, I expected to see the constaint being passed into the search
> routine.a It looked like you were adjusting the stolen_size probably the
> root of my confusion.
>
> Anyway we have a quandary. You want to reserve stolen space, and I want
> to make sure as much of it gets used as possible (especially for long
> lived objects).
>
> What I have in mind is adding a priority to the search and allow us to
> evict anything of lower priority. We can use a page replacement
> algorithm even for the pinned fbcon (since only the GGTT itself is
> pinned and we can swap the pages underneath). This of course requires a
> callback for low priority evictable objects. I have 3 priorities in mind
> plus the evictable flag: USER, KERNEL, POWER (with FBC taking the
> highest priority of POWER). That will allow us to do the BIOS takeover
> and only if we run out of space force the copy.

While I understand your idea, I just want to clarify one thing: it
does not apply to _this_ patch, right? I suppose we're discussing
about "[PATCH 15/16] Revert "drm/i915: Allocate fbcon from stolen
memory" here. The memory avoided by this patch (08/16) can still be
used by the other stolen memory users.

For the fbcon patch, can't we adopt a simpler "if FBC is enabled by
default on this platform, don't alloc fbcon from stolen"?.

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



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

Re: [Intel-gfx] [PATCH 08/16] drm/i915: avoid the last 8mb of stolen on BDW/SKL

2015-08-19 Thread ch...@chris-wilson.co.uk 
On Tue, Aug 18, 2015 at 09:49:34PM +, Zanoni, Paulo R wrote:
 Em Sáb, 2015-08-15 às 09:29 +0100, Chris Wilson escreveu:
  On Fri, Aug 14, 2015 at 06:34:13PM -0300, Paulo Zanoni wrote:
   The FBC hardware for these platforms doesn't have access to the
   bios_reserved range, so it always assumes the maximum (8mb) is 
   used.
   So avoid this range while allocating.
   
   This solves a bunch of FIFO underruns that happen if you end up
   putting the CFB in that memory range. On my machine, with 32mb of
   stolen, I need a 2560x1440 mode for that.
  
  If the restriction applies only to FBC, apply it to only fbc.
 
 That's what the patch is doing. The part where we set the unusual
 start/end is at intel_fbc.c:find_compression_threshold(). Everything
 else should be behaving just as before.
 
 Maybe you're being confused by the addition of the stolen_usable_size
 variable.

Hmm, I expected to see the constaint being passed into the search
routine.a It looked like you were adjusting the stolen_size probably the
root of my confusion.

Anyway we have a quandary. You want to reserve stolen space, and I want
to make sure as much of it gets used as possible (especially for long
lived objects).

What I have in mind is adding a priority to the search and allow us to
evict anything of lower priority. We can use a page replacement
algorithm even for the pinned fbcon (since only the GGTT itself is
pinned and we can swap the pages underneath). This of course requires a
callback for low priority evictable objects. I have 3 priorities in mind
plus the evictable flag: USER, KERNEL, POWER (with FBC taking the
highest priority of POWER). That will allow us to do the BIOS takeover
and only if we run out of space force the copy.
-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 08/16] drm/i915: avoid the last 8mb of stolen on BDW/SKL

2015-08-18 Thread Zanoni, Paulo R 
Em Sáb, 2015-08-15 às 09:29 +0100, Chris Wilson escreveu:
 On Fri, Aug 14, 2015 at 06:34:13PM -0300, Paulo Zanoni wrote:
  The FBC hardware for these platforms doesn't have access to the
  bios_reserved range, so it always assumes the maximum (8mb) is 
  used.
  So avoid this range while allocating.
  
  This solves a bunch of FIFO underruns that happen if you end up
  putting the CFB in that memory range. On my machine, with 32mb of
  stolen, I need a 2560x1440 mode for that.
 
 If the restriction applies only to FBC, apply it to only fbc.

That's what the patch is doing. The part where we set the unusual
start/end is at intel_fbc.c:find_compression_threshold(). Everything
else should be behaving just as before.

Maybe you're being confused by the addition of the stolen_usable_size
variable.

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

Re: [Intel-gfx] [PATCH 08/16] drm/i915: avoid the last 8mb of stolen on BDW/SKL

2015-08-15 Thread Chris Wilson 
On Fri, Aug 14, 2015 at 06:34:13PM -0300, Paulo Zanoni wrote:
 The FBC hardware for these platforms doesn't have access to the
 bios_reserved range, so it always assumes the maximum (8mb) is used.
 So avoid this range while allocating.
 
 This solves a bunch of FIFO underruns that happen if you end up
 putting the CFB in that memory range. On my machine, with 32mb of
 stolen, I need a 2560x1440 mode for that.

If the restriction applies only to FBC, apply it to only fbc.
-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 08/16] drm/i915: avoid the last 8mb of stolen on BDW/SKL

2015-08-14 Thread Paulo Zanoni 
The FBC hardware for these platforms doesn't have access to the
bios_reserved range, so it always assumes the maximum (8mb) is used.
So avoid this range while allocating.

This solves a bunch of FIFO underruns that happen if you end up
putting the CFB in that memory range. On my machine, with 32mb of
stolen, I need a 2560x1440 mode for that.

Testcase: igt/kms_frontbuffer_tracking/fbc-* (given the right setup)
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h|  4 
 drivers/gpu/drm/i915/i915_gem_gtt.h|  1 +
 drivers/gpu/drm/i915/i915_gem_stolen.c | 26 +++---
 drivers/gpu/drm/i915/intel_fbc.c   | 16 ++--
 4 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e74a844..f44d101 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3159,6 +3159,10 @@ static inline void i915_gem_chipset_flush(struct 
drm_device *dev)
 int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
struct drm_mm_node *node, u64 size,
unsigned alignment);
+int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
+struct drm_mm_node *node, u64 size,
+unsigned alignment, u64 start,
+u64 end);
 void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
 struct drm_mm_node *node);
 int i915_gem_init_stolen(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8275007..96ebb98 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -341,6 +341,7 @@ struct i915_gtt {
struct i915_address_space base;
 
size_t stolen_size; /* Total size of stolen memory */
+   size_t stolen_usable_size;  /* Total size minus BIOS reserved */
u64 mappable_end;   /* End offset that we can CPU map */
struct io_mapping *mappable;/* Mapping to our CPU mappable region */
phys_addr_t mappable_base;  /* PA of our GMADR */
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index a36cb95..824334d 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -42,9 +42,9 @@
  * for is a boon.
  */
 
-int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
-   struct drm_mm_node *node, u64 size,
-   unsigned alignment)
+int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
+struct drm_mm_node *node, u64 size,
+unsigned alignment, u64 start, u64 end)
 {
int ret;
 
@@ -52,13 +52,23 @@ int i915_gem_stolen_insert_node(struct drm_i915_private 
*dev_priv,
return -ENODEV;
 
mutex_lock(dev_priv-mm.stolen_lock);
-   ret = drm_mm_insert_node(dev_priv-mm.stolen, node, size, alignment,
-DRM_MM_SEARCH_DEFAULT);
+   ret = drm_mm_insert_node_in_range(dev_priv-mm.stolen, node, size,
+ alignment, start, end,
+ DRM_MM_SEARCH_DEFAULT);
mutex_unlock(dev_priv-mm.stolen_lock);
 
return ret;
 }
 
+int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
+   struct drm_mm_node *node, u64 size,
+   unsigned alignment)
+{
+   return i915_gem_stolen_insert_node_in_range(dev_priv, node, size,
+   alignment, 0,
+   dev_priv-gtt.stolen_usable_size);
+}
+
 void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
 struct drm_mm_node *node)
 {
@@ -352,9 +362,11 @@ int i915_gem_init_stolen(struct drm_device *dev)
  dev_priv-gtt.stolen_size  10,
  (dev_priv-gtt.stolen_size - reserved_total)  10);
 
+   dev_priv-gtt.stolen_usable_size = dev_priv-gtt.stolen_size -
+  reserved_total;
+
/* Basic memrange allocator for stolen space */
-   drm_mm_init(dev_priv-mm.stolen, 0, dev_priv-gtt.stolen_size -
-   reserved_total);
+   drm_mm_init(dev_priv-mm.stolen, 0, dev_priv-gtt.stolen_usable_size);
 
return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 28e569c..9fd7b74 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -570,6 +570,16 @@ static int find_compression_threshold(struct