diff mbox series

[v2,6/8] drm/amdgpu: Unmap entire device address space on device remove.

Message ID 1592719388-13819-7-git-send-email-andrey.grodzovsky@amd.com (mailing list archive)
State New, archived
Headers show
Series RFC Support hot device unplug in amdgpu | expand

Commit Message

Andrey Grodzovsky June 21, 2020, 6:03 a.m. UTC
Use the new TTM interface to invalidate all exsisting BO CPU mappings
form all user proccesses.

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Daniel Vetter June 22, 2020, 9:56 a.m. UTC | #1
On Sun, Jun 21, 2020 at 02:03:06AM -0400, Andrey Grodzovsky wrote:
> Use the new TTM interface to invalidate all exsisting BO CPU mappings
> form all user proccesses.
> 
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 43592dc..6932d75 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1135,6 +1135,7 @@ amdgpu_pci_remove(struct pci_dev *pdev)
>  	struct drm_device *dev = pci_get_drvdata(pdev);
>  
>  	drm_dev_unplug(dev);
> +	ttm_bo_unmap_virtual_address_space(&adev->mman.bdev);
>  	amdgpu_driver_unload_kms(dev);

Hm a ttm, or maybe even vram helper function which wraps drm_dev_unplug +
ttm unmapping into one would be nice I think? I suspect there's going to
be more in the future here.
-Daniel

>  
>  	pci_disable_device(pdev);
> -- 
> 2.7.4
>
Christian König June 22, 2020, 7:38 p.m. UTC | #2
Am 21.06.20 um 08:03 schrieb Andrey Grodzovsky:
> Use the new TTM interface to invalidate all exsisting BO CPU mappings
> form all user proccesses.
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

I think those two patches could already land in amd-staging-drm-next 
since they are a good idea independent of how else we fix the other issues.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 43592dc..6932d75 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1135,6 +1135,7 @@ amdgpu_pci_remove(struct pci_dev *pdev)
>   	struct drm_device *dev = pci_get_drvdata(pdev);
>   
>   	drm_dev_unplug(dev);
> +	ttm_bo_unmap_virtual_address_space(&adev->mman.bdev);
>   	amdgpu_driver_unload_kms(dev);
>   
>   	pci_disable_device(pdev);
Alex Deucher June 22, 2020, 7:48 p.m. UTC | #3
On Mon, Jun 22, 2020 at 3:38 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 21.06.20 um 08:03 schrieb Andrey Grodzovsky:
> > Use the new TTM interface to invalidate all exsisting BO CPU mappings
> > form all user proccesses.
> >
> > Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>
> Reviewed-by: Christian König <christian.koenig@amd.com>
>
> I think those two patches could already land in amd-staging-drm-next
> since they are a good idea independent of how else we fix the other issues.

Please make sure they land in drm-misc as well.

Alex

>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 43592dc..6932d75 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -1135,6 +1135,7 @@ amdgpu_pci_remove(struct pci_dev *pdev)
> >       struct drm_device *dev = pci_get_drvdata(pdev);
> >
> >       drm_dev_unplug(dev);
> > +     ttm_bo_unmap_virtual_address_space(&adev->mman.bdev);
> >       amdgpu_driver_unload_kms(dev);
> >
> >       pci_disable_device(pdev);
>
Daniel Vetter June 23, 2020, 10:22 a.m. UTC | #4
On Mon, Jun 22, 2020 at 03:48:29PM -0400, Alex Deucher wrote:
> On Mon, Jun 22, 2020 at 3:38 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
> >
> > Am 21.06.20 um 08:03 schrieb Andrey Grodzovsky:
> > > Use the new TTM interface to invalidate all exsisting BO CPU mappings
> > > form all user proccesses.
> > >
> > > Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> >
> > Reviewed-by: Christian König <christian.koenig@amd.com>
> >
> > I think those two patches could already land in amd-staging-drm-next
> > since they are a good idea independent of how else we fix the other issues.
> 
> Please make sure they land in drm-misc as well.

Not sure that's much use, since without any of the fault side changes you
just blow up on the first refault. Seems somewhat silly to charge ahead on
this with the other bits still very much under discussion.

Plus I suggested a possible bikeshed here :-)
-Daniel

> 
> Alex
> 
> >
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
> > >   1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > index 43592dc..6932d75 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > @@ -1135,6 +1135,7 @@ amdgpu_pci_remove(struct pci_dev *pdev)
> > >       struct drm_device *dev = pci_get_drvdata(pdev);
> > >
> > >       drm_dev_unplug(dev);
> > > +     ttm_bo_unmap_virtual_address_space(&adev->mman.bdev);
> > >       amdgpu_driver_unload_kms(dev);
> > >
> > >       pci_disable_device(pdev);
> >
Christian König June 23, 2020, 1:16 p.m. UTC | #5
Am 23.06.20 um 12:22 schrieb Daniel Vetter:
> On Mon, Jun 22, 2020 at 03:48:29PM -0400, Alex Deucher wrote:
>> On Mon, Jun 22, 2020 at 3:38 PM Christian König
>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>> Am 21.06.20 um 08:03 schrieb Andrey Grodzovsky:
>>>> Use the new TTM interface to invalidate all exsisting BO CPU mappings
>>>> form all user proccesses.
>>>>
>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>>
>>> I think those two patches could already land in amd-staging-drm-next
>>> since they are a good idea independent of how else we fix the other issues.
>> Please make sure they land in drm-misc as well.
> Not sure that's much use, since without any of the fault side changes you
> just blow up on the first refault. Seems somewhat silly to charge ahead on
> this with the other bits still very much under discussion.

Well what I wanted to say is that we don't need to send out those simple 
patches once more.

> Plus I suggested a possible bikeshed here :-)

No bikeshed, but indeed a rather good idea to not make this a TTM function.

Christian.

> -Daniel
>
>> Alex
>>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> index 43592dc..6932d75 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>> @@ -1135,6 +1135,7 @@ amdgpu_pci_remove(struct pci_dev *pdev)
>>>>        struct drm_device *dev = pci_get_drvdata(pdev);
>>>>
>>>>        drm_dev_unplug(dev);
>>>> +     ttm_bo_unmap_virtual_address_space(&adev->mman.bdev);
>>>>        amdgpu_driver_unload_kms(dev);
>>>>
>>>>        pci_disable_device(pdev);
Andrey Grodzovsky June 24, 2020, 3:12 a.m. UTC | #6
On 6/23/20 9:16 AM, Christian König wrote:
> Am 23.06.20 um 12:22 schrieb Daniel Vetter:
>> On Mon, Jun 22, 2020 at 03:48:29PM -0400, Alex Deucher wrote:
>>> On Mon, Jun 22, 2020 at 3:38 PM Christian König
>>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>>> Am 21.06.20 um 08:03 schrieb Andrey Grodzovsky:
>>>>> Use the new TTM interface to invalidate all exsisting BO CPU mappings
>>>>> form all user proccesses.
>>>>>
>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>>>
>>>> I think those two patches could already land in amd-staging-drm-next
>>>> since they are a good idea independent of how else we fix the other issues.
>>> Please make sure they land in drm-misc as well.
>> Not sure that's much use, since without any of the fault side changes you
>> just blow up on the first refault. Seems somewhat silly to charge ahead on
>> this with the other bits still very much under discussion.
>
> Well what I wanted to say is that we don't need to send out those simple 
> patches once more.
>
>> Plus I suggested a possible bikeshed here :-)
>
> No bikeshed, but indeed a rather good idea to not make this a TTM function.
>
> Christian.


So i will incorporate the changes suggested to turn the TTM part into generic 
DRM helper and will resend both patches as part of V3 (which might take a while 
now due to a context switch I am doing for another task).

Andrey


>
>> -Daniel
>>
>>> Alex
>>>
>>>>> ---
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
>>>>>    1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>> index 43592dc..6932d75 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>>>> @@ -1135,6 +1135,7 @@ amdgpu_pci_remove(struct pci_dev *pdev)
>>>>>        struct drm_device *dev = pci_get_drvdata(pdev);
>>>>>
>>>>>        drm_dev_unplug(dev);
>>>>> + ttm_bo_unmap_virtual_address_space(&adev->mman.bdev);
>>>>>        amdgpu_driver_unload_kms(dev);
>>>>>
>>>>>        pci_disable_device(pdev);
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 43592dc..6932d75 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1135,6 +1135,7 @@  amdgpu_pci_remove(struct pci_dev *pdev)
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
 	drm_dev_unplug(dev);
+	ttm_bo_unmap_virtual_address_space(&adev->mman.bdev);
 	amdgpu_driver_unload_kms(dev);
 
 	pci_disable_device(pdev);