drm/i915: Explicitly mark Global GTT address spaces

PATCH In review Tvrtko Ursulin 3 participants 6 messages Aug 31, 2018 – Sep 1, 2018

Subsystems (2)

DRM drivers Intel i915

lore

Share & tools

lore

Tvrtko Ursulin

author maintainer

2018-08-31 14:36 UTC #0 lore raw

PATCH 1/1 drm/i915: Explicitly mark Global GTT address spaces

From: Tvrtko Ursulin <[email protected]>

So far we have been relying on vm->file pointer being NULL to declare something GGTT.

This has the unfortunate consequence that the default kernel context is also declared GGTT and interferes with the following patch which wants to instantiate VMA's and execute requests against the kernel context.

Change the is_ggtt test to use an explicit flag in struct address_space to solve this issue.

Note that the bit used is free since there is an alignment hole in the struct.

v2:
* Mark mock ggtt.

Signed-off-by Tvrtko Ursulin <[email protected]>

Cc Chris Wilson <[email protected]>

---
 drivers/gpu/drm/i915/i915_gem_gtt.c       | 2 ++
 drivers/gpu/drm/i915/i915_gem_gtt.h       | 5 ++++-
 drivers/gpu/drm/i915/selftests/mock_gtt.c | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)

drivers/gpu/drm/i915/i915_gem_gtt.c +2 -0

index d9d44639ba26..eb0e446d6482 100644
@@ -3604,6 +3604,8 @@ int i915_ggtt_init_hw(struct drm_i915_private *dev_priv)
 	mutex_lock(&dev_priv->drm.struct_mutex);
 	i915_address_space_init(&ggtt->vm, dev_priv);
 
+	ggtt->vm.is_ggtt = true;
+
 	/* Only VLV supports read-only GGTT mappings */
 	ggtt->vm.has_read_only = IS_VALLEYVIEW(dev_priv);

drivers/gpu/drm/i915/i915_gem_gtt.h +4 -1

index 01d83a943142..7e2af5f4f39b 100644
@@ -324,6 +324,9 @@ struct i915_address_space {
 
 	struct pagestash free_pages;
 
+	/* Global GTT */
+	bool is_ggtt:1;
+
 	/* Some systems require uncached updates of the page directories */
 	bool pt_kmap_wc:1;

@@ -357,7 +360,7 @@ struct i915_address_space {
 	I915_SELFTEST_DECLARE(bool scrub_64K);
 };
 
-#define i915_is_ggtt(V) (!(V)->file)
+#define i915_is_ggtt(vm) ((vm)->is_ggtt)
 
 static inline bool
 i915_vm_is_48bit(const struct i915_address_space *vm)

drivers/gpu/drm/i915/selftests/mock_gtt.c +2 -0

index a140ea5c3a7c..6ae418c76015 100644
@@ -118,6 +118,8 @@ void mock_init_ggtt(struct drm_i915_private *i915)
 	ggtt->vm.vma_ops.clear_pages = clear_pages;
 
 	i915_address_space_init(&ggtt->vm, i915);
+
+	ggtt->vm.is_ggtt = true;
 }
 
 void mock_fini_ggtt(struct drm_i915_private *i915)

Signature · 6 lines

2.17.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patchwork

2018-08-31 15:21 UTC #1 lore raw

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Explicitly mark Global GTT address spaces

== Series Details ==

Series: drm/i915: Explicitly mark Global GTT address spaces
URL : https://patchwork.freedesktop.org/series/49018/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
a97af16a4fee drm/i915: Explicitly mark Global GTT address spaces
-:47: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield.  Prefer bool bitfields as unsigned int or u<8|16|32>
#47: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:328:
+	bool is_ggtt:1;

total: 0 errors, 1 warnings, 0 checks, 33 lines checked

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patchwork

2018-08-31 15:42 UTC #2 lore raw

✓ Fi.CI.BAT: success for drm/i915: Explicitly mark Global GTT address spaces

In reply to Tvrtko Ursulin

== Series Details ==

Series: drm/i915: Explicitly mark Global GTT address spaces
URL : https://patchwork.freedesktop.org/series/49018/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4749 -> Patchwork_10061 =

== Summary - SUCCESS ==

No regressions found.

External URL: https://patchwork.freedesktop.org/api/1.0/series/49018/revisions/1/mbox/

== Known issues ==

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

=== IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-kbl-guc:         PASS -> DMESG-FAIL (fdo#106947, fdo#107710)

    igt@gem_exec_suspend@basic-s3:
      {fi-kbl-soraka}:    NOTRUN -> INCOMPLETE (fdo#107556, fdo#107774)

    igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362, fdo#103191)

    {igt@pm_rpm@module-reload}:
      fi-cnl-psr:         PASS -> WARN (fdo#107708, fdo#107602)

    
    ==== Possible fixes ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         FAIL (fdo#107336) -> PASS

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

== Participating hosts (52 -> 47) ==

Additional (1): fi-kbl-soraka
Missing (6): fi-hsw-4770r fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600

== Build changes ==

* Linux: CI_DRM_4749 -> Patchwork_10061

CI_DRM_4749: 4a46c18fad0de38a78b4b0c848892de494324a17 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4616: 5800e46c6f851c370c944a7cb169e99657239f8d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10061: a97af16a4feebcee5d0076479ff281eba089ffc2 @ git://anongit.freedesktop.org/gfx-ci/linux

== Linux commits ==

a97af16a4fee drm/i915: Explicitly mark Global GTT address spaces

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10061/issues.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Chris Wilson

Reviewed-by on v1 1/1

2018-08-31 15:47 UTC #3 lore raw

In reply to Tvrtko Ursulin

Quoting Tvrtko Ursulin (2018-08-31 15:36:43)

Quoted 54 lines

From: Tvrtko Ursulin <[email protected]>
So far we have been relying on vm->file pointer being NULL to declare something GGTT.
This has the unfortunate consequence that the default kernel context is also declared GGTT and interferes with the following patch which wants to instantiate VMA's and execute requests against the kernel context.
Change the is_ggtt test to use an explicit flag in struct address_space to solve this issue.
Note that the bit used is free since there is an alignment hole in the struct.
v2:
* Mark mock ggtt.
Signed-off-by Tvrtko Ursulin <[email protected]>
Cc Chris Wilson <[email protected]>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c       | 2 ++
 drivers/gpu/drm/i915/i915_gem_gtt.h       | 5 ++++-
 drivers/gpu/drm/i915/selftests/mock_gtt.c | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_gem_gtt.c +2 -0
index d9d44639ba26..eb0e446d6482 100644
@@ -3604,6 +3604,8 @@ int i915_ggtt_init_hw(struct drm_i915_private *dev_priv)
        mutex_lock(&dev_priv->drm.struct_mutex);
        i915_address_space_init(&ggtt->vm, dev_priv);
 
+       ggtt->vm.is_ggtt = true;
+
        /* Only VLV supports read-only GGTT mappings */
        ggtt->vm.has_read_only = IS_VALLEYVIEW(dev_priv);
 
drivers/gpu/drm/i915/i915_gem_gtt.h +3 -0
index 01d83a943142..7e2af5f4f39b 100644
@@ -324,6 +324,9 @@ struct i915_address_space {
 
        struct pagestash free_pages;
 
+       /* Global GTT */
+       bool is_ggtt:1;
+
        /* Some systems require uncached updates of the page directories */
        bool pt_kmap_wc:1;
 

drivers/gpu/drm/i915/i915_gem_gtt.h struct i915_address_space { · line 363

Quoted 5 lines

drivers/gpu/drm/i915/i915_gem_gtt.h +1 -1

@@ -357,7 +360,7 @@ struct i915_address_space {
        I915_SELFTEST_DECLARE(bool scrub_64K);
 };
 
-#define i915_is_ggtt(V) (!(V)->file)
+#define i915_is_ggtt(vm) ((vm)->is_ggtt)

But it's so explicit!

Reviewed-by Chris Wilson <[email protected]>

-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Chris Wilson

2018-09-01 09:27 UTC #4 lore raw

Quoting Chris Wilson (2018-08-31 16:47:24)

Quoted 58 lines

Quoting Tvrtko Ursulin (2018-08-31 15:36:43)
Quoted 57 lines
From: Tvrtko Ursulin <[email protected]>
So far we have been relying on vm->file pointer being NULL to declare something GGTT.
This has the unfortunate consequence that the default kernel context is also declared GGTT and interferes with the following patch which wants to instantiate VMA's and execute requests against the kernel context.
Change the is_ggtt test to use an explicit flag in struct address_space to solve this issue.
Note that the bit used is free since there is an alignment hole in the struct.
v2:
* Mark mock ggtt.
Signed-off-by Tvrtko Ursulin <[email protected]>
Cc Chris Wilson <[email protected]>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c       | 2 ++
 drivers/gpu/drm/i915/i915_gem_gtt.h       | 5 ++++-
 drivers/gpu/drm/i915/selftests/mock_gtt.c | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_gem_gtt.c +2 -0
index d9d44639ba26..eb0e446d6482 100644
@@ -3604,6 +3604,8 @@ int i915_ggtt_init_hw(struct drm_i915_private *dev_priv)
        mutex_lock(&dev_priv->drm.struct_mutex);
        i915_address_space_init(&ggtt->vm, dev_priv);
 
+       ggtt->vm.is_ggtt = true;
+
        /* Only VLV supports read-only GGTT mappings */
        ggtt->vm.has_read_only = IS_VALLEYVIEW(dev_priv);
 
drivers/gpu/drm/i915/i915_gem_gtt.h +3 -0
index 01d83a943142..7e2af5f4f39b 100644
@@ -324,6 +324,9 @@ struct i915_address_space {
 
        struct pagestash free_pages;
 
+       /* Global GTT */
+       bool is_ggtt:1;
+
        /* Some systems require uncached updates of the page directories */
        bool pt_kmap_wc:1;
 
@@ -357,7 +360,7 @@ struct i915_address_space {
        I915_SELFTEST_DECLARE(bool scrub_64K);
 };
 

Quoted 5 lines

drivers/gpu/drm/i915/i915_gem_gtt.h struct i915_address_space { · line 363
Quoted 2 lines
drivers/gpu/drm/i915/i915_gem_gtt.h +1 -1
-#define i915_is_ggtt(V) (!(V)->file)
+#define i915_is_ggtt(vm) ((vm)->is_ggtt)
But it's so explicit!
Reviewed-by Chris Wilson <[email protected]>

And pushed, thanks for the patch.
-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patchwork

2018-08-31 22:58 UTC #5 lore raw

✓ Fi.CI.IGT: success for drm/i915: Explicitly mark Global GTT address spaces

In reply to Tvrtko Ursulin

== Series Details ==

Series: drm/i915: Explicitly mark Global GTT address spaces
URL : https://patchwork.freedesktop.org/series/49018/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4749_full -> Patchwork_10061_full =

== Summary - SUCCESS ==

No regressions found.

== Known issues ==

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

=== IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665, fdo#106886)

    igt@gem_exec_suspend@basic-s3:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    
    ==== Possible fixes ====

    igt@gem_eio@in-flight-contexts-10ms:
      shard-glk:          FAIL -> PASS

    igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
      shard-glk:          FAIL (fdo#104873) -> PASS

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          FAIL (fdo#105363, fdo#102887) -> PASS

fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886

== Participating hosts (6 -> 5) ==

Missing (1): pig-snb-2600

== Build changes ==

* Linux: CI_DRM_4749 -> Patchwork_10061

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10061/shards.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx