Re: [Mesa-dev] [PATCH RFC 1/2] dri: Let drivers reset the damage region
On Thu, 29 Aug 2019 13:54:15 +0200
Boris Brezillon wrote:
> dri2_set_damage_region() must call st_validate_state(UPDATE_FRAMEBUFFER)
> to make sure the BACK_LEFT attachment is up-to-date.
> Problem is, dri2_swap_buffers_xxx() functions are actually targeting
> the old backbuffer when they call ->set_damage_region(0, NULL), and
> more importantly, they are not expecting the caller to pull a new back
> buffer at this point, which will happen if st_validate_state() is
> called.
>
> Let's delegate the damage region reset to drivers (they should take
> care of that at flush/swap time) so we don't have to deal with this
> extra complexity at the EGL level.
>
> The only gallium driver implementing ->set_damage_region() is patched
> accordingly.
>
> Signed-off-by: Boris Brezillon
> ---
> include/GL/internal/dri_interface.h | 4
> src/egl/drivers/dri2/egl_dri2.c | 24 -
> src/gallium/drivers/panfrost/pan_context.c | 14 +++-
> src/gallium/drivers/panfrost/pan_resource.c | 2 +-
> src/gallium/drivers/panfrost/pan_resource.h | 2 ++
> 5 files changed, 20 insertions(+), 26 deletions(-)
>
> diff --git a/include/GL/internal/dri_interface.h
> b/include/GL/internal/dri_interface.h
> index 4b5583820ed0..4c60d349ddd5 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -521,6 +521,10 @@ struct __DRI2bufferDamageExtensionRec {
> * reset the region, followed by a second call with a populated region,
> * before a rendering call is made.
> *
> +* Drivers implementing this entrypoint should take care of resetting the
> +* damage region of resources being written to at swapbuffer/flush time.
> +* The DRI core will not automate that for you.
> +*
> * Used to implement EGL_KHR_partial_update.
> *
> * \param drawable affected drawable
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 526cb1969c18..ea8ae5d44c56 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -1767,7 +1767,6 @@ static EGLBoolean
> dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
> {
> struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> - __DRIdrawable *dri_drawable = dri2_dpy->vtbl->get_dri_drawable(surf);
> _EGLContext *ctx = _eglGetCurrentContext();
> EGLBoolean ret;
>
> @@ -1775,13 +1774,6 @@ dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp,
> _EGLSurface *surf)
>dri2_surf_update_fence_fd(ctx, disp, surf);
> ret = dri2_dpy->vtbl->swap_buffers(drv, disp, surf);
>
> - /* SwapBuffers marks the end of the frame; reset the damage region for
> -* use again next time.
> -*/
> - if (ret && dri2_dpy->buffer_damage &&
> - dri2_dpy->buffer_damage->set_damage_region)
> - dri2_dpy->buffer_damage->set_damage_region(dri_drawable, 0, NULL);
> -
> return ret;
> }
>
> @@ -1791,7 +1783,6 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
> _EGLDisplay *disp,
>const EGLint *rects, EGLint n_rects)
> {
> struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> - __DRIdrawable *dri_drawable = dri2_dpy->vtbl->get_dri_drawable(surf);
> _EGLContext *ctx = _eglGetCurrentContext();
> EGLBoolean ret;
>
> @@ -1800,13 +1791,6 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
> _EGLDisplay *disp,
> ret = dri2_dpy->vtbl->swap_buffers_with_damage(drv, disp, surf,
>rects, n_rects);
>
> - /* SwapBuffers marks the end of the frame; reset the damage region for
> -* use again next time.
> -*/
> - if (ret && dri2_dpy->buffer_damage &&
> - dri2_dpy->buffer_damage->set_damage_region)
> - dri2_dpy->buffer_damage->set_damage_region(dri_drawable, 0, NULL);
> -
> return ret;
> }
>
> @@ -1815,18 +1799,10 @@ dri2_swap_buffers_region(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf,
> EGLint numRects, const EGLint *rects)
> {
> struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> - __DRIdrawable *dri_drawable = dri2_dpy->vtbl->get_dri_drawable(surf);
> EGLBoolean ret;
>
> ret = dri2_dpy->vtbl->swap_buffers_region(drv, disp, surf, numRects,
> rects);
>
> - /* SwapBuffers marks the end of the frame; reset the damage region for
> -* use again next time.
> -*/
> - if (ret && dri2_dpy->buffer_damage &&
> - dri2_dpy->buffer_damage->set_damage_region)
> - dri2_dpy->buffer_damage->set_damage_region(dri_drawable, 0, NULL);
> -
> return ret;
> }
>
> diff --git a/src/gallium/drivers/panfrost/pan_context.c
> b/src/gallium/drivers/panfrost/pan_context.c
> index fa9c92af9f6a..4069ec238fe4 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -1461,7 +1461,8 @@ panfrost_flush(
> struct panfr
[Mesa-dev] [PATCH RFC 1/2] dri: Let drivers reset the damage region
dri2_set_damage_region() must call st_validate_state(UPDATE_FRAMEBUFFER)
to make sure the BACK_LEFT attachment is up-to-date.
Problem is, dri2_swap_buffers_xxx() functions are actually targeting
the old backbuffer when they call ->set_damage_region(0, NULL), and
more importantly, they are not expecting the caller to pull a new back
buffer at this point, which will happen if st_validate_state() is
called.
Let's delegate the damage region reset to drivers (they should take
care of that at flush/swap time) so we don't have to deal with this
extra complexity at the EGL level.
The only gallium driver implementing ->set_damage_region() is patched
accordingly.
Signed-off-by: Boris Brezillon
---
include/GL/internal/dri_interface.h | 4
src/egl/drivers/dri2/egl_dri2.c | 24 -
src/gallium/drivers/panfrost/pan_context.c | 14 +++-
src/gallium/drivers/panfrost/pan_resource.c | 2 +-
src/gallium/drivers/panfrost/pan_resource.h | 2 ++
5 files changed, 20 insertions(+), 26 deletions(-)
diff --git a/include/GL/internal/dri_interface.h
b/include/GL/internal/dri_interface.h
index 4b5583820ed0..4c60d349ddd5 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -521,6 +521,10 @@ struct __DRI2bufferDamageExtensionRec {
* reset the region, followed by a second call with a populated region,
* before a rendering call is made.
*
+* Drivers implementing this entrypoint should take care of resetting the
+* damage region of resources being written to at swapbuffer/flush time.
+* The DRI core will not automate that for you.
+*
* Used to implement EGL_KHR_partial_update.
*
* \param drawable affected drawable
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 526cb1969c18..ea8ae5d44c56 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1767,7 +1767,6 @@ static EGLBoolean
dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
{
struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
- __DRIdrawable *dri_drawable = dri2_dpy->vtbl->get_dri_drawable(surf);
_EGLContext *ctx = _eglGetCurrentContext();
EGLBoolean ret;
@@ -1775,13 +1774,6 @@ dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp,
_EGLSurface *surf)
dri2_surf_update_fence_fd(ctx, disp, surf);
ret = dri2_dpy->vtbl->swap_buffers(drv, disp, surf);
- /* SwapBuffers marks the end of the frame; reset the damage region for
-* use again next time.
-*/
- if (ret && dri2_dpy->buffer_damage &&
- dri2_dpy->buffer_damage->set_damage_region)
- dri2_dpy->buffer_damage->set_damage_region(dri_drawable, 0, NULL);
-
return ret;
}
@@ -1791,7 +1783,6 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
_EGLDisplay *disp,
const EGLint *rects, EGLint n_rects)
{
struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
- __DRIdrawable *dri_drawable = dri2_dpy->vtbl->get_dri_drawable(surf);
_EGLContext *ctx = _eglGetCurrentContext();
EGLBoolean ret;
@@ -1800,13 +1791,6 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv,
_EGLDisplay *disp,
ret = dri2_dpy->vtbl->swap_buffers_with_damage(drv, disp, surf,
rects, n_rects);
- /* SwapBuffers marks the end of the frame; reset the damage region for
-* use again next time.
-*/
- if (ret && dri2_dpy->buffer_damage &&
- dri2_dpy->buffer_damage->set_damage_region)
- dri2_dpy->buffer_damage->set_damage_region(dri_drawable, 0, NULL);
-
return ret;
}
@@ -1815,18 +1799,10 @@ dri2_swap_buffers_region(_EGLDriver *drv, _EGLDisplay
*disp, _EGLSurface *surf,
EGLint numRects, const EGLint *rects)
{
struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
- __DRIdrawable *dri_drawable = dri2_dpy->vtbl->get_dri_drawable(surf);
EGLBoolean ret;
ret = dri2_dpy->vtbl->swap_buffers_region(drv, disp, surf, numRects, rects);
- /* SwapBuffers marks the end of the frame; reset the damage region for
-* use again next time.
-*/
- if (ret && dri2_dpy->buffer_damage &&
- dri2_dpy->buffer_damage->set_damage_region)
- dri2_dpy->buffer_damage->set_damage_region(dri_drawable, 0, NULL);
-
return ret;
}
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index fa9c92af9f6a..4069ec238fe4 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -1461,7 +1461,8 @@ panfrost_flush(
struct panfrost_job *job = panfrost_get_job_for_fbo(ctx);
/* Nothing to do! */
-if (!job->last_job.gpu && !job->clear) return;
+if (!job->last_job.gpu && !job->clear)
+goto reset_damage;
if (!job->clear && job->last_tiler.gpu)
panfrost_draw
