Re: [PATCH 0/4] Drop wbinvd_on_all_cpus usage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 21/03/2022 13:40, Thomas Hellström wrote:
Hi,

On Mon, 2022-03-21 at 13:12 +0000, Tvrtko Ursulin wrote:

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?

System bos on discrete should always have

I915_BO_CACHE_COHERENT_FOR_READ | I915_BO_CACHE_COHERENT_FOR_WRITE

either by the gpu being fully cache coherent (or us mapping system
write-combined). Hence no need for cache clflushes or wbinvd() for
incoherent IO.

Hmm so you are talking about the shmem ttm backend. It ends up depending on the result of i915_ttm_cache_level, yes? It cannot end up with I915_CACHE_NONE from that function?

I also found in i915_drm.h:

	 * As caching mode when specifying `I915_MMAP_OFFSET_FIXED`, WC or WB will
	 * be used, depending on the object placement on creation. WB will be used
	 * when the object can only exist in system memory, WC otherwise.

If what you say is true, that on discrete it is _always_ WC, then that needs updating as well.


That's adhering to Linus'

"And I sincerely hope to the gods that no cache-incoherent i915 mess
ever makes it out of the x86 world. Incoherent IO was always a
historical mistake and should never ever happen again, so we should
not spread that horrific pattern around."

Sure, but I was not talking about IO - just the CPU side access to CPU side objects.

Regards,

Tvrtko



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux