Message ID | 20190523110434.23830-1-gert.wollny@collabora.com |
---|---|
State | New |
Headers | show |
Series | virtio_gpu_3d: make it possible to configure the fence poll time | expand |
Hi On Thu, May 23, 2019 at 3:27 PM Gert Wollny <gert.wollny@collabora.com> wrote: > > The default fence poll time of 10ms (100 Hz) is sufficent for normal > work loads, but if one wants to play games within the virtual machine > this value might be too high, so make it possible to configure this > value by using the environment variable QEMU_VIRGL_POLL_FREQ where the > poll is given in Hz. To acommodate higher poll frequencies also change > the timer to use micro seconds as base instead of milliseconds. > > This is what VIRGL_RENDERER_THREAD_SYNC helps with. You don't need to do regular polling, but I think it is currently limited to Linux/eventfd only. fwiw, vhost-user-gpu uses it. > Signed-off-by: Gert Wollny <gert.wollny@collabora.com> > --- > hw/display/virtio-gpu-3d.c | 18 ++++++++++++++++-- > include/hw/virtio/virtio-gpu.h | 1 + > 2 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c > index 5ee3566ae0..120e593e76 100644 > --- a/hw/display/virtio-gpu-3d.c > +++ b/hw/display/virtio-gpu-3d.c > @@ -17,6 +17,7 @@ > #include "trace.h" > #include "hw/virtio/virtio.h" > #include "hw/virtio/virtio-gpu.h" > +#include "qemu/cutils.h" > > #ifdef CONFIG_VIRGL > > @@ -580,7 +581,8 @@ static void virtio_gpu_fence_poll(void *opaque) > virgl_renderer_poll(); > virtio_gpu_process_cmdq(g); > if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) { > - timer_mod(g->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1); > + timer_mod(g->fence_poll, qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + > + g->fence_poll_timeout); > } > } > > @@ -605,13 +607,25 @@ void virtio_gpu_virgl_reset(VirtIOGPU *g) > int virtio_gpu_virgl_init(VirtIOGPU *g) > { > int ret; > + const char *val; > > ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); > if (ret != 0) { > return ret; > } > > - g->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL, > + g->fence_poll_timeout = 10000; /* default 10 ms */ > + val = getenv("QEMU_VIRGL_POLL_FREQ"); > + if (val) { > + unsigned long long poll_freq; > + if (parse_uint_full(val, &poll_freq, 10) || poll_freq > UINT32_MAX) { > + fprintf(stderr, "VIRGL_POLL_FREQ: Invalid integer `%s'\n", val); > + exit(1); > + } > + g->fence_poll_timeout = 1000000 / (uint32_t)poll_freq; > + } > + > + g->fence_poll = timer_new_us(QEMU_CLOCK_VIRTUAL, > virtio_gpu_fence_poll, g); > > if (virtio_gpu_stats_enabled(g->conf)) { > diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h > index 60425c5d58..a9e03b25aa 100644 > --- a/include/hw/virtio/virtio-gpu.h > +++ b/include/hw/virtio/virtio-gpu.h > @@ -116,6 +116,7 @@ typedef struct VirtIOGPU { > bool renderer_reset; > QEMUTimer *fence_poll; > QEMUTimer *print_stats; > + uint32_t fence_poll_timeout; > > uint32_t inflight; > struct { > -- > 2.20.1 > >
Am Donnerstag, den 23.05.2019, 16:01 +0200 schrieb Marc-André Lureau: > Hi > > On Thu, May 23, 2019 at 3:27 PM Gert Wollny < > gert.wollny@collabora.com> wrote: > > The default fence poll time of 10ms (100 Hz) is sufficent for > > normal > > work loads, but if one wants to play games within the virtual > > machine > > this value might be too high, so make it possible to configure this > > value by using the environment variable QEMU_VIRGL_POLL_FREQ where > > the > > poll is given in Hz. To acommodate higher poll frequencies also > > change > > the timer to use micro seconds as base instead of milliseconds. > > > > > > This is what VIRGL_RENDERER_THREAD_SYNC helps with. You don't need to > do regular polling, but I think it is currently limited to > Linux/eventfd only. As far as I can see only vtest uses this, not qemu. > > fwiw, vhost-user-gpu uses it. Yeah, I tested it on Intel, but AFAICS this hasn't landed yet, no? OTOH the drm calls were only implemented for Intel, and I have no idea how to implement this for other hardware, like e.g. radeonsi, or even the Nvidia blob ... Best, Gert > > > > Signed-off-by: Gert Wollny <gert.wollny@collabora.com> > > --- > > hw/display/virtio-gpu-3d.c | 18 ++++++++++++++++-- > > include/hw/virtio/virtio-gpu.h | 1 + > > 2 files changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu- > > 3d.c > > index 5ee3566ae0..120e593e76 100644 > > --- a/hw/display/virtio-gpu-3d.c > > +++ b/hw/display/virtio-gpu-3d.c > > @@ -17,6 +17,7 @@ > > #include "trace.h" > > #include "hw/virtio/virtio.h" > > #include "hw/virtio/virtio-gpu.h" > > +#include "qemu/cutils.h" > > > > #ifdef CONFIG_VIRGL > > > > @@ -580,7 +581,8 @@ static void virtio_gpu_fence_poll(void *opaque) > > virgl_renderer_poll(); > > virtio_gpu_process_cmdq(g); > > if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) { > > - timer_mod(g->fence_poll, > > qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1); > > + timer_mod(g->fence_poll, > > qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + > > + g->fence_poll_timeout); > > } > > } > > > > @@ -605,13 +607,25 @@ void virtio_gpu_virgl_reset(VirtIOGPU *g) > > int virtio_gpu_virgl_init(VirtIOGPU *g) > > { > > int ret; > > + const char *val; > > > > ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); > > if (ret != 0) { > > return ret; > > } > > > > - g->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL, > > + g->fence_poll_timeout = 10000; /* default 10 ms */ > > + val = getenv("QEMU_VIRGL_POLL_FREQ"); > > + if (val) { > > + unsigned long long poll_freq; > > + if (parse_uint_full(val, &poll_freq, 10) || poll_freq > > > UINT32_MAX) { > > + fprintf(stderr, "VIRGL_POLL_FREQ: Invalid integer > > `%s'\n", val); > > + exit(1); > > + } > > + g->fence_poll_timeout = 1000000 / (uint32_t)poll_freq; > > + } > > + > > + g->fence_poll = timer_new_us(QEMU_CLOCK_VIRTUAL, > > virtio_gpu_fence_poll, g); > > > > if (virtio_gpu_stats_enabled(g->conf)) { > > diff --git a/include/hw/virtio/virtio-gpu.h > > b/include/hw/virtio/virtio-gpu.h > > index 60425c5d58..a9e03b25aa 100644 > > --- a/include/hw/virtio/virtio-gpu.h > > +++ b/include/hw/virtio/virtio-gpu.h > > @@ -116,6 +116,7 @@ typedef struct VirtIOGPU { > > bool renderer_reset; > > QEMUTimer *fence_poll; > > QEMUTimer *print_stats; > > + uint32_t fence_poll_timeout; > > > > uint32_t inflight; > > struct { > > -- > > 2.20.1 > > > > > >
Hi On Thu, May 23, 2019 at 4:17 PM Gert Wollny <gert.wollny@collabora.com> wrote: > > Am Donnerstag, den 23.05.2019, 16:01 +0200 schrieb Marc-André Lureau: > > Hi > > > > On Thu, May 23, 2019 at 3:27 PM Gert Wollny < > > gert.wollny@collabora.com> wrote: > > > The default fence poll time of 10ms (100 Hz) is sufficent for > > > normal > > > work loads, but if one wants to play games within the virtual > > > machine > > > this value might be too high, so make it possible to configure this > > > value by using the environment variable QEMU_VIRGL_POLL_FREQ where > > > the > > > poll is given in Hz. To acommodate higher poll frequencies also > > > change > > > the timer to use micro seconds as base instead of milliseconds. > > > > > > > > > > This is what VIRGL_RENDERER_THREAD_SYNC helps with. You don't need to > > do regular polling, but I think it is currently limited to > > Linux/eventfd only. > > As far as I can see only vtest uses this, not qemu. I don't think there is anything preventing qemu from using it, except the portability which should be taken care more carefully. > > > > > fwiw, vhost-user-gpu uses it. > > Yeah, I tested it on Intel, but AFAICS this hasn't landed yet, no? OTOH > the drm calls were only implemented for Intel, and I have no idea how > to implement this for other hardware, like e.g. radeonsi, or even the > Nvidia blob ... I just sent v8 for review today: https://patchew.org/QEMU/20190523132035.20559-1-marcandre.lureau@redhat.com/ The DRM bits were replaced by GBM usage, which will give some portability. Note that those bits are for 2d ops only, and have a simpler ram/mem fallback. > Best, > Gert > > > > > > > > > Signed-off-by: Gert Wollny <gert.wollny@collabora.com> > > > --- > > > hw/display/virtio-gpu-3d.c | 18 ++++++++++++++++-- > > > include/hw/virtio/virtio-gpu.h | 1 + > > > 2 files changed, 17 insertions(+), 2 deletions(-) > > > > > > diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu- > > > 3d.c > > > index 5ee3566ae0..120e593e76 100644 > > > --- a/hw/display/virtio-gpu-3d.c > > > +++ b/hw/display/virtio-gpu-3d.c > > > @@ -17,6 +17,7 @@ > > > #include "trace.h" > > > #include "hw/virtio/virtio.h" > > > #include "hw/virtio/virtio-gpu.h" > > > +#include "qemu/cutils.h" > > > > > > #ifdef CONFIG_VIRGL > > > > > > @@ -580,7 +581,8 @@ static void virtio_gpu_fence_poll(void *opaque) > > > virgl_renderer_poll(); > > > virtio_gpu_process_cmdq(g); > > > if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) { > > > - timer_mod(g->fence_poll, > > > qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1); > > > + timer_mod(g->fence_poll, > > > qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + > > > + g->fence_poll_timeout); > > > } > > > } > > > > > > @@ -605,13 +607,25 @@ void virtio_gpu_virgl_reset(VirtIOGPU *g) > > > int virtio_gpu_virgl_init(VirtIOGPU *g) > > > { > > > int ret; > > > + const char *val; > > > > > > ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); > > > if (ret != 0) { > > > return ret; > > > } > > > > > > - g->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL, > > > + g->fence_poll_timeout = 10000; /* default 10 ms */ > > > + val = getenv("QEMU_VIRGL_POLL_FREQ"); > > > + if (val) { > > > + unsigned long long poll_freq; > > > + if (parse_uint_full(val, &poll_freq, 10) || poll_freq > > > > UINT32_MAX) { > > > + fprintf(stderr, "VIRGL_POLL_FREQ: Invalid integer > > > `%s'\n", val); > > > + exit(1); > > > + } > > > + g->fence_poll_timeout = 1000000 / (uint32_t)poll_freq; > > > + } > > > + > > > + g->fence_poll = timer_new_us(QEMU_CLOCK_VIRTUAL, > > > virtio_gpu_fence_poll, g); > > > > > > if (virtio_gpu_stats_enabled(g->conf)) { > > > diff --git a/include/hw/virtio/virtio-gpu.h > > > b/include/hw/virtio/virtio-gpu.h > > > index 60425c5d58..a9e03b25aa 100644 > > > --- a/include/hw/virtio/virtio-gpu.h > > > +++ b/include/hw/virtio/virtio-gpu.h > > > @@ -116,6 +116,7 @@ typedef struct VirtIOGPU { > > > bool renderer_reset; > > > QEMUTimer *fence_poll; > > > QEMUTimer *print_stats; > > > + uint32_t fence_poll_timeout; > > > > > > uint32_t inflight; > > > struct { > > > -- > > > 2.20.1 > > > > > > > > > > > >
Am Donnerstag, den 23.05.2019, 16:37 +0200 schrieb Marc-André Lureau: > Hi > > On Thu, May 23, 2019 at 4:17 PM Gert Wollny < > gert.wollny@collabora.com> wrote: > > Am Donnerstag, den 23.05.2019, 16:01 +0200 schrieb Marc-André > > Lureau: > > > Hi > > > > > > On Thu, May 23, 2019 at 3:27 PM Gert Wollny < > > > gert.wollny@collabora.com> wrote: > > > > The default fence poll time of 10ms (100 Hz) is sufficent for > > > > normal > > > > work loads, but if one wants to play games within the virtual > > > > machine > > > > this value might be too high, so make it possible to configure > > > > this > > > > value by using the environment variable QEMU_VIRGL_POLL_FREQ > > > > where > > > > the > > > > poll is given in Hz. To acommodate higher poll frequencies also > > > > change > > > > the timer to use micro seconds as base instead of milliseconds. > > > > > > > > > > > > > > This is what VIRGL_RENDERER_THREAD_SYNC helps with. You don't > > > need to > > > do regular polling, but I think it is currently limited to > > > Linux/eventfd only. > > > > As far as I can see only vtest uses this, not qemu. > > I don't think there is anything preventing qemu from using it, except > the portability which should be taken care more carefully. But to actually use it would need quite some work, wouldn't it? (I'm thinking of your series: http://qemu.11.n7.nabble.com/PATCH-00-18-virgl-use-a-seperate-rendering-thread-tt429557.html ) That's why I though that being able to configure the polling frequency might be an easy way to give some performance boost. > > > > fwiw, vhost-user-gpu uses it. > > > > Yeah, I tested it on Intel, but AFAICS this hasn't landed yet, no? > > OTOH > > the drm calls were only implemented for Intel, and I have no idea > > how > > to implement this for other hardware, like e.g. radeonsi, or even > > the > > Nvidia blob ... > > I just sent v8 for review today: > https://patchew.org/QEMU/20190523132035.20559-1-marcandre.lureau@redhat.com/ > > The DRM bits were replaced by GBM usage, which will give some > portability. That's great, I'll give it a spin in the next view days. Best, Gert > > > > > > > Signed-off-by: Gert Wollny <gert.wollny@collabora.com> > > > > --- > > > > hw/display/virtio-gpu-3d.c | 18 ++++++++++++++++-- > > > > include/hw/virtio/virtio-gpu.h | 1 + > > > > 2 files changed, 17 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio- > > > > gpu- > > > > 3d.c > > > > index 5ee3566ae0..120e593e76 100644 > > > > --- a/hw/display/virtio-gpu-3d.c > > > > +++ b/hw/display/virtio-gpu-3d.c > > > > @@ -17,6 +17,7 @@ > > > > #include "trace.h" > > > > #include "hw/virtio/virtio.h" > > > > #include "hw/virtio/virtio-gpu.h" > > > > +#include "qemu/cutils.h" > > > > > > > > #ifdef CONFIG_VIRGL > > > > > > > > @@ -580,7 +581,8 @@ static void virtio_gpu_fence_poll(void > > > > *opaque) > > > > virgl_renderer_poll(); > > > > virtio_gpu_process_cmdq(g); > > > > if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) > > > > { > > > > - timer_mod(g->fence_poll, > > > > qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1); > > > > + timer_mod(g->fence_poll, > > > > qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + > > > > + g->fence_poll_timeout); > > > > } > > > > } > > > > > > > > @@ -605,13 +607,25 @@ void virtio_gpu_virgl_reset(VirtIOGPU *g) > > > > int virtio_gpu_virgl_init(VirtIOGPU *g) > > > > { > > > > int ret; > > > > + const char *val; > > > > > > > > ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); > > > > if (ret != 0) { > > > > return ret; > > > > } > > > > > > > > - g->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL, > > > > + g->fence_poll_timeout = 10000; /* default 10 ms */ > > > > + val = getenv("QEMU_VIRGL_POLL_FREQ"); > > > > + if (val) { > > > > + unsigned long long poll_freq; > > > > + if (parse_uint_full(val, &poll_freq, 10) || poll_freq > > > > > > > > > UINT32_MAX) { > > > > + fprintf(stderr, "VIRGL_POLL_FREQ: Invalid integer > > > > `%s'\n", val); > > > > + exit(1); > > > > + } > > > > + g->fence_poll_timeout = 1000000 / (uint32_t)poll_freq; > > > > + } > > > > + > > > > + g->fence_poll = timer_new_us(QEMU_CLOCK_VIRTUAL, > > > > virtio_gpu_fence_poll, g); > > > > > > > > if (virtio_gpu_stats_enabled(g->conf)) { > > > > diff --git a/include/hw/virtio/virtio-gpu.h > > > > b/include/hw/virtio/virtio-gpu.h > > > > index 60425c5d58..a9e03b25aa 100644 > > > > --- a/include/hw/virtio/virtio-gpu.h > > > > +++ b/include/hw/virtio/virtio-gpu.h > > > > @@ -116,6 +116,7 @@ typedef struct VirtIOGPU { > > > > bool renderer_reset; > > > > QEMUTimer *fence_poll; > > > > QEMUTimer *print_stats; > > > > + uint32_t fence_poll_timeout; > > > > > > > > uint32_t inflight; > > > > struct { > > > > -- > > > > 2.20.1 > > > > > > > > > >
Hi On Thu, May 23, 2019 at 4:54 PM Gert Wollny <gert.wollny@collabora.com> wrote: > > Am Donnerstag, den 23.05.2019, 16:37 +0200 schrieb Marc-André Lureau: > > Hi > > > > On Thu, May 23, 2019 at 4:17 PM Gert Wollny < > > gert.wollny@collabora.com> wrote: > > > Am Donnerstag, den 23.05.2019, 16:01 +0200 schrieb Marc-André > > > Lureau: > > > > Hi > > > > > > > > On Thu, May 23, 2019 at 3:27 PM Gert Wollny < > > > > gert.wollny@collabora.com> wrote: > > > > > The default fence poll time of 10ms (100 Hz) is sufficent for > > > > > normal > > > > > work loads, but if one wants to play games within the virtual > > > > > machine > > > > > this value might be too high, so make it possible to configure > > > > > this > > > > > value by using the environment variable QEMU_VIRGL_POLL_FREQ > > > > > where > > > > > the > > > > > poll is given in Hz. To acommodate higher poll frequencies also > > > > > change > > > > > the timer to use micro seconds as base instead of milliseconds. > > > > > > > > > > > > > > > > > > This is what VIRGL_RENDERER_THREAD_SYNC helps with. You don't > > > > need to > > > > do regular polling, but I think it is currently limited to > > > > Linux/eventfd only. > > > > > > As far as I can see only vtest uses this, not qemu. > > > > I don't think there is anything preventing qemu from using it, except > > the portability which should be taken care more carefully. > > But to actually use it would need quite some work, wouldn't it? (I'm > thinking of your series: > http://qemu.11.n7.nabble.com/PATCH-00-18-virgl-use-a-seperate-rendering-thread-tt429557.html > ) > I think you could use VIRGL_RENDERER_THREAD_SYNC, without using a seperate thread for virgl. You need to register the pollfd virgl_renderer_get_poll_fd() with the main loop, but other than that it should, in theory, work.
diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c index 5ee3566ae0..120e593e76 100644 --- a/hw/display/virtio-gpu-3d.c +++ b/hw/display/virtio-gpu-3d.c @@ -17,6 +17,7 @@ #include "trace.h" #include "hw/virtio/virtio.h" #include "hw/virtio/virtio-gpu.h" +#include "qemu/cutils.h" #ifdef CONFIG_VIRGL @@ -580,7 +581,8 @@ static void virtio_gpu_fence_poll(void *opaque) virgl_renderer_poll(); virtio_gpu_process_cmdq(g); if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) { - timer_mod(g->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1); + timer_mod(g->fence_poll, qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + + g->fence_poll_timeout); } } @@ -605,13 +607,25 @@ void virtio_gpu_virgl_reset(VirtIOGPU *g) int virtio_gpu_virgl_init(VirtIOGPU *g) { int ret; + const char *val; ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); if (ret != 0) { return ret; } - g->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL, + g->fence_poll_timeout = 10000; /* default 10 ms */ + val = getenv("QEMU_VIRGL_POLL_FREQ"); + if (val) { + unsigned long long poll_freq; + if (parse_uint_full(val, &poll_freq, 10) || poll_freq > UINT32_MAX) { + fprintf(stderr, "VIRGL_POLL_FREQ: Invalid integer `%s'\n", val); + exit(1); + } + g->fence_poll_timeout = 1000000 / (uint32_t)poll_freq; + } + + g->fence_poll = timer_new_us(QEMU_CLOCK_VIRTUAL, virtio_gpu_fence_poll, g); if (virtio_gpu_stats_enabled(g->conf)) { diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 60425c5d58..a9e03b25aa 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -116,6 +116,7 @@ typedef struct VirtIOGPU { bool renderer_reset; QEMUTimer *fence_poll; QEMUTimer *print_stats; + uint32_t fence_poll_timeout; uint32_t inflight; struct {
The default fence poll time of 10ms (100 Hz) is sufficent for normal work loads, but if one wants to play games within the virtual machine this value might be too high, so make it possible to configure this value by using the environment variable QEMU_VIRGL_POLL_FREQ where the poll is given in Hz. To acommodate higher poll frequencies also change the timer to use micro seconds as base instead of milliseconds. Signed-off-by: Gert Wollny <gert.wollny@collabora.com> --- hw/display/virtio-gpu-3d.c | 18 ++++++++++++++++-- include/hw/virtio/virtio-gpu.h | 1 + 2 files changed, 17 insertions(+), 2 deletions(-)