[igt-dev] [PATCH i-g-t 10/10] tests/kms_ccs: Add option to check the CCS planes

Imre Deak imre.deak at intel.com
Mon Dec 30 13:12:20 UTC 2019


On Mon, Dec 30, 2019 at 02:47:04PM +0200, Juha-Pekka Heikkila wrote:
> Small nag on this patch number 10 but generally patches 6..10 are
> 
> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> 
> 
> On 30.12.2019 5.40, Imre Deak wrote:
> > Add an option to check whether the framebuffer content was really
> > compressed.
> > 
> > Cc: Mika Kahola <mika.kahola at intel.com>
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> >   lib/igt_fb.c    | 15 ++++++++++++
> >   lib/igt_fb.h    |  4 ++++
> >   tests/kms_ccs.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >   3 files changed, 80 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index e6a3ff07..c81b9de8 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -502,6 +502,11 @@ static bool is_ccs_plane(const struct igt_fb *fb, int plane)
> >   	return plane >= fb->num_planes / 2;
> >   }
> > +bool igt_fb_is_ccs_plane(const struct igt_fb *fb, int plane)
> > +{
> > +	return is_ccs_plane(fb, plane);
> > +}
> > +
> >   static bool is_gen12_ccs_plane(const struct igt_fb *fb, int plane)
> >   {
> >   	return is_gen12_ccs_modifier(fb->modifier) && is_ccs_plane(fb, plane);
> > @@ -513,6 +518,11 @@ static bool is_gen12_ccs_cc_plane(const struct igt_fb *fb, int plane)
> >   	       plane == 2;
> >   }
> > +bool igt_fb_is_gen12_ccs_cc_plane(const struct igt_fb *fb, int plane)
> > +{
> > +	return is_gen12_ccs_cc_plane(fb, plane);
> > +}
> > +
> >   static int ccs_to_main_plane(const struct igt_fb *fb, int plane)
> >   {
> >   	if (is_gen12_ccs_cc_plane(fb, plane))
> > @@ -521,6 +531,11 @@ static int ccs_to_main_plane(const struct igt_fb *fb, int plane)
> >   	return plane - fb->num_planes / 2;
> >   }
> > +int igt_fb_ccs_to_main_plane(const struct igt_fb *fb, int plane)
> > +{
> > +	return ccs_to_main_plane(fb, plane);
> > +}
> > +
> >   static unsigned fb_plane_width(const struct igt_fb *fb, int plane)
> >   {
> >   	const struct format_desc_struct *format = lookup_drm_format(fb->drm_format);
> > diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> > index 69132b41..5ed9e35a 100644
> > --- a/lib/igt_fb.h
> > +++ b/lib/igt_fb.h
> > @@ -170,6 +170,10 @@ void igt_fb_calc_crc(struct igt_fb *fb, igt_crc_t *crc);
> >   uint64_t igt_fb_mod_to_tiling(uint64_t modifier);
> >   uint64_t igt_fb_tiling_to_mod(uint64_t tiling);
> > +bool igt_fb_is_ccs_plane(const struct igt_fb *fb, int plane);
> > +bool igt_fb_is_gen12_ccs_cc_plane(const struct igt_fb *fb, int plane);
> > +int igt_fb_ccs_to_main_plane(const struct igt_fb *fb, int ccs_plane);
> > +
> >   /* cairo-based painting */
> >   cairo_surface_t *igt_get_cairo_surface(int fd, struct igt_fb *fb);
> >   cairo_surface_t *igt_cairo_image_surface_create_from_png(const char *filename);
> > diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
> > index 9e5bb559..34fb0138 100644
> > --- a/tests/kms_ccs.c
> > +++ b/tests/kms_ccs.c
> > @@ -89,6 +89,8 @@ static const uint64_t ccs_modifiers[] = {
> >   	LOCAL_I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS,
> >   };
> > +static bool check_ccs_planes;
> > +
> >   /*
> >    * Limit maximum used sprite plane width so this test will not mistakenly
> >    * fail on hardware limitations which are not interesting to this test.
> > @@ -115,6 +117,44 @@ static void addfb_init(struct igt_fb *fb, struct drm_mode_fb_cmd2 *f)
> >   	}
> >   }
> > +static void check_ccs_plane(int drm_fd, igt_fb_t *fb, int plane)
> > +{
> > +	void *map;
> > +	void *ccs_p;
> > +	size_t ccs_size;
> > +	int i;
> > +
> > +	ccs_size = fb->strides[plane] * fb->plane_height[plane];
> > +	igt_assert(ccs_size);
> > +
> > +	gem_set_domain(drm_fd, fb->gem_handle, I915_GEM_DOMAIN_CPU, 0);
> > +
> > +	map = gem_mmap__cpu(drm_fd, fb->gem_handle, 0, fb->size, PROT_READ);
> > +
> > +	ccs_size = fb->strides[plane] * fb->plane_height[plane];
> > +	ccs_p = map + fb->offsets[plane];
> > +	for (i = 0; i < ccs_size; i += sizeof(uint32_t))
> > +		if (*(uint32_t *)(ccs_p + i))
> > +			break;
> 
> This check for uint32_t zeros myself I'd write more clear what is
> happening..
> maybe even with comment why there's wish it's not all zeros since
> those fbs are coming from elsewhere. But that's just opinion.

How about the following comment in front of the function?:
"""
The CCS planes of compressed framebuffers have non-zero bytes if the
engine compressed effectively the framebuffer. The actual encoding of
these bytes is not specified, but we know that seeing an all-zero CCS
plane means that the engine left the FB uncompressed, which is not what
we expect in the test. Look for the first non-zero byte in the given CCS
plane to get a minimal assurance that compression took place.
"""

> 
> > +
> > +	munmap(map, fb->size);
> > +
> > +	igt_assert_f(i < ccs_size,
> > +		     "CCS plane %d (for main plane %d) lacks compression meta-data\n",
> > +		     plane, igt_fb_ccs_to_main_plane(fb, plane));
> > +}
> > +
> > +static void check_all_ccs_planes(int drm_fd, igt_fb_t *fb)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < fb->num_planes; i++) {
> > +		if (igt_fb_is_ccs_plane(fb, i) &&
> > +		    !igt_fb_is_gen12_ccs_cc_plane(fb, i))
> > +			check_ccs_plane(drm_fd, fb, i);
> > +	}
> > +}
> > +
> >   static void generate_fb(data_t *data, struct igt_fb *fb,
> >   			int width, int height,
> >   			enum test_fb_flags fb_flags)
> > @@ -198,6 +238,9 @@ static void generate_fb(data_t *data, struct igt_fb *fb,
> >   	} else
> >   		igt_assert_eq(ret, 0);
> > +	if (check_ccs_planes)
> > +		check_all_ccs_planes(data->drm_fd, fb);
> > +
> >   	fb->fb_id = f.fb_id;
> >   }
> > @@ -376,7 +419,24 @@ static void test_output(data_t *data)
> >   static data_t data;
> > -igt_main
> > +static int opt_handler(int opt, int opt_index, void *opt_data)
> > +{
> > +	switch (opt) {
> > +	case 'c':
> > +		check_ccs_planes = true;
> > +		break;
> > +	default:
> > +		return IGT_OPT_HANDLER_ERROR;
> > +	}
> > +
> > +	return IGT_OPT_HANDLER_SUCCESS;
> > +}
> > +
> > +static const char *help_str =
> > +"  -c\tCheck the presence of compression meta-data\n"
> > +;
> > +
> > +igt_main_args("c", NULL, help_str, opt_handler, NULL)
> >   {
> >   	enum pipe pipe;
> > 
> 


More information about the igt-dev mailing list