Re: [PATCH 01/16] drm/amdgpu/gmc: Don't hardcode GART page count before GTT

["Timur Kristóf" <timur.kristof@gmail.com>]

On Tue, 2025-11-04 at 16:08 +0100, Pierre-Eric Pelloux-Prayer wrote:
> 
> 
> Le 04/11/2025 à 14:23, Christian König a écrit :
> > 
> > 
> > On 11/3/25 23:23, Timur Kristóf wrote:
> > > GART contains some pages in its address space that come before
> > > the GTT and are used for BO copies.
> > > 
> > > Instead of hardcoding the size of the GART space before GTT,
> > > make it a field in the amdgpu_gmc struct. This allows us to map
> > > more things in GART before GTT.
> > > 
> > > Split this into a separate patch to make it easier to bisect,
> > > in case there are any errors in the future.
> > > 
> > > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c     | 2 ++
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h     | 1 +
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 2 +-
> > >   3 files changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> > > index 97b562a79ea8..bf31bd022d6d 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> > > @@ -325,6 +325,8 @@ void amdgpu_gmc_gart_location(struct
> > > amdgpu_device *adev, struct amdgpu_gmc *mc,
> > >   		break;
> > >   	}
> > >   
> > > +	mc->num_gart_pages_before_gtt =
> > > +		AMDGPU_GTT_MAX_TRANSFER_SIZE *
> > > AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
> > >   	mc->gart_start &= ~(four_gb - 1);
> > >   	mc->gart_end = mc->gart_start + mc->gart_size - 1;
> > >   	dev_info(adev->dev, "GART: %lluM 0x%016llX -
> > > 0x%016llX\n",
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> > > index 55097ca10738..568eed3eb557 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> > > @@ -266,6 +266,7 @@ struct amdgpu_gmc {
> > >   	u64			fb_end;
> > >   	unsigned		vram_width;
> > >   	u64			real_vram_size;
> > > +	u32			num_gart_pages_before_gtt;
> > >   	int			vram_mtrr;
> > >   	u64                     mc_mask;
> > >   	const struct firmware   *fw;	/* MC firmware */
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> > > index 0760e70402ec..4c2563a70c2b 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> > > @@ -283,7 +283,7 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device
> > > *adev, uint64_t gtt_size)
> > >   
> > >   	ttm_resource_manager_init(man, &adev->mman.bdev,
> > > gtt_size);
> > >   
> > > -	start = AMDGPU_GTT_MAX_TRANSFER_SIZE *
> > > AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
> > > +	start = adev->gmc.num_gart_pages_before_gtt;
> > 
> > I would prefer that we have a single function which returns the
> > number of GART pages we need before the dynamic GTT mapping.
> > 
> > But let me see how Pierre-Eric solved this first.
> > 
> 
> The only conflicting change with my series is that I changed this:
> 
> -	start = AMDGPU_GTT_MAX_TRANSFER_SIZE *
> AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
> +	start = AMDGPU_GTT_MAX_TRANSFER_SIZE * reserved_windows;
> 
> So to account for Timur's change, I would only need to adapt this
> part:
> 
> +	mc->num_gart_pages_before_gtt =
> +		AMDGPU_GTT_MAX_TRANSFER_SIZE * reserved_windows;
> 

Hi Christian & Pierre-Eric,

I assume this conversation is about the following series:
drm/amdgpu: use all SDMA instances for TTM clears and moves

My question is, which series do we want to land first? I would really
like VCE1 to get into Linux 6.19, but otherwise I'm OK with either of
these two options:

1. If Pierre-Eric's series lands sooner: I'm happy to rebase my work
and use Pierre-Eric's solution.
2. If my series lands first: I'm happy to contribute a patch to Pierre-
Eric's series to adapt my workaround to his solution, if that helps.

Please let me know how you guys prefer to proceed.

Thanks & best regards,
Timur

> > 
> > >   	size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
> > >   	drm_mm_init(&mgr->mm, start, size);
> > >   	spin_lock_init(&mgr->lock);