Re: [Intel-gfx] [PATCH] drm/i915: Skip remap_io_mapping() for non-x86 platforms

[Lucas De Marchi <lucas.demarchi@intel.com>]

On Sat, Nov 13, 2021 at 12:18:10AM +0200, Jani Nikula wrote:
>On Fri, 12 Nov 2021, Mullati Siva <siva.mullati@intel.com> wrote:
>> The _PAGE_CACHE_MASK macro is not defined in non-x86
>> architectures and it's been used in remap_io_mapping().
>> Only hw that supports mappable aperture would hit this path
>> remap_io_mapping(), So skip this code for non-x86 architectures.
>
>Patch changelog goes here.
>
>> Signed-off-by: Mullati Siva <siva.mullati@intel.com>

Is your git config correct?


>> ---
>>  drivers/gpu/drm/i915/i915_mm.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
>> index 666808cb3a32..d76feeaf3fd1 100644
>> --- a/drivers/gpu/drm/i915/i915_mm.c
>> +++ b/drivers/gpu/drm/i915/i915_mm.c
>> @@ -91,6 +91,7 @@ int remap_io_mapping(struct vm_area_struct *vma,
>>  		     unsigned long addr, unsigned long pfn, unsigned long size,
>>  		     struct io_mapping *iomap)
>>  {
>> +#if IS_ENABLED(CONFIG_X86)
>
>My feedback to the previous version was:
>
>Please don't add conditional compilation within functions.
>
>I mean it.

if it's not clear, that means we should have a stub remap_io_mapping()
in the header rather than doing the conditional compilation here.

However, I'm still not confident about the approach since in DG1 for
example we have mappable aperture. And even for the cases we don't need
it, are you sure we never call remap_io_mapping()?  Did you add a trace
or printk() in this function to confirm that while loading the module?

Lastly, see commit b87482dfe800 ("Revert "i915: use io_mapping_map_user"")
We had a function added in mm/io-mapping.c - io_mapping_map_user() that
is now unused.

We still want to use that api (see https://lore.kernel.org/all/20211021061839.GA27953@lst.de/so).
So since you are touching this area, it would be good if you can help
figure out why it was failing.

thanks
Lucas De Marchi