[Intel-gfx] [PATCH] drm/i915: Inline engine->init_context into its caller
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Mon Jul 29 14:49:29 UTC 2019
On 29/07/2019 12:37, Chris Wilson wrote:
> We only use the init_context vfunc once while recording the default
> context state, and we use the same sequence in each backend (eliding
> steps that do not apply). Remove the vfunc for simplicity and
> de-duplication.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 -
> drivers/gpu/drm/i915/gt/intel_lrc.c | 21 --------------------
> drivers/gpu/drm/i915/gt/intel_mocs.c | 5 ++++-
> drivers/gpu/drm/i915/gt/intel_mocs.h | 3 ++-
> drivers/gpu/drm/i915/gt/intel_renderstate.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 18 -----------------
> drivers/gpu/drm/i915/i915_gem.c | 21 +++++++++++++++++---
> 7 files changed, 25 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 8be63019d707..da61dd329210 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -383,7 +383,6 @@ struct intel_engine_cs {
> const struct intel_context_ops *cops;
>
> int (*request_alloc)(struct i915_request *rq);
> - int (*init_context)(struct i915_request *rq);
>
> int (*emit_flush)(struct i915_request *request, u32 mode);
> #define EMIT_INVALIDATE BIT(0)
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 884dfc1cb033..4d7c4d0dbf75 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -141,7 +141,6 @@
> #include "intel_gt.h"
> #include "intel_lrc_reg.h"
> #include "intel_mocs.h"
> -#include "intel_renderstate.h"
> #include "intel_reset.h"
> #include "intel_workarounds.h"
>
> @@ -2727,25 +2726,6 @@ static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
> return gen8_emit_wa_tail(request, cs);
> }
>
> -static int gen8_init_rcs_context(struct i915_request *rq)
> -{
> - int ret;
> -
> - ret = intel_engine_emit_ctx_wa(rq);
> - if (ret)
> - return ret;
> -
> - ret = intel_rcs_context_init_mocs(rq);
> - /*
> - * Failing to program the MOCS is non-fatal.The system will not
> - * run at peak performance. So generate an error and carry on.
> - */
> - if (ret)
> - DRM_ERROR("MOCS failed to program: expect performance issues.\n");
> -
> - return intel_renderstate_emit(rq);
> -}
> -
> static void execlists_park(struct intel_engine_cs *engine)
> {
> del_timer_sync(&engine->execlists.timer);
> @@ -2853,7 +2833,6 @@ int intel_execlists_submission_setup(struct intel_engine_cs *engine)
> logical_ring_default_irqs(engine);
>
> if (engine->class == RENDER_CLASS) {
> - engine->init_context = gen8_init_rcs_context;
> engine->emit_flush = gen8_emit_flush_render;
> engine->emit_fini_breadcrumb = gen8_emit_fini_breadcrumb_rcs;
> }
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index 290a5e9b90b9..e082b25d2db1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -568,11 +568,14 @@ void intel_mocs_init_l3cc_table(struct intel_gt *gt)
> *
> * Return: 0 on success, otherwise the error status.
> */
> -int intel_rcs_context_init_mocs(struct i915_request *rq)
> +int intel_mocs_emit(struct i915_request *rq)
> {
> struct drm_i915_mocs_table t;
> int ret;
>
> + if (rq->engine->class != RENDER_CLASS)
> + return 0;
> +
> if (get_mocs_settings(rq->engine->gt, &t)) {
> /* Program the RCS control registers */
> ret = emit_mocs_control_table(rq, &t);
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.h b/drivers/gpu/drm/i915/gt/intel_mocs.h
> index 8b9813e6f9ac..a334db2d6d6b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.h
> @@ -54,8 +54,9 @@ struct i915_request;
> struct intel_engine_cs;
> struct intel_gt;
>
> -int intel_rcs_context_init_mocs(struct i915_request *rq);
> void intel_mocs_init_l3cc_table(struct intel_gt *gt);
> void intel_mocs_init_engine(struct intel_engine_cs *engine);
>
> +int intel_mocs_emit(struct i915_request *rq);
> +
> #endif
> diff --git a/drivers/gpu/drm/i915/gt/intel_renderstate.c b/drivers/gpu/drm/i915/gt/intel_renderstate.c
> index 06a8dc40b19f..be37d4501c67 100644
> --- a/drivers/gpu/drm/i915/gt/intel_renderstate.c
> +++ b/drivers/gpu/drm/i915/gt/intel_renderstate.c
> @@ -41,7 +41,7 @@ struct intel_renderstate {
> static const struct intel_renderstate_rodata *
> render_state_get_rodata(const struct intel_engine_cs *engine)
> {
> - if (engine->id != RCS0)
> + if (engine->class != RENDER_CLASS)
> return NULL;
>
> switch (INTEL_GEN(engine->i915)) {
> diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> index 1de19dac4a14..5c7f2fdc5ec3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> @@ -37,7 +37,6 @@
> #include "i915_trace.h"
> #include "intel_context.h"
> #include "intel_gt.h"
> -#include "intel_renderstate.h"
> #include "intel_reset.h"
> #include "intel_workarounds.h"
>
> @@ -849,21 +848,6 @@ static void reset_finish(struct intel_engine_cs *engine)
> {
> }
>
> -static int intel_rcs_ctx_init(struct i915_request *rq)
> -{
> - int ret;
> -
> - ret = intel_engine_emit_ctx_wa(rq);
> - if (ret != 0)
> - return ret;
> -
> - ret = intel_renderstate_emit(rq);
> - if (ret)
> - return ret;
> -
> - return 0;
> -}
> -
> static int rcs_resume(struct intel_engine_cs *engine)
> {
> struct drm_i915_private *dev_priv = engine->i915;
> @@ -2227,11 +2211,9 @@ static void setup_rcs(struct intel_engine_cs *engine)
> engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
>
> if (INTEL_GEN(i915) >= 7) {
> - engine->init_context = intel_rcs_ctx_init;
> engine->emit_flush = gen7_render_ring_flush;
> engine->emit_fini_breadcrumb = gen7_rcs_emit_breadcrumb;
> } else if (IS_GEN(i915, 6)) {
> - engine->init_context = intel_rcs_ctx_init;
> engine->emit_flush = gen6_render_ring_flush;
> engine->emit_fini_breadcrumb = gen6_rcs_emit_breadcrumb;
> } else if (IS_GEN(i915, 5)) {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 01dd0d1d9bf6..65863e955f40 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -50,6 +50,7 @@
> #include "gt/intel_gt_pm.h"
> #include "gt/intel_mocs.h"
> #include "gt/intel_reset.h"
> +#include "gt/intel_renderstate.h"
> #include "gt/intel_workarounds.h"
>
> #include "i915_drv.h"
> @@ -1294,10 +1295,24 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
> goto err_active;
> }
>
> - err = 0;
> - if (rq->engine->init_context)
> - err = rq->engine->init_context(rq);
> + err = intel_engine_emit_ctx_wa(rq);
> + if (err)
> + goto err_rq;
> +
> + /*
> + * Failing to program the MOCS is non-fatal.The system will not
> + * run at peak performance. So warn the user and carry on.
> + */
> + err = intel_mocs_emit(rq);
I think platforms without MOCS will not be happy unless you also change
the MOCS code.
> + if (err)
> + dev_notice(i915->drm.dev,
> + "Failed to program MOCS registers; expect performance issues.\n");
> +
> + err = intel_renderstate_emit(rq);
> + if (err)
> + goto err_rq;
Would it be prettier to move all the tree into
intel_engine_cs.c/intel_engine_context_init() or something? Here it
seems a bit too much into details.
Regards,
Tvrtko
>
> +err_rq:
> i915_request_add(rq);
> if (err)
> goto err_active;
>
More information about the Intel-gfx
mailing list