Re: [Mesa-dev] [PATCH] i965/msaa: Add sample-alpha-to-coverage support for multiple render targets

2012-08-22 Thread Paul Berry
On 20 August 2012 16:59, Anuj Phogat anuj.pho...@gmail.com wrote:

 Render Target Write message should include source zero alpha value when
 sample-alpha-to-coverage is enabled for an FBO with  multiple render
 targets.
 Source zero alpha value is used as fragment coverage for all the render
 targets.

 This patch makes piglit tests draw-buffers-alpha-to-coverage and
 alpha-to-coverage-no-draw-buffer-zero to pass on Sandybridge. No
 regressions are observed with piglit all.tests.

 V2: Revert all the changes made in emit_color_write() function to
 include src0 alpha for targets  0. Now handling this case in a if
 block.

 V3: Correctly calculate the instruction length for buffer zero.
 Properly handle the case of dual_src_blend when alpha-to-coverage
 is enabled.

 Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
 ---
  src/mesa/drivers/dri/i965/brw_fs_emit.cpp|   12 ++
  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   30
 -
  src/mesa/drivers/dri/i965/brw_wm.c   |2 +
  src/mesa/drivers/dri/i965/brw_wm.h   |1 +
  4 files changed, 43 insertions(+), 2 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
 b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
 index 4564e3b..5900c0e 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
 @@ -59,6 +59,18 @@ fs_visitor::generate_fb_write(fs_inst *inst)
  retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD));
  brw_set_compression_control(p, BRW_COMPRESSION_NONE);

 + if (inst-target  0 
 +c-key.nr_color_regions  1 
 +c-key.sample_alpha_to_coverage) {
 +/* Set Source0 Alpha Present to RenderTarget bit in message
 + * header.
 + */
 +brw_OR(p,
 +  vec1(retype(brw_message_reg(inst-base_mrf),
 BRW_REGISTER_TYPE_UD)),
 +  vec1(retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD)),
 +  brw_imm_ud(0x1  11));
 + }
 +
  if (inst-target  0) {
 /* Set the render target index for choosing BLEND_STATE. */
 brw_MOV(p, retype(brw_vec1_reg(BRW_MESSAGE_REGISTER_FILE,
 diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 index 7a2f777..2309059 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 @@ -2057,6 +2057,7 @@ fs_visitor::emit_fb_writes()
 int nr = base_mrf;
 int reg_width = c-dispatch_width / 8;
 bool do_dual_src = this-dual_src_output.file != BAD_FILE;
 +   bool src0_alpha_to_render_target = false;

 if (c-dispatch_width == 16  do_dual_src) {
fail(GL_ARB_blend_func_extended not yet supported in 16-wide.);
 @@ -2078,6 +2079,10 @@ fs_visitor::emit_fb_writes()
 }

 if (header_present) {
 +  src0_alpha_to_render_target = intel-gen = 6 
 +   !do_dual_src 
 +   c-key.nr_color_regions  1 
 +   c-key.sample_alpha_to_coverage;
/* m2, m3 header */
nr += 2;
 }
 @@ -2094,6 +2099,8 @@ fs_visitor::emit_fb_writes()
 nr += 4 * reg_width;
 if (do_dual_src)
nr += 4;
 +   if (src0_alpha_to_render_target)
 +  nr += reg_width;

 if (c-source_depth_to_render_target) {
if (intel-gen == 6  c-dispatch_width == 16) {
 @@ -2165,13 +2172,32 @@ fs_visitor::emit_fb_writes()
this-current_annotation = ralloc_asprintf(this-mem_ctx,
  FB write target %d,
  target);
 +  /* If src0_alpha_to_render_target is true, include source zero alpha
 +   * data in RenderTargetWrite message for targets  0.
 +   */
 +  int write_color_mrf = color_mrf;
 +  if (src0_alpha_to_render_target  target) {


This condition is confusing to read because it makes target look like a
boolean.  It would be easier to read as if (src0_alpha_to_render_target 
target != 0).


 + fs_inst *inst;
 + fs_reg color = outputs[0];
 + color.reg_offset += 3;
 +
 + inst = emit(BRW_OPCODE_MOV,
 +fs_reg(MRF, write_color_mrf, color.type),
 +color);
 + inst-saturate = c-key.clamp_fragment_color;
 + write_color_mrf = color_mrf + reg_width;
 +  }
 +
for (unsigned i = 0; i  this-output_components[target]; i++)
 -emit_color_write(target, i, color_mrf);
 + emit_color_write(target, i, write_color_mrf);

fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
inst-target = target;
inst-base_mrf = base_mrf;
 -  inst-mlen = nr - base_mrf;
 +  if (src0_alpha_to_render_target  !target)


Similarly, this would be easier to read if we replaced !target with
target == 0.

With those changes, this patch is:

Reviewed-by: Paul 

Re: [Mesa-dev] [PATCH] i965/msaa: Add sample-alpha-to-coverage support for multiple render targets

2012-08-20 Thread Paul Berry
On 13 August 2012 16:46, Anuj Phogat anuj.pho...@gmail.com wrote:

 Render Target Write message should include source zero alpha value when
 sample-alpha-to-coverage is enabled for an FBO with  multiple render
 targets.
 Source zero alpha value is used as fragment coverage for all the render
 targets.

 This patch makes piglit tests draw-buffers-alpha-to-coverage and
 alpha-to-coverage-no-draw-buffer-zero to pass on Sandybridge. No
 regressions are observed with piglit all.tests.

 V2: Revert all the changes made in emit_color_write() function to
 include src0 alpha for targets  0. Now handling this case in a if
 block.

 Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
 ---
  src/mesa/drivers/dri/i965/brw_fs_emit.cpp|   12 
  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   25
 -
  src/mesa/drivers/dri/i965/brw_wm.c   |2 ++
  src/mesa/drivers/dri/i965/brw_wm.h   |1 +
  4 files changed, 39 insertions(+), 1 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
 b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
 index dc5f3e1..c039cab 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
 @@ -59,6 +59,18 @@ fs_visitor::generate_fb_write(fs_inst *inst)
  retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD));
  brw_set_compression_control(p, BRW_COMPRESSION_NONE);

 + if (inst-target  0 
 +c-key.nr_color_regions  1 
 +c-key.sample_alpha_to_coverage) {
 +/* Set Source0 Alpha Present to RenderTarget bit in message
 + * header.
 + */
 +brw_OR(p,
 +  vec1(retype(brw_message_reg(inst-base_mrf),
 BRW_REGISTER_TYPE_UD)),
 +  vec1(retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD)),
 +  brw_imm_ud(0x1  11));
 + }
 +

 if (inst-target  0) {
 /* Set the render target index for choosing BLEND_STATE. */
 brw_MOV(p, retype(brw_vec1_reg(BRW_MESSAGE_REGISTER_FILE,
 diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 index fefe2c7..3ede7ed 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 @@ -2014,6 +2014,7 @@ fs_visitor::emit_fb_writes()
 int nr = base_mrf;
 int reg_width = c-dispatch_width / 8;
 bool do_dual_src = this-dual_src_output.file != BAD_FILE;
 +   bool src0_alpha_to_render_target = false;

 if (c-dispatch_width == 16  do_dual_src) {
fail(GL_ARB_blend_func_extended not yet supported in 16-wide.);
 @@ -2035,6 +2036,8 @@ fs_visitor::emit_fb_writes()
 }

 if (header_present) {
 +  src0_alpha_to_render_target =  c-key.nr_color_regions  1 
 +c-key.sample_alpha_to_coverage;
/* m2, m3 header */
nr += 2;
 }
 @@ -2051,6 +2054,8 @@ fs_visitor::emit_fb_writes()
 nr += 4 * reg_width;
 if (do_dual_src)
nr += 4;
 +   if (src0_alpha_to_render_target)
 +  nr += reg_width;


(We discussed this in person, just sending it to the list so we don't
forget)
It looks like we should update this condition so that nr doesn't get
incremented when do_dual_src is true, since the do_dual_src branch doesn't
send alpha0 values.



 if (c-source_depth_to_render_target) {
if (intel-gen == 6  c-dispatch_width == 16) {
 @@ -2122,8 +2127,26 @@ fs_visitor::emit_fb_writes()
this-current_annotation = ralloc_asprintf(this-mem_ctx,
  FB write target %d,
  target);
 +  /* If src0_alpha_to_render_target is true, include source zero alpha
 +   * data in RenderTargetWrite message for targets  0.
 +   */
 +  int write_color_mrf = color_mrf;
 +  if (intel-gen = 6 
 + src0_alpha_to_render_target 
 + target) {
 + fs_inst *inst;
 + fs_reg color = outputs[0];
 + color.reg_offset += 3;
 +
 + inst = emit(BRW_OPCODE_MOV,
 +fs_reg(MRF, write_color_mrf, color.type),
 +color);
 + inst-saturate = c-key.clamp_fragment_color;
 + write_color_mrf = color_mrf + reg_width;
 +  }
 +
for (unsigned i = 0; i  this-output_components[target]; i++)
 -emit_color_write(target, i, color_mrf);
 + emit_color_write(target, i, write_color_mrf);

fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
inst-target = target;


A few lines below this there's a statement inst-mlen = nr - base_mrf;.
That's going to be wrong in the case where target == 0, since for target ==
0 we don't send the alpha0 values.


 diff --git a/src/mesa/drivers/dri/i965/brw_wm.c
 b/src/mesa/drivers/dri/i965/brw_wm.c
 index 5ab0547..8bf551e 100644
 --- a/src/mesa/drivers/dri/i965/brw_wm.c
 +++ 

[Mesa-dev] [PATCH] i965/msaa: Add sample-alpha-to-coverage support for multiple render targets

2012-08-20 Thread Anuj Phogat
Render Target Write message should include source zero alpha value when
sample-alpha-to-coverage is enabled for an FBO with  multiple render targets.
Source zero alpha value is used as fragment coverage for all the render
targets.

This patch makes piglit tests draw-buffers-alpha-to-coverage and
alpha-to-coverage-no-draw-buffer-zero to pass on Sandybridge. No
regressions are observed with piglit all.tests.

V2: Revert all the changes made in emit_color_write() function to
include src0 alpha for targets  0. Now handling this case in a if
block.

V3: Correctly calculate the instruction length for buffer zero.
Properly handle the case of dual_src_blend when alpha-to-coverage
is enabled.

Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
---
 src/mesa/drivers/dri/i965/brw_fs_emit.cpp|   12 ++
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   30 -
 src/mesa/drivers/dri/i965/brw_wm.c   |2 +
 src/mesa/drivers/dri/i965/brw_wm.h   |1 +
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
index 4564e3b..5900c0e 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
@@ -59,6 +59,18 @@ fs_visitor::generate_fb_write(fs_inst *inst)
 retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD));
 brw_set_compression_control(p, BRW_COMPRESSION_NONE);
 
+ if (inst-target  0 
+c-key.nr_color_regions  1 
+c-key.sample_alpha_to_coverage) {
+/* Set Source0 Alpha Present to RenderTarget bit in message
+ * header.
+ */
+brw_OR(p,
+  vec1(retype(brw_message_reg(inst-base_mrf), 
BRW_REGISTER_TYPE_UD)),
+  vec1(retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD)),
+  brw_imm_ud(0x1  11));
+ }
+
 if (inst-target  0) {
/* Set the render target index for choosing BLEND_STATE. */
brw_MOV(p, retype(brw_vec1_reg(BRW_MESSAGE_REGISTER_FILE,
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 7a2f777..2309059 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -2057,6 +2057,7 @@ fs_visitor::emit_fb_writes()
int nr = base_mrf;
int reg_width = c-dispatch_width / 8;
bool do_dual_src = this-dual_src_output.file != BAD_FILE;
+   bool src0_alpha_to_render_target = false;
 
if (c-dispatch_width == 16  do_dual_src) {
   fail(GL_ARB_blend_func_extended not yet supported in 16-wide.);
@@ -2078,6 +2079,10 @@ fs_visitor::emit_fb_writes()
}
 
if (header_present) {
+  src0_alpha_to_render_target = intel-gen = 6 
+   !do_dual_src 
+   c-key.nr_color_regions  1 
+   c-key.sample_alpha_to_coverage;
   /* m2, m3 header */
   nr += 2;
}
@@ -2094,6 +2099,8 @@ fs_visitor::emit_fb_writes()
nr += 4 * reg_width;
if (do_dual_src)
   nr += 4;
+   if (src0_alpha_to_render_target)
+  nr += reg_width;
 
if (c-source_depth_to_render_target) {
   if (intel-gen == 6  c-dispatch_width == 16) {
@@ -2165,13 +2172,32 @@ fs_visitor::emit_fb_writes()
   this-current_annotation = ralloc_asprintf(this-mem_ctx,
 FB write target %d,
 target);
+  /* If src0_alpha_to_render_target is true, include source zero alpha
+   * data in RenderTargetWrite message for targets  0.
+   */
+  int write_color_mrf = color_mrf;
+  if (src0_alpha_to_render_target  target) {
+ fs_inst *inst;
+ fs_reg color = outputs[0];
+ color.reg_offset += 3;
+
+ inst = emit(BRW_OPCODE_MOV,
+fs_reg(MRF, write_color_mrf, color.type),
+color);
+ inst-saturate = c-key.clamp_fragment_color;
+ write_color_mrf = color_mrf + reg_width;
+  }
+
   for (unsigned i = 0; i  this-output_components[target]; i++)
-emit_color_write(target, i, color_mrf);
+ emit_color_write(target, i, write_color_mrf);
 
   fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
   inst-target = target;
   inst-base_mrf = base_mrf;
-  inst-mlen = nr - base_mrf;
+  if (src0_alpha_to_render_target  !target)
+ inst-mlen = nr - base_mrf - reg_width;
+  else
+ inst-mlen = nr - base_mrf;
   if (target == c-key.nr_color_regions - 1)
 inst-eot = true;
   inst-header_present = header_present;
diff --git a/src/mesa/drivers/dri/i965/brw_wm.c 
b/src/mesa/drivers/dri/i965/brw_wm.c
index 323eabd..6e5163b 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/src/mesa/drivers/dri/i965/brw_wm.c
@@ -633,6 +633,8 @@ static void 

Re: [Mesa-dev] [PATCH] i965/msaa: Add sample-alpha-to-coverage support for multiple render targets

2012-08-13 Thread Anuj Phogat
On Tue, Aug 7, 2012 at 9:23 AM, Eric Anholt e...@anholt.net wrote:
 Anuj Phogat anuj.pho...@gmail.com writes:

 Render Target Write message should include source zero alpha value when
 sample-alpha-to-coverage is enabled for an FBO with  multiple render targets.
 Source zero alpha value is used as fragment coverage for all the render
 targets.

 diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 index fefe2c7..7fc28ac 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 @@ -1930,14 +1930,24 @@ fs_visitor::emit_color_write(int target, int index, 
 int first_color_mrf)
  {
 int reg_width = c-dispatch_width / 8;
 fs_inst *inst;
 -   fs_reg color = outputs[target];
 +   fs_reg color;
 +   bool src0_alpha_to_render_target = target  0 
 +   c-key.nr_color_regions  1 
 +   c-key.sample_alpha_to_coverage;
 +
 +   color = (src0_alpha_to_render_target  !index) ?
 + outputs[0] :
 + outputs[target];
 fs_reg mrf;

 /* If there's no color data to be written, skip it. */
 if (color.file == BAD_FILE)
return;

 -   color.reg_offset += index;
 +   if (src0_alpha_to_render_target)
 +  color.reg_offset += !index ? 3 : index - 1;
 +   else
 +  color.reg_offset += index;

 Ew, this is really awful.

 How about instead..,

 -  for (unsigned i = 0; i  this-output_components[target]; i++)
 -  emit_color_write(target, i, color_mrf);
 +  /* If src0_alpha_to_render_target is true, include source zero alpha
 +   * data in RenderTargetWrite message for targets  0.
 +   */
 +  output_components = (target  src0_alpha_to_render_target) ?
 +   (this-output_components[target] + 1) :
 +   this-output_components[target];

 -  fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
 +  for (unsigned i = 0; i  output_components; i++)
 + emit_color_write(target, i, color_mrf);

 Replace all of this change with:

   if (src0_alpha_to_render_target) {
  emit_color_write(0, 3, color_mrf);
Without any modifications in emit_color_write() function, this will
write src0 alpha in wrong register: color_mrf + 3 * reg_width.
We want the value to be written at color_mrf.

  color_mrf += reg_width);
This will permanently modify the initial value of color_mrf which
should stay same for all the render targets. I verified that
draw-buffers-alpha-to-coverage fails with above changes.

I will send out a new patch which removes all the changes you
didn't like in emit_color_write() function.
   }
   for (unsigned i = 0; i  this-output_components[target]; i++)
  emit_color_write(target, i, color_mrf);

 diff --git a/src/mesa/drivers/dri/i965/brw_wm.c 
 b/src/mesa/drivers/dri/i965/brw_wm.c
 index 5ab0547..210b078 100644
 --- a/src/mesa/drivers/dri/i965/brw_wm.c
 +++ b/src/mesa/drivers/dri/i965/brw_wm.c
 @@ -546,6 +546,8 @@ static void brw_wm_populate_key( struct brw_context *brw,
 /* _NEW_BUFFERS */
 key-nr_color_regions = ctx-DrawBuffer-_NumColorDrawBuffers;

 Needs
   /* _NEW_MULTISAMPLE */
 +   key-sample_alpha_to_coverage = ctx-Multisample.SampleAlphaToCoverage;

 and corresponding addition to the state struct below.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] i965/msaa: Add sample-alpha-to-coverage support for multiple render targets

2012-08-13 Thread Anuj Phogat
Render Target Write message should include source zero alpha value when
sample-alpha-to-coverage is enabled for an FBO with  multiple render targets.
Source zero alpha value is used as fragment coverage for all the render
targets.

This patch makes piglit tests draw-buffers-alpha-to-coverage and
alpha-to-coverage-no-draw-buffer-zero to pass on Sandybridge. No
regressions are observed with piglit all.tests.

V2: Revert all the changes made in emit_color_write() function to
include src0 alpha for targets  0. Now handling this case in a if
block.

Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
---
 src/mesa/drivers/dri/i965/brw_fs_emit.cpp|   12 
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   25 -
 src/mesa/drivers/dri/i965/brw_wm.c   |2 ++
 src/mesa/drivers/dri/i965/brw_wm.h   |1 +
 4 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
index dc5f3e1..c039cab 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
@@ -59,6 +59,18 @@ fs_visitor::generate_fb_write(fs_inst *inst)
 retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD));
 brw_set_compression_control(p, BRW_COMPRESSION_NONE);
 
+ if (inst-target  0 
+c-key.nr_color_regions  1 
+c-key.sample_alpha_to_coverage) {
+/* Set Source0 Alpha Present to RenderTarget bit in message
+ * header.
+ */
+brw_OR(p,
+  vec1(retype(brw_message_reg(inst-base_mrf), 
BRW_REGISTER_TYPE_UD)),
+  vec1(retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD)),
+  brw_imm_ud(0x1  11));
+ }
+
 if (inst-target  0) {
/* Set the render target index for choosing BLEND_STATE. */
brw_MOV(p, retype(brw_vec1_reg(BRW_MESSAGE_REGISTER_FILE,
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index fefe2c7..3ede7ed 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -2014,6 +2014,7 @@ fs_visitor::emit_fb_writes()
int nr = base_mrf;
int reg_width = c-dispatch_width / 8;
bool do_dual_src = this-dual_src_output.file != BAD_FILE;
+   bool src0_alpha_to_render_target = false;
 
if (c-dispatch_width == 16  do_dual_src) {
   fail(GL_ARB_blend_func_extended not yet supported in 16-wide.);
@@ -2035,6 +2036,8 @@ fs_visitor::emit_fb_writes()
}
 
if (header_present) {
+  src0_alpha_to_render_target =  c-key.nr_color_regions  1 
+c-key.sample_alpha_to_coverage;
   /* m2, m3 header */
   nr += 2;
}
@@ -2051,6 +2054,8 @@ fs_visitor::emit_fb_writes()
nr += 4 * reg_width;
if (do_dual_src)
   nr += 4;
+   if (src0_alpha_to_render_target)
+  nr += reg_width;
 
if (c-source_depth_to_render_target) {
   if (intel-gen == 6  c-dispatch_width == 16) {
@@ -2122,8 +2127,26 @@ fs_visitor::emit_fb_writes()
   this-current_annotation = ralloc_asprintf(this-mem_ctx,
 FB write target %d,
 target);
+  /* If src0_alpha_to_render_target is true, include source zero alpha
+   * data in RenderTargetWrite message for targets  0.
+   */
+  int write_color_mrf = color_mrf;
+  if (intel-gen = 6 
+ src0_alpha_to_render_target 
+ target) {
+ fs_inst *inst;
+ fs_reg color = outputs[0];
+ color.reg_offset += 3;
+
+ inst = emit(BRW_OPCODE_MOV,
+fs_reg(MRF, write_color_mrf, color.type),
+color);
+ inst-saturate = c-key.clamp_fragment_color;
+ write_color_mrf = color_mrf + reg_width;
+  }
+
   for (unsigned i = 0; i  this-output_components[target]; i++)
-emit_color_write(target, i, color_mrf);
+ emit_color_write(target, i, write_color_mrf);
 
   fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
   inst-target = target;
diff --git a/src/mesa/drivers/dri/i965/brw_wm.c 
b/src/mesa/drivers/dri/i965/brw_wm.c
index 5ab0547..8bf551e 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/src/mesa/drivers/dri/i965/brw_wm.c
@@ -545,6 +545,8 @@ static void brw_wm_populate_key( struct brw_context *brw,
 
/* _NEW_BUFFERS */
key-nr_color_regions = ctx-DrawBuffer-_NumColorDrawBuffers;
+  /* _NEW_MULTISAMPLE */
+   key-sample_alpha_to_coverage = ctx-Multisample.SampleAlphaToCoverage;
 
/* CACHE_NEW_VS_PROG */
key-vp_outputs_written = brw-vs.prog_data-outputs_written;
diff --git a/src/mesa/drivers/dri/i965/brw_wm.h 
b/src/mesa/drivers/dri/i965/brw_wm.h
index b976a60..730daf3 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.h
+++ b/src/mesa/drivers/dri/i965/brw_wm.h
@@ -64,6 +64,7 @@ struct 

Re: [Mesa-dev] [PATCH] i965/msaa: Add sample-alpha-to-coverage support for multiple render targets

2012-08-07 Thread Eric Anholt
Anuj Phogat anuj.pho...@gmail.com writes:

 Render Target Write message should include source zero alpha value when
 sample-alpha-to-coverage is enabled for an FBO with  multiple render targets.
 Source zero alpha value is used as fragment coverage for all the render
 targets.

 diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 index fefe2c7..7fc28ac 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
 @@ -1930,14 +1930,24 @@ fs_visitor::emit_color_write(int target, int index, 
 int first_color_mrf)
  {
 int reg_width = c-dispatch_width / 8;
 fs_inst *inst;
 -   fs_reg color = outputs[target];
 +   fs_reg color;
 +   bool src0_alpha_to_render_target = target  0 
 +   c-key.nr_color_regions  1 
 +   c-key.sample_alpha_to_coverage;
 +
 +   color = (src0_alpha_to_render_target  !index) ?
 + outputs[0] :
 + outputs[target];
 fs_reg mrf;
  
 /* If there's no color data to be written, skip it. */
 if (color.file == BAD_FILE)
return;
  
 -   color.reg_offset += index;
 +   if (src0_alpha_to_render_target)
 +  color.reg_offset += !index ? 3 : index - 1;
 +   else
 +  color.reg_offset += index;

Ew, this is really awful.

How about instead..,

 -  for (unsigned i = 0; i  this-output_components[target]; i++)
 -  emit_color_write(target, i, color_mrf);
 +  /* If src0_alpha_to_render_target is true, include source zero alpha
 +   * data in RenderTargetWrite message for targets  0.
 +   */
 +  output_components = (target  src0_alpha_to_render_target) ?
 +   (this-output_components[target] + 1) :
 +   this-output_components[target];
  
 -  fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
 +  for (unsigned i = 0; i  output_components; i++)
 + emit_color_write(target, i, color_mrf);

Replace all of this change with:

  if (src0_alpha_to_render_target) {
 emit_color_write(0, 3, color_mrf);
 color_mrf += reg_width);
  }
  for (unsigned i = 0; i  this-output_components[target]; i++)
 emit_color_write(target, i, color_mrf);

 diff --git a/src/mesa/drivers/dri/i965/brw_wm.c 
 b/src/mesa/drivers/dri/i965/brw_wm.c
 index 5ab0547..210b078 100644
 --- a/src/mesa/drivers/dri/i965/brw_wm.c
 +++ b/src/mesa/drivers/dri/i965/brw_wm.c
 @@ -546,6 +546,8 @@ static void brw_wm_populate_key( struct brw_context *brw,
 /* _NEW_BUFFERS */
 key-nr_color_regions = ctx-DrawBuffer-_NumColorDrawBuffers;

Needs
  /* _NEW_MULTISAMPLE */
 +   key-sample_alpha_to_coverage = ctx-Multisample.SampleAlphaToCoverage;

and corresponding addition to the state struct below.


pgpLkqngDbG9c.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] i965/msaa: Add sample-alpha-to-coverage support for multiple render targets

2012-08-02 Thread Anuj Phogat
Render Target Write message should include source zero alpha value when
sample-alpha-to-coverage is enabled for an FBO with  multiple render targets.
Source zero alpha value is used as fragment coverage for all the render
targets.

This patch makes piglit tests draw-buffers-alpha-to-coverage and
alpha-to-coverage-no-draw-buffer-zero to pass on Sandybridge. No
regressions are observed with piglit all.tests.

Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
---
 src/mesa/drivers/dri/i965/brw_fs_emit.cpp|   12 +
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   33 ++
 src/mesa/drivers/dri/i965/brw_wm.c   |2 +
 src/mesa/drivers/dri/i965/brw_wm.h   |1 +
 4 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
index dc5f3e1..c039cab 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
@@ -59,6 +59,18 @@ fs_visitor::generate_fb_write(fs_inst *inst)
 retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD));
 brw_set_compression_control(p, BRW_COMPRESSION_NONE);
 
+ if (inst-target  0 
+c-key.nr_color_regions  1 
+c-key.sample_alpha_to_coverage) {
+/* Set Source0 Alpha Present to RenderTarget bit in message
+ * header.
+ */
+brw_OR(p,
+  vec1(retype(brw_message_reg(inst-base_mrf), 
BRW_REGISTER_TYPE_UD)),
+  vec1(retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD)),
+  brw_imm_ud(0x1  11));
+ }
+
 if (inst-target  0) {
/* Set the render target index for choosing BLEND_STATE. */
brw_MOV(p, retype(brw_vec1_reg(BRW_MESSAGE_REGISTER_FILE,
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index fefe2c7..7fc28ac 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -1930,14 +1930,24 @@ fs_visitor::emit_color_write(int target, int index, int 
first_color_mrf)
 {
int reg_width = c-dispatch_width / 8;
fs_inst *inst;
-   fs_reg color = outputs[target];
+   fs_reg color;
+   bool src0_alpha_to_render_target = target  0 
+ c-key.nr_color_regions  1 
+ c-key.sample_alpha_to_coverage;
+
+   color = (src0_alpha_to_render_target  !index) ?
+   outputs[0] :
+   outputs[target];
fs_reg mrf;
 
/* If there's no color data to be written, skip it. */
if (color.file == BAD_FILE)
   return;
 
-   color.reg_offset += index;
+   if (src0_alpha_to_render_target)
+  color.reg_offset += !index ? 3 : index - 1;
+   else
+  color.reg_offset += index;
 
if (c-dispatch_width == 8 || intel-gen = 6) {
   /* SIMD8 write looks like:
@@ -2007,6 +2017,7 @@ fs_visitor::emit_fb_writes()
 {
this-current_annotation = FB write header;
bool header_present = true;
+   unsigned output_components;
/* We can potentially have a message length of up to 15, so we have to set
 * base_mrf to either 0 or 1 in order to fit in m0..m15.
 */
@@ -2014,6 +2025,7 @@ fs_visitor::emit_fb_writes()
int nr = base_mrf;
int reg_width = c-dispatch_width / 8;
bool do_dual_src = this-dual_src_output.file != BAD_FILE;
+   bool src0_alpha_to_render_target = false;
 
if (c-dispatch_width == 16  do_dual_src) {
   fail(GL_ARB_blend_func_extended not yet supported in 16-wide.);
@@ -2035,6 +2047,8 @@ fs_visitor::emit_fb_writes()
}
 
if (header_present) {
+  src0_alpha_to_render_target =  c-key.nr_color_regions  1 
+c-key.sample_alpha_to_coverage;
   /* m2, m3 header */
   nr += 2;
}
@@ -2051,6 +2065,8 @@ fs_visitor::emit_fb_writes()
nr += 4 * reg_width;
if (do_dual_src)
   nr += 4;
+   if (src0_alpha_to_render_target)
+  nr += reg_width;
 
if (c-source_depth_to_render_target) {
   if (intel-gen == 6  c-dispatch_width == 16) {
@@ -2122,10 +2138,17 @@ fs_visitor::emit_fb_writes()
   this-current_annotation = ralloc_asprintf(this-mem_ctx,
 FB write target %d,
 target);
-  for (unsigned i = 0; i  this-output_components[target]; i++)
-emit_color_write(target, i, color_mrf);
+  /* If src0_alpha_to_render_target is true, include source zero alpha
+   * data in RenderTargetWrite message for targets  0.
+   */
+  output_components = (target  src0_alpha_to_render_target) ?
+ (this-output_components[target] + 1) :
+ this-output_components[target];
 
-  fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
+  for (unsigned i = 0; i  output_components; i++)
+ emit_color_write(target, i,