Re: [PATCH v3 14/23] drm/qxl: cover all crtcs in shadow bo.

From: Noralf TrÃnnes
Date: Fri Jan 25 2019 - 12:08:39 EST

Next message: Noralf TrÃnnes: "Re: [PATCH v3 15/23] drm/qxl: use qxl_num_crtc directly"
Previous message: Kees Cook: "Re: [PATCH] writeback: tracing: Copy only up to 31 characters in strncpy() calls"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Den 18.01.2019 13.20, skrev Gerd Hoffmann:
> The qxl device supports only a single active framebuffer ("primary
> surface" in spice terminology). In multihead configurations are handled
> by defining rectangles within the primary surface for each head/crtc.
>
> Userspace which uses the qxl ioctl interface (xorg qxl driver) is aware
> of this limitation and will setup framebuffers and crtcs accordingly.
>
> Userspace which uses dumb framebuffers (xorg modesetting driver,
> wayland) is not aware of this limitation and tries to use two
> framebuffers (one for each crtc) instead.
>
> The qxl kms driver already has the dumb bo separated from the primary
> surface, by using a (shared) shadow bo as primary surface. This is
> needed to support pageflips without having to re-create the primary
> surface. The qxl driver will blit from the dumb bo to the shadow bo
> instead.
>
> So we can extend the shadow logic: Maintain a global shadow bo (aka
> primary surface), make it big enough that dumb bo's for all crtcs fit in
> side-by-side. Adjust the pageflip blits to place the heads next to each
> other in the shadow.
>
> With this patch in place multihead qxl works with wayland.
>
> Signed-off-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> ---
> drivers/gpu/drm/qxl/qxl_drv.h | 5 +-
> drivers/gpu/drm/qxl/qxl_display.c | 119 +++++++++++++++++++++++++++++---------
> drivers/gpu/drm/qxl/qxl_draw.c | 9 ++-
> 3 files changed, 104 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> index 150b1a4f66..43c6df9cf9 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.h
> +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> @@ -230,6 +230,8 @@ struct qxl_device {
> struct qxl_ram_header *ram_header;
>
> struct qxl_bo *primary_bo;
> + struct qxl_bo *dumb_shadow_bo;
> + struct qxl_head *dumb_heads;
>
> struct qxl_memslot main_slot;
> struct qxl_memslot surfaces_slot;
> @@ -437,7 +439,8 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
> struct qxl_bo *bo,
> unsigned int flags, unsigned int color,
> struct drm_clip_rect *clips,
> - unsigned int num_clips, int inc);
> + unsigned int num_clips, int inc,
> + uint32_t dumb_shadow_offset);
>
> void qxl_draw_fill(struct qxl_draw_fill *qxl_draw_fill_rec);
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> index ff13bc6a4a..d9de43e5fd 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -323,6 +323,8 @@ static void qxl_crtc_update_monitors_config(struct drm_crtc *crtc,
> head.y = crtc->y;
> if (qdev->monitors_config->count < i + 1)
> qdev->monitors_config->count = i + 1;
> + if (qdev->primary_bo == qdev->dumb_shadow_bo)
> + head.x += qdev->dumb_heads[i].x;
> } else if (i > 0) {
> head.width = 0;
> head.height = 0;
> @@ -426,7 +428,7 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
> }
>
> qxl_draw_dirty_fb(qdev, fb, qobj, flags, color,
> - clips, num_clips, inc);
> + clips, num_clips, inc, 0);
>
> drm_modeset_unlock_all(fb->dev);
>
> @@ -535,6 +537,7 @@ static void qxl_primary_atomic_update(struct drm_plane *plane,
> .x2 = plane->state->fb->width,
> .y2 = plane->state->fb->height
> };
> + uint32_t dumb_shadow_offset = 0;
>
> if (old_state->fb) {
> bo_old = gem_to_qxl_bo(old_state->fb->obj[0]);
> @@ -551,7 +554,12 @@ static void qxl_primary_atomic_update(struct drm_plane *plane,
> qxl_primary_apply_cursor(plane);
> }
>
> - qxl_draw_dirty_fb(qdev, plane->state->fb, bo, 0, 0, &norect, 1, 1);
> + if (bo->is_dumb)
> + dumb_shadow_offset =
> + qdev->dumb_heads[plane->state->crtc->index].x;
> +
> + qxl_draw_dirty_fb(qdev, plane->state->fb, bo, 0, 0, &norect, 1, 1,
> + dumb_shadow_offset);
> }
>
> static void qxl_primary_atomic_disable(struct drm_plane *plane,
> @@ -707,12 +715,68 @@ static void qxl_cursor_atomic_disable(struct drm_plane *plane,
> qxl_release_fence_buffer_objects(release);
> }
>
> +static void qxl_update_dumb_head(struct qxl_device *qdev,
> + int index, struct qxl_bo *bo)
> +{
> + uint32_t width, height;
> +
> + if (index >= qdev->monitors_config->max_allowed)
> + return;
> +
> + if (bo && bo->is_dumb) {
> + width = bo->surf.width;
> + height = bo->surf.height;
> + } else {
> + width = 0;
> + height = 0;
> + }
> +
> + if (qdev->dumb_heads[index].width == width &&
> + qdev->dumb_heads[index].height == height)
> + return;
> +
> + DRM_DEBUG("#%d: %dx%d -> %dx%d\n", index,
> + qdev->dumb_heads[index].width,
> + qdev->dumb_heads[index].height,
> + width, height);
> + qdev->dumb_heads[index].width = width;
> + qdev->dumb_heads[index].height = height;
> +}
> +
> +static void qxl_calc_dumb_shadow(struct qxl_device *qdev,
> + struct qxl_surface *surf)
> +{
> + struct qxl_head *head;
> + int i;
> +
> + memset(surf, 0, sizeof(*surf));
> + for (i = 0; i < qdev->monitors_config->max_allowed; i++) {
> + head = qdev->dumb_heads + i;
> + head->x = surf->width;
> + surf->width += head->width;
> + if (surf->height < head->height)
> + surf->height = head->height;
> + }
> + if (surf->width < 64)
> + surf->width = 64;
> + if (surf->height < 64)
> + surf->height = 64;
> + surf->format = SPICE_SURFACE_FMT_32_xRGB;
> + surf->stride = surf->width * 4;
> +
> + if (!qdev->dumb_shadow_bo ||
> + qdev->dumb_shadow_bo->surf.width != surf->width ||
> + qdev->dumb_shadow_bo->surf.height != surf->height)
> + DRM_DEBUG("%dx%d\n", surf->width, surf->height);
> +}
> +
> static int qxl_plane_prepare_fb(struct drm_plane *plane,
> struct drm_plane_state *new_state)
> {
> struct qxl_device *qdev = plane->dev->dev_private;
> struct drm_gem_object *obj;
> - struct qxl_bo *user_bo, *old_bo = NULL;
> + struct qxl_bo *user_bo;
> + struct qxl_surface surf;
> int ret;
>
> if (!new_state->fb)
> @@ -722,29 +786,30 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane,
> user_bo = gem_to_qxl_bo(obj);
>
> if (plane->type == DRM_PLANE_TYPE_PRIMARY &&
> - user_bo->is_dumb && !user_bo->shadow) {
> - if (plane->state->fb) {
> - obj = plane->state->fb->obj[0];
> - old_bo = gem_to_qxl_bo(obj);
> + user_bo->is_dumb) {
> + qxl_update_dumb_head(qdev, new_state->crtc->index,
> + user_bo);
> + qxl_calc_dumb_shadow(qdev, &surf);
> + if (!qdev->dumb_shadow_bo ||
> + qdev->dumb_shadow_bo->surf.width != surf.width ||
> + qdev->dumb_shadow_bo->surf.height != surf.height) {
> + if (qdev->dumb_shadow_bo) {
> + drm_gem_object_put_unlocked
> + (&qdev->dumb_shadow_bo->gem_base);
> + qdev->dumb_shadow_bo = NULL;
> + }
> + qxl_bo_create(qdev, surf.height * surf.stride,
> + true, true, QXL_GEM_DOMAIN_SURFACE, &surf,
> + &qdev->dumb_shadow_bo);
> }
> - if (old_bo && old_bo->shadow &&
> - user_bo->gem_base.size == old_bo->gem_base.size &&
> - plane->state->crtc == new_state->crtc &&
> - plane->state->crtc_w == new_state->crtc_w &&
> - plane->state->crtc_h == new_state->crtc_h &&
> - plane->state->src_x == new_state->src_x &&
> - plane->state->src_y == new_state->src_y &&
> - plane->state->src_w == new_state->src_w &&
> - plane->state->src_h == new_state->src_h &&
> - plane->state->rotation == new_state->rotation &&
> - plane->state->zpos == new_state->zpos) {
> - drm_gem_object_get(&old_bo->shadow->gem_base);
> - user_bo->shadow = old_bo->shadow;
> - } else {
> - qxl_bo_create(qdev, user_bo->gem_base.size,
> - true, true, QXL_GEM_DOMAIN_SURFACE, NULL,
> - &user_bo->shadow);
> - user_bo->shadow->surf = user_bo->surf;
> + if (user_bo->shadow != qdev->dumb_shadow_bo) {
> + if (user_bo->shadow) {
> + drm_gem_object_put_unlocked
> + (&user_bo->shadow->gem_base);
> + user_bo->shadow = NULL;
> + }
> + drm_gem_object_get(&qdev->dumb_shadow_bo->gem_base);
> + user_bo->shadow = qdev->dumb_shadow_bo;
> }
> }
>
> @@ -773,7 +838,7 @@ static void qxl_plane_cleanup_fb(struct drm_plane *plane,
> user_bo = gem_to_qxl_bo(obj);
> qxl_bo_unpin(user_bo);
>
> - if (user_bo->shadow && !user_bo->shadow->is_primary) {
> + if (old_state->fb != plane->state->fb && user_bo->shadow) {
> drm_gem_object_put_unlocked(&user_bo->shadow->gem_base);
> user_bo->shadow = NULL;
> }
> @@ -1106,6 +1171,8 @@ int qxl_create_monitors_object(struct qxl_device *qdev)
>
> memset(qdev->monitors_config, 0, monitors_config_size);
> qdev->monitors_config->max_allowed = max_allowed;
> +
> + qdev->dumb_heads = kcalloc(max_allowed, sizeof(qdev->dumb_heads[0]), GFP_KERNEL);

Needs an allocation failure check. With that:

Acked-by: Noralf TrÃnnes <noralf@xxxxxxxxxxx>

> return 0;
> }
>
> diff --git a/drivers/gpu/drm/qxl/qxl_draw.c b/drivers/gpu/drm/qxl/qxl_draw.c
> index c408bb83c7..5313ad21c1 100644
> --- a/drivers/gpu/drm/qxl/qxl_draw.c
> +++ b/drivers/gpu/drm/qxl/qxl_draw.c
> @@ -267,7 +267,8 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
> struct qxl_bo *bo,
> unsigned int flags, unsigned int color,
> struct drm_clip_rect *clips,
> - unsigned int num_clips, int inc)
> + unsigned int num_clips, int inc,
> + uint32_t dumb_shadow_offset)
> {
> /*
> * TODO: if flags & DRM_MODE_FB_DIRTY_ANNOTATE_FILL then we should
> @@ -295,6 +296,9 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
> if (ret)
> return;
>
> + clips->x1 += dumb_shadow_offset;
> + clips->x2 += dumb_shadow_offset;
> +
> left = clips->x1;
> right = clips->x2;
> top = clips->y1;
> @@ -342,7 +346,8 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
> goto out_release_backoff;
>
> ret = qxl_image_init(qdev, release, dimage, surface_base,
> - left, top, width, height, depth, stride);
> + left - dumb_shadow_offset,
> + top, width, height, depth, stride);
> qxl_bo_kunmap(bo);
> if (ret)
> goto out_release_backoff;
>

Next message: Noralf TrÃnnes: "Re: [PATCH v3 15/23] drm/qxl: use qxl_num_crtc directly"
Previous message: Kees Cook: "Re: [PATCH] writeback: tracing: Copy only up to 31 characters in strncpy() calls"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]