Re: [Mesa-dev] [PATCH 05/12] i965/vec4/skl+: Use lcd2dms_w instead of lcd2dms

[Neil Roberts]

Ben Widawsky  writes:

>>} else if (op == ir_txf_ms) {
>>   emit(MOV(dst_reg(MRF, param_base + 1, sample_index.type, 
>> WRITEMASK_X),
>>sample_index));
>> - if (devinfo->gen >= 7) {
>> + if (opcode == SHADER_OPCODE_TXF_CMS_W) {
>> +/* MCS data is stored in the first two channels of ‘mcs’, but we
>> + * need to get it into the .y and .z channels of the second vec4
>> + * of params.
>> + */
>
> I don't understand what you mean by the "second vec4". Do you just mean the .y
> and .z channel?

No, the arguments in the send message are laid out like this:

 Register 1Register 2  
┌┴──┐ ┌┴──┐
 .x .y .z .w.x .y .z .w.x .y   .z   .w.x .y   .z   .w 
 u  v  r  n/a   u  v  r  n/a   si mcsl mcsh n/a   si mcsl mcsh n/a
└┬───┘ └┬───┘ └───┬┘ └┬───┘
 vertex 1vertex 2  vertex 1vertex 2

So the “second vec4” refers to the pair of vec4s in register 2. This is
a bit confusing but it is the same wording that is used for the TXF_CMS
case just below this.

>> +mcs.swizzle = BRW_SWIZZLE4(0, 0, 1, 1);
>
> I don't really understand how the swizzle stuff works. Quick
> explanation on why it's not BRW_SWIZZLE_XYXY would be greatly
> appreciated.

The MCS data is returned as the two .xy pairs of components in a single
register. Because the sample index is the first argument in the second
register, we need to shift this over by 32 bits so that is occupies the
.yz pairs. This is done with the swizzle. The writemask in the MOV
instruction filters out the X and W components of the swizzled register.
Only the second and third arguments to BRW_SWIZZLE4 actually matter.
This could be something like BRW_SWIZZLE_XXYX except that that define
doesn't exist.

This would be easier to review if there was more context so you could
see the CMS case as well.

Regards,
- Neil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 05/12] i965/vec4/skl+: Use lcd2dms_w instead of lcd2dms

[Ben Widawsky]

On Thu, Sep 17, 2015 at 05:00:07PM +0100, Neil Roberts wrote:
> In order to support 16x MSAA, skl+ has a wider version of lcd2dms that
> takes two parameters for the MCS data. The MCS data in the response
> still fits in a single register so we just need to ensure we copy both
> values rather than just the lower one.
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp   |  1 +
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp |  5 +
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   | 14 --
>  3 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index ed49cd3..22c8d01 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -327,6 +327,7 @@ vec4_visitor::implied_mrf_writes(vec4_instruction *inst)
> case SHADER_OPCODE_TXD:
> case SHADER_OPCODE_TXF:
> case SHADER_OPCODE_TXF_CMS:
> +   case SHADER_OPCODE_TXF_CMS_W:
> case SHADER_OPCODE_TXF_MCS:
> case SHADER_OPCODE_TXS:
> case SHADER_OPCODE_TG4:
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index 1950333..c4ddff9 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -259,6 +259,10 @@ vec4_generator::generate_tex(vec4_instruction *inst,
>case SHADER_OPCODE_TXF:
>msg_type = GEN5_SAMPLER_MESSAGE_SAMPLE_LD;
>break;
> +  case SHADER_OPCODE_TXF_CMS_W:
> + assert(devinfo->gen >= 9);
> + msg_type = GEN9_SAMPLER_MESSAGE_SAMPLE_LD2DMS_W;
> + break;
>case SHADER_OPCODE_TXF_CMS:
>   if (devinfo->gen >= 7)
>  msg_type = GEN7_SAMPLER_MESSAGE_SAMPLE_LD2DMS;
> @@ -1372,6 +1376,7 @@ vec4_generator::generate_code(const cfg_t *cfg)
>case SHADER_OPCODE_TXD:
>case SHADER_OPCODE_TXF:
>case SHADER_OPCODE_TXF_CMS:
> +  case SHADER_OPCODE_TXF_CMS_W:
>case SHADER_OPCODE_TXF_MCS:
>case SHADER_OPCODE_TXL:
>case SHADER_OPCODE_TXS:
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 0465770..79f92d8 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -2545,7 +2545,8 @@ vec4_visitor::emit_texture(ir_texture_opcode op,
> case ir_txl: opcode = SHADER_OPCODE_TXL; break;
> case ir_txd: opcode = SHADER_OPCODE_TXD; break;
> case ir_txf: opcode = SHADER_OPCODE_TXF; break;
> -   case ir_txf_ms: opcode = SHADER_OPCODE_TXF_CMS; break;
> +   case ir_txf_ms: opcode = (devinfo->gen >= 9 ? SHADER_OPCODE_TXF_CMS_W :
> + SHADER_OPCODE_TXF_CMS); break;
> case ir_txs: opcode = SHADER_OPCODE_TXS; break;
> case ir_tg4: opcode = offset_value.file != BAD_FILE
>   ? SHADER_OPCODE_TG4_OFFSET : SHADER_OPCODE_TG4; 
> break;
> @@ -2637,7 +2638,16 @@ vec4_visitor::emit_texture(ir_texture_opcode op,
>} else if (op == ir_txf_ms) {
>   emit(MOV(dst_reg(MRF, param_base + 1, sample_index.type, 
> WRITEMASK_X),
>sample_index));
> - if (devinfo->gen >= 7) {
> + if (opcode == SHADER_OPCODE_TXF_CMS_W) {
> +/* MCS data is stored in the first two channels of ‘mcs’, but we
> + * need to get it into the .y and .z channels of the second vec4
> + * of params.
> + */

I don't understand what you mean by the "second vec4". Do you just mean the .y
and .z channel?

> +mcs.swizzle = BRW_SWIZZLE4(0, 0, 1, 1);

I don't really understand how the swizzle stuff works. Quick explanation on why
it's not BRW_SWIZZLE_XYXY would be greatly appreciated.

> +emit(MOV(dst_reg(MRF, param_base + 1,
> + glsl_type::uint_type, WRITEMASK_YZ),
> + mcs));
> + } else if (devinfo->gen >= 7) {
>  /* MCS data is in the first channel of `mcs`, but we need to get 
> it into
>   * the .y channel of the second vec4 of params, so replicate .x 
> across
>   * the whole vec4 and then mask off everything except .y
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 05/12] i965/vec4/skl+: Use lcd2dms_w instead of lcd2dms

[Chris Forbes]

s/lcd2dms/ld2dms/g in various places in this patch and others.

On Fri, Sep 18, 2015 at 4:00 AM, Neil Roberts  wrote:

> In order to support 16x MSAA, skl+ has a wider version of lcd2dms that
> takes two parameters for the MCS data. The MCS data in the response
> still fits in a single register so we just need to ensure we copy both
> values rather than just the lower one.
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp   |  1 +
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp |  5 +
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   | 14 --
>  3 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index ed49cd3..22c8d01 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -327,6 +327,7 @@ vec4_visitor::implied_mrf_writes(vec4_instruction
> *inst)
> case SHADER_OPCODE_TXD:
> case SHADER_OPCODE_TXF:
> case SHADER_OPCODE_TXF_CMS:
> +   case SHADER_OPCODE_TXF_CMS_W:
> case SHADER_OPCODE_TXF_MCS:
> case SHADER_OPCODE_TXS:
> case SHADER_OPCODE_TG4:
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index 1950333..c4ddff9 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -259,6 +259,10 @@ vec4_generator::generate_tex(vec4_instruction *inst,
>case SHADER_OPCODE_TXF:
>  msg_type = GEN5_SAMPLER_MESSAGE_SAMPLE_LD;
>  break;
> +  case SHADER_OPCODE_TXF_CMS_W:
> + assert(devinfo->gen >= 9);
> + msg_type = GEN9_SAMPLER_MESSAGE_SAMPLE_LD2DMS_W;
> + break;
>case SHADER_OPCODE_TXF_CMS:
>   if (devinfo->gen >= 7)
>  msg_type = GEN7_SAMPLER_MESSAGE_SAMPLE_LD2DMS;
> @@ -1372,6 +1376,7 @@ vec4_generator::generate_code(const cfg_t *cfg)
>case SHADER_OPCODE_TXD:
>case SHADER_OPCODE_TXF:
>case SHADER_OPCODE_TXF_CMS:
> +  case SHADER_OPCODE_TXF_CMS_W:
>case SHADER_OPCODE_TXF_MCS:
>case SHADER_OPCODE_TXL:
>case SHADER_OPCODE_TXS:
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 0465770..79f92d8 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -2545,7 +2545,8 @@ vec4_visitor::emit_texture(ir_texture_opcode op,
> case ir_txl: opcode = SHADER_OPCODE_TXL; break;
> case ir_txd: opcode = SHADER_OPCODE_TXD; break;
> case ir_txf: opcode = SHADER_OPCODE_TXF; break;
> -   case ir_txf_ms: opcode = SHADER_OPCODE_TXF_CMS; break;
> +   case ir_txf_ms: opcode = (devinfo->gen >= 9 ? SHADER_OPCODE_TXF_CMS_W :
> + SHADER_OPCODE_TXF_CMS); break;
> case ir_txs: opcode = SHADER_OPCODE_TXS; break;
> case ir_tg4: opcode = offset_value.file != BAD_FILE
>   ? SHADER_OPCODE_TG4_OFFSET : SHADER_OPCODE_TG4;
> break;
> @@ -2637,7 +2638,16 @@ vec4_visitor::emit_texture(ir_texture_opcode op,
>} else if (op == ir_txf_ms) {
>   emit(MOV(dst_reg(MRF, param_base + 1, sample_index.type,
> WRITEMASK_X),
>sample_index));
> - if (devinfo->gen >= 7) {
> + if (opcode == SHADER_OPCODE_TXF_CMS_W) {
> +/* MCS data is stored in the first two channels of ‘mcs’, but
> we
> + * need to get it into the .y and .z channels of the second
> vec4
> + * of params.
> + */
> +mcs.swizzle = BRW_SWIZZLE4(0, 0, 1, 1);
> +emit(MOV(dst_reg(MRF, param_base + 1,
> + glsl_type::uint_type, WRITEMASK_YZ),
> + mcs));
> + } else if (devinfo->gen >= 7) {
>  /* MCS data is in the first channel of `mcs`, but we need to
> get it into
>   * the .y channel of the second vec4 of params, so replicate
> .x across
>   * the whole vec4 and then mask off everything except .y
> --
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev