Re: [Qemu-devel] [PATCH] virtio_gpu_3d: make it possible to configure the fence poll time

[Gert Wollny]

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 <
> address@hidden> 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 <
> > > address@hidden> 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/address@hidden/
> 
> 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 <address@hidden>
> > > > ---
> > > >  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
> > > > 
> > > > 
> 
>