[PATCH v2] drm/qxl: Pin buffer objects for internal mappings

[Dmitry Osipenko]

On 7/8/24 17:21, Thomas Zimmermann wrote:
> Add qxl_bo_pin_and_vmap() that pins and vmaps a buffer object in one
> step. Update callers of the regular qxl_bo_vmap(). Fixes a bug where
> qxl accesses an unpinned buffer object while it is being moved; such
> as with the monitor-description BO. An typical error is shown below.
> 
> [    4.303586] [drm:drm_atomic_helper_commit_planes] *ERROR* head 1 wrong: 65376256x16777216+0+0
> [    4.586883] [drm:drm_atomic_helper_commit_planes] *ERROR* head 1 wrong: 65376256x16777216+0+0
> [    4.904036] [drm:drm_atomic_helper_commit_planes] *ERROR* head 1 wrong: 65335296x16777216+0+0
> [    5.374347] [drm:qxl_release_from_id_locked] *ERROR* failed to find id in release_idr
> 
> Commit b33651a5c98d ("drm/qxl: Do not pin buffer objects for vmap")
> removed the implicit pin operation from qxl's vmap code. This is the
> correct behavior for GEM and PRIME interfaces, but the pin is still
> needed for qxl internal operation.
> 
> Also add a corresponding function qxl_bo_vunmap_and_unpin() and remove
> the old qxl_bo_vmap() helpers.
> 
> Future directions: BOs should not be pinned or vmapped unnecessarily.
> The pin-and-vmap operation should be removed from the driver and a
> temporary mapping should be established with a vmap_local-like helper.
> See the client helper drm_client_buffer_vmap_local() for semantics.
> 
> v2:
> - unreserve BO on errors in qxl_bo_pin_and_vmap() (Dmitry)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> Fixes: b33651a5c98d ("drm/qxl: Do not pin buffer objects for vmap")
> Reported-by: David Kaplan <david.kaplan at amd.com>
> Closes: https://lore.kernel.org/dri-devel/ab0fb17d-0f96-4ee6-8b21-65d02bb02655@suse.de/
> Tested-by: David Kaplan <david.kaplan at amd.com>
> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: Dmitry Osipenko <dmitry.osipenko at collabora.com>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Zack Rusin <zack.rusin at broadcom.com>
> Cc: Dave Airlie <airlied at redhat.com>
> Cc: Gerd Hoffmann <kraxel at redhat.com>
> Cc: virtualization at lists.linux.dev
> Cc: spice-devel at lists.freedesktop.org
> ---
>  drivers/gpu/drm/qxl/qxl_display.c | 14 +++++++-------
>  drivers/gpu/drm/qxl/qxl_object.c  | 13 +++++++++++--
>  drivers/gpu/drm/qxl/qxl_object.h  |  4 ++--
>  3 files changed, 20 insertions(+), 11 deletions(-)

Reviewed-by: Dmitry Osipenko <dmitry.osipenko at collabora.com>

-- 
Best regards,
Dmitry