Re: [Intel-gfx] [PATCH] drm/i915/execlists: Use vfunc to check engine submission mode
Quoting Michal Wajdeczko (2019-10-28 14:22:29) > On Mon, 28 Oct 2019 14:09:05 +0100, Chris Wilson > wrote: > > > Quoting Michal Wajdeczko (2019-10-28 12:57:03) > >> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h > >> b/drivers/gpu/drm/i915/gt/intel_lrc.h > >> index 99dc576a4e25..23dde9083349 100644 > >> --- a/drivers/gpu/drm/i915/gt/intel_lrc.h > >> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.h > >> @@ -145,4 +145,6 @@ struct intel_engine_cs * > >> intel_virtual_engine_get_sibling(struct intel_engine_cs *engine, > >> unsigned int sibling); > >> > >> +bool intel_engine_in_execlists_submission_mode(struct intel_engine_cs > >> *engine); > > > > Planning to use it outside? > > Yes, there are few places where global USES_GUC_SUBMISSION(i915) is used, > and some of them can be replaced by in_execlists_submission_mode(e) right > away (i915_perf.c). Aye, perf looks like a good candidate to put this to immediate use. Care to include that with Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/execlists: Use vfunc to check engine submission mode
On Mon, 28 Oct 2019 14:09:05 +0100, Chris Wilson wrote: Quoting Michal Wajdeczko (2019-10-28 12:57:03) diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h b/drivers/gpu/drm/i915/gt/intel_lrc.h index 99dc576a4e25..23dde9083349 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.h +++ b/drivers/gpu/drm/i915/gt/intel_lrc.h @@ -145,4 +145,6 @@ struct intel_engine_cs * intel_virtual_engine_get_sibling(struct intel_engine_cs *engine, unsigned int sibling); +bool intel_engine_in_execlists_submission_mode(struct intel_engine_cs *engine); Planning to use it outside? Yes, there are few places where global USES_GUC_SUBMISSION(i915) is used, and some of them can be replaced by in_execlists_submission_mode(e) right away (i915_perf.c). And for others we can use same approach and provide twin in_guc_mode(e) and then use it as a base for improved uses_guc_submission(i915) if still needed. All above will help us review current usages of USES_GUC_SUBMISSION macro as it looks it's meaning is no longer immutable as it used to be. Should we have a dedicated flag for the submission mode? Potential problem with dedicated flag is that someone needs to maintain it. Use of set_default_submission vfunc, assuming it is different for different submission mode (and this is true today), gives us at least some confidence that we report correct submission mode as taken directly from engine setup. Michal ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/execlists: Use vfunc to check engine submission mode
Hi Michał,
On Monday, October 28, 2019 1:57:03 PM CET Michal Wajdeczko wrote:
> While processing CSB there is no need to look at GuC submission
> settings, just check if engine is configured for execlists mode.
>
> While today GuC submission is disabled it's settings are still
> based on modparam values that might not correctly reflect actual
> submission status in case of any fallback. Until that is fully
> fixed, use alternate method to confirm that engine really runs in
> execlists mode by comparing set_default_submission vfunc.
> Signed-off-by: Michal Wajdeczko
> Cc: Chris Wilson
> Cc: Janusz Krzysztofik
> ---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 8 +++-
> drivers/gpu/drm/i915/gt/intel_lrc.h | 2 ++
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/
intel_lrc.c
> index 16340740139d..c0d564b28beb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2022,7 +2022,7 @@ static void process_csb(struct intel_engine_cs
*engine)
>*/
> GEM_BUG_ON(!tasklet_is_locked(>tasklet) &&
> !reset_in_progress(execlists));
> - GEM_BUG_ON(USES_GUC_SUBMISSION(engine->i915));
> + GEM_BUG_ON(!intel_engine_in_execlists_submission_mode(engine));
>
> /*
>* Note that csb_write, csb_status may be either in HWSP or mmio.
> @@ -4711,6 +4711,12 @@ void intel_lr_context_reset(struct intel_engine_cs
*engine,
> __execlists_update_reg_state(ce, engine);
> }
>
> +bool intel_engine_in_execlists_submission_mode(struct intel_engine_cs
*engine)
> +{
> + return engine->set_default_submission ==
> +intel_execlists_set_default_submission;
> +}
> +
> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> #include "selftest_lrc.c"
> #endif
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h b/drivers/gpu/drm/i915/gt/
intel_lrc.h
> index 99dc576a4e25..23dde9083349 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.h
> @@ -145,4 +145,6 @@ struct intel_engine_cs *
> intel_virtual_engine_get_sibling(struct intel_engine_cs *engine,
>unsigned int sibling);
>
> +bool intel_engine_in_execlists_submission_mode(struct intel_engine_cs
*engine);
LGTM.
NIT: I'm wondering if the function should be made static if there is only one
local user.
Reviewed-by: Janusz Krzysztofik
Thanks,
Janusz
> +
> #endif /* _INTEL_LRC_H_ */
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/execlists: Use vfunc to check engine submission mode
Quoting Michal Wajdeczko (2019-10-28 12:57:03) > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h > b/drivers/gpu/drm/i915/gt/intel_lrc.h > index 99dc576a4e25..23dde9083349 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.h > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.h > @@ -145,4 +145,6 @@ struct intel_engine_cs * > intel_virtual_engine_get_sibling(struct intel_engine_cs *engine, > unsigned int sibling); > > +bool intel_engine_in_execlists_submission_mode(struct intel_engine_cs > *engine); Planning to use it outside? Should we have a dedicated flag for the submission mode? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/execlists: Use vfunc to check engine submission mode
While processing CSB there is no need to look at GuC submission
settings, just check if engine is configured for execlists mode.
While today GuC submission is disabled it's settings are still
based on modparam values that might not correctly reflect actual
submission status in case of any fallback. Until that is fully
fixed, use alternate method to confirm that engine really runs in
execlists mode by comparing set_default_submission vfunc.
Signed-off-by: Michal Wajdeczko
Cc: Chris Wilson
Cc: Janusz Krzysztofik
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 8 +++-
drivers/gpu/drm/i915/gt/intel_lrc.h | 2 ++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 16340740139d..c0d564b28beb 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2022,7 +2022,7 @@ static void process_csb(struct intel_engine_cs *engine)
*/
GEM_BUG_ON(!tasklet_is_locked(>tasklet) &&
!reset_in_progress(execlists));
- GEM_BUG_ON(USES_GUC_SUBMISSION(engine->i915));
+ GEM_BUG_ON(!intel_engine_in_execlists_submission_mode(engine));
/*
* Note that csb_write, csb_status may be either in HWSP or mmio.
@@ -4711,6 +4711,12 @@ void intel_lr_context_reset(struct intel_engine_cs
*engine,
__execlists_update_reg_state(ce, engine);
}
+bool intel_engine_in_execlists_submission_mode(struct intel_engine_cs *engine)
+{
+ return engine->set_default_submission ==
+ intel_execlists_set_default_submission;
+}
+
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
#include "selftest_lrc.c"
#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h
b/drivers/gpu/drm/i915/gt/intel_lrc.h
index 99dc576a4e25..23dde9083349 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.h
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.h
@@ -145,4 +145,6 @@ struct intel_engine_cs *
intel_virtual_engine_get_sibling(struct intel_engine_cs *engine,
unsigned int sibling);
+bool intel_engine_in_execlists_submission_mode(struct intel_engine_cs *engine);
+
#endif /* _INTEL_LRC_H_ */
--
2.19.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
