Hey Tomasz,

On 02/16/2018 05:10 AM, Tomasz Figa wrote:
On Fri, Feb 9, 2018 at 11:06 PM, Rob Herring <r...@kernel.org> wrote:
On Fri, Feb 9, 2018 at 3:58 AM, Tomasz Figa <tf...@chromium.org> wrote:
On Fri, Feb 2, 2018 at 11:51 PM, Tomasz Figa <tf...@chromium.org> wrote:
On Fri, Feb 2, 2018 at 11:00 PM, Rob Herring <r...@kernel.org> wrote:
On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa <tf...@chromium.org> wrote:
Hi Rob,

On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss <robert.f...@collabora.com> wrote:
       uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
       uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t
plane);
       uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
       uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
       ...
} gralloc_funcs_t;


These ones? >
Yeah, if we could retrieve such function pointer struct using perform
or any equivalent (like the implementation-specific methods in
gralloc1, but not sure if that's going to be used in practice
anywhere), it could work for us.


So this is where you and Rob Herring lose me, I don't think I understand
quite how the gralloc1 call would be used, and how it would tie into this
handle struct. I think I could do with some guidance on this.

This would be very similar to gralloc0 perform call. gralloc1
implementations need to provide getFunction() callback [1], which
returns a pointer to given function. The list of standard functions is
defined in the gralloc1.h header [2], but we could take some random
big number and use it for our function that fills in provided
gralloc_funcs_t struct with necessary pointers.

[1] 
https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
[2] 
https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134

This is a deadend because it won't work with a HIDL based
implementation (aka gralloc 2.0). You can't set function pointers (or
any pointers) because gralloc runs in a different process. Yes,
currently gralloc is a pass-thru HAL, but AIUI that will go away.

Part of it. I can't see IMapper being implemented by a separate
process. You can't map a buffer into one process from another process.

But anyway, it's a good point, thanks, I almost forgot about its
existence. I'll do further investigation.

Okay, so IMapper indeed breaks the approach I suggested. I'm not sure
at the moment what we could do about it. (The idea of a dynamic
library of a pre-defined name, exporting functions we specify, might
still work, though.)

Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be
impossible to implement with IAllocator/IMapper. (Although I still
think Mesa and Gralloc are free to have separate logic for choosing
the DRM device to use.)

I think the need for GET_FD goes away when the render node is used. We
may still need the card node for s/w rendering (if I can ever get that
working) though. Of course, if we use the vgem approach like CrOS then
we wouldn't.

Hmm, if so, then we probably wouldn't have any strict need for these
function pointers anymore. We already have a makeshift format resolve
in place and the only missing bits that we still need to patch up
downstream are removing GET_FD, dropping drm_gralloc.h and adding a
fallback to kms_swrast if hw driver loading fails.

So this discussion is slightly unrelated, but it is where me looking at this started.

So I've got a kms_swrast fallback series[1], that I've been wanting to test before trying to push upstream, but haven't been able to run it in the Android environment and the arc++ + chromiumos has also been problematic for various reasons (which are being looked into).

Tomasz: If you're interested in testing the series, it would be helpful. Hopefully testing is everything that needed for upstreaming.

[1] https://gitlab.collabora.com/robertfoss/mesa/commits/kms_swrast
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to