Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/qxl: hardcoded horizontal monitors_config breaks vertical multi-monitor layouts --
commit 8c2ba11a8cac6cfc531b9cc6ef9a78703db0c342 Author: Dillon Amburgey <[email protected]> drm/qxl: pack dumb heads from their plane source rectangles This commit sizes packed heads from their plane source rectangles and copies only those rectangles to their packed origins. It intercepts DirtyFB clips with every matching plane source and applies horizontal and vertical offsets to map modesets and dirty updates identically. > diff --git a/drivers/gpu/drm/qxl/qxl_display.c > b/drivers/gpu/drm/qxl/qxl_display.c > --- 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; > + } [Severity: High] Does overwriting head.x with the shadow buffer's horizontal offset and explicitly zeroing head.y here discard the guest's actual vertical layout information? If a user configures a multi-monitor layout in the guest that is not purely side-by-side (like a vertical or custom arrangement), it appears qxl_crtc_update_monitors_config() will override the layout data sent to the SPICE client. Could this break the SPICE client's window placement and destroy absolute mouse input mapping on secondary monitors, since the client's coordinate space would no longer match the guest's? [ ... ] > @@ -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); [Severity: High] Does accumulating surf.width here in qxl_prepare_shadow() force all CRTCs to be packed horizontally in the shadow buffer? It seems this horizontal packing might be what necessitates overriding the layout coordinates in qxl_crtc_update_monitors_config() above. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
