[Mesa-dev,02/10] i965: Drop broken front_buffer_reading/drawing optimization.

Submitted by Eric Anholt on March 4, 2014, 10:17 p.m.

Details

Message ID 1393971464-12892-3-git-send-email-eric@anholt.net
State New
Headers show

Not browsing as part of any series.

Commit Message

Eric Anholt March 4, 2014, 10:17 p.m.
The flag wasn't getting updated correctly when the ctx->DrawBuffer or
ctx->ReadBuffer changed.  It usually ended up working out because most
apps only have one window system framebuffer, or if they have more than
one and they have any front read/drawing, they will have called
glReadBuffer()/glDrawBuffer() on it when they get started on the new
buffer.
---
 src/mesa/drivers/dri/i965/brw_context.c   | 19 ++++++++-----
 src/mesa/drivers/dri/i965/brw_context.h   | 17 ------------
 src/mesa/drivers/dri/i965/brw_draw.c      |  3 ++-
 src/mesa/drivers/dri/i965/intel_buffers.c | 44 +++++++++++++++++++------------
 src/mesa/drivers/dri/i965/intel_buffers.h |  3 +++
 5 files changed, 44 insertions(+), 42 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index 1441b46..6e21bb8 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -204,7 +204,7 @@  intel_glFlush(struct gl_context *ctx)
 
    intel_batchbuffer_flush(brw);
    intel_flush_front(ctx);
-   if (brw->is_front_buffer_rendering)
+   if (brw_is_front_buffer_drawing(ctx->DrawBuffer))
       brw->need_throttle = true;
 }
 
@@ -1115,6 +1115,7 @@  intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable)
 void
 intel_prepare_render(struct brw_context *brw)
 {
+   struct gl_context *ctx = &brw->ctx;
    __DRIcontext *driContext = brw->driContext;
    __DRIdrawable *drawable;
 
@@ -1136,7 +1137,7 @@  intel_prepare_render(struct brw_context *brw)
     * that will happen next will probably dirty the front buffer.  So
     * mark it as dirty here.
     */
-   if (brw->is_front_buffer_rendering)
+   if (brw_is_front_buffer_drawing(ctx->DrawBuffer))
       brw->front_buffer_dirty = true;
 
    /* Wait for the swapbuffers before the one we just emitted, so we
@@ -1198,8 +1199,8 @@  intel_query_dri2_buffers(struct brw_context *brw,
    back_rb = intel_get_renderbuffer(fb, BUFFER_BACK_LEFT);
 
    memset(attachments, 0, sizeof(attachments));
-   if ((brw->is_front_buffer_rendering ||
-        brw->is_front_buffer_reading ||
+   if ((brw_is_front_buffer_drawing(fb) ||
+        brw_is_front_buffer_reading(fb) ||
         !back_rb) && front_rb) {
       /* If a fake front buffer is in use, then querying for
        * __DRI_BUFFER_FRONT_LEFT will cause the server to copy the image from
@@ -1261,6 +1262,7 @@  intel_process_dri2_buffer(struct brw_context *brw,
                           const char *buffer_name)
 {
    struct intel_region *region = NULL;
+   struct gl_framebuffer *fb = drawable->driverPrivate;
 
    if (!rb)
       return;
@@ -1310,7 +1312,7 @@  intel_process_dri2_buffer(struct brw_context *brw,
 
    intel_update_winsys_renderbuffer_miptree(brw, rb, region);
 
-   if (brw->is_front_buffer_rendering &&
+   if (brw_is_front_buffer_drawing(fb) &&
        (buffer->attachment == __DRI_BUFFER_FRONT_LEFT ||
         buffer->attachment == __DRI_BUFFER_FAKE_FRONT_LEFT) &&
        rb->Base.Base.NumSamples > 1) {
@@ -1346,6 +1348,7 @@  intel_update_image_buffer(struct brw_context *intel,
                           enum __DRIimageBufferMask buffer_type)
 {
    struct intel_region *region = buffer->region;
+   struct gl_framebuffer *fb = drawable->driverPrivate;
 
    if (!rb || !region)
       return;
@@ -1369,7 +1372,7 @@  intel_update_image_buffer(struct brw_context *intel,
 
    intel_update_winsys_renderbuffer_miptree(intel, rb, region);
 
-   if (intel->is_front_buffer_rendering &&
+   if (brw_is_front_buffer_drawing(fb) &&
        buffer_type == __DRI_IMAGE_BUFFER_FRONT &&
        rb->Base.Base.NumSamples > 1) {
       intel_renderbuffer_upsample(intel, rb);
@@ -1397,8 +1400,10 @@  intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable)
    else
       return;
 
-   if ((brw->is_front_buffer_rendering || brw->is_front_buffer_reading || !back_rb) && front_rb)
+   if (front_rb && (brw_is_front_buffer_drawing(fb) ||
+                    brw_is_front_buffer_reading(fb) || !back_rb)) {
       buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;
+   }
 
    if (back_rb)
       buffer_mask |= __DRI_IMAGE_BUFFER_BACK;
diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index dbb30f2..ccbe9ea 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1056,23 +1056,6 @@  struct brw_context
     */
    bool front_buffer_dirty;
 
-   /**
-    * Track whether front-buffer rendering is currently enabled
-    *
-    * A separate flag is used to track this in order to support MRT more
-    * easily.
-    */
-   bool is_front_buffer_rendering;
-
-   /**
-    * Track whether front-buffer is the current read target.
-    *
-    * This is closely associated with is_front_buffer_rendering, but may
-    * be set separately.  The DRI2 fake front buffer must be referenced
-    * either way.
-    */
-   bool is_front_buffer_reading;
-
    /** Framerate throttling: @{ */
    drm_intel_bo *first_post_swapbuffers_batch;
    bool need_throttle;
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
index bc887fe..f5aa327 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -48,6 +48,7 @@ 
 #include "brw_state.h"
 
 #include "intel_batchbuffer.h"
+#include "intel_buffers.h"
 #include "intel_fbo.h"
 #include "intel_mipmap_tree.h"
 #include "intel_regions.h"
@@ -347,7 +348,7 @@  static void brw_postdraw_set_buffers_need_resolve(struct brw_context *brw)
    struct intel_renderbuffer *depth_irb = intel_get_renderbuffer(fb, BUFFER_DEPTH);
    struct gl_renderbuffer_attachment *depth_att = &fb->Attachment[BUFFER_DEPTH];
 
-   if (brw->is_front_buffer_rendering)
+   if (brw_is_front_buffer_drawing(fb))
       front_irb = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT);
 
    if (front_irb)
diff --git a/src/mesa/drivers/dri/i965/intel_buffers.c b/src/mesa/drivers/dri/i965/intel_buffers.c
index 1ece875..23d936f 100644
--- a/src/mesa/drivers/dri/i965/intel_buffers.c
+++ b/src/mesa/drivers/dri/i965/intel_buffers.c
@@ -53,22 +53,36 @@  intel_check_front_buffer_rendering(struct brw_context *brw)
    }
 }
 
+bool
+brw_is_front_buffer_reading(struct gl_framebuffer *fb)
+{
+   if (!fb || _mesa_is_user_fbo(fb))
+      return false;
+
+   return fb->_ColorReadBufferIndex == BUFFER_FRONT_LEFT;
+}
+
+bool
+brw_is_front_buffer_drawing(struct gl_framebuffer *fb)
+{
+   if (!fb || _mesa_is_user_fbo(fb))
+      return false;
+
+   return (fb->_NumColorDrawBuffers >= 1 &&
+           fb->_ColorDrawBufferIndexes[0] == BUFFER_FRONT_LEFT);
+}
+
 static void
 intelDrawBuffer(struct gl_context * ctx, GLenum mode)
 {
-   if (ctx->DrawBuffer && _mesa_is_winsys_fbo(ctx->DrawBuffer)) {
+   if (brw_is_front_buffer_drawing(ctx->DrawBuffer)) {
       struct brw_context *const brw = brw_context(ctx);
-      const bool was_front_buffer_rendering = brw->is_front_buffer_rendering;
 
-      brw->is_front_buffer_rendering = (mode == GL_FRONT_LEFT)
-	|| (mode == GL_FRONT) || (mode == GL_FRONT_AND_BACK);
-
-      /* If we weren't front-buffer rendering before but we are now,
-       * invalidate our DRI drawable so we'll ask for new buffers
+      /* If we might be front-buffer rendering on this buffer for the first
+       * time, invalidate our DRI drawable so we'll ask for new buffers
        * (including the fake front) before we start rendering again.
        */
-      if (!was_front_buffer_rendering && brw->is_front_buffer_rendering)
-	 dri2InvalidateDrawable(brw->driContext->driDrawablePriv);
+      dri2InvalidateDrawable(brw->driContext->driDrawablePriv);
    }
 }
 
@@ -76,18 +90,14 @@  intelDrawBuffer(struct gl_context * ctx, GLenum mode)
 static void
 intelReadBuffer(struct gl_context * ctx, GLenum mode)
 {
-   if (ctx->ReadBuffer && _mesa_is_winsys_fbo(ctx->ReadBuffer)) {
+   if (brw_is_front_buffer_reading(ctx->ReadBuffer)) {
       struct brw_context *const brw = brw_context(ctx);
-      const bool was_front_buffer_reading = brw->is_front_buffer_reading;
-
-      brw->is_front_buffer_reading = mode == GL_FRONT_LEFT || mode == GL_FRONT;
 
-      /* If we weren't front-buffer reading before but we are now,
-       * invalidate our DRI drawable so we'll ask for new buffers
+      /* If we might be front-buffer reading on this buffer for the first
+       * time, invalidate our DRI drawable so we'll ask for new buffers
        * (including the fake front) before we start reading again.
        */
-      if (!was_front_buffer_reading && brw->is_front_buffer_reading)
-	 dri2InvalidateDrawable(brw->driContext->driReadablePriv);
+      dri2InvalidateDrawable(brw->driContext->driReadablePriv);
    }
 }
 
diff --git a/src/mesa/drivers/dri/i965/intel_buffers.h b/src/mesa/drivers/dri/i965/intel_buffers.h
index 6d698a1..f0f6b68 100644
--- a/src/mesa/drivers/dri/i965/intel_buffers.h
+++ b/src/mesa/drivers/dri/i965/intel_buffers.h
@@ -39,4 +39,7 @@  extern void intel_check_front_buffer_rendering(struct brw_context *brw);
 
 extern void intelInitBufferFuncs(struct dd_function_table *functions);
 
+bool brw_is_front_buffer_reading(struct gl_framebuffer *fb);
+bool brw_is_front_buffer_drawing(struct gl_framebuffer *fb);
+
 #endif /* INTEL_BUFFERS_H */

Comments

On 03/04/2014 02:17 PM, Eric Anholt wrote:
> The flag wasn't getting updated correctly when the ctx->DrawBuffer or
> ctx->ReadBuffer changed.  It usually ended up working out because most
> apps only have one window system framebuffer, or if they have more than
> one and they have any front read/drawing, they will have called
> glReadBuffer()/glDrawBuffer() on it when they get started on the new
> buffer.

Yep, it sure doesn't look like they're getting updated when the buffers
change.  I like your fix here.

Patches 1-4 are:
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/09/2014 09:04 PM, Kenneth Graunke wrote:
> On 03/04/2014 02:17 PM, Eric Anholt wrote:
>> The flag wasn't getting updated correctly when the
>> ctx->DrawBuffer or ctx->ReadBuffer changed.  It usually ended up
>> working out because most apps only have one window system
>> framebuffer, or if they have more than one and they have any
>> front read/drawing, they will have called 
>> glReadBuffer()/glDrawBuffer() on it when they get started on the
>> new buffer.
> 
> Yep, it sure doesn't look like they're getting updated when the
> buffers change.  I like your fix here.
> 
> Patches 1-4 are: Reviewed-by: Kenneth Graunke
> <kenneth@whitecape.org>

They are also

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

I'm still working through the others... and Ken's comments on them.

> _______________________________________________ mesa-dev mailing
> list mesa-dev@lists.freedesktop.org 
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)

iEYEARECAAYFAlMeOVoACgkQX1gOwKyEAw8olACeNkXRu0W73/L1T7mpyAUCRdTX
x5EAnjym2IfxElDUHcs8ZP3awV3/T117
=dg6G
-----END PGP SIGNATURE-----