[Intel-gfx] [PATCH] drm/i915: Inline engine->init_context into its caller
Chris Wilson
chris at chris-wilson.co.uk
Tue Jul 30 09:34:17 UTC 2019
Quoting Tvrtko Ursulin (2019-07-29 15:49:29)
>
> 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.
Older platforms fail gracefully (silently with err=0 as get_mocs_settings() returns false).
New platforms without MOCS descriptions issue a warning (as currently).
> > + 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.
Indeed, it's not staying here. But that is tied up in other in-flight
patches.
-Chris
More information about the Intel-gfx
mailing list