On Tue, Sep 15, 2015 at 02:03:26PM +0530, ankitprasad.r.sharma@xxxxxxxxx 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) > +{ > + if (obj->stolen == NULL) > + return false; BUG_ON(obj->stolen == NULL); > + > + if (obj->madv != I915_MADV_DONTNEED) > + return false; > + > + if (obj->pin_display) > + return false; > + > + if (I915_BO_IS_ACTIVE(obj)) > + return false; We want to add both active/inactive objects (ordering by the caller to prioritise inactive objects). > + 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; > + struct list_head unwind, evict; > + struct i915_stolen_node *iter; > int ret; > > - if (!drm_mm_initialized(&dev_priv->mm.stolen)) > - return NULL; > + drm_mm_init_scan(&dev_priv->mm.stolen, size, 0, 0); > + INIT_LIST_HEAD(&unwind); > + > + list_for_each_entry(iter, &dev_priv->mm.stolen_list, mm_link) { > + if (I915_BO_IS_ACTIVE(iter->obj)) > + continue; > + > + if (mark_free(iter->obj, &unwind)) > + goto found; > + } > + > + list_for_each_entry(iter, &dev_priv->mm.stolen_list, mm_link) { > + if (!I915_BO_IS_ACTIVE(iter->obj)) > + continue; > + > + if (mark_free(iter->obj, &unwind)) > + goto found; > + } If I have my micro-optimising hat on, I would actually rewrite this as soemthing like: for (int phase = 0; phase <= 1; phase++) { list_for_each_entry(iter, &dev_priv->mm.stolen_list, mm_link) { if (I915_BO_IS_ACTIVE(iter->obj) != phase) continue; if (mark_free(iter->obj, &unwind)) goto found; } } as the compiler will find that easier to perform magic with. We also probably want to do i915_gem_retire_requests() first (so that any recently idle objects are moved to the front queue). > +found: > + INIT_LIST_HEAD(&evict); > + while (!list_empty(&unwind)) { > + obj = list_first_entry(&unwind, > + struct drm_i915_gem_object, > + tmp_link); > + list_del_init(&obj->tmp_link); The fun thing with tmp_link is that we don't need to set it to clear (either here or during object construction). -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx