[Intel-gfx] [PATCH] Antigcc bitfield bikeshed
Chris Wilson
chris at chris-wilson.co.uk
Wed Jun 17 05:47:57 PDT 2015
Here's an idea I want to float to see if anyone has a better idea.
Daniel is very keen on using READ_ONCE/WRITE_ONCE/ACCESS_ONCE to
document where we play games with memory barriers instead outside of the
usual locks. However, that falls down given that we have a lot of
bitfields and the macros to ensure no compiler trickery cannot handle a
bitfield. One solution is to switch over to using unsigned long flags
and manual bit twiddling. Another is to mix and match between
readibility and speed, using a bitfield for convenience and flags for
when gcc is not helpful. Using flags requires a lot more manual
involvement in the bit layout, and obviously duplicates using a
bitfield. So to try and keep it maintainable, we only want one
definition that is as painless as possible. This is my attempt.
-Chris
P.S. You should see what happens with i915_vma!
http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=nightly&id=b93fd1fbdd7f82a7a045ff7e081907f3ac7ee806
---
drivers/gpu/drm/i915/i915_drv.h | 140 +++++++++++++++++++--------------
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem_tiling.c | 3 +-
3 files changed, 85 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 937f8fe385f5..a47ec76591db 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1987,6 +1987,82 @@ struct drm_i915_gem_object_ops {
#define INTEL_FRONTBUFFER_ALL_MASK(pipe) \
(0xf << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
+#define I915_BO_BIT(T, name, prev, width) \
+ T name:width;
+#define I915_BO_ENUM(T, name, prev, width) \
+ I915_BO_FLAG_SHIFT_##name = I915_BO_FLAG_SHIFT_##prev + I915_BO_FLAG_WIDTH_##prev, \
+ I915_BO_FLAG_WIDTH_##name = width,
+
+#define I915_BO_FLAG_SHIFT___first__ 0
+#define I915_BO_FLAG_WIDTH___first__ 0
+
+#define I915_BO_FLAGS(func) \
+ /** \
+ * This is set if the object is on the active lists (has pending \
+ * rendering and so a non-zero seqno), and is not set if it i s on \
+ * inactive (ready to be unbound) list. \
+ */ \
+ func(unsigned int, active, __first__, I915_NUM_RINGS) \
+ func(unsigned int, active_reference, active, 1) \
+\
+ /** \
+ * This is set if the object has been written to since last bound \
+ * to the GTT \
+ */ \
+ func(unsigned int, dirty, active_reference, 1) \
+\
+ /** \
+ * Fence register bits (if any) for this object. Will be set \
+ * as needed when mapped into the GTT. \
+ * Protected by dev->struct_mutex. \
+ */ \
+ func(signed int, fence_reg, dirty, I915_MAX_NUM_FENCE_BITS) \
+\
+ /** \
+ * Advice: are the backing pages purgeable? \
+ */ \
+ func(unsigned int, madv, fence_reg, 2) \
+\
+ /** \
+ * Current tiling mode for the object. \
+ */ \
+ func(unsigned int, tiling_mode, madv, 2) \
+ /** \
+ * Whether the tiling parameters for the currently associated fence \
+ * register have changed. Note that for the purposes of tracking \
+ * tiling changes we also treat the unfenced register, the register \
+ * slot that the object occupies whilst it executes a fenced \
+ * command (such as BLT on gen2/3), as a "fence". \
+ */ \
+ func(unsigned int, fence_dirty, tiling_mode, 1) \
+\
+ /** \
+ * Whether the current gtt mapping needs to be mappable (and isn't just \
+ * mappable by accident). Track pin and fault separate for a more \
+ * accurate mappable working set. \
+ */ \
+ func(unsigned int, fault_mappable, fence_dirty, 1) \
+\
+ /* \
+ * Is the object to be mapped as read-only to the GPU \
+ * Only honoured if hardware has relevant pte bit \
+ */ \
+ func(unsigned int, gt_ro, fault_mappable, 1) \
+ func(unsigned int, cache_level, gt_ro, 3) \
+ func(unsigned int, cache_dirty, cache_level, 1) \
+\
+ func(unsigned int, has_dma_mapping, cache_dirty, 1) \
+\
+ func(unsigned int, frontbuffer_bits, has_dma_mapping, INTEL_FRONTBUFFER_BITS) \
+ func(unsigned int, vma_ht_bits, frontbuffer_bits, 5)
+
+#define I915_BO_FLAG_MASK(name) (((I915_BO_FLAG_WIDTH_##name<<1) - 1) << I915_BO_FLAG_SHIFT_##name)
+#define I915_BO_FLAG_VALUE(flags, name) (((flags) >> I915_BO_FLAG_SHIFT_##name) & ((I915_BO_FLAG_WIDTH_##name<<1) - 1))
+
+enum {
+ I915_BO_FLAGS(I915_BO_ENUM)
+};
+
struct drm_i915_gem_object {
struct drm_gem_object base;
@@ -2004,64 +2080,12 @@ struct drm_i915_gem_object {
struct list_head batch_pool_link;
struct list_head tmp_link;
- /**
- * This is set if the object is on the active lists (has pending
- * rendering and so a non-zero seqno), and is not set if it i s on
- * inactive (ready to be unbound) list.
- */
- unsigned int active:I915_NUM_RINGS;
- unsigned int active_reference:1;
-
- /**
- * This is set if the object has been written to since last bound
- * to the GTT
- */
- unsigned int dirty:1;
-
- /**
- * Fence register bits (if any) for this object. Will be set
- * as needed when mapped into the GTT.
- * Protected by dev->struct_mutex.
- */
- signed int fence_reg:I915_MAX_NUM_FENCE_BITS;
-
- /**
- * Advice: are the backing pages purgeable?
- */
- unsigned int madv:2;
-
- /**
- * Current tiling mode for the object.
- */
- unsigned int tiling_mode:2;
- /**
- * Whether the tiling parameters for the currently associated fence
- * register have changed. Note that for the purposes of tracking
- * tiling changes we also treat the unfenced register, the register
- * slot that the object occupies whilst it executes a fenced
- * command (such as BLT on gen2/3), as a "fence".
- */
- unsigned int fence_dirty:1;
-
- /**
- * Whether the current gtt mapping needs to be mappable (and isn't just
- * mappable by accident). Track pin and fault separate for a more
- * accurate mappable working set.
- */
- unsigned int fault_mappable:1;
-
- /*
- * Is the object to be mapped as read-only to the GPU
- * Only honoured if hardware has relevant pte bit
- */
- unsigned long gt_ro:1;
- unsigned int cache_level:3;
- unsigned int cache_dirty:1;
-
- unsigned int has_dma_mapping:1;
-
- unsigned int frontbuffer_bits:INTEL_FRONTBUFFER_BITS;
- unsigned int vma_ht_bits:5;
+ union {
+ struct {
+ I915_BO_FLAGS(I915_BO_BIT);
+ };
+ unsigned long flags;
+ };
unsigned int bind_count;
unsigned int pin_display;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b2fc997e8f63..36c99757c3d2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4452,7 +4452,7 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
if (&obj->base == NULL)
return -ENOENT;
- if (obj->active) {
+ if (READ_ONCE(obj->flags) & I915_BO_FLAG_MASK(active)) {
ret = i915_mutex_lock_interruptible(dev);
if (ret)
goto unref;
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 8a2325c1101e..b7c9e6ea3e34 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -454,7 +454,8 @@ i915_gem_get_tiling(struct drm_device *dev, void *data,
if (&obj->base == NULL)
return -ENOENT;
- args->tiling_mode = obj->tiling_mode;
+ args->tiling_mode =
+ I915_BO_FLAG_VALUE(READ_ONCE(obj->flags), tiling_mode);
switch (args->tiling_mode) {
case I915_TILING_X:
args->swizzle_mode = dev_priv->mm.bit_6_swizzle_x;
--
2.1.4
More information about the Intel-gfx
mailing list