[PATCH v2 1/2] drm/ttm: Change ttm_device_init to use a struct instead of multiple bools

Thomas Hellström thomas.hellstrom at linux.intel.com
Wed Oct 9 05:56:18 UTC 2024


On Sat, 2024-10-05 at 04:14 +0000, Matthew Brost wrote:
> On Fri, Oct 04, 2024 at 04:28:29PM +0200, Thomas Hellström wrote:
> > On Wed, 2024-10-02 at 14:54 +0200, Thomas Hellström wrote:
> > > On Wed, 2024-10-02 at 14:45 +0200, Christian König wrote:
> > > > Am 02.10.24 um 14:24 schrieb Thomas Hellström:
> > > > > The ttm_device_init funcition uses multiple bool arguments.
> > > > > That
> > > > > means
> > > > > readability in the caller becomes poor, and all callers need
> > > > > to
> > > > > change if
> > > > > yet another bool is added.
> > > > > 
> > > > > Instead use a struct with multiple single-bit flags. This
> > > > > addresses
> > > > > both
> > > > > problems. Prefer it over using defines or enums with explicit
> > > > > bit
> > > > > shifts,
> > > > > since converting to and from these bit values uses logical
> > > > > operations or
> > > > > tests which are implicit with the struct usage, and ofc type-
> > > > > checking.
> > > > 
> > > > Ok, that style is new to me. I've mostly seen defined parameter
> > > > flags
> > > > in 
> > > > the kernel.
> > > > 
> > > > It obviously has some advantages, but do we have any precedence
> > > > in
> > > > the 
> > > > kernel for using that approach?
> 
> I agree multiple bool arguments are not a great idea. I spent 4-5
> hours
> a few weeks ago chasing bug in Xe where I transposed bool arguments.
> Complete waste of time likely could've been avoided with something
> like
> this.
> 
> Also agree this makes code much more readable at the caller.
> 
> FWIW plan on using style for bool arguments in gpusvm.
> 
> I do have a couple of nits with the style below.
> 
> > > 
> > > I don't know TBH, but I can take a look.
> > 
> > So it seems  parse_events__modifier_event() uses something similar
> > 
> > grep u32 `find . -name '*.h'` | grep ': 1' or
> > grep bool `find . -name '*.h'` | grep ': 1'
> > 
> > reveals a huge number of lines but most of it looks like it's not
> > used
> > as function parameters.
> > 
> > I'll ping Dave and Sima on IRC (I guess they're the ones to receive
> > most blame from Linus if it's deemed not good).
> > 
> > /Thomas
> > 
> > 
> > > 
> > > I've used the same style in some of the shrinker series patches,
> > > since
> > > using multiple bools in the prototype severely reduces
> > > readability
> > > and
> > > mixing the resulting flag defines without type-checking may
> > > introduce
> > > hard-to find bugs.
> > > 
> > > /Thomas
> > > 
> > > 
> > > 
> > > > 
> > > > Regards,
> > > > Christian.
> > > > 
> > > > > 
> > > > > This is in preparation of adding yet another bool flag
> > > > > parameter
> > > > > to
> > > > > the
> > > > > function.
> > > > > 
> > > > > Cc: Christian König <christian.koenig at amd.com>
> > > > > Cc: amd-gfx at lists.freedesktop.org
> > > > > Cc: intel-gfx at lists.freedesktop.org
> > > > > Cc: nouveau at lists.freedesktop.org
> > > > > Cc: spice-devel at lists.freedesktop.org
> > > > > Cc: dri-devel at lists.freedesktop.org
> > > > > Cc: Zack Rusin <zack.rusin at broadcom.com>
> > > > > Cc: <bcm-kernel-feedback-list at broadcom.com>
> > > > > Cc: Sui Jingfeng <suijingfeng at loongson.cn>
> > > > > Cc: <intel-xe at lists.freedesktop.org>
> > > > > Signed-off-by: Thomas Hellström
> > > > > <thomas.hellstrom at linux.intel.com>
> > > > > ---
> > > > >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c       |  6 ++++--
> > > > >   drivers/gpu/drm/drm_gem_vram_helper.c         |  7 ++++---
> > > > >   drivers/gpu/drm/i915/intel_region_ttm.c       |  3 ++-
> > > > >   drivers/gpu/drm/loongson/lsdc_ttm.c           |  5 ++++-
> > > > >   drivers/gpu/drm/nouveau/nouveau_ttm.c         |  7 +++++--
> > > > >   drivers/gpu/drm/qxl/qxl_ttm.c                 |  2 +-
> > > > >   drivers/gpu/drm/radeon/radeon_ttm.c           |  6 ++++--
> > > > >   drivers/gpu/drm/ttm/tests/ttm_bo_test.c       | 16 +++++++-
> > > > > ----
> > > > > --
> > > > > -
> > > > >   .../gpu/drm/ttm/tests/ttm_bo_validate_test.c  |  3 ++-
> > > > >   drivers/gpu/drm/ttm/tests/ttm_device_test.c   | 16
> > > > > ++++++++----
> > > > > --
> > > > > -
> > > > >   drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 20
> > > > > ++++++++----
> > > > > --
> > > > > -----
> > > > >   drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h |  6 ++----
> > > > >   drivers/gpu/drm/ttm/ttm_device.c              |  7 +++----
> > > > >   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c           |  4 ++--
> > > > >   drivers/gpu/drm/xe/xe_device.c                |  3 ++-
> > > > >   include/drm/ttm/ttm_device.h                  | 12
> > > > > ++++++++++-
> > > > >   16 files changed, 71 insertions(+), 52 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > > > index 74adb983ab03..e43635ac54fd 100644
> > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > > > @@ -1853,8 +1853,10 @@ int amdgpu_ttm_init(struct
> > > > > amdgpu_device
> > > > > *adev)
> > > > >   	r = ttm_device_init(&adev->mman.bdev,
> > > > > &amdgpu_bo_driver,
> > > > > adev->dev,
> > > > >   			       adev_to_drm(adev)-
> > > > > >anon_inode-
> > > > > > i_mapping,
> > > > >   			       adev_to_drm(adev)-
> > > > > > vma_offset_manager,
> > > > > -			       adev->need_swiotlb,
> > > > > -			       dma_addressing_limited(adev-
> > > > > > dev));
> > > > > +			       (struct
> > > > > ttm_device_init_flags){
> > > > > +				       .use_dma_alloc =
> > > > > adev-
> > > > > > need_swiotlb,
> > > > > +				       .use_dma32 =
> > > > > dma_addressing_limited(adev->dev)
> > > > > +			       });
> 
> I can't say I like declaring the ttm_device_init_flags as part of the
> function arguments. I'd prefer this a local variable but this just an
> opinion. Not blocker but if other agree with me, maybe change this.

