mbox

[PULL,00/25] Vga 20210510 patches

Message ID 20210510132051.2208563-1-kraxel@redhat.com
State New
Headers show

Pull-request

git://git.kraxel.org/qemu tags/vga-20210510-pull-request

Message

Gerd Hoffmann May 10, 2021, 1:20 p.m. UTC 
The following changes since commit d90f154867ec0ec22fd719164b88716e8fd48672:

  Merge remote-tracking branch 'remotes/dg-gitlab/tags/ppc-for-6.1-20210504' into staging (2021-05-05 20:29:14 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/vga-20210510-pull-request

for you to fetch changes up to b36eb8860f8f4a9c6f131c3fd380116a3017e022:

  virtio-gpu: add virtio-vga-gl (2021-05-10 13:55:28 +0200)

----------------------------------------------------------------
edid: display id support (for 5k+), bugfixes.
virtio-gpu: iommu fix, device split.

----------------------------------------------------------------

Akihiko Odaki (1):
  edid: Make refresh rate configurable

Gerd Hoffmann (21):
  qemu-edid: use qemu_edid_size()
  edid: edid_desc_next
  edid: move xtra3 descriptor
  edid: use dta extension block descriptors
  virtio-gpu: handle partial maps properly
  virtio-gpu: rename virgl source file.
  virtio-gpu: add virtio-gpu-gl-device
  virtio-gpu: move virgl realize + properties
  virtio-gpu: move virgl reset
  virtio-gpu: use class function for ctrl queue handlers
  virtio-gpu: move virgl handle_ctrl
  virtio-gpu: move virgl gl_flushed
  virtio-gpu: move virgl process_cmd
  virtio-gpu: move update_cursor_data
  virtio-gpu: drop VIRGL() macro
  virtio-gpu: move virtio-gpu-gl-device to separate module
  virtio-gpu: drop use_virgl_renderer
  virtio-gpu: move fields to struct VirtIOGPUGL
  virtio-gpu: add virtio-gpu-gl-pci
  modules: add have_vga
  virtio-gpu: add virtio-vga-gl

Konstantin Nazarov (3):
  edid: move timing generation into a separate function
  edid: allow arbitrary-length checksums
  edid: add support for DisplayID extension (5k resolution)

 include/hw/display/edid.h                     |  12 +-
 include/hw/display/vga.h                      |   6 +
 include/hw/virtio/virtio-gpu.h                |  34 ++-
 hw/display/edid-generate.c                    | 232 +++++++++++++-----
 hw/display/vga-pci.c                          |   2 +-
 hw/display/vga.c                              |   2 +
 hw/display/virtio-gpu-base.c                  |   6 +-
 hw/display/virtio-gpu-gl.c                    | 163 ++++++++++++
 hw/display/virtio-gpu-pci-gl.c                |  55 +++++
 .../{virtio-gpu-3d.c => virtio-gpu-virgl.c}   |   7 +-
 hw/display/virtio-gpu.c                       | 218 +++++-----------
 hw/display/virtio-vga-gl.c                    |  47 ++++
 qemu-edid.c                                   |   6 +-
 util/module.c                                 |   7 +
 hw/display/meson.build                        |  19 +-
 15 files changed, 579 insertions(+), 237 deletions(-)
 create mode 100644 hw/display/virtio-gpu-gl.c
 create mode 100644 hw/display/virtio-gpu-pci-gl.c
 rename hw/display/{virtio-gpu-3d.c => virtio-gpu-virgl.c} (99%)
 create mode 100644 hw/display/virtio-vga-gl.c

Comments

no-reply@patchew.org May 10, 2021, 1:43 p.m. UTC | #1 
Patchew URL: https://patchew.org/QEMU/20210510132051.2208563-1-kraxel@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210510132051.2208563-1-kraxel@redhat.com
Subject: [PULL 00/25] Vga 20210510 patches

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210510132051.2208563-1-kraxel@redhat.com -> patchew/20210510132051.2208563-1-kraxel@redhat.com
Switched to a new branch 'test'
13cb22d virtio-gpu: add virtio-vga-gl
ce63459 modules: add have_vga
ec77853 virtio-gpu: add virtio-gpu-gl-pci
9876e4d virtio-gpu: move fields to struct VirtIOGPUGL
db5bc9c virtio-gpu: drop use_virgl_renderer
ddd31ea virtio-gpu: move virtio-gpu-gl-device to separate module
ab7db8c virtio-gpu: drop VIRGL() macro
1adf390 virtio-gpu: move update_cursor_data
8905ff2 virtio-gpu: move virgl process_cmd
ac63831 virtio-gpu: move virgl gl_flushed
a3b8da4 virtio-gpu: move virgl handle_ctrl
89bff04 virtio-gpu: use class function for ctrl queue handlers
7221fc9 virtio-gpu: move virgl reset
ccd7f46 virtio-gpu: move virgl realize + properties
4f688a1 virtio-gpu: add virtio-gpu-gl-device
61598c7 virtio-gpu: rename virgl source file.
494bab3 virtio-gpu: handle partial maps properly
e4ad854 edid: add support for DisplayID extension (5k resolution)
f39498c edid: allow arbitrary-length checksums
dcb1807 edid: move timing generation into a separate function
2a89c33 edid: Make refresh rate configurable
59156e5 edid: use dta extension block descriptors
8dbf087 edid: move xtra3 descriptor
3dfd70f edid: edid_desc_next
3c226b3 qemu-edid: use qemu_edid_size()

=== OUTPUT BEGIN ===
1/25 Checking commit 3c226b321569 (qemu-edid: use qemu_edid_size())
2/25 Checking commit 3dfd70fb4782 (edid: edid_desc_next)
3/25 Checking commit 8dbf0877dbd6 (edid: move xtra3 descriptor)
4/25 Checking commit 59156e5e31ea (edid: use dta extension block descriptors)
5/25 Checking commit 2a89c3372a5c (edid: Make refresh rate configurable)
WARNING: line over 80 characters
#35: FILE: hw/display/edid-generate.c:239:
+    uint64_t clock  = (uint64_t)refresh_rate * (xres + xblank) * (yres + yblank);

ERROR: Macros with complex values should be enclosed in parenthesis
#80: FILE: include/hw/display/edid.h:25:
+#define DEFINE_EDID_PROPERTIES(_state, _edid_info)                         \
+    DEFINE_PROP_UINT32("xres", _state, _edid_info.prefx, 0),               \
+    DEFINE_PROP_UINT32("yres", _state, _edid_info.prefy, 0),               \
+    DEFINE_PROP_UINT32("xmax", _state, _edid_info.maxx, 0),                \
+    DEFINE_PROP_UINT32("ymax", _state, _edid_info.maxy, 0),                \
+    DEFINE_PROP_UINT32("refresh_rate", _state, _edid_info.refresh_rate, 0)

total: 1 errors, 1 warnings, 57 lines checked

Patch 5/25 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

6/25 Checking commit dcb1807991b5 (edid: move timing generation into a separate function)
7/25 Checking commit f39498ca0c3e (edid: allow arbitrary-length checksums)
8/25 Checking commit e4ad8545a719 (edid: add support for DisplayID extension (5k resolution))
9/25 Checking commit 494bab301857 (virtio-gpu: handle partial maps properly)
WARNING: line over 80 characters
#46: FILE: hw/display/virtio-gpu-3d.c:292:
+    ret = virtio_gpu_create_mapping_iov(g, &att_rb, cmd, NULL, &res_iovs, &res_niov);

WARNING: line over 80 characters
#118: FILE: hw/display/virtio-gpu.c:652:
+                qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for"

total: 0 errors, 2 warnings, 142 lines checked

Patch 9/25 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/25 Checking commit 61598c780eef (virtio-gpu: rename virgl source file.)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#16: 
 hw/display/{virtio-gpu-3d.c => virtio-gpu-virgl.c} | 0

total: 0 errors, 1 warnings, 8 lines checked

Patch 10/25 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/25 Checking commit 4f688a135e5c (virtio-gpu: add virtio-gpu-gl-device)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#34: 
new file mode 100644

total: 0 errors, 1 warnings, 75 lines checked

Patch 11/25 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/25 Checking commit ccd7f46f7d09 (virtio-gpu: move virgl realize + properties)
13/25 Checking commit 7221fc95ccb5 (virtio-gpu: move virgl reset)
14/25 Checking commit 89bff048f3c8 (virtio-gpu: use class function for ctrl queue handlers)
15/25 Checking commit a3b8da45651f (virtio-gpu: move virgl handle_ctrl)
16/25 Checking commit ac638311147a (virtio-gpu: move virgl gl_flushed)
17/25 Checking commit 8905ff2e695f (virtio-gpu: move virgl process_cmd)
WARNING: line over 80 characters
#103: FILE: include/hw/virtio/virtio-gpu.h:233:
+void virtio_gpu_simple_process_cmd(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd);

total: 0 errors, 1 warnings, 70 lines checked

Patch 17/25 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
18/25 Checking commit 1adf3905ab9f (virtio-gpu: move update_cursor_data)
19/25 Checking commit ab7db8cb32b1 (virtio-gpu: drop VIRGL() macro)
20/25 Checking commit ddd31ea968b8 (virtio-gpu: move virtio-gpu-gl-device to separate module)
21/25 Checking commit db5bc9ca6371 (virtio-gpu: drop use_virgl_renderer)
ERROR: "foo * bar" should be "foo *bar"
#98: FILE: hw/display/virtio-gpu-gl.c:47:
+    memcpy(s->current_cursor->data, data, pixels * sizeof(uint32_t));

total: 1 errors, 0 warnings, 116 lines checked

Patch 21/25 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

22/25 Checking commit 9876e4d010ed (virtio-gpu: move fields to struct VirtIOGPUGL)
23/25 Checking commit ec77853df000 (virtio-gpu: add virtio-gpu-gl-pci)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

total: 0 errors, 1 warnings, 80 lines checked

Patch 23/25 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
24/25 Checking commit ce63459c6416 (modules: add have_vga)
25/25 Checking commit 13cb22dd106b (virtio-gpu: add virtio-vga-gl)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

total: 0 errors, 1 warnings, 72 lines checked

Patch 25/25 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210510132051.2208563-1-kraxel@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Peter Maydell May 12, 2021, 3:05 p.m. UTC | #2 
On Mon, 10 May 2021 at 14:22, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit d90f154867ec0ec22fd719164b88716e8fd48672:
>
>   Merge remote-tracking branch 'remotes/dg-gitlab/tags/ppc-for-6.1-20210504' into staging (2021-05-05 20:29:14 +0100)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/vga-20210510-pull-request
>
> for you to fetch changes up to b36eb8860f8f4a9c6f131c3fd380116a3017e022:
>
>   virtio-gpu: add virtio-vga-gl (2021-05-10 13:55:28 +0200)
>
> ----------------------------------------------------------------
> edid: display id support (for 5k+), bugfixes.
> virtio-gpu: iommu fix, device split.
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1
for any user-visible changes.

-- PMM
Marc-André Lureau June 30, 2021, 6:16 p.m. UTC | #3 
Hi

On Wed, May 12, 2021 at 7:07 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Mon, 10 May 2021 at 14:22, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > The following changes since commit
> d90f154867ec0ec22fd719164b88716e8fd48672:
> >
> >   Merge remote-tracking branch
> 'remotes/dg-gitlab/tags/ppc-for-6.1-20210504' into staging (2021-05-05
> 20:29:14 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://git.kraxel.org/qemu tags/vga-20210510-pull-request
> >
> > for you to fetch changes up to b36eb8860f8f4a9c6f131c3fd380116a3017e022:
> >
> >   virtio-gpu: add virtio-vga-gl (2021-05-10 13:55:28 +0200)
> >
> > ----------------------------------------------------------------
> > edid: display id support (for 5k+), bugfixes.
> > virtio-gpu: iommu fix, device split.
> >
>
>
> Applied, thanks.
>
> Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1
> for any user-visible changes.
>
> -- PMM
>
>
After that series, the console is flooded with GTK warnings. When the
machine/device is reset, virgl crashes (also reported at
https://gitlab.freedesktop.org/virgl/virglrenderer/-/issues/226)

Unfortunately, it's not easy to pinpoint the responsible patch since it's
not easily bisectable (and it breaks the CLI). I will continue to
investigate.
Marc-André Lureau July 1, 2021, 6:29 a.m. UTC | #4 
Hi

On Wed, Jun 30, 2021 at 10:16 PM Marc-André Lureau <
marcandre.lureau@gmail.com> wrote:

> Hi
>
> On Wed, May 12, 2021 at 7:07 PM Peter Maydell <peter.maydell@linaro.org>
> wrote:
>
>> On Mon, 10 May 2021 at 14:22, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> >
>> > The following changes since commit
>> d90f154867ec0ec22fd719164b88716e8fd48672:
>> >
>> >   Merge remote-tracking branch
>> 'remotes/dg-gitlab/tags/ppc-for-6.1-20210504' into staging (2021-05-05
>> 20:29:14 +0100)
>> >
>> > are available in the Git repository at:
>> >
>> >   git://git.kraxel.org/qemu tags/vga-20210510-pull-request
>> >
>> > for you to fetch changes up to b36eb8860f8f4a9c6f131c3fd380116a3017e022:
>> >
>> >   virtio-gpu: add virtio-vga-gl (2021-05-10 13:55:28 +0200)
>> >
>> > ----------------------------------------------------------------
>> > edid: display id support (for 5k+), bugfixes.
>> > virtio-gpu: iommu fix, device split.
>> >
>>
>>
>> Applied, thanks.
>>
>> Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1
>> for any user-visible changes.
>>
>> -- PMM
>>
>>
> After that series, the console is flooded with GTK warnings. When the
> machine/device is reset, virgl crashes (also reported at
> https://gitlab.freedesktop.org/virgl/virglrenderer/-/issues/226)
>
> Unfortunately, it's not easy to pinpoint the responsible patch since it's
> not easily bisectable (and it breaks the CLI). I will continue to
> investigate.
>


It turns out that I was using -device to add the VGA device, and qemu
didn't treat virtio-vga-gl the same way virtio-vga was. See "[PATCH] vl:
add virtio-vga-gl to the default_list".

The reason for the warnings and virgl crash is that the secondary gfx
console GTK widgets aren't realized by the time virgl needs a context. This
is a pre-existing issue (-display gtk,gl=on -device virtio-vga,virgl=on
-device virtio-gpu,virgl=on qemu crashes)