* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.