[1/2] drm/i915/guc: Add GuC method to determine if submission is active.
diff mbox series

Message ID 20191106174003.26162-1-don.hiatt@intel.com
State New
Headers show
Series
  • [1/2] drm/i915/guc: Add GuC method to determine if submission is active.
Related show

Commit Message

Hiatt, Don Nov. 6, 2019, 5:40 p.m. UTC
From: Don Hiatt <don.hiatt@intel.com>

Add intel_guc_submission_is_enabled() function to determine if
GuC submission is active. Based on code by Michal Wajdeczko.

Signed-off-by: Don Hiatt <don.hiatt@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c |  2 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h |  1 +
 drivers/gpu/drm/i915/i915_drv.h                   | 14 ++++++++++++++
 3 files changed, 16 insertions(+), 1 deletion(-)

Comments

Daniele Ceraolo Spurio Nov. 7, 2019, 12:13 a.m. UTC | #1
On 11/6/19 9:40 AM, don.hiatt@intel.com wrote:
> From: Don Hiatt <don.hiatt@intel.com>
> 
> Add intel_guc_submission_is_enabled() function to determine if
> GuC submission is active. Based on code by Michal Wajdeczko.
> 
> Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c |  2 +-
>   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h |  1 +
>   drivers/gpu/drm/i915/i915_drv.h                   | 14 ++++++++++++++
>   3 files changed, 16 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 2498c55e0ea5..0aaef7c07879 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1081,7 +1081,7 @@ static void guc_interrupts_release(struct intel_gt *gt)
>   	rps->pm_intrmsk_mbz &= ~ARAT_EXPIRED_INTRMSK;
>   }
>   
> -static void guc_set_default_submission(struct intel_engine_cs *engine)
> +void guc_set_default_submission(struct intel_engine_cs *engine)
>   {
>   	/*
>   	 * We inherit a bunch of functions from execlists that we'd like
> 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..a0132f061ebc 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> @@ -63,5 +63,6 @@ void intel_guc_submission_disable(struct intel_guc *guc);
>   void intel_guc_submission_fini(struct intel_guc *guc);
>   int intel_guc_preempt_work_create(struct intel_guc *guc);
>   void intel_guc_preempt_work_destroy(struct intel_guc *guc);
> +void guc_set_default_submission(struct intel_engine_cs *engine);
>   
>   #endif
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7e0f67babe20..878d574bb1c1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -78,8 +78,10 @@
>   
>   #include "gt/intel_lrc.h"
>   #include "gt/intel_engine.h"
> +#include "gt/intel_gt.h"
>   #include "gt/intel_gt_types.h"
>   #include "gt/intel_workarounds.h"
> +#include "gt/uc/intel_guc_submission.h"
>   #include "gt/uc/intel_uc.h"
>   
>   #include "intel_device_info.h"
> @@ -2032,4 +2034,16 @@ i915_coherent_map_type(struct drm_i915_private *i915)
>   	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
>   }
>   
> +static inline bool intel_guc_submission_is_enabled(struct intel_guc *guc)
> +{
> +	struct intel_gt *gt = guc_to_gt(guc);
> +	struct intel_engine_cs *engine;
> +	enum intel_engine_id id;
> +
> +	for_each_engine(engine, gt, id)
> +		return engine->set_default_submission ==
> +			guc_set_default_submission;
> +	return false;

It feels weird that we need to go look into the engines to understand if 
GuC submission is enabled or not. One of the patches I've sent earlier 
today ("drm/i915/guc: kill doorbell code and selftests") makes it so 
that intel_guc_submission_enable() can't fail, so after that it should 
be possible to have something like:

bool intel_guc_submission_is_enabled(struct intel_guc *guc) {
	return intel_guc_is_submission_supported(guc) &&
		intel_guc_is_running(guc);
}

AFAICS, even without my patch we do sanitize the GuC is 
intel_guc_submission_enable() fails, so the above should still work. If 
something like this doesn't fly, my preference would be to set something 
in intel_guc_submission_enable and check that instead of going inside 
the engines.
Michal, any input here?

Thanks,
Daniele

> +}
> +
>   #endif
>
Tomas Janousek Nov. 10, 2019, 11:10 a.m. UTC | #2
On Wed, Nov 06, 2019 at 09:40:02AM -0800, don.hiatt@intel.com wrote:
> Add intel_guc_submission_is_enabled() function to determine if
> GuC submission is active. Based on code by Michal Wajdeczko.

Don't forget to update USES_GUC_SUBMISSION (and/or
intel_uc_uses_guc_submission) to use this new function.

(And I still think the "drm/i915/guc: Skip suspend/resume GuC action on" patch
should just use USES_GUC_SUBMISSION. The current version (v3 & v4) of the
patch which splits the logic between intel_guc.c and intel_uc.c just to avoid
the goto is making the code harder to maintain, in my opinion.)

> 
> Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c |  2 +-
>  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h |  1 +
>  drivers/gpu/drm/i915/i915_drv.h                   | 14 ++++++++++++++
>  3 files changed, 16 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 2498c55e0ea5..0aaef7c07879 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1081,7 +1081,7 @@ static void guc_interrupts_release(struct intel_gt *gt)
>  	rps->pm_intrmsk_mbz &= ~ARAT_EXPIRED_INTRMSK;
>  }
>  
> -static void guc_set_default_submission(struct intel_engine_cs *engine)
> +void guc_set_default_submission(struct intel_engine_cs *engine)
>  {
>  	/*
>  	 * We inherit a bunch of functions from execlists that we'd like
> 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..a0132f061ebc 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> @@ -63,5 +63,6 @@ void intel_guc_submission_disable(struct intel_guc *guc);
>  void intel_guc_submission_fini(struct intel_guc *guc);
>  int intel_guc_preempt_work_create(struct intel_guc *guc);
>  void intel_guc_preempt_work_destroy(struct intel_guc *guc);
> +void guc_set_default_submission(struct intel_engine_cs *engine);
>  
>  #endif
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7e0f67babe20..878d574bb1c1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -78,8 +78,10 @@
>  
>  #include "gt/intel_lrc.h"
>  #include "gt/intel_engine.h"
> +#include "gt/intel_gt.h"
>  #include "gt/intel_gt_types.h"
>  #include "gt/intel_workarounds.h"
> +#include "gt/uc/intel_guc_submission.h"
>  #include "gt/uc/intel_uc.h"
>  
>  #include "intel_device_info.h"
> @@ -2032,4 +2034,16 @@ i915_coherent_map_type(struct drm_i915_private *i915)
>  	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
>  }
>  
> +static inline bool intel_guc_submission_is_enabled(struct intel_guc *guc)
> +{
> +	struct intel_gt *gt = guc_to_gt(guc);
> +	struct intel_engine_cs *engine;
> +	enum intel_engine_id id;
> +
> +	for_each_engine(engine, gt, id)
> +		return engine->set_default_submission ==
> +			guc_set_default_submission;
> +	return false;
> +}
> +
>  #endif
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hiatt, Don Nov. 11, 2019, 6:04 p.m. UTC | #3
> From: Tomas Janousek <tomi@nomi.cz>
> Sent: Sunday, November 10, 2019 3:11 AM
> To: Hiatt, Don <don.hiatt@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 1/2] drm/i915/guc: Add GuC method to determine if
> submission is active.
> 
> On Wed, Nov 06, 2019 at 09:40:02AM -0800, don.hiatt@intel.com wrote:
> > Add intel_guc_submission_is_enabled() function to determine if
> > GuC submission is active. Based on code by Michal Wajdeczko.
> 
> Don't forget to update USES_GUC_SUBMISSION (and/or
> intel_uc_uses_guc_submission) to use this new function.
> 
Michal said that you have to use USE_GUC_SUBMISSION until the engines
are up and that this function can't replace it's usage.

Michal: Based upon the feedback, do you have any issue with me going back to
v2 of the patch?


> (And I still think the "drm/i915/guc: Skip suspend/resume GuC action on" patch
> should just use USES_GUC_SUBMISSION. The current version (v3 & v4) of the
> patch which splits the logic between intel_guc.c and intel_uc.c just to avoid
> the goto is making the code harder to maintain, in my opinion.)
> 
> >
> > Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c |  2 +-
> >  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h |  1 +
> >  drivers/gpu/drm/i915/i915_drv.h                   | 14 ++++++++++++++
> >  3 files changed, 16 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 2498c55e0ea5..0aaef7c07879 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > @@ -1081,7 +1081,7 @@ static void guc_interrupts_release(struct intel_gt
> *gt)
> >  	rps->pm_intrmsk_mbz &= ~ARAT_EXPIRED_INTRMSK;
> >  }
> >
> > -static void guc_set_default_submission(struct intel_engine_cs *engine)
> > +void guc_set_default_submission(struct intel_engine_cs *engine)
> >  {
> >  	/*
> >  	 * We inherit a bunch of functions from execlists that we'd like
> > 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..a0132f061ebc 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> > @@ -63,5 +63,6 @@ void intel_guc_submission_disable(struct intel_guc
> *guc);
> >  void intel_guc_submission_fini(struct intel_guc *guc);
> >  int intel_guc_preempt_work_create(struct intel_guc *guc);
> >  void intel_guc_preempt_work_destroy(struct intel_guc *guc);
> > +void guc_set_default_submission(struct intel_engine_cs *engine);
> >
> >  #endif
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> > index 7e0f67babe20..878d574bb1c1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -78,8 +78,10 @@
> >
> >  #include "gt/intel_lrc.h"
> >  #include "gt/intel_engine.h"
> > +#include "gt/intel_gt.h"
> >  #include "gt/intel_gt_types.h"
> >  #include "gt/intel_workarounds.h"
> > +#include "gt/uc/intel_guc_submission.h"
> >  #include "gt/uc/intel_uc.h"
> >
> >  #include "intel_device_info.h"
> > @@ -2032,4 +2034,16 @@ i915_coherent_map_type(struct drm_i915_private
> *i915)
> >  	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
> >  }
> >
> > +static inline bool intel_guc_submission_is_enabled(struct intel_guc *guc)
> > +{
> > +	struct intel_gt *gt = guc_to_gt(guc);
> > +	struct intel_engine_cs *engine;
> > +	enum intel_engine_id id;
> > +
> > +	for_each_engine(engine, gt, id)
> > +		return engine->set_default_submission ==
> > +			guc_set_default_submission;
> > +	return false;
> > +}
> > +
> >  #endif
> > --
> > 2.20.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Tomáš Janoušek, a.k.a. Pivník, a.k.a. Liskni_si, http://work.lisk.in/

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 2498c55e0ea5..0aaef7c07879 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1081,7 +1081,7 @@  static void guc_interrupts_release(struct intel_gt *gt)
 	rps->pm_intrmsk_mbz &= ~ARAT_EXPIRED_INTRMSK;
 }
 
-static void guc_set_default_submission(struct intel_engine_cs *engine)
+void guc_set_default_submission(struct intel_engine_cs *engine)
 {
 	/*
 	 * We inherit a bunch of functions from execlists that we'd like
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..a0132f061ebc 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -63,5 +63,6 @@  void intel_guc_submission_disable(struct intel_guc *guc);
 void intel_guc_submission_fini(struct intel_guc *guc);
 int intel_guc_preempt_work_create(struct intel_guc *guc);
 void intel_guc_preempt_work_destroy(struct intel_guc *guc);
+void guc_set_default_submission(struct intel_engine_cs *engine);
 
 #endif
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7e0f67babe20..878d574bb1c1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -78,8 +78,10 @@ 
 
 #include "gt/intel_lrc.h"
 #include "gt/intel_engine.h"
+#include "gt/intel_gt.h"
 #include "gt/intel_gt_types.h"
 #include "gt/intel_workarounds.h"
+#include "gt/uc/intel_guc_submission.h"
 #include "gt/uc/intel_uc.h"
 
 #include "intel_device_info.h"
@@ -2032,4 +2034,16 @@  i915_coherent_map_type(struct drm_i915_private *i915)
 	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
 }
 
+static inline bool intel_guc_submission_is_enabled(struct intel_guc *guc)
+{
+	struct intel_gt *gt = guc_to_gt(guc);
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+
+	for_each_engine(engine, gt, id)
+		return engine->set_default_submission ==
+			guc_set_default_submission;
+	return false;
+}
+
 #endif