[PATCH] drm/amdgpu: Enable doorbell selfring if resize BAR successfully

[Xiao, Shane]

[AMD Official Use Only - General]



> -----Original Message-----
> From: Xiao, Shane
> Sent: Monday, April 24, 2023 6:31 PM
> To: Christian König <ckoenig.leichtzumerken at gmail.com>; amd-
> gfx at lists.freedesktop.org; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Zhang, Hawking
> <Hawking.Zhang at amd.com>; Kuehling, Felix <Felix.Kuehling at amd.com>
> Cc: Hou, Xiaomeng (Matthew) <Xiaomeng.Hou at amd.com>; Liu, Aaron
> <Aaron.Liu at amd.com>
> Subject: RE: [PATCH] drm/amdgpu: Enable doorbell selfring if resize BAR
> successfully
> 
> [AMD Official Use Only - General]
> 
> 
> 
> > -----Original Message-----
> > From: Christian König <ckoenig.leichtzumerken at gmail.com>
> > Sent: Monday, April 24, 2023 5:07 PM
> > To: Xiao, Shane <shane.xiao at amd.com>; amd-gfx at lists.freedesktop.org;
> > Deucher, Alexander <Alexander.Deucher at amd.com>; Zhang, Hawking
> > <Hawking.Zhang at amd.com>; Kuehling, Felix <Felix.Kuehling at amd.com>
> > Cc: Hou, Xiaomeng (Matthew) <Xiaomeng.Hou at amd.com>; Liu, Aaron
> > <Aaron.Liu at amd.com>
> > Subject: Re: [PATCH] drm/amdgpu: Enable doorbell selfring if resize
> > BAR successfully
> >
> > Am 18.04.23 um 08:54 schrieb Shane Xiao:
> > > [Why]
> > > The selfring doorbell aperture will change when we resize FB BAR
> > > successfully during gmc sw init, we should reorder the sequence of
> > > enabling doorbell selfring aperture.
> >
> > That's a good catch.
> >
> > >
> > > [How]
> > > Move enable_doorbell_selfring_aperture from *_common_hw_init to
> > > *_common_late_init.
> >
> > But that sounds like a bad idea. Instead the full call to
> > nv_enable_doorbell_aperture() should be moved around.
> 
> Hi Christian,
> 
> Yes,  I get your idea. But as far as I can understand that, the gfx hw init will use
> doorbell.
> If so, we cannot enable doorbell after gfx hw init.

We have come up with two ways to resolve the issue.

1) Separate enable_doorbell_aperture and enable_doorbell_selfring_aperture. However,  the enable_doorbell_selfring_aperture
should be moved in *_common_ip_funcs->late_init.
2) The full call can be moved to gmc hw init.  But it seems strange to move nbio configuration into gmc hw init.

If neither of the above methods is suitable, could you please give us some advice on this issue?

Best Regards,
Shane

> 
> Best Regards,
> Shane
> 
> >
> > Regards,
> > Christian.
> >
> > >
> > > This fixes the potential issue that GPU ring its own doorbell when
> > > this device is in translated mode with iommu is on.
> > >
> > > Signed-off-by: Shane Xiao <shane.xiao at amd.com>
> > > Signed-off-by: Aaron Liu <aaron.liu at amd.com>
> > > Tested-by: Xiaomeng Hou <Xiaomeng.Hou at amd.com>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/nv.c    | 4 +++-
> > >   drivers/gpu/drm/amd/amdgpu/soc15.c | 4 +++-
> > >   drivers/gpu/drm/amd/amdgpu/soc21.c | 4 +++-
> > >   3 files changed, 9 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c
> > > b/drivers/gpu/drm/amd/amdgpu/nv.c index 47420b403871..f4c85634a4c8
> > > 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> > > @@ -535,7 +535,8 @@ static void nv_enable_doorbell_aperture(struct
> > amdgpu_device *adev,
> > >   					bool enable)
> > >   {
> > >   	adev->nbio.funcs->enable_doorbell_aperture(adev, enable);
> > > -	adev->nbio.funcs->enable_doorbell_selfring_aperture(adev, enable);
> > > +	if (!enable)
> > > +		adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
> > false);
> > >   }
> > >
> > >   const struct amdgpu_ip_block_version nv_common_ip_block = @@
> > > -999,6
> > > +1000,7 @@ static int nv_common_late_init(void *handle)
> > >   		}
> > >   	}
> > >
> > > +	adev->nbio.funcs->enable_doorbell_selfring_aperture(adev, true);
> > >   	return 0;
> > >   }
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c
> > > b/drivers/gpu/drm/amd/amdgpu/soc15.c
> > > index bc5dd80f10c1..0202de79a389 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> > > @@ -623,7 +623,8 @@ static void
> > > soc15_enable_doorbell_aperture(struct
> > amdgpu_device *adev,
> > >   					   bool enable)
> > >   {
> > >   	adev->nbio.funcs->enable_doorbell_aperture(adev, enable);
> > > -	adev->nbio.funcs->enable_doorbell_selfring_aperture(adev, enable);
> > > +	if (!enable)
> > > +		adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
> > false);
> > >   }
> > >
> > >   const struct amdgpu_ip_block_version vega10_common_ip_block = @@
> > > -1125,6 +1126,7 @@ static int soc15_common_late_init(void *handle)
> > >   	if (amdgpu_sriov_vf(adev))
> > >   		xgpu_ai_mailbox_get_irq(adev);
> > >
> > > +	adev->nbio.funcs->enable_doorbell_selfring_aperture(adev, true);
> > >   	return 0;
> > >   }
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c
> > > b/drivers/gpu/drm/amd/amdgpu/soc21.c
> > > index 514bfc705d5a..cd4619085d67 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
> > > @@ -454,7 +454,8 @@ static void
> > > soc21_enable_doorbell_aperture(struct
> > amdgpu_device *adev,
> > >   					bool enable)
> > >   {
> > >   	adev->nbio.funcs->enable_doorbell_aperture(adev, enable);
> > > -	adev->nbio.funcs->enable_doorbell_selfring_aperture(adev, enable);
> > > +	if (!enable)
> > > +		adev->nbio.funcs->enable_doorbell_selfring_aperture(adev,
> > false);
> > >   }
> > >
> > >   const struct amdgpu_ip_block_version soc21_common_ip_block = @@
> > > -764,6 +765,7 @@ static int soc21_common_late_init(void *handle)
> > >   			amdgpu_irq_get(adev, &adev-
> > >nbio.ras_err_event_athub_irq, 0);
> > >   	}
> > >
> > > +	adev->nbio.funcs->enable_doorbell_selfring_aperture(adev, true);
> > >   	return 0;
> > >   }
> > >