[igt-dev] [PATCH i-g-t v3 00/21] Chamelium VC4 plane fuzzy testing, with SAND and T-tiled mode

[igt-dev] [PATCH i-g-t v3 00/21] Chamelium VC4 plane fuzzy testing, with SAND and T-tiled mode

[Paul Kocialkowski]

This series introduces the required plumbing for allocating buffers in
the VC4 T-tiled and SAND_tiled modes, along with helpers to convert to
these tiling modes.

A new "checkerboard" comparison method is introduced to check whether
captured frames match their references in cases where pixel-perfect
results are not possible (e.g. with scaling) and the analog test does
not provide reliable results.

With that in place, new tests for the Chamelium are introduced to
perform testing of planes with randomized properties, both with a
CRC and frame comparison fashion.

This series is based on version 3 of Maxime Ripard's series:
  igt: chamelium: Test YUV buffers using the Chamelium

Changes since v2:
* Added support for SAND tiling;
* Generalized T-tiling to support both 16 and 32 bpp;
* Split the main test into more sub-functions;
* Removed explicit lists of DRM formats;
* Added helper to list known DRM formats;
* Various functional fixes in the main test;
* Removed unnecessary pointer init in map_bo;
* Rebased on v3 of Maxime Ripard's YUV series;
* Added collected Reviewed-By tags;

Changes since v1:
* Added collected Reviewed-by tags;
* Removed explicit size and constified tiling read order tables;
* Removed unnecessary MAP_FAILED addition in map_bo;
* Added pipe and output helpers to count and iterate planes of a given type;
* Moved CRC debug prints to the chamelium library;
* Fixed destination stride description for igt_fb_convert_with_stride;
* Split planes test into a few sub-routines;
* Removed explicit underrun detection since general error reporting is enough;
* Removed underrun detection bits due to ongoing Chamelium-specific issues;
* Deconfigured planes before removing their framebuffer, avoiding kmsg errors.

Paul Kocialkowski (21):
  lib: drmtest: Add helpers to check and require the VC4 driver
  lib/igt_fb: Add checks on i915 for i915-specific tiled buffer
    allocation
  lib/igt_fb: Add support for allocating T-tiled VC4 buffers
  lib/igt_fb: Add support for VC4 SAND tiling modes
  lib/igt_fb: Allow interpreting the tile height as a stride equivalent
  lib/igt_fb: Add a stride-provisioned fashion of igt_fb_convert
  lib/igt_fb: Add a helper to retreive the plane bpp for a given format
  lib/igt_fb: Add a helper to fill-in the available DRM formats
  lib/igt_vc4: Add helpers for converting linear to T-tiled RGB buffers
  lib/igt_vc4: Add helper for checking T-tiling support on plane
  lib/igt_vc4: Add helpers for converting linear to SAND-tiled buffers
  lib/igt_vc4: Add helper for checking SAND tiling support on plane
  lib/igt_kms: Add helpers to count and iterate planes from pipe
  lib/igt_kms: Add helpers to count and iterate planes from output
  lib/igt_chamelium: Fixup resources liberation in comparison helpers
  lib/igt_chamelium: Split frames dump logic and rework surroundings
  lib/igt_chamelium: Generalize the frame match helper with check type
  lib/igt_frame: Add a checkerboard frame comparison method
  lib/igt_chamelium: Hook-in checkerboard comparison method in helpers
  chamelium: Move the YUV tests over to the checkerboard checking method
  chamelium: Add a display test for randomized planes

 lib/drmtest.c         |  10 +
 lib/drmtest.h         |   2 +
 lib/igt_chamelium.c   | 124 +++++++----
 lib/igt_chamelium.h   |  15 +-
 lib/igt_fb.c          | 144 ++++++++++++-
 lib/igt_fb.h          |   6 +
 lib/igt_frame.c       | 121 +++++++++++
 lib/igt_frame.h       |   2 +
 lib/igt_kms.c         |  84 ++++++++
 lib/igt_kms.h         |   6 +
 lib/igt_vc4.c         | 242 +++++++++++++++++++++
 lib/igt_vc4.h         |  42 ++++
 tests/kms_chamelium.c | 481 ++++++++++++++++++++++++++++++++++++++++--
 13 files changed, 1204 insertions(+), 75 deletions(-)

-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v3 01/21] lib: drmtest: Add helpers to check and require the VC4 driver

[Paul Kocialkowski]

In order to add support for features specific to the VC4 driver, add
helpers for checking and requiring the driver like it's done for the
i915 driver.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/drmtest.c | 10 ++++++++++
 lib/drmtest.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 35914c502432..f7a93c2419c5 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -105,6 +105,11 @@ bool is_i915_device(int fd)
 	return __is_device(fd, "i915");
 }
 
+bool is_vc4_device(int fd)
+{
+	return __is_device(fd, "vc4");
+}
+
 static bool has_known_intel_chipset(int fd)
 {
 	struct drm_i915_getparam gp;
@@ -476,3 +481,8 @@ void igt_require_intel(int fd)
 {
 	igt_require(is_i915_device(fd) && has_known_intel_chipset(fd));
 }
+
+void igt_require_vc4(int fd)
+{
+	igt_require(is_vc4_device(fd));
+}
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 8743b1bb1ed3..ca347a7120db 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -80,8 +80,10 @@ int __drm_open_driver(int chipset);
 void gem_quiescent_gpu(int fd);
 
 void igt_require_intel(int fd);
+void igt_require_vc4(int fd);
 
 bool is_i915_device(int fd);
+bool is_vc4_device(int fd);
 
 /**
  * do_or_die:
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v3 02/21] lib/igt_fb: Add checks on i915 for i915-specific tiled buffer allocation

[Paul Kocialkowski]

The code path for allocating tiled buffers has a few i915-specific bits
without checks for the i915 driver. Add these missing checks.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_fb.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 9ceeb824fff4..9ecd10b8cde1 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -364,6 +364,7 @@ static uint32_t calc_plane_stride(struct igt_fb *fb, int plane)
 		(fb->plane_bpp[plane] / 8);
 
 	if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
+	    is_i915_device(fb->fd) &&
 	    intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
 		uint32_t stride;
 
@@ -392,6 +393,7 @@ static uint32_t calc_plane_stride(struct igt_fb *fb, int plane)
 static uint64_t calc_plane_size(struct igt_fb *fb, int plane)
 {
 	if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
+	    is_i915_device(fb->fd) &&
 	    intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
 		uint64_t min_size = (uint64_t) fb->strides[plane] *
 			fb->plane_height[plane];
@@ -1488,9 +1490,11 @@ static void *map_bo(int fd, struct igt_fb *fb)
 	if (fb->is_dumb)
 		ptr = kmstest_dumb_map_buffer(fd, fb->gem_handle, fb->size,
 					      PROT_READ | PROT_WRITE);
-	else
+	else if (is_i915_device(fd))
 		ptr = gem_mmap__gtt(fd, fb->gem_handle, fb->size,
 				    PROT_READ | PROT_WRITE);
+	else
+		igt_assert(false);
 
 	return ptr;
 }
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 02/21] lib/igt_fb: Add checks on i915 for i915-specific tiled buffer allocation

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 439 bytes --]

On Fri, Jan 11, 2019 at 10:05:13AM +0100, Paul Kocialkowski wrote:
> The code path for allocating tiled buffers has a few i915-specific bits
> without checks for the i915 driver. Add these missing checks.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 02/21] lib/igt_fb: Add checks on i915 for i915-specific tiled buffer allocation

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> The code path for allocating tiled buffers has a few i915-specific bits
> without checks for the i915 driver. Add these missing checks.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_fb.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 9ceeb824fff4..9ecd10b8cde1 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -364,6 +364,7 @@ static uint32_t calc_plane_stride(struct igt_fb *fb, int
> plane)
>  		(fb->plane_bpp[plane] / 8);
>  
>  	if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> +	    is_i915_device(fb->fd) &&
>  	    intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
>  		uint32_t stride;
>  
> @@ -392,6 +393,7 @@ static uint32_t calc_plane_stride(struct igt_fb *fb, int
> plane)
>  static uint64_t calc_plane_size(struct igt_fb *fb, int plane)
>  {
>  	if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> +	    is_i915_device(fb->fd) &&
>  	    intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
>  		uint64_t min_size = (uint64_t) fb->strides[plane] *
>  			fb->plane_height[plane];
> @@ -1488,9 +1490,11 @@ static void *map_bo(int fd, struct igt_fb *fb)
>  	if (fb->is_dumb)
>  		ptr = kmstest_dumb_map_buffer(fd, fb->gem_handle, fb->size,
>  					      PROT_READ | PROT_WRITE);
> -	else
> +	else if (is_i915_device(fd))
>  		ptr = gem_mmap__gtt(fd, fb->gem_handle, fb->size,
>  				    PROT_READ | PROT_WRITE);
> +	else
> +		igt_assert(false);
>  
>  	return ptr;
>  }
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 03/21] lib/igt_fb: Add support for allocating T-tiled VC4 buffers

[Paul Kocialkowski]

This introduces the required bits for allocating buffers with a T-tiled
disposition, that is specific to the VC4. It includes calculating the
top-tile width and creating a buffer object with the VC4-specific
helper. The tiling flag is set to the buffer object so that this can
be reused for GPU tests if needed later.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_fb.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 9ecd10b8cde1..73f33a26a81f 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -37,6 +37,7 @@
 #include "igt_fb.h"
 #include "igt_kms.h"
 #include "igt_matrix.h"
+#include "igt_vc4.h"
 #include "igt_x86.h"
 #include "ioctl_wrappers.h"
 #include "intel_batchbuffer.h"
@@ -237,6 +238,9 @@ static const struct format_desc_struct *lookup_drm_format(uint32_t drm_format)
 void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
 			  unsigned *width_ret, unsigned *height_ret)
 {
+	if (is_vc4_device(fd))
+		tiling = fourcc_mod_broadcom_mod(tiling);
+
 	switch (tiling) {
 	case LOCAL_DRM_FORMAT_MOD_NONE:
 		if (is_i915_device(fd))
@@ -290,6 +294,11 @@ void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
 			igt_assert(false);
 		}
 		break;
+	case DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED:
+		igt_require_vc4(fd);
+		*width_ret = 128;
+		*height_ret = 32;
+		break;
 	default:
 		igt_assert(false);
 	}
@@ -581,6 +590,13 @@ static int create_bo_for_fb(struct igt_fb *fb)
 			       igt_fb_mod_to_tiling(fb->tiling),
 			       fb->strides[0]);
 
+		goto out;
+	} else if (is_vc4_device(fd)) {
+		fb->gem_handle = igt_vc4_create_bo(fd, fb->size);
+
+		if (fb->tiling == DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED)
+			igt_vc4_set_tiling(fd, fb->gem_handle, fb->tiling);
+
 		goto out;
 	}
 
@@ -1493,6 +1509,9 @@ static void *map_bo(int fd, struct igt_fb *fb)
 	else if (is_i915_device(fd))
 		ptr = gem_mmap__gtt(fd, fb->gem_handle, fb->size,
 				    PROT_READ | PROT_WRITE);
+	else if (is_vc4_device(fd))
+		ptr = igt_vc4_mmap_bo(fd, fb->gem_handle, fb->size,
+				      PROT_READ | PROT_WRITE);
 	else
 		igt_assert(false);
 
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 03/21] lib/igt_fb: Add support for allocating T-tiled VC4 buffers

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 632 bytes --]

On Fri, Jan 11, 2019 at 10:05:14AM +0100, Paul Kocialkowski wrote:
> This introduces the required bits for allocating buffers with a T-tiled
> disposition, that is specific to the VC4. It includes calculating the
> top-tile width and creating a buffer object with the VC4-specific
> helper. The tiling flag is set to the buffer object so that this can
> be reused for GPU tests if needed later.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 03/21] lib/igt_fb: Add support for allocating T-tiled VC4 buffers

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> This introduces the required bits for allocating buffers with a T-tiled
> disposition, that is specific to the VC4. It includes calculating the
> top-tile width and creating a buffer object with the VC4-specific
> helper. The tiling flag is set to the buffer object so that this can
> be reused for GPU tests if needed later.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_fb.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 9ecd10b8cde1..73f33a26a81f 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -37,6 +37,7 @@
>  #include "igt_fb.h"
>  #include "igt_kms.h"
>  #include "igt_matrix.h"
> +#include "igt_vc4.h"
>  #include "igt_x86.h"
>  #include "ioctl_wrappers.h"
>  #include "intel_batchbuffer.h"
> @@ -237,6 +238,9 @@ static const struct format_desc_struct
> *lookup_drm_format(uint32_t drm_format)
>  void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
>  			  unsigned *width_ret, unsigned *height_ret)
>  {
> +	if (is_vc4_device(fd))
> +		tiling = fourcc_mod_broadcom_mod(tiling);
> +
>  	switch (tiling) {
>  	case LOCAL_DRM_FORMAT_MOD_NONE:
>  		if (is_i915_device(fd))
> @@ -290,6 +294,11 @@ void igt_get_fb_tile_size(int fd, uint64_t tiling, int
> fb_bpp,
>  			igt_assert(false);
>  		}
>  		break;
> +	case DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED:
> +		igt_require_vc4(fd);
> +		*width_ret = 128;
> +		*height_ret = 32;
> +		break;
>  	default:
>  		igt_assert(false);
>  	}
> @@ -581,6 +590,13 @@ static int create_bo_for_fb(struct igt_fb *fb)
>  			       igt_fb_mod_to_tiling(fb->tiling),
>  			       fb->strides[0]);
>  
> +		goto out;
> +	} else if (is_vc4_device(fd)) {
> +		fb->gem_handle = igt_vc4_create_bo(fd, fb->size);
> +
> +		if (fb->tiling == DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED)
> +			igt_vc4_set_tiling(fd, fb->gem_handle, fb->tiling);
> +
>  		goto out;
>  	}
>  
> @@ -1493,6 +1509,9 @@ static void *map_bo(int fd, struct igt_fb *fb)
>  	else if (is_i915_device(fd))
>  		ptr = gem_mmap__gtt(fd, fb->gem_handle, fb->size,
>  				    PROT_READ | PROT_WRITE);
> +	else if (is_vc4_device(fd))
> +		ptr = igt_vc4_mmap_bo(fd, fb->gem_handle, fb->size,
> +				      PROT_READ | PROT_WRITE);
>  	else
>  		igt_assert(false);
>  
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 04/21] lib/igt_fb: Add support for VC4 SAND tiling modes

[Paul Kocialkowski]

This introduces support for the VC4 SAND tiling modes, that take a
specific parameter indicating their column height. This parameter acts
as a height-based stride equivalent, that shall be equal or greater
than the displayed height.

The parameter is extracted and returned as tile height so that enough
memory can be reserved for column heights containing extra padding.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_fb.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 73f33a26a81f..7fb8e7d44bd2 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -238,8 +238,12 @@ static const struct format_desc_struct *lookup_drm_format(uint32_t drm_format)
 void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
 			  unsigned *width_ret, unsigned *height_ret)
 {
-	if (is_vc4_device(fd))
+	uint32_t vc4_tiling_param = 0;
+
+	if (is_vc4_device(fd)) {
+		vc4_tiling_param = fourcc_mod_broadcom_param(tiling);
 		tiling = fourcc_mod_broadcom_mod(tiling);
+	}
 
 	switch (tiling) {
 	case LOCAL_DRM_FORMAT_MOD_NONE:
@@ -299,6 +303,26 @@ void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
 		*width_ret = 128;
 		*height_ret = 32;
 		break;
+	case DRM_FORMAT_MOD_BROADCOM_SAND32:
+		igt_require_vc4(fd);
+		*width_ret = 32;
+		*height_ret = vc4_tiling_param;
+		break;
+	case DRM_FORMAT_MOD_BROADCOM_SAND64:
+		igt_require_vc4(fd);
+		*width_ret = 64;
+		*height_ret = vc4_tiling_param;
+		break;
+	case DRM_FORMAT_MOD_BROADCOM_SAND128:
+		igt_require_vc4(fd);
+		*width_ret = 128;
+		*height_ret = vc4_tiling_param;
+		break;
+	case DRM_FORMAT_MOD_BROADCOM_SAND256:
+		igt_require_vc4(fd);
+		*width_ret = 256;
+		*height_ret = vc4_tiling_param;
+		break;
 	default:
 		igt_assert(false);
 	}
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 04/21] lib/igt_fb: Add support for VC4 SAND tiling modes

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 692 bytes --]

On Fri, Jan 11, 2019 at 10:05:15AM +0100, Paul Kocialkowski wrote:
> This introduces support for the VC4 SAND tiling modes, that take a
> specific parameter indicating their column height. This parameter acts
> as a height-based stride equivalent, that shall be equal or greater
> than the displayed height.
> 
> The parameter is extracted and returned as tile height so that enough
> memory can be reserved for column heights containing extra padding.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 04/21] lib/igt_fb: Add support for VC4 SAND tiling modes

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> This introduces support for the VC4 SAND tiling modes, that take a
> specific parameter indicating their column height. This parameter acts
> as a height-based stride equivalent, that shall be equal or greater
> than the displayed height.
> 
> The parameter is extracted and returned as tile height so that enough
> memory can be reserved for column heights containing extra padding.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_fb.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 73f33a26a81f..7fb8e7d44bd2 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -238,8 +238,12 @@ static const struct format_desc_struct
> *lookup_drm_format(uint32_t drm_format)
>  void igt_get_fb_tile_size(int fd, uint64_t tiling, int fb_bpp,
>  			  unsigned *width_ret, unsigned *height_ret)
>  {
> -	if (is_vc4_device(fd))
> +	uint32_t vc4_tiling_param = 0;
> +
> +	if (is_vc4_device(fd)) {
> +		vc4_tiling_param = fourcc_mod_broadcom_param(tiling);
>  		tiling = fourcc_mod_broadcom_mod(tiling);
> +	}
>  
>  	switch (tiling) {
>  	case LOCAL_DRM_FORMAT_MOD_NONE:
> @@ -299,6 +303,26 @@ void igt_get_fb_tile_size(int fd, uint64_t tiling, int
> fb_bpp,
>  		*width_ret = 128;
>  		*height_ret = 32;
>  		break;
> +	case DRM_FORMAT_MOD_BROADCOM_SAND32:
> +		igt_require_vc4(fd);
> +		*width_ret = 32;
> +		*height_ret = vc4_tiling_param;
> +		break;
> +	case DRM_FORMAT_MOD_BROADCOM_SAND64:
> +		igt_require_vc4(fd);
> +		*width_ret = 64;
> +		*height_ret = vc4_tiling_param;
> +		break;
> +	case DRM_FORMAT_MOD_BROADCOM_SAND128:
> +		igt_require_vc4(fd);
> +		*width_ret = 128;
> +		*height_ret = vc4_tiling_param;
> +		break;
> +	case DRM_FORMAT_MOD_BROADCOM_SAND256:
> +		igt_require_vc4(fd);
> +		*width_ret = 256;
> +		*height_ret = vc4_tiling_param;
> +		break;
>  	default:
>  		igt_assert(false);
>  	}
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 05/21] lib/igt_fb: Allow interpreting the tile height as a stride equivalent

[Paul Kocialkowski]

The VC4 SAND tiling modes are disposed in columns that follow each
other in memory. The column height defines the number of fixed-width
lines from the beginning of one column to the other, which may be
greater than the display height. In this case, the extra lines are
used as padding and the column height becomes a height-based stride
equivalent.

Support this when calculating the plane size by using the tile height
directly if it is greater than the plane height. This works better than
alignment for non-power-of-two cases (no space is wasted) and it is
equivalent to alignment for power-of-two tile heights.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_fb.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 7fb8e7d44bd2..03a6b8be9618 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -450,6 +450,13 @@ static uint64_t calc_plane_size(struct igt_fb *fb, int plane)
 		igt_get_fb_tile_size(fb->fd, fb->tiling, fb->plane_bpp[plane],
 				     &tile_width, &tile_height);
 
+		/* Special case where the "tile height" represents a
+		 * height-based stride, such as with VC4 SAND tiling modes.
+		 */
+
+		if (tile_height > fb->plane_height[plane])
+			return fb->strides[plane] * tile_height;
+
 		return (uint64_t) fb->strides[plane] *
 			ALIGN(fb->plane_height[plane], tile_height);
 	}
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 05/21] lib/igt_fb: Allow interpreting the tile height as a stride equivalent

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 947 bytes --]

On Fri, Jan 11, 2019 at 10:05:16AM +0100, Paul Kocialkowski wrote:
> The VC4 SAND tiling modes are disposed in columns that follow each
> other in memory. The column height defines the number of fixed-width
> lines from the beginning of one column to the other, which may be
> greater than the display height. In this case, the extra lines are
> used as padding and the column height becomes a height-based stride
> equivalent.
> 
> Support this when calculating the plane size by using the tile height
> directly if it is greater than the plane height. This works better than
> alignment for non-power-of-two cases (no space is wasted) and it is
> equivalent to alignment for power-of-two tile heights.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 05/21] lib/igt_fb: Allow interpreting the tile height as a stride equivalent

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> The VC4 SAND tiling modes are disposed in columns that follow each
> other in memory. The column height defines the number of fixed-width
> lines from the beginning of one column to the other, which may be
> greater than the display height. In this case, the extra lines are
> used as padding and the column height becomes a height-based stride
> equivalent.
> 
> Support this when calculating the plane size by using the tile height
> directly if it is greater than the plane height. This works better than
> alignment for non-power-of-two cases (no space is wasted) and it is
> equivalent to alignment for power-of-two tile heights.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_fb.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 7fb8e7d44bd2..03a6b8be9618 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -450,6 +450,13 @@ static uint64_t calc_plane_size(struct igt_fb *fb, int
> plane)
>  		igt_get_fb_tile_size(fb->fd, fb->tiling, fb->plane_bpp[plane],
>  				     &tile_width, &tile_height);
>  
> +		/* Special case where the "tile height" represents a
> +		 * height-based stride, such as with VC4 SAND tiling modes.
> +		 */
> +
> +		if (tile_height > fb->plane_height[plane])
> +			return fb->strides[plane] * tile_height;
> +
>  		return (uint64_t) fb->strides[plane] *
>  			ALIGN(fb->plane_height[plane], tile_height);
>  	}
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 06/21] lib/igt_fb: Add a stride-provisioned fashion of igt_fb_convert

[Paul Kocialkowski]

The current implementation of igt_fb_convert does not allow passing
the destination stride, which is something we want to change for tests.

Add a new fashion of this function that allocates the desintation buffer
with a given stride. Since the current function does the same thing with
an unspecified stride (set to zero, which will be filled later), make it
call our new fashion with the stride set to zero to avoid duplication.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 38 ++++++++++++++++++++++++++++++++------
 lib/igt_fb.h |  3 +++
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 03a6b8be9618..0b10cc4a0d3e 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2233,14 +2233,15 @@ void igt_remove_fb(int fd, struct igt_fb *fb)
 }
 
 /**
- * igt_fb_convert:
+ * igt_fb_convert_with_stride:
  * @dst: pointer to the #igt_fb structure that will store the conversion result
  * @src: pointer to the #igt_fb structure that stores the frame we convert
  * @dst_fourcc: DRM format specifier to convert to
+ * @dst_stride: Stride for the resulting framebuffer (0 for automatic stride)
  *
  * This will convert a given @src content to the @dst_fourcc format,
  * storing the result in the @dst fb, allocating the @dst fb
- * underlying buffer.
+ * underlying buffer with a stride of @dst_stride stride.
  *
  * Once done with @dst, the caller will have to call igt_remove_fb()
  * on it to free the associated resources.
@@ -2248,15 +2249,18 @@ void igt_remove_fb(int fd, struct igt_fb *fb)
  * Returns:
  * The kms id of the created framebuffer.
  */
-unsigned int igt_fb_convert(struct igt_fb *dst, struct igt_fb *src,
-			    uint32_t dst_fourcc)
+unsigned int igt_fb_convert_with_stride(struct igt_fb *dst, struct igt_fb *src,
+					uint32_t dst_fourcc,
+					unsigned int dst_stride)
 {
 	struct fb_convert cvt = { };
 	void *dst_ptr, *src_ptr;
 	int fb_id;
 
-	fb_id = igt_create_fb(src->fd, src->width, src->height,
-			      dst_fourcc, LOCAL_DRM_FORMAT_MOD_NONE, dst);
+	fb_id = igt_create_fb_with_bo_size(src->fd, src->width, src->height,
+					   dst_fourcc,
+					   LOCAL_DRM_FORMAT_MOD_NONE,
+					   dst, 0, dst_stride);
 	igt_assert(fb_id > 0);
 
 	src_ptr = igt_fb_map_buffer(src->fd, src);
@@ -2277,6 +2281,28 @@ unsigned int igt_fb_convert(struct igt_fb *dst, struct igt_fb *src,
 	return fb_id;
 }
 
+/**
+ * igt_fb_convert:
+ * @dst: pointer to the #igt_fb structure that will store the conversion result
+ * @src: pointer to the #igt_fb structure that stores the frame we convert
+ * @dst_fourcc: DRM format specifier to convert to
+ *
+ * This will convert a given @src content to the @dst_fourcc format,
+ * storing the result in the @dst fb, allocating the @dst fb
+ * underlying buffer.
+ *
+ * Once done with @dst, the caller will have to call igt_remove_fb()
+ * on it to free the associated resources.
+ *
+ * Returns:
+ * The kms id of the created framebuffer.
+ */
+unsigned int igt_fb_convert(struct igt_fb *dst, struct igt_fb *src,
+			    uint32_t dst_fourcc)
+{
+	return igt_fb_convert_with_stride(dst, src, dst_fourcc, 0);
+}
+
 /**
  * igt_bpp_depth_to_drm_format:
  * @bpp: desired bits per pixel
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 9f027deba842..cb21fdbc77a3 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -130,6 +130,9 @@ unsigned int igt_create_image_fb(int drm_fd,  int width, int height,
 				 struct igt_fb *fb /* out */);
 unsigned int igt_create_stereo_fb(int drm_fd, drmModeModeInfo *mode,
 				  uint32_t format, uint64_t tiling);
+unsigned int igt_fb_convert_with_stride(struct igt_fb *dst, struct igt_fb *src,
+					uint32_t dst_fourcc,
+					unsigned int stride);
 unsigned int igt_fb_convert(struct igt_fb *dst, struct igt_fb *src,
 			    uint32_t dst_fourcc);
 void igt_remove_fb(int fd, struct igt_fb *fb);
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v3 07/21] lib/igt_fb: Add a helper to retreive the plane bpp for a given format

[Paul Kocialkowski]

The format bpp for a given plane is stored in the static format_desc
structure and is not accessible to tests, which is inconvenient to
get the minimum stride for a format when testing various strides.

Add a simple helper to expose the information.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 lib/igt_fb.c | 16 ++++++++++++++++
 lib/igt_fb.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 0b10cc4a0d3e..86ceb028bdae 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2404,3 +2404,19 @@ bool igt_format_is_yuv(uint32_t drm_format)
 		return false;
 	}
 }
+
+/**
+ * igt_format_plane_bpp:
+ * @drm_format: drm fourcc
+ * @plane: format plane index
+ *
+ * This functions returns the number of bits per pixel for the given @plane
+ * index of the @drm_format.
+ */
+int igt_format_plane_bpp(uint32_t drm_format, int plane)
+{
+	const struct format_desc_struct *format =
+		lookup_drm_format(drm_format);
+
+	return format->plane_bpp[plane];
+}
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index cb21fdbc77a3..a39c4d2fe30e 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -175,6 +175,7 @@ uint32_t igt_drm_format_to_bpp(uint32_t drm_format);
 const char *igt_format_str(uint32_t drm_format);
 bool igt_fb_supported_format(uint32_t drm_format);
 bool igt_format_is_yuv(uint32_t drm_format);
+int igt_format_plane_bpp(uint32_t drm_format, int plane);
 
 #endif /* __IGT_FB_H__ */
 
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t v3 08/21] lib/igt_fb: Add a helper to fill-in the available DRM formats

[Paul Kocialkowski]

Introduce a helper to allocate and fill-in a list of available DRM
formats, which is useful for picking one at random in tests.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_fb.c | 34 ++++++++++++++++++++++++++++++++++
 lib/igt_fb.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 86ceb028bdae..8adf3295df56 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2420,3 +2420,37 @@ int igt_format_plane_bpp(uint32_t drm_format, int plane)
 
 	return format->plane_bpp[plane];
 }
+
+/**
+ * igt_format_array_fill:
+ * @formats_array: a pointer to the formats array pointer to be allocated
+ * @count: a pointer to the number of elements contained in the allocated array
+ * @allow_yuv: a boolean indicating whether YUV formats should be included
+ *
+ * This functions allocates and fills a @formats_array that lists the DRM
+ * formats current available.
+ */
+void igt_format_array_fill(uint32_t **formats_array, unsigned int *count,
+			   bool allow_yuv)
+{
+	const struct format_desc_struct *format;
+	unsigned int index = 0;
+
+	*count = 0;
+
+	for_each_format(format) {
+		if (!allow_yuv && igt_format_is_yuv(format->drm_id))
+			continue;
+
+		(*count)++;
+	}
+
+	*formats_array = calloc(*count, sizeof(uint32_t));
+
+	for_each_format(format) {
+		if (!allow_yuv && igt_format_is_yuv(format->drm_id))
+			continue;
+
+		(*formats_array)[index++] = format->drm_id;
+	}
+}
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index a39c4d2fe30e..c6edddec1d76 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -176,6 +176,8 @@ const char *igt_format_str(uint32_t drm_format);
 bool igt_fb_supported_format(uint32_t drm_format);
 bool igt_format_is_yuv(uint32_t drm_format);
 int igt_format_plane_bpp(uint32_t drm_format, int plane);
+void igt_format_array_fill(uint32_t **formats_array, unsigned int *count,
+			   bool allow_yuv);
 
 #endif /* __IGT_FB_H__ */
 
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 08/21] lib/igt_fb: Add a helper to fill-in the available DRM formats

[Lyude Paul]

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> Introduce a helper to allocate and fill-in a list of available DRM
> formats, which is useful for picking one at random in tests.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_fb.c | 34 ++++++++++++++++++++++++++++++++++
>  lib/igt_fb.h |  2 ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 86ceb028bdae..8adf3295df56 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -2420,3 +2420,37 @@ int igt_format_plane_bpp(uint32_t drm_format, int
> plane)
>  
>  	return format->plane_bpp[plane];
>  }
> +
> +/**
> + * igt_format_array_fill:
> + * @formats_array: a pointer to the formats array pointer to be allocated
> + * @count: a pointer to the number of elements contained in the allocated
> array
> + * @allow_yuv: a boolean indicating whether YUV formats should be included
> + *
> + * This functions allocates and fills a @formats_array that lists the DRM
> + * formats current available.
> + */
> +void igt_format_array_fill(uint32_t **formats_array, unsigned int *count,
> +			   bool allow_yuv)
> +{
> +	const struct format_desc_struct *format;
> +	unsigned int index = 0;
> +
> +	*count = 0;
> +
> +	for_each_format(format) {
> +		if (!allow_yuv && igt_format_is_yuv(format->drm_id))
> +			continue;
> +
> +		(*count)++;
> +	}
> +
> +	*formats_array = calloc(*count, sizeof(uint32_t));
Probably a good idea to put an igt_assert() after this to make sure that
formats_array got allocated properly. With that fixed:

Reviewed-by: Lyude Paul <lyude@redhat.com>
> +
> +	for_each_format(format) {
> +		if (!allow_yuv && igt_format_is_yuv(format->drm_id))
> +			continue;
> +
> +		(*formats_array)[index++] = format->drm_id;
> +	}
> +}
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index a39c4d2fe30e..c6edddec1d76 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -176,6 +176,8 @@ const char *igt_format_str(uint32_t drm_format);
>  bool igt_fb_supported_format(uint32_t drm_format);
>  bool igt_format_is_yuv(uint32_t drm_format);
>  int igt_format_plane_bpp(uint32_t drm_format, int plane);
> +void igt_format_array_fill(uint32_t **formats_array, unsigned int *count,
> +			   bool allow_yuv);
>  
>  #endif /* __IGT_FB_H__ */
>  
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 09/21] lib/igt_vc4: Add helpers for converting linear to T-tiled RGB buffers

[Paul Kocialkowski]

In order to integrate testing of T-tiled buffers, the easiest path is to
generate patterns (with the already-existing functions) in linear
buffers and convert them to T-tiled subsequently.

Add helpers to do that conversion, with a first helper that returns the
memory offset for a given position in a T-tiled buffer and a second
helper that uses it for converting between framebuffers.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_vc4.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_vc4.h |   4 ++
 2 files changed, 135 insertions(+)

diff --git a/lib/igt_vc4.c b/lib/igt_vc4.c
index 16dfe67a44b1..b4b9c2fc6914 100644
--- a/lib/igt_vc4.c
+++ b/lib/igt_vc4.c
@@ -34,6 +34,7 @@
 #include "drmtest.h"
 #include "igt_aux.h"
 #include "igt_core.h"
+#include "igt_fb.h"
 #include "igt_vc4.h"
 #include "ioctl_wrappers.h"
 #include "intel_reg.h"
@@ -176,3 +177,133 @@ bool igt_vc4_purgeable_bo(int fd, int handle, bool purgeable)
 
 	return arg.retained;
 }
+
+unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct igt_fb *src)
+{
+	unsigned int fb_id;
+	unsigned int i, j;
+	void *src_buf;
+	void *dst_buf;
+	size_t bpp = src->plane_bpp[0];
+	size_t dst_stride = ALIGN(src->strides[0], 128);
+
+	fb_id = igt_create_fb_with_bo_size(src->fd, src->width, src->height,
+					   src->drm_format,
+					   DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED,
+					   dst, 0, dst_stride);
+	igt_assert(fb_id > 0);
+
+	src_buf = igt_fb_map_buffer(src->fd, src);
+	igt_assert(src_buf);
+
+	dst_buf = igt_fb_map_buffer(dst->fd, dst);
+	igt_assert(dst_buf);
+
+	for (i = 0; i < src->height; i++) {
+		for (j = 0; j < src->width; j++) {
+			size_t src_offset = src->offsets[0];
+			size_t dst_offset = dst->offsets[0];
+
+			src_offset += src->strides[0] * i + j * bpp / 8;
+			dst_offset += igt_vc4_t_tiled_offset(dst_stride,
+							     src->height,
+							     bpp, j, i);
+
+			switch (bpp) {
+			case 16:
+				*(uint16_t *)(dst_buf + dst_offset) =
+					*(uint16_t *)(src_buf + src_offset);
+				break;
+			case 32:
+				*(uint32_t *)(dst_buf + dst_offset) =
+					*(uint32_t *)(src_buf + src_offset);
+				break;
+			default:
+				igt_assert(false);
+			}
+		}
+	}
+
+	igt_fb_unmap_buffer(src, src_buf);
+	igt_fb_unmap_buffer(dst, dst_buf);
+
+	return fb_id;
+}
+
+/* Calculate the t-tile width so that size = width * height * bpp / 8. */
+#define VC4_T_TILE_W(size, height, bpp) ((size) / (height) / ((bpp) / 8))
+
+size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
+			      size_t x, size_t y)
+{
+	const size_t t1k_map_even[] = { 0, 3, 1, 2 };
+	const size_t t1k_map_odd[] = { 2, 1, 3, 0 };
+	const size_t t4k_t_h = 32;
+	const size_t t1k_t_h = 16;
+	const size_t t64_t_h = 4;
+	size_t offset = 0;
+	size_t t4k_t_w, t4k_w, t4k_x, t4k_y;
+	size_t t1k_t_w, t1k_x, t1k_y;
+	size_t t64_t_w, t64_x, t64_y;
+	size_t pix_x, pix_y;
+	unsigned int index;
+
+	/* T-tiling is only supported for 16 and 32 bpp. */
+	igt_assert(bpp == 16 || bpp == 32);
+
+	/* T-tiling stride must be aligned to the 4K tiles strides. */
+	igt_assert((stride % (4096 / t4k_t_h)) == 0);
+
+	/* Calculate the tile width for the bpp. */
+	t4k_t_w = VC4_T_TILE_W(4096, t4k_t_h, bpp);
+	t1k_t_w = VC4_T_TILE_W(1024, t1k_t_h, bpp);
+	t64_t_w = VC4_T_TILE_W(64, t64_t_h, bpp);
+
+	/* Aligned total width in number of 4K tiles. */
+	t4k_w = (stride / (bpp / 8)) / t4k_t_w;
+
+	/* X and y coordinates in number of 4K tiles. */
+	t4k_x = x / t4k_t_w;
+	t4k_y = y / t4k_t_h;
+
+	/* Increase offset to the beginning of the 4K tile row. */
+	offset += t4k_y * t4k_w * 4096;
+
+	/* X and Y coordinates in number of 1K tiles within the 4K tile. */
+	t1k_x = (x % t4k_t_w) / t1k_t_w;
+	t1k_y = (y % t4k_t_h) / t1k_t_h;
+
+	/* Index for 1K tile map lookup. */
+	index = 2 * t1k_y + t1k_x;
+
+	/* Odd rows start from the right, even rows from the left. */
+	if (t4k_y % 2) {
+		/* Increase offset to the 4K tile (starting from the right). */
+		offset += (t4k_w - t4k_x - 1) * 4096;
+
+		/* Incrase offset to the beginning of the (odd) 1K tile. */
+		offset += t1k_map_odd[index] * 1024;
+	} else {
+		/* Increase offset to the 4K tile (starting from the left). */
+		offset += t4k_x * 4096;
+
+		/* Incrase offset to the beginning of the (even) 1K tile. */
+		offset += t1k_map_even[index] * 1024;
+	}
+
+	/* X and Y coordinates in number of 64 byte tiles within the 1K tile. */
+	t64_x = (x % t1k_t_w) / t64_t_w;
+	t64_y = (y % t1k_t_h) / t64_t_h;
+
+	/* Increase offset to the beginning of the 64-byte tile. */
+	offset += (t64_y * (t1k_t_w / t64_t_w) + t64_x) * 64;
+
+	/* X and Y coordinates in number of pixels within the 64-byte tile. */
+	pix_x = x % t64_t_w;
+	pix_y = y % t64_t_h;
+
+	/* Increase offset to the correct pixel. */
+	offset += (pix_y * t64_t_w + pix_x) * bpp / 8;
+
+	return offset;
+}
diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
index ebc8a3881b5e..d5c529bbccda 100644
--- a/lib/igt_vc4.h
+++ b/lib/igt_vc4.h
@@ -33,4 +33,8 @@ bool igt_vc4_purgeable_bo(int fd, int handle, bool purgeable);
 void igt_vc4_set_tiling(int fd, uint32_t handle, uint64_t modifier);
 uint64_t igt_vc4_get_tiling(int fd, uint32_t handle);
 
+unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct igt_fb *src);
+size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
+			      size_t x, size_t y);
+
 #endif /* IGT_VC4_H */
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 09/21] lib/igt_vc4: Add helpers for converting linear to T-tiled RGB buffers

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 892 bytes --]

On Fri, Jan 11, 2019 at 10:05:20AM +0100, Paul Kocialkowski wrote:
> In order to integrate testing of T-tiled buffers, the easiest path is to
> generate patterns (with the already-existing functions) in linear
> buffers and convert them to T-tiled subsequently.
> 
> Add helpers to do that conversion, with a first helper that returns the
> memory offset for a given position in a T-tiled buffer and a second
> helper that uses it for converting between framebuffers.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

I'd really like to plug that in igt_convert. The modifiers are part of
of how an image is represented, so it should really be plugged in
igt_fb_convert, as a second step after the format conversion.

This also applies to your patch 11.

maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 09/21] lib/igt_vc4: Add helpers for converting linear to T-tiled RGB buffers

[Paul Kocialkowski]

Hi,

On Fri, 2019-01-11 at 16:25 +0100, Maxime Ripard wrote:
> On Fri, Jan 11, 2019 at 10:05:20AM +0100, Paul Kocialkowski wrote:
> > In order to integrate testing of T-tiled buffers, the easiest path is to
> > generate patterns (with the already-existing functions) in linear
> > buffers and convert them to T-tiled subsequently.
> > 
> > Add helpers to do that conversion, with a first helper that returns the
> > memory offset for a given position in a T-tiled buffer and a second
> > helper that uses it for converting between framebuffers.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> 
> I'd really like to plug that in igt_convert. The modifiers are part of
> of how an image is represented, so it should really be plugged in
> igt_fb_convert, as a second step after the format conversion.
> 
> This also applies to your patch 11.

Sounds like a very good thing to do indeed!

I think having tiling-specific functions for converting a plane would
be a good fit, since everything above that can certainly be made
generic.

I'll include an implementation of that in the next version of the
series.

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

Re: [igt-dev] [PATCH i-g-t v3 09/21] lib/igt_vc4: Add helpers for converting linear to T-tiled RGB buffers

[Lyude Paul]

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> In order to integrate testing of T-tiled buffers, the easiest path is to
> generate patterns (with the already-existing functions) in linear
> buffers and convert them to T-tiled subsequently.
> 
> Add helpers to do that conversion, with a first helper that returns the
> memory offset for a given position in a T-tiled buffer and a second
> helper that uses it for converting between framebuffers.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_vc4.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_vc4.h |   4 ++
>  2 files changed, 135 insertions(+)
> 
> diff --git a/lib/igt_vc4.c b/lib/igt_vc4.c
> index 16dfe67a44b1..b4b9c2fc6914 100644
> --- a/lib/igt_vc4.c
> +++ b/lib/igt_vc4.c
> @@ -34,6 +34,7 @@
>  #include "drmtest.h"
>  #include "igt_aux.h"
>  #include "igt_core.h"
> +#include "igt_fb.h"
>  #include "igt_vc4.h"
>  #include "ioctl_wrappers.h"
>  #include "intel_reg.h"
> @@ -176,3 +177,133 @@ bool igt_vc4_purgeable_bo(int fd, int handle, bool
> purgeable)
>  
>  	return arg.retained;
>  }
> +
> +unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct igt_fb
> *src)
> +{
> +	unsigned int fb_id;
> +	unsigned int i, j;
> +	void *src_buf;
> +	void *dst_buf;
> +	size_t bpp = src->plane_bpp[0];
> +	size_t dst_stride = ALIGN(src->strides[0], 128);
> +
> +	fb_id = igt_create_fb_with_bo_size(src->fd, src->width, src->height,
> +					   src->drm_format,
> +					   DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED
> ,
> +					   dst, 0, dst_stride);
> +	igt_assert(fb_id > 0);
> +
> +	src_buf = igt_fb_map_buffer(src->fd, src);
> +	igt_assert(src_buf);
> +
> +	dst_buf = igt_fb_map_buffer(dst->fd, dst);
> +	igt_assert(dst_buf);
> +
> +	for (i = 0; i < src->height; i++) {
> +		for (j = 0; j < src->width; j++) {
> +			size_t src_offset = src->offsets[0];
> +			size_t dst_offset = dst->offsets[0];
> +
> +			src_offset += src->strides[0] * i + j * bpp / 8;
> +			dst_offset += igt_vc4_t_tiled_offset(dst_stride,
> +							     src->height,
> +							     bpp, j, i);
> +
> +			switch (bpp) {
> +			case 16:
> +				*(uint16_t *)(dst_buf + dst_offset) =
> +					*(uint16_t *)(src_buf + src_offset);
> +				break;
> +			case 32:
> +				*(uint32_t *)(dst_buf + dst_offset) =
> +					*(uint32_t *)(src_buf + src_offset);
> +				break;
> +			default:
Maybe we should also have the two igt_fb_unmap_buffer() calls below this get
invoked before calling igt_assert(false) so we don't leak FBs if this fails

With that fixed:

Acked-by: Lyude Paul <lyude@redhat.com>
> +				igt_assert(false);
> +			}
> +		}
> +	}
> +
> +	igt_fb_unmap_buffer(src, src_buf);
> +	igt_fb_unmap_buffer(dst, dst_buf);
> +
> +	return fb_id;
> +}
> +
> +/* Calculate the t-tile width so that size = width * height * bpp / 8. */
> +#define VC4_T_TILE_W(size, height, bpp) ((size) / (height) / ((bpp) / 8))
> +
> +size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
> +			      size_t x, size_t y)
> +{
> +	const size_t t1k_map_even[] = { 0, 3, 1, 2 };
> +	const size_t t1k_map_odd[] = { 2, 1, 3, 0 };
> +	const size_t t4k_t_h = 32;
> +	const size_t t1k_t_h = 16;
> +	const size_t t64_t_h = 4;
> +	size_t offset = 0;
> +	size_t t4k_t_w, t4k_w, t4k_x, t4k_y;
> +	size_t t1k_t_w, t1k_x, t1k_y;
> +	size_t t64_t_w, t64_x, t64_y;
> +	size_t pix_x, pix_y;
> +	unsigned int index;
> +
> +	/* T-tiling is only supported for 16 and 32 bpp. */
> +	igt_assert(bpp == 16 || bpp == 32);
> +
> +	/* T-tiling stride must be aligned to the 4K tiles strides. */
> +	igt_assert((stride % (4096 / t4k_t_h)) == 0);
> +
> +	/* Calculate the tile width for the bpp. */
> +	t4k_t_w = VC4_T_TILE_W(4096, t4k_t_h, bpp);
> +	t1k_t_w = VC4_T_TILE_W(1024, t1k_t_h, bpp);
> +	t64_t_w = VC4_T_TILE_W(64, t64_t_h, bpp);
> +
> +	/* Aligned total width in number of 4K tiles. */
> +	t4k_w = (stride / (bpp / 8)) / t4k_t_w;
> +
> +	/* X and y coordinates in number of 4K tiles. */
> +	t4k_x = x / t4k_t_w;
> +	t4k_y = y / t4k_t_h;
> +
> +	/* Increase offset to the beginning of the 4K tile row. */
> +	offset += t4k_y * t4k_w * 4096;
> +
> +	/* X and Y coordinates in number of 1K tiles within the 4K tile. */
> +	t1k_x = (x % t4k_t_w) / t1k_t_w;
> +	t1k_y = (y % t4k_t_h) / t1k_t_h;
> +
> +	/* Index for 1K tile map lookup. */
> +	index = 2 * t1k_y + t1k_x;
> +
> +	/* Odd rows start from the right, even rows from the left. */
> +	if (t4k_y % 2) {
> +		/* Increase offset to the 4K tile (starting from the right).
> */
> +		offset += (t4k_w - t4k_x - 1) * 4096;
> +
> +		/* Incrase offset to the beginning of the (odd) 1K tile. */
> +		offset += t1k_map_odd[index] * 1024;
> +	} else {
> +		/* Increase offset to the 4K tile (starting from the left). */
> +		offset += t4k_x * 4096;
> +
> +		/* Incrase offset to the beginning of the (even) 1K tile. */
> +		offset += t1k_map_even[index] * 1024;
> +	}
> +
> +	/* X and Y coordinates in number of 64 byte tiles within the 1K tile.
> */
> +	t64_x = (x % t1k_t_w) / t64_t_w;
> +	t64_y = (y % t1k_t_h) / t64_t_h;
> +
> +	/* Increase offset to the beginning of the 64-byte tile. */
> +	offset += (t64_y * (t1k_t_w / t64_t_w) + t64_x) * 64;
> +
> +	/* X and Y coordinates in number of pixels within the 64-byte tile. */
> +	pix_x = x % t64_t_w;
> +	pix_y = y % t64_t_h;
> +
> +	/* Increase offset to the correct pixel. */
> +	offset += (pix_y * t64_t_w + pix_x) * bpp / 8;
> +
> +	return offset;
> +}
> diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> index ebc8a3881b5e..d5c529bbccda 100644
> --- a/lib/igt_vc4.h
> +++ b/lib/igt_vc4.h
> @@ -33,4 +33,8 @@ bool igt_vc4_purgeable_bo(int fd, int handle, bool
> purgeable);
>  void igt_vc4_set_tiling(int fd, uint32_t handle, uint64_t modifier);
>  uint64_t igt_vc4_get_tiling(int fd, uint32_t handle);
>  
> +unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct igt_fb
> *src);
> +size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
> +			      size_t x, size_t y);
> +
>  #endif /* IGT_VC4_H */
-- 
Cheers,
	Lyude Paul

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

Re: [igt-dev] [PATCH i-g-t v3 09/21] lib/igt_vc4: Add helpers for converting linear to T-tiled RGB buffers

[Paul Kocialkowski]

Hi and thanks for the review,

On Tue, 2019-01-15 at 16:20 -0500, Lyude Paul wrote:
> On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> > In order to integrate testing of T-tiled buffers, the easiest path is to
> > generate patterns (with the already-existing functions) in linear
> > buffers and convert them to T-tiled subsequently.
> > 
> > Add helpers to do that conversion, with a first helper that returns the
> > memory offset for a given position in a T-tiled buffer and a second
> > helper that uses it for converting between framebuffers.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  lib/igt_vc4.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/igt_vc4.h |   4 ++
> >  2 files changed, 135 insertions(+)
> > 
> > diff --git a/lib/igt_vc4.c b/lib/igt_vc4.c
> > index 16dfe67a44b1..b4b9c2fc6914 100644
> > --- a/lib/igt_vc4.c
> > +++ b/lib/igt_vc4.c
> > @@ -34,6 +34,7 @@
> >  #include "drmtest.h"
> >  #include "igt_aux.h"
> >  #include "igt_core.h"
> > +#include "igt_fb.h"
> >  #include "igt_vc4.h"
> >  #include "ioctl_wrappers.h"
> >  #include "intel_reg.h"
> > @@ -176,3 +177,133 @@ bool igt_vc4_purgeable_bo(int fd, int handle, bool
> > purgeable)
> >  
> >  	return arg.retained;
> >  }
> > +
> > +unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct igt_fb
> > *src)
> > +{
> > +	unsigned int fb_id;
> > +	unsigned int i, j;
> > +	void *src_buf;
> > +	void *dst_buf;
> > +	size_t bpp = src->plane_bpp[0];
> > +	size_t dst_stride = ALIGN(src->strides[0], 128);
> > +
> > +	fb_id = igt_create_fb_with_bo_size(src->fd, src->width, src->height,
> > +					   src->drm_format,
> > +					   DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED
> > ,
> > +					   dst, 0, dst_stride);
> > +	igt_assert(fb_id > 0);
> > +
> > +	src_buf = igt_fb_map_buffer(src->fd, src);
> > +	igt_assert(src_buf);
> > +
> > +	dst_buf = igt_fb_map_buffer(dst->fd, dst);
> > +	igt_assert(dst_buf);
> > +
> > +	for (i = 0; i < src->height; i++) {
> > +		for (j = 0; j < src->width; j++) {
> > +			size_t src_offset = src->offsets[0];
> > +			size_t dst_offset = dst->offsets[0];
> > +
> > +			src_offset += src->strides[0] * i + j * bpp / 8;
> > +			dst_offset += igt_vc4_t_tiled_offset(dst_stride,
> > +							     src->height,
> > +							     bpp, j, i);
> > +
> > +			switch (bpp) {
> > +			case 16:
> > +				*(uint16_t *)(dst_buf + dst_offset) =
> > +					*(uint16_t *)(src_buf + src_offset);
> > +				break;
> > +			case 32:
> > +				*(uint32_t *)(dst_buf + dst_offset) =
> > +					*(uint32_t *)(src_buf + src_offset);
> > +				break;
> > +			default:
> Maybe we should also have the two igt_fb_unmap_buffer() calls below this get
> invoked before calling igt_assert(false) so we don't leak FBs if this fails

If that's fine with you, I'd rather have an igt_assert(bpp == 16 || bpp
== 32) earlier so we don't reach mapping the buffers if the condition
cannot be guaranteed.

Cheers!

Paul

> With that fixed:
> 
> Acked-by: Lyude Paul <lyude@redhat.com>
> > +				igt_assert(false);
> > +			}
> > +		}
> > +	}
> > +
> > +	igt_fb_unmap_buffer(src, src_buf);
> > +	igt_fb_unmap_buffer(dst, dst_buf);
> > +
> > +	return fb_id;
> > +}
> > +
> > +/* Calculate the t-tile width so that size = width * height * bpp / 8. */
> > +#define VC4_T_TILE_W(size, height, bpp) ((size) / (height) / ((bpp) / 8))
> > +
> > +size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
> > +			      size_t x, size_t y)
> > +{
> > +	const size_t t1k_map_even[] = { 0, 3, 1, 2 };
> > +	const size_t t1k_map_odd[] = { 2, 1, 3, 0 };
> > +	const size_t t4k_t_h = 32;
> > +	const size_t t1k_t_h = 16;
> > +	const size_t t64_t_h = 4;
> > +	size_t offset = 0;
> > +	size_t t4k_t_w, t4k_w, t4k_x, t4k_y;
> > +	size_t t1k_t_w, t1k_x, t1k_y;
> > +	size_t t64_t_w, t64_x, t64_y;
> > +	size_t pix_x, pix_y;
> > +	unsigned int index;
> > +
> > +	/* T-tiling is only supported for 16 and 32 bpp. */
> > +	igt_assert(bpp == 16 || bpp == 32);
> > +
> > +	/* T-tiling stride must be aligned to the 4K tiles strides. */
> > +	igt_assert((stride % (4096 / t4k_t_h)) == 0);
> > +
> > +	/* Calculate the tile width for the bpp. */
> > +	t4k_t_w = VC4_T_TILE_W(4096, t4k_t_h, bpp);
> > +	t1k_t_w = VC4_T_TILE_W(1024, t1k_t_h, bpp);
> > +	t64_t_w = VC4_T_TILE_W(64, t64_t_h, bpp);
> > +
> > +	/* Aligned total width in number of 4K tiles. */
> > +	t4k_w = (stride / (bpp / 8)) / t4k_t_w;
> > +
> > +	/* X and y coordinates in number of 4K tiles. */
> > +	t4k_x = x / t4k_t_w;
> > +	t4k_y = y / t4k_t_h;
> > +
> > +	/* Increase offset to the beginning of the 4K tile row. */
> > +	offset += t4k_y * t4k_w * 4096;
> > +
> > +	/* X and Y coordinates in number of 1K tiles within the 4K tile. */
> > +	t1k_x = (x % t4k_t_w) / t1k_t_w;
> > +	t1k_y = (y % t4k_t_h) / t1k_t_h;
> > +
> > +	/* Index for 1K tile map lookup. */
> > +	index = 2 * t1k_y + t1k_x;
> > +
> > +	/* Odd rows start from the right, even rows from the left. */
> > +	if (t4k_y % 2) {
> > +		/* Increase offset to the 4K tile (starting from the right).
> > */
> > +		offset += (t4k_w - t4k_x - 1) * 4096;
> > +
> > +		/* Incrase offset to the beginning of the (odd) 1K tile. */
> > +		offset += t1k_map_odd[index] * 1024;
> > +	} else {
> > +		/* Increase offset to the 4K tile (starting from the left). */
> > +		offset += t4k_x * 4096;
> > +
> > +		/* Incrase offset to the beginning of the (even) 1K tile. */
> > +		offset += t1k_map_even[index] * 1024;
> > +	}
> > +
> > +	/* X and Y coordinates in number of 64 byte tiles within the 1K tile.
> > */
> > +	t64_x = (x % t1k_t_w) / t64_t_w;
> > +	t64_y = (y % t1k_t_h) / t64_t_h;
> > +
> > +	/* Increase offset to the beginning of the 64-byte tile. */
> > +	offset += (t64_y * (t1k_t_w / t64_t_w) + t64_x) * 64;
> > +
> > +	/* X and Y coordinates in number of pixels within the 64-byte tile. */
> > +	pix_x = x % t64_t_w;
> > +	pix_y = y % t64_t_h;
> > +
> > +	/* Increase offset to the correct pixel. */
> > +	offset += (pix_y * t64_t_w + pix_x) * bpp / 8;
> > +
> > +	return offset;
> > +}
> > diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> > index ebc8a3881b5e..d5c529bbccda 100644
> > --- a/lib/igt_vc4.h
> > +++ b/lib/igt_vc4.h
> > @@ -33,4 +33,8 @@ bool igt_vc4_purgeable_bo(int fd, int handle, bool
> > purgeable);
> >  void igt_vc4_set_tiling(int fd, uint32_t handle, uint64_t modifier);
> >  uint64_t igt_vc4_get_tiling(int fd, uint32_t handle);
> >  
> > +unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct igt_fb
> > *src);
> > +size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
> > +			      size_t x, size_t y);
> > +
> >  #endif /* IGT_VC4_H */
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

