[PATCH 1/4] drm/i915: Remove i915_gem_obj_size

[PATCH 1/4] drm/i915: Remove i915_gem_obj_size

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Only caller is i915_gem_obj_ggtt_size which only cares about
GGTT so simplify it and implement under that name.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 12 ++----------
 drivers/gpu/drm/i915/i915_gem.c | 13 ++++---------
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6f1e0f127c0a..b059304a850b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3215,8 +3215,6 @@ bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o,
 bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
 			struct i915_address_space *vm);
 
-unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o,
-				struct i915_address_space *vm);
 struct i915_vma *
 i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
 		    struct i915_address_space *vm);
@@ -3251,14 +3249,8 @@ static inline bool i915_gem_obj_ggtt_bound(struct drm_i915_gem_object *obj)
 	return i915_gem_obj_ggtt_bound_view(obj, &i915_ggtt_view_normal);
 }
 
-static inline unsigned long
-i915_gem_obj_ggtt_size(struct drm_i915_gem_object *obj)
-{
-	struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
-
-	return i915_gem_obj_size(obj, &ggtt->base);
-}
+unsigned long
+i915_gem_obj_ggtt_size(struct drm_i915_gem_object *obj);
 
 static inline int __must_check
 i915_gem_obj_ggtt_pin(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 261a3ef72828..f7e9558178d4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5306,23 +5306,18 @@ bool i915_gem_obj_bound_any(struct drm_i915_gem_object *o)
 	return false;
 }
 
-unsigned long i915_gem_obj_size(struct drm_i915_gem_object *o,
-				struct i915_address_space *vm)
+unsigned long i915_gem_obj_ggtt_size(struct drm_i915_gem_object *o)
 {
-	struct drm_i915_private *dev_priv = o->base.dev->dev_private;
 	struct i915_vma *vma;
 
-	WARN_ON(vm == &dev_priv->mm.aliasing_ppgtt->base);
-
-	BUG_ON(list_empty(&o->vma_list));
+	GEM_BUG_ON(list_empty(&o->vma_list));
 
 	list_for_each_entry(vma, &o->vma_list, obj_link) {
 		if (vma->is_ggtt &&
-		    vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL)
-			continue;
-		if (vma->vm == vm)
+		    vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL)
 			return vma->node.size;
 	}
+
 	return 0;
 }
 
-- 
1.9.1

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

[PATCH 2/4] drm/i915: Simplify i915_gem_obj_to_ggtt_view

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Can use vma->is_ggtt to simplify the check and also switch the
BUG_ON to GEM_BUG_ON which is more appropriate for this.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f7e9558178d4..4abaa364da32 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4648,16 +4648,12 @@ struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
 struct i915_vma *i915_gem_obj_to_ggtt_view(struct drm_i915_gem_object *obj,
 					   const struct i915_ggtt_view *view)
 {
-	struct drm_device *dev = obj->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	struct i915_vma *vma;
 
-	BUG_ON(!view);
+	GEM_BUG_ON(!view);
 
 	list_for_each_entry(vma, &obj->vma_list, obj_link)
-		if (vma->vm == &ggtt->base &&
-		    i915_ggtt_view_equal(&vma->ggtt_view, view))
+		if (vma->is_ggtt && i915_ggtt_view_equal(&vma->ggtt_view, view))
 			return vma;
 	return NULL;
 }
-- 
1.9.1

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

