<div dir="auto"><div><div class="gmail_extra"><div class="gmail_quote">On Jan 11, 2017 9:30 AM, "Chad Versace" <<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="quoted-text">On Wed 11 Jan 2017, Emil Velikov wrote:<br>
> On 11 January 2017 at 03:59, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> > On Tue, Jan 10, 2017 at 7:17 PM, Chad Versace <<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>><br>
> > wrote:<br>
> >><br>
> >> Loader interface v2 differs from v1 in that the first ICD entrypoint<br>
> >> called by the loader is vk_<wbr>icdNegotiateLoaderICDInterface<wbr>Version(), not<br>
> >> vk_icdGetInstanceProcAddr(). The ICD must statically expose this<br>
> >> entrypoint.<br>
> >> ---<br>
> >>  src/intel/vulkan/anv_device.c | 43<br>
> >> ++++++++++++++++++++++++++++++<wbr>+++++++++++++<br>
> >>  1 file changed, 43 insertions(+)<br>
<br>
<br>
<br>
</div><div class="quoted-text">> >> +/* Suppress warnings because vk_icd.h does not declare this function.<br>
> >> + * Clang supports this pragma too.<br>
> >> + */<br>
> >> +#pragma GCC diagnostic push<br>
> >> +#pragma GCC diagnostic ignored "-Wmissing-prototypes"<br>
><br>
> Since neither new nor old headers provide a declaration I'd just add<br>
> one here, add a comment "XXX: vk_icd.h does not provide the<br>
> declaration of ..." and drop the pragmas ?<br>
<br>
</div>Sure. I'll resubmit with the pragma dropped and a different comment.<br>
<div class="quoted-text"><br>
> >> +    *    - Loader interface v3 differs from v2 in:<br>
> >> +    *        - The ICD must implement vkCreate{PLATFORM}Surface() and<br>
> >> +    *          vkDestroySurface() because the loader no longer does so.<br>
<br>
> Nit: "...SurfaceKHR, vkDestroySurfaceKHR() and other API which uses<br>
> KHR_surface ..."<br>
<br>
</div>Done. I'll resubmit with fixed comment.<br>
<div class="quoted-text"><br>
> ><br>
> > We've implemented these since the dawn of time.  I think we can claim v3.<br>
> ><br>
> I believe you're spot on. We're missing support for some extensions<br>
> (VK_KHR_display, VK_KHR_{android,mir,win32}_<wbr>surface) and thus the<br>
> respective API, but that's orthogonal.<br>
><br>
> >><br>
> >> +    */<br>
> >> +   *pSupportedVersion = 2;<br>
<br>
> Since we should advertise the lowest version common across both loader<br>
> and ICD, the following will be the more robust solution ?<br>
><br>
>  *pSupportedVersion = MIN(*pSupportedVersion, 3);<br>
<br>
</div>I hardcoded 2 because no loader will call<br>
vk_<wbr>icdNegotiateLoaderICDInterface<wbr>Version() with a version less than 2.<br>
But, now since we're going to advertise v3, I agree with you. We should<br>
use MIN.<br>
<div class="quoted-text"><br>
> Reading through the v3 changes, I'm leaning that we want the series in<br>
> -stable, since a) it's not new functionality b) things might end up<br>
> pretty nasty as one mixes old loader + new icd and vise-versa.<br>
> What do you guys thing ?<br>
<br>
</div>That sounds good to me. I'd like to hear Jason's opinion too on<br>
backporting this. (Though I expect him to say yes).<br>
</blockquote></div><br></div></div><div class="gmail_extra" dir="auto">Seems fine to me</div></div>