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

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

[Mullati Siva]

From: Siva Mullati <siva.mullati@intel.com>

Only hw that supports mappable aperture would hit this path
vm_fault_gtt/vm_fault_tmm, So we never hit this function
remap_io_mapping() in discrete, So skip this code for non-x86
architectures.

Signed-off-by: Siva Mullati <siva.mullati@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c |  1 +
 drivers/gpu/drm/i915/i915_drv.h          |  8 ------
 drivers/gpu/drm/i915/i915_mm.c           |  1 +
 drivers/gpu/drm/i915/i915_mm.h           | 32 ++++++++++++++++++++++++
 4 files changed, 34 insertions(+), 8 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_mm.h

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 65fc6ff5f59d..39bb15eafc07 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -17,6 +17,7 @@
 #include "i915_gem_ioctls.h"
 #include "i915_gem_object.h"
 #include "i915_gem_mman.h"
+#include "i915_mm.h"
 #include "i915_trace.h"
 #include "i915_user_extensions.h"
 #include "i915_gem_ttm.h"
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1bfadd9127fc..7ae0f0cc6866 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1967,14 +1967,6 @@ mkwrite_device_info(struct drm_i915_private *dev_priv)
 int i915_reg_read_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file);
 
-/* i915_mm.c */
-int remap_io_mapping(struct vm_area_struct *vma,
-		     unsigned long addr, unsigned long pfn, unsigned long size,
-		     struct io_mapping *iomap);
-int remap_io_sg(struct vm_area_struct *vma,
-		unsigned long addr, unsigned long size,
-		struct scatterlist *sgl, resource_size_t iobase);
-
 static inline int intel_hws_csb_write_index(struct drm_i915_private *i915)
 {
 	if (GRAPHICS_VER(i915) >= 11)
diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
index 666808cb3a32..f4df15fe7cf8 100644
--- a/drivers/gpu/drm/i915/i915_mm.c
+++ b/drivers/gpu/drm/i915/i915_mm.c
@@ -27,6 +27,7 @@
 
 
 #include "i915_drv.h"
+#include "i915_mm.h"
 
 struct remap_pfn {
 	struct mm_struct *mm;
diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h
new file mode 100644
index 000000000000..1d3bbb9cbf43
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_mm.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2021 Intel Corporation
+ */
+
+#ifndef __I915_MM_H__
+#define __I915_MM_H__
+
+#include <linux/types.h>
+
+struct vm_area_struct;
+struct io_mapping;
+struct scatterlist;
+
+#if IS_ENABLED(CONFIG_X86)
+int remap_io_mapping(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn, unsigned long size,
+		struct io_mapping *iomap);
+#else
+static inline int remap_io_mapping(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn, unsigned long size,
+		struct io_mapping *iomap)
+{
+	return 0;
+}
+#endif
+
+int remap_io_sg(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long size,
+		struct scatterlist *sgl, resource_size_t iobase);
+
+#endif /* __I915_MM_H__ */
-- 
2.33.0

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Skip remap_io_mapping() for non-x86 platforms (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Skip remap_io_mapping() for non-x86 platforms (rev3)
URL   : https://patchwork.freedesktop.org/series/96855/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
235bc0244f4a drm/i915: Skip remap_io_mapping() for non-x86 platforms
-:57: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#57: 
new file mode 100644

-:78: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#78: FILE: drivers/gpu/drm/i915/i915_mm.h:17:
+int remap_io_mapping(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn, unsigned long size,

-:82: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#82: FILE: drivers/gpu/drm/i915/i915_mm.h:21:
+static inline int remap_io_mapping(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn, unsigned long size,

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

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Skip remap_io_mapping() for non-x86 platforms (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Skip remap_io_mapping() for non-x86 platforms (rev3)
URL   : https://patchwork.freedesktop.org/series/96855/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Skip remap_io_mapping() for non-x86 platforms (rev3)

[Patchwork]

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

== Series Details ==

Series: drm/i915: Skip remap_io_mapping() for non-x86 platforms (rev3)
URL   : https://patchwork.freedesktop.org/series/96855/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10915 -> Patchwork_21655
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_21655 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_21655, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

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

Participating hosts (41 -> 35)
------------------------------

  Additional (2): fi-kbl-soraka fi-icl-u2 
  Missing    (8): bat-dg1-6 fi-tgl-u2 bat-dg1-5 fi-bsw-cyan bat-adlp-6 bat-adlp-4 bat-jsl-2 bat-jsl-1 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_21655:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-skl-6600u:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10915/fi-skl-6600u/igt@gem_exec_suspend@basic-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-skl-6600u/igt@gem_exec_suspend@basic-s3.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@fork-gfx0:
    - fi-icl-u2:          NOTRUN -> [SKIP][3] ([fdo#109315]) +17 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-icl-u2/igt@amdgpu/amd_cs_nop@fork-gfx0.html

  * igt@amdgpu/amd_cs_nop@sync-gfx0:
    - fi-bsw-n3050:       NOTRUN -> [SKIP][4] ([fdo#109271]) +17 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-bsw-n3050/igt@amdgpu/amd_cs_nop@sync-gfx0.html

  * igt@gem_exec_fence@basic-busy@bcs0:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][5] ([fdo#109271]) +12 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-kbl-soraka/igt@gem_exec_fence@basic-busy@bcs0.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#2190])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html
    - fi-icl-u2:          NOTRUN -> [SKIP][7] ([i915#2190])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-icl-u2/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - fi-icl-u2:          NOTRUN -> [SKIP][8] ([i915#4555]) +3 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-icl-u2/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][9] ([i915#1886] / [i915#2291])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

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

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          NOTRUN -> [SKIP][11] ([fdo#111827]) +8 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-icl-u2:          NOTRUN -> [SKIP][12] ([fdo#109278]) +2 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-icl-u2:          NOTRUN -> [SKIP][13] ([fdo#109285])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-icl-u2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][14] ([fdo#109271] / [i915#533])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-kbl-soraka/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@prime_vgem@basic-userptr:
    - fi-icl-u2:          NOTRUN -> [SKIP][15] ([i915#3301])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-icl-u2/igt@prime_vgem@basic-userptr.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gtt:
    - {fi-tgl-dsi}:       [FAIL][16] ([i915#2927]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10915/fi-tgl-dsi/igt@i915_selftest@live@gtt.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-tgl-dsi/igt@i915_selftest@live@gtt.html

  * igt@i915_selftest@live@late_gt_pm:
    - fi-bsw-n3050:       [DMESG-FAIL][18] ([i915#2927] / [i915#3428]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10915/fi-bsw-n3050/igt@i915_selftest@live@late_gt_pm.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21655/fi-bsw-n3050/igt@i915_selftest@live@late_gt_pm.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#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [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#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2291]: https://gitlab.freedesktop.org/drm/intel/issues/2291
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2927]: https://gitlab.freedesktop.org/drm/intel/issues/2927
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3428]: https://gitlab.freedesktop.org/drm/intel/issues/3428
  [i915#4555]: https://gitlab.freedesktop.org/drm/intel/issues/4555
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533


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

  * Linux: CI_DRM_10915 -> Patchwork_21655

  CI-20190529: 20190529
  CI_DRM_10915: 84c86240b97d22658b6939903d77b644764a0ae8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6286: cdcbf81f734fdb1d102e84490e49e9fec23760cd @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_21655: 235bc0244f4a6324c6ab882adbfc34d2eb11377c @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

235bc0244f4a drm/i915: Skip remap_io_mapping() for non-x86 platforms

== Logs ==

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

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

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

[Lucas De Marchi]

On Mon, Nov 22, 2021 at 06:01:42PM +0530, Mullati Siva wrote:
>From: Siva Mullati <siva.mullati@intel.com>
>
>Only hw that supports mappable aperture would hit this path
>vm_fault_gtt/vm_fault_tmm, So we never hit this function
>remap_io_mapping() in discrete, So skip this code for non-x86
>architectures.
>
>Signed-off-by: Siva Mullati <siva.mullati@intel.com>
>---
> drivers/gpu/drm/i915/gem/i915_gem_mman.c |  1 +
> drivers/gpu/drm/i915/i915_drv.h          |  8 ------
> drivers/gpu/drm/i915/i915_mm.c           |  1 +
> drivers/gpu/drm/i915/i915_mm.h           | 32 ++++++++++++++++++++++++
> 4 files changed, 34 insertions(+), 8 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/i915_mm.h
>
>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>index 65fc6ff5f59d..39bb15eafc07 100644
>--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>@@ -17,6 +17,7 @@
> #include "i915_gem_ioctls.h"
> #include "i915_gem_object.h"
> #include "i915_gem_mman.h"
>+#include "i915_mm.h"
> #include "i915_trace.h"
> #include "i915_user_extensions.h"
> #include "i915_gem_ttm.h"
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index 1bfadd9127fc..7ae0f0cc6866 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -1967,14 +1967,6 @@ mkwrite_device_info(struct drm_i915_private *dev_priv)
> int i915_reg_read_ioctl(struct drm_device *dev, void *data,
> 			struct drm_file *file);
>
>-/* i915_mm.c */
>-int remap_io_mapping(struct vm_area_struct *vma,
>-		     unsigned long addr, unsigned long pfn, unsigned long size,
>-		     struct io_mapping *iomap);
>-int remap_io_sg(struct vm_area_struct *vma,
>-		unsigned long addr, unsigned long size,
>-		struct scatterlist *sgl, resource_size_t iobase);
>-
> static inline int intel_hws_csb_write_index(struct drm_i915_private *i915)
> {
> 	if (GRAPHICS_VER(i915) >= 11)
>diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
>index 666808cb3a32..f4df15fe7cf8 100644
>--- a/drivers/gpu/drm/i915/i915_mm.c
>+++ b/drivers/gpu/drm/i915/i915_mm.c
>@@ -27,6 +27,7 @@
>
>
> #include "i915_drv.h"
>+#include "i915_mm.h"
>
> struct remap_pfn {
> 	struct mm_struct *mm;
>diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h
>new file mode 100644
>index 000000000000..1d3bbb9cbf43
>--- /dev/null
>+++ b/drivers/gpu/drm/i915/i915_mm.h
>@@ -0,0 +1,32 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2021 Intel Corporation
>+ */
>+
>+#ifndef __I915_MM_H__
>+#define __I915_MM_H__
>+
>+#include <linux/types.h>
>+
>+struct vm_area_struct;
>+struct io_mapping;
>+struct scatterlist;
>+
>+#if IS_ENABLED(CONFIG_X86)
>+int remap_io_mapping(struct vm_area_struct *vma,
>+		unsigned long addr, unsigned long pfn, unsigned long size,
>+		struct io_mapping *iomap);
>+#else
>+static inline int remap_io_mapping(struct vm_area_struct *vma,
>+		unsigned long addr, unsigned long pfn, unsigned long size,
>+		struct io_mapping *iomap)
>+{

would probably be good to add:

	pr_err("Architecture has no remap_io_mapping() and shouldn't be calling this function\n");
	WARN_ON_ONCE(1);

the same way that is done in drivers/gpu/drm/drm_cache.c

Other than that:


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Since you're adding this header, can you follow up with one additional
patch to move the rest of the prototypes off i915_drv.h and into this
new header?


thanks
Lucas De Marchi


>+	return 0;
>+}
>+#endif
>+
>+int remap_io_sg(struct vm_area_struct *vma,
>+		unsigned long addr, unsigned long size,
>+		struct scatterlist *sgl, resource_size_t iobase);
>+
>+#endif /* __I915_MM_H__ */
>-- 
>2.33.0
>

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

[Jani Nikula]

On Tue, 30 Nov 2021, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Mon, Nov 22, 2021 at 06:01:42PM +0530, Mullati Siva wrote:
>>From: Siva Mullati <siva.mullati@intel.com>
>>
>>Only hw that supports mappable aperture would hit this path
>>vm_fault_gtt/vm_fault_tmm, So we never hit this function
>>remap_io_mapping() in discrete, So skip this code for non-x86
>>architectures.
>>
>>Signed-off-by: Siva Mullati <siva.mullati@intel.com>
>>---
>> drivers/gpu/drm/i915/gem/i915_gem_mman.c |  1 +
>> drivers/gpu/drm/i915/i915_drv.h          |  8 ------
>> drivers/gpu/drm/i915/i915_mm.c           |  1 +
>> drivers/gpu/drm/i915/i915_mm.h           | 32 ++++++++++++++++++++++++
>> 4 files changed, 34 insertions(+), 8 deletions(-)
>> create mode 100644 drivers/gpu/drm/i915/i915_mm.h
>>
>>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>>index 65fc6ff5f59d..39bb15eafc07 100644
>>--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
>>@@ -17,6 +17,7 @@
>> #include "i915_gem_ioctls.h"
>> #include "i915_gem_object.h"
>> #include "i915_gem_mman.h"
>>+#include "i915_mm.h"
>> #include "i915_trace.h"
>> #include "i915_user_extensions.h"
>> #include "i915_gem_ttm.h"
>>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>index 1bfadd9127fc..7ae0f0cc6866 100644
>>--- a/drivers/gpu/drm/i915/i915_drv.h
>>+++ b/drivers/gpu/drm/i915/i915_drv.h
>>@@ -1967,14 +1967,6 @@ mkwrite_device_info(struct drm_i915_private *dev_priv)
>> int i915_reg_read_ioctl(struct drm_device *dev, void *data,
>> 			struct drm_file *file);
>>
>>-/* i915_mm.c */
>>-int remap_io_mapping(struct vm_area_struct *vma,
>>-		     unsigned long addr, unsigned long pfn, unsigned long size,
>>-		     struct io_mapping *iomap);
>>-int remap_io_sg(struct vm_area_struct *vma,
>>-		unsigned long addr, unsigned long size,
>>-		struct scatterlist *sgl, resource_size_t iobase);
>>-
>> static inline int intel_hws_csb_write_index(struct drm_i915_private *i915)
>> {
>> 	if (GRAPHICS_VER(i915) >= 11)
>>diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
>>index 666808cb3a32..f4df15fe7cf8 100644
>>--- a/drivers/gpu/drm/i915/i915_mm.c
>>+++ b/drivers/gpu/drm/i915/i915_mm.c
>>@@ -27,6 +27,7 @@
>>
>>
>> #include "i915_drv.h"
>>+#include "i915_mm.h"
>>
>> struct remap_pfn {
>> 	struct mm_struct *mm;
>>diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h
>>new file mode 100644
>>index 000000000000..1d3bbb9cbf43
>>--- /dev/null
>>+++ b/drivers/gpu/drm/i915/i915_mm.h
>>@@ -0,0 +1,32 @@
>>+/* SPDX-License-Identifier: MIT */
>>+/*
>>+ * Copyright © 2021 Intel Corporation
>>+ */
>>+
>>+#ifndef __I915_MM_H__
>>+#define __I915_MM_H__
>>+
>>+#include <linux/types.h>
>>+
>>+struct vm_area_struct;
>>+struct io_mapping;
>>+struct scatterlist;
>>+
>>+#if IS_ENABLED(CONFIG_X86)
>>+int remap_io_mapping(struct vm_area_struct *vma,
>>+		unsigned long addr, unsigned long pfn, unsigned long size,
>>+		struct io_mapping *iomap);
>>+#else
>>+static inline int remap_io_mapping(struct vm_area_struct *vma,
>>+		unsigned long addr, unsigned long pfn, unsigned long size,
>>+		struct io_mapping *iomap)
>>+{
>
> would probably be good to add:
>
> 	pr_err("Architecture has no remap_io_mapping() and shouldn't be calling this function\n");
> 	WARN_ON_ONCE(1);
>
> the same way that is done in drivers/gpu/drm/drm_cache.c
>
> Other than that:
>
>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
> Since you're adding this header, can you follow up with one additional
> patch to move the rest of the prototypes off i915_drv.h and into this
> new header?

There's only the two functions?

BR,
Jani.


>
>
> thanks
> Lucas De Marchi
>
>
>>+	return 0;
>>+}
>>+#endif
>>+
>>+int remap_io_sg(struct vm_area_struct *vma,
>>+		unsigned long addr, unsigned long size,
>>+		struct scatterlist *sgl, resource_size_t iobase);
>>+
>>+#endif /* __I915_MM_H__ */
>>-- 
>>2.33.0
>>

-- 
Jani Nikula, Intel Open Source Graphics Center

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

[Mullati Siva]

From: Siva Mullati <siva.mullati@intel.com>

Only hw that supports mappable aperture would hit this path
vm_fault_gtt/vm_fault_tmm, So we never hit this function
remap_io_mapping() in discrete, So skip this code for non-x86
architectures.

v2: use IS_ENABLED () instead of #if defined

v3: move function prototypes from i915_drv.h to i915_mm.h

v4: added kernel error message in stub function

v5: fixed compilation warnings

v6: checkpatch style

Signed-off-by: Siva Mullati <siva.mullati@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c |  1 +
 drivers/gpu/drm/i915/i915_drv.h          |  8 ------
 drivers/gpu/drm/i915/i915_mm.c           | 28 +++++++++++--------
 drivers/gpu/drm/i915/i915_mm.h           | 35 ++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 20 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_mm.h

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 65fc6ff5f59d..39bb15eafc07 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -17,6 +17,7 @@
 #include "i915_gem_ioctls.h"
 #include "i915_gem_object.h"
 #include "i915_gem_mman.h"
+#include "i915_mm.h"
 #include "i915_trace.h"
 #include "i915_user_extensions.h"
 #include "i915_gem_ttm.h"
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 85bb8d3107f0..3949cafa59e2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1783,14 +1783,6 @@ mkwrite_device_info(struct drm_i915_private *dev_priv)
 int i915_reg_read_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file);
 
-/* i915_mm.c */
-int remap_io_mapping(struct vm_area_struct *vma,
-		     unsigned long addr, unsigned long pfn, unsigned long size,
-		     struct io_mapping *iomap);
-int remap_io_sg(struct vm_area_struct *vma,
-		unsigned long addr, unsigned long size,
-		struct scatterlist *sgl, resource_size_t iobase);
-
 static inline int intel_hws_csb_write_index(struct drm_i915_private *i915)
 {
 	if (GRAPHICS_VER(i915) >= 11)
diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
index 666808cb3a32..7998bc74ab49 100644
--- a/drivers/gpu/drm/i915/i915_mm.c
+++ b/drivers/gpu/drm/i915/i915_mm.c
@@ -27,6 +27,7 @@
 
 
 #include "i915_drv.h"
+#include "i915_mm.h"
 
 struct remap_pfn {
 	struct mm_struct *mm;
@@ -37,17 +38,6 @@ struct remap_pfn {
 	resource_size_t iobase;
 };
 
-static int remap_pfn(pte_t *pte, unsigned long addr, void *data)
-{
-	struct remap_pfn *r = data;
-
-	/* Special PTE are not associated with any struct page */
-	set_pte_at(r->mm, addr, pte, pte_mkspecial(pfn_pte(r->pfn, r->prot)));
-	r->pfn++;
-
-	return 0;
-}
-
 #define use_dma(io) ((io) != -1)
 
 static inline unsigned long sgt_pfn(const struct remap_pfn *r)
@@ -77,6 +67,20 @@ static int remap_sg(pte_t *pte, unsigned long addr, void *data)
 	return 0;
 }
 
+#define EXPECTED_FLAGS (VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP)
+
+#if IS_ENABLED(CONFIG_X86)
+static int remap_pfn(pte_t *pte, unsigned long addr, void *data)
+{
+	struct remap_pfn *r = data;
+
+	/* Special PTE are not associated with any struct page */
+	set_pte_at(r->mm, addr, pte, pte_mkspecial(pfn_pte(r->pfn, r->prot)));
+	r->pfn++;
+
+	return 0;
+}
+
 /**
  * remap_io_mapping - remap an IO mapping to userspace
  * @vma: user vma to map to
@@ -94,7 +98,6 @@ int remap_io_mapping(struct vm_area_struct *vma,
 	struct remap_pfn r;
 	int err;
 
-#define EXPECTED_FLAGS (VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP)
 	GEM_BUG_ON((vma->vm_flags & EXPECTED_FLAGS) != EXPECTED_FLAGS);
 
 	/* We rely on prevalidation of the io-mapping to skip track_pfn(). */
@@ -111,6 +114,7 @@ int remap_io_mapping(struct vm_area_struct *vma,
 
 	return 0;
 }
+#endif
 
 /**
  * remap_io_sg - remap an IO mapping to userspace
diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h
new file mode 100644
index 000000000000..76f1d53bdf34
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_mm.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2021 Intel Corporation
+ */
+
+#ifndef __I915_MM_H__
+#define __I915_MM_H__
+
+#include <linux/types.h>
+
+struct vm_area_struct;
+struct io_mapping;
+struct scatterlist;
+
+#if IS_ENABLED(CONFIG_X86)
+int remap_io_mapping(struct vm_area_struct *vma,
+		     unsigned long addr, unsigned long pfn, unsigned long size,
+		     struct io_mapping *iomap);
+#else
+static inline
+int remap_io_mapping(struct vm_area_struct *vma,
+		     unsigned long addr, unsigned long pfn, unsigned long size,
+		     struct io_mapping *iomap)
+{
+	pr_err("Architecture has no %s() and shouldn't be calling this function\n", __func__);
+	WARN_ON_ONCE(1);
+	return 0;
+}
+#endif
+
+int remap_io_sg(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long size,
+		struct scatterlist *sgl, resource_size_t iobase);
+
+#endif /* __I915_MM_H__ */
-- 
2.33.0

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

[Lucas De Marchi]

On Wed, Dec 08, 2021 at 09:42:15AM +0530, Mullati Siva wrote:
>From: Siva Mullati <siva.mullati@intel.com>
>
>Only hw that supports mappable aperture would hit this path
>vm_fault_gtt/vm_fault_tmm, So we never hit this function
>remap_io_mapping() in discrete, So skip this code for non-x86
>architectures.
>
>v2: use IS_ENABLED () instead of #if defined
>
>v3: move function prototypes from i915_drv.h to i915_mm.h
>
>v4: added kernel error message in stub function
>
>v5: fixed compilation warnings
>
>v6: checkpatch style
>
>Signed-off-by: Siva Mullati <siva.mullati@intel.com>

the change in gem/ is just adding the additional include - looking at
the other changes, drm-intel-next seems a better place to merge this
since it can be considered as part of the i915_drv.h cleanup going on.

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

and applied, thanks.

Lucas De Marchi

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

[Mullati Siva]

From: Siva Mullati <siva.mullati@intel.com>

Only hw that supports mappable aperture would hit this path
vm_fault_gtt/vm_fault_tmm, So we never hit this function
remap_io_mapping() in discrete, So skip this code for non-x86
architectures.

v2: use IS_ENABLED () instead of #if defined

v3: move function prototypes from i915_drv.h to i915_mm.h

v4: added kernel error message in stub function

v5: fixed compilation warnings

Signed-off-by: Siva Mullati <siva.mullati@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c |  1 +
 drivers/gpu/drm/i915/i915_drv.h          |  8 ------
 drivers/gpu/drm/i915/i915_mm.c           | 28 ++++++++++---------
 drivers/gpu/drm/i915/i915_mm.h           | 34 ++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 20 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_mm.h

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 65fc6ff5f59d..39bb15eafc07 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -17,6 +17,7 @@
 #include "i915_gem_ioctls.h"
 #include "i915_gem_object.h"
 #include "i915_gem_mman.h"
+#include "i915_mm.h"
 #include "i915_trace.h"
 #include "i915_user_extensions.h"
 #include "i915_gem_ttm.h"
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 85bb8d3107f0..3949cafa59e2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1783,14 +1783,6 @@ mkwrite_device_info(struct drm_i915_private *dev_priv)
 int i915_reg_read_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file);
 
-/* i915_mm.c */
-int remap_io_mapping(struct vm_area_struct *vma,
-		     unsigned long addr, unsigned long pfn, unsigned long size,
-		     struct io_mapping *iomap);
-int remap_io_sg(struct vm_area_struct *vma,
-		unsigned long addr, unsigned long size,
-		struct scatterlist *sgl, resource_size_t iobase);
-
 static inline int intel_hws_csb_write_index(struct drm_i915_private *i915)
 {
 	if (GRAPHICS_VER(i915) >= 11)
diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
index 666808cb3a32..7998bc74ab49 100644
--- a/drivers/gpu/drm/i915/i915_mm.c
+++ b/drivers/gpu/drm/i915/i915_mm.c
@@ -27,6 +27,7 @@
 
 
 #include "i915_drv.h"
+#include "i915_mm.h"
 
 struct remap_pfn {
 	struct mm_struct *mm;
@@ -37,17 +38,6 @@ struct remap_pfn {
 	resource_size_t iobase;
 };
 
-static int remap_pfn(pte_t *pte, unsigned long addr, void *data)
-{
-	struct remap_pfn *r = data;
-
-	/* Special PTE are not associated with any struct page */
-	set_pte_at(r->mm, addr, pte, pte_mkspecial(pfn_pte(r->pfn, r->prot)));
-	r->pfn++;
-
-	return 0;
-}
-
 #define use_dma(io) ((io) != -1)
 
 static inline unsigned long sgt_pfn(const struct remap_pfn *r)
@@ -77,6 +67,20 @@ static int remap_sg(pte_t *pte, unsigned long addr, void *data)
 	return 0;
 }
 
+#define EXPECTED_FLAGS (VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP)
+
+#if IS_ENABLED(CONFIG_X86)
+static int remap_pfn(pte_t *pte, unsigned long addr, void *data)
+{
+	struct remap_pfn *r = data;
+
+	/* Special PTE are not associated with any struct page */
+	set_pte_at(r->mm, addr, pte, pte_mkspecial(pfn_pte(r->pfn, r->prot)));
+	r->pfn++;
+
+	return 0;
+}
+
 /**
  * remap_io_mapping - remap an IO mapping to userspace
  * @vma: user vma to map to
@@ -94,7 +98,6 @@ int remap_io_mapping(struct vm_area_struct *vma,
 	struct remap_pfn r;
 	int err;
 
-#define EXPECTED_FLAGS (VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP)
 	GEM_BUG_ON((vma->vm_flags & EXPECTED_FLAGS) != EXPECTED_FLAGS);
 
 	/* We rely on prevalidation of the io-mapping to skip track_pfn(). */
@@ -111,6 +114,7 @@ int remap_io_mapping(struct vm_area_struct *vma,
 
 	return 0;
 }
+#endif
 
 /**
  * remap_io_sg - remap an IO mapping to userspace
diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h
new file mode 100644
index 000000000000..b2b38a5678e3
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_mm.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2021 Intel Corporation
+ */
+
+#ifndef __I915_MM_H__
+#define __I915_MM_H__
+
+#include <linux/types.h>
+
+struct vm_area_struct;
+struct io_mapping;
+struct scatterlist;
+
+#if IS_ENABLED(CONFIG_X86)
+int remap_io_mapping(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn, unsigned long size,
+		struct io_mapping *iomap);
+#else
+static inline int remap_io_mapping(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn, unsigned long size,
+		struct io_mapping *iomap)
+{
+	pr_err("Architecture has no remap_io_mapping() and shouldn't be calling this function\n");
+	WARN_ON_ONCE(1);
+	return 0;
+}
+#endif
+
+int remap_io_sg(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long size,
+		struct scatterlist *sgl, resource_size_t iobase);
+
+#endif /* __I915_MM_H__ */
-- 
2.33.0

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

[Mullati Siva]

From: Siva Mullati <siva.mullati@intel.com>

Only hw that supports mappable aperture would hit this path
vm_fault_gtt/vm_fault_tmm, So we never hit this function
remap_io_mapping() in discrete, So skip this code for non-x86
architectures.

v2: use IS_ENABLED () instead of #if defined

v3: move function prototypes from i915_drv.h to i915_mm.h

v4: added kernel error message in stub function

Signed-off-by: Siva Mullati <siva.mullati@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c |  1 +
 drivers/gpu/drm/i915/i915_drv.h          |  8 ------
 drivers/gpu/drm/i915/i915_mm.c           |  1 +
 drivers/gpu/drm/i915/i915_mm.h           | 35 ++++++++++++++++++++++++
 4 files changed, 37 insertions(+), 8 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_mm.h

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 65fc6ff5f59d..39bb15eafc07 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -17,6 +17,7 @@
 #include "i915_gem_ioctls.h"
 #include "i915_gem_object.h"
 #include "i915_gem_mman.h"
+#include "i915_mm.h"
 #include "i915_trace.h"
 #include "i915_user_extensions.h"
 #include "i915_gem_ttm.h"
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1bfadd9127fc..7ae0f0cc6866 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1967,14 +1967,6 @@ mkwrite_device_info(struct drm_i915_private *dev_priv)
 int i915_reg_read_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file);
 
-/* i915_mm.c */
-int remap_io_mapping(struct vm_area_struct *vma,
-		     unsigned long addr, unsigned long pfn, unsigned long size,
-		     struct io_mapping *iomap);
-int remap_io_sg(struct vm_area_struct *vma,
-		unsigned long addr, unsigned long size,
-		struct scatterlist *sgl, resource_size_t iobase);
-
 static inline int intel_hws_csb_write_index(struct drm_i915_private *i915)
 {
 	if (GRAPHICS_VER(i915) >= 11)
diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
index 666808cb3a32..f4df15fe7cf8 100644
--- a/drivers/gpu/drm/i915/i915_mm.c
+++ b/drivers/gpu/drm/i915/i915_mm.c
@@ -27,6 +27,7 @@
 
 
 #include "i915_drv.h"
+#include "i915_mm.h"
 
 struct remap_pfn {
 	struct mm_struct *mm;
diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h
new file mode 100644
index 000000000000..dc5333356c8f
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_mm.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2021 Intel Corporation
+ */
+
+#ifndef __I915_MM_H__
+#define __I915_MM_H__
+
+#include <linux/types.h>
+
+struct vm_area_struct;
+struct io_mapping;
+struct scatterlist;
+
+#if IS_ENABLED(CONFIG_X86)
+int remap_io_mapping(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn, unsigned long size,
+		struct io_mapping *iomap);
+#else
+static inline int remap_io_mapping(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long pfn, unsigned long size,
+		struct io_mapping *iomap)
+{
+	pr_err(DRIVER_NAME
+		": Architecture has no remap_io_mapping() and shouldn't be calling this function\n");
+	WARN_ON_ONCE(1);
+	return 0;
+}
+#endif
+
+int remap_io_sg(struct vm_area_struct *vma,
+		unsigned long addr, unsigned long size,
+		struct scatterlist *sgl, resource_size_t iobase);
+
+#endif /* __I915_MM_H__ */
-- 
2.33.0

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

[Mullati Siva]

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.

Signed-off-by: Mullati Siva <siva.mullati@intel.com>
---
 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)
 	struct remap_pfn r;
 	int err;
 
@@ -108,6 +109,7 @@ int remap_io_mapping(struct vm_area_struct *vma,
 		zap_vma_ptes(vma, addr, (r.pfn - pfn) << PAGE_SHIFT);
 		return err;
 	}
+#endif
 
 	return 0;
 }
-- 
2.33.0

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

[Jani Nikula]

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

>  	struct remap_pfn r;
>  	int err;
>  
> @@ -108,6 +109,7 @@ int remap_io_mapping(struct vm_area_struct *vma,
>  		zap_vma_ptes(vma, addr, (r.pfn - pfn) << PAGE_SHIFT);
>  		return err;
>  	}
> +#endif
>  
>  	return 0;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center

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

[Lucas De Marchi]

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

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

[Matthew Auld]

On Sat, 13 Nov 2021 at 17:33, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>
> 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?

We don't probe the gmadr on discrete, and so i915_ggtt_has_aperture()
should always return false, which should make hitting this path
impossible. Also for discrete the fault handler stuff all goes though
vm_fault_ttm only, which doesn't use any of this stuff.

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

Apparently some IGTs were able to reproduce it in case that helps:
https://gitlab.freedesktop.org/drm/intel/-/issues/3468

There was also an issue with allocation vs mapping cache type
mismatch, but I guess the BUG_ON(!pte_none(*pte)) is something else.

>
> thanks
> Lucas De Marchi

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

[Mullati Siva]

From: "Mullati, Siva" <siva.mullati@intel.com>

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.

Signed-off-by: Mullati, Siva <siva.mullati@intel.com>
---
 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..5e2a1868b957 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 defined(CONFIG_X86)
 	struct remap_pfn r;
 	int err;
 
@@ -108,6 +109,7 @@ int remap_io_mapping(struct vm_area_struct *vma,
 		zap_vma_ptes(vma, addr, (r.pfn - pfn) << PAGE_SHIFT);
 		return err;
 	}
+#endif
 
 	return 0;
 }
-- 
2.33.0

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

[Jani Nikula]

On Fri, 12 Nov 2021, Mullati Siva <siva.mullati@intel.com> wrote:
> From: "Mullati, Siva" <siva.mullati@intel.com>
>
> 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.
>
> Signed-off-by: Mullati, Siva <siva.mullati@intel.com>
> ---
>  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..5e2a1868b957 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 defined(CONFIG_X86)

Please don't add conditional compilation within functions. Please use
#if IS_ENABLED() instead of #if defined or #ifdef.

BR,
Jani.

>  	struct remap_pfn r;
>  	int err;
>  
> @@ -108,6 +109,7 @@ int remap_io_mapping(struct vm_area_struct *vma,
>  		zap_vma_ptes(vma, addr, (r.pfn - pfn) << PAGE_SHIFT);
>  		return err;
>  	}
> +#endif
>  
>  	return 0;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center