Re: [igt-dev] [PATCH i-g-t v3 09/21] lib/igt_vc4: Add helpers for converting linear to T-tiled RGB buffers

[Lyude Paul]

On Tue, 2019-01-22 at 17:07 +0100, Paul Kocialkowski wrote:
> Hi and thanks for the review,
> 
> On Tue, 2019-01-15 at 16:20 -0500, Lyude Paul wrote:
> > On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> > > In order to integrate testing of T-tiled buffers, the easiest path is to
> > > generate patterns (with the already-existing functions) in linear
> > > buffers and convert them to T-tiled subsequently.
> > > 
> > > Add helpers to do that conversion, with a first helper that returns the
> > > memory offset for a given position in a T-tiled buffer and a second
> > > helper that uses it for converting between framebuffers.
> > > 
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > ---
> > >  lib/igt_vc4.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  lib/igt_vc4.h |   4 ++
> > >  2 files changed, 135 insertions(+)
> > > 
> > > diff --git a/lib/igt_vc4.c b/lib/igt_vc4.c
> > > index 16dfe67a44b1..b4b9c2fc6914 100644
> > > --- a/lib/igt_vc4.c
> > > +++ b/lib/igt_vc4.c
> > > @@ -34,6 +34,7 @@
> > >  #include "drmtest.h"
> > >  #include "igt_aux.h"
> > >  #include "igt_core.h"
> > > +#include "igt_fb.h"
> > >  #include "igt_vc4.h"
> > >  #include "ioctl_wrappers.h"
> > >  #include "intel_reg.h"
> > > @@ -176,3 +177,133 @@ bool igt_vc4_purgeable_bo(int fd, int handle, bool
> > > purgeable)
> > >  
> > >  	return arg.retained;
> > >  }
> > > +
> > > +unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct
> > > igt_fb
> > > *src)
> > > +{
> > > +	unsigned int fb_id;
> > > +	unsigned int i, j;
> > > +	void *src_buf;
> > > +	void *dst_buf;
> > > +	size_t bpp = src->plane_bpp[0];
> > > +	size_t dst_stride = ALIGN(src->strides[0], 128);
> > > +
> > > +	fb_id = igt_create_fb_with_bo_size(src->fd, src->width, src->height,
> > > +					   src->drm_format,
> > > +					   DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED
> > > ,
> > > +					   dst, 0, dst_stride);
> > > +	igt_assert(fb_id > 0);
> > > +
> > > +	src_buf = igt_fb_map_buffer(src->fd, src);
> > > +	igt_assert(src_buf);
> > > +
> > > +	dst_buf = igt_fb_map_buffer(dst->fd, dst);
> > > +	igt_assert(dst_buf);
> > > +
> > > +	for (i = 0; i < src->height; i++) {
> > > +		for (j = 0; j < src->width; j++) {
> > > +			size_t src_offset = src->offsets[0];
> > > +			size_t dst_offset = dst->offsets[0];
> > > +
> > > +			src_offset += src->strides[0] * i + j * bpp / 8;
> > > +			dst_offset += igt_vc4_t_tiled_offset(dst_stride,
> > > +							     src->height,
> > > +							     bpp, j, i);
> > > +
> > > +			switch (bpp) {
> > > +			case 16:
> > > +				*(uint16_t *)(dst_buf + dst_offset) =
> > > +					*(uint16_t *)(src_buf + src_offset);
> > > +				break;
> > > +			case 32:
> > > +				*(uint32_t *)(dst_buf + dst_offset) =
> > > +					*(uint32_t *)(src_buf + src_offset);
> > > +				break;
> > > +			default:
> > Maybe we should also have the two igt_fb_unmap_buffer() calls below this
> > get
> > invoked before calling igt_assert(false) so we don't leak FBs if this
> > fails
> 
> If that's fine with you, I'd rather have an igt_assert(bpp == 16 || bpp
> == 32) earlier so we don't reach mapping the buffers if the condition
> cannot be guaranteed.
> 
> Cheers!
> 
> Paul

Sounds good to me!
> 
> > With that fixed:
> > 
> > Acked-by: Lyude Paul <lyude@redhat.com>
> > > +				igt_assert(false);
> > > +			}
> > > +		}
> > > +	}
> > > +
> > > +	igt_fb_unmap_buffer(src, src_buf);
> > > +	igt_fb_unmap_buffer(dst, dst_buf);
> > > +
> > > +	return fb_id;
> > > +}
> > > +
> > > +/* Calculate the t-tile width so that size = width * height * bpp / 8.
> > > */
> > > +#define VC4_T_TILE_W(size, height, bpp) ((size) / (height) / ((bpp) /
> > > 8))
> > > +
> > > +size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
> > > +			      size_t x, size_t y)
> > > +{
> > > +	const size_t t1k_map_even[] = { 0, 3, 1, 2 };
> > > +	const size_t t1k_map_odd[] = { 2, 1, 3, 0 };
> > > +	const size_t t4k_t_h = 32;
> > > +	const size_t t1k_t_h = 16;
> > > +	const size_t t64_t_h = 4;
> > > +	size_t offset = 0;
> > > +	size_t t4k_t_w, t4k_w, t4k_x, t4k_y;
> > > +	size_t t1k_t_w, t1k_x, t1k_y;
> > > +	size_t t64_t_w, t64_x, t64_y;
> > > +	size_t pix_x, pix_y;
> > > +	unsigned int index;
> > > +
> > > +	/* T-tiling is only supported for 16 and 32 bpp. */
> > > +	igt_assert(bpp == 16 || bpp == 32);
> > > +
> > > +	/* T-tiling stride must be aligned to the 4K tiles strides. */
> > > +	igt_assert((stride % (4096 / t4k_t_h)) == 0);
> > > +
> > > +	/* Calculate the tile width for the bpp. */
> > > +	t4k_t_w = VC4_T_TILE_W(4096, t4k_t_h, bpp);
> > > +	t1k_t_w = VC4_T_TILE_W(1024, t1k_t_h, bpp);
> > > +	t64_t_w = VC4_T_TILE_W(64, t64_t_h, bpp);
> > > +
> > > +	/* Aligned total width in number of 4K tiles. */
> > > +	t4k_w = (stride / (bpp / 8)) / t4k_t_w;
> > > +
> > > +	/* X and y coordinates in number of 4K tiles. */
> > > +	t4k_x = x / t4k_t_w;
> > > +	t4k_y = y / t4k_t_h;
> > > +
> > > +	/* Increase offset to the beginning of the 4K tile row. */
> > > +	offset += t4k_y * t4k_w * 4096;
> > > +
> > > +	/* X and Y coordinates in number of 1K tiles within the 4K tile. */
> > > +	t1k_x = (x % t4k_t_w) / t1k_t_w;
> > > +	t1k_y = (y % t4k_t_h) / t1k_t_h;
> > > +
> > > +	/* Index for 1K tile map lookup. */
> > > +	index = 2 * t1k_y + t1k_x;
> > > +
> > > +	/* Odd rows start from the right, even rows from the left. */
> > > +	if (t4k_y % 2) {
> > > +		/* Increase offset to the 4K tile (starting from the right).
> > > */
> > > +		offset += (t4k_w - t4k_x - 1) * 4096;
> > > +
> > > +		/* Incrase offset to the beginning of the (odd) 1K tile. */
> > > +		offset += t1k_map_odd[index] * 1024;
> > > +	} else {
> > > +		/* Increase offset to the 4K tile (starting from the left). */
> > > +		offset += t4k_x * 4096;
> > > +
> > > +		/* Incrase offset to the beginning of the (even) 1K tile. */
> > > +		offset += t1k_map_even[index] * 1024;
> > > +	}
> > > +
> > > +	/* X and Y coordinates in number of 64 byte tiles within the 1K tile.
> > > */
> > > +	t64_x = (x % t1k_t_w) / t64_t_w;
> > > +	t64_y = (y % t1k_t_h) / t64_t_h;
> > > +
> > > +	/* Increase offset to the beginning of the 64-byte tile. */
> > > +	offset += (t64_y * (t1k_t_w / t64_t_w) + t64_x) * 64;
> > > +
> > > +	/* X and Y coordinates in number of pixels within the 64-byte tile. */
> > > +	pix_x = x % t64_t_w;
> > > +	pix_y = y % t64_t_h;
> > > +
> > > +	/* Increase offset to the correct pixel. */
> > > +	offset += (pix_y * t64_t_w + pix_x) * bpp / 8;
> > > +
> > > +	return offset;
> > > +}
> > > diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> > > index ebc8a3881b5e..d5c529bbccda 100644
> > > --- a/lib/igt_vc4.h
> > > +++ b/lib/igt_vc4.h
> > > @@ -33,4 +33,8 @@ bool igt_vc4_purgeable_bo(int fd, int handle, bool
> > > purgeable);
> > >  void igt_vc4_set_tiling(int fd, uint32_t handle, uint64_t modifier);
> > >  uint64_t igt_vc4_get_tiling(int fd, uint32_t handle);
> > >  
> > > +unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct
> > > igt_fb
> > > *src);
> > > +size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
> > > +			      size_t x, size_t y);
> > > +
> > >  #endif /* IGT_VC4_H */
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 10/21] lib/igt_vc4: Add helper for checking T-tiling support on plane

[Paul Kocialkowski]

This introduces a convenience helper for checking whether a plane
supports T-tiling for a given format.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_vc4.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
index d5c529bbccda..ee7af737ade9 100644
--- a/lib/igt_vc4.h
+++ b/lib/igt_vc4.h
@@ -24,6 +24,15 @@
 #ifndef IGT_VC4_H
 #define IGT_VC4_H
 
+#include "igt_kms.h"
+
+static inline bool igt_vc4_plane_supports_t_tiling(igt_plane_t *plane,
+						   uint32_t format)
+{
+	return igt_plane_has_format_mod(plane, format,
+					DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED);
+}
+
 uint32_t igt_vc4_get_cleared_bo(int fd, size_t size, uint32_t clearval);
 int igt_vc4_create_bo(int fd, size_t size);
 void *igt_vc4_mmap_bo(int fd, uint32_t handle, uint32_t size, unsigned prot);
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 10/21] lib/igt_vc4: Add helper for checking T-tiling support on plane

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 409 bytes --]

On Fri, Jan 11, 2019 at 10:05:21AM +0100, Paul Kocialkowski wrote:
> This introduces a convenience helper for checking whether a plane
> supports T-tiling for a given format.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 10/21] lib/igt_vc4: Add helper for checking T-tiling support on plane

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> This introduces a convenience helper for checking whether a plane
> supports T-tiling for a given format.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_vc4.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> index d5c529bbccda..ee7af737ade9 100644
> --- a/lib/igt_vc4.h
> +++ b/lib/igt_vc4.h
> @@ -24,6 +24,15 @@
>  #ifndef IGT_VC4_H
>  #define IGT_VC4_H
>  
> +#include "igt_kms.h"
> +
> +static inline bool igt_vc4_plane_supports_t_tiling(igt_plane_t *plane,
> +						   uint32_t format)
> +{
> +	return igt_plane_has_format_mod(plane, format,
> +					DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED);
> +}
> +
>  uint32_t igt_vc4_get_cleared_bo(int fd, size_t size, uint32_t clearval);
>  int igt_vc4_create_bo(int fd, size_t size);
>  void *igt_vc4_mmap_bo(int fd, uint32_t handle, uint32_t size, unsigned
> prot);
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 11/21] lib/igt_vc4: Add helpers for converting linear to SAND-tiled buffers

[Paul Kocialkowski]

In order to test buffers with SAND tiling, it is useful to convert
linear buffers to SAND tiling mode.

Introduce helpers to assist in that direction, one that calculates the
memory offset in the SAND-tiled buffer for a given pixel position and
one that makes use of the latter for framebuffer conversion.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_vc4.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_vc4.h |   5 +++
 2 files changed, 116 insertions(+)

diff --git a/lib/igt_vc4.c b/lib/igt_vc4.c
index b4b9c2fc6914..3137c0ef85fa 100644
--- a/lib/igt_vc4.c
+++ b/lib/igt_vc4.c
@@ -307,3 +307,114 @@ size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
 
 	return offset;
 }
+
+static void vc4_fb_sand_tiled_convert_plane(struct igt_fb *dst, void *dst_buf,
+					    struct igt_fb *src, void *src_buf,
+					    size_t column_width_bytes,
+					    size_t column_height,
+					    unsigned int plane)
+{
+	size_t bpp = dst->plane_bpp[plane];
+	size_t column_width = column_width_bytes * dst->plane_width[plane] /
+			      dst->width;
+	size_t column_size = column_width_bytes * column_height;
+	unsigned int i, j;
+
+	for (i = 0; i < dst->plane_height[plane]; i++) {
+		for (j = 0; j < src->plane_width[plane]; j++) {
+			size_t src_offset = src->offsets[plane];
+			size_t dst_offset = dst->offsets[plane];
+
+			src_offset += src->strides[plane] * i + j * bpp / 8;
+			dst_offset += vc4_sand_tiled_offset(column_width,
+							    column_size, j, i,
+							    bpp);
+
+			switch (bpp) {
+			case 8:
+				*(uint8_t *)(dst_buf + dst_offset) =
+					*(uint8_t *)(src_buf + src_offset);
+				break;
+			case 16:
+				*(uint16_t *)(dst_buf + dst_offset) =
+					*(uint16_t *)(src_buf + src_offset);
+				break;
+			default:
+				igt_assert(false);
+			}
+		}
+	}
+}
+
+unsigned int vc4_fb_sand_tiled_convert(struct igt_fb *dst, struct igt_fb *src,
+				       size_t column_width_bytes,
+				       size_t column_height)
+{
+	unsigned int fb_id;
+	unsigned int i;
+	uint64_t modifier;
+	void *src_buf;
+	void *dst_buf;
+
+	if (!column_height)
+		column_height = src->height;
+
+	switch (column_width_bytes) {
+	case 32:
+		modifier =
+			DRM_FORMAT_MOD_BROADCOM_SAND32_COL_HEIGHT(column_height);
+		break;
+	case 64:
+		modifier =
+			DRM_FORMAT_MOD_BROADCOM_SAND64_COL_HEIGHT(column_height);
+		break;
+	case 128:
+		modifier =
+			DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(column_height);
+		break;
+	case 256:
+		modifier =
+			DRM_FORMAT_MOD_BROADCOM_SAND256_COL_HEIGHT(column_height);
+		break;
+	default:
+		igt_assert(false);
+	}
+
+	fb_id = igt_create_fb(src->fd, src->width, src->height, src->drm_format,
+			      modifier, dst);
+	igt_assert(fb_id > 0);
+
+	src_buf = igt_fb_map_buffer(src->fd, src);
+	igt_assert(src_buf);
+
+	dst_buf = igt_fb_map_buffer(dst->fd, dst);
+	igt_assert(dst_buf);
+
+	for (i = 0; i < dst->num_planes; i++)
+		vc4_fb_sand_tiled_convert_plane(dst, dst_buf, src, src_buf,
+						column_width_bytes,
+						column_height, i);
+
+	igt_fb_unmap_buffer(src, src_buf);
+	igt_fb_unmap_buffer(dst, dst_buf);
+
+	return fb_id;
+}
+
+size_t vc4_sand_tiled_offset(size_t column_width, size_t column_size, size_t x,
+			     size_t y, size_t bpp)
+{
+	size_t offset = 0;
+	size_t cols_x;
+	size_t pix_x;
+
+	/* Offset to the beginning of the relevant column. */
+	cols_x = x / column_width;
+	offset += cols_x * column_size;
+
+	/* Offset to the relevant pixel. */
+	pix_x = x % column_width;
+	offset += (column_width * y + pix_x) * bpp / 8;
+
+	return offset;
+}
diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
index ee7af737ade9..9118ae193f60 100644
--- a/lib/igt_vc4.h
+++ b/lib/igt_vc4.h
@@ -45,5 +45,10 @@ uint64_t igt_vc4_get_tiling(int fd, uint32_t handle);
 unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct igt_fb *src);
 size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
 			      size_t x, size_t y);
+unsigned int vc4_fb_sand_tiled_convert(struct igt_fb *dst, struct igt_fb *src,
+				       size_t column_width_bytes,
+				       size_t column_height);
+size_t vc4_sand_tiled_offset(size_t column_width, size_t column_size, size_t x,
+			     size_t y, size_t bpp);
 
 #endif /* IGT_VC4_H */
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 11/21] lib/igt_vc4: Add helpers for converting linear to SAND-tiled buffers

[Lyude Paul]

Acked-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> In order to test buffers with SAND tiling, it is useful to convert
> linear buffers to SAND tiling mode.
> 
> Introduce helpers to assist in that direction, one that calculates the
> memory offset in the SAND-tiled buffer for a given pixel position and
> one that makes use of the latter for framebuffer conversion.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_vc4.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_vc4.h |   5 +++
>  2 files changed, 116 insertions(+)
> 
> diff --git a/lib/igt_vc4.c b/lib/igt_vc4.c
> index b4b9c2fc6914..3137c0ef85fa 100644
> --- a/lib/igt_vc4.c
> +++ b/lib/igt_vc4.c
> @@ -307,3 +307,114 @@ size_t igt_vc4_t_tiled_offset(size_t stride, size_t
> height, size_t bpp,
>  
>  	return offset;
>  }
> +
> +static void vc4_fb_sand_tiled_convert_plane(struct igt_fb *dst, void
> *dst_buf,
> +					    struct igt_fb *src, void *src_buf,
> +					    size_t column_width_bytes,
> +					    size_t column_height,
> +					    unsigned int plane)
> +{
> +	size_t bpp = dst->plane_bpp[plane];
> +	size_t column_width = column_width_bytes * dst->plane_width[plane] /
> +			      dst->width;
> +	size_t column_size = column_width_bytes * column_height;
> +	unsigned int i, j;
> +
> +	for (i = 0; i < dst->plane_height[plane]; i++) {
> +		for (j = 0; j < src->plane_width[plane]; j++) {
> +			size_t src_offset = src->offsets[plane];
> +			size_t dst_offset = dst->offsets[plane];
> +
> +			src_offset += src->strides[plane] * i + j * bpp / 8;
> +			dst_offset += vc4_sand_tiled_offset(column_width,
> +							    column_size, j, i,
> +							    bpp);
> +
> +			switch (bpp) {
> +			case 8:
> +				*(uint8_t *)(dst_buf + dst_offset) =
> +					*(uint8_t *)(src_buf + src_offset);
> +				break;
> +			case 16:
> +				*(uint16_t *)(dst_buf + dst_offset) =
> +					*(uint16_t *)(src_buf + src_offset);
> +				break;
> +			default:
> +				igt_assert(false);
> +			}
> +		}
> +	}
> +}
> +
> +unsigned int vc4_fb_sand_tiled_convert(struct igt_fb *dst, struct igt_fb
> *src,
> +				       size_t column_width_bytes,
> +				       size_t column_height)
> +{
> +	unsigned int fb_id;
> +	unsigned int i;
> +	uint64_t modifier;
> +	void *src_buf;
> +	void *dst_buf;
> +
> +	if (!column_height)
> +		column_height = src->height;
> +
> +	switch (column_width_bytes) {
> +	case 32:
> +		modifier =
> +			DRM_FORMAT_MOD_BROADCOM_SAND32_COL_HEIGHT(column_heigh
> t);
> +		break;
> +	case 64:
> +		modifier =
> +			DRM_FORMAT_MOD_BROADCOM_SAND64_COL_HEIGHT(column_heigh
> t);
> +		break;
> +	case 128:
> +		modifier =
> +			DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(column_heig
> ht);
> +		break;
> +	case 256:
> +		modifier =
> +			DRM_FORMAT_MOD_BROADCOM_SAND256_COL_HEIGHT(column_heig
> ht);
> +		break;
> +	default:
> +		igt_assert(false);
> +	}
> +
> +	fb_id = igt_create_fb(src->fd, src->width, src->height, src-
> >drm_format,
> +			      modifier, dst);
> +	igt_assert(fb_id > 0);
> +
> +	src_buf = igt_fb_map_buffer(src->fd, src);
> +	igt_assert(src_buf);
> +
> +	dst_buf = igt_fb_map_buffer(dst->fd, dst);
> +	igt_assert(dst_buf);
> +
> +	for (i = 0; i < dst->num_planes; i++)
> +		vc4_fb_sand_tiled_convert_plane(dst, dst_buf, src, src_buf,
> +						column_width_bytes,
> +						column_height, i);
> +
> +	igt_fb_unmap_buffer(src, src_buf);
> +	igt_fb_unmap_buffer(dst, dst_buf);
> +
> +	return fb_id;
> +}
> +
> +size_t vc4_sand_tiled_offset(size_t column_width, size_t column_size,
> size_t x,
> +			     size_t y, size_t bpp)
> +{
> +	size_t offset = 0;
> +	size_t cols_x;
> +	size_t pix_x;
> +
> +	/* Offset to the beginning of the relevant column. */
> +	cols_x = x / column_width;
> +	offset += cols_x * column_size;
> +
> +	/* Offset to the relevant pixel. */
> +	pix_x = x % column_width;
> +	offset += (column_width * y + pix_x) * bpp / 8;
> +
> +	return offset;
> +}
> diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> index ee7af737ade9..9118ae193f60 100644
> --- a/lib/igt_vc4.h
> +++ b/lib/igt_vc4.h
> @@ -45,5 +45,10 @@ uint64_t igt_vc4_get_tiling(int fd, uint32_t handle);
>  unsigned int igt_vc4_fb_t_tiled_convert(struct igt_fb *dst, struct igt_fb
> *src);
>  size_t igt_vc4_t_tiled_offset(size_t stride, size_t height, size_t bpp,
>  			      size_t x, size_t y);
> +unsigned int vc4_fb_sand_tiled_convert(struct igt_fb *dst, struct igt_fb
> *src,
> +				       size_t column_width_bytes,
> +				       size_t column_height);
> +size_t vc4_sand_tiled_offset(size_t column_width, size_t column_size,
> size_t x,
> +			     size_t y, size_t bpp);
>  
>  #endif /* IGT_VC4_H */
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 12/21] lib/igt_vc4: Add helper for checking SAND tiling support on plane

[Paul Kocialkowski]

This introduces a convenience helper for checking whether a plane
supports SAND tiling for a given format.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_vc4.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
index 9118ae193f60..cb980541a61f 100644
--- a/lib/igt_vc4.h
+++ b/lib/igt_vc4.h
@@ -33,6 +33,30 @@ static inline bool igt_vc4_plane_supports_t_tiling(igt_plane_t *plane,
 					DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED);
 }
 
+static inline bool igt_vc4_plane_supports_sand_tiling(igt_plane_t *plane,
+						      uint32_t format,
+						      size_t column_width_bytes)
+{
+	uint64_t modifier;
+
+	switch (column_width_bytes) {
+	case 32:
+		modifier = DRM_FORMAT_MOD_BROADCOM_SAND32;
+		break;
+	case 64:
+		modifier = DRM_FORMAT_MOD_BROADCOM_SAND64;
+		break;
+	case 128:
+		modifier = DRM_FORMAT_MOD_BROADCOM_SAND128;
+		break;
+	case 256:
+		modifier = DRM_FORMAT_MOD_BROADCOM_SAND256;
+		break;
+	}
+
+	return igt_plane_has_format_mod(plane, format, modifier);
+}
+
 uint32_t igt_vc4_get_cleared_bo(int fd, size_t size, uint32_t clearval);
 int igt_vc4_create_bo(int fd, size_t size);
 void *igt_vc4_mmap_bo(int fd, uint32_t handle, uint32_t size, unsigned prot);
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 12/21] lib/igt_vc4: Add helper for checking SAND tiling support on plane

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1663 bytes --]

On Fri, Jan 11, 2019 at 10:05:23AM +0100, Paul Kocialkowski wrote:
> This introduces a convenience helper for checking whether a plane
> supports SAND tiling for a given format.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_vc4.h | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> index 9118ae193f60..cb980541a61f 100644
> --- a/lib/igt_vc4.h
> +++ b/lib/igt_vc4.h
> @@ -33,6 +33,30 @@ static inline bool igt_vc4_plane_supports_t_tiling(igt_plane_t *plane,
>  					DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED);
>  }
>  
> +static inline bool igt_vc4_plane_supports_sand_tiling(igt_plane_t *plane,
> +						      uint32_t format,
> +						      size_t column_width_bytes)
> +{
> +	uint64_t modifier;
> +
> +	switch (column_width_bytes) {
> +	case 32:
> +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND32;
> +		break;
> +	case 64:
> +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND64;
> +		break;
> +	case 128:
> +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND128;
> +		break;
> +	case 256:
> +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND256;
> +		break;
> +	}
> +
> +	return igt_plane_has_format_mod(plane, format, modifier);
> +}
> +

I'm not quite sure what that function is supposed to be doing, or
rather, why do you need the column_width_bytes parameter.

If that function was just about checking whether the sand tiling is
supported, then you can just return an or'd igt_plane_has_format_mod
of all the modifiers supported, right?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 12/21] lib/igt_vc4: Add helper for checking SAND tiling support on plane

[Paul Kocialkowski]

Hi,

On Fri, 2019-01-11 at 16:15 +0100, Maxime Ripard wrote:
> On Fri, Jan 11, 2019 at 10:05:23AM +0100, Paul Kocialkowski wrote:
> > This introduces a convenience helper for checking whether a plane
> > supports SAND tiling for a given format.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  lib/igt_vc4.h | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> > index 9118ae193f60..cb980541a61f 100644
> > --- a/lib/igt_vc4.h
> > +++ b/lib/igt_vc4.h
> > @@ -33,6 +33,30 @@ static inline bool igt_vc4_plane_supports_t_tiling(igt_plane_t *plane,
> >  					DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED);
> >  }
> >  
> > +static inline bool igt_vc4_plane_supports_sand_tiling(igt_plane_t *plane,
> > +						      uint32_t format,
> > +						      size_t column_width_bytes)
> > +{
> > +	uint64_t modifier;
> > +
> > +	switch (column_width_bytes) {
> > +	case 32:
> > +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND32;
> > +		break;
> > +	case 64:
> > +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND64;
> > +		break;
> > +	case 128:
> > +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND128;
> > +		break;
> > +	case 256:
> > +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND256;
> > +		break;
> > +	}
> > +
> > +	return igt_plane_has_format_mod(plane, format, modifier);
> > +}
> > +
> 
> I'm not quite sure what that function is supposed to be doing, or
> rather, why do you need the column_width_bytes parameter.
> 
> If that function was just about checking whether the sand tiling is
> supported, then you can just return an or'd igt_plane_has_format_mod
> of all the modifiers supported, right?

Well, some variants of SAND are only valid for a reduced set of
formats. IIRC, SAND256 only applies to NV12 while others apply to other
YUV formats as well.

So I believe it's relevant to distinguish between the different SAND
tilings when checking for support.

However, the function could be simplified by simply passing the
modifier, extracting the base modifier (without column height, that is
coded in the modifier) and calling igt_plane_has_format_mod.

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

Re: [igt-dev] [PATCH i-g-t v3 12/21] lib/igt_vc4: Add helper for checking SAND tiling support on plane

[Maxime Ripard]

On Mon, Jan 14, 2019 at 04:01:05PM +0100, Paul Kocialkowski wrote:
> Hi,
> 
> On Fri, 2019-01-11 at 16:15 +0100, Maxime Ripard wrote:
> > On Fri, Jan 11, 2019 at 10:05:23AM +0100, Paul Kocialkowski wrote:
> > > This introduces a convenience helper for checking whether a plane
> > > supports SAND tiling for a given format.
> > > 
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > ---
> > >  lib/igt_vc4.h | 24 ++++++++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > > 
> > > diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> > > index 9118ae193f60..cb980541a61f 100644
> > > --- a/lib/igt_vc4.h
> > > +++ b/lib/igt_vc4.h
> > > @@ -33,6 +33,30 @@ static inline bool igt_vc4_plane_supports_t_tiling(igt_plane_t *plane,
> > >  					DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED);
> > >  }
> > >  
> > > +static inline bool igt_vc4_plane_supports_sand_tiling(igt_plane_t *plane,
> > > +						      uint32_t format,
> > > +						      size_t column_width_bytes)
> > > +{
> > > +	uint64_t modifier;
> > > +
> > > +	switch (column_width_bytes) {
> > > +	case 32:
> > > +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND32;
> > > +		break;
> > > +	case 64:
> > > +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND64;
> > > +		break;
> > > +	case 128:
> > > +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND128;
> > > +		break;
> > > +	case 256:
> > > +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND256;
> > > +		break;
> > > +	}
> > > +
> > > +	return igt_plane_has_format_mod(plane, format, modifier);
> > > +}
> > > +
> > 
> > I'm not quite sure what that function is supposed to be doing, or
> > rather, why do you need the column_width_bytes parameter.
> > 
> > If that function was just about checking whether the sand tiling is
> > supported, then you can just return an or'd igt_plane_has_format_mod
> > of all the modifiers supported, right?
> 
> Well, some variants of SAND are only valid for a reduced set of
> formats. IIRC, SAND256 only applies to NV12 while others apply to other
> YUV formats as well.
>
> So I believe it's relevant to distinguish between the different SAND
> tilings when checking for support.

Ok

> However, the function could be simplified by simply passing the
> modifier, extracting the base modifier (without column height, that is
> coded in the modifier) and calling igt_plane_has_format_mod.

Yeah, that sounds much better.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 12/21] lib/igt_vc4: Add helper for checking SAND tiling support on plane

[Lyude Paul]

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> This introduces a convenience helper for checking whether a plane
> supports SAND tiling for a given format.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_vc4.h | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> index 9118ae193f60..cb980541a61f 100644
> --- a/lib/igt_vc4.h
> +++ b/lib/igt_vc4.h
> @@ -33,6 +33,30 @@ static inline bool
> igt_vc4_plane_supports_t_tiling(igt_plane_t *plane,
>  					DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED);
>  }
>  
> +static inline bool igt_vc4_plane_supports_sand_tiling(igt_plane_t *plane,
> +						      uint32_t format,
> +						      size_t
> column_width_bytes)
> +{
> +	uint64_t modifier;
> +
> +	switch (column_width_bytes) {
> +	case 32:
> +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND32;
> +		break;
> +	case 64:
> +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND64;
> +		break;
> +	case 128:
> +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND128;
> +		break;
> +	case 256:
> +		modifier = DRM_FORMAT_MOD_BROADCOM_SAND256;
> +		break;
Optional bikeshed: Maybe add a default: case with an igt_assert()?

Either way:

Reviewed-by: Lyude Paul <lyude@redhat.com>
> +	}
> +
> +	return igt_plane_has_format_mod(plane, format, modifier);
> +}
> +
>  uint32_t igt_vc4_get_cleared_bo(int fd, size_t size, uint32_t clearval);
>  int igt_vc4_create_bo(int fd, size_t size);
>  void *igt_vc4_mmap_bo(int fd, uint32_t handle, uint32_t size, unsigned
> prot);
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 13/21] lib/igt_kms: Add helpers to count and iterate planes from pipe

[Paul Kocialkowski]

This introduces helpers that allow counting how many planes of a given
type are present from a pipe and getting the n-th plane of a given type.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_kms.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_kms.h |  3 +++
 2 files changed, 51 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 684a599ca674..e5c4ee72884a 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2276,6 +2276,54 @@ igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type)
 	return &pipe->planes[plane_idx];
 }
 
+/**
+ * igt_pipe_count_plane_type:
+ * @pipe: Target pipe
+ * @plane_type: Cursor, primary or an overlay plane
+ *
+ * Counts the number of planes of type @plane_type for the provided @pipe.
+ *
+ * Returns: The number of planes that match the requested plane type
+ */
+int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type)
+{
+	int i, count = 0;
+
+	for(i = 0; i < pipe->n_planes; i++)
+		if (pipe->planes[i].type == plane_type)
+			count++;
+
+	return count;
+}
+
+/**
+ * igt_pipe_get_plane_type_index:
+ * @pipe: Target pipe
+ * @plane_type: Cursor, primary or an overlay plane
+ * @index: the index of the plane among planes of the same type
+ *
+ * Get the @index th plane of type @plane_type for the provided @pipe.
+ *
+ * Returns: The @index th plane that matches the requested plane type
+ */
+igt_plane_t *igt_pipe_get_plane_type_index(igt_pipe_t *pipe, int plane_type,
+					   int index)
+{
+	int i, type_index = 0;
+
+	for(i = 0; i < pipe->n_planes; i++) {
+		if (pipe->planes[i].type != plane_type)
+			continue;
+
+		if (type_index == index)
+			return &pipe->planes[i];
+
+		type_index++;
+	}
+
+	return NULL;
+}
+
 static bool output_is_internal_panel(igt_output_t *output)
 {
 	switch (output->config.connector->connector_type) {
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 4a7c3c97957f..0f99535e2c6e 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -405,6 +405,9 @@ igt_output_t *igt_output_from_connector(igt_display_t *display,
     drmModeConnector *connector);
 
 igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
+int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type);
+igt_plane_t *igt_pipe_get_plane_type_index(igt_pipe_t *pipe, int plane_type,
+					   int index);
 igt_output_t *igt_get_single_output_for_pipe(igt_display_t *display, enum pipe pipe);
 
 void igt_pipe_request_out_fence(igt_pipe_t *pipe);
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 13/21] lib/igt_kms: Add helpers to count and iterate planes from pipe

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 449 bytes --]

On Fri, Jan 11, 2019 at 10:05:24AM +0100, Paul Kocialkowski wrote:
> This introduces helpers that allow counting how many planes of a given
> type are present from a pipe and getting the n-th plane of a given type.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 13/21] lib/igt_kms: Add helpers to count and iterate planes from pipe

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> This introduces helpers that allow counting how many planes of a given
> type are present from a pipe and getting the n-th plane of a given type.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_kms.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_kms.h |  3 +++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 684a599ca674..e5c4ee72884a 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -2276,6 +2276,54 @@ igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t
> *pipe, int plane_type)
>  	return &pipe->planes[plane_idx];
>  }
>  
> +/**
> + * igt_pipe_count_plane_type:
> + * @pipe: Target pipe
> + * @plane_type: Cursor, primary or an overlay plane
> + *
> + * Counts the number of planes of type @plane_type for the provided @pipe.
> + *
> + * Returns: The number of planes that match the requested plane type
> + */
> +int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type)
> +{
> +	int i, count = 0;
> +
> +	for(i = 0; i < pipe->n_planes; i++)
> +		if (pipe->planes[i].type == plane_type)
> +			count++;
> +
> +	return count;
> +}
> +
> +/**
> + * igt_pipe_get_plane_type_index:
> + * @pipe: Target pipe
> + * @plane_type: Cursor, primary or an overlay plane
> + * @index: the index of the plane among planes of the same type
> + *
> + * Get the @index th plane of type @plane_type for the provided @pipe.
> + *
> + * Returns: The @index th plane that matches the requested plane type
> + */
> +igt_plane_t *igt_pipe_get_plane_type_index(igt_pipe_t *pipe, int
> plane_type,
> +					   int index)
> +{
> +	int i, type_index = 0;
> +
> +	for(i = 0; i < pipe->n_planes; i++) {
> +		if (pipe->planes[i].type != plane_type)
> +			continue;
> +
> +		if (type_index == index)
> +			return &pipe->planes[i];
> +
> +		type_index++;
> +	}
> +
> +	return NULL;
> +}
> +
>  static bool output_is_internal_panel(igt_output_t *output)
>  {
>  	switch (output->config.connector->connector_type) {
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 4a7c3c97957f..0f99535e2c6e 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -405,6 +405,9 @@ igt_output_t *igt_output_from_connector(igt_display_t
> *display,
>      drmModeConnector *connector);
>  
>  igt_plane_t *igt_pipe_get_plane_type(igt_pipe_t *pipe, int plane_type);
> +int igt_pipe_count_plane_type(igt_pipe_t *pipe, int plane_type);
> +igt_plane_t *igt_pipe_get_plane_type_index(igt_pipe_t *pipe, int
> plane_type,
> +					   int index);
>  igt_output_t *igt_get_single_output_for_pipe(igt_display_t *display, enum
> pipe pipe);
>  
>  void igt_pipe_request_out_fence(igt_pipe_t *pipe);
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 14/21] lib/igt_kms: Add helpers to count and iterate planes from output

[Paul Kocialkowski]

With helpers to count and iterate among planes of a given type from the
pipe in place, we can use them with the current pipe for the output to
make it possible for tests to use them (the pipe struct is not currently
easily exposed to tests and exposing it adds unnecessary complexity).

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_kms.c | 36 ++++++++++++++++++++++++++++++++++++
 lib/igt_kms.h |  3 +++
 2 files changed, 39 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index e5c4ee72884a..f87c869fd3b7 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -3718,6 +3718,42 @@ igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type)
 	return igt_pipe_get_plane_type(pipe, plane_type);
 }
 
+/**
+ * igt_output_count_plane_type:
+ * @output: Target output
+ * @plane_type: Cursor, primary or an overlay plane
+ *
+ * Counts the number of planes of type @plane_type for the provided @output.
+ *
+ * Returns: The number of planes that match the requested plane type
+ */
+int igt_output_count_plane_type(igt_output_t *output, int plane_type)
+{
+	igt_pipe_t *pipe = igt_output_get_driving_pipe(output);
+	igt_assert(pipe);
+
+	return igt_pipe_count_plane_type(pipe, plane_type);
+}
+
+/**
+ * igt_output_get_plane_type_index:
+ * @output: Target output
+ * @plane_type: Cursor, primary or an overlay plane
+ * @index: the index of the plane among planes of the same type
+ *
+ * Get the @index th plane of type @plane_type for the provided @output.
+ *
+ * Returns: The @index th plane that matches the requested plane type
+ */
+igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
+					     int plane_type, int index)
+{
+	igt_pipe_t *pipe = igt_output_get_driving_pipe(output);
+	igt_assert(pipe);
+
+	return igt_pipe_get_plane_type_index(pipe, plane_type, index);
+}
+
 /**
  * igt_plane_set_fb:
  * @plane: Plane
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 0f99535e2c6e..1cfce9647637 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -401,6 +401,9 @@ void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode);
 void igt_output_set_pipe(igt_output_t *output, enum pipe pipe);
 igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx);
 igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);
+int igt_output_count_plane_type(igt_output_t *output, int plane_type);
+igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
+					     int plane_type, int index);
 igt_output_t *igt_output_from_connector(igt_display_t *display,
     drmModeConnector *connector);
 
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 14/21] lib/igt_kms: Add helpers to count and iterate planes from output

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 597 bytes --]

On Fri, Jan 11, 2019 at 10:05:25AM +0100, Paul Kocialkowski wrote:
> With helpers to count and iterate among planes of a given type from the
> pipe in place, we can use them with the current pipe for the output to
> make it possible for tests to use them (the pipe struct is not currently
> easily exposed to tests and exposing it adds unnecessary complexity).
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 14/21] lib/igt_kms: Add helpers to count and iterate planes from output

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> With helpers to count and iterate among planes of a given type from the
> pipe in place, we can use them with the current pipe for the output to
> make it possible for tests to use them (the pipe struct is not currently
> easily exposed to tests and exposing it adds unnecessary complexity).
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_kms.c | 36 ++++++++++++++++++++++++++++++++++++
>  lib/igt_kms.h |  3 +++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index e5c4ee72884a..f87c869fd3b7 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -3718,6 +3718,42 @@ igt_plane_t *igt_output_get_plane_type(igt_output_t
> *output, int plane_type)
>  	return igt_pipe_get_plane_type(pipe, plane_type);
>  }
>  
> +/**
> + * igt_output_count_plane_type:
> + * @output: Target output
> + * @plane_type: Cursor, primary or an overlay plane
> + *
> + * Counts the number of planes of type @plane_type for the provided
> @output.
> + *
> + * Returns: The number of planes that match the requested plane type
> + */
> +int igt_output_count_plane_type(igt_output_t *output, int plane_type)
> +{
> +	igt_pipe_t *pipe = igt_output_get_driving_pipe(output);
> +	igt_assert(pipe);
> +
> +	return igt_pipe_count_plane_type(pipe, plane_type);
> +}
> +
> +/**
> + * igt_output_get_plane_type_index:
> + * @output: Target output
> + * @plane_type: Cursor, primary or an overlay plane
> + * @index: the index of the plane among planes of the same type
> + *
> + * Get the @index th plane of type @plane_type for the provided @output.
> + *
> + * Returns: The @index th plane that matches the requested plane type
> + */
> +igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
> +					     int plane_type, int index)
> +{
> +	igt_pipe_t *pipe = igt_output_get_driving_pipe(output);
> +	igt_assert(pipe);
> +
> +	return igt_pipe_get_plane_type_index(pipe, plane_type, index);
> +}
> +
>  /**
>   * igt_plane_set_fb:
>   * @plane: Plane
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 0f99535e2c6e..1cfce9647637 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -401,6 +401,9 @@ void igt_output_override_mode(igt_output_t *output,
> drmModeModeInfo *mode);
>  void igt_output_set_pipe(igt_output_t *output, enum pipe pipe);
>  igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx);
>  igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int
> plane_type);
> +int igt_output_count_plane_type(igt_output_t *output, int plane_type);
> +igt_plane_t *igt_output_get_plane_type_index(igt_output_t *output,
> +					     int plane_type, int index);
>  igt_output_t *igt_output_from_connector(igt_display_t *display,
>      drmModeConnector *connector);
>  
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 15/21] lib/igt_chamelium: Fixup resources liberation in comparison helpers

[Paul Kocialkowski]

This fixes a bunch of occurrences of memory not being properly
liberated after its use in helpers revolving around frame/CRC
comparison.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_chamelium.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 32b859eac4a7..0b6ac37a3d89 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -1060,9 +1060,9 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium *chamelium,
 		free(reference_suffix);
 		free(capture_suffix);
 
-		chamelium_destroy_frame_dump(frame);
-
+		cairo_surface_destroy(reference);
 		cairo_surface_destroy(capture);
+		chamelium_destroy_frame_dump(frame);
 	}
 
 	igt_assert(eq);
@@ -1120,11 +1120,14 @@ void chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
 
 		free(reference_suffix);
 		free(capture_suffix);
+		free(reference_crc);
+		free(capture_crc);
 	}
 
-	cairo_surface_destroy(capture);
-
 	igt_assert(match);
+
+	cairo_surface_destroy(reference);
+	cairo_surface_destroy(capture);
 }
 
 
@@ -1323,6 +1326,8 @@ igt_crc_t *chamelium_calculate_fb_crc(int fd, struct igt_fb *fb)
 
 	chamelium_do_calculate_fb_crc(fb_surface, ret);
 
+	cairo_surface_destroy(fb_surface);
+
 	return ret;
 }
 
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 15/21] lib/igt_chamelium: Fixup resources liberation in comparison helpers

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 445 bytes --]

On Fri, Jan 11, 2019 at 10:05:26AM +0100, Paul Kocialkowski wrote:
> This fixes a bunch of occurrences of memory not being properly
> liberated after its use in helpers revolving around frame/CRC
> comparison.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 15/21] lib/igt_chamelium: Fixup resources liberation in comparison helpers

[Lyude Paul]

Nice catch here!

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> This fixes a bunch of occurrences of memory not being properly
> liberated after its use in helpers revolving around frame/CRC
> comparison.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_chamelium.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 32b859eac4a7..0b6ac37a3d89 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -1060,9 +1060,9 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium
> *chamelium,
>  		free(reference_suffix);
>  		free(capture_suffix);
>  
> -		chamelium_destroy_frame_dump(frame);
> -
> +		cairo_surface_destroy(reference);
>  		cairo_surface_destroy(capture);
> +		chamelium_destroy_frame_dump(frame);
>  	}
>  
>  	igt_assert(eq);
> @@ -1120,11 +1120,14 @@ void
> chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
>  
>  		free(reference_suffix);
>  		free(capture_suffix);
> +		free(reference_crc);
> +		free(capture_crc);
>  	}
>  
> -	cairo_surface_destroy(capture);
> -
>  	igt_assert(match);
> +
> +	cairo_surface_destroy(reference);
> +	cairo_surface_destroy(capture);
>  }
>  
>  
> @@ -1323,6 +1326,8 @@ igt_crc_t *chamelium_calculate_fb_crc(int fd, struct
> igt_fb *fb)
>  
>  	chamelium_do_calculate_fb_crc(fb_surface, ret);
>  
> +	cairo_surface_destroy(fb_surface);
> +
>  	return ret;
>  }
>  
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 16/21] lib/igt_chamelium: Split frames dump logic and rework surroundings

[Paul Kocialkowski]

The frame dump logic is the same for all comparison helpers, so split
it to a dedicated function and adapt helpers using it to avoid
duplicating operations.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_chamelium.c | 86 +++++++++++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 35 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 0b6ac37a3d89..5966b5ce0743 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -124,6 +124,9 @@ struct chamelium {
 
 static struct chamelium *cleanup_instance;
 
+static void chamelium_do_calculate_fb_crc(cairo_surface_t *fb_surface,
+					  igt_crc_t *out);
+
 /**
  * chamelium_get_ports:
  * @chamelium: The Chamelium instance to use
@@ -967,6 +970,39 @@ static cairo_surface_t *convert_frame_dump_argb32(const struct chamelium_frame_d
 	return dump_surface;
 }
 
+static void compared_frames_dump(cairo_surface_t *reference,
+				 cairo_surface_t *capture,
+				 igt_crc_t *reference_crc,
+				 igt_crc_t *capture_crc)
+{
+	char *reference_suffix;
+	char *capture_suffix;
+	igt_crc_t local_reference_crc;
+	igt_crc_t local_capture_crc;
+
+	igt_assert(reference && capture);
+
+	if (!reference_crc) {
+		chamelium_do_calculate_fb_crc(reference, &local_reference_crc);
+		reference_crc = &local_reference_crc;
+	}
+
+	if (!capture_crc) {
+		chamelium_do_calculate_fb_crc(reference, &local_capture_crc);
+		capture_crc = &local_capture_crc;
+	}
+
+	reference_suffix = igt_crc_to_string_extended(reference_crc, '-', 2);
+	capture_suffix = igt_crc_to_string_extended(capture_crc, '-', 2);
+
+	/* Write reference and capture frames to png. */
+	igt_write_compared_frames_to_png(reference, capture, reference_suffix,
+					 capture_suffix);
+
+	free(reference_suffix);
+	free(capture_suffix);
+}
+
 /**
  * chamelium_assert_frame_eq:
  * @chamelium: The chamelium instance the frame dump belongs to
@@ -1029,8 +1065,6 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium *chamelium,
 	struct chamelium_frame_dump *frame;
 	cairo_surface_t *reference;
 	cairo_surface_t *capture;
-	char *reference_suffix;
-	char *capture_suffix;
 	bool eq;
 
 	igt_debug("Reference CRC: %s\n", igt_crc_to_string(reference_crc));
@@ -1038,27 +1072,19 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium *chamelium,
 
 	eq = igt_check_crc_equal(reference_crc, capture_crc);
 	if (!eq && igt_frame_dump_is_enabled()) {
-		/* Grab the reference frame from framebuffer */
+		/* Convert the reference framebuffer to cairo. */
 		reference = igt_get_cairo_surface(chamelium->drm_fd, fb);
 
-		/* Grab the captured frame from chamelium */
+		/* Grab the captured frame from the Chamelium. */
 		frame = chamelium_read_captured_frame(chamelium, index);
 		igt_assert(frame);
 
+		/* Convert the captured frame to cairo. */
 		capture = convert_frame_dump_argb32(frame);
+		igt_assert(capture);
 
-		reference_suffix = igt_crc_to_string_extended(reference_crc,
-							      '-', 2);
-		capture_suffix = igt_crc_to_string_extended(capture_crc, '-',
-							    2);
-
-		/* Write reference and capture frames to png */
-		igt_write_compared_frames_to_png(reference, capture,
-						 reference_suffix,
-						 capture_suffix);
-
-		free(reference_suffix);
-		free(capture_suffix);
+		compared_frames_dump(reference, capture, reference_crc,
+				     capture_crc);
 
 		cairo_surface_destroy(reference);
 		cairo_surface_destroy(capture);
@@ -1087,8 +1113,6 @@ void chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
 	cairo_surface_t *capture;
 	igt_crc_t *reference_crc;
 	igt_crc_t *capture_crc;
-	char *reference_suffix;
-	char *capture_suffix;
 	bool match;
 
 	/* Grab the reference frame from framebuffer */
@@ -1099,27 +1123,19 @@ void chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
 
 	match = igt_check_analog_frame_match(reference, capture);
 	if (!match && igt_frame_dump_is_enabled()) {
-		reference_crc = chamelium_calculate_fb_crc(chamelium->drm_fd,
-							   fb);
-		capture_crc = chamelium_get_crc_for_area(chamelium, port, 0, 0,
-							 0, 0);
+		reference_crc = malloc(sizeof(igt_crc_t));
 
-		igt_debug("Reference CRC: %s\n",
-			  igt_crc_to_string(reference_crc));
-		igt_debug("Captured CRC: %s\n", igt_crc_to_string(capture_crc));
+		/* Calculate the reference frame CRC. */
+		chamelium_do_calculate_fb_crc(reference, reference_crc);
 
-		reference_suffix = igt_crc_to_string_extended(reference_crc,
-							      '-', 2);
-		capture_suffix = igt_crc_to_string_extended(capture_crc, '-',
-							    2);
+		/* Get the captured frame CRC from the Chamelium. */
+		capture_crc = chamelium_get_crc_for_area(chamelium, port, 0, 0,
+							 0, 0);
+		igt_assert(capture_crc);
 
-		/* Write reference and capture frames to png */
-		igt_write_compared_frames_to_png(reference, capture,
-						 reference_suffix,
-						 capture_suffix);
+		compared_frames_dump(reference, capture, reference_crc,
+				     capture_crc);
 
-		free(reference_suffix);
-		free(capture_suffix);
 		free(reference_crc);
 		free(capture_crc);
 	}
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 16/21] lib/igt_chamelium: Split frames dump logic and rework surroundings

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 465 bytes --]

On Fri, Jan 11, 2019 at 10:05:27AM +0100, Paul Kocialkowski wrote:
> The frame dump logic is the same for all comparison helpers, so split
> it to a dedicated function and adapt helpers using it to avoid
> duplicating operations.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 16/21] lib/igt_chamelium: Split frames dump logic and rework surroundings

[Lyude Paul]

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> The frame dump logic is the same for all comparison helpers, so split
> it to a dedicated function and adapt helpers using it to avoid
> duplicating operations.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_chamelium.c | 86 +++++++++++++++++++++++++++------------------
>  1 file changed, 51 insertions(+), 35 deletions(-)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 0b6ac37a3d89..5966b5ce0743 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -124,6 +124,9 @@ struct chamelium {
>  
>  static struct chamelium *cleanup_instance;
>  
> +static void chamelium_do_calculate_fb_crc(cairo_surface_t *fb_surface,
> +					  igt_crc_t *out);
> +
>  /**
>   * chamelium_get_ports:
>   * @chamelium: The Chamelium instance to use
> @@ -967,6 +970,39 @@ static cairo_surface_t *convert_frame_dump_argb32(const
> struct chamelium_frame_d
>  	return dump_surface;
>  }
>  
> +static void compared_frames_dump(cairo_surface_t *reference,
> +				 cairo_surface_t *capture,
> +				 igt_crc_t *reference_crc,
> +				 igt_crc_t *capture_crc)
> +{
> +	char *reference_suffix;
> +	char *capture_suffix;
> +	igt_crc_t local_reference_crc;
> +	igt_crc_t local_capture_crc;
> +
> +	igt_assert(reference && capture);
> +
> +	if (!reference_crc) {
> +		chamelium_do_calculate_fb_crc(reference,
> &local_reference_crc);
> +		reference_crc = &local_reference_crc;
> +	}
> +
> +	if (!capture_crc) {
> +		chamelium_do_calculate_fb_crc(reference, &local_capture_crc);
> +		capture_crc = &local_capture_crc;
> +	}
> +
> +	reference_suffix = igt_crc_to_string_extended(reference_crc, '-', 2);
> +	capture_suffix = igt_crc_to_string_extended(capture_crc, '-', 2);
> +
> +	/* Write reference and capture frames to png. */
> +	igt_write_compared_frames_to_png(reference, capture, reference_suffix,
> +					 capture_suffix);
> +
> +	free(reference_suffix);
> +	free(capture_suffix);
> +}
> +
>  /**
>   * chamelium_assert_frame_eq:
>   * @chamelium: The chamelium instance the frame dump belongs to
> @@ -1029,8 +1065,6 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium
> *chamelium,
>  	struct chamelium_frame_dump *frame;
>  	cairo_surface_t *reference;
>  	cairo_surface_t *capture;
> -	char *reference_suffix;
> -	char *capture_suffix;
>  	bool eq;
>  
>  	igt_debug("Reference CRC: %s\n", igt_crc_to_string(reference_crc));
> @@ -1038,27 +1072,19 @@ void chamelium_assert_crc_eq_or_dump(struct
> chamelium *chamelium,
>  
>  	eq = igt_check_crc_equal(reference_crc, capture_crc);
>  	if (!eq && igt_frame_dump_is_enabled()) {
> -		/* Grab the reference frame from framebuffer */
> +		/* Convert the reference framebuffer to cairo. */
>  		reference = igt_get_cairo_surface(chamelium->drm_fd, fb);
>  
> -		/* Grab the captured frame from chamelium */
> +		/* Grab the captured frame from the Chamelium. */
>  		frame = chamelium_read_captured_frame(chamelium, index);
>  		igt_assert(frame);
>  
> +		/* Convert the captured frame to cairo. */
>  		capture = convert_frame_dump_argb32(frame);
> +		igt_assert(capture);
>  
> -		reference_suffix = igt_crc_to_string_extended(reference_crc,
> -							      '-', 2);
> -		capture_suffix = igt_crc_to_string_extended(capture_crc, '-',
> -							    2);
> -
> -		/* Write reference and capture frames to png */
> -		igt_write_compared_frames_to_png(reference, capture,
> -						 reference_suffix,
> -						 capture_suffix);
> -
> -		free(reference_suffix);
> -		free(capture_suffix);
> +		compared_frames_dump(reference, capture, reference_crc,
> +				     capture_crc);
>  
>  		cairo_surface_destroy(reference);
>  		cairo_surface_destroy(capture);
> @@ -1087,8 +1113,6 @@ void
> chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
>  	cairo_surface_t *capture;
>  	igt_crc_t *reference_crc;
>  	igt_crc_t *capture_crc;
> -	char *reference_suffix;
> -	char *capture_suffix;
>  	bool match;
>  
>  	/* Grab the reference frame from framebuffer */
> @@ -1099,27 +1123,19 @@ void
> chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
>  
>  	match = igt_check_analog_frame_match(reference, capture);
>  	if (!match && igt_frame_dump_is_enabled()) {
> -		reference_crc = chamelium_calculate_fb_crc(chamelium->drm_fd,
> -							   fb);
> -		capture_crc = chamelium_get_crc_for_area(chamelium, port, 0,
> 0,
> -							 0, 0);
> +		reference_crc = malloc(sizeof(igt_crc_t));

Need an igt_assert(reference_crc) here. With that fixed:

Reviewed-by: Lyude Paul <lyude@redhat.com>

>  
> -		igt_debug("Reference CRC: %s\n",
> -			  igt_crc_to_string(reference_crc));
> -		igt_debug("Captured CRC: %s\n",
> igt_crc_to_string(capture_crc));
> +		/* Calculate the reference frame CRC. */
> +		chamelium_do_calculate_fb_crc(reference, reference_crc);
>  
> -		reference_suffix = igt_crc_to_string_extended(reference_crc,
> -							      '-', 2);
> -		capture_suffix = igt_crc_to_string_extended(capture_crc, '-',
> -							    2);
> +		/* Get the captured frame CRC from the Chamelium. */
> +		capture_crc = chamelium_get_crc_for_area(chamelium, port, 0,
> 0,
> +							 0, 0);
> +		igt_assert(capture_crc);
>  
> -		/* Write reference and capture frames to png */
> -		igt_write_compared_frames_to_png(reference, capture,
> -						 reference_suffix,
> -						 capture_suffix);
> +		compared_frames_dump(reference, capture, reference_crc,
> +				     capture_crc);
>  
> -		free(reference_suffix);
> -		free(capture_suffix);
>  		free(reference_crc);
>  		free(capture_crc);
>  	}
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 17/21] lib/igt_chamelium: Generalize the frame match helper with check type

[Paul Kocialkowski]

In prevision of adding support for another type of frame matching,
rename chamelium_assert_analog_frame_match_or_dump to drop the
analog part and feed it the check type. This way, the bulk of the
helper can apply to other frame matching types.

This requires moving the chamelium_check enum from the test to the
common chamelium header.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_chamelium.c   | 21 +++++++++++++++------
 lib/igt_chamelium.h   | 14 ++++++++++----
 tests/kms_chamelium.c |  9 ++-------
 3 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 5966b5ce0743..17321a2cef89 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -1095,19 +1095,21 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium *chamelium,
 }
 
 /**
- * chamelium_assert_analog_frame_match_or_dump:
+ * chamelium_assert_frame_match_or_dump:
  * @chamelium: The chamelium instance the frame dump belongs to
  * @frame: The chamelium frame dump to match
  * @fb: pointer to an #igt_fb structure
+ * @check: the type of frame matching check to use
  *
  * Asserts that the provided captured frame matches the reference frame from
  * the framebuffer. If they do not, this saves the reference and captured frames
  * to a png file.
  */
-void chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
-						 struct chamelium_port *port,
-						 const struct chamelium_frame_dump *frame,
-						 struct igt_fb *fb)
+void chamelium_assert_frame_match_or_dump(struct chamelium *chamelium,
+					  struct chamelium_port *port,
+					  const struct chamelium_frame_dump *frame,
+					  struct igt_fb *fb,
+					  enum chamelium_check check)
 {
 	cairo_surface_t *reference;
 	cairo_surface_t *capture;
@@ -1121,7 +1123,14 @@ void chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
 	/* Grab the captured frame from chamelium */
 	capture = convert_frame_dump_argb32(frame);
 
-	match = igt_check_analog_frame_match(reference, capture);
+	switch (check) {
+	case CHAMELIUM_CHECK_ANALOG:
+		match = igt_check_analog_frame_match(reference, capture);
+		break;
+	default:
+		igt_assert(false);
+	}
+
 	if (!match && igt_frame_dump_is_enabled()) {
 		reference_crc = malloc(sizeof(igt_crc_t));
 
diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
index af9655a0b1cf..042ac019ccdb 100644
--- a/lib/igt_chamelium.h
+++ b/lib/igt_chamelium.h
@@ -40,6 +40,11 @@ struct chamelium_port;
 struct chamelium_frame_dump;
 struct chamelium_fb_crc_async_data;
 
+enum chamelium_check {
+	CHAMELIUM_CHECK_ANALOG,
+	CHAMELIUM_CHECK_CRC,
+};
+
 struct chamelium *chamelium_init(int drm_fd);
 void chamelium_deinit(struct chamelium *chamelium);
 void chamelium_reset(struct chamelium *chamelium);
@@ -110,10 +115,11 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium *chamelium,
 				     igt_crc_t *reference_crc,
 				     igt_crc_t *capture_crc, struct igt_fb *fb,
 				     int index);
-void chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
-						 struct chamelium_port *port,
-						 const struct chamelium_frame_dump *frame,
-						 struct igt_fb *fb);
+void chamelium_assert_frame_match_or_dump(struct chamelium *chamelium,
+					  struct chamelium_port *port,
+					  const struct chamelium_frame_dump *frame,
+					  struct igt_fb *fb,
+					  enum chamelium_check check);
 void chamelium_crop_analog_frame(struct chamelium_frame_dump *dump, int width,
 				 int height);
 void chamelium_destroy_frame_dump(struct chamelium_frame_dump *dump);
diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 64f87d3ae474..fa6fee7d9e3e 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -527,11 +527,6 @@ static int chamelium_get_pattern_fb(data_t *data, size_t width, size_t height,
 	return fb_id;
 }
 
-enum chamelium_check {
-	CHAMELIUM_CHECK_ANALOG,
-	CHAMELIUM_CHECK_CRC,
-};
-
 static void do_test_display(data_t *data, struct chamelium_port *port,
 			    igt_output_t *output, drmModeModeInfo *mode,
 			    uint32_t fourcc, enum chamelium_check check,
@@ -589,8 +584,8 @@ static void do_test_display(data_t *data, struct chamelium_port *port,
 						  0, 0);
 		chamelium_crop_analog_frame(dump, mode->hdisplay,
 					    mode->vdisplay);
-		chamelium_assert_analog_frame_match_or_dump(data->chamelium,
-							    port, dump, &fb);
+		chamelium_assert_frame_match_or_dump(data->chamelium, port,
+						     dump, &fb, check);
 		chamelium_destroy_frame_dump(dump);
 	}
 
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 17/21] lib/igt_chamelium: Generalize the frame match helper with check type

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 657 bytes --]

On Fri, Jan 11, 2019 at 10:05:28AM +0100, Paul Kocialkowski wrote:
> In prevision of adding support for another type of frame matching,
> rename chamelium_assert_analog_frame_match_or_dump to drop the
> analog part and feed it the check type. This way, the bulk of the
> helper can apply to other frame matching types.
> 
> This requires moving the chamelium_check enum from the test to the
> common chamelium header.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 17/21] lib/igt_chamelium: Generalize the frame match helper with check type

[Lyude Paul]

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> In prevision of adding support for another type of frame matching,
> rename chamelium_assert_analog_frame_match_or_dump to drop the
> analog part and feed it the check type. This way, the bulk of the
> helper can apply to other frame matching types.
> 
> This requires moving the chamelium_check enum from the test to the
> common chamelium header.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_chamelium.c   | 21 +++++++++++++++------
>  lib/igt_chamelium.h   | 14 ++++++++++----
>  tests/kms_chamelium.c |  9 ++-------
>  3 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 5966b5ce0743..17321a2cef89 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -1095,19 +1095,21 @@ void chamelium_assert_crc_eq_or_dump(struct
> chamelium *chamelium,
>  }
>  
>  /**
> - * chamelium_assert_analog_frame_match_or_dump:
> + * chamelium_assert_frame_match_or_dump:
>   * @chamelium: The chamelium instance the frame dump belongs to
>   * @frame: The chamelium frame dump to match
>   * @fb: pointer to an #igt_fb structure
> + * @check: the type of frame matching check to use
>   *
>   * Asserts that the provided captured frame matches the reference frame
> from
>   * the framebuffer. If they do not, this saves the reference and captured
> frames
>   * to a png file.
>   */
> -void chamelium_assert_analog_frame_match_or_dump(struct chamelium
> *chamelium,
> -						 struct chamelium_port *port,
> -						 const struct
> chamelium_frame_dump *frame,
> -						 struct igt_fb *fb)
> +void chamelium_assert_frame_match_or_dump(struct chamelium *chamelium,
> +					  struct chamelium_port *port,
> +					  const struct chamelium_frame_dump
> *frame,
> +					  struct igt_fb *fb,
> +					  enum chamelium_check check)
>  {
>  	cairo_surface_t *reference;
>  	cairo_surface_t *capture;
> @@ -1121,7 +1123,14 @@ void
> chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
>  	/* Grab the captured frame from chamelium */
>  	capture = convert_frame_dump_argb32(frame);
>  
> -	match = igt_check_analog_frame_match(reference, capture);
> +	switch (check) {
> +	case CHAMELIUM_CHECK_ANALOG:
> +		match = igt_check_analog_frame_match(reference, capture);
> +		break;
> +	default:
> +		igt_assert(false);
> +	}
> +
>  	if (!match && igt_frame_dump_is_enabled()) {
>  		reference_crc = malloc(sizeof(igt_crc_t));
>  
> diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
> index af9655a0b1cf..042ac019ccdb 100644
> --- a/lib/igt_chamelium.h
> +++ b/lib/igt_chamelium.h
> @@ -40,6 +40,11 @@ struct chamelium_port;
>  struct chamelium_frame_dump;
>  struct chamelium_fb_crc_async_data;
>  
> +enum chamelium_check {
> +	CHAMELIUM_CHECK_ANALOG,
> +	CHAMELIUM_CHECK_CRC,
> +};
> +
We should add some docs to this while we're at it. With that fixed:

Reviewed-by: Lyude Paul <lyude@redhat.com>
>  struct chamelium *chamelium_init(int drm_fd);
>  void chamelium_deinit(struct chamelium *chamelium);
>  void chamelium_reset(struct chamelium *chamelium);
> @@ -110,10 +115,11 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium
> *chamelium,
>  				     igt_crc_t *reference_crc,
>  				     igt_crc_t *capture_crc, struct igt_fb
> *fb,
>  				     int index);
> -void chamelium_assert_analog_frame_match_or_dump(struct chamelium
> *chamelium,
> -						 struct chamelium_port *port,
> -						 const struct
> chamelium_frame_dump *frame,
> -						 struct igt_fb *fb);
> +void chamelium_assert_frame_match_or_dump(struct chamelium *chamelium,
> +					  struct chamelium_port *port,
> +					  const struct chamelium_frame_dump
> *frame,
> +					  struct igt_fb *fb,
> +					  enum chamelium_check check);
>  void chamelium_crop_analog_frame(struct chamelium_frame_dump *dump, int
> width,
>  				 int height);
>  void chamelium_destroy_frame_dump(struct chamelium_frame_dump *dump);
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 64f87d3ae474..fa6fee7d9e3e 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -527,11 +527,6 @@ static int chamelium_get_pattern_fb(data_t *data,
> size_t width, size_t height,
>  	return fb_id;
>  }
>  
> -enum chamelium_check {
> -	CHAMELIUM_CHECK_ANALOG,
> -	CHAMELIUM_CHECK_CRC,
> -};
> -
>  static void do_test_display(data_t *data, struct chamelium_port *port,
>  			    igt_output_t *output, drmModeModeInfo *mode,
>  			    uint32_t fourcc, enum chamelium_check check,
> @@ -589,8 +584,8 @@ static void do_test_display(data_t *data, struct
> chamelium_port *port,
>  						  0, 0);
>  		chamelium_crop_analog_frame(dump, mode->hdisplay,
>  					    mode->vdisplay);
> -		chamelium_assert_analog_frame_match_or_dump(data->chamelium,
> -							    port, dump, &fb);
> +		chamelium_assert_frame_match_or_dump(data->chamelium, port,
> +						     dump, &fb, check);
>  		chamelium_destroy_frame_dump(dump);
>  	}
>  
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 18/21] lib/igt_frame: Add a checkerboard frame comparison method

[Paul Kocialkowski]

This introduces a new frame comparison method that was designed for
patterns that follow a checkerboard style. These patterns are made of
consecutive rectangular shapes with alternating solid colors. They are
currently used for some Chamelium-based tests.

The method is particularly adapted for cases where the edges of the
shapes might be blurred (e.g. due to scaling), which makes it impossible
to use pixel-perfect or CRC-based comparisons to decide whether the
captured frame matches the reference.

Overall, this test will first detect the edges of the pattern and later
exclude them from comparison. Colors are compared between the reference
and capture with a low threshold for error. A percentage of the faulty
pixels is calculated and the captured frame is considered invalid if
more than one percent of the pixels are erroneous.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_frame.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_frame.h |   2 +
 2 files changed, 123 insertions(+)

diff --git a/lib/igt_frame.c b/lib/igt_frame.c
index 6984c02e9912..e0215660ea6e 100644
--- a/lib/igt_frame.c
+++ b/lib/igt_frame.c
@@ -267,3 +267,124 @@ complete:
 
 	return match;
 }
+
+#define XR24_COLOR_VALUE(data, stride, x, y, c) \
+	*((uint8_t *)(data) + (y) * (stride) + 4 * (x) + (c))
+
+/**
+ * igt_check_checkerboard_frame_match:
+ * @reference: The reference cairo surface
+ * @capture: The captured cairo surface
+ *
+ * Checks that the reference frame matches the captured frame using a
+ * method designed for checkerboard patterns. These patterns are made of
+ * consecutive rectangular shapes with alternating solid colors.
+ *
+ * The intent of this method is to cover cases where the captured result is
+ * pixel-perfect due to features such as scaling or YUV conversion and
+ * subsampling. Such effects are mostly noticeable on the edges of the
+ * patterns, so they are detected and excluded from the comparison.
+ *
+ * Returns: a boolean indicating whether the frames match
+ */
+bool igt_check_checkerboard_frame_match(cairo_surface_t *reference,
+					cairo_surface_t *capture)
+{
+	unsigned int width, height, ref_stride, cap_stride;
+	void *ref_data, *cap_data;
+	unsigned char *edges_map;
+	unsigned int x, y, c;
+	unsigned int errors = 0, pixels = 0;
+	unsigned int edge_threshold = 100;
+	unsigned int color_error_threshold = 24;
+	double error_rate_threshold = 0.01;
+	double error_rate;
+	unsigned int span = 2;
+	bool match = false;
+
+	width = cairo_image_surface_get_width(reference);
+	height = cairo_image_surface_get_height(reference);
+
+	ref_stride = cairo_image_surface_get_stride(reference);
+	ref_data = cairo_image_surface_get_data(reference);
+	igt_assert(ref_data);
+
+	cap_stride = cairo_image_surface_get_stride(capture);
+	cap_data = cairo_image_surface_get_data(capture);
+	igt_assert(cap_data);
+
+	edges_map = calloc(1, width * height);
+
+	/* First pass to detect the pattern edges. */
+	for (y = 0; y < height; y++) {
+		if (y < span || y > (height - span - 1))
+			continue;
+
+		for (x = 0; x < width; x++) {
+			unsigned int xdiff = 0, ydiff = 0;
+
+			if (x < span || x > (width - span - 1))
+				continue;
+
+			for (c = 0; c < 3; c++) {
+				xdiff += abs(XR24_COLOR_VALUE(ref_data, ref_stride, x + span, y, c) -
+					     XR24_COLOR_VALUE(ref_data, ref_stride, x - span, y, c));
+				ydiff += abs(XR24_COLOR_VALUE(ref_data, ref_stride, x, y + span, c) -
+					     XR24_COLOR_VALUE(ref_data, ref_stride, x, y - span, c));
+			}
+
+			edges_map[y * width + x] = (xdiff > edge_threshold ||
+						    ydiff > edge_threshold);
+		}
+	}
+
+	/* Second pass to detect errors. */
+	for (y = 0; y < height; y++) {
+		for (x = 0; x < width; x++) {
+			bool error = false;
+
+			if (edges_map[y * width + x])
+				continue;
+
+			for (c = 0; c < 3; c++) {
+				unsigned int diff;
+
+				/* Compare the reference and capture values. */
+				diff = abs(XR24_COLOR_VALUE(ref_data, ref_stride, x, y, c) -
+					   XR24_COLOR_VALUE(cap_data, cap_stride, x, y, c));
+
+				if (diff > color_error_threshold)
+					error = true;
+			}
+
+			/* Allow error if coming on or off an edge (on x). */
+			if (error && x >= span && x <= (width - span - 1) &&
+			    edges_map[y * width + (x - span)] !=
+			    edges_map[y * width + (x + span)])
+				continue;
+
+			/* Allow error if coming on or off an edge (on y). */
+			if (error && y >= span && y <= (height - span - 1) &&
+			    edges_map[(y - span) * width + x] !=
+			    edges_map[(y + span) * width + x] && error)
+				continue;
+
+			if (error)
+				errors++;
+
+			pixels++;
+		}
+	}
+
+	free(edges_map);
+
+	error_rate = (double) errors / pixels;
+
+	if (error_rate < error_rate_threshold)
+		match = true;
+
+	igt_debug("Checkerboard pattern %s with error rate %f %%\n",
+		  match ? "matched" : "not matched", error_rate * 100);
+
+	return match;
+}
diff --git a/lib/igt_frame.h b/lib/igt_frame.h
index 11f96cbea203..f44f57d7ce73 100644
--- a/lib/igt_frame.h
+++ b/lib/igt_frame.h
@@ -38,5 +38,7 @@ void igt_write_compared_frames_to_png(cairo_surface_t *reference,
 				      const char *capture_suffix);
 bool igt_check_analog_frame_match(cairo_surface_t *reference,
 				  cairo_surface_t *capture);
+bool igt_check_checkerboard_frame_match(cairo_surface_t *reference,
+					cairo_surface_t *capture);
 
 #endif
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 18/21] lib/igt_frame: Add a checkerboard frame comparison method

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 6133 bytes --]

On Fri, Jan 11, 2019 at 10:05:29AM +0100, Paul Kocialkowski wrote:
> This introduces a new frame comparison method that was designed for
> patterns that follow a checkerboard style. These patterns are made of
> consecutive rectangular shapes with alternating solid colors. They are
> currently used for some Chamelium-based tests.
> 
> The method is particularly adapted for cases where the edges of the
> shapes might be blurred (e.g. due to scaling), which makes it impossible
> to use pixel-perfect or CRC-based comparisons to decide whether the
> captured frame matches the reference.
> 
> Overall, this test will first detect the edges of the pattern and later
> exclude them from comparison. Colors are compared between the reference
> and capture with a low threshold for error. A percentage of the faulty
> pixels is calculated and the captured frame is considered invalid if
> more than one percent of the pixels are erroneous.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_frame.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_frame.h |   2 +
>  2 files changed, 123 insertions(+)
> 
> diff --git a/lib/igt_frame.c b/lib/igt_frame.c
> index 6984c02e9912..e0215660ea6e 100644
> --- a/lib/igt_frame.c
> +++ b/lib/igt_frame.c
> @@ -267,3 +267,124 @@ complete:
>  
>  	return match;
>  }
> +
> +#define XR24_COLOR_VALUE(data, stride, x, y, c) \
> +	*((uint8_t *)(data) + (y) * (stride) + 4 * (x) + (c))
> +
> +/**
> + * igt_check_checkerboard_frame_match:
> + * @reference: The reference cairo surface
> + * @capture: The captured cairo surface
> + *
> + * Checks that the reference frame matches the captured frame using a
> + * method designed for checkerboard patterns. These patterns are made of
> + * consecutive rectangular shapes with alternating solid colors.
> + *
> + * The intent of this method is to cover cases where the captured result is
> + * pixel-perfect due to features such as scaling or YUV conversion and

is *not* pixel perfect?

> + * subsampling. Such effects are mostly noticeable on the edges of the
> + * patterns, so they are detected and excluded from the comparison.
> + *
> + * Returns: a boolean indicating whether the frames match
> + */
> +bool igt_check_checkerboard_frame_match(cairo_surface_t *reference,
> +					cairo_surface_t *capture)
> +{
> +	unsigned int width, height, ref_stride, cap_stride;
> +	void *ref_data, *cap_data;
> +	unsigned char *edges_map;
> +	unsigned int x, y, c;
> +	unsigned int errors = 0, pixels = 0;
> +	unsigned int edge_threshold = 100;
> +	unsigned int color_error_threshold = 24;
> +	double error_rate_threshold = 0.01;
> +	double error_rate;
> +	unsigned int span = 2;
> +	bool match = false;
> +
> +	width = cairo_image_surface_get_width(reference);
> +	height = cairo_image_surface_get_height(reference);
> +
> +	ref_stride = cairo_image_surface_get_stride(reference);
> +	ref_data = cairo_image_surface_get_data(reference);
> +	igt_assert(ref_data);
> +
> +	cap_stride = cairo_image_surface_get_stride(capture);
> +	cap_data = cairo_image_surface_get_data(capture);
> +	igt_assert(cap_data);
> +
> +	edges_map = calloc(1, width * height);
> +
> +	/* First pass to detect the pattern edges. */
> +	for (y = 0; y < height; y++) {
> +		if (y < span || y > (height - span - 1))
> +			continue;
> +
> +		for (x = 0; x < width; x++) {
> +			unsigned int xdiff = 0, ydiff = 0;
> +
> +			if (x < span || x > (width - span - 1))
> +				continue;
> +
> +			for (c = 0; c < 3; c++) {
> +				xdiff += abs(XR24_COLOR_VALUE(ref_data, ref_stride, x + span, y, c) -
> +					     XR24_COLOR_VALUE(ref_data, ref_stride, x - span, y, c));
> +				ydiff += abs(XR24_COLOR_VALUE(ref_data, ref_stride, x, y + span, c) -
> +					     XR24_COLOR_VALUE(ref_data, ref_stride, x, y - span, c));
> +			}
> +
> +			edges_map[y * width + x] = (xdiff > edge_threshold ||
> +						    ydiff > edge_threshold);
> +		}
> +	}
> +
> +	/* Second pass to detect errors. */
> +	for (y = 0; y < height; y++) {
> +		for (x = 0; x < width; x++) {
> +			bool error = false;
> +
> +			if (edges_map[y * width + x])
> +				continue;
> +
> +			for (c = 0; c < 3; c++) {
> +				unsigned int diff;
> +
> +				/* Compare the reference and capture values. */
> +				diff = abs(XR24_COLOR_VALUE(ref_data, ref_stride, x, y, c) -
> +					   XR24_COLOR_VALUE(cap_data, cap_stride, x, y, c));
> +
> +				if (diff > color_error_threshold)
> +					error = true;
> +			}
> +
> +			/* Allow error if coming on or off an edge (on x). */
> +			if (error && x >= span && x <= (width - span - 1) &&
> +			    edges_map[y * width + (x - span)] !=
> +			    edges_map[y * width + (x + span)])
> +				continue;
> +
> +			/* Allow error if coming on or off an edge (on y). */
> +			if (error && y >= span && y <= (height - span - 1) &&
> +			    edges_map[(y - span) * width + x] !=
> +			    edges_map[(y + span) * width + x] && error)
> +				continue;
> +
> +			if (error)
> +				errors++;
> +
> +			pixels++;
> +		}
> +	}
> +
> +	free(edges_map);
> +
> +	error_rate = (double) errors / pixels;
> +
> +	if (error_rate < error_rate_threshold)
> +		match = true;
> +
> +	igt_debug("Checkerboard pattern %s with error rate %f %%\n",
> +		  match ? "matched" : "not matched", error_rate * 100);
> +
> +	return match;
> +}
> diff --git a/lib/igt_frame.h b/lib/igt_frame.h
> index 11f96cbea203..f44f57d7ce73 100644
> --- a/lib/igt_frame.h
> +++ b/lib/igt_frame.h
> @@ -38,5 +38,7 @@ void igt_write_compared_frames_to_png(cairo_surface_t *reference,
>  				      const char *capture_suffix);
>  bool igt_check_analog_frame_match(cairo_surface_t *reference,
>  				  cairo_surface_t *capture);
> +bool igt_check_checkerboard_frame_match(cairo_surface_t *reference,
> +					cairo_surface_t *capture);

Looks good otherwise. I guess it's missing some comments about the
algorithm being used though.

Maxmie

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 18/21] lib/igt_frame: Add a checkerboard frame comparison method

[Paul Kocialkowski]

Hi,

