| Message ID | 1444285469-8187-5-git-send-email-ankitprasad.r.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 08/10/15 07:24, ankitprasad.r.sharma@intel.com wrote: > From: Chris Wilson <chris at chris-wilson.co.uk> > > If we run out of stolen memory when trying to allocate an object, see if > we can reap enough purgeable objects to free up enough contiguous free > space for the allocation. This is in principle very much like evicting > objects to free up enough contiguous space in the vma when binding > a new object - and you will be forgiven for thinking that the code looks > very similar. > > At the moment, we do not allow userspace to allocate objects in stolen, > so there is neither the memory pressure to trigger stolen eviction nor > any purgeable objects inside the stolen arena. However, this will change > in the near future, and so better management and defragmentation of > stolen memory will become a real issue. > > v2: Remember to remove the drm_mm_node. > > v3: Rebased to the latest drm-intel-nightly (Ankit) > > v4: corrected if-else braces format (Tvrtko/kerneldoc) > > v5: Rebased to the latest drm-intel-nightly (Ankit) > Added a seperate list to maintain purgable objects from stolen memory > region (Chris/Daniel) > > v6: Compiler optimization (merging 2 single loops into one for() loop), > corrected code for object eviction, retire_requests before starting > object eviction (Chris) > > v7: Added kernel doc for i915_gem_object_create_stolen() > > Testcase: igt/gem_stolen > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk> > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 4 +- > drivers/gpu/drm/i915/i915_drv.h | 17 +++- > drivers/gpu/drm/i915/i915_gem.c | 16 ++++ > drivers/gpu/drm/i915/i915_gem_stolen.c | 169 +++++++++++++++++++++++++++++---- > drivers/gpu/drm/i915/intel_pm.c | 4 +- > 5 files changed, 186 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 8797717..13762c1 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -174,7 +174,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) > seq_puts(m, ")"); > } > if (obj->stolen) > - seq_printf(m, " (stolen: %08llx)", obj->stolen->start); > + seq_printf(m, " (stolen: %08llx)", obj->stolen->base.start); > if (obj->pin_display || obj->fault_mappable) { > char s[3], *t = s; > if (obj->pin_display) > @@ -253,7 +253,7 @@ static int obj_rank_by_stolen(void *priv, > struct drm_i915_gem_object *b = > container_of(B, struct drm_i915_gem_object, obj_exec_link); > > - return a->stolen->start - b->stolen->start; > + return a->stolen->base.start - b->stolen->base.start; > } > > static int i915_gem_stolen_list_info(struct seq_file *m, void *data) > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index bca1572..5612df3 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -850,6 +850,12 @@ struct i915_ctx_hang_stats { > bool banned; > }; > > +struct i915_stolen_node { > + struct drm_mm_node base; > + struct list_head mm_link; > + struct drm_i915_gem_object *obj; > +}; > + > /* This must match up with the value previously used for execbuf2.rsvd1. */ > #define DEFAULT_CONTEXT_HANDLE 0 > > @@ -1279,6 +1285,13 @@ struct i915_gem_mm { > */ > struct list_head unbound_list; > > + /** > + * List of stolen objects that have been marked as purgeable and > + * thus available for reaping if we need more space for a new > + * allocation. Ordered by time of marking purgeable. > + */ > + struct list_head stolen_list; > + > /** Usable portion of the GTT for GEM */ > unsigned long stolen_base; /* limited to low memory (32-bit) */ > > @@ -2047,7 +2060,7 @@ struct drm_i915_gem_object { > struct list_head vma_list; > > /** Stolen memory for this object, instead of being backed by shmem. */ > - struct drm_mm_node *stolen; > + struct i915_stolen_node *stolen; > struct list_head global_list; > > struct list_head ring_list[I915_NUM_RINGS]; > @@ -2055,6 +2068,7 @@ struct drm_i915_gem_object { > struct list_head obj_exec_link; > > struct list_head batch_pool_link; > + struct list_head tmp_link; > > /** > * This is set if the object is on the active lists (has pending > @@ -2171,6 +2185,7 @@ struct drm_i915_gem_object { > }; > }; > #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base) > +#define I915_BO_IS_ACTIVE(__obj) (__obj->active) > > void i915_gem_track_fb(struct drm_i915_gem_object *old, > struct drm_i915_gem_object *new, > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 54f7df1..91a2e97 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4233,6 +4233,20 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data, > if (obj->madv == I915_MADV_DONTNEED && obj->pages == NULL) > i915_gem_object_truncate(obj); > > + if (obj->stolen) { > + switch (obj->madv) { > + case I915_MADV_WILLNEED: > + list_del_init(&obj->stolen->mm_link); > + break; > + case I915_MADV_DONTNEED: > + list_move(&obj->stolen->mm_link, > + &dev_priv->mm.stolen_list); > + break; > + default: > + break; > + } > + } > + > args->retained = obj->madv != __I915_MADV_PURGED; > > out: > @@ -4253,6 +4267,7 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj, > INIT_LIST_HEAD(&obj->obj_exec_link); > INIT_LIST_HEAD(&obj->vma_list); > INIT_LIST_HEAD(&obj->batch_pool_link); > + INIT_LIST_HEAD(&obj->tmp_link); > > obj->ops = ops; > > @@ -4895,6 +4910,7 @@ i915_gem_load(struct drm_device *dev) > INIT_LIST_HEAD(&dev_priv->context_list); > INIT_LIST_HEAD(&dev_priv->mm.unbound_list); > INIT_LIST_HEAD(&dev_priv->mm.bound_list); > + INIT_LIST_HEAD(&dev_priv->mm.stolen_list); > INIT_LIST_HEAD(&dev_priv->mm.fence_list); > for (i = 0; i < I915_NUM_RINGS; i++) > init_ring_lists(&dev_priv->ring[i]); > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > index d3e6813..bd5b2ea 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -456,7 +456,8 @@ i915_gem_object_release_stolen(struct drm_i915_gem_object *obj) > struct drm_i915_private *dev_priv = obj->base.dev->dev_private; > > if (obj->stolen) { > - i915_gem_stolen_remove_node(dev_priv, obj->stolen); > + list_del(&obj->stolen->mm_link); > + i915_gem_stolen_remove_node(dev_priv, &obj->stolen->base); > kfree(obj->stolen); > obj->stolen = NULL; > } > @@ -469,7 +470,7 @@ static const struct drm_i915_gem_object_ops i915_gem_object_stolen_ops = { > > static struct drm_i915_gem_object * > _i915_gem_object_create_stolen(struct drm_device *dev, > - struct drm_mm_node *stolen) > + struct i915_stolen_node *stolen) > { > struct drm_i915_gem_object *obj; > int ret = 0; > @@ -478,11 +479,12 @@ _i915_gem_object_create_stolen(struct drm_device *dev, > if (obj == NULL) > return ERR_PTR(-ENOMEM); > > - drm_gem_private_object_init(dev, &obj->base, stolen->size); > + drm_gem_private_object_init(dev, &obj->base, stolen->base.size); > i915_gem_object_init(obj, &i915_gem_object_stolen_ops); > > obj->pages = i915_pages_create_for_stolen(dev, > - stolen->start, stolen->size); > + stolen->base.start, > + stolen->base.size); > if (IS_ERR(obj->pages)) { > ret = PTR_ERR(obj->pages); > goto cleanup; > @@ -491,6 +493,9 @@ _i915_gem_object_create_stolen(struct drm_device *dev, > i915_gem_object_pin_pages(obj); > obj->stolen = stolen; > > + stolen->obj = obj; > + INIT_LIST_HEAD(&stolen->mm_link); > + > obj->base.read_domains = I915_GEM_DOMAIN_CPU | I915_GEM_DOMAIN_GTT; > obj->cache_level = HAS_LLC(dev) ? I915_CACHE_LLC : I915_CACHE_NONE; > > @@ -501,18 +506,102 @@ cleanup: > return ERR_PTR(ret); > } > > -struct drm_i915_gem_object * > -i915_gem_object_create_stolen(struct drm_device *dev, u64 size) > +static bool > +mark_free(struct drm_i915_gem_object *obj, struct list_head *unwind) > +{ > + BUG_ON(obj->stolen == NULL); I am fundamentally opposed to BUG_ONs which can be avoided. In this I see no value in hanging the machine while we could WARN_ON and return false. > + if (obj->madv != I915_MADV_DONTNEED) > + return false; > + > + if (obj->pin_display) > + return false; > + > + list_add(&obj->tmp_link, unwind); > + return drm_mm_scan_add_block(&obj->stolen->base); > +} > + > +static int > +stolen_evict(struct drm_i915_private *dev_priv, u64 size) > { > - struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_i915_gem_object *obj; > - struct drm_mm_node *stolen; > - int ret; > + struct list_head unwind, evict; > + struct i915_stolen_node *iter; > + int ret, active; > > - if (!drm_mm_initialized(&dev_priv->mm.stolen)) > - return ERR_PTR(-ENODEV); > + drm_mm_init_scan(&dev_priv->mm.stolen, size, 0, 0); > + INIT_LIST_HEAD(&unwind); > + > + /* Retire all requests before creating the evict list */ > + i915_gem_retire_requests(dev_priv->dev); > + > + for (active = 0; active <= 1; active++) { > + list_for_each_entry(iter, &dev_priv->mm.stolen_list, mm_link) { > + if (I915_BO_IS_ACTIVE(iter->obj) != active) > + continue; > + > + if (mark_free(iter->obj, &unwind)) > + goto found; > + } > + } > + > +found: > + INIT_LIST_HEAD(&evict); > + while (!list_empty(&unwind)) { > + obj = list_first_entry(&unwind, > + struct drm_i915_gem_object, > + tmp_link); > + list_del(&obj->tmp_link); > + > + if (drm_mm_scan_remove_block(&obj->stolen->base)) { > + list_add(&obj->tmp_link, &evict); > + drm_gem_object_reference(&obj->base); > + } > + } > + > + ret = 0; > + while (!list_empty(&evict)) { > + obj = list_first_entry(&evict, > + struct drm_i915_gem_object, > + tmp_link); > + list_del(&obj->tmp_link); > + > + if (ret == 0) { > + struct i915_vma *vma, *vma_next; > + > + list_for_each_entry_safe(vma, vma_next, > + &obj->vma_list, > + vma_link) > + if (i915_vma_unbind(vma)) > + break; > + > + /* Stolen pins its pages to prevent the > + * normal shrinker from processing stolen > + * objects. > + */ > + i915_gem_object_unpin_pages(obj); > + > + ret = i915_gem_object_put_pages(obj); > + if (ret == 0) { > + i915_gem_object_release_stolen(obj); > + obj->madv = __I915_MADV_PURGED; > + } else { > + i915_gem_object_pin_pages(obj); > + } > + } > + > + drm_gem_object_unreference(&obj->base); > + } > + > + return ret; > +} > + > +static struct i915_stolen_node * > +stolen_alloc(struct drm_i915_private *dev_priv, u64 size) > +{ > + struct i915_stolen_node *stolen; > + int ret; > > - DRM_DEBUG_KMS("creating stolen object: size=%llx\n", size); > if (size == 0) > return ERR_PTR(-EINVAL); > > @@ -520,17 +609,59 @@ i915_gem_object_create_stolen(struct drm_device *dev, u64 size) > if (!stolen) > return ERR_PTR(-ENOMEM); > > - ret = i915_gem_stolen_insert_node(dev_priv, stolen, size, 4096); > + ret = i915_gem_stolen_insert_node(dev_priv, &stolen->base, size, 4096); > + if (ret == 0) > + goto out; > + > + /* No more stolen memory available, or too fragmented. > + * Try evicting purgeable objects and search again. > + */ > + ret = stolen_evict(dev_priv, size); I have raised this question of struct_mutex in the previous round. Correct me if I am wrong, but I thought there was some effort made to make stolen object allocation run without struct mutex? With this change it requires it again. At the moment callers seem to hold it anyway. But I think lockdep_assert_held is needed now at least to document the requirement, probably in top level i915_gem_object_create_stolen. Regards, Tvrtko
On Thu, Oct 08, 2015 at 11:43:29AM +0100, Tvrtko Ursulin wrote: > >-struct drm_i915_gem_object * > >-i915_gem_object_create_stolen(struct drm_device *dev, u64 size) > >+static bool > >+mark_free(struct drm_i915_gem_object *obj, struct list_head *unwind) > >+{ > >+ BUG_ON(obj->stolen == NULL); > > I am fundamentally opposed to BUG_ONs which can be avoided. In this > I see no value in hanging the machine while we could WARN_ON and > return false. Don't bother with the WARN_ON. Either take the BUG_ON or accept that to get to this point the machine is dead anyway and a warning here doesn't help identify the root cause (better off with list debugging and memory debugging). I have personally been converting these asserts over to a dev-only compiletime option as I still find the BUGs more useful than WARNs in the GEM code. > >+ if (obj->madv != I915_MADV_DONTNEED) > >+ return false; > >+ > >+ if (obj->pin_display) > >+ return false; > >+ > >+ list_add(&obj->tmp_link, unwind); > >+ return drm_mm_scan_add_block(&obj->stolen->base); > >+} > >@@ -520,17 +609,59 @@ i915_gem_object_create_stolen(struct drm_device *dev, u64 size) > > if (!stolen) > > return ERR_PTR(-ENOMEM); > > > >- ret = i915_gem_stolen_insert_node(dev_priv, stolen, size, 4096); > >+ ret = i915_gem_stolen_insert_node(dev_priv, &stolen->base, size, 4096); > >+ if (ret == 0) > >+ goto out; > >+ > >+ /* No more stolen memory available, or too fragmented. > >+ * Try evicting purgeable objects and search again. > >+ */ > >+ ret = stolen_evict(dev_priv, size); > > I have raised this question of struct_mutex in the previous round. > > Correct me if I am wrong, but I thought there was some effort made > to make stolen object allocation run without struct mutex? Correct. But note that we do GEM operations inside the eviction logic and the struct_mutex is the only one we have for them. > With this change it requires it again. At the moment callers seem to > hold it anyway. But I think lockdep_assert_held is needed now at > least to document the requirement, probably in top level > i915_gem_object_create_stolen. And a comment as to why, might as well also try and document the logic behind such decisions. -Chris
On 08/10/15 12:09, Chris Wilson wrote: > On Thu, Oct 08, 2015 at 11:43:29AM +0100, Tvrtko Ursulin wrote: >>> -struct drm_i915_gem_object * >>> -i915_gem_object_create_stolen(struct drm_device *dev, u64 size) >>> +static bool >>> +mark_free(struct drm_i915_gem_object *obj, struct list_head *unwind) >>> +{ >>> + BUG_ON(obj->stolen == NULL); >> >> I am fundamentally opposed to BUG_ONs which can be avoided. In this >> I see no value in hanging the machine while we could WARN_ON and >> return false. > > Don't bother with the WARN_ON. Either take the BUG_ON or accept that to > get to this point the machine is dead anyway and a warning here doesn't > help identify the root cause (better off with list debugging and memory > debugging). I have personally been converting these asserts over to a > dev-only compiletime option as I still find the BUGs more useful than > WARNs in the GEM code. This is one of the ones which are to be expected in development only. At that time I much prefer a WARN_ON since it potentially saves you one reboot cycle and there aren't really any downsides to it. Especially if, as you say, machine is dead already. >>> + if (obj->madv != I915_MADV_DONTNEED) >>> + return false; >>> + >>> + if (obj->pin_display) >>> + return false; >>> + >>> + list_add(&obj->tmp_link, unwind); >>> + return drm_mm_scan_add_block(&obj->stolen->base); >>> +} > >>> @@ -520,17 +609,59 @@ i915_gem_object_create_stolen(struct drm_device *dev, u64 size) >>> if (!stolen) >>> return ERR_PTR(-ENOMEM); >>> >>> - ret = i915_gem_stolen_insert_node(dev_priv, stolen, size, 4096); >>> + ret = i915_gem_stolen_insert_node(dev_priv, &stolen->base, size, 4096); >>> + if (ret == 0) >>> + goto out; >>> + >>> + /* No more stolen memory available, or too fragmented. >>> + * Try evicting purgeable objects and search again. >>> + */ >>> + ret = stolen_evict(dev_priv, size); >> >> I have raised this question of struct_mutex in the previous round. >> >> Correct me if I am wrong, but I thought there was some effort made >> to make stolen object allocation run without struct mutex? > > Correct. But note that we do GEM operations inside the eviction logic > and the struct_mutex is the only one we have for them. > >> With this change it requires it again. At the moment callers seem to >> hold it anyway. But I think lockdep_assert_held is needed now at >> least to document the requirement, probably in top level >> i915_gem_object_create_stolen. > > And a comment as to why, might as well also try and document the logic > behind such decisions. Agreed. Regards, Tvrtko
On Thu, Oct 08, 2015 at 03:31:08PM +0100, Tvrtko Ursulin wrote: > > On 08/10/15 12:09, Chris Wilson wrote: > >On Thu, Oct 08, 2015 at 11:43:29AM +0100, Tvrtko Ursulin wrote: > >>>-struct drm_i915_gem_object * > >>>-i915_gem_object_create_stolen(struct drm_device *dev, u64 size) > >>>+static bool > >>>+mark_free(struct drm_i915_gem_object *obj, struct list_head *unwind) > >>>+{ > >>>+ BUG_ON(obj->stolen == NULL); > >> > >>I am fundamentally opposed to BUG_ONs which can be avoided. In this > >>I see no value in hanging the machine while we could WARN_ON and > >>return false. > > > >Don't bother with the WARN_ON. Either take the BUG_ON or accept that to > >get to this point the machine is dead anyway and a warning here doesn't > >help identify the root cause (better off with list debugging and memory > >debugging). I have personally been converting these asserts over to a > >dev-only compiletime option as I still find the BUGs more useful than > >WARNs in the GEM code. > > This is one of the ones which are to be expected in development > only. At that time I much prefer a WARN_ON since it potentially > saves you one reboot cycle and there aren't really any downsides to > it. Especially if, as you say, machine is dead already. panic-on-oops ftw :-p -Chris
On Thu, Oct 08, 2015 at 11:43:29AM +0100, Tvrtko Ursulin wrote: > > Hi, > > On 08/10/15 07:24, ankitprasad.r.sharma@intel.com wrote: > >From: Chris Wilson <chris at chris-wilson.co.uk> > > > >If we run out of stolen memory when trying to allocate an object, see if > >we can reap enough purgeable objects to free up enough contiguous free > >space for the allocation. This is in principle very much like evicting > >objects to free up enough contiguous space in the vma when binding > >a new object - and you will be forgiven for thinking that the code looks > >very similar. > > > >At the moment, we do not allow userspace to allocate objects in stolen, > >so there is neither the memory pressure to trigger stolen eviction nor > >any purgeable objects inside the stolen arena. However, this will change > >in the near future, and so better management and defragmentation of > >stolen memory will become a real issue. > > > >v2: Remember to remove the drm_mm_node. > > > >v3: Rebased to the latest drm-intel-nightly (Ankit) > > > >v4: corrected if-else braces format (Tvrtko/kerneldoc) > > > >v5: Rebased to the latest drm-intel-nightly (Ankit) > >Added a seperate list to maintain purgable objects from stolen memory > >region (Chris/Daniel) > > > >v6: Compiler optimization (merging 2 single loops into one for() loop), > >corrected code for object eviction, retire_requests before starting > >object eviction (Chris) > > > >v7: Added kernel doc for i915_gem_object_create_stolen() > > > >Testcase: igt/gem_stolen > > > >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk> > >Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com> > >--- > > drivers/gpu/drm/i915/i915_debugfs.c | 4 +- > > drivers/gpu/drm/i915/i915_drv.h | 17 +++- > > drivers/gpu/drm/i915/i915_gem.c | 16 ++++ > > drivers/gpu/drm/i915/i915_gem_stolen.c | 169 +++++++++++++++++++++++++++++---- > > drivers/gpu/drm/i915/intel_pm.c | 4 +- > > 5 files changed, 186 insertions(+), 24 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > >index 8797717..13762c1 100644 > >--- a/drivers/gpu/drm/i915/i915_debugfs.c > >+++ b/drivers/gpu/drm/i915/i915_debugfs.c > >@@ -174,7 +174,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) > > seq_puts(m, ")"); > > } > > if (obj->stolen) > >- seq_printf(m, " (stolen: %08llx)", obj->stolen->start); > >+ seq_printf(m, " (stolen: %08llx)", obj->stolen->base.start); > > if (obj->pin_display || obj->fault_mappable) { > > char s[3], *t = s; > > if (obj->pin_display) > >@@ -253,7 +253,7 @@ static int obj_rank_by_stolen(void *priv, > > struct drm_i915_gem_object *b = > > container_of(B, struct drm_i915_gem_object, obj_exec_link); > > > >- return a->stolen->start - b->stolen->start; > >+ return a->stolen->base.start - b->stolen->base.start; > > } > > > > static int i915_gem_stolen_list_info(struct seq_file *m, void *data) > >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >index bca1572..5612df3 100644 > >--- a/drivers/gpu/drm/i915/i915_drv.h > >+++ b/drivers/gpu/drm/i915/i915_drv.h > >@@ -850,6 +850,12 @@ struct i915_ctx_hang_stats { > > bool banned; > > }; > > > >+struct i915_stolen_node { > >+ struct drm_mm_node base; > >+ struct list_head mm_link; > >+ struct drm_i915_gem_object *obj; > >+}; > >+ > > /* This must match up with the value previously used for execbuf2.rsvd1. */ > > #define DEFAULT_CONTEXT_HANDLE 0 > > > >@@ -1279,6 +1285,13 @@ struct i915_gem_mm { > > */ > > struct list_head unbound_list; > > > >+ /** > >+ * List of stolen objects that have been marked as purgeable and > >+ * thus available for reaping if we need more space for a new > >+ * allocation. Ordered by time of marking purgeable. > >+ */ > >+ struct list_head stolen_list; > >+ > > /** Usable portion of the GTT for GEM */ > > unsigned long stolen_base; /* limited to low memory (32-bit) */ > > > >@@ -2047,7 +2060,7 @@ struct drm_i915_gem_object { > > struct list_head vma_list; > > > > /** Stolen memory for this object, instead of being backed by shmem. */ > >- struct drm_mm_node *stolen; > >+ struct i915_stolen_node *stolen; > > struct list_head global_list; > > > > struct list_head ring_list[I915_NUM_RINGS]; > >@@ -2055,6 +2068,7 @@ struct drm_i915_gem_object { > > struct list_head obj_exec_link; > > > > struct list_head batch_pool_link; > >+ struct list_head tmp_link; > > > > /** > > * This is set if the object is on the active lists (has pending > >@@ -2171,6 +2185,7 @@ struct drm_i915_gem_object { > > }; > > }; > > #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base) > >+#define I915_BO_IS_ACTIVE(__obj) (__obj->active) > > > > void i915_gem_track_fb(struct drm_i915_gem_object *old, > > struct drm_i915_gem_object *new, > >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >index 54f7df1..91a2e97 100644 > >--- a/drivers/gpu/drm/i915/i915_gem.c > >+++ b/drivers/gpu/drm/i915/i915_gem.c > >@@ -4233,6 +4233,20 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data, > > if (obj->madv == I915_MADV_DONTNEED && obj->pages == NULL) > > i915_gem_object_truncate(obj); > > > >+ if (obj->stolen) { > >+ switch (obj->madv) { > >+ case I915_MADV_WILLNEED: > >+ list_del_init(&obj->stolen->mm_link); > >+ break; > >+ case I915_MADV_DONTNEED: > >+ list_move(&obj->stolen->mm_link, > >+ &dev_priv->mm.stolen_list); > >+ break; > >+ default: > >+ break; > >+ } > >+ } > >+ > > args->retained = obj->madv != __I915_MADV_PURGED; > > > > out: > >@@ -4253,6 +4267,7 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj, > > INIT_LIST_HEAD(&obj->obj_exec_link); > > INIT_LIST_HEAD(&obj->vma_list); > > INIT_LIST_HEAD(&obj->batch_pool_link); > >+ INIT_LIST_HEAD(&obj->tmp_link); > > > > obj->ops = ops; > > > >@@ -4895,6 +4910,7 @@ i915_gem_load(struct drm_device *dev) > > INIT_LIST_HEAD(&dev_priv->context_list); > > INIT_LIST_HEAD(&dev_priv->mm.unbound_list); > > INIT_LIST_HEAD(&dev_priv->mm.bound_list); > >+ INIT_LIST_HEAD(&dev_priv->mm.stolen_list); > > INIT_LIST_HEAD(&dev_priv->mm.fence_list); > > for (i = 0; i < I915_NUM_RINGS; i++) > > init_ring_lists(&dev_priv->ring[i]); > >diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > >index d3e6813..bd5b2ea 100644 > >--- a/drivers/gpu/drm/i915/i915_gem_stolen.c > >+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > >@@ -456,7 +456,8 @@ i915_gem_object_release_stolen(struct drm_i915_gem_object *obj) > > struct drm_i915_private *dev_priv = obj->base.dev->dev_private; > > > > if (obj->stolen) { > >- i915_gem_stolen_remove_node(dev_priv, obj->stolen); > >+ list_del(&obj->stolen->mm_link); > >+ i915_gem_stolen_remove_node(dev_priv, &obj->stolen->base); > > kfree(obj->stolen); > > obj->stolen = NULL; > > } > >@@ -469,7 +470,7 @@ static const struct drm_i915_gem_object_ops i915_gem_object_stolen_ops = { > > > > static struct drm_i915_gem_object * > > _i915_gem_object_create_stolen(struct drm_device *dev, > >- struct drm_mm_node *stolen) > >+ struct i915_stolen_node *stolen) > > { > > struct drm_i915_gem_object *obj; > > int ret = 0; > >@@ -478,11 +479,12 @@ _i915_gem_object_create_stolen(struct drm_device *dev, > > if (obj == NULL) > > return ERR_PTR(-ENOMEM); > > > >- drm_gem_private_object_init(dev, &obj->base, stolen->size); > >+ drm_gem_private_object_init(dev, &obj->base, stolen->base.size); > > i915_gem_object_init(obj, &i915_gem_object_stolen_ops); > > > > obj->pages = i915_pages_create_for_stolen(dev, > >- stolen->start, stolen->size); > >+ stolen->base.start, > >+ stolen->base.size); > > if (IS_ERR(obj->pages)) { > > ret = PTR_ERR(obj->pages); > > goto cleanup; > >@@ -491,6 +493,9 @@ _i915_gem_object_create_stolen(struct drm_device *dev, > > i915_gem_object_pin_pages(obj); > > obj->stolen = stolen; > > > >+ stolen->obj = obj; > >+ INIT_LIST_HEAD(&stolen->mm_link); > >+ > > obj->base.read_domains = I915_GEM_DOMAIN_CPU | I915_GEM_DOMAIN_GTT; > > obj->cache_level = HAS_LLC(dev) ? I915_CACHE_LLC : I915_CACHE_NONE; > > > >@@ -501,18 +506,102 @@ cleanup: > > return ERR_PTR(ret); > > } > > > >-struct drm_i915_gem_object * > >-i915_gem_object_create_stolen(struct drm_device *dev, u64 size) > >+static bool > >+mark_free(struct drm_i915_gem_object *obj, struct list_head *unwind) > >+{ > >+ BUG_ON(obj->stolen == NULL); > > I am fundamentally opposed to BUG_ONs which can be avoided. In this I see no > value in hanging the machine while we could WARN_ON and return false. > > >+ if (obj->madv != I915_MADV_DONTNEED) > >+ return false; > >+ > >+ if (obj->pin_display) > >+ return false; > >+ > >+ list_add(&obj->tmp_link, unwind); > >+ return drm_mm_scan_add_block(&obj->stolen->base); > >+} > >+ > >+static int > >+stolen_evict(struct drm_i915_private *dev_priv, u64 size) > > { > >- struct drm_i915_private *dev_priv = dev->dev_private; > > struct drm_i915_gem_object *obj; > >- struct drm_mm_node *stolen; > >- int ret; > >+ struct list_head unwind, evict; > >+ struct i915_stolen_node *iter; > >+ int ret, active; > > > >- if (!drm_mm_initialized(&dev_priv->mm.stolen)) > >- return ERR_PTR(-ENODEV); > >+ drm_mm_init_scan(&dev_priv->mm.stolen, size, 0, 0); > >+ INIT_LIST_HEAD(&unwind); > >+ > >+ /* Retire all requests before creating the evict list */ > >+ i915_gem_retire_requests(dev_priv->dev); > >+ > >+ for (active = 0; active <= 1; active++) { > >+ list_for_each_entry(iter, &dev_priv->mm.stolen_list, mm_link) { > >+ if (I915_BO_IS_ACTIVE(iter->obj) != active) > >+ continue; > >+ > >+ if (mark_free(iter->obj, &unwind)) > >+ goto found; > >+ } > >+ } > >+ > >+found: > >+ INIT_LIST_HEAD(&evict); > >+ while (!list_empty(&unwind)) { > >+ obj = list_first_entry(&unwind, > >+ struct drm_i915_gem_object, > >+ tmp_link); > >+ list_del(&obj->tmp_link); > >+ > >+ if (drm_mm_scan_remove_block(&obj->stolen->base)) { > >+ list_add(&obj->tmp_link, &evict); > >+ drm_gem_object_reference(&obj->base); > >+ } > >+ } > >+ > >+ ret = 0; > >+ while (!list_empty(&evict)) { > >+ obj = list_first_entry(&evict, > >+ struct drm_i915_gem_object, > >+ tmp_link); > >+ list_del(&obj->tmp_link); > >+ > >+ if (ret == 0) { > >+ struct i915_vma *vma, *vma_next; > >+ > >+ list_for_each_entry_safe(vma, vma_next, > >+ &obj->vma_list, > >+ vma_link) > >+ if (i915_vma_unbind(vma)) > >+ break; > >+ > >+ /* Stolen pins its pages to prevent the > >+ * normal shrinker from processing stolen > >+ * objects. > >+ */ > >+ i915_gem_object_unpin_pages(obj); > >+ > >+ ret = i915_gem_object_put_pages(obj); > >+ if (ret == 0) { > >+ i915_gem_object_release_stolen(obj); > >+ obj->madv = __I915_MADV_PURGED; > >+ } else { > >+ i915_gem_object_pin_pages(obj); > >+ } > >+ } > >+ > >+ drm_gem_object_unreference(&obj->base); > >+ } > >+ > >+ return ret; > >+} > >+ > >+static struct i915_stolen_node * > >+stolen_alloc(struct drm_i915_private *dev_priv, u64 size) > >+{ > >+ struct i915_stolen_node *stolen; > >+ int ret; > > > >- DRM_DEBUG_KMS("creating stolen object: size=%llx\n", size); > > if (size == 0) > > return ERR_PTR(-EINVAL); > > > >@@ -520,17 +609,59 @@ i915_gem_object_create_stolen(struct drm_device *dev, u64 size) > > if (!stolen) > > return ERR_PTR(-ENOMEM); > > > >- ret = i915_gem_stolen_insert_node(dev_priv, stolen, size, 4096); > >+ ret = i915_gem_stolen_insert_node(dev_priv, &stolen->base, size, 4096); > >+ if (ret == 0) > >+ goto out; > >+ > >+ /* No more stolen memory available, or too fragmented. > >+ * Try evicting purgeable objects and search again. > >+ */ > >+ ret = stolen_evict(dev_priv, size); > > I have raised this question of struct_mutex in the previous round. > > Correct me if I am wrong, but I thought there was some effort made to make > stolen object allocation run without struct mutex? > > With this change it requires it again. At the moment callers seem to hold it > anyway. But I think lockdep_assert_held is needed now at least to document > the requirement, probably in top level i915_gem_object_create_stolen. Please use WARN_ON(!mutex_is_locked()) because lockdep_assert_held is compiled out when lockdep is disabled. And that's for almost everyone. The reason for that is that the semantics aren't a perfect match - lockdep_assert_held also makes sure that it's the calling process holding the lock, not just that it's locked. -Daniel
On Thu, Oct 08, 2015 at 04:08:52PM +0100, Chris Wilson wrote: > On Thu, Oct 08, 2015 at 03:31:08PM +0100, Tvrtko Ursulin wrote: > > > > On 08/10/15 12:09, Chris Wilson wrote: > > >On Thu, Oct 08, 2015 at 11:43:29AM +0100, Tvrtko Ursulin wrote: > > >>>-struct drm_i915_gem_object * > > >>>-i915_gem_object_create_stolen(struct drm_device *dev, u64 size) > > >>>+static bool > > >>>+mark_free(struct drm_i915_gem_object *obj, struct list_head *unwind) > > >>>+{ > > >>>+ BUG_ON(obj->stolen == NULL); > > >> > > >>I am fundamentally opposed to BUG_ONs which can be avoided. In this > > >>I see no value in hanging the machine while we could WARN_ON and > > >>return false. > > > > > >Don't bother with the WARN_ON. Either take the BUG_ON or accept that to > > >get to this point the machine is dead anyway and a warning here doesn't > > >help identify the root cause (better off with list debugging and memory > > >debugging). I have personally been converting these asserts over to a > > >dev-only compiletime option as I still find the BUGs more useful than > > >WARNs in the GEM code. > > > > This is one of the ones which are to be expected in development > > only. At that time I much prefer a WARN_ON since it potentially > > saves you one reboot cycle and there aren't really any downsides to > > it. Especially if, as you say, machine is dead already. > > panic-on-oops ftw :-p We killed drm panic handling, and if it gets resurrect it will be super-minimal to avoid any kind of "my real oops scrolled off the screen because i915.ko was dying even harder ..." bug reports. We've done this because it's pretty much impossible to avoid piles of WARN_ON, lockdep splats and other crap once you're trying to do a full modeset from panic context. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 8797717..13762c1 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -174,7 +174,7 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) seq_puts(m, ")"); } if (obj->stolen) - seq_printf(m, " (stolen: %08llx)", obj->stolen->start); + seq_printf(m, " (stolen: %08llx)", obj->stolen->base.start); if (obj->pin_display || obj->fault_mappable) { char s[3], *t = s; if (obj->pin_display) @@ -253,7 +253,7 @@ static int obj_rank_by_stolen(void *priv, struct drm_i915_gem_object *b = container_of(B, struct drm_i915_gem_object, obj_exec_link); - return a->stolen->start - b->stolen->start; + return a->stolen->base.start - b->stolen->base.start; } static int i915_gem_stolen_list_info(struct seq_file *m, void *data) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bca1572..5612df3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -850,6 +850,12 @@ struct i915_ctx_hang_stats { bool banned; }; +struct i915_stolen_node { + struct drm_mm_node base; + struct list_head mm_link; + struct drm_i915_gem_object *obj; +}; + /* This must match up with the value previously used for execbuf2.rsvd1. */ #define DEFAULT_CONTEXT_HANDLE 0 @@ -1279,6 +1285,13 @@ struct i915_gem_mm { */ struct list_head unbound_list; + /** + * List of stolen objects that have been marked as purgeable and + * thus available for reaping if we need more space for a new + * allocation. Ordered by time of marking purgeable. + */ + struct list_head stolen_list; + /** Usable portion of the GTT for GEM */ unsigned long stolen_base; /* limited to low memory (32-bit) */ @@ -2047,7 +2060,7 @@ struct drm_i915_gem_object { struct list_head vma_list; /** Stolen memory for this object, instead of being backed by shmem. */ - struct drm_mm_node *stolen; + struct i915_stolen_node *stolen; struct list_head global_list; struct list_head ring_list[I915_NUM_RINGS]; @@ -2055,6 +2068,7 @@ struct drm_i915_gem_object { struct list_head obj_exec_link; struct list_head batch_pool_link; + struct list_head tmp_link; /** * This is set if the object is on the active lists (has pending @@ -2171,6 +2185,7 @@ struct drm_i915_gem_object { }; }; #define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base) +#define I915_BO_IS_ACTIVE(__obj) (__obj->active) void i915_gem_track_fb(struct drm_i915_gem_object *old, struct drm_i915_gem_object *new, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 54f7df1..91a2e97 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4233,6 +4233,20 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data, if (obj->madv == I915_MADV_DONTNEED && obj->pages == NULL) i915_gem_object_truncate(obj); + if (obj->stolen) { + switch (obj->madv) { + case I915_MADV_WILLNEED: + list_del_init(&obj->stolen->mm_link); + break; + case I915_MADV_DONTNEED: + list_move(&obj->stolen->mm_link, + &dev_priv->mm.stolen_list); + break; + default: + break; + } + } + args->retained = obj->madv != __I915_MADV_PURGED; out: @@ -4253,6 +4267,7 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj, INIT_LIST_HEAD(&obj->obj_exec_link); INIT_LIST_HEAD(&obj->vma_list); INIT_LIST_HEAD(&obj->batch_pool_link); + INIT_LIST_HEAD(&obj->tmp_link); obj->ops = ops; @@ -4895,6 +4910,7 @@ i915_gem_load(struct drm_device *dev) INIT_LIST_HEAD(&dev_priv->context_list); INIT_LIST_HEAD(&dev_priv->mm.unbound_list); INIT_LIST_HEAD(&dev_priv->mm.bound_list); + INIT_LIST_HEAD(&dev_priv->mm.stolen_list); INIT_LIST_HEAD(&dev_priv->mm.fence_list); for (i = 0; i < I915_NUM_RINGS; i++) init_ring_lists(&dev_priv->ring[i]); diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index d3e6813..bd5b2ea 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -456,7 +456,8 @@ i915_gem_object_release_stolen(struct drm_i915_gem_object *obj) struct drm_i915_private *dev_priv = obj->base.dev->dev_private; if (obj->stolen) { - i915_gem_stolen_remove_node(dev_priv, obj->stolen); + list_del(&obj->stolen->mm_link); + i915_gem_stolen_remove_node(dev_priv, &obj->stolen->base); kfree(obj->stolen); obj->stolen = NULL; } @@ -469,7 +470,7 @@ static const struct drm_i915_gem_object_ops i915_gem_object_stolen_ops = { static struct drm_i915_gem_object * _i915_gem_object_create_stolen(struct drm_device *dev, - struct drm_mm_node *stolen) + struct i915_stolen_node *stolen) { struct drm_i915_gem_object *obj; int ret = 0; @@ -478,11 +479,12 @@ _i915_gem_object_create_stolen(struct drm_device *dev, if (obj == NULL) return ERR_PTR(-ENOMEM); - drm_gem_private_object_init(dev, &obj->base, stolen->size); + drm_gem_private_object_init(dev, &obj->base, stolen->base.size); i915_gem_object_init(obj, &i915_gem_object_stolen_ops); obj->pages = i915_pages_create_for_stolen(dev, - stolen->start, stolen->size); + stolen->base.start, + stolen->base.size); if (IS_ERR(obj->pages)) { ret = PTR_ERR(obj->pages); goto cleanup; @@ -491,6 +493,9 @@ _i915_gem_object_create_stolen(struct drm_device *dev, i915_gem_object_pin_pages(obj); obj->stolen = stolen; + stolen->obj = obj; + INIT_LIST_HEAD(&stolen->mm_link); + obj->base.read_domains = I915_GEM_DOMAIN_CPU | I915_GEM_DOMAIN_GTT; obj->cache_level = HAS_LLC(dev) ? I915_CACHE_LLC : I915_CACHE_NONE; @@ -501,18 +506,102 @@ cleanup: return ERR_PTR(ret); } -struct drm_i915_gem_object * -i915_gem_object_create_stolen(struct drm_device *dev, u64 size) +static bool +mark_free(struct drm_i915_gem_object *obj, struct list_head *unwind) +{ + BUG_ON(obj->stolen == NULL); + + if (obj->madv != I915_MADV_DONTNEED) + return false; + + if (obj->pin_display) + return false; + + list_add(&obj->tmp_link, unwind); + return drm_mm_scan_add_block(&obj->stolen->base); +} + +static int +stolen_evict(struct drm_i915_private *dev_priv, u64 size) { - struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; - struct drm_mm_node *stolen; - int ret; + struct list_head unwind, evict; + struct i915_stolen_node *iter; + int ret, active; - if (!drm_mm_initialized(&dev_priv->mm.stolen)) - return ERR_PTR(-ENODEV); + drm_mm_init_scan(&dev_priv->mm.stolen, size, 0, 0); + INIT_LIST_HEAD(&unwind); + + /* Retire all requests before creating the evict list */ + i915_gem_retire_requests(dev_priv->dev); + + for (active = 0; active <= 1; active++) { + list_for_each_entry(iter, &dev_priv->mm.stolen_list, mm_link) { + if (I915_BO_IS_ACTIVE(iter->obj) != active) + continue; + + if (mark_free(iter->obj, &unwind)) + goto found; + } + } + +found: + INIT_LIST_HEAD(&evict); + while (!list_empty(&unwind)) { + obj = list_first_entry(&unwind, + struct drm_i915_gem_object, + tmp_link); + list_del(&obj->tmp_link); + + if (drm_mm_scan_remove_block(&obj->stolen->base)) { + list_add(&obj->tmp_link, &evict); + drm_gem_object_reference(&obj->base); + } + } + + ret = 0; + while (!list_empty(&evict)) { + obj = list_first_entry(&evict, + struct drm_i915_gem_object, + tmp_link); + list_del(&obj->tmp_link); + + if (ret == 0) { + struct i915_vma *vma, *vma_next; + + list_for_each_entry_safe(vma, vma_next, + &obj->vma_list, + vma_link) + if (i915_vma_unbind(vma)) + break; + + /* Stolen pins its pages to prevent the + * normal shrinker from processing stolen + * objects. + */ + i915_gem_object_unpin_pages(obj); + + ret = i915_gem_object_put_pages(obj); + if (ret == 0) { + i915_gem_object_release_stolen(obj); + obj->madv = __I915_MADV_PURGED; + } else { + i915_gem_object_pin_pages(obj); + } + } + + drm_gem_object_unreference(&obj->base); + } + + return ret; +} + +static struct i915_stolen_node * +stolen_alloc(struct drm_i915_private *dev_priv, u64 size) +{ + struct i915_stolen_node *stolen; + int ret; - DRM_DEBUG_KMS("creating stolen object: size=%llx\n", size); if (size == 0) return ERR_PTR(-EINVAL); @@ -520,17 +609,59 @@ i915_gem_object_create_stolen(struct drm_device *dev, u64 size) if (!stolen) return ERR_PTR(-ENOMEM); - ret = i915_gem_stolen_insert_node(dev_priv, stolen, size, 4096); + ret = i915_gem_stolen_insert_node(dev_priv, &stolen->base, size, 4096); + if (ret == 0) + goto out; + + /* No more stolen memory available, or too fragmented. + * Try evicting purgeable objects and search again. + */ + ret = stolen_evict(dev_priv, size); + + if (ret == 0) + ret = i915_gem_stolen_insert_node(dev_priv, &stolen->base, + size, 0); +out: if (ret) { kfree(stolen); return ERR_PTR(ret); } + return stolen; +} + +/** + * i915_gem_object_create_stolen() - creates object using the stolen memory + * @dev: drm device + * @size: size of the object requested + * + * i915_gem_object_create_stolen() tries to allocate memory for the object + * from the stolen memory region. If not enough memory is found, it tries + * evicting purgeable objects and searching again. + * + * Returns: Object pointer - success and error pointer - failure + */ +struct drm_i915_gem_object * +i915_gem_object_create_stolen(struct drm_device *dev, u64 size) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_gem_object *obj; + struct i915_stolen_node *stolen; + + if (!drm_mm_initialized(&dev_priv->mm.stolen)) + return ERR_PTR(-ENODEV); + + DRM_DEBUG_KMS("creating stolen object: size=%llx\n", size); + + stolen = stolen_alloc(dev_priv, size); + if (IS_ERR(stolen)) + return ERR_PTR(-ENOMEM); + obj = _i915_gem_object_create_stolen(dev, stolen); if (!IS_ERR(obj)) return obj; - i915_gem_stolen_remove_node(dev_priv, stolen); + i915_gem_stolen_remove_node(dev_priv, &stolen->base); kfree(stolen); return obj; } @@ -544,7 +675,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct i915_address_space *ggtt = &dev_priv->gtt.base; struct drm_i915_gem_object *obj; - struct drm_mm_node *stolen; + struct i915_stolen_node *stolen; struct i915_vma *vma; int ret; @@ -563,10 +694,10 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, if (!stolen) return ERR_PTR(-ENOMEM); - stolen->start = stolen_offset; - stolen->size = size; + stolen->base.start = stolen_offset; + stolen->base.size = size; mutex_lock(&dev_priv->mm.stolen_lock); - ret = drm_mm_reserve_node(&dev_priv->mm.stolen, stolen); + ret = drm_mm_reserve_node(&dev_priv->mm.stolen, &stolen->base); mutex_unlock(&dev_priv->mm.stolen_lock); if (ret) { DRM_DEBUG_KMS("failed to allocate stolen space\n"); @@ -577,7 +708,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, obj = _i915_gem_object_create_stolen(dev, stolen); if (IS_ERR(obj)) { DRM_DEBUG_KMS("failed to allocate stolen object\n"); - i915_gem_stolen_remove_node(dev_priv, stolen); + i915_gem_stolen_remove_node(dev_priv, &stolen->base); kfree(stolen); return obj; } diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 0d35d7f..12bf162 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5318,7 +5318,7 @@ static void valleyview_check_pctx(struct drm_i915_private *dev_priv) unsigned long pctx_addr = I915_READ(VLV_PCBR) & ~4095; WARN_ON(pctx_addr != dev_priv->mm.stolen_base + - dev_priv->vlv_pctx->stolen->start); + dev_priv->vlv_pctx->stolen->base.start); } @@ -5392,7 +5392,7 @@ static void valleyview_setup_pctx(struct drm_device *dev) return; } - pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start; + pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->base.start; I915_WRITE(VLV_PCBR, pctx_paddr); out: