Re: [Intel-gfx] [PATCH 0/4] Drop wbinvd_on_all_cpus usage

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

On 21/03/2022 12:33, Thomas Hellström wrote:
> On Mon, 2022-03-21 at 12:22 +0000, Tvrtko Ursulin wrote:
>>
>> On 21/03/2022 11:03, Thomas Hellström wrote:
>>> Hi, Tvrtko.
>>>
>>> On 3/21/22 11:27, Tvrtko Ursulin wrote:
>>>>
>>>> On 19/03/2022 19:42, Michael Cheng wrote:
>>>>> To align with the discussion in [1][2], this patch series drops
>>>>> all
>>>>> usage of
>>>>> wbvind_on_all_cpus within i915 by either replacing the call
>>>>> with certain
>>>>> drm clflush helpers, or reverting to a previous logic.
>>>>
>>>> AFAIU, complaint from [1] was that it is wrong to provide non x86
>>>> implementations under the wbinvd_on_all_cpus name. Instead an
>>>> arch
>>>> agnostic helper which achieves the same effect could be created.
>>>> Does
>>>> Arm have such concept?
>>>
>>> I also understand Linus' email like we shouldn't leak incoherent IO
>>> to
>>> other architectures, meaning any remaining wbinvd()s should be X86
>>> only.
>>
>> The last part is completely obvious since it is a x86 instruction
>> name.
> 
> Yeah, I meant the function implementing wbinvd() semantics.
> 
>>
>> But I think we can't pick a solution until we know how the concept
>> maps
>> to Arm and that will also include seeing how the drm_clflush_sg for
>> Arm
>> would look. Is there a range based solution, or just a big hammer
>> there.
>> If the latter, then it is no good to churn all these reverts but
>> instead
>> an arch agnostic wrapper, with a generic name, would be the way to
>> go.
> 
> But my impression was that ARM would not need the range-based interface
> either, because ARM is only for discrete and with discrete we're always
> coherent.

Not sure what you mean here - what about flushing system memory objects 
on discrete? Those still need flushing on paths like suspend which this 
series touches. Am I missing something?

If I am not, then that means we either keep the current, presumably 
optimised (wasn't personally involved so I don't know), flush once code 
paths and add a wrapper i915_flush_caches/whatever, or convert all those 
back into piece-meal flushes so range flushing can be done. Assuming Arm 
does range flushing. That's why I asked what does Arm have here.

> So in essence it all would become:
> 
> 1) Any cache flushing intended for incoherent IO is x86 only.
> 2) Prefer range-based flushing if possible and any implications sorted
> out.

Yes, the question is how to do it.

Regards,

Tvrtko