I think it's merely providing an initializer. The generated code (at
least with -O2) will be more or less the identical to if a 'u32 flags'
argument with defines or an enum was used instead of the struct.

So basically it boils down to whether the c code function call becomes
_too_ noisy, and I think that's a matter of taste.

/Thomas


> 
> Matt
> 
> > > > >   	if (r) {
> > > > >   		DRM_ERROR("failed initializing buffer object
> > > > > driver(%d).\n", r);
> > > > >   		return r;
> > > > > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
> > > > > b/drivers/gpu/drm/drm_gem_vram_helper.c
> > > > > index 22b1fe9c03b8..7c3165b00378 100644
> > > > > --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> > > > > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> > > > > @@ -931,9 +931,10 @@ static int drm_vram_mm_init(struct
> > > > > drm_vram_mm
> > > > > *vmm, struct drm_device *dev,
> > > > >   	vmm->vram_size = vram_size;
> > > > >   
> > > > >   	ret = ttm_device_init(&vmm->bdev, &bo_driver, dev-
> > > > > >dev,
> > > > > -				 dev->anon_inode->i_mapping,
> > > > > -				 dev->vma_offset_manager,
> > > > > -				 false, true);
> > > > > +			      dev->anon_inode->i_mapping,
> > > > > +			      dev->vma_offset_manager,
> > > > > +			      (struct ttm_device_init_flags)
> > > > > +			      {.use_dma32 = true});
> > > > >   	if (ret)
> > > > >   		return ret;
> > > > >   
> > > > > diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c
> > > > > b/drivers/gpu/drm/i915/intel_region_ttm.c
> > > > > index 04525d92bec5..db34da63814c 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_region_ttm.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_region_ttm.c
> > > > > @@ -34,7 +34,8 @@ int intel_region_ttm_device_init(struct
> > > > > drm_i915_private *dev_priv)
> > > > >   
> > > > >   	return ttm_device_init(&dev_priv->bdev,
> > > > > i915_ttm_driver(),
> > > > >   			       drm->dev, drm->anon_inode-
> > > > > > i_mapping,
> > > > > -			       drm->vma_offset_manager,
> > > > > false,
> > > > > false);
> > > > > +			       drm->vma_offset_manager,
> > > > > +			       (struct
> > > > > ttm_device_init_flags){});
> > > > >   }
> > > > >   
> > > > >   /**
> > > > > diff --git a/drivers/gpu/drm/loongson/lsdc_ttm.c
> > > > > b/drivers/gpu/drm/loongson/lsdc_ttm.c
> > > > > index 2e42c6970c9f..c684f1636f3f 100644
> > > > > --- a/drivers/gpu/drm/loongson/lsdc_ttm.c
> > > > > +++ b/drivers/gpu/drm/loongson/lsdc_ttm.c
> > > > > @@ -544,7 +544,10 @@ int lsdc_ttm_init(struct lsdc_device
> > > > > *ldev)
> > > > >   
> > > > >   	ret = ttm_device_init(&ldev->bdev, &lsdc_bo_driver,
> > > > > ddev-
> > > > > > dev,
> > > > >   			      ddev->anon_inode->i_mapping,
> > > > > -			      ddev->vma_offset_manager,
> > > > > false,
> > > > > true);
> > > > > +			      ddev->vma_offset_manager,
> > > > > +			      (struct
> > > > > ttm_device_init_flags){
> > > > > +				      .use_dma32 = true
> > > > > +			      });
> > > > >   	if (ret)
> > > > >   		return ret;
> > > > >   
> > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c
> > > > > b/drivers/gpu/drm/nouveau/nouveau_ttm.c
> > > > > index e244927eb5d4..5f89d2b40425 100644
> > > > > --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
> > > > > +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
> > > > > @@ -302,8 +302,11 @@ nouveau_ttm_init(struct nouveau_drm
> > > > > *drm)
> > > > >   	ret = ttm_device_init(&drm->ttm.bdev,
> > > > > &nouveau_bo_driver,
> > > > > drm->dev->dev,
> > > > >   				  dev->anon_inode-
> > > > > >i_mapping,
> > > > >   				  dev->vma_offset_manager,
> > > > > -				  drm_need_swiotlb(drm-
> > > > > > client.mmu.dmabits),
> > > > > -				  drm->client.mmu.dmabits <=
> > > > > 32);
> > > > > +				  (struct
> > > > > ttm_device_init_flags){
> > > > > +					  .use_dma_alloc =
> > > > > +					 
> > > > > drm_need_swiotlb(drm-
> > > > > > client.mmu.dmabits),
> > > > > +					  .use_dma32 = (drm-
> > > > > > client.mmu.dmabits <= 32)
> > > > > +				  });
> > > > >   	if (ret) {
> > > > >   		NV_ERROR(drm, "error initialising bo driver,
> > > > > %d\n", ret);
> > > > >   		return ret;
> > > > > diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c
> > > > > b/drivers/gpu/drm/qxl/qxl_ttm.c
> > > > > index 765a144cea14..ced4c60c4650 100644
> > > > > --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> > > > > +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> > > > > @@ -196,7 +196,7 @@ int qxl_ttm_init(struct qxl_device *qdev)
> > > > >   	r = ttm_device_init(&qdev->mman.bdev,
> > > > > &qxl_bo_driver,
> > > > > NULL,
> > > > >   			    qdev->ddev.anon_inode-
> > > > > >i_mapping,
> > > > >   			    qdev->ddev.vma_offset_manager,
> > > > > -			    false, false);
> > > > > +			    (struct
> > > > > ttm_device_init_flags){});
> > > > >   	if (r) {
> > > > >   		DRM_ERROR("failed initializing buffer object
> > > > > driver(%d).\n", r);
> > > > >   		return r;
> > > > > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
> > > > > b/drivers/gpu/drm/radeon/radeon_ttm.c
> > > > > index 69d0c12fa419..2915473e4956 100644
> > > > > --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> > > > > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> > > > > @@ -684,8 +684,10 @@ int radeon_ttm_init(struct radeon_device
> > > > > *rdev)
> > > > >   	r = ttm_device_init(&rdev->mman.bdev,
> > > > > &radeon_bo_driver,
> > > > > rdev->dev,
> > > > >   			       rdev_to_drm(rdev)-
> > > > > >anon_inode-
> > > > > > i_mapping,
> > > > >   			       rdev_to_drm(rdev)-
> > > > > > vma_offset_manager,
> > > > > -			       rdev->need_swiotlb,
> > > > > -			       dma_addressing_limited(&rdev-
> > > > > > pdev-
> > > > > > dev));
> > > > > +			       (struct
> > > > > ttm_device_init_flags){
> > > > > +					.use_dma_alloc =
> > > > > rdev-
> > > > > > need_swiotlb,
> > > > > +					.use_dma32 =
> > > > > dma_addressing_limited(&rdev->pdev->dev)
> > > > > +				});
> > > > >   	if (r) {
> > > > >   		DRM_ERROR("failed initializing buffer object
> > > > > driver(%d).\n", r);
> > > > >   		return r;
> > > > > diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> > > > > b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> > > > > index f0a7eb62116c..8a2ab90c6e8f 100644
> > > > > --- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> > > > > +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> > > > > @@ -251,7 +251,7 @@ static void ttm_bo_unreserve_basic(struct
> > > > > kunit
> > > > > *test)
> > > > >   	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   	priv->ttm_dev = ttm_dev;
> > > > >   
> > > > > @@ -290,7 +290,7 @@ static void
> > > > > ttm_bo_unreserve_pinned(struct
> > > > > kunit *test)
> > > > >   	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   	priv->ttm_dev = ttm_dev;
> > > > >   
> > > > > @@ -342,7 +342,7 @@ static void ttm_bo_unreserve_bulk(struct
> > > > > kunit
> > > > > *test)
> > > > >   	resv = kunit_kzalloc(test, sizeof(*resv),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   	priv->ttm_dev = ttm_dev;
> > > > >   
> > > > > @@ -394,7 +394,7 @@ static void ttm_bo_put_basic(struct kunit
> > > > > *test)
> > > > >   	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   	priv->ttm_dev = ttm_dev;
> > > > >   
> > > > > @@ -437,7 +437,7 @@ static void ttm_bo_put_shared_resv(struct
> > > > > kunit
> > > > > *test)
> > > > >   	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   	priv->ttm_dev = ttm_dev;
> > > > >   
> > > > > @@ -477,7 +477,7 @@ static void ttm_bo_pin_basic(struct kunit
> > > > > *test)
> > > > >   	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   	priv->ttm_dev = ttm_dev;
> > > > >   
> > > > > @@ -512,7 +512,7 @@ static void
> > > > > ttm_bo_pin_unpin_resource(struct
> > > > > kunit *test)
> > > > >   	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   	priv->ttm_dev = ttm_dev;
> > > > >   
> > > > > @@ -563,7 +563,7 @@ static void
> > > > > ttm_bo_multiple_pin_one_unpin(struct kunit *test)
> > > > >   	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   	priv->ttm_dev = ttm_dev;
> > > > >   
> > > > > diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> > > > > b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> > > > > index 1adf18481ea0..c69b9707e6d2 100644
> > > > > --- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> > > > > +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> > > > > @@ -1044,7 +1044,8 @@ static void
> > > > > ttm_bo_validate_busy_domain_evict(struct kunit *test)
> > > > >   	 */
> > > > >   	ttm_device_fini(priv->ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init_bad_evict(test->priv,
> > > > > priv-
> > > > > > ttm_dev, false, false);
> > > > > +	err = ttm_device_kunit_init_bad_evict(test->priv,
> > > > > priv-
> > > > > > ttm_dev,
> > > > > +					      (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   
> > > > >   	ttm_mock_manager_init(priv->ttm_dev, mem_type,
> > > > > MANAGER_SIZE);
> > > > > diff --git a/drivers/gpu/drm/ttm/tests/ttm_device_test.c
> > > > > b/drivers/gpu/drm/ttm/tests/ttm_device_test.c
> > > > > index 1621903818e5..9415c7b76e3b 100644
> > > > > --- a/drivers/gpu/drm/ttm/tests/ttm_device_test.c
> > > > > +++ b/drivers/gpu/drm/ttm/tests/ttm_device_test.c
> > > > > @@ -25,7 +25,7 @@ static void ttm_device_init_basic(struct
> > > > > kunit
> > > > > *test)
> > > > >   	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   
> > > > >   	KUNIT_EXPECT_PTR_EQ(test, ttm_dev->funcs,
> > > > > &ttm_dev_funcs);
> > > > > @@ -55,7 +55,8 @@ static void ttm_device_init_multiple(struct
> > > > > kunit
> > > > > *test)
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_devs);
> > > > >   
> > > > >   	for (i = 0; i < num_dev; i++) {
> > > > > -		err = ttm_device_kunit_init(priv,
> > > > > &ttm_devs[i],
> > > > > false, false);
> > > > > +		err = ttm_device_kunit_init(priv,
> > > > > &ttm_devs[i],
> > > > > +					    (struct
> > > > > ttm_device_init_flags){});
> > > > >   		KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   
> > > > >   		KUNIT_EXPECT_PTR_EQ(test,
> > > > > ttm_devs[i].dev_mapping,
> > > > > @@ -81,7 +82,7 @@ static void ttm_device_fini_basic(struct
> > > > > kunit
> > > > > *test)
> > > > >   	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   
> > > > >   	man = ttm_manager_type(ttm_dev, TTM_PL_SYSTEM);
> > > > > @@ -109,7 +110,7 @@ static void
> > > > > ttm_device_init_no_vma_man(struct
> > > > > kunit *test)
> > > > >   	vma_man = drm->vma_offset_manager;
> > > > >   	drm->vma_offset_manager = NULL;
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_EXPECT_EQ(test, err, -EINVAL);
> > > > >   
> > > > >   	/* Bring the manager back for a graceful cleanup */
> > > > > @@ -162,9 +163,10 @@ static void ttm_device_init_pools(struct
> > > > > kunit
> > > > > *test)
> > > > >   	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(priv, ttm_dev,
> > > > > -				    params->use_dma_alloc,
> > > > > -				    params->use_dma32);
> > > > > +	err = ttm_device_kunit_init(priv, ttm_dev, (struct
> > > > > ttm_device_init_flags){
> > > > > +			.use_dma_alloc = params-
> > > > > >use_dma_alloc,
> > > > > +			.use_dma32 = params->use_dma32
> > > > > +		});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   
> > > > >   	pool = &ttm_dev->pool;
> > > > > diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > index b91c13f46225..24706fabb1c9 100644
> > > > > --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
> > > > > @@ -114,8 +114,7 @@ static void bad_evict_flags(struct
> > > > > ttm_buffer_object *bo,
> > > > >   
> > > > >   static int ttm_device_kunit_init_with_funcs(struct
> > > > > ttm_test_devices *priv,
> > > > >   					    struct
> > > > > ttm_device
> > > > > *ttm,
> > > > > -					    bool
> > > > > use_dma_alloc,
> > > > > -					    bool use_dma32,
> > > > > +					    const struct
> > > > > ttm_device_init_flags flags,
> > > > >   					    struct
> > > > > ttm_device_funcs *funcs)
> > > > >   {
> > > > >   	struct drm_device *drm = priv->drm;
> > > > > @@ -124,7 +123,7 @@ static int
> > > > > ttm_device_kunit_init_with_funcs(struct ttm_test_devices
> > > > > *priv,
> > > > >   	err = ttm_device_init(ttm, funcs, drm->dev,
> > > > >   			      drm->anon_inode->i_mapping,
> > > > >   			      drm->vma_offset_manager,
> > > > > -			      use_dma_alloc, use_dma32);
> > > > > +			      flags);
> > > > >   
> > > > >   	return err;
> > > > >   }
> > > > > @@ -140,11 +139,9 @@ EXPORT_SYMBOL_GPL(ttm_dev_funcs);
> > > > >   
> > > > >   int ttm_device_kunit_init(struct ttm_test_devices *priv,
> > > > >   			  struct ttm_device *ttm,
> > > > > -			  bool use_dma_alloc,
> > > > > -			  bool use_dma32)
> > > > > +			  const struct ttm_device_init_flags
> > > > > flags)
> > > > >   {
> > > > > -	return ttm_device_kunit_init_with_funcs(priv, ttm,
> > > > > use_dma_alloc,
> > > > > -						use_dma32,
> > > > > &ttm_dev_funcs);
> > > > > +	return ttm_device_kunit_init_with_funcs(priv, ttm,
> > > > > flags,
> > > > > &ttm_dev_funcs);
> > > > >   }
> > > > >   EXPORT_SYMBOL_GPL(ttm_device_kunit_init);
> > > > >   
> > > > > @@ -159,11 +156,10 @@
> > > > > EXPORT_SYMBOL_GPL(ttm_dev_funcs_bad_evict);
> > > > >   
> > > > >   int ttm_device_kunit_init_bad_evict(struct ttm_test_devices
> > > > > *priv,
> > > > >   				    struct ttm_device *ttm,
> > > > > -				    bool use_dma_alloc,
> > > > > -				    bool use_dma32)
> > > > > +				    const struct
> > > > > ttm_device_init_flags flags)
> > > > >   {
> > > > > -	return ttm_device_kunit_init_with_funcs(priv, ttm,
> > > > > use_dma_alloc,
> > > > > -						use_dma32,
> > > > > &ttm_dev_funcs_bad_evict);
> > > > > +	return ttm_device_kunit_init_with_funcs(priv, ttm,
> > > > > flags,
> > > > > +						&ttm_dev_fun
> > > > > cs_b
> > > > > ad
> > > > > _evict);
> > > > >   }
> > > > >   EXPORT_SYMBOL_GPL(ttm_device_kunit_init_bad_evict);
> > > > >   
> > > > > @@ -249,7 +245,7 @@ struct ttm_test_devices
> > > > > *ttm_test_devices_all(struct kunit *test)
> > > > >   	ttm_dev = kunit_kzalloc(test, sizeof(*ttm_dev),
> > > > > GFP_KERNEL);
> > > > >   	KUNIT_ASSERT_NOT_NULL(test, ttm_dev);
> > > > >   
> > > > > -	err = ttm_device_kunit_init(devs, ttm_dev, false,
> > > > > false);
> > > > > +	err = ttm_device_kunit_init(devs, ttm_dev, (struct
> > > > > ttm_device_init_flags){});
> > > > >   	KUNIT_ASSERT_EQ(test, err, 0);
> > > > >   
> > > > >   	devs->ttm_dev = ttm_dev;
> > > > > diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
> > > > > b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
> > > > > index c7da23232ffa..fbf0d3984f25 100644
> > > > > --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
> > > > > +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h
> > > > > @@ -28,12 +28,10 @@ struct ttm_test_devices {
> > > > >   /* Building blocks for test-specific init functions */
> > > > >   int ttm_device_kunit_init(struct ttm_test_devices *priv,
> > > > >   			  struct ttm_device *ttm,
> > > > > -			  bool use_dma_alloc,
> > > > > -			  bool use_dma32);
> > > > > +			  const struct ttm_device_init_flags
> > > > > flags);
> > > > >   int ttm_device_kunit_init_bad_evict(struct ttm_test_devices
> > > > > *priv,
> > > > >   				    struct ttm_device *ttm,
> > > > > -				    bool use_dma_alloc,
> > > > > -				    bool use_dma32);
> > > > > +				    const struct
> > > > > ttm_device_init_flags flags);
> > > > >   struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit
> > > > > *test,
> > > > >   					    struct
> > > > > ttm_test_devices *devs,
> > > > >   					    size_t size,
> > > > > diff --git a/drivers/gpu/drm/ttm/ttm_device.c
> > > > > b/drivers/gpu/drm/ttm/ttm_device.c
> > > > > index e7cc4954c1bc..0c85d10e5e0b 100644
> > > > > --- a/drivers/gpu/drm/ttm/ttm_device.c
> > > > > +++ b/drivers/gpu/drm/ttm/ttm_device.c
> > > > > @@ -174,8 +174,7 @@ EXPORT_SYMBOL(ttm_device_swapout);
> > > > >    * @dev: The core kernel device pointer for DMA mappings
> > > > > and
> > > > > allocations.
> > > > >    * @mapping: The address space to use for this bo.
> > > > >    * @vma_manager: A pointer to a vma manager.
> > > > > - * @use_dma_alloc: If coherent DMA allocation API should be
> > > > > used.
> > > > > - * @use_dma32: If we should use GFP_DMA32 for device memory
> > > > > allocations.
> > > > > + * @flags: Flags governing the device behaviour.
> > > > >    *
> > > > >    * Initializes a struct ttm_device:
> > > > >    * Returns:
> > > > > @@ -184,7 +183,7 @@ EXPORT_SYMBOL(ttm_device_swapout);
> > > > >   int ttm_device_init(struct ttm_device *bdev, const struct
> > > > > ttm_device_funcs *funcs,
> > > > >   		    struct device *dev, struct address_space
> > > > > *mapping,
> > > > >   		    struct drm_vma_offset_manager
> > > > > *vma_manager,
> > > > > -		    bool use_dma_alloc, bool use_dma32)
> > > > > +		    const struct ttm_device_init_flags
> > > > > flags)
> > > > >   {
> > > > >   	struct ttm_global *glob = &ttm_glob;
> > > > >   	int ret, nid;
> > > > > @@ -212,7 +211,7 @@ int ttm_device_init(struct ttm_device
> > > > > *bdev,
> > > > > const struct ttm_device_funcs *func
> > > > >   	else
> > > > >   		nid = NUMA_NO_NODE;
> > > > >   
> > > > > -	ttm_pool_init(&bdev->pool, dev, nid, use_dma_alloc,
> > > > > use_dma32);
> > > > > +	ttm_pool_init(&bdev->pool, dev, nid,
> > > > > flags.use_dma_alloc,
> > > > > flags.use_dma32);
> > > > >   
> > > > >   	bdev->vma_manager = vma_manager;
> > > > >   	spin_lock_init(&bdev->lru_lock);
> > > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > > > > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > > > > index cf7a89d002e4..7ff7e5d7a9f3 100644
> > > > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > > > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> > > > > @@ -1049,8 +1049,8 @@ static int vmw_driver_load(struct
> > > > > vmw_private
> > > > > *dev_priv, u32 pci_id)
> > > > >   			      dev_priv->drm.dev,
> > > > >   			      dev_priv->drm.anon_inode-
> > > > > > i_mapping,
> > > > >   			      dev_priv-
> > > > > >drm.vma_offset_manager,
> > > > > -			      dev_priv->map_mode ==
> > > > > vmw_dma_alloc_coherent,
> > > > > -			      false);
> > > > > +			      (struct ttm_device_init_flags)
> > > > > +			      {.use_dma_alloc = (dev_priv-
> > > > > > map_mode == vmw_dma_alloc_coherent)});
> > > > >   	if (unlikely(ret != 0)) {
> > > > >   		drm_err(&dev_priv->drm,
> > > > >   			"Failed initializing TTM buffer
> > > > > object
> > > > > driver.\n");
> > > > > diff --git a/drivers/gpu/drm/xe/xe_device.c
> > > > > b/drivers/gpu/drm/xe/xe_device.c
> > > > > index 09a7ad830e69..f9317f56ff9c 100644
> > > > > --- a/drivers/gpu/drm/xe/xe_device.c
> > > > > +++ b/drivers/gpu/drm/xe/xe_device.c
> > > > > @@ -320,7 +320,8 @@ struct xe_device *xe_device_create(struct
> > > > > pci_dev *pdev,
> > > > >   
> > > > >   	err = ttm_device_init(&xe->ttm, &xe_ttm_funcs, xe-
> > > > > > drm.dev,
> > > > >   			      xe->drm.anon_inode->i_mapping,
> > > > > -			      xe->drm.vma_offset_manager,
> > > > > false,
> > > > > false);
> > > > > +			      xe->drm.vma_offset_manager,
> > > > > +			      (struct
> > > > > ttm_device_init_flags){});
> > > > >   	if (WARN_ON(err))
> > > > >   		goto err;
> > > > >   
> > > > > diff --git a/include/drm/ttm/ttm_device.h
> > > > > b/include/drm/ttm/ttm_device.h
> > > > > index c22f30535c84..1534bd946c78 100644
> > > > > --- a/include/drm/ttm/ttm_device.h
> > > > > +++ b/include/drm/ttm/ttm_device.h
> > > > > @@ -287,10 +287,20 @@ static inline void
> > > > > ttm_set_driver_manager(struct ttm_device *bdev, int type,
> > > > >   	bdev->man_drv[type] = manager;
> > > > >   }
> > > > >   
> > > > > +/**
> > > > > + * struct ttm_device_init_flags - Flags for ttm device
> > > > > creation
> > > > > + */
> > > > > +struct ttm_device_init_flags {
> > > > > +	/** @use_dma_alloc: Whether coherent DMA allocation
> > > > > API
> > > > > should be used. */
> > > > > +	u32 use_dma_alloc : 1;
> > > > > +	/** @use_dma32: If we should use GFP_DMA32 for
> > > > > device
> > > > > memory allocations. */
> > > > > +	u32 use_dma32 : 1;
> > > > > +};
> > > > > +
> > > > >   int ttm_device_init(struct ttm_device *bdev, const struct
> > > > > ttm_device_funcs *funcs,
> > > > >   		    struct device *dev, struct address_space
> > > > > *mapping,
> > > > >   		    struct drm_vma_offset_manager
> > > > > *vma_manager,
> > > > > -		    bool use_dma_alloc, bool use_dma32);
> > > > > +		    const struct ttm_device_init_flags
> > > > > flags);
> > > > >   void ttm_device_fini(struct ttm_device *bdev);
> > > > >   void ttm_device_clear_dma_mappings(struct ttm_device
> > > > > *bdev);
> > > > >   
> > > > 
> > > 
> > 



More information about the Intel-gfx mailing list