[Mesa-dev] [PATCH v2] i965/skl: Fix the order of the arguments for the LD sampler message

[Kenneth Graunke]

On Monday, March 09, 2015 05:17:56 PM Neil Roberts wrote:
> In Skylake the order of the arguments for sample messages with the LD
> type are u, v, lod, r whereas previously they were u, lod, v, r.
> 
> This fixes 144 Piglit tests including ones that directly use
> texelFetch and also some using the meta stencil blit path which
> appears to use texelFetch in its shader.
> 
> v2: Fix sampling 1D textures
> ---
> 
> I realised that v1 of the patch would end up putting the lod in the
> wrong argument for 1D textures so here is a v2. This time I have run
> it through a full Piglit run and it doesn't cause any regressions.
> 
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 6b48f70..287ee47 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -1742,15 +1742,26 @@ fs_visitor::emit_texture_gen7(ir_texture_opcode op, fs_reg dst,
>        length++;
>        break;
>     case ir_txf:
> -      /* Unfortunately, the parameters for LD are intermixed: u, lod, v, r. */
> +      /* Unfortunately, the parameters for LD are intermixed: u, lod, v, r.
> +       * On Gen9 they are u, v, lod, r
> +       */
> +
>        emit(MOV(retype(sources[length], BRW_REGISTER_TYPE_D), coordinate));
>        coordinate = offset(coordinate, 1);
>        length++;
>  
> +      if (brw->gen >= 9) {
> +         if (coord_components >= 2) {
> +            emit(MOV(retype(sources[length], BRW_REGISTER_TYPE_D), coordinate));
> +            coordinate = offset(coordinate, 1);
> +         }
> +         length++;
> +      }
> +
>        emit(MOV(retype(sources[length], BRW_REGISTER_TYPE_D), lod));
>        length++;
>  
> -      for (int i = 1; i < coord_components; i++) {
> +      for (int i = brw->gen >= 9 ? 2 : 1; i < coord_components; i++) {
>  	 emit(MOV(retype(sources[length], BRW_REGISTER_TYPE_D), coordinate));
>  	 coordinate = offset(coordinate, 1);
>  	 length++;
> 

They changed it, but it's still weird!  Heh :D

Good eye!  Thanks for fixing this.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Also...if people don't respond to your patches in nearly a month...feel
free to bump the thread/ping people on IRC/etc.  I'd completely missed
this patch until you mentioned it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150401/6f5a7d47/attachment.sig>