All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] render/gen7: Don't use a message register to store vl
@ 2013-02-14 14:20 Damien Lespiau
  2013-02-14 16:52 ` Kenneth Graunke
  0 siblings, 1 reply; 3+ messages in thread
From: Damien Lespiau @ 2013-02-14 14:20 UTC (permalink / raw)
  To: intel-gfx

Turns out the "new" assembler that uses mesa's opcode emission hits the
path that automatically transforms MRF registers into GRF ones in the
exa_wm_src_projective shader.

The diff with the new assembler is:

$ intel-gen4disasm -g7 -
-   { 0x00600041, 0x208077be, 0x008d03c0, 0x008d0180 },
+   { 0x00600041, 0x2e8077bd, 0x008d03c0, 0x008d0180 },
mul(8)      m4<1>F          g30<8,8,1>F     g12<8,8,1>F     { align1 };
mul(8)      g116<1>F        g30<8,8,1>F     g12<8,8,1>F     { align1 };

Of course, message registers are no more in gen7, so the shader is
trying to do something shaddy (ahem!).

Instead of using m4, let's make exa_wm_src_projective use g68 for v (aka
vl) which makes sense since:

1/ vh is g69
2/ exa_wm_src_affine uses g68 for vl already

This commit changes the generated assembly, here's the decoded diff:

$ intel-gen4disasm -g7 -
-   { 0x00600041, 0x208077be, 0x008d03c0, 0x008d0180 },
+   { 0x00600041, 0x288077bd, 0x008d03c0, 0x008d0180 },
mul(8)      m4<1>F          g30<8,8,1>F     g12<8,8,1>F     { align1 };
mul(8)      g68<1>F         g30<8,8,1>F     g12<8,8,1>F     { align1 };

Cc: Kenneth Graunke <kenneth@whitecape.org>
Reported-by: Xiang, Haihao <haihao.xiang@intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 src/render_program/exa_wm_src_projective.g7a |    4 ++--
 src/render_program/exa_wm_src_projective.g7b |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/render_program/exa_wm_src_projective.g7a b/src/render_program/exa_wm_src_projective.g7a
index 9fd495c..465e99d 100644
--- a/src/render_program/exa_wm_src_projective.g7a
+++ b/src/render_program/exa_wm_src_projective.g7a
@@ -30,8 +30,8 @@ include(`exa_wm.g4i')
 define(`u',     `g66')
 define(`ul',    `g66')
 define(`uh',    `g67')
-define(`v',     `src_v')
-define(`vl',    `src_v')
+define(`v',     `g68')
+define(`vl',    `g68')
 define(`vh',    `g69')
 define(`w',     `src_w')
 define(`wl',    `src_w_0')
diff --git a/src/render_program/exa_wm_src_projective.g7b b/src/render_program/exa_wm_src_projective.g7b
index 73727ff..cadcaa3 100644
--- a/src/render_program/exa_wm_src_projective.g7b
+++ b/src/render_program/exa_wm_src_projective.g7b
@@ -8,5 +8,5 @@
    { 0x00600041, 0x286077bd, 0x008d03e0, 0x008d01a0 },
    { 0x0060005a, 0x23c077bd, 0x000000d0, 0x008d0040 },
    { 0x0060005a, 0x23e077bd, 0x000000d0, 0x008d0080 },
-   { 0x00600041, 0x208077be, 0x008d03c0, 0x008d0180 },
+   { 0x00600041, 0x288077bd, 0x008d03c0, 0x008d0180 },
    { 0x00600041, 0x28a077bd, 0x008d03e0, 0x008d01a0 },
-- 
1.7.7.5

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] render/gen7: Don't use a message register to store vl
  2013-02-14 14:20 [PATCH] render/gen7: Don't use a message register to store vl Damien Lespiau
@ 2013-02-14 16:52 ` Kenneth Graunke
  2013-02-18 13:49   ` Chris Wilson
  0 siblings, 1 reply; 3+ messages in thread
From: Kenneth Graunke @ 2013-02-14 16:52 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx

On 02/14/2013 06:20 AM, Damien Lespiau wrote:
> Turns out the "new" assembler that uses mesa's opcode emission hits the
> path that automatically transforms MRF registers into GRF ones in the
> exa_wm_src_projective shader.
>
> The diff with the new assembler is:
>
> $ intel-gen4disasm -g7 -
> -   { 0x00600041, 0x208077be, 0x008d03c0, 0x008d0180 },
> +   { 0x00600041, 0x2e8077bd, 0x008d03c0, 0x008d0180 },
> mul(8)      m4<1>F          g30<8,8,1>F     g12<8,8,1>F     { align1 };
> mul(8)      g116<1>F        g30<8,8,1>F     g12<8,8,1>F     { align1 };
>
> Of course, message registers are no more in gen7, so the shader is
> trying to do something shaddy (ahem!).
>
> Instead of using m4, let's make exa_wm_src_projective use g68 for v (aka
> vl) which makes sense since:
>
> 1/ vh is g69
> 2/ exa_wm_src_affine uses g68 for vl already
>
> This commit changes the generated assembly, here's the decoded diff:
>
> $ intel-gen4disasm -g7 -
> -   { 0x00600041, 0x208077be, 0x008d03c0, 0x008d0180 },
> +   { 0x00600041, 0x288077bd, 0x008d03c0, 0x008d0180 },
> mul(8)      m4<1>F          g30<8,8,1>F     g12<8,8,1>F     { align1 };
> mul(8)      g68<1>F         g30<8,8,1>F     g12<8,8,1>F     { align1 };
>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Reported-by: Xiang, Haihao <haihao.xiang@intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

If I recall correctly, the only actual changes between the Gen6 and Gen7 
render assembly was converting MRFs to GRFs, and I arbitrarily picked 
something in the 60s rather than 112+.  If you want, I think you could 
actually just delete all the Gen7 custom assembly and let 
gen7_convert_mrf_to_grf do it for you.

But this looks okay to me, if you'd rather continue doing everything 
explicitly...
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] render/gen7: Don't use a message register to store vl
  2013-02-14 16:52 ` Kenneth Graunke
@ 2013-02-18 13:49   ` Chris Wilson
  0 siblings, 0 replies; 3+ messages in thread
From: Chris Wilson @ 2013-02-18 13:49 UTC (permalink / raw)
  To: Kenneth Graunke; +Cc: intel-gfx

On Thu, Feb 14, 2013 at 08:52:16AM -0800, Kenneth Graunke wrote:
> On 02/14/2013 06:20 AM, Damien Lespiau wrote:
> >Turns out the "new" assembler that uses mesa's opcode emission hits the
> >path that automatically transforms MRF registers into GRF ones in the
> >exa_wm_src_projective shader.
> >
> >The diff with the new assembler is:
> >
> >$ intel-gen4disasm -g7 -
> >-   { 0x00600041, 0x208077be, 0x008d03c0, 0x008d0180 },
> >+   { 0x00600041, 0x2e8077bd, 0x008d03c0, 0x008d0180 },
> >mul(8)      m4<1>F          g30<8,8,1>F     g12<8,8,1>F     { align1 };
> >mul(8)      g116<1>F        g30<8,8,1>F     g12<8,8,1>F     { align1 };
> >
> >Of course, message registers are no more in gen7, so the shader is
> >trying to do something shaddy (ahem!).
> >
> >Instead of using m4, let's make exa_wm_src_projective use g68 for v (aka
> >vl) which makes sense since:
> >
> >1/ vh is g69
> >2/ exa_wm_src_affine uses g68 for vl already
> >
> >This commit changes the generated assembly, here's the decoded diff:
> >
> >$ intel-gen4disasm -g7 -
> >-   { 0x00600041, 0x208077be, 0x008d03c0, 0x008d0180 },
> >+   { 0x00600041, 0x288077bd, 0x008d03c0, 0x008d0180 },
> >mul(8)      m4<1>F          g30<8,8,1>F     g12<8,8,1>F     { align1 };
> >mul(8)      g68<1>F         g30<8,8,1>F     g12<8,8,1>F     { align1 };
> >
> >Cc: Kenneth Graunke <kenneth@whitecape.org>
> >Reported-by: Xiang, Haihao <haihao.xiang@intel.com>
> >Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> If I recall correctly, the only actual changes between the Gen6 and
> Gen7 render assembly was converting MRFs to GRFs, and I arbitrarily
> picked something in the 60s rather than 112+.  If you want, I think
> you could actually just delete all the Gen7 custom assembly and let
> gen7_convert_mrf_to_grf do it for you.
> 
> But this looks okay to me, if you'd rather continue doing everything
> explicitly...
> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

Thanks for the patch and review, pushed.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-02-18 13:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-14 14:20 [PATCH] render/gen7: Don't use a message register to store vl Damien Lespiau
2013-02-14 16:52 ` Kenneth Graunke
2013-02-18 13:49   ` Chris Wilson

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.