* [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.