From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerome Glisse Subject: Re: [PATCH] drm/ttm: Only try to preserve caching in moves in the same space Date: Mon, 1 Feb 2010 14:33:17 +0100 Message-ID: <20100201133317.GH29000@localhost.localdomain> References: <1264670799-16101-1-git-send-email-luca@luca-barbieri.com> <21d7e9971001311735q3718ad8dy5298f19175a79a6e@mail.gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <21d7e9971001311735q3718ad8dy5298f19175a79a6e@mail.gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.sourceforge.net To: Dave Airlie Cc: airlied@linux.ie, Jerome Glisse , Luca Barbieri , dri-devel@lists.sourceforge.net, thellstrom@vmware.com List-Id: dri-devel@lists.freedesktop.org On Mon, Feb 01, 2010 at 11:35:21AM +1000, Dave Airlie wrote: > On Thu, Jan 28, 2010 at 7:26 PM, Luca Barbieri w= rote: > > Currently TTM tries to preserve the previous caching even for moves > > between unrelated memory spaces. > > > > This results for instance in moves from VRAM to PCIE GART changing > > system memory to WC state needlessly. > > > > This patch changes TTM to only try to preserve caching if moving from > > system/TT to system/TT. > > > > In theory, we should also do that if moving between two device memory > > spaces that are backend by the same physical memory area. > > However, I'm not sure how to do that in the current TTM framework and > > I suspect no card/driver uses memory spaces in that way. > = > Thomas or Jerome (since I think placement changes were you), any > chance of ack or review? > = > Dave. > = I think i will NACK, the idea to avoid switching to WC is good but i think = it can be achieve with current code by using cleverly the infrastructure. What happen is that WC is prefered over cached in ttm_tt_set_placement_cach= ing so to avoid WC taking over cached all you have to do is : At memory type init you init default caching as : SYSTEM.default_caching =3D TTM_PL_MASK_CACHING if PCI|PCIE TT.default_caching =3D TTM_PL_MASK_CACHED else TT.default_caching =3D TTM_PL_MASK_UNCACHED VRAM.default_caching =3D TTM_PL_FLAG_WC Now when ever you move a buffer for the proposed placement the flag you supply for caching are the default_caching of the memory type you want to use. So on VRAM->TT (PCIE case) ttm_bo_select_caching get call with : cur_placement =3D TTM_PL_FLAG_WC & proposed_placement =3D TTM_PL_MASK_CACHED the first if will fail but the second one will pass and so we get: result =3D TTM_PL_MASK_CACHED AGP is special case for the driver, instead of using the default_cahing from the memory type you are targetting the driver should always ask for uncached on TT->SYSTEM and VRAM->TT, and WC on SYSTEM|TT->VRAM. To avoid having if/else you can simply change SYSTEM.default_caching to UNCACHED for AGP and as before always use default_caching flag of the type you are moving to in the proposed placement. This way the only if/else is in driver's ttm initialization. Radeon is more or less (well more less than more ;)) doing that, i gonna test with tweaking the default_caching stuff and see how it performs. Btw maybe we can add a debugfs files to monitor how much page cache flag transition we are doing. Cheers, Jerome > > > > Signed-off-by: Luca Barbieri > > --- > > =A0drivers/gpu/drm/ttm/ttm_bo.c | =A0 18 ++++++++++++++---- > > =A01 files changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > > index 2920f9a..27ee1be 100644 > > --- a/drivers/gpu/drm/ttm/ttm_bo.c > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > > @@ -876,6 +876,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, > > > > =A0 =A0 =A0 =A0mem->mm_node =3D NULL; > > =A0 =A0 =A0 =A0for (i =3D 0; i < placement->num_placement; ++i) { > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 int is_tt_move; > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ret =3D ttm_mem_type_from_flags(placemen= t->placement[i], > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0&mem_type); > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (ret) > > @@ -891,8 +892,12 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!type_ok) > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0continue; > > > > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 cur_flags =3D ttm_bo_select_caching(man, = bo->mem.placement, > > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 cur_flags); > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 is_tt_move =3D !(man->flags & TTM_MEMTYPE= _FLAG_FIXED) && > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 !(bdev->man[bo->mem.mem_t= ype].flags & TTM_MEMTYPE_FLAG_FIXED); > > + > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Only try to keep the current flags if = we are in the same memory space */ > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 cur_flags =3D ttm_bo_select_caching(man, = is_tt_move ? bo->mem.placement : 0, cur_flags); > > + > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * Use the access and other non-mapping-= related flag bits from > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * the memory placement flags to the cur= rent flags > > @@ -927,6 +932,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EINVAL; > > > > =A0 =A0 =A0 =A0for (i =3D 0; i < placement->num_busy_placement; ++i) { > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 int is_tt_move; > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ret =3D ttm_mem_type_from_flags(placemen= t->busy_placement[i], > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0&mem_type); > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (ret) > > @@ -941,8 +947,12 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo, > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0&cur_flags)) > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0continue; > > > > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 cur_flags =3D ttm_bo_select_caching(man, = bo->mem.placement, > > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 cur_flags); > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 is_tt_move =3D !(man->flags & TTM_MEMTYPE= _FLAG_FIXED) && > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 !(bdev->man[bo->mem.mem_t= ype].flags & TTM_MEMTYPE_FLAG_FIXED); > > + > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Only try to keep the current flags if = we are in the same memory space */ > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 cur_flags =3D ttm_bo_select_caching(man, = is_tt_move ? bo->mem.placement : 0, cur_flags); > > + > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * Use the access and other non-mapping-= related flag bits from > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * the memory placement flags to the cur= rent flags > > -- > > 1.6.6.1.476.g01ddb > > > > > > -----------------------------------------------------------------------= ------- > > The Planet: dedicated and managed hosting, cloud storage, colocation > > Stay online with enterprise data centers and the best network in the bu= siness > > Choose flexible plans and management services without long-term contrac= ts > > Personal 24x7 support from experience hosting pros just a phone call aw= ay. > > http://p.sf.net/sfu/theplanet-com > > -- > > _______________________________________________ > > Dri-devel mailing list > > Dri-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/dri-devel > > > = > -------------------------------------------------------------------------= ----- > The Planet: dedicated and managed hosting, cloud storage, colocation > Stay online with enterprise data centers and the best network in the busi= ness > Choose flexible plans and management services without long-term contracts > Personal 24x7 support from experience hosting pros just a phone call away. > http://p.sf.net/sfu/theplanet-com > -- > _______________________________________________ > Dri-devel mailing list > Dri-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/dri-devel > = ---------------------------------------------------------------------------= --- The Planet: dedicated and managed hosting, cloud storage, colocation Stay online with enterprise data centers and the best network in the busine= ss Choose flexible plans and management services without long-term contracts Personal 24x7 support from experience hosting pros just a phone call away. http://p.sf.net/sfu/theplanet-com --