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

Message ID 20190828004558.11903-3-fernando.pacheco@intel.com
State New
Headers show
Series
  • Add/modify checks within intel_uc_fini_hw
Related show

Commit Message

Fernando Pacheco Aug. 28, 2019, 12:45 a.m. UTC
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(-)

Comments

Janusz Krzysztofik Sept. 9, 2019, 1:29 p.m. UTC | #1
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))
>

Patch
diff mbox series

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))