| Message ID | 1589050310-19666-2-git-send-email-andrey.grodzovsky@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RFC Support hot device unplug in amdgpu | expand |
Am 09.05.20 um 20:51 schrieb Andrey Grodzovsky: > Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com> > --- > drivers/gpu/drm/ttm/ttm_bo.c | 22 +++++++++++++++++++++- > include/drm/ttm/ttm_bo_driver.h | 2 ++ > 2 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index c5b516f..eae61cc 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -1750,9 +1750,29 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo) > ttm_bo_unmap_virtual_locked(bo); > ttm_mem_io_unlock(man); > } > +EXPORT_SYMBOL(ttm_bo_unmap_virtual); > > +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev) > +{ > + struct ttm_mem_type_manager *man; > + int i; > > -EXPORT_SYMBOL(ttm_bo_unmap_virtual); > + for (i = 0; i < TTM_NUM_MEM_TYPES; i++) { > + man = &bdev->man[i]; > + if (man->has_type && man->use_type) > + ttm_mem_io_lock(man, false); > + } You should drop that it will just result in a deadlock warning for Nouveau and has no effect at all. Apart from that looks good to me, Christian. > + > + unmap_mapping_range(bdev->dev_mapping, 0, 0 , 1); > + /*TODO What about ttm_mem_io_free_vm(bo) ? */ > + > + for (i = TTM_NUM_MEM_TYPES - 1; i >= 0; i--) { > + man = &bdev->man[i]; > + if (man->has_type && man->use_type) > + ttm_mem_io_unlock(man); > + } > +} > +EXPORT_SYMBOL(ttm_bo_unmap_virtual_address_space); > > int ttm_bo_wait(struct ttm_buffer_object *bo, > bool interruptible, bool no_wait) > diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h > index c9e0fd0..3133463 100644 > --- a/include/drm/ttm/ttm_bo_driver.h > +++ b/include/drm/ttm/ttm_bo_driver.h > @@ -600,6 +600,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, > */ > void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo); > > +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev); > + > /** > * ttm_bo_unmap_virtual > *
On 5/11/20 2:45 AM, Christian König wrote: > Am 09.05.20 um 20:51 schrieb Andrey Grodzovsky: >> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com> >> --- >> drivers/gpu/drm/ttm/ttm_bo.c | 22 +++++++++++++++++++++- >> include/drm/ttm/ttm_bo_driver.h | 2 ++ >> 2 files changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c >> index c5b516f..eae61cc 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo.c >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >> @@ -1750,9 +1750,29 @@ void ttm_bo_unmap_virtual(struct >> ttm_buffer_object *bo) >> ttm_bo_unmap_virtual_locked(bo); >> ttm_mem_io_unlock(man); >> } >> +EXPORT_SYMBOL(ttm_bo_unmap_virtual); >> +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev) >> +{ >> + struct ttm_mem_type_manager *man; >> + int i; >> -EXPORT_SYMBOL(ttm_bo_unmap_virtual); > >> + for (i = 0; i < TTM_NUM_MEM_TYPES; i++) { >> + man = &bdev->man[i]; >> + if (man->has_type && man->use_type) >> + ttm_mem_io_lock(man, false); >> + } > > You should drop that it will just result in a deadlock warning for > Nouveau and has no effect at all. > > Apart from that looks good to me, > Christian. As I am considering to re-include this in V2 of the patchsets, can you clarify please why this will have no effect at all ? Andrey > >> + >> + unmap_mapping_range(bdev->dev_mapping, 0, 0 , 1); >> + /*TODO What about ttm_mem_io_free_vm(bo) ? */ >> + >> + for (i = TTM_NUM_MEM_TYPES - 1; i >= 0; i--) { >> + man = &bdev->man[i]; >> + if (man->has_type && man->use_type) >> + ttm_mem_io_unlock(man); >> + } >> +} >> +EXPORT_SYMBOL(ttm_bo_unmap_virtual_address_space); >> int ttm_bo_wait(struct ttm_buffer_object *bo, >> bool interruptible, bool no_wait) >> diff --git a/include/drm/ttm/ttm_bo_driver.h >> b/include/drm/ttm/ttm_bo_driver.h >> index c9e0fd0..3133463 100644 >> --- a/include/drm/ttm/ttm_bo_driver.h >> +++ b/include/drm/ttm/ttm_bo_driver.h >> @@ -600,6 +600,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, >> */ >> void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo); >> +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev); >> + >> /** >> * ttm_bo_unmap_virtual >> * >
Am 05.06.20 um 16:29 schrieb Andrey Grodzovsky: > > On 5/11/20 2:45 AM, Christian König wrote: >> Am 09.05.20 um 20:51 schrieb Andrey Grodzovsky: >>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com> >>> --- >>> drivers/gpu/drm/ttm/ttm_bo.c | 22 +++++++++++++++++++++- >>> include/drm/ttm/ttm_bo_driver.h | 2 ++ >>> 2 files changed, 23 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c >>> b/drivers/gpu/drm/ttm/ttm_bo.c >>> index c5b516f..eae61cc 100644 >>> --- a/drivers/gpu/drm/ttm/ttm_bo.c >>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >>> @@ -1750,9 +1750,29 @@ void ttm_bo_unmap_virtual(struct >>> ttm_buffer_object *bo) >>> ttm_bo_unmap_virtual_locked(bo); >>> ttm_mem_io_unlock(man); >>> } >>> +EXPORT_SYMBOL(ttm_bo_unmap_virtual); >>> +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev) >>> +{ >>> + struct ttm_mem_type_manager *man; >>> + int i; >>> -EXPORT_SYMBOL(ttm_bo_unmap_virtual); >> >>> + for (i = 0; i < TTM_NUM_MEM_TYPES; i++) { >>> + man = &bdev->man[i]; >>> + if (man->has_type && man->use_type) >>> + ttm_mem_io_lock(man, false); >>> + } >> >> You should drop that it will just result in a deadlock warning for >> Nouveau and has no effect at all. >> >> Apart from that looks good to me, >> Christian. > > > As I am considering to re-include this in V2 of the patchsets, can you > clarify please why this will have no effect at all ? The locks are exclusive for Nouveau to allocate/free the io address space. Since we don't do this here we don't need the locks. Christian. > > Andrey > > >> >>> + >>> + unmap_mapping_range(bdev->dev_mapping, 0, 0 , 1); >>> + /*TODO What about ttm_mem_io_free_vm(bo) ? */ >>> + >>> + for (i = TTM_NUM_MEM_TYPES - 1; i >= 0; i--) { >>> + man = &bdev->man[i]; >>> + if (man->has_type && man->use_type) >>> + ttm_mem_io_unlock(man); >>> + } >>> +} >>> +EXPORT_SYMBOL(ttm_bo_unmap_virtual_address_space); >>> int ttm_bo_wait(struct ttm_buffer_object *bo, >>> bool interruptible, bool no_wait) >>> diff --git a/include/drm/ttm/ttm_bo_driver.h >>> b/include/drm/ttm/ttm_bo_driver.h >>> index c9e0fd0..3133463 100644 >>> --- a/include/drm/ttm/ttm_bo_driver.h >>> +++ b/include/drm/ttm/ttm_bo_driver.h >>> @@ -600,6 +600,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, >>> */ >>> void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo); >>> +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev); >>> + >>> /** >>> * ttm_bo_unmap_virtual >>> * >>
On 6/5/20 2:40 PM, Christian König wrote: > Am 05.06.20 um 16:29 schrieb Andrey Grodzovsky: >> >> On 5/11/20 2:45 AM, Christian König wrote: >>> Am 09.05.20 um 20:51 schrieb Andrey Grodzovsky: >>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com> >>>> --- >>>> drivers/gpu/drm/ttm/ttm_bo.c | 22 +++++++++++++++++++++- >>>> include/drm/ttm/ttm_bo_driver.h | 2 ++ >>>> 2 files changed, 23 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c >>>> b/drivers/gpu/drm/ttm/ttm_bo.c >>>> index c5b516f..eae61cc 100644 >>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c >>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >>>> @@ -1750,9 +1750,29 @@ void ttm_bo_unmap_virtual(struct >>>> ttm_buffer_object *bo) >>>> ttm_bo_unmap_virtual_locked(bo); >>>> ttm_mem_io_unlock(man); >>>> } >>>> +EXPORT_SYMBOL(ttm_bo_unmap_virtual); >>>> +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev) >>>> +{ >>>> + struct ttm_mem_type_manager *man; >>>> + int i; >>>> -EXPORT_SYMBOL(ttm_bo_unmap_virtual); >>> >>>> + for (i = 0; i < TTM_NUM_MEM_TYPES; i++) { >>>> + man = &bdev->man[i]; >>>> + if (man->has_type && man->use_type) >>>> + ttm_mem_io_lock(man, false); >>>> + } >>> >>> You should drop that it will just result in a deadlock warning for >>> Nouveau and has no effect at all. >>> >>> Apart from that looks good to me, >>> Christian. >> >> >> As I am considering to re-include this in V2 of the patchsets, can >> you clarify please why this will have no effect at all ? > > The locks are exclusive for Nouveau to allocate/free the io address > space. > > Since we don't do this here we don't need the locks. > > Christian. So basically calling unmap_mapping_range doesn't require any extra locking around it and whatever locks are taken within the function should be enough ? Andrey > >> >> Andrey >> >> >>> >>>> + >>>> + unmap_mapping_range(bdev->dev_mapping, 0, 0 , 1); >>>> + /*TODO What about ttm_mem_io_free_vm(bo) ? */ >>>> + >>>> + for (i = TTM_NUM_MEM_TYPES - 1; i >= 0; i--) { >>>> + man = &bdev->man[i]; >>>> + if (man->has_type && man->use_type) >>>> + ttm_mem_io_unlock(man); >>>> + } >>>> +} >>>> +EXPORT_SYMBOL(ttm_bo_unmap_virtual_address_space); >>>> int ttm_bo_wait(struct ttm_buffer_object *bo, >>>> bool interruptible, bool no_wait) >>>> diff --git a/include/drm/ttm/ttm_bo_driver.h >>>> b/include/drm/ttm/ttm_bo_driver.h >>>> index c9e0fd0..3133463 100644 >>>> --- a/include/drm/ttm/ttm_bo_driver.h >>>> +++ b/include/drm/ttm/ttm_bo_driver.h >>>> @@ -600,6 +600,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, >>>> */ >>>> void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo); >>>> +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device >>>> *bdev); >>>> + >>>> /** >>>> * ttm_bo_unmap_virtual >>>> * >>> >
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index c5b516f..eae61cc 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -1750,9 +1750,29 @@ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo) ttm_bo_unmap_virtual_locked(bo); ttm_mem_io_unlock(man); } +EXPORT_SYMBOL(ttm_bo_unmap_virtual); +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev) +{ + struct ttm_mem_type_manager *man; + int i; -EXPORT_SYMBOL(ttm_bo_unmap_virtual); + for (i = 0; i < TTM_NUM_MEM_TYPES; i++) { + man = &bdev->man[i]; + if (man->has_type && man->use_type) + ttm_mem_io_lock(man, false); + } + + unmap_mapping_range(bdev->dev_mapping, 0, 0 , 1); + /*TODO What about ttm_mem_io_free_vm(bo) ? */ + + for (i = TTM_NUM_MEM_TYPES - 1; i >= 0; i--) { + man = &bdev->man[i]; + if (man->has_type && man->use_type) + ttm_mem_io_unlock(man); + } +} +EXPORT_SYMBOL(ttm_bo_unmap_virtual_address_space); int ttm_bo_wait(struct ttm_buffer_object *bo, bool interruptible, bool no_wait) diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index c9e0fd0..3133463 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -600,6 +600,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev, */ void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo); +void ttm_bo_unmap_virtual_address_space(struct ttm_bo_device *bdev); + /** * ttm_bo_unmap_virtual *
Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com> --- drivers/gpu/drm/ttm/ttm_bo.c | 22 +++++++++++++++++++++- include/drm/ttm/ttm_bo_driver.h | 2 ++ 2 files changed, 23 insertions(+), 1 deletion(-)