[PATCH i-g-t] tests/intel/kms_ccs: add access to control surface on xe2

[Zbigniew Kempczyński]

On Wed, Mar 20, 2024 at 03:12:53PM +0200, Juha-Pekka Heikkila wrote:
> On 20.3.2024 13.34, Zbigniew Kempczyński wrote:
> > On Wed, Mar 20, 2024 at 12:07:39PM +0200, Juha-Pekka Heikkila wrote:
> > 
> > <cut>
> > 
> > > > > +	bb_size = xe_bb_size(fb->fd, SZ_4K);
> > > > > +	bb1 = xe_bo_create(fb->fd, 0, bb_size, sysmem, 0);
> > > > > +	blt_set_batch(&surf.bb, bb1, bb_size, sysmem);
> > > > > +	blt_ctrl_surf_copy(fb->fd, surf_ctx, NULL, surf_ahnd, &surf);
> > > > > +	intel_ctx_xe_sync(surf_ctx, true);
> > > > > +
> > > > > +	ccsmap = xe_bo_map(fb->fd, ccs, surf.dst.size);
> > > > > +
> > > > > +	if (verify_compression) {
> > > > > +		for (int i = 0; i < surf.dst.size / sizeof(uint32_t); i++) {
> > > > > +			if (ccsmap[i] != 0)
> > > > > +				goto ccs_vefiried;
> > 
> > Problem I see for noise data in fb ccsmap[] will contain only zeros
> > so we catch assert below.
> > 
> 
> but this part is on the other side of if() like this
> 
> +	if (verify_compression) {
> +		for (int i = 0; i < surf.dst.size / sizeof(uint32_t); i++) {
> +			if (ccsmap[i] != 0)
> +				goto ccs_vefiried;
> +		}
> +		igt_assert_f(false, "framebuffer was not compressed!\n");
> +	} else {
> +		for (int i = 0; i < surf.dst.size / sizeof(uint32_t); i++)
> +			ccsmap[i] = rand();
> +	}
> 
> that verify_compression flag is set only when coming here from
> xe2_ccs_blit() where on fb is always supposing to be compressed test image.
> I made it this way just to avoid duplicating all the above code.

Ok, now I get your point. You're skipping verification when comming from
fill_fb_random(). That's makes sense and code looks correct imo. Fix
only "vefiried" and:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>

--
Zbigniew


> 
> > > > 
> > > > That's minor nit, s/vefiried/verified/g.
> > > > 
> > > > Second doesn't reside in this code, it is in the test itself:
> > > > 
> > > > static void fill_fb_random(int drm_fd, igt_fb_t *fb)
> > > > {
> > > > ...
> > > > 	for (i = 0; i < fb->size; i++)
> > > > 		p[i] = rand();
> > > > ...
> > > > 
> > > > Randomized data might lead your surface won't be surprised at all (data
> > > > are noise). I experimented with this some time ago when I played with
> > > > ctrl-surf-copy. Noise wasn't compressible so I think you should less
> > > > random data in case you want to check the ccs data.
> > > 
> > > Idea of this test is exatcly as you described above. Throw completely random
> > > data at control surface (same way as is done in aux ccs version of this
> > > test) and see nothing breaks, ie. no hw hangs, no unexpected display
> > > behaviors etc. Plan with this test is not to behave correctly towards hw but
> > > just see hw survived unexpected situation.
> > > 
> > > /Juha-Pekka
> > > 
> > > > > +		}
> > > > > +		igt_assert_f(false, "framebuffer was not compressed!\n");
> > 
> > I mean this one. So if I'm not wrong on some noise you may be catched
> > on assertion when fb won't be compressed due to data are not
> > compressible. Really assertion is your intention?
> 
> When this side of that if() is being run we're coming from xe2_ccs_blit()
> and on framebuffer is supposing to be compressed test image. If there's no
> compression information it mean blit didn't compress image hence assert as
> we have non-compressed test image.
> 
> /Juha-Pekka