[PATCH 0/3] Add/modify checks within intel_uc_fini_hw

[PATCH 0/3] Add/modify checks within intel_uc_fini_hw

[Fernando Pacheco]

The third patch in the series attempts to downgrade the check for
is_guc_running to is_fw_available within intel_uc_fini_hw. We cannot
rely on is_guc_running because we will completely skip our attempt to
fini uC hw during unload. However, this exposes a new set of problems due
to the driver being able to continue after GuC initialization failures:
we can now try to disable submission or communication that was never enabled
in the first place.

The first two patches attempt to resolve those issues by adding
(hopefully stronger) checks around submission_disable and
communication_disable.

Thanks,
Fernando

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Fernando Pacheco (3):
  drm/i915/uc: Extract common code from GuC stop/disable comm
  drm/i915/uc: Disable GuC submission only if currently enabled
  drm/i915/uc: Fini hw even if GuC is not running

 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 23 +++++++++++++
 .../gpu/drm/i915/gt/uc/intel_guc_submission.h |  1 +
 drivers/gpu/drm/i915/gt/uc/intel_uc.c         | 34 +++++++++++--------
 3 files changed, 44 insertions(+), 14 deletions(-)

-- 
2.23.0

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

[PATCH 1/3] drm/i915/uc: Extract common code from GuC stop/disable comm

[Fernando Pacheco]

During normal driver unload we attempt to disable GuC communication
while it is currently stopped. This results in a nop'd call to
intel_guc_ct_disable within guc_disable_communication because
stop/disable rely on the same flag to prevent further comms with CT.

We can avoid the call to disable and still leave communication in a
satisfactory state by extracting a set of shared steps from stop/disable.
This set can include guc_disable_interrupts as we do not require the
single caller of guc_stop_communication to be atomic:
"drm/i915/selftests: Fixup atomic reset checking".

This situation (stop -> disable) only occurs during intel_uc_fini_hw,
so during fini, call guc_disable_communication only if currently enabled.
The symmetric calls to enable/disable remain unmodified for all other
scenarios.

Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 30 ++++++++++++++++-----------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 71ee7ab035cc..29a9eec60d2e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -224,17 +224,7 @@ static int guc_enable_communication(struct intel_guc *guc)
 	return 0;
 }
 
-static void guc_stop_communication(struct intel_guc *guc)
-{
-	intel_guc_ct_stop(&guc->ct);
-
-	guc->send = intel_guc_send_nop;
-	guc->handler = intel_guc_to_host_event_handler_nop;
-
-	guc_clear_mmio_msg(guc);
-}
-
-static void guc_disable_communication(struct intel_guc *guc)
+static void __guc_stop_communication(struct intel_guc *guc)
 {
 	/*
 	 * Events generated during or after CT disable are logged by guc in
@@ -247,6 +237,20 @@ static void guc_disable_communication(struct intel_guc *guc)
 
 	guc->send = intel_guc_send_nop;
 	guc->handler = intel_guc_to_host_event_handler_nop;
+}
+
+static void guc_stop_communication(struct intel_guc *guc)
+{
+	intel_guc_ct_stop(&guc->ct);
+
+	__guc_stop_communication(guc);
+
+	DRM_INFO("GuC communication stopped\n");
+}
+
+static void guc_disable_communication(struct intel_guc *guc)
+{
+	__guc_stop_communication(guc);
 
 	intel_guc_ct_disable(&guc->ct);
 
@@ -537,7 +541,9 @@ void intel_uc_fini_hw(struct intel_uc *uc)
 	if (intel_uc_supports_guc_submission(uc))
 		intel_guc_submission_disable(guc);
 
-	guc_disable_communication(guc);
+	if (guc_communication_enabled(guc))
+		guc_disable_communication(guc);
+
 	__uc_sanitize(uc);
 }
 
-- 
2.23.0

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

[PATCH 2/3] drm/i915/uc: Disable GuC submission only if currently enabled

[Fernando Pacheco]

It is not enough to check that uc supports GuC submission now
that we can continue to load the driver after GuC initialization
failure (support != enabled). Instead we should explicitly check
that we enabled GuC submission.

Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 23 +++++++++++++++++++
 .../gpu/drm/i915/gt/uc/intel_guc_submission.h |  1 +
 drivers/gpu/drm/i915/gt/uc/intel_uc.c         |  2 +-
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index f325d3dd564f..d4aff9a96c7a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -191,6 +191,16 @@ static bool __doorbell_valid(struct intel_guc *guc, u16 db_id)
 	return intel_uncore_read(uncore, GEN8_DRBREGL(db_id)) & GEN8_DRB_VALID;
 }
 
+static bool __doorbell_enabled(struct intel_guc_client *client)
+{
+	struct guc_doorbell_info *doorbell;
+
+	GEM_BUG_ON(!has_doorbell(client));
+
+	doorbell = __get_doorbell(client);
+	return doorbell->db_status == GUC_DOORBELL_ENABLED;
+}
+
 static void __init_doorbell(struct intel_guc_client *client)
 {
 	struct guc_doorbell_info *doorbell;
@@ -1112,6 +1122,19 @@ static void guc_set_default_submission(struct intel_engine_cs *engine)
 	GEM_BUG_ON(engine->irq_enable || engine->irq_disable);
 }
 
+bool intel_guc_is_submission_enabled(struct intel_guc *guc)
+{
+	if (!intel_guc_is_submission_supported(guc))
+		return false;
+
+	/*
+	 * Use the fact that we enable the guc execbuf_client
+	 * and its doorbell when enabling GuC submission as a proxy
+	 * for the latter.
+	 */
+	return guc->execbuf_client && __doorbell_enabled(guc->execbuf_client);
+}
+
 int intel_guc_submission_enable(struct intel_guc *guc)
 {
 	struct intel_gt *gt = guc_to_gt(guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index 54d716828352..80b18a2c885a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -58,6 +58,7 @@ struct intel_guc_client {
 
 void intel_guc_submission_init_early(struct intel_guc *guc);
 int intel_guc_submission_init(struct intel_guc *guc);
+bool intel_guc_is_submission_enabled(struct intel_guc *guc);
 int intel_guc_submission_enable(struct intel_guc *guc);
 void intel_guc_submission_disable(struct intel_guc *guc);
 void intel_guc_submission_fini(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 29a9eec60d2e..b2eb340ce87e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -538,7 +538,7 @@ void intel_uc_fini_hw(struct intel_uc *uc)
 	if (!intel_guc_is_running(guc))
 		return;
 
-	if (intel_uc_supports_guc_submission(uc))
+	if (intel_guc_is_submission_enabled(guc))
 		intel_guc_submission_disable(guc);
 
 	if (guc_communication_enabled(guc))
-- 
2.23.0

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

[PATCH 3/3] drm/i915/uc: Fini hw even if GuC is not running

[Fernando Pacheco]

We should not be skipping uc_fini_hw on finding GuC
is no longer running. There is plenty of hw and internal
state that can be cleaned up without having to communicate
with GuC.

v2: better to check if fw is available (Michal)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110943
Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index b2eb340ce87e..ad5b928fe36a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -535,7 +535,7 @@ void intel_uc_fini_hw(struct intel_uc *uc)
 {
 	struct intel_guc *guc = &uc->guc;
 
-	if (!intel_guc_is_running(guc))
+	if (!intel_uc_fw_is_available(&guc->fw))
 		return;
 
 	if (intel_guc_is_submission_enabled(guc))
-- 
2.23.0

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

✓ Fi.CI.BAT: success for Add/modify checks within intel_uc_fini_hw

[Patchwork]

== Series Details ==

Series: Add/modify checks within intel_uc_fini_hw
URL   : https://patchwork.freedesktop.org/series/65901/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6794 -> Patchwork_14210
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/

Known issues
------------

  Here are the changes found in Patchwork_14210 that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       [FAIL][1] ([fdo#109483] / [fdo#109635 ]) -> [PASS][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
  [fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 


Participating hosts (51 -> 44)
------------------------------

  Additional (2): fi-hsw-peppy fi-gdg-551 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-tgl-u fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6794 -> Patchwork_14210

  CI-20190529: 20190529
  CI_DRM_6794: a1a45a21f6fef00f6150dc151c23555aa851bf74 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5150: a4e8217bcdfef9bb523f26a9084bbf615a6e8abb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14210: bce669a82188c58d6149f733f0a4e5187e88bdd7 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

bce669a82188 drm/i915/uc: Fini hw even if GuC is not running
b8c333dbc712 drm/i915/uc: Disable GuC submission only if currently enabled
169836b86c9a drm/i915/uc: Extract common code from GuC stop/disable comm

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/uc: Extract common code from GuC stop/disable comm

[Daniele Ceraolo Spurio]

On 8/27/19 5:45 PM, Fernando Pacheco wrote:
> During normal driver unload we attempt to disable GuC communication
> while it is currently stopped. This results in a nop'd call to
> intel_guc_ct_disable within guc_disable_communication because
> stop/disable rely on the same flag to prevent further comms with CT.
> 
> We can avoid the call to disable and still leave communication in a
> satisfactory state by extracting a set of shared steps from stop/disable.
> This set can include guc_disable_interrupts as we do not require the
> single caller of guc_stop_communication to be atomic:
> "drm/i915/selftests: Fixup atomic reset checking".
> 
> This situation (stop -> disable) only occurs during intel_uc_fini_hw,
> so during fini, call guc_disable_communication only if currently enabled.
> The symmetric calls to enable/disable remain unmodified for all other
> scenarios.
> 
> Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_uc.c | 30 ++++++++++++++++-----------
>   1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 71ee7ab035cc..29a9eec60d2e 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -224,17 +224,7 @@ static int guc_enable_communication(struct intel_guc *guc)
>   	return 0;
>   }
>   
> -static void guc_stop_communication(struct intel_guc *guc)
> -{
> -	intel_guc_ct_stop(&guc->ct);
> -
> -	guc->send = intel_guc_send_nop;
> -	guc->handler = intel_guc_to_host_event_handler_nop;
> -
> -	guc_clear_mmio_msg(guc);
> -}
> -
> -static void guc_disable_communication(struct intel_guc *guc)
> +static void __guc_stop_communication(struct intel_guc *guc)
>   {
>   	/*
>   	 * Events generated during or after CT disable are logged by guc in
> @@ -247,6 +237,20 @@ static void guc_disable_communication(struct intel_guc *guc)
>   
>   	guc->send = intel_guc_send_nop;
>   	guc->handler = intel_guc_to_host_event_handler_nop;
> +}
> +
> +static void guc_stop_communication(struct intel_guc *guc)
> +{
> +	intel_guc_ct_stop(&guc->ct);
> +

The only difference between intel_guc_ct_stop() and 
intel_guc_ct_disable() is that in the latter we also tell guc that we've 
disabled the buffers. We could probably just add a check to return early 
if !intel_guc_is_running() in intel_guc_ct_disable() and drop the 
stop/disable differentiation entirely, but that doesn't need to happen 
in this patch.


> +	__guc_stop_communication(guc);
> +
> +	DRM_INFO("GuC communication stopped\n");
> +}
> +
> +static void guc_disable_communication(struct intel_guc *guc)
> +{

Are we now guaranteed that guc_disable_communication() is called only of 
communication is actually enabled? if so, we could add here a:

	GEM_BUG_ON(!guc_communication_enabled(guc));

with or without that:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

> +	__guc_stop_communication(guc);
>   
>   	intel_guc_ct_disable(&guc->ct);
>   
> @@ -537,7 +541,9 @@ void intel_uc_fini_hw(struct intel_uc *uc)
>   	if (intel_uc_supports_guc_submission(uc))
>   		intel_guc_submission_disable(guc);
>   
> -	guc_disable_communication(guc);
> +	if (guc_communication_enabled(guc))
> +		guc_disable_communication(guc);
> +
>   	__uc_sanitize(uc);
>   }
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/uc: Extract common code from GuC stop/disable comm

[Daniele Ceraolo Spurio]

BTW, this patch should be enough for 
https://bugs.freedesktop.org/show_bug.cgi?id=110943, because we disable 
interrupts on _stop and therefore we don't leave them enabled even if we 
skip fini_hw (which still needs to be fixed for the submission side of 
things).

Daniele

On 8/28/19 1:47 PM, Daniele Ceraolo Spurio wrote:
> 
> 
> On 8/27/19 5:45 PM, Fernando Pacheco wrote:
>> During normal driver unload we attempt to disable GuC communication
>> while it is currently stopped. This results in a nop'd call to
>> intel_guc_ct_disable within guc_disable_communication because
>> stop/disable rely on the same flag to prevent further comms with CT.
>>
>> We can avoid the call to disable and still leave communication in a
>> satisfactory state by extracting a set of shared steps from stop/disable.
>> This set can include guc_disable_interrupts as we do not require the
>> single caller of guc_stop_communication to be atomic:
>> "drm/i915/selftests: Fixup atomic reset checking".
>>
>> This situation (stop -> disable) only occurs during intel_uc_fini_hw,
>> so during fini, call guc_disable_communication only if currently enabled.
>> The symmetric calls to enable/disable remain unmodified for all other
>> scenarios.
>>
>> Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/uc/intel_uc.c | 30 ++++++++++++++++-----------
>>   1 file changed, 18 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c 
>> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> index 71ee7ab035cc..29a9eec60d2e 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> @@ -224,17 +224,7 @@ static int guc_enable_communication(struct 
>> intel_guc *guc)
>>       return 0;
>>   }
>> -static void guc_stop_communication(struct intel_guc *guc)
>> -{
>> -    intel_guc_ct_stop(&guc->ct);
>> -
>> -    guc->send = intel_guc_send_nop;
>> -    guc->handler = intel_guc_to_host_event_handler_nop;
>> -
>> -    guc_clear_mmio_msg(guc);
>> -}
>> -
>> -static void guc_disable_communication(struct intel_guc *guc)
>> +static void __guc_stop_communication(struct intel_guc *guc)
>>   {
>>       /*
>>        * Events generated during or after CT disable are logged by guc in
>> @@ -247,6 +237,20 @@ static void guc_disable_communication(struct 
>> intel_guc *guc)
>>       guc->send = intel_guc_send_nop;
>>       guc->handler = intel_guc_to_host_event_handler_nop;
>> +}
>> +
>> +static void guc_stop_communication(struct intel_guc *guc)
>> +{
>> +    intel_guc_ct_stop(&guc->ct);
>> +
> 
> The only difference between intel_guc_ct_stop() and 
> intel_guc_ct_disable() is that in the latter we also tell guc that we've 
> disabled the buffers. We could probably just add a check to return early 
> if !intel_guc_is_running() in intel_guc_ct_disable() and drop the 
> stop/disable differentiation entirely, but that doesn't need to happen 
> in this patch.
> 
> 
>> +    __guc_stop_communication(guc);
>> +
>> +    DRM_INFO("GuC communication stopped\n");
>> +}
>> +
>> +static void guc_disable_communication(struct intel_guc *guc)
>> +{
> 
> Are we now guaranteed that guc_disable_communication() is called only of 
> communication is actually enabled? if so, we could add here a:
> 
>      GEM_BUG_ON(!guc_communication_enabled(guc));
> 
> with or without that:
> 
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> Daniele
> 
>> +    __guc_stop_communication(guc);
>>       intel_guc_ct_disable(&guc->ct);
>> @@ -537,7 +541,9 @@ void intel_uc_fini_hw(struct intel_uc *uc)
>>       if (intel_uc_supports_guc_submission(uc))
>>           intel_guc_submission_disable(guc);
>> -    guc_disable_communication(guc);
>> +    if (guc_communication_enabled(guc))
>> +        guc_disable_communication(guc);
>> +
>>       __uc_sanitize(uc);
>>   }
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for Add/modify checks within intel_uc_fini_hw

[Patchwork]

== Series Details ==

Series: Add/modify checks within intel_uc_fini_hw
URL   : https://patchwork.freedesktop.org/series/65901/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6794_full -> Patchwork_14210_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_14210_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@preempt-bsd:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#111325]) +3 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb7/igt@gem_exec_schedule@preempt-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb4/igt@gem_exec_schedule@preempt-bsd.html

  * igt@i915_hangman@error-state-capture-rcs0:
    - shard-snb:          [PASS][3] -> [INCOMPLETE][4] ([fdo#105411])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-snb1/igt@i915_hangman@error-state-capture-rcs0.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-snb1/igt@i915_hangman@error-state-capture-rcs0.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen:
    - shard-apl:          [PASS][5] -> [FAIL][6] ([fdo#103232])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-apl6/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-apl6/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html
    - shard-kbl:          [PASS][7] -> [FAIL][8] ([fdo#103232])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-128x42-onscreen.html

  * igt@kms_cursor_crc@pipe-a-cursor-64x64-random:
    - shard-hsw:          [PASS][9] -> [INCOMPLETE][10] ([fdo#103540])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-hsw7/igt@kms_cursor_crc@pipe-a-cursor-64x64-random.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-hsw7/igt@kms_cursor_crc@pipe-a-cursor-64x64-random.html

  * igt@kms_flip@basic-flip-vs-modeset:
    - shard-apl:          [PASS][11] -> [INCOMPLETE][12] ([fdo#103927]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-apl1/igt@kms_flip@basic-flip-vs-modeset.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-apl8/igt@kms_flip@basic-flip-vs-modeset.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-apl:          [PASS][13] -> [DMESG-WARN][14] ([fdo#108566]) +5 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-apl6/igt@kms_flip@flip-vs-suspend-interruptible.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-apl6/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-skl:          [PASS][15] -> [FAIL][16] ([fdo#103167]) +2 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-skl1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-wc.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-skl6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-wc.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-iclb:         [PASS][17] -> [FAIL][18] ([fdo#103167]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109642] / [fdo#111068])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb1/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_dpms:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109441])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb2/igt@kms_psr@psr2_dpms.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb6/igt@kms_psr@psr2_dpms.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#109276]) +19 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb2/igt@prime_vgem@fence-wait-bsd2.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb6/igt@prime_vgem@fence-wait-bsd2.html

  
#### Possible fixes ####

  * igt@gem_eio@reset-stress:
    - shard-snb:          [FAIL][25] ([fdo#109661]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-snb5/igt@gem_eio@reset-stress.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-snb4/igt@gem_eio@reset-stress.html

  * igt@gem_exec_parallel@bcs0-contexts:
    - shard-hsw:          [FAIL][27] ([fdo#111469]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-hsw1/igt@gem_exec_parallel@bcs0-contexts.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-hsw2/igt@gem_exec_parallel@bcs0-contexts.html

  * igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd:
    - shard-iclb:         [SKIP][29] ([fdo#111325]) -> [PASS][30] +5 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb4/igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb6/igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd.html

  * igt@gem_softpin@noreloc-s3:
    - shard-apl:          [DMESG-WARN][31] ([fdo#108566]) -> [PASS][32] +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-apl2/igt@gem_softpin@noreloc-s3.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-apl1/igt@gem_softpin@noreloc-s3.html

  * igt@kms_color@pipe-b-ctm-blue-to-red:
    - shard-skl:          [FAIL][33] ([fdo#107201]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-skl3/igt@kms_color@pipe-b-ctm-blue-to-red.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-skl8/igt@kms_color@pipe-b-ctm-blue-to-red.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x42-random:
    - shard-skl:          [FAIL][35] ([fdo#103232]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-skl7/igt@kms_cursor_crc@pipe-a-cursor-128x42-random.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-skl9/igt@kms_cursor_crc@pipe-a-cursor-128x42-random.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
    - shard-glk:          [FAIL][37] ([fdo#104873]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-glk9/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-glk7/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html

  * igt@kms_cursor_legacy@cursor-vs-flip-toggle:
    - shard-hsw:          [FAIL][39] ([fdo#103355]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-toggle.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-hsw1/igt@kms_cursor_legacy@cursor-vs-flip-toggle.html

  * igt@kms_cursor_legacy@flip-vs-cursor-varying-size:
    - shard-skl:          [FAIL][41] ([fdo#102670] / [fdo#106081]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-skl2/igt@kms_cursor_legacy@flip-vs-cursor-varying-size.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor-varying-size.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][43] ([fdo#105363]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-skl2/igt@kms_flip@flip-vs-expired-vblank.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-skl7/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [FAIL][45] ([fdo#103167]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - shard-skl:          [FAIL][47] ([fdo#103191]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-skl8/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-skl3/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [FAIL][49] ([fdo#103166]) -> [PASS][50] +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [SKIP][51] ([fdo#109441]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb8/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [SKIP][53] ([fdo#109276]) -> [PASS][54] +20 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb6/igt@prime_busy@hang-bsd2.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb2/igt@prime_busy@hang-bsd2.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [SKIP][55] ([fdo#109276]) -> [FAIL][56] ([fdo#111329])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb7/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb4/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][57] ([fdo#111330]) -> [SKIP][58] ([fdo#109276]) +1 similar issue
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb4/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb8/igt@gem_mocs_settings@mocs-reset-bsd2.html

  * igt@gem_mocs_settings@mocs-settings-bsd2:
    - shard-iclb:         [SKIP][59] ([fdo#109276]) -> [FAIL][60] ([fdo#111330])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6794/shard-iclb7/igt@gem_mocs_settings@mocs-settings-bsd2.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/shard-iclb4/igt@gem_mocs_settings@mocs-settings-bsd2.html

  
  [fdo#102670]: https://bugs.freedesktop.org/show_bug.cgi?id=102670
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#106081]: https://bugs.freedesktop.org/show_bug.cgi?id=106081
  [fdo#107201]: https://bugs.freedesktop.org/show_bug.cgi?id=107201
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#111469]: https://bugs.freedesktop.org/show_bug.cgi?id=111469


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6794 -> Patchwork_14210

  CI-20190529: 20190529
  CI_DRM_6794: a1a45a21f6fef00f6150dc151c23555aa851bf74 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5150: a4e8217bcdfef9bb523f26a9084bbf615a6e8abb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14210: bce669a82188c58d6149f733f0a4e5187e88bdd7 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14210/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/uc: Disable GuC submission only if currently enabled

[Janusz Krzysztofik]

Hi Fernando,

On Wednesday, August 28, 2019 2:45:57 AM CEST Fernando Pacheco wrote:
> It is not enough to check that uc supports GuC submission now
> that we can continue to load the driver after GuC initialization
> failure (support != enabled). Instead we should explicitly check
> that we enabled GuC submission.

What's the status of this patch?

I think that having your intel_guc_is_submission_enabled() helper available I 
would be able to resolve a few related issues, which I've accidentally taken 
upon myself, without inventing my own version.

Thanks,
Janusz


> Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 23 +++++++++++++++++++
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.h |  1 +
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c         |  2 +-
>  3 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/
gpu/drm/i915/gt/uc/intel_guc_submission.c
> index f325d3dd564f..d4aff9a96c7a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -191,6 +191,16 @@ static bool __doorbell_valid(struct intel_guc *guc, u16 
db_id)
>  	return intel_uncore_read(uncore, GEN8_DRBREGL(db_id)) & 
GEN8_DRB_VALID;
>  }
>  
> +static bool __doorbell_enabled(struct intel_guc_client *client)
> +{
> +	struct guc_doorbell_info *doorbell;
> +
> +	GEM_BUG_ON(!has_doorbell(client));
> +
> +	doorbell = __get_doorbell(client);
> +	return doorbell->db_status == GUC_DOORBELL_ENABLED;
> +}
> +
>  static void __init_doorbell(struct intel_guc_client *client)
>  {
>  	struct guc_doorbell_info *doorbell;
> @@ -1112,6 +1122,19 @@ static void guc_set_default_submission(struct 
intel_engine_cs *engine)
>  	GEM_BUG_ON(engine->irq_enable || engine->irq_disable);
>  }
>  
> +bool intel_guc_is_submission_enabled(struct intel_guc *guc)
> +{
> +	if (!intel_guc_is_submission_supported(guc))
> +		return false;
> +
> +	/*
> +	 * Use the fact that we enable the guc execbuf_client
> +	 * and its doorbell when enabling GuC submission as a proxy
> +	 * for the latter.
> +	 */
> +	return guc->execbuf_client && __doorbell_enabled(guc-
>execbuf_client);
> +}
> +
>  int intel_guc_submission_enable(struct intel_guc *guc)
>  {
>  	struct intel_gt *gt = guc_to_gt(guc);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/
gpu/drm/i915/gt/uc/intel_guc_submission.h
> index 54d716828352..80b18a2c885a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> @@ -58,6 +58,7 @@ struct intel_guc_client {
>  
>  void intel_guc_submission_init_early(struct intel_guc *guc);
>  int intel_guc_submission_init(struct intel_guc *guc);
> +bool intel_guc_is_submission_enabled(struct intel_guc *guc);
>  int intel_guc_submission_enable(struct intel_guc *guc);
>  void intel_guc_submission_disable(struct intel_guc *guc);
>  void intel_guc_submission_fini(struct intel_guc *guc);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/
gt/uc/intel_uc.c
> index 29a9eec60d2e..b2eb340ce87e 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -538,7 +538,7 @@ void intel_uc_fini_hw(struct intel_uc *uc)
>  	if (!intel_guc_is_running(guc))
>  		return;
>  
> -	if (intel_uc_supports_guc_submission(uc))
> +	if (intel_guc_is_submission_enabled(guc))
>  		intel_guc_submission_disable(guc);
>  
>  	if (guc_communication_enabled(guc))
> 




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