qxl: make qxl_render_update async
RHBZ# 747011 Removes the last user of QXL_SYNC when using update drivers that use the _ASYNC io ports. The last user is qxl_render_update, it is called both by qxl_hw_update which is the vga_hw_update_ptr passed to graphic_console_init, and by qxl_hw_screen_dump. At the same time the QXLRect area being passed to the red_worker thread is passed as a copy, as part of the QXLCookie. The implementation uses interface_update_area_complete with a bh to make sure dpy_update and qxl_flip are called from the io thread, otherwise the vga->ds->surface.data can change under our feet. With this patch sdl+spice works fine. But spice by itself doesn't produce the expected screendumps unless repeated a few times, due to ppm_save being called before update_area (rendering done in spice server thread) having a chance to complete. Fixed by next patch, but see commit message for problem introduced by it. Signed-off-by: Alon Levy <alevy@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This commit is contained in:
		
							parent
							
								
									2e1a98c9c1
								
							
						
					
					
						commit
						81fb6f1504
					
				| 
						 | 
				
			
			@ -82,17 +82,25 @@ void qxl_render_resize(PCIQXLDevice *qxl)
 | 
			
		|||
    }
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
void qxl_render_update(PCIQXLDevice *qxl)
 | 
			
		||||
static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area)
 | 
			
		||||
{
 | 
			
		||||
    area->left   = 0;
 | 
			
		||||
    area->right  = qxl->guest_primary.surface.width;
 | 
			
		||||
    area->top    = 0;
 | 
			
		||||
    area->bottom = qxl->guest_primary.surface.height;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
 | 
			
		||||
{
 | 
			
		||||
    VGACommonState *vga = &qxl->vga;
 | 
			
		||||
    QXLRect dirty[32], update;
 | 
			
		||||
    int i, redraw = 0;
 | 
			
		||||
    int i;
 | 
			
		||||
    DisplaySurface *surface = vga->ds->surface;
 | 
			
		||||
 | 
			
		||||
    if (qxl->guest_primary.resized) {
 | 
			
		||||
        qxl->guest_primary.resized = 0;
 | 
			
		||||
 | 
			
		||||
        qxl->guest_primary.data = memory_region_get_ram_ptr(&qxl->vga.vram);
 | 
			
		||||
        qxl_set_rect_to_surface(qxl, &qxl->dirty[0]);
 | 
			
		||||
        qxl->num_dirty_rects = 1;
 | 
			
		||||
        dprint(qxl, 1, "%s: %dx%d, stride %d, bpp %d, depth %d\n",
 | 
			
		||||
               __FUNCTION__,
 | 
			
		||||
               qxl->guest_primary.surface.width,
 | 
			
		||||
| 
						 | 
				
			
			@ -103,9 +111,9 @@ void qxl_render_update(PCIQXLDevice *qxl)
 | 
			
		|||
    }
 | 
			
		||||
    if (surface->width != qxl->guest_primary.surface.width ||
 | 
			
		||||
        surface->height != qxl->guest_primary.surface.height) {
 | 
			
		||||
        dprint(qxl, 1, "%s: resizing displaysurface to guest_primary\n",
 | 
			
		||||
               __func__);
 | 
			
		||||
        if (qxl->guest_primary.qxl_stride > 0) {
 | 
			
		||||
            dprint(qxl, 1, "%s: using guest_primary for displaysurface\n",
 | 
			
		||||
                   __func__);
 | 
			
		||||
            qemu_free_displaysurface(vga->ds);
 | 
			
		||||
            qemu_create_displaysurface_from(qxl->guest_primary.surface.width,
 | 
			
		||||
                                            qxl->guest_primary.surface.height,
 | 
			
		||||
| 
						 | 
				
			
			@ -113,36 +121,70 @@ void qxl_render_update(PCIQXLDevice *qxl)
 | 
			
		|||
                                            qxl->guest_primary.abs_stride,
 | 
			
		||||
                                            qxl->guest_primary.data);
 | 
			
		||||
        } else {
 | 
			
		||||
            dprint(qxl, 1, "%s: resizing displaysurface to guest_primary\n",
 | 
			
		||||
                   __func__);
 | 
			
		||||
            qemu_resize_displaysurface(vga->ds,
 | 
			
		||||
                    qxl->guest_primary.surface.width,
 | 
			
		||||
                    qxl->guest_primary.surface.height);
 | 
			
		||||
        }
 | 
			
		||||
    }
 | 
			
		||||
    update.left   = 0;
 | 
			
		||||
    update.right  = qxl->guest_primary.surface.width;
 | 
			
		||||
    update.top    = 0;
 | 
			
		||||
    update.bottom = qxl->guest_primary.surface.height;
 | 
			
		||||
 | 
			
		||||
    memset(dirty, 0, sizeof(dirty));
 | 
			
		||||
    if (runstate_is_running() && qxl->guest_primary.commands) {
 | 
			
		||||
        qxl->guest_primary.commands = 0;
 | 
			
		||||
        qxl_spice_update_area(qxl, 0, &update,
 | 
			
		||||
                              dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC, NULL);
 | 
			
		||||
    }
 | 
			
		||||
    if (redraw) {
 | 
			
		||||
        memset(dirty, 0, sizeof(dirty));
 | 
			
		||||
        dirty[0] = update;
 | 
			
		||||
    }
 | 
			
		||||
    for (i = 0; i < ARRAY_SIZE(dirty); i++) {
 | 
			
		||||
        if (qemu_spice_rect_is_empty(dirty+i)) {
 | 
			
		||||
    for (i = 0; i < qxl->num_dirty_rects; i++) {
 | 
			
		||||
        if (qemu_spice_rect_is_empty(qxl->dirty+i)) {
 | 
			
		||||
            break;
 | 
			
		||||
        }
 | 
			
		||||
        qxl_flip(qxl, dirty+i);
 | 
			
		||||
        qxl_flip(qxl, qxl->dirty+i);
 | 
			
		||||
        dpy_update(vga->ds,
 | 
			
		||||
                   dirty[i].left, dirty[i].top,
 | 
			
		||||
                   dirty[i].right - dirty[i].left,
 | 
			
		||||
                   dirty[i].bottom - dirty[i].top);
 | 
			
		||||
                   qxl->dirty[i].left, qxl->dirty[i].top,
 | 
			
		||||
                   qxl->dirty[i].right - qxl->dirty[i].left,
 | 
			
		||||
                   qxl->dirty[i].bottom - qxl->dirty[i].top);
 | 
			
		||||
    }
 | 
			
		||||
    qxl->num_dirty_rects = 0;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
/*
 | 
			
		||||
 * use ssd.lock to protect render_update_cookie_num.
 | 
			
		||||
 * qxl_render_update is called by io thread or vcpu thread, and the completion
 | 
			
		||||
 * callbacks are called by spice_server thread, defering to bh called from the
 | 
			
		||||
 * io thread.
 | 
			
		||||
 */
 | 
			
		||||
void qxl_render_update(PCIQXLDevice *qxl)
 | 
			
		||||
{
 | 
			
		||||
    QXLCookie *cookie;
 | 
			
		||||
 | 
			
		||||
    qemu_mutex_lock(&qxl->ssd.lock);
 | 
			
		||||
 | 
			
		||||
    if (!runstate_is_running() || !qxl->guest_primary.commands) {
 | 
			
		||||
        qxl_render_update_area_unlocked(qxl);
 | 
			
		||||
        qemu_mutex_unlock(&qxl->ssd.lock);
 | 
			
		||||
        return;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    qxl->guest_primary.commands = 0;
 | 
			
		||||
    qxl->render_update_cookie_num++;
 | 
			
		||||
    qemu_mutex_unlock(&qxl->ssd.lock);
 | 
			
		||||
    cookie = qxl_cookie_new(QXL_COOKIE_TYPE_RENDER_UPDATE_AREA,
 | 
			
		||||
                            0);
 | 
			
		||||
    qxl_set_rect_to_surface(qxl, &cookie->u.render.area);
 | 
			
		||||
    qxl_spice_update_area(qxl, 0, &cookie->u.render.area, NULL,
 | 
			
		||||
                          0, 1 /* clear_dirty_region */, QXL_ASYNC, cookie);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
void qxl_render_update_area_bh(void *opaque)
 | 
			
		||||
{
 | 
			
		||||
    PCIQXLDevice *qxl = opaque;
 | 
			
		||||
 | 
			
		||||
    qemu_mutex_lock(&qxl->ssd.lock);
 | 
			
		||||
    qxl_render_update_area_unlocked(qxl);
 | 
			
		||||
    qemu_mutex_unlock(&qxl->ssd.lock);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie)
 | 
			
		||||
{
 | 
			
		||||
    qemu_mutex_lock(&qxl->ssd.lock);
 | 
			
		||||
    qemu_bh_schedule(qxl->update_area_bh);
 | 
			
		||||
    qxl->render_update_cookie_num--;
 | 
			
		||||
    qemu_mutex_unlock(&qxl->ssd.lock);
 | 
			
		||||
    g_free(cookie);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor)
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
							
								
								
									
										69
									
								
								hw/qxl.c
								
								
								
								
							
							
						
						
									
										69
									
								
								hw/qxl.c
								
								
								
								
							| 
						 | 
				
			
			@ -747,6 +747,11 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)
 | 
			
		|||
                __func__, current_async, cookie->io);
 | 
			
		||||
    }
 | 
			
		||||
    switch (current_async) {
 | 
			
		||||
    case QXL_IO_MEMSLOT_ADD_ASYNC:
 | 
			
		||||
    case QXL_IO_DESTROY_PRIMARY_ASYNC:
 | 
			
		||||
    case QXL_IO_UPDATE_AREA_ASYNC:
 | 
			
		||||
    case QXL_IO_FLUSH_SURFACES_ASYNC:
 | 
			
		||||
        break;
 | 
			
		||||
    case QXL_IO_CREATE_PRIMARY_ASYNC:
 | 
			
		||||
        qxl_create_guest_primary_complete(qxl);
 | 
			
		||||
        break;
 | 
			
		||||
| 
						 | 
				
			
			@ -756,10 +761,53 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)
 | 
			
		|||
    case QXL_IO_DESTROY_SURFACE_ASYNC:
 | 
			
		||||
        qxl_spice_destroy_surface_wait_complete(qxl, cookie->u.surface_id);
 | 
			
		||||
        break;
 | 
			
		||||
    default:
 | 
			
		||||
        fprintf(stderr, "qxl: %s: unexpected current_async %d\n", __func__,
 | 
			
		||||
                current_async);
 | 
			
		||||
    }
 | 
			
		||||
    qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
/* called from spice server thread context only */
 | 
			
		||||
static void interface_update_area_complete(QXLInstance *sin,
 | 
			
		||||
        uint32_t surface_id,
 | 
			
		||||
        QXLRect *dirty, uint32_t num_updated_rects)
 | 
			
		||||
{
 | 
			
		||||
    PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
 | 
			
		||||
    int i;
 | 
			
		||||
    int qxl_i;
 | 
			
		||||
 | 
			
		||||
    qemu_mutex_lock(&qxl->ssd.lock);
 | 
			
		||||
    if (surface_id != 0 || !qxl->render_update_cookie_num) {
 | 
			
		||||
        qemu_mutex_unlock(&qxl->ssd.lock);
 | 
			
		||||
        return;
 | 
			
		||||
    }
 | 
			
		||||
    if (qxl->num_dirty_rects + num_updated_rects > QXL_NUM_DIRTY_RECTS) {
 | 
			
		||||
        /*
 | 
			
		||||
         * overflow - treat this as a full update. Not expected to be common.
 | 
			
		||||
         */
 | 
			
		||||
        dprint(qxl, 1, "%s: overflow of dirty rects\n", __func__);
 | 
			
		||||
        qxl->guest_primary.resized = 1;
 | 
			
		||||
    }
 | 
			
		||||
    if (qxl->guest_primary.resized) {
 | 
			
		||||
        /*
 | 
			
		||||
         * Don't bother copying or scheduling the bh since we will flip
 | 
			
		||||
         * the whole area anyway on completion of the update_area async call
 | 
			
		||||
         */
 | 
			
		||||
        qemu_mutex_unlock(&qxl->ssd.lock);
 | 
			
		||||
        return;
 | 
			
		||||
    }
 | 
			
		||||
    qxl_i = qxl->num_dirty_rects;
 | 
			
		||||
    for (i = 0; i < num_updated_rects; i++) {
 | 
			
		||||
        qxl->dirty[qxl_i++] = dirty[i];
 | 
			
		||||
    }
 | 
			
		||||
    qxl->num_dirty_rects += num_updated_rects;
 | 
			
		||||
    dprint(qxl, 1, "%s: scheduling update_area_bh, #dirty %d\n",
 | 
			
		||||
           __func__, qxl->num_dirty_rects);
 | 
			
		||||
    qemu_bh_schedule(qxl->update_area_bh);
 | 
			
		||||
    qemu_mutex_unlock(&qxl->ssd.lock);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
/* called from spice server thread context only */
 | 
			
		||||
static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token)
 | 
			
		||||
{
 | 
			
		||||
| 
						 | 
				
			
			@ -769,13 +817,17 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token)
 | 
			
		|||
    switch (cookie->type) {
 | 
			
		||||
    case QXL_COOKIE_TYPE_IO:
 | 
			
		||||
        interface_async_complete_io(qxl, cookie);
 | 
			
		||||
        g_free(cookie);
 | 
			
		||||
        break;
 | 
			
		||||
    case QXL_COOKIE_TYPE_RENDER_UPDATE_AREA:
 | 
			
		||||
        qxl_render_update_area_done(qxl, cookie);
 | 
			
		||||
        break;
 | 
			
		||||
    default:
 | 
			
		||||
        fprintf(stderr, "qxl: %s: unexpected cookie type %d\n",
 | 
			
		||||
                __func__, cookie->type);
 | 
			
		||||
    }
 | 
			
		||||
        g_free(cookie);
 | 
			
		||||
    }
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static const QXLInterface qxl_interface = {
 | 
			
		||||
    .base.type               = SPICE_INTERFACE_QXL,
 | 
			
		||||
| 
						 | 
				
			
			@ -797,6 +849,7 @@ static const QXLInterface qxl_interface = {
 | 
			
		|||
    .notify_update           = interface_notify_update,
 | 
			
		||||
    .flush_resources         = interface_flush_resources,
 | 
			
		||||
    .async_complete          = interface_async_complete,
 | 
			
		||||
    .update_area_complete    = interface_update_area_complete,
 | 
			
		||||
};
 | 
			
		||||
 | 
			
		||||
static void qxl_enter_vga_mode(PCIQXLDevice *d)
 | 
			
		||||
| 
						 | 
				
			
			@ -1213,11 +1266,17 @@ async_common:
 | 
			
		|||
    switch (io_port) {
 | 
			
		||||
    case QXL_IO_UPDATE_AREA:
 | 
			
		||||
    {
 | 
			
		||||
        QXLCookie *cookie = NULL;
 | 
			
		||||
        QXLRect update = d->ram->update_area;
 | 
			
		||||
 | 
			
		||||
        if (async == QXL_ASYNC) {
 | 
			
		||||
            cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
 | 
			
		||||
                                    QXL_IO_UPDATE_AREA_ASYNC);
 | 
			
		||||
            cookie->u.area = update;
 | 
			
		||||
        }
 | 
			
		||||
        qxl_spice_update_area(d, d->ram->update_surface,
 | 
			
		||||
                              &update, NULL, 0, 0, async,
 | 
			
		||||
                              qxl_cookie_new(QXL_COOKIE_TYPE_IO,
 | 
			
		||||
                                             QXL_IO_UPDATE_AREA_ASYNC));
 | 
			
		||||
                              cookie ? &cookie->u.area : &update,
 | 
			
		||||
                              NULL, 0, 0, async, cookie);
 | 
			
		||||
        break;
 | 
			
		||||
    }
 | 
			
		||||
    case QXL_IO_NOTIFY_CMD:
 | 
			
		||||
| 
						 | 
				
			
			@ -1649,6 +1708,8 @@ static int qxl_init_common(PCIQXLDevice *qxl)
 | 
			
		|||
    init_pipe_signaling(qxl);
 | 
			
		||||
    qxl_reset_state(qxl);
 | 
			
		||||
 | 
			
		||||
    qxl->update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl);
 | 
			
		||||
 | 
			
		||||
    return 0;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
							
								
								
									
										10
									
								
								hw/qxl.h
								
								
								
								
							
							
						
						
									
										10
									
								
								hw/qxl.h
								
								
								
								
							| 
						 | 
				
			
			@ -18,6 +18,8 @@ enum qxl_mode {
 | 
			
		|||
 | 
			
		||||
#define QXL_UNDEFINED_IO UINT32_MAX
 | 
			
		||||
 | 
			
		||||
#define QXL_NUM_DIRTY_RECTS 64
 | 
			
		||||
 | 
			
		||||
typedef struct PCIQXLDevice {
 | 
			
		||||
    PCIDevice          pci;
 | 
			
		||||
    SimpleSpiceDisplay ssd;
 | 
			
		||||
| 
						 | 
				
			
			@ -93,6 +95,12 @@ typedef struct PCIQXLDevice {
 | 
			
		|||
    /* user-friendly properties (in megabytes) */
 | 
			
		||||
    uint32_t          ram_size_mb;
 | 
			
		||||
    uint32_t          vram_size_mb;
 | 
			
		||||
 | 
			
		||||
    /* qxl_render_update state */
 | 
			
		||||
    int                render_update_cookie_num;
 | 
			
		||||
    int                num_dirty_rects;
 | 
			
		||||
    QXLRect            dirty[QXL_NUM_DIRTY_RECTS];
 | 
			
		||||
    QEMUBH            *update_area_bh;
 | 
			
		||||
} PCIQXLDevice;
 | 
			
		||||
 | 
			
		||||
#define PANIC_ON(x) if ((x)) {                         \
 | 
			
		||||
| 
						 | 
				
			
			@ -134,3 +142,5 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext);
 | 
			
		|||
void qxl_render_resize(PCIQXLDevice *qxl);
 | 
			
		||||
void qxl_render_update(PCIQXLDevice *qxl);
 | 
			
		||||
void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext);
 | 
			
		||||
void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie);
 | 
			
		||||
void qxl_render_update_area_bh(void *opaque);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -50,6 +50,7 @@ typedef enum qxl_async_io {
 | 
			
		|||
 | 
			
		||||
enum {
 | 
			
		||||
    QXL_COOKIE_TYPE_IO,
 | 
			
		||||
    QXL_COOKIE_TYPE_RENDER_UPDATE_AREA,
 | 
			
		||||
};
 | 
			
		||||
 | 
			
		||||
typedef struct QXLCookie {
 | 
			
		||||
| 
						 | 
				
			
			@ -57,6 +58,11 @@ typedef struct QXLCookie {
 | 
			
		|||
    uint64_t io;
 | 
			
		||||
    union {
 | 
			
		||||
        uint32_t surface_id;
 | 
			
		||||
        QXLRect area;
 | 
			
		||||
        struct {
 | 
			
		||||
            QXLRect area;
 | 
			
		||||
            int redraw;
 | 
			
		||||
        } render;
 | 
			
		||||
    } u;
 | 
			
		||||
} QXLCookie;
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in New Issue