Re: [Mesa-dev] [PATCH v2 05/20] i965: Use <0, 2, 1> region for scalar DF sources on IVB/BYT.

[Samuel Iglesias Gonsálvez]

On Wed, 2017-01-18 at 11:39 -0800, Matt Turner wrote:
> On Wed, Jan 18, 2017 at 2:45 AM, Samuel Iglesias Gonsálvez
>  wrote:
> > On Tue, 2017-01-17 at 13:33 -0800, Francisco Jerez wrote:
> > > Samuel Iglesias Gonsálvez  writes:
> > > 
> > > > From: Matt Turner 
> > > > 
> > > > On HSW+, scalar DF sources can be accessed using the normal
> > > > <0,1,0>
> > > > region, but on IVB and BYT DF regions must be programmed in
> > > > terms
> > > > of
> > > > floats. A <0,2,1> region accomplishes this.
> > > 
> > > Any reason you're doing this here twice instead of during fs_reg
> > > to
> > > brw_reg conversion?
> > > 
> > 
> > I have modified locally this patch to do it in
> > brw_reg_from_fs_reg().
> > Matt, if you agree with Curro's suggestion, I can keep the new
> > version
> > in my branch instead of this.
> 
> I found your patch. Looks good to me.
> 
> I just have one request: order the comparisons and assignments as
> vstride, width, hstride -- the same way they appear in the 
> regioning notation.
> 

OK, thanks Matt.

Sam

signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 05/20] i965: Use <0, 2, 1> region for scalar DF sources on IVB/BYT.

[Matt Turner]

On Wed, Jan 18, 2017 at 2:45 AM, Samuel Iglesias Gonsálvez
 wrote:
> On Tue, 2017-01-17 at 13:33 -0800, Francisco Jerez wrote:
>> Samuel Iglesias Gonsálvez  writes:
>>
>> > From: Matt Turner 
>> >
>> > On HSW+, scalar DF sources can be accessed using the normal <0,1,0>
>> > region, but on IVB and BYT DF regions must be programmed in terms
>> > of
>> > floats. A <0,2,1> region accomplishes this.
>>
>> Any reason you're doing this here twice instead of during fs_reg to
>> brw_reg conversion?
>>
>
> I have modified locally this patch to do it in brw_reg_from_fs_reg().
> Matt, if you agree with Curro's suggestion, I can keep the new version
> in my branch instead of this.

I found your patch. Looks good to me.

I just have one request: order the comparisons and assignments as
vstride, width, hstride -- the same way they appear in the 
regioning notation.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 05/20] i965: Use <0, 2, 1> region for scalar DF sources on IVB/BYT.

[Matt Turner]

On Tue, Jan 17, 2017 at 1:33 PM, Francisco Jerez  wrote:
> Samuel Iglesias Gonsálvez  writes:
>
>> From: Matt Turner 
>>
>> On HSW+, scalar DF sources can be accessed using the normal <0,1,0>
>> region, but on IVB and BYT DF regions must be programmed in terms of
>> floats. A <0,2,1> region accomplishes this.
>
> Any reason you're doing this here twice instead of during fs_reg to
> brw_reg conversion?

I think the reason I didn't do it that way is that sources of type
FIXED_GRF and VGRF both need the fix up, and those are handled by
different code paths in brw_reg_from_fs_reg() (and FIXED_GRF shares a
codepath with ARF and IMM). Fixing it here also handles the case for
the vec4 backend, if that's ever a problem.

I don't have a preference.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 05/20] i965: Use <0, 2, 1> region for scalar DF sources on IVB/BYT.

[Samuel Iglesias Gonsálvez]

On Tue, 2017-01-17 at 13:33 -0800, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez  writes:
> 
> > From: Matt Turner 
> > 
> > On HSW+, scalar DF sources can be accessed using the normal <0,1,0>
> > region, but on IVB and BYT DF regions must be programmed in terms
> > of
> > floats. A <0,2,1> region accomplishes this.
> 
> Any reason you're doing this here twice instead of during fs_reg to
> brw_reg conversion?
> 

I have modified locally this patch to do it in brw_reg_from_fs_reg().
Matt, if you agree with Curro's suggestion, I can keep the new version
in my branch instead of this.

Sam

