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@intel.com > Reviewed-by: Matt Roper matthew.d.roper@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