* [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/ @ 2014-11-14 16:16 ville.syrjala 2014-11-15 4:43 ` [PATCH] drm/i915: shuang.he 2014-12-05 12:51 ` [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/ Ville Syrjälä 0 siblings, 2 replies; 5+ messages in thread From: ville.syrjala @ 2014-11-14 16:16 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> 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 <oscar.mateo@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: 2014-11-14 16:16 [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/ ville.syrjala @ 2014-11-15 4:43 ` shuang.he 2014-12-05 12:51 ` [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/ Ville Syrjälä 1 sibling, 0 replies; 5+ messages in thread From: shuang.he @ 2014-11-15 4:43 UTC (permalink / raw) To: shuang.he, intel-gfx, ville.syrjala Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) -------------------------------------Summary------------------------------------- Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate BYT: pass/total=290/291->290/291 PNV: pass/total=352/356->356/356 ILK: pass/total=371/372->371/372 IVB: pass/total=545/546->544/546 SNB: pass/total=424/425->424/425 HSW: pass/total=579/579->579/579 BDW: pass/total=434/435->434/435 -------------------------------------Detailed------------------------------------- test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)... PNV: Intel_gpu_tools, igt_gen3_mixed_blits, DMESG_WARN(1, M23) -> PASS(4, M25) PNV: Intel_gpu_tools, igt_gen3_render_mixed_blits, CRASH(1, M23) -> PASS(1, M25) PNV: Intel_gpu_tools, igt_gen3_render_tiledx_blits, CRASH(1, M23) -> PASS(1, M25) PNV: Intel_gpu_tools, igt_gen3_render_tiledy_blits, CRASH(1, M23) -> PASS(1, M25) IVB: Intel_gpu_tools, igt_kms_pipe_crc_basic_hang-read-crc-pipe-C, PASS(1, M4) -> TIMEOUT(1, M34)PASS(3, M34) _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/ 2014-11-14 16:16 [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/ ville.syrjala 2014-11-15 4:43 ` [PATCH] drm/i915: shuang.he @ 2014-12-05 12:51 ` Ville Syrjälä 2014-12-08 14:57 ` Dave Gordon 1 sibling, 1 reply; 5+ messages in thread From: Ville Syrjälä @ 2014-12-05 12:51 UTC (permalink / raw) To: intel-gfx On Fri, Nov 14, 2014 at 06:16:56PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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 <oscar.mateo@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> 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 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/ 2014-12-05 12:51 ` [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/ Ville Syrjälä @ 2014-12-08 14:57 ` Dave Gordon 2014-12-08 16:05 ` Ville Syrjälä 0 siblings, 1 reply; 5+ messages in thread From: Dave Gordon @ 2014-12-08 14:57 UTC (permalink / raw) To: Ville Syrjälä, intel-gfx On 05/12/14 12:51, Ville Syrjälä wrote: > On Fri, Nov 14, 2014 at 06:16:56PM +0200, ville.syrjala@linux.intel.com wrote: >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> 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 <oscar.mateo@intel.com> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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 <david.s.gordon@intel.com> _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/ 2014-12-08 14:57 ` Dave Gordon @ 2014-12-08 16:05 ` Ville Syrjälä 0 siblings, 0 replies; 5+ messages in thread From: Ville Syrjälä @ 2014-12-08 16:05 UTC (permalink / raw) To: Dave Gordon; +Cc: intel-gfx On Mon, Dec 08, 2014 at 02:57:31PM +0000, Dave Gordon wrote: > On 05/12/14 12:51, Ville Syrjälä wrote: > > On Fri, Nov 14, 2014 at 06:16:56PM +0200, ville.syrjala@linux.intel.com wrote: > >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> > >> 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 <oscar.mateo@intel.com> > >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > 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. We tend to name things based on the oldest platform supporting it, so I think it's fairly clear the _IMM is for old stuff, and _IMM_GEN4 is for gen4+. To make it more clear we could add a _GEN2 suffix for the old platform define, but I don't see much point with that myself. Oh and source backwards compatibility is something we never really have to worry about. All the code is in the same tree, so we can just change all the users at the same time. Doing otherwise would just lead to a mess. > > The above bikeshedding notwithstanding, it gets my R-b anyway: > > Reviewed-by: Dave Gordon <david.s.gordon@intel.com> -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-12-08 16:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-14 16:16 [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/ ville.syrjala 2014-11-15 4:43 ` [PATCH] drm/i915: shuang.he 2014-12-05 12:51 ` [PATCH] drm/i915: s/MI_STORE_DWORD_IMM_GEN8/MI_STORE_DWORD_IMM_GEN4/ Ville Syrjälä 2014-12-08 14:57 ` Dave Gordon 2014-12-08 16:05 ` Ville Syrjälä
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox