[Intel-gfx] [PATCH v2 05/12] drm/i915/pvc: Remove additional 3D flags from PIPE_CONTROL
Lucas De Marchi
lucas.demarchi at intel.com
Fri May 6 17:23:41 UTC 2022
On Thu, May 05, 2022 at 02:38:05PM -0700, Matt Roper wrote:
>From: Stuart Summers <stuart.summers at intel.com>
>
>Although we already strip 3D-specific flags from PIPE_CONTROL
>instructions when submitting to a compute engine, there are some
>additional flags that need to be removed when the platform as a whole
>lacks a 3D pipeline. Add those restrictions here.
>
>Bspec: 47112
>Signed-off-by: Stuart Summers <stuart.summers at intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>---
> drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 18 ++++++++++++------
> drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 12 ++++++++++--
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_pci.c | 3 ++-
> drivers/gpu/drm/i915/intel_device_info.h | 3 ++-
> 5 files changed, 28 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>index 3e13960615bd..11c72792573d 100644
>--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
>@@ -197,8 +197,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
>
> flags |= PIPE_CONTROL_CS_STALL;
>
>- if (engine->class == COMPUTE_CLASS)
>- flags &= ~PIPE_CONTROL_3D_FLAGS;
>+ if (LACKS_3D_PIPELINE(engine->i915))
>+ flags &= ~PIPE_CONTROL_3D_ARCH_FLAGS;
>+ else if (engine->class == COMPUTE_CLASS)
>+ flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
>
> cs = intel_ring_begin(rq, 6);
> if (IS_ERR(cs))
>@@ -227,8 +229,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
>
> flags |= PIPE_CONTROL_CS_STALL;
>
>- if (engine->class == COMPUTE_CLASS)
>- flags &= ~PIPE_CONTROL_3D_FLAGS;
>+ if (LACKS_3D_PIPELINE(engine->i915))
>+ flags &= ~PIPE_CONTROL_3D_ARCH_FLAGS;
>+ else if (engine->class == COMPUTE_CLASS)
>+ flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
>
> if (!HAS_FLAT_CCS(rq->engine->i915))
> count = 8 + 4;
>@@ -716,8 +720,10 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
> /* Wa_1409600907 */
> flags |= PIPE_CONTROL_DEPTH_STALL;
>
>- if (rq->engine->class == COMPUTE_CLASS)
>- flags &= ~PIPE_CONTROL_3D_FLAGS;
>+ if (LACKS_3D_PIPELINE(rq->engine->i915))
>+ flags &= ~PIPE_CONTROL_3D_ARCH_FLAGS;
>+ else if (rq->engine->class == COMPUTE_CLASS)
>+ flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
>
> cs = gen12_emit_ggtt_write_rcs(cs,
> rq->fence.seqno,
>diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
>index 556bca3be804..900755f4b787 100644
>--- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
>+++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
>@@ -288,8 +288,8 @@
> #define PIPE_CONTROL_DEPTH_CACHE_FLUSH (1<<0)
> #define PIPE_CONTROL_GLOBAL_GTT (1<<2) /* in addr dword */
>
>-/* 3D-related flags can't be set on compute engine */
>-#define PIPE_CONTROL_3D_FLAGS (\
>+/* 3D-related flags that can't be set on _engines_ that lack a 3D pipeline */
>+#define PIPE_CONTROL_3D_ENGINE_FLAGS (\
> PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH | \
> PIPE_CONTROL_DEPTH_CACHE_FLUSH | \
> PIPE_CONTROL_TILE_CACHE_FLUSH | \
>@@ -300,6 +300,14 @@
> PIPE_CONTROL_VF_CACHE_INVALIDATE | \
> PIPE_CONTROL_GLOBAL_SNAPSHOT_RESET)
>
>+/* 3D-related flags that can't be set on _platforms_ that lack a 3D pipeline */
>+#define PIPE_CONTROL_3D_ARCH_FLAGS ( \
names and comments here I think are confusing. In the code we do:
if (LACKS_3D_PIPELINE(engine->i915))
flags &= ~PIPE_CONTROL_3D_ARCH_FLAGS;
else if (engine->class == COMPUTE_CLASS)
flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
coments give the _engines_ that lack a 3D pipeline doens't seem accurate
as bcs, vcs, vecs also lack a 3d pipeline. Maybe be explicit about the
flags being set on compute engine: PIPE_CONTROL_COMPUTE_ENGINE_FLAGS
And LACKS_3D_PIPELINE... may be better to invert the condition to follow
the other macros: HAS_3D_PIPELINE.
>+ PIPE_CONTROL_3D_ENGINE_FLAGS | \
>+ PIPE_CONTROL_INDIRECT_STATE_DISABLE | \
>+ PIPE_CONTROL_FLUSH_ENABLE | \
>+ PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE | \
>+ PIPE_CONTROL_DC_FLUSH_ENABLE)
>+
> #define MI_MATH(x) MI_INSTR(0x1a, (x) - 1)
> #define MI_MATH_INSTR(opcode, op1, op2) ((opcode) << 20 | (op1) << 10 | (op2))
> /* Opcodes for MI_MATH_INSTR */
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index b389674b5210..1e153cefc92e 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -1403,6 +1403,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>
> #define HAS_MBUS_JOINING(i915) (IS_ALDERLAKE_P(i915))
>
>+#define LACKS_3D_PIPELINE(i915) (INTEL_INFO(i915)->lacks_3d_pipeline)
>+
> /* i915_gem.c */
> void i915_gem_init_early(struct drm_i915_private *dev_priv);
> void i915_gem_cleanup_early(struct drm_i915_private *dev_priv);
>diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>index 07722cdf63ac..14e0e8225324 100644
>--- a/drivers/gpu/drm/i915/i915_pci.c
>+++ b/drivers/gpu/drm/i915/i915_pci.c
>@@ -1077,7 +1077,8 @@ static const struct intel_device_info ats_m_info = {
> #define XE_HPC_FEATURES \
> XE_HP_FEATURES, \
> .dma_mask_size = 52, \
>- .has_l3_ccs_read = 1
>+ .has_l3_ccs_read = 1, \
>+ .lacks_3d_pipeline = 1
isn't the absence of RCS sufficient so this could be done only in the
macro rather than adding a flag here?
Lucas De Marchi
>
> __maybe_unused
> static const struct intel_device_info pvc_info = {
>diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
>index 09e33296157a..972084676984 100644
>--- a/drivers/gpu/drm/i915/intel_device_info.h
>+++ b/drivers/gpu/drm/i915/intel_device_info.h
>@@ -165,7 +165,8 @@ enum intel_ppgtt_type {
> func(has_snoop); \
> func(has_coherent_ggtt); \
> func(unfenced_needs_alignment); \
>- func(hws_needs_physical);
>+ func(hws_needs_physical); \
>+ func(lacks_3d_pipeline);
>
> #define DEV_INFO_DISPLAY_FOR_EACH_FLAG(func) \
> /* Keep in alphabetical order */ \
>--
>2.35.1
>
More information about the Intel-gfx
mailing list