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

[Michael Cheng <michael.cheng@intel.com>]

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.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