Re: [Intel-gfx] [PATCH v3 02/16] drm/i915: cleanup i915_add_request

[Ben Widawsky]

On Thu, Apr 04, 2013 at 06:32:34PM +0300, Mika Kuoppala wrote:
 Only execbuffer needs all the parameters. Cleanup everything
 else behind macro.
 
 v2: _i915_add_request as function name (Chris Wilson)
 

Wouldn't the convention be __i915_add_request?

 Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com

In terms of cleanups however, I would like to put the return parameter
(seqno) last. A later patch in the series makes that even more desirable
IMO.

I wouldn't bother with this patch, personally.

Anyway, because I can't find anything functionally incorrect, an
unenthusiastic:
Reviewed-by: Ben Widawsky b...@bwidawsk.net

If you fix up the argument list ordering, a somewhat enthusiastic:
Reviewed-by: Ben Widawsky b...@bwidawsk.net

[snip]
-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v3 02/16] drm/i915: cleanup i915_add_request

[Mika Kuoppala]

Only execbuffer needs all the parameters. Cleanup everything
else behind macro.

v2: _i915_add_request as function name (Chris Wilson)

Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h|8 +---
 drivers/gpu/drm/i915/i915_gem.c|   11 +--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +-
 drivers/gpu/drm/i915/intel_overlay.c   |4 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.c|2 +-
 5 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 630982b..e5e2083 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1644,9 +1644,11 @@ void i915_gem_init_ppgtt(struct drm_device *dev);
 void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
 int __must_check i915_gpu_idle(struct drm_device *dev);
 int __must_check i915_gem_idle(struct drm_device *dev);
-int i915_add_request(struct intel_ring_buffer *ring,
-struct drm_file *file,
-u32 *seqno);
+int _i915_add_request(struct intel_ring_buffer *ring,
+ u32 *seqno,
+ struct drm_file *file);
+#define i915_add_request(ring, seqno) \
+   _i915_add_request(ring, seqno, NULL)
 int __must_check i915_wait_seqno(struct intel_ring_buffer *ring,
 uint32_t seqno);
 int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index db804cc..d3ce0a7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -959,7 +959,7 @@ i915_gem_check_olr(struct intel_ring_buffer *ring, u32 
seqno)
 
ret = 0;
if (seqno == ring-outstanding_lazy_request)
-   ret = i915_add_request(ring, NULL, NULL);
+   ret = i915_add_request(ring, NULL);
 
return ret;
 }
@@ -1999,10 +1999,9 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
return 0;
 }
 
-int
-i915_add_request(struct intel_ring_buffer *ring,
-struct drm_file *file,
-u32 *out_seqno)
+int _i915_add_request(struct intel_ring_buffer *ring,
+ u32 *out_seqno,
+ struct drm_file *file)
 {
drm_i915_private_t *dev_priv = ring-dev-dev_private;
struct drm_i915_gem_request *request;
@@ -2267,7 +2266,7 @@ i915_gem_retire_work_handler(struct work_struct *work)
idle = true;
for_each_ring(ring, dev_priv, i) {
if (ring-gpu_caches_dirty)
-   i915_add_request(ring, NULL, NULL);
+   i915_add_request(ring, NULL);
 
idle = list_empty(ring-request_list);
}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index e0686ca..b413962 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -802,7 +802,7 @@ i915_gem_execbuffer_retire_commands(struct drm_device *dev,
ring-gpu_caches_dirty = true;
 
/* Add a breadcrumb for the completion of the batch buffer */
-   (void)i915_add_request(ring, file, NULL);
+   (void)_i915_add_request(ring, NULL, file);
 }
 
 static int
diff --git a/drivers/gpu/drm/i915/intel_overlay.c 
b/drivers/gpu/drm/i915/intel_overlay.c
index 67a2501..40e509c 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -217,7 +217,7 @@ static int intel_overlay_do_wait_request(struct 
intel_overlay *overlay,
int ret;
 
BUG_ON(overlay-last_flip_req);
-   ret = i915_add_request(ring, NULL, overlay-last_flip_req);
+   ret = i915_add_request(ring, overlay-last_flip_req);
if (ret)
return ret;
 
@@ -286,7 +286,7 @@ static int intel_overlay_continue(struct intel_overlay 
*overlay,
intel_ring_emit(ring, flip_addr);
intel_ring_advance(ring);
 
-   return i915_add_request(ring, NULL, overlay-last_flip_req);
+   return i915_add_request(ring, overlay-last_flip_req);
 }
 
 static void intel_overlay_release_old_vid_tail(struct intel_overlay *overlay)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 1d5d613..9584bc1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1423,7 +1423,7 @@ int intel_ring_idle(struct intel_ring_buffer *ring)
 
/* We need to add any requests required to flush the objects and ring */
if (ring-outstanding_lazy_request) {
-   ret = i915_add_request(ring, NULL, NULL);
+   ret = i915_add_request(ring, NULL);
if (ret)
return ret;
}
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org