On Tue, Sep 15, 2015 at 02:03:26PM +0530, [email protected] 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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to