[Mesa-dev] [PATCH 2/2] i965: Implement compute sampler state atom.

[Francisco Jerez]

Ben Widawsky <ben at bwidawsk.net> writes:

> On Mon, Jan 18, 2016 at 12:37:18PM -0800, Francisco Jerez wrote:
>> Fixes a number of GLES31 CTS failures and hangs on various hardware:
>> 
>>  ES31-CTS.texture_gather.plain-gather-depth-2d
>>  ES31-CTS.texture_gather.plain-gather-depth-2darray
>>  ES31-CTS.texture_gather.plain-gather-depth-cube
>>  ES31-CTS.texture_gather.offset-gather-depth-2d
>>  ES31-CTS.texture_gather.offset-gather-depth-2darray
>>  ES31-CTS.layout_binding.sampler2D_layout_binding_texture_ComputeShader
>>  ES31-CTS.layout_binding.sampler2DArray_layout_binding_texture_ComputeShader
>>  ES31-CTS.explicit_uniform_location.uniform-loc-types-samplers
>>  ES31-CTS.compute_shader.resources-texture
>> 
>> Some of them were actually passing by luck on some generations even
>> though we weren't uploading sampler state tables explicitly for the
>> compute stage, most likely because they relied on the cached sampler
>> state left from previous rendering to be close enough.
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92589
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93312
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93325
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93407
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93725
>> Reported-by: Marta Lofstedt <marta.lofstedt at intel.com>
>
> Thanks a lot for tracking this down. I was going with Marta's insight of the
> DEPTH_32F format thing and getting nowhere. It's really incredible that anything
> worked at all.
>
> [snip]
>
>> +
>> +static void
>> +brw_upload_cs_samplers(struct brw_context *brw)
>> +{
>> +   /* BRW_NEW_COMPUTE_PROGRAM */
>> +   struct gl_program *cs = (struct gl_program *) brw->compute_program;
>> +   if (!cs)
>> +      return;
>> +
>> +   brw_upload_sampler_state_table(brw, cs, &brw->cs.base);
>> +}
>> +
>
> Could you explain how brw->compute_program can be NULL here? I'm just trying to
> understand this a bit better...
>
It might well be non-NULL at present, but I'm not aware of any strong
reason why it would be guaranteed to be non-NULL, and some of the other
brw_context::*_program fields are definitely allowed to be NULL, so it
seemed sensible to check ::compute_program for NULL too.  Shouldn't hurt
anyway.

>> +const struct brw_tracked_state brw_cs_samplers = {
>> +   .dirty = {
>> +      .mesa = _NEW_TEXTURE,
>> +      .brw = BRW_NEW_BATCH |
>> +             BRW_NEW_COMPUTE_PROGRAM,
>> +   },
>> +   .emit = brw_upload_cs_samplers,
>
> [snip]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20160119/24e4e85f/attachment.sig>