qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 0/2] vhost-user-gpu get_edid feature


From: Marc-André Lureau
Subject: Re: [PATCH 0/2] vhost-user-gpu get_edid feature
Date: Mon, 15 May 2023 15:38:04 +0400

Hi

On Thu, May 11, 2023 at 5:02 PM Erico Nunes <ernunes@redhat.com> wrote:
This adds support to the virtio-gpu get_edid command when using the
vhost-user-gpu implementation in contrib/.
So far, qemu has been outputting the following message:
  EDID requested but the backend doesn't support it.
when using that implementation.

This is tested with vhost-user-gpu, the dbus ui backend and the
monitor-edid application, which now shows complete "QEMU Monitor" edid
data.

In this v1, I would appreciate some feedback especially regarding:
- Can we enable it by default or do need to create another config option
  flag for it?

Enabled as default is ok I think
 
- Can we now also remove the "EDID requested but the backend doesn't
  support it." warning and logic from hw/display or do we still want to
  keep that around for other potential implementations of
  vhost-user-gpu?

Imho, that should remain. (vhost-user-gpu could have set edid=false by default to avoid this error, but then it would need to be explicitly turned on to match the backend implementation)
 
- The structs used as payloads of the vhost-user-gpu messages. Looks
  like there was no command so far requiring bidirectional messages with
  different payloads so I just based it on similar available ones.


That looks fine to me
 
Thanks


Erico Nunes (2):
  virtio-gpu: refactor generate_edid function to virtio_gpu_base
  vhost-user-gpu: implement get_edid feature

 contrib/vhost-user-gpu/vhost-user-gpu.c | 53 ++++++++++++++++++++++++-
 contrib/vhost-user-gpu/virgl.c          |  3 ++
 contrib/vhost-user-gpu/vugpu.h          |  8 ++++
 docs/interop/vhost-user-gpu.rst         |  9 +++++
 hw/display/vhost-user-gpu.c             | 31 +++++++++++++++
 hw/display/virtio-gpu-base.c            | 17 ++++++++
 hw/display/virtio-gpu.c                 | 20 +---------
 include/hw/virtio/virtio-gpu.h          |  2 +
 8 files changed, 122 insertions(+), 21 deletions(-)

--
2.39.2



I wonder if the backend couldn't have avoided the need for calling the front-end (the VHOST_USER_GPU_GET_EDID call). But after all, it can still implement it on its own, so it's optional anyway.

However, I worry about using the new backend (calling GET_EDID) with an older front-end/QEMU. It may just hang, since vhost_user_gpu_handle_display() won't reply to unknown messages. That's what PROTOCOL_FEATURES were meant for, iirc. Can you check? thanks


--
Marc-André Lureau

reply via email to

[Prev in Thread] Current Thread [Next in Thread]