Re: [Mesa-dev] [PATCH 0/5] Cleanup dead code
This patch is Reviewed-by: Robert Foss On 2/9/19 2:21 AM, Alyssa Rosenzweig wrote: Via the prototyping cycle, a substantial amount of dead code and other cruft has accumulated in the Panfrost tree, which is a burden for both maintenance and aesthetic. This patch series removes dead code when it's trivial to do so, or refactors in order to remove dead code when nontrivial to do so. After these patches, all "#if 0" code is removed except for the wallpapering routine which will be another series in and of itself ;) Alyssa Rosenzweig (5): panfrost: Remove if 0'd dead code panfrost: Specify supported draw modes per-context panfrost: Overhaul viewport code; fix scissor test panfrost: Remove speculative if 0'd format bit code panfrost: Elucidate texture op scheduling comment .../drivers/panfrost/midgard/cmdline.c| 11 -- .../panfrost/midgard/midgard_compile.c| 9 +- src/gallium/drivers/panfrost/pan_assemble.c | 14 -- .../drivers/panfrost/pan_blend_shaders.c | 16 --- src/gallium/drivers/panfrost/pan_context.c| 129 +- src/gallium/drivers/panfrost/pan_context.h| 3 + src/gallium/drivers/panfrost/pan_swizzle.c| 37 - 7 files changed, 69 insertions(+), 150 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/5] Cleanup dead code
On 2/10/19 6:58 PM, Robert Foss wrote: This patch is s/patch/series Reviewed-by: Robert Foss On 2/9/19 2:21 AM, Alyssa Rosenzweig wrote: Via the prototyping cycle, a substantial amount of dead code and other cruft has accumulated in the Panfrost tree, which is a burden for both maintenance and aesthetic. This patch series removes dead code when it's trivial to do so, or refactors in order to remove dead code when nontrivial to do so. After these patches, all "#if 0" code is removed except for the wallpapering routine which will be another series in and of itself ;) Alyssa Rosenzweig (5): panfrost: Remove if 0'd dead code panfrost: Specify supported draw modes per-context panfrost: Overhaul viewport code; fix scissor test panfrost: Remove speculative if 0'd format bit code panfrost: Elucidate texture op scheduling comment .../drivers/panfrost/midgard/cmdline.c | 11 -- .../panfrost/midgard/midgard_compile.c | 9 +- src/gallium/drivers/panfrost/pan_assemble.c | 14 -- .../drivers/panfrost/pan_blend_shaders.c | 16 --- src/gallium/drivers/panfrost/pan_context.c | 129 +- src/gallium/drivers/panfrost/pan_context.h | 3 + src/gallium/drivers/panfrost/pan_swizzle.c | 37 - 7 files changed, 69 insertions(+), 150 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] virgl: Don't try handling server fences when they are not supported
This patch has been: Reviewed-by: Robert Foss On 2018-11-27 20:50, Gert Wollny wrote: From: Gert Wollny vtest doesn't implement the according API and would segfault: Program received signal SIGSEGV, Segmentation fault. #0 0x in ?? () #1 in virgl_fence_server_sync at src/gallium/drivers/virgl/virgl_context.c:1049 #2 in st_server_wait_sync at src/mesa/state_tracker/st_cb_syncobj.c:155 so just don't do the call when the function pointers are not set. Fixes dEQP: dEQP-GLES3.functional.fence_sync.wait_sync_smalldraw dEQP-GLES3.functional.fence_sync.wait_sync_largedraw Fixes: d1a1c21e7621b5177febf191fcd3d3b8ef69dc96 virgl: native fence fd support Signed-off-by: Gert Wollny --- src/gallium/drivers/virgl/virgl_context.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/virgl/virgl_context.c b/src/gallium/drivers/virgl/virgl_context.c index 892fef76c7..f0ee64c145 100644 --- a/src/gallium/drivers/virgl/virgl_context.c +++ b/src/gallium/drivers/virgl/virgl_context.c @@ -1037,7 +1037,8 @@ static void virgl_create_fence_fd(struct pipe_context *ctx, assert(type == PIPE_FD_TYPE_NATIVE_SYNC); struct virgl_screen *rs = virgl_screen(ctx->screen); - *fence = rs->vws->cs_create_fence(rs->vws, fd); + if (rs->vws->cs_create_fence) + *fence = rs->vws->cs_create_fence(rs->vws, fd); } static void virgl_fence_server_sync(struct pipe_context *ctx, @@ -1046,7 +1047,8 @@ static void virgl_fence_server_sync(struct pipe_context *ctx, struct virgl_context *vctx = virgl_context(ctx); struct virgl_screen *rs = virgl_screen(ctx->screen); - rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence); + if (rs->vws->fence_server_sync) + rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence); } static void virgl_set_shader_images(struct pipe_context *ctx, ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] virgl,vtest: Initialize return value
This patch has been: Reviewed-by: Robert Foss On 2018-11-27 20:50, Gert Wollny wrote: From: Gert Wollny Avoids: Conditional jump or move depends on uninitialised value(s) at 0x9E2B39F: virgl_vtest_winsys_resource_cache_create (virgl_vtest_winsys.c:379) by 0x9E2725F: virgl_buffer_create (virgl_buffer.c:169) by 0x9E246D5: virgl_resource_create (virgl_resource.c:60) by 0xA0C1B9F: bufferobj_data (st_cb_bufferobjects.c:344) by 0xA0C1B9F: st_bufferobj_data (st_cb_bufferobjects.c:390) by 0x9F4ACE3: vbo_use_buffer_objects (vbo_exec_api.c:1136) by 0xA0C68C3: st_create_context_priv (st_context.c:416) by 0xA0C707A: st_create_context (st_context.c:598) by 0x9F81C6B: st_api_create_context (st_manager.c:918) by 0x9BBE591: dri_create_context (dri_context.c:161) by 0x9BB6931: driCreateContextAttribs (dri_util.c:473) by 0x4E97A44: drisw_create_context_attribs (drisw_glx.c:630) by 0x4E7C591: glXCreateContextAttribsARB (create_context.c:78) Uninitialised value was created by a stack allocation at 0x9E2B249: virgl_vtest_winsys_resource_cache_create (virgl_vtest_winsys.c:342) Signed-off-by: Gert Wollny --- src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c index a23f853924..65963ad50e 100644 --- a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c +++ b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c @@ -344,7 +344,7 @@ virgl_vtest_winsys_resource_cache_create(struct virgl_winsys *vws, struct virgl_hw_res *res, *curr_res; struct list_head *curr, *next; int64_t now; - int ret; + int ret = -1; /* only store binds for vertex/index/const buffers */ if (bind != VIRGL_BIND_CONSTANT_BUFFER && bind != VIRGL_BIND_INDEX_BUFFER && ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v1] virgl: add assert and missing function parameter
Verify the pipe_fd_type to be of PIPE_FD_TYPE_NATIVE_SYNC. Suggested-by: Eric Engestrom Signed-off-by: Robert Foss --- src/gallium/drivers/virgl/virgl_context.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/gallium/drivers/virgl/virgl_context.c b/src/gallium/drivers/virgl/virgl_context.c index 9be7775abd3..892fef76c75 100644 --- a/src/gallium/drivers/virgl/virgl_context.c +++ b/src/gallium/drivers/virgl/virgl_context.c @@ -1030,8 +1030,11 @@ static void virgl_set_shader_buffers(struct pipe_context *ctx, } static void virgl_create_fence_fd(struct pipe_context *ctx, - struct pipe_fence_handle **fence, int fd) + struct pipe_fence_handle **fence, + int fd, + enum pipe_fd_type type) { + assert(type == PIPE_FD_TYPE_NATIVE_SYNC); struct virgl_screen *rs = virgl_screen(ctx->screen); *fence = rs->vws->cs_create_fence(rs->vws, fd); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa 1/2] virgl: add missing function parameter
Hey Eric, On 2018-11-20 13:36, Eric Engestrom wrote: Fixes: d1a1c21e7621b5177feb "virgl: native fence fd support" Cc: Robert Foss Cc: Emil Velikov Signed-off-by: Eric Engestrom --- I don't know if something should be done with the fd type in this function, so one of you might want to send another patch to fix this? Otherwise, at least this fixes the function signature. --- src/gallium/drivers/virgl/virgl_context.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/gallium/drivers/virgl/virgl_context.c b/src/gallium/drivers/virgl/virgl_context.c index 9be7775abd3178d4b625..f6849598ee525f5728eb 100644 --- a/src/gallium/drivers/virgl/virgl_context.c +++ b/src/gallium/drivers/virgl/virgl_context.c @@ -1030,7 +1030,9 @@ static void virgl_set_shader_buffers(struct pipe_context *ctx, } static void virgl_create_fence_fd(struct pipe_context *ctx, - struct pipe_fence_handle **fence, int fd) + struct pipe_fence_handle **fence, + int fd, + enum pipe_fd_type type) { struct virgl_screen *rs = virgl_screen(ctx->screen); I think maybe an assert should be added, so we would end up with something like this: static void virgl_create_fence_fd(struct pipe_context *ctx, struct pipe_fence_handle **fence, int fd, enum pipe_fd_type type) { assert(type == PIPE_FD_TYPE_NATIVE_SYNC); struct virgl_screen *rs = virgl_screen(ctx->screen); *fence = rs->vws->cs_create_fence(rs->vws, fd); } ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] virgl: fix vtest regression since fencing changes.
Hey Dave, Thanks for catching this, pushed! On 2018-11-19 06:47, Dave Airlie wrote: From: Dave Airlie The in_fence_fd needs to be initialised to -1. Fixes: d1a1c21e7 (virgl: native fence fd support) --- src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c index bc7a8965be..a23f853924 100644 --- a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c +++ b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c @@ -427,6 +427,7 @@ static struct virgl_cmd_buf *virgl_vtest_cmd_buf_create(struct virgl_winsys *vws } cbuf->ws = vws; cbuf->base.buf = cbuf->buf; + cbuf->base.in_fence_fd = -1; return >base; } ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] virgl: Use file descriptor instead of un-allocated object
Hey Gert, Nice catch! Reviewed-by: Robert Foss On 2018-11-19 10:56, Gert Wollny wrote: From: Gert Wollny The structure qdws is not allocated at this point, nor is the file descriptor set to it's member. Use the fd directly instead. Fixes: d1a1c21e7621b5177febf191fcd3d3b8ef69dc96 virgl: native fence fd support Signed-off-by: Gert Wollny --- src/gallium/winsys/virgl/drm/virgl_drm_winsys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c index 2c02d3ccd7..26de8c702d 100644 --- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c +++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c @@ -892,7 +892,7 @@ virgl_drm_winsys_create(int drmFD) if (ret < 0 || !gl) return NULL; - drm_version = virgl_drm_get_version(qdws->fd); + drm_version = virgl_drm_get_version(drmFD); if (drm_version < 0) return NULL; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v1] virgl: Clean up fences commit
Remove a dead variable, an unneeded int->bool conversion and some whitespace changes. Signed-off-by: Robert Foss --- Sorry about the spam, and this not being included in v5 which was upstreamed. But nevertheless it should be fixed, and here's the commit. src/gallium/drivers/virgl/virgl_screen.c| 2 +- src/gallium/winsys/virgl/drm/virgl_drm_winsys.c | 1 - src/gallium/winsys/virgl/drm/virgl_drm_winsys.h | 1 - src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/gallium/drivers/virgl/virgl_screen.c b/src/gallium/drivers/virgl/virgl_screen.c index 8addbea9e4c..3d6424e39f6 100644 --- a/src/gallium/drivers/virgl/virgl_screen.c +++ b/src/gallium/drivers/virgl/virgl_screen.c @@ -340,7 +340,7 @@ virgl_get_param(struct pipe_screen *screen, enum pipe_cap param) case PIPE_CAP_VIDEO_MEMORY: return 0; case PIPE_CAP_NATIVE_FENCE_FD: - return !!vscreen->vws->supports_fences; + return vscreen->vws->supports_fences; default: return u_pipe_screen_get_param_defaults(screen, param); } diff --git a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c index 5b604044598..2c02d3ccd75 100644 --- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c +++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c @@ -933,7 +933,6 @@ virgl_drm_winsys_create(int drmFD) qdws->base.get_caps = virgl_drm_get_caps; - uint32_t value = 0; getparam.param = VIRTGPU_PARAM_CAPSET_QUERY_FIX; getparam.value = (uint64_t)(uintptr_t) diff --git a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.h b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.h index 4316a74977d..659c2d77568 100644 --- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.h +++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.h @@ -57,7 +57,6 @@ struct virgl_drm_winsys { struct virgl_winsys base; int fd; - int drm_version; struct list_head delayed; int num_delayed; unsigned usecs; diff --git a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c index c1c88151ad3..bc7a8965be9 100644 --- a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c +++ b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c @@ -702,6 +702,5 @@ virgl_vtest_winsys_wrap(struct sw_winsys *sws) vtws->base.flush_frontbuffer = virgl_vtest_flush_frontbuffer; - return >base; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v5] virgl: native fence fd support
Hey Emil, I just pushed v5 upstream. Let me spin a cleanup patch and send that to the ML. Rob. On 2018-11-16 14:03, Emil Velikov wrote: Hi Rob, I've spotted a couple of nitpicks, but I don't feel too strongly either way. On Thu, 15 Nov 2018 at 13:28, Robert Foss wrote: case PIPE_CAP_VIDEO_MEMORY: return 0; case PIPE_CAP_NATIVE_FENCE_FD: - return 0; + return !!vscreen->vws->supports_fences; Do we need the !! here? AFAICT it's usually done to convert int to bool. qdws->base.cs_create_fence = virgl_cs_create_fence; qdws->base.fence_wait = virgl_fence_wait; qdws->base.fence_reference = virgl_fence_reference; + qdws->base.fence_server_sync = virgl_fence_server_sync; + qdws->base.fence_get_fd = virgl_fence_get_fd; + qdws->base.supports_fences = drm_version >= VIRGL_DRM_VERSION_FENCE_FD; qdws->base.get_caps = virgl_drm_get_caps; + Unneeded whitespace changes. uint32_t value = 0; getparam.param = VIRTGPU_PARAM_CAPSET_QUERY_FIX; getparam.value = (uint64_t)(uintptr_t) struct virgl_drm_winsys { struct virgl_winsys base; int fd; + int drm_version; Never assigned? I'd assign it or drop it. vtws->base.cs_create_fence = virgl_cs_create_fence; vtws->base.fence_wait = virgl_fence_wait; vtws->base.fence_reference = virgl_fence_reference; + vtws->base.supports_fences = 0; vtws->base.flush_frontbuffer = virgl_vtest_flush_frontbuffer; + Unneeded whitespace changes. return >base; } HTH Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5] virgl: native fence fd support
Following the support for fences on the virtio driver add support for native fence on virgl. This was somewhat based on the freedeno one. Signed-off-by: Gustavo Padovan Signed-off-by: Robert Foss Reviewed-by: Emil Velikov --- Sorry about the spam, but I realized Emils r-b wasn't included in v4. This patch has been tested using Qemu & Virglrenderer. Linux virtgpu fences branch: https://gitlab.collabora.com/robertfoss/linux/tree/virtio_fences_v5 This branch: https://gitlab.collabora.com/robertfoss/mesa/tree/virtio_fences_v5 Changes since v4: - Add Reviewed-by: Emil Velikov Changes since v3: - Emil: Replaced driver_version() winsys func. with supports_fence variable - Emil: Replaced virgl_fence_server_sync() assert with it - Emil: Refactored virgl_fence_server_sync() to use sync_accumulate() Changes since v2: - Add fence creation on flush src/gallium/drivers/virgl/virgl_context.c | 46 -- src/gallium/drivers/virgl/virgl_screen.c | 12 ++- src/gallium/drivers/virgl/virgl_winsys.h | 14 +++- .../winsys/virgl/drm/virgl_drm_winsys.c | 84 ++- .../winsys/virgl/drm/virgl_drm_winsys.h | 2 + src/gallium/winsys/virgl/drm/virtgpu_drm.h| 14 +++- .../winsys/virgl/vtest/virgl_vtest_winsys.c | 10 ++- 7 files changed, 166 insertions(+), 16 deletions(-) diff --git a/src/gallium/drivers/virgl/virgl_context.c b/src/gallium/drivers/virgl/virgl_context.c index 96932c473d8..9be7775abd3 100644 --- a/src/gallium/drivers/virgl/virgl_context.c +++ b/src/gallium/drivers/virgl/virgl_context.c @@ -21,6 +21,7 @@ * USE OR OTHER DEALINGS IN THE SOFTWARE. */ +#include #include "pipe/p_shader_tokens.h" #include "pipe/p_context.h" @@ -709,13 +710,20 @@ static void virgl_draw_vbo(struct pipe_context *ctx, } -static void virgl_flush_eq(struct virgl_context *ctx, void *closure) +static void virgl_flush_eq(struct virgl_context *ctx, void *closure, + struct pipe_fence_handle **fence) { struct virgl_screen *rs = virgl_screen(ctx->base.screen); + int out_fence_fd = -1; /* send the buffer to the remote side for decoding */ ctx->num_transfers = ctx->num_draws = 0; - rs->vws->submit_cmd(rs->vws, ctx->cbuf); + + rs->vws->submit_cmd(rs->vws, ctx->cbuf, ctx->cbuf->in_fence_fd, + ctx->cbuf->needs_out_fence_fd ? _fence_fd : NULL); + + if (fence) + *fence = rs->vws->cs_create_fence(rs->vws, out_fence_fd); virgl_encoder_set_sub_ctx(ctx, ctx->hw_sub_ctx_id); @@ -728,11 +736,10 @@ static void virgl_flush_from_st(struct pipe_context *ctx, enum pipe_flush_flags flags) { struct virgl_context *vctx = virgl_context(ctx); - struct virgl_screen *rs = virgl_screen(ctx->screen); struct virgl_buffer *buf, *tmp; - if (fence) - *fence = rs->vws->cs_create_fence(rs->vws); + if (flags & PIPE_FLUSH_FENCE_FD) + vctx->cbuf->needs_out_fence_fd = true; LIST_FOR_EACH_ENTRY_SAFE(buf, tmp, >to_flush_bufs, flush_list) { struct pipe_resource *res = >base.u.b; @@ -742,7 +749,13 @@ static void virgl_flush_from_st(struct pipe_context *ctx, pipe_resource_reference(, NULL); } - virgl_flush_eq(vctx, vctx); + virgl_flush_eq(vctx, vctx, fence); + + if (vctx->cbuf->in_fence_fd != -1) { + close(vctx->cbuf->in_fence_fd); + vctx->cbuf->in_fence_fd = -1; + } + vctx->cbuf->needs_out_fence_fd = false; } static struct pipe_sampler_view *virgl_create_sampler_view(struct pipe_context *ctx, @@ -1016,6 +1029,23 @@ static void virgl_set_shader_buffers(struct pipe_context *ctx, virgl_encode_set_shader_buffers(vctx, shader, start_slot, count, buffers); } +static void virgl_create_fence_fd(struct pipe_context *ctx, + struct pipe_fence_handle **fence, int fd) +{ + struct virgl_screen *rs = virgl_screen(ctx->screen); + + *fence = rs->vws->cs_create_fence(rs->vws, fd); +} + +static void virgl_fence_server_sync(struct pipe_context *ctx, + struct pipe_fence_handle *fence) +{ + struct virgl_context *vctx = virgl_context(ctx); + struct virgl_screen *rs = virgl_screen(ctx->screen); + + rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence); +} + static void virgl_set_shader_images(struct pipe_context *ctx, enum pipe_shader_type shader, unsigned start_slot, unsigned count, @@ -1108,7 +1138,7 @@ virgl_context_destroy( struct pipe_context *ctx ) vctx->framebuffer.zsbuf = NULL; vctx->framebuffer.nr_cbufs = 0; virgl_encoder_destroy_sub_ctx(vctx, vctx->hw_sub_ctx_id); - virgl_flush_eq(vctx, vctx); + virgl_flush_eq(vctx, vctx, NULL); rs->vws->cmd_buf_destroy(vctx->cbuf); if (vctx-&
[Mesa-dev] [PATCH v4] virgl: native fence fd support
Following the support for fences on the virtio driver add support for native fence on virgl. This was somewhat based on the freedeno one. Signed-off-by: Gustavo Padovan Signed-off-by: Robert Foss --- This patch has been tested using Qemu & Virglrenderer. Linux virtgpu fences branch: https://gitlab.collabora.com/robertfoss/linux/tree/virtio_fences_v5 This branch: https://gitlab.collabora.com/robertfoss/mesa/tree/virtio_fences_v4 Changes since v3: - Emil: Replaced driver_version() winsys func. with supports_fence variable - Emil: Replaced virgl_fence_server_sync() assert with it - Emil: Refactored virgl_fence_server_sync() to use sync_accumulate() Changes since v2: - Add fence creation on flush src/gallium/drivers/virgl/virgl_context.c | 46 -- src/gallium/drivers/virgl/virgl_screen.c | 12 ++- src/gallium/drivers/virgl/virgl_winsys.h | 14 +++- .../winsys/virgl/drm/virgl_drm_winsys.c | 84 ++- .../winsys/virgl/drm/virgl_drm_winsys.h | 2 + src/gallium/winsys/virgl/drm/virtgpu_drm.h| 14 +++- .../winsys/virgl/vtest/virgl_vtest_winsys.c | 10 ++- 7 files changed, 166 insertions(+), 16 deletions(-) diff --git a/src/gallium/drivers/virgl/virgl_context.c b/src/gallium/drivers/virgl/virgl_context.c index 96932c473d8..9be7775abd3 100644 --- a/src/gallium/drivers/virgl/virgl_context.c +++ b/src/gallium/drivers/virgl/virgl_context.c @@ -21,6 +21,7 @@ * USE OR OTHER DEALINGS IN THE SOFTWARE. */ +#include #include "pipe/p_shader_tokens.h" #include "pipe/p_context.h" @@ -709,13 +710,20 @@ static void virgl_draw_vbo(struct pipe_context *ctx, } -static void virgl_flush_eq(struct virgl_context *ctx, void *closure) +static void virgl_flush_eq(struct virgl_context *ctx, void *closure, + struct pipe_fence_handle **fence) { struct virgl_screen *rs = virgl_screen(ctx->base.screen); + int out_fence_fd = -1; /* send the buffer to the remote side for decoding */ ctx->num_transfers = ctx->num_draws = 0; - rs->vws->submit_cmd(rs->vws, ctx->cbuf); + + rs->vws->submit_cmd(rs->vws, ctx->cbuf, ctx->cbuf->in_fence_fd, + ctx->cbuf->needs_out_fence_fd ? _fence_fd : NULL); + + if (fence) + *fence = rs->vws->cs_create_fence(rs->vws, out_fence_fd); virgl_encoder_set_sub_ctx(ctx, ctx->hw_sub_ctx_id); @@ -728,11 +736,10 @@ static void virgl_flush_from_st(struct pipe_context *ctx, enum pipe_flush_flags flags) { struct virgl_context *vctx = virgl_context(ctx); - struct virgl_screen *rs = virgl_screen(ctx->screen); struct virgl_buffer *buf, *tmp; - if (fence) - *fence = rs->vws->cs_create_fence(rs->vws); + if (flags & PIPE_FLUSH_FENCE_FD) + vctx->cbuf->needs_out_fence_fd = true; LIST_FOR_EACH_ENTRY_SAFE(buf, tmp, >to_flush_bufs, flush_list) { struct pipe_resource *res = >base.u.b; @@ -742,7 +749,13 @@ static void virgl_flush_from_st(struct pipe_context *ctx, pipe_resource_reference(, NULL); } - virgl_flush_eq(vctx, vctx); + virgl_flush_eq(vctx, vctx, fence); + + if (vctx->cbuf->in_fence_fd != -1) { + close(vctx->cbuf->in_fence_fd); + vctx->cbuf->in_fence_fd = -1; + } + vctx->cbuf->needs_out_fence_fd = false; } static struct pipe_sampler_view *virgl_create_sampler_view(struct pipe_context *ctx, @@ -1016,6 +1029,23 @@ static void virgl_set_shader_buffers(struct pipe_context *ctx, virgl_encode_set_shader_buffers(vctx, shader, start_slot, count, buffers); } +static void virgl_create_fence_fd(struct pipe_context *ctx, + struct pipe_fence_handle **fence, int fd) +{ + struct virgl_screen *rs = virgl_screen(ctx->screen); + + *fence = rs->vws->cs_create_fence(rs->vws, fd); +} + +static void virgl_fence_server_sync(struct pipe_context *ctx, + struct pipe_fence_handle *fence) +{ + struct virgl_context *vctx = virgl_context(ctx); + struct virgl_screen *rs = virgl_screen(ctx->screen); + + rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence); +} + static void virgl_set_shader_images(struct pipe_context *ctx, enum pipe_shader_type shader, unsigned start_slot, unsigned count, @@ -1108,7 +1138,7 @@ virgl_context_destroy( struct pipe_context *ctx ) vctx->framebuffer.zsbuf = NULL; vctx->framebuffer.nr_cbufs = 0; virgl_encoder_destroy_sub_ctx(vctx, vctx->hw_sub_ctx_id); - virgl_flush_eq(vctx, vctx); + virgl_flush_eq(vctx, vctx, NULL); rs->vws->cmd_buf_destroy(vctx->cbuf); if (vctx->uploader) @@ -1244,6 +1274,8 @@ struct pipe_context *virgl_context_create(struct pipe_screen *pscreen, vctx->base.resource_copy_region
[Mesa-dev] [PATCH v3] virgl: native fence fd support
From: Gustavo Padovan Following the support for fences on the virtio driver add support for native fence on virgl. This was somewhat based on the freedeno one. Signed-off-by: Gustavo Padovan Signed-off-by: Robert Foss --- Retransmission: Missed CC-ing mesa-dev Linux virtgpu fences branch: https://gitlab.collabora.com/robertfoss/linux/tree/virtio_fences_v5 Changes since v2: - Add fence creation on flush src/gallium/drivers/virgl/virgl_context.c | 46 -- src/gallium/drivers/virgl/virgl_screen.c | 12 ++- src/gallium/drivers/virgl/virgl_winsys.h | 15 +++- .../winsys/virgl/drm/virgl_drm_winsys.c | 85 ++- .../winsys/virgl/drm/virgl_drm_winsys.h | 2 + src/gallium/winsys/virgl/drm/virtgpu_drm.h| 14 ++- .../winsys/virgl/vtest/virgl_vtest_winsys.c | 8 +- 7 files changed, 166 insertions(+), 16 deletions(-) diff --git a/src/gallium/drivers/virgl/virgl_context.c b/src/gallium/drivers/virgl/virgl_context.c index 4511bf3b2fb..22a545200e7 100644 --- a/src/gallium/drivers/virgl/virgl_context.c +++ b/src/gallium/drivers/virgl/virgl_context.c @@ -21,6 +21,7 @@ * USE OR OTHER DEALINGS IN THE SOFTWARE. */ +#include #include "pipe/p_shader_tokens.h" #include "pipe/p_context.h" @@ -709,13 +710,20 @@ static void virgl_draw_vbo(struct pipe_context *ctx, } -static void virgl_flush_eq(struct virgl_context *ctx, void *closure) +static void virgl_flush_eq(struct virgl_context *ctx, void *closure, + struct pipe_fence_handle **fence) { struct virgl_screen *rs = virgl_screen(ctx->base.screen); + int out_fence_fd = -1; /* send the buffer to the remote side for decoding */ ctx->num_transfers = ctx->num_draws = 0; - rs->vws->submit_cmd(rs->vws, ctx->cbuf); + + rs->vws->submit_cmd(rs->vws, ctx->cbuf, ctx->cbuf->in_fence_fd, + ctx->cbuf->needs_out_fence_fd ? _fence_fd : NULL); + + if (fence) + *fence = rs->vws->cs_create_fence(rs->vws, out_fence_fd); virgl_encoder_set_sub_ctx(ctx, ctx->hw_sub_ctx_id); @@ -728,11 +736,10 @@ static void virgl_flush_from_st(struct pipe_context *ctx, enum pipe_flush_flags flags) { struct virgl_context *vctx = virgl_context(ctx); - struct virgl_screen *rs = virgl_screen(ctx->screen); struct virgl_buffer *buf, *tmp; - if (fence) - *fence = rs->vws->cs_create_fence(rs->vws); + if (flags & PIPE_FLUSH_FENCE_FD) + vctx->cbuf->needs_out_fence_fd = true; LIST_FOR_EACH_ENTRY_SAFE(buf, tmp, >to_flush_bufs, flush_list) { struct pipe_resource *res = >base.u.b; @@ -742,7 +749,13 @@ static void virgl_flush_from_st(struct pipe_context *ctx, pipe_resource_reference(, NULL); } - virgl_flush_eq(vctx, vctx); + virgl_flush_eq(vctx, vctx, fence); + + if (vctx->cbuf->in_fence_fd != -1) { + close(vctx->cbuf->in_fence_fd); + vctx->cbuf->in_fence_fd = -1; + } + vctx->cbuf->needs_out_fence_fd = false; } static struct pipe_sampler_view *virgl_create_sampler_view(struct pipe_context *ctx, @@ -1016,6 +1029,23 @@ static void virgl_set_shader_buffers(struct pipe_context *ctx, virgl_encode_set_shader_buffers(vctx, shader, start_slot, count, buffers); } +static void virgl_create_fence_fd(struct pipe_context *ctx, + struct pipe_fence_handle **fence, int fd) +{ + struct virgl_screen *rs = virgl_screen(ctx->screen); + + *fence = rs->vws->cs_create_fence(rs->vws, fd); +} + +static void virgl_fence_server_sync(struct pipe_context *ctx, + struct pipe_fence_handle *fence) +{ + struct virgl_context *vctx = virgl_context(ctx); + struct virgl_screen *rs = virgl_screen(ctx->screen); + + rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence); +} + static void virgl_set_shader_images(struct pipe_context *ctx, enum pipe_shader_type shader, unsigned start_slot, unsigned count, @@ -1108,7 +1138,7 @@ virgl_context_destroy( struct pipe_context *ctx ) vctx->framebuffer.zsbuf = NULL; vctx->framebuffer.nr_cbufs = 0; virgl_encoder_destroy_sub_ctx(vctx, vctx->hw_sub_ctx_id); - virgl_flush_eq(vctx, vctx); + virgl_flush_eq(vctx, vctx, NULL); rs->vws->cmd_buf_destroy(vctx->cbuf); if (vctx->uploader) @@ -1243,6 +1273,8 @@ struct pipe_context *virgl_context_create(struct pipe_screen *pscreen, vctx->base.resource_copy_region = virgl_resource_copy_region; vctx->base.flush_resource = virgl_flush_resource; vctx->base.blit = virgl_blit; + vctx->base.create_fence_fd = virgl_create_fence_fd; + vctx->base.fence_server_sync = virgl_fence_server_sync; vctx->base.set_shader_buffers = virgl_set_shad
Re: [Mesa-dev] [PATCH] egl/android: rework device probing
Hey, On 27/08/2018 11.47, Emil Velikov wrote: On 24 August 2018 at 18:55, Robert Foss wrote: Hey Emil, On 24/08/2018 14.21, Emil Velikov wrote: From: Emil Velikov Unlike the other platforms, here we aim do guess if the device that we somewhat arbitrarily picked, is supported or not. In particular: when a vendor is _not_ requested we loop through all devices, picking the first one which can create a DRI screen. When a vendor is requested - we use that and do _not_ fall-back to any other device. The former seems a bit fiddly, but considering EGL_EXT_explicit_device and EGL_MESA_query_renderer are MIA, this is the best we can do for the moment. With those (proposed) extensions userspace will be able to create a separate EGL display for each device, query device details and make the conscious decision which one to use. Cc: Robert Foss Cc: Tomasz Figa Signed-off-by: Emil Velikov --- Thanks for the clarification Tomasz. The original code was using a fall-back even a vendor was explicitly requested, confusing me a bit ;-) --- src/egl/drivers/dri2/platform_android.c | 71 +++-- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 1f9fe27ab85..5bf627dec7d 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1420,13 +1420,32 @@ droid_filter_device(_EGLDisplay *disp, int fd, const char *vendor) return 0; } +static int +droid_probe_device(_EGLDisplay *disp) +{ + /* Check that the device is supported, by attempting to: + * - load the dri module + * - and, create a screen + */ + if (!droid_load_driver(disp)) { + _eglLog(_EGL_WARNING, "DRI2: failed to load driver"); + return -1; + } + + if (!dri2_create_screen(disp)) { + _eglLog(_EGL_WARNING, "DRI2: failed to create screen"); + return -1; + } + return 0; +} + static int droid_open_device(_EGLDisplay *disp) { #define MAX_DRM_DEVICES 32 drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL }; int prop_set, num_devices; - int fd = -1, fallback_fd = -1; + int fd = -1; char *vendor_name = NULL; char vendor_buf[PROPERTY_VALUE_MAX]; @@ -1451,33 +1470,39 @@ droid_open_device(_EGLDisplay *disp) continue; } - if (vendor_name && droid_filter_device(disp, fd, vendor_name)) { - /* Match requested, but not found - set as fallback */ - if (fallback_fd == -1) { -fallback_fd = fd; - } else { + /* If a vendor is explicitly provided, we use only that. + * Otherwise we fall-back the first device that is supported. + */ + if (vendor_name) { + if (droid_filter_device(disp, fd, vendor_name)) { +/* Device does not match - try next device */ close(fd); fd = -1; +continue; } - + /* If the requested device matches use it, regardless if + * init fails. Do not fall-back to any other device. + */ + if (droid_probbe_device(disp)) { Typo in function name. Thanks for having a look Rob. Will fix (plus second instance below). +close(fd); +fd = -1; + } Isn't the above comment saying that the if statement just below it shouldn't be there? Or am I misparsing something? I think it matches with the comment - note the function returns an int 0 on success. We could change that esp. since others - droid_load_driver return an EGLBoolean. Alternatively we could use if (droid_probbe_device(disp) != 0) I'm fine either way. Upon re-reading this, I'm happy with the way it is. Feel free to add my r-b with the above typo fixed. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: rework device probing
Hey Emil, On 24/08/2018 14.21, Emil Velikov wrote: From: Emil Velikov Unlike the other platforms, here we aim do guess if the device that we somewhat arbitrarily picked, is supported or not. In particular: when a vendor is _not_ requested we loop through all devices, picking the first one which can create a DRI screen. When a vendor is requested - we use that and do _not_ fall-back to any other device. The former seems a bit fiddly, but considering EGL_EXT_explicit_device and EGL_MESA_query_renderer are MIA, this is the best we can do for the moment. With those (proposed) extensions userspace will be able to create a separate EGL display for each device, query device details and make the conscious decision which one to use. Cc: Robert Foss Cc: Tomasz Figa Signed-off-by: Emil Velikov --- Thanks for the clarification Tomasz. The original code was using a fall-back even a vendor was explicitly requested, confusing me a bit ;-) --- src/egl/drivers/dri2/platform_android.c | 71 +++-- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 1f9fe27ab85..5bf627dec7d 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1420,13 +1420,32 @@ droid_filter_device(_EGLDisplay *disp, int fd, const char *vendor) return 0; } +static int +droid_probe_device(_EGLDisplay *disp) +{ + /* Check that the device is supported, by attempting to: + * - load the dri module + * - and, create a screen + */ + if (!droid_load_driver(disp)) { + _eglLog(_EGL_WARNING, "DRI2: failed to load driver"); + return -1; + } + + if (!dri2_create_screen(disp)) { + _eglLog(_EGL_WARNING, "DRI2: failed to create screen"); + return -1; + } + return 0; +} + static int droid_open_device(_EGLDisplay *disp) { #define MAX_DRM_DEVICES 32 drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL }; int prop_set, num_devices; - int fd = -1, fallback_fd = -1; + int fd = -1; char *vendor_name = NULL; char vendor_buf[PROPERTY_VALUE_MAX]; @@ -1451,33 +1470,39 @@ droid_open_device(_EGLDisplay *disp) continue; } - if (vendor_name && droid_filter_device(disp, fd, vendor_name)) { - /* Match requested, but not found - set as fallback */ - if (fallback_fd == -1) { -fallback_fd = fd; - } else { + /* If a vendor is explicitly provided, we use only that. + * Otherwise we fall-back the first device that is supported. + */ + if (vendor_name) { + if (droid_filter_device(disp, fd, vendor_name)) { +/* Device does not match - try next device */ close(fd); fd = -1; +continue; } - + /* If the requested device matches use it, regardless if + * init fails. Do not fall-back to any other device. + */ + if (droid_probbe_device(disp)) { Typo in function name. +close(fd); +fd = -1; + } Isn't the above comment saying that the if statement just below it shouldn't be there? Or am I misparsing something? + break; + } + /* No explicit request - attempt the next device */ + if (droid_probbe_device(disp)) { Typo in function name. + close(fd); + fd = -1; continue; } - /* Found a device */ break; } drmFreeDevices(devices, num_devices); - if (fallback_fd < 0 && fd < 0) { - _eglLog(_EGL_WARNING, "Failed to open any DRM device"); - return -1; - } - - if (fd < 0) { - _eglLog(_EGL_WARNING, "Failed to open desired DRM device, using fallback"); - return fallback_fd; - } + if (fd < 0) + _eglLog(_EGL_WARNING, "Failed to open %s DRM device", +vendor_name ? "desired": "any"); - close(fallback_fd); return fd; #undef MAX_DRM_DEVICES } @@ -1519,16 +1544,6 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) goto cleanup; } - if (!droid_load_driver(disp)) { - err = "DRI2: failed to load driver"; - goto cleanup; - } - - if (!dri2_create_screen(disp)) { - err = "DRI2: failed to create screen"; - goto cleanup; - } - if (!dri2_setup_extensions(disp)) { err = "DRI2: failed to setup extensions"; goto cleanup; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] egl/android: fix regression in drm_gralloc path
drivers/dri2/platform_android.c index cc16fd8118..834bbd258e 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1134,6 +1134,25 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } +#ifdef HAVE_DRM_GRALLOC +static int +droid_open_device_drm_gralloc(struct dri2_egl_display *dri2_dpy) +{ + int fd = -1, err = -EINVAL; + + if (dri2_dpy->gralloc->perform) + err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc, + GRALLOC_MODULE_PERFORM_GET_DRM_FD, + ); + if (err || fd < 0) { + _eglLog(_EGL_WARNING, "fail to get drm fd"); + fd = -1; + } + + return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1; +} +#endif /* HAVE_DRM_GRALLOC */ + static const struct dri2_egl_display_vtbl droid_display_vtbl = { .authenticate = NULL, .create_window_surface = droid_create_window_surface, @@ -1384,7 +1403,11 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) disp->DriverData = (void *) dri2_dpy; + #ifdef HAVE_DRM_GRALLOC + dri2_dpy->fd = droid_open_device_drm_gralloc(dri2_dpy); + #else dri2_dpy->fd = droid_open_device(disp); + #endif if (dri2_dpy->fd < 0) { err = "DRI2: failed to open device"; goto cleanup; This does seem fairly un-intrusive if the GRALLOC_MODULE_PERFORM_GET_DRM_FD define is already being provided by drm_gralloc. If we are going to support the drm_gralloc usecase and this patch is needed to do so, I'm all for it. With the above suggestion fixed: Reviewed-by: Robert Foss Rob. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/5] egl/android: use drmDevice instead of the manual /dev/dri iteration
Hey again On 13/08/2018 21.01, Robert Foss wrote: Hey Emil, On 13/08/2018 16.44, Emil Velikov wrote: From: Emil Velikov Replace the manual handling of /dev/dri in famour of the drmDevice API. The latter provides a consistent way of enumerating the devices, providing device details as needed. Cc: Robert Foss Cc: Tomasz Figa Signed-off-by: Emil Velikov --- If using VGEM the following patch is needed [1]. It's been on the list for ages - will double-check and commmit it to drm-misc. [1] https://lists.freedesktop.org/archives/dri-devel/2018-March/170944.html --- src/egl/drivers/dri2/platform_android.c | 23 +-- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index cc16fd8118f..685851acfc2 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1290,6 +1290,7 @@ static int droid_open_device(_EGLDisplay *disp) { const int MAX_DRM_DEVICES = 32; + drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL }; The android-8.0.0_r35 AOSP checkout I'm running doesn't want to compile the above line. external/mesa3d/src/egl/drivers/dri2/platform_android.c:1259:33: error: variable-sized object may not be initialized drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL }; ^~~ external/mesa3d/src/egl/drivers/dri2/platform_android.c:1282:28: error: use of undeclared identifier 'dev_path' __func__, dev_path); Sorry about the churn, but this error of course doesn't relate to the above one. Still, this error string needs to be fixed. int prop_set, num_devices; int fd = -1, fallback_fd = -1; @@ -1299,23 +1300,17 @@ droid_open_device(_EGLDisplay *disp) if (property_get("drm.gpu.vendor_name", vendor_buf, NULL) > 0) vendor_name = vendor_buf; - const char *drm_dir_name = "/dev/dri"; - DIR *sysdir = opendir(drm_dir_name); + num_devices = drmGetDevices2(0, devices, MAX_DRM_DEVICES); + if (num_devices < 0) + return num_devices; - if (!sysdir) - return -errno; + for (int i = 0; i < num_devices; i++) { + device = devices[i]; - struct dirent *dent; - while ((dent = readdir(sysdir))) { - char dev_path[128]; - const char render_dev_prefix[] = "renderD"; - size_t prefix_len = sizeof(render_dev_prefix) - 1; - - if (strncmp(render_dev_prefix, dent->d_name, prefix_len) != 0) + if (!(device->available_nodes & (1 << DRM_NODE_RENDER))) continue; - snprintf(dev_path, sizeof(dev_path), "%s/%s", drm_dir_name, dent->d_name); - fd = loader_open_device(dev_path); + fd = loader_open_device(device->nodes[DRM_NODE_RENDER]); if (fd < 0) { _eglLog(_EGL_WARNING, "%s() Failed to open DRM device %s", __func__, dev_path); @@ -1341,7 +1336,7 @@ droid_open_device(_EGLDisplay *disp) } success: - closedir(sysdir); + drmFreeDevices(devices, num_devices); if (fallback_fd < 0 && fd < 0) { _eglLog(_EGL_WARNING, "Failed to open any DRM device"); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/5] egl/android: use drmDevice instead of the manual /dev/dri iteration
Hey Emil, On 13/08/2018 16.44, Emil Velikov wrote: From: Emil Velikov Replace the manual handling of /dev/dri in famour of the drmDevice API. The latter provides a consistent way of enumerating the devices, providing device details as needed. Cc: Robert Foss Cc: Tomasz Figa Signed-off-by: Emil Velikov --- If using VGEM the following patch is needed [1]. It's been on the list for ages - will double-check and commmit it to drm-misc. [1] https://lists.freedesktop.org/archives/dri-devel/2018-March/170944.html --- src/egl/drivers/dri2/platform_android.c | 23 +-- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index cc16fd8118f..685851acfc2 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1290,6 +1290,7 @@ static int droid_open_device(_EGLDisplay *disp) { const int MAX_DRM_DEVICES = 32; + drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL }; The android-8.0.0_r35 AOSP checkout I'm running doesn't want to compile the above line. external/mesa3d/src/egl/drivers/dri2/platform_android.c:1259:33: error: variable-sized object may not be initialized drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL }; ^~~ external/mesa3d/src/egl/drivers/dri2/platform_android.c:1282:28: error: use of undeclared identifier 'dev_path' __func__, dev_path); int prop_set, num_devices; int fd = -1, fallback_fd = -1; @@ -1299,23 +1300,17 @@ droid_open_device(_EGLDisplay *disp) if (property_get("drm.gpu.vendor_name", vendor_buf, NULL) > 0) vendor_name = vendor_buf; - const char *drm_dir_name = "/dev/dri"; - DIR *sysdir = opendir(drm_dir_name); + num_devices = drmGetDevices2(0, devices, MAX_DRM_DEVICES); + if (num_devices < 0) + return num_devices; - if (!sysdir) - return -errno; + for (int i = 0; i < num_devices; i++) { + device = devices[i]; - struct dirent *dent; - while ((dent = readdir(sysdir))) { - char dev_path[128]; - const char render_dev_prefix[] = "renderD"; - size_t prefix_len = sizeof(render_dev_prefix) - 1; - - if (strncmp(render_dev_prefix, dent->d_name, prefix_len) != 0) + if (!(device->available_nodes & (1 << DRM_NODE_RENDER))) continue; - snprintf(dev_path, sizeof(dev_path), "%s/%s", drm_dir_name, dent->d_name); - fd = loader_open_device(dev_path); + fd = loader_open_device(device->nodes[DRM_NODE_RENDER]); if (fd < 0) { _eglLog(_EGL_WARNING, "%s() Failed to open DRM device %s", __func__, dev_path); @@ -1341,7 +1336,7 @@ droid_open_device(_EGLDisplay *disp) } success: - closedir(sysdir); + drmFreeDevices(devices, num_devices); if (fallback_fd < 0 && fd < 0) { _eglLog(_EGL_WARNING, "Failed to open any DRM device"); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/5] egl/android: use drmDevice instead of the manual /dev/dri iteration
Hey, On 13/08/2018 16.44, Emil Velikov wrote: From: Emil Velikov Replace the manual handling of /dev/dri in famour of the drmDevice API. The latter provides a consistent way of enumerating the devices, providing device details as needed. Thanks for coding this up, it's neater and the way to go since vitio-gpu support landed in libdrm. Reviewed-by: Robert Foss Cc: Robert Foss Cc: Tomasz Figa Signed-off-by: Emil Velikov --- If using VGEM the following patch is needed [1]. It's been on the list for ages - will double-check and commmit it to drm-misc. [1] https://lists.freedesktop.org/archives/dri-devel/2018-March/170944.html --- src/egl/drivers/dri2/platform_android.c | 23 +-- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index cc16fd8118f..685851acfc2 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1290,6 +1290,7 @@ static int droid_open_device(_EGLDisplay *disp) { const int MAX_DRM_DEVICES = 32; + drmDevicePtr device, devices[MAX_DRM_DEVICES] = { NULL }; int prop_set, num_devices; int fd = -1, fallback_fd = -1; @@ -1299,23 +1300,17 @@ droid_open_device(_EGLDisplay *disp) if (property_get("drm.gpu.vendor_name", vendor_buf, NULL) > 0) vendor_name = vendor_buf; - const char *drm_dir_name = "/dev/dri"; - DIR *sysdir = opendir(drm_dir_name); + num_devices = drmGetDevices2(0, devices, MAX_DRM_DEVICES); + if (num_devices < 0) + return num_devices; - if (!sysdir) - return -errno; + for (int i = 0; i < num_devices; i++) { + device = devices[i]; - struct dirent *dent; - while ((dent = readdir(sysdir))) { - char dev_path[128]; - const char render_dev_prefix[] = "renderD"; - size_t prefix_len = sizeof(render_dev_prefix) - 1; - - if (strncmp(render_dev_prefix, dent->d_name, prefix_len) != 0) + if (!(device->available_nodes & (1 << DRM_NODE_RENDER))) continue; - snprintf(dev_path, sizeof(dev_path), "%s/%s", drm_dir_name, dent->d_name); - fd = loader_open_device(dev_path); + fd = loader_open_device(device->nodes[DRM_NODE_RENDER]); if (fd < 0) { _eglLog(_EGL_WARNING, "%s() Failed to open DRM device %s", __func__, dev_path); @@ -1341,7 +1336,7 @@ droid_open_device(_EGLDisplay *disp) } success: - closedir(sysdir); + drmFreeDevices(devices, num_devices); if (fallback_fd < 0 && fd < 0) { _eglLog(_EGL_WARNING, "Failed to open any DRM device"); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 0/4] Android kms_swrast support
Hey Rob, On 2018-07-18 15:30, Rob Herring wrote: On Tue, Jul 17, 2018 at 4:33 AM Robert Foss wrote: This series implements kms_swrast support for the Android platform. And since having to debug a null pointer dereference, simplify that process for the next guy. So is this working for you now? As it stands now, any kernel must have the following ioctls flagged with DRM_RENDER_ALLOW[1], which isn't the case in the mainline kernel. DRM_IOCTL_MODE_CREATE_DUMB DRM_IOCTL_MODE_MAP_DUMB Ah, sorry. I should have mentioned this. We have discussed this issue in the past, but to no further conclusion. But as I recall, I thought the issue was also allowing import and export of dumb buffers? While it would be possible to open a non-render node to pass the authentication check, this would still cause authentication issues when the /dev/dri/cardX node needs to be opened as master by both mesa and the compositor. Right. We've pretty much stripped the support that was there out. Plus I don't think it will work with Treble. I don't know how acceptable this series is for upstreaming, while relying on a non-mainline kernel. I think the policy is to not accept changes that don't have both a user and kernel space solution in place. Like I noted yesterday[2] the alternative to using dumb buffers and having authentication issues is using VGEM, which is new territory to me, and it would take me a little bit of time to figure exactly how it fits into the current kms_swrast approach. Input, like noted before, is very much welcome. I'm very much in favor of the former approach. VGEM seems like an overly complicated solution when there's a very simple solution. I've started a discussion about this on the LKML[1], but I havent really managed to get it off the ground. But, about VGEM, what are the complicated parts you're referring to ^^? I'm only asking, because I have a pretty poor understanding of what it would look like. [1] https://lkml.org/lkml/2018/7/24/187 Rob ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/4] radv: generate entrypoints for VK_ANDROID_native_buffer
Hey Mauro, This looks good to me. Signed-off-by: Robert Foss On 2018-07-22 10:14, Mauro Rossi wrote: Patch changes radv entrypoints generator to not skip this extension even though it is set as disabled in the vk.xml Reference: 63525ba730 ("android: enable VK_ANDROID_native_buffer") Fixes: 69f447553c ("vulkan: Drop vk_android_native_buffer.xml") Signed-off-by: Mauro Rossi --- src/amd/vulkan/radv_entrypoints_gen.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/amd/vulkan/radv_entrypoints_gen.py b/src/amd/vulkan/radv_entrypoints_gen.py index bef0c447f6..16aa30797e 100644 --- a/src/amd/vulkan/radv_entrypoints_gen.py +++ b/src/amd/vulkan/radv_entrypoints_gen.py @@ -413,9 +413,6 @@ def get_entrypoints(doc, entrypoints_to_defines, start_index): if ext_name not in supported_exts: continue -if extension.attrib['supported'] != 'vulkan': -continue - ext = supported_exts[ext_name] ext.type = extension.attrib['type'] ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/4] android: radv: enable build of vulkan.radv HAL module
Hey Mauro, This looks good to me. Reviewed-by: Robert Foss On 2018-07-22 10:15, Mauro Rossi wrote: src/amd/Android.mk requires to include src/amd/vulkan/Android.mk to enable the build of vulkan.radv module Signed-off-by: Mauro Rossi --- src/amd/Android.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/src/amd/Android.mk b/src/amd/Android.mk index 07af05287f..6129e360cb 100644 --- a/src/amd/Android.mk +++ b/src/amd/Android.mk @@ -27,3 +27,4 @@ include $(LOCAL_PATH)/Makefile.sources include $(LOCAL_PATH)/Android.addrlib.mk include $(LOCAL_PATH)/Android.common.mk +include $(LOCAL_PATH)/vulkan/Android.mk ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] radv: move vk_format_table.c to generated sources
Hey Mauro, This looks good to me. Signed-off-by: Robert Foss On 2018-07-22 10:14, Mauro Rossi wrote: Android build system will try to compile vk_format_table.c as a shipped source, but at compile time it will be missing, we move it to generated source, where it belongs Signed-off-by: Mauro Rossi --- src/amd/vulkan/Makefile.sources | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/amd/vulkan/Makefile.sources b/src/amd/vulkan/Makefile.sources index 152fdd7cb7..53a638362b 100644 --- a/src/amd/vulkan/Makefile.sources +++ b/src/amd/vulkan/Makefile.sources @@ -69,7 +69,6 @@ VULKAN_FILES := \ radv_util.h \ radv_wsi.c \ si_cmd_buffer.c \ - vk_format_table.c \ vk_format.h \ $(RADV_WS_AMDGPU_FILES) @@ -89,5 +88,6 @@ VULKAN_GENERATED_FILES := \ radv_entrypoints.c \ radv_entrypoints.h \ radv_extensions.c \ - radv_extensions.h + radv_extensions.h \ + vk_format_table.c ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 1/4] softpipe: Add assert verifying successful pipe_transfer_map
This failure mode is a bit tricky to debug and manifests itself later as a null pointer dereference, for which finding the origin is needlessly tricky. Signed-off-by: Robert Foss Reviewed-by: Brian Paul --- Changes since v2: - Added r-b Changes since v1: - Patch added src/gallium/drivers/softpipe/sp_tile_cache.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gallium/drivers/softpipe/sp_tile_cache.c b/src/gallium/drivers/softpipe/sp_tile_cache.c index 351736ee421..211cc5ef4b0 100644 --- a/src/gallium/drivers/softpipe/sp_tile_cache.c +++ b/src/gallium/drivers/softpipe/sp_tile_cache.c @@ -209,6 +209,7 @@ sp_tile_cache_set_surface(struct softpipe_tile_cache *tc, PIPE_TRANSFER_UNSYNCHRONIZED, 0, 0, ps->width, ps->height, >transfer[i]); +assert(tc->transfer_map[i]); } } else { -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 3/4] platform/android: Enable kms_swrast fallback
Add support for the ForceSoftware option, which is togglable on the Android platform through setting the "drm.gpu.force_software" property to a non-zero value. kms_swrast is also enabled as a fallback for when a driver is not able to be loaded for for a drm node that was opened. Signed-off-by: Robert Foss Reviewed-by: Eric Engestrom --- Changes since v2: - Further remove variable/argument that is just a copy of other variable Changes since v1: - Added r-b - Removed local variable Changes since RFC: - Removed whitespace change - Switched variable type from int to EGLBoolean - Removed software renderer fallback from platform_android, since it is already implemented in _eglMatchDriver() src/egl/drivers/dri2/platform_android.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 92b2d2b343e..669881a7152 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1198,7 +1198,11 @@ droid_load_driver(_EGLDisplay *disp) struct dri2_egl_display *dri2_dpy = disp->DriverData; const char *err; - dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (disp->Options.ForceSoftware) + dri2_dpy->driver_name = strdup("kms_swrast"); + else + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) return false; @@ -1362,10 +1366,6 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) const char *err; int ret; - /* Not supported yet */ - if (disp->Options.ForceSoftware) - return EGL_FALSE; - loader_set_logger(_eglLog); dri2_dpy = calloc(1, sizeof(*dri2_dpy)); @@ -1384,10 +1384,12 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) dri2_dpy->fd = droid_open_device(disp); if (dri2_dpy->fd < 0) { - err = "DRI2: failed to open device"; + err = "DRI2: failed to open device, trying software device"; goto cleanup; } + /* Fallback to forcing software rendering is implemented using +* disp->Options.ForceSoftware in egldriver.c */ if (!droid_load_driver(disp)) { err = "DRI2: failed to load driver"; goto cleanup; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 0/4] Android kms_swrast support
This series implements kms_swrast support for the Android platform. And since having to debug a null pointer dereference, simplify that process for the next guy. As it stands now, any kernel must have the following ioctls flagged with DRM_RENDER_ALLOW[1], which isn't the case in the mainline kernel. DRM_IOCTL_MODE_CREATE_DUMB DRM_IOCTL_MODE_MAP_DUMB I've started a seperate discussion on the kernel ML[1] to seeks some more answers about what the upstream linux community views as the best solution [1] https://lkml.org/lkml/2018/7/24/187 Changes since v2: - Added r-b - Actually implement all of Erics feedback Changes since v1: - Added r-b - Added patch for asserting on failed pipe_transfer_map() Changes since RFC: - Dropped "st/dri: Allow kms_swrast to work without a device FD" - Removed software renderer fallback from platform_android - Fixed various smaller issues Rob Herring (1): android: Build kms_swrast for the Android platform Robert Foss (3): softpipe: Add assert verifying successful pipe_transfer_map egl/android: Add Android property for forcing software rendering platform/android: Enable kms_swrast fallback src/egl/drivers/dri2/platform_android.c | 14 + src/egl/main/egldriver.c | 10 ++ src/gallium/Android.mk | 2 +- src/gallium/auxiliary/pipe-loader/Android.mk | 1 + src/gallium/drivers/softpipe/Android.mk | 4 +-- src/gallium/drivers/softpipe/sp_tile_cache.c | 1 + src/gallium/state_trackers/dri/Android.mk| 1 + src/gallium/winsys/sw/kms-dri/Android.mk | 33 8 files changed, 57 insertions(+), 9 deletions(-) create mode 100644 src/gallium/winsys/sw/kms-dri/Android.mk -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 4/4] android: Build kms_swrast for the Android platform
From: Rob Herring Signed-off-by: Rob Herring Signed-off-by: Robert Foss Reviewed-by: Emil Velikov --- Changes since RFC: - Added r-b src/gallium/Android.mk | 2 +- src/gallium/auxiliary/pipe-loader/Android.mk | 1 + src/gallium/drivers/softpipe/Android.mk | 4 +-- src/gallium/state_trackers/dri/Android.mk| 1 + src/gallium/winsys/sw/kms-dri/Android.mk | 33 5 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 src/gallium/winsys/sw/kms-dri/Android.mk diff --git a/src/gallium/Android.mk b/src/gallium/Android.mk index c5f31ef0ac6..21a1620974e 100644 --- a/src/gallium/Android.mk +++ b/src/gallium/Android.mk @@ -34,7 +34,7 @@ SUBDIRS += auxiliary/pipe-loader # Gallium drivers and their respective winsys # -SUBDIRS += winsys/sw/dri drivers/softpipe +SUBDIRS += winsys/sw/kms-dri winsys/sw/dri drivers/softpipe SUBDIRS += winsys/freedreno/drm drivers/freedreno SUBDIRS += winsys/i915/drm drivers/i915 SUBDIRS += winsys/nouveau/drm drivers/nouveau diff --git a/src/gallium/auxiliary/pipe-loader/Android.mk b/src/gallium/auxiliary/pipe-loader/Android.mk index ab844143c30..075bf8af436 100644 --- a/src/gallium/auxiliary/pipe-loader/Android.mk +++ b/src/gallium/auxiliary/pipe-loader/Android.mk @@ -30,6 +30,7 @@ include $(CLEAR_VARS) LOCAL_CFLAGS := \ -DHAVE_PIPE_LOADER_DRI \ + -DHAVE_PIPE_LOADER_KMS \ -DDROP_PIPE_LOADER_MISC \ -DGALLIUM_STATIC_TARGETS diff --git a/src/gallium/drivers/softpipe/Android.mk b/src/gallium/drivers/softpipe/Android.mk index 29cc317327c..7e879161090 100644 --- a/src/gallium/drivers/softpipe/Android.mk +++ b/src/gallium/drivers/softpipe/Android.mk @@ -37,6 +37,6 @@ include $(GALLIUM_COMMON_MK) include $(BUILD_STATIC_LIBRARY) ifneq ($(HAVE_GALLIUM_SOFTPIPE),) -GALLIUM_TARGET_DRIVERS += swrast -$(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_sw_dri) +GALLIUM_TARGET_DRIVERS += swrast kms_swrast +$(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_sw_dri libmesa_winsys_sw_kms_dri) endif diff --git a/src/gallium/state_trackers/dri/Android.mk b/src/gallium/state_trackers/dri/Android.mk index a867e50d86c..d6f8a6ed247 100644 --- a/src/gallium/state_trackers/dri/Android.mk +++ b/src/gallium/state_trackers/dri/Android.mk @@ -44,6 +44,7 @@ LOCAL_STATIC_LIBRARIES := \ ifneq ($(HAVE_GALLIUM_SOFTPIPE),) LOCAL_SRC_FILES += $(drisw_SOURCES) +LOCAL_CFLAGS += -DGALLIUM_SOFTPIPE endif LOCAL_MODULE := libmesa_st_dri diff --git a/src/gallium/winsys/sw/kms-dri/Android.mk b/src/gallium/winsys/sw/kms-dri/Android.mk new file mode 100644 index 000..f5c1ccab12a --- /dev/null +++ b/src/gallium/winsys/sw/kms-dri/Android.mk @@ -0,0 +1,33 @@ +# +# Copyright (C) 2017 Linaro, Ltd., Rob Herring +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +# DEALINGS IN THE SOFTWARE. + +LOCAL_PATH := $(call my-dir) + +include $(LOCAL_PATH)/Makefile.sources + +include $(CLEAR_VARS) + +LOCAL_SRC_FILES := $(C_SOURCES) + +LOCAL_MODULE := libmesa_winsys_sw_kms_dri + +include $(GALLIUM_COMMON_MK) +include $(BUILD_STATIC_LIBRARY) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 2/4] egl/android: Add Android property for forcing software rendering
In order to simplify Android bringup on new devices, provide the property "drm.gpu.force_software" which forces kms_swrast to be used. Signed-off-by: Robert Foss Reviewed-by: Emil Velikov Reviewed-by: Eric Engestrom --- Changes since RFC: - Fixed EGLBoolean comparison - Added r-b src/egl/main/egldriver.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/egl/main/egldriver.c b/src/egl/main/egldriver.c index 218b3daef25..46814448f3d 100644 --- a/src/egl/main/egldriver.c +++ b/src/egl/main/egldriver.c @@ -39,6 +39,10 @@ #include #include "c11/threads.h" +#ifdef HAVE_ANDROID_PLATFORM +#include +#endif + #include "egldefines.h" #include "egldisplay.h" #include "egldriver.h" @@ -87,6 +91,12 @@ _eglMatchDriver(_EGLDisplay *dpy) dpy->Options.ForceSoftware = env_var_as_boolean("LIBGL_ALWAYS_SOFTWARE", false); +#ifdef HAVE_ANDROID_PLATFORM + char prop_val[PROPERTY_VALUE_MAX]; + property_get("drm.gpu.force_software", prop_val, "0"); + dpy->Options.ForceSoftware |= strncmp(prop_val, "0", PROPERTY_VALUE_MAX) != 0; +#endif + best_drv = _eglMatchAndInitialize(dpy); if (!best_drv && !dpy->Options.ForceSoftware) { dpy->Options.ForceSoftware = EGL_TRUE; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 0/4] Android kms_swrast support
Hey Rob, On 2018-07-19 09:26, Tomasz Figa wrote: On Thu, Jul 19, 2018 at 12:08 AM Robert Foss wrote: Hey Rob, On 2018-07-18 15:30, Rob Herring wrote: On Tue, Jul 17, 2018 at 4:33 AM Robert Foss wrote: This series implements kms_swrast support for the Android platform. And since having to debug a null pointer dereference, simplify that process for the next guy. So is this working for you now? I'm seeing page-flips happen in the logs, but have no graphical output on the Qemu-based setup I'm using now. When using virgl I'm seeing the same page-flipping in the logs, but no graphical output. As it stands now, any kernel must have the following ioctls flagged with DRM_RENDER_ALLOW[1], which isn't the case in the mainline kernel. DRM_IOCTL_MODE_CREATE_DUMB DRM_IOCTL_MODE_MAP_DUMB Ah, sorry. I should have mentioned this. We have discussed this issue in the past, but to no further conclusion. But as I recall, I thought the issue was also allowing import and export of dumb buffers? Yeah, it's a two-parter for any AOSP Treble build. 1) Allow dumb buffer ioctls fom render nodes 2) Support moving buffers across processes. Wouldn't 2) be automatically solved by 1), since we should be able to run drmPrimeHandleToFD for dumb buffers already? I thought, perhaps wrongly that drmPrimeHandleToFD was only applicable to dmabufs. I think I've misunderstood the restrictions of dumb buffers, if they're shareable across processes using drmPrimeHandleToFD then only 1) is in our way. While it would be possible to open a non-render node to pass the authentication check, this would still cause authentication issues when the /dev/dri/cardX node needs to be opened as master by both mesa and the compositor. Right. We've pretty much stripped the support that was there out. Plus I don't think it will work with Treble. I don't know how acceptable this series is for upstreaming, while relying on a non-mainline kernel. I think the policy is to not accept changes that don't have both a user and kernel space solution in place. Like I noted yesterday[2] the alternative to using dumb buffers and having authentication issues is using VGEM, which is new territory to me, and it would take me a little bit of time to figure exactly how it fits into the current kms_swrast approach. Input, like noted before, is very much welcome. I'm very much in favor of the former approach. VGEM seems like an overly complicated solution when there's a very simple solution. The former solution being what we have now, dumb buffers? I don't think dumb buffers are a viable path due to 2) listed above. I don't understand what 2) is about. Could you elaborate on it? See above! I'd personally be for dropping those strange restrictions from render nodes. I don't see why a render node couldn't allocate and map a dumb buffer (for software rendering) and share it with another process that opened a control node (to display it). From my understanding the wider communitys idea is to minimize the use of dumb buffers. A part of not allowing render nodes to use map dumb buffers is meant to incentivize proprietary drivers to not do the simplest thing that could possibly work, as far as I understand. So while I'm happy to push that change upstream, if for no other reason than to generate a dialogue, maybe it's not all that likely that it will be accepted. If there are any other options I'm not aware of, I'm very much listening. One could just call mmap() on DMA-buf FDs directly rather than importing them, but that could open another can of worms, because FDs don't give us any way to deduplicate buffers (you might be given several FDs pointing to the same buffer, which in case of importing to DRM would end up with the same GEM handle every time). So mmap()ing dmabuf FDs dealing with that can of worms is preferable to looking into a VGEM based approach? Rob Herring, presumably rightly, seems to think going down the VGEM route is opening a pretty bad can of worms too. Rob. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 0/4] Android kms_swrast support
Hey Rob, On 2018-07-18 15:30, Rob Herring wrote: On Tue, Jul 17, 2018 at 4:33 AM Robert Foss wrote: This series implements kms_swrast support for the Android platform. And since having to debug a null pointer dereference, simplify that process for the next guy. So is this working for you now? I'm seeing page-flips happen in the logs, but have no graphical output on the Qemu-based setup I'm using now. When using virgl I'm seeing the same page-flipping in the logs, but no graphical output. As it stands now, any kernel must have the following ioctls flagged with DRM_RENDER_ALLOW[1], which isn't the case in the mainline kernel. DRM_IOCTL_MODE_CREATE_DUMB DRM_IOCTL_MODE_MAP_DUMB Ah, sorry. I should have mentioned this. We have discussed this issue in the past, but to no further conclusion. But as I recall, I thought the issue was also allowing import and export of dumb buffers? Yeah, it's a two-parter for any AOSP Treble build. 1) Allow dumb buffer ioctls fom render nodes 2) Support moving buffers across processes. While it would be possible to open a non-render node to pass the authentication check, this would still cause authentication issues when the /dev/dri/cardX node needs to be opened as master by both mesa and the compositor. Right. We've pretty much stripped the support that was there out. Plus I don't think it will work with Treble. I don't know how acceptable this series is for upstreaming, while relying on a non-mainline kernel. I think the policy is to not accept changes that don't have both a user and kernel space solution in place. Like I noted yesterday[2] the alternative to using dumb buffers and having authentication issues is using VGEM, which is new territory to me, and it would take me a little bit of time to figure exactly how it fits into the current kms_swrast approach. Input, like noted before, is very much welcome. I'm very much in favor of the former approach. VGEM seems like an overly complicated solution when there's a very simple solution. The former solution being what we have now, dumb buffers? I don't think dumb buffers are a viable path due to 2) listed above. If there are any other options I'm not aware of, I'm very much listening. Rob. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 1/4] softpipe: Add assert verifying successful pipe_transfer_map
Thanks Brian! On 2018-07-17 16:18, Brian Paul wrote: On 07/17/2018 04:32 AM, Robert Foss wrote: This failure mode is a bit tricky to debug and manifests itself later as a null pointer dereference, for which finding the origin is needlessly tricky. Signed-off-by: Robert Foss --- Changes since v1: - Patch added src/gallium/drivers/softpipe/sp_tile_cache.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gallium/drivers/softpipe/sp_tile_cache.c b/src/gallium/drivers/softpipe/sp_tile_cache.c index 351736ee421..211cc5ef4b0 100644 --- a/src/gallium/drivers/softpipe/sp_tile_cache.c +++ b/src/gallium/drivers/softpipe/sp_tile_cache.c @@ -209,6 +209,7 @@ sp_tile_cache_set_surface(struct softpipe_tile_cache *tc, PIPE_TRANSFER_UNSYNCHRONIZED, 0, 0, ps->width, ps->height, >transfer[i]); + assert(tc->transfer_map[i]); } } else { Looks OK to me. Reviewed-by: Brian Paul ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 3/4] platform/android: Enable kms_swrast fallback
Hey Eric, Thanks for the review, adding you r-b. On 2018-07-17 12:48, Eric Engestrom wrote: On Tuesday, 2018-07-17 12:32:59 +0200, Robert Foss wrote: Add support for the ForceSoftware option, which is togglable on the Android platform through setting the "drm.gpu.force_software" property to a non-zero value. kms_swrast is also enabled as a fallback for when a driver is not able to be loaded for for a drm node that was opened. Signed-off-by: Robert Foss Reviewed-by: Eric Engestrom --- Changes since v1: - Added r-b - Removed local variable Changes since RFC: - Removed whitespace change - Switched variable type from int to EGLBoolean - Removed software renderer fallback from platform_android, since it is already implemented in _eglMatchDriver() src/egl/drivers/dri2/platform_android.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 92b2d2b343e..d17b2fb6073 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1193,12 +1193,16 @@ static const __DRIextension *droid_image_loader_extensions[] = { }; EGLBoolean -droid_load_driver(_EGLDisplay *disp) +droid_load_driver(_EGLDisplay *disp, EGLBoolean force_software) { struct dri2_egl_display *dri2_dpy = disp->DriverData; const char *err; - dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (force_software) My suggestion was actually to use `disp->Options.ForceSoftware` directly here, instead of passing both `disp` and `disp->Options.ForceSoftware` as parameters :) + dri2_dpy->driver_name = strdup("kms_swrast"); + else + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) return false; @@ -1362,10 +1366,6 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) const char *err; int ret; - /* Not supported yet */ - if (disp->Options.ForceSoftware) - return EGL_FALSE; - loader_set_logger(_eglLog); dri2_dpy = calloc(1, sizeof(*dri2_dpy)); @@ -1384,11 +1384,13 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) dri2_dpy->fd = droid_open_device(disp); if (dri2_dpy->fd < 0) { - err = "DRI2: failed to open device"; + err = "DRI2: failed to open device, trying software device"; goto cleanup; } - if (!droid_load_driver(disp)) { + /* Fallback to forcing software rendering is implemented using +* disp->Options.ForceSoftware in egldriver.c */ + if (!droid_load_driver(disp, disp->Options.ForceSoftware)) { err = "DRI2: failed to load driver"; goto cleanup; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 3/4] platform/android: Enable kms_swrast fallback
Hey Eric, On 2018-07-17 12:48, Eric Engestrom wrote: On Tuesday, 2018-07-17 12:32:59 +0200, Robert Foss wrote: Add support for the ForceSoftware option, which is togglable on the Android platform through setting the "drm.gpu.force_software" property to a non-zero value. kms_swrast is also enabled as a fallback for when a driver is not able to be loaded for for a drm node that was opened. Signed-off-by: Robert Foss Reviewed-by: Eric Engestrom --- Changes since v1: - Added r-b - Removed local variable Changes since RFC: - Removed whitespace change - Switched variable type from int to EGLBoolean - Removed software renderer fallback from platform_android, since it is already implemented in _eglMatchDriver() src/egl/drivers/dri2/platform_android.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 92b2d2b343e..d17b2fb6073 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1193,12 +1193,16 @@ static const __DRIextension *droid_image_loader_extensions[] = { }; EGLBoolean -droid_load_driver(_EGLDisplay *disp) +droid_load_driver(_EGLDisplay *disp, EGLBoolean force_software) { struct dri2_egl_display *dri2_dpy = disp->DriverData; const char *err; - dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (force_software) My suggestion was actually to use `disp->Options.ForceSoftware` directly here, instead of passing both `disp` and `disp->Options.ForceSoftware` as parameters :) Ah, of course. Fixing that. + dri2_dpy->driver_name = strdup("kms_swrast"); + else + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) return false; @@ -1362,10 +1366,6 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) const char *err; int ret; - /* Not supported yet */ - if (disp->Options.ForceSoftware) - return EGL_FALSE; - loader_set_logger(_eglLog); dri2_dpy = calloc(1, sizeof(*dri2_dpy)); @@ -1384,11 +1384,13 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) dri2_dpy->fd = droid_open_device(disp); if (dri2_dpy->fd < 0) { - err = "DRI2: failed to open device"; + err = "DRI2: failed to open device, trying software device"; goto cleanup; } - if (!droid_load_driver(disp)) { + /* Fallback to forcing software rendering is implemented using +* disp->Options.ForceSoftware in egldriver.c */ + if (!droid_load_driver(disp, disp->Options.ForceSoftware)) { err = "DRI2: failed to load driver"; goto cleanup; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 1/4] softpipe: Add assert verifying successful pipe_transfer_map
This failure mode is a bit tricky to debug and manifests itself later as a null pointer dereference, for which finding the origin is needlessly tricky. Signed-off-by: Robert Foss --- Changes since v1: - Patch added src/gallium/drivers/softpipe/sp_tile_cache.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gallium/drivers/softpipe/sp_tile_cache.c b/src/gallium/drivers/softpipe/sp_tile_cache.c index 351736ee421..211cc5ef4b0 100644 --- a/src/gallium/drivers/softpipe/sp_tile_cache.c +++ b/src/gallium/drivers/softpipe/sp_tile_cache.c @@ -209,6 +209,7 @@ sp_tile_cache_set_surface(struct softpipe_tile_cache *tc, PIPE_TRANSFER_UNSYNCHRONIZED, 0, 0, ps->width, ps->height, >transfer[i]); +assert(tc->transfer_map[i]); } } else { -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 0/4] Android kms_swrast support
This series implements kms_swrast support for the Android platform. And since having to debug a null pointer dereference, simplify that process for the next guy. As it stands now, any kernel must have the following ioctls flagged with DRM_RENDER_ALLOW[1], which isn't the case in the mainline kernel. DRM_IOCTL_MODE_CREATE_DUMB DRM_IOCTL_MODE_MAP_DUMB While it would be possible to open a non-render node to pass the authentication check, this would still cause authentication issues when the /dev/dri/cardX node needs to be opened as master by both mesa and the compositor. I don't know how acceptable this series is for upstreaming, while relying on a non-mainline kernel. I think the policy is to not accept changes that don't have both a user and kernel space solution in place. Like I noted yesterday[2] the alternative to using dumb buffers and having authentication issues is using VGEM, which is new territory to me, and it would take me a little bit of time to figure exactly how it fits into the current kms_swrast approach. Input, like noted before, is very much welcome. The series is available here: https://gitlab.collabora.com/robertfoss/mesa/tree/kms_swrast_v2 [1] https://groups.google.com/a/chromium.org/forum/#!topic/chromium-os-reviews/5nOxn-JXJUk [2] https://lists.freedesktop.org/archives/mesa-dev/2018-July/200229.html Changes since v1: - Added r-b - Added patch for asserting on failed pipe_transfer_map() Changes since RFC: - Dropped "st/dri: Allow kms_swrast to work without a device FD" - Removed software renderer fallback from platform_android - Fixed various smaller issues Rob Herring (1): android: Build kms_swrast for the Android platform Robert Foss (3): softpipe: Add assert verifying successful pipe_transfer_map egl/android: Add Android property for forcing software rendering platform/android: Enable kms_swrast fallback src/egl/drivers/dri2/platform_android.c | 18 ++- src/egl/main/egldriver.c | 10 ++ src/gallium/Android.mk | 2 +- src/gallium/auxiliary/pipe-loader/Android.mk | 1 + src/gallium/drivers/softpipe/Android.mk | 4 +-- src/gallium/drivers/softpipe/sp_tile_cache.c | 1 + src/gallium/state_trackers/dri/Android.mk| 1 + src/gallium/winsys/sw/kms-dri/Android.mk | 33 8 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 src/gallium/winsys/sw/kms-dri/Android.mk -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 4/4] android: Build kms_swrast for the Android platform
From: Rob Herring Signed-off-by: Rob Herring Signed-off-by: Robert Foss Reviewed-by: Emil Velikov --- Changes since RFC: - Added r-b src/gallium/Android.mk | 2 +- src/gallium/auxiliary/pipe-loader/Android.mk | 1 + src/gallium/drivers/softpipe/Android.mk | 4 +-- src/gallium/state_trackers/dri/Android.mk| 1 + src/gallium/winsys/sw/kms-dri/Android.mk | 33 5 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 src/gallium/winsys/sw/kms-dri/Android.mk diff --git a/src/gallium/Android.mk b/src/gallium/Android.mk index c5f31ef0ac6..21a1620974e 100644 --- a/src/gallium/Android.mk +++ b/src/gallium/Android.mk @@ -34,7 +34,7 @@ SUBDIRS += auxiliary/pipe-loader # Gallium drivers and their respective winsys # -SUBDIRS += winsys/sw/dri drivers/softpipe +SUBDIRS += winsys/sw/kms-dri winsys/sw/dri drivers/softpipe SUBDIRS += winsys/freedreno/drm drivers/freedreno SUBDIRS += winsys/i915/drm drivers/i915 SUBDIRS += winsys/nouveau/drm drivers/nouveau diff --git a/src/gallium/auxiliary/pipe-loader/Android.mk b/src/gallium/auxiliary/pipe-loader/Android.mk index ab844143c30..075bf8af436 100644 --- a/src/gallium/auxiliary/pipe-loader/Android.mk +++ b/src/gallium/auxiliary/pipe-loader/Android.mk @@ -30,6 +30,7 @@ include $(CLEAR_VARS) LOCAL_CFLAGS := \ -DHAVE_PIPE_LOADER_DRI \ + -DHAVE_PIPE_LOADER_KMS \ -DDROP_PIPE_LOADER_MISC \ -DGALLIUM_STATIC_TARGETS diff --git a/src/gallium/drivers/softpipe/Android.mk b/src/gallium/drivers/softpipe/Android.mk index 29cc317327c..7e879161090 100644 --- a/src/gallium/drivers/softpipe/Android.mk +++ b/src/gallium/drivers/softpipe/Android.mk @@ -37,6 +37,6 @@ include $(GALLIUM_COMMON_MK) include $(BUILD_STATIC_LIBRARY) ifneq ($(HAVE_GALLIUM_SOFTPIPE),) -GALLIUM_TARGET_DRIVERS += swrast -$(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_sw_dri) +GALLIUM_TARGET_DRIVERS += swrast kms_swrast +$(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_sw_dri libmesa_winsys_sw_kms_dri) endif diff --git a/src/gallium/state_trackers/dri/Android.mk b/src/gallium/state_trackers/dri/Android.mk index a867e50d86c..d6f8a6ed247 100644 --- a/src/gallium/state_trackers/dri/Android.mk +++ b/src/gallium/state_trackers/dri/Android.mk @@ -44,6 +44,7 @@ LOCAL_STATIC_LIBRARIES := \ ifneq ($(HAVE_GALLIUM_SOFTPIPE),) LOCAL_SRC_FILES += $(drisw_SOURCES) +LOCAL_CFLAGS += -DGALLIUM_SOFTPIPE endif LOCAL_MODULE := libmesa_st_dri diff --git a/src/gallium/winsys/sw/kms-dri/Android.mk b/src/gallium/winsys/sw/kms-dri/Android.mk new file mode 100644 index 000..f5c1ccab12a --- /dev/null +++ b/src/gallium/winsys/sw/kms-dri/Android.mk @@ -0,0 +1,33 @@ +# +# Copyright (C) 2017 Linaro, Ltd., Rob Herring +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +# DEALINGS IN THE SOFTWARE. + +LOCAL_PATH := $(call my-dir) + +include $(LOCAL_PATH)/Makefile.sources + +include $(CLEAR_VARS) + +LOCAL_SRC_FILES := $(C_SOURCES) + +LOCAL_MODULE := libmesa_winsys_sw_kms_dri + +include $(GALLIUM_COMMON_MK) +include $(BUILD_STATIC_LIBRARY) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 3/4] platform/android: Enable kms_swrast fallback
Add support for the ForceSoftware option, which is togglable on the Android platform through setting the "drm.gpu.force_software" property to a non-zero value. kms_swrast is also enabled as a fallback for when a driver is not able to be loaded for for a drm node that was opened. Signed-off-by: Robert Foss Reviewed-by: Eric Engestrom --- Changes since v1: - Added r-b - Removed local variable Changes since RFC: - Removed whitespace change - Switched variable type from int to EGLBoolean - Removed software renderer fallback from platform_android, since it is already implemented in _eglMatchDriver() src/egl/drivers/dri2/platform_android.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 92b2d2b343e..d17b2fb6073 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1193,12 +1193,16 @@ static const __DRIextension *droid_image_loader_extensions[] = { }; EGLBoolean -droid_load_driver(_EGLDisplay *disp) +droid_load_driver(_EGLDisplay *disp, EGLBoolean force_software) { struct dri2_egl_display *dri2_dpy = disp->DriverData; const char *err; - dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (force_software) + dri2_dpy->driver_name = strdup("kms_swrast"); + else + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) return false; @@ -1362,10 +1366,6 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) const char *err; int ret; - /* Not supported yet */ - if (disp->Options.ForceSoftware) - return EGL_FALSE; - loader_set_logger(_eglLog); dri2_dpy = calloc(1, sizeof(*dri2_dpy)); @@ -1384,11 +1384,13 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) dri2_dpy->fd = droid_open_device(disp); if (dri2_dpy->fd < 0) { - err = "DRI2: failed to open device"; + err = "DRI2: failed to open device, trying software device"; goto cleanup; } - if (!droid_load_driver(disp)) { + /* Fallback to forcing software rendering is implemented using +* disp->Options.ForceSoftware in egldriver.c */ + if (!droid_load_driver(disp, disp->Options.ForceSoftware)) { err = "DRI2: failed to load driver"; goto cleanup; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 2/4] egl/android: Add Android property for forcing software rendering
In order to simplify Android bringup on new devices, provide the property "drm.gpu.force_software" which forces kms_swrast to be used. Signed-off-by: Robert Foss Reviewed-by: Emil Velikov --- Changes since RFC: - Fixed EGLBoolean comparison - Added r-b src/egl/main/egldriver.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/egl/main/egldriver.c b/src/egl/main/egldriver.c index 218b3daef25..46814448f3d 100644 --- a/src/egl/main/egldriver.c +++ b/src/egl/main/egldriver.c @@ -39,6 +39,10 @@ #include #include "c11/threads.h" +#ifdef HAVE_ANDROID_PLATFORM +#include +#endif + #include "egldefines.h" #include "egldisplay.h" #include "egldriver.h" @@ -87,6 +91,12 @@ _eglMatchDriver(_EGLDisplay *dpy) dpy->Options.ForceSoftware = env_var_as_boolean("LIBGL_ALWAYS_SOFTWARE", false); +#ifdef HAVE_ANDROID_PLATFORM + char prop_val[PROPERTY_VALUE_MAX]; + property_get("drm.gpu.force_software", prop_val, "0"); + dpy->Options.ForceSoftware |= strncmp(prop_val, "0", PROPERTY_VALUE_MAX) != 0; +#endif + best_drv = _eglMatchAndInitialize(dpy); if (!best_drv && !dpy->Options.ForceSoftware) { dpy->Options.ForceSoftware = EGL_TRUE; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v1 0/3] Android kms_swrast support
Hey, On 2018-07-09 13:01, Robert Foss wrote: NOTE: This series has not been tested successfully, and I'm seeing a segfault during the boot process. Which I'm currently looking into. Tracking down the segfault I've been seeing through mesa and aosp and bootanimation. It occurs when ioctl authentication causes DRM_IOCTL_MODE_MAP_DUMB to fail for render nodes. Disabling the authentication using [1] fixes the problem. Currently the kms_sw_displaytarget_map() function in mesa will always use DRM_IOCTL_MODE_MAP_DUMB. To avoid having to use [1], dumb buffers must be avoided. Which means looking at VGEM. Exactly what this would look like I'm not quite sure of yet. But I would like some input. [1] https://groups.google.com/a/chromium.org/forum/#!topic/chromium-os-reviews/5nOxn-JXJUk This series implements kms_swrast support for the Android platform. It's available here: https://gitlab.collabora.com/robertfoss/mesa/tree/kms_swrast_v1 and here with some debug: https://gitlab.collabora.com/robertfoss/mesa/tree/kms_swrast_v1_debug Changes since RFC: - Dropped "st/dri: Allow kms_swrast to work without a device FD" - Removed software renderer fallback from platform_android - Fixed various smaller issues Rob. Rob Herring (1): android: Build kms_swrast for the Android platform Robert Foss (2): egl/android: Add Android property for forcing software rendering platform/android: Enable kms_swrast fallback src/egl/drivers/dri2/platform_android.c | 19 ++- src/egl/main/egldriver.c | 10 ++ src/gallium/Android.mk | 2 +- src/gallium/auxiliary/pipe-loader/Android.mk | 1 + src/gallium/drivers/softpipe/Android.mk | 4 +-- src/gallium/state_trackers/dri/Android.mk| 1 + src/gallium/winsys/sw/kms-dri/Android.mk | 33 7 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 src/gallium/winsys/sw/kms-dri/Android.mk ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v1 0/3] Android kms_swrast support
Hey Mauro, On 09/07/18 14:27, Mauro Rossi wrote: Hi Robert, Il giorno lun 9 lug 2018 alle ore 13:01 Robert Foss <mailto:robert.f...@collabora.com>> ha scritto: NOTE: This series has not been tested successfully, and I'm seeing a segfault during the boot process. Which I'm currently looking into. What is your testing setup (hwcomposer, gralloc)? I am asking because I am seeing segfaults with other drivers with drm_hwcomposer (master) + gralloc.gbm (handle-rework-v2) and also I would propose to verify also no regression on gralloc.drm. That's a good data point. With the virgl driver, I've had no segfaults and gotten a bit further, but not to any actual visual BootAnimation or desktop. Since this series should be compatible with the rock stable gralloc.drm, I could perform some tests with former stack ( gralloc.drm pipe w/o hwcomposer) where compatibility should be guaranteed by BOARD_USES_DRM_GRALLOC:=true in BoardConfig.mk and report back, if useful. That would be quite welcome. Note that "drm.gpu.force_software" property has to be set to '1'. Please let me ask also a question to Rob Herring about gralloc.gbm compliance with gralloc0 w/o hwcomposer, can we also rely on this stack/configuration, but testing it only with mesa having prime fd support? Mauro This series implements kms_swrast support for the Android platform. It's available here: https://gitlab.collabora.com/robertfoss/mesa/tree/kms_swrast_v1 and here with some debug: https://gitlab.collabora.com/robertfoss/mesa/tree/kms_swrast_v1_debug Changes since RFC: - Dropped "st/dri: Allow kms_swrast to work without a device FD" - Removed software renderer fallback from platform_android - Fixed various smaller issues Rob. Rob Herring (1): android: Build kms_swrast for the Android platform Robert Foss (2): egl/android: Add Android property for forcing software rendering platform/android: Enable kms_swrast fallback src/egl/drivers/dri2/platform_android.c | 19 ++- src/egl/main/egldriver.c | 10 ++ src/gallium/Android.mk | 2 +- src/gallium/auxiliary/pipe-loader/Android.mk | 1 + src/gallium/drivers/softpipe/Android.mk | 4 +-- src/gallium/state_trackers/dri/Android.mk | 1 + src/gallium/winsys/sw/kms-dri/Android.mk | 33 7 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 src/gallium/winsys/sw/kms-dri/Android.mk -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v1 2/3] platform/android: Enable kms_swrast fallback
Hey Eric, Thanks for the quick reviews! On 09/07/18 15:45, Eric Engestrom wrote: On Monday, 2018-07-09 13:01:49 +0200, Robert Foss wrote: Add support for the ForceSoftware option, which is togglable on the Android platform through setting the "drm.gpu.force_software" property to a non-zero value. kms_swrast is also enabled as a fallback for when a driver is not able to be loaded for for a drm node that was opened. Signed-off-by: Robert Foss --- Changes since RFC: - Removed whitespace change - Switched variable type from int to EGLBoolean - Removed software renderer fallback from platform_android, since it is already implemented in _eglMatchDriver() src/egl/drivers/dri2/platform_android.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 92b2d2b343e..14a69abbc04 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1193,12 +1193,16 @@ static const __DRIextension *droid_image_loader_extensions[] = { }; EGLBoolean -droid_load_driver(_EGLDisplay *disp) +droid_load_driver(_EGLDisplay *disp, EGLBoolean force_software) { struct dri2_egl_display *dri2_dpy = disp->DriverData; const char *err; - dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (force_software) You could use `disp->Options.ForceSoftware` directly here, and drop the added parameter as well as the local var below and the call change. With that: Reviewed-by: Eric Engestrom Ack. + dri2_dpy->driver_name = strdup("kms_swrast"); + else + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) return false; @@ -1359,13 +1363,10 @@ EGLBoolean dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) { struct dri2_egl_display *dri2_dpy; + EGLBoolean force_software = disp->Options.ForceSoftware; const char *err; int ret; - /* Not supported yet */ - if (disp->Options.ForceSoftware) - return EGL_FALSE; - loader_set_logger(_eglLog); dri2_dpy = calloc(1, sizeof(*dri2_dpy)); @@ -1384,11 +1385,13 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) dri2_dpy->fd = droid_open_device(disp); if (dri2_dpy->fd < 0) { - err = "DRI2: failed to open device"; + err = "DRI2: failed to open device, trying software device"; goto cleanup; } - if (!droid_load_driver(disp)) { + /* Fallback to forcing software rendering is implemented using +* disp->Options.ForceSoftware in egldriver.c */ + if (!droid_load_driver(disp, force_software)) { err = "DRI2: failed to load driver"; goto cleanup; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v1 0/3] Android kms_swrast support
NOTE: This series has not been tested successfully, and I'm seeing a segfault during the boot process. Which I'm currently looking into. This series implements kms_swrast support for the Android platform. It's available here: https://gitlab.collabora.com/robertfoss/mesa/tree/kms_swrast_v1 and here with some debug: https://gitlab.collabora.com/robertfoss/mesa/tree/kms_swrast_v1_debug Changes since RFC: - Dropped "st/dri: Allow kms_swrast to work without a device FD" - Removed software renderer fallback from platform_android - Fixed various smaller issues Rob. Rob Herring (1): android: Build kms_swrast for the Android platform Robert Foss (2): egl/android: Add Android property for forcing software rendering platform/android: Enable kms_swrast fallback src/egl/drivers/dri2/platform_android.c | 19 ++- src/egl/main/egldriver.c | 10 ++ src/gallium/Android.mk | 2 +- src/gallium/auxiliary/pipe-loader/Android.mk | 1 + src/gallium/drivers/softpipe/Android.mk | 4 +-- src/gallium/state_trackers/dri/Android.mk| 1 + src/gallium/winsys/sw/kms-dri/Android.mk | 33 7 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 src/gallium/winsys/sw/kms-dri/Android.mk -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v1 1/3] egl/android: Add Android property for forcing software rendering
In order to simplify Android bringup on new devices, provide the property "drm.gpu.force_software" which forces kms_swrast to be used. Signed-off-by: Robert Foss Reviewed-by: Emil Velikov --- Changes since RFC: - Fixed EGLBoolean comparison - Added r-b src/egl/main/egldriver.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/egl/main/egldriver.c b/src/egl/main/egldriver.c index 218b3daef25..46814448f3d 100644 --- a/src/egl/main/egldriver.c +++ b/src/egl/main/egldriver.c @@ -39,6 +39,10 @@ #include #include "c11/threads.h" +#ifdef HAVE_ANDROID_PLATFORM +#include +#endif + #include "egldefines.h" #include "egldisplay.h" #include "egldriver.h" @@ -87,6 +91,12 @@ _eglMatchDriver(_EGLDisplay *dpy) dpy->Options.ForceSoftware = env_var_as_boolean("LIBGL_ALWAYS_SOFTWARE", false); +#ifdef HAVE_ANDROID_PLATFORM + char prop_val[PROPERTY_VALUE_MAX]; + property_get("drm.gpu.force_software", prop_val, "0"); + dpy->Options.ForceSoftware |= strncmp(prop_val, "0", PROPERTY_VALUE_MAX) != 0; +#endif + best_drv = _eglMatchAndInitialize(dpy); if (!best_drv && !dpy->Options.ForceSoftware) { dpy->Options.ForceSoftware = EGL_TRUE; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v1 3/3] android: Build kms_swrast for the Android platform
From: Rob Herring Signed-off-by: Rob Herring Signed-off-by: Robert Foss Reviewed-by: Emil Velikov --- Changes since RFC: - Added r-b src/gallium/Android.mk | 2 +- src/gallium/auxiliary/pipe-loader/Android.mk | 1 + src/gallium/drivers/softpipe/Android.mk | 4 +-- src/gallium/state_trackers/dri/Android.mk| 1 + src/gallium/winsys/sw/kms-dri/Android.mk | 33 5 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 src/gallium/winsys/sw/kms-dri/Android.mk diff --git a/src/gallium/Android.mk b/src/gallium/Android.mk index c5f31ef0ac6..21a1620974e 100644 --- a/src/gallium/Android.mk +++ b/src/gallium/Android.mk @@ -34,7 +34,7 @@ SUBDIRS += auxiliary/pipe-loader # Gallium drivers and their respective winsys # -SUBDIRS += winsys/sw/dri drivers/softpipe +SUBDIRS += winsys/sw/kms-dri winsys/sw/dri drivers/softpipe SUBDIRS += winsys/freedreno/drm drivers/freedreno SUBDIRS += winsys/i915/drm drivers/i915 SUBDIRS += winsys/nouveau/drm drivers/nouveau diff --git a/src/gallium/auxiliary/pipe-loader/Android.mk b/src/gallium/auxiliary/pipe-loader/Android.mk index ab844143c30..075bf8af436 100644 --- a/src/gallium/auxiliary/pipe-loader/Android.mk +++ b/src/gallium/auxiliary/pipe-loader/Android.mk @@ -30,6 +30,7 @@ include $(CLEAR_VARS) LOCAL_CFLAGS := \ -DHAVE_PIPE_LOADER_DRI \ + -DHAVE_PIPE_LOADER_KMS \ -DDROP_PIPE_LOADER_MISC \ -DGALLIUM_STATIC_TARGETS diff --git a/src/gallium/drivers/softpipe/Android.mk b/src/gallium/drivers/softpipe/Android.mk index 29cc317327c..7e879161090 100644 --- a/src/gallium/drivers/softpipe/Android.mk +++ b/src/gallium/drivers/softpipe/Android.mk @@ -37,6 +37,6 @@ include $(GALLIUM_COMMON_MK) include $(BUILD_STATIC_LIBRARY) ifneq ($(HAVE_GALLIUM_SOFTPIPE),) -GALLIUM_TARGET_DRIVERS += swrast -$(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_sw_dri) +GALLIUM_TARGET_DRIVERS += swrast kms_swrast +$(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_sw_dri libmesa_winsys_sw_kms_dri) endif diff --git a/src/gallium/state_trackers/dri/Android.mk b/src/gallium/state_trackers/dri/Android.mk index a867e50d86c..d6f8a6ed247 100644 --- a/src/gallium/state_trackers/dri/Android.mk +++ b/src/gallium/state_trackers/dri/Android.mk @@ -44,6 +44,7 @@ LOCAL_STATIC_LIBRARIES := \ ifneq ($(HAVE_GALLIUM_SOFTPIPE),) LOCAL_SRC_FILES += $(drisw_SOURCES) +LOCAL_CFLAGS += -DGALLIUM_SOFTPIPE endif LOCAL_MODULE := libmesa_st_dri diff --git a/src/gallium/winsys/sw/kms-dri/Android.mk b/src/gallium/winsys/sw/kms-dri/Android.mk new file mode 100644 index 000..f5c1ccab12a --- /dev/null +++ b/src/gallium/winsys/sw/kms-dri/Android.mk @@ -0,0 +1,33 @@ +# +# Copyright (C) 2017 Linaro, Ltd., Rob Herring +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +# DEALINGS IN THE SOFTWARE. + +LOCAL_PATH := $(call my-dir) + +include $(LOCAL_PATH)/Makefile.sources + +include $(CLEAR_VARS) + +LOCAL_SRC_FILES := $(C_SOURCES) + +LOCAL_MODULE := libmesa_winsys_sw_kms_dri + +include $(GALLIUM_COMMON_MK) +include $(BUILD_STATIC_LIBRARY) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v1 2/3] platform/android: Enable kms_swrast fallback
Add support for the ForceSoftware option, which is togglable on the Android platform through setting the "drm.gpu.force_software" property to a non-zero value. kms_swrast is also enabled as a fallback for when a driver is not able to be loaded for for a drm node that was opened. Signed-off-by: Robert Foss --- Changes since RFC: - Removed whitespace change - Switched variable type from int to EGLBoolean - Removed software renderer fallback from platform_android, since it is already implemented in _eglMatchDriver() src/egl/drivers/dri2/platform_android.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 92b2d2b343e..14a69abbc04 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1193,12 +1193,16 @@ static const __DRIextension *droid_image_loader_extensions[] = { }; EGLBoolean -droid_load_driver(_EGLDisplay *disp) +droid_load_driver(_EGLDisplay *disp, EGLBoolean force_software) { struct dri2_egl_display *dri2_dpy = disp->DriverData; const char *err; - dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (force_software) + dri2_dpy->driver_name = strdup("kms_swrast"); + else + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) return false; @@ -1359,13 +1363,10 @@ EGLBoolean dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) { struct dri2_egl_display *dri2_dpy; + EGLBoolean force_software = disp->Options.ForceSoftware; const char *err; int ret; - /* Not supported yet */ - if (disp->Options.ForceSoftware) - return EGL_FALSE; - loader_set_logger(_eglLog); dri2_dpy = calloc(1, sizeof(*dri2_dpy)); @@ -1384,11 +1385,13 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) dri2_dpy->fd = droid_open_device(disp); if (dri2_dpy->fd < 0) { - err = "DRI2: failed to open device"; + err = "DRI2: failed to open device, trying software device"; goto cleanup; } - if (!droid_load_driver(disp)) { + /* Fallback to forcing software rendering is implemented using +* disp->Options.ForceSoftware in egldriver.c */ + if (!droid_load_driver(disp, force_software)) { err = "DRI2: failed to load driver"; goto cleanup; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 3/5] platform/android: Enable kms_swrast fallback
Hey Emil, On 05/07/18 15:04, Emil Velikov wrote: Hi Rob, On 5 July 2018 at 11:07, Robert Foss wrote: Add support for the ForceSoftware option, which is togglable on the Android platform through setting the "drm.gpu.force_software" property to a non-zero value. kms_swrast is also enabled as a fallback for when a driver is not able to be loaded for for a drm node that was opened. Signed-off-by: Robert Foss --- src/egl/drivers/dri2/platform_android.c | 26 + 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 92b2d2b343e..bc644c25bf9 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1193,17 +1193,21 @@ static const __DRIextension *droid_image_loader_extensions[] = { }; EGLBoolean -droid_load_driver(_EGLDisplay *disp) +droid_load_driver(_EGLDisplay *disp, EGLBoolean force_software) { struct dri2_egl_display *dri2_dpy = disp->DriverData; const char *err; - dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (force_software) { + dri2_dpy->driver_name = strdup("kms_swrast"); + } else { + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + } + Nit: no brackets are needed here. Ack. if (dri2_dpy->driver_name == NULL) return false; dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER; - if (!dri2_dpy->is_render_node) { #ifdef HAVE_DRM_GRALLOC /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names @@ -1359,6 +1363,7 @@ EGLBoolean dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) { struct dri2_egl_display *dri2_dpy; + int force_software = disp->Options.ForceSoftware; ForceSoftware is EGLBoolean and droid_load_driver() uses the same. Yet "int" is used here and "= true" below. Ack. const char *err; int ret; @@ -1384,13 +1389,18 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) dri2_dpy->fd = droid_open_device(disp); if (dri2_dpy->fd < 0) { - err = "DRI2: failed to open device"; - goto cleanup; + err = "DRI2: failed to open device, trying software device"; Ignoring the first patch for a second - the goto cleanup should stay. Ack. } - if (!droid_load_driver(disp)) { - err = "DRI2: failed to load driver"; - goto cleanup; +load_driver: + if (!droid_load_driver(disp, force_software)) { + if (force_software) { + err = "DRI2: failed to load driver"; + goto cleanup; + } else { + force_software = true; + goto load_driver; + } You can trivially eliminate the goto statement here. As Tomasz noted this fallback is already implemented in egldriver.c, so it can be removed entirely. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 5/5] DEBUG
On 05/07/18 17:50, Robert Foss wrote: Hey Emil, On 05/07/18 15:13, Emil Velikov wrote: Hi Rob, On 5 July 2018 at 11:07, Robert Foss wrote: @@ -511,7 +515,7 @@ dri2_open_driver(_EGLDisplay *disp) char path[PATH_MAX], *search_paths, *next, *end; char *get_extensions_name; const __DRIextension **(*get_extensions)(void); - + ALOGE("%s() 1 driver_name=%s", __func__, dri2_dpy->driver_name); Aside: Personally, I try to use "before/after X". Otherwise I find myself always bouncing back and forth, relate the 1, 1.1... with the actual call chain. Hmmph, yeah. Maybe that is a more pleasant way of going about it. I'll have to feel it out :) @@ -1367,10 +1379,6 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) const char *err; int ret; - /* Not supported yet */ - if (disp->Options.ForceSoftware) - return EGL_FALSE; - Even with the issues you mentioned in the cover letter, this could be fleshed out or even squashed with 3/5. Up-to you really. This chunk shouldn't really be in this patch. I had some issues setting the Android property and while figuring out that the property wasn't propagated properly this chunk was removed. The debug patch will be dropped in v1, so this will be removed. Actueally, when re-reading this comment I realize I misunderstood it. I'll fold this into 3/5 as you're suggesting. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 3/5] platform/android: Enable kms_swrast fallback
Hey Tomasz, On 05/07/18 16:26, Tomasz Figa wrote: Hi Rob, On Thu, Jul 5, 2018 at 7:07 PM Robert Foss wrote: Add support for the ForceSoftware option, which is togglable on the Android platform through setting the "drm.gpu.force_software" property to a non-zero value. kms_swrast is also enabled as a fallback for when a driver is not able to be loaded for for a drm node that was opened. Signed-off-by: Robert Foss --- src/egl/drivers/dri2/platform_android.c | 26 + 1 file changed, 18 insertions(+), 8 deletions(-) Thanks for the patch. Please see my comments inline. diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 92b2d2b343e..bc644c25bf9 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1193,17 +1193,21 @@ static const __DRIextension *droid_image_loader_extensions[] = { }; EGLBoolean -droid_load_driver(_EGLDisplay *disp) +droid_load_driver(_EGLDisplay *disp, EGLBoolean force_software) { struct dri2_egl_display *dri2_dpy = disp->DriverData; const char *err; - dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (force_software) { + dri2_dpy->driver_name = strdup("kms_swrast"); + } else { + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + } + if (dri2_dpy->driver_name == NULL) return false; dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER; - Unrelated whitespace change. Ack. if (!dri2_dpy->is_render_node) { #ifdef HAVE_DRM_GRALLOC /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names @@ -1359,6 +1363,7 @@ EGLBoolean dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) { struct dri2_egl_display *dri2_dpy; + int force_software = disp->Options.ForceSoftware; EGLBoolean Ack. const char *err; int ret; @@ -1384,13 +1389,18 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) dri2_dpy->fd = droid_open_device(disp); if (dri2_dpy->fd < 0) { - err = "DRI2: failed to open device"; - goto cleanup; + err = "DRI2: failed to open device, trying software device"; } - if (!droid_load_driver(disp)) { - err = "DRI2: failed to load driver"; - goto cleanup; +load_driver: + if (!droid_load_driver(disp, force_software)) { + if (force_software) { + err = "DRI2: failed to load driver"; + goto cleanup; + } else { + force_software = true; + goto load_driver; + } I believe we don't need this retry here, because if we fail here once, _eglMatchDriver() would retry us with ForceSoftware = EGL_TRUE [1]. [1] https://cgit.freedesktop.org/mesa/mesa/tree/src/egl/main/egldriver.c#n80 Oh nice. I didn't realize that the fallback was implemented somewhere else already. Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 5/5] DEBUG
Hey Emil, On 05/07/18 15:13, Emil Velikov wrote: Hi Rob, On 5 July 2018 at 11:07, Robert Foss wrote: @@ -511,7 +515,7 @@ dri2_open_driver(_EGLDisplay *disp) char path[PATH_MAX], *search_paths, *next, *end; char *get_extensions_name; const __DRIextension **(*get_extensions)(void); - + ALOGE("%s() 1 driver_name=%s", __func__, dri2_dpy->driver_name); Aside: Personally, I try to use "before/after X". Otherwise I find myself always bouncing back and forth, relate the 1, 1.1... with the actual call chain. Hmmph, yeah. Maybe that is a more pleasant way of going about it. I'll have to feel it out :) @@ -1367,10 +1379,6 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) const char *err; int ret; - /* Not supported yet */ - if (disp->Options.ForceSoftware) - return EGL_FALSE; - Even with the issues you mentioned in the cover letter, this could be fleshed out or even squashed with 3/5. Up-to you really. This chunk shouldn't really be in this patch. I had some issues setting the Android property and while figuring out that the property wasn't propagated properly this chunk was removed. The debug patch will be dropped in v1, so this will be removed. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 2/5] egl/android: Add Android property for forcing kms_swrast
Hey Tomasz, On 05/07/18 16:16, Tomasz Figa wrote: On Thu, Jul 5, 2018 at 7:07 PM Robert Foss wrote: In order to simplify Android bringup on new devices, provide the property "drm.gpu.force_software" which forces kms_swrast to be used. Signed-off-by: Robert Foss --- src/egl/main/egldriver.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/egl/main/egldriver.c b/src/egl/main/egldriver.c index 218b3daef25..bb9e90c157d 100644 --- a/src/egl/main/egldriver.c +++ b/src/egl/main/egldriver.c @@ -39,6 +39,10 @@ #include #include "c11/threads.h" +#ifdef HAVE_ANDROID_PLATFORM +#include +#endif + #include "egldefines.h" #include "egldisplay.h" #include "egldriver.h" @@ -87,6 +91,12 @@ _eglMatchDriver(_EGLDisplay *dpy) dpy->Options.ForceSoftware = env_var_as_boolean("LIBGL_ALWAYS_SOFTWARE", false); +#ifdef HAVE_ANDROID_PLATFORM + char prop_val[PROPERTY_VALUE_MAX]; + property_get("drm.gpu.force_software", prop_val, "0"); + dpy->Options.ForceSoftware |= strncmp(prop_val, "0", PROPERTY_VALUE_MAX); ForceSoftware is an EGLBoolean, which is just an unsigned int, not stdbool, so no implicit conversion into {0, 1} for us here. I think what we want here is dpy->Options.ForceSoftware |= strncmp(prop_val, "0", PROPERTY_VALUE_MAX) != 0; Ack Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 1/5] st/dri: Allow kms_swrast to work without a device FD
Hey Tomasz, On 05/07/18 15:07, Tomasz Figa wrote: Hi Emil, Robert, On Thu, Jul 5, 2018 at 9:57 PM Emil Velikov wrote: On 5 July 2018 at 12:32, Robert Foss wrote: Hey Eric! On 05/07/18 12:35, Eric Engestrom wrote: On Thursday, 2018-07-05 12:07:36 +0200, Robert Foss wrote: From: Tomeu Vizoso A KMS device isn't strictly needed for the kms_swrast to work, so stop failing to init if the FD is -1. Also don't call DRM_IOCTL_GET_CAP in that case. This allows the driver to be used in machines where no KMS device at all is present. Signed-off-by: Tomeu Vizoso --- src/gallium/state_trackers/dri/dri2.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c index 58a6757f037..c262c0ca118 100644 --- a/src/gallium/state_trackers/dri/dri2.c +++ b/src/gallium/state_trackers/dri/dri2.c @@ -2189,7 +2189,8 @@ dri_kms_init_screen(__DRIscreen * sPriv) sPriv->driverPrivate = (void *)screen; - if (screen->fd < 0 || (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0) + /* We don't really need a device FD if we are soft-rendering */ + if (screen->fd >= 0 && (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0) goto free_screen; if (pipe_loader_sw_probe_kms(>dev, fd)) { Aren't you going to use an uninitialised `fd` here if `screen->fd < 0`? From my understanding during dri2_initialize_android(), droid_open_device[1] will always allocate an FD or fail, before a screen is created in dri2_create_screen[2]. But maybe there is some part I don't quite understand. You're tracking a single instance instead of looking at the function in itself. When screen->fd is invalid, fd will be uninitialised. The pipe_loader_sw_probe_kms() call will then use that uninitialised value. I believe a better solution is to have distinct dri/null/kms backends to swrast, instead of hacking it in this way. Some ideas are listed in here [1]. -Emil [1] https://gitlab.collabora.com/tomeu/mesa/commit/54adda6a4d7b5c783d54dfd37d38d1a5a0f3187f First of all, do we really need this patch at all? If you ended up booting Android with Mesa, you must have had a DRM driver for the display anyway, so what prevents you from using it as the KMS device for kms_swrast? Just dropping this patch does indeed cause no regressions. Emil: Do you see any problems with that? Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 0/5] Android kms_swrast support
+ Wu Zhen On 05/07/18 12:07, Robert Foss wrote: This series is an early draft a kms_swrast implementation for the Android platform. It's available here: https://gitlab.collabora.com/robertfoss/mesa/tree/kms_swrast_rfc The current state isn't entirely functional, but the driver loading and initialization does seem to work. As to why the Android userland explodes, I'm not quite sure why yet. If you have any ideas as to why, they're very much welcome. I've got some cleaned up logcat logs here, with corresponding debug prints in the last patch: http://paste.debian.net/1032162/ Rob. Rob Herring (1): android: Build kms_swrast for the Android platform Robert Foss (3): egl/android: Add Android property for forcing kms_swrast platform/android: Enable kms_swrast fallback DEBUG Tomeu Vizoso (1): st/dri: Allow kms_swrast to work without a device FD src/egl/drivers/dri2/egl_dri2.c | 29 +-- src/egl/drivers/dri2/platform_android.c | 51 ++- src/egl/main/egldriver.c | 16 ++ src/gallium/Android.mk| 2 +- src/gallium/auxiliary/Android.mk | 5 ++ src/gallium/auxiliary/pipe-loader/Android.mk | 1 + src/gallium/drivers/softpipe/Android.mk | 4 +- src/gallium/state_trackers/dri/Android.mk | 1 + src/gallium/state_trackers/dri/dri2.c | 6 ++- src/gallium/winsys/sw/kms-dri/Android.mk | 39 ++ .../winsys/sw/kms-dri/kms_dri_sw_winsys.c | 6 ++- 11 files changed, 135 insertions(+), 25 deletions(-) create mode 100644 src/gallium/winsys/sw/kms-dri/Android.mk ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 1/5] st/dri: Allow kms_swrast to work without a device FD
Hey Eric! On 05/07/18 12:35, Eric Engestrom wrote: On Thursday, 2018-07-05 12:07:36 +0200, Robert Foss wrote: From: Tomeu Vizoso A KMS device isn't strictly needed for the kms_swrast to work, so stop failing to init if the FD is -1. Also don't call DRM_IOCTL_GET_CAP in that case. This allows the driver to be used in machines where no KMS device at all is present. Signed-off-by: Tomeu Vizoso --- src/gallium/state_trackers/dri/dri2.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c index 58a6757f037..c262c0ca118 100644 --- a/src/gallium/state_trackers/dri/dri2.c +++ b/src/gallium/state_trackers/dri/dri2.c @@ -2189,7 +2189,8 @@ dri_kms_init_screen(__DRIscreen * sPriv) sPriv->driverPrivate = (void *)screen; - if (screen->fd < 0 || (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0) + /* We don't really need a device FD if we are soft-rendering */ + if (screen->fd >= 0 && (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0) goto free_screen; if (pipe_loader_sw_probe_kms(>dev, fd)) { Aren't you going to use an uninitialised `fd` here if `screen->fd < 0`? From my understanding during dri2_initialize_android(), droid_open_device[1] will always allocate an FD or fail, before a screen is created in dri2_create_screen[2]. But maybe there is some part I don't quite understand. [1] https://gitlab.collabora.com/robertfoss/mesa/blob/9e5f456fee2cd3f8b8dd70cc313d0ad56166ecb0/src/egl/drivers/dri2/platform_android.c#L1385 [2] https://gitlab.collabora.com/robertfoss/mesa/blob/9e5f456fee2cd3f8b8dd70cc313d0ad56166ecb0/src/egl/drivers/dri2/platform_android.c#L1396 @@ -2204,7 +2205,8 @@ dri_kms_init_screen(__DRIscreen * sPriv) dri2ImageExtension.createImageWithModifiers = dri2_create_image_with_modifiers; - if (drmGetCap(sPriv->fd, DRM_CAP_PRIME, ) == 0 && + if (sPriv->fd >= 0 && + drmGetCap(sPriv->fd, DRM_CAP_PRIME, ) == 0 && (cap & DRM_PRIME_CAP_IMPORT)) { dri2ImageExtension.createImageFromFds = dri2_from_fds; dri2ImageExtension.createImageFromDmaBufs = dri2_from_dma_bufs; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC 5/5] DEBUG
Signed-off-by: Robert Foss --- src/egl/drivers/dri2/egl_dri2.c | 29 +++ src/egl/drivers/dri2/platform_android.c | 27 + src/egl/main/egldriver.c | 8 - src/gallium/auxiliary/Android.mk | 5 src/gallium/winsys/sw/kms-dri/Android.mk | 6 .../winsys/sw/kms-dri/kms_dri_sw_winsys.c | 6 ++-- 6 files changed, 67 insertions(+), 14 deletions(-) diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c index 45d0c7275c5..64b65042fda 100644 --- a/src/egl/drivers/dri2/egl_dri2.c +++ b/src/egl/drivers/dri2/egl_dri2.c @@ -65,6 +65,10 @@ #include "util/u_vector.h" #include "mapi/glapi/glapi.h" +#define ATRACE_TAG ATRACE_TAG_GRAPHICS +#define LOG_TAG "egl-dri2" +#include + /* The kernel header drm_fourcc.h defines the DRM formats below. We duplicate * some of the definitions here so that building Mesa won't bleeding-edge * kernel headers. @@ -511,7 +515,7 @@ dri2_open_driver(_EGLDisplay *disp) char path[PATH_MAX], *search_paths, *next, *end; char *get_extensions_name; const __DRIextension **(*get_extensions)(void); - + ALOGE("%s() 1 driver_name=%s", __func__, dri2_dpy->driver_name); search_paths = NULL; if (geteuid() == getuid()) { /* don't allow setuid apps to use LIBGL_DRIVERS_PATH */ @@ -523,6 +527,7 @@ dri2_open_driver(_EGLDisplay *disp) dri2_dpy->driver = NULL; end = search_paths + strlen(search_paths); for (char *p = search_paths; p < end; p = next + 1) { + ALOGE("%s() 1.1 driver path: %s", __func__, p); int len; next = strchr(p, ':'); if (next == NULL) @@ -532,14 +537,19 @@ dri2_open_driver(_EGLDisplay *disp) #if GLX_USE_TLS snprintf(path, sizeof path, "%.*s/tls/%s_dri.so", len, p, dri2_dpy->driver_name); + ALOGE("%s() 2 .so path: %s", __func__, path); + dri2_dpy->driver = dlopen(path, RTLD_NOW | RTLD_GLOBAL); #endif if (dri2_dpy->driver == NULL) { snprintf(path, sizeof path, "%.*s/%s_dri.so", len, p, dri2_dpy->driver_name); + ALOGE("%s() 3 .so path: %s", __func__, path); dri2_dpy->driver = dlopen(path, RTLD_NOW | RTLD_GLOBAL); - if (dri2_dpy->driver == NULL) + if (dri2_dpy->driver == NULL){ _eglLog(_EGL_DEBUG, "failed to open %s: %s\n", path, dlerror()); +ALOGE("%s() failed to open %s: %s\n", __func__, path, dlerror()); + } } /* not need continue to loop all paths once the driver is found */ if (dri2_dpy->driver != NULL) @@ -550,6 +560,8 @@ dri2_open_driver(_EGLDisplay *disp) _eglLog(_EGL_WARNING, "DRI2: failed to open %s (search paths %s)", dri2_dpy->driver_name, search_paths); + ALOGE("%s() DRI2: failed to open %s (search paths %s)", + __func__, dri2_dpy->driver_name, search_paths); return NULL; } @@ -821,8 +833,9 @@ EGLBoolean dri2_create_screen(_EGLDisplay *disp) { struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp); - + ALOGE("%s() 1", __func__); if (dri2_dpy->image_driver) { + ALOGE("%s() 1 1 image_driver", __func__); dri2_dpy->dri_screen = dri2_dpy->image_driver->createNewScreen2(0, dri2_dpy->fd, dri2_dpy->loader_extensions, @@ -830,19 +843,23 @@ dri2_create_screen(_EGLDisplay *disp) _dpy->driver_configs, disp); } else if (dri2_dpy->dri2) { + ALOGE("%s() 1 2 dri2", __func__); if (dri2_dpy->dri2->base.version >= 4) { + ALOGE("%s() 1 2 1 dri2", __func__); dri2_dpy->dri_screen = dri2_dpy->dri2->createNewScreen2(0, dri2_dpy->fd, dri2_dpy->loader_extensions, dri2_dpy->driver_extensions, _dpy->driver_configs, disp); } else { + ALOGE("%s() 1 2 2 !dri2", __func__); dri2_dpy->dri_screen = dri2_dpy->dri2->createNewScreen(0, dri2_dpy->fd, dri2_dpy->loader_extensions, _dpy->driver_configs, disp); } } else { + ALOGE("%s() 1 3 1 swrast", __func__); assert(dri2_dpy->swrast); if (dri2_dpy->swrast->base.version >= 4) { dri2_dpy->dri_screen = @@ -850,17 +867,19 @@ dri2_create_screen(_EGLDisplay *disp)
[Mesa-dev] [RFC 4/5] android: Build kms_swrast for the Android platform
From: Rob Herring Signed-off-by: Rob Herring Signed-off-by: Robert Foss --- src/gallium/Android.mk | 2 +- src/gallium/auxiliary/pipe-loader/Android.mk | 1 + src/gallium/drivers/softpipe/Android.mk | 4 +-- src/gallium/state_trackers/dri/Android.mk| 1 + src/gallium/winsys/sw/kms-dri/Android.mk | 33 5 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 src/gallium/winsys/sw/kms-dri/Android.mk diff --git a/src/gallium/Android.mk b/src/gallium/Android.mk index c5f31ef0ac6..21a1620974e 100644 --- a/src/gallium/Android.mk +++ b/src/gallium/Android.mk @@ -34,7 +34,7 @@ SUBDIRS += auxiliary/pipe-loader # Gallium drivers and their respective winsys # -SUBDIRS += winsys/sw/dri drivers/softpipe +SUBDIRS += winsys/sw/kms-dri winsys/sw/dri drivers/softpipe SUBDIRS += winsys/freedreno/drm drivers/freedreno SUBDIRS += winsys/i915/drm drivers/i915 SUBDIRS += winsys/nouveau/drm drivers/nouveau diff --git a/src/gallium/auxiliary/pipe-loader/Android.mk b/src/gallium/auxiliary/pipe-loader/Android.mk index ab844143c30..075bf8af436 100644 --- a/src/gallium/auxiliary/pipe-loader/Android.mk +++ b/src/gallium/auxiliary/pipe-loader/Android.mk @@ -30,6 +30,7 @@ include $(CLEAR_VARS) LOCAL_CFLAGS := \ -DHAVE_PIPE_LOADER_DRI \ + -DHAVE_PIPE_LOADER_KMS \ -DDROP_PIPE_LOADER_MISC \ -DGALLIUM_STATIC_TARGETS diff --git a/src/gallium/drivers/softpipe/Android.mk b/src/gallium/drivers/softpipe/Android.mk index 29cc317327c..7e879161090 100644 --- a/src/gallium/drivers/softpipe/Android.mk +++ b/src/gallium/drivers/softpipe/Android.mk @@ -37,6 +37,6 @@ include $(GALLIUM_COMMON_MK) include $(BUILD_STATIC_LIBRARY) ifneq ($(HAVE_GALLIUM_SOFTPIPE),) -GALLIUM_TARGET_DRIVERS += swrast -$(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_sw_dri) +GALLIUM_TARGET_DRIVERS += swrast kms_swrast +$(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_sw_dri libmesa_winsys_sw_kms_dri) endif diff --git a/src/gallium/state_trackers/dri/Android.mk b/src/gallium/state_trackers/dri/Android.mk index a867e50d86c..d6f8a6ed247 100644 --- a/src/gallium/state_trackers/dri/Android.mk +++ b/src/gallium/state_trackers/dri/Android.mk @@ -44,6 +44,7 @@ LOCAL_STATIC_LIBRARIES := \ ifneq ($(HAVE_GALLIUM_SOFTPIPE),) LOCAL_SRC_FILES += $(drisw_SOURCES) +LOCAL_CFLAGS += -DGALLIUM_SOFTPIPE endif LOCAL_MODULE := libmesa_st_dri diff --git a/src/gallium/winsys/sw/kms-dri/Android.mk b/src/gallium/winsys/sw/kms-dri/Android.mk new file mode 100644 index 000..f5c1ccab12a --- /dev/null +++ b/src/gallium/winsys/sw/kms-dri/Android.mk @@ -0,0 +1,33 @@ +# +# Copyright (C) 2017 Linaro, Ltd., Rob Herring +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +# DEALINGS IN THE SOFTWARE. + +LOCAL_PATH := $(call my-dir) + +include $(LOCAL_PATH)/Makefile.sources + +include $(CLEAR_VARS) + +LOCAL_SRC_FILES := $(C_SOURCES) + +LOCAL_MODULE := libmesa_winsys_sw_kms_dri + +include $(GALLIUM_COMMON_MK) +include $(BUILD_STATIC_LIBRARY) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC 3/5] platform/android: Enable kms_swrast fallback
Add support for the ForceSoftware option, which is togglable on the Android platform through setting the "drm.gpu.force_software" property to a non-zero value. kms_swrast is also enabled as a fallback for when a driver is not able to be loaded for for a drm node that was opened. Signed-off-by: Robert Foss --- src/egl/drivers/dri2/platform_android.c | 26 + 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 92b2d2b343e..bc644c25bf9 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -1193,17 +1193,21 @@ static const __DRIextension *droid_image_loader_extensions[] = { }; EGLBoolean -droid_load_driver(_EGLDisplay *disp) +droid_load_driver(_EGLDisplay *disp, EGLBoolean force_software) { struct dri2_egl_display *dri2_dpy = disp->DriverData; const char *err; - dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (force_software) { + dri2_dpy->driver_name = strdup("kms_swrast"); + } else { + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + } + if (dri2_dpy->driver_name == NULL) return false; dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER; - if (!dri2_dpy->is_render_node) { #ifdef HAVE_DRM_GRALLOC /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names @@ -1359,6 +1363,7 @@ EGLBoolean dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) { struct dri2_egl_display *dri2_dpy; + int force_software = disp->Options.ForceSoftware; const char *err; int ret; @@ -1384,13 +1389,18 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) dri2_dpy->fd = droid_open_device(disp); if (dri2_dpy->fd < 0) { - err = "DRI2: failed to open device"; - goto cleanup; + err = "DRI2: failed to open device, trying software device"; } - if (!droid_load_driver(disp)) { - err = "DRI2: failed to load driver"; - goto cleanup; +load_driver: + if (!droid_load_driver(disp, force_software)) { + if (force_software) { + err = "DRI2: failed to load driver"; + goto cleanup; + } else { + force_software = true; + goto load_driver; + } } if (!dri2_create_screen(disp)) { -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC 1/5] st/dri: Allow kms_swrast to work without a device FD
From: Tomeu Vizoso A KMS device isn't strictly needed for the kms_swrast to work, so stop failing to init if the FD is -1. Also don't call DRM_IOCTL_GET_CAP in that case. This allows the driver to be used in machines where no KMS device at all is present. Signed-off-by: Tomeu Vizoso --- src/gallium/state_trackers/dri/dri2.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c index 58a6757f037..c262c0ca118 100644 --- a/src/gallium/state_trackers/dri/dri2.c +++ b/src/gallium/state_trackers/dri/dri2.c @@ -2189,7 +2189,8 @@ dri_kms_init_screen(__DRIscreen * sPriv) sPriv->driverPrivate = (void *)screen; - if (screen->fd < 0 || (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0) + /* We don't really need a device FD if we are soft-rendering */ + if (screen->fd >= 0 && (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0) goto free_screen; if (pipe_loader_sw_probe_kms(>dev, fd)) { @@ -2204,7 +2205,8 @@ dri_kms_init_screen(__DRIscreen * sPriv) dri2ImageExtension.createImageWithModifiers = dri2_create_image_with_modifiers; - if (drmGetCap(sPriv->fd, DRM_CAP_PRIME, ) == 0 && + if (sPriv->fd >= 0 && + drmGetCap(sPriv->fd, DRM_CAP_PRIME, ) == 0 && (cap & DRM_PRIME_CAP_IMPORT)) { dri2ImageExtension.createImageFromFds = dri2_from_fds; dri2ImageExtension.createImageFromDmaBufs = dri2_from_dma_bufs; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC 0/5] Android kms_swrast support
This series is an early draft a kms_swrast implementation for the Android platform. It's available here: https://gitlab.collabora.com/robertfoss/mesa/tree/kms_swrast_rfc The current state isn't entirely functional, but the driver loading and initialization does seem to work. As to why the Android userland explodes, I'm not quite sure why yet. If you have any ideas as to why, they're very much welcome. I've got some cleaned up logcat logs here, with corresponding debug prints in the last patch: http://paste.debian.net/1032162/ Rob. Rob Herring (1): android: Build kms_swrast for the Android platform Robert Foss (3): egl/android: Add Android property for forcing kms_swrast platform/android: Enable kms_swrast fallback DEBUG Tomeu Vizoso (1): st/dri: Allow kms_swrast to work without a device FD src/egl/drivers/dri2/egl_dri2.c | 29 +-- src/egl/drivers/dri2/platform_android.c | 51 ++- src/egl/main/egldriver.c | 16 ++ src/gallium/Android.mk| 2 +- src/gallium/auxiliary/Android.mk | 5 ++ src/gallium/auxiliary/pipe-loader/Android.mk | 1 + src/gallium/drivers/softpipe/Android.mk | 4 +- src/gallium/state_trackers/dri/Android.mk | 1 + src/gallium/state_trackers/dri/dri2.c | 6 ++- src/gallium/winsys/sw/kms-dri/Android.mk | 39 ++ .../winsys/sw/kms-dri/kms_dri_sw_winsys.c | 6 ++- 11 files changed, 135 insertions(+), 25 deletions(-) create mode 100644 src/gallium/winsys/sw/kms-dri/Android.mk -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC 2/5] egl/android: Add Android property for forcing kms_swrast
In order to simplify Android bringup on new devices, provide the property "drm.gpu.force_software" which forces kms_swrast to be used. Signed-off-by: Robert Foss --- src/egl/main/egldriver.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/egl/main/egldriver.c b/src/egl/main/egldriver.c index 218b3daef25..bb9e90c157d 100644 --- a/src/egl/main/egldriver.c +++ b/src/egl/main/egldriver.c @@ -39,6 +39,10 @@ #include #include "c11/threads.h" +#ifdef HAVE_ANDROID_PLATFORM +#include +#endif + #include "egldefines.h" #include "egldisplay.h" #include "egldriver.h" @@ -87,6 +91,12 @@ _eglMatchDriver(_EGLDisplay *dpy) dpy->Options.ForceSoftware = env_var_as_boolean("LIBGL_ALWAYS_SOFTWARE", false); +#ifdef HAVE_ANDROID_PLATFORM + char prop_val[PROPERTY_VALUE_MAX]; + property_get("drm.gpu.force_software", prop_val, "0"); + dpy->Options.ForceSoftware |= strncmp(prop_val, "0", PROPERTY_VALUE_MAX); +#endif + best_drv = _eglMatchAndInitialize(dpy); if (!best_drv && !dpy->Options.ForceSoftware) { dpy->Options.ForceSoftware = EGL_TRUE; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RESEND PATCH v5 0/3] egl/android: Add DRM node probing and filtering
Pushed. On 2018-06-25 13:39, Robert Foss wrote: [RESEND] due to the previous version sent to the ML mistakenly being v4 again. Please excuse the spam. This series replaces the dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD with DRM node probing and disables the support for drm_gralloc. The series has been tested on Qemu+AOSP, where a virtio gpu was successfully probed for and opened. The current branch can be found here: https://gitlab.collabora.com/robertfoss/mesa/tree/drm_probing_v5 Changes since v4: - Removed dead continue statement - Switched function argument to const char* from char* Changes since v3: - Reduced number of probing return codes - Simplified driver vendor check in droid_probe_device() - Fixed type with ';' prepended to a if-statement - Removed a strlen call - Switched a sprintf to snprintf - Replaced fd == -1 check with < 0 - Simplified switch+goto statements Changes since v2: - Fixed whitespace issue - Diversified return codes from probing functions - Switched away from using drmGetDevices2, to iterating over /dev/dir/renderD nodes manually Changes since v1: - Added fix for build issue - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased work on the libdrm patch [2]. - Included patch from Rob Herring disabling drm_gralloc/flink support by default. - Added device handler driver probing. Rob Herring (1): egl/android: #ifdef out flink name support Robert Foss (2): gallium/util: Fix build error due to cast to different size egl/android: Add DRM node probing and filtering src/egl/Android.mk| 6 +- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 227 +++--- .../auxiliary/util/u_debug_stack_android.cpp | 4 +- 4 files changed, 195 insertions(+), 44 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RESEND PATCH v5 3/3] egl/android: Add DRM node probing and filtering
This patch both adds support for probing & filtering DRM nodes and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD gralloc call. Currently the filtering is based just on the driver name, and the desired name is supplied using the "drm.gpu.vendor_name" Android property. Signed-off-by: Robert Foss Reviewed-by: Tomasz Figa --- Changes since v4: - Removed dead continue statement - Switched function argument to const char* from char* - Added r-b from Tomasz Changes since v3: - Reduced number of probing return codes - Simplified driver vendor check in droid_probe_device() - Fixed type with ';' prepended to a if-statement - Removed a strlen call - Switched a sprintf to snprintf - Replaced fd == -1 check with < 0 - Simplified switch+goto statements Changes since v2: - Switch from drmGetDevices2 to manual renderD node iteration - Add probe_res enum to communicate probing results better - Avoid using _eglError() in internal static functions - Avoid actually loading the driver while probing, just verify that it exists. - Replace strlen call with the assumed length PROPERTY_VALUE_MAX Changes since v1: - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased on newer libdrm drmHandleMatch patch - Added support for driver probing src/egl/drivers/dri2/platform_android.c | 223 ++-- 1 file changed, 170 insertions(+), 53 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 4ba96aad90..cc16fd8118 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -27,12 +27,16 @@ * DEALINGS IN THE SOFTWARE. */ +#include #include +#include #include #include #include #include +#include #include +#include #include "loader.h" #include "egl_dri2.h" @@ -1130,31 +1134,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } -enum { -/* perform(const struct gralloc_module_t *mod, - * int op, - * int *fd); - */ -GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, -}; - -static int -droid_open_device(struct dri2_egl_display *dri2_dpy) -{ - int fd = -1, err = -EINVAL; - - if (dri2_dpy->gralloc->perform) - err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc, - GRALLOC_MODULE_PERFORM_GET_DRM_FD, - ); - if (err || fd < 0) { - _eglLog(_EGL_WARNING, "fail to get drm fd"); - fd = -1; - } - - return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1; -} - static const struct dri2_egl_display_vtbl droid_display_vtbl = { .authenticate = NULL, .create_window_surface = droid_create_window_surface, @@ -1215,6 +1194,169 @@ static const __DRIextension *droid_image_loader_extensions[] = { NULL, }; +EGLBoolean +droid_load_driver(_EGLDisplay *disp) +{ + struct dri2_egl_display *dri2_dpy = disp->DriverData; + const char *err; + + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) + return false; + + dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER; + + if (!dri2_dpy->is_render_node) { + #ifdef HAVE_DRM_GRALLOC + /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names +* for backwards compatibility with drm_gralloc. (Do not use on new +* systems.) */ + dri2_dpy->loader_extensions = droid_dri2_loader_extensions; + if (!dri2_load_driver(disp)) { + err = "DRI2: failed to load driver"; + goto error; + } + #else + err = "DRI2: handle is not for a render node"; + goto error; + #endif + } else { + dri2_dpy->loader_extensions = droid_image_loader_extensions; + if (!dri2_load_driver_dri3(disp)) { + err = "DRI3: failed to load driver"; + goto error; + } +} + + return true; + +error: + free(dri2_dpy->driver_name); + dri2_dpy->driver_name = NULL; + return false; +} + +static bool +droid_probe_driver(int fd) +{ + char *driver_name; + + driver_name = loader_get_driver_for_fd(fd); + if (driver_name == NULL) + return false; + + free(driver_name); + return true; +} + +typedef enum { + probe_fail = -1, + probe_success = 0, + probe_filtered_out = 1, +} probe_ret_t; + +static probe_ret_t +droid_probe_device(_EGLDisplay *disp, int fd, const char *vendor) +{ + int ret; + + drmVersionPtr ver = drmGetVersion(fd); + if (!ver) + return probe_fail; + + if (!ver->name) { + ret = probe_fail; + goto cleanup; + } + + if (vendor && strncmp(vendor, ver->name, PROPERTY_VALUE_M
[Mesa-dev] [RESEND PATCH v5 2/3] egl/android: #ifdef out flink name support
From: Rob Herring Maintaining both flink names and prime fd support which are provided by 2 different gralloc implementations is problematic because we have a dependency on a specific gralloc implementation header. This mostly disables the dependency on the gralloc implementation and headers. The dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD remains for now, but the definition is added locally to remove the header dependency. drm_gralloc support can be enabled by setting BOARD_USES_DRM_GRALLOC=true in BoardConfig.mk. Signed-off-by: Rob Herring Signed-off-by: Robert Foss Reviewed-by: Tomasz Figa --- Changes since v3: - Added r-b from Tomasz src/egl/Android.mk | 6 ++- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 56 +++-- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/egl/Android.mk b/src/egl/Android.mk index 11818694f4..8412aeb798 100644 --- a/src/egl/Android.mk +++ b/src/egl/Android.mk @@ -57,9 +57,13 @@ LOCAL_SHARED_LIBRARIES := \ libhardware \ liblog \ libcutils \ - libgralloc_drm \ libsync +ifeq ($(BOARD_USES_DRM_GRALLOC),true) + LOCAL_CFLAGS += -DHAVE_DRM_GRALLOC + LOCAL_SHARED_LIBRARIES += libgralloc_drm +endif + ifeq ($(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5 6 7),) LOCAL_SHARED_LIBRARIES += libnativewindow endif diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index adabc527f8..5d8fbfa235 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -67,8 +67,6 @@ struct zwp_linux_dmabuf_v1; #include #include -#include - #endif /* HAVE_ANDROID_PLATFORM */ #include "eglconfig.h" diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 1d6ed92bd6..4ba96aad90 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -37,7 +37,11 @@ #include "loader.h" #include "egl_dri2.h" #include "egl_dri2_fallbacks.h" + +#ifdef HAVE_DRM_GRALLOC +#include #include "gralloc_drm.h" +#endif /* HAVE_DRM_GRALLOC */ #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1)) @@ -164,11 +168,13 @@ get_native_buffer_fd(struct ANativeWindowBuffer *buf) return (handle && handle->numFds) ? handle->data[0] : -1; } +#ifdef HAVE_DRM_GRALLOC static int get_native_buffer_name(struct ANativeWindowBuffer *buf) { return gralloc_drm_get_gem_handle(buf->handle); } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf) @@ -838,6 +844,7 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx, return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list); } +#ifdef HAVE_DRM_GRALLOC static _EGLImage * droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, struct ANativeWindowBuffer *buf) @@ -881,6 +888,7 @@ droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, return _img->base; } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf, @@ -937,7 +945,11 @@ dri2_create_image_android_native_buffer(_EGLDisplay *disp, if (fd >= 0) return droid_create_image_from_prime_fd(disp, ctx, buf, fd); +#ifdef HAVE_DRM_GRALLOC return droid_create_image_from_name(disp, ctx, buf); +#else + return NULL; +#endif } static _EGLImage * @@ -959,6 +971,7 @@ droid_flush_front_buffer(__DRIdrawable * driDrawable, void *loaderPrivate) { } +#ifdef HAVE_DRM_GRALLOC static int droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf, unsigned int *attachments, int count) @@ -1034,6 +1047,7 @@ droid_get_buffers_with_format(__DRIdrawable * driDrawable, return dri2_surf->buffers; } +#endif /* HAVE_DRM_GRALLOC */ static unsigned droid_get_capability(void *loaderPrivate, enum dri_loader_cap cap) @@ -1116,6 +1130,14 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } +enum { +/* perform(const struct gralloc_module_t *mod, + * int op, + * int *fd); + */ +GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, +}; + static int droid_open_device(struct dri2_egl_display *dri2_dpy) { @@ -1158,6 +1180,7 @@ static const struct dri2_egl_display_vtbl droid_display_vtbl = { .get_dri_drawable = dri2_surface_get_dri_drawable, }; +#ifdef HAVE_DRM_GRALLOC static const __DRIdri2LoaderExtension droid_dri2_loader_extension = { .base = { __DRI_DRI2_LOADER, 4 }, @@ -1166,6 +1189,7 @@ static const __DRIdri2LoaderExtension droid_dri2_loader_extension = { .getBuffersWithFormat = droid_get_buffers_with_format, .getCapabi
[Mesa-dev] [RESEND PATCH v5 1/3] gallium/util: Fix build error due to cast to different size
Signed-off-by: Robert Foss Reviewed-by: Tomasz Figa Reviewed-by: Marek Olšák --- Changes since v4: - Added r-b from Marek Changes since v3: - Added r-b from Tomasz src/gallium/auxiliary/util/u_debug_stack_android.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gallium/auxiliary/util/u_debug_stack_android.cpp b/src/gallium/auxiliary/util/u_debug_stack_android.cpp index b3d56aebe6..395a1fe911 100644 --- a/src/gallium/auxiliary/util/u_debug_stack_android.cpp +++ b/src/gallium/auxiliary/util/u_debug_stack_android.cpp @@ -49,10 +49,10 @@ debug_backtrace_capture(debug_stack_frame *mesa_backtrace, backtrace_table = _mesa_hash_table_create(NULL, _mesa_hash_pointer, _mesa_key_pointer_equal); - backtrace_entry = _mesa_hash_table_search(backtrace_table, (void*) tid); + backtrace_entry = _mesa_hash_table_search(backtrace_table, (void*) (uintptr_t)tid); if (!backtrace_entry) { backtrace = Backtrace::Create(getpid(), tid); - _mesa_hash_table_insert(backtrace_table, (void*) tid, backtrace); + _mesa_hash_table_insert(backtrace_table, (void*) (uintptr_t)tid, backtrace); } else { backtrace = (Backtrace *) backtrace_entry->data; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RESEND PATCH v5 0/3] egl/android: Add DRM node probing and filtering
[RESEND] due to the previous version sent to the ML mistakenly being v4 again. Please excuse the spam. This series replaces the dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD with DRM node probing and disables the support for drm_gralloc. The series has been tested on Qemu+AOSP, where a virtio gpu was successfully probed for and opened. The current branch can be found here: https://gitlab.collabora.com/robertfoss/mesa/tree/drm_probing_v5 Changes since v4: - Removed dead continue statement - Switched function argument to const char* from char* Changes since v3: - Reduced number of probing return codes - Simplified driver vendor check in droid_probe_device() - Fixed type with ';' prepended to a if-statement - Removed a strlen call - Switched a sprintf to snprintf - Replaced fd == -1 check with < 0 - Simplified switch+goto statements Changes since v2: - Fixed whitespace issue - Diversified return codes from probing functions - Switched away from using drmGetDevices2, to iterating over /dev/dir/renderD nodes manually Changes since v1: - Added fix for build issue - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased work on the libdrm patch [2]. - Included patch from Rob Herring disabling drm_gralloc/flink support by default. - Added device handler driver probing. Rob Herring (1): egl/android: #ifdef out flink name support Robert Foss (2): gallium/util: Fix build error due to cast to different size egl/android: Add DRM node probing and filtering src/egl/Android.mk| 6 +- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 227 +++--- .../auxiliary/util/u_debug_stack_android.cpp | 4 +- 4 files changed, 195 insertions(+), 44 deletions(-) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 0/3] egl/android: Add DRM node probing and filtering
This series replaces the dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD with DRM node probing and disables the support for drm_gralloc. The series has been tested on Qemu+AOSP, where a virtio gpu was successfully probed for and opened. The current branch can be found here: https://gitlab.collabora.com/robertfoss/mesa/tree/drm_probing_v5 Changes since v4: - Removed dead continue statement - Switched function argument to const char* from char* Changes since v3: - Reduced number of probing return codes - Simplified driver vendor check in droid_probe_device() - Fixed type with ';' prepended to a if-statement - Removed a strlen call - Switched a sprintf to snprintf - Replaced fd == -1 check with < 0 - Simplified switch+goto statements Changes since v2: - Fixed whitespace issue - Diversified return codes from probing functions - Switched away from using drmGetDevices2, to iterating over /dev/dir/renderD nodes manually Changes since v1: - Added fix for build issue - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased work on the libdrm patch [2]. - Included patch from Rob Herring disabling drm_gralloc/flink support by default. - Added device handler driver probing. Rob Herring (1): egl/android: #ifdef out flink name support Robert Foss (2): gallium/util: Fix build error due to cast to different size egl/android: Add DRM node probing and filtering src/egl/Android.mk| 6 +- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 228 +++--- .../auxiliary/util/u_debug_stack_android.cpp | 4 +- 4 files changed, 196 insertions(+), 44 deletions(-) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 2/3] egl/android: #ifdef out flink name support
From: Rob Herring Maintaining both flink names and prime fd support which are provided by 2 different gralloc implementations is problematic because we have a dependency on a specific gralloc implementation header. This mostly disables the dependency on the gralloc implementation and headers. The dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD remains for now, but the definition is added locally to remove the header dependency. drm_gralloc support can be enabled by setting BOARD_USES_DRM_GRALLOC=true in BoardConfig.mk. Signed-off-by: Rob Herring Signed-off-by: Robert Foss Reviewed-by: Tomasz Figa --- Changes since v3: - Added r-b from Tomasz src/egl/Android.mk | 6 ++- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 56 +++-- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/egl/Android.mk b/src/egl/Android.mk index 11818694f4..8412aeb798 100644 --- a/src/egl/Android.mk +++ b/src/egl/Android.mk @@ -57,9 +57,13 @@ LOCAL_SHARED_LIBRARIES := \ libhardware \ liblog \ libcutils \ - libgralloc_drm \ libsync +ifeq ($(BOARD_USES_DRM_GRALLOC),true) + LOCAL_CFLAGS += -DHAVE_DRM_GRALLOC + LOCAL_SHARED_LIBRARIES += libgralloc_drm +endif + ifeq ($(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5 6 7),) LOCAL_SHARED_LIBRARIES += libnativewindow endif diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index adabc527f8..5d8fbfa235 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -67,8 +67,6 @@ struct zwp_linux_dmabuf_v1; #include #include -#include - #endif /* HAVE_ANDROID_PLATFORM */ #include "eglconfig.h" diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 1d6ed92bd6..4ba96aad90 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -37,7 +37,11 @@ #include "loader.h" #include "egl_dri2.h" #include "egl_dri2_fallbacks.h" + +#ifdef HAVE_DRM_GRALLOC +#include #include "gralloc_drm.h" +#endif /* HAVE_DRM_GRALLOC */ #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1)) @@ -164,11 +168,13 @@ get_native_buffer_fd(struct ANativeWindowBuffer *buf) return (handle && handle->numFds) ? handle->data[0] : -1; } +#ifdef HAVE_DRM_GRALLOC static int get_native_buffer_name(struct ANativeWindowBuffer *buf) { return gralloc_drm_get_gem_handle(buf->handle); } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf) @@ -838,6 +844,7 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx, return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list); } +#ifdef HAVE_DRM_GRALLOC static _EGLImage * droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, struct ANativeWindowBuffer *buf) @@ -881,6 +888,7 @@ droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, return _img->base; } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf, @@ -937,7 +945,11 @@ dri2_create_image_android_native_buffer(_EGLDisplay *disp, if (fd >= 0) return droid_create_image_from_prime_fd(disp, ctx, buf, fd); +#ifdef HAVE_DRM_GRALLOC return droid_create_image_from_name(disp, ctx, buf); +#else + return NULL; +#endif } static _EGLImage * @@ -959,6 +971,7 @@ droid_flush_front_buffer(__DRIdrawable * driDrawable, void *loaderPrivate) { } +#ifdef HAVE_DRM_GRALLOC static int droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf, unsigned int *attachments, int count) @@ -1034,6 +1047,7 @@ droid_get_buffers_with_format(__DRIdrawable * driDrawable, return dri2_surf->buffers; } +#endif /* HAVE_DRM_GRALLOC */ static unsigned droid_get_capability(void *loaderPrivate, enum dri_loader_cap cap) @@ -1116,6 +1130,14 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } +enum { +/* perform(const struct gralloc_module_t *mod, + * int op, + * int *fd); + */ +GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, +}; + static int droid_open_device(struct dri2_egl_display *dri2_dpy) { @@ -1158,6 +1180,7 @@ static const struct dri2_egl_display_vtbl droid_display_vtbl = { .get_dri_drawable = dri2_surface_get_dri_drawable, }; +#ifdef HAVE_DRM_GRALLOC static const __DRIdri2LoaderExtension droid_dri2_loader_extension = { .base = { __DRI_DRI2_LOADER, 4 }, @@ -1166,6 +1189,7 @@ static const __DRIdri2LoaderExtension droid_dri2_loader_extension = { .getBuffersWithFormat = droid_get_buffers_with_format, .getCapabi
[Mesa-dev] [PATCH v5 1/3] gallium/util: Fix build error due to cast to different size
Signed-off-by: Robert Foss Reviewed-by: Tomasz Figa Reviewed-by: Marek Olšák --- Changes since v4: - Added r-b from Marek Changes since v3: - Added r-b from Tomasz src/gallium/auxiliary/util/u_debug_stack_android.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gallium/auxiliary/util/u_debug_stack_android.cpp b/src/gallium/auxiliary/util/u_debug_stack_android.cpp index b3d56aebe6..395a1fe911 100644 --- a/src/gallium/auxiliary/util/u_debug_stack_android.cpp +++ b/src/gallium/auxiliary/util/u_debug_stack_android.cpp @@ -49,10 +49,10 @@ debug_backtrace_capture(debug_stack_frame *mesa_backtrace, backtrace_table = _mesa_hash_table_create(NULL, _mesa_hash_pointer, _mesa_key_pointer_equal); - backtrace_entry = _mesa_hash_table_search(backtrace_table, (void*) tid); + backtrace_entry = _mesa_hash_table_search(backtrace_table, (void*) (uintptr_t)tid); if (!backtrace_entry) { backtrace = Backtrace::Create(getpid(), tid); - _mesa_hash_table_insert(backtrace_table, (void*) tid, backtrace); + _mesa_hash_table_insert(backtrace_table, (void*) (uintptr_t)tid, backtrace); } else { backtrace = (Backtrace *) backtrace_entry->data; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 3/3] egl/android: Add DRM node probing and filtering
This patch both adds support for probing & filtering DRM nodes and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD gralloc call. Currently the filtering is based just on the driver name, and the desired name is supplied using the "drm.gpu.vendor_name" Android property. Signed-off-by: Robert Foss Reviewed-by: Tomasz Figa --- Changes since v4: - Removed dead continue statement - Switched function argument to const char* from char* - Added r-b from Tomasz Changes since v3: - Reduced number of probing return codes - Simplified driver vendor check in droid_probe_device() - Fixed type with ';' prepended to a if-statement - Removed a strlen call - Switched a sprintf to snprintf - Replaced fd == -1 check with < 0 - Simplified switch+goto statements Changes since v2: - Switch from drmGetDevices2 to manual renderD node iteration - Add probe_res enum to communicate probing results better - Avoid using _eglError() in internal static functions - Avoid actually loading the driver while probing, just verify that it exists. - Replace strlen call with the assumed length PROPERTY_VALUE_MAX Changes since v1: - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased on newer libdrm drmHandleMatch patch - Added support for driver probing src/egl/drivers/dri2/platform_android.c | 224 ++-- 1 file changed, 171 insertions(+), 53 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 4ba96aad90..b9d1862bd1 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -27,12 +27,16 @@ * DEALINGS IN THE SOFTWARE. */ +#include #include +#include #include #include #include #include +#include #include +#include #include "loader.h" #include "egl_dri2.h" @@ -1130,31 +1134,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } -enum { -/* perform(const struct gralloc_module_t *mod, - * int op, - * int *fd); - */ -GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, -}; - -static int -droid_open_device(struct dri2_egl_display *dri2_dpy) -{ - int fd = -1, err = -EINVAL; - - if (dri2_dpy->gralloc->perform) - err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc, - GRALLOC_MODULE_PERFORM_GET_DRM_FD, - ); - if (err || fd < 0) { - _eglLog(_EGL_WARNING, "fail to get drm fd"); - fd = -1; - } - - return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1; -} - static const struct dri2_egl_display_vtbl droid_display_vtbl = { .authenticate = NULL, .create_window_surface = droid_create_window_surface, @@ -1215,6 +1194,170 @@ static const __DRIextension *droid_image_loader_extensions[] = { NULL, }; +EGLBoolean +droid_load_driver(_EGLDisplay *disp) +{ + struct dri2_egl_display *dri2_dpy = disp->DriverData; + const char *err; + + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) + return false; + + dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER; + + if (!dri2_dpy->is_render_node) { + #ifdef HAVE_DRM_GRALLOC + /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names +* for backwards compatibility with drm_gralloc. (Do not use on new +* systems.) */ + dri2_dpy->loader_extensions = droid_dri2_loader_extensions; + if (!dri2_load_driver(disp)) { + err = "DRI2: failed to load driver"; + goto error; + } + #else + err = "DRI2: handle is not for a render node"; + goto error; + #endif + } else { + dri2_dpy->loader_extensions = droid_image_loader_extensions; + if (!dri2_load_driver_dri3(disp)) { + err = "DRI3: failed to load driver"; + goto error; + } +} + + return true; + +error: + free(dri2_dpy->driver_name); + dri2_dpy->driver_name = NULL; + return false; +} + +static bool +droid_probe_driver(int fd) +{ + char *driver_name; + + driver_name = loader_get_driver_for_fd(fd); + if (driver_name == NULL) + return false; + + free(driver_name); + return true; +} + +typedef enum { + probe_fail = -1, + probe_success = 0, + probe_filtered_out = 1, +} probe_ret_t; + +static probe_ret_t +droid_probe_device(_EGLDisplay *disp, int fd, char *vendor) +{ + int ret; + + drmVersionPtr ver = drmGetVersion(fd); + if (!ver) + return probe_fail; + + if (!ver->name) { + ret = probe_fail; + goto cleanup; + } + + if (vendor && strncmp(vendor, ver->name, PROPERTY_VALUE_MAX) != 0
[Mesa-dev] [PATCH v4 0/3] egl/android: Add DRM node probing and filtering
This series replaces the dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD with DRM node probing and disables the support for drm_gralloc. The series has been tested on Qemu+AOSP, where a virtio gpu was successfully probed for and opened. The current branch can be found here: https://gitlab.collabora.com/robertfoss/mesa/tree/drm_probing_v4 Changes since v3: - Reduced number of probing return codes - Simplified driver vendor check in droid_probe_device() - Fixed type with ';' prepended to a if-statement - Removed a strlen call - Switched a sprintf to snprintf - Replaced fd == -1 check with < 0 - Simplified switch+goto statements Changes since v2: - Fixed whitespace issue - Diversified return codes from probing functions - Switched away from using drmGetDevices2, to iterating over /dev/dir/renderD nodes manually Changes since v1: - Added fix for build issue - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased work on the libdrm patch [2]. - Included patch from Rob Herring disabling drm_gralloc/flink support by default. - Added device handler driver probing. Rob Herring (1): egl/android: #ifdef out flink name support Robert Foss (2): gallium/util: Fix build error due to cast to different size egl/android: Add DRM node probing and filtering src/egl/Android.mk| 6 +- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 228 +++--- .../auxiliary/util/u_debug_stack_android.cpp | 4 +- 4 files changed, 196 insertions(+), 44 deletions(-) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v4 1/3] gallium/util: Fix build error due to cast to different size
Signed-off-by: Robert Foss Reviewed-by: Tomasz Figa --- Changes since v3: - Added r-b from Tomasz src/gallium/auxiliary/util/u_debug_stack_android.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gallium/auxiliary/util/u_debug_stack_android.cpp b/src/gallium/auxiliary/util/u_debug_stack_android.cpp index b3d56aebe6..395a1fe911 100644 --- a/src/gallium/auxiliary/util/u_debug_stack_android.cpp +++ b/src/gallium/auxiliary/util/u_debug_stack_android.cpp @@ -49,10 +49,10 @@ debug_backtrace_capture(debug_stack_frame *mesa_backtrace, backtrace_table = _mesa_hash_table_create(NULL, _mesa_hash_pointer, _mesa_key_pointer_equal); - backtrace_entry = _mesa_hash_table_search(backtrace_table, (void*) tid); + backtrace_entry = _mesa_hash_table_search(backtrace_table, (void*) (uintptr_t)tid); if (!backtrace_entry) { backtrace = Backtrace::Create(getpid(), tid); - _mesa_hash_table_insert(backtrace_table, (void*) tid, backtrace); + _mesa_hash_table_insert(backtrace_table, (void*) (uintptr_t)tid, backtrace); } else { backtrace = (Backtrace *) backtrace_entry->data; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v4 3/3] egl/android: Add DRM node probing and filtering
This patch both adds support for probing & filtering DRM nodes and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD gralloc call. Currently the filtering is based just on the driver name, and the desired name is supplied using the "drm.gpu.vendor_name" Android property. Signed-off-by: Robert Foss --- Changes since v3: - Reduced number of probing return codes - Simplified driver vendor check in droid_probe_device() - Fixed type with ';' prepended to a if-statement - Removed a strlen call - Switched a sprintf to snprintf - Replaced fd == -1 check with < 0 - Simplified switch+goto statements Changes since v2: - Switch from drmGetDevices2 to manual renderD node iteration - Add probe_res enum to communicate probing results better - Avoid using _eglError() in internal static functions - Avoid actually loading the driver while probing, just verify that it exists. - Replace strlen call with the assumed length PROPERTY_VALUE_MAX Changes since v1: - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased on newer libdrm drmHandleMatch patch - Added support for driver probing src/egl/drivers/dri2/platform_android.c | 224 ++-- 1 file changed, 171 insertions(+), 53 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 4ba96aad90..b9d1862bd1 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -27,12 +27,16 @@ * DEALINGS IN THE SOFTWARE. */ +#include #include +#include #include #include #include #include +#include #include +#include #include "loader.h" #include "egl_dri2.h" @@ -1130,31 +1134,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } -enum { -/* perform(const struct gralloc_module_t *mod, - * int op, - * int *fd); - */ -GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, -}; - -static int -droid_open_device(struct dri2_egl_display *dri2_dpy) -{ - int fd = -1, err = -EINVAL; - - if (dri2_dpy->gralloc->perform) - err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc, - GRALLOC_MODULE_PERFORM_GET_DRM_FD, - ); - if (err || fd < 0) { - _eglLog(_EGL_WARNING, "fail to get drm fd"); - fd = -1; - } - - return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1; -} - static const struct dri2_egl_display_vtbl droid_display_vtbl = { .authenticate = NULL, .create_window_surface = droid_create_window_surface, @@ -1215,6 +1194,170 @@ static const __DRIextension *droid_image_loader_extensions[] = { NULL, }; +EGLBoolean +droid_load_driver(_EGLDisplay *disp) +{ + struct dri2_egl_display *dri2_dpy = disp->DriverData; + const char *err; + + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) + return false; + + dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER; + + if (!dri2_dpy->is_render_node) { + #ifdef HAVE_DRM_GRALLOC + /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names +* for backwards compatibility with drm_gralloc. (Do not use on new +* systems.) */ + dri2_dpy->loader_extensions = droid_dri2_loader_extensions; + if (!dri2_load_driver(disp)) { + err = "DRI2: failed to load driver"; + goto error; + } + #else + err = "DRI2: handle is not for a render node"; + goto error; + #endif + } else { + dri2_dpy->loader_extensions = droid_image_loader_extensions; + if (!dri2_load_driver_dri3(disp)) { + err = "DRI3: failed to load driver"; + goto error; + } +} + + return true; + +error: + free(dri2_dpy->driver_name); + dri2_dpy->driver_name = NULL; + return false; +} + +static bool +droid_probe_driver(int fd) +{ + char *driver_name; + + driver_name = loader_get_driver_for_fd(fd); + if (driver_name == NULL) + return false; + + free(driver_name); + return true; +} + +typedef enum { + probe_fail = -1, + probe_success = 0, + probe_filtered_out = 1, +} probe_ret_t; + +static probe_ret_t +droid_probe_device(_EGLDisplay *disp, int fd, char *vendor) +{ + int ret; + + drmVersionPtr ver = drmGetVersion(fd); + if (!ver) + return probe_fail; + + if (!ver->name) { + ret = probe_fail; + goto cleanup; + } + + if (vendor && strncmp(vendor, ver->name, PROPERTY_VALUE_MAX) != 0) { + ret = probe_filtered_out; + goto cleanup; + } + + if (!droid_probe_driver(fd)) { + ret = probe_fail; + goto cleanup; + } + +
[Mesa-dev] [PATCH v4 2/3] egl/android: #ifdef out flink name support
From: Rob Herring Maintaining both flink names and prime fd support which are provided by 2 different gralloc implementations is problematic because we have a dependency on a specific gralloc implementation header. This mostly disables the dependency on the gralloc implementation and headers. The dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD remains for now, but the definition is added locally to remove the header dependency. drm_gralloc support can be enabled by setting BOARD_USES_DRM_GRALLOC=true in BoardConfig.mk. Signed-off-by: Rob Herring Signed-off-by: Robert Foss Reviewed-by: Tomasz Figa --- Changes since v3: - Added r-b from Tomasz Changes since RFC: - Instead of removing code, #ifdef it out src/egl/Android.mk | 6 ++- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 56 +++-- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/egl/Android.mk b/src/egl/Android.mk index 11818694f4..8412aeb798 100644 --- a/src/egl/Android.mk +++ b/src/egl/Android.mk @@ -57,9 +57,13 @@ LOCAL_SHARED_LIBRARIES := \ libhardware \ liblog \ libcutils \ - libgralloc_drm \ libsync +ifeq ($(BOARD_USES_DRM_GRALLOC),true) + LOCAL_CFLAGS += -DHAVE_DRM_GRALLOC + LOCAL_SHARED_LIBRARIES += libgralloc_drm +endif + ifeq ($(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5 6 7),) LOCAL_SHARED_LIBRARIES += libnativewindow endif diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index adabc527f8..5d8fbfa235 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -67,8 +67,6 @@ struct zwp_linux_dmabuf_v1; #include #include -#include - #endif /* HAVE_ANDROID_PLATFORM */ #include "eglconfig.h" diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 1d6ed92bd6..4ba96aad90 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -37,7 +37,11 @@ #include "loader.h" #include "egl_dri2.h" #include "egl_dri2_fallbacks.h" + +#ifdef HAVE_DRM_GRALLOC +#include #include "gralloc_drm.h" +#endif /* HAVE_DRM_GRALLOC */ #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1)) @@ -164,11 +168,13 @@ get_native_buffer_fd(struct ANativeWindowBuffer *buf) return (handle && handle->numFds) ? handle->data[0] : -1; } +#ifdef HAVE_DRM_GRALLOC static int get_native_buffer_name(struct ANativeWindowBuffer *buf) { return gralloc_drm_get_gem_handle(buf->handle); } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf) @@ -838,6 +844,7 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx, return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list); } +#ifdef HAVE_DRM_GRALLOC static _EGLImage * droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, struct ANativeWindowBuffer *buf) @@ -881,6 +888,7 @@ droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, return _img->base; } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf, @@ -937,7 +945,11 @@ dri2_create_image_android_native_buffer(_EGLDisplay *disp, if (fd >= 0) return droid_create_image_from_prime_fd(disp, ctx, buf, fd); +#ifdef HAVE_DRM_GRALLOC return droid_create_image_from_name(disp, ctx, buf); +#else + return NULL; +#endif } static _EGLImage * @@ -959,6 +971,7 @@ droid_flush_front_buffer(__DRIdrawable * driDrawable, void *loaderPrivate) { } +#ifdef HAVE_DRM_GRALLOC static int droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf, unsigned int *attachments, int count) @@ -1034,6 +1047,7 @@ droid_get_buffers_with_format(__DRIdrawable * driDrawable, return dri2_surf->buffers; } +#endif /* HAVE_DRM_GRALLOC */ static unsigned droid_get_capability(void *loaderPrivate, enum dri_loader_cap cap) @@ -1116,6 +1130,14 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } +enum { +/* perform(const struct gralloc_module_t *mod, + * int op, + * int *fd); + */ +GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, +}; + static int droid_open_device(struct dri2_egl_display *dri2_dpy) { @@ -1158,6 +1180,7 @@ static const struct dri2_egl_display_vtbl droid_display_vtbl = { .get_dri_drawable = dri2_surface_get_dri_drawable, }; +#ifdef HAVE_DRM_GRALLOC static const __DRIdri2LoaderExtension droid_dri2_loader_extension = { .base = { __DRI_DRI2_LOADER, 4 }, @@ -1166,6 +1189,7 @@ static const __DRIdri2LoaderExtension droid_dri2_loader_extension =
Re: [Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
Hey Tomasz, Thanks for the quick feedback. On 2018-06-14 08:30, Tomasz Figa wrote: Hi Rob, Thanks for sending v3. Please see few more comments inline. On Sun, Jun 10, 2018 at 2:28 AM Robert Foss wrote: This patch both adds support for probing & filtering DRM nodes and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD gralloc call. Currently the filtering is based just on the driver name, and the desired name is supplied using the "drm.gpu.vendor_name" Android property. Signed-off-by: Robert Foss --- Changes since v2: - Switch from drmGetDevices2 to manual renderD node iteration - Add probe_res enum to communicate probing results better - Avoid using _eglError() in internal static functions - Avoid actually loading the driver while probing, just verify that it exists. - Replace strlen call with the assumed length PROPERTY_VALUE_MAX [snip] +static probe_ret_t +droid_probe_device(_EGLDisplay *disp, int fd, char *vendor) +{ + int ret; + + drmVersionPtr ver = drmGetVersion(fd); + if (!ver) + return probe_error; + + if (vendor != NULL && ver->name != NULL && + strncmp(vendor, ver->name, PROPERTY_VALUE_MAX) != 0) { Shouldn't we fail the match if vendor != NULL, but ver->name == NULL? i.e. if (vendor && (!ver->name || strcmp(...)) { ... Yeah, overall that if-case is too much. I'll split out the NULL check separate check that return an error. + ret = probe_filtered_out; + goto cleanup; + } + + + if (!droid_probe_driver(fd)) { + ret = probe_no_driver; + goto cleanup; + } + + ret = probe_success; + +cleanup: + drmFreeVersion(ver); + return ret; +} + +static int +droid_open_device(_EGLDisplay *disp) +{ + const int MAX_DRM_DEVICES = 32; + int prop_set, num_devices; + int fd = -1, fallback_fd = -1; + + char *vendor_name = NULL; + char vendor_buf[PROPERTY_VALUE_MAX]; + if (property_get("drm.gpu.vendor_name", vendor_buf, NULL) > 0); + vendor_name = vendor_buf; + + const char *drm_dir_name = "/dev/dri"; + DIR *sysdir = opendir(drm_dir_name); + if (!sysdir) + return -errno; + + struct dirent *dent; + while ((dent = readdir(sysdir))) { + char dev_path[128]; + char *render_dev_prefix = "renderD"; + size_t prefix_len = strlen(render_dev_prefix); If a const array like below is used instead const char render_dev_prefix[] = "renderD"; you can just use sizeof(render_dev_prefix), without the need for strlen(). Ack. + + if (strncmp(render_dev_prefix, dent->d_name, prefix_len) != 0) + continue; + + sprintf(dev_path, "%s/%s", drm_dir_name, dent->d_name); snprintf(dev_path, sizeof(dev_path), ...); Ack. + fd = loader_open_device(dev_path); + if (fd == -1) { nit: Perhaps fd < 0, just to be safe? Ack. + _eglLog(_EGL_WARNING, "%s() Failed to open DRM device %s", + __func__, dev_path); + continue; + } + + int ret = droid_probe_device(disp, fd, vendor_name); + switch (ret) { + case probe_success: + goto success; + case probe_filtered_out: + goto allow_fallback; The 2 lines of code at the "allow_fallback" label could be just moved here. Ack. + case probe_error: + case probe_no_driver: Do we need 2 separate cases for these? Just one "probe_fail" should be enough. Ack. + goto next; If we move the fallback handling to "probe_filtered_out" case, we could remove the "next" label too and simply "break" here. Ack. + } + +allow_fallback: + if (fallback_fd == -1) + fallback_fd = fd; +next: + if (fallback_fd != fd) + close(fd); + fd = -1; + continue; + } + +success: + closedir(sysdir); + + if (fallback_fd < 0 && fd < 0) { + _eglLog(_EGL_WARNING, "Failed to open any DRM device"); + return -1; + } + + if (fd < 0) { + _eglLog(_EGL_WARNING, "Failed to open desired DRM device, using fallback"); + return fallback_fd; + } + + close(fallback_fd); + return fd; +} [Leaving context for readability.] Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
On 2018-06-14 04:54, Tomasz Figa wrote: On Thu, Jun 14, 2018 at 4:14 AM Rob Herring wrote: On Wed, Jun 13, 2018 at 12:19 PM, Amit Pundir wrote: On 13 June 2018 at 20:45, Rob Herring wrote: +Amit and John On Sat, Jun 9, 2018 at 11:27 AM, Robert Foss wrote: This patch both adds support for probing & filtering DRM nodes and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD gralloc call. Currently the filtering is based just on the driver name, and the desired name is supplied using the "drm.gpu.vendor_name" Android property. There's a potential issue with this whole approach and that is SELinux. With the way SELinux locks down accesses, getting probing thru device files to work can be a pain. It may be better now than the prior version because sysfs is not probed. I'll leave it to Amit or John to comment. Right.. so ICYMI, this patch is already pulled into external/mesa3d project of AOSP and I stumbled upon one such /dev/dri/ access denial on db820c recently. A prior version of the patch series which accesses sysfs too (via libdrm). In AOSP, zygote spawned apps already have access to GPU device nodes in the form of /dev/gpu_device file, but the missing part is the It's "gpu_device" in terms a a SELinux context, right? Not an actual /dev path? open-read access to "/dev/dri/" which need to be allowed explicitly. Or we need a way to just open a specific device. How does this work in drm_gralloc (or any other existing gralloc that implements GRALLOC_MODULE_PERFORM_GET_DRM_FD)? What device node does it open? I have some loose recollection that it had a property specifying exact device. I guess we could use that property as the top level override, bypassing the whole probing logic, if present. drm_gralloc in aosp/master uses the "gralloc.drm.device" property. Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 1/3] gallium/util: Fix build error due to cast to different size
Signed-off-by: Robert Foss --- src/gallium/auxiliary/util/u_debug_stack_android.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gallium/auxiliary/util/u_debug_stack_android.cpp b/src/gallium/auxiliary/util/u_debug_stack_android.cpp index b3d56aebe6..395a1fe911 100644 --- a/src/gallium/auxiliary/util/u_debug_stack_android.cpp +++ b/src/gallium/auxiliary/util/u_debug_stack_android.cpp @@ -49,10 +49,10 @@ debug_backtrace_capture(debug_stack_frame *mesa_backtrace, backtrace_table = _mesa_hash_table_create(NULL, _mesa_hash_pointer, _mesa_key_pointer_equal); - backtrace_entry = _mesa_hash_table_search(backtrace_table, (void*) tid); + backtrace_entry = _mesa_hash_table_search(backtrace_table, (void*) (uintptr_t)tid); if (!backtrace_entry) { backtrace = Backtrace::Create(getpid(), tid); - _mesa_hash_table_insert(backtrace_table, (void*) tid, backtrace); + _mesa_hash_table_insert(backtrace_table, (void*) (uintptr_t)tid, backtrace); } else { backtrace = (Backtrace *) backtrace_entry->data; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 2/3] egl/android: #ifdef out flink name support
From: Rob Herring Maintaining both flink names and prime fd support which are provided by 2 different gralloc implementations is problematic because we have a dependency on a specific gralloc implementation header. This mostly disables the dependency on the gralloc implementation and headers. The dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD remains for now, but the definition is added locally to remove the header dependency. drm_gralloc support can be enabled by setting BOARD_USES_DRM_GRALLOC=true in BoardConfig.mk. Signed-off-by: Rob Herring Signed-off-by: Robert Foss --- Changes since RFC: - Instead of removing code, #ifdef it out. src/egl/Android.mk | 6 ++- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 56 +++-- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/egl/Android.mk b/src/egl/Android.mk index 11818694f4..8412aeb798 100644 --- a/src/egl/Android.mk +++ b/src/egl/Android.mk @@ -57,9 +57,13 @@ LOCAL_SHARED_LIBRARIES := \ libhardware \ liblog \ libcutils \ - libgralloc_drm \ libsync +ifeq ($(BOARD_USES_DRM_GRALLOC),true) + LOCAL_CFLAGS += -DHAVE_DRM_GRALLOC + LOCAL_SHARED_LIBRARIES += libgralloc_drm +endif + ifeq ($(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5 6 7),) LOCAL_SHARED_LIBRARIES += libnativewindow endif diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index adabc527f8..5d8fbfa235 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -67,8 +67,6 @@ struct zwp_linux_dmabuf_v1; #include #include -#include - #endif /* HAVE_ANDROID_PLATFORM */ #include "eglconfig.h" diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 1d6ed92bd6..4ba96aad90 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -37,7 +37,11 @@ #include "loader.h" #include "egl_dri2.h" #include "egl_dri2_fallbacks.h" + +#ifdef HAVE_DRM_GRALLOC +#include #include "gralloc_drm.h" +#endif /* HAVE_DRM_GRALLOC */ #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1)) @@ -164,11 +168,13 @@ get_native_buffer_fd(struct ANativeWindowBuffer *buf) return (handle && handle->numFds) ? handle->data[0] : -1; } +#ifdef HAVE_DRM_GRALLOC static int get_native_buffer_name(struct ANativeWindowBuffer *buf) { return gralloc_drm_get_gem_handle(buf->handle); } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf) @@ -838,6 +844,7 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx, return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list); } +#ifdef HAVE_DRM_GRALLOC static _EGLImage * droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, struct ANativeWindowBuffer *buf) @@ -881,6 +888,7 @@ droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, return _img->base; } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf, @@ -937,7 +945,11 @@ dri2_create_image_android_native_buffer(_EGLDisplay *disp, if (fd >= 0) return droid_create_image_from_prime_fd(disp, ctx, buf, fd); +#ifdef HAVE_DRM_GRALLOC return droid_create_image_from_name(disp, ctx, buf); +#else + return NULL; +#endif } static _EGLImage * @@ -959,6 +971,7 @@ droid_flush_front_buffer(__DRIdrawable * driDrawable, void *loaderPrivate) { } +#ifdef HAVE_DRM_GRALLOC static int droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf, unsigned int *attachments, int count) @@ -1034,6 +1047,7 @@ droid_get_buffers_with_format(__DRIdrawable * driDrawable, return dri2_surf->buffers; } +#endif /* HAVE_DRM_GRALLOC */ static unsigned droid_get_capability(void *loaderPrivate, enum dri_loader_cap cap) @@ -1116,6 +1130,14 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } +enum { +/* perform(const struct gralloc_module_t *mod, + * int op, + * int *fd); + */ +GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, +}; + static int droid_open_device(struct dri2_egl_display *dri2_dpy) { @@ -1158,6 +1180,7 @@ static const struct dri2_egl_display_vtbl droid_display_vtbl = { .get_dri_drawable = dri2_surface_get_dri_drawable, }; +#ifdef HAVE_DRM_GRALLOC static const __DRIdri2LoaderExtension droid_dri2_loader_extension = { .base = { __DRI_DRI2_LOADER, 4 }, @@ -1166,6 +1189,7 @@ static const __DRIdri2LoaderExtension droid_dri2_loader_extension = { .getBuffersWithFormat = droid_get_buffers_with_format, .getCapability
[Mesa-dev] [PATCH v3 3/3] egl/android: Add DRM node probing and filtering
This patch both adds support for probing & filtering DRM nodes and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD gralloc call. Currently the filtering is based just on the driver name, and the desired name is supplied using the "drm.gpu.vendor_name" Android property. Signed-off-by: Robert Foss --- Changes since v2: - Switch from drmGetDevices2 to manual renderD node iteration - Add probe_res enum to communicate probing results better - Avoid using _eglError() in internal static functions - Avoid actually loading the driver while probing, just verify that it exists. - Replace strlen call with the assumed length PROPERTY_VALUE_MAX Changes since v1: - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased on newer libdrm drmHandleMatch patch - Added support for driver probing src/egl/drivers/dri2/platform_android.c | 222 ++-- 1 file changed, 169 insertions(+), 53 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 4ba96aad90..a2cbe92d93 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -27,12 +27,16 @@ * DEALINGS IN THE SOFTWARE. */ +#include #include +#include #include #include #include #include +#include #include +#include #include "loader.h" #include "egl_dri2.h" @@ -1130,31 +1134,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } -enum { -/* perform(const struct gralloc_module_t *mod, - * int op, - * int *fd); - */ -GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, -}; - -static int -droid_open_device(struct dri2_egl_display *dri2_dpy) -{ - int fd = -1, err = -EINVAL; - - if (dri2_dpy->gralloc->perform) - err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc, - GRALLOC_MODULE_PERFORM_GET_DRM_FD, - ); - if (err || fd < 0) { - _eglLog(_EGL_WARNING, "fail to get drm fd"); - fd = -1; - } - - return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1; -} - static const struct dri2_egl_display_vtbl droid_display_vtbl = { .authenticate = NULL, .create_window_surface = droid_create_window_surface, @@ -1215,6 +1194,168 @@ static const __DRIextension *droid_image_loader_extensions[] = { NULL, }; +EGLBoolean +droid_load_driver(_EGLDisplay *disp) +{ + struct dri2_egl_display *dri2_dpy = disp->DriverData; + const char *err; + + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) + return false; + + dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER; + + if (!dri2_dpy->is_render_node) { + #ifdef HAVE_DRM_GRALLOC + /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names +* for backwards compatibility with drm_gralloc. (Do not use on new +* systems.) */ + dri2_dpy->loader_extensions = droid_dri2_loader_extensions; + if (!dri2_load_driver(disp)) { + err = "DRI2: failed to load driver"; + goto error; + } + #else + err = "DRI2: handle is not for a render node"; + goto error; + #endif + } else { + dri2_dpy->loader_extensions = droid_image_loader_extensions; + if (!dri2_load_driver_dri3(disp)) { + err = "DRI3: failed to load driver"; + goto error; + } +} + + return true; + +error: + free(dri2_dpy->driver_name); + dri2_dpy->driver_name = NULL; + return false; +} + +static bool +droid_probe_driver(int fd) +{ + char *driver_name; + + driver_name = loader_get_driver_for_fd(fd); + if (driver_name == NULL) + return false; + + free(driver_name); + return true; +} + +typedef enum { + probe_error = -1, + probe_success = 0, + probe_filtered_out = 1, + probe_no_driver = 2 +} probe_ret_t; + +static probe_ret_t +droid_probe_device(_EGLDisplay *disp, int fd, char *vendor) +{ + int ret; + + drmVersionPtr ver = drmGetVersion(fd); + if (!ver) + return probe_error; + + if (vendor != NULL && ver->name != NULL && + strncmp(vendor, ver->name, PROPERTY_VALUE_MAX) != 0) { + ret = probe_filtered_out; + goto cleanup; + } + + + if (!droid_probe_driver(fd)) { + ret = probe_no_driver; + goto cleanup; + } + + ret = probe_success; + +cleanup: + drmFreeVersion(ver); + return ret; +} + +static int +droid_open_device(_EGLDisplay *disp) +{ + const int MAX_DRM_DEVICES = 32; + int prop_set, num_devices; + int fd = -1, fallback_fd = -1; + + char *vendor_name = NULL; + char vendor_buf[PR
[Mesa-dev] [PATCH v3 0/3] egl/android: Remove dependencies on specific grallocs
This series replaces the dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD with DRM node probing and disables the support for drm_gralloc. The series has been tested on Qemu+AOSP, where a virtio gpu was successfully probed for and opened. Changes since v2: - Fixed whitespace issue - Diversified return codes from probing functions - Switched away from using drmGetDevices2, to iterating over /dev/dir/renderD nodes manually Changes since v1: - Added fix for build issue - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased work on the libdrm patch [2]. - Included patch from Rob Herring disabling drm_gralloc/flink support by default. - Added device handler driver probing. Rob Herring (1): egl/android: #ifdef out flink name support Robert Foss (2): gallium/util: Fix build error due to cast to different size egl/android: Add DRM node probing and filtering src/egl/Android.mk| 6 +- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 226 +++--- .../auxiliary/util/u_debug_stack_android.cpp | 4 +- 4 files changed, 194 insertions(+), 44 deletions(-) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] egl/android: Remove dependencies on specific grallocs
Hey, On 2018-05-25 17:38, Rob Herring wrote: On Fri, May 25, 2018 at 9:25 AM, Tomasz Figa wrote: On Fri, May 25, 2018 at 10:59 PM Rob Herring wrote: On Fri, May 25, 2018 at 4:15 AM, Robert Foss wrote: On 2018-05-25 10:38, Tomasz Figa wrote: On Fri, May 25, 2018 at 5:33 PM Robert Foss wrote: Hey, On 2018-05-25 02:17, Rob Herring wrote: On Thu, May 24, 2018 at 6:23 AM, Robert Foss < robert.f...@collabora.com> wrote: Hey, I don't think I've received any feedback on this version yet. If anyone has some time to spare, it would be nice to get it merged. Just to be clear about the libdrm branch linked in the cover letter, it is not required. Only for virgl platforms which happens to be what I tested on. virgl will still fallback to using the first render node without those libdrm changes, right? If not, I don't think we should apply until we're not breaking a platform... No it will not fall back. I agree that holding off makes more sense. What's the reason of this problems? Is it because of drmGetDevices()? Since we don't really use it for anything other than getting the list of render nodes in the system, maybe we could just iterate over any /dev/renderD* nodes explicitly and avoid introducing new problems? That's exactly the problem, and yes we could 100% solve by iterating over /dev/renderD* nodes. I originally assumed we wouldn't want to do that, but rather use the libdrm interfaces. But for the next spin I could avoid using libdrm, should I? I don't have an opinion on libdrm really, but I do think we should fallback to the 1st (only) render node rather than just fail. We do, even with libdrm. AFAICT, the problem with virgl seems to be that drmGetDevices() doesn't include devices on virtio bus in the results, which means that there likely wouldn't be any render node returned. Okay. I still don't get why we search by bus in the first place. Who cares what bus the gpu sits on. Now I have an opinion. We should just iterate over render nodes matching by name or use the first node if we don't have a set name. For v3 I implemented a version with drmGetDevices2 replaced with iteration over the /dev/dri/renderD* nodes. That still means doing all of the other filtering, however. Rob. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] egl/android: Add DRM node probing and filtering
Hey Tomasz, On 2018-05-25 09:27, Tomasz Figa wrote: > Hi Rob, > > Finally got to review this. Please see my comments inline. > > On Fri, May 11, 2018 at 10:48 PM Robert Foss > wrote: > [snip] >> +EGLBoolean >> +droid_load_driver(_EGLDisplay *disp) > > Since this is not EGL-facing, I'd personally use bool. > >> +{ >> + struct dri2_egl_display *dri2_dpy = disp->DriverData; >> + const char *err; >> + >> + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); >> + if (dri2_dpy->driver_name == NULL) { >> + err = "DRI2: failed to get driver name"; >> + goto error; > > It shouldn't be an error if there is no driver for given render node. We > should just skip it and try next one, which I believe would be achieved by > just returning false here. > >> + } >> + >> + dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == > DRM_NODE_RENDER; >> + >> + if (!dri2_dpy->is_render_node) { >> + #ifdef HAVE_DRM_GRALLOC >> + /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM > names >> +* for backwards compatibility with drm_gralloc. (Do not use on > new >> +* systems.) */ >> + dri2_dpy->loader_extensions = droid_dri2_loader_extensions; >> + if (!dri2_load_driver(disp)) { >> + err = "DRI2: failed to load driver"; >> + goto error; >> + } >> + #else >> + err = "DRI2: handle is not for a render node"; >> + goto error; >> + #endif >> + } else { >> + dri2_dpy->loader_extensions = droid_image_loader_extensions; >> + if (!dri2_load_driver_dri3(disp)) { >> + err = "DRI3: failed to load driver"; >> + goto error; >> + } >> +} >> + >> + return EGL_TRUE; >> + >> +error: >> + free(dri2_dpy->driver_name); >> + dri2_dpy->driver_name = NULL; >> + return _eglError(EGL_NOT_INITIALIZED, err); > > Hmm, if we signal EGL error here, we should break the probing loop and just > bail out. This would suggest that a boolean is not the right type for this > function to return. Perhaps something like negative error, 0 for skip and 1 > for success would make sense? > > Also, how does it play with the _eglError() called from the error path of > dri2_initialize_android()? I can't say I put any though into that aspect, but dri2_initialize_android() would overwrite it. So maybe completely avoiding _eglError() in droid_load_driver() is the way to go. > >> +} >> + >> +static int >> +droid_probe_driver(_EGLDisplay *disp, int fd) >> +{ >> + struct dri2_egl_display *dri2_dpy = disp->DriverData; >> + dri2_dpy->fd = fd; >> + >> + if (!droid_load_driver(disp)) >> + return false; > > Given my other suggestion about distinguishing failure, render node skip > and success, I think it should be more like this: > > int ret = droid_load_driver(disp); > if (ret <= 0) > return ret; > > Or actually, maybe we don't really need to go as far as loading the driver. > I'd say it should be enough to just check if we have a driver for the > device by looking at what loader_get_driver_for_fd() returns. (In that > case, we can ignore my comment about returning error on > loader_get_driver_for_fd() failure in droid_load_driver(), since the > skipping would be handling only here.) If we don't need to actually load the driver, I think all of the above comments can be fixed by just removing chunks out of dri related setup. > >> + >> + /* Since this probe can succeed, but another filter may fail, > > What another filter could fail? I can see the vendor name being checked > before calling this function. > > The free() below is actually needed, just the comment is off. We need to > free the name to be able to probe remaining nodes, without leaking the name. Ack, fixed by the above change. > >> + this string needs to be deallocated either way. >> + Once an FD has been found, this string will be set a second time. > */ >> + free(dri2_dpy->driver_name); > > Don't we also need to unload the driver? Ack, fixed by the above change. > >> + dri2_dpy->driver_name = NULL; >> + return true; > > To match the change above: > > return 1; > Ack, fixed by the above change. >> +} >> + >> +static int >> +droid_probe_device(_EGLDisplay *disp, int fd, drmDevicePtr dev, char > *vendor) >> +{ >> + drmVersionPtr ver =
Re: [Mesa-dev] [PATCH 0/3] egl/android: Remove dependencies on specific grallocs
On 2018-05-25 10:38, Tomasz Figa wrote: On Fri, May 25, 2018 at 5:33 PM Robert Foss <robert.f...@collabora.com> wrote: Hey, On 2018-05-25 02:17, Rob Herring wrote: On Thu, May 24, 2018 at 6:23 AM, Robert Foss <robert.f...@collabora.com> wrote: Hey, I don't think I've received any feedback on this version yet. If anyone has some time to spare, it would be nice to get it merged. Just to be clear about the libdrm branch linked in the cover letter, it is not required. Only for virgl platforms which happens to be what I tested on. virgl will still fallback to using the first render node without those libdrm changes, right? If not, I don't think we should apply until we're not breaking a platform... No it will not fall back. I agree that holding off makes more sense. What's the reason of this problems? Is it because of drmGetDevices()? Since we don't really use it for anything other than getting the list of render nodes in the system, maybe we could just iterate over any /dev/renderD* nodes explicitly and avoid introducing new problems? That's exactly the problem, and yes we could 100% solve by iterating over /dev/renderD* nodes. I originally assumed we wouldn't want to do that, but rather use the libdrm interfaces. But for the next spin I could avoid using libdrm, should I? Rob. Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] egl/android: Remove dependencies on specific grallocs
Hey, On 2018-05-25 02:17, Rob Herring wrote: On Thu, May 24, 2018 at 6:23 AM, Robert Foss <robert.f...@collabora.com> wrote: Hey, I don't think I've received any feedback on this version yet. If anyone has some time to spare, it would be nice to get it merged. Just to be clear about the libdrm branch linked in the cover letter, it is not required. Only for virgl platforms which happens to be what I tested on. virgl will still fallback to using the first render node without those libdrm changes, right? If not, I don't think we should apply until we're not breaking a platform... No it will not fall back. I agree that holding off makes more sense. Emil Velikov had some objections to the approach in the libdrm branch, and started a new branch from scratch with the same goals. It isn't yet fully functional, but I'm working with him to have it sent out as soon as possible. Rob. Rob Rob. On 2018-05-11 15:47, Robert Foss wrote: This series replaces the dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD with DRM node probing and disables the support for drm_gralloc. The series has been tested on Qemu+AOSP, where a virtio gpu was successfully probed for and opened. This however required adding support in libdrm for virtio gpus, and virtio buses. An initial patch for this can be found here: https://gitlab.collabora.com/robertfoss/libdrm/tree/virtio_rfc Changes since v1: - Added fix for build issue - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased work on the libdrm patch [2]. - Included patch from Rob Herring disabling drm_gralloc/flink support by default. - Added device handler driver probing. Rob Herring (1): egl/android: #ifdef out flink name support Robert Foss (2): gallium/util: Fix build error due to cast to different size egl/android: Add DRM node probing and filtering src/egl/Android.mk| 6 +- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 206 ++ .../auxiliary/util/u_debug_stack_android.cpp | 4 +- 4 files changed, 174 insertions(+), 44 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] egl/android: Remove dependencies on specific grallocs
Hey, I don't think I've received any feedback on this version yet. If anyone has some time to spare, it would be nice to get it merged. Just to be clear about the libdrm branch linked in the cover letter, it is not required. Only for virgl platforms which happens to be what I tested on. Rob. On 2018-05-11 15:47, Robert Foss wrote: This series replaces the dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD with DRM node probing and disables the support for drm_gralloc. The series has been tested on Qemu+AOSP, where a virtio gpu was successfully probed for and opened. This however required adding support in libdrm for virtio gpus, and virtio buses. An initial patch for this can be found here: https://gitlab.collabora.com/robertfoss/libdrm/tree/virtio_rfc Changes since v1: - Added fix for build issue - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased work on the libdrm patch [2]. - Included patch from Rob Herring disabling drm_gralloc/flink support by default. - Added device handler driver probing. Rob Herring (1): egl/android: #ifdef out flink name support Robert Foss (2): gallium/util: Fix build error due to cast to different size egl/android: Add DRM node probing and filtering src/egl/Android.mk| 6 +- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 206 ++ .../auxiliary/util/u_debug_stack_android.cpp | 4 +- 4 files changed, 174 insertions(+), 44 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] egl/android: Remove dependencies on specific grallocs
Hey, After today I'll be out of the office until the 23rd, So if this series looks ok, I won't be able to re-spin it with more tags. So feel free to just push it if possible. Rob. On 2018-05-11 15:47, Robert Foss wrote: This series replaces the dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD with DRM node probing and disables the support for drm_gralloc. The series has been tested on Qemu+AOSP, where a virtio gpu was successfully probed for and opened. This however required adding support in libdrm for virtio gpus, and virtio buses. An initial patch for this can be found here: https://gitlab.collabora.com/robertfoss/libdrm/tree/virtio_rfc Changes since v1: - Added fix for build issue - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased work on the libdrm patch [2]. - Included patch from Rob Herring disabling drm_gralloc/flink support by default. - Added device handler driver probing. Rob Herring (1): egl/android: #ifdef out flink name support Robert Foss (2): gallium/util: Fix build error due to cast to different size egl/android: Add DRM node probing and filtering src/egl/Android.mk| 6 +- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 206 ++ .../auxiliary/util/u_debug_stack_android.cpp | 4 +- 4 files changed, 174 insertions(+), 44 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/3] egl/android: Remove dependencies on specific grallocs
This series replaces the dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD with DRM node probing and disables the support for drm_gralloc. The series has been tested on Qemu+AOSP, where a virtio gpu was successfully probed for and opened. This however required adding support in libdrm for virtio gpus, and virtio buses. An initial patch for this can be found here: https://gitlab.collabora.com/robertfoss/libdrm/tree/virtio_rfc Changes since v1: - Added fix for build issue - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased work on the libdrm patch [2]. - Included patch from Rob Herring disabling drm_gralloc/flink support by default. - Added device handler driver probing. Rob Herring (1): egl/android: #ifdef out flink name support Robert Foss (2): gallium/util: Fix build error due to cast to different size egl/android: Add DRM node probing and filtering src/egl/Android.mk| 6 +- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 206 ++ .../auxiliary/util/u_debug_stack_android.cpp | 4 +- 4 files changed, 174 insertions(+), 44 deletions(-) -- 2.17.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] egl/android: #ifdef out flink name support
From: Rob Herring <r...@kernel.org> Maintaining both flink names and prime fd support which are provided by 2 different gralloc implementations is problematic because we have a dependency on a specific gralloc implementation header. This mostly disables the dependency on the gralloc implementation and headers. The dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD remains for now, but the definition is added locally to remove the header dependency. drm_gralloc support can be enabled by setting BOARD_USES_DRM_GRALLOC=true in BoardConfig.mk. Signed-off-by: Rob Herring <r...@kernel.org> Signed-off-by: Robert Foss <robert.f...@collabora.com> --- Changes since RFC: - Instead of removing code, #ifdef it out. src/egl/Android.mk | 6 ++- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 56 +++-- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/egl/Android.mk b/src/egl/Android.mk index 11818694f4..8412aeb798 100644 --- a/src/egl/Android.mk +++ b/src/egl/Android.mk @@ -57,9 +57,13 @@ LOCAL_SHARED_LIBRARIES := \ libhardware \ liblog \ libcutils \ - libgralloc_drm \ libsync +ifeq ($(BOARD_USES_DRM_GRALLOC),true) + LOCAL_CFLAGS += -DHAVE_DRM_GRALLOC + LOCAL_SHARED_LIBRARIES += libgralloc_drm +endif + ifeq ($(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5 6 7),) LOCAL_SHARED_LIBRARIES += libnativewindow endif diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index adabc527f8..5d8fbfa235 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -67,8 +67,6 @@ struct zwp_linux_dmabuf_v1; #include #include -#include - #endif /* HAVE_ANDROID_PLATFORM */ #include "eglconfig.h" diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 1d6ed92bd6..4ba96aad90 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -37,7 +37,11 @@ #include "loader.h" #include "egl_dri2.h" #include "egl_dri2_fallbacks.h" + +#ifdef HAVE_DRM_GRALLOC +#include #include "gralloc_drm.h" +#endif /* HAVE_DRM_GRALLOC */ #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1)) @@ -164,11 +168,13 @@ get_native_buffer_fd(struct ANativeWindowBuffer *buf) return (handle && handle->numFds) ? handle->data[0] : -1; } +#ifdef HAVE_DRM_GRALLOC static int get_native_buffer_name(struct ANativeWindowBuffer *buf) { return gralloc_drm_get_gem_handle(buf->handle); } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf) @@ -838,6 +844,7 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx, return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list); } +#ifdef HAVE_DRM_GRALLOC static _EGLImage * droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, struct ANativeWindowBuffer *buf) @@ -881,6 +888,7 @@ droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, return _img->base; } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf, @@ -937,7 +945,11 @@ dri2_create_image_android_native_buffer(_EGLDisplay *disp, if (fd >= 0) return droid_create_image_from_prime_fd(disp, ctx, buf, fd); +#ifdef HAVE_DRM_GRALLOC return droid_create_image_from_name(disp, ctx, buf); +#else + return NULL; +#endif } static _EGLImage * @@ -959,6 +971,7 @@ droid_flush_front_buffer(__DRIdrawable * driDrawable, void *loaderPrivate) { } +#ifdef HAVE_DRM_GRALLOC static int droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf, unsigned int *attachments, int count) @@ -1034,6 +1047,7 @@ droid_get_buffers_with_format(__DRIdrawable * driDrawable, return dri2_surf->buffers; } +#endif /* HAVE_DRM_GRALLOC */ static unsigned droid_get_capability(void *loaderPrivate, enum dri_loader_cap cap) @@ -1116,6 +1130,14 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } +enum { +/* perform(const struct gralloc_module_t *mod, + * int op, + * int *fd); + */ +GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, +}; + static int droid_open_device(struct dri2_egl_display *dri2_dpy) { @@ -1158,6 +1180,7 @@ static const struct dri2_egl_display_vtbl droid_display_vtbl = { .get_dri_drawable = dri2_surface_get_dri_drawable, }; +#ifdef HAVE_DRM_GRALLOC static const __DRIdri2LoaderExtension droid_dri2_loader_extension = { .base = { __DRI_DRI2_LOADER, 4 }, @@ -1166,6 +1189,7 @@ static const __DRIdri2LoaderExtension droid_dri2_
[Mesa-dev] [PATCH 3/3] egl/android: Add DRM node probing and filtering
This patch both adds support for probing & filtering DRM nodes and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD gralloc call. Currently the filtering is based just on the driver name, and the desired name is supplied using the "drm.gpu.vendor_name" Android property. The filtering itself is done using the newly introduced libdrm drmHandleMatch() call. Signed-off-by: Robert Foss <robert.f...@collabora.com> --- Changes since v1: - Do not rely on libdrm for probing - Distinguish between errors and when no drm devices are found Changes since RFC: - Rebased on newer libdrm drmHandleMatch patch - Added support for driver probing src/egl/drivers/dri2/platform_android.c | 202 +--- 1 file changed, 149 insertions(+), 53 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 4ba96aad90..76e5474e48 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -27,6 +27,7 @@ * DEALINGS IN THE SOFTWARE. */ +#include #include #include #include @@ -1130,31 +1131,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } -enum { -/* perform(const struct gralloc_module_t *mod, - * int op, - * int *fd); - */ -GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, -}; - -static int -droid_open_device(struct dri2_egl_display *dri2_dpy) -{ - int fd = -1, err = -EINVAL; - - if (dri2_dpy->gralloc->perform) - err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc, - GRALLOC_MODULE_PERFORM_GET_DRM_FD, - ); - if (err || fd < 0) { - _eglLog(_EGL_WARNING, "fail to get drm fd"); - fd = -1; - } - - return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1; -} - static const struct dri2_egl_display_vtbl droid_display_vtbl = { .authenticate = NULL, .create_window_surface = droid_create_window_surface, @@ -1215,6 +1191,151 @@ static const __DRIextension *droid_image_loader_extensions[] = { NULL, }; +EGLBoolean +droid_load_driver(_EGLDisplay *disp) +{ + struct dri2_egl_display *dri2_dpy = disp->DriverData; + const char *err; + + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) { + err = "DRI2: failed to get driver name"; + goto error; + } + + dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER; + + if (!dri2_dpy->is_render_node) { + #ifdef HAVE_DRM_GRALLOC + /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names +* for backwards compatibility with drm_gralloc. (Do not use on new +* systems.) */ + dri2_dpy->loader_extensions = droid_dri2_loader_extensions; + if (!dri2_load_driver(disp)) { + err = "DRI2: failed to load driver"; + goto error; + } + #else + err = "DRI2: handle is not for a render node"; + goto error; + #endif + } else { + dri2_dpy->loader_extensions = droid_image_loader_extensions; + if (!dri2_load_driver_dri3(disp)) { + err = "DRI3: failed to load driver"; + goto error; + } +} + + return EGL_TRUE; + +error: + free(dri2_dpy->driver_name); + dri2_dpy->driver_name = NULL; + return _eglError(EGL_NOT_INITIALIZED, err); +} + +static int +droid_probe_driver(_EGLDisplay *disp, int fd) +{ + struct dri2_egl_display *dri2_dpy = disp->DriverData; + dri2_dpy->fd = fd; + + if (!droid_load_driver(disp)) + return false; + + /* Since this probe can succeed, but another filter may fail, + this string needs to be deallocated either way. + Once an FD has been found, this string will be set a second time. */ + free(dri2_dpy->driver_name); + dri2_dpy->driver_name = NULL; + return true; +} + +static int +droid_probe_device(_EGLDisplay *disp, int fd, drmDevicePtr dev, char *vendor) +{ + drmVersionPtr ver = drmGetVersion(fd); + if (!ver) + goto fail; + + size_t vendor_len = strlen(vendor); + if (vendor_len != 0 && strncmp(vendor, ver->name, vendor_len)) + goto fail; + + if (!droid_probe_driver(disp, fd)) + goto fail; + + drmFreeVersion(ver); + return true; + +fail: + drmFreeVersion(ver); + return false; +} + +static int +droid_open_device(_EGLDisplay *disp) +{ + const int MAX_DRM_DEVICES = 32; + int prop_set, num_devices, ret; + int fd = -1, fallback_fd = -1; + + char vendor_name[PROPERTY_VALUE_MAX]; + property_get("drm.gpu.vendor_name", vendor_name, NULL); + + drmDevicePtr devices[MAX_DRM_DEVICES]; + num_devices = drmGetDevices2(0, devices, MAX_DRM_DEVICES); + + if (num_devices &l
[Mesa-dev] [PATCH 1/3] gallium/util: Fix build error due to cast to different size
Signed-off-by: Robert Foss <robert.f...@collabora.com> --- src/gallium/auxiliary/util/u_debug_stack_android.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gallium/auxiliary/util/u_debug_stack_android.cpp b/src/gallium/auxiliary/util/u_debug_stack_android.cpp index b3d56aebe6..395a1fe911 100644 --- a/src/gallium/auxiliary/util/u_debug_stack_android.cpp +++ b/src/gallium/auxiliary/util/u_debug_stack_android.cpp @@ -49,10 +49,10 @@ debug_backtrace_capture(debug_stack_frame *mesa_backtrace, backtrace_table = _mesa_hash_table_create(NULL, _mesa_hash_pointer, _mesa_key_pointer_equal); - backtrace_entry = _mesa_hash_table_search(backtrace_table, (void*) tid); + backtrace_entry = _mesa_hash_table_search(backtrace_table, (void*) (uintptr_t)tid); if (!backtrace_entry) { backtrace = Backtrace::Create(getpid(), tid); - _mesa_hash_table_insert(backtrace_table, (void*) tid, backtrace); + _mesa_hash_table_insert(backtrace_table, (void*) (uintptr_t)tid, backtrace); } else { backtrace = (Backtrace *) backtrace_entry->data; } -- 2.17.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v1 1/2] egl/android: #ifdef out flink name support
Heya, On 2018-05-01 18:44, Rob Herring wrote: On Tue, May 1, 2018 at 3:13 AM, Robert Foss <robert.f...@collabora.com> wrote: Hey Rob, On 2018-05-01 04:20, Rob Herring wrote: On Fri, Apr 27, 2018 at 6:57 AM, Robert Foss <robert.f...@collabora.com> wrote: From: Rob Herring <r...@kernel.org> Maintaining both flink names and prime fd support which are provided by 2 different gralloc implementations is problematic because we have a dependency on a specific gralloc implementation header. This mostly disables the dependency on the gralloc implementation and headers. The dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD remains for now, but the definition is added locally to remove the header dependency. drm_gralloc support can be enabled by setting BOARD_USES_DRM_GRALLOC=true in BoardConfig.mk. Signed-off-by: Rob Herring <r...@kernel.org> Signed-off-by: Robert Foss <robert.f...@collabora.com> --- Changes since RFC: - Rebased on newer libdrm drmHandleMatch patch - Added support for driver probing src/egl/Android.mk | 6 - src/egl/drivers/dri2/egl_dri2.h | 2 -- src/egl/drivers/dri2/platform_android.c | 41 +++-- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/egl/Android.mk b/src/egl/Android.mk index 11818694f4..8412aeb798 100644 --- a/src/egl/Android.mk +++ b/src/egl/Android.mk @@ -57,9 +57,13 @@ LOCAL_SHARED_LIBRARIES := \ libhardware \ liblog \ libcutils \ - libgralloc_drm \ libsync +ifeq ($(BOARD_USES_DRM_GRALLOC),true) + LOCAL_CFLAGS += -DHAVE_DRM_GRALLOC + LOCAL_SHARED_LIBRARIES += libgralloc_drm +endif + ifeq ($(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5 6 7),) LOCAL_SHARED_LIBRARIES += libnativewindow endif diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index adabc527f8..5d8fbfa235 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -67,8 +67,6 @@ struct zwp_linux_dmabuf_v1; #include #include -#include - #endif /* HAVE_ANDROID_PLATFORM */ #include "eglconfig.h" diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 7f1a496ea2..ab1337f750 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -37,7 +37,11 @@ #include "loader.h" #include "egl_dri2.h" #include "egl_dri2_fallbacks.h" + +#ifdef HAVE_DRM_GRALLOC +#include #include "gralloc_drm.h" +#endif /* HAVE_DRM_GRALLOC */ #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1)) @@ -164,11 +168,13 @@ get_native_buffer_fd(struct ANativeWindowBuffer *buf) return (handle && handle->numFds) ? handle->data[0] : -1; } +#ifdef HAVE_DRM_GRALLOC static int get_native_buffer_name(struct ANativeWindowBuffer *buf) { return gralloc_drm_get_gem_handle(buf->handle); } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf) @@ -836,6 +842,7 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx, return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list); } +#ifdef HAVE_DRM_GRALLOC static _EGLImage * droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, struct ANativeWindowBuffer *buf) @@ -879,6 +886,7 @@ droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, return _img->base; } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf, @@ -935,7 +943,11 @@ dri2_create_image_android_native_buffer(_EGLDisplay *disp, if (fd >= 0) return droid_create_image_from_prime_fd(disp, ctx, buf, fd); +#ifdef HAVE_DRM_GRALLOC return droid_create_image_from_name(disp, ctx, buf); +#else + return NULL; +#endif } static _EGLImage * @@ -957,6 +969,7 @@ droid_flush_front_buffer(__DRIdrawable * driDrawable, void *loaderPrivate) { } +#ifdef HAVE_DRM_GRALLOC static int droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf, unsigned int *attachments, int count) @@ -1032,6 +1045,7 @@ droid_get_buffers_with_format(__DRIdrawable * driDrawable, return dri2_surf->buffers; } +#endif /* HAVE_DRM_GRALLOC */ static unsigned droid_get_capability(void *loaderPrivate, enum dri_loader_cap cap) @@ -1114,6 +1128,14 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } +enum { +/* perform(const struct gralloc_module_t *mod, + * int op, + * int *fd); + */ +GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, +}; Since you are keeping the hea
Re: [Mesa-dev] [PATCH v1 1/2] egl/android: #ifdef out flink name support
Hey Rob, On 2018-05-01 04:20, Rob Herring wrote: On Fri, Apr 27, 2018 at 6:57 AM, Robert Foss <robert.f...@collabora.com> wrote: From: Rob Herring <r...@kernel.org> Maintaining both flink names and prime fd support which are provided by 2 different gralloc implementations is problematic because we have a dependency on a specific gralloc implementation header. This mostly disables the dependency on the gralloc implementation and headers. The dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD remains for now, but the definition is added locally to remove the header dependency. drm_gralloc support can be enabled by setting BOARD_USES_DRM_GRALLOC=true in BoardConfig.mk. Signed-off-by: Rob Herring <r...@kernel.org> Signed-off-by: Robert Foss <robert.f...@collabora.com> --- Changes since RFC: - Rebased on newer libdrm drmHandleMatch patch - Added support for driver probing src/egl/Android.mk | 6 - src/egl/drivers/dri2/egl_dri2.h | 2 -- src/egl/drivers/dri2/platform_android.c | 41 +++-- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/egl/Android.mk b/src/egl/Android.mk index 11818694f4..8412aeb798 100644 --- a/src/egl/Android.mk +++ b/src/egl/Android.mk @@ -57,9 +57,13 @@ LOCAL_SHARED_LIBRARIES := \ libhardware \ liblog \ libcutils \ - libgralloc_drm \ libsync +ifeq ($(BOARD_USES_DRM_GRALLOC),true) + LOCAL_CFLAGS += -DHAVE_DRM_GRALLOC + LOCAL_SHARED_LIBRARIES += libgralloc_drm +endif + ifeq ($(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5 6 7),) LOCAL_SHARED_LIBRARIES += libnativewindow endif diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index adabc527f8..5d8fbfa235 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -67,8 +67,6 @@ struct zwp_linux_dmabuf_v1; #include #include -#include - #endif /* HAVE_ANDROID_PLATFORM */ #include "eglconfig.h" diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 7f1a496ea2..ab1337f750 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -37,7 +37,11 @@ #include "loader.h" #include "egl_dri2.h" #include "egl_dri2_fallbacks.h" + +#ifdef HAVE_DRM_GRALLOC +#include #include "gralloc_drm.h" +#endif /* HAVE_DRM_GRALLOC */ #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1)) @@ -164,11 +168,13 @@ get_native_buffer_fd(struct ANativeWindowBuffer *buf) return (handle && handle->numFds) ? handle->data[0] : -1; } +#ifdef HAVE_DRM_GRALLOC static int get_native_buffer_name(struct ANativeWindowBuffer *buf) { return gralloc_drm_get_gem_handle(buf->handle); } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf) @@ -836,6 +842,7 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx, return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list); } +#ifdef HAVE_DRM_GRALLOC static _EGLImage * droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, struct ANativeWindowBuffer *buf) @@ -879,6 +886,7 @@ droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, return _img->base; } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf, @@ -935,7 +943,11 @@ dri2_create_image_android_native_buffer(_EGLDisplay *disp, if (fd >= 0) return droid_create_image_from_prime_fd(disp, ctx, buf, fd); +#ifdef HAVE_DRM_GRALLOC return droid_create_image_from_name(disp, ctx, buf); +#else + return NULL; +#endif } static _EGLImage * @@ -957,6 +969,7 @@ droid_flush_front_buffer(__DRIdrawable * driDrawable, void *loaderPrivate) { } +#ifdef HAVE_DRM_GRALLOC static int droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf, unsigned int *attachments, int count) @@ -1032,6 +1045,7 @@ droid_get_buffers_with_format(__DRIdrawable * driDrawable, return dri2_surf->buffers; } +#endif /* HAVE_DRM_GRALLOC */ static unsigned droid_get_capability(void *loaderPrivate, enum dri_loader_cap cap) @@ -1114,6 +1128,14 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } +enum { +/* perform(const struct gralloc_module_t *mod, + * int op, + * int *fd); + */ +GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, +}; Since you are keeping the header dependency, you can drop this hunk. I'm a bit confused by this comment, which header dependency are you thinking of? The gralloc_drm.h inclusion in platform_android.h is the on
Re: [Mesa-dev] [PATCH v1 1/2] egl/android: #ifdef out flink name support
Hey, On 2018-05-01 08:29, Tomasz Figa wrote: On Tue, May 1, 2018 at 11:20 AM Rob Herring <r...@kernel.org> wrote: On Fri, Apr 27, 2018 at 6:57 AM, Robert Foss <robert.f...@collabora.com> wrote: From: Rob Herring <r...@kernel.org> [snip] @@ -1228,20 +1254,31 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp) dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER; - /* render nodes cannot use Gem names, and thus do not support -* the __DRI_DRI2_LOADER extension */ if (!dri2_dpy->is_render_node) { +#ifdef HAVE_DRM_GRALLOC dri2_dpy->loader_extensions = droid_dri2_loader_extensions; if (!dri2_load_driver(disp)) { err = "DRI2: failed to load driver"; goto cleanup; } } else { + /* render nodes cannot use Gem names, and thus do not support + * the __DRI_DRI2_LOADER extension */ I think we can replace this comment with something properly reflecting the bad nature of this code. See below. dri2_dpy->loader_extensions = droid_image_loader_extensions; if (!dri2_load_driver_dri3(disp)) { err = "DRI3: failed to load driver"; goto cleanup; } +#else + err = "DRI2: handle is not for a render node"; + goto cleanup; + } + + dri2_dpy->loader_extensions = droid_image_loader_extensions; + if (!dri2_load_driver_dri3(disp)) { Do we really need this twice? Yeah, I think we could do with something like below: if (!dri2_dpy->is_render_node) { #ifdef HAVE_DRM_GRALLOC /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names * for backwards compatibility with drm_gralloc. (Do not use on new systems.) */ dri2_dpy->loader_extensions = droid_dri2_loader_extensions; if (!dri2_load_driver(disp)) { err = "DRI2: failed to load driver"; goto cleanup; } #else err = "DRI2: handle is not for a render node"; goto cleanup; #endif } else { dri2_dpy->loader_extensions = droid_image_loader_extensions; if (!dri2_load_driver_dri3(disp)) { err = "DRI3: failed to load driver"; goto cleanup; } } This does look better to me too, I'll use it for v2. Rob. Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v1 2/2] egl/android: Add DRM node probing and filtering
This patch both adds support for probing & filtering DRM nodes and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD gralloc call. Currently the filtering is based just on the driver name, and the desired name is supplied using the "drm.gpu.vendor_name" Android property. The filtering itself is done using the newly introduced libdrm drmHandleMatch() call. Signed-off-by: Robert Foss <robert.f...@collabora.com> --- Changes since RFC: - Instead of removing code, #ifdef it out. src/egl/drivers/dri2/platform_android.c | 193 ++-- 1 file changed, 135 insertions(+), 58 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index ab1337f750..c1690f996e 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -27,6 +27,7 @@ * DEALINGS IN THE SOFTWARE. */ +#include #include #include #include @@ -1128,31 +1129,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } -enum { -/* perform(const struct gralloc_module_t *mod, - * int op, - * int *fd); - */ -GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, -}; - -static int -droid_open_device(struct dri2_egl_display *dri2_dpy) -{ - int fd = -1, err = -EINVAL; - - if (dri2_dpy->gralloc->perform) - err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc, - GRALLOC_MODULE_PERFORM_GET_DRM_FD, - ); - if (err || fd < 0) { - _eglLog(_EGL_WARNING, "fail to get drm fd"); - fd = -1; - } - - return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1; -} - static const struct dri2_egl_display_vtbl droid_display_vtbl = { .authenticate = NULL, .create_window_surface = droid_create_window_surface, @@ -1213,6 +1189,137 @@ static const __DRIextension *droid_image_loader_extensions[] = { NULL, }; +EGLBoolean +droid_load_driver(_EGLDisplay *disp) +{ + struct dri2_egl_display *dri2_dpy = disp->DriverData; + const char *err; + + dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd); + if (dri2_dpy->driver_name == NULL) { + err = "DRI2: failed to get driver name"; + goto error; + } + + dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER; + + if (!dri2_dpy->is_render_node) { +#ifdef HAVE_DRM_GRALLOC + dri2_dpy->loader_extensions = droid_dri2_loader_extensions; + if (!dri2_load_driver(disp)) { + err = "DRI2: failed to load driver"; + goto error; + } + } else { + /* render nodes cannot use Gem names, and thus do not support + * the __DRI_DRI2_LOADER extension */ + dri2_dpy->loader_extensions = droid_image_loader_extensions; + if (!dri2_load_driver_dri3(disp)) { + err = "DRI3: failed to load driver"; + goto error; + } +#else + err = "DRI2: handle is not for a render node"; + goto error; + } + + dri2_dpy->loader_extensions = droid_image_loader_extensions; + if (!dri2_load_driver_dri3(disp)) { + err = "DRI3: failed to load driver"; + goto error; +#endif + } + + return EGL_TRUE; + +error: + free(dri2_dpy->driver_name); + dri2_dpy->driver_name = NULL; + return _eglError(EGL_NOT_INITIALIZED, err); +} + +static int +droid_probe_driver(int fd, void *data) +{ + _EGLDisplay *disp = data; + struct dri2_egl_display *dri2_dpy = disp->DriverData; + dri2_dpy->fd = fd; + + if (!droid_load_driver(disp)) + return false; + + /* Since this probe can succeed, but another filter may failed + this string needs to be deallocated either way. + Once an FD has been found, this string will be set a second time. */ + free(dri2_dpy->driver_name); + dri2_dpy->driver_name = NULL; + return true; +} + +static int +droid_open_device(_EGLDisplay *disp) +{ + const int MAX_DRM_DEVICES = 32; + int prop_set, num_devices, ret; + int fd = -1, fallback_fd = -1; + + char vendor_name[PROPERTY_VALUE_MAX]; + property_get("drm.gpu.vendor_name", vendor_name, NULL); + + drm_match_t filters[] = { + {DRM_MATCH_DRIVER_NAME, .str = vendor_name }, + {DRM_MATCH_FUNCTION, .func = { .fp = droid_probe_driver, + .data = disp }}, + }; + const int nbr_filters = sizeof(filters)/sizeof(drm_match_t); + + drmDevicePtr devices[MAX_DRM_DEVICES]; + num_devices = drmGetDevices2(0, devices, MAX_DRM_DEVICES); + + if (num_devices < 0) { + _eglLog(_EGL_WARNING, "Failed to find any DRM devices"); + return -1; + } + + for (int i = 0; i < num_devices; i++) { + char *dev_path = devices[i]->nodes[DRM_NODE_RENDER]; + fd = l
[Mesa-dev] [PATCH v1 1/2] egl/android: #ifdef out flink name support
From: Rob Herring <r...@kernel.org> Maintaining both flink names and prime fd support which are provided by 2 different gralloc implementations is problematic because we have a dependency on a specific gralloc implementation header. This mostly disables the dependency on the gralloc implementation and headers. The dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD remains for now, but the definition is added locally to remove the header dependency. drm_gralloc support can be enabled by setting BOARD_USES_DRM_GRALLOC=true in BoardConfig.mk. Signed-off-by: Rob Herring <r...@kernel.org> Signed-off-by: Robert Foss <robert.f...@collabora.com> --- Changes since RFC: - Rebased on newer libdrm drmHandleMatch patch - Added support for driver probing src/egl/Android.mk | 6 - src/egl/drivers/dri2/egl_dri2.h | 2 -- src/egl/drivers/dri2/platform_android.c | 41 +++-- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/egl/Android.mk b/src/egl/Android.mk index 11818694f4..8412aeb798 100644 --- a/src/egl/Android.mk +++ b/src/egl/Android.mk @@ -57,9 +57,13 @@ LOCAL_SHARED_LIBRARIES := \ libhardware \ liblog \ libcutils \ - libgralloc_drm \ libsync +ifeq ($(BOARD_USES_DRM_GRALLOC),true) + LOCAL_CFLAGS += -DHAVE_DRM_GRALLOC + LOCAL_SHARED_LIBRARIES += libgralloc_drm +endif + ifeq ($(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5 6 7),) LOCAL_SHARED_LIBRARIES += libnativewindow endif diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index adabc527f8..5d8fbfa235 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -67,8 +67,6 @@ struct zwp_linux_dmabuf_v1; #include #include -#include - #endif /* HAVE_ANDROID_PLATFORM */ #include "eglconfig.h" diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 7f1a496ea2..ab1337f750 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -37,7 +37,11 @@ #include "loader.h" #include "egl_dri2.h" #include "egl_dri2_fallbacks.h" + +#ifdef HAVE_DRM_GRALLOC +#include #include "gralloc_drm.h" +#endif /* HAVE_DRM_GRALLOC */ #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1)) @@ -164,11 +168,13 @@ get_native_buffer_fd(struct ANativeWindowBuffer *buf) return (handle && handle->numFds) ? handle->data[0] : -1; } +#ifdef HAVE_DRM_GRALLOC static int get_native_buffer_name(struct ANativeWindowBuffer *buf) { return gralloc_drm_get_gem_handle(buf->handle); } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf) @@ -836,6 +842,7 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx, return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list); } +#ifdef HAVE_DRM_GRALLOC static _EGLImage * droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, struct ANativeWindowBuffer *buf) @@ -879,6 +886,7 @@ droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, return _img->base; } +#endif /* HAVE_DRM_GRALLOC */ static EGLBoolean droid_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf, @@ -935,7 +943,11 @@ dri2_create_image_android_native_buffer(_EGLDisplay *disp, if (fd >= 0) return droid_create_image_from_prime_fd(disp, ctx, buf, fd); +#ifdef HAVE_DRM_GRALLOC return droid_create_image_from_name(disp, ctx, buf); +#else + return NULL; +#endif } static _EGLImage * @@ -957,6 +969,7 @@ droid_flush_front_buffer(__DRIdrawable * driDrawable, void *loaderPrivate) { } +#ifdef HAVE_DRM_GRALLOC static int droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf, unsigned int *attachments, int count) @@ -1032,6 +1045,7 @@ droid_get_buffers_with_format(__DRIdrawable * driDrawable, return dri2_surf->buffers; } +#endif /* HAVE_DRM_GRALLOC */ static unsigned droid_get_capability(void *loaderPrivate, enum dri_loader_cap cap) @@ -1114,6 +1128,14 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) return (config_count != 0); } +enum { +/* perform(const struct gralloc_module_t *mod, + * int op, + * int *fd); + */ +GRALLOC_MODULE_PERFORM_GET_DRM_FD = 0x4002, +}; + static int droid_open_device(struct dri2_egl_display *dri2_dpy) { @@ -1156,6 +1178,7 @@ static const struct dri2_egl_display_vtbl droid_display_vtbl = { .get_dri_drawable = dri2_surface_get_dri_drawable, }; +#ifdef HAVE_DRM_GRALLOC static const __DRIdri2LoaderExtension droid_dri2_loader_extension = { .base = { __DRI_DRI2_LOADER, 4 }, @@ -1164,6 +1187,7 @
[Mesa-dev] [PATCH v1 0/2] egl/android: Remove dependencies on specific grallocs
v1 of this series is based on the previous RFC[1]. This series is intended to remove the dependency on any one specific gralloc implementation, as well as introduce device probing. The probing support is built ontop of [2], which supplies a drmHandleMatch() function which allows is to test opened device FD against the properties we are interested in. Changes since RFC: - Rebased work on the libdrm patch [2]. - Included patch from Rob Herring disabling drm_gralloc/flink support by default. - Added device handler driver probing. [1] https://patchwork.freedesktop.org/patch/217750/ [2] https://patchwork.freedesktop.org/patch/219194/ Rob Herring (1): egl/android: #ifdef out flink name support Robert Foss (1): egl/android: Add DRM node probing and filtering src/egl/Android.mk | 6 +- src/egl/drivers/dri2/egl_dri2.h | 2 - src/egl/drivers/dri2/platform_android.c | 192 +--- 3 files changed, 158 insertions(+), 42 deletions(-) -- 2.14.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: remove flink name support
On 04/26/2018 07:43 PM, Emil Velikov wrote: On 26 April 2018 at 18:34, Rob Herring <r...@kernel.org> wrote: On Thu, Apr 26, 2018 at 11:56 AM, Emil Velikov <emil.l.veli...@gmail.com> wrote: On 26 April 2018 at 03:30, Chih-Wei Huang <cwhu...@linux.org.tw> wrote: 2018-04-25 19:55 GMT+08:00 Robert Foss <robert.f...@collabora.com>: Hey Emil & Chih-Wei, On 04/24/2018 01:59 PM, Emil Velikov wrote: On 24 April 2018 at 12:28, Emil Velikov <emil.l.veli...@gmail.com> wrote: On the topic of keeping the old code behind a #define or just removing it, it'll be great if interested parties can reach a consensus. Actually one can simply drop this code and drm_gralloc users can add a drm_ioctl_permit() hack. Namely: loosen the restrictions to consider render nodes identical to primary/card ones. Yes, it's a nasty hack, yet no worse than the existing one that removes the auth :-\ I'm fine with adding a #define. Chih-Wei: Do you have any objections? "keeping the old code behind a #define"? Sounds good to me. Thank you! Having a look at the Android-x86 kernel: The current hack in the permission check (see drm_ioctl_permit in [1]) effectively threats render nodes and primary nodes as the same thing. Thus, on top of the current patch the !dri2_dpy->is_render_node check should be removed, for Android-x86. There's no need to keep the flink code around ;-) Except I don't think drm_gralloc fills in a dma-buf fd in it's handle... There was an AOSP version that did IIRC. Or maybe I'm missing something. You're probably right - I haven't looked closely at drm_gralloc in ages. Yet again, considering the hacked auth, there should be no blockers to tweaking drm_gralloc. Either way, I'm just overly hyped looking for ways to remove the flink code. Feel free to ignore my suggestions. I've spent some time today preparing a #ifdef version of what robher submitted. It's fine, but there is no way automatically enable the HAVE_DRM_GRALLOC #define through the build tools. So a small simple patch adding the shared library dependency, and adding the -DHAVE_DRM_GRALLOC cflag is needed. I'll send it out tomorrow, a bit late in the day for sending out patches now ;) Rob. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: remove flink name support
Hey Emil & Chih-Wei, On 04/24/2018 01:59 PM, Emil Velikov wrote: On 24 April 2018 at 12:28, Emil Velikovwrote: On the topic of keeping the old code behind a #define or just removing it, it'll be great if interested parties can reach a consensus. Actually one can simply drop this code and drm_gralloc users can add a drm_ioctl_permit() hack. Namely: loosen the restrictions to consider render nodes identical to primary/card ones. Yes, it's a nasty hack, yet no worse than the existing one that removes the auth :-\ I'm fine with adding a #define. Chih-Wei: Do you have any objections? Rob. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/android: remove flink name support
Hey Chih-Wei, On 04/23/2018 04:17 AM, Chih-Wei Huang wrote: What's the impact to drm_gralloc? I'm not terribly familiar with drm_gralloc, but as far as I understand it depends on flink name support at the moment. The overarching idea here is to make mesa gralloc independent, but also at the same time provide some infrastructure for making the grallocs able to be separate. I'm not sure what the best way to keep drm_gralloc support alive and at the same time make mesa gralloc independent is. Rob. 2018-04-20 5:09 GMT+08:00 Rob Herring: Maintaining both flink names and prime fd support which are provided by 2 different gralloc implementations is problematic because we have a dependency on a specific gralloc implementation header. This mostly removes the dependency on the gralloc implementation and headers. The dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD remains for now, but the definition is added locally to remove the header dependency. Signed-off-by: Rob Herring --- With this plus Robert's probing patch, we remove any gralloc implementation dependency (other than it has to be a pre 1.0 implementation...). src/egl/drivers/dri2/egl_dri2.h | 1 - src/egl/drivers/dri2/platform_android.c | 172 2 files changed, 17 insertions(+), 156 deletions(-) diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index cc76c73eab2f..d9e1f466fbeb 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -61,7 +61,6 @@ struct zwp_linux_dmabuf_v1; #include #include -#include #endif /* HAVE_ANDROID_PLATFORM */ diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 4f25cb746915..68c85d7f99fa 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -37,7 +37,6 @@ #include "loader.h" #include "egl_dri2.h" #include "egl_dri2_fallbacks.h" -#include "gralloc_drm.h" #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1)) @@ -164,12 +163,6 @@ get_native_buffer_fd(struct ANativeWindowBuffer *buf) return (handle && handle->numFds) ? handle->data[0] : -1; } -static int -get_native_buffer_name(struct ANativeWindowBuffer *buf) -{ - return gralloc_drm_get_gem_handle(buf->handle); -} - static EGLBoolean droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf) { @@ -822,50 +815,6 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx, return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list); } -static _EGLImage * -droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, - struct ANativeWindowBuffer *buf) -{ - struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp); - struct dri2_egl_image *dri2_img; - int name; - int format; - - name = get_native_buffer_name(buf); - if (!name) { - _eglError(EGL_BAD_PARAMETER, "eglCreateEGLImageKHR"); - return NULL; - } - - format = get_format(buf->format); - if (format == -1) - return NULL; - - dri2_img = calloc(1, sizeof(*dri2_img)); - if (!dri2_img) { - _eglError(EGL_BAD_ALLOC, "droid_create_image_mesa_drm"); - return NULL; - } - - _eglInitImage(_img->base, disp); - - dri2_img->dri_image = - dri2_dpy->image->createImageFromName(dri2_dpy->dri_screen, - buf->width, - buf->height, - format, - name, - buf->stride, - dri2_img); - if (!dri2_img->dri_image) { - free(dri2_img); - _eglError(EGL_BAD_ALLOC, "droid_create_image_mesa_drm"); - return NULL; - } - - return _img->base; -} - static EGLBoolean droid_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf, EGLint attribute, EGLint *value) @@ -921,7 +870,7 @@ dri2_create_image_android_native_buffer(_EGLDisplay *disp, if (fd >= 0) return droid_create_image_from_prime_fd(disp, ctx, buf, fd); - return droid_create_image_from_name(disp, ctx, buf); + return NULL; } static _EGLImage * @@ -943,82 +892,6 @@ droid_flush_front_buffer(__DRIdrawable * driDrawable, void *loaderPrivate) { } -static int -droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf, -unsigned int *attachments, int count) -{ - int num_buffers = 0; - - /* fill dri2_surf->buffers */ - for (int i = 0; i < count * 2; i += 2) { - __DRIbuffer *buf, *local; - - assert(num_buffers < ARRAY_SIZE(dri2_surf->buffers)); - buf = _surf->buffers[num_buffers]; - - switch (attachments[i]) { - case __DRI_BUFFER_BACK_LEFT: - if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
Re: [Mesa-dev] [PATCH] egl/android: remove flink name support
Hey Rob, This looks good to me. Do you mind if I carry this patch my series to get it all pushed upstream at once? Rob. On 04/19/2018 11:09 PM, Rob Herring wrote: Maintaining both flink names and prime fd support which are provided by 2 different gralloc implementations is problematic because we have a dependency on a specific gralloc implementation header. This mostly removes the dependency on the gralloc implementation and headers. The dependency on GRALLOC_MODULE_PERFORM_GET_DRM_FD remains for now, but the definition is added locally to remove the header dependency. Signed-off-by: Rob Herring--- With this plus Robert's probing patch, we remove any gralloc implementation dependency (other than it has to be a pre 1.0 implementation...). src/egl/drivers/dri2/egl_dri2.h | 1 - src/egl/drivers/dri2/platform_android.c | 172 2 files changed, 17 insertions(+), 156 deletions(-) diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index cc76c73eab2f..d9e1f466fbeb 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -61,7 +61,6 @@ struct zwp_linux_dmabuf_v1; #include #include -#include #endif /* HAVE_ANDROID_PLATFORM */ diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 4f25cb746915..68c85d7f99fa 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -37,7 +37,6 @@ #include "loader.h" #include "egl_dri2.h" #include "egl_dri2_fallbacks.h" -#include "gralloc_drm.h" #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1)) @@ -164,12 +163,6 @@ get_native_buffer_fd(struct ANativeWindowBuffer *buf) return (handle && handle->numFds) ? handle->data[0] : -1; } -static int -get_native_buffer_name(struct ANativeWindowBuffer *buf) -{ - return gralloc_drm_get_gem_handle(buf->handle); -} - static EGLBoolean droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf) { @@ -822,50 +815,6 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx, return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list); } -static _EGLImage * -droid_create_image_from_name(_EGLDisplay *disp, _EGLContext *ctx, - struct ANativeWindowBuffer *buf) -{ - struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp); - struct dri2_egl_image *dri2_img; - int name; - int format; - - name = get_native_buffer_name(buf); - if (!name) { - _eglError(EGL_BAD_PARAMETER, "eglCreateEGLImageKHR"); - return NULL; - } - - format = get_format(buf->format); - if (format == -1) - return NULL; - - dri2_img = calloc(1, sizeof(*dri2_img)); - if (!dri2_img) { - _eglError(EGL_BAD_ALLOC, "droid_create_image_mesa_drm"); - return NULL; - } - - _eglInitImage(_img->base, disp); - - dri2_img->dri_image = - dri2_dpy->image->createImageFromName(dri2_dpy->dri_screen, - buf->width, - buf->height, - format, - name, - buf->stride, - dri2_img); - if (!dri2_img->dri_image) { - free(dri2_img); - _eglError(EGL_BAD_ALLOC, "droid_create_image_mesa_drm"); - return NULL; - } - - return _img->base; -} - static EGLBoolean droid_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf, EGLint attribute, EGLint *value) @@ -921,7 +870,7 @@ dri2_create_image_android_native_buffer(_EGLDisplay *disp, if (fd >= 0) return droid_create_image_from_prime_fd(disp, ctx, buf, fd); - return droid_create_image_from_name(disp, ctx, buf); + return NULL; } static _EGLImage * @@ -943,82 +892,6 @@ droid_flush_front_buffer(__DRIdrawable * driDrawable, void *loaderPrivate) { } -static int -droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf, -unsigned int *attachments, int count) -{ - int num_buffers = 0; - - /* fill dri2_surf->buffers */ - for (int i = 0; i < count * 2; i += 2) { - __DRIbuffer *buf, *local; - - assert(num_buffers < ARRAY_SIZE(dri2_surf->buffers)); - buf = _surf->buffers[num_buffers]; - - switch (attachments[i]) { - case __DRI_BUFFER_BACK_LEFT: - if (dri2_surf->base.Type == EGL_WINDOW_BIT) { -buf->attachment = attachments[i]; -buf->name = get_native_buffer_name(dri2_surf->buffer); -buf->cpp = get_format_bpp(dri2_surf->buffer->format); -buf->pitch = dri2_surf->buffer->stride * buf->cpp; -buf->flags = 0; - -if (buf->name) - num_buffers++; - -break; - } - /*
Re: [Mesa-dev] [RFC] egl/android: Add DRM node probing and filtering
On 04/20/2018 09:38 AM, Tomasz Figa wrote: On Fri, Apr 20, 2018 at 4:17 PM Robert Foss <robert.f...@collabora.com> wrote: On 04/20/2018 06:41 AM, Tomasz Figa wrote: Hi Rob, On Thu, Apr 19, 2018 at 1:03 AM Robert Foss <robert.f...@collabora.com> wrote: This patch both adds support for probing & filtering DRM nodes and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD gralloc call. Currently the filtering is based just on the driver name, and the desired name is supplied using the "drm.gpu.vendor_name" Android property. The filtering itself is done using the newly introduced libdrm drmHandleMatch() call. Signed-off-by: Robert Foss <robert.f...@collabora.com> --- This patch is based on[1], which contains a new libdrm function, called drmHandleMatch(), which allows for matching an opened drm node handle against some desired properties. A choice that was made for this patch was to add support for falling back to to DRM nodes that have failed the filtering, if no node passes the filter. If this wouldn't be useful to anyone, I would suggest ripping it out since it is a little bit ugly This is actually not only useful, but quite of a requirement for Chrome OS, where we use the same Android image for devices with different GPU vendors. (Technically we could hack it up by injecting some per-board properties, but it would be painful from maintenance point of view.) Ack, I'll remove the fallback. Sorry, I mean a fallback is needed. :) (+/- the missing driver check) My mistake, I'll leave it in then! But actually, the way it is implemented in this patch will not work. The code assumes that the first DRM node that could be opened (loader_open_device()) is fine, but if you look at what we do in our Chromium patch [2], it is actually necessary to check if Mesa includes a driver for it (loader_get_driver_for_fd()). [2] https://chromium-review.googlesource.com/c/chromiumos/third_party/mesa/+/780797 Ack, I'll add a check for a driver existing. SGTM. Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] egl/android: Add DRM node probing and filtering
On 04/20/2018 06:41 AM, Tomasz Figa wrote: Hi Rob, On Thu, Apr 19, 2018 at 1:03 AM Robert Foss <robert.f...@collabora.com> wrote: This patch both adds support for probing & filtering DRM nodes and switches away from using the GRALLOC_MODULE_PERFORM_GET_DRM_FD gralloc call. Currently the filtering is based just on the driver name, and the desired name is supplied using the "drm.gpu.vendor_name" Android property. The filtering itself is done using the newly introduced libdrm drmHandleMatch() call. Signed-off-by: Robert Foss <robert.f...@collabora.com> --- This patch is based on[1], which contains a new libdrm function, called drmHandleMatch(), which allows for matching an opened drm node handle against some desired properties. A choice that was made for this patch was to add support for falling back to to DRM nodes that have failed the filtering, if no node passes the filter. If this wouldn't be useful to anyone, I would suggest ripping it out since it is a little bit ugly This is actually not only useful, but quite of a requirement for Chrome OS, where we use the same Android image for devices with different GPU vendors. (Technically we could hack it up by injecting some per-board properties, but it would be painful from maintenance point of view.) Ack, I'll remove the fallback. But actually, the way it is implemented in this patch will not work. The code assumes that the first DRM node that could be opened (loader_open_device()) is fine, but if you look at what we do in our Chromium patch [2], it is actually necessary to check if Mesa includes a driver for it (loader_get_driver_for_fd()). [2] https://chromium-review.googlesource.com/c/chromiumos/third_party/mesa/+/780797 Ack, I'll add a check for a driver existing. [1] https://www.spinics.net/lists/dri-devel/msg172497.html src/egl/drivers/dri2/platform_android.c | 72 - 1 file changed, 62 insertions(+), 10 deletions(-) diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 7f1a496ea24..0b082fe5dcc 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -27,6 +27,7 @@ * DEALINGS IN THE SOFTWARE. */ +#include #include #include #include @@ -1117,18 +1118,69 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy) static int droid_open_device(struct dri2_egl_display *dri2_dpy) { - int fd = -1, err = -EINVAL; - - if (dri2_dpy->gralloc->perform) - err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc, - GRALLOC_MODULE_PERFORM_GET_DRM_FD, - ); - if (err || fd < 0) { - _eglLog(_EGL_WARNING, "fail to get drm fd"); - fd = -1; + int prop_set, num_devices, ret; + const int node_type = DRM_NODE_RENDER; + int fd = -1, fallback_fd = -1; + + const int MAX_DRM_DEVICES = 32; + char vendor_name[PROPERTY_VALUE_MAX]; + prop_set = property_get("drm.gpu.vendor_name", vendor_name, NULL); + drmVersion ver_filter; + ver_filter.name = vendor_name; + + drmDevicePtr devices[MAX_DRM_DEVICES]; + num_devices = drmGetDevices2(0, devices, MAX_DRM_DEVICES); + if (num_devices < 0) { + _eglLog(_EGL_WARNING, "Failed to find any DRM devices"); + return -1; + } + + for (int i = 0; i < num_devices; i++) { + /* Filter out DRM_NODE_ types we aren't interested in */ + if (!(devices[i]->available_nodes & (1 << node_type))) { + continue; + } + + /* Open DRM node FD */ + fd = loader_open_device(devices[i]->nodes[node_type]); + if (fd == -1 && errno == EINVAL) { + _eglLog(_EGL_WARNING, "%s() node #%d failed to open", __func__, i); + continue; + } What should happen if (fd == -1 && errno != EINVAL)? I don't think we should run the code below in such case. + + /* See if FD matches the driver vendor we want */ + if (prop_set && !drmHandleMatch(fd, _filter, NULL)){ + _eglLog(_EGL_WARNING, "%s() node #%d FD=%d does not match filter", __func__, i , fd); How about printing the actual node (i.e. devices[i]->nodes[node_type]), rather than index in devices array? Also, should it be _EGL_DEBUG? I'd expect it to be a normal event, which should not concern the user. + goto next; + } + + /* Successfully found matching FD */ + close(fallback_fd); + fallback_fd = -1; + break; + +next: + if (fallback_fd == -1) { + fallback_fd = fd; + fd = -1; + } else { + close(fd); + fd = -1; + } + continue; + } How about simplifying the code a bit: + for (int i = 0; i < num_devices; i++) { + /* Filter out DRM_NODE_ types we aren't interested in */ + if (!(devices[i]->available