[Mesa-dev] [PATCH] egl/x11: Send invalidate to the driver on dri2_copy_region
Michel Dänzer
michel at daenzer.net
Thu Apr 26 09:56:05 UTC 2018
On 2018-04-26 11:33 AM, Thomas Hellstrom wrote:
> On 04/26/2018 10:30 AM, Michel Dänzer wrote:
>> On 2018-04-25 07:49 PM, Deepak Rawat wrote:
>>> Similar to what is done in dri2_x11_swap_buffers_msc send invalidate
>>> to the driver because egl/X11 is not watching for for server's
>>> invalidate events. The dri2_copy_region path is trigerred when
>>> server supports DRI2 version minor 1.
>>>
>>> Tested with piglit egl tests for regression.
>>>
>>> Cc: <mesa-stable at lists.freedesktop.org>
>>> Signed-off-by: Deepak Rawat <drawat at vmware.com>
>>> Reviewed-by: Thomas Hellstrom <thellstrom at vmware.com>
>>> ---
>>> src/egl/drivers/dri2/platform_x11.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/src/egl/drivers/dri2/platform_x11.c
>>> b/src/egl/drivers/dri2/platform_x11.c
>>> index 6c287b4d06..e99434ea3a 100644
>>> --- a/src/egl/drivers/dri2/platform_x11.c
>>> +++ b/src/egl/drivers/dri2/platform_x11.c
>>> @@ -841,6 +841,13 @@ dri2_copy_region(_EGLDriver *drv, _EGLDisplay
>>> *disp,
>>> render_attachment);
>>> free(xcb_dri2_copy_region_reply(dri2_dpy->conn, cookie, NULL));
>>> + /*
>>> + * Just like as done in dri2_x11_swap_buffers_msc we aren't
>>> watching for
>>> + * server's invalidate events, so just send invalidate to driver.
>>> + */
>>> + if (dri2_dpy->flush->base.version >= 3 &&
>>> dri2_dpy->flush->invalidate)
>>> + dri2_dpy->flush->invalidate(dri2_surf->dri_drawable);
>>> +
>>> return EGL_TRUE;
>>> }
>> Why is invalidate needed after copy_region? That's surprising, I suspect
>> it just papers over the real problem.
>>
>>
> I had a quick look into the platform_x11 code. dri2_copy_region is
> called only from the various swap_buffers() paths,
> dri2_x11_swap_buffers() and dri2_x11_swap_buffers_region(). Explicit
> invalidation is, before this patch, done only if the server supports
> dri2 swaps. Probably because most drivers do, vmware does not, so we can
> hit swapbuffers without doing explicit invalidation.
>
> In comparison, GLX does explicit invalidation on swapbuffers,
> bind_context and bind_texture for the same protocol version, so this
> patch should only bring the EGL swapbuffer paths closer to what GLX is
> doing, while still not addressing bind_context and bind_texture.
>
> FWIW, EGL platform_x11 (dri2) seems to not parse server invalidate
> events for the higher protocol versions, relying solely on explicit
> invalidation.
The purpose of the invalidate callback is to trigger updating the DRI2
buffers before drawing the next frame. This isn't necessary after a
copy_region operation, so you seem to be relying on some kind of side
effect of the invalidate callback. Which may be okay, but I think it
would be clearer not to put this code in dri2_copy_region itself.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
More information about the mesa-dev
mailing list