>
> From: Dave Airlie <airlied@xxxxxxxxxx>
>
> QXL hw can't change primary surfaces easily, the server sends a msg
> and the client flickers a lo when it does. The old pre-atomic
typo: lo -> lot
> page flip code endeavoured to workaround this with a copy operation.
>
not clear. Do you mean a memcpy like operation or a DRAW_COPY ?
> This worked by another accident of how the qxl virtual gpu is designed,
> it does lazy operation evaluation on the host, so this operation
> wouldn't generally trash the memory, and result in correct display.
>
> This tries to work out when the commit is likely just a pageflip
> and avoid touching the primary surface, this might go wrong at
> some point but I believe it's the same level as wrong as the old code
> base.
>
This sentence contains lot of hopes :-)
> Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>
> ---
> drivers/gpu/drm/qxl/qxl_display.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> b/drivers/gpu/drm/qxl/qxl_display.c
> index afbf50d..3b702d1 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -502,6 +502,7 @@ static void qxl_primary_atomic_update(struct drm_plane
> *plane,
> struct qxl_framebuffer *qfb_old;
> struct qxl_bo *bo = gem_to_qxl_bo(qfb->obj);
> struct qxl_bo *bo_old;
> + bool update_primary = true;
> struct drm_clip_rect norect = {
> .x1 = 0,
> .y1 = 0,
> @@ -519,15 +520,31 @@ static void qxl_primary_atomic_update(struct drm_plane
> *plane,
> if (bo == bo_old)
> return;
>
> + if (bo && bo_old &&
> + plane->state->crtc == old_state->crtc &&
> + plane->state->crtc_w == old_state->crtc_w &&
> + plane->state->crtc_h == old_state->crtc_h &&
> + plane->state->src_x == old_state->src_x &&
> + plane->state->src_y == old_state->src_y &&
> + plane->state->src_w == old_state->src_w &&
> + plane->state->src_h == old_state->src_h &&
> + plane->state->rotation == old_state->rotation &&
> + plane->state->zpos == old_state->zpos)
> + /* this is likely a pageflip */
> + update_primary = false;
> +
> if (bo_old && bo_old->is_primary) {
> - qxl_io_destroy_primary(qdev);
> + if (update_primary)
> + qxl_io_destroy_primary(qdev);
> bo_old->is_primary = false;
> }
>
> if (!bo->is_primary) {
> - qxl_io_create_primary(qdev, 0, bo);
> + if (update_primary)
> + qxl_io_create_primary(qdev, 0, bo);
> bo->is_primary = true;
> }
> +
> qxl_draw_dirty_fb(qdev, qfb, bo, 0, 0, &norect, 1, 1);
> }
>
What is actually supposed to do this function?
It looks weird. Looks like it's mixing drawing and surfaces together.
I don't see the reason to issue a DRAW_COPY (qxl_draw_dirty_fb) if
you just created the primary surface with data.
Also there's a big difference with and without this patch.
You are not changing the primary surface so the frame buffer (exactly
where spice-server draws the commands in QXL memory and where a screen
capture is supposed to find updated pixel after an update command)
still refers to bo_old object. However I don't see a related change in
the life management of bo_old. So or is going to be broken or was broken
before.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel