[v4,4/5] st/dri2: Implement DRI2bufferDamageExtension

Submitted by Boris Brezillon on June 25, 2019, 4:37 p.m.

Details

Message ID 20190625163749.15721-5-boris.brezillon@collabora.com
State New
Headers show
Series "EGL_KHR_partial_update support" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Boris Brezillon June 25, 2019, 4:37 p.m.
From: Daniel Stone <daniels@collabora.com>

Add a pipe_screen->set_damage_region() hook to propagate
set-damage-region requests to the driver, it's then up to the driver to
decide what to do with this piece of information.

If the hook is left unassigned, the buffer-damage extension is
considered unsupported.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 src/gallium/include/pipe/p_screen.h   |  7 +++++++
 src/gallium/state_trackers/dri/dri2.c | 22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
index 3f9bad470950..8df12ee4f865 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -464,6 +464,13 @@  struct pipe_screen {
    bool (*is_parallel_shader_compilation_finished)(struct pipe_screen *screen,
                                                    void *shader,
                                                    unsigned shader_type);
+
+   /**
+    * Set damage region.
+    */
+   void (*set_damage_region)(struct pipe_screen *screen,
+                             struct pipe_resource *resource,
+                             unsigned int nrects, int *rects);
 };
 
 
diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c
index 5caaa9deac41..df22e7c41642 100644
--- a/src/gallium/state_trackers/dri/dri2.c
+++ b/src/gallium/state_trackers/dri/dri2.c
@@ -1806,6 +1806,23 @@  static const __DRI2interopExtension dri2InteropExtension = {
    .export_object = dri2_interop_export_object
 };
 
+/**
+ * \brief the DRI2bufferDamageExtension set_damage_region method
+ */
+static void
+dri2_set_damage_region(__DRIdrawable *dPriv, unsigned int nrects, int *rects)
+{
+   struct dri_drawable *drawable = dri_drawable(dPriv);
+   struct pipe_resource *resource = drawable->textures[ST_ATTACHMENT_BACK_LEFT];
+   struct pipe_screen *screen = resource->screen;
+
+   screen->set_damage_region(screen, resource, nrects, rects);
+}
+
+static __DRI2bufferDamageExtension dri2BufferDamageExtension = {
+   .base = { __DRI2_BUFFER_DAMAGE, 1 },
+};
+
 /**
  * \brief the DRI2ConfigQueryExtension configQueryb method
  */
@@ -1907,6 +1924,7 @@  static const __DRIextension *dri_screen_extensions[] = {
    &dri2GalliumConfigQueryExtension.base,
    &dri2ThrottleExtension.base,
    &dri2FenceExtension.base,
+   &dri2BufferDamageExtension.base,
    &dri2InteropExtension.base,
    &dri2NoErrorExtension.base,
    &driBlobExtension.base,
@@ -1922,6 +1940,7 @@  static const __DRIextension *dri_robust_screen_extensions[] = {
    &dri2ThrottleExtension.base,
    &dri2FenceExtension.base,
    &dri2InteropExtension.base,
+   &dri2BufferDamageExtension.base,
    &dri2Robustness.base,
    &dri2NoErrorExtension.base,
    &driBlobExtension.base,
@@ -1982,6 +2001,9 @@  dri2_init_screen(__DRIscreen * sPriv)
       }
    }
 
+   if (pscreen->set_damage_region)
+      dri2BufferDamageExtension.set_damage_region = dri2_set_damage_region;
+
    if (pscreen->get_param(pscreen, PIPE_CAP_DEVICE_RESET_STATUS_QUERY)) {
       sPriv->extensions = dri_robust_screen_extensions;
       screen->has_reset_status_query = true;

Comments


On Wed, 2019-06-26 at 10:34 -0700, Alyssa Rosenzweig wrote:
> Ah-ha, now we're into parts of the stack I can claim to understand!
> >:)
> 
> Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> 
> On Tue, Jun 25, 2019 at 06:37:48PM +0200, Boris Brezillon wrote:
> > From: Daniel Stone <daniels@collabora.com>
> > 
> > Add a pipe_screen->set_damage_region() hook to propagate
> > set-damage-region requests to the driver, it's then up to the
> > driver to
> > decide what to do with this piece of information.
> > 
> > If the hook is left unassigned, the buffer-damage extension is
> > considered unsupported.
> > 
> > Signed-off-by: Daniel Stone <daniels@collabora.com>
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> >  src/gallium/include/pipe/p_screen.h   |  7 +++++++
> >  src/gallium/state_trackers/dri/dri2.c | 22 ++++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> > 
> > diff --git a/src/gallium/include/pipe/p_screen.h
> > b/src/gallium/include/pipe/p_screen.h
> > index 3f9bad470950..8df12ee4f865 100644
> > --- a/src/gallium/include/pipe/p_screen.h
> > +++ b/src/gallium/include/pipe/p_screen.h
> > @@ -464,6 +464,13 @@ struct pipe_screen {
> >     bool (*is_parallel_shader_compilation_finished)(struct
> > pipe_screen *screen,
> >                                                     void *shader,
> >                                                     unsigned
> > shader_type);
> > +
> > +   /**
> > +    * Set damage region.
> > +    */
> > +   void (*set_damage_region)(struct pipe_screen *screen,
> > +                             struct pipe_resource *resource,
> > +                             unsigned int nrects, int *rects);

I would kinda have expected rects to be an array of pipe_box instead of
just an array of integers, as that'd be a bit easier to know the
semantics of...

> >  };
> >  
> >  
> > diff --git a/src/gallium/state_trackers/dri/dri2.c
> > b/src/gallium/state_trackers/dri/dri2.c
> > index 5caaa9deac41..df22e7c41642 100644
> > --- a/src/gallium/state_trackers/dri/dri2.c
> > +++ b/src/gallium/state_trackers/dri/dri2.c
> > @@ -1806,6 +1806,23 @@ static const __DRI2interopExtension
> > dri2InteropExtension = {
> >     .export_object = dri2_interop_export_object
> >  };
> >  
> > +/**
> > + * \brief the DRI2bufferDamageExtension set_damage_region method
> > + */
> > +static void
> > +dri2_set_damage_region(__DRIdrawable *dPriv, unsigned int nrects,
> > int *rects)
> > +{
> > +   struct dri_drawable *drawable = dri_drawable(dPriv);
> > +   struct pipe_resource *resource = drawable-
> > >textures[ST_ATTACHMENT_BACK_LEFT];
> > +   struct pipe_screen *screen = resource->screen;
> > +
> > +   screen->set_damage_region(screen, resource, nrects, rects);
> > +}
> > +
> > +static __DRI2bufferDamageExtension dri2BufferDamageExtension = {
> > +   .base = { __DRI2_BUFFER_DAMAGE, 1 },
> > +};
> > +
> >  /**
> >   * \brief the DRI2ConfigQueryExtension configQueryb method
> >   */
> > @@ -1907,6 +1924,7 @@ static const __DRIextension
> > *dri_screen_extensions[] = {
> >     &dri2GalliumConfigQueryExtension.base,
> >     &dri2ThrottleExtension.base,
> >     &dri2FenceExtension.base,
> > +   &dri2BufferDamageExtension.base,
> >     &dri2InteropExtension.base,
> >     &dri2NoErrorExtension.base,
> >     &driBlobExtension.base,
> > @@ -1922,6 +1940,7 @@ static const __DRIextension
> > *dri_robust_screen_extensions[] = {
> >     &dri2ThrottleExtension.base,
> >     &dri2FenceExtension.base,
> >     &dri2InteropExtension.base,
> > +   &dri2BufferDamageExtension.base,
> >     &dri2Robustness.base,
> >     &dri2NoErrorExtension.base,
> >     &driBlobExtension.base,
> > @@ -1982,6 +2001,9 @@ dri2_init_screen(__DRIscreen * sPriv)
> >        }
> >     }
> >  
> > +   if (pscreen->set_damage_region)
> > +      dri2BufferDamageExtension.set_damage_region =
> > dri2_set_damage_region;
> > +
> >     if (pscreen->get_param(pscreen,
> > PIPE_CAP_DEVICE_RESET_STATUS_QUERY)) {
> >        sPriv->extensions = dri_robust_screen_extensions;
> >        screen->has_reset_status_query = true;
> > -- 
> > 2.20.1
> > 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Wed, 03 Jul 2019 11:54:29 +0200
Erik Faye-Lund <erik.faye-lund@collabora.com> wrote:

> On Wed, 2019-06-26 at 10:34 -0700, Alyssa Rosenzweig wrote:
> > Ah-ha, now we're into parts of the stack I can claim to understand!  
> > >:)  
> > 
> > Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> > 
> > On Tue, Jun 25, 2019 at 06:37:48PM +0200, Boris Brezillon wrote:  
> > > From: Daniel Stone <daniels@collabora.com>
> > > 
> > > Add a pipe_screen->set_damage_region() hook to propagate
> > > set-damage-region requests to the driver, it's then up to the
> > > driver to
> > > decide what to do with this piece of information.
> > > 
> > > If the hook is left unassigned, the buffer-damage extension is
> > > considered unsupported.
> > > 
> > > Signed-off-by: Daniel Stone <daniels@collabora.com>
> > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > > ---
> > >  src/gallium/include/pipe/p_screen.h   |  7 +++++++
> > >  src/gallium/state_trackers/dri/dri2.c | 22 ++++++++++++++++++++++
> > >  2 files changed, 29 insertions(+)
> > > 
> > > diff --git a/src/gallium/include/pipe/p_screen.h
> > > b/src/gallium/include/pipe/p_screen.h
> > > index 3f9bad470950..8df12ee4f865 100644
> > > --- a/src/gallium/include/pipe/p_screen.h
> > > +++ b/src/gallium/include/pipe/p_screen.h
> > > @@ -464,6 +464,13 @@ struct pipe_screen {
> > >     bool (*is_parallel_shader_compilation_finished)(struct
> > > pipe_screen *screen,
> > >                                                     void *shader,
> > >                                                     unsigned
> > > shader_type);
> > > +
> > > +   /**
> > > +    * Set damage region.
> > > +    */
> > > +   void (*set_damage_region)(struct pipe_screen *screen,
> > > +                             struct pipe_resource *resource,
> > > +                             unsigned int nrects, int *rects);  
> 
> I would kinda have expected rects to be an array of pipe_box instead of
> just an array of integers, as that'd be a bit easier to know the
> semantics of...

Sure, I can do that. Should I do the Y-flip as part of the ints -> box
conversion or should I keep the "origin is bottom-left" semantic?
On Wed, 2019-07-03 at 12:33 +0200, Boris Brezillon wrote:
> On Wed, 03 Jul 2019 11:54:29 +0200
> Erik Faye-Lund <erik.faye-lund@collabora.com> wrote:
> 
> > On Wed, 2019-06-26 at 10:34 -0700, Alyssa Rosenzweig wrote:
> > > Ah-ha, now we're into parts of the stack I can claim to
> > > understand!  
> > > > :)  
> > > 
> > > Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
> > > 
> > > On Tue, Jun 25, 2019 at 06:37:48PM +0200, Boris Brezillon
> > > wrote:  
> > > > From: Daniel Stone <daniels@collabora.com>
> > > > 
> > > > Add a pipe_screen->set_damage_region() hook to propagate
> > > > set-damage-region requests to the driver, it's then up to the
> > > > driver to
> > > > decide what to do with this piece of information.
> > > > 
> > > > If the hook is left unassigned, the buffer-damage extension is
> > > > considered unsupported.
> > > > 
> > > > Signed-off-by: Daniel Stone <daniels@collabora.com>
> > > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > > > ---
> > > >  src/gallium/include/pipe/p_screen.h   |  7 +++++++
> > > >  src/gallium/state_trackers/dri/dri2.c | 22
> > > > ++++++++++++++++++++++
> > > >  2 files changed, 29 insertions(+)
> > > > 
> > > > diff --git a/src/gallium/include/pipe/p_screen.h
> > > > b/src/gallium/include/pipe/p_screen.h
> > > > index 3f9bad470950..8df12ee4f865 100644
> > > > --- a/src/gallium/include/pipe/p_screen.h
> > > > +++ b/src/gallium/include/pipe/p_screen.h
> > > > @@ -464,6 +464,13 @@ struct pipe_screen {
> > > >     bool (*is_parallel_shader_compilation_finished)(struct
> > > > pipe_screen *screen,
> > > >                                                     void
> > > > *shader,
> > > >                                                     unsigned
> > > > shader_type);
> > > > +
> > > > +   /**
> > > > +    * Set damage region.
> > > > +    */
> > > > +   void (*set_damage_region)(struct pipe_screen *screen,
> > > > +                             struct pipe_resource *resource,
> > > > +                             unsigned int nrects, int
> > > > *rects);  
> > 
> > I would kinda have expected rects to be an array of pipe_box
> > instead of
> > just an array of integers, as that'd be a bit easier to know the
> > semantics of...
> 
> Sure, I can do that. Should I do the Y-flip as part of the ints ->
> box
> conversion or should I keep the "origin is bottom-left" semantic?

Good question? :)

My instinct would be to have the regions in "memory layout order"
rather than "window sytem presentation order", which I guess would be
without any sort of y-flipping or rotation applied. I guess that would
make the answer "no"? But I'm not sure, I haven't read up on the spec
here.