On 05/12/14 12:51, Ville Syrjälä wrote: > On Fri, Nov 14, 2014 at 06:16:56PM +0200, [email protected] wrote: >> From: Ville Syrjälä <[email protected]> >> >> MI_STORE_DWORD_IMM length has been the same ever since gen4. Rename >> the define to avoid potential confusion if someone tries to use this >> on pre-gen8. >> >> Also correct the comment on MI_MEM_VIRTUAL bit. It's present on 945,g33 >> and 965 only. >> >> Cc: Oscar Mateo <[email protected]> >> Signed-off-by: Ville Syrjälä <[email protected]> > > ping > >> --- >> drivers/gpu/drm/i915/i915_reg.h | 4 ++-- >> drivers/gpu/drm/i915/intel_lrc.c | 2 +- >> 2 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index 3de58ac..5228493 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -280,8 +280,8 @@ >> #define MI_SEMAPHORE_POLL (1<<15) >> #define MI_SEMAPHORE_SAD_GTE_SDD (1<<12) >> #define MI_STORE_DWORD_IMM MI_INSTR(0x20, 1) >> -#define MI_STORE_DWORD_IMM_GEN8 MI_INSTR(0x20, 2) >> -#define MI_MEM_VIRTUAL (1 << 22) /* 965+ only */ >> +#define MI_STORE_DWORD_IMM_GEN4 MI_INSTR(0x20, 2) >> +#define MI_MEM_VIRTUAL (1 << 22) /* 945,g33,965 */ >> #define MI_STORE_DWORD_INDEX MI_INSTR(0x21, 1) >> #define MI_STORE_DWORD_INDEX_SHIFT 2 >> /* Official intel docs are somewhat sloppy concerning MI_LOAD_REGISTER_IMM: >> diff --git a/drivers/gpu/drm/i915/intel_lrc.c >> b/drivers/gpu/drm/i915/intel_lrc.c >> index 6025ac7..649d9ba 100644 >> --- a/drivers/gpu/drm/i915/intel_lrc.c >> +++ b/drivers/gpu/drm/i915/intel_lrc.c >> @@ -1188,7 +1188,7 @@ static int gen8_emit_request(struct intel_ringbuffer >> *ringbuf) >> if (ret) >> return ret; >> >> - cmd = MI_STORE_DWORD_IMM_GEN8; >> + cmd = MI_STORE_DWORD_IMM_GEN4; >> cmd |= MI_GLOBAL_GTT; >> >> intel_logical_ring_emit(ringbuf, cmd); >> -- >> 2.0.4
The GEN8->GEN4 change looks sensible to me. But I was puzzled over the definition of MI_STORE_DWORD_IMM -- presumably this apparently more generic definition is actually a pre-GEN4 legacy from the days when the instruction had only 3 words? Perhaps a comment here from somebody who knows -- or even something like #define MI_STORE_DWORD_IMM_LEGACY MI_INSTR(0x20, 1) /* pre-GEN4 */ #define MI_STORE_DWORD_IMM_GEN4 MI_INSTR(0x20, 2) /* GEN4+ */ #define MI_STORE_DWORD_IMM MI_STORE_DWORD_IMM_LEGACY /* for source backward compatibility */ so it's clearer that (for now) the generic definition is actually the legacy one. The above bikeshedding notwithstanding, it gets my R-b anyway: Reviewed-by: Dave Gordon <[email protected]> _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