On Fri, 2019-01-11 at 16:19 +0100, Maxime Ripard wrote:
> On Fri, Jan 11, 2019 at 10:05:29AM +0100, Paul Kocialkowski wrote:
> > This introduces a new frame comparison method that was designed for
> > patterns that follow a checkerboard style. These patterns are made of
> > consecutive rectangular shapes with alternating solid colors. They are
> > currently used for some Chamelium-based tests.
> > 
> > The method is particularly adapted for cases where the edges of the
> > shapes might be blurred (e.g. due to scaling), which makes it impossible
> > to use pixel-perfect or CRC-based comparisons to decide whether the
> > captured frame matches the reference.
> > 
> > Overall, this test will first detect the edges of the pattern and later
> > exclude them from comparison. Colors are compared between the reference
> > and capture with a low threshold for error. A percentage of the faulty
> > pixels is calculated and the captured frame is considered invalid if
> > more than one percent of the pixels are erroneous.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  lib/igt_frame.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/igt_frame.h |   2 +
> >  2 files changed, 123 insertions(+)
> > 
> > diff --git a/lib/igt_frame.c b/lib/igt_frame.c
> > index 6984c02e9912..e0215660ea6e 100644
> > --- a/lib/igt_frame.c
> > +++ b/lib/igt_frame.c
> > @@ -267,3 +267,124 @@ complete:
> >  
> >  	return match;
> >  }
> > +
> > +#define XR24_COLOR_VALUE(data, stride, x, y, c) \
> > +	*((uint8_t *)(data) + (y) * (stride) + 4 * (x) + (c))
> > +
> > +/**
> > + * igt_check_checkerboard_frame_match:
> > + * @reference: The reference cairo surface
> > + * @capture: The captured cairo surface
> > + *
> > + * Checks that the reference frame matches the captured frame using a
> > + * method designed for checkerboard patterns. These patterns are made of
> > + * consecutive rectangular shapes with alternating solid colors.
> > + *
> > + * The intent of this method is to cover cases where the captured result is
> > + * pixel-perfect due to features such as scaling or YUV conversion and
> 
> is *not* pixel perfect?

Woops, good catch!

[...]

> > diff --git a/lib/igt_frame.h b/lib/igt_frame.h
> > index 11f96cbea203..f44f57d7ce73 100644
> > --- a/lib/igt_frame.h
> > +++ b/lib/igt_frame.h
> > @@ -38,5 +38,7 @@ void igt_write_compared_frames_to_png(cairo_surface_t *reference,
> >  				      const char *capture_suffix);
> >  bool igt_check_analog_frame_match(cairo_surface_t *reference,
> >  				  cairo_surface_t *capture);
> > +bool igt_check_checkerboard_frame_match(cairo_surface_t *reference,
> > +					cairo_surface_t *capture);
> 
> Looks good otherwise. I guess it's missing some comments about the
> algorithm being used though.

I tried to include comments where I felt was necessary, but might have
missed out on some aspects. Or maybe you meant that the documentation
block does not provide enough detail about the algorithm itself (as
opposed to what it does)?

> Maxmie

Cheers,

Pual

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

Re: [igt-dev] [PATCH i-g-t v3 18/21] lib/igt_frame: Add a checkerboard frame comparison method

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3481 bytes --]

On Mon, Jan 14, 2019 at 04:04:16PM +0100, Paul Kocialkowski wrote:
> On Fri, 2019-01-11 at 16:19 +0100, Maxime Ripard wrote:
> > On Fri, Jan 11, 2019 at 10:05:29AM +0100, Paul Kocialkowski wrote:
> > > This introduces a new frame comparison method that was designed for
> > > patterns that follow a checkerboard style. These patterns are made of
> > > consecutive rectangular shapes with alternating solid colors. They are
> > > currently used for some Chamelium-based tests.
> > > 
> > > The method is particularly adapted for cases where the edges of the
> > > shapes might be blurred (e.g. due to scaling), which makes it impossible
> > > to use pixel-perfect or CRC-based comparisons to decide whether the
> > > captured frame matches the reference.
> > > 
> > > Overall, this test will first detect the edges of the pattern and later
> > > exclude them from comparison. Colors are compared between the reference
> > > and capture with a low threshold for error. A percentage of the faulty
> > > pixels is calculated and the captured frame is considered invalid if
> > > more than one percent of the pixels are erroneous.
> > > 
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > ---
> > >  lib/igt_frame.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  lib/igt_frame.h |   2 +
> > >  2 files changed, 123 insertions(+)
> > > 
> > > diff --git a/lib/igt_frame.c b/lib/igt_frame.c
> > > index 6984c02e9912..e0215660ea6e 100644
> > > --- a/lib/igt_frame.c
> > > +++ b/lib/igt_frame.c
> > > @@ -267,3 +267,124 @@ complete:
> > >  
> > >  	return match;
> > >  }
> > > +
> > > +#define XR24_COLOR_VALUE(data, stride, x, y, c) \
> > > +	*((uint8_t *)(data) + (y) * (stride) + 4 * (x) + (c))
> > > +
> > > +/**
> > > + * igt_check_checkerboard_frame_match:
> > > + * @reference: The reference cairo surface
> > > + * @capture: The captured cairo surface
> > > + *
> > > + * Checks that the reference frame matches the captured frame using a
> > > + * method designed for checkerboard patterns. These patterns are made of
> > > + * consecutive rectangular shapes with alternating solid colors.
> > > + *
> > > + * The intent of this method is to cover cases where the captured result is
> > > + * pixel-perfect due to features such as scaling or YUV conversion and
> > 
> > is *not* pixel perfect?
> 
> Woops, good catch!
> 
> [...]
> 
> > > diff --git a/lib/igt_frame.h b/lib/igt_frame.h
> > > index 11f96cbea203..f44f57d7ce73 100644
> > > --- a/lib/igt_frame.h
> > > +++ b/lib/igt_frame.h
> > > @@ -38,5 +38,7 @@ void igt_write_compared_frames_to_png(cairo_surface_t *reference,
> > >  				      const char *capture_suffix);
> > >  bool igt_check_analog_frame_match(cairo_surface_t *reference,
> > >  				  cairo_surface_t *capture);
> > > +bool igt_check_checkerboard_frame_match(cairo_surface_t *reference,
> > > +					cairo_surface_t *capture);
> > 
> > Looks good otherwise. I guess it's missing some comments about the
> > algorithm being used though.
> 
> I tried to include comments where I felt was necessary, but might have
> missed out on some aspects. Or maybe you meant that the documentation
> block does not provide enough detail about the algorithm itself (as
> opposed to what it does)?

Sorry if it wasn't clear enough, that's indeed what I meant.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 18/21] lib/igt_frame: Add a checkerboard frame comparison method

[Lyude Paul]

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> This introduces a new frame comparison method that was designed for
> patterns that follow a checkerboard style. These patterns are made of
> consecutive rectangular shapes with alternating solid colors. They are
> currently used for some Chamelium-based tests.
> 
> The method is particularly adapted for cases where the edges of the
> shapes might be blurred (e.g. due to scaling), which makes it impossible
> to use pixel-perfect or CRC-based comparisons to decide whether the
> captured frame matches the reference.
> 
> Overall, this test will first detect the edges of the pattern and later
> exclude them from comparison. Colors are compared between the reference
> and capture with a low threshold for error. A percentage of the faulty
> pixels is calculated and the captured frame is considered invalid if
> more than one percent of the pixels are erroneous.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_frame.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_frame.h |   2 +
>  2 files changed, 123 insertions(+)
> 
> diff --git a/lib/igt_frame.c b/lib/igt_frame.c
> index 6984c02e9912..e0215660ea6e 100644
> --- a/lib/igt_frame.c
> +++ b/lib/igt_frame.c
> @@ -267,3 +267,124 @@ complete:
>  
>  	return match;
>  }
> +
> +#define XR24_COLOR_VALUE(data, stride, x, y, c) \
> +	*((uint8_t *)(data) + (y) * (stride) + 4 * (x) + (c))
> +
> +/**
> + * igt_check_checkerboard_frame_match:
> + * @reference: The reference cairo surface
> + * @capture: The captured cairo surface
> + *
> + * Checks that the reference frame matches the captured frame using a
> + * method designed for checkerboard patterns. These patterns are made of
> + * consecutive rectangular shapes with alternating solid colors.
> + *
> + * The intent of this method is to cover cases where the captured result is
> + * pixel-perfect due to features such as scaling or YUV conversion and
> + * subsampling. Such effects are mostly noticeable on the edges of the
> + * patterns, so they are detected and excluded from the comparison.
> + *
> + * Returns: a boolean indicating whether the frames match
> + */
> +bool igt_check_checkerboard_frame_match(cairo_surface_t *reference,
> +					cairo_surface_t *capture)
> +{
> +	unsigned int width, height, ref_stride, cap_stride;
> +	void *ref_data, *cap_data;
> +	unsigned char *edges_map;
> +	unsigned int x, y, c;
> +	unsigned int errors = 0, pixels = 0;
> +	unsigned int edge_threshold = 100;
> +	unsigned int color_error_threshold = 24;
> +	double error_rate_threshold = 0.01;
> +	double error_rate;
> +	unsigned int span = 2;
> +	bool match = false;
> +
> +	width = cairo_image_surface_get_width(reference);
> +	height = cairo_image_surface_get_height(reference);
> +
> +	ref_stride = cairo_image_surface_get_stride(reference);
> +	ref_data = cairo_image_surface_get_data(reference);
> +	igt_assert(ref_data);
> +
> +	cap_stride = cairo_image_surface_get_stride(capture);
> +	cap_data = cairo_image_surface_get_data(capture);
> +	igt_assert(cap_data);
> +
> +	edges_map = calloc(1, width * height);
Need an igt_assert here

With that + the documentation changes that Maxime asked for:

Reviewed-by: Lyude Paul <lyude@redhat.com>
> +
> +	/* First pass to detect the pattern edges. */
> +	for (y = 0; y < height; y++) {
> +		if (y < span || y > (height - span - 1))
> +			continue;
> +
> +		for (x = 0; x < width; x++) {
> +			unsigned int xdiff = 0, ydiff = 0;
> +
> +			if (x < span || x > (width - span - 1))
> +				continue;
> +
> +			for (c = 0; c < 3; c++) {
> +				xdiff += abs(XR24_COLOR_VALUE(ref_data,
> ref_stride, x + span, y, c) -
> +					     XR24_COLOR_VALUE(ref_data,
> ref_stride, x - span, y, c));
> +				ydiff += abs(XR24_COLOR_VALUE(ref_data,
> ref_stride, x, y + span, c) -
> +					     XR24_COLOR_VALUE(ref_data,
> ref_stride, x, y - span, c));
> +			}
> +
> +			edges_map[y * width + x] = (xdiff > edge_threshold ||
> +						    ydiff > edge_threshold);
> +		}
> +	}
> +
> +	/* Second pass to detect errors. */
> +	for (y = 0; y < height; y++) {
> +		for (x = 0; x < width; x++) {
> +			bool error = false;
> +
> +			if (edges_map[y * width + x])
> +				continue;
> +
> +			for (c = 0; c < 3; c++) {
> +				unsigned int diff;
> +
> +				/* Compare the reference and capture values.
> */
> +				diff = abs(XR24_COLOR_VALUE(ref_data,
> ref_stride, x, y, c) -
> +					   XR24_COLOR_VALUE(cap_data,
> cap_stride, x, y, c));
> +
> +				if (diff > color_error_threshold)
> +					error = true;
> +			}
> +
> +			/* Allow error if coming on or off an edge (on x). */
> +			if (error && x >= span && x <= (width - span - 1) &&
> +			    edges_map[y * width + (x - span)] !=
> +			    edges_map[y * width + (x + span)])
> +				continue;
> +
> +			/* Allow error if coming on or off an edge (on y). */
> +			if (error && y >= span && y <= (height - span - 1) &&
> +			    edges_map[(y - span) * width + x] !=
> +			    edges_map[(y + span) * width + x] && error)
> +				continue;
> +
> +			if (error)
> +				errors++;
> +
> +			pixels++;
> +		}
> +	}
> +
> +	free(edges_map);
> +
> +	error_rate = (double) errors / pixels;
> +
> +	if (error_rate < error_rate_threshold)
> +		match = true;
> +
> +	igt_debug("Checkerboard pattern %s with error rate %f %%\n",
> +		  match ? "matched" : "not matched", error_rate * 100);
> +
> +	return match;
> +}
> diff --git a/lib/igt_frame.h b/lib/igt_frame.h
> index 11f96cbea203..f44f57d7ce73 100644
> --- a/lib/igt_frame.h
> +++ b/lib/igt_frame.h
> @@ -38,5 +38,7 @@ void igt_write_compared_frames_to_png(cairo_surface_t
> *reference,
>  				      const char *capture_suffix);
>  bool igt_check_analog_frame_match(cairo_surface_t *reference,
>  				  cairo_surface_t *capture);
> +bool igt_check_checkerboard_frame_match(cairo_surface_t *reference,
> +					cairo_surface_t *capture);
>  
>  #endif
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 19/21] lib/igt_chamelium: Hook-in checkerboard comparison method in helpers

[Paul Kocialkowski]

This introduces the checkerboard chamelium checking type and hooks the
call to the associated igt_frame matching helper in the frame match
chamelium helper.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 lib/igt_chamelium.c | 4 +++-
 lib/igt_chamelium.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 17321a2cef89..731259f2f196 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -1127,6 +1127,9 @@ void chamelium_assert_frame_match_or_dump(struct chamelium *chamelium,
 	case CHAMELIUM_CHECK_ANALOG:
 		match = igt_check_analog_frame_match(reference, capture);
 		break;
+	case CHAMELIUM_CHECK_CHECKERBOARD:
+		match = igt_check_checkerboard_frame_match(reference, capture);
+		break;
 	default:
 		igt_assert(false);
 	}
@@ -1155,7 +1158,6 @@ void chamelium_assert_frame_match_or_dump(struct chamelium *chamelium,
 	cairo_surface_destroy(capture);
 }
 
-
 /**
  * chamelium_analog_frame_crop:
  * @chamelium: The Chamelium instance to use
diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
index 042ac019ccdb..d6159307f240 100644
--- a/lib/igt_chamelium.h
+++ b/lib/igt_chamelium.h
@@ -42,6 +42,7 @@ struct chamelium_fb_crc_async_data;
 
 enum chamelium_check {
 	CHAMELIUM_CHECK_ANALOG,
+	CHAMELIUM_CHECK_CHECKERBOARD,
 	CHAMELIUM_CHECK_CRC,
 };
 
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 19/21] lib/igt_chamelium: Hook-in checkerboard comparison method in helpers

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 465 bytes --]

On Fri, Jan 11, 2019 at 10:05:30AM +0100, Paul Kocialkowski wrote:
> This introduces the checkerboard chamelium checking type and hooks the
> call to the associated igt_frame matching helper in the frame match
> chamelium helper.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 19/21] lib/igt_chamelium: Hook-in checkerboard comparison method in helpers

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> This introduces the checkerboard chamelium checking type and hooks the
> call to the associated igt_frame matching helper in the frame match
> chamelium helper.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  lib/igt_chamelium.c | 4 +++-
>  lib/igt_chamelium.h | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 17321a2cef89..731259f2f196 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -1127,6 +1127,9 @@ void chamelium_assert_frame_match_or_dump(struct
> chamelium *chamelium,
>  	case CHAMELIUM_CHECK_ANALOG:
>  		match = igt_check_analog_frame_match(reference, capture);
>  		break;
> +	case CHAMELIUM_CHECK_CHECKERBOARD:
> +		match = igt_check_checkerboard_frame_match(reference,
> capture);
> +		break;
>  	default:
>  		igt_assert(false);
>  	}
> @@ -1155,7 +1158,6 @@ void chamelium_assert_frame_match_or_dump(struct
> chamelium *chamelium,
>  	cairo_surface_destroy(capture);
>  }
>  
> -
>  /**
>   * chamelium_analog_frame_crop:
>   * @chamelium: The Chamelium instance to use
> diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
> index 042ac019ccdb..d6159307f240 100644
> --- a/lib/igt_chamelium.h
> +++ b/lib/igt_chamelium.h
> @@ -42,6 +42,7 @@ struct chamelium_fb_crc_async_data;
>  
>  enum chamelium_check {
>  	CHAMELIUM_CHECK_ANALOG,
> +	CHAMELIUM_CHECK_CHECKERBOARD,
>  	CHAMELIUM_CHECK_CRC,
>  };
>  
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 20/21] chamelium: Move the YUV tests over to the checkerboard checking method

[Paul Kocialkowski]

Now that the checkerboard frame checking method is available through
the frame match helper, make use of it in YUV tests to increase the
reliability of the results.

The analog test tends to provide false positives, which are properly
detected by the checkerboard method in most instances.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 tests/kms_chamelium.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index fa6fee7d9e3e..2779d74f55b6 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -575,15 +575,19 @@ static void do_test_display(data_t *data, struct chamelium_port *port,
 
 		free(expected_crc);
 		free(crc);
-	} else if (check == CHAMELIUM_CHECK_ANALOG) {
+	} else if (check == CHAMELIUM_CHECK_ANALOG ||
+		   check == CHAMELIUM_CHECK_CHECKERBOARD) {
 		struct chamelium_frame_dump *dump;
 
 		igt_assert(count == 1);
 
 		dump = chamelium_port_dump_pixels(data->chamelium, port, 0, 0,
 						  0, 0);
-		chamelium_crop_analog_frame(dump, mode->hdisplay,
-					    mode->vdisplay);
+
+		if (check == CHAMELIUM_CHECK_ANALOG)
+			chamelium_crop_analog_frame(dump, mode->hdisplay,
+						    mode->vdisplay);
+
 		chamelium_assert_frame_match_or_dump(data->chamelium, port,
 						     dump, &fb, check);
 		chamelium_destroy_frame_dump(dump);
@@ -974,35 +978,35 @@ igt_main
 
 		connector_subtest("hdmi-cmp-nv12", HDMIA)
 			test_display_one_mode(&data, port, DRM_FORMAT_NV12,
-					      CHAMELIUM_CHECK_ANALOG, 1);
+					      CHAMELIUM_CHECK_CHECKERBOARD, 1);
 
 		connector_subtest("hdmi-cmp-nv16", HDMIA)
 			test_display_one_mode(&data, port, DRM_FORMAT_NV16,
-					      CHAMELIUM_CHECK_ANALOG, 1);
+					      CHAMELIUM_CHECK_CHECKERBOARD, 1);
 
 		connector_subtest("hdmi-cmp-nv21", HDMIA)
 			test_display_one_mode(&data, port, DRM_FORMAT_NV21,
-					      CHAMELIUM_CHECK_ANALOG, 1);
+					      CHAMELIUM_CHECK_CHECKERBOARD, 1);
 
 		connector_subtest("hdmi-cmp-nv61", HDMIA)
 			test_display_one_mode(&data, port, DRM_FORMAT_NV61,
-					      CHAMELIUM_CHECK_ANALOG, 1);
+					      CHAMELIUM_CHECK_CHECKERBOARD, 1);
 
 		connector_subtest("hdmi-cmp-yu12", HDMIA)
 			test_display_one_mode(&data, port, DRM_FORMAT_YUV420,
-					      CHAMELIUM_CHECK_ANALOG, 1);
+					      CHAMELIUM_CHECK_CHECKERBOARD, 1);
 
 		connector_subtest("hdmi-cmp-yu16", HDMIA)
 			test_display_one_mode(&data, port, DRM_FORMAT_YUV422,
-					      CHAMELIUM_CHECK_ANALOG, 1);
+					      CHAMELIUM_CHECK_CHECKERBOARD, 1);
 
 		connector_subtest("hdmi-cmp-yv12", HDMIA)
 			test_display_one_mode(&data, port, DRM_FORMAT_YVU420,
-					      CHAMELIUM_CHECK_ANALOG, 1);
+					      CHAMELIUM_CHECK_CHECKERBOARD, 1);
 
 		connector_subtest("hdmi-cmp-yv16", HDMIA)
 			test_display_one_mode(&data, port, DRM_FORMAT_YVU422,
-					      CHAMELIUM_CHECK_ANALOG, 1);
+					      CHAMELIUM_CHECK_CHECKERBOARD, 1);
 
 		connector_subtest("hdmi-frame-dump", HDMIA)
 			test_display_frame_dump(&data, port);
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 20/21] chamelium: Move the YUV tests over to the checkerboard checking method

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 607 bytes --]

On Fri, Jan 11, 2019 at 10:05:31AM +0100, Paul Kocialkowski wrote:
> Now that the checkerboard frame checking method is available through
> the frame match helper, make use of it in YUV tests to increase the
> reliability of the results.
> 
> The analog test tends to provide false positives, which are properly
> detected by the checkerboard method in most instances.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 20/21] chamelium: Move the YUV tests over to the checkerboard checking method

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> Now that the checkerboard frame checking method is available through
> the frame match helper, make use of it in YUV tests to increase the
> reliability of the results.
> 
> The analog test tends to provide false positives, which are properly
> detected by the checkerboard method in most instances.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  tests/kms_chamelium.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index fa6fee7d9e3e..2779d74f55b6 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -575,15 +575,19 @@ static void do_test_display(data_t *data, struct
> chamelium_port *port,
>  
>  		free(expected_crc);
>  		free(crc);
> -	} else if (check == CHAMELIUM_CHECK_ANALOG) {
> +	} else if (check == CHAMELIUM_CHECK_ANALOG ||
> +		   check == CHAMELIUM_CHECK_CHECKERBOARD) {
>  		struct chamelium_frame_dump *dump;
>  
>  		igt_assert(count == 1);
>  
>  		dump = chamelium_port_dump_pixels(data->chamelium, port, 0, 0,
>  						  0, 0);
> -		chamelium_crop_analog_frame(dump, mode->hdisplay,
> -					    mode->vdisplay);
> +
> +		if (check == CHAMELIUM_CHECK_ANALOG)
> +			chamelium_crop_analog_frame(dump, mode->hdisplay,
> +						    mode->vdisplay);
> +
>  		chamelium_assert_frame_match_or_dump(data->chamelium, port,
>  						     dump, &fb, check);
>  		chamelium_destroy_frame_dump(dump);
> @@ -974,35 +978,35 @@ igt_main
>  
>  		connector_subtest("hdmi-cmp-nv12", HDMIA)
>  			test_display_one_mode(&data, port, DRM_FORMAT_NV12,
> -					      CHAMELIUM_CHECK_ANALOG, 1);
> +					      CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
>  
>  		connector_subtest("hdmi-cmp-nv16", HDMIA)
>  			test_display_one_mode(&data, port, DRM_FORMAT_NV16,
> -					      CHAMELIUM_CHECK_ANALOG, 1);
> +					      CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
>  
>  		connector_subtest("hdmi-cmp-nv21", HDMIA)
>  			test_display_one_mode(&data, port, DRM_FORMAT_NV21,
> -					      CHAMELIUM_CHECK_ANALOG, 1);
> +					      CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
>  
>  		connector_subtest("hdmi-cmp-nv61", HDMIA)
>  			test_display_one_mode(&data, port, DRM_FORMAT_NV61,
> -					      CHAMELIUM_CHECK_ANALOG, 1);
> +					      CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
>  
>  		connector_subtest("hdmi-cmp-yu12", HDMIA)
>  			test_display_one_mode(&data, port, DRM_FORMAT_YUV420,
> -					      CHAMELIUM_CHECK_ANALOG, 1);
> +					      CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
>  
>  		connector_subtest("hdmi-cmp-yu16", HDMIA)
>  			test_display_one_mode(&data, port, DRM_FORMAT_YUV422,
> -					      CHAMELIUM_CHECK_ANALOG, 1);
> +					      CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
>  
>  		connector_subtest("hdmi-cmp-yv12", HDMIA)
>  			test_display_one_mode(&data, port, DRM_FORMAT_YVU420,
> -					      CHAMELIUM_CHECK_ANALOG, 1);
> +					      CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
>  
>  		connector_subtest("hdmi-cmp-yv16", HDMIA)
>  			test_display_one_mode(&data, port, DRM_FORMAT_YVU422,
> -					      CHAMELIUM_CHECK_ANALOG, 1);
> +					      CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
>  
>  		connector_subtest("hdmi-frame-dump", HDMIA)
>  			test_display_frame_dump(&data, port);
-- 
Cheers,
	Lyude Paul

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

[igt-dev] [PATCH i-g-t v3 21/21] chamelium: Add a display test for randomized planes

[Paul Kocialkowski]

This introduces a new test for the Chamelium, that sets up planes
with randomized properties such as the format, dimensions, position,
in-framebuffer offsets and stride. The Chamelium capture is checked
against the reference generated by cairo with either a CRC or the
checkerboard-specific comparison method.

This test also includes testing of the VC4-specific T-tiled and
SAND-tiled modes, in all formats supported by the hardware.

Since this test does not share much with previous Chamelium display
tests (especially regarding KMS configuration), most of the code is
not shared with other tests.

This test can be derived with reproducible properties for regression
testing in the future. For now, it serves as a fuzzing test

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 tests/kms_chamelium.c | 446 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 446 insertions(+)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 2779d74f55b6..5963964878ba 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -26,6 +26,7 @@
 
 #include "config.h"
 #include "igt.h"
+#include "igt_vc4.h"
 
 #include <fcntl.h>
 #include <string.h>
@@ -702,6 +703,443 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
 	drmModeFreeConnector(connector);
 }
 
+static void randomize_plane_format_stride(igt_plane_t *plane, uint32_t width,
+					  uint32_t *format, size_t *stride,
+					  bool *tiled, bool allow_yuv)
+{
+	size_t stride_min;
+	uint32_t *formats_array;
+	unsigned int formats_count;
+	unsigned int count = 0;
+	unsigned int i;
+	int index;
+
+	igt_format_array_fill(&formats_array, &formats_count, allow_yuv);
+
+	/* First pass to count the supported formats. */
+	for (i = 0; i < formats_count; i++)
+		if (igt_plane_has_format_mod(plane, formats_array[i],
+					     DRM_FORMAT_MOD_LINEAR))
+			count++;
+
+	igt_assert(count > 0);
+
+	index = rand() % count;
+
+	/* Second pass to get the index-th supported format. */
+	for (i = 0; i < formats_count; i++) {
+		if (!igt_plane_has_format_mod(plane, formats_array[i],
+					      DRM_FORMAT_MOD_LINEAR))
+			continue;
+
+		if (!index--) {
+			*format = formats_array[i];
+			break;
+		}
+	}
+
+	free(formats_array);
+
+	igt_assert(index < 0);
+
+	stride_min = width * igt_format_plane_bpp(*format, 0) / 8;
+
+	/* Randomize the stride with at most twice the minimum. */
+	*stride = (rand() % stride_min) + stride_min;
+
+	/* Pixman requires the stride to be aligned to 32-byte words. */
+	*stride = ALIGN(*stride, sizeof(uint32_t));
+
+	/* Randomize the use of a tiled mode with a 1/4 probability. */
+	*tiled = ((rand() % 4) == 0);
+}
+
+static void randomize_plane_dimensions(drmModeModeInfo *mode,
+				       uint32_t *width, uint32_t *height,
+				       uint32_t *src_w, uint32_t *src_h,
+				       uint32_t *src_x, uint32_t *src_y,
+				       uint32_t *crtc_w, uint32_t *crtc_h,
+				       int32_t *crtc_x, int32_t *crtc_y,
+				       bool allow_scaling)
+{
+	double ratio;
+
+	/* Randomize width and height in the mode dimensions range. */
+	*width = (rand() % mode->hdisplay) + 1;
+	*height = (rand() % mode->vdisplay) + 1;
+
+	/*
+	 * Randomize source offset, but keep at least half of the
+	 * original size.
+	 */
+	*src_x = rand() % (*width / 2);
+	*src_y = rand() % (*height / 2);
+
+	/* The source size only includes the active source area. */
+	*src_w = *width - *src_x;
+	*src_h = *height - *src_y;
+
+	if (allow_scaling) {
+		*crtc_w = (rand() % mode->hdisplay) + 1;
+		*crtc_h = (rand() % mode->vdisplay) + 1;
+
+		/*
+		 * Don't bother with scaling if dimensions are quite close in
+		 * order to get non-scaling cases more frequently. Also limit
+		 * scaling to 3x to avoid agressive filtering that makes
+		 * comparison less reliable.
+		 */
+
+		ratio = ((double) *crtc_w / *src_w);
+		if (ratio > 0.8 && ratio < 1.2)
+			*crtc_w = *src_w;
+		else if (ratio > 3.0)
+			*crtc_w = *src_w * 3;
+
+		ratio = ((double) *crtc_h / *src_h);
+		if (ratio > 0.8 && ratio < 1.2)
+			*crtc_h = *src_h;
+		else if (ratio > 3.0)
+			*crtc_h = *src_h * 3;
+	} else {
+		*crtc_w = *src_w;
+		*crtc_h = *src_h;
+	}
+
+	if (*crtc_w != *src_w || *crtc_h != *src_h) {
+		/*
+		 * When scaling is involved, make sure to not go off-bounds or
+		 * scaled clipping may result in decimal dimensions, that most
+		 * drivers don't support.
+		 */
+		*crtc_x = rand() % (mode->hdisplay - *crtc_w);
+		*crtc_y = rand() % (mode->vdisplay - *crtc_h);
+	} else {
+		/*
+		 * Randomize the on-crtc position and allow the plane to go
+		 * off-display by up to half of its on-crtc width and height.
+		 */
+		*crtc_x = (rand() % mode->hdisplay) - *crtc_w / 2;
+		*crtc_y = (rand() % mode->vdisplay) - *crtc_h / 2;
+	}
+}
+
+static void blit_plane_cairo(data_t *data, cairo_surface_t *result,
+			     uint32_t src_w, uint32_t src_h,
+			     uint32_t src_x, uint32_t src_y,
+			     uint32_t crtc_w, uint32_t crtc_h,
+			     int32_t crtc_x, int32_t crtc_y,
+			     struct igt_fb *fb)
+{
+	cairo_surface_t *surface;
+	cairo_surface_t *clipped_surface;
+	cairo_t *cr;
+
+	surface = igt_get_cairo_surface(data->drm_fd, fb);
+
+	if (src_x || src_y) {
+		clipped_surface = cairo_image_surface_create(CAIRO_FORMAT_RGB24,
+							     src_w, src_h);
+
+		cr = cairo_create(clipped_surface);
+
+		cairo_translate(cr, -1. * src_x, -1. * src_y);
+
+		cairo_set_source_surface(cr, surface, 0, 0);
+
+		cairo_paint(cr);
+		cairo_surface_flush(clipped_surface);
+
+		cairo_destroy(cr);
+	} else {
+		clipped_surface = surface;
+	}
+
+	cr = cairo_create(result);
+
+	cairo_translate(cr, crtc_x, crtc_y);
+
+	if (src_w != crtc_w || src_h != crtc_h) {
+		cairo_scale(cr, (double) crtc_w / src_w,
+			    (double) crtc_h / src_h);
+	}
+
+	cairo_set_source_surface(cr, clipped_surface, 0, 0);
+	cairo_surface_destroy(clipped_surface);
+
+	if (src_w != crtc_w || src_h != crtc_h) {
+		cairo_pattern_set_filter(cairo_get_source(cr),
+					 CAIRO_FILTER_BILINEAR);
+		cairo_pattern_set_extend(cairo_get_source(cr),
+					 CAIRO_EXTEND_NONE);
+	}
+
+	cairo_paint(cr);
+	cairo_surface_flush(result);
+
+	cairo_destroy(cr);
+}
+
+static void configure_plane(igt_plane_t *plane, uint32_t src_w, uint32_t src_h,
+			    uint32_t src_x, uint32_t src_y, uint32_t crtc_w,
+			    uint32_t crtc_h, int32_t crtc_x, int32_t crtc_y,
+			    struct igt_fb *fb)
+{
+	igt_plane_set_fb(plane, fb);
+
+	igt_plane_set_position(plane, crtc_x, crtc_y);
+	igt_plane_set_size(plane, crtc_w, crtc_h);
+
+	igt_fb_set_position(fb, plane, src_x, src_y);
+	igt_fb_set_size(fb, plane, src_w, src_h);
+}
+
+static void prepare_tiled_plane(data_t *data, igt_plane_t *plane,
+				unsigned int index, uint32_t format,
+				struct igt_fb *overlay_fb)
+{
+	struct igt_fb tiled_fb;
+	unsigned int fb_id;
+
+	if (is_vc4_device(data->drm_fd) &&
+	    igt_vc4_plane_supports_t_tiling(plane, format)) {
+		igt_debug("Plane %d: using VC4 T-tiling\n", index);
+
+		fb_id = igt_vc4_fb_t_tiled_convert(&tiled_fb, overlay_fb);
+	} else if (is_vc4_device(data->drm_fd) &&
+		   igt_vc4_plane_supports_sand_tiling(plane, format, 256)) {
+		/* Randomize the column height with at most twice the height. */
+		size_t column_height = (rand() % overlay_fb->height) +
+				       overlay_fb->height;
+
+		igt_debug("Plane %d: using VC4 SAND256 tiling with column height %ld\n",
+			  index, column_height);
+
+		fb_id = vc4_fb_sand_tiled_convert(&tiled_fb, overlay_fb, 256,
+						  column_height);
+	} else {
+		return;
+	}
+
+	igt_assert(fb_id > 0);
+
+	/* Make the tiled framebuffer the overlay. */
+	igt_remove_fb(data->drm_fd, overlay_fb);
+	*overlay_fb = tiled_fb;
+}
+
+static void prepare_randomized_plane(data_t *data,
+				     drmModeModeInfo *mode,
+				     igt_plane_t *plane,
+				     struct igt_fb *overlay_fb,
+				     unsigned int index,
+				     cairo_surface_t *result_surface,
+				     bool allow_scaling, bool allow_yuv)
+{
+	struct igt_fb pattern_fb;
+	uint32_t overlay_fb_w, overlay_fb_h;
+	uint32_t overlay_src_w, overlay_src_h;
+	uint32_t overlay_src_x, overlay_src_y;
+	int32_t overlay_crtc_x, overlay_crtc_y;
+	uint32_t overlay_crtc_w, overlay_crtc_h;
+	uint32_t format;
+	size_t stride;
+	bool tiled;
+	int fb_id;
+
+	randomize_plane_dimensions(mode, &overlay_fb_w, &overlay_fb_h,
+				   &overlay_src_w, &overlay_src_h,
+				   &overlay_src_x, &overlay_src_y,
+				   &overlay_crtc_w, &overlay_crtc_h,
+				   &overlay_crtc_x, &overlay_crtc_y,
+				   allow_scaling);
+
+	igt_debug("Plane %d: framebuffer size %dx%d\n", index,
+		  overlay_fb_w, overlay_fb_h);
+	igt_debug("Plane %d: on-crtc size %dx%d\n", index,
+		  overlay_crtc_w, overlay_crtc_h);
+	igt_debug("Plane %d: on-crtc position %dx%d\n", index,
+		  overlay_crtc_x, overlay_crtc_y);
+	igt_debug("Plane %d: in-framebuffer size %dx%d\n", index,
+		  overlay_src_w, overlay_src_h);
+	igt_debug("Plane %d: in-framebuffer position %dx%d\n", index,
+		  overlay_src_x, overlay_src_y);
+
+	/* Get a pattern framebuffer for the overlay plane. */
+	fb_id = chamelium_get_pattern_fb(data, overlay_fb_w, overlay_fb_h,
+					 DRM_FORMAT_XRGB8888, 32, &pattern_fb);
+	igt_assert(fb_id > 0);
+
+	randomize_plane_format_stride(plane, overlay_fb_w, &format, &stride,
+				      &tiled, allow_yuv);
+
+	igt_debug("Plane %d: %s format with stride %ld\n", index,
+		  igt_format_str(format), stride);
+
+	fb_id = igt_fb_convert_with_stride(overlay_fb, &pattern_fb, format,
+					   stride);
+	igt_assert(fb_id > 0);
+
+	if (tiled)
+		prepare_tiled_plane(data, plane, index, format, overlay_fb);
+
+	blit_plane_cairo(data, result_surface, overlay_src_w, overlay_src_h,
+			 overlay_src_x, overlay_src_y,
+			 overlay_crtc_w, overlay_crtc_h,
+			 overlay_crtc_x, overlay_crtc_y, &pattern_fb);
+
+	configure_plane(plane, overlay_src_w, overlay_src_h,
+			overlay_src_x, overlay_src_y,
+			overlay_crtc_w, overlay_crtc_h,
+			overlay_crtc_x, overlay_crtc_y, overlay_fb);
+
+	/* Remove the original pattern framebuffer. */
+	igt_remove_fb(data->drm_fd, &pattern_fb);
+}
+
+static void test_display_planes_random(data_t *data,
+				       struct chamelium_port *port,
+				       enum chamelium_check check)
+{
+	igt_output_t *output;
+	drmModeModeInfo *mode;
+	igt_plane_t *primary_plane;
+	struct igt_fb primary_fb;
+	struct igt_fb result_fb;
+	struct igt_fb *overlay_fbs;
+	igt_crc_t *crc;
+	igt_crc_t *expected_crc;
+	struct chamelium_fb_crc_async_data *fb_crc;
+	unsigned int overlay_planes_max = 0;
+	unsigned int overlay_planes_count;
+	cairo_surface_t *result_surface;
+	int captured_frame_count;
+	bool allow_scaling;
+	bool allow_yuv;
+	unsigned int i;
+	unsigned int fb_id;
+
+	switch (check) {
+	case CHAMELIUM_CHECK_CRC:
+		allow_scaling = false;
+		allow_yuv = false;
+		break;
+	case CHAMELIUM_CHECK_CHECKERBOARD:
+		allow_scaling = true;
+		allow_yuv = true;
+		break;
+	default:
+		igt_assert(false);
+	}
+
+	srand(time(NULL));
+
+	reset_state(data, port);
+
+	/* Find the connector and pipe. */
+	output = prepare_output(data, port);
+
+	mode = igt_output_get_mode(output);
+
+	/* Get a framebuffer for the primary plane. */
+	primary_plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_assert(primary_plane);
+
+	fb_id = chamelium_get_pattern_fb(data, mode->hdisplay, mode->vdisplay,
+					 DRM_FORMAT_XRGB8888, 64, &primary_fb);
+	igt_assert(fb_id > 0);
+
+	/* Get a framebuffer for the cairo composition result. */
+	fb_id = igt_create_fb(data->drm_fd, mode->hdisplay,
+			      mode->vdisplay, DRM_FORMAT_XRGB8888,
+			      LOCAL_DRM_FORMAT_MOD_NONE, &result_fb);
+	igt_assert(fb_id > 0);
+
+	result_surface = igt_get_cairo_surface(data->drm_fd, &result_fb);
+
+	/* Paint the primary framebuffer on the result surface. */
+	blit_plane_cairo(data, result_surface, 0, 0, 0, 0, 0, 0, 0, 0,
+			 &primary_fb);
+
+	/* Configure the primary plane. */
+	igt_plane_set_fb(primary_plane, &primary_fb);
+
+	overlay_planes_max =
+		igt_output_count_plane_type(output, DRM_PLANE_TYPE_OVERLAY);
+
+	/* Limit the number of planes to a reasonable scene. */
+	if (overlay_planes_max > 4)
+		overlay_planes_max = 4;
+
+	overlay_planes_count = (rand() % overlay_planes_max) + 1;
+	igt_debug("Using %d overlay planes\n", overlay_planes_count);
+
+	overlay_fbs = calloc(sizeof(struct igt_fb), overlay_planes_count);
+
+	for (i = 0; i < overlay_planes_count; i++) {
+		struct igt_fb *overlay_fb = &overlay_fbs[i];
+		igt_plane_t *plane =
+			igt_output_get_plane_type_index(output,
+							DRM_PLANE_TYPE_OVERLAY,
+							i);
+		igt_assert(plane);
+
+		prepare_randomized_plane(data, mode, plane, overlay_fb, i,
+					 result_surface, allow_scaling,
+					 allow_yuv);
+	}
+
+	cairo_surface_destroy(result_surface);
+
+	if (check == CHAMELIUM_CHECK_CRC)
+		fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
+								&result_fb);
+
+	igt_display_commit2(&data->display, COMMIT_ATOMIC);
+
+	if (check == CHAMELIUM_CHECK_CRC) {
+		chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
+		crc = chamelium_read_captured_crcs(data->chamelium,
+						   &captured_frame_count);
+
+		igt_assert(captured_frame_count == 1);
+
+		expected_crc = chamelium_calculate_fb_crc_async_finish(fb_crc);
+
+		chamelium_assert_crc_eq_or_dump(data->chamelium,
+						expected_crc, crc,
+						&result_fb, 0);
+
+		free(expected_crc);
+		free(crc);
+	} else if (check == CHAMELIUM_CHECK_CHECKERBOARD) {
+		struct chamelium_frame_dump *dump;
+
+		dump = chamelium_port_dump_pixels(data->chamelium, port, 0, 0,
+						  0, 0);
+		chamelium_assert_frame_match_or_dump(data->chamelium, port,
+						     dump, &result_fb, check);
+		chamelium_destroy_frame_dump(dump);
+	}
+
+	for (i = 0; i < overlay_planes_count; i++) {
+		struct igt_fb *overlay_fb = &overlay_fbs[i];
+		igt_plane_t *plane;
+
+		plane = igt_output_get_plane_type_index(output,
+							DRM_PLANE_TYPE_OVERLAY,
+							i);
+		igt_assert(plane);
+
+		igt_remove_fb(data->drm_fd, overlay_fb);
+	}
+
+	free(overlay_fbs);
+
+	igt_remove_fb(data->drm_fd, &primary_fb);
+	igt_remove_fb(data->drm_fd, &result_fb);
+}
+
 static void
 test_hpd_without_ddc(data_t *data, struct chamelium_port *port)
 {
@@ -976,6 +1414,10 @@ igt_main
 			test_display_one_mode(&data, port, DRM_FORMAT_XRGB1555,
 					      CHAMELIUM_CHECK_CRC, 1);
 
+		connector_subtest("hdmi-crc-planes-random", HDMIA)
+			test_display_planes_random(&data, port,
+						   CHAMELIUM_CHECK_CRC);
+
 		connector_subtest("hdmi-cmp-nv12", HDMIA)
 			test_display_one_mode(&data, port, DRM_FORMAT_NV12,
 					      CHAMELIUM_CHECK_CHECKERBOARD, 1);
@@ -1008,6 +1450,10 @@ igt_main
 			test_display_one_mode(&data, port, DRM_FORMAT_YVU422,
 					      CHAMELIUM_CHECK_CHECKERBOARD, 1);
 
+		connector_subtest("hdmi-cmp-planes-random", HDMIA)
+			test_display_planes_random(&data, port,
+						   CHAMELIUM_CHECK_CHECKERBOARD);
+
 		connector_subtest("hdmi-frame-dump", HDMIA)
 			test_display_frame_dump(&data, port);
 	}
