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. 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." /Thomas
