| Message ID | 1464972953-2726-11-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 03, 2016 at 05:55:25PM +0100, Chris Wilson wrote: > Since i915_gem_obj_ggtt_pin() is an idiom breaking curry function for > i915_gem_object_ggtt_pin(), spare us the confustion and remove it. > Removing it now simplifies later patches to change the i915_vma_pin() > (and friends) interface. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Diff looks like accidentally squashed in speed-up to help gcc along with bitfields in vma. Needs to be unsquashed. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.h | 35 ++++++++------------- > drivers/gpu/drm/i915/i915_gem.c | 46 +++++++++++++-------------- > drivers/gpu/drm/i915/i915_gem_context.c | 5 ++- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 +++--- > drivers/gpu/drm/i915/i915_gem_gtt.h | 47 +++++++++++++++------------- > drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +- > drivers/gpu/drm/i915/i915_guc_submission.c | 4 +-- > drivers/gpu/drm/i915/intel_guc_loader.c | 2 +- > drivers/gpu/drm/i915/intel_lrc.c | 8 +++-- > drivers/gpu/drm/i915/intel_overlay.c | 3 +- > drivers/gpu/drm/i915/intel_ringbuffer.c | 16 +++++----- > 11 files changed, 89 insertions(+), 89 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f537d8fc5e0f..861d132b2fe4 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2934,32 +2934,32 @@ void i915_gem_free_object(struct drm_gem_object *obj); > int __must_check > i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags); > /* Flags used by pin/bind&friends. */ > -#define PIN_MAPPABLE (1<<0) > -#define PIN_NONBLOCK (1<<1) > -#define PIN_GLOBAL (1<<2) > -#define PIN_OFFSET_BIAS (1<<3) > -#define PIN_USER (1<<4) > -#define PIN_UPDATE (1<<5) > -#define PIN_ZONE_4G (1<<6) > -#define PIN_HIGH (1<<7) > -#define PIN_OFFSET_FIXED (1<<8) > +#define PIN_GLOBAL (1<<0) > +#define PIN_USER (1<<1) > +#define PIN_UPDATE (1<<2) > +#define PIN_MAPPABLE (1<<3) > +#define PIN_ZONE_4G (1<<4) > +#define PIN_NONBLOCK (1<<5) > +#define PIN_HIGH (1<<6) > +#define PIN_OFFSET_BIAS (1<<7) > +#define PIN_OFFSET_FIXED (1<<8) > #define PIN_OFFSET_MASK (~4095) > > static inline void __i915_vma_pin(struct i915_vma *vma) > { > GEM_BUG_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT); > - vma->pin_count++; > + vma->flags++; > } > > static inline bool i915_vma_is_pinned(struct i915_vma *vma) > { > - return vma->pin_count; > + return vma->flags & DRM_I915_GEM_OBJECT_MAX_PIN_COUNT; > } > > static inline void __i915_vma_unpin(struct i915_vma *vma) > { > GEM_BUG_ON(!i915_vma_is_pinned(vma)); > - vma->pin_count--; > + vma->flags--; > } > > static inline void i915_vma_unpin(struct i915_vma *vma) > @@ -2972,7 +2972,7 @@ int __must_check > i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj, > const struct i915_ggtt_view *view, > uint64_t size, > - uint32_t alignment, > + uint64_t alignment, > uint64_t flags); > > int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > @@ -3223,15 +3223,6 @@ static inline bool i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *obj) > unsigned long > i915_gem_obj_ggtt_size(struct drm_i915_gem_object *obj); > > -static inline int __must_check > -i915_gem_obj_ggtt_pin(struct drm_i915_gem_object *obj, > - uint32_t alignment, > - unsigned flags) > -{ > - return i915_gem_object_ggtt_pin(obj, &i915_ggtt_view_normal, > - 0, alignment, flags); > -} > - > void i915_gem_object_ggtt_unpin_view(struct drm_i915_gem_object *obj, > const struct i915_ggtt_view *view); > static inline void > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 71a32a9f9858..53776a071ce7 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -772,7 +772,9 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, > char __user *user_data; > int page_offset, page_length, ret; > > - ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE | PIN_NONBLOCK); > + ret = i915_gem_object_ggtt_pin(obj, NULL, > + 0, 0, > + PIN_MAPPABLE | PIN_NONBLOCK); > if (ret) > goto out; > > @@ -3408,32 +3410,35 @@ void __i915_vma_set_map_and_fenceable(struct i915_vma *vma) > int > i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags) > { > - unsigned bound = vma->bound; > + unsigned bound; > int ret; > > GEM_BUG_ON((flags & (PIN_GLOBAL | PIN_USER)) == 0); > GEM_BUG_ON((flags & PIN_GLOBAL) && !vma->is_ggtt); > > - if (WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)) > - return -EBUSY; > - > /* Pin early to prevent the shrinker/eviction logic from destroying > * our vma as we insert and bind. > */ > - __i915_vma_pin(vma); > + bound = vma->flags++; > + if (WARN_ON((bound & 0xf) == (DRM_I915_GEM_OBJECT_MAX_PIN_COUNT-1))) { > + ret = -EBUSY; > + goto err; > + } > > - if (!bound) { > + if ((bound & 0xff) == 0) { > ret = i915_vma_insert(vma, size, alignment, flags); > if (ret) > goto err; > } > > - ret = i915_vma_bind(vma, vma->obj->cache_level, flags); > - if (ret) > - goto err; > + if (~(bound >> 4) & (flags & (GLOBAL_BIND | LOCAL_BIND))) { > + ret = i915_vma_bind(vma, vma->obj->cache_level, flags); > + if (ret) > + goto err; > > - if ((bound ^ vma->bound) & GLOBAL_BIND) > - __i915_vma_set_map_and_fenceable(vma); > + if ((bound ^ vma->flags) & (GLOBAL_BIND << 4)) > + __i915_vma_set_map_and_fenceable(vma); > + } > > GEM_BUG_ON(i915_vma_misplaced(vma, size, alignment, flags)); > return 0; > @@ -3447,13 +3452,14 @@ int > i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj, > const struct i915_ggtt_view *view, > uint64_t size, > - uint32_t alignment, > + uint64_t alignment, > uint64_t flags) > { > struct i915_vma *vma; > int ret; > > - BUG_ON(!view); > + if (view == NULL) > + view = &i915_ggtt_view_normal; > > vma = i915_gem_obj_lookup_or_create_ggtt_vma(obj, view); > if (IS_ERR(vma)) > @@ -3465,11 +3471,11 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj, > > WARN(vma->pin_count, > "bo is already pinned in ggtt with incorrect alignment:" > - " offset=%08x %08x, req.alignment=%x, req.map_and_fenceable=%d," > + " offset=%08x %08x, req.alignment=%llx, req.map_and_fenceable=%d," > " obj->map_and_fenceable=%d\n", > upper_32_bits(vma->node.start), > lower_32_bits(vma->node.start), > - alignment, > + (long long)alignment, > !!(flags & PIN_MAPPABLE), > obj->map_and_fenceable); > ret = i915_vma_unbind(vma); > @@ -3484,13 +3490,7 @@ void > i915_gem_object_ggtt_unpin_view(struct drm_i915_gem_object *obj, > const struct i915_ggtt_view *view) > { > - struct i915_vma *vma = i915_gem_obj_to_ggtt_view(obj, view); > - > - GEM_BUG_ON(!vma); > - WARN_ON(i915_vma_is_pinned(vma)); > - WARN_ON(!i915_gem_obj_ggtt_bound_view(obj, view)); > - > - __i915_vma_unpin(vma); > + i915_vma_unpin(i915_gem_obj_to_ggtt_view(obj, view)); > } > > int > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 5ed91406d4e9..c9b8c2c62828 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -722,9 +722,8 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) > return 0; > > /* Trying to pin first makes error handling easier. */ > - ret = i915_gem_obj_ggtt_pin(to->engine[RCS].state, > - to->ggtt_alignment, > - 0); > + ret = i915_gem_object_ggtt_pin(to->engine[RCS].state, NULL, 0, > + to->ggtt_alignment, 0); > if (ret) > return ret; > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index cc9c0e4073ff..69bf73b51df9 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -34,10 +34,10 @@ > #include <linux/dma_remapping.h> > #include <linux/uaccess.h> > > -#define __EXEC_OBJECT_HAS_PIN (1<<31) > -#define __EXEC_OBJECT_HAS_FENCE (1<<30) > -#define __EXEC_OBJECT_NEEDS_MAP (1<<29) > -#define __EXEC_OBJECT_NEEDS_BIAS (1<<28) > +#define __EXEC_OBJECT_HAS_PIN (1U<<31) > +#define __EXEC_OBJECT_HAS_FENCE (1U<<30) > +#define __EXEC_OBJECT_NEEDS_MAP (1U<<29) > +#define __EXEC_OBJECT_NEEDS_BIAS (1U<<28) > > #define BATCH_OFFSET_BIAS (256*1024) > > @@ -1263,7 +1263,7 @@ i915_gem_execbuffer_parse(struct intel_engine_cs *engine, > if (ret) > goto err; > > - ret = i915_gem_obj_ggtt_pin(shadow_batch_obj, 0, 0); > + ret = i915_gem_object_ggtt_pin(shadow_batch_obj, NULL, 0, 0, 0); > if (ret) > goto err; > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > index 2bd8ec7e1948..5655358a60e1 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -184,13 +184,30 @@ struct i915_vma { > > struct i915_gem_active last_read[I915_NUM_ENGINES]; > > - /** Flags and address space this VMA is bound to */ > + union { > + struct { > + /** > + * 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. */ > + unsigned int pin_count : 4; > +#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf > + > + /** Flags and address space this VMA is bound to */ > #define GLOBAL_BIND (1<<0) > #define LOCAL_BIND (1<<1) > - unsigned int bound : 4; > - unsigned int active : I915_NUM_ENGINES; > - bool is_ggtt : 1; > - bool closed : 1; > + unsigned int bound : 4; > + unsigned int active : I915_NUM_ENGINES; > + bool is_ggtt : 1; > + bool closed : 1; > + }; > + unsigned int flags; > + }; > > /** > * Support different GGTT views into the same object. > @@ -215,39 +232,27 @@ struct i915_vma { > struct hlist_node exec_node; > unsigned long exec_handle; > struct drm_i915_gem_exec_object2 *exec_entry; > - > - /** > - * 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. */ > - unsigned int pin_count:4; > -#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf > }; > > static inline bool i915_vma_is_active(const struct i915_vma *vma) > { > - return vma->active; > + return vma->flags & (((1 << I915_NUM_ENGINES) - 1) << 8); > } > > static inline void i915_vma_set_active(struct i915_vma *vma, unsigned engine) > { > - vma->active |= 1 << engine; > + vma->flags |= 0x100 << engine; > } > > static inline void i915_vma_unset_active(struct i915_vma *vma, unsigned engine) > { > - vma->active &= ~(1 << engine); > + vma->flags &= ~(0x100 << engine); > } > > static inline bool i915_vma_has_active_engine(const struct i915_vma *vma, > unsigned engine) > { > - return vma->active & (1 << engine); > + return vma->flags & (0x100 << engine); > } > > struct i915_page_dma { > diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c > index c0abe9a2210f..4cf82697b3db 100644 > --- a/drivers/gpu/drm/i915/i915_gem_render_state.c > +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c > @@ -72,7 +72,7 @@ static int render_state_init(struct render_state *so, > if (IS_ERR(so->obj)) > return PTR_ERR(so->obj); > > - ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0); > + ret = i915_gem_object_ggtt_pin(so->obj, NULL, 0, 0, 0); > if (ret) > goto free_gem; > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > index cc4792df249d..63ef34c78494 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -613,8 +613,8 @@ static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev, > return NULL; > } > > - if (i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, > - PIN_OFFSET_BIAS | GUC_WOPCM_TOP)) { > + if (i915_gem_object_ggtt_pin(obj, NULL, 0, PAGE_SIZE, > + PIN_OFFSET_BIAS | GUC_WOPCM_TOP)) { > i915_gem_object_put(obj); > return NULL; > } > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c > index 74a5f11a5689..be93b458968a 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -321,7 +321,7 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv) > return ret; > } > > - ret = i915_gem_obj_ggtt_pin(guc_fw->guc_fw_obj, 0, 0); > + ret = i915_gem_object_ggtt_pin(guc_fw->guc_fw_obj, NULL, 0, 0, 0); > if (ret) { > DRM_DEBUG_DRIVER("pin failed %d\n", ret); > return ret; > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 964108cbb9c0..6cdc421fdc37 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -774,8 +774,9 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx, > if (ce->pin_count++) > return 0; > > - ret = i915_gem_obj_ggtt_pin(ce->state, GEN8_LR_CONTEXT_ALIGN, > - PIN_OFFSET_BIAS | GUC_WOPCM_TOP); > + ret = i915_gem_object_ggtt_pin(ce->state, NULL, > + 0, GEN8_LR_CONTEXT_ALIGN, > + PIN_OFFSET_BIAS | GUC_WOPCM_TOP); > if (ret) > goto err; > > @@ -1154,7 +1155,8 @@ static int lrc_setup_wa_ctx_obj(struct intel_engine_cs *engine, u32 size) > return ret; > } > > - ret = i915_gem_obj_ggtt_pin(engine->wa_ctx.obj, PAGE_SIZE, 0); > + ret = i915_gem_object_ggtt_pin(engine->wa_ctx.obj, NULL, > + 0, PAGE_SIZE, 0); > if (ret) { > DRM_DEBUG_DRIVER("pin LRC WA ctx backing obj failed: %d\n", > ret); > diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c > index 5f645ad2babd..9b0fb7e23cbb 100644 > --- a/drivers/gpu/drm/i915/intel_overlay.c > +++ b/drivers/gpu/drm/i915/intel_overlay.c > @@ -1412,7 +1412,8 @@ void intel_setup_overlay(struct drm_i915_private *dev_priv) > } > overlay->flip_addr = reg_bo->phys_handle->busaddr; > } else { > - ret = i915_gem_obj_ggtt_pin(reg_bo, PAGE_SIZE, PIN_MAPPABLE); > + ret = i915_gem_object_ggtt_pin(reg_bo, NULL, > + 0, PAGE_SIZE, PIN_MAPPABLE); > if (ret) { > DRM_ERROR("failed to pin overlay register bo\n"); > goto out_free_bo; > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index d63e4fdc60de..f86039455c5a 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -648,7 +648,7 @@ int intel_init_pipe_control(struct intel_engine_cs *engine, int size) > goto err; > } > > - ret = i915_gem_obj_ggtt_pin(obj, 4096, PIN_HIGH); > + ret = i915_gem_object_ggtt_pin(obj, NULL, 0, 4096, PIN_HIGH); > if (ret) > goto err_unref; > > @@ -1816,7 +1816,7 @@ static int init_status_page(struct intel_engine_cs *engine) > * actualy map it). > */ > flags |= PIN_MAPPABLE; > - ret = i915_gem_obj_ggtt_pin(obj, 4096, flags); > + ret = i915_gem_object_ggtt_pin(obj, NULL, 0, 4096, flags); > if (ret) { > err_unref: > i915_gem_object_put(obj); > @@ -1863,7 +1863,7 @@ int intel_ring_pin(struct intel_ring *ring) > int ret; > > if (HAS_LLC(dev_priv) && !obj->stolen) { > - ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, flags); > + ret = i915_gem_object_ggtt_pin(obj, NULL, 0, PAGE_SIZE, flags); > if (ret) > return ret; > > @@ -1877,8 +1877,8 @@ int intel_ring_pin(struct intel_ring *ring) > goto err_unpin; > } > } else { > - ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, > - flags | PIN_MAPPABLE); > + ret = i915_gem_object_ggtt_pin(obj, NULL, 0, PAGE_SIZE, > + flags | PIN_MAPPABLE); > if (ret) > return ret; > > @@ -2007,7 +2007,8 @@ static int intel_ring_context_pin(struct i915_gem_context *ctx, > return 0; > > if (ce->state) { > - ret = i915_gem_obj_ggtt_pin(ce->state, ctx->ggtt_alignment, 0); > + ret = i915_gem_object_ggtt_pin(ce->state, NULL, 0, > + ctx->ggtt_alignment, 0); > if (ret) > goto error; > } > @@ -2574,7 +2575,8 @@ int intel_init_render_ring_buffer(struct drm_device *dev) > i915.semaphores = 0; > } else { > i915_gem_object_set_cache_level(obj, I915_CACHE_LLC); > - ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_NONBLOCK); > + ret = i915_gem_object_ggtt_pin(obj, NULL, > + 0, 0, 0); > if (ret != 0) { > i915_gem_object_put(obj); > DRM_ERROR("Failed to pin semaphore bo. Disabling semaphores\n"); > -- > 2.8.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Jun 08, 2016 at 11:43:57AM +0200, Daniel Vetter wrote: > On Fri, Jun 03, 2016 at 05:55:25PM +0100, Chris Wilson wrote: > > Since i915_gem_obj_ggtt_pin() is an idiom breaking curry function for > > i915_gem_object_ggtt_pin(), spare us the confustion and remove it. > > Removing it now simplifies later patches to change the i915_vma_pin() > > (and friends) interface. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Diff looks like accidentally squashed in speed-up to help gcc along with > bitfields in vma. Needs to be unsquashed. That change was made a few months ago, at least outside of reflog's history. I guess I got fed up of having a few patches doing very small overlapping tasks of changing the function prototypes. -Chris
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f537d8fc5e0f..861d132b2fe4 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2934,32 +2934,32 @@ void i915_gem_free_object(struct drm_gem_object *obj); int __must_check i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags); /* Flags used by pin/bind&friends. */ -#define PIN_MAPPABLE (1<<0) -#define PIN_NONBLOCK (1<<1) -#define PIN_GLOBAL (1<<2) -#define PIN_OFFSET_BIAS (1<<3) -#define PIN_USER (1<<4) -#define PIN_UPDATE (1<<5) -#define PIN_ZONE_4G (1<<6) -#define PIN_HIGH (1<<7) -#define PIN_OFFSET_FIXED (1<<8) +#define PIN_GLOBAL (1<<0) +#define PIN_USER (1<<1) +#define PIN_UPDATE (1<<2) +#define PIN_MAPPABLE (1<<3) +#define PIN_ZONE_4G (1<<4) +#define PIN_NONBLOCK (1<<5) +#define PIN_HIGH (1<<6) +#define PIN_OFFSET_BIAS (1<<7) +#define PIN_OFFSET_FIXED (1<<8) #define PIN_OFFSET_MASK (~4095) static inline void __i915_vma_pin(struct i915_vma *vma) { GEM_BUG_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT); - vma->pin_count++; + vma->flags++; } static inline bool i915_vma_is_pinned(struct i915_vma *vma) { - return vma->pin_count; + return vma->flags & DRM_I915_GEM_OBJECT_MAX_PIN_COUNT; } static inline void __i915_vma_unpin(struct i915_vma *vma) { GEM_BUG_ON(!i915_vma_is_pinned(vma)); - vma->pin_count--; + vma->flags--; } static inline void i915_vma_unpin(struct i915_vma *vma) @@ -2972,7 +2972,7 @@ int __must_check i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj, const struct i915_ggtt_view *view, uint64_t size, - uint32_t alignment, + uint64_t alignment, uint64_t flags); int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, @@ -3223,15 +3223,6 @@ static inline bool i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *obj) unsigned long i915_gem_obj_ggtt_size(struct drm_i915_gem_object *obj); -static inline int __must_check -i915_gem_obj_ggtt_pin(struct drm_i915_gem_object *obj, - uint32_t alignment, - unsigned flags) -{ - return i915_gem_object_ggtt_pin(obj, &i915_ggtt_view_normal, - 0, alignment, flags); -} - void i915_gem_object_ggtt_unpin_view(struct drm_i915_gem_object *obj, const struct i915_ggtt_view *view); static inline void diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 71a32a9f9858..53776a071ce7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -772,7 +772,9 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, char __user *user_data; int page_offset, page_length, ret; - ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE | PIN_NONBLOCK); + ret = i915_gem_object_ggtt_pin(obj, NULL, + 0, 0, + PIN_MAPPABLE | PIN_NONBLOCK); if (ret) goto out; @@ -3408,32 +3410,35 @@ void __i915_vma_set_map_and_fenceable(struct i915_vma *vma) int i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags) { - unsigned bound = vma->bound; + unsigned bound; int ret; GEM_BUG_ON((flags & (PIN_GLOBAL | PIN_USER)) == 0); GEM_BUG_ON((flags & PIN_GLOBAL) && !vma->is_ggtt); - if (WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)) - return -EBUSY; - /* Pin early to prevent the shrinker/eviction logic from destroying * our vma as we insert and bind. */ - __i915_vma_pin(vma); + bound = vma->flags++; + if (WARN_ON((bound & 0xf) == (DRM_I915_GEM_OBJECT_MAX_PIN_COUNT-1))) { + ret = -EBUSY; + goto err; + } - if (!bound) { + if ((bound & 0xff) == 0) { ret = i915_vma_insert(vma, size, alignment, flags); if (ret) goto err; } - ret = i915_vma_bind(vma, vma->obj->cache_level, flags); - if (ret) - goto err; + if (~(bound >> 4) & (flags & (GLOBAL_BIND | LOCAL_BIND))) { + ret = i915_vma_bind(vma, vma->obj->cache_level, flags); + if (ret) + goto err; - if ((bound ^ vma->bound) & GLOBAL_BIND) - __i915_vma_set_map_and_fenceable(vma); + if ((bound ^ vma->flags) & (GLOBAL_BIND << 4)) + __i915_vma_set_map_and_fenceable(vma); + } GEM_BUG_ON(i915_vma_misplaced(vma, size, alignment, flags)); return 0; @@ -3447,13 +3452,14 @@ int i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj, const struct i915_ggtt_view *view, uint64_t size, - uint32_t alignment, + uint64_t alignment, uint64_t flags) { struct i915_vma *vma; int ret; - BUG_ON(!view); + if (view == NULL) + view = &i915_ggtt_view_normal; vma = i915_gem_obj_lookup_or_create_ggtt_vma(obj, view); if (IS_ERR(vma)) @@ -3465,11 +3471,11 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj, WARN(vma->pin_count, "bo is already pinned in ggtt with incorrect alignment:" - " offset=%08x %08x, req.alignment=%x, req.map_and_fenceable=%d," + " offset=%08x %08x, req.alignment=%llx, req.map_and_fenceable=%d," " obj->map_and_fenceable=%d\n", upper_32_bits(vma->node.start), lower_32_bits(vma->node.start), - alignment, + (long long)alignment, !!(flags & PIN_MAPPABLE), obj->map_and_fenceable); ret = i915_vma_unbind(vma); @@ -3484,13 +3490,7 @@ void i915_gem_object_ggtt_unpin_view(struct drm_i915_gem_object *obj, const struct i915_ggtt_view *view) { - struct i915_vma *vma = i915_gem_obj_to_ggtt_view(obj, view); - - GEM_BUG_ON(!vma); - WARN_ON(i915_vma_is_pinned(vma)); - WARN_ON(!i915_gem_obj_ggtt_bound_view(obj, view)); - - __i915_vma_unpin(vma); + i915_vma_unpin(i915_gem_obj_to_ggtt_view(obj, view)); } int diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 5ed91406d4e9..c9b8c2c62828 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -722,9 +722,8 @@ static int do_rcs_switch(struct drm_i915_gem_request *req) return 0; /* Trying to pin first makes error handling easier. */ - ret = i915_gem_obj_ggtt_pin(to->engine[RCS].state, - to->ggtt_alignment, - 0); + ret = i915_gem_object_ggtt_pin(to->engine[RCS].state, NULL, 0, + to->ggtt_alignment, 0); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index cc9c0e4073ff..69bf73b51df9 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -34,10 +34,10 @@ #include <linux/dma_remapping.h> #include <linux/uaccess.h> -#define __EXEC_OBJECT_HAS_PIN (1<<31) -#define __EXEC_OBJECT_HAS_FENCE (1<<30) -#define __EXEC_OBJECT_NEEDS_MAP (1<<29) -#define __EXEC_OBJECT_NEEDS_BIAS (1<<28) +#define __EXEC_OBJECT_HAS_PIN (1U<<31) +#define __EXEC_OBJECT_HAS_FENCE (1U<<30) +#define __EXEC_OBJECT_NEEDS_MAP (1U<<29) +#define __EXEC_OBJECT_NEEDS_BIAS (1U<<28) #define BATCH_OFFSET_BIAS (256*1024) @@ -1263,7 +1263,7 @@ i915_gem_execbuffer_parse(struct intel_engine_cs *engine, if (ret) goto err; - ret = i915_gem_obj_ggtt_pin(shadow_batch_obj, 0, 0); + ret = i915_gem_object_ggtt_pin(shadow_batch_obj, NULL, 0, 0, 0); if (ret) goto err; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 2bd8ec7e1948..5655358a60e1 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -184,13 +184,30 @@ struct i915_vma { struct i915_gem_active last_read[I915_NUM_ENGINES]; - /** Flags and address space this VMA is bound to */ + union { + struct { + /** + * 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. */ + unsigned int pin_count : 4; +#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf + + /** Flags and address space this VMA is bound to */ #define GLOBAL_BIND (1<<0) #define LOCAL_BIND (1<<1) - unsigned int bound : 4; - unsigned int active : I915_NUM_ENGINES; - bool is_ggtt : 1; - bool closed : 1; + unsigned int bound : 4; + unsigned int active : I915_NUM_ENGINES; + bool is_ggtt : 1; + bool closed : 1; + }; + unsigned int flags; + }; /** * Support different GGTT views into the same object. @@ -215,39 +232,27 @@ struct i915_vma { struct hlist_node exec_node; unsigned long exec_handle; struct drm_i915_gem_exec_object2 *exec_entry; - - /** - * 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. */ - unsigned int pin_count:4; -#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf }; static inline bool i915_vma_is_active(const struct i915_vma *vma) { - return vma->active; + return vma->flags & (((1 << I915_NUM_ENGINES) - 1) << 8); } static inline void i915_vma_set_active(struct i915_vma *vma, unsigned engine) { - vma->active |= 1 << engine; + vma->flags |= 0x100 << engine; } static inline void i915_vma_unset_active(struct i915_vma *vma, unsigned engine) { - vma->active &= ~(1 << engine); + vma->flags &= ~(0x100 << engine); } static inline bool i915_vma_has_active_engine(const struct i915_vma *vma, unsigned engine) { - return vma->active & (1 << engine); + return vma->flags & (0x100 << engine); } struct i915_page_dma { diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c index c0abe9a2210f..4cf82697b3db 100644 --- a/drivers/gpu/drm/i915/i915_gem_render_state.c +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c @@ -72,7 +72,7 @@ static int render_state_init(struct render_state *so, if (IS_ERR(so->obj)) return PTR_ERR(so->obj); - ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0); + ret = i915_gem_object_ggtt_pin(so->obj, NULL, 0, 0, 0); if (ret) goto free_gem; diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index cc4792df249d..63ef34c78494 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -613,8 +613,8 @@ static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev, return NULL; } - if (i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, - PIN_OFFSET_BIAS | GUC_WOPCM_TOP)) { + if (i915_gem_object_ggtt_pin(obj, NULL, 0, PAGE_SIZE, + PIN_OFFSET_BIAS | GUC_WOPCM_TOP)) { i915_gem_object_put(obj); return NULL; } diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 74a5f11a5689..be93b458968a 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -321,7 +321,7 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv) return ret; } - ret = i915_gem_obj_ggtt_pin(guc_fw->guc_fw_obj, 0, 0); + ret = i915_gem_object_ggtt_pin(guc_fw->guc_fw_obj, NULL, 0, 0, 0); if (ret) { DRM_DEBUG_DRIVER("pin failed %d\n", ret); return ret; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 964108cbb9c0..6cdc421fdc37 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -774,8 +774,9 @@ static int intel_lr_context_pin(struct i915_gem_context *ctx, if (ce->pin_count++) return 0; - ret = i915_gem_obj_ggtt_pin(ce->state, GEN8_LR_CONTEXT_ALIGN, - PIN_OFFSET_BIAS | GUC_WOPCM_TOP); + ret = i915_gem_object_ggtt_pin(ce->state, NULL, + 0, GEN8_LR_CONTEXT_ALIGN, + PIN_OFFSET_BIAS | GUC_WOPCM_TOP); if (ret) goto err; @@ -1154,7 +1155,8 @@ static int lrc_setup_wa_ctx_obj(struct intel_engine_cs *engine, u32 size) return ret; } - ret = i915_gem_obj_ggtt_pin(engine->wa_ctx.obj, PAGE_SIZE, 0); + ret = i915_gem_object_ggtt_pin(engine->wa_ctx.obj, NULL, + 0, PAGE_SIZE, 0); if (ret) { DRM_DEBUG_DRIVER("pin LRC WA ctx backing obj failed: %d\n", ret); diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c index 5f645ad2babd..9b0fb7e23cbb 100644 --- a/drivers/gpu/drm/i915/intel_overlay.c +++ b/drivers/gpu/drm/i915/intel_overlay.c @@ -1412,7 +1412,8 @@ void intel_setup_overlay(struct drm_i915_private *dev_priv) } overlay->flip_addr = reg_bo->phys_handle->busaddr; } else { - ret = i915_gem_obj_ggtt_pin(reg_bo, PAGE_SIZE, PIN_MAPPABLE); + ret = i915_gem_object_ggtt_pin(reg_bo, NULL, + 0, PAGE_SIZE, PIN_MAPPABLE); if (ret) { DRM_ERROR("failed to pin overlay register bo\n"); goto out_free_bo; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index d63e4fdc60de..f86039455c5a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -648,7 +648,7 @@ int intel_init_pipe_control(struct intel_engine_cs *engine, int size) goto err; } - ret = i915_gem_obj_ggtt_pin(obj, 4096, PIN_HIGH); + ret = i915_gem_object_ggtt_pin(obj, NULL, 0, 4096, PIN_HIGH); if (ret) goto err_unref; @@ -1816,7 +1816,7 @@ static int init_status_page(struct intel_engine_cs *engine) * actualy map it). */ flags |= PIN_MAPPABLE; - ret = i915_gem_obj_ggtt_pin(obj, 4096, flags); + ret = i915_gem_object_ggtt_pin(obj, NULL, 0, 4096, flags); if (ret) { err_unref: i915_gem_object_put(obj); @@ -1863,7 +1863,7 @@ int intel_ring_pin(struct intel_ring *ring) int ret; if (HAS_LLC(dev_priv) && !obj->stolen) { - ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, flags); + ret = i915_gem_object_ggtt_pin(obj, NULL, 0, PAGE_SIZE, flags); if (ret) return ret; @@ -1877,8 +1877,8 @@ int intel_ring_pin(struct intel_ring *ring) goto err_unpin; } } else { - ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, - flags | PIN_MAPPABLE); + ret = i915_gem_object_ggtt_pin(obj, NULL, 0, PAGE_SIZE, + flags | PIN_MAPPABLE); if (ret) return ret; @@ -2007,7 +2007,8 @@ static int intel_ring_context_pin(struct i915_gem_context *ctx, return 0; if (ce->state) { - ret = i915_gem_obj_ggtt_pin(ce->state, ctx->ggtt_alignment, 0); + ret = i915_gem_object_ggtt_pin(ce->state, NULL, 0, + ctx->ggtt_alignment, 0); if (ret) goto error; } @@ -2574,7 +2575,8 @@ int intel_init_render_ring_buffer(struct drm_device *dev) i915.semaphores = 0; } else { i915_gem_object_set_cache_level(obj, I915_CACHE_LLC); - ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_NONBLOCK); + ret = i915_gem_object_ggtt_pin(obj, NULL, + 0, 0, 0); if (ret != 0) { i915_gem_object_put(obj); DRM_ERROR("Failed to pin semaphore bo. Disabling semaphores\n");
Since i915_gem_obj_ggtt_pin() is an idiom breaking curry function for i915_gem_object_ggtt_pin(), spare us the confustion and remove it. Removing it now simplifies later patches to change the i915_vma_pin() (and friends) interface. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_drv.h | 35 ++++++++------------- drivers/gpu/drm/i915/i915_gem.c | 46 +++++++++++++-------------- drivers/gpu/drm/i915/i915_gem_context.c | 5 ++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 +++--- drivers/gpu/drm/i915/i915_gem_gtt.h | 47 +++++++++++++++------------- drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +- drivers/gpu/drm/i915/i915_guc_submission.c | 4 +-- drivers/gpu/drm/i915/intel_guc_loader.c | 2 +- drivers/gpu/drm/i915/intel_lrc.c | 8 +++-- drivers/gpu/drm/i915/intel_overlay.c | 3 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 16 +++++----- 11 files changed, 89 insertions(+), 89 deletions(-)