[PATCH 0/2] Minor __DRIimage fixes for i965

[PATCH 0/2] Minor __DRIimage fixes for i965

[Keith Packard]

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

[PATCH 1/2] i965: Correct check for re-bound buffer in intel_update_image_buffer

[Keith Packard]

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,
-- 
1.8.4.3

[PATCH 2/2] i965: Set fast color clear mcs_state on newly allocated image miptrees

[Keith Packard]

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);
    }
-- 
1.8.4.3

Re: [PATCH 2/2] i965: Set fast color clear mcs_state on newly allocated image miptrees

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 250 bytes --]

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>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 835 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

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

Re: [PATCH 0/2] Minor __DRIimage fixes for i965

[Paul Berry]

[-- Attachment #1.1: Type: text/plain, Size: 1504 bytes --]

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.

[-- Attachment #1.2: Type: text/html, Size: 2219 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

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