[PATCH 2/5] drm/prime: make the pages array optional for drm_prime_sg_to_page_addr_arrays

[Daniel Vetter]

On Tue, Mar 06, 2018 at 10:25:03AM +0100, Christian König wrote:
> Am 06.03.2018 um 10:21 schrieb Daniel Vetter:
> > On Tue, Feb 27, 2018 at 12:49:57PM +0100, Christian König wrote:
> > > Most of the time we only need the dma addresses.
> > > 
> > > Signed-off-by: Christian König <christian.koenig at amd.com>
> > > ---
> > >   drivers/gpu/drm/drm_prime.c | 20 ++++++++++----------
> > >   1 file changed, 10 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > > index c38dacda6119..7856a9b3f8a8 100644
> > > --- a/drivers/gpu/drm/drm_prime.c
> > > +++ b/drivers/gpu/drm/drm_prime.c
> > > @@ -922,40 +922,40 @@ EXPORT_SYMBOL(drm_prime_pages_to_sg);
> > >   /**
> > >    * drm_prime_sg_to_page_addr_arrays - convert an sg table into a page array
> > >    * @sgt: scatter-gather table to convert
> > > - * @pages: array of page pointers to store the page array in
> > > + * @pages: optional array of page pointers to store the page array in
> > >    * @addrs: optional array to store the dma bus address of each page
> > > - * @max_pages: size of both the passed-in arrays
> > > + * @max_entries: size of both the passed-in arrays
> > >    *
> > >    * Exports an sg table into an array of pages and addresses. This is currently
> > >    * required by the TTM driver in order to do correct fault handling.
> > >    */
> > Can't we just teach ttm to use sgts wherever needed, and deprecate
> > exporting dma-bufs to page arrays (which really breaks the abstraction
> > entirely and was just a quick hack to get things going that stuck around
> > for years). Last time I looked into ttm the only thing it did is convert
> > it back to sgts again (after calling dma_map once more, which the exporter
> > should have done already for you).
> 
> Thought about that as well, but the problem here isn't TTM.
> 
> We need to be able to access the SGT by an index in amdgpu to be able to
> build up the VM page tables and that is not possible because the SGT is
> potentially chained.
> 
> We could add a new sg_table access helper function to work around that
> thought.

There's some neat per-page sgt iter functions that we've build for i915.
See i915_gem_gtt.c. But yeah that's probably a pile more work, but imo
from the i915 code shuffling the end result looks fairly neat.
-Daniel
> 
> BTW: TTM isn't mapping anything in that case, we just fill in the arrays
> from the SGT.
> 
> Christian.
> 
> > -Daniel
> > 
> > >   int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,
> > > -				     dma_addr_t *addrs, int max_pages)
> > > +				     dma_addr_t *addrs, int max_entries)
> > >   {
> > >   	unsigned count;
> > >   	struct scatterlist *sg;
> > >   	struct page *page;
> > > -	u32 len;
> > > -	int pg_index;
> > > +	u32 len, index;
> > >   	dma_addr_t addr;
> > > -	pg_index = 0;
> > > +	index = 0;
> > >   	for_each_sg(sgt->sgl, sg, sgt->nents, count) {
> > >   		len = sg->length;
> > >   		page = sg_page(sg);
> > >   		addr = sg_dma_address(sg);
> > >   		while (len > 0) {
> > > -			if (WARN_ON(pg_index >= max_pages))
> > > +			if (WARN_ON(index >= max_entries))
> > >   				return -1;
> > > -			pages[pg_index] = page;
> > > +			if (pages)
> > > +				pages[index] = page;
> > >   			if (addrs)
> > > -				addrs[pg_index] = addr;
> > > +				addrs[index] = addr;
> > >   			page++;
> > >   			addr += PAGE_SIZE;
> > >   			len -= PAGE_SIZE;
> > > -			pg_index++;
> > > +			index++;
> > >   		}
> > >   	}
> > >   	return 0;
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch