<div dir="auto">Just to double check, is there anything else I need to do to have this patch committed?<div dir="auto">Jacob Lifshay</div></div><div class="gmail_extra"> <div class="gmail_quote">On Feb 19, 2017 02:08, "Kai Wasserbäch" <<a href="mailto:kai@dev.carbon-project.org">kai@dev.carbon-project.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Jason Ekstrand wrote on 19.02.2017 06:01: > On Feb 18, 2017 12:37 PM, "Kai Wasserbäch" <<a href="mailto:kai@dev.carbon-project.org">kai@dev.carbon-project.org</a>> > wrote: > > Hey Jacob, > sorry for not spotting this the first time, but I have an additional > comment. > Please see below. > > Jacob Lifshay wrote on 18.02.2017 18:48:> This commit improves the message > by > telling them that they could probably >> enable DRI3. More importantly, it includes a little heuristic to check >> to see if we're running on AMD or NVIDIA's proprietary X11 drivers and, >> if we are, doesn't emit the warning. This way, users with both a discrete >> card and Intel graphics don't get the warning when they're just running >> on the discrete card. >> >> Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=99715" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=99715</a> >> Co-authored-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> >> --- >> src/vulkan/wsi/wsi_common_x11.c | 47 ++++++++++++++++++++++++++++++ > ++--------- >> 1 file changed, 37 insertions(+), 10 deletions(-) >> >> diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_ > x11.c >> index 64ba921..b3a017a 100644 >> --- a/src/vulkan/wsi/wsi_common_x11.c >> +++ b/src/vulkan/wsi/wsi_common_x11.c >> @@ -49,6 +49,7 @@ >> struct wsi_x11_connection { >> bool has_dri3; >> bool has_present; >> + bool is_proprietary_x11; >> }; >> >> struct wsi_x11 { >> @@ -63,8 +64,8 @@ static struct wsi_x11_connection * >> wsi_x11_connection_create(const VkAllocationCallbacks *alloc, >> xcb_connection_t *conn) >> { >> - xcb_query_extension_cookie_t dri3_cookie, pres_cookie; >> - xcb_query_extension_reply_t *dri3_reply, *pres_reply; >> + xcb_query_extension_cookie_t dri3_cookie, pres_cookie, amd_cookie, > nv_cookie; >> + xcb_query_extension_reply_t *dri3_reply, *pres_reply, *amd_reply, > *nv_reply; >> >> struct wsi_x11_connection *wsi_conn = >> vk_alloc(alloc, sizeof(*wsi_conn), 8, >> @@ -75,20 +76,39 @@ wsi_x11_connection_create(const VkAllocationCallbacks > *alloc, >> dri3_cookie = xcb_query_extension(conn, 4, "DRI3"); >> pres_cookie = xcb_query_extension(conn, 7, "PRESENT"); >> >> + /* We try to be nice to users and emit a warning if they try to use a >> + * Vulkan application on a system without DRI3 enabled. However, > this ends >> + * up spewing the warning when a user has, for example, both Intel >> + * integrated graphics and a discrete card with proprietary driers > and are >> + * running on the discrete card with the proprietary DDX. In this > case, we >> + * really don't want to print the warning because it just confuses > users. >> + * As a heuristic to detect this case, we check for a couple of > proprietary >> + * X11 extensions. >> + */ >> + amd_cookie = xcb_query_extension(conn, 11, "ATIFGLRXDRI"); >> + nv_cookie = xcb_query_extension(conn, 10, "NV-CONTROL"); >> + >> dri3_reply = xcb_query_extension_reply(conn, dri3_cookie, NULL); >> pres_reply = xcb_query_extension_reply(conn, pres_cookie, NULL); >> - if (dri3_reply == NULL || pres_reply == NULL) { >> + amd_reply = xcb_query_extension_reply(conn, amd_cookie, NULL); >> + nv_reply = xcb_query_extension_reply(conn, nv_cookie, NULL); >> + if (!dri3_reply || !pres_reply || !amd_reply || !nv_reply) { > > I don't feel wsi_x11_connection_create should fail if there's no amd_reply > or > nv_reply. That should just lead to unconditionally warning, in case there's > no > DRI3 support. > > > Of there is no reply then we either lost our connection to the X server or > ran out of memory. Either of those seem like a valid excuse to fail. The > chances of successfully connecting to X to create a swapchain at that point > is pretty close to zero. Fair enough. > With that fixed, this patch is > Reviewed-by: Kai Wasserbäch <<a href="mailto:kai@dev.carbon-project.org">kai@dev.carbon-project.org</a>> > > Cheers, > Kai > >> free(dri3_reply); >> free(pres_reply); >> + free(amd_reply); >> + free(nv_reply); >> vk_free(alloc, wsi_conn); >> return NULL; >> } >> >> wsi_conn->has_dri3 = dri3_reply->present != 0; >> wsi_conn->has_present = pres_reply->present != 0; >> + wsi_conn->is_proprietary_x11 = amd_reply->present || > nv_reply->present; >> >> free(dri3_reply); >> free(pres_reply); >> + free(amd_reply); >> + free(nv_reply); >> >> return wsi_conn; >> } >> @@ -100,6 +120,18 @@ wsi_x11_connection_destroy(const > VkAllocationCallbacks *alloc, >> vk_free(alloc, conn); >> } >> >> +static bool >> +wsi_x11_check_for_dri3(struct wsi_x11_connection *wsi_conn) >> +{ >> + if (wsi_conn->has_dri3) >> + return true; >> + if (!wsi_conn->is_proprietary_x11) { >> + fprintf(stderr, "vulkan: No DRI3 support detected - required for > presentation\n"); >> + "Note: you can probably enable DRI3 in your Xorg > config\n"); >> + } >> + return false; >> +} >> + >> static struct wsi_x11_connection * >> wsi_x11_get_connection(struct wsi_device *wsi_dev, >> const VkAllocationCallbacks *alloc, >> @@ -264,11 +296,8 @@ VkBool32 wsi_get_physical_device_xcb_ > presentation_support( >> if (!wsi_conn) >> return false; >> >> - if (!wsi_conn->has_dri3) { >> - fprintf(stderr, "vulkan: No DRI3 support detected - required for > presentation\n"); >> - fprintf(stderr, "Note: Buggy applications may crash, if they do > please report to vendor\n"); >> + if (!wsi_x11_check_for_dri3(wsi_conn)) >> return false; >> - } >> >> unsigned visual_depth; >> if (!connection_get_visualtype(connection, visual_id, &visual_depth)) >> @@ -313,9 +342,7 @@ x11_surface_get_support(VkIcdSurfaceBase *icd_surface, >> if (!wsi_conn) >> return VK_ERROR_OUT_OF_HOST_MEMORY; >> >> - if (!wsi_conn->has_dri3) { >> - fprintf(stderr, "vulkan: No DRI3 support detected - required for > presentation\n"); >> - fprintf(stderr, "Note: Buggy applications may crash, if they do > please report to vendor\n"); >> + if (!wsi_x11_check_for_dri3(wsi_conn)) { >> *pSupported = false; >> return VK_SUCCESS; >> } </blockquote></div></div>