[igt-dev] [PATCH i-g-t v5 3/3] i915/gem_render_copy.c: Simplify code by switch to rendercopy bufmgr

[Chris Wilson]

Quoting Zbigniew Kempczyński (2020-01-08 07:54:11)
> Switch to rendercopy bufmgr to simplify working with tiled surfaces.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Katarzyna Dec <katarzyna.dec at intel.com>
> ---
>  lib/intel_batchbuffer.c      |  47 +++++
>  lib/intel_batchbuffer.h      |   2 +
>  tests/i915/gem_render_copy.c | 370 +++++++----------------------------
>  3 files changed, 122 insertions(+), 297 deletions(-)
> 
> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> index 3dc89024..c23e5e48 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -43,6 +43,7 @@
>  #include "ioctl_wrappers.h"
>  #include "media_spin.h"
>  #include "gpgpu_fill.h"
> +#include "igt_aux.h"
>  
>  #include <i915_drm.h>
>  
> @@ -529,6 +530,52 @@ unsigned igt_buf_height(const struct igt_buf *buf)
>         return buf->surface[0].size/buf->surface[0].stride;
>  }
>  
> +/**
> + * igt_buf_ccs_width:
> + * @buf: the i-g-t buffer object
> + * @gen: device generation
> + *
> + * Computes the width of ccs buffer data.
> + *
> + * Returns:
> + * The width of the ccs buffer data.
> + */
> +unsigned int igt_buf_ccs_width(int gen, const struct igt_buf *buf)
> +{
> +       /*
> +        * GEN12+: The CCS unit size is 64 bytes mapping 4 main surface
> +        * tiles. Thus the width of the CCS unit is 4*32=128 pixels on the
> +        * main surface.
> +        */
> +       if (gen >= 12)
> +               return DIV_ROUND_UP(igt_buf_width(buf), 128) * 64;

This should not be igt_, since it is not vendor neutral.

Atm we have
	intel_: uses libdrm_intel (deprecated)
	gem_: uses DRM_I915_GEM ioctls (bad prefix, needs to be i915_gem?)
	rendercopy_: sounds generic, could be, but isn't really atm :)
	igt_kms_: mostly generic, at least tries to support equivalent
	          paths on different vendors

I would probably also change igt_buf to igt_image for the
implication of it supporting multidimension [tiled] surfaces, but igt_
is a good prefix for a generic handle + surface info.

I'm tempted to move all the mediacopy, gpgpucopy, rendercopy under
lib/gpucopy/ and hope one day we have a generic interface, but really
not sure if that's ever going to be required. (When we get to cross
vendor behaviour, we're better off looking at Vulkan or some other
lowlevel generic api, at which point we are clearly outside the scope of
ioctl ABI testing.) So, I'd suggest moving these to lib/i915/gpucopy.
-Chris