Since commit 9a40401cfa13 ("lib/scatterlist: Do not limit max_segment to PAGE_ALIGNED values") the max_segment input to sg_alloc_table_from_pages() does not have to be any special value. The new algorithm will always create something less than what the user provides. Thus eliminate this confusing constant.
- vmwgfx should use the HW capability, not mix in the OS page size for calling dma_set_max_seg_size()
- i915 uses i915_sg_segment_size() both for sg_alloc_table_from_pages and for some open coded sgl construction. This doesn't change the value since rounddown(size, UINT_MAX) == SCATTERLIST_MAX_SEGMENT
- drm_prime_pages_to_sg uses it as a default if max_segment is zero, UINT_MAX is fine to use directly.
Cc: Gerd Hoffmann kraxel@redhat.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Qian Cai cai@lca.pw Cc: "Ursulin, Tvrtko" tvrtko.ursulin@intel.com Suggested-by: Christoph Hellwig hch@lst.de Signed-off-by: Jason Gunthorpe jgg@nvidia.com --- drivers/gpu/drm/drm_prime.c | 4 ++-- drivers/gpu/drm/i915/i915_scatterlist.h | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 3 +-- include/linux/scatterlist.h | 6 ------ tools/testing/scatterlist/main.c | 2 +- 5 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index d6808f678db541..c3693e5e8b74b0 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -816,8 +816,8 @@ struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev,
if (dev) max_segment = dma_max_mapping_size(dev->dev); - if (max_segment == 0 || max_segment > SCATTERLIST_MAX_SEGMENT) - max_segment = SCATTERLIST_MAX_SEGMENT; + if (max_segment == 0) + max_segment = UINT_MAX; sge = __sg_alloc_table_from_pages(sg, pages, nr_pages, 0, nr_pages << PAGE_SHIFT, max_segment, diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h b/drivers/gpu/drm/i915/i915_scatterlist.h index b7b59328cb76ab..883dd8d09d6bf2 100644 --- a/drivers/gpu/drm/i915/i915_scatterlist.h +++ b/drivers/gpu/drm/i915/i915_scatterlist.h @@ -112,7 +112,7 @@ static inline unsigned int i915_sg_segment_size(void) unsigned int size = swiotlb_max_segment();
if (size == 0) - return SCATTERLIST_MAX_SEGMENT; + size = UINT_MAX;
size = rounddown(size, PAGE_SIZE); /* swiotlb_max_segment_size can return 1 byte when it means one page. */ diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 31e3e5c9f36223..c1817f1a3006e0 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -792,8 +792,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) if (unlikely(ret != 0)) goto out_err0;
- dma_set_max_seg_size(dev->dev, min_t(unsigned int, U32_MAX & PAGE_MASK, - SCATTERLIST_MAX_SEGMENT)); + dma_set_max_seg_size(dev->dev, U32_MAX);
if (dev_priv->capabilities & SVGA_CAP_GMR2) { DRM_INFO("Max GMR ids is %u\n", diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 36c47e7e66a203..6f70572b2938be 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -18,12 +18,6 @@ struct scatterlist { #endif };
-/* - * Since the above length field is an unsigned int, below we define the maximum - * length in bytes that can be stored in one scatterlist entry. - */ -#define SCATTERLIST_MAX_SEGMENT (UINT_MAX & PAGE_MASK) - /* * These macros should be used after a dma_map_sg call has been done * to get bus addresses of each of the SG entries and their lengths. diff --git a/tools/testing/scatterlist/main.c b/tools/testing/scatterlist/main.c index b2c7e9f7b8d3dc..d264bf853034bd 100644 --- a/tools/testing/scatterlist/main.c +++ b/tools/testing/scatterlist/main.c @@ -50,7 +50,7 @@ static void fail(struct test *test, struct sg_table *st, const char *cond)
int main(void) { - const unsigned int sgmax = SCATTERLIST_MAX_SEGMENT; + const unsigned int sgmax = UINT_MAX; struct test *test, tests[] = { { -EINVAL, 1, pfn(0), PAGE_SIZE, PAGE_SIZE + 1, 1 }, { -EINVAL, 1, pfn(0), PAGE_SIZE, 0, 1 },
On Wed, Oct 28, 2020 at 04:15:26PM -0300, Jason Gunthorpe wrote:
Since commit 9a40401cfa13 ("lib/scatterlist: Do not limit max_segment to PAGE_ALIGNED values") the max_segment input to sg_alloc_table_from_pages() does not have to be any special value. The new algorithm will always create something less than what the user provides. Thus eliminate this confusing constant.
vmwgfx should use the HW capability, not mix in the OS page size for calling dma_set_max_seg_size()
i915 uses i915_sg_segment_size() both for sg_alloc_table_from_pages and for some open coded sgl construction. This doesn't change the value since rounddown(size, UINT_MAX) == SCATTERLIST_MAX_SEGMENT
drm_prime_pages_to_sg uses it as a default if max_segment is zero, UINT_MAX is fine to use directly.
Cc: Gerd Hoffmann kraxel@redhat.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Qian Cai cai@lca.pw Cc: "Ursulin, Tvrtko" tvrtko.ursulin@intel.com Suggested-by: Christoph Hellwig hch@lst.de Signed-off-by: Jason Gunthorpe jgg@nvidia.com
lgtm. Do you want to push this through some other queue, or should I put this into drm trees? Prefer 5.10 or 5.11?
If you want to merge this Acked-by: Daniel Vetter daniel.vetter@ffwll.ch -Daniel
drivers/gpu/drm/drm_prime.c | 4 ++-- drivers/gpu/drm/i915/i915_scatterlist.h | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 3 +-- include/linux/scatterlist.h | 6 ------ tools/testing/scatterlist/main.c | 2 +- 5 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index d6808f678db541..c3693e5e8b74b0 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -816,8 +816,8 @@ struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev,
if (dev) max_segment = dma_max_mapping_size(dev->dev);
- if (max_segment == 0 || max_segment > SCATTERLIST_MAX_SEGMENT)
max_segment = SCATTERLIST_MAX_SEGMENT;
- if (max_segment == 0)
max_segment = UINT_MAX;diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h b/drivers/gpu/drm/i915/i915_scatterlist.h index b7b59328cb76ab..883dd8d09d6bf2 100644 --- a/drivers/gpu/drm/i915/i915_scatterlist.h +++ b/drivers/gpu/drm/i915/i915_scatterlist.h @@ -112,7 +112,7 @@ static inline unsigned int i915_sg_segment_size(void) unsigned int size = swiotlb_max_segment();
if (size == 0)
return SCATTERLIST_MAX_SEGMENT;
size = UINT_MAX;size = rounddown(size, PAGE_SIZE); /* swiotlb_max_segment_size can return 1 byte when it means one page. */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 31e3e5c9f36223..c1817f1a3006e0 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -792,8 +792,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) if (unlikely(ret != 0)) goto out_err0;
- dma_set_max_seg_size(dev->dev, min_t(unsigned int, U32_MAX & PAGE_MASK,
SCATTERLIST_MAX_SEGMENT));
dma_set_max_seg_size(dev->dev, U32_MAX);
if (dev_priv->capabilities & SVGA_CAP_GMR2) { DRM_INFO("Max GMR ids is %u\n",
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 36c47e7e66a203..6f70572b2938be 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -18,12 +18,6 @@ struct scatterlist { #endif };
-/*
- Since the above length field is an unsigned int, below we define the maximum
- length in bytes that can be stored in one scatterlist entry.
- */
-#define SCATTERLIST_MAX_SEGMENT (UINT_MAX & PAGE_MASK)
/*
- These macros should be used after a dma_map_sg call has been done
- to get bus addresses of each of the SG entries and their lengths.
diff --git a/tools/testing/scatterlist/main.c b/tools/testing/scatterlist/main.c index b2c7e9f7b8d3dc..d264bf853034bd 100644 --- a/tools/testing/scatterlist/main.c +++ b/tools/testing/scatterlist/main.c @@ -50,7 +50,7 @@ static void fail(struct test *test, struct sg_table *st, const char *cond)
int main(void) {
- const unsigned int sgmax = SCATTERLIST_MAX_SEGMENT;
- const unsigned int sgmax = UINT_MAX; struct test *test, tests[] = { { -EINVAL, 1, pfn(0), PAGE_SIZE, PAGE_SIZE + 1, 1 }, { -EINVAL, 1, pfn(0), PAGE_SIZE, 0, 1 },
-- 2.28.0
On Wed, Oct 28, 2020 at 08:49:11PM +0100, Daniel Vetter wrote:
On Wed, Oct 28, 2020 at 04:15:26PM -0300, Jason Gunthorpe wrote:
Since commit 9a40401cfa13 ("lib/scatterlist: Do not limit max_segment to PAGE_ALIGNED values") the max_segment input to sg_alloc_table_from_pages() does not have to be any special value. The new algorithm will always create something less than what the user provides. Thus eliminate this confusing constant.
vmwgfx should use the HW capability, not mix in the OS page size for calling dma_set_max_seg_size()
i915 uses i915_sg_segment_size() both for sg_alloc_table_from_pages and for some open coded sgl construction. This doesn't change the value since rounddown(size, UINT_MAX) == SCATTERLIST_MAX_SEGMENT
drm_prime_pages_to_sg uses it as a default if max_segment is zero, UINT_MAX is fine to use directly.
Cc: Gerd Hoffmann kraxel@redhat.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Qian Cai cai@lca.pw Cc: "Ursulin, Tvrtko" tvrtko.ursulin@intel.com Suggested-by: Christoph Hellwig hch@lst.de Signed-off-by: Jason Gunthorpe jgg@nvidia.com
lgtm. Do you want to push this through some other queue, or should I put this into drm trees? Prefer 5.10 or 5.11?
I think DRM tree is best
Thanks, Jason
On Thu, Oct 29, 2020 at 7:20 PM Jason Gunthorpe jgg@nvidia.com wrote:
On Wed, Oct 28, 2020 at 08:49:11PM +0100, Daniel Vetter wrote:
On Wed, Oct 28, 2020 at 04:15:26PM -0300, Jason Gunthorpe wrote:
Since commit 9a40401cfa13 ("lib/scatterlist: Do not limit max_segment to PAGE_ALIGNED values") the max_segment input to sg_alloc_table_from_pages() does not have to be any special value. The new algorithm will always create something less than what the user provides. Thus eliminate this confusing constant.
vmwgfx should use the HW capability, not mix in the OS page size for calling dma_set_max_seg_size()
i915 uses i915_sg_segment_size() both for sg_alloc_table_from_pages and for some open coded sgl construction. This doesn't change the value since rounddown(size, UINT_MAX) == SCATTERLIST_MAX_SEGMENT
drm_prime_pages_to_sg uses it as a default if max_segment is zero, UINT_MAX is fine to use directly.
Cc: Gerd Hoffmann kraxel@redhat.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Qian Cai cai@lca.pw Cc: "Ursulin, Tvrtko" tvrtko.ursulin@intel.com Suggested-by: Christoph Hellwig hch@lst.de Signed-off-by: Jason Gunthorpe jgg@nvidia.com
lgtm. Do you want to push this through some other queue, or should I put this into drm trees? Prefer 5.10 or 5.11?
I think DRM tree is best
Ok, I'll try to remember and apply this to -next after -rc2. -rc1 is supremely busted for us, I want to wait with pulling the merge window into the -next pile until that's settled. Please ping if your patch isn't in linux-next within a week in case I forget. -Daniel
On Wed, Oct 28, 2020 at 04:15:26PM -0300, Jason Gunthorpe wrote:
Since commit 9a40401cfa13 ("lib/scatterlist: Do not limit max_segment to PAGE_ALIGNED values") the max_segment input to sg_alloc_table_from_pages() does not have to be any special value. The new algorithm will always create something less than what the user provides. Thus eliminate this confusing constant.
vmwgfx should use the HW capability, not mix in the OS page size for calling dma_set_max_seg_size()
i915 uses i915_sg_segment_size() both for sg_alloc_table_from_pages and for some open coded sgl construction. This doesn't change the value since rounddown(size, UINT_MAX) == SCATTERLIST_MAX_SEGMENT
drm_prime_pages_to_sg uses it as a default if max_segment is zero, UINT_MAX is fine to use directly.
Cc: Gerd Hoffmann kraxel@redhat.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Thomas Hellstrom thellstrom@vmware.com Cc: Qian Cai cai@lca.pw Cc: "Ursulin, Tvrtko" tvrtko.ursulin@intel.com Suggested-by: Christoph Hellwig hch@lst.de Signed-off-by: Jason Gunthorpe jgg@nvidia.com
Merged to drm-misc-next, thanks for your patch. -Daniel
drivers/gpu/drm/drm_prime.c | 4 ++-- drivers/gpu/drm/i915/i915_scatterlist.h | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 3 +-- include/linux/scatterlist.h | 6 ------ tools/testing/scatterlist/main.c | 2 +- 5 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index d6808f678db541..c3693e5e8b74b0 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -816,8 +816,8 @@ struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev,
if (dev) max_segment = dma_max_mapping_size(dev->dev);
- if (max_segment == 0 || max_segment > SCATTERLIST_MAX_SEGMENT)
max_segment = SCATTERLIST_MAX_SEGMENT;
- if (max_segment == 0)
max_segment = UINT_MAX;diff --git a/drivers/gpu/drm/i915/i915_scatterlist.h b/drivers/gpu/drm/i915/i915_scatterlist.h index b7b59328cb76ab..883dd8d09d6bf2 100644 --- a/drivers/gpu/drm/i915/i915_scatterlist.h +++ b/drivers/gpu/drm/i915/i915_scatterlist.h @@ -112,7 +112,7 @@ static inline unsigned int i915_sg_segment_size(void) unsigned int size = swiotlb_max_segment();
if (size == 0)
return SCATTERLIST_MAX_SEGMENT;
size = UINT_MAX;size = rounddown(size, PAGE_SIZE); /* swiotlb_max_segment_size can return 1 byte when it means one page. */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 31e3e5c9f36223..c1817f1a3006e0 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -792,8 +792,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset) if (unlikely(ret != 0)) goto out_err0;
- dma_set_max_seg_size(dev->dev, min_t(unsigned int, U32_MAX & PAGE_MASK,
SCATTERLIST_MAX_SEGMENT));
dma_set_max_seg_size(dev->dev, U32_MAX);
if (dev_priv->capabilities & SVGA_CAP_GMR2) { DRM_INFO("Max GMR ids is %u\n",
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 36c47e7e66a203..6f70572b2938be 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -18,12 +18,6 @@ struct scatterlist { #endif };
-/*
- Since the above length field is an unsigned int, below we define the maximum
- length in bytes that can be stored in one scatterlist entry.
- */
-#define SCATTERLIST_MAX_SEGMENT (UINT_MAX & PAGE_MASK)
/*
- These macros should be used after a dma_map_sg call has been done
- to get bus addresses of each of the SG entries and their lengths.
diff --git a/tools/testing/scatterlist/main.c b/tools/testing/scatterlist/main.c index b2c7e9f7b8d3dc..d264bf853034bd 100644 --- a/tools/testing/scatterlist/main.c +++ b/tools/testing/scatterlist/main.c @@ -50,7 +50,7 @@ static void fail(struct test *test, struct sg_table *st, const char *cond)
int main(void) {
- const unsigned int sgmax = SCATTERLIST_MAX_SEGMENT;
- const unsigned int sgmax = UINT_MAX; struct test *test, tests[] = { { -EINVAL, 1, pfn(0), PAGE_SIZE, PAGE_SIZE + 1, 1 }, { -EINVAL, 1, pfn(0), PAGE_SIZE, 0, 1 },
-- 2.28.0
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Jason Gunthorpe