| Message ID | 20230531143214.1650701-1-tejas.upadhyay@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V3] drm/i915/gt: Add workaround 14016712196 | expand |
Hi Tejas, On Wed, May 31, 2023 at 08:02:14PM +0530, Tejas Upadhyay wrote: > Wa_14016712196 implementation for mtl > > Bspec: 72197 > > V3: > - Wrap dummy pipe control stuff in API - Andi > V2: > - Fix kernel test robot warnings > > Closes: https://lore.kernel.org/oe-kbuild-all/202305121525.3EWdGoBY-lkp@intel.com/ > Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com> > --- > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 39 ++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > index e1c76e5bfa82..206947f1fc7c 100644 > --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > @@ -177,6 +177,30 @@ u32 *gen12_emit_aux_table_inv(struct intel_gt *gt, u32 *cs, const i915_reg_t inv > return cs; > } > > +static u32 *mtl_dummy_pipe_control(struct i915_request *rq) > +{ > + u32 *cs; > + > + /* Wa_14016712196 */ > + if (IS_MTL_GRAPHICS_STEP(rq->engine->i915, M, STEP_A0, STEP_B0) || > + IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0)) { > + int ret; > + > + /* dummy PIPE_CONTROL + depth flush */ > + cs = intel_ring_begin(rq, 6); > + ret = IS_ERR(cs); > + if (ret) > + return ERR_PTR(ret); > + cs = gen12_emit_pipe_control(cs, > + 0, > + PIPE_CONTROL_DEPTH_CACHE_FLUSH, > + LRC_PPHWSP_SCRATCH_ADDR); > + intel_ring_advance(rq, cs); > + } > + > + return cs; mmmhhh... the value of cs at the end is not used, unless it's an error. Isn't it better to just have this function as a "static int" that returns '0' or -errno? This might also confuse some static analyzers, I guess. Andi > +} > + > int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) > { > struct intel_engine_cs *engine = rq->engine; > @@ -185,6 +209,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) > u32 flags = 0; > u32 *cs; > > + cs = mtl_dummy_pipe_control(rq); > + if (IS_ERR(cs)) > + return PTR_ERR(cs); > + > flags |= PIPE_CONTROL_TILE_CACHE_FLUSH; > flags |= PIPE_CONTROL_FLUSH_L3; > flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; > @@ -218,6 +246,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) > u32 flags = 0; > u32 *cs, count; > > + cs = mtl_dummy_pipe_control(rq); > + if (IS_ERR(cs)) > + return PTR_ERR(cs); > + > flags |= PIPE_CONTROL_COMMAND_CACHE_INVALIDATE; > flags |= PIPE_CONTROL_TLB_INVALIDATE; > flags |= PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE; > @@ -733,6 +765,13 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs) > PIPE_CONTROL_DC_FLUSH_ENABLE | > PIPE_CONTROL_FLUSH_ENABLE); > > + /* Wa_14016712196 */ > + if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > + IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > + /* dummy PIPE_CONTROL + depth flush */ > + cs = gen12_emit_pipe_control(cs, 0, > + PIPE_CONTROL_DEPTH_CACHE_FLUSH, 0); > + > if (GRAPHICS_VER(i915) == 12 && GRAPHICS_VER_FULL(i915) < IP_VER(12, 50)) > /* Wa_1409600907 */ > flags |= PIPE_CONTROL_DEPTH_STALL; > -- > 2.25.1
On Wed, May 31, 2023 at 08:02:14PM +0530, Tejas Upadhyay wrote: > Wa_14016712196 implementation for mtl Since this isn't one of the trivial "write XXX to register YYY" workarounds, you might want to give a brief description of what we're being directed to do. E.g., "Wa_14016712196 asks us to submit a "dummy" PIPE_CONTROL when XYZ" That way it will be more obvious to reviewers if you truly got all the spots in the driver (e.g., it makes it easier for someone to point out mistakes like "XYZ is also true in function foo() too"). Matt > > Bspec: 72197 > > V3: > - Wrap dummy pipe control stuff in API - Andi > V2: > - Fix kernel test robot warnings > > Closes: https://lore.kernel.org/oe-kbuild-all/202305121525.3EWdGoBY-lkp@intel.com/ > Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com> > --- > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 39 ++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > index e1c76e5bfa82..206947f1fc7c 100644 > --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c > @@ -177,6 +177,30 @@ u32 *gen12_emit_aux_table_inv(struct intel_gt *gt, u32 *cs, const i915_reg_t inv > return cs; > } > > +static u32 *mtl_dummy_pipe_control(struct i915_request *rq) > +{ > + u32 *cs; > + > + /* Wa_14016712196 */ > + if (IS_MTL_GRAPHICS_STEP(rq->engine->i915, M, STEP_A0, STEP_B0) || > + IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0)) { > + int ret; > + > + /* dummy PIPE_CONTROL + depth flush */ > + cs = intel_ring_begin(rq, 6); > + ret = IS_ERR(cs); > + if (ret) > + return ERR_PTR(ret); > + cs = gen12_emit_pipe_control(cs, > + 0, > + PIPE_CONTROL_DEPTH_CACHE_FLUSH, > + LRC_PPHWSP_SCRATCH_ADDR); > + intel_ring_advance(rq, cs); > + } > + > + return cs; > +} > + > int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) > { > struct intel_engine_cs *engine = rq->engine; > @@ -185,6 +209,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) > u32 flags = 0; > u32 *cs; > > + cs = mtl_dummy_pipe_control(rq); > + if (IS_ERR(cs)) > + return PTR_ERR(cs); > + > flags |= PIPE_CONTROL_TILE_CACHE_FLUSH; > flags |= PIPE_CONTROL_FLUSH_L3; > flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; > @@ -218,6 +246,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) > u32 flags = 0; > u32 *cs, count; > > + cs = mtl_dummy_pipe_control(rq); > + if (IS_ERR(cs)) > + return PTR_ERR(cs); > + > flags |= PIPE_CONTROL_COMMAND_CACHE_INVALIDATE; > flags |= PIPE_CONTROL_TLB_INVALIDATE; > flags |= PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE; > @@ -733,6 +765,13 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs) > PIPE_CONTROL_DC_FLUSH_ENABLE | > PIPE_CONTROL_FLUSH_ENABLE); > > + /* Wa_14016712196 */ > + if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || > + IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) > + /* dummy PIPE_CONTROL + depth flush */ > + cs = gen12_emit_pipe_control(cs, 0, > + PIPE_CONTROL_DEPTH_CACHE_FLUSH, 0); > + > if (GRAPHICS_VER(i915) == 12 && GRAPHICS_VER_FULL(i915) < IP_VER(12, 50)) > /* Wa_1409600907 */ > flags |= PIPE_CONTROL_DEPTH_STALL; > -- > 2.25.1 >
diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index e1c76e5bfa82..206947f1fc7c 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -177,6 +177,30 @@ u32 *gen12_emit_aux_table_inv(struct intel_gt *gt, u32 *cs, const i915_reg_t inv return cs; } +static u32 *mtl_dummy_pipe_control(struct i915_request *rq) +{ + u32 *cs; + + /* Wa_14016712196 */ + if (IS_MTL_GRAPHICS_STEP(rq->engine->i915, M, STEP_A0, STEP_B0) || + IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0)) { + int ret; + + /* dummy PIPE_CONTROL + depth flush */ + cs = intel_ring_begin(rq, 6); + ret = IS_ERR(cs); + if (ret) + return ERR_PTR(ret); + cs = gen12_emit_pipe_control(cs, + 0, + PIPE_CONTROL_DEPTH_CACHE_FLUSH, + LRC_PPHWSP_SCRATCH_ADDR); + intel_ring_advance(rq, cs); + } + + return cs; +} + int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) { struct intel_engine_cs *engine = rq->engine; @@ -185,6 +209,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) u32 flags = 0; u32 *cs; + cs = mtl_dummy_pipe_control(rq); + if (IS_ERR(cs)) + return PTR_ERR(cs); + flags |= PIPE_CONTROL_TILE_CACHE_FLUSH; flags |= PIPE_CONTROL_FLUSH_L3; flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; @@ -218,6 +246,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) u32 flags = 0; u32 *cs, count; + cs = mtl_dummy_pipe_control(rq); + if (IS_ERR(cs)) + return PTR_ERR(cs); + flags |= PIPE_CONTROL_COMMAND_CACHE_INVALIDATE; flags |= PIPE_CONTROL_TLB_INVALIDATE; flags |= PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE; @@ -733,6 +765,13 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs) PIPE_CONTROL_DC_FLUSH_ENABLE | PIPE_CONTROL_FLUSH_ENABLE); + /* Wa_14016712196 */ + if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || + IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) + /* dummy PIPE_CONTROL + depth flush */ + cs = gen12_emit_pipe_control(cs, 0, + PIPE_CONTROL_DEPTH_CACHE_FLUSH, 0); + if (GRAPHICS_VER(i915) == 12 && GRAPHICS_VER_FULL(i915) < IP_VER(12, 50)) /* Wa_1409600907 */ flags |= PIPE_CONTROL_DEPTH_STALL;
Wa_14016712196 implementation for mtl Bspec: 72197 V3: - Wrap dummy pipe control stuff in API - Andi V2: - Fix kernel test robot warnings Closes: https://lore.kernel.org/oe-kbuild-all/202305121525.3EWdGoBY-lkp@intel.com/ Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com> --- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 39 ++++++++++++++++++++++++ 1 file changed, 39 insertions(+)