diff mbox series

drm: Remove SCATTERLIST_MAX_SEGMENT

Message ID 0-v1-44733fccd781+13d-rm_scatterlist_max_jgg@nvidia.com
State New, archived
Headers show
Series drm: Remove SCATTERLIST_MAX_SEGMENT | expand

Commit Message

Jason Gunthorpe Oct. 28, 2020, 7:15 p.m. UTC
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(-)

Comments

Jason Gunthorpe Oct. 29, 2020, 6:20 p.m. UTC | #1
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
Daniel Vetter Oct. 29, 2020, 6:29 p.m. UTC | #2
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
Daniel Vetter Nov. 2, 2020, 2:49 p.m. UTC | #3
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 mbox series

Patch

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 },