On 05/02/16 22:56, Rodrigo Vivi wrote:
On Fri, Feb 5, 2016 at 11:31 AM, Dave Gordon <david.s.gordon@xxxxxxxxx> wrote:
From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Currently emit-request starts writing to the ring and reserves space
for a workaround to be emitted later whilst submitting the request. It
is easier to read if the caller only allocates sufficient space for
its access (then the reader can quickly verify that the ring begin
allocates the exact space for the number of dwords emitted) and closes
the access to the ring. During submit, if we need to add the workaround,
we can reacquire ring access, in the assurance that we reserved space
for ourselves when beginning the request.
v3:
Reinstated #define for WA_TAIL_DWORDS so we could accommodate GPUs
that required different amounts of padding, generalised NOOP fill
[Rodrigo Vivi], added W/A space to reserved amount and updated
code comments [Dave Gordon],
Originally-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Rewritten-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Further-tweaked-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Dave Gordon <david.s.gordon@xxxxxxxxx>
Cc: Ben Widawsky <benjamin.widawsky@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_lrc.c | 80 ++++++++++++++++++++++++----------------
1 file changed, 49 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3a03646..8b278f1 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -225,6 +225,13 @@ enum {
#define GEN8_CTX_ID_SHIFT 32
#define CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT 0x17
+/*
+ * Reserve space for 2 NOOPs at the end of each request,
+ * to be used as a workaround for not being allowed to
+ * do lite restore with HEAD==TAIL (WaIdleLiteRestore).
+ */
+#define WA_TAIL_DWORDS 2
This patch really organize things better, but I still don't like the
WA_TAIL_DWORDS hardcoded here instead in a place where I can switch
for different platofms.
1. it's a *workaround* for a hardware limitation. Let's hope it's not
even necessary on future chips!
2. it's a really small overhead, so why not just define it as the max
over all supported platforms?
3. it's a macro, that means it's not *hardcoded*; if there's ever a
platform where we actually need this fix and it's not the same number,
we can just redefine this as a function call. Would you prefer it to be
a function-type macro:
#define WA_TAIL_DWORDS(dev) (2) /* doesn't currently depend on dev */
so that it could more easily be converted to a platform-specific value
in future?
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
