Re: [Mesa-dev] [PATCH 2/2] loader/dri3: Make sure we invalidate a drawable on size change

2017-10-17 Thread Michel Dänzer 
On 16/10/17 07:16 PM, Thomas Hellstrom wrote:
> On 10/16/2017 04:39 PM, Michel Dänzer wrote:
>> On 16/10/17 02:21 PM, Thomas Hellstrom wrote:
>>> On 10/16/2017 12:53 PM, Thomas Hellstrom wrote:
 Hi, Michel,

 On 10/16/2017 12:35 PM, Michel Dänzer wrote:
> Hi Thomas,
>
> On 05/09/17 10:15 AM, Thomas Hellstrom wrote:
>> If we're seeing a drawable size change, in particular after
>> processing a
>> configure notify event, make sure we invalidate so that the state
>> tracker
>> picks up the new geometry.
>>
>> Signed-off-by: Thomas Hellstrom 
>> ---
>>    src/loader/loader_dri3_helper.c | 2 ++
>>    1 file changed, 2 insertions(+)
>>
>> diff --git a/src/loader/loader_dri3_helper.c
>> b/src/loader/loader_dri3_helper.c
>> index 51e4e97..bcd5a66 100644
>> --- a/src/loader/loader_dri3_helper.c
>> +++ b/src/loader/loader_dri3_helper.c
>> @@ -348,6 +348,7 @@ dri3_handle_present_event(struct
>> loader_dri3_drawable *draw,
>>  draw->width = ce->width;
>>  draw->height = ce->height;
>>  draw->vtable->set_drawable_size(draw, draw->width,
>> draw->height);
>> + draw->ext->flush->invalidate(draw->dri_drawable);
>>  break;
>>   }
>>   case XCB_PRESENT_COMPLETE_NOTIFY: {
>> @@ -1592,6 +1593,7 @@ loader_dri3_update_drawable_geometry(struct
>> loader_dri3_drawable *draw)
>>  draw->width = geom_reply->width;
>>  draw->height = geom_reply->height;
>>  draw->vtable->set_drawable_size(draw, draw->width,
>> draw->height);
>> + draw->ext->flush->invalidate(draw->dri_drawable);
>>        free(geom_reply);
>>   }
>>
> unfortunately, I just bisected a regression to this commit. With
> it, the
> pipe_resource textures backing DRI3 back buffers are leaked when the
> window is resized:
>
> ==3408== 273,760 (228,464 direct, 45,296 indirect) bytes in 131
> blocks are definitely lost in loss record 1,285 of 1,286
> ==3408==    at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
> ==3408==    by 0xA04D5D3: r600_texture_create_object
> (r600_texture.c:1125)
> ==3408==    by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
> ==3408==    by 0x9CE3A17: dri2_allocate_textures (dri2.c:803)
> ==3408==    by 0x9CDDAE2: dri_st_framebuffer_validate
> (dri_drawable.c:85)
> ==3408==    by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
> ==3408==    by 0x9B6EE84: st_manager_validate_framebuffers
> (st_manager.c:1063)
> ==3408==    by 0x9B21807: st_validate_state (st_atom.c:202)
> ==3408==    by 0x9B2AF75: st_Clear (st_cb_clear.c:411)
> ==3408==    by 0x10A6BD: ??? (in /usr/bin/glxgears)
> ==3408==    by 0x109E37: ??? (in /usr/bin/glxgears)
> ==3408==    by 0x5E202E0: (below main) (libc-start.c:291)
> ==3408==
> ==3408== 362,768 (230,208 direct, 132,560 indirect) bytes in 132
> blocks are definitely lost in loss record 1,286 of 1,286
> ==3408==    at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
> ==3408==    by 0xA04D5D3: r600_texture_create_object
> (r600_texture.c:1125)
> ==3408==    by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
> ==3408==    by 0x9CE0C0D: dri2_create_image_common (dri2.c:1119)
> ==3408==    by 0x9CE0C6F: dri2_create_image (dri2.c:1140)
> ==3408==    by 0x5386BA8: dri3_alloc_render_buffer
> (loader_dri3_helper.c:1030)
> ==3408==    by 0x53876F4: dri3_get_buffer.isra.15
> (loader_dri3_helper.c:1364)
> ==3408==    by 0x53886DC: loader_dri3_get_buffers
> (loader_dri3_helper.c:1549)
> ==3408==    by 0x9CE298C: dri_image_drawable_get_buffers (dri2.c:452)
> ==3408==    by 0x9CE298C: dri2_allocate_textures (dri2.c:576)
> ==3408==    by 0x9CDDAE2: dri_st_framebuffer_validate
> (dri_drawable.c:85)
> ==3408==    by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
> ==3408==    by 0x9B6EE84: st_manager_validate_framebuffers
> (st_manager.c:1063)
>
>
> Can you reproduce this with vmwgfx as well? Any ideas why this is
> happening / how to fix it?
>
>
 Looks like other buffers (depth ?) are leaked too, outside dri3, right?

 I'll take a look.

 /Thomas


>>> Could you try the attached patch? Fixes the issue on my side.
>> Yes, it fixes the problem for me as well, thanks! I was looking at that
>> code as well, but didn't realize what's going on.
>>
>> I think the attached patch would be a cleaner solution though.
> 
> I agree. I had that in mind but had a bit too much on my plate today.
> 
> Perhaps we should not restrict the application of the patch patch with a
> Fixes: tag, though:
> Although improbable, the bug could be hit also without that dri3 patch.

Fair enough, replaced with a generic stable tag in v2.


> Also do we need to do a os_calloc()

Re: [Mesa-dev] [PATCH 2/2] loader/dri3: Make sure we invalidate a drawable on size change

2017-10-16 Thread Thomas Hellstrom 

On 10/16/2017 04:39 PM, Michel Dänzer wrote:

On 16/10/17 02:21 PM, Thomas Hellstrom wrote:

On 10/16/2017 12:53 PM, Thomas Hellstrom wrote:

Hi, Michel,

On 10/16/2017 12:35 PM, Michel Dänzer wrote:

Hi Thomas,

On 05/09/17 10:15 AM, Thomas Hellstrom wrote:

If we're seeing a drawable size change, in particular after
processing a
configure notify event, make sure we invalidate so that the state
tracker
picks up the new geometry.

Signed-off-by: Thomas Hellstrom 
---
   src/loader/loader_dri3_helper.c | 2 ++
   1 file changed, 2 insertions(+)

diff --git a/src/loader/loader_dri3_helper.c
b/src/loader/loader_dri3_helper.c
index 51e4e97..bcd5a66 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -348,6 +348,7 @@ dri3_handle_present_event(struct
loader_dri3_drawable *draw,
     draw->width = ce->width;
     draw->height = ce->height;
     draw->vtable->set_drawable_size(draw, draw->width,
draw->height);
+ draw->ext->flush->invalidate(draw->dri_drawable);
     break;
  }
  case XCB_PRESENT_COMPLETE_NOTIFY: {
@@ -1592,6 +1593,7 @@ loader_dri3_update_drawable_geometry(struct
loader_dri3_drawable *draw)
     draw->width = geom_reply->width;
     draw->height = geom_reply->height;
     draw->vtable->set_drawable_size(draw, draw->width,
draw->height);
+ draw->ext->flush->invalidate(draw->dri_drawable);
       free(geom_reply);
  }


unfortunately, I just bisected a regression to this commit. With it, the
pipe_resource textures backing DRI3 back buffers are leaked when the
window is resized:

==3408== 273,760 (228,464 direct, 45,296 indirect) bytes in 131
blocks are definitely lost in loss record 1,285 of 1,286
==3408==    at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
==3408==    by 0xA04D5D3: r600_texture_create_object
(r600_texture.c:1125)
==3408==    by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
==3408==    by 0x9CE3A17: dri2_allocate_textures (dri2.c:803)
==3408==    by 0x9CDDAE2: dri_st_framebuffer_validate
(dri_drawable.c:85)
==3408==    by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
==3408==    by 0x9B6EE84: st_manager_validate_framebuffers
(st_manager.c:1063)
==3408==    by 0x9B21807: st_validate_state (st_atom.c:202)
==3408==    by 0x9B2AF75: st_Clear (st_cb_clear.c:411)
==3408==    by 0x10A6BD: ??? (in /usr/bin/glxgears)
==3408==    by 0x109E37: ??? (in /usr/bin/glxgears)
==3408==    by 0x5E202E0: (below main) (libc-start.c:291)
==3408==
==3408== 362,768 (230,208 direct, 132,560 indirect) bytes in 132
blocks are definitely lost in loss record 1,286 of 1,286
==3408==    at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
==3408==    by 0xA04D5D3: r600_texture_create_object
(r600_texture.c:1125)
==3408==    by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
==3408==    by 0x9CE0C0D: dri2_create_image_common (dri2.c:1119)
==3408==    by 0x9CE0C6F: dri2_create_image (dri2.c:1140)
==3408==    by 0x5386BA8: dri3_alloc_render_buffer
(loader_dri3_helper.c:1030)
==3408==    by 0x53876F4: dri3_get_buffer.isra.15
(loader_dri3_helper.c:1364)
==3408==    by 0x53886DC: loader_dri3_get_buffers
(loader_dri3_helper.c:1549)
==3408==    by 0x9CE298C: dri_image_drawable_get_buffers (dri2.c:452)
==3408==    by 0x9CE298C: dri2_allocate_textures (dri2.c:576)
==3408==    by 0x9CDDAE2: dri_st_framebuffer_validate
(dri_drawable.c:85)
==3408==    by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
==3408==    by 0x9B6EE84: st_manager_validate_framebuffers
(st_manager.c:1063)


Can you reproduce this with vmwgfx as well? Any ideas why this is
happening / how to fix it?



Looks like other buffers (depth ?) are leaked too, outside dri3, right?

I'll take a look.

/Thomas



Could you try the attached patch? Fixes the issue on my side.

Yes, it fixes the problem for me as well, thanks! I was looking at that
code as well, but didn't realize what's going on.

I think the attached patch would be a cleaner solution though.


I agree. I had that in mind but had a bit too much on my plate today.

Perhaps we should not restrict the application of the patch patch with a 
Fixes: tag, though:

Although improbable, the bug could be hit also without that dri3 patch.

Also do we need to do a os_calloc() of the texture pointer array in 
st_framebuffer_validate(),

cant we just memset() the fixed size array?  Your decision.

Reviewed-by:
Thomas Hellstrom 

Thanks,
Thomas



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] loader/dri3: Make sure we invalidate a drawable on size change

2017-10-16 Thread Michel Dänzer 
On 16/10/17 02:21 PM, Thomas Hellstrom wrote:
> 
> On 10/16/2017 12:53 PM, Thomas Hellstrom wrote:
>> Hi, Michel,
>>
>> On 10/16/2017 12:35 PM, Michel Dänzer wrote:
>>> Hi Thomas,
>>>
>>> On 05/09/17 10:15 AM, Thomas Hellstrom wrote:
 If we're seeing a drawable size change, in particular after
 processing a
 configure notify event, make sure we invalidate so that the state
 tracker
 picks up the new geometry.

 Signed-off-by: Thomas Hellstrom 
 ---
   src/loader/loader_dri3_helper.c | 2 ++
   1 file changed, 2 insertions(+)

 diff --git a/src/loader/loader_dri3_helper.c
 b/src/loader/loader_dri3_helper.c
 index 51e4e97..bcd5a66 100644
 --- a/src/loader/loader_dri3_helper.c
 +++ b/src/loader/loader_dri3_helper.c
 @@ -348,6 +348,7 @@ dri3_handle_present_event(struct
 loader_dri3_drawable *draw,
     draw->width = ce->width;
     draw->height = ce->height;
     draw->vtable->set_drawable_size(draw, draw->width,
 draw->height);
 + draw->ext->flush->invalidate(draw->dri_drawable);
     break;
  }
  case XCB_PRESENT_COMPLETE_NOTIFY: {
 @@ -1592,6 +1593,7 @@ loader_dri3_update_drawable_geometry(struct
 loader_dri3_drawable *draw)
     draw->width = geom_reply->width;
     draw->height = geom_reply->height;
     draw->vtable->set_drawable_size(draw, draw->width,
 draw->height);
 + draw->ext->flush->invalidate(draw->dri_drawable);
       free(geom_reply);
  }

