While debugging the libdrm duplicate buffer object adventure, I managed to temporarily understand object lifetimes in the __DRIimage getBuffers path, and also to compare that to the DRI2 getBuffers path. Here are a couple of small fixes.
I haven't looked at the i915 code, but I suspect the first one, and parts of the second one would apply.
[PATCH 1/2] i965: Correct check for re-bound buffer in
The code was attempting to avoid re-creating the miptree when the loader handed back the same bufffer we already had, but it was confused about how to tell -- turns out the object actually shared between the loader and the driver is the BO, not the region. And so, we compare BO pointers to see if the image buffer needs to have a new miptree.
[PATCH 2/2] i965: Set fast color clear mcs_state on newly allocated
Here's a chunk of code that was just missing from the __DRIimage path as a result of rebasing across some new driver additions.
Both of these fixes are candidates for 10.0 as they affect the __DRIimage paths now used by Wayland.
-keith
The buffer-object is the persistent thing passed through the loader, so when updating an image buffer, check to see if it is already bound to the provided bo. The region, on the other hand, is allocated separately for the miptree, and so will never be the same as that passed back from the loader.
Signed-off-by: Keith Packard keithp@keithp.com --- src/mesa/drivers/dri/i965/brw_context.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index bee98e3..64ff855 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -1339,10 +1339,21 @@ intel_update_image_buffer(struct brw_context *intel,
unsigned num_samples = rb->Base.Base.NumSamples;
- if (rb->mt && - rb->mt->region && - rb->mt->region == region) - return; + /* Check and see if we're already bound to the right + * buffer object + */ + if (num_samples == 0) { + if (rb->mt && + rb->mt->region && + rb->mt->region->bo == region->bo) + return; + } else { + if (rb->mt && + rb->mt->singlesample_mt && + rb->mt->singlesample_mt->region && + rb->mt->singlesample_mt->region->bo == region->bo) + return; + }
intel_miptree_release(&rb->mt); rb->mt = intel_miptree_create_for_image_buffer(intel,
Just copying code from the dri2 path to set up the fast color clear state.
This also removes a couple of bogus intel_region_reference calls.
Signed-off-by: Keith Packard keithp@keithp.com --- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 884ddef..ca78f32 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c @@ -762,7 +762,13 @@ intel_miptree_create_for_image_buffer(struct brw_context *intel, if (!singlesample_mt) return NULL;
- intel_region_reference(&singlesample_mt->region, region); + /* If this miptree is capable of supporting fast color clears, set + * mcs_state appropriately to ensure that fast clears will occur. + * Allocation of the MCS miptree will be deferred until the first fast + * clear actually occurs. + */ + if (intel_is_non_msrt_mcs_buffer_supported(intel, singlesample_mt)) + singlesample_mt->mcs_state = INTEL_MCS_STATE_RESOLVED;
if (num_samples == 0) return singlesample_mt; @@ -780,8 +786,6 @@ intel_miptree_create_for_image_buffer(struct brw_context *intel, multisample_mt->singlesample_mt = singlesample_mt; multisample_mt->need_downsample = false;
- intel_region_reference(&multisample_mt->region, region); - if (intel->is_front_buffer_rendering && buffer_type == __DRI_IMAGE_BUFFER_FRONT) { intel_miptree_upsample(intel, multisample_mt); }
Keith Packard keithp@keithp.com writes:
Just copying code from the dri2 path to set up the fast color clear state.
This also removes a couple of bogus intel_region_reference calls.
These two are:
Reviewed-by: Eric Anholt eric@anholt.net
On 22 November 2013 05:52, Keith Packard keithp@keithp.com wrote:
While debugging the libdrm duplicate buffer object adventure, I managed to temporarily understand object lifetimes in the __DRIimage getBuffers path, and also to compare that to the DRI2 getBuffers path. Here are a couple of small fixes.
I haven't looked at the i915 code, but I suspect the first one, and parts of the second one would apply.
[PATCH 1/2] i965: Correct check for re-bound buffer in
The code was attempting to avoid re-creating the miptree when the loader handed back the same bufffer we already had, but it was confused about how to tell -- turns out the object actually shared between the loader and the driver is the BO, not the region. And so, we compare BO pointers to see if the image buffer needs to have a new miptree.
[PATCH 2/2] i965: Set fast color clear mcs_state on newly allocated
Here's a chunk of code that was just missing from the __DRIimage path as a result of rebasing across some new driver additions.
Both of these fixes are candidates for 10.0 as they affect the __DRIimage paths now used by Wayland.
For future reference, patches that are candidates for the latest stable branch should include the line
Cc: mesa-stable@lists.freedesktop.org
in the patch. (see http://lists.freedesktop.org/archives/mesa-dev/2013-August/042667.html)
I've forwarded this email to mesa-stable@lists.freedesktop.org, which should also be sufficient.
dri-devel@lists.freedesktop.org
-
Eric Anholt -
Keith Packard -
Paul Berry