From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v2 2/2] kms_fbc_crc: Add a CRC based FBC test Date: Mon, 25 Nov 2013 16:19:21 +0100 Message-ID: <20131125151921.GA27344@phenom.ffwll.local> References: <1385062420-19782-2-git-send-email-ville.syrjala@linux.intel.com> <1385392121-28237-1-git-send-email-ville.syrjala@linux.intel.com> <1385392121-28237-2-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ea0-f175.google.com (mail-ea0-f175.google.com [209.85.215.175]) by gabe.freedesktop.org (Postfix) with ESMTP id 02323FA8AD for ; Mon, 25 Nov 2013 07:18:40 -0800 (PST) Received: by mail-ea0-f175.google.com with SMTP id z10so2513863ead.34 for ; Mon, 25 Nov 2013 07:18:40 -0800 (PST) Content-Disposition: inline In-Reply-To: <1385392121-28237-2-git-send-email-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: ville.syrjala@linux.intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Nov 25, 2013 at 05:08:41PM +0200, ville.syrjala@linux.intel.com wro= te: > From: Ville Syrj=E4l=E4 > = > kms_fbc_crc will perform various write operations to the scanout buffer > whilc FBC is enabled. CRC checks will be used to make sure the > modifcations to scanout buffer are detected. > = > The operations include: > - page flip > - pwrite > - GTT mmap > - CPU mmap > - blit > - rendercopy > - context switch + rendercopy > - combination of a page flip and each operation listed above > = > v2: Use gem_sw_finish instead of drmModeDirtyFB after CPU access > = > Signed-off-by: Ville Syrj=E4l=E4 [snip] > + switch (mode) { > + void *ptr; > + case TEST_PAGE_FLIP: > + igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id, > + data->fb_id[1], 0, NULL) =3D=3D 0); > + break; > + case TEST_PWRITE: > + case TEST_PAGE_FLIP_AND_PWRITE: > + gem_write(data->drm_fd, handle, 0, buf, 1); > + gem_sw_finish(data->drm_fd, handle); This one's too much. Like I've said current rules for pwrite is that a) it's only used for cursors b) no explicit flushing afterwards. So the sw_finish here could potentially paper over kernel bugs (but only for psr since fbc doesn't care about cursors). > + break; > + case TEST_MMAP_CPU: > + case TEST_PAGE_FLIP_AND_MMAP_CPU: > + ptr =3D gem_mmap__cpu(data->drm_fd, handle, 4096, PROT_WRITE); > + gem_set_domain(data->drm_fd, handle, I915_GEM_DOMAIN_CPU, I915_GEM_DOM= AIN_CPU); > + memset(ptr, 0xff, 4); > + gem_set_domain(data->drm_fd, handle, I915_GEM_DOMAIN_GTT, 0); Nope, that's a bug. There's not set_domain(GTT, 0) after a cpu mmap operation, just the sw_finish ioctl call. set_domain is only done _before_ mmap access (yeah, a bit a design but in the original gem interface to no have a begin/end pair). > + munmap(ptr, 4096); > + gem_sw_finish(data->drm_fd, handle); > + break; > + case TEST_MMAP_GTT: > + case TEST_PAGE_FLIP_AND_MMAP_GTT: > + ptr =3D gem_mmap__gtt(data->drm_fd, handle, 4096, PROT_WRITE); > + gem_set_domain(data->drm_fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOM= AIN_GTT); > + memset(ptr, 0xff, 4); > + gem_set_domain(data->drm_fd, handle, I915_GEM_DOMAIN_GTT, 0); Again no set_domain after the writes have landed. > + munmap(ptr, 4096); > + break; > + case TEST_BLT: > + case TEST_PAGE_FLIP_AND_BLT: > + fill_blt(data, handle, 0xff); > + break; > + case TEST_RENDER: > + case TEST_CONTEXT: > + case TEST_PAGE_FLIP_AND_RENDER: > + case TEST_PAGE_FLIP_AND_CONTEXT: > + fill_render(data, handle, > + (mode =3D=3D TEST_CONTEXT || mode =3D=3D TEST_PAGE_FLIP_AND_CONTE= XT) ? > + data->ctx[1] : NULL, 0xff); busy ioctl call on the render target/frontbuffer still seems to be missing. -Daniel -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch