Message ID | 20230714153900.475857-1-ernunes@redhat.com (mailing list archive) |
---|---|
Headers | show |
Series | vhost-user-gpu: support dmabuf modifiers | expand |
Hi On Fri, Jul 14, 2023 at 7:42 PM Erico Nunes <ernunes@redhat.com> wrote: > virglrenderer recently added virgl_renderer_resource_get_info_ext as a > new api, which gets resource information, including dmabuf modifiers. > https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1024 > > We have to support dmabuf modifiers since the driver may choose to > allocate buffers with these modifiers to improve performance, and > importing buffers without modifiers information may result in completely > broken rendering. > > Currently trying to use vhost-user-gpu for rendering backend and using > the qemu dbus ui as a ui backend results in a broken framebuffer with > Intel GPUs as the buffer is allocated with a modifier. With this > patchset, that is fixed. > > > It is tricky to support since it requires to keep compatibility at the > same time with: > (1) build against older virglrenderer which do not provide > virgl_renderer_resource_get_info_ext; > (2) runtime between frontend (qemu) and backend (vhost-user-gpu) due to > increased size and a new field in the VHOST_USER_GPU_DMABUF_SCANOUT > message. > > I tried to reach a compromise here by not defining a completely new > message and duplicate VHOST_USER_GPU_DMABUF_SCANOUT but it still feels > like a bit of a hack, so I appreciate feedback if there is a better way > (or naming) to handle it. > looks fine to me, we may consider this as a fix for 8.1 imho Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > > Erico Nunes (3): > docs: vhost-user-gpu: add protocol changes for dmabuf modifiers > contrib/vhost-user-gpu: add support for sending dmabuf modifiers > vhost-user-gpu: support dmabuf modifiers > > contrib/vhost-user-gpu/vhost-user-gpu.c | 5 ++- > contrib/vhost-user-gpu/virgl.c | 51 +++++++++++++++++++++++-- > contrib/vhost-user-gpu/vugpu.h | 9 +++++ > docs/interop/vhost-user-gpu.rst | 26 ++++++++++++- > hw/display/vhost-user-gpu.c | 17 ++++++++- > 5 files changed, 102 insertions(+), 6 deletions(-) > > -- > 2.40.1 > > >
On Fri, Jul 14, 2023 at 05:38:57PM +0200, Erico Nunes wrote: > virglrenderer recently added virgl_renderer_resource_get_info_ext as a > new api, which gets resource information, including dmabuf modifiers. > https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1024 > > We have to support dmabuf modifiers since the driver may choose to > allocate buffers with these modifiers to improve performance, and > importing buffers without modifiers information may result in completely > broken rendering. > > Currently trying to use vhost-user-gpu for rendering backend and using > the qemu dbus ui as a ui backend results in a broken framebuffer with > Intel GPUs as the buffer is allocated with a modifier. With this > patchset, that is fixed. > > > It is tricky to support since it requires to keep compatibility at the > same time with: > (1) build against older virglrenderer which do not provide > virgl_renderer_resource_get_info_ext; > (2) runtime between frontend (qemu) and backend (vhost-user-gpu) due to > increased size and a new field in the VHOST_USER_GPU_DMABUF_SCANOUT > message. > > I tried to reach a compromise here by not defining a completely new > message and duplicate VHOST_USER_GPU_DMABUF_SCANOUT but it still feels > like a bit of a hack, so I appreciate feedback if there is a better way > (or naming) to handle it. > > > Erico Nunes (3): > docs: vhost-user-gpu: add protocol changes for dmabuf modifiers > contrib/vhost-user-gpu: add support for sending dmabuf modifiers > vhost-user-gpu: support dmabuf modifiers > > contrib/vhost-user-gpu/vhost-user-gpu.c | 5 ++- > contrib/vhost-user-gpu/virgl.c | 51 +++++++++++++++++++++++-- > contrib/vhost-user-gpu/vugpu.h | 9 +++++ > docs/interop/vhost-user-gpu.rst | 26 ++++++++++++- > hw/display/vhost-user-gpu.c | 17 ++++++++- > 5 files changed, 102 insertions(+), 6 deletions(-) Series: Reviewed-by: Sergio Lopez <slp@redhat.com>
Hello, On 14/07/2023 22:03, Marc-André Lureau wrote: > Hi > > On Fri, Jul 14, 2023 at 7:42 PM Erico Nunes <ernunes@redhat.com > <mailto:ernunes@redhat.com>> wrote: > > virglrenderer recently added virgl_renderer_resource_get_info_ext as a > new api, which gets resource information, including dmabuf modifiers. > https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1024 <https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1024> > > We have to support dmabuf modifiers since the driver may choose to > allocate buffers with these modifiers to improve performance, and > importing buffers without modifiers information may result in completely > broken rendering. > > Currently trying to use vhost-user-gpu for rendering backend and using > the qemu dbus ui as a ui backend results in a broken framebuffer with > Intel GPUs as the buffer is allocated with a modifier. With this > patchset, that is fixed. > > > It is tricky to support since it requires to keep compatibility at the > same time with: > (1) build against older virglrenderer which do not provide > virgl_renderer_resource_get_info_ext; > (2) runtime between frontend (qemu) and backend (vhost-user-gpu) due to > increased size and a new field in the VHOST_USER_GPU_DMABUF_SCANOUT > message. > > I tried to reach a compromise here by not defining a completely new > message and duplicate VHOST_USER_GPU_DMABUF_SCANOUT but it still feels > like a bit of a hack, so I appreciate feedback if there is a better way > (or naming) to handle it. > > > looks fine to me, we may consider this as a fix for 8.1 imho > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com > <mailto:marcandre.lureau@redhat.com>> Just making sure this one didn't fall through the cracks; should I do something about this series or is it set to be in an upcoming merge? Thanks Erico