| Message ID | 0-v1-44733fccd781+13d-rm_scatterlist_max_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Remove SCATTERLIST_MAX_SEGMENT | expand |
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; > 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 }, > -- > 2.28.0 >
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 },
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(-)