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 X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6DC1BC433B4 for ; Mon, 17 May 2021 07:28:11 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 36AF361209 for ; Mon, 17 May 2021 07:28:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 36AF361209 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 768106E8D1; Mon, 17 May 2021 07:28:10 +0000 (UTC) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by gabe.freedesktop.org (Postfix) with ESMTPS id 62D4A6E8D1; Mon, 17 May 2021 07:28:09 +0000 (UTC) IronPort-SDR: av+pxS9r/r+PkNDjm4sT9usIfFA0RHuNEhzmXIOsFNBgxEWTyxJBNx/bYgohLbDlHUlzrNwvF5 2FLrD1XbtFOg== X-IronPort-AV: E=McAfee;i="6200,9189,9986"; a="180680535" X-IronPort-AV: E=Sophos;i="5.82,306,1613462400"; d="scan'208";a="180680535" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 May 2021 00:28:07 -0700 IronPort-SDR: EgOeUbN+Mg5uP5SuBh2OcXFmLfFdzDqAQgpMBGFOuB6n9ChwMhOgM9uyqGX2vGX+JsDsBmMr2g 6Rq/FSqvXM/w== X-IronPort-AV: E=Sophos;i="5.82,306,1613462400"; d="scan'208";a="472279644" Received: from rminkov-mobl3.ger.corp.intel.com ([10.249.254.204]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 May 2021 00:28:03 -0700 Message-ID: <737577c5d27d27518d6363143ad4d50fb5d14bb4.camel@linux.intel.com> Subject: Re: [Intel-gfx] [PATCH 3/7] drm/i915/ttm, drm/ttm: Initialize the ttm device and memory managers. From: Thomas =?ISO-8859-1?Q?Hellstr=F6m?= To: Matthew Auld Date: Mon, 17 May 2021 09:28:01 +0200 In-Reply-To: References: <20210511132525.377190-1-thomas.hellstrom@linux.intel.com> <20210511132525.377190-4-thomas.hellstrom@linux.intel.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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: Intel Graphics Development , Christian =?ISO-8859-1?Q?K=F6nig?= , ML dri-devel Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Mon, 2021-05-17 at 09:18 +0200, Thomas Hellström wrote: > On Wed, 2021-05-12 at 09:57 +0100, Matthew Auld wrote: > > On Tue, 11 May 2021 at 14:26, Thomas Hellström > > wrote: > > > > > > Temporarily remove the buddy allocator and related selftests > > > and hook up the TTM range manager for i915 regions. > > > > > > In order to support some of the mock region-related selftests, we > > > need to > > > be able to initialize the TTM range-manager standalone without a > > > struct > > > ttm_device. Add two functions to allow that to the TTM api. > > > > > > Finally modify the mock region selftests somewhat to account for > > > a > > > fragmenting manager. > > > > > > Cc: Christian König > > > Signed-off-by: Thomas Hellström > > > > > > --- > > >  drivers/gpu/drm/i915/Kconfig                  |   1 + > > >  drivers/gpu/drm/i915/Makefile                 |   2 +- > > >  drivers/gpu/drm/i915/gem/i915_gem_lmem.c      |  58 +- > > >  .../gpu/drm/i915/gem/i915_gem_object_types.h  |   6 +- > > >  drivers/gpu/drm/i915/gem/i915_gem_pages.c     |   3 +- > > >  drivers/gpu/drm/i915/gem/i915_gem_region.c    | 120 --- > > >  drivers/gpu/drm/i915/gem/i915_gem_region.h    |   4 - > > >  drivers/gpu/drm/i915/gem/i915_gem_shmem.c     |   4 +- > > >  drivers/gpu/drm/i915/gem/i915_gem_stolen.c    |  10 +- > > >  drivers/gpu/drm/i915/gem/i915_gem_stolen.h    |   9 +- > > >  drivers/gpu/drm/i915/gt/intel_gt.c            |   2 - > > >  drivers/gpu/drm/i915/gt/intel_region_lmem.c   |  27 +- > > >  drivers/gpu/drm/i915/i915_buddy.c             | 435 ---------- > > >  drivers/gpu/drm/i915/i915_buddy.h             | 131 --- > > >  drivers/gpu/drm/i915/i915_drv.c               |   8 + > > >  drivers/gpu/drm/i915/i915_drv.h               |   7 +- > > >  drivers/gpu/drm/i915/i915_gem.c               |   1 + > > >  drivers/gpu/drm/i915/i915_globals.c           |   1 - > > >  drivers/gpu/drm/i915/i915_globals.h           |   1 - > > >  drivers/gpu/drm/i915/i915_scatterlist.c       |  70 ++ > > >  drivers/gpu/drm/i915/i915_scatterlist.h       |  35 + > > >  drivers/gpu/drm/i915/intel_memory_region.c    | 180 ++-- > > >  drivers/gpu/drm/i915/intel_memory_region.h    |  44 +- > > >  drivers/gpu/drm/i915/intel_region_ttm.c       | 246 ++++++ > > >  drivers/gpu/drm/i915/intel_region_ttm.h       |  29 + > > >  drivers/gpu/drm/i915/selftests/i915_buddy.c   | 789 ------------ > > > -- > > > ---- > > >  .../drm/i915/selftests/i915_mock_selftests.h  |   1 - > > >  .../drm/i915/selftests/intel_memory_region.c  | 133 +-- > > >  drivers/gpu/drm/i915/selftests/mock_region.c  |  51 +- > > >  drivers/gpu/drm/ttm/ttm_range_manager.c       |  55 +- > > >  include/drm/ttm/ttm_bo_driver.h               |  23 + > > >  31 files changed, 715 insertions(+), 1771 deletions(-) > > >  delete mode 100644 drivers/gpu/drm/i915/i915_buddy.c > > >  delete mode 100644 drivers/gpu/drm/i915/i915_buddy.h > > >  create mode 100644 drivers/gpu/drm/i915/intel_region_ttm.c > > >  create mode 100644 drivers/gpu/drm/i915/intel_region_ttm.h > > >  delete mode 100644 drivers/gpu/drm/i915/selftests/i915_buddy.c > > > > > > diff --git a/drivers/gpu/drm/i915/Kconfig > > > b/drivers/gpu/drm/i915/Kconfig > > > index 1e1cb245fca7..b63d374dff23 100644 > > > --- a/drivers/gpu/drm/i915/Kconfig > > > +++ b/drivers/gpu/drm/i915/Kconfig > > > @@ -26,6 +26,7 @@ config DRM_I915 > > >         select SND_HDA_I915 if SND_HDA_CORE > > >         select CEC_CORE if CEC_NOTIFIER > > >         select VMAP_PFN > > > +       select DRM_TTM > > >         help > > >           Choose this option if you have a system that has "Intel > > > Graphics > > >           Media Accelerator" or "HD Graphics" integrated > > > graphics, > > > diff --git a/drivers/gpu/drm/i915/Makefile > > > b/drivers/gpu/drm/i915/Makefile > > > index d0d936d9137b..cb8823570996 100644 > > > --- a/drivers/gpu/drm/i915/Makefile > > > +++ b/drivers/gpu/drm/i915/Makefile > > > @@ -50,6 +50,7 @@ i915-y += i915_drv.o \ > > >           intel_memory_region.o \ > > >           intel_pch.o \ > > >           intel_pm.o \ > > > +         intel_region_ttm.o \ > > >           intel_runtime_pm.o \ > > >           intel_sideband.o \ > > >           intel_step.o \ > > > @@ -160,7 +161,6 @@ gem-y += \ > > >  i915-y += \ > > >           $(gem-y) \ > > >           i915_active.o \ > > > -         i915_buddy.o \ > > >           i915_cmd_parser.o \ > > >           i915_gem_evict.o \ > > >           i915_gem_gtt.o \ > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c > > > b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c > > > index f44bdd08f7cb..f42803ea48f2 100644 > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c > > > @@ -4,16 +4,70 @@ > > >   */ > > > > > >  #include "intel_memory_region.h" > > > +#include "intel_region_ttm.h" > > >  #include "gem/i915_gem_region.h" > > >  #include "gem/i915_gem_lmem.h" > > >  #include "i915_drv.h" > > > > > > +static void lmem_put_pages(struct drm_i915_gem_object *obj, > > > +                                 struct sg_table *pages) > > > +{ > > > +       intel_region_ttm_node_free(obj->mm.region, obj- > > > > mm.st_mm_node); > > > +       obj->mm.dirty = false; > > > +       sg_free_table(pages); > > > +       kfree(pages); > > > +} > > > + > > > +static int lmem_get_pages(struct drm_i915_gem_object *obj) > > > +{ > > > +       unsigned int flags; > > > +       struct sg_table *pages; > > > + > > > +       flags = I915_ALLOC_MIN_PAGE_SIZE; > > > +       if (obj->flags & I915_BO_ALLOC_CONTIGUOUS) > > > +               flags |= I915_ALLOC_CONTIGUOUS; > > > + > > > +       obj->mm.st_mm_node = intel_region_ttm_node_alloc(obj- > > > > mm.region, > > > +                                                        obj- > > > > base.size, > > > +                                                        flags); > > > +       if (IS_ERR(obj->mm.st_mm_node)) > > > +               return PTR_ERR(obj->mm.st_mm_node); > > > + > > > +       /* Range manager is always contigous */ > > > +       if (obj->mm.region->is_range_manager) > > > +               obj->flags |= I915_BO_ALLOC_CONTIGUOUS; > > > +       pages = intel_region_ttm_node_to_st(obj->mm.region, obj- > > > > mm.st_mm_node); > > > +       if (IS_ERR(pages)) > > > +               return PTR_ERR(pages); > > > > error unwind? > > > > > + > > > +       __i915_gem_object_set_pages(obj, pages, > > > +                                   i915_sg_dma_page_sizes(pages- > > > > sgl)); > > > + > > > +       if (obj->flags & I915_BO_ALLOC_CPU_CLEAR) { > > > +               void __iomem *vaddr = > > > +                       i915_gem_object_lmem_io_map(obj, 0, obj- > > > > base.size); > > > + > > > +               if (!vaddr) { > > > +                       struct sg_table *pages = > > > +                               > > > __i915_gem_object_unset_pages(obj); > > > + > > > +                       if (!IS_ERR_OR_NULL(pages)) > > > +                               lmem_put_pages(obj, pages); > > > +               } > > > + > > > +               memset_io(vaddr, 0, obj->base.size); > > > +               io_mapping_unmap(vaddr); > > > +       } > > > + > > > +       return 0; > > > +} > > > > > > > > > +/** > > > + * i915_sg_dma_page_sizes - Calculate page sizes from a > > > scatterlist > > > + * @sg: The scatterlist from which to calculate page sizes > > > + * > > > + * Return: a value with bits sets for relevant page sizes. > > > + */ > > > +static inline unsigned int i915_sg_dma_page_sizes(struct > > > scatterlist *sg) > > > +{ > > > +       unsigned int page_sizes; > > > + > > > +       page_sizes = 0; > > > +       while (sg) { > > > +               GEM_BUG_ON(sg->offset); > > > +               GEM_BUG_ON(!IS_ALIGNED(sg->dma_length, > > > PAGE_SIZE)); > > > +               page_sizes |= sg->dma_length; > > > +               sg = __sg_next(sg); > > > +       } > > > + > > > +       /* > > > +        * Is this necessary to support building large GPU pages > > > from > > > +        * even larger segments? > > > +        */ > > > > We just need to know the above sg page_sizes, which is the mask of > > sg->dma_length for each sg. Later in __i915_gem_object_set_pages() > > we > > use this to determine the potential GPU page sizes for the object, > > depending on what the device supports. So the below stuff shouldn't > > be > > needed. Also we can just use i915_sg_page_sizes() here. > > I noticed that i915_sg_page_sizes() uses sg->length() instead of sg- > > dma_length(). That doesn't look like what we're after here. Do you > know if there is a reason for that? Actually, looking at the users, that seems like a bug. I'll make a separate patch out of it. /Thomas > > Thanks, > Thomas >