[Intel-gfx] [PATCH v6] drm/i915: Add IOCTL Param to control data port coherency.

[Lis, Tomasz]

On 2018-07-19 09:12, Joonas Lahtinen wrote:
> Quoting Lis, Tomasz (2018-07-18 18:28:32)
>> On 2018-07-18 16:42, Tvrtko Ursulin wrote:
>>> On 18/07/2018 14:24, Joonas Lahtinen wrote:
>>>> Quoting Tomasz Lis (2018-07-16 16:07:16)
> <SNIP>
>
>>>>> +++ b/include/uapi/drm/i915_drm.h
>>>>> @@ -1456,6 +1456,13 @@ struct drm_i915_gem_context_param {
>>>>>    #define   I915_CONTEXT_MAX_USER_PRIORITY       1023 /* inclusive */
>>>>>    #define   I915_CONTEXT_DEFAULT_PRIORITY                0
>>>>>    #define   I915_CONTEXT_MIN_USER_PRIORITY       -1023 /* inclusive */
>>>>> +/*
>>>>> + * When data port level coherency is enabled, the GPU will update
>>>>> memory
>>>>> + * buffers shared with CPU, by forcing internal cache units to send
>>>>> memory
>>>>> + * writes to higher level caches faster. Enabling data port
>>>>> coherency has
>>>>> + * a performance cost.
>>>>> + */
>>>> I was under impression this is enabled by default and it can be disabled
>>>> for a performance optimization?
>> This is true, coherency is kept by default. We disable it as a
>> workaround: performance-related for gen11, and due to minor hardware
>> issue on previous platforms. See WaForceEnableNonCoherent.
> Ok, then you definitely want to rephrase the comment to bake that
> information in it. Now it sounds like it needs to be turned on to have
> coherency.
I'm not sure if I understand what you're asking for.
Should I emphasize that the feature is disabled unless the flag is set? 
This seem obvious...
Or should I provide the reason why it is disabled on specific platforms? 
This should probably be done within workaround setup, not in user api 
definition. Or maybe it's enough to have it in Bspec? Bspec links are 
provided in the patch.
Or should I just mention the workaround name?
-Tomasz
> Regards, Joonas