Re: [igt-dev] [PATCH v2 3/3] tests/gem_render_copy: Add compressed src to compressed dst subtests

[Imre Deak <imre.deak@intel.com>]

On Tue, Nov 12, 2019 at 12:15:50PM -0800, Brian Welty wrote:
> 
> On 11/5/2019 11:34 AM, Imre Deak wrote:
> > Add new subtests that blit from a compressed source to a compressed
> > destination buffer.
> > 
> > v2:
> > - Use the correct buffer when dumping the png for the compressed buf.
> > 
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Cc: Brian Welty <brian.welty@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> 
> 
> Perhaps add a:
> References: https://bugs.freedesktop.org/show_bug.cgi?id=111771
> and close that out?

Ok, added that.

> I had been looking at gem_render_copy as part of above and your changes
> look fine to me.
> So for patches 2 and 3 you can have my:
> Reviewed-by: Brian Welty <brian.welty@intel.com>
> 
> Up to you if you need additional reviewed-by or not for this.

Thanks. I added back a subtest I removed by mistake in patch 3. I'll add
your r-b to the latest version of patch 2 and 3.

> 
> -Brian
> 
> 
> >  tests/i915/gem_render_copy.c | 126 ++++++++++++++++++++++++++---------
> >  1 file changed, 93 insertions(+), 33 deletions(-)
> > 
> > diff --git a/tests/i915/gem_render_copy.c b/tests/i915/gem_render_copy.c
> > index fd139e91..383e86bc 100644
> > --- a/tests/i915/gem_render_copy.c
> > +++ b/tests/i915/gem_render_copy.c
> > @@ -572,9 +572,13 @@ static void scratch_buf_aux_check(data_t *data,
> >  		     "Aux surface indicates that nothing was compressed\n");
> >  }
> >  
> > -static void test(data_t *data, uint32_t tiling, uint64_t ccs_modifier)
> > +#define DST_COMPRESSED	1
> > +#define SRC_COMPRESSED	2
> > +
> > +static void test(data_t *data, uint32_t dst_tiling, uint32_t src_tiling,
> > +		 int flags)
> >  {
> > -	struct igt_buf dst, ccs, ref;
> > +	struct igt_buf dst, src_ccs, dst_ccs, ref;
> >  	struct {
> >  		struct igt_buf buf;
> >  		const char *filename;
> > @@ -602,22 +606,34 @@ static void test(data_t *data, uint32_t tiling, uint64_t ccs_modifier)
> >  			.x = 1, .y = 1,
> >  		},
> >  	};
> > -
> >  	int opt_dump_aub = igt_aub_dump_enabled();
> >  	int num_src = ARRAY_SIZE(src);
> > +	bool src_compressed = flags & SRC_COMPRESSED;
> > +	bool dst_compressed = flags & DST_COMPRESSED;
> > +
> > +	/*
> > +	 * The tiling for uncompressed source buffers is determined by the
> > +	 * tiling of the src[] buffers above.
> > +	 */
> > +	igt_assert(!src_tiling || src_compressed);
> >  
> >  	/* no Yf before gen9 */
> >  	if (intel_gen(data->devid) < 9)
> >  		num_src--;
> >  
> > -	if (tiling == I915_TILING_Yf || ccs_modifier)
> > +	if (dst_tiling == I915_TILING_Yf || src_tiling == I915_TILING_Yf ||
> > +	    src_compressed || dst_compressed)
> >  		igt_require(intel_gen(data->devid) >= 9);
> >  
> >  	for (int i = 0; i < num_src; i++)
> >  		scratch_buf_init(data, &src[i].buf, WIDTH, HEIGHT, src[i].tiling, false);
> > -	scratch_buf_init(data, &dst, WIDTH, HEIGHT, tiling, false);
> > -	if (ccs_modifier)
> > -		scratch_buf_init(data, &ccs, WIDTH, HEIGHT, ccs_modifier, true);
> > +	scratch_buf_init(data, &dst, WIDTH, HEIGHT, dst_tiling, false);
> > +	if (src_compressed)
> > +		scratch_buf_init(data, &src_ccs, WIDTH, HEIGHT,
> > +				 src_tiling, true);
> > +	if (dst_compressed)
> > +		scratch_buf_init(data, &dst_ccs, WIDTH, HEIGHT,
> > +				 dst_tiling, true);
> >  	scratch_buf_init(data, &ref, WIDTH, HEIGHT, I915_TILING_NONE, false);
> >  
> >  	for (int i = 0; i < num_src; i++)
> > @@ -657,26 +673,45 @@ static void test(data_t *data, uint32_t tiling, uint64_t ccs_modifier)
> >  	 *	 |dst|src|
> >  	 *	  -------
> >  	 */
> > -	if (ccs_modifier)
> > +	if (src_compressed)
> >  		data->render_copy(data->batch, NULL,
> >  				  &dst, 0, 0, WIDTH, HEIGHT,
> > -				  &ccs, 0, 0);
> > +				  &src_ccs, 0, 0);
> >  
> >  	for (int i = 0; i < num_src; i++)
> >  		data->render_copy(data->batch, NULL,
> > -				  &src[i].buf, WIDTH/4, HEIGHT/4, WIDTH/2-2, HEIGHT/2-2,
> > -				  ccs_modifier ? &ccs : &dst, src[i].x, src[i].y);
> > +				  &src[i].buf,
> > +				  WIDTH/4, HEIGHT/4, WIDTH/2-2, HEIGHT/2-2,
> > +				  src_compressed ? &src_ccs : &dst,
> > +				  src[i].x, src[i].y);
> > +
> > +	if (src_compressed || dst_compressed)
> > +		data->render_copy(data->batch, NULL,
> > +				  src_compressed ? &src_ccs : &dst,
> > +				  0, 0, WIDTH, HEIGHT,
> > +				  dst_compressed ? &dst_ccs : &dst,
> > +				  0, 0);
> >  
> > -	if (ccs_modifier)
> > +	if (dst_compressed)
> >  		data->render_copy(data->batch, NULL,
> > -				  &ccs, 0, 0, WIDTH, HEIGHT,
> > -				  &dst, 0, 0);
> > +				  &dst_ccs,
> > +				  0, 0, WIDTH, HEIGHT,
> > +				  &dst,
> > +				  0, 0);
> >  
> >  	if (opt_dump_png){
> >  		scratch_buf_write_to_png(data, &dst, "result.png");
> > -		if (ccs_modifier) {
> > -			scratch_buf_write_to_png(data, &ccs, "compressed.png");
> > -			scratch_buf_aux_write_to_png(data, &ccs, "compressed-aux.png");
> > +		if (src_compressed) {
> > +			scratch_buf_write_to_png(data, &src_ccs,
> > +						 "compressed-src.png");
> > +			scratch_buf_aux_write_to_png(data, &src_ccs,
> > +						     "compressed-src-aux.png");
> > +		}
> > +		if (dst_compressed) {
> > +			scratch_buf_write_to_png(data, &dst_ccs,
> > +						 "compressed-dst.png");
> > +			scratch_buf_aux_write_to_png(data, &dst_ccs,
> > +						     "compressed-dst-aux.png");
> >  		}
> >  	}
> >  
> > @@ -694,12 +729,16 @@ static void test(data_t *data, uint32_t tiling, uint64_t ccs_modifier)
> >  		scratch_buf_check(data, &dst, &ref, WIDTH - 10, HEIGHT - 10);
> >  	}
> >  
> > -	if (ccs_modifier)
> > -		scratch_buf_aux_check(data, &ccs);
> > +	if (src_compressed)
> > +		scratch_buf_aux_check(data, &src_ccs);
> > +	if (dst_compressed)
> > +		scratch_buf_aux_check(data, &dst_ccs);
> >  
> >  	scratch_buf_fini(&ref);
> > -	if (ccs_modifier)
> > -		scratch_buf_fini(&ccs);
> > +	if (dst_compressed)
> > +		scratch_buf_fini(&dst_ccs);
> > +	if (src_compressed)
> > +		scratch_buf_fini(&src_ccs);
> >  	scratch_buf_fini(&dst);
> >  	for (int i = 0; i < num_src; i++)
> >  		scratch_buf_fini(&src[i].buf);
> > @@ -749,31 +788,52 @@ igt_main_args("da", NULL, help_str, opt_handler, NULL)
> >  	}
> >  
> >  	igt_subtest("linear")
> > -		test(&data, I915_TILING_NONE, 0);
> > +		test(&data, I915_TILING_NONE, 0, 0);
> >  	igt_subtest("x-tiled")
> > -		test(&data, I915_TILING_X, 0);
> > +		test(&data, I915_TILING_X, 0, 0);
> >  	igt_subtest("y-tiled")
> > -		test(&data, I915_TILING_Y, 0);
> > +		test(&data, I915_TILING_Y, 0, 0);
> >  	igt_subtest("yf-tiled")
> > -		test(&data, I915_TILING_Yf, 0);
> > +		test(&data, I915_TILING_Yf, 0, 0);
> >  
> >  	igt_subtest("y-tiled-ccs-to-linear")
> > -		test(&data, I915_TILING_NONE, I915_TILING_Y);
> > +		test(&data, I915_TILING_NONE, I915_TILING_Y,
> > +		     SRC_COMPRESSED);
> >  	igt_subtest("y-tiled-ccs-to-x-tiled")
> > -		test(&data, I915_TILING_X, I915_TILING_Y);
> > +		test(&data, I915_TILING_X, I915_TILING_Y,
> > +		     SRC_COMPRESSED);
> >  	igt_subtest("y-tiled-ccs-to-y-tiled")
> > -		test(&data, I915_TILING_Y, I915_TILING_Y);
> > +		test(&data, I915_TILING_Y, I915_TILING_Y,
> > +		     SRC_COMPRESSED);
> >  	igt_subtest("y-tiled-ccs-to-yf-tiled")
> > -		test(&data, I915_TILING_Yf, I915_TILING_Y);
> > +		test(&data, I915_TILING_Yf, I915_TILING_Y,
> > +		     SRC_COMPRESSED);
> >  
> >  	igt_subtest("yf-tiled-ccs-to-linear")
> > -		test(&data, I915_TILING_NONE, I915_TILING_Yf);
> > +		test(&data, I915_TILING_NONE, I915_TILING_Yf,
> > +		     SRC_COMPRESSED);
> >  	igt_subtest("yf-tiled-ccs-to-x-tiled")
> > -		test(&data, I915_TILING_X, I915_TILING_Yf);
> > +		test(&data, I915_TILING_X, I915_TILING_Yf,
> > +		     SRC_COMPRESSED);
> >  	igt_subtest("yf-tiled-ccs-to-y-tiled")
> > -		test(&data, I915_TILING_Y, I915_TILING_Yf);
> > +		test(&data, I915_TILING_Y, I915_TILING_Yf,
> > +		     SRC_COMPRESSED);
> >  	igt_subtest("yf-tiled-ccs-to-yf-tiled")
> > -		test(&data, I915_TILING_Yf, I915_TILING_Yf);
> > +		test(&data, I915_TILING_Yf, I915_TILING_Yf,
> > +		     SRC_COMPRESSED);
> > +
> > +	igt_subtest("y-tiled-ccs-to-y-tiled-ccs")
> > +		test(&data, I915_TILING_Y, I915_TILING_Y,
> > +		     SRC_COMPRESSED | DST_COMPRESSED);
> > +	igt_subtest("yf-tiled-ccs-to-yf-tiled-ccs")
> > +		test(&data, I915_TILING_Yf, I915_TILING_Yf,
> > +		     SRC_COMPRESSED | DST_COMPRESSED);
> > +	igt_subtest("y-tiled-ccs-to-yf-tiled-ccs")
> > +		test(&data, I915_TILING_Yf, I915_TILING_Y,
> > +		     SRC_COMPRESSED | DST_COMPRESSED);
> > +	igt_subtest("yf-tiled-ccs-to-y-tiled-ccs")
> > +		test(&data, I915_TILING_Y, I915_TILING_Yf,
> > +		     SRC_COMPRESSED | DST_COMPRESSED);
> >  
> >  	igt_fixture {
> >  		igt_stop_hang_detector();
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev