On Wed, Jul 19, 2023 at 01:07:25PM +0200, Andi Shyti wrote:
> Just a trivial refactoring for reducing the number of code
> duplicate. This will come at handy in the next commits.
>
> Signed-off-by: Andi Shyti <andi.sh...@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 44 +++++++++++++-----------
> 1 file changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index 7566c89d9def3..1b1dadacfbf42 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -177,23 +177,31 @@ u32 *gen12_emit_aux_table_inv(struct intel_gt *gt, u32
> *cs, const i915_reg_t inv
> return cs;
> }
>
> +static u32 *intel_emit_pipe_control_cs(struct i915_request *rq, u32
> bit_group_0,
This is another case where it gets confusing because this function name
sounds like it's something generic, but it actually only applies to a
small subset of platforms (gen12).
> + u32 bit_group_1, u32 offset)
> +{
> + u32 *cs;
> +
> + cs = intel_ring_begin(rq, 6);
> + if (IS_ERR(cs))
> + return cs;
We're not actually checking for this error at the callsites. Should we
be checking for it and propagating it farther up the call stack?
> +
> + cs = gen12_emit_pipe_control(cs, bit_group_0, bit_group_1,
> + LRC_PPHWSP_SCRATCH_ADDR);
> + intel_ring_advance(rq, cs);
> +
> + return cs;
This cursor never gets used for anything. We can probably just make
this function return an int error code.
Matt
> +}
> +
> static int mtl_dummy_pipe_control(struct i915_request *rq)
> {
> /* 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)) {
> - u32 *cs;
> -
> - /* dummy PIPE_CONTROL + depth flush */
> - cs = intel_ring_begin(rq, 6);
> - if (IS_ERR(cs))
> - return PTR_ERR(cs);
> - cs = gen12_emit_pipe_control(cs,
> - 0,
> - PIPE_CONTROL_DEPTH_CACHE_FLUSH,
> - LRC_PPHWSP_SCRATCH_ADDR);
> - intel_ring_advance(rq, cs);
> - }
> + IS_MTL_GRAPHICS_STEP(rq->engine->i915, P, STEP_A0, STEP_B0))
> + intel_emit_pipe_control_cs(rq,
> + 0,
> + PIPE_CONTROL_DEPTH_CACHE_FLUSH,
> + LRC_PPHWSP_SCRATCH_ADDR);
>
> return 0;
> }
> @@ -210,7 +218,6 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32
> mode)
> u32 bit_group_0 = 0;
> u32 bit_group_1 = 0;
> int err;
> - u32 *cs;
>
> err = mtl_dummy_pipe_control(rq);
> if (err)
> @@ -237,13 +244,8 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32
> mode)
> else if (engine->class == COMPUTE_CLASS)
> bit_group_1 &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
>
> - cs = intel_ring_begin(rq, 6);
> - if (IS_ERR(cs))
> - return PTR_ERR(cs);
> -
> - cs = gen12_emit_pipe_control(cs, bit_group_0, bit_group_1,
> - LRC_PPHWSP_SCRATCH_ADDR);
> - intel_ring_advance(rq, cs);
> + intel_emit_pipe_control_cs(rq, bit_group_0, bit_group_1,
> + LRC_PPHWSP_SCRATCH_ADDR);
> }
>
> if (mode & EMIT_INVALIDATE) {
> --
> 2.40.1
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