> > ---
> >  src/mesa/drivers/dri/i965/brw_eu_emit.c | 26 -
> > -
> >  1 file changed, 20 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > index 05c097f66ef..3201c885cb9 100644
> > --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > @@ -444,9 +444,16 @@ brw_set_src0(struct brw_codegen *p, brw_inst
> > *inst, struct brw_reg reg)
> >  brw_inst_set_src0_width(devinfo, inst, BRW_WIDTH_1);
> >  brw_inst_set_src0_vstride(devinfo, inst,
> > BRW_VERTICAL_STRIDE_0);
> >      } else {
> > -brw_inst_set_src0_hstride(devinfo, inst, reg.hstride);
> > -brw_inst_set_src0_width(devinfo, inst, reg.width);
> > -brw_inst_set_src0_vstride(devinfo, inst, reg.vstride);
> > +if (devinfo->gen == 7 && !devinfo->is_haswell &&
> > +reg.type == BRW_REGISTER_TYPE_DF &&
> > has_scalar_region(reg)) {
> > +   brw_inst_set_src0_vstride(devinfo, inst,
> > BRW_VERTICAL_STRIDE_0);
> > +   brw_inst_set_src0_width(devinfo, inst,
> > BRW_WIDTH_2);
> > +   brw_inst_set_src0_hstride(devinfo, inst,
> > BRW_HORIZONTAL_STRIDE_1);
> > +} else {
> > +   brw_inst_set_src0_vstride(devinfo, inst,
> > reg.vstride);
> > +   brw_inst_set_src0_width(devinfo, inst, reg.width);
> > +   brw_inst_set_src0_hstride(devinfo, inst,
> > reg.hstride);
> > +}
> >      }
> >    } else {
> >   brw_inst_set_src0_da16_swiz_x(devinfo, inst,
> > @@ -526,9 +533,16 @@ brw_set_src1(struct brw_codegen *p, brw_inst
> > *inst, struct brw_reg reg)
> >  brw_inst_set_src1_width(devinfo, inst, BRW_WIDTH_1);
> >  brw_inst_set_src1_vstride(devinfo, inst,
> > BRW_VERTICAL_STRIDE_0);
> >      } else {
> > -brw_inst_set_src1_hstride(devinfo, inst, reg.hstride);
> > -brw_inst_set_src1_width(devinfo, inst, reg.width);
> > -brw_inst_set_src1_vstride(devinfo, inst, reg.vstride);
> > +if (devinfo->gen == 7 && !devinfo->is_haswell &&
> > +reg.type == BRW_REGISTER_TYPE_DF &&
> > has_scalar_region(reg)) {
> > +   brw_inst_set_src1_vstride(devinfo, inst,
> > BRW_VERTICAL_STRIDE_0);
> > +   brw_inst_set_src1_width(devinfo, inst,
> > BRW_WIDTH_2);
> > +   brw_inst_set_src1_hstride(devinfo, inst,
> > BRW_HORIZONTAL_STRIDE_1);
> > +} else {
> > +   brw_inst_set_src1_vstride(devinfo, inst,
> > reg.vstride);
> > +   brw_inst_set_src1_width(devinfo, inst, reg.width);
> > +   brw_inst_set_src1_hstride(devinfo, inst,
> > reg.hstride);
> > +}
> >      }
> >    } else {
> >   brw_inst_set_src1_da16_swiz_x(devinfo, inst,
> > -- 
> > 2.11.0
> > 
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev

signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 05/20] i965: Use <0, 2, 1> region for scalar DF sources on IVB/BYT.

[Francisco Jerez]

Samuel Iglesias Gonsálvez  writes:

> From: Matt Turner 
>
> On HSW+, scalar DF sources can be accessed using the normal <0,1,0>
> region, but on IVB and BYT DF regions must be programmed in terms of
> floats. A <0,2,1> region accomplishes this.

Any reason you're doing this here twice instead of during fs_reg to
brw_reg conversion?

> ---
>  src/mesa/drivers/dri/i965/brw_eu_emit.c | 26 --
>  1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index 05c097f66ef..3201c885cb9 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -444,9 +444,16 @@ brw_set_src0(struct brw_codegen *p, brw_inst *inst, 
> struct brw_reg reg)
>  brw_inst_set_src0_width(devinfo, inst, BRW_WIDTH_1);
>  brw_inst_set_src0_vstride(devinfo, inst, BRW_VERTICAL_STRIDE_0);
>} else {
> -brw_inst_set_src0_hstride(devinfo, inst, reg.hstride);
> -brw_inst_set_src0_width(devinfo, inst, reg.width);
> -brw_inst_set_src0_vstride(devinfo, inst, reg.vstride);
> +if (devinfo->gen == 7 && !devinfo->is_haswell &&
> +reg.type == BRW_REGISTER_TYPE_DF && has_scalar_region(reg)) {
> +   brw_inst_set_src0_vstride(devinfo, inst, 
> BRW_VERTICAL_STRIDE_0);
> +   brw_inst_set_src0_width(devinfo, inst, BRW_WIDTH_2);
> +   brw_inst_set_src0_hstride(devinfo, inst, 
> BRW_HORIZONTAL_STRIDE_1);
> +} else {
> +   brw_inst_set_src0_vstride(devinfo, inst, reg.vstride);
> +   brw_inst_set_src0_width(devinfo, inst, reg.width);
> +   brw_inst_set_src0_hstride(devinfo, inst, reg.hstride);
> +}
>}
>} else {
>   brw_inst_set_src0_da16_swiz_x(devinfo, inst,
> @@ -526,9 +533,16 @@ brw_set_src1(struct brw_codegen *p, brw_inst *inst, 
> struct brw_reg reg)
>  brw_inst_set_src1_width(devinfo, inst, BRW_WIDTH_1);
>  brw_inst_set_src1_vstride(devinfo, inst, BRW_VERTICAL_STRIDE_0);
>} else {
> -brw_inst_set_src1_hstride(devinfo, inst, reg.hstride);
> -brw_inst_set_src1_width(devinfo, inst, reg.width);
> -brw_inst_set_src1_vstride(devinfo, inst, reg.vstride);
> +if (devinfo->gen == 7 && !devinfo->is_haswell &&
> +reg.type == BRW_REGISTER_TYPE_DF && has_scalar_region(reg)) {
> +   brw_inst_set_src1_vstride(devinfo, inst, 
> BRW_VERTICAL_STRIDE_0);
> +   brw_inst_set_src1_width(devinfo, inst, BRW_WIDTH_2);
> +   brw_inst_set_src1_hstride(devinfo, inst, 
> BRW_HORIZONTAL_STRIDE_1);
> +} else {
> +   brw_inst_set_src1_vstride(devinfo, inst, reg.vstride);
> +   brw_inst_set_src1_width(devinfo, inst, reg.width);
> +   brw_inst_set_src1_hstride(devinfo, inst, reg.hstride);
> +}
>}
>} else {
>   brw_inst_set_src1_da16_swiz_x(devinfo, inst,
> -- 
> 2.11.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev