On 1/7/2019 03:55, Chris Wilson wrote:
Previously we only accommodated having a vma pinned by a small number of
users, with the maximum being pinned for use by the display engine. As
such, we used a small bitfield only large enough to allow the vma to
be pinned twice (for back/front buffers) in each scanout plane. Keeping
the maximum permissible pin_count small allows us to quickly catch a
potential leak. However, as we want to split a 4096B page into 64
different cachelines and pin each cacheline for use by a different
timeline, we will exceed the current maximum permissible vma->pin_count
and so time has come to enlarge it.

Signed-off-by: Chris Wilson <[email protected]>
---
  drivers/gpu/drm/i915/i915_gem_gtt.h | 26 +++++++++++++-------------
  drivers/gpu/drm/i915/i915_vma.h     | 28 +++++++++-------------------
  2 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index bd679c8c56dd..03ade71b8d9a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -642,19 +642,19 @@ int i915_gem_gtt_insert(struct i915_address_space *vm,
/* Flags used by pin/bind&friends. */
  #define PIN_NONBLOCK          BIT_ULL(0)
-#define PIN_MAPPABLE           BIT_ULL(1)
-#define PIN_ZONE_4G            BIT_ULL(2)
-#define PIN_NONFAULT           BIT_ULL(3)
-#define PIN_NOEVICT            BIT_ULL(4)
-
-#define PIN_MBZ                        BIT_ULL(5) /* I915_VMA_PIN_OVERFLOW */
-#define PIN_GLOBAL             BIT_ULL(6) /* I915_VMA_GLOBAL_BIND */
-#define PIN_USER               BIT_ULL(7) /* I915_VMA_LOCAL_BIND */
-#define PIN_UPDATE             BIT_ULL(8)
-
-#define PIN_HIGH               BIT_ULL(9)
-#define PIN_OFFSET_BIAS                BIT_ULL(10)
-#define PIN_OFFSET_FIXED       BIT_ULL(11)
+#define PIN_NONFAULT           BIT_ULL(1)
+#define PIN_NOEVICT            BIT_ULL(2)
+#define PIN_MAPPABLE           BIT_ULL(3)
+#define PIN_ZONE_4G            BIT_ULL(4)
+#define PIN_HIGH               BIT_ULL(5)
+#define PIN_OFFSET_BIAS                BIT_ULL(6)
+#define PIN_OFFSET_FIXED       BIT_ULL(7)
+
+#define PIN_MBZ                        BIT_ULL(8) /* I915_VMA_PIN_OVERFLOW */
+#define PIN_GLOBAL             BIT_ULL(9) /* I915_VMA_GLOBAL_BIND */
+#define PIN_USER               BIT_ULL(10) /* I915_VMA_LOCAL_BIND */
+#define PIN_UPDATE             BIT_ULL(11)
+
The upper bits need moving to accommodate the larger count. And the HIGH/OFFSET_* fields are not shared with vma-flags so can be moved down with the other pin only flags. But I don't see a reason to shuffle the lower bits around? MAPPABLE to NOEVICT were 1,2,3,4 but are now 3,4,1,2. Is there some semantic meaning to the new order?


  #define PIN_OFFSET_MASK               (-I915_GTT_PAGE_SIZE)
#endif
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 7252abc73d3e..266b226ebef2 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -70,30 +70,20 @@ struct i915_vma {
         */
        unsigned int open_count;
        unsigned long flags;
-       /**
-        * How many users have pinned this object in GTT space. The following
-        * users can each hold at most one reference: pwrite/pread, execbuffer
-        * (objects are not allowed multiple times for the same batchbuffer),
-        * and the framebuffer code. When switching/pageflipping, the
-        * framebuffer code has at most two buffers pinned per crtc.
-        *
-        * In the worst case this is 1 + 1 + 1 + 2*2 = 7. That would fit into 3
-        * bits with absolutely no headroom. So use 4 bits.
-        */
Is it not worth keeping some comment about the maximum pin count being bounded so 8-bits is guaranteed to be sufficient? Also, is the old comment actually valid? Surely modern hardware has more than two CRTCs so the limit of 7 was wrong anyway? Maybe even have a compile time assert that the mask size is greater than max(1 + 1 + 1 + 1 + 2*MAX_CRTC, PAGE_SIZE/CACHELINE_SIZE)?


-#define I915_VMA_PIN_MASK 0xf
-#define I915_VMA_PIN_OVERFLOW  BIT(5)
+#define I915_VMA_PIN_MASK 0xff
+#define I915_VMA_PIN_OVERFLOW  BIT(8)
/** Flags and address space this VMA is bound to */
-#define I915_VMA_GLOBAL_BIND   BIT(6)
-#define I915_VMA_LOCAL_BIND    BIT(7)
+#define I915_VMA_GLOBAL_BIND   BIT(9)
+#define I915_VMA_LOCAL_BIND    BIT(10)
  #define I915_VMA_BIND_MASK (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND | 
I915_VMA_PIN_OVERFLOW)
-#define I915_VMA_GGTT BIT(8)
-#define I915_VMA_CAN_FENCE     BIT(9)
-#define I915_VMA_CLOSED                BIT(10)
-#define I915_VMA_USERFAULT_BIT 11
+#define I915_VMA_GGTT          BIT(11)
+#define I915_VMA_CAN_FENCE     BIT(12)
+#define I915_VMA_CLOSED                BIT(13)
+#define I915_VMA_USERFAULT_BIT 14
  #define I915_VMA_USERFAULT    BIT(I915_VMA_USERFAULT_BIT)
-#define I915_VMA_GGTT_WRITE    BIT(12)
+#define I915_VMA_GGTT_WRITE    BIT(15)
unsigned int active_count;
        struct rb_root active;

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to