Created attachment 20001 [details] [review] Patch as described With the current intel driver, small 1x1 pixel reads often (perhaps even usually) give the wrong result. I think the problem is that when the code falls back to _swrast_ReadPixels(), it doesn't wait for the GPU to finish. The patch I'll attach here adds an intelFinish() call before calling _swrast_ReadPixels(). It fixes the problems I'm observing. Comments: - I think the intelFlush() call at the beginning of of intelReadPixels() can be removed. do_blit_readpixels() calls intelFlush() itself, as does intelFinish(). Performance difference for doing so is probably minimal. - intelFinish() does a bit more than is necessary, since it waits for rendering to all color buffers to finish, not just the one we are going to read from. Seems unlikely to matter, since presumably the last thing done before glReadPixels() is to render to the buffer you are going to read from. - I don't see where do_blit_readpixels() waits for the blit it emitted to finish, but I don't have a test case for that code path, and maybe that happens implicitly or in the upper layers. - I don't know the code at all, so please double check for sanity :-)
bugzilla-daemon@freedesktop.org schrieb: > - intelFinish() does a bit more than is necessary, since it waits for > rendering to all color buffers to finish, not just the one we > are going to read from. Seems unlikely to matter, since presumably > the last thing done before glReadPixels() is to render to the > buffer you are going to read from. I don't think so. AFAIR using multiple buffers, rendering the next one before reading the current one is used by some applications to improve speed. Philipp
In further testing, the patch works as advertised for direct rendering but when running inside the server (AIGLX) seems to have no effect. My only idea is that the operation dri_bo_wait_rendering() (which calls DRM_IOCTL_I915_GEM_SET_DOMAIN with read_domains of I915_GEM_DOMAIN_GTT) is ineffective inside the server for some reason.
We should figure out why the existing code is failing to synchronize correctly, since swrast_ReadPixels should call the span read functions, which use dri_bo_get_subdata, which should correctly synchronize with any flushed rendering. My guess is you're running into a kernel bug and papering over it with adding more waiting, and losing in the server case where the timing/cache usage is different.
Any advice about how to investigate further?
Turns out after discussion on IRC that the mesa patch here is a red herring. The problem is basically what is discussed in: http://lists.freedesktop.org/archives/intel-gfx/2008-October/000482.html "i915_gem_object_set_domain is also used from the set_domain ioctl. In this case, the domains will be mashed and a flush operation pushed into the ring, but the object will not be placed on the active list when that flush is posted." The patch in there doesn't quite fix the problem, since i915_gem_object_set_domain_range() thinks that it can tell whether to call i915_gem_object_wait_rendering() by just looking at object->write_domain. A small additional patch ... removing the check and just always calling wait_rendering() gets things working.
Created attachment 20014 [details] [review] Patch on top of keithp's patch
Applied the patch to for-airlied and drm-intel-next, since it was obviously needed. Is it good enough to fix your bug on its own?
I'll try this evening, but my understanding is no. To quote Keith's mail: i915_gem_object_set_domain is also used from the set_domain ioctl. In this case, the domains will be mashed and a flush operation pushed into the ring, but the object will not be placed on the active list when that flush is posted. This means that no-one knows to wait for the operation to complete before accessing the object. and (as far as I recall) I think that the situation I was hitting fit that situation. So, checking if the texture is on the active list doesn't do any good if the object didn't get added to the active list.
Contrary to my expectation, my patch in isolation does fix the problem I was observing!
Excellent! I'll mark this one closed, then.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.