[This time with mesa-dev@ in the list, and less typos]
Hi Rob,
On Mon, 12 Nov 2018 at 15:14, Robert Foss <robert.f...@collabora.com> wrote:
> +++ b/src/gallium/drivers/virgl/virgl_screen.c
> @@ -340,7 +340,7 @@ virgl_get_param(struct pipe_screen *screen, enum pipe_cap
> param)
> case PIPE_CAP_VIDEO_MEMORY:
> return 0;
> case PIPE_CAP_NATIVE_FENCE_FD:
> - return 0;
> + return vscreen->vws->driver_version(vscreen->vws) >= 1;
It seems like the driver_version() vfunc is missing for the vtest winsys.
One could go with an empty stub or drop the function in faviour of a winsys
variable (or bitmask). Personally, I'm leaning towards the latter, although
either will do.
> +static void virgl_fence_server_sync(struct virgl_winsys *vws,
> + struct virgl_cmd_buf *cbuf,
> + struct pipe_fence_handle *fence)
> +{
> + struct virgl_hw_res *hw_res = virgl_hw_res(fence);
> +
> + assert(hw_res->fence_fd != -1);
> +
Skimming at other drivers - they're not using an assert, so I'd change this to
an if statement.
> + if (cbuf->in_fence_fd == -1) {
> + cbuf->in_fence_fd = dup(hw_res->fence_fd);
> + } else {
> + int new_fd = sync_merge("virgl", cbuf->in_fence_fd,
> hw_res->fence_fd);
> + close(cbuf->in_fence_fd);
> + cbuf->in_fence_fd = new_fd;
The above if/else seems like an open-coded version of sync_accumulate().
Despite the above comments, the kernel interface seems reasonable IMHO.
Would be great if one more person else double-checks it though.
With the three bits handled the patch is:
Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
HTH
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
