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:26 +0100, Timur Kristóf wrote:
> 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

We just had a conversation with Christian and Pierre-Eric about this.
We arrived at the following suggested solution:

1. Add a new function to the VCE code which returns the amount of GTT
pages the VCE needs in the GART before GTT. The function would return
the amound of pages necessary for this "hack" on VCE1, and 0 on other
VCE versions.
2. Call that function from amdgpu_gtt_mgr_init(), and add the result to
the "start" variable there.
3. Then there is no need for a "num_gart_pages_before_gtt" variable.

If everyone agrees with this, I will do that in the next version of
this series.


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