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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
