[Mesa-dev] [PATCH 05/22] i965: correctly assign SamplerCount of INTERFACE_DESCRIPTOR_DATA

[Rogovin, Kevin]

Hi,

 AFAIK, for all shader types both binding table entry count and number of samplers can be zero; the hardware uses those values to pre-fetch data. The docs say one may wish to leave it at zero if there is a risk of state thrashing if the number (for either) is large. FWIW, my main reason for fixing these two values was so that the logger could decode the binding tables and samplers correctly. 

If people want to push these 2 patches without having the entire series RB'd, that is fine with me, I am going to need to rebase anyways as this is just an RFC. Moreover all the patches that are fixes/enhancements to existing code, i.e. patches 0004-0005 and patches 0007-0015, I would be happy to see them pushed even if the main thrush of this series, the BatchbufferLogger, has comments (which I think it will). 

-Kevin

-----Original Message-----
From: Justen, Jordan L 
Sent: Tuesday, September 26, 2017 2:53 AM
To: Landwerlin, Lionel G <lionel.g.landwerlin at intel.com>; Rogovin, Kevin <kevin.rogovin at intel.com>; mesa-dev at lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 05/22] i965: correctly assign SamplerCount of INTERFACE_DESCRIPTOR_DATA

On 2017-09-25 05:46:32, Lionel Landwerlin wrote:
> I'm genuinely surprised we didn't noticed this problem before :|

Indeed. Did jenkins show any 'fixes' from this patch?

I think this patch should be handled separately from this RFC series.

Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

> Fixes: 71bfb44005bf ("i965: Port brw_cs_state tracked state to genxml.")
> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> Cc: "17.2" <mesa-stable at lists.freedesktop.org>
> 
> On 25/09/17 11:34, kevin.rogovin at intel.com wrote:
> > From: Kevin Rogovin <kevin.rogovin at intel.com>
> >
> > Signed-off-by: Kevin Rogovin <kevin.rogovin at intel.com>
> > ---
> >   src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > index bbb47ea..32c7d22 100644
> > --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> > +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > @@ -4197,7 +4197,7 @@ genX(upload_cs_state)(struct brw_context *brw)
> >      const struct GENX(INTERFACE_DESCRIPTOR_DATA) idd = {
> >         .KernelStartPointer = brw->cs.base.prog_offset,
> >         .SamplerStatePointer = stage_state->sampler_offset,
> > -      .SamplerCount = DIV_ROUND_UP(stage_state->sampler_count, 4) >> 2,
> > +      .SamplerCount = DIV_ROUND_UP(stage_state->sampler_count, 4),
> >         .BindingTablePointer = stage_state->bind_bo_offset,
> >         .BindingTableEntryCount = prog_data->binding_table.size_bytes / 4,
> >         .ConstantURBEntryReadLength = cs_prog_data->push.per_thread.regs,
> 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev