On Thu, Jun 21, 2018 at 7:37 AM, Keith Packard <kei...@keithp.com> wrote:
> Jason Ekstrand <ja...@jlekstrand.net> writes:
>
> >> + if (!ret)
> >> + return VK_SUCCESS;
> >> +
> >> + if (errno != ENOMEM) {
> >
> > This strikes me as a bit odd. What does ENOMEM mean if not "out of
> > memory"?
>
> ENOMEM means that the queue is full and that we should drain it and try
> again; that's what the wait_for_event call is below.
>
> The other-than-ENOMEM case is for some other failure, such as VT switch
> or lease revoke. For RegisterDisplayEvent, there aren't any return
> values other than VK_SUCCESS defined, and we're already assuming we can
> use VK_OUT_OF_HOST_MEMORY for any function which allocates memory.
>
> I think the correct value might be VK_ERROR_DEVICE_LOST or
> VK_ERROR_OUT_OF_DATE_KHR as something "bad" has clearly happened? The
> other place this is called is from QueuePresent, where either of those
> error codes are allowed. I could convert that message to
> VK_OUT_OF_HOST_MEMORY for RegisterDisplayEvent if you think that's a
> good idea.
>
> The sleep prevents an application from spinning at this failure,
> allowing the user to gracefully terminate the application.
>
> >
> >> + wsi_display_debug("queue vblank event %lu failed\n",
> >> fence->sequence);
> >> + struct timespec delay = {
> >> + .tv_sec = 0,
> >> + .tv_nsec = 100000000ull,
> >> + };
> >> + nanosleep(&delay, NULL);
> >> + return VK_ERROR_OUT_OF_HOST_MEMORY;
> >
> > Given your previous explanation, I think this is ok but I think it
> deserves
> > a comment.
>
> Wilco.
>
> I've added comments to this section to try and explain what's going on:
>
> if (!ret)
> return VK_SUCCESS;
>
> if (errno != ENOMEM) {
>
> /* Something unexpected happened. Pause for a moment so the
> * application doesn't just spin and then return a failure
> indication
> */
>
> wsi_display_debug("queue vblank event %lu failed\n",
> fence->sequence);
> struct timespec delay = {
> .tv_sec = 0,
> .tv_nsec = 100000000ull,
> };
> nanosleep(&delay, NULL);
> return VK_ERROR_OUT_OF_HOST_MEMORY;
>
I don't really like VK_ERROR_OUT_OF_HOST_MEMORY here but I don't know what
else to do at the moment. The error codes for this extension are not
well-defined... I think I'm fine with it for now.
> }
>
> /* The kernel event queue is full. Wait for some events to be
> * processed and try again
> */
>
> pthread_mutex_lock(&wsi->wait_mutex);
> ret = wsi_display_wait_for_event(wsi, wsi_rel_to_abs_time(100000000u
> ll));
> pthread_mutex_unlock(&wsi->wait_mutex);
>
> if (ret) {
> wsi_display_debug("vblank queue full, event wait failed\n");
> return VK_ERROR_OUT_OF_HOST_MEMORY;
> }
Looks good. With that, patches 1-3 are
Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net>
I'll let Dave or Bas review your fence hackery in radv.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
