* [PATCH v3 1/5] drm/qxl: unpin the framebuffer when plane preparation fails
2026-09-26 16:31 [PATCH v3 0/5] drm/qxl: fix shared-primary lifetime and source packing Dillon Amburgey
@ 2026-09-26 16:31 ` Dillon Amburgey
2026-09-26 16:31 ` [PATCH v3 2/5] drm/qxl: wait for pending commits before applying DirtyFB Dillon Amburgey
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Dillon Amburgey @ 2026-09-26 16:31 UTC (permalink / raw)
To: Dave Airlie, Gerd Hoffmann
Cc: Dillon Amburgey, dri-devel, virtualization, spice-devel,
linux-kernel, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, christian.koenig, krisman, noralf
qxl_plane_prepare_fb() pins the framebuffer before preparing its GEM
fences. If GEM preparation fails, the atomic helper cleans up only the
previously prepared planes, leaving this framebuffer pinned.
Unpin this plane's framebuffer before returning the preparation error.
Fixes: 6071c4c2a319 ("drm/qxl: add drm_gem_plane_helper_prepare_fb")
Assisted-by: LLM sparse
Signed-off-by: Dillon Amburgey <dillona@gmail.com>
---
drivers/gpu/drm/qxl/qxl_display.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 0719fc6a52d5..1f869734e14d 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -887,7 +887,10 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane,
if (ret)
return ret;
- return drm_gem_plane_helper_prepare_fb(plane, new_state);
+ ret = drm_gem_plane_helper_prepare_fb(plane, new_state);
+ if (ret)
+ qxl_bo_unpin(user_bo);
+ return ret;
}
static void qxl_plane_cleanup_fb(struct drm_plane *plane,
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 2/5] drm/qxl: wait for pending commits before applying DirtyFB
2026-09-26 16:31 [PATCH v3 0/5] drm/qxl: fix shared-primary lifetime and source packing Dillon Amburgey
2026-09-26 16:31 ` [PATCH v3 1/5] drm/qxl: unpin the framebuffer when plane preparation fails Dillon Amburgey
@ 2026-09-26 16:31 ` Dillon Amburgey
2026-09-26 16:31 ` [PATCH v3 3/5] drm/qxl: clear the monitor configuration for disabled head zero Dillon Amburgey
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Dillon Amburgey @ 2026-09-26 16:31 UTC (permalink / raw)
To: Dave Airlie, Gerd Hoffmann
Cc: Dillon Amburgey, dri-devel, virtualization, spice-devel,
linux-kernel, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, christian.koenig, krisman, noralf
The modeset locks protect software state, but a nonblocking commit can
swap that state before its hardware updates finish. DirtyFB can then
race primary-surface replacement and draw into the preceding surface.
Wait for the current CRTC commits while holding the modeset locks before
checking the primary and issuing dirty updates. Return wait and lock
errors to the caller instead of reporting success.
Fixes: 9973c879cff7 ("drm: qxl: Atomic phase 3: Wire up atomic page_flip helper")
Assisted-by: LLM sparse
Signed-off-by: Dillon Amburgey <dillona@gmail.com>
---
drivers/gpu/drm/qxl/qxl_display.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 1f869734e14d..51087cacff74 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -437,10 +437,17 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
struct qxl_bo *qobj;
struct drm_modeset_acquire_ctx ctx;
bool is_primary;
+ struct drm_crtc *crtc;
int inc = 1, ret;
DRM_MODESET_LOCK_ALL_BEGIN(fb->dev, ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret);
+ drm_for_each_crtc(crtc, &qdev->ddev) {
+ ret = drm_crtc_commit_wait(crtc->state->commit);
+ if (ret)
+ goto out_lock_end;
+ }
+
qobj = gem_to_qxl_bo(fb->obj[0]);
/* if we aren't primary surface ignore this */
is_primary = qobj->shadow ? qobj->shadow->is_primary : qobj->is_primary;
@@ -464,7 +471,7 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
out_lock_end:
DRM_MODESET_LOCK_ALL_END(fb->dev, ctx, ret);
- return 0;
+ return ret;
}
static const struct drm_framebuffer_funcs qxl_fb_funcs = {
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 3/5] drm/qxl: clear the monitor configuration for disabled head zero
2026-09-26 16:31 [PATCH v3 0/5] drm/qxl: fix shared-primary lifetime and source packing Dillon Amburgey
2026-09-26 16:31 ` [PATCH v3 1/5] drm/qxl: unpin the framebuffer when plane preparation fails Dillon Amburgey
2026-09-26 16:31 ` [PATCH v3 2/5] drm/qxl: wait for pending commits before applying DirtyFB Dillon Amburgey
@ 2026-09-26 16:31 ` Dillon Amburgey
2026-09-26 16:31 ` [PATCH v3 4/5] drm/qxl: own packed primary shadows in atomic plane state Dillon Amburgey
2026-09-26 16:31 ` [PATCH v3 5/5] drm/qxl: pack dumb heads from their plane source rectangles Dillon Amburgey
4 siblings, 0 replies; 6+ messages in thread
From: Dillon Amburgey @ 2026-09-26 16:31 UTC (permalink / raw)
To: Dave Airlie, Gerd Hoffmann
Cc: Dillon Amburgey, dri-devel, virtualization, spice-devel,
linux-kernel, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, christian.koenig, krisman, noralf
Disabling head zero leaves its previous monitor entry intact because
the inactive-head path only clears heads with a nonzero index. With
another head still active, QXL continues publishing the disabled head
as a visible monitor.
Clear head zero using the same path as the other disabled heads.
Fixes: a6d3c4d79822 ("qxl: hook monitors_config updates into crtc, not encoder.")
Assisted-by: LLM sparse
Signed-off-by: Dillon Amburgey <dillona@gmail.com>
---
drivers/gpu/drm/qxl/qxl_display.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 51087cacff74..05de8dd3144d 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -350,16 +350,13 @@ static void qxl_crtc_update_monitors_config(struct drm_crtc *crtc,
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) {
+ } else {
head.width = 0;
head.height = 0;
head.x = 0;
head.y = 0;
if (qdev->monitors_config->count == i + 1)
qdev->monitors_config->count = i;
- } else {
- DRM_DEBUG_KMS("inactive head 0, skip (%s)\n", reason);
- return;
}
if (head.width == qdev->monitors_config->heads[i].width &&
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 4/5] drm/qxl: own packed primary shadows in atomic plane state
2026-09-26 16:31 [PATCH v3 0/5] drm/qxl: fix shared-primary lifetime and source packing Dillon Amburgey
` (2 preceding siblings ...)
2026-09-26 16:31 ` [PATCH v3 3/5] drm/qxl: clear the monitor configuration for disabled head zero Dillon Amburgey
@ 2026-09-26 16:31 ` Dillon Amburgey
2026-09-26 16:31 ` [PATCH v3 5/5] drm/qxl: pack dumb heads from their plane source rectangles Dillon Amburgey
4 siblings, 0 replies; 6+ messages in thread
From: Dillon Amburgey @ 2026-09-26 16:31 UTC (permalink / raw)
To: Dave Airlie, Gerd Hoffmann
Cc: Dillon Amburgey, dri-devel, virtualization, spice-devel,
linux-kernel, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, christian.koenig, krisman, noralf
Preparing a dumb framebuffer changes device-wide layout and BO shadow
bindings before the atomic commit succeeds. An aborted preparation can
redirect live framebuffers to an uncommitted shadow. Per-plane cleanup
can also remove a shadow binding still needed by another head using the
same framebuffer.
Give each prepared primary plane state a reference and pin to a shared
shadow. Compute the complete new layout and prepare one matching shadow
without changing the displayed states. Include all primary planes and
CRTCs so replacement and repacking redraw every head under the same
commit dependencies. Release each state's resources independently.
Keep the primary while any new primary plane remains active, and destroy
it when the last head turns off, including DPMS with retained framebuffer
bindings. Route DirtyFB clips through each matching active plane state
to its packed slot; copy clips before the draw helper translates them.
Retain allocation-based packing and full-framebuffer drawing here; plane
source geometry is handled separately.
Fixes: 90adda2ce898 ("drm/qxl: cover all crtcs in shadow bo.")
Assisted-by: LLM sparse
Signed-off-by: Dillon Amburgey <dillona@gmail.com>
---
drivers/gpu/drm/qxl/qxl_display.c | 310 +++++++++++++++++-------------
drivers/gpu/drm/qxl/qxl_drv.h | 3 -
2 files changed, 176 insertions(+), 137 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 05de8dd3144d..af36d4d1ea57 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -45,6 +45,27 @@
#include "qxl_drv.h"
#include "qxl_object.h"
+/* Each prepared primary state owns a pin and reference to its shadow. */
+struct qxl_plane_state {
+ struct drm_plane_state base;
+ struct qxl_bo *shadow;
+ u32 x;
+};
+
+#define to_qxl_plane_state(state) \
+ container_of(state, struct qxl_plane_state, base)
+
+static bool qxl_primary_active(struct drm_atomic_commit *state,
+ struct drm_plane_state *ps)
+{
+ struct drm_crtc_state *cs;
+
+ if (!ps || !ps->fb || !ps->crtc)
+ return false;
+ cs = drm_atomic_get_new_crtc_state(state, ps->crtc);
+ return cs && cs->active;
+}
+
static bool qxl_head_enabled(struct qxl_head *head)
{
return head->width && head->height;
@@ -348,8 +369,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;
+ if (to_qxl_plane_state(crtc->primary->state)->shadow)
+ head.x += to_qxl_plane_state(crtc->primary->state)->x;
} else {
head.width = 0;
head.height = 0;
@@ -428,12 +449,10 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
struct drm_clip_rect *clips,
unsigned int num_clips)
{
- /* TODO: vmwgfx where this was cribbed from had locking. Why? */
struct qxl_device *qdev = to_qxl(fb->dev);
struct drm_clip_rect norect;
struct qxl_bo *qobj;
struct drm_modeset_acquire_ctx ctx;
- bool is_primary;
struct drm_crtc *crtc;
int inc = 1, ret;
@@ -446,9 +465,7 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
}
qobj = gem_to_qxl_bo(fb->obj[0]);
- /* if we aren't primary surface ignore this */
- is_primary = qobj->shadow ? qobj->shadow->is_primary : qobj->is_primary;
- if (!is_primary)
+ if (!qobj->is_dumb && !qobj->is_primary)
goto out_lock_end;
if (!num_clips) {
@@ -462,8 +479,29 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
inc = 2; /* skip source rects */
}
- qxl_draw_dirty_fb(qdev, fb, qobj, flags, color,
- clips, num_clips, inc, 0);
+ if (qobj->is_dumb) {
+ unsigned int n;
+
+ drm_for_each_crtc(crtc, &qdev->ddev) {
+ struct drm_plane_state *st;
+
+ st = crtc->primary->state;
+ if (st->fb != fb || !crtc->state->active ||
+ !to_qxl_plane_state(st)->shadow ||
+ !to_qxl_plane_state(st)->shadow->is_primary)
+ continue;
+ for (n = 0; n < num_clips; n++) {
+ struct drm_clip_rect c = clips[n * inc];
+
+ qxl_draw_dirty_fb(qdev, fb, qobj, flags, color,
+ &c, 1, 1,
+ to_qxl_plane_state(st)->x);
+ }
+ }
+ } else {
+ qxl_draw_dirty_fb(qdev, fb, qobj, flags, color,
+ clips, num_clips, inc, 0);
+ }
out_lock_end:
DRM_MODESET_LOCK_ALL_END(fb->dev, ctx, ret);
@@ -499,20 +537,31 @@ static const struct drm_crtc_helper_funcs qxl_crtc_helper_funcs = {
.atomic_disable = qxl_crtc_atomic_disable,
};
+/* The primary surface is shared, so every primary update must serialize
+ * with, and redraw, the other heads, including heads whose packed x moves.
+ */
static int qxl_primary_atomic_check(struct drm_plane *plane,
struct drm_atomic_commit *state)
{
- struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state,
- plane);
- struct qxl_device *qdev = to_qxl(plane->dev);
- struct qxl_bo *bo;
+ struct drm_plane_state *ps = drm_atomic_get_new_plane_state(state, plane);
+ struct drm_crtc *crtc;
- if (!new_plane_state->crtc || !new_plane_state->fb)
- return 0;
+ drm_for_each_crtc(crtc, plane->dev) {
+ struct drm_crtc_state *cs;
+ struct drm_plane_state *other;
- bo = gem_to_qxl_bo(new_plane_state->fb->obj[0]);
+ cs = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(cs))
+ return PTR_ERR(cs);
+ other = drm_atomic_get_plane_state(state, crtc->primary);
+ if (IS_ERR(other))
+ return PTR_ERR(other);
+ }
- return qxl_check_framebuffer(qdev, bo);
+ if (!ps->fb || !ps->crtc)
+ return 0;
+ return qxl_check_framebuffer(to_qxl(plane->dev),
+ gem_to_qxl_bo(ps->fb->obj[0]));
}
static int qxl_primary_apply_cursor(struct qxl_device *qdev,
@@ -666,6 +715,23 @@ static void qxl_free_cursor(struct qxl_bo *cursor_bo)
qxl_bo_unref(&cursor_bo);
}
+static void qxl_primary_atomic_disable(struct drm_plane *plane,
+ struct drm_atomic_commit *state)
+{
+ struct qxl_device *qdev = to_qxl(plane->dev);
+ struct drm_plane *other;
+ struct drm_plane_state *ps;
+ int i;
+
+ for_each_new_plane_in_state(state, other, ps, i) {
+ if (other->type == DRM_PLANE_TYPE_PRIMARY &&
+ qxl_primary_active(state, ps))
+ return;
+ }
+ if (qdev->primary_bo)
+ qxl_io_destroy_primary(qdev);
+}
+
static void qxl_primary_atomic_update(struct drm_plane *plane,
struct drm_atomic_commit *state)
{
@@ -682,7 +748,12 @@ static void qxl_primary_atomic_update(struct drm_plane *plane,
};
uint32_t dumb_shadow_offset = 0;
- primary = bo->shadow ? bo->shadow : bo;
+ if (!qxl_primary_active(state, new_state)) {
+ qxl_primary_atomic_disable(plane, state);
+ return;
+ }
+
+ primary = bo->is_dumb ? to_qxl_plane_state(new_state)->shadow : bo;
if (!primary->is_primary) {
if (qdev->primary_bo)
@@ -692,30 +763,12 @@ static void qxl_primary_atomic_update(struct drm_plane *plane,
}
if (bo->is_dumb)
- dumb_shadow_offset =
- qdev->dumb_heads[new_state->crtc->index].x;
+ dumb_shadow_offset = to_qxl_plane_state(new_state)->x;
qxl_draw_dirty_fb(qdev, new_state->fb, bo, 0, 0, &norect, 1, 1,
dumb_shadow_offset);
}
-static void qxl_primary_atomic_disable(struct drm_plane *plane,
- struct drm_atomic_commit *state)
-{
- struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state,
- plane);
- struct qxl_device *qdev = to_qxl(plane->dev);
-
- if (old_state->fb) {
- struct qxl_bo *bo = gem_to_qxl_bo(old_state->fb->obj[0]);
-
- if (bo->shadow)
- bo = bo->shadow;
- if (bo->is_primary)
- qxl_io_destroy_primary(qdev);
- }
-}
-
static void qxl_cursor_atomic_update(struct drm_plane *plane,
struct drm_atomic_commit *state)
{
@@ -768,93 +821,98 @@ static void qxl_cursor_atomic_disable(struct drm_plane *plane,
qcrtc->cursor_bo = NULL;
}
-static void qxl_update_dumb_head(struct qxl_device *qdev,
- int index, struct qxl_bo *bo)
+static int qxl_prepare_shadow(struct drm_plane *plane,
+ struct drm_plane_state *new_state)
{
- uint32_t width, height;
+ struct qxl_plane_state *qps = to_qxl_plane_state(new_state);
+ struct drm_atomic_commit *state = new_state->state;
+ struct qxl_device *qdev = to_qxl(plane->dev);
+ struct qxl_surface surf = { 0 };
+ struct qxl_bo *shadow = NULL;
+ struct drm_crtc *crtc;
+ int ret;
- if (index >= qdev->monitors_config->max_allowed)
- return;
+ if (!qxl_primary_active(state, new_state))
+ return 0;
- if (bo && bo->is_dumb) {
- width = bo->surf.width;
- height = bo->surf.height;
- } else {
- width = 0;
- height = 0;
+ drm_for_each_crtc(crtc, plane->dev) {
+ struct drm_plane_state *ps;
+ struct qxl_plane_state *other;
+ struct qxl_bo *bo;
+
+ ps = drm_atomic_get_new_plane_state(state, crtc->primary);
+ if (!qxl_primary_active(state, ps) ||
+ !gem_to_qxl_bo(ps->fb->obj[0])->is_dumb)
+ continue;
+ other = to_qxl_plane_state(ps);
+ if (ps == new_state)
+ qps->x = surf.width;
+ bo = gem_to_qxl_bo(ps->fb->obj[0]);
+ surf.width += bo->surf.width;
+ surf.height = max_t(u32, surf.height, bo->surf.height);
+ if (other->shadow)
+ shadow = other->shadow;
}
+ surf.width = max_t(u32, surf.width, 64);
+ surf.height = max_t(u32, surf.height, 64);
+ surf.format = SPICE_SURFACE_FMT_32_xRGB;
+ surf.stride = surf.width * 4;
+
+ if (!shadow) {
+ shadow = to_qxl_plane_state(plane->state)->shadow;
+ if (shadow && (shadow->surf.width != surf.width ||
+ shadow->surf.height != surf.height))
+ shadow = NULL;
+ }
+ if (!shadow)
+ return qxl_bo_create(qdev, surf.height * surf.stride,
+ true, true, QXL_GEM_DOMAIN_SURFACE, 0,
+ &surf, &qps->shadow);
- 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;
+ ret = qxl_bo_pin(shadow);
+ if (ret)
+ return ret;
+ drm_gem_object_get(&shadow->tbo.base);
+ qps->shadow = shadow;
+ return 0;
}
-static void qxl_calc_dumb_shadow(struct qxl_device *qdev,
- struct qxl_surface *surf)
+static void qxl_primary_destroy_state(struct drm_plane *plane,
+ struct drm_plane_state *state)
{
- struct qxl_head *head;
- int i;
+ struct qxl_plane_state *qps = to_qxl_plane_state(state);
- 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 (qps->shadow) {
+ ttm_bo_reserve(&qps->shadow->tbo, false, false, NULL);
+ qxl_bo_unpin_locked(qps->shadow);
+ qxl_bo_unreserve(qps->shadow);
+ drm_gem_object_put(&qps->shadow->tbo.base);
}
- 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);
+ __drm_atomic_helper_plane_destroy_state(state);
+ kfree(qps);
}
-static void qxl_prepare_shadow(struct qxl_device *qdev, struct qxl_bo *user_bo,
- int crtc_index)
+static void qxl_primary_reset(struct drm_plane *plane)
{
- struct qxl_surface surf;
-
- qxl_update_dumb_head(qdev, 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) {
- qxl_bo_unpin(qdev->dumb_shadow_bo);
- drm_gem_object_put
- (&qdev->dumb_shadow_bo->tbo.base);
- qdev->dumb_shadow_bo = NULL;
- }
- qxl_bo_create(qdev, surf.height * surf.stride,
- true, true, QXL_GEM_DOMAIN_SURFACE, 0,
- &surf, &qdev->dumb_shadow_bo);
- }
- if (user_bo->shadow != qdev->dumb_shadow_bo) {
- if (user_bo->shadow) {
- qxl_bo_unpin(user_bo->shadow);
- drm_gem_object_put
- (&user_bo->shadow->tbo.base);
- user_bo->shadow = NULL;
- }
- drm_gem_object_get(&qdev->dumb_shadow_bo->tbo.base);
- user_bo->shadow = qdev->dumb_shadow_bo;
- qxl_bo_pin(user_bo->shadow);
- }
+ struct qxl_plane_state *qps;
+
+ if (plane->state)
+ qxl_primary_destroy_state(plane, plane->state);
+ qps = kzalloc_obj(*qps);
+ __drm_atomic_helper_plane_reset(plane, qps ? &qps->base : NULL);
+}
+
+static struct drm_plane_state *qxl_primary_duplicate_state(struct drm_plane *plane)
+{
+ struct qxl_plane_state *qps;
+
+ if (WARN_ON(!plane->state))
+ return NULL;
+ qps = kzalloc_obj(*qps);
+ if (!qps)
+ return NULL;
+ __drm_atomic_helper_plane_duplicate_state(plane, &qps->base);
+ return &qps->base;
}
static int qxl_plane_prepare_fb(struct drm_plane *plane,
@@ -873,7 +931,9 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane,
if (plane->type == DRM_PLANE_TYPE_PRIMARY &&
user_bo->is_dumb) {
- qxl_prepare_shadow(qdev, user_bo, new_state->crtc->index);
+ ret = qxl_prepare_shadow(plane, new_state);
+ if (ret)
+ return ret;
}
if (plane->type == DRM_PLANE_TYPE_CURSOR &&
@@ -914,12 +974,6 @@ static void qxl_plane_cleanup_fb(struct drm_plane *plane,
obj = old_state->fb->obj[0];
user_bo = gem_to_qxl_bo(obj);
qxl_bo_unpin(user_bo);
-
- if (old_state->fb != plane->state->fb && user_bo->shadow) {
- qxl_bo_unpin(user_bo->shadow);
- drm_gem_object_put(&user_bo->shadow->tbo.base);
- user_bo->shadow = NULL;
- }
}
static const uint32_t qxl_cursor_plane_formats[] = {
@@ -959,9 +1013,9 @@ static const struct drm_plane_funcs qxl_primary_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = drm_plane_helper_destroy,
- .reset = drm_atomic_helper_plane_reset,
- .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+ .reset = qxl_primary_reset,
+ .atomic_duplicate_state = qxl_primary_duplicate_state,
+ .atomic_destroy_state = qxl_primary_destroy_state,
};
static struct drm_plane *qxl_create_plane(struct qxl_device *qdev,
@@ -1255,11 +1309,7 @@ int qxl_create_monitors_object(struct qxl_device *qdev)
qxl_bo_physical_address(qdev, qdev->monitors_config_bo, 0);
memset(qdev->monitors_config, 0, monitors_config_size);
- qdev->dumb_heads = kzalloc_objs(qdev->dumb_heads[0], qxl_num_crtc);
- if (!qdev->dumb_heads) {
- qxl_destroy_monitors_object(qdev);
- return -ENOMEM;
- }
+
return 0;
}
@@ -1270,9 +1320,6 @@ int qxl_destroy_monitors_object(struct qxl_device *qdev)
if (!qdev->monitors_config_bo)
return 0;
- kfree(qdev->dumb_heads);
- qdev->dumb_heads = NULL;
-
qdev->monitors_config = NULL;
qdev->ram_header->monitors_config = 0;
@@ -1325,10 +1372,5 @@ int qxl_modeset_init(struct qxl_device *qdev)
void qxl_modeset_fini(struct qxl_device *qdev)
{
- if (qdev->dumb_shadow_bo) {
- qxl_bo_unpin(qdev->dumb_shadow_bo);
- drm_gem_object_put(&qdev->dumb_shadow_bo->tbo.base);
- qdev->dumb_shadow_bo = NULL;
- }
qxl_destroy_monitors_object(qdev);
}
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index cc02b5f10ad9..f29ce77eef7b 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -85,7 +85,6 @@ struct qxl_bo {
/* Constant after initialization */
unsigned int is_primary:1; /* is this now a primary surface */
unsigned int is_dumb:1;
- struct qxl_bo *shadow;
unsigned int hw_surf_alloc:1;
struct qxl_surface surf;
uint32_t surface_id;
@@ -201,8 +200,6 @@ 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;
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 5/5] drm/qxl: pack dumb heads from their plane source rectangles
2026-09-26 16:31 [PATCH v3 0/5] drm/qxl: fix shared-primary lifetime and source packing Dillon Amburgey
` (3 preceding siblings ...)
2026-09-26 16:31 ` [PATCH v3 4/5] drm/qxl: own packed primary shadows in atomic plane state Dillon Amburgey
@ 2026-09-26 16:31 ` Dillon Amburgey
4 siblings, 0 replies; 6+ messages in thread
From: Dillon Amburgey @ 2026-09-26 16:31 UTC (permalink / raw)
To: Dave Airlie, Gerd Hoffmann
Cc: Dillon Amburgey, dri-devel, virtualization, spice-devel,
linux-kernel, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, christian.koenig, krisman, noralf
Packing a 1280x800 source from a 2048x1024 dumb framebuffer beside a
1024x768 head creates a 3072x1024 primary instead of 2304x800. Copying
the full framebuffer also paints pixels outside the visible source.
Size packed heads from their plane source rectangles and copy only those
rectangles to their packed origins. Publish the same origins without
adding the framebuffer source position a second time.
Intersect DirtyFB clips with every matching plane source and translate
each intersection to its packed origin. Apply both horizontal and
vertical offsets so separate framebuffers, shared framebuffers and
nonzero source coordinates use the same mapping for modesets and dirty
updates.
Fixes: 90adda2ce898 ("drm/qxl: cover all crtcs in shadow bo.")
Assisted-by: LLM sparse
Signed-off-by: Dillon Amburgey <dillona@gmail.com>
---
drivers/gpu/drm/qxl/qxl_display.c | 63 ++++++++++++++++++++++---------
drivers/gpu/drm/qxl/qxl_draw.c | 6 ++-
drivers/gpu/drm/qxl/qxl_drv.h | 2 +-
3 files changed, 51 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index af36d4d1ea57..1b12458ef7c9 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -369,8 +369,10 @@ 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 (to_qxl_plane_state(crtc->primary->state)->shadow)
- head.x += to_qxl_plane_state(crtc->primary->state)->x;
+ if (to_qxl_plane_state(crtc->primary->state)->shadow) {
+ head.x = to_qxl_plane_state(crtc->primary->state)->x;
+ head.y = 0;
+ }
} else {
head.width = 0;
head.height = 0;
@@ -484,23 +486,40 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
drm_for_each_crtc(crtc, &qdev->ddev) {
struct drm_plane_state *st;
+ u32 sx, sy, sw, sh;
st = crtc->primary->state;
if (st->fb != fb || !crtc->state->active ||
!to_qxl_plane_state(st)->shadow ||
!to_qxl_plane_state(st)->shadow->is_primary)
continue;
+ sx = st->src_x >> 16;
+ sy = st->src_y >> 16;
+ sw = st->src_w >> 16;
+ sh = st->src_h >> 16;
for (n = 0; n < num_clips; n++) {
- struct drm_clip_rect c = clips[n * inc];
-
+ struct drm_clip_rect *in = clips + n * inc;
+ struct drm_clip_rect c;
+ u32 x1 = max_t(u32, in->x1, sx);
+ u32 y1 = max_t(u32, in->y1, sy);
+ u32 x2 = min_t(u32, in->x2, sx + sw);
+ u32 y2 = min_t(u32, in->y2, sy + sh);
+
+ if (x1 >= x2 || y1 >= y2)
+ continue;
+ c.x1 = x1;
+ c.y1 = y1;
+ c.x2 = x2;
+ c.y2 = y2;
qxl_draw_dirty_fb(qdev, fb, qobj, flags, color,
&c, 1, 1,
- to_qxl_plane_state(st)->x);
+ to_qxl_plane_state(st)->x - sx,
+ -(int)sy);
}
}
} else {
qxl_draw_dirty_fb(qdev, fb, qobj, flags, color,
- clips, num_clips, inc, 0);
+ clips, num_clips, inc, 0, 0);
}
out_lock_end:
@@ -741,12 +760,17 @@ static void qxl_primary_atomic_update(struct drm_plane *plane,
struct qxl_bo *bo = gem_to_qxl_bo(new_state->fb->obj[0]);
struct qxl_bo *primary;
struct drm_clip_rect norect = {
- .x1 = 0,
- .y1 = 0,
- .x2 = new_state->fb->width,
- .y2 = new_state->fb->height
+ .x1 = 0,
+ .y1 = 0,
+ .x2 = new_state->fb->width,
+ .y2 = new_state->fb->height,
};
uint32_t dumb_shadow_offset = 0;
+ int y_off = 0;
+ u32 src_x = new_state->src_x >> 16;
+ u32 src_y = new_state->src_y >> 16;
+ u32 src_w = new_state->src_w >> 16;
+ u32 src_h = new_state->src_h >> 16;
if (!qxl_primary_active(state, new_state)) {
qxl_primary_atomic_disable(plane, state);
@@ -762,11 +786,18 @@ static void qxl_primary_atomic_update(struct drm_plane *plane,
qxl_primary_apply_cursor(qdev, plane->state);
}
- if (bo->is_dumb)
- dumb_shadow_offset = to_qxl_plane_state(new_state)->x;
+ if (bo->is_dumb) {
+ norect.x1 = src_x;
+ norect.y1 = src_y;
+ norect.x2 = src_x + src_w;
+ norect.y2 = src_y + src_h;
+ dumb_shadow_offset =
+ to_qxl_plane_state(new_state)->x - src_x;
+ y_off = -(int)src_y;
+ }
qxl_draw_dirty_fb(qdev, new_state->fb, bo, 0, 0, &norect, 1, 1,
- dumb_shadow_offset);
+ dumb_shadow_offset, y_off);
}
static void qxl_cursor_atomic_update(struct drm_plane *plane,
@@ -838,7 +869,6 @@ static int qxl_prepare_shadow(struct drm_plane *plane,
drm_for_each_crtc(crtc, plane->dev) {
struct drm_plane_state *ps;
struct qxl_plane_state *other;
- struct qxl_bo *bo;
ps = drm_atomic_get_new_plane_state(state, crtc->primary);
if (!qxl_primary_active(state, ps) ||
@@ -847,9 +877,8 @@ static int qxl_prepare_shadow(struct drm_plane *plane,
other = to_qxl_plane_state(ps);
if (ps == new_state)
qps->x = surf.width;
- bo = gem_to_qxl_bo(ps->fb->obj[0]);
- surf.width += bo->surf.width;
- surf.height = max_t(u32, surf.height, bo->surf.height);
+ surf.width += ps->src_w >> 16;
+ surf.height = max_t(u32, surf.height, ps->src_h >> 16);
if (other->shadow)
shadow = other->shadow;
}
diff --git a/drivers/gpu/drm/qxl/qxl_draw.c b/drivers/gpu/drm/qxl/qxl_draw.c
index 3a3e127ce297..f99f4afa46d9 100644
--- a/drivers/gpu/drm/qxl/qxl_draw.c
+++ b/drivers/gpu/drm/qxl/qxl_draw.c
@@ -129,7 +129,7 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
unsigned int flags, unsigned int color,
struct drm_clip_rect *clips,
unsigned int num_clips, int inc,
- uint32_t dumb_shadow_offset)
+ u32 dumb_shadow_offset, int y_off)
{
/*
* TODO: if flags & DRM_MODE_FB_DIRTY_ANNOTATE_FILL then we should
@@ -160,6 +160,8 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
clips->x1 += dumb_shadow_offset;
clips->x2 += dumb_shadow_offset;
+ clips->y1 += y_off;
+ clips->y2 += y_off;
left = clips->x1;
right = clips->x2;
@@ -210,7 +212,7 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
ret = qxl_image_init(qdev, release, dimage, surface_base,
left - dumb_shadow_offset,
- top, width, height, depth, stride);
+ top - y_off, width, height, depth, stride);
qxl_bo_vunmap_locked(bo);
if (ret)
goto out_release_backoff;
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index f29ce77eef7b..24137f934fa1 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -391,7 +391,7 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
unsigned int flags, unsigned int color,
struct drm_clip_rect *clips,
unsigned int num_clips, int inc,
- uint32_t dumb_shadow_offset);
+ u32 dumb_shadow_offset, int y_off);
void qxl_release_free(struct qxl_device *qdev,
struct qxl_release *release);
^ permalink raw reply [flat|nested] 6+ messages in thread