[PATCH 3/4] drm/i915: Simplify i915_gem_obj_ggtt_offset_view

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Can use the new vma->is_ggtt to simplify the check and
remove the local variables.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4abaa364da32..135e1e9c40c2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5246,13 +5246,10 @@ u64 i915_gem_obj_offset(struct drm_i915_gem_object *o,
 u64 i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
 				  const struct i915_ggtt_view *view)
 {
-	struct drm_i915_private *dev_priv = to_i915(o->base.dev);
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	struct i915_vma *vma;
 
 	list_for_each_entry(vma, &o->vma_list, obj_link)
-		if (vma->vm == &ggtt->base &&
-		    i915_ggtt_view_equal(&vma->ggtt_view, view))
+		if (vma->is_ggtt && i915_ggtt_view_equal(&vma->ggtt_view, view))
 			return vma->node.start;
 
 	WARN(1, "global vma for this object not found. (view=%u)\n", view->type);
-- 
1.9.1

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

[PATCH 4/4] drm/i915: Simplify i915_gem_obj_ggtt_bound_view

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Can use the new vma->is_gtt to simplify the check and
get rid of the local variables.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 135e1e9c40c2..c5ca53d2e35d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5275,12 +5275,10 @@ bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
 bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o,
 				  const struct i915_ggtt_view *view)
 {
-	struct drm_i915_private *dev_priv = to_i915(o->base.dev);
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	struct i915_vma *vma;
 
 	list_for_each_entry(vma, &o->vma_list, obj_link)
-		if (vma->vm == &ggtt->base &&
+		if (vma->is_ggtt &&
 		    i915_ggtt_view_equal(&vma->ggtt_view, view) &&
 		    drm_mm_node_allocated(&vma->node))
 			return true;
-- 
1.9.1

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

Re: [PATCH 1/4] drm/i915: Remove i915_gem_obj_size

[Chris Wilson]

On Thu, Apr 21, 2016 at 01:04:43PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Almost all of these vma family of functions need to be deleted and their
object level inconvenience wrappers.

I don't feel this set of patches is heading in the right direction.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: Remove i915_gem_obj_size

[Tvrtko Ursulin]

On 21/04/16 13:19, Chris Wilson wrote:
> On Thu, Apr 21, 2016 at 01:04:43PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Almost all of these vma family of functions need to be deleted and their
> object level inconvenience wrappers.
>
> I don't feel this set of patches is heading in the right direction.

There is no direction really, just accidentally spotted we could remove 
some code (source and binary), mostly which was missed with the 
vm(a)->is_ggtt addition.

Regards,

Tvrtko

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

Re: [PATCH 4/4] drm/i915: Simplify i915_gem_obj_ggtt_bound_view

[Dave Gordon]

On 21/04/16 13:04, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Can use the new vma->is_gtt to simplify the check and
> get rid of the local variables.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 135e1e9c40c2..c5ca53d2e35d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5275,12 +5275,10 @@ bool i915_gem_obj_bound(struct drm_i915_gem_object *o,
>   bool i915_gem_obj_ggtt_bound_view(struct drm_i915_gem_object *o,
>   				  const struct i915_ggtt_view *view)
>   {
> -	struct drm_i915_private *dev_priv = to_i915(o->base.dev);
> -	struct i915_ggtt *ggtt = &dev_priv->ggtt;
>   	struct i915_vma *vma;
>
>   	list_for_each_entry(vma, &o->vma_list, obj_link)
> -		if (vma->vm == &ggtt->base &&
> +		if (vma->is_ggtt &&
>   		    i915_ggtt_view_equal(&vma->ggtt_view, view) &&
>   		    drm_mm_node_allocated(&vma->node))
>   			return true;
>

For patches 2-4,
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>

As these three all now contain
	if (vma->is_ggtt &&
	    i915_ggtt_view_equal(&vma->ggtt_view, view)
	    ...
and these are the ONLY three uses of i915_ggtt_view_equal(), perhaps
the latter could be deleted from the header file and replaced by a local 
(static) inline that encapsulates the is_ggtt test as well?

The Cocci script below found one more candidate for updating:

--- drivers/gpu/drm/i915/i915_gem_gtt.c
+++ cocci-output-24681-2f2e3b-i915_gem_gtt.c
@@ -3255,7 +3255,7 @@ void i915_gem_restore_gtt_mappings(struc
  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
  		flush = false;
  		list_for_each_entry(vma, &obj->vma_list, obj_link) {
-			if (vma->vm != &ggtt->base)
+			if (!vma->is_ggtt)
  				continue;

Cocci:
@is_ggtt@
   idexpression struct i915_ggtt *GGTT;
   idexpression struct i915_vma *VMA;
@@
   {
     <...
-     VMA->vm == &GGTT->base
+     VMA->is_ggtt
     ...>
   }

@not_ggtt@
   idexpression struct i915_ggtt *GGTT;
   idexpression struct i915_vma *VMA;
@@
   {
     <...
-     VMA->vm != &GGTT->base
+     !VMA->is_ggtt
     ...>
   }

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

✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Remove i915_gem_obj_size

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Remove i915_gem_obj_size
URL   : https://patchwork.freedesktop.org/series/6060/
State : warning

== Summary ==

Series 6060v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/6060/revisions/1/mbox/

Test kms_force_connector_basic:
        Subgroup force-connector-state:
                pass       -> SKIP       (snb-x220t)

bdw-nuci7        total:193  pass:181  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:193  pass:170  dwarn:0   dfail:0   fail:0   skip:23 
bsw-nuc-2        total:192  pass:153  dwarn:0   dfail:0   fail:0   skip:39 
byt-nuc          total:192  pass:154  dwarn:0   dfail:0   fail:0   skip:38 
ilk-hp8440p      total:193  pass:136  dwarn:0   dfail:0   fail:0   skip:57 
ivb-t430s        total:193  pass:165  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:193  pass:168  dwarn:0   dfail:0   fail:0   skip:25 
skl-nuci5        total:193  pass:182  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:193  pass:155  dwarn:0   dfail:0   fail:0   skip:38 
snb-x220t        total:193  pass:154  dwarn:0   dfail:0   fail:1   skip:38 

Results at /archive/results/CI_IGT_test/Patchwork_2021/

340c485ad98d0ec0369a3b18d4a09938f3f5537d drm-intel-nightly: 2016y-04m-22d-17h-32m-25s UTC integration manifest
a430046 drm/i915: Simplify i915_gem_obj_ggtt_bound_view
3388a30 drm/i915: Simplify i915_gem_obj_ggtt_offset_view
9d90cdf drm/i915: Simplify i915_gem_obj_to_ggtt_view
e3d8799 drm/i915: Remove i915_gem_obj_size

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

Re: ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Remove i915_gem_obj_size

[Dave Gordon]

On 23/04/16 16:58, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [1/4] drm/i915: Remove i915_gem_obj_size
> URL   : https://patchwork.freedesktop.org/series/6060/
> State : warning
>
> == Summary ==
>
> Series 6060v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/6060/revisions/1/mbox/
>
> Test kms_force_connector_basic:
>          Subgroup force-connector-state:
>                  pass       -> SKIP       (snb-x220t)

That's the phantom VGA connector again!

Everything else looks good :)

> bdw-nuci7        total:193  pass:181  dwarn:0   dfail:0   fail:0   skip:12
> bdw-ultra        total:193  pass:170  dwarn:0   dfail:0   fail:0   skip:23
> bsw-nuc-2        total:192  pass:153  dwarn:0   dfail:0   fail:0   skip:39
> byt-nuc          total:192  pass:154  dwarn:0   dfail:0   fail:0   skip:38
> ilk-hp8440p      total:193  pass:136  dwarn:0   dfail:0   fail:0   skip:57
> ivb-t430s        total:193  pass:165  dwarn:0   dfail:0   fail:0   skip:28
> skl-i7k-2        total:193  pass:168  dwarn:0   dfail:0   fail:0   skip:25
> skl-nuci5        total:193  pass:182  dwarn:0   dfail:0   fail:0   skip:11
> snb-dellxps      total:193  pass:155  dwarn:0   dfail:0   fail:0   skip:38
> snb-x220t        total:193  pass:154  dwarn:0   dfail:0   fail:1   skip:38
>
> Results at /archive/results/CI_IGT_test/Patchwork_2021/
>
> 340c485ad98d0ec0369a3b18d4a09938f3f5537d drm-intel-nightly: 2016y-04m-22d-17h-32m-25s UTC integration manifest
> a430046 drm/i915: Simplify i915_gem_obj_ggtt_bound_view
> 3388a30 drm/i915: Simplify i915_gem_obj_ggtt_offset_view
> 9d90cdf drm/i915: Simplify i915_gem_obj_to_ggtt_view
> e3d8799 drm/i915: Remove i915_gem_obj_size
>

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

Re: ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Remove i915_gem_obj_size

[Tvrtko Ursulin]

On 25/04/16 09:29, Dave Gordon wrote:
> On 23/04/16 16:58, Patchwork wrote:
>> == Series Details ==
>>
>> Series: series starting with [1/4] drm/i915: Remove i915_gem_obj_size
>> URL   : https://patchwork.freedesktop.org/series/6060/
>> State : warning
>>
>> == Summary ==
>>
>> Series 6060v1 Series without cover letter
>> http://patchwork.freedesktop.org/api/1.0/series/6060/revisions/1/mbox/
>>
>> Test kms_force_connector_basic:
>>          Subgroup force-connector-state:
>>                  pass       -> SKIP       (snb-x220t)
>
> That's the phantom VGA connector again!
>
> Everything else looks good :)
>
>> bdw-nuci7        total:193  pass:181  dwarn:0   dfail:0   fail:0
>> skip:12
>> bdw-ultra        total:193  pass:170  dwarn:0   dfail:0   fail:0
>> skip:23
>> bsw-nuc-2        total:192  pass:153  dwarn:0   dfail:0   fail:0
>> skip:39
>> byt-nuc          total:192  pass:154  dwarn:0   dfail:0   fail:0
>> skip:38
>> ilk-hp8440p      total:193  pass:136  dwarn:0   dfail:0   fail:0
>> skip:57
>> ivb-t430s        total:193  pass:165  dwarn:0   dfail:0   fail:0
>> skip:28
>> skl-i7k-2        total:193  pass:168  dwarn:0   dfail:0   fail:0
>> skip:25
>> skl-nuci5        total:193  pass:182  dwarn:0   dfail:0   fail:0
>> skip:11
>> snb-dellxps      total:193  pass:155  dwarn:0   dfail:0   fail:0
>> skip:38
>> snb-x220t        total:193  pass:154  dwarn:0   dfail:0   fail:1
>> skip:38
>>
>> Results at /archive/results/CI_IGT_test/Patchwork_2021/
>>
>> 340c485ad98d0ec0369a3b18d4a09938f3f5537d drm-intel-nightly:
>> 2016y-04m-22d-17h-32m-25s UTC integration manifest
>> a430046 drm/i915: Simplify i915_gem_obj_ggtt_bound_view
>> 3388a30 drm/i915: Simplify i915_gem_obj_ggtt_offset_view
>> 9d90cdf drm/i915: Simplify i915_gem_obj_to_ggtt_view
>> e3d8799 drm/i915: Remove i915_gem_obj_size

Merged, thanks for the reviews!

Regards,

Tvrtko

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