[PATCH 00/15] virtio-gpu: split into two devices.

Gerd Hoffmann posted 15 patches 3 years, 1 month ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210319112147.4138943-1-kraxel@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
include/hw/virtio/virtio-gpu.h                |  31 +++-
hw/display/virtio-gpu-base.c                  |   6 +-
hw/display/virtio-gpu-gl.c                    | 163 ++++++++++++++++++
hw/display/virtio-gpu-pci.c                   |  27 +++
.../{virtio-gpu-3d.c => virtio-gpu-virgl.c}   |   0
hw/display/virtio-gpu.c                       | 142 +++------------
hw/display/virtio-vga.c                       |  30 ++++
util/module.c                                 |   5 +
hw/display/meson.build                        |  11 +-
9 files changed, 281 insertions(+), 134 deletions(-)
create mode 100644 hw/display/virtio-gpu-gl.c
rename hw/display/{virtio-gpu-3d.c => virtio-gpu-virgl.c} (100%)
[PATCH 00/15] virtio-gpu: split into two devices.
Posted by Gerd Hoffmann 3 years, 1 month ago
Currently we have one virtio-gpu device.  Problem with this approach is
that if you compile a full-featured qemu you'll get a virtio-gpu device
which depends on opengl and virgl, so these dependencies must be
installed and the libraries will be loaded into memory even if you don't
use virgl.  Also the code is cluttered with #ifdefs and a bit messy.

This patch series splits the virtio-gpu device into two:

 (1) virtio-gpu-device becomes the non-virgl device, same as
     virtio-gpu-device,virgl=off today.
 (2) virtio-gpu-gl-device is the new virgl device, same as
     virtio-gpu-device,virgl=on today.

When compiling qemu without virglrenderer support virtio-gpu-device
behavior doesn't change.

Gerd Hoffmann (15):
  virtio-gpu: rename virgl source file.
  virtio-gpu: add virtio-gpu-gl-device
  virtio-gpu: add virtio-gpu-gl-pci
  virtio-gpu: add virtio-vga-gl
  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

 include/hw/virtio/virtio-gpu.h                |  31 +++-
 hw/display/virtio-gpu-base.c                  |   6 +-
 hw/display/virtio-gpu-gl.c                    | 163 ++++++++++++++++++
 hw/display/virtio-gpu-pci.c                   |  27 +++
 .../{virtio-gpu-3d.c => virtio-gpu-virgl.c}   |   0
 hw/display/virtio-gpu.c                       | 142 +++------------
 hw/display/virtio-vga.c                       |  30 ++++
 util/module.c                                 |   5 +
 hw/display/meson.build                        |  11 +-
 9 files changed, 281 insertions(+), 134 deletions(-)
 create mode 100644 hw/display/virtio-gpu-gl.c
 rename hw/display/{virtio-gpu-3d.c => virtio-gpu-virgl.c} (100%)

-- 
2.30.2



Re: [PATCH 00/15] virtio-gpu: split into two devices.
Posted by Marc-André Lureau 3 years, 1 month ago
Hi Gerd

On Fri, Mar 19, 2021 at 3:22 PM Gerd Hoffmann <kraxel@redhat.com> wrote:

> Currently we have one virtio-gpu device.  Problem with this approach is
> that if you compile a full-featured qemu you'll get a virtio-gpu device
> which depends on opengl and virgl, so these dependencies must be
> installed and the libraries will be loaded into memory even if you don't
> use virgl.  Also the code is cluttered with #ifdefs and a bit messy.
>
> This patch series splits the virtio-gpu device into two:
>
>  (1) virtio-gpu-device becomes the non-virgl device, same as
>      virtio-gpu-device,virgl=off today.
>  (2) virtio-gpu-gl-device is the new virgl device, same as
>      virtio-gpu-device,virgl=on today.
>
> When compiling qemu without virglrenderer support virtio-gpu-device
> behavior doesn't change.
>

Nice!
We are post 6.0 soft freeze. I suppose you target those changes for 6.1?