>>> unfortunately, I just bisected a regression to this commit. With it, the
>>> pipe_resource textures backing DRI3 back buffers are leaked when the
>>> window is resized:
>>>
>>> ==3408== 273,760 (228,464 direct, 45,296 indirect) bytes in 131
>>> blocks are definitely lost in loss record 1,285 of 1,286
>>> ==3408==    at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
>>> ==3408==    by 0xA04D5D3: r600_texture_create_object
>>> (r600_texture.c:1125)
>>> ==3408==    by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
>>> ==3408==    by 0x9CE3A17: dri2_allocate_textures (dri2.c:803)
>>> ==3408==    by 0x9CDDAE2: dri_st_framebuffer_validate
>>> (dri_drawable.c:85)
>>> ==3408==    by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
>>> ==3408==    by 0x9B6EE84: st_manager_validate_framebuffers
>>> (st_manager.c:1063)
>>> ==3408==    by 0x9B21807: st_validate_state (st_atom.c:202)
>>> ==3408==    by 0x9B2AF75: st_Clear (st_cb_clear.c:411)
>>> ==3408==    by 0x10A6BD: ??? (in /usr/bin/glxgears)
>>> ==3408==    by 0x109E37: ??? (in /usr/bin/glxgears)
>>> ==3408==    by 0x5E202E0: (below main) (libc-start.c:291)
>>> ==3408==
>>> ==3408== 362,768 (230,208 direct, 132,560 indirect) bytes in 132
>>> blocks are definitely lost in loss record 1,286 of 1,286
>>> ==3408==    at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
>>> ==3408==    by 0xA04D5D3: r600_texture_create_object
>>> (r600_texture.c:1125)
>>> ==3408==    by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
>>> ==3408==    by 0x9CE0C0D: dri2_create_image_common (dri2.c:1119)
>>> ==3408==    by 0x9CE0C6F: dri2_create_image (dri2.c:1140)
>>> ==3408==    by 0x5386BA8: dri3_alloc_render_buffer
>>> (loader_dri3_helper.c:1030)
>>> ==3408==    by 0x53876F4: dri3_get_buffer.isra.15
>>> (loader_dri3_helper.c:1364)
>>> ==3408==    by 0x53886DC: loader_dri3_get_buffers
>>> (loader_dri3_helper.c:1549)
>>> ==3408==    by 0x9CE298C: dri_image_drawable_get_buffers (dri2.c:452)
>>> ==3408==    by 0x9CE298C: dri2_allocate_textures (dri2.c:576)
>>> ==3408==    by 0x9CDDAE2: dri_st_framebuffer_validate
>>> (dri_drawable.c:85)
>>> ==3408==    by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
>>> ==3408==    by 0x9B6EE84: st_manager_validate_framebuffers
>>> (st_manager.c:1063)
>>>
>>>
>>> Can you reproduce this with vmwgfx as well? Any ideas why this is
>>> happening / how to fix it?
>>>
>>>
>> Looks like other buffers (depth ?) are leaked too, outside dri3, right?
>>
>> I'll take a look.
>>
>> /Thomas
>>
>>
> Could you try the attached patch? Fixes the issue on my side.

Yes, it fixes the problem for me as well, thanks! I was looking at that
code as well, but didn't realize what's going on.

I think the attached patch would be a cleaner solution though.

Some comments on your patch, assuming we end up going with that:


> +  /* If we need to rerun the validation, make sure we unreference the
> +   * textures first. The validate function will just clear the pointers.
> +   */
> +
> +  if (stfb->iface_stamp != new_stamp) {
> + for (i = 0; i < stfb->num_statts; ++i) {
> +pipe_resource_reference([i], NULL);
> + }
> +  }

I'd drop the blank line after the comment and the curly braces around
the inner loop, but either way, with

Fixes: e96d175c7d2e "loader/dri3: Make sure we invalidate a drawable on
 size change"

added to the commit log,

Re: [Mesa-dev] [PATCH 2/2] loader/dri3: Make sure we invalidate a drawable on size change

2017-10-16 Thread Thomas Hellstrom 

Hi, Michel,

On 10/16/2017 12:35 PM, Michel Dänzer wrote:

Hi Thomas,

On 05/09/17 10:15 AM, Thomas Hellstrom wrote:

If we're seeing a drawable size change, in particular after processing a
configure notify event, make sure we invalidate so that the state tracker
picks up the new geometry.

Signed-off-by: Thomas Hellstrom 
---
  src/loader/loader_dri3_helper.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
index 51e4e97..bcd5a66 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -348,6 +348,7 @@ dri3_handle_present_event(struct loader_dri3_drawable *draw,
draw->width = ce->width;
draw->height = ce->height;
draw->vtable->set_drawable_size(draw, draw->width, draw->height);
+  draw->ext->flush->invalidate(draw->dri_drawable);
break;
 }
 case XCB_PRESENT_COMPLETE_NOTIFY: {
@@ -1592,6 +1593,7 @@ loader_dri3_update_drawable_geometry(struct 
loader_dri3_drawable *draw)
draw->width = geom_reply->width;
draw->height = geom_reply->height;
draw->vtable->set_drawable_size(draw, draw->width, draw->height);
+  draw->ext->flush->invalidate(draw->dri_drawable);
  
free(geom_reply);

 }


unfortunately, I just bisected a regression to this commit. With it, the
pipe_resource textures backing DRI3 back buffers are leaked when the
window is resized:

==3408== 273,760 (228,464 direct, 45,296 indirect) bytes in 131 blocks are 
definitely lost in loss record 1,285 of 1,286
==3408==at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
==3408==by 0xA04D5D3: r600_texture_create_object (r600_texture.c:1125)
==3408==by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
==3408==by 0x9CE3A17: dri2_allocate_textures (dri2.c:803)
==3408==by 0x9CDDAE2: dri_st_framebuffer_validate (dri_drawable.c:85)
==3408==by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
==3408==by 0x9B6EE84: st_manager_validate_framebuffers (st_manager.c:1063)
==3408==by 0x9B21807: st_validate_state (st_atom.c:202)
==3408==by 0x9B2AF75: st_Clear (st_cb_clear.c:411)
==3408==by 0x10A6BD: ??? (in /usr/bin/glxgears)
==3408==by 0x109E37: ??? (in /usr/bin/glxgears)
==3408==by 0x5E202E0: (below main) (libc-start.c:291)
==3408==
==3408== 362,768 (230,208 direct, 132,560 indirect) bytes in 132 blocks are 
definitely lost in loss record 1,286 of 1,286
==3408==at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
==3408==by 0xA04D5D3: r600_texture_create_object (r600_texture.c:1125)
==3408==by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
==3408==by 0x9CE0C0D: dri2_create_image_common (dri2.c:1119)
==3408==by 0x9CE0C6F: dri2_create_image (dri2.c:1140)
==3408==by 0x5386BA8: dri3_alloc_render_buffer (loader_dri3_helper.c:1030)
==3408==by 0x53876F4: dri3_get_buffer.isra.15 (loader_dri3_helper.c:1364)
==3408==by 0x53886DC: loader_dri3_get_buffers (loader_dri3_helper.c:1549)
==3408==by 0x9CE298C: dri_image_drawable_get_buffers (dri2.c:452)
==3408==by 0x9CE298C: dri2_allocate_textures (dri2.c:576)
==3408==by 0x9CDDAE2: dri_st_framebuffer_validate (dri_drawable.c:85)
==3408==by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
==3408==by 0x9B6EE84: st_manager_validate_framebuffers (st_manager.c:1063)


Can you reproduce this with vmwgfx as well? Any ideas why this is
happening / how to fix it?



Looks like other buffers (depth ?) are leaked too, outside dri3, right?

I'll take a look.

/Thomas


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] loader/dri3: Make sure we invalidate a drawable on size change

2017-10-16 Thread Thomas Hellstrom 


On 10/16/2017 12:53 PM, Thomas Hellstrom wrote:

Hi, Michel,

On 10/16/2017 12:35 PM, Michel Dänzer wrote:

Hi Thomas,

On 05/09/17 10:15 AM, Thomas Hellstrom wrote:
If we're seeing a drawable size change, in particular after 
processing a
configure notify event, make sure we invalidate so that the state 
tracker

picks up the new geometry.

Signed-off-by: Thomas Hellstrom 
---
  src/loader/loader_dri3_helper.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/src/loader/loader_dri3_helper.c 
b/src/loader/loader_dri3_helper.c

index 51e4e97..bcd5a66 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -348,6 +348,7 @@ dri3_handle_present_event(struct 
loader_dri3_drawable *draw,

    draw->width = ce->width;
    draw->height = ce->height;
    draw->vtable->set_drawable_size(draw, draw->width, 
draw->height);

+ draw->ext->flush->invalidate(draw->dri_drawable);
    break;
 }
 case XCB_PRESENT_COMPLETE_NOTIFY: {
@@ -1592,6 +1593,7 @@ loader_dri3_update_drawable_geometry(struct 
loader_dri3_drawable *draw)

    draw->width = geom_reply->width;
    draw->height = geom_reply->height;
    draw->vtable->set_drawable_size(draw, draw->width, 
draw->height);

+ draw->ext->flush->invalidate(draw->dri_drawable);
      free(geom_reply);
 }


unfortunately, I just bisected a regression to this commit. With it, the
pipe_resource textures backing DRI3 back buffers are leaked when the
window is resized:

==3408== 273,760 (228,464 direct, 45,296 indirect) bytes in 131 
blocks are definitely lost in loss record 1,285 of 1,286

==3408==    at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
==3408==    by 0xA04D5D3: r600_texture_create_object 
(r600_texture.c:1125)

==3408==    by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
==3408==    by 0x9CE3A17: dri2_allocate_textures (dri2.c:803)
==3408==    by 0x9CDDAE2: dri_st_framebuffer_validate 
(dri_drawable.c:85)

==3408==    by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
==3408==    by 0x9B6EE84: st_manager_validate_framebuffers 
(st_manager.c:1063)

==3408==    by 0x9B21807: st_validate_state (st_atom.c:202)
==3408==    by 0x9B2AF75: st_Clear (st_cb_clear.c:411)
==3408==    by 0x10A6BD: ??? (in /usr/bin/glxgears)
==3408==    by 0x109E37: ??? (in /usr/bin/glxgears)
==3408==    by 0x5E202E0: (below main) (libc-start.c:291)
==3408==
==3408== 362,768 (230,208 direct, 132,560 indirect) bytes in 132 
blocks are definitely lost in loss record 1,286 of 1,286

==3408==    at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
==3408==    by 0xA04D5D3: r600_texture_create_object 
(r600_texture.c:1125)

==3408==    by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
==3408==    by 0x9CE0C0D: dri2_create_image_common (dri2.c:1119)
==3408==    by 0x9CE0C6F: dri2_create_image (dri2.c:1140)
==3408==    by 0x5386BA8: dri3_alloc_render_buffer 
(loader_dri3_helper.c:1030)
==3408==    by 0x53876F4: dri3_get_buffer.isra.15 
(loader_dri3_helper.c:1364)
==3408==    by 0x53886DC: loader_dri3_get_buffers 
(loader_dri3_helper.c:1549)

==3408==    by 0x9CE298C: dri_image_drawable_get_buffers (dri2.c:452)
==3408==    by 0x9CE298C: dri2_allocate_textures (dri2.c:576)
==3408==    by 0x9CDDAE2: dri_st_framebuffer_validate 
(dri_drawable.c:85)

==3408==    by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
==3408==    by 0x9B6EE84: st_manager_validate_framebuffers 
(st_manager.c:1063)



Can you reproduce this with vmwgfx as well? Any ideas why this is
happening / how to fix it?



Looks like other buffers (depth ?) are leaked too, outside dri3, right?

I'll take a look.

/Thomas



Could you try the attached patch? Fixes the issue on my side.

/Thomas


>From db02c2b57d84f0ce5a4292204f3cfdfa3547be8d Mon Sep 17 00:00:00 2001
From: Thomas Hellstrom 
Date: Mon, 16 Oct 2017 14:13:37 +0200
Subject: [PATCH] st/mesa: Fix a texture reference leak on revalidation

If the state tracker needed to rerun the validation due to a stamp update
while validating, the references from the previous validation iterations were
never unreferenced.

Signed-off-by: Thomas Hellstrom 
---
 src/mesa/state_tracker/st_manager.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c
index 50bc3c3..52492fe 100644
--- a/src/mesa/state_tracker/st_manager.c
+++ b/src/mesa/state_tracker/st_manager.c
@@ -198,6 +198,16 @@ st_framebuffer_validate(struct st_framebuffer *stfb,
 
   stfb->iface_stamp = new_stamp;
   new_stamp = p_atomic_read(>iface->stamp);
+
+  /* If we need to rerun the validation, make sure we unreference the
+   * textures first. The validate function will just clear the pointers.
+   */
+
+  if (stfb->iface_stamp != new_stamp) {
+ for (i = 0; i < stfb->num_statts; ++i) {
+pipe_resource_reference([i], NULL);
+ }
+  }
}

Re: [Mesa-dev] [PATCH 2/2] loader/dri3: Make sure we invalidate a drawable on size change

2017-10-16 Thread Michel Dänzer 

Hi Thomas,

On 05/09/17 10:15 AM, Thomas Hellstrom wrote:
> If we're seeing a drawable size change, in particular after processing a
> configure notify event, make sure we invalidate so that the state tracker
> picks up the new geometry.
> 
> Signed-off-by: Thomas Hellstrom 
> ---
>  src/loader/loader_dri3_helper.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
> index 51e4e97..bcd5a66 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
> @@ -348,6 +348,7 @@ dri3_handle_present_event(struct loader_dri3_drawable 
> *draw,
>draw->width = ce->width;
>draw->height = ce->height;
>draw->vtable->set_drawable_size(draw, draw->width, draw->height);
> +  draw->ext->flush->invalidate(draw->dri_drawable);
>break;
> }
> case XCB_PRESENT_COMPLETE_NOTIFY: {
> @@ -1592,6 +1593,7 @@ loader_dri3_update_drawable_geometry(struct 
> loader_dri3_drawable *draw)
>draw->width = geom_reply->width;
>draw->height = geom_reply->height;
>draw->vtable->set_drawable_size(draw, draw->width, draw->height);
> +  draw->ext->flush->invalidate(draw->dri_drawable);
>  
>free(geom_reply);
> }
> 

unfortunately, I just bisected a regression to this commit. With it, the
pipe_resource textures backing DRI3 back buffers are leaked when the
window is resized:

==3408== 273,760 (228,464 direct, 45,296 indirect) bytes in 131 blocks are 
definitely lost in loss record 1,285 of 1,286
==3408==at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
==3408==by 0xA04D5D3: r600_texture_create_object (r600_texture.c:1125)
==3408==by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
==3408==by 0x9CE3A17: dri2_allocate_textures (dri2.c:803)
==3408==by 0x9CDDAE2: dri_st_framebuffer_validate (dri_drawable.c:85)
==3408==by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
==3408==by 0x9B6EE84: st_manager_validate_framebuffers (st_manager.c:1063)
==3408==by 0x9B21807: st_validate_state (st_atom.c:202)
==3408==by 0x9B2AF75: st_Clear (st_cb_clear.c:411)
==3408==by 0x10A6BD: ??? (in /usr/bin/glxgears)
==3408==by 0x109E37: ??? (in /usr/bin/glxgears)
==3408==by 0x5E202E0: (below main) (libc-start.c:291)
==3408==
==3408== 362,768 (230,208 direct, 132,560 indirect) bytes in 132 blocks are 
definitely lost in loss record 1,286 of 1,286
==3408==at 0x4C2DC05: calloc (vg_replace_malloc.c:711)
==3408==by 0xA04D5D3: r600_texture_create_object (r600_texture.c:1125)
==3408==by 0xA04E1C1: si_texture_create (r600_texture.c:1382)
==3408==by 0x9CE0C0D: dri2_create_image_common (dri2.c:1119)
==3408==by 0x9CE0C6F: dri2_create_image (dri2.c:1140)
==3408==by 0x5386BA8: dri3_alloc_render_buffer (loader_dri3_helper.c:1030)
==3408==by 0x53876F4: dri3_get_buffer.isra.15 (loader_dri3_helper.c:1364)
==3408==by 0x53886DC: loader_dri3_get_buffers (loader_dri3_helper.c:1549)
==3408==by 0x9CE298C: dri_image_drawable_get_buffers (dri2.c:452)
==3408==by 0x9CE298C: dri2_allocate_textures (dri2.c:576)
==3408==by 0x9CDDAE2: dri_st_framebuffer_validate (dri_drawable.c:85)
==3408==by 0x9B6D07F: st_framebuffer_validate (st_manager.c:195)
==3408==by 0x9B6EE84: st_manager_validate_framebuffers (st_manager.c:1063)


Can you reproduce this with vmwgfx as well? Any ideas why this is
happening / how to fix it?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] loader/dri3: Make sure we invalidate a drawable on size change

2017-09-05 Thread Thomas Hellstrom 
If we're seeing a drawable size change, in particular after processing a
configure notify event, make sure we invalidate so that the state tracker
picks up the new geometry.

Signed-off-by: Thomas Hellstrom 
---
 src/loader/loader_dri3_helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
index 51e4e97..bcd5a66 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -348,6 +348,7 @@ dri3_handle_present_event(struct loader_dri3_drawable *draw,
   draw->width = ce->width;
   draw->height = ce->height;
   draw->vtable->set_drawable_size(draw, draw->width, draw->height);
+  draw->ext->flush->invalidate(draw->dri_drawable);
   break;
}
case XCB_PRESENT_COMPLETE_NOTIFY: {
@@ -1592,6 +1593,7 @@ loader_dri3_update_drawable_geometry(struct 
loader_dri3_drawable *draw)
   draw->width = geom_reply->width;
   draw->height = geom_reply->height;
   draw->vtable->set_drawable_size(draw, draw->width, draw->height);
+  draw->ext->flush->invalidate(draw->dri_drawable);
 
   free(geom_reply);
}
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev