[igt-dev] [PATCH v3 5/8] tests/kms_draw_crc: Fix generating reference CRCs on platforms w/o aperture

[Imre Deak]

On Wed, Feb 12, 2020 at 08:20:36PM -0800, Dixit, Ashutosh wrote:
> On Wed, 12 Feb 2020 13:50:34 -0800, Imre Deak wrote:
> >
> > Generate reference CRCs by drawing through a WC mapping on platforms w/o
> > a GTT aperture.
> >
> > v2:
> > - Fix code comment in draw_method_subtest(). (Matt)
> > v3:
> > - Use WC instead of CPU mapping. (Ashutosh)
> >
> > Cc: Ashutosh Dixit <ashutosh.dixit at intel.com>
> > Cc: Matt Roper <matthew.d.roper at intel.com>
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > Reviewed-by: Matt Roper <matthew.d.roper at intel.com> (v2)
> > ---
> >  tests/kms_draw_crc.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
> > index 6de9feaeb..fa7433ab2 100644
> > --- a/tests/kms_draw_crc.c
> > +++ b/tests/kms_draw_crc.c
> > @@ -183,11 +183,13 @@ static void draw_method_subtest(enum igt_draw_method method,
> >
> >	igt_require(format_is_supported(formats[format_index], tiling));
> >
> > -	/* Use IGT_DRAW_MMAP_GTT on an untiled buffer as the parameter for
> > +	/* Use IGT_DRAW_MMAP_GTT/WC on an untiled buffer as the parameter for
> >	 * comparison. Cache the value so we don't recompute it for every single
> >	 * subtest. */
> >	if (!base_crcs[format_index].set) {
> > -		get_method_crc(IGT_DRAW_MMAP_GTT, formats[format_index],
> > +		get_method_crc(gem_has_mappable_ggtt(drm_fd) ? IGT_DRAW_MMAP_GTT :
> > +							       IGT_DRAW_MMAP_WC,
> > +			       formats[format_index],
> >			       LOCAL_DRM_FORMAT_MOD_NONE,
> >			       &base_crcs[format_index].crc);
> >		base_crcs[format_index].set = true;
> > @@ -225,7 +227,9 @@ static void fill_fb_subtest(void)
> >	igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode->vdisplay,
> >		      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &fb);
> >
> > -	igt_draw_rect_fb(drm_fd, bufmgr, NULL, &fb, IGT_DRAW_MMAP_GTT,
> > +	igt_draw_rect_fb(drm_fd, bufmgr, NULL, &fb,
> > +			 gem_has_mappable_ggtt(drm_fd) ? IGT_DRAW_MMAP_GTT :
> > +							 IGT_DRAW_MMAP_WC,
> 
> Not sure if the conditional switch between GTT and WC is really needed.

We can't use either one unconditionally since neither is available on
all platforms. So using GTT if available as before and if not WC.

> For example, __gem_mmap__device_coherent() unconditionally prefers WC over
> GTT. In any case:
> 
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit at intel.com>