Re: [PATCH 3/6] drm/i915: Split the batch pool by engine

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 03/18/2015 05:33 PM, Tvrtko Ursulin wrote:
>
> On 03/18/2015 05:27 PM, Chris Wilson wrote:
>> On Wed, Mar 18, 2015 at 04:51:58PM +0000, Tvrtko Ursulin wrote:
>>> On 03/09/2015 09:55 AM, Chris Wilson wrote:
>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c
>>>> b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
>>>> index 21f3356cc0ab..1287abf55b84 100644
>>>> --- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c
>>>> +++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
>>>> @@ -96,8 +96,9 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool
>>>> *pool,
>>>>
>>>>       list_for_each_entry_safe(tmp, next,
>>>>                    &pool->cache_list, batch_pool_list) {
>>>> +        /* The batches are strictly LRU ordered */
>>>>           if (tmp->active)
>>>> -            continue;
>>>> +            break;
>>>
>>> This hunk would be a better fit for 2/6, correct?
>>
>> No. The explanation is given by the comment + changelog.
>
> I don't see this patch introducing this strict LRU ordering, rather it
> was there before this patch. Am I missing something? If I am not, then I
> see this hunk as a better fit with "Tidy batch pool logic", than "Split
> the batch pool by engine".

Oh right, I got it, but not sure I like it that much. I don't think 
batch pool implementation is well enough decoupled from the users. Well 
in a way at least where when we talk about LRU ordering, it depends on 
retiring working properly and that is not obvious from code layout and 
module separation.

And then with this me move traversal inefficiency to possible more 
resource use. Would it be better to fix the cause rather than symptoms? 
Is it feasible? What would be the downside of retiring all rings before 
submission?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx