From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [RFC] ttm merge ttm_backend & ttm_tt Date: Wed, 2 Nov 2011 13:09:03 -0400 Message-ID: <20111102170903.GA24601@phenom.dumpdata.com> References: <20111102150443.GA23950@phenom.dumpdata.com> <20111102164821.GF1823@homer.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from rcsinet15.oracle.com (rcsinet15.oracle.com [148.87.113.117]) by gabe.freedesktop.org (Postfix) with ESMTP id 9A68D9E75D for ; Wed, 2 Nov 2011 10:09:15 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20111102164821.GF1823@homer.localdomain> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Jerome Glisse Cc: Thomas Hellstrom , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org > > I don't know why it was done that way, but I wrote the TTM DMA code > > to be optimized for that (and it has the code to reverse direction - b/c > > the testcases I used initially had the a,b,c,d,e,f order when doing get_pages > > and put_pages) - but I can alter the code to do it in the forward fashion instead > > of the reverse fashion.. Better yet, it removes around 70 lines of code from the > > TTM DMA. > > Order in which we put them back on the list can be easily change, either > by use of add_tail or by iterating array from end to begining. i am not > sure how much this can impact things. Neither am I. I can run some perf numbers when playing tuxracer and see if there is a disadvantage/advantage. > > > Anyhow, it might be noting that in the commit description and perhaps make a bug-fix > > for the put_pages being called in the error path instead of ttm_put_pages as a seperate > > patch as you suggested. > > > > > > [PATCH 6/6] drm/ttm: merge ttm_backend and ttm_tt > > > > That is going to be a bit tricky. You are using the pci_map_page, which should not > > be used if the pages have been allocated via the pci_alloc_coherent (they are already > > mapped). Perhaps a simple check such as: > > > > if !(ttm->be && ttm->be->func && ttm->be->func->get_pages) { > > ttm->dma_address[i] = pci_map_page(dev->pdev, ...) > > } > > Your dma change are not suppose to be on top of that, idea is to add yet > another callbacks populate+free_page which will do either pci map page > or just use your code (ie use dma alloc and store dma addr into the > array). So there is a missing piece here before adding your dma code. > I just wanted to keep ttm_tt & ttm_backend merge as simple as possible > without major change. Adding the populate + get_page callback sounded > like good candidate for another patch. > > What base should I be looking at? Is this the base that Dave has for 3.2? > > Should apply on top of linus master kernel as of yesterday. Oh.. very very *very* fresh.