[Mesa-dev] [PATCH] i965: Rewrite ir_tex to ir_txl with lod 0 for vertex shaders

[Kristian Høgsberg]

On Wed, Apr 15, 2015 at 6:06 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 04/15/2015 04:28 PM, Kenneth Graunke wrote:
>> On Wednesday, April 15, 2015 02:47:01 PM Ian Romanick wrote:
>>> On 04/15/2015 12:41 PM, Kristian Høgsberg wrote:
>>>> The ir_tex opcode turns into a sample or sample_c message, which will try to
>>>> compute derivatives to determine the lod. This produces garbage for
>>>> non-fragment shaders where the sample coordinates don't correspond to
>>>> subspans.
>>>>
>>>> We fix this by rewriting the opcode from ir_tex to ir_txl and setting the
>>>> lod to 0.
>>>
>>> This seems logical, but... why the heck hasn't this been a problem
>>> before?  Is this some weirdness of just BSW?  Should we perhaps only do
>>> this on BSW?
>>
>> In the vec4 backend, we've always made ir_tex use the SAMPLE_LOD message
>> with lod set to 0.0 - so Gen4-7.5 have always done this workaround.
>>
>> When Kristian added SIMD8 VS support (Oct 2014, Gen8+), we started using
>> SAMPLE (by virtue of not changing the FS behavior).  We honestly have no
>
> Yes, that's the part the slipped my mind.  In that case, FWIW,
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

Appreciate  it. Patch pushed.

Kristian

>> idea why it appears to work on Broadwell - it sure seems like it ought
>> to be broken there.  Maybe we've just been lucky.
>>
>> There's also a message header bit for "Force LOD to Zero", which the
>> docs say is mandatory if you're going to use the "SAMPLE" message in
>> SIMD4x2 stages (i.e. not fragment).  We've wondered whether headerless
>> messages in non-fragment stages magically get that right.  That could
>> make the issue less noticable...
>>
>> Using SAMPLE_LOD is easier than setting the message header bit, and
>> seems just as effective.  I think we should do it on Broadwell too.
>>
>> Thanks for fixing this, Kristian!
>>
>> Cc: "10.5" <mesa-stable at lists.freedesktop.org>
>> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>>
>>>
>>>> https://bugs.freedesktop.org/show_bug.cgi?id=89457
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89457
>>>
>>> That allows the bin/bugzilla_mesa.sh script to find it for generating
>>> release notes.
>>>
>>>> Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
>>>> ---
>>>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>>> index 0049b2d..4e99366 100644
>>>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>>> @@ -1839,6 +1839,15 @@ fs_visitor::emit_texture_gen7(ir_texture_opcode op, fs_reg dst,
>>>>        offset_value.file != BAD_FILE && offset_value.file != IMM;
>>>>     bool coordinate_done = false;
>>>>
>>>> +   /* The sampler can only meaningfully compute LOD for fragment shader
>>>> +    * messages. For all other stages, we change the opcode to ir_txl and
>>>> +    * hardcode the LOD to 0.
>>>> +    */
>>>> +   if (stage != MESA_SHADER_FRAGMENT && op == ir_tex) {
>>>> +      op = ir_txl;
>>>> +      lod = fs_reg(0.0f);
>>>> +   }
>>>> +
>>>>     /* Set up the LOD info */
>>>>     switch (op) {
>>>>     case ir_tex:
>>>>
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>
>