[Intel-gfx] [RFC v1] drm/i915: Add Exec param to control data port coherency.

[Lis, Tomasz]

On 2018-03-21 20:42, Oscar Mateo wrote:
>
>
> On 3/21/2018 3:16 AM, Chris Wilson wrote:
>> Quoting Oscar Mateo (2018-03-20 18:43:45)
>>>
>>> On 3/19/2018 7:14 AM, Lis, Tomasz wrote:
>>>>
>>>> On 2018-03-19 13:43, Chris Wilson wrote:
>>>>> Quoting Tomasz Lis (2018-03-19 12:37:35)
>>>>>> The patch adds a parameter to control the data port coherency
>>>>>> functionality
>>>>>> on a per-exec call basis. When data port coherency flag value is
>>>>>> different
>>>>>> than what it was in previous call for the context, a command to
>>>>>> switch data
>>>>>> port coherency state is added before the buffer to be executed.
>>>>> So this is part of the context? Why do it at exec level?
>>>> It is part of the context, stored within HDC chicken bit register.
>>>> The exec level was requested by the OCL team, due to concerns about
>>>> performance cost of context setparam calls.
>>>>
>>>>>    If exec level
>>>>> is desired, why not whitelist it?
>>>>> -Chris
>>>> If we have no issue in whitelisting the register, I'm sure OCL will
>>>> agree to that.
>>>> I assumed the whitelisting will be unacceptable because of security
>>>> concerns with some options.
>>>> The register also changes its position and content between gens, which
>>>> makes whitelisting hard to manage.
>>>>
>>> I think a security analysis of this register was already done, and the
>>> result was that it contains some other bits that could be dangerous. In
>>> CNL those bits were moved out of the way and the HDC_CHICKEN0 register
>>> can be whitelisted (WaAllowUMDToControlCoherency). In ICL the register
>>> should already be non-privileged.
>> The previous alternative to whitelisting was running through a command
>> parser for validation. That's a very general mechanism suitable for a
>> wide range of sins.
>> -Chris
>
> Are you suggesting that we enable the cmd parser for every Gen < CNL 
> for this particular usage only? :P
>
It is a solution that would allow to do what we want without any 
additions to module interface.
It may be worth considering if we think the coherency setting will be 
temporary and removed in future gens, as we wouldn't want to have 
obsolete flags.

I think the setting will stay with us, as it is needed to support 
CL_MEM_SVM_FINE_GRAIN_BUFFER flag from OpenCL spec.
Keeping coherency will always cost performance, so we will likely always 
have a hardware setting to switch it.
But the bspec says coherency override control will be removed in future 
projects...