Re: [PATCH v2 1/7] drm/i915/gen8: Add infrastructure to initialize WA batch buffers

["Siluvery, Arun" <arun.siluvery@linux.intel.com>]

On 15/06/2015 11:41, Daniel Vetter wrote:
> On Thu, Jun 04, 2015 at 03:30:56PM +0100, Siluvery, Arun wrote:
>> On 02/06/2015 19:47, Dave Gordon wrote:
>>> On 02/06/15 19:36, Siluvery, Arun wrote:
>>>> On 01/06/2015 11:22, Daniel, Thomas wrote:
>>>>>>
>>>>>> Indeed, allocating an extra scratch page in the context would simplify
>>>>>> vma/mm management. A trick might be to allocate the scratch page at the
>>>>>> start, then offset the lrc regs etc - that would then be consistent
>>>>>> amongst gen and be easy enough to extend if we need more per-context
>>>>>> scratch space in future.
>>>>>> -Chris
>>>>>
>>>>> Yes, I think we already have another use for more per-context space at
>>>>> the start.  The GuC is planning to do this.  Arun, you probably should
>>>>> work with Alex Dai and Dave Gordon to avoid conflicts here.
>>>>>
>>>>> Thomas.
>>>>>
>>>>
>>>> Thanks for the heads-up Thomas.
>>>> I have discussed with Dave and agreed to share this page;
>>>> GuC probably doesn't need whole page so first half is reserved for it's
>>>> use and second half is used for WA.
>>>>
>>>> I have modified my patches to use context page for applying these WA and
>>>> don't see any issues.
>>>>
>>>> During the discussions Dave proposed another approach. Even though these
>>>> WA are called per context they are only initialized once and not changed
>>>> afterwards, same set of WA are applied for each context so instead of
>>>> adding them in each context, does it make sense to create a separate
>>>> page and share across all contexts? but of course GuC will anyway add a
>>>> new page to context so I might as well share that page.
>>>>
>>>> Chris/Dave, do you see any problems with sharing page with GuC or you
>>>> prefer to allocate a separate page for these WA and share across all
>>>> contexts? Please give your comments.
>>>>
>>>> regards
>>>> Arun
>>>
>>> I think we have to consider which is more future-proof i.e. which is
>>> least likely:
>>> (1) the area shared with the GuC grows (definitely still in flux), or
>>> (2) workarounds need to be context-specific (possible, but unlikely)
>>>
>>> So I'd prefer a single area set up just once to contain the pre- and
>>> post-context restore workaround batches. If necessary, the one area
>>> could contain multiple batches at different offsets, so we could point
>>> different contexts at different (shared) batches as required. I think
>>> they're unlikely to actually need per-context customisation[*], but
>>> there might be a need for different workarounds according to workload
>>> type or privilege level or some other criterion ... ?
>>>
>>> .Dave.
>>>
>>> [*] unless they need per-context memory addresses coded into them?
>>>
>>
>> Considering these WA are initialized only once and not changed afterwards
>> and GuC area probably grows in future which may run into the space used by
>> WA, independent single area setup makes senses.
>> I also checked spec and it is not clear whether any customization is going
>> to be required for different contexts.
>> I have modified patches to setup a single page with WA when default_context
>> is initialized and this is used by all contexts.
>>
>> I will send patches but please let me know if there are any other comments.
>
> Yeah if the wa batches aren't ctx specific, then there's really no need to
> allocate one of them per ctx. One global buffer with all the wa combined
> should really be all we need.
> -Daniel
>
Hi Daniel,

Agree, this is already taken into account in the next revision v3 
(already sent to mailing list).

I can see you are still going through the list but when you get there, 
please let me know if you have any other comments.

regards
Arun

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