From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1C7A18F44 for ; Thu, 23 Nov 2023 14:48:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="GUBVz5eU" Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 847C980DD3 for ; Thu, 23 Nov 2023 14:48:57 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 847C980DD3 Authentication-Results: smtp1.osuosl.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.a=rsa-sha256 header.s=mail header.b=GUBVz5eU X-Virus-Scanned: amavisd-new at osuosl.org X-Spam-Flag: NO X-Spam-Score: -2.1 X-Spam-Level: Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id IQFRCBbAxhty for ; Thu, 23 Nov 2023 14:48:56 +0000 (UTC) Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by smtp1.osuosl.org (Postfix) with ESMTPS id AE15580DCD for ; Thu, 23 Nov 2023 14:48:56 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org AE15580DCD Received: from localhost (cola.collaboradmins.com [195.201.22.229]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by madras.collabora.co.uk (Postfix) with ESMTPSA id C2B82660739B; Thu, 23 Nov 2023 14:48:51 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1700750932; bh=TOxOSufmcOtURwsWljuFpDtQMmDKswF940eCd0/40e8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=GUBVz5eUSRzQhnr6IkWj+jo4rJm341qH1y3W881oKdKubsHsULT1YCS3ioMDZwpOk ssD5sNLakFDgu7dS5Tk0PaAgh/hMAMZSwynz1ccdNpYDxkC2mXV4ltmrpw0TxKcr0Y ejGZ8g02aLRVdDIGsom6njnd/GnR/GR8PtMIupQIRnWdEOcFWDnUw1bHWNO1rgY/d4 0IgKTiFj8JKT43OsH+hoW3pIq1Vho62mmbtrbQQaHZhXeLB+saxCqR1swFBEa12Gtn 4594UIN8+PPbXGQFijdjHu65/TC45og1ePfRGgUCaxv/Bih77MoQcIGiEm9ATQNTDG tUaj9so8WEU+Q== Date: Thu, 23 Nov 2023 15:48:48 +0100 From: Boris Brezillon To: Dmitry Osipenko Cc: David Airlie , Gerd Hoffmann , Gurchetan Singh , Chia-I Wu , Daniel Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Christian =?UTF-8?B?S8O2bmln?= , Qiang Yu , Steven Price , Emma Anholt , Melissa Wen , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, kernel@collabora.com, virtualization@lists.linux-foundation.org Subject: Re: [PATCH v18 15/26] drm/panfrost: Explicitly get and put drm-shmem pages Message-ID: <20231123154848.034f4710@collabora.com> In-Reply-To: <20231029230205.93277-16-dmitry.osipenko@collabora.com> References: <20231029230205.93277-1-dmitry.osipenko@collabora.com> <20231029230205.93277-16-dmitry.osipenko@collabora.com> Organization: Collabora X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-redhat-linux-gnu) Precedence: bulk X-Mailing-List: virtualization@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Mon, 30 Oct 2023 02:01:54 +0300 Dmitry Osipenko wrote: > To simplify the drm-shmem refcnt handling, we're moving away from > the implicit get_pages() that is used by get_pages_sgt(). From now on > drivers will have to pin pages while they use sgt. Panfrost's shrinker > doesn't support swapping out BOs, hence pages are pinned and sgt is valid > as long as pages' use-count > 0. > > Signed-off-by: Dmitry Osipenko > --- > drivers/gpu/drm/panfrost/panfrost_gem.c | 17 +++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_mmu.c | 6 ++---- > 2 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > index 6b77d8cebcb2..bb9d43cf7c3c 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -47,8 +47,13 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) > } > } > kvfree(bo->sgts); > + > + drm_gem_shmem_put_pages(&bo->base); > } > > + if (!bo->is_heap && !obj->import_attach) > + drm_gem_shmem_put_pages(&bo->base); > + > drm_gem_shmem_free(&bo->base); > } > > @@ -269,6 +274,7 @@ panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags) > { > struct drm_gem_shmem_object *shmem; > struct panfrost_gem_object *bo; > + int err; > > /* Round up heap allocations to 2MB to keep fault handling simple */ > if (flags & PANFROST_BO_HEAP) > @@ -282,7 +288,18 @@ panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags) > bo->noexec = !!(flags & PANFROST_BO_NOEXEC); > bo->is_heap = !!(flags & PANFROST_BO_HEAP); > > + if (!bo->is_heap) { > + err = drm_gem_shmem_get_pages(shmem); I really hate the fact we request pages here while we call panfrost_mmu_map() in panfrost_gem_open(), because ultimately, pages are requested for the MMU mapping. Also hate the quirk we have in shmem to call free_pages() instead of put_pages_locked() when the BO refcount dropped to zero, and I was hoping we could get rid of it at some point by teaching drivers to request pages when they actually need it instead of tying pages lifetime to the GEM object lifetime. Maybe what we should do instead is move the get/put_pages() in panfrost_mmu_map/unmap() (as I suggested), but have a special mapping panfrost_mmu_evict/restore() helpers that kill/restore the MMU mappings without releasing/acquiring the pages ref. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 257BFC61DF7 for ; Thu, 23 Nov 2023 14:48:57 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 54B0410E084; Thu, 23 Nov 2023 14:48:56 +0000 (UTC) Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by gabe.freedesktop.org (Postfix) with ESMTPS id 141A510E084 for ; Thu, 23 Nov 2023 14:48:54 +0000 (UTC) Received: from localhost (cola.collaboradmins.com [195.201.22.229]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by madras.collabora.co.uk (Postfix) with ESMTPSA id C2B82660739B; Thu, 23 Nov 2023 14:48:51 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1700750932; bh=TOxOSufmcOtURwsWljuFpDtQMmDKswF940eCd0/40e8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=GUBVz5eUSRzQhnr6IkWj+jo4rJm341qH1y3W881oKdKubsHsULT1YCS3ioMDZwpOk ssD5sNLakFDgu7dS5Tk0PaAgh/hMAMZSwynz1ccdNpYDxkC2mXV4ltmrpw0TxKcr0Y ejGZ8g02aLRVdDIGsom6njnd/GnR/GR8PtMIupQIRnWdEOcFWDnUw1bHWNO1rgY/d4 0IgKTiFj8JKT43OsH+hoW3pIq1Vho62mmbtrbQQaHZhXeLB+saxCqR1swFBEa12Gtn 4594UIN8+PPbXGQFijdjHu65/TC45og1ePfRGgUCaxv/Bih77MoQcIGiEm9ATQNTDG tUaj9so8WEU+Q== Date: Thu, 23 Nov 2023 15:48:48 +0100 From: Boris Brezillon To: Dmitry Osipenko Subject: Re: [PATCH v18 15/26] drm/panfrost: Explicitly get and put drm-shmem pages Message-ID: <20231123154848.034f4710@collabora.com> In-Reply-To: <20231029230205.93277-16-dmitry.osipenko@collabora.com> References: <20231029230205.93277-1-dmitry.osipenko@collabora.com> <20231029230205.93277-16-dmitry.osipenko@collabora.com> Organization: Collabora X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel@collabora.com, Thomas Zimmermann , Emma Anholt , Christian =?UTF-8?B?S8O2bmln?= , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Maxime Ripard , Gurchetan Singh , Melissa Wen , Gerd Hoffmann , Steven Price , virtualization@lists.linux-foundation.org, Qiang Yu Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Mon, 30 Oct 2023 02:01:54 +0300 Dmitry Osipenko wrote: > To simplify the drm-shmem refcnt handling, we're moving away from > the implicit get_pages() that is used by get_pages_sgt(). From now on > drivers will have to pin pages while they use sgt. Panfrost's shrinker > doesn't support swapping out BOs, hence pages are pinned and sgt is valid > as long as pages' use-count > 0. > > Signed-off-by: Dmitry Osipenko > --- > drivers/gpu/drm/panfrost/panfrost_gem.c | 17 +++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_mmu.c | 6 ++---- > 2 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > index 6b77d8cebcb2..bb9d43cf7c3c 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -47,8 +47,13 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) > } > } > kvfree(bo->sgts); > + > + drm_gem_shmem_put_pages(&bo->base); > } > > + if (!bo->is_heap && !obj->import_attach) > + drm_gem_shmem_put_pages(&bo->base); > + > drm_gem_shmem_free(&bo->base); > } > > @@ -269,6 +274,7 @@ panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags) > { > struct drm_gem_shmem_object *shmem; > struct panfrost_gem_object *bo; > + int err; > > /* Round up heap allocations to 2MB to keep fault handling simple */ > if (flags & PANFROST_BO_HEAP) > @@ -282,7 +288,18 @@ panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags) > bo->noexec = !!(flags & PANFROST_BO_NOEXEC); > bo->is_heap = !!(flags & PANFROST_BO_HEAP); > > + if (!bo->is_heap) { > + err = drm_gem_shmem_get_pages(shmem); I really hate the fact we request pages here while we call panfrost_mmu_map() in panfrost_gem_open(), because ultimately, pages are requested for the MMU mapping. Also hate the quirk we have in shmem to call free_pages() instead of put_pages_locked() when the BO refcount dropped to zero, and I was hoping we could get rid of it at some point by teaching drivers to request pages when they actually need it instead of tying pages lifetime to the GEM object lifetime. Maybe what we should do instead is move the get/put_pages() in panfrost_mmu_map/unmap() (as I suggested), but have a special mapping panfrost_mmu_evict/restore() helpers that kill/restore the MMU mappings without releasing/acquiring the pages ref.