On 25/02/2022 18:23, Michael Cheng wrote:
These seem to be pretty old arch and are day0 warnings, please refer to
[1] to see the warnings. Also I am not sure why my patch series didn't
append to the old one.
[1] https://patchwork.freedesktop.org/patch/475829/?series=99450&rev=11
include/linux/cacheflush.h:12:46: warning: declaration of 'struct folio' will not be visible outside of this function [-Wvisibility]
That?
Looks like the #else path needs to forward declare struct folio or include the relevant header.
+Matthew Wilcox
Matthew, what do you think fix for this build warning on h8300 and s390 should be? Or perhaps a build environment issue with kernel test robot?
Regards,
Tvrtko
2022-02-25 10:19 a.m., Tvrtko Ursulin wrote:
On 25/02/2022 17:40, Michael Cheng wrote:
Ah, thanks for pointing that out, when I do include it though, it
causes a few warning other systems such as h8300 and s390.
Errors look like? I haven't heard that kernel code is not allowed to
include something from linux/ on some arch yet.
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?
Unlikely, now you made me curious why it does not work.
Regards,
Tvrtko
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.cheng@xxxxxxxxx>
Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
---
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
