All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions
@ 2016-06-10 17:29 Dave Gordon
  2016-06-10 17:29 ` [PATCH 2/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for intra-module functions Dave Gordon
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Dave Gordon @ 2016-06-10 17:29 UTC (permalink / raw)
  To: intel-gfx

Convert all static functions in i915_guc_submission.c that currently
take a 'dev' pointer to take 'dev_priv' instead (there are three,
guc_client_alloc(), guc_client_free(), and gem_allocate_guc_obj().

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 39 +++++++++++++++---------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 2db1182..1bd0fac 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -591,7 +591,7 @@ int i915_guc_submit(struct drm_i915_gem_request *rq)
 
 /**
  * gem_allocate_guc_obj() - Allocate gem object for GuC usage
- * @dev:	drm device
+ * @dev_priv:	driver private data structure
  * @size:	size of object
  *
  * This is a wrapper to create a gem obj. In order to use it inside GuC, the
@@ -600,13 +600,12 @@ int i915_guc_submit(struct drm_i915_gem_request *rq)
  *
  * Return:	A drm_i915_gem_object if successful, otherwise NULL.
  */
-static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev,
-							u32 size)
+static struct drm_i915_gem_object *
+gem_allocate_guc_obj(struct drm_i915_private *dev_priv, u32 size)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj;
 
-	obj = i915_gem_object_create(dev, size);
+	obj = i915_gem_object_create(dev_priv->dev, size);
 	if (IS_ERR(obj))
 		return NULL;
 
@@ -642,10 +641,10 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 	drm_gem_object_unreference(&obj->base);
 }
 
-static void guc_client_free(struct drm_device *dev,
-			    struct i915_guc_client *client)
+static void
+guc_client_free(struct drm_i915_private *dev_priv,
+		struct i915_guc_client *client)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
 
 	if (!client)
@@ -688,7 +687,7 @@ static void guc_client_free(struct drm_device *dev,
 
 /**
  * guc_client_alloc() - Allocate an i915_guc_client
- * @dev:	drm device
+ * @dev_priv:	driver private data structure
  * @priority:	four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
  * 		The kernel client to replace ExecList submission is created with
  * 		NORMAL priority. Priority of a client for scheduler can be HIGH,
@@ -698,12 +697,12 @@ static void guc_client_free(struct drm_device *dev,
  *
  * Return:	An i915_guc_client object if success, else NULL.
  */
-static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
-						uint32_t priority,
-						struct i915_gem_context *ctx)
+static struct i915_guc_client *
+guc_client_alloc(struct drm_i915_private *dev_priv,
+		 uint32_t priority,
+		 struct i915_gem_context *ctx)
 {
 	struct i915_guc_client *client;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
 	struct drm_i915_gem_object *obj;
 
@@ -724,7 +723,7 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
 	}
 
 	/* The first page is doorbell/proc_desc. Two followed pages are wq. */
-	obj = gem_allocate_guc_obj(dev, GUC_DB_SIZE + GUC_WQ_SIZE);
+	obj = gem_allocate_guc_obj(dev_priv, GUC_DB_SIZE + GUC_WQ_SIZE);
 	if (!obj)
 		goto err;
 
@@ -768,7 +767,7 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
 err:
 	DRM_ERROR("FAILED to create priority %u GuC client!\n", priority);
 
-	guc_client_free(dev, client);
+	guc_client_free(dev_priv, client);
 	return NULL;
 }
 
@@ -793,7 +792,7 @@ static void guc_create_log(struct intel_guc *guc)
 
 	obj = guc->log_obj;
 	if (!obj) {
-		obj = gem_allocate_guc_obj(dev_priv->dev, size);
+		obj = gem_allocate_guc_obj(dev_priv, size);
 		if (!obj) {
 			/* logging will be off */
 			i915.guc_log_level = -1;
@@ -853,7 +852,7 @@ static void guc_create_ads(struct intel_guc *guc)
 
 	obj = guc->ads_obj;
 	if (!obj) {
-		obj = gem_allocate_guc_obj(dev_priv->dev, PAGE_ALIGN(size));
+		obj = gem_allocate_guc_obj(dev_priv, PAGE_ALIGN(size));
 		if (!obj)
 			return;
 
@@ -925,7 +924,7 @@ int i915_guc_submission_init(struct drm_device *dev)
 	if (guc->ctx_pool_obj)
 		return 0; /* already allocated */
 
-	guc->ctx_pool_obj = gem_allocate_guc_obj(dev_priv->dev, gemsize);
+	guc->ctx_pool_obj = gem_allocate_guc_obj(dev_priv, gemsize);
 	if (!guc->ctx_pool_obj)
 		return -ENOMEM;
 
@@ -943,7 +942,7 @@ int i915_guc_submission_enable(struct drm_device *dev)
 	struct i915_guc_client *client;
 
 	/* client for execbuf submission */
-	client = guc_client_alloc(dev,
+	client = guc_client_alloc(dev_priv,
 				  GUC_CTX_PRIORITY_KMD_NORMAL,
 				  dev_priv->kernel_context);
 	if (!client) {
@@ -963,7 +962,7 @@ void i915_guc_submission_disable(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
 
-	guc_client_free(dev, guc->execbuf_client);
+	guc_client_free(dev_priv, guc->execbuf_client);
 	guc->execbuf_client = NULL;
 }
 
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for intra-module functions
  2016-06-10 17:29 [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions Dave Gordon
@ 2016-06-10 17:29 ` Dave Gordon
  2016-06-13  9:15   ` Tvrtko Ursulin
  2016-06-10 19:57 ` [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions Chris Wilson
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Dave Gordon @ 2016-06-10 17:29 UTC (permalink / raw)
  To: intel-gfx

There are four non-static functions in i915_guc_submission.c that take a
'dev' parameter. All are called only from GuC loader code, and can be
easily converted to accept 'dev_priv' instead.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 14 +++++---------
 drivers/gpu/drm/i915/intel_guc.h           |  8 ++++----
 drivers/gpu/drm/i915/intel_guc_loader.c    | 12 ++++++------
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 1bd0fac..65e67f0 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -906,9 +906,8 @@ static void guc_create_ads(struct intel_guc *guc)
  * Set up the memory resources to be shared with the GuC.  At this point,
  * we require just one object that can be mapped through the GGTT.
  */
-int i915_guc_submission_init(struct drm_device *dev)
+int i915_guc_submission_init(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	const size_t ctxsize = sizeof(struct guc_context_desc);
 	const size_t poolsize = GUC_MAX_GPU_CONTEXTS * ctxsize;
 	const size_t gemsize = round_up(poolsize, PAGE_SIZE);
@@ -916,7 +915,7 @@ int i915_guc_submission_init(struct drm_device *dev)
 
 	/* Wipe bitmap & delete client in case of reinitialisation */
 	bitmap_clear(guc->doorbell_bitmap, 0, GUC_MAX_DOORBELLS);
-	i915_guc_submission_disable(dev);
+	i915_guc_submission_disable(dev_priv);
 
 	if (!i915.enable_guc_submission)
 		return 0; /* not enabled  */
@@ -935,9 +934,8 @@ int i915_guc_submission_init(struct drm_device *dev)
 	return 0;
 }
 
-int i915_guc_submission_enable(struct drm_device *dev)
+int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
 	struct i915_guc_client *client;
 
@@ -957,18 +955,16 @@ int i915_guc_submission_enable(struct drm_device *dev)
 	return 0;
 }
 
