Re: [Intel-gfx] [PATCH v12 1/6] drm: Add arch arm64 for drm_clflush_virt_range

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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