[Intel-gfx] [RFC] drm/i915: Simplify page allocation loop a bit

2014-03-27 Thread Tvrtko Ursulin 
From: Tvrtko Ursulin tvrtko.ursu...@intel.com

Looks like there are some redundant lines in the main loop of
i915_gem_object_get_pages_gtt.

v2: Put CONFIG_SWIOTLB back in.

I haven't tested this so just RFC please. Not sure that
seven lines net less is worth this. It just made it clerer
to a newby to sg building to think of it in terms of
merge entries always except when

Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
Cc: Imre Deak imre.d...@intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_gem.c | 15 ---
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 00c8361..b435db1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1912,15 +1912,11 @@ i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
gfp = ~(__GFP_IO | __GFP_WAIT);
}
+   if (!i || page_to_pfn(page) != last_pfn + 1
 #ifdef CONFIG_SWIOTLB
-   if (swiotlb_nr_tbl()) {
-   st-nents++;
-   sg_set_page(sg, page, PAGE_SIZE, 0);
-   sg = sg_next(sg);
-   continue;
-   }
+  || swiotlb_nr_tbl()
 #endif
-   if (!i || page_to_pfn(page) != last_pfn + 1) {
+ ) {
if (i)
sg = sg_next(sg);
st-nents++;
@@ -1933,10 +1929,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object 
*obj)
/* Check that the i965g/gm workaround works. */
WARN_ON((gfp  __GFP_DMA32)  (last_pfn = 0x0010UL));
}
-#ifdef CONFIG_SWIOTLB
-   if (!swiotlb_nr_tbl())
-#endif
-   sg_mark_end(sg);
+   sg_mark_end(sg);
obj-pages = st;
 
if (i915_gem_object_needs_bit17_swizzle(obj))
-- 
1.9.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC] drm/i915: Simplify page allocation loop a bit

2014-03-27 Thread Tvrtko Ursulin 


On 03/27/2014 10:18 AM, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin tvrtko.ursu...@intel.com

Looks like there are some redundant lines in the main loop of
i915_gem_object_get_pages_gtt.

v2: Put CONFIG_SWIOTLB back in.

I haven't tested this so just RFC please. Not sure that
seven lines net less is worth this. It just made it clerer
to a newby to sg building to think of it in terms of
merge entries always except when


For what it is worth, this builds and doesn't blow up on some smoke tests.

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

[Intel-gfx] [RFC] drm/i915: Simplify page allocation loop a bit

2014-03-26 Thread Tvrtko Ursulin 
From: Tvrtko Ursulin tvrtko.ursu...@intel.com

Looks like there are some redundant lines in the main loop of
i915_gem_object_get_pages_gtt.

I haven't tested this so just RFC please.

Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
Cc: Imre Deak imre.d...@intel.com
---
 drivers/gpu/drm/i915/i915_gem.c | 16 +++-
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 00c8361..5af3537 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1912,15 +1912,8 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object 
*obj)
gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
gfp = ~(__GFP_IO | __GFP_WAIT);
}
-#ifdef CONFIG_SWIOTLB
-   if (swiotlb_nr_tbl()) {
-   st-nents++;
-   sg_set_page(sg, page, PAGE_SIZE, 0);
-   sg = sg_next(sg);
-   continue;
-   }
-#endif
-   if (!i || page_to_pfn(page) != last_pfn + 1) {
+   if (!i || page_to_pfn(page) != last_pfn + 1
+  || swiotlb_nr_tbl()) {
if (i)
sg = sg_next(sg);
st-nents++;
@@ -1933,10 +1926,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object 
*obj)
/* Check that the i965g/gm workaround works. */
WARN_ON((gfp  __GFP_DMA32)  (last_pfn = 0x0010UL));
}
-#ifdef CONFIG_SWIOTLB
-   if (!swiotlb_nr_tbl())
-#endif
-   sg_mark_end(sg);
+   sg_mark_end(sg);
obj-pages = st;
 
if (i915_gem_object_needs_bit17_swizzle(obj))
-- 
1.9.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC] drm/i915: Simplify page allocation loop a bit

2014-03-26 Thread Imre Deak 
On Wed, 2014-03-26 at 16:48 +, Tvrtko Ursulin wrote:
 From: Tvrtko Ursulin tvrtko.ursu...@intel.com
 
 Looks like there are some redundant lines in the main loop of
 i915_gem_object_get_pages_gtt.
 
 I haven't tested this so just RFC please.
 
 Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
 Cc: Imre Deak imre.d...@intel.com

Looks good to me. Fwiw:
Reviewed-by: Imre Deak imre.d...@intel.com

 ---
  drivers/gpu/drm/i915/i915_gem.c | 16 +++-
  1 file changed, 3 insertions(+), 13 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index 00c8361..5af3537 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -1912,15 +1912,8 @@ i915_gem_object_get_pages_gtt(struct 
 drm_i915_gem_object *obj)
   gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
   gfp = ~(__GFP_IO | __GFP_WAIT);
   }
 -#ifdef CONFIG_SWIOTLB
 - if (swiotlb_nr_tbl()) {
 - st-nents++;
 - sg_set_page(sg, page, PAGE_SIZE, 0);
 - sg = sg_next(sg);
 - continue;
 - }
 -#endif
 - if (!i || page_to_pfn(page) != last_pfn + 1) {
 + if (!i || page_to_pfn(page) != last_pfn + 1
 +|| swiotlb_nr_tbl()) {
   if (i)
   sg = sg_next(sg);
   st-nents++;
 @@ -1933,10 +1926,7 @@ i915_gem_object_get_pages_gtt(struct 
 drm_i915_gem_object *obj)
   /* Check that the i965g/gm workaround works. */
   WARN_ON((gfp  __GFP_DMA32)  (last_pfn = 0x0010UL));
   }
 -#ifdef CONFIG_SWIOTLB
 - if (!swiotlb_nr_tbl())
 -#endif
 - sg_mark_end(sg);
 + sg_mark_end(sg);
   obj-pages = st;
  
   if (i915_gem_object_needs_bit17_swizzle(obj))



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC] drm/i915: Simplify page allocation loop a bit

2014-03-26 Thread Chris Wilson 
On Wed, Mar 26, 2014 at 04:48:47PM +, Tvrtko Ursulin wrote:
 From: Tvrtko Ursulin tvrtko.ursu...@intel.com
 
 Looks like there are some redundant lines in the main loop of
 i915_gem_object_get_pages_gtt.
 
 I haven't tested this so just RFC please.
 
 Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
 Cc: Imre Deak imre.d...@intel.com

I am pretty sure we can't drop CONFIG_SWIOTLB as swiotlb_nr_tbl only
exists when lib/swiotlb.c is built i.e. #if IS_ENABLED(CONFIG_SWIOTLB).
-Chris

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

Re: [Intel-gfx] [RFC] drm/i915: Simplify page allocation loop a bit

2014-03-26 Thread Tvrtko Ursulin 


On 03/26/2014 05:11 PM, Chris Wilson wrote:

On Wed, Mar 26, 2014 at 04:48:47PM +, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin tvrtko.ursu...@intel.com

Looks like there are some redundant lines in the main loop of
i915_gem_object_get_pages_gtt.

I haven't tested this so just RFC please.

Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
Cc: Imre Deak imre.d...@intel.com


I am pretty sure we can't drop CONFIG_SWIOTLB as swiotlb_nr_tbl only
exists when lib/swiotlb.c is built i.e. #if IS_ENABLED(CONFIG_SWIOTLB).


I think you're right. I'll repost. Although it diminishes the 
attractiveness of the whole line reducing enterprise a bit.


Thanks,

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