[Mesa-dev] [PATCH 09/10] i965/meta: Remove fast_clear_color variable

[Ben Widawsky]

On Wed, Oct 14, 2015 at 11:52:03AM +0200, Neil Roberts wrote:
> This patch doesn't look right. See this sentence in “Render Target Fast
> Clear”:
> 
> “The pixel shader kernel requires no attributes, and delivers a value of
>  0xFFFFFFFF in all channels of the render target write message”
> 
> Presumably the fast_clear_color is trying to implement this restriction.
> 
> Regards,
> - Neil
> 

You're right. Originally the patch only touched the color in the resolve pass,
which I believe doesn't matter (nor does the actual shader we bind). However,
I'm sort of baffled now why I'd see no piglit regressions since the clear color
will *never* be all F.

Either way, I'll drop this patch - but my confusion level has increased.

> Ben Widawsky <benjamin.widawsky at intel.com> writes:
> 
> > It doesn't actually serve a purpose AFAICT (in fact, I'm not certain what it's
> > meant to do).
> >
> > Cc: Kristian Høgsberg <krh at bitplanet.net>
> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > ---
> >  src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> > index 41afc9a..9c51ffb 100644
> > --- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> > +++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> > @@ -390,8 +390,6 @@ set_fast_clear_color(struct brw_context *brw,
> >     }
> >  }
> >  
> > -static const uint32_t fast_clear_color[4] = { ~0, ~0, ~0, ~0 };
> > -
> >  static void
> >  set_fast_clear_op(struct brw_context *brw, uint32_t op)
> >  {
> > @@ -472,7 +470,7 @@ fast_clear_attachments(struct brw_context *brw,
> >  
> >        use_rectlist(brw, true);
> >  
> > -      brw_bind_rep_write_shader(brw, (float *) fast_clear_color);
> > +      brw_bind_rep_write_shader(brw, ctx->Color.ClearColor.f);
> >  
> >        /* SKL+ also has a resolve mode for compressed render targets and thus more
> >         * bits to let us select the type of resolve.  For fast clear resolves, it
> > @@ -670,7 +668,7 @@ brw_meta_fast_clear(struct brw_context *brw, struct gl_framebuffer *fb,
> >        fast_clear_attachments(brw, fb, fast_clear_buffers, fast_clear_rect);
> >     } else if (fast_clear_buffers) {
> >        _mesa_meta_drawbuffers_from_bitfield(fast_clear_buffers);
> > -      brw_bind_rep_write_shader(brw, (float *) fast_clear_color);
> > +      brw_bind_rep_write_shader(brw, ctx->Color.ClearColor.f);
> >        set_fast_clear_op(brw, GEN7_PS_RENDER_TARGET_FAST_CLEAR_ENABLE);
> >        brw_draw_rectlist(ctx, &fast_clear_rect, layers);
> >        set_fast_clear_op(brw, 0);
> > @@ -785,7 +783,7 @@ brw_meta_resolve_color(struct brw_context *brw,
> >  
> >     use_rectlist(brw, true);
> >  
> > -   brw_bind_rep_write_shader(brw, (float *) fast_clear_color);
> > +   brw_bind_rep_write_shader(brw, ctx->Color.ClearColor.f);
> >  
> >     /* SKL+ also has a resolve mode for compressed render targets and thus more
> >      * bits to let us select the type of resolve.  For fast clear resolves, it
> > -- 
> > 2.6.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev