[Intel-gfx] [PATCH] drm/i915/gem: stop using PAGE_KERNEL_IO

[Intel-gfx] [PATCH] drm/i915/gem: stop using PAGE_KERNEL_IO

[Lucas De Marchi]

PAGE_KERNEL_IO is only defined for x86 and is the same as PAGE_KERNEL.
Use the latter since that is also available on other archs, which should
help us getting i915 there.

This is the same that was done done in commit 80c33624e472 ("io-mapping:
Fixup for different names of writecombine"). Later the commit
80c33624e472 ("io-mapping: Fixup for different names of writecombine")
added a "Fixes" tag to the first one, but that is actually fixing a
separate issue:  the different names for pgprot_writecombine().

Fast-forward today, it seems the only 2 archs that define
pgprot_noncached_wc() are microblaze and powerpc. Microblaze has the
same definition for pgprot_writecombine() since commit
97ccedd793ac ("microblaze: Provide pgprot_device/writecombine macros for
nommu"). Powerpc has 3 variants and all of them have the same behavior
for pgprot_writecombine() and pgprot_noncached_wc(). From the commit message
and linked issue, the fallback was needed for arm, but apparently today
all the variants there also have pgprot_writecombine().

So, just use PAGE_KERNEL, and just use pgprot_writecombine().

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_pages.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 8eb1c3a6fc9c..68fe1837ef54 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -289,7 +289,7 @@ static void *i915_gem_object_map_page(struct drm_i915_gem_object *obj,
 		pgprot = PAGE_KERNEL;
 		break;
 	case I915_MAP_WC:
-		pgprot = pgprot_writecombine(PAGE_KERNEL_IO);
+		pgprot = pgprot_writecombine(PAGE_KERNEL);
 		break;
 	}
 
@@ -333,7 +333,7 @@ static void *i915_gem_object_map_pfn(struct drm_i915_gem_object *obj,
 	i = 0;
 	for_each_sgt_daddr(addr, iter, obj->mm.pages)
 		pfns[i++] = (iomap + addr) >> PAGE_SHIFT;
-	vaddr = vmap_pfn(pfns, n_pfn, pgprot_writecombine(PAGE_KERNEL_IO));
+	vaddr = vmap_pfn(pfns, n_pfn, pgprot_writecombine(PAGE_KERNEL));
 	if (pfns != stack)
 		kvfree(pfns);
 
-- 
2.33.1

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gem: stop using PAGE_KERNEL_IO

[Patchwork]

== Series Details ==

Series: drm/i915/gem: stop using PAGE_KERNEL_IO
URL   : https://patchwork.freedesktop.org/series/96045/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
419b331a6c11 drm/i915/gem: stop using PAGE_KERNEL_IO
-:10: WARNING:REPEATED_WORD: Possible repeated word: 'done'
#10: 
This is the same that was done done in commit 80c33624e472 ("io-mapping:

-:18: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 97ccedd793ac ("microblaze: Provide pgprot_device/writecombine macros for nommu")'
#18: 
same definition for pgprot_writecombine() since commit

-:21: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#21: 
for pgprot_writecombine() and pgprot_noncached_wc(). From the commit message

total: 1 errors, 2 warnings, 0 checks, 16 lines checked

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gem: stop using PAGE_KERNEL_IO

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 6512 bytes --]

== Series Details ==

Series: drm/i915/gem: stop using PAGE_KERNEL_IO
URL   : https://patchwork.freedesktop.org/series/96045/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10763 -> Patchwork_21387
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/index.html

Known issues
------------

  Here are the changes found in Patchwork_21387 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@query-info:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][1] ([fdo#109315])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-tgl-1115g4/igt@amdgpu/amd_basic@query-info.html
    - fi-kbl-soraka:      NOTRUN -> [SKIP][2] ([fdo#109271])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-kbl-soraka/igt@amdgpu/amd_basic@query-info.html

  * igt@amdgpu/amd_cs_nop@nop-gfx0:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][3] ([fdo#109315] / [i915#2575]) +16 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-tgl-1115g4/igt@amdgpu/amd_cs_nop@nop-gfx0.html

  * igt@gem_huc_copy@huc-copy:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][4] ([i915#2190])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-tgl-1115g4/igt@gem_huc_copy@huc-copy.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][5] ([i915#1155])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-tgl-1115g4/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_selftest@live@hangcheck:
    - fi-icl-y:           [PASS][6] -> [INCOMPLETE][7] ([i915#3965])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10763/fi-icl-y/igt@i915_selftest@live@hangcheck.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-icl-y/igt@i915_selftest@live@hangcheck.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][8] ([fdo#111827]) +8 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-tgl-1115g4/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][9] ([i915#4103]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-tgl-1115g4/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][10] ([fdo#109285])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-tgl-1115g4/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][11] ([i915#1072]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-tgl-1115g4/igt@kms_psr@primary_mmap_gtt.html

  * igt@prime_vgem@basic-userptr:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][12] ([i915#3301])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-tgl-1115g4/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - fi-icl-y:           NOTRUN -> [FAIL][13] ([i915#4006] / [i915#4312])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-icl-y/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@kms_flip@basic-flip-vs-modeset@c-dp1:
    - fi-cfl-8109u:       [FAIL][14] ([i915#4165]) -> [PASS][15] +1 similar issue
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10763/fi-cfl-8109u/igt@kms_flip@basic-flip-vs-modeset@c-dp1.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-cfl-8109u/igt@kms_flip@basic-flip-vs-modeset@c-dp1.html

  * igt@kms_flip@basic-plain-flip@c-dp2:
    - fi-cfl-8109u:       [DMESG-WARN][16] ([i915#295]) -> [PASS][17] +2 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10763/fi-cfl-8109u/igt@kms_flip@basic-plain-flip@c-dp2.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-cfl-8109u/igt@kms_flip@basic-plain-flip@c-dp2.html

  
#### Warnings ####

  * igt@kms_flip@basic-plain-flip@c-dp1:
    - fi-cfl-8109u:       [DMESG-WARN][18] ([i915#295]) -> [FAIL][19] ([i915#4165])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10763/fi-cfl-8109u/igt@kms_flip@basic-plain-flip@c-dp1.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/fi-cfl-8109u/igt@kms_flip@basic-plain-flip@c-dp1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#295]: https://gitlab.freedesktop.org/drm/intel/issues/295
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#3965]: https://gitlab.freedesktop.org/drm/intel/issues/3965
  [i915#4006]: https://gitlab.freedesktop.org/drm/intel/issues/4006
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4165]: https://gitlab.freedesktop.org/drm/intel/issues/4165
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312


Participating hosts (37 -> 36)
------------------------------

  Additional (1): fi-tgl-1115g4 
  Missing    (2): fi-bsw-cyan bat-dg1-6 


Build changes
-------------

  * Linux: CI_DRM_10763 -> Patchwork_21387

  CI-20190529: 20190529
  CI_DRM_10763: 4ee44225b0be875097a80b06ed976f27c87b1368 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6257: 5e32858ad62342f9ae5207e0432bf6f4c896d165 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_21387: 419b331a6c1143b5fde2be706deb647ab67800ce @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

419b331a6c11 drm/i915/gem: stop using PAGE_KERNEL_IO

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21387/index.html

[-- Attachment #2: Type: text/html, Size: 7623 bytes --]

Re: [Intel-gfx] [PATCH] drm/i915/gem: stop using PAGE_KERNEL_IO

[Daniel Vetter]

On Wed, Oct 20, 2021 at 02:06:25AM -0700, Lucas De Marchi wrote:
> PAGE_KERNEL_IO is only defined for x86 and is the same as PAGE_KERNEL.
> Use the latter since that is also available on other archs, which should
> help us getting i915 there.
> 
> This is the same that was done done in commit 80c33624e472 ("io-mapping:
> Fixup for different names of writecombine"). Later the commit
> 80c33624e472 ("io-mapping: Fixup for different names of writecombine")
> added a "Fixes" tag to the first one, but that is actually fixing a
> separate issue:  the different names for pgprot_writecombine().
> 
> Fast-forward today, it seems the only 2 archs that define
> pgprot_noncached_wc() are microblaze and powerpc. Microblaze has the
> same definition for pgprot_writecombine() since commit
> 97ccedd793ac ("microblaze: Provide pgprot_device/writecombine macros for
> nommu"). Powerpc has 3 variants and all of them have the same behavior
> for pgprot_writecombine() and pgprot_noncached_wc(). From the commit message
> and linked issue, the fallback was needed for arm, but apparently today
> all the variants there also have pgprot_writecombine().
> 
> So, just use PAGE_KERNEL, and just use pgprot_writecombine().
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

I think a bit more history on PAGE_KERNEL_IO is useful to add. It was
added in be43d72835ba ("x86: add _PAGE_IOMAP pte flag for IO mappings").
The one and only user was lost in f955371ca9d3 ("x86: remove the
Xen-specific _PAGE_IOMAP PTE flag"), therefore it's safe to do this.

With that added Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Also if you're motivated, maybe delete PAGE_KERNEL_IO across the tree and
get x86 maintainers to merge the entire series?
-Daniel


> ---
>  drivers/gpu/drm/i915/gem/i915_gem_pages.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> index 8eb1c3a6fc9c..68fe1837ef54 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
> @@ -289,7 +289,7 @@ static void *i915_gem_object_map_page(struct drm_i915_gem_object *obj,
>  		pgprot = PAGE_KERNEL;
>  		break;
>  	case I915_MAP_WC:
> -		pgprot = pgprot_writecombine(PAGE_KERNEL_IO);
> +		pgprot = pgprot_writecombine(PAGE_KERNEL);
>  		break;
>  	}
>  
> @@ -333,7 +333,7 @@ static void *i915_gem_object_map_pfn(struct drm_i915_gem_object *obj,
>  	i = 0;
>  	for_each_sgt_daddr(addr, iter, obj->mm.pages)
>  		pfns[i++] = (iomap + addr) >> PAGE_SHIFT;
> -	vaddr = vmap_pfn(pfns, n_pfn, pgprot_writecombine(PAGE_KERNEL_IO));
> +	vaddr = vmap_pfn(pfns, n_pfn, pgprot_writecombine(PAGE_KERNEL));
>  	if (pfns != stack)
>  		kvfree(pfns);
>  
> -- 
> 2.33.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch