On Fri, Feb 26, 2016 at 03:37:35PM +0000, Tvrtko Ursulin wrote:
> -     if (ring->disable_lite_restore_wa) {
> -             /* Prevent a ctx to preempt itself */
> -             if ((status & GEN8_CTX_STATUS_ACTIVE_IDLE) &&
> -                 (submit_contexts != 0))
> -                     execlists_context_unqueue(ring);
> -     } else if (submit_contexts != 0) {
> +     if (submit_contexts && (!ring->disable_lite_restore_wa ||
> +         (ring->disable_lite_restore_wa && (status &
> +         GEN8_CTX_STATUS_ACTIVE_IDLE))))

A little clumsy.

if (submit_contexts) {
        if (!ring->disable_lite_restore_wa == 0 ||
            status & GEN8_CTX_STATUS_ACTIVE_IDLE)
                execlists_context_unqueue__locked(ring);
}

i.e. checking for ring->disable_lite_restore_wa != 0 is redundant (as it
must be true along the false branch of !ring->disable_lite_restore)

And if we take a moment to clean up the logic there

>       list_add_tail(&request->execlist_link, &ring->execlist_queue);
> -     if (num_elements == 0)
> +     if (num_elements == 0) {
> +             spin_lock(&dev_priv->uncore.lock);
> +             intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
> +
>               execlists_context_unqueue(ring);
>  
> -     spin_unlock_irq(&ring->execlist_lock);
> +             intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL);
> +             spin_unlock(&dev_priv->uncore.lock);
> +     }

should we hide the locks here with
void execlists_context_unqueue()
{
        spin_lock(&dev_priv->uncore.lock);
        intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);

        execlists_context_unqueue__locked(ring);

        intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL);
        spin_unlock(&dev_priv->uncore.lock);

}
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to