All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check
@ 2016-06-07  8:14 Dave Gordon
  2016-06-07  8:14 ` [PATCH 2/3] drm/i915/guc: disable GuC submission earlier during GuC (re)load Dave Gordon
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Dave Gordon @ 2016-06-07  8:14 UTC (permalink / raw)
  To: intel-gfx

The last stage of the GuC loader also sanitises the GuC submission
settings, so should be called unconditionally (even on platforms
without a GuC) to ensure consistent settings; in particular, this
prevents any attempt to use GuC submission on GuCless platforms!

Also fix error path handling and clarify DRM_INFO fallback message.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |  8 +++-----
 drivers/gpu/drm/i915/intel_guc_loader.c | 12 ++++++++----
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1bfc260..eae8d7a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4930,11 +4930,9 @@ int i915_gem_init_engines(struct drm_device *dev)
 	intel_mocs_init_l3cc_table(dev);
 
 	/* We can't enable contexts until all firmware is loaded */
-	if (HAS_GUC(dev)) {
-		ret = intel_guc_setup(dev);
-		if (ret)
-			goto out;
-	}
+	ret = intel_guc_setup(dev);
+	if (ret)
+		goto out;
 
 	/*
 	 * Increment the next seqno by 0x100 so we have a visible break
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index f2b88c7..4e34c2e 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -425,9 +425,13 @@ int intel_guc_setup(struct drm_device *dev)
 	if (!i915.enable_guc_loading) {
 		err = 0;
 		goto fail;
-	} else if (fw_path == NULL || *fw_path == '\0') {
-		if (*fw_path == '\0')
-			DRM_INFO("No GuC firmware known for this platform\n");
+	} else if (fw_path == NULL) {
+		/* Device is known to have no uCode (e.g. no GuC) */
+		err = -ENXIO;
+		goto fail;
+	} else if (*fw_path == '\0') {
+		/* Device has a GuC but we don't know what f/w to load? */
+		DRM_INFO("No GuC firmware known for this platform\n");
 		err = -ENODEV;
 		goto fail;
 	}
@@ -535,7 +539,7 @@ int intel_guc_setup(struct drm_device *dev)
 		if (fw_path == NULL)
 			DRM_INFO("GuC submission without firmware not supported\n");
 		if (ret == 0)
-			DRM_INFO("Falling back to execlist mode\n");
+			DRM_INFO("Falling back from GuC submission to execlist mode\n");
 		else
 			DRM_ERROR("GuC init failed: %d\n", ret);
 	}
-- 
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] 19+ messages in thread

* [PATCH 2/3] drm/i915/guc: disable GuC submission earlier during GuC (re)load
  2016-06-07  8:14 [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check Dave Gordon
@ 2016-06-07  8:14 ` Dave Gordon
  2016-06-07  9:51   ` Tvrtko Ursulin
  2016-06-07  8:14 ` [PATCH 3/3] drm/i915/guc: enable GuC loading & submission by default Dave Gordon
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Dave Gordon @ 2016-06-07  8:14 UTC (permalink / raw)
  To: intel-gfx

When resetting and reloading the GuC, the GuC submission management code
also needs to destroy and recreate the GuC client(s). Currently this is
done by a separate call from the GuC loader, but really, it's just an
internal detail of the submission code. So here we remove the call from
the loader (which is too late, really, because the GuC has already been
reloaded at this point) and put it into guc_submission_init() instead.
This means that any preexisting client is destroyed *before* the GuC
(re)load and then recreated after, iff the firmware was successfully
loaded. If the GuC reload fails, we don't recreate the client, so
fallback to execlists mode (if active) won't leak the client object
(previously, the now-unusable client would have been left allocated,
and leaked if the driver were unloaded).

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

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index ac72451..2db1182 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -657,6 +657,8 @@ static void guc_client_free(struct drm_device *dev,
 	 */
 
 	if (client->client_base) {
+		uint16_t db_id = client->doorbell_id;
+
 		/*
 		 * If we got as far as setting up a doorbell, make sure
 		 * we shut it down before unmapping & deallocating the
@@ -664,10 +666,11 @@ static void guc_client_free(struct drm_device *dev,
 		 * GuC that we've finished with it, finally deallocate
 		 * it in our bitmap
 		 */
-		if (client->doorbell_id != GUC_INVALID_DOORBELL_ID) {
+		if (db_id != GUC_INVALID_DOORBELL_ID) {
 			guc_disable_doorbell(guc, client);
-			host2guc_release_doorbell(guc, client);
-			release_doorbell(guc, client->doorbell_id);
+			if (test_bit(db_id, guc->doorbell_bitmap))
+				host2guc_release_doorbell(guc, client);
+			release_doorbell(guc, db_id);
 		}
 
 		kunmap(kmap_to_page(client->client_base));
@@ -912,6 +915,10 @@ int i915_guc_submission_init(struct drm_device *dev)
 	const size_t gemsize = round_up(poolsize, PAGE_SIZE);
 	struct intel_guc *guc = &dev_priv->guc;
 
+	/* Wipe bitmap & delete client in case of reinitialisation */
+	bitmap_clear(guc->doorbell_bitmap, 0, GUC_MAX_DOORBELLS);
+	i915_guc_submission_disable(dev);
+
 	if (!i915.enable_guc_submission)
 		return 0; /* not enabled  */
 
@@ -923,9 +930,7 @@ int i915_guc_submission_init(struct drm_device *dev)
 		return -ENOMEM;
 
 	ida_init(&guc->ctx_ids);
-
 	guc_create_log(guc);
-
 	guc_create_ads(guc);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 4e34c2e..41f7c7d 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -492,9 +492,6 @@ int intel_guc_setup(struct drm_device *dev)
 		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
 
 	if (i915.enable_guc_submission) {
-		/* The execbuf_client will be recreated. Release it first. */
-		i915_guc_submission_disable(dev);
-
 		err = i915_guc_submission_enable(dev);
 		if (err)
 			goto fail;
-- 
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] 19+ messages in thread

* [PATCH 3/3] drm/i915/guc: enable GuC loading & submission by default
  2016-06-07  8:14 [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check Dave Gordon
  2016-06-07  8:14 ` [PATCH 2/3] drm/i915/guc: disable GuC submission earlier during GuC (re)load Dave Gordon
@ 2016-06-07  8:14 ` Dave Gordon
  2016-06-07  9:53   ` Tvrtko Ursulin
  2016-06-07  8:41 ` [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check Tvrtko Ursulin
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Dave Gordon @ 2016-06-07  8:14 UTC (permalink / raw)
  To: intel-gfx

The recent patch
. fce91f2 drm/i915/guc: add enable_guc_loading parameter
enabled GuC loading and submission by default, but as issues
were found with warnings being issued during suspend-resume
cycles, GuC loading was disabled by default, by patch
. 2335986 drm/i915/guc: Disable automatic GuC firmware loading

Those warnings have been resolved, so this patch re-enables GuC
loading and submission by default.

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

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 5e18cf9..573e787 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -54,8 +54,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = -1,
+	.enable_guc_submission = -1,
 	.guc_log_level = -1,
 	.enable_dp_mst = true,
 	.inject_load_failure = 0,
@@ -202,12 +202,12 @@ struct i915_params i915 __read_mostly = {
 module_param_named_unsafe(enable_guc_loading, i915.enable_guc_loading, int, 0400);
 MODULE_PARM_DESC(enable_guc_loading,
 		"Enable GuC firmware loading "
-		"(-1=auto, 0=never [default], 1=if available, 2=required)");
+		"(-1=auto [default], 0=never, 1=if available, 2=required)");
 
 module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, int, 0400);
 MODULE_PARM_DESC(enable_guc_submission,
 		"Enable GuC submission "
-		"(-1=auto, 0=never [default], 1=if available, 2=required)");
+		"(-1=auto [default], 0=never, 1=if available, 2=required)");
 
 module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
 MODULE_PARM_DESC(guc_log_level,
-- 
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] 19+ messages in thread

* Re: [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check
  2016-06-07  8:14 [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check Dave Gordon
  2016-06-07  8:14 ` [PATCH 2/3] drm/i915/guc: disable GuC submission earlier during GuC (re)load Dave Gordon
  2016-06-07  8:14 ` [PATCH 3/3] drm/i915/guc: enable GuC loading & submission by default Dave Gordon
@ 2016-06-07  8:41 ` Tvrtko Ursulin
  2016-06-09 11:04   ` Tvrtko Ursulin
  2016-06-07  8:43 ` ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915/guc: fix GuC loading/submission check Patchwork
  2016-06-10 16:59 ` ✗ Ro.CI.BAT: failure for series starting with drm/i915/guc: suppress GuC-related message on non-GuC platforms (rev2) Patchwork
  4 siblings, 1 reply; 19+ messages in thread
From: Tvrtko Ursulin @ 2016-06-07  8:41 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx


On 07/06/16 09:14, Dave Gordon wrote:
> The last stage of the GuC loader also sanitises the GuC submission
> settings, so should be called unconditionally (even on platforms
> without a GuC) to ensure consistent settings; in particular, this
> prevents any attempt to use GuC submission on GuCless platforms!
>
> Also fix error path handling and clarify DRM_INFO fallback message.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c         |  8 +++-----
>   drivers/gpu/drm/i915/intel_guc_loader.c | 12 ++++++++----
>   2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1bfc260..eae8d7a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4930,11 +4930,9 @@ int i915_gem_init_engines(struct drm_device *dev)
>   	intel_mocs_init_l3cc_table(dev);
>
>   	/* We can't enable contexts until all firmware is loaded */
> -	if (HAS_GUC(dev)) {
> -		ret = intel_guc_setup(dev);
> -		if (ret)
> -			goto out;
> -	}
> +	ret = intel_guc_setup(dev);
> +	if (ret)
> +		goto out;
>
>   	/*
>   	 * Increment the next seqno by 0x100 so we have a visible break
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index f2b88c7..4e34c2e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -425,9 +425,13 @@ int intel_guc_setup(struct drm_device *dev)
>   	if (!i915.enable_guc_loading) {
>   		err = 0;
>   		goto fail;
> -	} else if (fw_path == NULL || *fw_path == '\0') {
> -		if (*fw_path == '\0')

Ops. I can only assume I meant !fw_path.

> -			DRM_INFO("No GuC firmware known for this platform\n");
> +	} else if (fw_path == NULL) {
> +		/* Device is known to have no uCode (e.g. no GuC) */
> +		err = -ENXIO;
> +		goto fail;
> +	} else if (*fw_path == '\0') {
> +		/* Device has a GuC but we don't know what f/w to load? */
> +		DRM_INFO("No GuC firmware known for this platform\n");
>   		err = -ENODEV;
>   		goto fail;
>   	}
> @@ -535,7 +539,7 @@ int intel_guc_setup(struct drm_device *dev)
>   		if (fw_path == NULL)
>   			DRM_INFO("GuC submission without firmware not supported\n");
>   		if (ret == 0)
> -			DRM_INFO("Falling back to execlist mode\n");
> +			DRM_INFO("Falling back from GuC submission to execlist mode\n");
>   		else
>   			DRM_ERROR("GuC init failed: %d\n", ret);
>   	}
>

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] 19+ messages in thread

* ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915/guc: fix GuC loading/submission check
  2016-06-07  8:14 [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check Dave Gordon
                   ` (2 preceding siblings ...)
  2016-06-07  8:41 ` [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check Tvrtko Ursulin
@ 2016-06-07  8:43 ` Patchwork
  2016-06-07 10:54   ` Dave Gordon
  2016-06-10 16:59 ` ✗ Ro.CI.BAT: failure for series starting with drm/i915/guc: suppress GuC-related message on non-GuC platforms (rev2) Patchwork
  4 siblings, 1 reply; 19+ messages in thread
From: Patchwork @ 2016-06-07  8:43 UTC (permalink / raw)
  To: Dave Gordon; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/guc: fix GuC loading/submission check
URL   : https://patchwork.freedesktop.org/series/8380/
State : failure

== Summary ==

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

Test core_auth:
        Subgroup basic-auth:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test drv_module_reload_basic:
                pass       -> DMESG-WARN (ro-byt-n2820)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                pass       -> FAIL       (ro-byt-n2820)
        Subgroup basic-uc-ro-default:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup basic-wb-ro-before-default:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test gem_exec_store:
        Subgroup basic-bsd:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_mmap_gtt:
        Subgroup basic-short:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup basic-write-gtt:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test gem_pwrite:
        Subgroup basic:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test kms_addfb_basic:
        Subgroup addfb25-bad-modifier:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup addfb25-yf-tiled:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup bad-pitch-0:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup bad-pitch-1024:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-x-tiled:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-y-tiled:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup clobberred-modifier:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup tile-pitch-mismatch:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)

fi-bdw-i7-5557u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8  
fi-hsw-i7-4770k  total:209  pass:190  dwarn:0   dfail:0   fail:0   skip:19 
fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8  
ro-bdw-i7-5600u  total:102  pass:75   dwarn:0   dfail:0   fail:0   skip:26 
ro-bsw-n3050     total:209  pass:168  dwarn:0   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:209  pass:168  dwarn:1   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:102  pass:82   dwarn:0   dfail:0   fail:0   skip:19 
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-skl-i7-6700hq total:204  pass:172  dwarn:11  dfail:0   fail:0   skip:21 
ro-snb-i7-2620M  total:102  pass:72   dwarn:0   dfail:0   fail:0   skip:29 
fi-skl-i5-6260u failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot
ro-ivb2-i7-3770 failed to connect after reboot
ro-ivb-i7-3770 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1125/

55d1291 drm-intel-nightly: 2016y-06m-06d-16h-28m-05s UTC integration manifest
57871c9 drm/i915/guc: enable GuC loading & submission by default
3fdd902 drm/i915/guc: disable GuC submission earlier during GuC (re)load
5492d57 drm/i915/guc: fix GuC loading/submission check

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

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

* Re: [PATCH 2/3] drm/i915/guc: disable GuC submission earlier during GuC (re)load
  2016-06-07  8:14 ` [PATCH 2/3] drm/i915/guc: disable GuC submission earlier during GuC (re)load Dave Gordon
@ 2016-06-07  9:51   ` Tvrtko Ursulin
  2016-06-07 10:13     ` Dave Gordon
  0 siblings, 1 reply; 19+ messages in thread
From: Tvrtko Ursulin @ 2016-06-07  9:51 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx


On 07/06/16 09:14, Dave Gordon wrote:
> When resetting and reloading the GuC, the GuC submission management code
> also needs to destroy and recreate the GuC client(s). Currently this is
> done by a separate call from the GuC loader, but really, it's just an
> internal detail of the submission code. So here we remove the call from
> the loader (which is too late, really, because the GuC has already been
> reloaded at this point) and put it into guc_submission_init() instead.
> This means that any preexisting client is destroyed *before* the GuC
> (re)load and then recreated after, iff the firmware was successfully
> loaded. If the GuC reload fails, we don't recreate the client, so
> fallback to execlists mode (if active) won't leak the client object
> (previously, the now-unusable client would have been left allocated,
> and leaked if the driver were unloaded).
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++++++++++-----
>   drivers/gpu/drm/i915/intel_guc_loader.c    |  3 ---
>   2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index ac72451..2db1182 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -657,6 +657,8 @@ static void guc_client_free(struct drm_device *dev,
>   	 */
>
>   	if (client->client_base) {
> +		uint16_t db_id = client->doorbell_id;
> +
>   		/*
>   		 * If we got as far as setting up a doorbell, make sure
>   		 * we shut it down before unmapping & deallocating the
> @@ -664,10 +666,11 @@ static void guc_client_free(struct drm_device *dev,
>   		 * GuC that we've finished with it, finally deallocate
>   		 * it in our bitmap
>   		 */
> -		if (client->doorbell_id != GUC_INVALID_DOORBELL_ID) {
> +		if (db_id != GUC_INVALID_DOORBELL_ID) {
>   			guc_disable_doorbell(guc, client);
> -			host2guc_release_doorbell(guc, client);
> -			release_doorbell(guc, client->doorbell_id);
> +			if (test_bit(db_id, guc->doorbell_bitmap))
> +				host2guc_release_doorbell(guc, client);
> +			release_doorbell(guc, db_id);
>   		}
>
>   		kunmap(kmap_to_page(client->client_base));
> @@ -912,6 +915,10 @@ int i915_guc_submission_init(struct drm_device *dev)
>   	const size_t gemsize = round_up(poolsize, PAGE_SIZE);
>   	struct intel_guc *guc = &dev_priv->guc;
>
> +	/* Wipe bitmap & delete client in case of reinitialisation */
> +	bitmap_clear(guc->doorbell_bitmap, 0, GUC_MAX_DOORBELLS);
> +	i915_guc_submission_disable(dev);
> +
>   	if (!i915.enable_guc_submission)
>   		return 0; /* not enabled  */
>
> @@ -923,9 +930,7 @@ int i915_guc_submission_init(struct drm_device *dev)
>   		return -ENOMEM;
>
>   	ida_init(&guc->ctx_ids);
> -
>   	guc_create_log(guc);
> -
>   	guc_create_ads(guc);
>
>   	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 4e34c2e..41f7c7d 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -492,9 +492,6 @@ int intel_guc_setup(struct drm_device *dev)
>   		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
>
>   	if (i915.enable_guc_submission) {
> -		/* The execbuf_client will be recreated. Release it first. */
> -		i915_guc_submission_disable(dev);
> -
>   		err = i915_guc_submission_enable(dev);
>   		if (err)
>   			goto fail;
>

This fixes the errors on suspend/resume? It would be useful for the 
commit message to explain what was happening.

It is a bit strange since the first disable now comes before the init, 
but since the disable path already does handle that case I suppose it is OK.

Anyway,

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] 19+ messages in thread

* Re: [PATCH 3/3] drm/i915/guc: enable GuC loading & submission by default
  2016-06-07  8:14 ` [PATCH 3/3] drm/i915/guc: enable GuC loading & submission by default Dave Gordon
@ 2016-06-07  9:53   ` Tvrtko Ursulin
  0 siblings, 0 replies; 19+ messages in thread
From: Tvrtko Ursulin @ 2016-06-07  9:53 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx


On 07/06/16 09:14, Dave Gordon wrote:
> The recent patch
> . fce91f2 drm/i915/guc: add enable_guc_loading parameter
> enabled GuC loading and submission by default, but as issues

Not submission, just the loading.

> were found with warnings being issued during suspend-resume
> cycles, GuC loading was disabled by default, by patch
> . 2335986 drm/i915/guc: Disable automatic GuC firmware loading
>
> Those warnings have been resolved, so this patch re-enables GuC
> loading and submission by default.

Enabling submission by default was okayed by everyone now?

>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_params.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 5e18cf9..573e787 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -54,8 +54,8 @@ struct i915_params i915 __read_mostly = {
>   	.verbose_state_checks = 1,
>   	.nuclear_pageflip = 0,
>   	.edp_vswing = 0,
> -	.enable_guc_loading = 0,
> -	.enable_guc_submission = 0,
> +	.enable_guc_loading = -1,
> +	.enable_guc_submission = -1,
>   	.guc_log_level = -1,
>   	.enable_dp_mst = true,
>   	.inject_load_failure = 0,
> @@ -202,12 +202,12 @@ struct i915_params i915 __read_mostly = {
>   module_param_named_unsafe(enable_guc_loading, i915.enable_guc_loading, int, 0400);
>   MODULE_PARM_DESC(enable_guc_loading,
>   		"Enable GuC firmware loading "
> -		"(-1=auto, 0=never [default], 1=if available, 2=required)");
> +		"(-1=auto [default], 0=never, 1=if available, 2=required)");
>
>   module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, int, 0400);
>   MODULE_PARM_DESC(enable_guc_submission,
>   		"Enable GuC submission "
> -		"(-1=auto, 0=never [default], 1=if available, 2=required)");
> +		"(-1=auto [default], 0=never, 1=if available, 2=required)");
>
>   module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
>   MODULE_PARM_DESC(guc_log_level,
>

Patch itself is fine so:

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] 19+ messages in thread

* Re: [PATCH 2/3] drm/i915/guc: disable GuC submission earlier during GuC (re)load
  2016-06-07  9:51   ` Tvrtko Ursulin
@ 2016-06-07 10:13     ` Dave Gordon
  0 siblings, 0 replies; 19+ messages in thread
From: Dave Gordon @ 2016-06-07 10:13 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

On 07/06/16 10:51, Tvrtko Ursulin wrote:
>
> On 07/06/16 09:14, Dave Gordon wrote:
>> When resetting and reloading the GuC, the GuC submission management code
>> also needs to destroy and recreate the GuC client(s). Currently this is
>> done by a separate call from the GuC loader, but really, it's just an
>> internal detail of the submission code. So here we remove the call from
>> the loader (which is too late, really, because the GuC has already been
>> reloaded at this point) and put it into guc_submission_init() instead.
>> This means that any preexisting client is destroyed *before* the GuC
>> (re)load and then recreated after, iff the firmware was successfully
>> loaded. If the GuC reload fails, we don't recreate the client, so
>> fallback to execlists mode (if active) won't leak the client object
>> (previously, the now-unusable client would have been left allocated,
>> and leaked if the driver were unloaded).
>>
>> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++++++++++-----
>>   drivers/gpu/drm/i915/intel_guc_loader.c    |  3 ---
>>   2 files changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index ac72451..2db1182 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -657,6 +657,8 @@ static void guc_client_free(struct drm_device *dev,
>>        */
>>
>>       if (client->client_base) {
>> +        uint16_t db_id = client->doorbell_id;
>> +
>>           /*
>>            * If we got as far as setting up a doorbell, make sure
>>            * we shut it down before unmapping & deallocating the
>> @@ -664,10 +666,11 @@ static void guc_client_free(struct drm_device *dev,
>>            * GuC that we've finished with it, finally deallocate
>>            * it in our bitmap
>>            */
>> -        if (client->doorbell_id != GUC_INVALID_DOORBELL_ID) {
>> +        if (db_id != GUC_INVALID_DOORBELL_ID) {
>>               guc_disable_doorbell(guc, client);
>> -            host2guc_release_doorbell(guc, client);
>> -            release_doorbell(guc, client->doorbell_id);
>> +            if (test_bit(db_id, guc->doorbell_bitmap))
>> +                host2guc_release_doorbell(guc, client);
>> +            release_doorbell(guc, db_id);
>>           }
>>
>>           kunmap(kmap_to_page(client->client_base));
>> @@ -912,6 +915,10 @@ int i915_guc_submission_init(struct drm_device *dev)
>>       const size_t gemsize = round_up(poolsize, PAGE_SIZE);
>>       struct intel_guc *guc = &dev_priv->guc;
>>
>> +    /* Wipe bitmap & delete client in case of reinitialisation */
>> +    bitmap_clear(guc->doorbell_bitmap, 0, GUC_MAX_DOORBELLS);
>> +    i915_guc_submission_disable(dev);
>> +
>>       if (!i915.enable_guc_submission)
>>           return 0; /* not enabled  */
>>
>> @@ -923,9 +930,7 @@ int i915_guc_submission_init(struct drm_device *dev)
>>           return -ENOMEM;
>>
>>       ida_init(&guc->ctx_ids);
>> -
>>       guc_create_log(guc);
>> -
>>       guc_create_ads(guc);
>>
>>       return 0;
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index 4e34c2e..41f7c7d 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -492,9 +492,6 @@ int intel_guc_setup(struct drm_device *dev)
>>           intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
>>
>>       if (i915.enable_guc_submission) {
>> -        /* The execbuf_client will be recreated. Release it first. */
>> -        i915_guc_submission_disable(dev);
>> -
>>           err = i915_guc_submission_enable(dev);
>>           if (err)
>>               goto fail;
>
> This fixes the errors on suspend/resume? It would be useful for the
> commit message to explain what was happening.

The error message I've seen was actually a timeout from trying to talk 
to the GuC during a TDR reset. So this fixes it by *not* talking to the 
GuC during a reset - even if the GuC were still working, there's not 
much point in updating it just before the reload, which will reset all 
its state anyway.

> It is a bit strange since the first disable now comes before the init,

The first disable is now *inside* i915_guc_submission_init().
But it does come before the first i915_guc_submission_enable().
It's not an error to disable something that's not (yet) enabled :)

.Dave.

> but since the disable path already does handle that case I suppose it is
> OK.
>
> Anyway,
>
> 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] 19+ messages in thread

* Re: ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915/guc: fix GuC loading/submission check
  2016-06-07  8:43 ` ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915/guc: fix GuC loading/submission check Patchwork
@ 2016-06-07 10:54   ` Dave Gordon
  2016-06-07 13:23     ` Tvrtko Ursulin
  0 siblings, 1 reply; 19+ messages in thread
From: Dave Gordon @ 2016-06-07 10:54 UTC (permalink / raw)
  To: intel-gfx, Tvrtko Ursulin

On 07/06/16 09:43, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [1/3] drm/i915/guc: fix GuC loading/submission check
> URL   : https://patchwork.freedesktop.org/series/8380/
> State : failure
>
> == Summary ==
>
> Series 8380v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/8380/revisions/1/mbox
>
> Test core_auth:
>          Subgroup basic-auth:
>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> Test drv_module_reload_basic:
>                  pass       -> DMESG-WARN (ro-byt-n2820)

Bug 93381 - [BAT BYT] dmesg WARNING in snd-hda

> Test gem_exec_flush:
>          Subgroup basic-batch-kernel-default-cmd:
>                  pass       -> FAIL       (ro-byt-n2820)

Bug 95372 - [BAT BYT] Sporadic failure from 
igt/gem_exec_flush@basic-batch-kernel-default-cmd

>          Subgroup basic-uc-ro-default:
>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>          Subgroup basic-wb-ro-before-default:
>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> Test gem_exec_store:
>          Subgroup basic-bsd:
>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
> Test gem_mmap_gtt:
>          Subgroup basic-short:
>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>          Subgroup basic-write-gtt:
>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> Test gem_pwrite:
>          Subgroup basic:
>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> Test kms_addfb_basic:
>          Subgroup addfb25-bad-modifier:
>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>          Subgroup addfb25-yf-tiled:
>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>          Subgroup bad-pitch-0:
>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>          Subgroup bad-pitch-1024:
>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>          Subgroup basic-x-tiled:
>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>          Subgroup basic-y-tiled:
>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>          Subgroup clobberred-modifier:
>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>          Subgroup tile-pitch-mismatch:
>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)

All the warnings on 'ro-skl-i7-6700hq' appear to be
https://bugs.freedesktop.org/show_bug.cgi?id=95632
"[BAT SKL] *ERROR* Potential atomic update failure on pipe A"

So all issues accounted for, patchset ready for merge :)

.Dave.

> fi-bdw-i7-5557u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8
> fi-hsw-i7-4770k  total:209  pass:190  dwarn:0   dfail:0   fail:0   skip:19
> fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0   skip:25
> fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39
> ro-bdw-i5-5250u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8
> ro-bdw-i7-5600u  total:102  pass:75   dwarn:0   dfail:0   fail:0   skip:26
> ro-bsw-n3050     total:209  pass:168  dwarn:0   dfail:0   fail:2   skip:39
> ro-byt-n2820     total:209  pass:168  dwarn:1   dfail:0   fail:3   skip:37
> ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23
> ro-hsw-i7-4770r  total:102  pass:82   dwarn:0   dfail:0   fail:0   skip:19
> ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57
> ro-skl-i7-6700hq total:204  pass:172  dwarn:11  dfail:0   fail:0   skip:21
> ro-snb-i7-2620M  total:102  pass:72   dwarn:0   dfail:0   fail:0   skip:29
> fi-skl-i5-6260u failed to connect after reboot
> ro-bdw-i7-5557U failed to connect after reboot
> ro-ivb2-i7-3770 failed to connect after reboot
> ro-ivb-i7-3770 failed to connect after reboot
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1125/
>
> 55d1291 drm-intel-nightly: 2016y-06m-06d-16h-28m-05s UTC integration manifest
> 57871c9 drm/i915/guc: enable GuC loading & submission by default
> 3fdd902 drm/i915/guc: disable GuC submission earlier during GuC (re)load
> 5492d57 drm/i915/guc: fix GuC loading/submission check


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

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

* Re: ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915/guc: fix GuC loading/submission check
  2016-06-07 10:54   ` Dave Gordon
@ 2016-06-07 13:23     ` Tvrtko Ursulin
  2016-06-07 20:00       ` Chris Wilson
  0 siblings, 1 reply; 19+ messages in thread
From: Tvrtko Ursulin @ 2016-06-07 13:23 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx


On 07/06/16 11:54, Dave Gordon wrote:
> On 07/06/16 09:43, Patchwork wrote:
>> == Series Details ==
>>
>> Series: series starting with [1/3] drm/i915/guc: fix GuC
>> loading/submission check
>> URL   : https://patchwork.freedesktop.org/series/8380/
>> State : failure
>>
>> == Summary ==
>>
>> Series 8380v1 Series without cover letter
>> http://patchwork.freedesktop.org/api/1.0/series/8380/revisions/1/mbox
>>
>> Test core_auth:
>>          Subgroup basic-auth:
>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>> Test drv_module_reload_basic:
>>                  pass       -> DMESG-WARN (ro-byt-n2820)
>
> Bug 93381 - [BAT BYT] dmesg WARNING in snd-hda
>
>> Test gem_exec_flush:
>>          Subgroup basic-batch-kernel-default-cmd:
>>                  pass       -> FAIL       (ro-byt-n2820)
>
> Bug 95372 - [BAT BYT] Sporadic failure from
> igt/gem_exec_flush@basic-batch-kernel-default-cmd
>
>>          Subgroup basic-uc-ro-default:
>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>          Subgroup basic-wb-ro-before-default:
>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>> Test gem_exec_store:
>>          Subgroup basic-bsd:
>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>> Test gem_mmap_gtt:
>>          Subgroup basic-short:
>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>          Subgroup basic-write-gtt:
>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>> Test gem_pwrite:
>>          Subgroup basic:
>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>> Test kms_addfb_basic:
>>          Subgroup addfb25-bad-modifier:
>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>          Subgroup addfb25-yf-tiled:
>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>          Subgroup bad-pitch-0:
>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>          Subgroup bad-pitch-1024:
>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>>          Subgroup basic-x-tiled:
>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>>          Subgroup basic-y-tiled:
>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>          Subgroup clobberred-modifier:
>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>>          Subgroup tile-pitch-mismatch:
>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>
> All the warnings on 'ro-skl-i7-6700hq' appear to be
> https://bugs.freedesktop.org/show_bug.cgi?id=95632
> "[BAT SKL] *ERROR* Potential atomic update failure on pipe A"
>
> So all issues accounted for, patchset ready for merge :)
>
> .Dave.
>
>> fi-bdw-i7-5557u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8
>> fi-hsw-i7-4770k  total:209  pass:190  dwarn:0   dfail:0   fail:0
>> skip:19
>> fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0
>> skip:25
>> fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0
>> skip:39
>> ro-bdw-i5-5250u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8
>> ro-bdw-i7-5600u  total:102  pass:75   dwarn:0   dfail:0   fail:0
>> skip:26
>> ro-bsw-n3050     total:209  pass:168  dwarn:0   dfail:0   fail:2
>> skip:39
>> ro-byt-n2820     total:209  pass:168  dwarn:1   dfail:0   fail:3
>> skip:37
>> ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0
>> skip:23
>> ro-hsw-i7-4770r  total:102  pass:82   dwarn:0   dfail:0   fail:0
>> skip:19
>> ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1
>> skip:57
>> ro-skl-i7-6700hq total:204  pass:172  dwarn:11  dfail:0   fail:0
>> skip:21
>> ro-snb-i7-2620M  total:102  pass:72   dwarn:0   dfail:0   fail:0
>> skip:29
>> fi-skl-i5-6260u failed to connect after reboot
>> ro-bdw-i7-5557U failed to connect after reboot
>> ro-ivb2-i7-3770 failed to connect after reboot
>> ro-ivb-i7-3770 failed to connect after reboot
>>
>> Results at /archive/results/CI_IGT_test/RO_Patchwork_1125/
>>
>> 55d1291 drm-intel-nightly: 2016y-06m-06d-16h-28m-05s UTC integration
>> manifest
>> 57871c9 drm/i915/guc: enable GuC loading & submission by default
>> 3fdd902 drm/i915/guc: disable GuC submission earlier during GuC (re)load
>> 5492d57 drm/i915/guc: fix GuC loading/submission check

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] 19+ messages in thread

* Re: ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915/guc: fix GuC loading/submission check
  2016-06-07 13:23     ` Tvrtko Ursulin
@ 2016-06-07 20:00       ` Chris Wilson
  2016-06-08  8:18         ` Dave Gordon
  0 siblings, 1 reply; 19+ messages in thread
From: Chris Wilson @ 2016-06-07 20:00 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

On Tue, Jun 07, 2016 at 02:23:34PM +0100, Tvrtko Ursulin wrote:
> 
> On 07/06/16 11:54, Dave Gordon wrote:
> >On 07/06/16 09:43, Patchwork wrote:
> >>== Series Details ==
> >>
> >>Series: series starting with [1/3] drm/i915/guc: fix GuC
> >>loading/submission check
> >>URL   : https://patchwork.freedesktop.org/series/8380/
> >>State : failure
> >>
> >>== Summary ==
> >>
> >>Series 8380v1 Series without cover letter
> >>http://patchwork.freedesktop.org/api/1.0/series/8380/revisions/1/mbox
> >>
> >>Test core_auth:
> >>         Subgroup basic-auth:
> >>                 dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> >>Test drv_module_reload_basic:
> >>                 pass       -> DMESG-WARN (ro-byt-n2820)
> >
> >Bug 93381 - [BAT BYT] dmesg WARNING in snd-hda
> >
> >>Test gem_exec_flush:
> >>         Subgroup basic-batch-kernel-default-cmd:
> >>                 pass       -> FAIL       (ro-byt-n2820)
> >
> >Bug 95372 - [BAT BYT] Sporadic failure from
> >igt/gem_exec_flush@basic-batch-kernel-default-cmd
> >
> >>         Subgroup basic-uc-ro-default:
> >>                 pass       -> DMESG-WARN (ro-skl-i7-6700hq)
> >>         Subgroup basic-wb-ro-before-default:
> >>                 dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> >>Test gem_exec_store:
> >>         Subgroup basic-bsd:
> >>                 pass       -> DMESG-WARN (ro-skl-i7-6700hq)
> >>Test gem_mmap_gtt:
> >>         Subgroup basic-short:
> >>                 pass       -> DMESG-WARN (ro-skl-i7-6700hq)
> >>         Subgroup basic-write-gtt:
> >>                 dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> >>Test gem_pwrite:
> >>         Subgroup basic:
> >>                 dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> >>Test kms_addfb_basic:
> >>         Subgroup addfb25-bad-modifier:
> >>                 pass       -> DMESG-WARN (ro-skl-i7-6700hq)
> >>         Subgroup addfb25-yf-tiled:
> >>                 pass       -> DMESG-WARN (ro-skl-i7-6700hq)
> >>         Subgroup bad-pitch-0:
> >>                 pass       -> DMESG-WARN (ro-skl-i7-6700hq)
> >>         Subgroup bad-pitch-1024:
> >>                 dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> >>         Subgroup basic-x-tiled:
> >>                 dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> >>         Subgroup basic-y-tiled:
> >>                 pass       -> DMESG-WARN (ro-skl-i7-6700hq)
> >>         Subgroup clobberred-modifier:
> >>                 dmesg-warn -> PASS       (ro-skl-i7-6700hq)
> >>         Subgroup tile-pitch-mismatch:
> >>                 pass       -> DMESG-WARN (ro-skl-i7-6700hq)
> >
> >All the warnings on 'ro-skl-i7-6700hq' appear to be
> >https://bugs.freedesktop.org/show_bug.cgi?id=95632
> >"[BAT SKL] *ERROR* Potential atomic update failure on pipe A"
> >
> >So all issues accounted for, patchset ready for merge :)
> >
> >.Dave.
> >
> >>fi-bdw-i7-5557u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8
> >>fi-hsw-i7-4770k  total:209  pass:190  dwarn:0   dfail:0   fail:0
> >>skip:19
> >>fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0
> >>skip:25
> >>fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0
> >>skip:39
> >>ro-bdw-i5-5250u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8
> >>ro-bdw-i7-5600u  total:102  pass:75   dwarn:0   dfail:0   fail:0
> >>skip:26
> >>ro-bsw-n3050     total:209  pass:168  dwarn:0   dfail:0   fail:2
> >>skip:39
> >>ro-byt-n2820     total:209  pass:168  dwarn:1   dfail:0   fail:3
> >>skip:37
> >>ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0
> >>skip:23
> >>ro-hsw-i7-4770r  total:102  pass:82   dwarn:0   dfail:0   fail:0
> >>skip:19
> >>ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1
> >>skip:57
> >>ro-skl-i7-6700hq total:204  pass:172  dwarn:11  dfail:0   fail:0
> >>skip:21
> >>ro-snb-i7-2620M  total:102  pass:72   dwarn:0   dfail:0   fail:0
> >>skip:29
> >>fi-skl-i5-6260u failed to connect after reboot
> >>ro-bdw-i7-5557U failed to connect after reboot
> >>ro-ivb2-i7-3770 failed to connect after reboot
> >>ro-ivb-i7-3770 failed to connect after reboot
> >>
> >>Results at /archive/results/CI_IGT_test/RO_Patchwork_1125/
> >>
> >>55d1291 drm-intel-nightly: 2016y-06m-06d-16h-28m-05s UTC integration
> >>manifest
> >>57871c9 drm/i915/guc: enable GuC loading & submission by default
> >>3fdd902 drm/i915/guc: disable GuC submission earlier during GuC (re)load
> >>5492d57 drm/i915/guc: fix GuC loading/submission check
> 
> Merged, thanks for the patches and review.

What review? This patch has not addressed my concerns at all.

It is changes the default submission and makes Skylake slower for what
benefit? The changelog still doesn't explain why we would want to take
the risk and current regressions.
-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] 19+ messages in thread

* Re: ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915/guc: fix GuC loading/submission check
  2016-06-07 20:00       ` Chris Wilson
@ 2016-06-08  8:18         ` Dave Gordon
  0 siblings, 0 replies; 19+ messages in thread
From: Dave Gordon @ 2016-06-08  8:18 UTC (permalink / raw)
  To: Chris Wilson, Tvrtko Ursulin; +Cc: intel-gfx

On 07/06/16 21:00, Chris Wilson wrote:
> On Tue, Jun 07, 2016 at 02:23:34PM +0100, Tvrtko Ursulin wrote:
>>
>> On 07/06/16 11:54, Dave Gordon wrote:
>>> On 07/06/16 09:43, Patchwork wrote:
>>>> == Series Details ==
>>>>
>>>> Series: series starting with [1/3] drm/i915/guc: fix GuC
>>>> loading/submission check
>>>> URL   : https://patchwork.freedesktop.org/series/8380/
>>>> State : failure
>>>>
>>>> == Summary ==
>>>>
>>>> Series 8380v1 Series without cover letter
>>>> http://patchwork.freedesktop.org/api/1.0/series/8380/revisions/1/mbox
>>>>
>>>> Test core_auth:
>>>>          Subgroup basic-auth:
>>>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>>>> Test drv_module_reload_basic:
>>>>                  pass       -> DMESG-WARN (ro-byt-n2820)
>>>
>>> Bug 93381 - [BAT BYT] dmesg WARNING in snd-hda
>>>
>>>> Test gem_exec_flush:
>>>>          Subgroup basic-batch-kernel-default-cmd:
>>>>                  pass       -> FAIL       (ro-byt-n2820)
>>>
>>> Bug 95372 - [BAT BYT] Sporadic failure from
>>> igt/gem_exec_flush@basic-batch-kernel-default-cmd
>>>
>>>>          Subgroup basic-uc-ro-default:
>>>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>>>          Subgroup basic-wb-ro-before-default:
>>>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>>>> Test gem_exec_store:
>>>>          Subgroup basic-bsd:
>>>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>>> Test gem_mmap_gtt:
>>>>          Subgroup basic-short:
>>>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>>>          Subgroup basic-write-gtt:
>>>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>>>> Test gem_pwrite:
>>>>          Subgroup basic:
>>>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>>>> Test kms_addfb_basic:
>>>>          Subgroup addfb25-bad-modifier:
>>>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>>>          Subgroup addfb25-yf-tiled:
>>>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>>>          Subgroup bad-pitch-0:
>>>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>>>          Subgroup bad-pitch-1024:
>>>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>>>>          Subgroup basic-x-tiled:
>>>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>>>>          Subgroup basic-y-tiled:
>>>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>>>          Subgroup clobberred-modifier:
>>>>                  dmesg-warn -> PASS       (ro-skl-i7-6700hq)
>>>>          Subgroup tile-pitch-mismatch:
>>>>                  pass       -> DMESG-WARN (ro-skl-i7-6700hq)
>>>
>>> All the warnings on 'ro-skl-i7-6700hq' appear to be
>>> https://bugs.freedesktop.org/show_bug.cgi?id=95632
>>> "[BAT SKL] *ERROR* Potential atomic update failure on pipe A"
>>>
>>> So all issues accounted for, patchset ready for merge :)
>>>
>>> .Dave.
>>>
>>>> fi-bdw-i7-5557u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8
>>>> fi-hsw-i7-4770k  total:209  pass:190  dwarn:0   dfail:0   fail:0
>>>> skip:19
>>>> fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0
>>>> skip:25
>>>> fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0
>>>> skip:39
>>>> ro-bdw-i5-5250u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8
>>>> ro-bdw-i7-5600u  total:102  pass:75   dwarn:0   dfail:0   fail:0
>>>> skip:26
>>>> ro-bsw-n3050     total:209  pass:168  dwarn:0   dfail:0   fail:2
>>>> skip:39
>>>> ro-byt-n2820     total:209  pass:168  dwarn:1   dfail:0   fail:3
>>>> skip:37
>>>> ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0
>>>> skip:23
>>>> ro-hsw-i7-4770r  total:102  pass:82   dwarn:0   dfail:0   fail:0
>>>> skip:19
>>>> ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1
>>>> skip:57
>>>> ro-skl-i7-6700hq total:204  pass:172  dwarn:11  dfail:0   fail:0
>>>> skip:21
>>>> ro-snb-i7-2620M  total:102  pass:72   dwarn:0   dfail:0   fail:0
>>>> skip:29
>>>> fi-skl-i5-6260u failed to connect after reboot
>>>> ro-bdw-i7-5557U failed to connect after reboot
>>>> ro-ivb2-i7-3770 failed to connect after reboot
>>>> ro-ivb-i7-3770 failed to connect after reboot
>>>>
>>>> Results at /archive/results/CI_IGT_test/RO_Patchwork_1125/
>>>>
>>>> 55d1291 drm-intel-nightly: 2016y-06m-06d-16h-28m-05s UTC integration
>>>> manifest
>>>> 57871c9 drm/i915/guc: enable GuC loading & submission by default
>>>> 3fdd902 drm/i915/guc: disable GuC submission earlier during GuC (re)load
>>>> 5492d57 drm/i915/guc: fix GuC loading/submission check
>>
>> Merged, thanks for the patches and review.
>
> What review? This patch has not addressed my concerns at all.
>
> It is changes the default submission and makes Skylake slower for what
> benefit? The changelog still doesn't explain why we would want to take
> the risk and current regressions.
> -Chris

*"It's Intel POR"*

That means people higher up the foodchain have decided that the GuC is 
the future and the minions have just got to get on with it.

Of course, if it introduces real regressions we'll turn it off again 
until they're resolved, but we're more likely to discover any remaining 
issues with it on by default -- otherwise it's just not getting much 
exposure beyond the in-house validation.

Let's just hope they're all exposed and resolved before September!

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

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

* Re: [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check
  2016-06-07  8:41 ` [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check Tvrtko Ursulin
@ 2016-06-09 11:04   ` Tvrtko Ursulin
  2016-06-10 15:45     ` Dave Gordon
  2016-06-10 16:21     ` [PATCH] drm/i915/guc: suppress GuC-related message on non-GuC platforms Dave Gordon
  0 siblings, 2 replies; 19+ messages in thread
From: Tvrtko Ursulin @ 2016-06-09 11:04 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx


On 07/06/16 09:41, Tvrtko Ursulin wrote:
>
> On 07/06/16 09:14, Dave Gordon wrote:
>> The last stage of the GuC loader also sanitises the GuC submission
>> settings, so should be called unconditionally (even on platforms
>> without a GuC) to ensure consistent settings; in particular, this
>> prevents any attempt to use GuC submission on GuCless platforms!
>>
>> Also fix error path handling and clarify DRM_INFO fallback message.
>>
>> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_gem.c         |  8 +++-----
>>   drivers/gpu/drm/i915/intel_guc_loader.c | 12 ++++++++----
>>   2 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
>> b/drivers/gpu/drm/i915/i915_gem.c
>> index 1bfc260..eae8d7a 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -4930,11 +4930,9 @@ int i915_gem_init_engines(struct drm_device *dev)
>>       intel_mocs_init_l3cc_table(dev);
>>
>>       /* We can't enable contexts until all firmware is loaded */
>> -    if (HAS_GUC(dev)) {
>> -        ret = intel_guc_setup(dev);
>> -        if (ret)
>> -            goto out;
>> -    }
>> +    ret = intel_guc_setup(dev);
>> +    if (ret)
>> +        goto out;
>>
>>       /*
>>        * Increment the next seqno by 0x100 so we have a visible break
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index f2b88c7..4e34c2e 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -425,9 +425,13 @@ int intel_guc_setup(struct drm_device *dev)
>>       if (!i915.enable_guc_loading) {
>>           err = 0;
>>           goto fail;
>> -    } else if (fw_path == NULL || *fw_path == '\0') {
>> -        if (*fw_path == '\0')
>
> Ops. I can only assume I meant !fw_path.
>
>> -            DRM_INFO("No GuC firmware known for this platform\n");
>> +    } else if (fw_path == NULL) {
>> +        /* Device is known to have no uCode (e.g. no GuC) */
>> +        err = -ENXIO;
>> +        goto fail;
>> +    } else if (*fw_path == '\0') {
>> +        /* Device has a GuC but we don't know what f/w to load? */
>> +        DRM_INFO("No GuC firmware known for this platform\n");
>>           err = -ENODEV;
>>           goto fail;
>>       }
>> @@ -535,7 +539,7 @@ int intel_guc_setup(struct drm_device *dev)
>>           if (fw_path == NULL)
>>               DRM_INFO("GuC submission without firmware not
>> supported\n");
>>           if (ret == 0)
>> -            DRM_INFO("Falling back to execlist mode\n");
>> +            DRM_INFO("Falling back from GuC submission to execlist
>> mode\n");
>>           else
>>               DRM_ERROR("GuC init failed: %d\n", ret);
>>       }
>>
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Bah now on BDW we get on boot:

[drm:gen8_init_common_ring] Execlists enabled for render ring
[drm:gen8_init_common_ring] Execlists enabled for blitter ring
[drm:gen8_init_common_ring] Execlists enabled for bsd ring
[drm:gen8_init_common_ring] Execlists enabled for video enhancement ring
[drm:intel_guc_setup] GuC fw status: path (null), fetch NONE, load NONE
[drm] GuC firmware load skipped
[drm] GuC submission without firmware not supported
[drm] Falling back from GuC submission to execlist mode

Which is a bit untidy. :(

Regards,

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

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

* Re: [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check
  2016-06-09 11:04   ` Tvrtko Ursulin
@ 2016-06-10 15:45     ` Dave Gordon
  2016-06-10 16:21     ` [PATCH] drm/i915/guc: suppress GuC-related message on non-GuC platforms Dave Gordon
  1 sibling, 0 replies; 19+ messages in thread
From: Dave Gordon @ 2016-06-10 15:45 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

On 09/06/16 12:04, Tvrtko Ursulin wrote:
>
> On 07/06/16 09:41, Tvrtko Ursulin wrote:
>>
>> On 07/06/16 09:14, Dave Gordon wrote:
>>> The last stage of the GuC loader also sanitises the GuC submission
>>> settings, so should be called unconditionally (even on platforms
>>> without a GuC) to ensure consistent settings; in particular, this
>>> prevents any attempt to use GuC submission on GuCless platforms!
>>>
>>> Also fix error path handling and clarify DRM_INFO fallback message.
>>>
>>> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_gem.c         |  8 +++-----
>>>   drivers/gpu/drm/i915/intel_guc_loader.c | 12 ++++++++----
>>>   2 files changed, 11 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
>>> b/drivers/gpu/drm/i915/i915_gem.c
>>> index 1bfc260..eae8d7a 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>> @@ -4930,11 +4930,9 @@ int i915_gem_init_engines(struct drm_device *dev)
>>>       intel_mocs_init_l3cc_table(dev);
>>>
>>>       /* We can't enable contexts until all firmware is loaded */
>>> -    if (HAS_GUC(dev)) {
>>> -        ret = intel_guc_setup(dev);
>>> -        if (ret)
>>> -            goto out;
>>> -    }
>>> +    ret = intel_guc_setup(dev);
>>> +    if (ret)
>>> +        goto out;
>>>
>>>       /*
>>>        * Increment the next seqno by 0x100 so we have a visible break
>>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>>> index f2b88c7..4e34c2e 100644
>>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>>> @@ -425,9 +425,13 @@ int intel_guc_setup(struct drm_device *dev)
>>>       if (!i915.enable_guc_loading) {
>>>           err = 0;
>>>           goto fail;
>>> -    } else if (fw_path == NULL || *fw_path == '\0') {
>>> -        if (*fw_path == '\0')
>>
>> Ops. I can only assume I meant !fw_path.
>>
>>> -            DRM_INFO("No GuC firmware known for this platform\n");
>>> +    } else if (fw_path == NULL) {
>>> +        /* Device is known to have no uCode (e.g. no GuC) */
>>> +        err = -ENXIO;
>>> +        goto fail;
>>> +    } else if (*fw_path == '\0') {
>>> +        /* Device has a GuC but we don't know what f/w to load? */
>>> +        DRM_INFO("No GuC firmware known for this platform\n");
>>>           err = -ENODEV;
>>>           goto fail;
>>>       }
>>> @@ -535,7 +539,7 @@ int intel_guc_setup(struct drm_device *dev)
>>>           if (fw_path == NULL)
>>>               DRM_INFO("GuC submission without firmware not
>>> supported\n");
>>>           if (ret == 0)
>>> -            DRM_INFO("Falling back to execlist mode\n");
>>> +            DRM_INFO("Falling back from GuC submission to execlist
>>> mode\n");
>>>           else
>>>               DRM_ERROR("GuC init failed: %d\n", ret);
>>>       }
>>>
>>
>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Bah now on BDW we get on boot:
>
> [drm:gen8_init_common_ring] Execlists enabled for render ring
> [drm:gen8_init_common_ring] Execlists enabled for blitter ring
> [drm:gen8_init_common_ring] Execlists enabled for bsd ring
> [drm:gen8_init_common_ring] Execlists enabled for video enhancement ring
> [drm:intel_guc_setup] GuC fw status: path (null), fetch NONE, load NONE
> [drm] GuC firmware load skipped
> [drm] GuC submission without firmware not supported
> [drm] Falling back from GuC submission to execlist mode
>
> Which is a bit untidy. :(
>
> Regards,
> Tvrtko

You shouldn't get the second or third [drm] message, if you haven't 
overridden the default values. The default for enable_guc_submission
is (-1) which gets mapped to HAS_GUC_SCHED() which is 0 on BDW; you only 
get the extra messages if you've set it to a non-default value.

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

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

* [PATCH] drm/i915/guc: suppress GuC-related message on non-GuC platforms
  2016-06-09 11:04   ` Tvrtko Ursulin
  2016-06-10 15:45     ` Dave Gordon
@ 2016-06-10 16:21     ` Dave Gordon
  2016-06-13  9:00       ` Tvrtko Ursulin
  1 sibling, 1 reply; 19+ messages in thread
From: Dave Gordon @ 2016-06-10 16:21 UTC (permalink / raw)
  To: intel-gfx

If the user doesn't override the default values of the GuC-related
kernel parameters, then on a non-GuC-based platform we shouldn't
mention that we haven't loaded the GuC firmware.

The various messages have been reordered into a least->most severe
cascade (none/INFO/INFO/ERROR) for ease of comprehension.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 41f7c7d..05732e3 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -525,12 +525,14 @@ int intel_guc_setup(struct drm_device *dev)
 		ret = 0;
 	}
 
-	if (err == 0)
+	if (err == 0 && !HAS_GUC_UCODE(dev))
+		;	/* Don't mention the GuC! */
+	else if (err == 0)
 		DRM_INFO("GuC firmware load skipped\n");
-	else if (ret == -EIO)
-		DRM_ERROR("GuC firmware load failed: %d\n", err);
-	else
+	else if (ret != -EIO)
 		DRM_INFO("GuC firmware load failed: %d\n", err);
+	else
+		DRM_ERROR("GuC firmware load failed: %d\n", err);
 
 	if (i915.enable_guc_submission) {
 		if (fw_path == 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] 19+ messages in thread

* ✗ Ro.CI.BAT: failure for series starting with drm/i915/guc: suppress GuC-related message on non-GuC platforms (rev2)
  2016-06-07  8:14 [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check Dave Gordon
                   ` (3 preceding siblings ...)
  2016-06-07  8:43 ` ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915/guc: fix GuC loading/submission check Patchwork
@ 2016-06-10 16:59 ` Patchwork
  2016-06-10 18:14   ` Dave Gordon
  4 siblings, 1 reply; 19+ messages in thread
From: Patchwork @ 2016-06-10 16:59 UTC (permalink / raw)
  To: Dave Gordon; +Cc: intel-gfx

== Series Details ==

Series: series starting with drm/i915/guc: suppress GuC-related message on non-GuC platforms (rev2)
URL   : https://patchwork.freedesktop.org/series/8380/
State : failure

== Summary ==

Series 8380v2 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/8380/revisions/2/mbox

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                pass       -> FAIL       (ro-byt-n2820)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                fail       -> PASS       (ro-ivb-i7-3770)
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> SKIP       (ro-bdw-i7-5557U)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> SKIP       (ro-bdw-i7-5557U)
        Subgroup suspend-read-crc-pipe-c:
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)

ro-bdw-i5-5250u  total:213  pass:197  dwarn:4   dfail:0   fail:0   skip:12 
ro-bdw-i7-5557U  total:213  pass:197  dwarn:1   dfail:0   fail:0   skip:15 
ro-bdw-i7-5600u  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:213  pass:171  dwarn:1   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:213  pass:173  dwarn:0   dfail:0   fail:3   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:213  pass:150  dwarn:0   dfail:0   fail:1   skip:62 
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 

Results at /archive/results/CI_IGT_test/RO_Patchwork_1160/

5de1a00 drm-intel-nightly: 2016y-06m-10d-15h-26m-55s UTC integration manifest
8f7efa4 drm/i915/guc: suppress GuC-related message on non-GuC platforms

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

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

* Re: ✗ Ro.CI.BAT: failure for series starting with drm/i915/guc: suppress GuC-related message on non-GuC platforms (rev2)
  2016-06-10 16:59 ` ✗ Ro.CI.BAT: failure for series starting with drm/i915/guc: suppress GuC-related message on non-GuC platforms (rev2) Patchwork
@ 2016-06-10 18:14   ` Dave Gordon
  2016-06-13  9:06     ` Tvrtko Ursulin
  0 siblings, 1 reply; 19+ messages in thread
From: Dave Gordon @ 2016-06-10 18:14 UTC (permalink / raw)
  To: intel-gfx

On 10/06/16 17:59, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with drm/i915/guc: suppress GuC-related message on non-GuC platforms (rev2)
> URL   : https://patchwork.freedesktop.org/series/8380/
> State : failure
>
> == Summary ==
>
> Series 8380v2 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/8380/revisions/2/mbox
>
> Test gem_exec_flush:
>          Subgroup basic-batch-kernel-default-cmd:
>                  pass       -> FAIL       (ro-byt-n2820)

Bug 95372 - [BAT BYT] Sporadic failure from 
igt/gem_exec_flush@basic-batch-kernel-default-cmd

> Test kms_pipe_crc_basic:
>          Subgroup nonblocking-crc-pipe-c-frame-sequence:
>                  fail       -> PASS       (ro-ivb-i7-3770)
>          Subgroup suspend-read-crc-pipe-a:
>                  dmesg-warn -> SKIP       (ro-bdw-i7-5557U)
>          Subgroup suspend-read-crc-pipe-b:
>                  dmesg-warn -> SKIP       (ro-bdw-i7-5557U)
>          Subgroup suspend-read-crc-pipe-c:
>                  skip       -> DMESG-WARN (ro-bdw-i5-5250u)

Looks like https://bugs.freedesktop.org/show_bug.cgi?id=94605#c17
as do the two that have gone from WARN to SKIP. Connector problem?

> ro-bdw-i5-5250u  total:213  pass:197  dwarn:4   dfail:0   fail:0   skip:12
> ro-bdw-i7-5557U  total:213  pass:197  dwarn:1   dfail:0   fail:0   skip:15
> ro-bdw-i7-5600u  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28
> ro-bsw-n3050     total:213  pass:171  dwarn:1   dfail:0   fail:2   skip:39
> ro-byt-n2820     total:213  pass:173  dwarn:0   dfail:0   fail:3   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:213  pass:150  dwarn:0   dfail:0   fail:1   skip:62
> 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
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1160/
>
> 5de1a00 drm-intel-nightly: 2016y-06m-10d-15h-26m-55s UTC integration manifest
> 8f7efa4 drm/i915/guc: suppress GuC-related message on non-GuC platforms
>

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

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

* Re: [PATCH] drm/i915/guc: suppress GuC-related message on non-GuC platforms
  2016-06-10 16:21     ` [PATCH] drm/i915/guc: suppress GuC-related message on non-GuC platforms Dave Gordon
@ 2016-06-13  9:00       ` Tvrtko Ursulin
  0 siblings, 0 replies; 19+ messages in thread
From: Tvrtko Ursulin @ 2016-06-13  9:00 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx


On 10/06/16 17:21, Dave Gordon wrote:
> If the user doesn't override the default values of the GuC-related
> kernel parameters, then on a non-GuC-based platform we shouldn't
> mention that we haven't loaded the GuC firmware.
>
> The various messages have been reordered into a least->most severe
> cascade (none/INFO/INFO/ERROR) for ease of comprehension.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_loader.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 41f7c7d..05732e3 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -525,12 +525,14 @@ int intel_guc_setup(struct drm_device *dev)
>   		ret = 0;
>   	}
>
> -	if (err == 0)
> +	if (err == 0 && !HAS_GUC_UCODE(dev))
> +		;	/* Don't mention the GuC! */
> +	else if (err == 0)
>   		DRM_INFO("GuC firmware load skipped\n");
> -	else if (ret == -EIO)
> -		DRM_ERROR("GuC firmware load failed: %d\n", err);
> -	else
> +	else if (ret != -EIO)
>   		DRM_INFO("GuC firmware load failed: %d\n", err);
> +	else
> +		DRM_ERROR("GuC firmware load failed: %d\n", err);
>
>   	if (i915.enable_guc_submission) {
>   		if (fw_path == NULL)
>

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] 19+ messages in thread

* Re: ✗ Ro.CI.BAT: failure for series starting with drm/i915/guc: suppress GuC-related message on non-GuC platforms (rev2)
  2016-06-10 18:14   ` Dave Gordon
@ 2016-06-13  9:06     ` Tvrtko Ursulin
  0 siblings, 0 replies; 19+ messages in thread
From: Tvrtko Ursulin @ 2016-06-13  9:06 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx


On 10/06/16 19:14, Dave Gordon wrote:
> On 10/06/16 17:59, Patchwork wrote:
>> == Series Details ==
>>
>> Series: series starting with drm/i915/guc: suppress GuC-related
>> message on non-GuC platforms (rev2)
>> URL   : https://patchwork.freedesktop.org/series/8380/
>> State : failure
>>
>> == Summary ==
>>
>> Series 8380v2 Series without cover letter
>> http://patchwork.freedesktop.org/api/1.0/series/8380/revisions/2/mbox
>>
>> Test gem_exec_flush:
>>          Subgroup basic-batch-kernel-default-cmd:
>>                  pass       -> FAIL       (ro-byt-n2820)
>
> Bug 95372 - [BAT BYT] Sporadic failure from
> igt/gem_exec_flush@basic-batch-kernel-default-cmd
>
>> Test kms_pipe_crc_basic:
>>          Subgroup nonblocking-crc-pipe-c-frame-sequence:
>>                  fail       -> PASS       (ro-ivb-i7-3770)
>>          Subgroup suspend-read-crc-pipe-a:
>>                  dmesg-warn -> SKIP       (ro-bdw-i7-5557U)
>>          Subgroup suspend-read-crc-pipe-b:
>>                  dmesg-warn -> SKIP       (ro-bdw-i7-5557U)
>>          Subgroup suspend-read-crc-pipe-c:
>>                  skip       -> DMESG-WARN (ro-bdw-i5-5250u)
>
> Looks like https://bugs.freedesktop.org/show_bug.cgi?id=94605#c17
> as do the two that have gone from WARN to SKIP. Connector problem?
>
>> ro-bdw-i5-5250u  total:213  pass:197  dwarn:4   dfail:0   fail:0
>> skip:12
>> ro-bdw-i7-5557U  total:213  pass:197  dwarn:1   dfail:0   fail:0
>> skip:15
>> ro-bdw-i7-5600u  total:213  pass:185  dwarn:0   dfail:0   fail:0
>> skip:28
>> ro-bsw-n3050     total:213  pass:171  dwarn:1   dfail:0   fail:2
>> skip:39
>> ro-byt-n2820     total:213  pass:173  dwarn:0   dfail:0   fail:3
>> 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:213  pass:150  dwarn:0   dfail:0   fail:1
>> skip:62
>> 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
>>
>> Results at /archive/results/CI_IGT_test/RO_Patchwork_1160/
>>
>> 5de1a00 drm-intel-nightly: 2016y-06m-10d-15h-26m-55s UTC integration
>> manifest
>> 8f7efa4 drm/i915/guc: suppress GuC-related message on non-GuC platforms

Merged, thanks for the patch 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] 19+ messages in thread

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

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-07  8:14 [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check Dave Gordon
2016-06-07  8:14 ` [PATCH 2/3] drm/i915/guc: disable GuC submission earlier during GuC (re)load Dave Gordon
2016-06-07  9:51   ` Tvrtko Ursulin
2016-06-07 10:13     ` Dave Gordon
2016-06-07  8:14 ` [PATCH 3/3] drm/i915/guc: enable GuC loading & submission by default Dave Gordon
2016-06-07  9:53   ` Tvrtko Ursulin
2016-06-07  8:41 ` [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check Tvrtko Ursulin
2016-06-09 11:04   ` Tvrtko Ursulin
2016-06-10 15:45     ` Dave Gordon
2016-06-10 16:21     ` [PATCH] drm/i915/guc: suppress GuC-related message on non-GuC platforms Dave Gordon
2016-06-13  9:00       ` Tvrtko Ursulin
2016-06-07  8:43 ` ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915/guc: fix GuC loading/submission check Patchwork
2016-06-07 10:54   ` Dave Gordon
2016-06-07 13:23     ` Tvrtko Ursulin
2016-06-07 20:00       ` Chris Wilson
2016-06-08  8:18         ` Dave Gordon
2016-06-10 16:59 ` ✗ Ro.CI.BAT: failure for series starting with drm/i915/guc: suppress GuC-related message on non-GuC platforms (rev2) Patchwork
2016-06-10 18:14   ` Dave Gordon
2016-06-13  9:06     ` Tvrtko Ursulin

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.