-void i915_guc_submission_disable(struct drm_device *dev)
+void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
 
 	guc_client_free(dev_priv, guc->execbuf_client);
 	guc->execbuf_client = NULL;
 }
 
-void i915_guc_submission_fini(struct drm_device *dev)
+void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc *guc = &dev_priv->guc;
 
 	gem_release_guc_obj(dev_priv->guc.ads_obj);
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 41601c7..4df80cc 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -156,11 +156,11 @@ extern int intel_guc_suspend(struct drm_device *dev);
 extern int intel_guc_resume(struct drm_device *dev);
 
 /* i915_guc_submission.c */
-int i915_guc_submission_init(struct drm_device *dev);
-int i915_guc_submission_enable(struct drm_device *dev);
+int i915_guc_submission_init(struct drm_i915_private *dev_priv);
+int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
 int i915_guc_wq_check_space(struct drm_i915_gem_request *rq);
 int i915_guc_submit(struct drm_i915_gem_request *rq);
-void i915_guc_submission_disable(struct drm_device *dev);
-void i915_guc_submission_fini(struct drm_device *dev);
+void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
+void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 41f7c7d..d29137f 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -453,7 +453,7 @@ int intel_guc_setup(struct drm_device *dev)
 		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
 		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
 
-	err = i915_guc_submission_init(dev);
+	err = i915_guc_submission_init(dev_priv);
 	if (err)
 		goto fail;
 
