[Mesa-dev] [PATCH v2 4/4] i965/gen10: Implement Wa3DStateMode

[Nanley Chery]

On Fri, Nov 03, 2017 at 09:50:41AM -0700, Nanley Chery wrote:
> On Thu, Nov 02, 2017 at 06:13:43PM -0700, Matt Turner wrote:
> > On Thu, Nov 2, 2017 at 11:26 AM, Nanley Chery <nanleychery at gmail.com> wrote:
> > > On Wed, Nov 01, 2017 at 03:52:15PM -0700, Anuj Phogat wrote:
> > >> V2: Remove the bits enabling Float blend optimization. It is
> > >>     enabled through CACHE_MODE_SS register.
> > >>     Update the comment.
> > >>
> > >> This workaround doesn't fix any of the piglit hangs we've seen
> > >> on CNL.
> > >>
> > >
> > > I haven't seen this format in the git logs before..
> > >
> > >> Cc: Nanley Chery <nanley.g.chery at intel.com>
> > >> Cc: Jason Ekstrand <jason at jlekstrand.net>
> > >> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> > >> Reviewed-by: Rafael Antognolli <rafael.antognolli at intel.com>
> > >> ---
> > >>  src/mesa/drivers/dri/i965/brw_defines.h      |  2 ++
> > >>  src/mesa/drivers/dri/i965/brw_state_upload.c | 12 ++++++++++++
> > >>  2 files changed, 14 insertions(+)
> > >>
> > >> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> > >> index 3008a1b8a7..1c2f06546e 100644
> > >> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> > >> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> > >> @@ -1333,6 +1333,8 @@ enum brw_pixel_shader_coverage_mask_mode {
> > >>  /* DW2: start address */
> > >>  /* DW3: end address. */
> > >>
> > >> +#define _3DSTATE_3D_MODE                     0x791e
> > >> +
> > >>  #define CMD_MI_FLUSH                  0x0200
> > >>
> > >>  # define BLT_X_SHIFT                                 0
> > >> diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
> > >> index 7df06a6e4d..76c137c90e 100644
> > >> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> > >> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> > >> @@ -89,6 +89,18 @@ brw_upload_initial_gpu_state(struct brw_context *brw)
> > >>        brw_load_register_imm32(brw, GEN10_CACHE_MODE_SS,
> > >>                                REG_MASK(GEN10_FLOAT_BLEND_OPTIMIZATION_ENABLE) |
> > >>                                GEN10_FLOAT_BLEND_OPTIMIZATION_ENABLE);
> > >> +
> > >> +      /* From gen10 workaround table in h/w specs:
> > >> +       * "On 3DSTATE_3D_MODE, driver must always program bits 31:16 of DW1
> > >> +       *  a value of 0xFFFF"
> > >> +       *  This means that we end up setting the entire 3D_MODE state. Bits
> > >> +       *  in this register control things such as slice hashing and we want
> > >> +       *  the default values of zero at the moment.
> > >             ^ Extra space for the last 3 lines here.
> > 
> > No, it's not, actually. The correct style is to indent the lines so
> > that the " is in its own column.
> 
> Isn't that just the case for what you're quoting? There's extra
> indentation on the commentary.

Oh, I just looked at my first comment and realized that it's unclear.
I meant to say: There's an extra space in the last 3 lines.
Implying that they should be removed. Sorry for the confusion.