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

Katarzyna Dec
2019-09-18T08:35:03Z

On Tue, Sep 17, 2019 at 09:51:50PM -0700, Ashutosh Dixit wrote:
> I ran the kernel checkpatch on the patch and found a couple of minor
> issues. Perhaps it is a good idea to run checkpatch even on non-kernel
> patches in the future and fix up whatever can be fixed up.

I always run checkpatch before sending series or during review, but - there are
constraints with its output. Please keep in mind that this is a script to check
kernel code standards, not userspace.

>
> On Tue, 17 Sep 2019 02:13:51 -0700, Katarzyna Dec wrote:
> >
> > Adding gen12_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.
> >
> > v2: Switched to using IS_GEN12
> >
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> > Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> > Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > ---
> > lib/gpgpu_fill.c | 22 +++++++++++++++++++
> > lib/gpgpu_fill.h | 7 ++++++
> > lib/i915/shaders/README | 2 ++
> > lib/i915/shaders/gpgpu/gen12_gpgpu_kernel.asm | 12 ++++++++++
> > lib/intel_batchbuffer.c | 2 ++
> > 5 files changed, 45 insertions(+)
> > create mode 100644 lib/i915/shaders/gpgpu/gen12_gpgpu_kernel.asm
> >
> > diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
> > index 003f4616..9a48c645 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 gen12_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));
> > }
>
> CHECK: Please use a blank line after function/struct/union/enum declarations
>
I can change that.
> > +void gen12_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,
> > + gen12_gpgpu_kernel, sizeof(gen12_gpgpu_kernel));
> > +}
> > diff --git a/lib/gpgpu_fill.h b/lib/gpgpu_fill.h
> > index e405df3e..acee5bdc 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
> > +gen12_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);
>
> CHECK: Alignment should match open parenthesis
>
I can change that.

Thanks,
Kasia
> > +
> > #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 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/gen12_gpgpu_kernel.asm b/lib/i915/shaders/gpgpu/gen12_gpgpu_kernel.asm
> > new file mode 100644
> > index 00000000..ede87a05
> > --- /dev/null
> > +++ b/lib/i915/shaders/gpgpu/gen12_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..269c8982 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_GEN12(devid))
> > + fill = gen12_gpgpu_fillfunc;
> >
> > return fill;
> > }
> > --
> > 2.20.1
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev