Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 6 Dec 2018 05:55:58 -0500 (EST)
From:   Frediano Ziglio <fziglio@redhat.com>
To:     Gerd Hoffmann <kraxel@redhat.com>
Cc:     dri-devel@lists.freedesktop.org, David Airlie <airlied@redhat.com>,
        David Airlie <airlied@linux.ie>,
        "open list:DRM DRIVER FOR QXL VIRTUAL GPU" 
        <spice-devel@lists.freedesktop.org>,
        open list <linux-kernel@vger.kernel.org>,
        "open list:DRM DRIVER FOR QXL VIRTUAL GPU" 
        <virtualization@lists.linux-foundation.org>
Message-ID: <1897496108.48579647.1544093758850.JavaMail.zimbra@redhat.com>
In-Reply-To: <20181206104638.23330-2-kraxel@redhat.com>
References: <20181206104638.23330-1-kraxel@redhat.com> <20181206104638.23330-2-kraxel@redhat.com>
Subject: Re: [Spice-devel] [PATCH 1/3] drm/qxl: allow both PRIV and VRAM
 placement for QXL_GEM_DOMAIN_SURFACE
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Thread-Topic: drm/qxl: allow both PRIV and VRAM placement for QXL_GEM_DOMAIN_SURFACE
Thread-Index: 9S5T4qEzv8vQxy3NLdtcuo4mK/4vLA==
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> 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