[Mesa-dev] [android-x86-devel] Re: need-help: how to change to newest mesa in android-x86?

[Rob Clark]

On Mon, Jan 18, 2016 at 5:18 PM, Rob Herring <robherring2 at gmail.com> wrote:
> On Fri, Jan 15, 2016 at 3:10 PM, Dave Airlie <airlied at gmail.com> wrote:
>>>
>>> well, nothing specific, but for example early on we had some confusion
>>> in drm_gralloc (when adding dmabuf fd support) about who close()d the
>>> fd's.  Resulting in same fd getting closed twice (although some
>>> completely unrelated file handle might have snuck into that slot
>>> between the two close()s).
>>>
>>> Or you could end up w/ the same sort of thing for gem handles (which
>>> are unique to the drm device fd, so if you open() the device multiple
>>> times..).  I did initially have some confusion about this, w/ multiple
>>> fd_bo's (or pipe_resource's) being created for a given gem handle, and
>>> first one that gets deleted takes away the backing store that both
>>> userspace references where using.
>>>
>>> You might want to look at freedreno_drm_winsys.c and the crazy things
>>> it does..  I suspect virgl might need similar hacks.  That is really
>>> one part of two, that ensures that we share the same pipe_screen, and
>>> therefore (from libdrm) fd_device, for a given drm device fd.  The
>>> other part is that fd_device has hashtables to deal w/ double import
>>> of gem buffers.
>>
>> I think Rob Clark might be on the right track here.
>>
>> it sounds like the host is sending double deletes for some objects, which
>> sounds like a possible reference counting bug in the host.
>>
>> So possibly something drm_gralloc, but it could be a bug in mesa, though
>> I haven't triggered anything like this yet.
>
> Some more details. If I comment out the freeing of imported buffers in
> gralloc_drm_handle_unregister, things work a lot better. I think there
> is a missing reference taken on an alloc with an existing dmabuf FD.
> I've been comparing freedreno vs. pipe alloc/free implementations for
> gralloc. The freedreno alloc function will call drmPrimeFDToHandle and
> increment the BO ref count (in lookup_bo). The equivalent for virgl is
> in virgl_drm_winsys_resource_create_handle. It calls
> drmPrimeFDToHandle, but there is no reference taken. The shared handle
> case in that function does take a reference with a
> virgl_drm_resource_reference call. I may be off as my attempted fix
> doesn't work...

Hmm, so I guess I should have removed gralloc_drm_freedreno, but by
the time I actually got things working properly on ifc6410 I was using
gralloc_drm_pipe.c..  sorry if you had been looking at the wrong code.

I originally abandoned gralloc_drm_freedreno once I realized that for
the refcnt'ing to work out I needed gralloc to share the same
fd_device ptr as mesa/gl.  And by using gralloc_drm_pipe (plus that
freedreno winsys code I pointed to earlier) both gralloc and the gl
context share the same pipe_screen (and fd_device).

(And fwiw, I think we should drop nouveau and radeon backend in
gralloc_drm as well, and use gralloc_drm_pipe for all of the gallium
drivers..)

BR,
-R