-- 
Marc-André Lureau
Re: [PATCH 00/15] virtio-gpu: split into two devices.
Posted by Gerd Hoffmann 3 years ago
On Fri, Mar 19, 2021 at 03:48:42PM +0400, Marc-André Lureau wrote:
> Hi Gerd
> 
> On Fri, Mar 19, 2021 at 3:22 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> > Currently we have one virtio-gpu device.  Problem with this approach is
> > that if you compile a full-featured qemu you'll get a virtio-gpu device
> > which depends on opengl and virgl, so these dependencies must be
> > installed and the libraries will be loaded into memory even if you don't
> > use virgl.  Also the code is cluttered with #ifdefs and a bit messy.
> >
> > This patch series splits the virtio-gpu device into two:
> >
> >  (1) virtio-gpu-device becomes the non-virgl device, same as
> >      virtio-gpu-device,virgl=off today.
> >  (2) virtio-gpu-gl-device is the new virgl device, same as
> >      virtio-gpu-device,virgl=on today.
> >
> > When compiling qemu without virglrenderer support virtio-gpu-device
> > behavior doesn't change.
> >
> 
> Nice!
> We are post 6.0 soft freeze. I suppose you target those changes for 6.1?

Yes, it's clearly 6.1 material at this point.

take care,
  Gerd


RE: [PATCH 00/15] virtio-gpu: split into two devices.
Posted by Kasireddy, Vivek 3 years ago
Hi Gerd,

> On Fri, Mar 19, 2021 at 03:48:42PM +0400, Marc-André Lureau wrote:
> > Hi Gerd
> >
> > On Fri, Mar 19, 2021 at 3:22 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > > Currently we have one virtio-gpu device.  Problem with this approach is
> > > that if you compile a full-featured qemu you'll get a virtio-gpu device
> > > which depends on opengl and virgl, so these dependencies must be
> > > installed and the libraries will be loaded into memory even if you don't
> > > use virgl.  Also the code is cluttered with #ifdefs and a bit messy.
> > >
> > > This patch series splits the virtio-gpu device into two:
> > >
> > >  (1) virtio-gpu-device becomes the non-virgl device, same as
> > >      virtio-gpu-device,virgl=off today.
> > >  (2) virtio-gpu-gl-device is the new virgl device, same as
> > >      virtio-gpu-device,virgl=on today.
> > >
> > > When compiling qemu without virglrenderer support virtio-gpu-device
> > > behavior doesn't change.
[Kasireddy, Vivek] Just a random thought: if a user enables both these devices 
either intentionally or accidentally, can they play nice with each other?
If this case is not allowed, where and how is that being enforced?

Thanks,
Vivek

> > >
> >
> > Nice!
> > We are post 6.0 soft freeze. I suppose you target those changes for 6.1?
> 
> Yes, it's clearly 6.1 material at this point.
> 
> take care,
>   Gerd


Re: [PATCH 00/15] virtio-gpu: split into two devices.
Posted by Gerd Hoffmann 3 years ago
On Mon, Mar 22, 2021 at 06:41:47PM +0000, Kasireddy, Vivek wrote:
> Hi Gerd,
> 
> > On Fri, Mar 19, 2021 at 03:48:42PM +0400, Marc-André Lureau wrote:
> > > Hi Gerd
> > >
> > > On Fri, Mar 19, 2021 at 3:22 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > > > Currently we have one virtio-gpu device.  Problem with this approach is
> > > > that if you compile a full-featured qemu you'll get a virtio-gpu device
> > > > which depends on opengl and virgl, so these dependencies must be
> > > > installed and the libraries will be loaded into memory even if you don't
> > > > use virgl.  Also the code is cluttered with #ifdefs and a bit messy.
> > > >
> > > > This patch series splits the virtio-gpu device into two:
> > > >
> > > >  (1) virtio-gpu-device becomes the non-virgl device, same as
> > > >      virtio-gpu-device,virgl=off today.
> > > >  (2) virtio-gpu-gl-device is the new virgl device, same as
> > > >      virtio-gpu-device,virgl=on today.
> > > >
> > > > When compiling qemu without virglrenderer support virtio-gpu-device
> > > > behavior doesn't change.
> [Kasireddy, Vivek] Just a random thought: if a user enables both these devices 
> either intentionally or accidentally, can they play nice with each other?

Yes, should work fine.

take care,
  Gerd


Re: [PATCH 00/15] virtio-gpu: split into two devices.
Posted by no-reply@patchew.org 3 years, 1 month ago
Patchew URL: https://patchew.org/QEMU/20210319112147.4138943-1-kraxel@redhat.com/



Hi,

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

Type: series
Message-id: 20210319112147.4138943-1-kraxel@redhat.com
Subject: [PATCH 00/15] virtio-gpu: split into two devices.

=== 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
 - [tag update]      patchew/20210311165947.27470-1-peter.maydell@linaro.org -> patchew/20210311165947.27470-1-peter.maydell@linaro.org
 * [new tag]         patchew/20210319112147.4138943-1-kraxel@redhat.com -> patchew/20210319112147.4138943-1-kraxel@redhat.com
Switched to a new branch 'test'
d701bd8 virtio-gpu: move fields to struct VirtIOGPUGL
3efa9d2 virtio-gpu: drop use_virgl_renderer
b347213 virtio-gpu: move virtio-gpu-gl-device to separate module
d09c1f0 virtio-gpu: drop VIRGL() macro
05d9255 virtio-gpu: move update_cursor_data
192d336 virtio-gpu: move virgl process_cmd
8eeb29f virtio-gpu: move virgl gl_flushed
9e76d10 virtio-gpu: move virgl handle_ctrl
d272555 virtio-gpu: use class function for ctrl queue handlers
c37479b virtio-gpu: move virgl reset
576cff3 virtio-gpu: move virgl realize + properties
04c9c17 virtio-gpu: add virtio-vga-gl
e6934ec virtio-gpu: add virtio-gpu-gl-pci
57c1b29 virtio-gpu: add virtio-gpu-gl-device
660100e virtio-gpu: rename virgl source file.

=== OUTPUT BEGIN ===
1/15 Checking commit 660100eb846e (virtio-gpu: rename virgl source file.)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#14: 
 hw/display/{virtio-gpu-3d.c => virtio-gpu-virgl.c} | 0

total: 0 errors, 1 warnings, 8 lines checked

Patch 1/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/15 Checking commit 57c1b2934802 (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?
#32: 
new file mode 100644

total: 0 errors, 1 warnings, 75 lines checked

Patch 2/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/15 Checking commit e6934ec37d3d (virtio-gpu: add virtio-gpu-gl-pci)
4/15 Checking commit 04c9c179a093 (virtio-gpu: add virtio-vga-gl)
5/15 Checking commit 576cff3a938e (virtio-gpu: move virgl realize + properties)
6/15 Checking commit c37479b06276 (virtio-gpu: move virgl reset)
7/15 Checking commit d2725554fce3 (virtio-gpu: use class function for ctrl queue handlers)
8/15 Checking commit 9e76d10e3190 (virtio-gpu: move virgl handle_ctrl)
9/15 Checking commit 8eeb29f94541 (virtio-gpu: move virgl gl_flushed)
10/15 Checking commit 192d3360b319 (virtio-gpu: move virgl process_cmd)
WARNING: line over 80 characters
#101: FILE: include/hw/virtio/virtio-gpu.h:232:
+void virtio_gpu_simple_process_cmd(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd);

total: 0 errors, 1 warnings, 70 lines checked

Patch 10/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/15 Checking commit 05d925520ac0 (virtio-gpu: move update_cursor_data)
12/15 Checking commit d09c1f0ac45a (virtio-gpu: drop VIRGL() macro)
13/15 Checking commit b347213cfde6 (virtio-gpu: move virtio-gpu-gl-device to separate module)
14/15 Checking commit 3efa9d26ea18 (virtio-gpu: drop use_virgl_renderer)
ERROR: "foo * bar" should be "foo *bar"
#96: 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 14/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

15/15 Checking commit d701bd87ca8e (virtio-gpu: move fields to struct VirtIOGPUGL)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210319112147.4138943-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