On ke, 2016-07-27 at 12:14 +0100, Chris Wilson wrote:
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -34,6 +34,19 @@
>  #include "i915_trace.h"
>  
>  static bool
> +gpu_is_idle(struct drm_i915_private *dev_priv)
> +{
> +     struct intel_engine_cs *engine;
> +
> +     for_each_engine(engine, dev_priv) {
> +             if (!list_empty(&engine->request_list))
> +                     return false;
> +     }

Braces are not necessary here.
 
>       /*
>        * The goal is to evict objects and amalgamate space in LRU order.
>        * The oldest idle objects reside on the inactive list, which is in
> -      * retirement order. The next objects to retire are those on the (per
> -      * ring) active list that do not have an outstanding flush. Once the
> -      * hardware reports completion (the seqno is updated after the
> -      * batchbuffer has been finished) the clean buffer objects would
> -      * be retired to the inactive list. Any dirty objects would be added
> -      * to the tail of the flushing list. So after processing the clean
> -      * active objects we need to emit a MI_FLUSH to retire the flushing
> -      * list, hence the retirement order of the flushing list is in
> -      * advance of the dirty objects on the active lists.
> +      * retirement order. The next objects to retire are those in flight,
> +      * on the active list, again in retirement order.
>        *
>        * The retirement sequence is thus:
>        *   1. Inactive objects (already retired)
> -      *   2. Clean active objects
> -      *   3. Flushing list
> -      *   4. Dirty active objects.
> +      *   2. Active objects (will stall on unbinding)

Not quite sure how good a sequence list is for two phases :)
 
>  found:
>       /* drm_mm doesn't allow any other other operations while
> -      * scanning, therefore store to be evicted objects on a
> -      * temporary list. */
> -     INIT_LIST_HEAD(&eviction_list);
> -     while (!list_empty(&unwind_list)) {
> -             vma = list_first_entry(&unwind_list,
> -                                    struct i915_vma,
> -                                    exec_list);
> -             if (drm_mm_scan_remove_block(&vma->node)) {
> +      * scanning, therefore store to-be-evicted objects on a
> +      * temporary list and take a reference for all before
> +      * calling unbind (which may remove the active reference
> +      * of any of our objects, thus corrupting the list).
> +      */
> +     list_for_each_entry_safe(vma, next, &eviction_list, exec_list) {

s/exec_list/exec_link/ at some point in future.

> +             if (drm_mm_scan_remove_block(&vma->node))
>                       vma->pin_count++;
> -                     list_move(&vma->exec_list, &eviction_list);
> -                     continue;
> -             }
> -             list_del_init(&vma->exec_list);
> +             else
> +                     list_del_init(&vma->exec_list);

Current behaviour is not changed, but gotta ask why no putting back to
to the list vma originated from?

Reviewed-by: Joonas Lahtinen <[email protected]>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to