Re: [igt-dev] [PATCH i-g-t] lib/rendercopy: Add gen4/5 rendercopy

["Kalamarz, Lukasz" <lukasz.kalamarz@intel.com>]

On Mon, 2018-06-11 at 19:14 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add rendercopy implementation for gen4/5. Basic structure
> copied from the gen6 implementation, 

After refactoring some part of rendercopy libs I don't like that
sentence :(

> and the gen4/5 specific
> bits were mostly lifted from sna.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/Makefile.sources    |   2 +
>  lib/gen4_render.h       | 628
> ++++++++++++++++++++++++++++++++++++++++++
>  lib/intel_batchbuffer.c |   2 +
>  lib/meson.build         |   1 +
>  lib/rendercopy.h        |   5 +
>  lib/rendercopy_gen4.c   | 704
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 1342 insertions(+)
>  create mode 100644 lib/gen4_render.h
>  create mode 100644 lib/rendercopy_gen4.c
> 
> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> index 042c1d3bb44a..e0ebd02c1661 100644
> --- a/lib/Makefile.sources
> +++ b/lib/Makefile.sources
> @@ -71,10 +71,12 @@ lib_source_list =	 	\
>  	gen8_media.h            \
>  	rendercopy_i915.c	\
>  	rendercopy_i830.c	\
> +	gen4_render.h		\
>  	gen6_render.h		\
>  	gen7_render.h		\
>  	gen8_render.h		\
>  	gen9_render.h		\
> +	rendercopy_gen4.c	\
>  	rendercopy_gen6.c	\
>  	rendercopy_gen7.c	\
>  	rendercopy_gen8.c	\
> diff --git a/lib/gen4_render.h b/lib/gen4_render.h
> new file mode 100644
> index 000000000000..ab1158e3c6d2
> --- /dev/null
> +++ b/lib/gen4_render.h

With having in mind refactoring of genX_render libs introduced in patch
series: https://patchwork.freedesktop.org/series/44624/ Could You check
if registers defined here with GEN4/5 prefix are reimplmented in
gen6_render? If so, then maybe it will be good idea to modify those
definitions and not add more duplicated definitions?

> @@ -0,0 +1,628 @@
> +#ifndef GEN4_RENDER_H
> +#define GEN4_RENDER_H

<snip>

> diff --git a/lib/rendercopy_gen4.c b/lib/rendercopy_gen4.c
> new file mode 100644
> index 000000000000..acd4be8de9da
> --- /dev/null
> +++ b/lib/rendercopy_gen4.c
> @@ -0,0 +1,704 @@
> +#include "rendercopy.h"
> +#include "intel_chipset.h"
> +#include "gen4_render.h"
> +#include "surfaceformat.h"
> +
> +#include <assert.h>
> +
> +#define VERTEX_SIZE (3*4)
> +
> +#define URB_VS_ENTRY_SIZE	1
> +#define URB_GS_ENTRY_SIZE	0
> +#define URB_CL_ENTRY_SIZE	0
> +#define URB_SF_ENTRY_SIZE	2
> +#define URB_CS_ENTRY_SIZE	1
> +
> +#define GEN4_GRF_BLOCKS(nreg) (((nreg) + 15) / 16 - 1)
> +#define SF_KERNEL_NUM_GRF 16
> +#define PS_KERNEL_NUM_GRF 32
> +
> +static const uint32_t gen4_sf_kernel_nomask[][4] = {
> +	{ 0x00400031, 0x20c01fbd, 0x0069002c, 0x01110001 },
> +	{ 0x00600001, 0x206003be, 0x00690060, 0x00000000 },
> +	{ 0x00600040, 0x20e077bd, 0x00690080, 0x006940a0 },
> +	{ 0x00600041, 0x202077be, 0x008d00e0, 0x000000c0 },
> +	{ 0x00600040, 0x20e077bd, 0x006900a0, 0x00694060 },
> +	{ 0x00600041, 0x204077be, 0x008d00e0, 0x000000c8 },
> +	{ 0x00600031, 0x20001fbc, 0x008d0000, 0x8640c800 },
> +};
> +
> +static const uint32_t gen5_sf_kernel_nomask[][4] = {
> +	{ 0x00400031, 0x20c01fbd, 0x1069002c, 0x02100001 },
> +	{ 0x00600001, 0x206003be, 0x00690060, 0x00000000 },
> +	{ 0x00600040, 0x20e077bd, 0x00690080, 0x006940a0 },
> +	{ 0x00600041, 0x202077be, 0x008d00e0, 0x000000c0 },
> +	{ 0x00600040, 0x20e077bd, 0x006900a0, 0x00694060 },
> +	{ 0x00600041, 0x204077be, 0x008d00e0, 0x000000c8 },
> +	{ 0x00600031, 0x20001fbc, 0x648d0000, 0x8808c800 },
> +};
> +
> +static const uint32_t gen4_ps_kernel_nomask_affine[][4] = {
> +	{ 0x00800040, 0x23c06d29, 0x00480028, 0x10101010 },
> +	{ 0x00800040, 0x23806d29, 0x0048002a, 0x11001100 },
> +	{ 0x00802040, 0x2100753d, 0x008d03c0, 0x00004020 },
> +	{ 0x00802040, 0x2140753d, 0x008d0380, 0x00004024 },
> +	{ 0x00802059, 0x200077bc, 0x00000060, 0x008d0100 },
> +	{ 0x00802048, 0x204077be, 0x00000064, 0x008d0140 },
> +	{ 0x00802059, 0x200077bc, 0x00000070, 0x008d0100 },
> +	{ 0x00802048, 0x208077be, 0x00000074, 0x008d0140 },
> +	{ 0x00600201, 0x20200022, 0x008d0000, 0x00000000 },
> +	{ 0x00000201, 0x20280062, 0x00000000, 0x00000000 },
> +	{ 0x01800031, 0x21801d09, 0x008d0000, 0x02580001 },
> +	{ 0x00600001, 0x204003be, 0x008d0180, 0x00000000 },
> +	{ 0x00601001, 0x20c003be, 0x008d01a0, 0x00000000 },
> +	{ 0x00600001, 0x206003be, 0x008d01c0, 0x00000000 },
> +	{ 0x00601001, 0x20e003be, 0x008d01e0, 0x00000000 },
> +	{ 0x00600001, 0x208003be, 0x008d0200, 0x00000000 },
> +	{ 0x00601001, 0x210003be, 0x008d0220, 0x00000000 },
> +	{ 0x00600001, 0x20a003be, 0x008d0240, 0x00000000 },
> +	{ 0x00601001, 0x212003be, 0x008d0260, 0x00000000 },
> +	{ 0x00600201, 0x202003be, 0x008d0020, 0x00000000 },
> +	{ 0x00800031, 0x20001d28, 0x008d0000, 0x85a04800 },
> +};
> +
> +static const uint32_t gen5_ps_kernel_nomask_affine[][4] = {
> +	{ 0x00800040, 0x23c06d29, 0x00480028, 0x10101010 },
> +	{ 0x00800040, 0x23806d29, 0x0048002a, 0x11001100 },
> +	{ 0x00802040, 0x2100753d, 0x008d03c0, 0x00004020 },
> +	{ 0x00802040, 0x2140753d, 0x008d0380, 0x00004024 },
> +	{ 0x00802059, 0x200077bc, 0x00000060, 0x008d0100 },
> +	{ 0x00802048, 0x204077be, 0x00000064, 0x008d0140 },
> +	{ 0x00802059, 0x200077bc, 0x00000070, 0x008d0100 },
> +	{ 0x00802048, 0x208077be, 0x00000074, 0x008d0140 },
> +	{ 0x01800031, 0x21801fa9, 0x208d0000, 0x0a8a0001 },
> +	{ 0x00802001, 0x304003be, 0x008d0180, 0x00000000 },
> +	{ 0x00802001, 0x306003be, 0x008d01c0, 0x00000000 },
> +	{ 0x00802001, 0x308003be, 0x008d0200, 0x00000000 },
> +	{ 0x00802001, 0x30a003be, 0x008d0240, 0x00000000 },
> +	{ 0x00600201, 0x202003be, 0x008d0020, 0x00000000 },
> +	{ 0x00800031, 0x20001d28, 0x548d0000, 0x94084800 },
> +};
> +
> +static uint32_t
> +batch_used(struct intel_batchbuffer *batch)
> +{
> +	return batch->ptr - batch->buffer;
> +}
> +
> +static uint32_t
> +batch_round_upto(struct intel_batchbuffer *batch, uint32_t divisor)
> +{
> +	uint32_t offset = batch_used(batch);
> +	offset = (offset + divisor - 1) / divisor * divisor;
> +	batch->ptr = batch->buffer + offset;
> +	return offset;
> +}

With the usage of the same methods in two libs (gen4 and gen6), maybe
it will be worth to move those functions to intel_batchbuffer (like it
was done with previous copy/pasted functions)?

<snip>

----
Lukasz
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev