| Message ID | 20180227115000.4105-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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). -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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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@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. 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@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
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@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@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
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. */ 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;
Most of the time we only need the dma addresses. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/drm_prime.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)