[PATCH 1/3] drm/i915: Remove defunct i915->vm_list

[PATCH 1/3] drm/i915: Remove defunct i915->vm_list

[Chris Wilson]

No longer used and can be removed. One less global that currently
demands struct_mutex protection.

References: e9e7dc4144cd ("drm/i915/gtt: Make gen6 page directories evictable")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h           | 1 -
 drivers/gpu/drm/i915/i915_gem_gtt.c       | 5 -----
 drivers/gpu/drm/i915/i915_gem_gtt.h       | 1 -
 drivers/gpu/drm/i915/selftests/mock_gtt.c | 3 ---
 4 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e5a0a65ec2e9..550e86dfbfe8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1744,7 +1744,6 @@ struct drm_i915_private {
 	struct drm_atomic_state *modeset_restore_state;
 	struct drm_modeset_acquire_ctx reset_ctx;
 
-	struct list_head vm_list; /* Global list of all address spaces */
 	struct i915_ggtt ggtt; /* VM representing the global address space */
 
 	struct i915_gem_mm mm;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 3326b4e29b5b..2f10f5580230 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -540,8 +540,6 @@ static void i915_address_space_init(struct i915_address_space *vm,
 	INIT_LIST_HEAD(&vm->active_list);
 	INIT_LIST_HEAD(&vm->inactive_list);
 	INIT_LIST_HEAD(&vm->unbound_list);
-
-	list_add_tail(&vm->global_link, &dev_priv->vm_list);
 }
 
 static void i915_address_space_fini(struct i915_address_space *vm)
@@ -553,7 +551,6 @@ static void i915_address_space_fini(struct i915_address_space *vm)
 	spin_unlock(&vm->free_pages.lock);
 
 	drm_mm_takedown(&vm->mm);
-	list_del(&vm->global_link);
 }
 
 static int __setup_page_dma(struct i915_address_space *vm,
@@ -3572,8 +3569,6 @@ int i915_ggtt_init_hw(struct drm_i915_private *dev_priv)
 
 	stash_init(&dev_priv->mm.wc_stash);
 
-	INIT_LIST_HEAD(&dev_priv->vm_list);
-
 	/* Note that we use page colouring to enforce a guard page at the
 	 * end of the address space. This is required as the CS may prefetch
 	 * beyond the end of the batch buffer, across the page boundary,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f298e72b79ca..feda45dfd481 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -288,7 +288,6 @@ struct i915_address_space {
 	 * assign blame.
 	 */
 	struct drm_i915_file_private *file;
-	struct list_head global_link;
 	u64 total;		/* size addr space maps (ex. 2GB for ggtt) */
 	u64 reserved;		/* size addr space reserved */
 
diff --git a/drivers/gpu/drm/i915/selftests/mock_gtt.c b/drivers/gpu/drm/i915/selftests/mock_gtt.c
index 0da5b8c6d912..07df5c399ec1 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gtt.c
@@ -74,7 +74,6 @@ mock_ppgtt(struct drm_i915_private *i915,
 	INIT_LIST_HEAD(&ppgtt->vm.inactive_list);
 	INIT_LIST_HEAD(&ppgtt->vm.unbound_list);
 
-	INIT_LIST_HEAD(&ppgtt->vm.global_link);
 	drm_mm_init(&ppgtt->vm.mm, 0, ppgtt->vm.total);
 
 	ppgtt->vm.clear_range = nop_clear_range;
@@ -106,8 +105,6 @@ void mock_init_ggtt(struct drm_i915_private *i915)
 {
 	struct i915_ggtt *ggtt = &i915->ggtt;
 
-	INIT_LIST_HEAD(&i915->vm_list);
-
 	ggtt->vm.i915 = i915;
 
 	ggtt->gmadr = (struct resource) DEFINE_RES_MEM(0, 2048 * PAGE_SIZE);
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915/selftests: Use full release for local ppgtt allocation

[Chris Wilson]

We can now use the full release mechanism (i915_ppgtt_put) for our local
ppgtt allocation in igt_ppgtt_alloc.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 4bfb0537f9be..e108fe4e0fd9 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -202,9 +202,8 @@ static int igt_ppgtt_alloc(void *arg)
 
 err_ppgtt_cleanup:
 	mutex_lock(&dev_priv->drm.struct_mutex);
-	ppgtt->vm.cleanup(&ppgtt->vm);
+	i915_ppgtt_put(ppgtt);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
-	kfree(ppgtt);
 	return err;
 }
 
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915/selftests: Replace open-coded i915_address_space_init()

[Chris Wilson]

Use i915_address_space_init() rather than open-code it inside
mock_ppgtt() as we will forget to keep it in sync.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/selftests/mock_gtt.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/mock_gtt.c b/drivers/gpu/drm/i915/selftests/mock_gtt.c
index 07df5c399ec1..a140ea5c3a7c 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gtt.c
@@ -70,11 +70,7 @@ mock_ppgtt(struct drm_i915_private *i915,
 	ppgtt->vm.total = round_down(U64_MAX, PAGE_SIZE);
 	ppgtt->vm.file = ERR_PTR(-ENODEV);
 
-	INIT_LIST_HEAD(&ppgtt->vm.active_list);
-	INIT_LIST_HEAD(&ppgtt->vm.inactive_list);
-	INIT_LIST_HEAD(&ppgtt->vm.unbound_list);
-
-	drm_mm_init(&ppgtt->vm.mm, 0, ppgtt->vm.total);
+	i915_address_space_init(&ppgtt->vm, i915);
 
 	ppgtt->vm.clear_range = nop_clear_range;
 	ppgtt->vm.insert_page = mock_insert_page;
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915: Remove defunct i915->vm_list

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Remove defunct i915->vm_list
URL   : https://patchwork.freedesktop.org/series/45958/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
3546360a221f drm/i915: Remove defunct i915->vm_list
-:9: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#9: 
References: e9e7dc4144cd ("drm/i915/gtt: Make gen6 page directories evictable")

-:9: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit e9e7dc4144cd ("drm/i915/gtt: Make gen6 page directories evictable")'
#9: 
References: e9e7dc4144cd ("drm/i915/gtt: Make gen6 page directories evictable")

total: 1 errors, 1 warnings, 0 checks, 52 lines checked
60253e0ac830 drm/i915/selftests: Use full release for local ppgtt allocation
70778821bf65 drm/i915/selftests: Replace open-coded i915_address_space_init()

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915: Remove defunct i915->vm_list

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Remove defunct i915->vm_list
URL   : https://patchwork.freedesktop.org/series/45958/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Remove defunct i915->vm_list
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3664:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3663:16: warning: expression using sizeof(void)

Commit: drm/i915/selftests: Use full release for local ppgtt allocation
Okay!

Commit: drm/i915/selftests: Replace open-coded i915_address_space_init()
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Remove defunct i915->vm_list

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Remove defunct i915->vm_list
URL   : https://patchwork.freedesktop.org/series/45958/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4429 -> Patchwork_9532 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713


== Participating hosts (46 -> 13) ==

  Missing    (33): fi-skl-6770hq fi-skl-6260u fi-bxt-j4205 fi-blb-e6850 fi-skl-6700hq fi-skl-6600u fi-snb-2600 fi-bxt-dsi fi-bdw-5557u fi-glk-dsi fi-ilk-650 fi-kbl-7500u fi-ctg-p8600 fi-hsw-4770 fi-gdg-551 fi-ivb-3770 fi-elk-e7500 fi-ivb-3520m fi-skl-6700k2 fi-kbl-r fi-kbl-7567u fi-ilk-m540 fi-skl-guc fi-cfl-8700k fi-glk-j4005 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-cfl-guc fi-kbl-guc fi-whl-u fi-kbl-x1275 fi-cfl-8109u 


== Build changes ==

    * Linux: CI_DRM_4429 -> Patchwork_9532

  CI_DRM_4429: 6b02112a5dacd9e464c08c6d54f762bafd24937f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4536: aaa23eff21a148809beb22e928f3cd72530ea3de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9532: 70778821bf65729445b35e5f3e2bafa5993a5875 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

70778821bf65 drm/i915/selftests: Replace open-coded i915_address_space_init()
60253e0ac830 drm/i915/selftests: Use full release for local ppgtt allocation
3546360a221f drm/i915: Remove defunct i915->vm_list

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9532/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Remove defunct i915->vm_list

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Remove defunct i915->vm_list
URL   : https://patchwork.freedesktop.org/series/45958/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4430 -> Patchwork_9533 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_module_reload@basic-reload:
      fi-ilk-650:         PASS -> DMESG-WARN (fdo#106387) +2

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387


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

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4430 -> Patchwork_9533

  CI_DRM_4430: 6b02112a5dacd9e464c08c6d54f762bafd24937f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4537: 5a160e9e1fe19c67e58e9c298303cb94c96aeb7d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9533: 29dd0a07c48b9d2b4e5190cf14b23770e571bc84 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

29dd0a07c48b drm/i915/selftests: Replace open-coded i915_address_space_init()
8593f7732b92 drm/i915/selftests: Use full release for local ppgtt allocation
42d03a05ca74 drm/i915: Remove defunct i915->vm_list

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9533/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Remove defunct i915->vm_list

[Tvrtko Ursulin]

On 05/07/2018 07:56, Chris Wilson wrote:
> No longer used and can be removed. One less global that currently
> demands struct_mutex protection.
> 
> References: e9e7dc4144cd ("drm/i915/gtt: Make gen6 page directories evictable")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_drv.h           | 1 -
>   drivers/gpu/drm/i915/i915_gem_gtt.c       | 5 -----
>   drivers/gpu/drm/i915/i915_gem_gtt.h       | 1 -
>   drivers/gpu/drm/i915/selftests/mock_gtt.c | 3 ---
>   4 files changed, 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e5a0a65ec2e9..550e86dfbfe8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1744,7 +1744,6 @@ struct drm_i915_private {
>   	struct drm_atomic_state *modeset_restore_state;
>   	struct drm_modeset_acquire_ctx reset_ctx;
>   
> -	struct list_head vm_list; /* Global list of all address spaces */
>   	struct i915_ggtt ggtt; /* VM representing the global address space */
>   
>   	struct i915_gem_mm mm;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 3326b4e29b5b..2f10f5580230 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -540,8 +540,6 @@ static void i915_address_space_init(struct i915_address_space *vm,
>   	INIT_LIST_HEAD(&vm->active_list);
>   	INIT_LIST_HEAD(&vm->inactive_list);
>   	INIT_LIST_HEAD(&vm->unbound_list);
> -
> -	list_add_tail(&vm->global_link, &dev_priv->vm_list);
>   }
>   
>   static void i915_address_space_fini(struct i915_address_space *vm)
> @@ -553,7 +551,6 @@ static void i915_address_space_fini(struct i915_address_space *vm)
>   	spin_unlock(&vm->free_pages.lock);
>   
>   	drm_mm_takedown(&vm->mm);
> -	list_del(&vm->global_link);
>   }
>   
>   static int __setup_page_dma(struct i915_address_space *vm,
> @@ -3572,8 +3569,6 @@ int i915_ggtt_init_hw(struct drm_i915_private *dev_priv)
>   
>   	stash_init(&dev_priv->mm.wc_stash);
>   
> -	INIT_LIST_HEAD(&dev_priv->vm_list);
> -
>   	/* Note that we use page colouring to enforce a guard page at the
>   	 * end of the address space. This is required as the CS may prefetch
>   	 * beyond the end of the batch buffer, across the page boundary,
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index f298e72b79ca..feda45dfd481 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -288,7 +288,6 @@ struct i915_address_space {
>   	 * assign blame.
>   	 */
>   	struct drm_i915_file_private *file;
> -	struct list_head global_link;
>   	u64 total;		/* size addr space maps (ex. 2GB for ggtt) */
>   	u64 reserved;		/* size addr space reserved */
>   
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gtt.c b/drivers/gpu/drm/i915/selftests/mock_gtt.c
> index 0da5b8c6d912..07df5c399ec1 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gtt.c
> @@ -74,7 +74,6 @@ mock_ppgtt(struct drm_i915_private *i915,
>   	INIT_LIST_HEAD(&ppgtt->vm.inactive_list);
>   	INIT_LIST_HEAD(&ppgtt->vm.unbound_list);
>   
> -	INIT_LIST_HEAD(&ppgtt->vm.global_link);
>   	drm_mm_init(&ppgtt->vm.mm, 0, ppgtt->vm.total);
>   
>   	ppgtt->vm.clear_range = nop_clear_range;
> @@ -106,8 +105,6 @@ void mock_init_ggtt(struct drm_i915_private *i915)
>   {
>   	struct i915_ggtt *ggtt = &i915->ggtt;
>   
> -	INIT_LIST_HEAD(&i915->vm_list);
> -
>   	ggtt->vm.i915 = i915;
>   
>   	ggtt->gmadr = (struct resource) DEFINE_RES_MEM(0, 2048 * PAGE_SIZE);
> 

No list iterators removed so easy to establish it is indeed unused. (Or 
won't compile.) :)

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/selftests: Use full release for local ppgtt allocation

[Tvrtko Ursulin]

On 05/07/2018 07:56, Chris Wilson wrote:
> We can now use the full release mechanism (i915_ppgtt_put) for our local
> ppgtt allocation in igt_ppgtt_alloc.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index 4bfb0537f9be..e108fe4e0fd9 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -202,9 +202,8 @@ static int igt_ppgtt_alloc(void *arg)
>   
>   err_ppgtt_cleanup:
>   	mutex_lock(&dev_priv->drm.struct_mutex);
> -	ppgtt->vm.cleanup(&ppgtt->vm);
> +	i915_ppgtt_put(ppgtt);
>   	mutex_unlock(&dev_priv->drm.struct_mutex);
> -	kfree(ppgtt);
>   	return err;
>   }
>   
> 

Hm, it's not obvious why this works, if it does. This ppgtt it creates 
does not call kref_init on ppgtt->ref, or i915_address_space_init. 
Colour me confused.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/selftests: Use full release for local ppgtt allocation

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-07-05 10:50:36)
> 
> On 05/07/2018 07:56, Chris Wilson wrote:
> > We can now use the full release mechanism (i915_ppgtt_put) for our local
> > ppgtt allocation in igt_ppgtt_alloc.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > index 4bfb0537f9be..e108fe4e0fd9 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > @@ -202,9 +202,8 @@ static int igt_ppgtt_alloc(void *arg)
> >   
> >   err_ppgtt_cleanup:
> >       mutex_lock(&dev_priv->drm.struct_mutex);
> > -     ppgtt->vm.cleanup(&ppgtt->vm);
> > +     i915_ppgtt_put(ppgtt);
> >       mutex_unlock(&dev_priv->drm.struct_mutex);
> > -     kfree(ppgtt);
> >       return err;
> >   }
> >   
> > 
> 
> Hm, it's not obvious why this works, if it does. This ppgtt it creates 
> does not call kref_init on ppgtt->ref, or i915_address_space_init. 
> Colour me confused.

Are you sure? There were a few ordering issues that required pushing
i915_address_space_init earlier into __hw_ppgtt_create() itself that
dragged the kref_init along for the fun.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/selftests: Use full release for local ppgtt allocation

[Tvrtko Ursulin]

On 05/07/2018 10:50, Tvrtko Ursulin wrote:
> 
> On 05/07/2018 07:56, Chris Wilson wrote:
>> We can now use the full release mechanism (i915_ppgtt_put) for our local
>> ppgtt allocation in igt_ppgtt_alloc.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c 
>> b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
>> index 4bfb0537f9be..e108fe4e0fd9 100644
>> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
>> @@ -202,9 +202,8 @@ static int igt_ppgtt_alloc(void *arg)
>>   err_ppgtt_cleanup:
>>       mutex_lock(&dev_priv->drm.struct_mutex);
>> -    ppgtt->vm.cleanup(&ppgtt->vm);
>> +    i915_ppgtt_put(ppgtt);
>>       mutex_unlock(&dev_priv->drm.struct_mutex);
>> -    kfree(ppgtt);
>>       return err;
>>   }
>>
> 
> Hm, it's not obvious why this works, if it does. This ppgtt it creates 
> does not call kref_init on ppgtt->ref, or i915_address_space_init. 
> Colour me confused.

I was looking at the too old tree, in the meantime you sneaked in v4 of 
the wc stash patch which changes these things.

Slightly unfortunate that common parts of the init are now duplicated..

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/selftests: Replace open-coded i915_address_space_init()

[Tvrtko Ursulin]

On 05/07/2018 07:56, Chris Wilson wrote:
> Use i915_address_space_init() rather than open-code it inside
> mock_ppgtt() as we will forget to keep it in sync.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/selftests/mock_gtt.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gtt.c b/drivers/gpu/drm/i915/selftests/mock_gtt.c
> index 07df5c399ec1..a140ea5c3a7c 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gtt.c
> @@ -70,11 +70,7 @@ mock_ppgtt(struct drm_i915_private *i915,
>   	ppgtt->vm.total = round_down(U64_MAX, PAGE_SIZE);
>   	ppgtt->vm.file = ERR_PTR(-ENODEV);
>   
> -	INIT_LIST_HEAD(&ppgtt->vm.active_list);
> -	INIT_LIST_HEAD(&ppgtt->vm.inactive_list);
> -	INIT_LIST_HEAD(&ppgtt->vm.unbound_list);
> -
> -	drm_mm_init(&ppgtt->vm.mm, 0, ppgtt->vm.total);
> +	i915_address_space_init(&ppgtt->vm, i915);
>   
>   	ppgtt->vm.clear_range = nop_clear_range;
>   	ppgtt->vm.insert_page = mock_insert_page;
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/selftests: Use full release for local ppgtt allocation

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-07-05 10:55:23)
> 
> On 05/07/2018 10:50, Tvrtko Ursulin wrote:
> > 
> > On 05/07/2018 07:56, Chris Wilson wrote:
> >> We can now use the full release mechanism (i915_ppgtt_put) for our local
> >> ppgtt allocation in igt_ppgtt_alloc.
> >>
> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> ---
> >>   drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 +--
> >>   1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c 
> >> b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> >> index 4bfb0537f9be..e108fe4e0fd9 100644
> >> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> >> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> >> @@ -202,9 +202,8 @@ static int igt_ppgtt_alloc(void *arg)
> >>   err_ppgtt_cleanup:
> >>       mutex_lock(&dev_priv->drm.struct_mutex);
> >> -    ppgtt->vm.cleanup(&ppgtt->vm);
> >> +    i915_ppgtt_put(ppgtt);
> >>       mutex_unlock(&dev_priv->drm.struct_mutex);
> >> -    kfree(ppgtt);
> >>       return err;
> >>   }
> >>
> > 
> > Hm, it's not obvious why this works, if it does. This ppgtt it creates 
> > does not call kref_init on ppgtt->ref, or i915_address_space_init. 
> > Colour me confused.
> 
> I was looking at the too old tree, in the meantime you sneaked in v4 of 
> the wc stash patch which changes these things.

I thought it was clean and rushed before checking the selftests.
 
> Slightly unfortunate that common parts of the init are now duplicated..

I think we should just pull the kref into i915_address_space, then all
the common parts are inited together. The only reason it is not there is
to avoid refcounting ggtt, which I'm not that concerned about as we
should only be tweaking ppgtt krefs on the fly.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915: Remove defunct i915->vm_list

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Remove defunct i915->vm_list
URL   : https://patchwork.freedesktop.org/series/45958/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4430_full -> Patchwork_9533_full =

== Summary - WARNING ==

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

  

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          SKIP -> PASS +1

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      shard-kbl:          PASS -> DMESG-FAIL (fdo#106560, fdo#106947)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)
      shard-kbl:          PASS -> FAIL (fdo#99912)

    igt@perf_pmu@other-read-4:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_gtt:
      shard-snb:          DMESG-WARN (fdo#107120) -> PASS
      shard-hsw:          DMESG-WARN (fdo#107120) -> PASS

    igt@drv_selftest@mock_requests:
      shard-kbl:          DMESG-WARN (fdo#107121) -> PASS
      shard-hsw:          DMESG-WARN (fdo#107121) -> PASS
      shard-snb:          DMESG-WARN (fdo#107121) -> PASS
      shard-apl:          DMESG-WARN (fdo#107121) -> PASS

    igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
      shard-hsw:          FAIL (fdo#103355) -> PASS

    
    ==== Warnings ====

    igt@drv_selftest@live_gtt:
      shard-kbl:          DMESG-FAIL (fdo#107120) -> FAIL (fdo#105347)
      shard-apl:          DMESG-FAIL (fdo#107120, fdo#106609) -> INCOMPLETE (fdo#103927)

    
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106609 https://bugs.freedesktop.org/show_bug.cgi?id=106609
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107120 https://bugs.freedesktop.org/show_bug.cgi?id=107120
  fdo#107121 https://bugs.freedesktop.org/show_bug.cgi?id=107121
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4430 -> Patchwork_9533

  CI_DRM_4430: 6b02112a5dacd9e464c08c6d54f762bafd24937f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4537: 5a160e9e1fe19c67e58e9c298303cb94c96aeb7d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9533: 29dd0a07c48b9d2b4e5190cf14b23770e571bc84 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9533/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx