Hi On Thu, May 11, 2023 at 5:02 PM Erico Nunes <ernu...@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
