Re: [Mesa-dev] Mesa commit a5e733c

2016-12-21 Thread Charmaine Lee

>From: Rob Clark 

>On Wed, Dec 21, 2016 at 12:32 PM, Charmaine Lee  wrote:
>>
>> Hi Rob,
>>
>> Your mesa commit a5e733c mesa: drop current draw/read buffer when ctx is 
>> released
>> is causing rendering issue when running with vmware svga driver.
>>
>> The problem is when the winsys draw/read buffers are unreferenced, the whole 
>> object
>> including the underlying resource can be prematurely destroyed. When the 
>> buffers are bound
>> to a context again, the whole object hierarchy is recreated but we already 
>> lost the previous
>> content, hence causing rendering corruption.

>It's possible perhaps that something else needs to be holding a
>reference to the corresponding EGL level object?  IIRC the EGL level
>object was freed but not mesa/st level object, leading to new EGL
>surface being allocated w/ same ptr address and confusing mesa/st.
>Which *definitely* seems wrong.

Dangling object is dangerous.

>> I believe the winsys draw/read buffers are purposely there to be not 
>> unreferenced at
>> context unbind time. There are timestamps in st_framebuffer and 
>> st_framebuffer_iface
>> to keep track of when the frame buffer needs to be re-validated. Could it be
>> somehow those stamps are out-of-sync in your case? Is there a better fix
>> to your original problem?

>*Maybe* but I don't totally understand how the timestamp thing is
>expected to work.  But it is already a couple months ago that I was
>debugging it and I won't really have the android setup again for a
>while.

>But somehow we either need to hold reference to the EGL surface while
>we have a ptr to it or drop the reference so we don't have a stale ptr
>that we can be confused by after a new EGL surface is created.

It will be ideal if st_framebuffer_iface object can hold a ptr to the associated
st_framebuffer object. So when an EGL surface is destroyed, it can follow the
chain to free the st objects as well. But since st_framebuffer object is 
context specific,
there can be multiple st_framebuffer objects for an st_framebuffer_iface object,
so currently the mapping of st_framebuffer to st_framebuffer_iface is kept in 
the
st_framebuffer object, causing it difficult to drop the reference when
st_framebuffer_iface for the EGL surface is deleted.
We can probably move the mapping to st_manager itself which already knows
the st_framebuffer_iface objects bound to a context, so when a
st_framebuffer_iface for an EGL surface is destroyed, we can notify
the st_manager to unreference the st_framebuffer_iface object from any of the 
st_framebuffer_iface to context mapping.

-Charmaine

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


Re: [Mesa-dev] Mesa commit a5e733c

2016-12-21 Thread Rob Clark
On Wed, Dec 21, 2016 at 12:32 PM, Charmaine Lee  wrote:
>
> Hi Rob,
>
> Your mesa commit a5e733c mesa: drop current draw/read buffer when ctx is 
> released
> is causing rendering issue when running with vmware svga driver.
>
> The problem is when the winsys draw/read buffers are unreferenced, the whole 
> object
> including the underlying resource can be prematurely destroyed. When the 
> buffers are bound
> to a context again, the whole object hierarchy is recreated but we already 
> lost the previous
> content, hence causing rendering corruption.

It's possible perhaps that something else needs to be holding a
reference to the corresponding EGL level object?  IIRC the EGL level
object was freed but not mesa/st level object, leading to new EGL
surface being allocated w/ same ptr address and confusing mesa/st.
Which *definitely* seems wrong.

> I believe the winsys draw/read buffers are purposely there to be not 
> unreferenced at
> context unbind time. There are timestamps in st_framebuffer and 
> st_framebuffer_iface
> to keep track of when the frame buffer needs to be re-validated. Could it be
> somehow those stamps are out-of-sync in your case? Is there a better fix
> to your original problem?

*Maybe* but I don't totally understand how the timestamp thing is
expected to work.  But it is already a couple months ago that I was
debugging it and I won't really have the android setup again for a
while.

But somehow we either need to hold reference to the EGL surface while
we have a ptr to it or drop the reference so we don't have a stale ptr
that we can be confused by after a new EGL surface is created.

BR,
-R

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