@@ -492,7 +492,7 @@ int intel_guc_setup(struct drm_device *dev)
 		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
 
 	if (i915.enable_guc_submission) {
-		err = i915_guc_submission_enable(dev);
+		err = i915_guc_submission_enable(dev_priv);
 		if (err)
 			goto fail;
 		direct_interrupts_to_guc(dev_priv);
@@ -505,8 +505,8 @@ int intel_guc_setup(struct drm_device *dev)
 		guc_fw->guc_fw_load_status = GUC_FIRMWARE_FAIL;
 
 	direct_interrupts_to_host(dev_priv);
-	i915_guc_submission_disable(dev);
-	i915_guc_submission_fini(dev);
+	i915_guc_submission_disable(dev_priv);
+	i915_guc_submission_fini(dev_priv);
 
 	/*
 	 * We've failed to load the firmware :(
@@ -731,8 +731,8 @@ void intel_guc_fini(struct drm_device *dev)
 
 	mutex_lock(&dev->struct_mutex);
 	direct_interrupts_to_host(dev_priv);
-	i915_guc_submission_disable(dev);
-	i915_guc_submission_fini(dev);
+	i915_guc_submission_disable(dev_priv);
+	i915_guc_submission_fini(dev_priv);
 
 	if (guc_fw->guc_fw_obj)
 		drm_gem_object_unreference(&guc_fw->guc_fw_obj->base);
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions
  2016-06-10 17:29 [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions Dave Gordon
  2016-06-10 17:29 ` [PATCH 2/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for intra-module functions Dave Gordon
@ 2016-06-10 19:57 ` Chris Wilson
  2016-06-11  5:50 ` ✗ Ro.CI.BAT: warning for series starting with [1/2] " Patchwork
  2016-06-13  9:13 ` [PATCH 1/2] " Tvrtko Ursulin
  3 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2016-06-10 19:57 UTC (permalink / raw)
  To: Dave Gordon; +Cc: intel-gfx

On Fri, Jun 10, 2016 at 06:29:25PM +0100, Dave Gordon wrote:
> -static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev,
> -							u32 size)
> +static struct drm_i915_gem_object *
> +gem_allocate_guc_obj(struct drm_i915_private *dev_priv, u32 size)

Whilst you are looking at these, this is not GEM allocating an object
for the guc, but guc allocating a GEM object for itself.
-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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✗ Ro.CI.BAT: warning for series starting with [1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions
  2016-06-10 17:29 [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions Dave Gordon
  2016-06-10 17:29 ` [PATCH 2/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for intra-module functions Dave Gordon
  2016-06-10 19:57 ` [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions Chris Wilson
@ 2016-06-11  5:50 ` Patchwork
  2016-06-13 15:42   ` Dave Gordon
  2016-06-13  9:13 ` [PATCH 1/2] " Tvrtko Ursulin
  3 siblings, 1 reply; 9+ messages in thread
From: Patchwork @ 2016-06-11  5:50 UTC (permalink / raw)
  To: Dave Gordon; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions
URL   : https://patchwork.freedesktop.org/series/8556/
State : warning

== Summary ==

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

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                fail       -> PASS       (ro-byt-n2820)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                dmesg-warn -> PASS       (ro-hsw-i3-4010u)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                skip       -> PASS       (fi-skl-i5-6260u)
        Subgroup suspend-read-crc-pipe-a:
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)
        Subgroup suspend-read-crc-pipe-b:
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)
        Subgroup suspend-read-crc-pipe-c:
                skip       -> PASS       (fi-skl-i5-6260u)

fi-bdw-i7-5557u  total:213  pass:201  dwarn:0   dfail:0   fail:0   skip:12 
fi-skl-i5-6260u  total:213  pass:202  dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-i7-6700k  total:213  pass:188  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:213  pass:174  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:213  pass:197  dwarn:4   dfail:0   fail:0   skip:12 
ro-bdw-i7-5600u  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:213  pass:172  dwarn:0   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:213  pass:174  dwarn:0   dfail:0   fail:2   skip:37 
ro-hsw-i3-4010u  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:177  pass:120  dwarn:2   dfail:2   fail:1   skip:51 
ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28 
ro-skl3-i5-6260u total:213  pass:201  dwarn:1   dfail:0   fail:0   skip:11 
ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1   skip:38 
fi-hsw-i7-4770k failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1162/

94fd582 drm-intel-nightly: 2016y-06m-10d-16h-42m-36s UTC integration manifest
4c20b95 drm/i915/guc: prefer 'dev_priv' to 'dev' for intra-module functions
b0b12a6 drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions
  2016-06-10 17:29 [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions Dave Gordon
                   ` (2 preceding siblings ...)
  2016-06-11  5:50 ` ✗ Ro.CI.BAT: warning for series starting with [1/2] " Patchwork
@ 2016-06-13  9:13 ` Tvrtko Ursulin
  2016-06-14 13:11   ` Dave Gordon
  3 siblings, 1 reply; 9+ messages in thread
From: Tvrtko Ursulin @ 2016-06-13  9:13 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx


On 10/06/16 18:29, Dave Gordon wrote:
> Convert all static functions in i915_guc_submission.c that currently
> take a 'dev' pointer to take 'dev_priv' instead (there are three,
> guc_client_alloc(), guc_client_free(), and gem_allocate_guc_obj().
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 39 +++++++++++++++---------------
>   1 file changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 2db1182..1bd0fac 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -591,7 +591,7 @@ int i915_guc_submit(struct drm_i915_gem_request *rq)
>
>   /**
>    * gem_allocate_guc_obj() - Allocate gem object for GuC usage
> - * @dev:	drm device
> + * @dev_priv:	driver private data structure
>    * @size:	size of object
>    *
>    * This is a wrapper to create a gem obj. In order to use it inside GuC, the
> @@ -600,13 +600,12 @@ int i915_guc_submit(struct drm_i915_gem_request *rq)
>    *
>    * Return:	A drm_i915_gem_object if successful, otherwise NULL.
>    */
> -static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev,
> -							u32 size)
> +static struct drm_i915_gem_object *
> +gem_allocate_guc_obj(struct drm_i915_private *dev_priv, u32 size)
>   {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct drm_i915_gem_object *obj;
>
> -	obj = i915_gem_object_create(dev, size);
> +	obj = i915_gem_object_create(dev_priv->dev, size);
>   	if (IS_ERR(obj))
>   		return NULL;
>
> @@ -642,10 +641,10 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
>   	drm_gem_object_unreference(&obj->base);
>   }
>
> -static void guc_client_free(struct drm_device *dev,
> -			    struct i915_guc_client *client)
> +static void
> +guc_client_free(struct drm_i915_private *dev_priv,
> +		struct i915_guc_client *client)
>   {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
>
>   	if (!client)
> @@ -688,7 +687,7 @@ static void guc_client_free(struct drm_device *dev,
>
>   /**
>    * guc_client_alloc() - Allocate an i915_guc_client
> - * @dev:	drm device
> + * @dev_priv:	driver private data structure
>    * @priority:	four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
>    * 		The kernel client to replace ExecList submission is created with
>    * 		NORMAL priority. Priority of a client for scheduler can be HIGH,
> @@ -698,12 +697,12 @@ static void guc_client_free(struct drm_device *dev,
>    *
>    * Return:	An i915_guc_client object if success, else NULL.
>    */
> -static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
> -						uint32_t priority,
> -						struct i915_gem_context *ctx)
> +static struct i915_guc_client *
> +guc_client_alloc(struct drm_i915_private *dev_priv,
> +		 uint32_t priority,
> +		 struct i915_gem_context *ctx)
>   {
>   	struct i915_guc_client *client;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
>   	struct drm_i915_gem_object *obj;
>
> @@ -724,7 +723,7 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
>   	}
>
>   	/* The first page is doorbell/proc_desc. Two followed pages are wq. */
> -	obj = gem_allocate_guc_obj(dev, GUC_DB_SIZE + GUC_WQ_SIZE);
> +	obj = gem_allocate_guc_obj(dev_priv, GUC_DB_SIZE + GUC_WQ_SIZE);
>   	if (!obj)
>   		goto err;
>
> @@ -768,7 +767,7 @@ static struct i915_guc_client *guc_client_alloc(struct drm_device *dev,
>   err:
>   	DRM_ERROR("FAILED to create priority %u GuC client!\n", priority);
>
> -	guc_client_free(dev, client);
> +	guc_client_free(dev_priv, client);
>   	return NULL;
>   }
>
> @@ -793,7 +792,7 @@ static void guc_create_log(struct intel_guc *guc)
>
>   	obj = guc->log_obj;
>   	if (!obj) {
> -		obj = gem_allocate_guc_obj(dev_priv->dev, size);
> +		obj = gem_allocate_guc_obj(dev_priv, size);
>   		if (!obj) {
>   			/* logging will be off */
>   			i915.guc_log_level = -1;
> @@ -853,7 +852,7 @@ static void guc_create_ads(struct intel_guc *guc)
>
>   	obj = guc->ads_obj;
>   	if (!obj) {
> -		obj = gem_allocate_guc_obj(dev_priv->dev, PAGE_ALIGN(size));
> +		obj = gem_allocate_guc_obj(dev_priv, PAGE_ALIGN(size));
>   		if (!obj)
>   			return;
>
> @@ -925,7 +924,7 @@ int i915_guc_submission_init(struct drm_device *dev)
>   	if (guc->ctx_pool_obj)
>   		return 0; /* already allocated */
>
> -	guc->ctx_pool_obj = gem_allocate_guc_obj(dev_priv->dev, gemsize);
> +	guc->ctx_pool_obj = gem_allocate_guc_obj(dev_priv, gemsize);
>   	if (!guc->ctx_pool_obj)
>   		return -ENOMEM;
>
> @@ -943,7 +942,7 @@ int i915_guc_submission_enable(struct drm_device *dev)
>   	struct i915_guc_client *client;
>
>   	/* client for execbuf submission */
> -	client = guc_client_alloc(dev,
> +	client = guc_client_alloc(dev_priv,
>   				  GUC_CTX_PRIORITY_KMD_NORMAL,
>   				  dev_priv->kernel_context);
>   	if (!client) {
> @@ -963,7 +962,7 @@ void i915_guc_submission_disable(struct drm_device *dev)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
>
> -	guc_client_free(dev, guc->execbuf_client);
> +	guc_client_free(dev_priv, guc->execbuf_client);
>   	guc->execbuf_client = NULL;
>   }
>
>

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

And you can keep the r-b if you rename gem_allocate_guc_obj as Chris has 
suggested.

Regards,

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for intra-module functions
  2016-06-10 17:29 ` [PATCH 2/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for intra-module functions Dave Gordon
@ 2016-06-13  9:15   ` Tvrtko Ursulin
  0 siblings, 0 replies; 9+ messages in thread
From: Tvrtko Ursulin @ 2016-06-13  9:15 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx


On 10/06/16 18:29, Dave Gordon wrote:
> There are four non-static functions in i915_guc_submission.c that take a
> 'dev' parameter. All are called only from GuC loader code, and can be
> easily converted to accept 'dev_priv' instead.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 14 +++++---------
>   drivers/gpu/drm/i915/intel_guc.h           |  8 ++++----
>   drivers/gpu/drm/i915/intel_guc_loader.c    | 12 ++++++------
>   3 files changed, 15 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 1bd0fac..65e67f0 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -906,9 +906,8 @@ static void guc_create_ads(struct intel_guc *guc)
>    * Set up the memory resources to be shared with the GuC.  At this point,
>    * we require just one object that can be mapped through the GGTT.
>    */
> -int i915_guc_submission_init(struct drm_device *dev)
> +int i915_guc_submission_init(struct drm_i915_private *dev_priv)
>   {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>   	const size_t ctxsize = sizeof(struct guc_context_desc);
>   	const size_t poolsize = GUC_MAX_GPU_CONTEXTS * ctxsize;
>   	const size_t gemsize = round_up(poolsize, PAGE_SIZE);
> @@ -916,7 +915,7 @@ int i915_guc_submission_init(struct drm_device *dev)
>
>   	/* Wipe bitmap & delete client in case of reinitialisation */
>   	bitmap_clear(guc->doorbell_bitmap, 0, GUC_MAX_DOORBELLS);
> -	i915_guc_submission_disable(dev);
> +	i915_guc_submission_disable(dev_priv);
>
>   	if (!i915.enable_guc_submission)
>   		return 0; /* not enabled  */
> @@ -935,9 +934,8 @@ int i915_guc_submission_init(struct drm_device *dev)
>   	return 0;
>   }
>
> -int i915_guc_submission_enable(struct drm_device *dev)
> +int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
>   {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
>   	struct i915_guc_client *client;
>
> @@ -957,18 +955,16 @@ int i915_guc_submission_enable(struct drm_device *dev)
>   	return 0;
>   }
>
> -void i915_guc_submission_disable(struct drm_device *dev)
> +void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
>   {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
>
>   	guc_client_free(dev_priv, guc->execbuf_client);
>   	guc->execbuf_client = NULL;
>   }
>
> -void i915_guc_submission_fini(struct drm_device *dev)
> +void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
>   {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc *guc = &dev_priv->guc;
>
>   	gem_release_guc_obj(dev_priv->guc.ads_obj);
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 41601c7..4df80cc 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -156,11 +156,11 @@ extern int intel_guc_suspend(struct drm_device *dev);
>   extern int intel_guc_resume(struct drm_device *dev);
>
>   /* i915_guc_submission.c */
> -int i915_guc_submission_init(struct drm_device *dev);
> -int i915_guc_submission_enable(struct drm_device *dev);
> +int i915_guc_submission_init(struct drm_i915_private *dev_priv);
> +int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
>   int i915_guc_wq_check_space(struct drm_i915_gem_request *rq);
>   int i915_guc_submit(struct drm_i915_gem_request *rq);
> -void i915_guc_submission_disable(struct drm_device *dev);
> -void i915_guc_submission_fini(struct drm_device *dev);
> +void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
> +void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
>
>   #endif
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 41f7c7d..d29137f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -453,7 +453,7 @@ int intel_guc_setup(struct drm_device *dev)
>   		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
>   		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
>
> -	err = i915_guc_submission_init(dev);
> +	err = i915_guc_submission_init(dev_priv);
>   	if (err)
>   		goto fail;
>
> @@ -492,7 +492,7 @@ int intel_guc_setup(struct drm_device *dev)
>   		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
>
>   	if (i915.enable_guc_submission) {
> -		err = i915_guc_submission_enable(dev);
> +		err = i915_guc_submission_enable(dev_priv);
>   		if (err)
>   			goto fail;
>   		direct_interrupts_to_guc(dev_priv);
> @@ -505,8 +505,8 @@ int intel_guc_setup(struct drm_device *dev)
>   		guc_fw->guc_fw_load_status = GUC_FIRMWARE_FAIL;
>
>   	direct_interrupts_to_host(dev_priv);
> -	i915_guc_submission_disable(dev);
> -	i915_guc_submission_fini(dev);
> +	i915_guc_submission_disable(dev_priv);
> +	i915_guc_submission_fini(dev_priv);
>
>   	/*
>   	 * We've failed to load the firmware :(
> @@ -731,8 +731,8 @@ void intel_guc_fini(struct drm_device *dev)
>
>   	mutex_lock(&dev->struct_mutex);
>   	direct_interrupts_to_host(dev_priv);
> -	i915_guc_submission_disable(dev);
> -	i915_guc_submission_fini(dev);
> +	i915_guc_submission_disable(dev_priv);
> +	i915_guc_submission_fini(dev_priv);
>
>   	if (guc_fw->guc_fw_obj)
>   		drm_gem_object_unreference(&guc_fw->guc_fw_obj->base);
>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ✗ Ro.CI.BAT: warning for series starting with [1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions
  2016-06-11  5:50 ` ✗ Ro.CI.BAT: warning for series starting with [1/2] " Patchwork
@ 2016-06-13 15:42   ` Dave Gordon
  2016-06-13 15:51     ` Tvrtko Ursulin
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Gordon @ 2016-06-13 15:42 UTC (permalink / raw)
  To: intel-gfx

On 11/06/16 06:50, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions
> URL   : https://patchwork.freedesktop.org/series/8556/
> State : warning
>
> == Summary ==
>
> Series 8556v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/8556/revisions/1/mbox
>
> Test gem_exec_flush:
>          Subgroup basic-batch-kernel-default-cmd:
>                  fail       -> PASS       (ro-byt-n2820)
> Test kms_pipe_crc_basic:
>          Subgroup hang-read-crc-pipe-a:
>                  dmesg-warn -> PASS       (ro-hsw-i3-4010u)
>          Subgroup nonblocking-crc-pipe-b-frame-sequence:
>                  skip       -> PASS       (fi-skl-i5-6260u)
>          Subgroup suspend-read-crc-pipe-a:
>                  skip       -> DMESG-WARN (ro-bdw-i5-5250u)
>          Subgroup suspend-read-crc-pipe-b:
>                  skip       -> DMESG-WARN (ro-bdw-i5-5250u)

These are both an existing issue, logged as
https://bugs.freedesktop.org/show_bug.cgi?id=96448

>          Subgroup suspend-read-crc-pipe-c:
>                  skip       -> PASS       (fi-skl-i5-6260u)
>
> fi-bdw-i7-5557u  total:213  pass:201  dwarn:0   dfail:0   fail:0   skip:12
> fi-skl-i5-6260u  total:213  pass:202  dwarn:0   dfail:0   fail:0   skip:11
> fi-skl-i7-6700k  total:213  pass:188  dwarn:0   dfail:0   fail:0   skip:25
> fi-snb-i7-2600   total:213  pass:174  dwarn:0   dfail:0   fail:0   skip:39
> ro-bdw-i5-5250u  total:213  pass:197  dwarn:4   dfail:0   fail:0   skip:12
> ro-bdw-i7-5600u  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28
> ro-bsw-n3050     total:213  pass:172  dwarn:0   dfail:0   fail:2   skip:39
> ro-byt-n2820     total:213  pass:174  dwarn:0   dfail:0   fail:2   skip:37
> ro-hsw-i3-4010u  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23
> ro-hsw-i7-4770r  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23
> ro-ilk-i7-620lm  total:177  pass:120  dwarn:2   dfail:2   fail:1   skip:51
> ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1   skip:57
> ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0   skip:32
> ro-ivb2-i7-3770  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28
> ro-skl3-i5-6260u total:213  pass:201  dwarn:1   dfail:0   fail:0   skip:11
> ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1   skip:38
> fi-hsw-i7-4770k failed to connect after reboot
> ro-bdw-i7-5557U failed to connect after reboot
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1162/
>
> 94fd582 drm-intel-nightly: 2016y-06m-10d-16h-42m-36s UTC integration manifest
> 4c20b95 drm/i915/guc: prefer 'dev_priv' to 'dev' for intra-module functions
> b0b12a6 drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions
>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ✗ Ro.CI.BAT: warning for series starting with [1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions
  2016-06-13 15:42   ` Dave Gordon
@ 2016-06-13 15:51     ` Tvrtko Ursulin
  0 siblings, 0 replies; 9+ messages in thread
From: Tvrtko Ursulin @ 2016-06-13 15:51 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx


On 13/06/16 16:42, Dave Gordon wrote:
> On 11/06/16 06:50, Patchwork wrote:
>> == Series Details ==
>>
>> Series: series starting with [1/2] drm/i915/guc: prefer 'dev_priv' to
>> 'dev' for static functions
>> URL   : https://patchwork.freedesktop.org/series/8556/
>> State : warning
>>
>> == Summary ==
>>
>> Series 8556v1 Series without cover letter
>> http://patchwork.freedesktop.org/api/1.0/series/8556/revisions/1/mbox
>>
>> Test gem_exec_flush:
>>          Subgroup basic-batch-kernel-default-cmd:
>>                  fail       -> PASS       (ro-byt-n2820)
>> Test kms_pipe_crc_basic:
>>          Subgroup hang-read-crc-pipe-a:
>>                  dmesg-warn -> PASS       (ro-hsw-i3-4010u)
>>          Subgroup nonblocking-crc-pipe-b-frame-sequence:
>>                  skip       -> PASS       (fi-skl-i5-6260u)
>>          Subgroup suspend-read-crc-pipe-a:
>>                  skip       -> DMESG-WARN (ro-bdw-i5-5250u)
>>          Subgroup suspend-read-crc-pipe-b:
>>                  skip       -> DMESG-WARN (ro-bdw-i5-5250u)
>
> These are both an existing issue, logged as
> https://bugs.freedesktop.org/show_bug.cgi?id=96448
>
>>          Subgroup suspend-read-crc-pipe-c:
>>                  skip       -> PASS       (fi-skl-i5-6260u)
>>
>> fi-bdw-i7-5557u  total:213  pass:201  dwarn:0   dfail:0   fail:0
>> skip:12
>> fi-skl-i5-6260u  total:213  pass:202  dwarn:0   dfail:0   fail:0
>> skip:11
>> fi-skl-i7-6700k  total:213  pass:188  dwarn:0   dfail:0   fail:0
>> skip:25
>> fi-snb-i7-2600   total:213  pass:174  dwarn:0   dfail:0   fail:0
>> skip:39
>> ro-bdw-i5-5250u  total:213  pass:197  dwarn:4   dfail:0   fail:0
>> skip:12
>> ro-bdw-i7-5600u  total:213  pass:185  dwarn:0   dfail:0   fail:0
>> skip:28
>> ro-bsw-n3050     total:213  pass:172  dwarn:0   dfail:0   fail:2
>> skip:39
>> ro-byt-n2820     total:213  pass:174  dwarn:0   dfail:0   fail:2
>> skip:37
>> ro-hsw-i3-4010u  total:213  pass:190  dwarn:0   dfail:0   fail:0
>> skip:23
>> ro-hsw-i7-4770r  total:213  pass:190  dwarn:0   dfail:0   fail:0
>> skip:23
>> ro-ilk-i7-620lm  total:177  pass:120  dwarn:2   dfail:2   fail:1
>> skip:51
>> ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1
>> skip:57
>> ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0
>> skip:32
>> ro-ivb2-i7-3770  total:213  pass:185  dwarn:0   dfail:0   fail:0
>> skip:28
>> ro-skl3-i5-6260u total:213  pass:201  dwarn:1   dfail:0   fail:0
>> skip:11
>> ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1
>> skip:38
>> fi-hsw-i7-4770k failed to connect after reboot
>> ro-bdw-i7-5557U failed to connect after reboot
>>
>> Results at /archive/results/CI_IGT_test/RO_Patchwork_1162/
>>
>> 94fd582 drm-intel-nightly: 2016y-06m-10d-16h-42m-36s UTC integration
>> manifest
>> 4c20b95 drm/i915/guc: prefer 'dev_priv' to 'dev' for intra-module
>> functions
>> b0b12a6 drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions

Merged, thanks for the patches and review! :)

Regards,

Tvrtko


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions
  2016-06-13  9:13 ` [PATCH 1/2] " Tvrtko Ursulin
@ 2016-06-14 13:11   ` Dave Gordon
  0 siblings, 0 replies; 9+ messages in thread
From: Dave Gordon @ 2016-06-14 13:11 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

On 13/06/16 10:13, Tvrtko Ursulin wrote:
>
> On 10/06/16 18:29, Dave Gordon wrote:
>> Convert all static functions in i915_guc_submission.c that currently
>> take a 'dev' pointer to take 'dev_priv' instead (there are three,
>> guc_client_alloc(), guc_client_free(), and gem_allocate_guc_obj().
>>
>> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_guc_submission.c | 39
>> +++++++++++++++---------------
>>   1 file changed, 19 insertions(+), 20 deletions(-)

[snip]

> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> And you can keep the r-b if you rename gem_allocate_guc_obj as Chris has
> suggested.
>
> Regards,
> Tvrtko

I was thinking more of doing a bulk-renaming patch later to make all the 
GuC functions use a more consistent naming scheme, probably along the 
lines of "<prefix><topic>_<object>_<operation>", where <prefix> is empty 
for local functions or intel_/i915_ for externally-visible ones, <topic> 
is probably "guc" for all the functions in the loader and submission 
code, <object> is the class we're manipulating (if it's not the GuC 
itself) and <operation> is what we're doing to it. Fairly standard 
object-based RPN naming, in fact.

Many functions are already named this way, but there's still some legacy 
of Alex's original style which is more topic-verb-object.

So gem_allocate_guc_obj() would become guc_gem_object_alloc(). But I 
probably won't do it until I've finished all the other GuC enhancements ;)

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-06-14 13:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-10 17:29 [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions Dave Gordon
2016-06-10 17:29 ` [PATCH 2/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for intra-module functions Dave Gordon
2016-06-13  9:15   ` Tvrtko Ursulin
2016-06-10 19:57 ` [PATCH 1/2] drm/i915/guc: prefer 'dev_priv' to 'dev' for static functions Chris Wilson
2016-06-11  5:50 ` ✗ Ro.CI.BAT: warning for series starting with [1/2] " Patchwork
2016-06-13 15:42   ` Dave Gordon
2016-06-13 15:51     ` Tvrtko Ursulin
2016-06-13  9:13 ` [PATCH 1/2] " Tvrtko Ursulin
2016-06-14 13:11   ` Dave Gordon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.