On 10/17/2023 2:23 AM, Belgaumkar, Vinay wrote:
On 10/16/2023 4:24 PM, John Harrison wrote:
On 10/16/2023 15:55, Vinay Belgaumkar wrote:
This bit does not cause an explicit L3 flush. We already use
At all? Or only on newer hardware? And as a genuine spec change or as
a bug / workaround?
If the hardware has re-purposed the bit then it is probably worth at
least adding a comment to the bit definition to say that it is only
valid up to IP version 12.70.
At this point, this is a bug on MTL since this bit is not related to
L3 flushes as per spec. Regarding older platforms, still checking the
reason why this was added (i.e if it fixed something and will regress
if removed). If not, we can extend the change for others as well in a
separate patch. On older platforms, this bit seems to cause an
implicit flush at best.
PIPE_CONTROL_DC_FLUSH_ENABLE for that purpose.
Cc: Nirmoy Das <nirmoy.das@xxxxxxxxx>
Cc: Mikka Kuoppala <mika.kuoppala@xxxxxxxxx>
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx>
---
drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index ba4c2422b340..abbc02f3e66e 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -247,6 +247,7 @@ static int mtl_dummy_pipe_control(struct
i915_request *rq)
int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
{
struct intel_engine_cs *engine = rq->engine;
+ struct intel_gt *gt = rq->engine->gt;
/*
* On Aux CCS platforms the invalidation of the Aux
@@ -278,7 +279,8 @@ int gen12_emit_flush_rcs(struct i915_request
*rq, u32 mode)
* deals with Protected Memory which is not needed for
* AUX CCS invalidation and lead to unwanted side effects.
*/
- if (mode & EMIT_FLUSH)
+ if ((mode & EMIT_FLUSH) &&
+ !(IS_GFX_GT_IP_RANGE(gt, IP_VER(12, 70), IP_VER(12, 71))))
Why stop at 12.71? Is the meaning only changed for 12.70 and the
old/correct version will be restored in later hardware?
Was trying to keep this limited to MTL for now until the above
statements are verified.
If we don't need it for MTL, we are most likely not going to need it
after MTL too. It should be fine if you make it >= MTL until
we get more clarity about lower platforms.
With that, this is
Reviewed-by: Nirmoy Das <nirmoy.das@xxxxxxxxx>
Thanks,
Vinay.
John.
bit_group_1 |= PIPE_CONTROL_FLUSH_L3;
bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH;
@@ -812,12 +814,14 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct
i915_request *rq, u32 *cs)
u32 flags = (PIPE_CONTROL_CS_STALL |
PIPE_CONTROL_TLB_INVALIDATE |
PIPE_CONTROL_TILE_CACHE_FLUSH |
- PIPE_CONTROL_FLUSH_L3 |
PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
PIPE_CONTROL_DEPTH_CACHE_FLUSH |
PIPE_CONTROL_DC_FLUSH_ENABLE |
PIPE_CONTROL_FLUSH_ENABLE);
+ if (!(IS_GFX_GT_IP_RANGE(gt, IP_VER(12, 70), IP_VER(12, 71))))
+ flags |= PIPE_CONTROL_FLUSH_L3;
+
/* Wa_14016712196 */
if (IS_GFX_GT_IP_RANGE(gt, IP_VER(12, 70), IP_VER(12, 71)) ||
IS_DG2(i915))
/* dummy PIPE_CONTROL + depth flush */