-- 
2.20.1

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

Re: [igt-dev] [PATCH i-g-t v3 21/21] chamelium: Add a display test for randomized planes

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1075 bytes --]

On Fri, Jan 11, 2019 at 10:05:32AM +0100, Paul Kocialkowski wrote:
> This introduces a new test for the Chamelium, that sets up planes
> with randomized properties such as the format, dimensions, position,
> in-framebuffer offsets and stride. The Chamelium capture is checked
> against the reference generated by cairo with either a CRC or the
> checkerboard-specific comparison method.
> 
> This test also includes testing of the VC4-specific T-tiled and
> SAND-tiled modes, in all formats supported by the hardware.
> 
> Since this test does not share much with previous Chamelium display
> tests (especially regarding KMS configuration), most of the code is
> not shared with other tests.
> 
> This test can be derived with reproducible properties for regression
> testing in the future. For now, it serves as a fuzzing test
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Reviewed-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [igt-dev] [PATCH i-g-t v3 21/21] chamelium: Add a display test for randomized planes

[Lyude Paul]

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> This introduces a new test for the Chamelium, that sets up planes
> with randomized properties such as the format, dimensions, position,
> in-framebuffer offsets and stride. The Chamelium capture is checked
> against the reference generated by cairo with either a CRC or the
> checkerboard-specific comparison method.
> 
> This test also includes testing of the VC4-specific T-tiled and
> SAND-tiled modes, in all formats supported by the hardware.
> 
> Since this test does not share much with previous Chamelium display
> tests (especially regarding KMS configuration), most of the code is
> not shared with other tests.
> 
> This test can be derived with reproducible properties for regression
> testing in the future. For now, it serves as a fuzzing test
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  tests/kms_chamelium.c | 446 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 446 insertions(+)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 2779d74f55b6..5963964878ba 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -26,6 +26,7 @@
>  
>  #include "config.h"
>  #include "igt.h"
> +#include "igt_vc4.h"
>  
>  #include <fcntl.h>
>  #include <string.h>
> @@ -702,6 +703,443 @@ test_display_frame_dump(data_t *data, struct
> chamelium_port *port)
>  	drmModeFreeConnector(connector);
>  }
>  
> +static void randomize_plane_format_stride(igt_plane_t *plane, uint32_t
> width,
> +					  uint32_t *format, size_t *stride,
> +					  bool *tiled, bool allow_yuv)
> +{
> +	size_t stride_min;
> +	uint32_t *formats_array;
> +	unsigned int formats_count;
> +	unsigned int count = 0;
> +	unsigned int i;
> +	int index;
> +
> +	igt_format_array_fill(&formats_array, &formats_count, allow_yuv);
> +
> +	/* First pass to count the supported formats. */
> +	for (i = 0; i < formats_count; i++)
> +		if (igt_plane_has_format_mod(plane, formats_array[i],
> +					     DRM_FORMAT_MOD_LINEAR))
> +			count++;
> +
> +	igt_assert(count > 0);
> +
> +	index = rand() % count;
> +
> +	/* Second pass to get the index-th supported format. */
> +	for (i = 0; i < formats_count; i++) {
> +		if (!igt_plane_has_format_mod(plane, formats_array[i],
> +					      DRM_FORMAT_MOD_LINEAR))
> +			continue;
> +
> +		if (!index--) {
> +			*format = formats_array[i];
> +			break;
> +		}
> +	}
> +
> +	free(formats_array);
> +
> +	igt_assert(index < 0);
> +
> +	stride_min = width * igt_format_plane_bpp(*format, 0) / 8;
> +
> +	/* Randomize the stride with at most twice the minimum. */
> +	*stride = (rand() % stride_min) + stride_min;
Maybe need to change the comment or fix this line? Given

rand() = 9999999
stride_min = 10

This would come out to 19, e.g. less then twice the maximum. Maybe you meant

*stride = (rand() % (stride_min + 1)) + stride_min;

> +
> +	/* Pixman requires the stride to be aligned to 32-byte words. */
> +	*stride = ALIGN(*stride, sizeof(uint32_t));
> +
> +	/* Randomize the use of a tiled mode with a 1/4 probability. */
> +	*tiled = ((rand() % 4) == 0);
> +}
> +
> +static void randomize_plane_dimensions(drmModeModeInfo *mode,
> +				       uint32_t *width, uint32_t *height,
> +				       uint32_t *src_w, uint32_t *src_h,
> +				       uint32_t *src_x, uint32_t *src_y,
> +				       uint32_t *crtc_w, uint32_t *crtc_h,
> +				       int32_t *crtc_x, int32_t *crtc_y,
> +				       bool allow_scaling)
> +{
> +	double ratio;
> +
> +	/* Randomize width and height in the mode dimensions range. */
> +	*width = (rand() % mode->hdisplay) + 1;
> +	*height = (rand() % mode->vdisplay) + 1;
> +
> +	/*
> +	 * Randomize source offset, but keep at least half of the
> +	 * original size.
> +	 */
> +	*src_x = rand() % (*width / 2);
> +	*src_y = rand() % (*height / 2);
Given a width of 1920, this would make

*src_x >= 0 && *src_x < 960 (e.g. src_x is between 0 and 959)

So I think you're missing a +1 here

> +
> +	/* The source size only includes the active source area. */
> +	*src_w = *width - *src_x;
> +	*src_h = *height - *src_y;
> +
> +	if (allow_scaling) {
> +		*crtc_w = (rand() % mode->hdisplay) + 1;
> +		*crtc_h = (rand() % mode->vdisplay) + 1;
> +
> +		/*
> +		 * Don't bother with scaling if dimensions are quite close in
> +		 * order to get non-scaling cases more frequently. Also limit
> +		 * scaling to 3x to avoid agressive filtering that makes
> +		 * comparison less reliable.
> +		 */
> +
> +		ratio = ((double) *crtc_w / *src_w);
> +		if (ratio > 0.8 && ratio < 1.2)
> +			*crtc_w = *src_w;
> +		else if (ratio > 3.0)
> +			*crtc_w = *src_w * 3;
> +
> +		ratio = ((double) *crtc_h / *src_h);
> +		if (ratio > 0.8 && ratio < 1.2)
> +			*crtc_h = *src_h;
> +		else if (ratio > 3.0)
> +			*crtc_h = *src_h * 3;
> +	} else {
> +		*crtc_w = *src_w;
> +		*crtc_h = *src_h;
> +	}
> +
> +	if (*crtc_w != *src_w || *crtc_h != *src_h) {
> +		/*
> +		 * When scaling is involved, make sure to not go off-bounds or
> +		 * scaled clipping may result in decimal dimensions, that most
> +		 * drivers don't support.
> +		 */
> +		*crtc_x = rand() % (mode->hdisplay - *crtc_w);
> +		*crtc_y = rand() % (mode->vdisplay - *crtc_h);
> +	} else {
> +		/*
> +		 * Randomize the on-crtc position and allow the plane to go
> +		 * off-display by up to half of its on-crtc width and height.
> +		 */
> +		*crtc_x = (rand() % mode->hdisplay) - *crtc_w / 2;
> +		*crtc_y = (rand() % mode->vdisplay) - *crtc_h / 2;
I think you're missing +1s here as well (should be rand() % mode->hdisplay+1,
etc.)
> +	}
> +}
> +
> +static void blit_plane_cairo(data_t *data, cairo_surface_t *result,
> +			     uint32_t src_w, uint32_t src_h,
> +			     uint32_t src_x, uint32_t src_y,
> +			     uint32_t crtc_w, uint32_t crtc_h,
> +			     int32_t crtc_x, int32_t crtc_y,
> +			     struct igt_fb *fb)
> +{
> +	cairo_surface_t *surface;
> +	cairo_surface_t *clipped_surface;
> +	cairo_t *cr;
> +
> +	surface = igt_get_cairo_surface(data->drm_fd, fb);
> +
> +	if (src_x || src_y) {
> +		clipped_surface =
> cairo_image_surface_create(CAIRO_FORMAT_RGB24,
> +							     src_w, src_h);
> +
> +		cr = cairo_create(clipped_surface);
> +
> +		cairo_translate(cr, -1. * src_x, -1. * src_y);
> +
> +		cairo_set_source_surface(cr, surface, 0, 0);
> +
> +		cairo_paint(cr);
> +		cairo_surface_flush(clipped_surface);
> +
> +		cairo_destroy(cr);
> +	} else {
> +		clipped_surface = surface;
> +	}
> +
> +	cr = cairo_create(result);
> +
> +	cairo_translate(cr, crtc_x, crtc_y);
> +
> +	if (src_w != crtc_w || src_h != crtc_h) {
> +		cairo_scale(cr, (double) crtc_w / src_w,
> +			    (double) crtc_h / src_h);
> +	}
> +
> +	cairo_set_source_surface(cr, clipped_surface, 0, 0);
> +	cairo_surface_destroy(clipped_surface);
> +
> +	if (src_w != crtc_w || src_h != crtc_h) {
> +		cairo_pattern_set_filter(cairo_get_source(cr),
> +					 CAIRO_FILTER_BILINEAR);
> +		cairo_pattern_set_extend(cairo_get_source(cr),
> +					 CAIRO_EXTEND_NONE);
> +	}
> +
> +	cairo_paint(cr);
> +	cairo_surface_flush(result);
> +
> +	cairo_destroy(cr);
> +}
> +
> +static void configure_plane(igt_plane_t *plane, uint32_t src_w, uint32_t
> src_h,
> +			    uint32_t src_x, uint32_t src_y, uint32_t crtc_w,
> +			    uint32_t crtc_h, int32_t crtc_x, int32_t crtc_y,
> +			    struct igt_fb *fb)
> +{
> +	igt_plane_set_fb(plane, fb);
> +
> +	igt_plane_set_position(plane, crtc_x, crtc_y);
> +	igt_plane_set_size(plane, crtc_w, crtc_h);
> +
> +	igt_fb_set_position(fb, plane, src_x, src_y);
> +	igt_fb_set_size(fb, plane, src_w, src_h);
> +}
> +
> +static void prepare_tiled_plane(data_t *data, igt_plane_t *plane,
> +				unsigned int index, uint32_t format,
> +				struct igt_fb *overlay_fb)
> +{
> +	struct igt_fb tiled_fb;
> +	unsigned int fb_id;
> +
> +	if (is_vc4_device(data->drm_fd) &&
> +	    igt_vc4_plane_supports_t_tiling(plane, format)) {
> +		igt_debug("Plane %d: using VC4 T-tiling\n", index);
> +
> +		fb_id = igt_vc4_fb_t_tiled_convert(&tiled_fb, overlay_fb);
> +	} else if (is_vc4_device(data->drm_fd) &&
> +		   igt_vc4_plane_supports_sand_tiling(plane, format, 256)) {
> +		/* Randomize the column height with at most twice the height.
> */
> +		size_t column_height = (rand() % overlay_fb->height) +
> +				       overlay_fb->height;
> +
> +		igt_debug("Plane %d: using VC4 SAND256 tiling with column
> height %ld\n",
> +			  index, column_height);
> +
> +		fb_id = vc4_fb_sand_tiled_convert(&tiled_fb, overlay_fb, 256,
> +						  column_height);
> +	} else {
> +		return;
> +	}

Bikeshed: Why not remove the two is_vc4_device(data->drm_fd) checks and just
put this at the top of prepare_tiled_plane():

if (!is_vc4_device(data->drm_fd))
	return;

Would make these conditionals a bit easier to read
> +
> +	igt_assert(fb_id > 0);
> +
> +	/* Make the tiled framebuffer the overlay. */
> +	igt_remove_fb(data->drm_fd, overlay_fb);
> +	*overlay_fb = tiled_fb;
> +}
> +
> +static void prepare_randomized_plane(data_t *data,
> +				     drmModeModeInfo *mode,
> +				     igt_plane_t *plane,
> +				     struct igt_fb *overlay_fb,
> +				     unsigned int index,
> +				     cairo_surface_t *result_surface,
> +				     bool allow_scaling, bool allow_yuv)
> +{
> +	struct igt_fb pattern_fb;
> +	uint32_t overlay_fb_w, overlay_fb_h;
> +	uint32_t overlay_src_w, overlay_src_h;
> +	uint32_t overlay_src_x, overlay_src_y;
> +	int32_t overlay_crtc_x, overlay_crtc_y;
> +	uint32_t overlay_crtc_w, overlay_crtc_h;
> +	uint32_t format;
> +	size_t stride;
> +	bool tiled;
> +	int fb_id;
> +
> +	randomize_plane_dimensions(mode, &overlay_fb_w, &overlay_fb_h,
> +				   &overlay_src_w, &overlay_src_h,
> +				   &overlay_src_x, &overlay_src_y,
> +				   &overlay_crtc_w, &overlay_crtc_h,
> +				   &overlay_crtc_x, &overlay_crtc_y,
> +				   allow_scaling);
> +
> +	igt_debug("Plane %d: framebuffer size %dx%d\n", index,
> +		  overlay_fb_w, overlay_fb_h);
> +	igt_debug("Plane %d: on-crtc size %dx%d\n", index,
> +		  overlay_crtc_w, overlay_crtc_h);
> +	igt_debug("Plane %d: on-crtc position %dx%d\n", index,
> +		  overlay_crtc_x, overlay_crtc_y);
> +	igt_debug("Plane %d: in-framebuffer size %dx%d\n", index,
> +		  overlay_src_w, overlay_src_h);
> +	igt_debug("Plane %d: in-framebuffer position %dx%d\n", index,
> +		  overlay_src_x, overlay_src_y);
> +
> +	/* Get a pattern framebuffer for the overlay plane. */
> +	fb_id = chamelium_get_pattern_fb(data, overlay_fb_w, overlay_fb_h,
> +					 DRM_FORMAT_XRGB8888, 32,
> &pattern_fb);
> +	igt_assert(fb_id > 0);
> +
> +	randomize_plane_format_stride(plane, overlay_fb_w, &format, &stride,
> +				      &tiled, allow_yuv);
> +
> +	igt_debug("Plane %d: %s format with stride %ld\n", index,
> +		  igt_format_str(format), stride);
> +
> +	fb_id = igt_fb_convert_with_stride(overlay_fb, &pattern_fb, format,
> +					   stride);
> +	igt_assert(fb_id > 0);
> +
> +	if (tiled)
> +		prepare_tiled_plane(data, plane, index, format, overlay_fb);
> +
> +	blit_plane_cairo(data, result_surface, overlay_src_w, overlay_src_h,
> +			 overlay_src_x, overlay_src_y,
> +			 overlay_crtc_w, overlay_crtc_h,
> +			 overlay_crtc_x, overlay_crtc_y, &pattern_fb);
> +
> +	configure_plane(plane, overlay_src_w, overlay_src_h,
> +			overlay_src_x, overlay_src_y,
> +			overlay_crtc_w, overlay_crtc_h,
> +			overlay_crtc_x, overlay_crtc_y, overlay_fb);
> +
> +	/* Remove the original pattern framebuffer. */
> +	igt_remove_fb(data->drm_fd, &pattern_fb);
> +}
> +
> +static void test_display_planes_random(data_t *data,
> +				       struct chamelium_port *port,
> +				       enum chamelium_check check)
> +{
> +	igt_output_t *output;
> +	drmModeModeInfo *mode;
> +	igt_plane_t *primary_plane;
> +	struct igt_fb primary_fb;
> +	struct igt_fb result_fb;
> +	struct igt_fb *overlay_fbs;
> +	igt_crc_t *crc;
> +	igt_crc_t *expected_crc;
> +	struct chamelium_fb_crc_async_data *fb_crc;
> +	unsigned int overlay_planes_max = 0;
> +	unsigned int overlay_planes_count;
> +	cairo_surface_t *result_surface;
> +	int captured_frame_count;
> +	bool allow_scaling;
> +	bool allow_yuv;
> +	unsigned int i;
> +	unsigned int fb_id;
> +
> +	switch (check) {
> +	case CHAMELIUM_CHECK_CRC:
> +		allow_scaling = false;
> +		allow_yuv = false;
> +		break;
> +	case CHAMELIUM_CHECK_CHECKERBOARD:
> +		allow_scaling = true;
> +		allow_yuv = true;
> +		break;
> +	default:
> +		igt_assert(false);
> +	}
> +
> +	srand(time(NULL));
> +
> +	reset_state(data, port);
> +
> +	/* Find the connector and pipe. */
> +	output = prepare_output(data, port);
> +
> +	mode = igt_output_get_mode(output);
> +
> +	/* Get a framebuffer for the primary plane. */
> +	primary_plane = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> +	igt_assert(primary_plane);
> +
> +	fb_id = chamelium_get_pattern_fb(data, mode->hdisplay, mode->vdisplay,
> +					 DRM_FORMAT_XRGB8888, 64,
> &primary_fb);
> +	igt_assert(fb_id > 0);
> +
> +	/* Get a framebuffer for the cairo composition result. */
> +	fb_id = igt_create_fb(data->drm_fd, mode->hdisplay,
> +			      mode->vdisplay, DRM_FORMAT_XRGB8888,
> +			      LOCAL_DRM_FORMAT_MOD_NONE, &result_fb);
> +	igt_assert(fb_id > 0);
> +
> +	result_surface = igt_get_cairo_surface(data->drm_fd, &result_fb);
> +
> +	/* Paint the primary framebuffer on the result surface. */
> +	blit_plane_cairo(data, result_surface, 0, 0, 0, 0, 0, 0, 0, 0,
> +			 &primary_fb);
> +
> +	/* Configure the primary plane. */
> +	igt_plane_set_fb(primary_plane, &primary_fb);
> +
> +	overlay_planes_max =
> +		igt_output_count_plane_type(output, DRM_PLANE_TYPE_OVERLAY);
> +
> +	/* Limit the number of planes to a reasonable scene. */
> +	if (overlay_planes_max > 4)
> +		overlay_planes_max = 4;

Why not just use max() here?

> +
> +	overlay_planes_count = (rand() % overlay_planes_max) + 1;
> +	igt_debug("Using %d overlay planes\n", overlay_planes_count);
> +
> +	overlay_fbs = calloc(sizeof(struct igt_fb), overlay_planes_count);
igt_assert(overlay_fbs);

Other then that, looks really good!
> +
> +	for (i = 0; i < overlay_planes_count; i++) {
> +		struct igt_fb *overlay_fb = &overlay_fbs[i];
> +		igt_plane_t *plane =
> +			igt_output_get_plane_type_index(output,
> +							DRM_PLANE_TYPE_OVERLAY
> ,
> +							i);
> +		igt_assert(plane);
> +
> +		prepare_randomized_plane(data, mode, plane, overlay_fb, i,
> +					 result_surface, allow_scaling,
> +					 allow_yuv);
> +	}
> +
> +	cairo_surface_destroy(result_surface);
> +
> +	if (check == CHAMELIUM_CHECK_CRC)
> +		fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
> +								&result_fb);
> +
> +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +
> +	if (check == CHAMELIUM_CHECK_CRC) {
> +		chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
> +		crc = chamelium_read_captured_crcs(data->chamelium,
> +						   &captured_frame_count);
> +
> +		igt_assert(captured_frame_count == 1);
> +
> +		expected_crc =
> chamelium_calculate_fb_crc_async_finish(fb_crc);
> +
> +		chamelium_assert_crc_eq_or_dump(data->chamelium,
> +						expected_crc, crc,
> +						&result_fb, 0);
> +
> +		free(expected_crc);
> +		free(crc);
> +	} else if (check == CHAMELIUM_CHECK_CHECKERBOARD) {
> +		struct chamelium_frame_dump *dump;
> +
> +		dump = chamelium_port_dump_pixels(data->chamelium, port, 0, 0,
> +						  0, 0);
> +		chamelium_assert_frame_match_or_dump(data->chamelium, port,
> +						     dump, &result_fb, check);
> +		chamelium_destroy_frame_dump(dump);
> +	}
> +
> +	for (i = 0; i < overlay_planes_count; i++) {
> +		struct igt_fb *overlay_fb = &overlay_fbs[i];
> +		igt_plane_t *plane;
> +
> +		plane = igt_output_get_plane_type_index(output,
> +							DRM_PLANE_TYPE_OVERLAY
> ,
> +							i);
> +		igt_assert(plane);
> +
> +		igt_remove_fb(data->drm_fd, overlay_fb);
> +	}
> +
> +	free(overlay_fbs);
> +
> +	igt_remove_fb(data->drm_fd, &primary_fb);
> +	igt_remove_fb(data->drm_fd, &result_fb);
> +}
> +
>  static void
>  test_hpd_without_ddc(data_t *data, struct chamelium_port *port)
>  {
> @@ -976,6 +1414,10 @@ igt_main
>  			test_display_one_mode(&data, port,
> DRM_FORMAT_XRGB1555,
>  					      CHAMELIUM_CHECK_CRC, 1);
>  
> +		connector_subtest("hdmi-crc-planes-random", HDMIA)
> +			test_display_planes_random(&data, port,
> +						   CHAMELIUM_CHECK_CRC);
> +
>  		connector_subtest("hdmi-cmp-nv12", HDMIA)
>  			test_display_one_mode(&data, port, DRM_FORMAT_NV12,
>  					      CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
> @@ -1008,6 +1450,10 @@ igt_main
>  			test_display_one_mode(&data, port, DRM_FORMAT_YVU422,
>  					      CHAMELIUM_CHECK_CHECKERBOARD,
> 1);
>  
> +		connector_subtest("hdmi-cmp-planes-random", HDMIA)
> +			test_display_planes_random(&data, port,
> +						   CHAMELIUM_CHECK_CHECKERBOAR
> D);
> +
>  		connector_subtest("hdmi-frame-dump", HDMIA)
>  			test_display_frame_dump(&data, port);
>  	}
-- 
Cheers,
	Lyude Paul

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

Re: [igt-dev] [PATCH i-g-t v3 21/21] chamelium: Add a display test for randomized planes

[Paul Kocialkowski]

Hi,

On Tue, 2019-01-15 at 17:56 -0500, Lyude Paul wrote:
> On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> > This introduces a new test for the Chamelium, that sets up planes
> > with randomized properties such as the format, dimensions, position,
> > in-framebuffer offsets and stride. The Chamelium capture is checked
> > against the reference generated by cairo with either a CRC or the
> > checkerboard-specific comparison method.
> > 
> > This test also includes testing of the VC4-specific T-tiled and
> > SAND-tiled modes, in all formats supported by the hardware.
> > 
> > Since this test does not share much with previous Chamelium display
> > tests (especially regarding KMS configuration), most of the code is
> > not shared with other tests.
> > 
> > This test can be derived with reproducible properties for regression
> > testing in the future. For now, it serves as a fuzzing test
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  tests/kms_chamelium.c | 446 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 446 insertions(+)
> > 
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 2779d74f55b6..5963964878ba 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -26,6 +26,7 @@
> >  
> >  #include "config.h"
> >  #include "igt.h"
> > +#include "igt_vc4.h"
> >  
> >  #include <fcntl.h>
> >  #include <string.h>
> > @@ -702,6 +703,443 @@ test_display_frame_dump(data_t *data, struct
> > chamelium_port *port)
> >  	drmModeFreeConnector(connector);
> >  }
> >  
> > +static void randomize_plane_format_stride(igt_plane_t *plane, uint32_t
> > width,
> > +					  uint32_t *format, size_t *stride,
> > +					  bool *tiled, bool allow_yuv)
> > +{
> > +	size_t stride_min;
> > +	uint32_t *formats_array;
> > +	unsigned int formats_count;
> > +	unsigned int count = 0;
> > +	unsigned int i;
> > +	int index;
> > +
> > +	igt_format_array_fill(&formats_array, &formats_count, allow_yuv);
> > +
> > +	/* First pass to count the supported formats. */
> > +	for (i = 0; i < formats_count; i++)
> > +		if (igt_plane_has_format_mod(plane, formats_array[i],
> > +					     DRM_FORMAT_MOD_LINEAR))
> > +			count++;
> > +
> > +	igt_assert(count > 0);
> > +
> > +	index = rand() % count;
> > +
> > +	/* Second pass to get the index-th supported format. */
> > +	for (i = 0; i < formats_count; i++) {
> > +		if (!igt_plane_has_format_mod(plane, formats_array[i],
> > +					      DRM_FORMAT_MOD_LINEAR))
> > +			continue;
> > +
> > +		if (!index--) {
> > +			*format = formats_array[i];
> > +			break;
> > +		}
> > +	}
> > +
> > +	free(formats_array);
> > +
> > +	igt_assert(index < 0);
> > +
> > +	stride_min = width * igt_format_plane_bpp(*format, 0) / 8;
> > +
> > +	/* Randomize the stride with at most twice the minimum. */
> > +	*stride = (rand() % stride_min) + stride_min;
> Maybe need to change the comment or fix this line? Given
> 
> rand() = 9999999
> stride_min = 10
> 
> This would come out to 19, e.g. less then twice the maximum. Maybe you meant
> 
> *stride = (rand() % (stride_min + 1)) + stride_min;

Good catch, thanks! I think I'll just update the comments to be
consistent though, having the limit included or not does not cause
adverse effects in practice (in these cases only, I think I had already
taken care of the ones where it's significant) and I find it slightly
more readable with the limit excluded.

[...]

> > +static void prepare_tiled_plane(data_t *data, igt_plane_t *plane,
> > +				unsigned int index, uint32_t format,
> > +				struct igt_fb *overlay_fb)
> > +{
> > +	struct igt_fb tiled_fb;
> > +	unsigned int fb_id;
> > +
> > +	if (is_vc4_device(data->drm_fd) &&
> > +	    igt_vc4_plane_supports_t_tiling(plane, format)) {
> > +		igt_debug("Plane %d: using VC4 T-tiling\n", index);
> > +
> > +		fb_id = igt_vc4_fb_t_tiled_convert(&tiled_fb, overlay_fb);
> > +	} else if (is_vc4_device(data->drm_fd) &&
> > +		   igt_vc4_plane_supports_sand_tiling(plane, format, 256)) {
> > +		/* Randomize the column height with at most twice the height.
> > */
> > +		size_t column_height = (rand() % overlay_fb->height) +
> > +				       overlay_fb->height;
> > +
> > +		igt_debug("Plane %d: using VC4 SAND256 tiling with column
> > height %ld\n",
> > +			  index, column_height);
> > +
> > +		fb_id = vc4_fb_sand_tiled_convert(&tiled_fb, overlay_fb, 256,
> > +						  column_height);
> > +	} else {
> > +		return;
> > +	}
> 
> Bikeshed: Why not remove the two is_vc4_device(data->drm_fd) checks and just
> put this at the top of prepare_tiled_plane():
> 
> if (!is_vc4_device(data->drm_fd))
> 	return;
> 
> Would make these conditionals a bit easier to read

I have dropped that check altogether. After all, there is no need to
double-check for VC4 if the modifiers were reported by the driver.

> > +
> > +	igt_assert(fb_id > 0);
> > +
> > +	/* Make the tiled framebuffer the overlay. */
> > +	igt_remove_fb(data->drm_fd, overlay_fb);
> > +	*overlay_fb = tiled_fb;
> > +}
> > +
> > +static void prepare_randomized_plane(data_t *data,
> > +				     drmModeModeInfo *mode,
> > +				     igt_plane_t *plane,
> > +				     struct igt_fb *overlay_fb,
> > +				     unsigned int index,
> > +				     cairo_surface_t *result_surface,
> > +				     bool allow_scaling, bool allow_yuv)
> > +{
> > +	struct igt_fb pattern_fb;
> > +	uint32_t overlay_fb_w, overlay_fb_h;
> > +	uint32_t overlay_src_w, overlay_src_h;
> > +	uint32_t overlay_src_x, overlay_src_y;
> > +	int32_t overlay_crtc_x, overlay_crtc_y;
> > +	uint32_t overlay_crtc_w, overlay_crtc_h;
> > +	uint32_t format;
> > +	size_t stride;
> > +	bool tiled;
> > +	int fb_id;
> > +
> > +	randomize_plane_dimensions(mode, &overlay_fb_w, &overlay_fb_h,
> > +				   &overlay_src_w, &overlay_src_h,
> > +				   &overlay_src_x, &overlay_src_y,
> > +				   &overlay_crtc_w, &overlay_crtc_h,
> > +				   &overlay_crtc_x, &overlay_crtc_y,
> > +				   allow_scaling);
> > +
> > +	igt_debug("Plane %d: framebuffer size %dx%d\n", index,
> > +		  overlay_fb_w, overlay_fb_h);
> > +	igt_debug("Plane %d: on-crtc size %dx%d\n", index,
> > +		  overlay_crtc_w, overlay_crtc_h);
> > +	igt_debug("Plane %d: on-crtc position %dx%d\n", index,
> > +		  overlay_crtc_x, overlay_crtc_y);
> > +	igt_debug("Plane %d: in-framebuffer size %dx%d\n", index,
> > +		  overlay_src_w, overlay_src_h);
> > +	igt_debug("Plane %d: in-framebuffer position %dx%d\n", index,
> > +		  overlay_src_x, overlay_src_y);
> > +
> > +	/* Get a pattern framebuffer for the overlay plane. */
> > +	fb_id = chamelium_get_pattern_fb(data, overlay_fb_w, overlay_fb_h,
> > +					 DRM_FORMAT_XRGB8888, 32,
> > &pattern_fb);
> > +	igt_assert(fb_id > 0);
> > +
> > +	randomize_plane_format_stride(plane, overlay_fb_w, &format, &stride,
> > +				      &tiled, allow_yuv);
> > +
> > +	igt_debug("Plane %d: %s format with stride %ld\n", index,
> > +		  igt_format_str(format), stride);
> > +
> > +	fb_id = igt_fb_convert_with_stride(overlay_fb, &pattern_fb, format,
> > +					   stride);
> > +	igt_assert(fb_id > 0);
> > +
> > +	if (tiled)
> > +		prepare_tiled_plane(data, plane, index, format, overlay_fb);
> > +
> > +	blit_plane_cairo(data, result_surface, overlay_src_w, overlay_src_h,
> > +			 overlay_src_x, overlay_src_y,
> > +			 overlay_crtc_w, overlay_crtc_h,
> > +			 overlay_crtc_x, overlay_crtc_y, &pattern_fb);
> > +
> > +	configure_plane(plane, overlay_src_w, overlay_src_h,
> > +			overlay_src_x, overlay_src_y,
> > +			overlay_crtc_w, overlay_crtc_h,
> > +			overlay_crtc_x, overlay_crtc_y, overlay_fb);
> > +
> > +	/* Remove the original pattern framebuffer. */
> > +	igt_remove_fb(data->drm_fd, &pattern_fb);
> > +}
> > +
> > +static void test_display_planes_random(data_t *data,
> > +				       struct chamelium_port *port,
> > +				       enum chamelium_check check)
> > +{
> > +	igt_output_t *output;
> > +	drmModeModeInfo *mode;
> > +	igt_plane_t *primary_plane;
> > +	struct igt_fb primary_fb;
> > +	struct igt_fb result_fb;
> > +	struct igt_fb *overlay_fbs;
> > +	igt_crc_t *crc;
> > +	igt_crc_t *expected_crc;
> > +	struct chamelium_fb_crc_async_data *fb_crc;
> > +	unsigned int overlay_planes_max = 0;
> > +	unsigned int overlay_planes_count;
> > +	cairo_surface_t *result_surface;
> > +	int captured_frame_count;
> > +	bool allow_scaling;
> > +	bool allow_yuv;
> > +	unsigned int i;
> > +	unsigned int fb_id;
> > +
> > +	switch (check) {
> > +	case CHAMELIUM_CHECK_CRC:
> > +		allow_scaling = false;
> > +		allow_yuv = false;
> > +		break;
> > +	case CHAMELIUM_CHECK_CHECKERBOARD:
> > +		allow_scaling = true;
> > +		allow_yuv = true;
> > +		break;
> > +	default:
> > +		igt_assert(false);
> > +	}
> > +
> > +	srand(time(NULL));
> > +
> > +	reset_state(data, port);
> > +
> > +	/* Find the connector and pipe. */
> > +	output = prepare_output(data, port);
> > +
> > +	mode = igt_output_get_mode(output);
> > +
> > +	/* Get a framebuffer for the primary plane. */
> > +	primary_plane = igt_output_get_plane_type(output,
> > DRM_PLANE_TYPE_PRIMARY);
> > +	igt_assert(primary_plane);
> > +
> > +	fb_id = chamelium_get_pattern_fb(data, mode->hdisplay, mode->vdisplay,
> > +					 DRM_FORMAT_XRGB8888, 64,
> > &primary_fb);
> > +	igt_assert(fb_id > 0);
> > +
> > +	/* Get a framebuffer for the cairo composition result. */
> > +	fb_id = igt_create_fb(data->drm_fd, mode->hdisplay,
> > +			      mode->vdisplay, DRM_FORMAT_XRGB8888,
> > +			      LOCAL_DRM_FORMAT_MOD_NONE, &result_fb);
> > +	igt_assert(fb_id > 0);
> > +
> > +	result_surface = igt_get_cairo_surface(data->drm_fd, &result_fb);
> > +
> > +	/* Paint the primary framebuffer on the result surface. */
> > +	blit_plane_cairo(data, result_surface, 0, 0, 0, 0, 0, 0, 0, 0,
> > +			 &primary_fb);
> > +
> > +	/* Configure the primary plane. */
> > +	igt_plane_set_fb(primary_plane, &primary_fb);
> > +
> > +	overlay_planes_max =
> > +		igt_output_count_plane_type(output, DRM_PLANE_TYPE_OVERLAY);
> > +
> > +	/* Limit the number of planes to a reasonable scene. */
> > +	if (overlay_planes_max > 4)
> > +		overlay_planes_max = 4;
> 
> Why not just use max() here?

Sure, I'll do that!

> > +
> > +	overlay_planes_count = (rand() % overlay_planes_max) + 1;
> > +	igt_debug("Using %d overlay planes\n", overlay_planes_count);
> > +
> > +	overlay_fbs = calloc(sizeof(struct igt_fb), overlay_planes_count);
> igt_assert(overlay_fbs);
> 
> Other then that, looks really good!

Great, thanks a lot for the review!

Cheers,

Paul

> > +
> > +	for (i = 0; i < overlay_planes_count; i++) {
> > +		struct igt_fb *overlay_fb = &overlay_fbs[i];
> > +		igt_plane_t *plane =
> > +			igt_output_get_plane_type_index(output,
> > +							DRM_PLANE_TYPE_OVERLAY
> > ,
> > +							i);
> > +		igt_assert(plane);
> > +
> > +		prepare_randomized_plane(data, mode, plane, overlay_fb, i,
> > +					 result_surface, allow_scaling,
> > +					 allow_yuv);
> > +	}
> > +
> > +	cairo_surface_destroy(result_surface);
> > +
> > +	if (check == CHAMELIUM_CHECK_CRC)
> > +		fb_crc = chamelium_calculate_fb_crc_async_start(data->drm_fd,
> > +								&result_fb);
> > +
> > +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > +
> > +	if (check == CHAMELIUM_CHECK_CRC) {
> > +		chamelium_capture(data->chamelium, port, 0, 0, 0, 0, 1);
> > +		crc = chamelium_read_captured_crcs(data->chamelium,
> > +						   &captured_frame_count);
> > +
> > +		igt_assert(captured_frame_count == 1);
> > +
> > +		expected_crc =
> > chamelium_calculate_fb_crc_async_finish(fb_crc);
> > +
> > +		chamelium_assert_crc_eq_or_dump(data->chamelium,
> > +						expected_crc, crc,
> > +						&result_fb, 0);
> > +
> > +		free(expected_crc);
> > +		free(crc);
> > +	} else if (check == CHAMELIUM_CHECK_CHECKERBOARD) {
> > +		struct chamelium_frame_dump *dump;
> > +
> > +		dump = chamelium_port_dump_pixels(data->chamelium, port, 0, 0,
> > +						  0, 0);
> > +		chamelium_assert_frame_match_or_dump(data->chamelium, port,
> > +						     dump, &result_fb, check);
> > +		chamelium_destroy_frame_dump(dump);
> > +	}
> > +
> > +	for (i = 0; i < overlay_planes_count; i++) {
> > +		struct igt_fb *overlay_fb = &overlay_fbs[i];
> > +		igt_plane_t *plane;
> > +
> > +		plane = igt_output_get_plane_type_index(output,
> > +							DRM_PLANE_TYPE_OVERLAY
> > ,
> > +							i);
> > +		igt_assert(plane);
> > +
> > +		igt_remove_fb(data->drm_fd, overlay_fb);
> > +	}
> > +
> > +	free(overlay_fbs);
> > +
> > +	igt_remove_fb(data->drm_fd, &primary_fb);
> > +	igt_remove_fb(data->drm_fd, &result_fb);
> > +}
> > +
> >  static void
> >  test_hpd_without_ddc(data_t *data, struct chamelium_port *port)
> >  {
> > @@ -976,6 +1414,10 @@ igt_main
> >  			test_display_one_mode(&data, port,
> > DRM_FORMAT_XRGB1555,
> >  					      CHAMELIUM_CHECK_CRC, 1);
> >  
> > +		connector_subtest("hdmi-crc-planes-random", HDMIA)
> > +			test_display_planes_random(&data, port,
> > +						   CHAMELIUM_CHECK_CRC);
> > +
> >  		connector_subtest("hdmi-cmp-nv12", HDMIA)
> >  			test_display_one_mode(&data, port, DRM_FORMAT_NV12,
> >  					      CHAMELIUM_CHECK_CHECKERBOARD,
> > 1);
> > @@ -1008,6 +1450,10 @@ igt_main
> >  			test_display_one_mode(&data, port, DRM_FORMAT_YVU422,
> >  					      CHAMELIUM_CHECK_CHECKERBOARD,
> > 1);
> >  
> > +		connector_subtest("hdmi-cmp-planes-random", HDMIA)
> > +			test_display_planes_random(&data, port,
> > +						   CHAMELIUM_CHECK_CHECKERBOAR
> > D);
> > +
> >  		connector_subtest("hdmi-frame-dump", HDMIA)
> >  			test_display_frame_dump(&data, port);
> >  	}
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[igt-dev] ✗ Fi.CI.BAT: failure for Chamelium VC4 plane fuzzy testing, with SAND and T-tiled mode

[Patchwork]

== Series Details ==

Series: Chamelium VC4 plane fuzzy testing, with SAND and T-tiled mode
URL   : https://patchwork.freedesktop.org/series/55044/
State : failure

== Summary ==

Applying: lib: drmtest: Add helpers to check and require the VC4 driver
Applying: lib/igt_fb: Add checks on i915 for i915-specific tiled buffer allocation
Applying: lib/igt_fb: Add support for allocating T-tiled VC4 buffers
Patch failed at 0003 lib/igt_fb: Add support for allocating T-tiled VC4 buffers
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

Re: [igt-dev] [PATCH i-g-t v3 00/21] Chamelium VC4 plane fuzzy testing, with SAND and T-tiled mode

[Lyude Paul]

Got up to patch #8, but I haven't finished reviewing yet. Will try to get that
done first thing tommorrow

For now, 1-7 lgtm

On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> This series introduces the required plumbing for allocating buffers in
> the VC4 T-tiled and SAND_tiled modes, along with helpers to convert to
> these tiling modes.
> 
> A new "checkerboard" comparison method is introduced to check whether
> captured frames match their references in cases where pixel-perfect
> results are not possible (e.g. with scaling) and the analog test does
> not provide reliable results.
> 
> With that in place, new tests for the Chamelium are introduced to
> perform testing of planes with randomized properties, both with a
> CRC and frame comparison fashion.
> 
> This series is based on version 3 of Maxime Ripard's series:
>   igt: chamelium: Test YUV buffers using the Chamelium
> 
> Changes since v2:
> * Added support for SAND tiling;
> * Generalized T-tiling to support both 16 and 32 bpp;
> * Split the main test into more sub-functions;
> * Removed explicit lists of DRM formats;
> * Added helper to list known DRM formats;
> * Various functional fixes in the main test;
> * Removed unnecessary pointer init in map_bo;
> * Rebased on v3 of Maxime Ripard's YUV series;
> * Added collected Reviewed-By tags;
> 
> Changes since v1:
> * Added collected Reviewed-by tags;
> * Removed explicit size and constified tiling read order tables;
> * Removed unnecessary MAP_FAILED addition in map_bo;
> * Added pipe and output helpers to count and iterate planes of a given type;
> * Moved CRC debug prints to the chamelium library;
> * Fixed destination stride description for igt_fb_convert_with_stride;
> * Split planes test into a few sub-routines;
> * Removed explicit underrun detection since general error reporting is
> enough;
> * Removed underrun detection bits due to ongoing Chamelium-specific issues;
> * Deconfigured planes before removing their framebuffer, avoiding kmsg
> errors.
> 
> Paul Kocialkowski (21):
>   lib: drmtest: Add helpers to check and require the VC4 driver
>   lib/igt_fb: Add checks on i915 for i915-specific tiled buffer
>     allocation
>   lib/igt_fb: Add support for allocating T-tiled VC4 buffers
>   lib/igt_fb: Add support for VC4 SAND tiling modes
>   lib/igt_fb: Allow interpreting the tile height as a stride equivalent
>   lib/igt_fb: Add a stride-provisioned fashion of igt_fb_convert
>   lib/igt_fb: Add a helper to retreive the plane bpp for a given format
>   lib/igt_fb: Add a helper to fill-in the available DRM formats
>   lib/igt_vc4: Add helpers for converting linear to T-tiled RGB buffers
>   lib/igt_vc4: Add helper for checking T-tiling support on plane
>   lib/igt_vc4: Add helpers for converting linear to SAND-tiled buffers
>   lib/igt_vc4: Add helper for checking SAND tiling support on plane
>   lib/igt_kms: Add helpers to count and iterate planes from pipe
>   lib/igt_kms: Add helpers to count and iterate planes from output
>   lib/igt_chamelium: Fixup resources liberation in comparison helpers
>   lib/igt_chamelium: Split frames dump logic and rework surroundings
>   lib/igt_chamelium: Generalize the frame match helper with check type
>   lib/igt_frame: Add a checkerboard frame comparison method
>   lib/igt_chamelium: Hook-in checkerboard comparison method in helpers
>   chamelium: Move the YUV tests over to the checkerboard checking method
>   chamelium: Add a display test for randomized planes
> 
>  lib/drmtest.c         |  10 +
>  lib/drmtest.h         |   2 +
>  lib/igt_chamelium.c   | 124 +++++++----
>  lib/igt_chamelium.h   |  15 +-
>  lib/igt_fb.c          | 144 ++++++++++++-
>  lib/igt_fb.h          |   6 +
>  lib/igt_frame.c       | 121 +++++++++++
>  lib/igt_frame.h       |   2 +
>  lib/igt_kms.c         |  84 ++++++++
>  lib/igt_kms.h         |   6 +
>  lib/igt_vc4.c         | 242 +++++++++++++++++++++
>  lib/igt_vc4.h         |  42 ++++
>  tests/kms_chamelium.c | 481 ++++++++++++++++++++++++++++++++++++++++--
>  13 files changed, 1204 insertions(+), 75 deletions(-)
> 
-- 
Cheers,
	Lyude Paul

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

Re: [igt-dev] [PATCH i-g-t v3 00/21] Chamelium VC4 plane fuzzy testing, with SAND and T-tiled mode

[Paul Kocialkowski]

Hi,

On Mon, 2019-01-14 at 19:46 -0500, Lyude Paul wrote:
> Got up to patch #8, but I haven't finished reviewing yet. Will try to get that
> done first thing tommorrow
> 
> For now, 1-7 lgtm

Thanks a lot for the review, it's greatly appreciated :)

Cheers,

Paul

> On Fri, 2019-01-11 at 10:05 +0100, Paul Kocialkowski wrote:
> > This series introduces the required plumbing for allocating buffers in
> > the VC4 T-tiled and SAND_tiled modes, along with helpers to convert to
> > these tiling modes.
> > 
> > A new "checkerboard" comparison method is introduced to check whether
> > captured frames match their references in cases where pixel-perfect
> > results are not possible (e.g. with scaling) and the analog test does
> > not provide reliable results.
> > 
> > With that in place, new tests for the Chamelium are introduced to
> > perform testing of planes with randomized properties, both with a
> > CRC and frame comparison fashion.
> > 
> > This series is based on version 3 of Maxime Ripard's series:
> >   igt: chamelium: Test YUV buffers using the Chamelium
> > 
> > Changes since v2:
> > * Added support for SAND tiling;
> > * Generalized T-tiling to support both 16 and 32 bpp;
> > * Split the main test into more sub-functions;
> > * Removed explicit lists of DRM formats;
> > * Added helper to list known DRM formats;
> > * Various functional fixes in the main test;
> > * Removed unnecessary pointer init in map_bo;
> > * Rebased on v3 of Maxime Ripard's YUV series;
> > * Added collected Reviewed-By tags;
> > 
> > Changes since v1:
> > * Added collected Reviewed-by tags;
> > * Removed explicit size and constified tiling read order tables;
> > * Removed unnecessary MAP_FAILED addition in map_bo;
> > * Added pipe and output helpers to count and iterate planes of a given type;
> > * Moved CRC debug prints to the chamelium library;
> > * Fixed destination stride description for igt_fb_convert_with_stride;
> > * Split planes test into a few sub-routines;
> > * Removed explicit underrun detection since general error reporting is
> > enough;
> > * Removed underrun detection bits due to ongoing Chamelium-specific issues;
> > * Deconfigured planes before removing their framebuffer, avoiding kmsg
> > errors.
> > 
> > Paul Kocialkowski (21):
> >   lib: drmtest: Add helpers to check and require the VC4 driver
> >   lib/igt_fb: Add checks on i915 for i915-specific tiled buffer
> >     allocation
> >   lib/igt_fb: Add support for allocating T-tiled VC4 buffers
> >   lib/igt_fb: Add support for VC4 SAND tiling modes
> >   lib/igt_fb: Allow interpreting the tile height as a stride equivalent
> >   lib/igt_fb: Add a stride-provisioned fashion of igt_fb_convert
> >   lib/igt_fb: Add a helper to retreive the plane bpp for a given format
> >   lib/igt_fb: Add a helper to fill-in the available DRM formats
> >   lib/igt_vc4: Add helpers for converting linear to T-tiled RGB buffers
> >   lib/igt_vc4: Add helper for checking T-tiling support on plane
> >   lib/igt_vc4: Add helpers for converting linear to SAND-tiled buffers
> >   lib/igt_vc4: Add helper for checking SAND tiling support on plane
> >   lib/igt_kms: Add helpers to count and iterate planes from pipe
> >   lib/igt_kms: Add helpers to count and iterate planes from output
> >   lib/igt_chamelium: Fixup resources liberation in comparison helpers
> >   lib/igt_chamelium: Split frames dump logic and rework surroundings
> >   lib/igt_chamelium: Generalize the frame match helper with check type
> >   lib/igt_frame: Add a checkerboard frame comparison method
> >   lib/igt_chamelium: Hook-in checkerboard comparison method in helpers
> >   chamelium: Move the YUV tests over to the checkerboard checking method
> >   chamelium: Add a display test for randomized planes
> > 
> >  lib/drmtest.c         |  10 +
> >  lib/drmtest.h         |   2 +
> >  lib/igt_chamelium.c   | 124 +++++++----
> >  lib/igt_chamelium.h   |  15 +-
> >  lib/igt_fb.c          | 144 ++++++++++++-
> >  lib/igt_fb.h          |   6 +
> >  lib/igt_frame.c       | 121 +++++++++++
> >  lib/igt_frame.h       |   2 +
> >  lib/igt_kms.c         |  84 ++++++++
> >  lib/igt_kms.h         |   6 +
> >  lib/igt_vc4.c         | 242 +++++++++++++++++++++
> >  lib/igt_vc4.h         |  42 ++++
> >  tests/kms_chamelium.c | 481 ++++++++++++++++++++++++++++++++++++++++--
> >  13 files changed, 1204 insertions(+), 75 deletions(-)
> > 
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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