From patchwork Thu Sep 4 02:36:57 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jerome Glisse X-Patchwork-Id: 4842281 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 751DAC0338 for ; Thu, 4 Sep 2014 02:37:06 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 3BF082020F for ; Thu, 4 Sep 2014 02:37:05 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 17169201EF for ; Thu, 4 Sep 2014 02:37:04 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 78D596E0CD; Wed, 3 Sep 2014 19:37:02 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-qc0-f178.google.com (mail-qc0-f178.google.com [209.85.216.178]) by gabe.freedesktop.org (Postfix) with ESMTP id B5D536E0CD for ; Wed, 3 Sep 2014 19:37:00 -0700 (PDT) Received: by mail-qc0-f178.google.com with SMTP id x13so9633105qcv.23 for ; Wed, 03 Sep 2014 19:37:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=2xFED20xpKQ2s2i632b2Mp5ek1sLi5coLeodEH+nid0=; b=UuXftxF8Z7dvnWgowYb3BNlijw4MyPPWKPz/lCwPnBojKTTx6ltFpbsXEWYBY7d72a w5cn+ajnpJQ3npRQvZZkJ2jDYSAhyl0MPzG2mMRli7cfOltEDa9ULpc9ekHfxK+/X08e px0HGEXYQXC8xMrLYG+4p9YBAYLDwBTpt8MdcbmFbsAXIKzHNBOXrYHxrNjZcsjPWlIj SLI/E3+7DS8GlMEhrFbsTgAv2Ol3j8aMzcIr3bX2/Rif1RLq7q90j7SKzKQHQQYOf6Qb J5Bkc7nLNv543RmZ4NHCGmrV+IMrVFc+xvKNpryDl1+FS66ckBa2kCBaRfvacMCaBW6J Qi+Q== X-Received: by 10.224.131.8 with SMTP id v8mr2482598qas.31.1409798220357; Wed, 03 Sep 2014 19:37:00 -0700 (PDT) Received: from gmail.com (c-66-31-44-77.hsd1.ma.comcast.net. [66.31.44.77]) by mx.google.com with ESMTPSA id a4sm18586262qab.9.2014.09.03.19.36.59 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 03 Sep 2014 19:36:59 -0700 (PDT) Date: Wed, 3 Sep 2014 22:36:57 -0400 From: Jerome Glisse To: Benjamin Herrenschmidt Subject: Re: TTM placement & caching issue/questions Message-ID: <20140904023656.GF4835@gmail.com> References: <1409789547.30640.136.camel@pasglop> <20140904015548.GB4835@gmail.com> <20140904020742.GC4835@gmail.com> <1409797523.25089.8.camel@pasglop> <20140904023117.GD4835@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140904023117.GD4835@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: Alex Deucher , linuxppc-dev@ozlabs.org, Michel =?iso-8859-1?Q?D=E4nzer?= , Christian Koenig , dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, FSL_HELO_FAKE, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Sep 03, 2014 at 10:31:18PM -0400, Jerome Glisse wrote: > On Thu, Sep 04, 2014 at 12:25:23PM +1000, Benjamin Herrenschmidt wrote: > > On Wed, 2014-09-03 at 22:07 -0400, Jerome Glisse wrote: > > > > > So in the meantime the attached patch should work, it just silently ignore > > > the caching attribute request on non x86 instead of pretending that things > > > are setup as expected and then latter the radeon ou nouveau hw unsetting > > > the snoop bit. > > > > > > It's not tested but i think it should work. > > > > I'm still getting placements with !CACHED going from bo_memcpy in > > ttm_io_prot() though ... I'm looking at filtering the placement > > attributes instead. > > > > Ben. > > Ok so this one should do the trick. Ok final version ... famous last word. > > > > > > > > > > > > Cheers, > > > > Jérôme > > > > > > > > > > > > > > Cheers, > > > > > Ben. > > > > > > > > > > > > > > > _______________________________________________ > > > > > dri-devel mailing list > > > > > dri-devel@lists.freedesktop.org > > > > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > > From def7a056d042220f91016d0a7c245ba8e96f90ba Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= > Date: Wed, 3 Sep 2014 22:04:34 -0400 > Subject: [PATCH] drm/ttm: force cached mapping on non x86 platform. > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > People interested in providing uncached or write combined mapping > on there architecture need to do the ground work inside there arch > specific code to allow to break the linear kernel mapping so that > page mapping attributes can be updated, in the meantime force cached > mapping for non x86 architecture. > > Signed-off-by: Jérôme Glisse > --- > drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- > drivers/gpu/drm/ttm/ttm_bo.c | 2 +- > drivers/gpu/drm/ttm/ttm_tt.c | 47 ++++++++++++++++++++++++++++--------- > include/drm/ttm/ttm_bo_driver.h | 2 +- > 4 files changed, 39 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c > index 72afe82..4dd5060 100644 > --- a/drivers/gpu/drm/radeon/radeon_ttm.c > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c > @@ -304,7 +304,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo, > return r; > } > > - r = ttm_tt_set_placement_caching(bo->ttm, tmp_mem.placement); > + r = ttm_tt_set_placement_caching(bo->ttm, &tmp_mem.placement); > if (unlikely(r)) { > goto out_cleanup; > } > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 3da89d5..4dc21c3 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -305,7 +305,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, > goto out_err; > } > > - ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement); > + ret = ttm_tt_set_placement_caching(bo->ttm, &mem->placement); > if (ret) > goto out_err; > > diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c > index bf080ab..7cbdb48 100644 > --- a/drivers/gpu/drm/ttm/ttm_tt.c > +++ b/drivers/gpu/drm/ttm/ttm_tt.c > @@ -89,14 +89,6 @@ static inline int ttm_tt_set_page_caching(struct page *p, > > return ret; > } > -#else /* CONFIG_X86 */ > -static inline int ttm_tt_set_page_caching(struct page *p, > - enum ttm_caching_state c_old, > - enum ttm_caching_state c_new) > -{ > - return 0; > -} > -#endif /* CONFIG_X86 */ > > /* > * Change caching policy for the linear kernel map > @@ -149,19 +141,52 @@ out_err: > return ret; > } > > -int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement) > +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement) > { > enum ttm_caching_state state; > > - if (placement & TTM_PL_FLAG_WC) > + if (*placement & TTM_PL_FLAG_WC) > state = tt_wc; > - else if (placement & TTM_PL_FLAG_UNCACHED) > + else if (*placement & TTM_PL_FLAG_UNCACHED) > state = tt_uncached; > else > state = tt_cached; > > return ttm_tt_set_caching(ttm, state); > } > +#else /* CONFIG_X86 */ > +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement) > +{ > + if (placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) { > + ttm->caching_state = tt_cached; > + } else { > + if (placement & TTM_PL_FLAG_WC) > + ttm->caching_state = tt_wc; > + else if (placement & TTM_PL_FLAG_UNCACHED) > + ttm->caching_state = tt_uncached; > + else > + ttm->caching_state = tt_cached; > + } > + /* > + * Some architecture force cached so we need to reflect the > + * new ttm->caching_state into the mem->placement flags. > + */ > + *placement &= ~TTM_PL_MASK_CACHING; > + switch (bo->ttm->caching_state) { > + case tt_wc: > + *placement |= TTM_PL_FLAG_WC; > + break; > + case tt_uncached: > + *placement |= TTM_PL_FLAG_UNCACHED; > + break; > + case tt_cached: > + default: > + *placement |= TTM_PL_FLAG_CACHED; > + break; > + } > + return 0; > +} > +#endif /* CONFIG_X86 */ > EXPORT_SYMBOL(ttm_tt_set_placement_caching); > > void ttm_tt_destroy(struct ttm_tt *ttm) > diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h > index 1d9f0f1..cbc5ad2 100644 > --- a/include/drm/ttm/ttm_bo_driver.h > +++ b/include/drm/ttm/ttm_bo_driver.h > @@ -669,7 +669,7 @@ extern int ttm_tt_swapin(struct ttm_tt *ttm); > * hit RAM. This function may be very costly as it involves global TLB > * and cache flushes and potential page splitting / combining. > */ > -extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement); > +extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement); > extern int ttm_tt_swapout(struct ttm_tt *ttm, > struct file *persistent_swap_storage); > > -- > 1.9.3 > From 236038e18dc303bb9aa877922e01963d3fb0b7af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= Date: Wed, 3 Sep 2014 22:04:34 -0400 Subject: [PATCH] drm/ttm: force cached mapping on non x86 platform. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit People interested in providing uncached or write combined mapping on there architecture need to do the ground work inside there arch specific code to allow to break the linear kernel mapping so that page mapping attributes can be updated, in the meantime force cached mapping for non x86 architecture. Signed-off-by: Jérôme Glisse --- drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 2 +- drivers/gpu/drm/ttm/ttm_tt.c | 32 +++++++++++++++++++++----------- include/drm/ttm/ttm_bo_driver.h | 2 +- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 72afe82..4dd5060 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -304,7 +304,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo, return r; } - r = ttm_tt_set_placement_caching(bo->ttm, tmp_mem.placement); + r = ttm_tt_set_placement_caching(bo->ttm, &tmp_mem.placement); if (unlikely(r)) { goto out_cleanup; } diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 3da89d5..4dc21c3 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -305,7 +305,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, goto out_err; } - ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement); + ret = ttm_tt_set_placement_caching(bo->ttm, &mem->placement); if (ret) goto out_err; diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index bf080ab..a0df803 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -89,14 +89,6 @@ static inline int ttm_tt_set_page_caching(struct page *p, return ret; } -#else /* CONFIG_X86 */ -static inline int ttm_tt_set_page_caching(struct page *p, - enum ttm_caching_state c_old, - enum ttm_caching_state c_new) -{ - return 0; -} -#endif /* CONFIG_X86 */ /* * Change caching policy for the linear kernel map @@ -149,19 +141,37 @@ out_err: return ret; } -int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement) +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement) { enum ttm_caching_state state; - if (placement & TTM_PL_FLAG_WC) + if (*placement & TTM_PL_FLAG_WC) state = tt_wc; - else if (placement & TTM_PL_FLAG_UNCACHED) + else if (*placement & TTM_PL_FLAG_UNCACHED) state = tt_uncached; else state = tt_cached; return ttm_tt_set_caching(ttm, state); } +#else /* CONFIG_X86 */ +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement) +{ + if (*placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) { + ttm->caching_state = tt_cached; + *placement &= ~TTM_PL_MASK_CACHING; + *placement |= TTM_PL_FLAG_CACHED; + } else { + if (*placement & TTM_PL_FLAG_WC) + ttm->caching_state = tt_wc; + else if (placement & TTM_PL_FLAG_UNCACHED) + ttm->caching_state = tt_uncached; + else + ttm->caching_state = tt_cached; + } + return 0; +} +#endif /* CONFIG_X86 */ EXPORT_SYMBOL(ttm_tt_set_placement_caching); void ttm_tt_destroy(struct ttm_tt *ttm) diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index 1d9f0f1..cbc5ad2 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -669,7 +669,7 @@ extern int ttm_tt_swapin(struct ttm_tt *ttm); * hit RAM. This function may be very costly as it involves global TLB * and cache flushes and potential page splitting / combining. */ -extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement); +extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement); extern int ttm_tt_swapout(struct ttm_tt *ttm, struct file *persistent_swap_storage);