Re: [PATCH] drm/i915: Update WaFlushCoherentL3CacheLinesAtContextSwitch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/07/2015 12:52, Dave Gordon wrote:
On 03/07/15 16:42, Chris Wilson wrote:
On Fri, Jul 03, 2015 at 02:27:31PM +0100, Arun Siluvery wrote:
In this WA we need to set GEN8_L3SQCREG4[21:21] and reset it after PIPE_CONTROL
instruction but there is a slight complication as this is applied in WA batch
where the values are only initialized once.
Dave identified an issue with the current implementation where the register value
is read once at the beginning and it is reused; this patch corrects this by saving
the register value to memory, update register with the bit of our interest and
restore it back with original value.

This implementation uses MI_LOAD_REGISTER_MEM which is currently only used
by command parser and was using a default length of 0. This is now updated
with correct length and moved to appropriate place.

Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Dave Gordon <david.s.gordon@xxxxxxxxx>
Signed-off-by: Arun Siluvery <arun.siluvery@xxxxxxxxxxxxxxx>
---
   drivers/gpu/drm/i915/i915_cmd_parser.c |  6 +--
   drivers/gpu/drm/i915/i915_reg.h        |  3 +-
   drivers/gpu/drm/i915/intel_lrc.c       | 72 +++++++++++++++++++++++++---------
   3 files changed, 58 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 306d9e4..430571b 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -131,7 +131,7 @@ static const struct drm_i915_cmd_descriptor common_cmds[] = {
   			.mask = MI_GLOBAL_GTT,
   			.expected = 0,
   	      }},						       ),
-	CMD(  MI_LOAD_REGISTER_MEM,             SMI,   !F,  0xFF,   W | B,
+	CMD(  MI_LOAD_REGISTER_MEM(1),             SMI,   !F,  0xFF,   W | B,
   	      .reg = { .offset = 1, .mask = 0x007FFFFC },
   	      .bits = {{
   			.offset = 0,
@@ -1021,7 +1021,7 @@ static bool check_cmd(const struct intel_engine_cs *ring,
   			 * only MI_LOAD_REGISTER_IMM commands.
   			 */
   			if (reg_addr == OACONTROL) {
-				if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
+				if (desc->cmd.value == MI_LOAD_REGISTER_MEM(1)) {

I had a double take here, but it all comes out in the wash. For one
moment, I thought the cmd matching had changed, but that has the length
masked out.

Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxx>

Who will start to complain about all the extra frequent register writes,
probably into common power wells....
-Chris

Hmm ... that is quite confusing, especially as the actual opcode in the
instruction stream will be MI_LOAD_REGISTER_MEM(2) on GEN8+. It might
true, but cmd parser is only upto GEN7.

regards
Arun

almost be better to use MI_LOAD_REGISTER_MEM(0) to emphasise that the
length field is a wildcard and not something that will be matched exactly.

.Dave.


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux