| Message ID | 20200813083644.31711-2-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Convert all remaining drivers to GEM object functions | expand |
Am 13.08.20 um 10:36 schrieb Thomas Zimmermann: > GEM object functions deprecate several similar callback interfaces in > struct drm_driver. This patch replaces the per-driver callbacks with > per-instance callbacks in amdgpu. The only exception is gem_prime_mmap, > which is non-trivial to convert. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ------ > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 ++++++++++++ > 2 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > index 81a79760ca61..51525b8774c9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > @@ -1468,19 +1468,13 @@ static struct drm_driver kms_driver = { > .lastclose = amdgpu_driver_lastclose_kms, > .irq_handler = amdgpu_irq_handler, > .ioctls = amdgpu_ioctls_kms, > - .gem_free_object_unlocked = amdgpu_gem_object_free, > - .gem_open_object = amdgpu_gem_object_open, > - .gem_close_object = amdgpu_gem_object_close, > .dumb_create = amdgpu_mode_dumb_create, > .dumb_map_offset = amdgpu_mode_dumb_mmap, > .fops = &amdgpu_driver_kms_fops, > > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > - .gem_prime_export = amdgpu_gem_prime_export, > .gem_prime_import = amdgpu_gem_prime_import, > - .gem_prime_vmap = amdgpu_gem_prime_vmap, > - .gem_prime_vunmap = amdgpu_gem_prime_vunmap, > .gem_prime_mmap = amdgpu_gem_prime_mmap, > > .name = DRIVER_NAME, > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index 43f4966331dd..ca2b79f94e99 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -36,6 +36,7 @@ > #include <drm/amdgpu_drm.h> > #include <drm/drm_cache.h> > #include "amdgpu.h" > +#include "amdgpu_dma_buf.h" > #include "amdgpu_trace.h" > #include "amdgpu_amdkfd.h" > > @@ -510,6 +511,15 @@ bool amdgpu_bo_support_uswc(u64 bo_flags) > #endif > } > > +static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = { > + .free = amdgpu_gem_object_free, > + .open = amdgpu_gem_object_open, > + .close = amdgpu_gem_object_close, > + .export = amdgpu_gem_prime_export, > + .vmap = amdgpu_gem_prime_vmap, > + .vunmap = amdgpu_gem_prime_vunmap, > +}; > + Wrong file, this belongs into amdgpu_gem.c > static int amdgpu_bo_do_create(struct amdgpu_device *adev, > struct amdgpu_bo_param *bp, > struct amdgpu_bo **bo_ptr) > @@ -552,6 +562,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, > bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL); > if (bo == NULL) > return -ENOMEM; > + > + bo->tbo.base.funcs = &amdgpu_gem_object_funcs; And this should probably go into amdgpu_gem_object_create(). Apart from that looks like a good idea to me. Christian. > drm_gem_private_object_init(adev->ddev, &bo->tbo.base, size); > INIT_LIST_HEAD(&bo->shadow_list); > bo->vm_bo = NULL;
Hi Am 13.08.20 um 12:22 schrieb Christian König: > Am 13.08.20 um 10:36 schrieb Thomas Zimmermann: >> GEM object functions deprecate several similar callback interfaces in >> struct drm_driver. This patch replaces the per-driver callbacks with >> per-instance callbacks in amdgpu. The only exception is gem_prime_mmap, >> which is non-trivial to convert. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ------ >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 ++++++++++++ >> 2 files changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> index 81a79760ca61..51525b8774c9 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> @@ -1468,19 +1468,13 @@ static struct drm_driver kms_driver = { >> .lastclose = amdgpu_driver_lastclose_kms, >> .irq_handler = amdgpu_irq_handler, >> .ioctls = amdgpu_ioctls_kms, >> - .gem_free_object_unlocked = amdgpu_gem_object_free, >> - .gem_open_object = amdgpu_gem_object_open, >> - .gem_close_object = amdgpu_gem_object_close, >> .dumb_create = amdgpu_mode_dumb_create, >> .dumb_map_offset = amdgpu_mode_dumb_mmap, >> .fops = &amdgpu_driver_kms_fops, >> .prime_handle_to_fd = drm_gem_prime_handle_to_fd, >> .prime_fd_to_handle = drm_gem_prime_fd_to_handle, >> - .gem_prime_export = amdgpu_gem_prime_export, >> .gem_prime_import = amdgpu_gem_prime_import, >> - .gem_prime_vmap = amdgpu_gem_prime_vmap, >> - .gem_prime_vunmap = amdgpu_gem_prime_vunmap, >> .gem_prime_mmap = amdgpu_gem_prime_mmap, >> .name = DRIVER_NAME, >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> index 43f4966331dd..ca2b79f94e99 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> @@ -36,6 +36,7 @@ >> #include <drm/amdgpu_drm.h> >> #include <drm/drm_cache.h> >> #include "amdgpu.h" >> +#include "amdgpu_dma_buf.h" >> #include "amdgpu_trace.h" >> #include "amdgpu_amdkfd.h" >> @@ -510,6 +511,15 @@ bool amdgpu_bo_support_uswc(u64 bo_flags) >> #endif >> } >> +static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = { >> + .free = amdgpu_gem_object_free, >> + .open = amdgpu_gem_object_open, >> + .close = amdgpu_gem_object_close, >> + .export = amdgpu_gem_prime_export, >> + .vmap = amdgpu_gem_prime_vmap, >> + .vunmap = amdgpu_gem_prime_vunmap, >> +}; >> + > > Wrong file, this belongs into amdgpu_gem.c > >> static int amdgpu_bo_do_create(struct amdgpu_device *adev, >> struct amdgpu_bo_param *bp, >> struct amdgpu_bo **bo_ptr) >> @@ -552,6 +562,8 @@ static int amdgpu_bo_do_create(struct >> amdgpu_device *adev, >> bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL); >> if (bo == NULL) >> return -ENOMEM; >> + >> + bo->tbo.base.funcs = &amdgpu_gem_object_funcs; > > And this should probably go into amdgpu_gem_object_create(). I'm trying to understand what amdgpu does. What about all the places where amdgpu calls amdgpu_bo_create() internally? Wouldn't these miss the free callback for the GEM object? Best regards Thomas > > Apart from that looks like a good idea to me. > > Christian. > >> drm_gem_private_object_init(adev->ddev, &bo->tbo.base, size); >> INIT_LIST_HEAD(&bo->shadow_list); >> bo->vm_bo = NULL; >
Am 14.09.20 um 17:05 schrieb Thomas Zimmermann: > Hi > > Am 13.08.20 um 12:22 schrieb Christian König: >> Am 13.08.20 um 10:36 schrieb Thomas Zimmermann: >>> GEM object functions deprecate several similar callback interfaces in >>> struct drm_driver. This patch replaces the per-driver callbacks with >>> per-instance callbacks in amdgpu. The only exception is gem_prime_mmap, >>> which is non-trivial to convert. >>> >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ------ >>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 ++++++++++++ >>> 2 files changed, 12 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>> index 81a79760ca61..51525b8774c9 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>> @@ -1468,19 +1468,13 @@ static struct drm_driver kms_driver = { >>> .lastclose = amdgpu_driver_lastclose_kms, >>> .irq_handler = amdgpu_irq_handler, >>> .ioctls = amdgpu_ioctls_kms, >>> - .gem_free_object_unlocked = amdgpu_gem_object_free, >>> - .gem_open_object = amdgpu_gem_object_open, >>> - .gem_close_object = amdgpu_gem_object_close, >>> .dumb_create = amdgpu_mode_dumb_create, >>> .dumb_map_offset = amdgpu_mode_dumb_mmap, >>> .fops = &amdgpu_driver_kms_fops, >>> .prime_handle_to_fd = drm_gem_prime_handle_to_fd, >>> .prime_fd_to_handle = drm_gem_prime_fd_to_handle, >>> - .gem_prime_export = amdgpu_gem_prime_export, >>> .gem_prime_import = amdgpu_gem_prime_import, >>> - .gem_prime_vmap = amdgpu_gem_prime_vmap, >>> - .gem_prime_vunmap = amdgpu_gem_prime_vunmap, >>> .gem_prime_mmap = amdgpu_gem_prime_mmap, >>> .name = DRIVER_NAME, >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>> index 43f4966331dd..ca2b79f94e99 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>> @@ -36,6 +36,7 @@ >>> #include <drm/amdgpu_drm.h> >>> #include <drm/drm_cache.h> >>> #include "amdgpu.h" >>> +#include "amdgpu_dma_buf.h" >>> #include "amdgpu_trace.h" >>> #include "amdgpu_amdkfd.h" >>> @@ -510,6 +511,15 @@ bool amdgpu_bo_support_uswc(u64 bo_flags) >>> #endif >>> } >>> +static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = { >>> + .free = amdgpu_gem_object_free, >>> + .open = amdgpu_gem_object_open, >>> + .close = amdgpu_gem_object_close, >>> + .export = amdgpu_gem_prime_export, >>> + .vmap = amdgpu_gem_prime_vmap, >>> + .vunmap = amdgpu_gem_prime_vunmap, >>> +}; >>> + >> Wrong file, this belongs into amdgpu_gem.c >> >>> static int amdgpu_bo_do_create(struct amdgpu_device *adev, >>> struct amdgpu_bo_param *bp, >>> struct amdgpu_bo **bo_ptr) >>> @@ -552,6 +562,8 @@ static int amdgpu_bo_do_create(struct >>> amdgpu_device *adev, >>> bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL); >>> if (bo == NULL) >>> return -ENOMEM; >>> + >>> + bo->tbo.base.funcs = &amdgpu_gem_object_funcs; >> And this should probably go into amdgpu_gem_object_create(). > I'm trying to understand what amdgpu does. What about all the places > where amdgpu calls amdgpu_bo_create() internally? Wouldn't these miss > the free callback for the GEM object? Those shouldn't have a GEM object in the first place. Or otherwise we would have a reference counting issue. Regards, Christian. > > Best regards > Thomas > >> Apart from that looks like a good idea to me. >> >> Christian. >> >>> drm_gem_private_object_init(adev->ddev, &bo->tbo.base, size); >>> INIT_LIST_HEAD(&bo->shadow_list); >>> bo->vm_bo = NULL; > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 81a79760ca61..51525b8774c9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -1468,19 +1468,13 @@ static struct drm_driver kms_driver = { .lastclose = amdgpu_driver_lastclose_kms, .irq_handler = amdgpu_irq_handler, .ioctls = amdgpu_ioctls_kms, - .gem_free_object_unlocked = amdgpu_gem_object_free, - .gem_open_object = amdgpu_gem_object_open, - .gem_close_object = amdgpu_gem_object_close, .dumb_create = amdgpu_mode_dumb_create, .dumb_map_offset = amdgpu_mode_dumb_mmap, .fops = &amdgpu_driver_kms_fops, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_export = amdgpu_gem_prime_export, .gem_prime_import = amdgpu_gem_prime_import, - .gem_prime_vmap = amdgpu_gem_prime_vmap, - .gem_prime_vunmap = amdgpu_gem_prime_vunmap, .gem_prime_mmap = amdgpu_gem_prime_mmap, .name = DRIVER_NAME, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 43f4966331dd..ca2b79f94e99 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -36,6 +36,7 @@ #include <drm/amdgpu_drm.h> #include <drm/drm_cache.h> #include "amdgpu.h" +#include "amdgpu_dma_buf.h" #include "amdgpu_trace.h" #include "amdgpu_amdkfd.h" @@ -510,6 +511,15 @@ bool amdgpu_bo_support_uswc(u64 bo_flags) #endif } +static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = { + .free = amdgpu_gem_object_free, + .open = amdgpu_gem_object_open, + .close = amdgpu_gem_object_close, + .export = amdgpu_gem_prime_export, + .vmap = amdgpu_gem_prime_vmap, + .vunmap = amdgpu_gem_prime_vunmap, +}; + static int amdgpu_bo_do_create(struct amdgpu_device *adev, struct amdgpu_bo_param *bp, struct amdgpu_bo **bo_ptr) @@ -552,6 +562,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL); if (bo == NULL) return -ENOMEM; + + bo->tbo.base.funcs = &amdgpu_gem_object_funcs; drm_gem_private_object_init(adev->ddev, &bo->tbo.base, size); INIT_LIST_HEAD(&bo->shadow_list); bo->vm_bo = NULL;
GEM object functions deprecate several similar callback interfaces in struct drm_driver. This patch replaces the per-driver callbacks with per-instance callbacks in amdgpu. The only exception is gem_prime_mmap, which is non-trivial to convert. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ------ drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 12 ++++++++++++ 2 files changed, 12 insertions(+), 6 deletions(-)