Ah, thanks for pointing that out, when I do include it though, it causes a few warning other systems such as h8300 and s390.

Since it is already pulled is, would it be OK to leave it out for this case? Or we could use something like !IS_H8300 and !IS_S390

around the header file?

On 2022-02-25 9:33 a.m., Tvrtko Ursulin wrote:

On 25/02/2022 16:52, Michael Cheng wrote:
Hi Tvrtko,

It seems without cacheflush.h being included, when I build for arm64 or x86, it stills pulls in cacheflush.h:

./.drm_cache.o.cmd:838: include/linux/cacheflush.h \
./.drm_cache.o.cmd:839: arch/x86/include/asm/cacheflush.h \
./.drm_cache.o.cmd:920: include/asm-generic/cacheflush.h \
./.drm_cache.o.cmd:830: include/linux/cacheflush.h \
./.drm_cache.o.cmd:831: arch/arm64/include/asm/cacheflush.h \
./.drm_cache.o.cmd:1085: include/asm-generic/cacheflush.h \
So it seems without including it, cacheflush.h stills get pulled in,
I think its because its a required kernel source to build the kernel
per specific architecture, but please correct if I am wrong,as I am still
trying to understand how things works!

Probably:

drm_cache.c:

#include <linux/highmem.h>

linux/highmem.h:

#include <linux/cacheflush.h>

But it is more correct to explicitly include what you use. So if drm_cache.c uses stuff declared in cacheflush.h, it should include it.

Regards,

Tvrtko

Michael Cheng
On 2022-02-25 8:28 a.m., Tvrtko Ursulin wrote:

On 25/02/2022 03:24, Michael Cheng wrote:
Add arm64 support for drm_clflush_virt_range. caches_clean_inval_pou
performs a flush by first performing a clean, follow by an invalidation
operation.

v2 (Michael Cheng): Use correct macro for cleaning and invalidation the
            dcache. Thanks Tvrtko for the suggestion.

v3 (Michael Cheng): Replace asm/cacheflush.h with linux/cacheflush.h

v4 (Michael Cheng): Arm64 does not export dcache_clean_inval_poc as a
            symbol that could be use by other modules, thus use
            caches_clean_inval_pou instead. Also this version
                removes include for cacheflush, since its already
            included base on architecture type.

What does it mean that it is included based on architecture type? Some of the other header already pulls it in?

Regards,

Tvrtko

Signed-off-by: Michael Cheng <michael.ch...@intel.com>
Reviewed-by: Matt Roper <matthew.d.ro...@intel.com>
---
  drivers/gpu/drm/drm_cache.c | 5 +++++
  1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index c3e6e615bf09..81c28714f930 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -174,6 +174,11 @@ drm_clflush_virt_range(void *addr, unsigned long length)
        if (wbinvd_on_all_cpus())
          pr_err("Timed out waiting for cache flush\n");
+
+#elif defined(CONFIG_ARM64)
+    void *end = addr + length;
+    caches_clean_inval_pou((unsigned long)addr, (unsigned long)end);
+
  #else
      WARN_ONCE(1, "Architecture has no drm_cache.c support\n");
  #endif

Reply via email to