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