<div dir="auto"><div><div class="gmail_extra"><div class="gmail_quote">On Feb 18, 2017 1:59 AM, "Kai Wasserbäch" <<a href="mailto:kai@dev.carbon-project.org">kai@dev.carbon-project.org</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hey Jacob,<br>
<div class="quoted-text">Jacob Lifshay wrote on 18.02.2017 10:40:<br>
> This commit improves the message by telling them that they could probably<br>
> enable DRI3 and giving a url to a Ask Ubuntu question showing how to do<br>
> that.<br>
<br>
</div>wouldn't it be better to add a page to the mesa user topics section (ie. add a<br>
file to docs and then add the link in docs/contents.html)? That way nobody is<br>
dependant on whether AskUbuntu works, nobody gets thrown off, if they don't use<br>
Ubuntu and might think the link won't work for them and lastly it would allow<br>
for easy referencing of the relevant driver man pages with more details on the<br>
DRI option, including information if/when this might be the default for a driver.</blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">I agree.  Honestly, I don't think a link is really needed.  Of the user has the Intel or amdgpu DDX and they don't have DRI3, then they almost certainly disabled it themselves.  If they figured out how to do that, one would think they could figure out how to undo it.  At the very least, they could Google this warning message and that will probably get them to the right ask Ubuntu page or whatever.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="quoted-text">
> More importantly, it includes a little heuristic to check to see<br>
> if we're running on AMD or NVIDIA's proprietary X11 drivers and, if we<br>
> 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>
</div>Wouldn't it be better to check for the driver name? Right now, you would have to<br>
check for "amdgpu" and "intel" (maybe modesetting as well?). That way you don't<br>
need to rely on the open drivers never implementing one of the proprietary<br>
extensions (even though it seems unlikely they'd do that). Or am I missing<br>
something here.<br></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto">Possibly?  Of there's an easy way to get the driver name, that would work just as well.  That said, I don't think we should get too tied up in knots over it.  This is a heuristic for determining whether or not to print a warning message.</div><div dir="auto"><br></div><div dir="auto">Before too much longer (maybe another year or two?), we can probably drop the warning message all together because hopefully the DRI3 fear well have subsided and few people will even have thous problem.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Cheers,<br>
Kai<br>
<div class="elided-text"><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 | 55 ++++++++++++++++++++++++++++++<wbr>+++--------<br>
>  1 file changed, 45 insertions(+), 10 deletions(-)<br>
><br>
> diff --git a/src/vulkan/wsi/wsi_common_<wbr>x11.c b/src/vulkan/wsi/wsi_common_<wbr>x11.c<br>
> index 64ba921..ac7a972 100644<br>
> --- a/src/vulkan/wsi/wsi_common_<wbr>x11.c<br>
> +++ b/src/vulkan/wsi/wsi_common_<wbr>x11.c<br>
> @@ -48,7 +48,9 @@<br>
><br>
>  struct wsi_x11_connection {<br>
>     bool has_dri3;<br>
> +   bool has_dri2;<br>
>     bool has_present;<br>
> +   bool is_proprietary_x11;<br>
>  };<br>
><br>
>  struct wsi_x11 {<br>
> @@ -63,8 +65,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, dri2_cookie, pres_cookie, amd_cookie, nv_cookie;<br>
> +   xcb_query_extension_reply_t *dri3_reply, *dri2_reply, *pres_reply, *amd_reply, *nv_reply;<br>
><br>
>     struct wsi_x11_connection *wsi_conn =<br>
>        vk_alloc(alloc, sizeof(*wsi_conn), 8,<br>
> @@ -73,22 +75,46 @@ wsi_x11_connection_create(<wbr>const VkAllocationCallbacks *alloc,<br>
>        return NULL;<br>
><br>
>     dri3_cookie = xcb_query_extension(conn, 4, "DRI3");<br>
> +   dri2_cookie = xcb_query_extension(conn, 4, "DRI2");<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, 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 and are<br>
> +    * running on the discrete card with the proprietary DDX.  In this case, we<br>
> +    * really don't want to print the warning because it just confuses users.<br>
> +    * As a heuristic to detect this case, we check for a couple of 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>
> +   dri2_reply = xcb_query_extension_reply(<wbr>conn, dri2_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 || !dri2_reply || !pres_reply || !amd_reply || !nv_reply) {<br>
>        free(dri3_reply);<br>
> +      free(dri2_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_dri2 = dri2_reply->present != 0;<br>
>     wsi_conn->has_present = pres_reply->present != 0;<br>
> +   wsi_conn->is_proprietary_x11 = amd_reply->present || nv_reply->present;<br>
><br>
>     free(dri3_reply);<br>
> +   free(dri2_reply);<br>
>     free(pres_reply);<br>
> +   free(amd_reply);<br>
> +   free(nv_reply);<br>
><br>
>     return wsi_conn;<br>
>  }<br>
> @@ -100,6 +126,20 @@ wsi_x11_connection_destroy(<wbr>const 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 presentation\n");<br>
> +    if (wsi_conn->has_dri2)<br></div></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">I really don't see what checking for DRI2 is gaining us here.  Even the proprietary X drivers advertise it so out will basically always be true.  It seems to just make the code more complicated.  Also, if they happen to be running on a software-only X driver, they probably still want the warning message.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="elided-text">
> +      fprintf(stderr, "Note: DRI2 support detected, you can probably enable DRI3 in your Xorg config;\n"<br>
> +                      "      see <a href="http://askubuntu.com/questions/817226/how-to-enable-dri3-on-ubuntu-16-04\n" rel="noreferrer" target="_blank">http://askubuntu.com/<wbr>questions/817226/how-to-<wbr>enable-dri3-on-ubuntu-16-04\n</a>"<wbr>);<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 +304,8 @@ VkBool32 wsi_get_physical_device_xcb_<wbr>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 presentation\n");<br>
> -      fprintf(stderr, "Note: Buggy applications may crash, if they do 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 +350,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 presentation\n");<br>
> -      fprintf(stderr, "Note: Buggy applications may crash, if they do 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>
<br>
--<br>
<br>
</div>Kai Wasserbäch (Kai Wasserbaech)<br>
<br>
E-Mail: <a href="mailto:kai@dev.carbon-project.org">kai@dev.carbon-project.org</a><br>
<br>
<br>______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
<br></blockquote></div><br></div></div></div>