On Feb 18, 2017 12:37 PM, "Kai Wasserbäch" <k...@dev.carbon-project.org> 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: https://bugs.freedesktop.org/show_bug.cgi?id=99715 > Co-authored-by: Jason Ekstrand <jason.ekstr...@intel.com> > --- > 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. With that fixed, this patch is Reviewed-by: Kai Wasserbäch <k...@dev.carbon-project.org> 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; > } > _______________________________________________ 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
