[igt-dev] [PATCH i-g-t 1/2] lib/gpgpu_fill/tgl: Implement gpgpu_fillfunc for TGL

Arkadiusz Hiler arkadiusz.hiler at intel.com
Mon Sep 16 15:44:35 UTC 2019


On Fri, Sep 13, 2019 at 02:49:02PM +0200, Katarzyna Dec wrote:
> Adding gen12p1_gpgpu_fillfunc to have gpgpu_fill running on TGL.
> Gpgpu shader was generated using IGA (Intel Graphics Assembler)
> based on Gen11 binary adding necessary SWSB dependencies and
> changes in SEND instruction.
> 
> Shader source code and array containing its assembled version
> have same names now to avoid ambiguity.
> 
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Antonio Argenziano <antonio.argenziano at intel.com>
> Signed-off-by: Katarzyna Dec <katarzyna.dec at intel.com>
> Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz at intel.com>
> Signed-off-by: Zbigniew Kempczynski <zbigniew.kempczynski at intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  lib/gpgpu_fill.c                              | 22 +++++++++++++++++++
>  lib/gpgpu_fill.h                              |  7 ++++++
>  lib/i915/shaders/README                       |  2 ++
>  .../shaders/gpgpu/gen12p1_gpgpu_kernel.asm    | 12 ++++++++++
>  lib/intel_batchbuffer.c                       |  2 ++
>  5 files changed, 45 insertions(+)
>  create mode 100644 lib/i915/shaders/gpgpu/gen12p1_gpgpu_kernel.asm
> 
> diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
> index 003f4616..ec7204b8 100644
> --- a/lib/gpgpu_fill.c
> +++ b/lib/gpgpu_fill.c
> @@ -87,6 +87,19 @@ static const uint32_t gen11_gpgpu_kernel[][4] = {
>  	{ 0x07800031, 0x20000a40, 0x06000e00, 0x82000010 },
>  };
>  
> +static const uint32_t gen12p1_gpgpu_kernel[][4] = {
> +	{ 0x00020061, 0x01050000, 0x00000104, 0x00000000 },
> +	{ 0x00000069, 0x02058220, 0x02000024, 0x00000004 },
> +	{ 0x00000061, 0x02250220, 0x000000c4, 0x00000000 },
> +	{ 0x00030061, 0x04050220, 0x00460005, 0x00000000 },
> +	{ 0x00010261, 0x04050220, 0x00220205, 0x00000000 },
> +	{ 0x00000061, 0x04454220, 0x00000000, 0x0000000f },
> +	{ 0x00040661, 0x05050220, 0x00000104, 0x00000000 },
> +	{ 0x00049031, 0x00000000, 0xc0000414, 0x02a00000 },
> +	{ 0x00030061, 0x70050220, 0x00460005, 0x00000000 },
> +	{ 0x00040131, 0x00000004, 0x7020700c, 0x10000000 },
> +};
> +
>  /*
>   * This sets up the gpgpu pipeline,
>   *
> @@ -282,3 +295,12 @@ void gen11_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>  	__gen9_gpgpu_fillfunc(batch, dst, x, y, width, height, color,
>  			      gen11_gpgpu_kernel, sizeof(gen11_gpgpu_kernel));
>  }
> +void gen12p1_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> +			  const struct igt_buf *dst,
> +			  unsigned int x, unsigned int y,
> +			  unsigned int width, unsigned int height,
> +			  uint8_t color)
> +{
> +	__gen9_gpgpu_fillfunc(batch, dst, x, y, width, height, color,
> +			      gen12p1_gpgpu_kernel, sizeof(gen12p1_gpgpu_kernel));
> +}
> diff --git a/lib/gpgpu_fill.h b/lib/gpgpu_fill.h
> index e405df3e..91c7ef71 100644
> --- a/lib/gpgpu_fill.h
> +++ b/lib/gpgpu_fill.h
> @@ -57,4 +57,11 @@ gen11_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>  		     unsigned int width, unsigned int height,
>  		     uint8_t color);
>  
> +void
> +gen12p1_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> +		       const struct igt_buf *dst,
> +		       unsigned int x, unsigned int y,
> +		       unsigned int width, unsigned int height,
> +		       uint8_t color);
> +
>  #endif /* GPGPU_FILL_H */
> diff --git a/lib/i915/shaders/README b/lib/i915/shaders/README
> index 06b9883c..6f80111b 100644
> --- a/lib/i915/shaders/README
> +++ b/lib/i915/shaders/README
> @@ -23,3 +23,5 @@ Commands used to generate the shader on gen8
>  $> m4 media_fill.gxa > media_fill.gxm
>  $> intel-gen4asm -g 8 -o <output> media_fill.gxm
>  
> +Gen11+ shader is generated using IGA (Intel Graphics Assembler).
> +Binary can no longer be generated using intel-gen4asm.
> diff --git a/lib/i915/shaders/gpgpu/gen12p1_gpgpu_kernel.asm b/lib/i915/shaders/gpgpu/gen12p1_gpgpu_kernel.asm
> new file mode 100644
> index 00000000..ede87a05
> --- /dev/null
> +++ b/lib/i915/shaders/gpgpu/gen12p1_gpgpu_kernel.asm
> @@ -0,0 +1,12 @@
> +L0:
> +         mov (4|M0)               r1.0<1>:ub    r1.0<0;1,0>:ub
> +         shl (1|M0)               r2.0<1>:ud    r0.1<0;1,0>:ud    0x4:ud
> +         mov (1|M0)               r2.1<1>:ud    r0.6<0;1,0>:ud
> +         mov (8|M0)               r4.0<1>:ud    r0.0<8;8,1>:ud
> +         mov (2|M0)               r4.0<1>:ud    r2.0<2;2,1>:ud                   {@2}
> +         mov (1|M0)               r4.2<1>:ud    0xF:ud
> +         mov (16|M0)              r5.0<1>:ud    r1.0<0;1,0>:ud                   {@6}
> +         send.dc1 (16|M0)         null     r4      null    0x0         0x40A8000  {@1, $0} //    wr:2h+0, rd:0, Media Block Write msc:0, to #0
> +         mov (8|M0)               r112.0<1>:ud  r0.0<8;8,1>:ud
> +         send.ts (16|M0)          null     r112    null    0x10000000  0x2000010  {EOT, @1} //    wr:1+0, rd:0, fc: 0x10
> +L160:
> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> index 07de5cbb..7fac25b8 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -901,6 +901,8 @@ igt_fillfunc_t igt_get_gpgpu_fillfunc(int devid)
>  		fill = gen9_gpgpu_fillfunc;
>  	else if (IS_GEN11(devid))
>  		fill = gen11_gpgpu_fillfunc;
> +	else if (IS_TIGERLAKE(devid))
> +		fill = gen12p1_gpgpu_fillfunc;

I find this naming a bit odd.

If you don't want to overenable it for all GEN12 because you don't know
what the future brings I think we should go with tgl_*_fillfunc, so it's
closer to what we do in kernel (using platform codenames as function
prefixes). This way it isalso  direcly related to check
(IS_TIGERLAKE()).

If it proves to be generic then we can alway change the check to
IS_GEN12() or whatever and rename the function accordingly.

This applies to the media variant too.

Other than that looks good, let's see what the CI says.

-- 
Cheers,
Arek


More information about the igt-dev mailing list