Re: [Spice-devel] [PATCH 1/3] drm/qxl: allow both PRIV and VRAM placement for QXL_GEM_DOMAIN_SURFACE

[Frediano Ziglio <fziglio@redhat.com>]

> qxl surfaces (used for framebuffers and gem objects) can live in both
> VRAM and PRIV ttm domains.  Update placement setup to include both.  Put
> PRIV first in the list so it is preferred, so VRAM will have more room
> for objects which must be allocated there.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

I remember these kind of changes in the past made migration
fails. I proposed similar patches years ago and they were rejected
for these reasons. Why now they are safe?

Looks like we are improving QXL, so that means we are actively working
on it. Should we not then thinking about moving feature in the proper
places (like spice-server for atomic mode setting instead of implementin
work around) ??

> ---
>  drivers/gpu/drm/qxl/qxl_object.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_object.c
> b/drivers/gpu/drm/qxl/qxl_object.c
> index 91f3bbc73e..f7f9f4f4fe 100644
> --- a/drivers/gpu/drm/qxl/qxl_object.c
> +++ b/drivers/gpu/drm/qxl/qxl_object.c
> @@ -58,10 +58,10 @@ void qxl_ttm_placement_from_domain(struct qxl_bo *qbo,
> u32 domain, bool pinned)
>  
>  	qbo->placement.placement = qbo->placements;
>  	qbo->placement.busy_placement = qbo->placements;
> -	if (domain == QXL_GEM_DOMAIN_VRAM)
> -		qbo->placements[c++].flags = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_VRAM |
> pflag;
>  	if (domain == QXL_GEM_DOMAIN_SURFACE)
>  		qbo->placements[c++].flags = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_PRIV |
>  		pflag;
> +	if (domain == QXL_GEM_DOMAIN_SURFACE || domain == QXL_GEM_DOMAIN_VRAM)
> +		qbo->placements[c++].flags = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_VRAM |
> pflag;
>  	if (domain == QXL_GEM_DOMAIN_CPU)
>  		qbo->placements[c++].flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM |
>  		pflag;
>  	if (!c)

Frediano