On 07/23/2012 03:16 PM, Paul Berry wrote: > On 21 July 2012 17:36, Chad Versace <chad.vers...@linux.intel.com > <mailto:chad.vers...@linux.intel.com>> wrote: > > Immediately after obtaining, with DRI2GetBuffersWithFormat, the DRM buffer > handle for a DRI2 buffer, we wrap that DRM buffer handle with a region and > a miptre. This patch additionally allocates an accompanying multisample > > > "miptree"
Fixed. > > > miptree if the DRI2 buffer is multisampled. > > Since we do not yet advertise multisample GL configs, the code for > allocating the multisample miptree is currently inactive. > > CC: Eric Anholt <e...@anholt.net <mailto:e...@anholt.net>> > CC: Paul Berry <stereotype...@gmail.com <mailto:stereotype...@gmail.com>> > Signed-off-by: Chad Versace <chad.vers...@linux.intel.com > <mailto:chad.vers...@linux.intel.com>> > --- > src/mesa/drivers/dri/intel/intel_context.c | 26 +++++++++---- > src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 52 > ++++++++++++++++++++++++++ > src/mesa/drivers/dri/intel/intel_mipmap_tree.h | 6 +++ > 3 files changed, 76 insertions(+), 8 deletions(-) > > diff --git a/src/mesa/drivers/dri/intel/intel_context.c > b/src/mesa/drivers/dri/intel/intel_context.c > index 378859c..9a85d83 100644 > --- a/src/mesa/drivers/dri/intel/intel_context.c > +++ b/src/mesa/drivers/dri/intel/intel_context.c > @@ -893,14 +893,24 @@ intel_process_dri2_buffer(struct intel_context > *intel, > if (!rb) > return; > > + unsigned num_samples = rb->Base.Base.NumSamples; > + > /* We try to avoid closing and reopening the same BO name, because > the first > * use of a mapping of the buffer involves a bunch of page faulting > which is > * moderately expensive. > */ > - if (rb->mt && > - rb->mt->region && > - rb->mt->region->name == buffer->name) > - return; > + if (num_samples == 0) { > + if (rb->mt && > + rb->mt->region && > + rb->mt->region->name == buffer->name) > + return; > + } else { > + if (rb->mt && > + rb->mt->singlesample_mt && > + rb->mt->singlesample_mt->region && > + rb->mt->singlesample_mt->region->name == buffer->name) > + return; > + } > > if (unlikely(INTEL_DEBUG & DEBUG_DRI)) { > fprintf(stderr, > @@ -920,9 +930,9 @@ intel_process_dri2_buffer(struct intel_context *intel, > if (!region) > return; > > - rb->mt = intel_miptree_create_for_region(intel, > - GL_TEXTURE_2D, > - intel_rb_format(rb), > - region); > + rb->mt = intel_miptree_create_for_dri2_buffer(intel, > + intel_rb_format(rb), > + num_samples, > + region); > intel_region_release(®ion); > } > diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c > b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c > index b402099..c4496ea 100644 > --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c > +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c > @@ -324,6 +324,58 @@ compute_msaa_layout(struct intel_context *intel, > gl_format format) > } > } > > +/** > + * If the DRI2 buffer is multisampled, then its content is undefined > + * after calling this. This behavior violates the GLX spec for the > + * benefit of avoiding a performance penalty. > + */ > +struct intel_mipmap_tree* > +intel_miptree_create_for_dri2_buffer(struct intel_context *intel, > + gl_format format, > + uint32_t num_samples, > + struct intel_region *region) > +{ > + struct intel_mipmap_tree *singlesample_mt = NULL; > + struct intel_mipmap_tree *multisample_mt = NULL; > + GLenum base_format = _mesa_get_format_base_format(format); > + > + /* Only the front and back buffers, which are color buffers, are > shared > + * through DRI2. > + */ > + assert(base_format == GL_RGB || base_format == GL_RGBA); > + > + singlesample_mt = intel_miptree_create_for_region(intel, > GL_TEXTURE_2D, > + format, region); > + if (!singlesample_mt) > + return NULL; > + > + if (num_samples == 0) { > + return singlesample_mt; > + } else { > + multisample_mt = intel_miptree_create_for_renderbuffer(intel, > + format, > + > region->width, > + > region->height, > + > num_samples); > + if (!multisample_mt) { > + intel_miptree_release(&singlesample_mt); > + return NULL; > + } > + > + multisample_mt->singlesample_mt = singlesample_mt; > + multisample_mt->need_downsample = false; > + > + /* If we wanted to preserve the contents of the DRI2 buffer, here > we > + * would need to do an upsample from singlesample_mt to > multisample_mt. > + * However, it is unlikely that any app desires that behavior. So > we > + * invalidate its content for the benefit of avoiding the upsample > + * performance penalty. > + */ > > > Is this function called after every buffer swap, or just on the first draw and > after a window resize? If it's called after every buffer swap, I agree with > you > that the performance penalty of upsampling isn't justified. But then I'm > worried about the performance penalty of intel_miptree_create_for_renderbuffer > (and especially intel_miptree_alloc_mcs, which it calls--that function spends > CPU time clearing the MCS buffer, which seems wasteful to do on every frame). > > If this function is only called for the first draw and after a resize, then it > might be better to go ahead and upsample, just so that the behaviour is more > consistent between multisampled and single-sampled visuals. It's called only on first draw and resize. After our discussion at lunch, I plan to upsample in intel_miptree_create_for_dri2_buffer if intel->is_front_buffer_rendering. I'll resubmit the patch series with that change. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
