| Message ID | 20191106174003.26162-1-don.hiatt@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/guc: Add GuC method to determine if submission is active. | expand |
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 >
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
> 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/
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