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