This is a collection of improvements for GEM VRAM helpers. The patches were part of other patchsets that didn't make it into upstream, but the changes are nevertheless useful.
Thomas Zimmermann (3): drm/vram-helper: Integrate drm_gem_vram_init() into drm_gem_vram_create() drm/vram-helper: Set object function iff they are not provided by driver drm/vram-helper: Don't put new BOs into VRAM
drivers/gpu/drm/drm_gem_vram_helper.c | 88 ++++++++++++--------------- 1 file changed, 38 insertions(+), 50 deletions(-)
-- 2.28.0
The drm_gem_vram_create() function is the only caller of the internal helper drm_gem_vram_init(). Streamline the code by putting all code into the create function.
v2: * rebased onto latest drm-tip * fixed typo in commit message (Sam)
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de Acked-by: Sam Ravnborg sam@ravnborg.org --- drivers/gpu/drm/drm_gem_vram_helper.c | 77 ++++++++++----------------- 1 file changed, 29 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 375c79e23ca5..9fa7f0ec9308 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -167,52 +167,6 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo, } }
-/* - * Note that on error, drm_gem_vram_init will free the buffer object. - */ - -static int drm_gem_vram_init(struct drm_device *dev, - struct drm_gem_vram_object *gbo, - size_t size, unsigned long pg_align) -{ - struct drm_vram_mm *vmm = dev->vram_mm; - struct ttm_bo_device *bdev; - int ret; - size_t acc_size; - - if (WARN_ONCE(!vmm, "VRAM MM not initialized")) { - kfree(gbo); - return -EINVAL; - } - bdev = &vmm->bdev; - - gbo->bo.base.funcs = &drm_gem_vram_object_funcs; - - ret = drm_gem_object_init(dev, &gbo->bo.base, size); - if (ret) { - kfree(gbo); - return ret; - } - - acc_size = ttm_bo_dma_acc_size(bdev, size, sizeof(*gbo)); - - gbo->bo.bdev = bdev; - drm_gem_vram_placement(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM | - DRM_GEM_VRAM_PL_FLAG_SYSTEM); - - ret = ttm_bo_init(bdev, &gbo->bo, size, ttm_bo_type_device, - &gbo->placement, pg_align, false, acc_size, - NULL, NULL, ttm_buffer_object_destroy); - if (ret) - /* - * A failing ttm_bo_init will call ttm_buffer_object_destroy - * to release gbo->bo.base and kfree gbo. - */ - return ret; - - return 0; -} - /** * drm_gem_vram_create() - Creates a VRAM-backed GEM object * @dev: the DRM device @@ -228,7 +182,13 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, unsigned long pg_align) { struct drm_gem_vram_object *gbo; + struct drm_vram_mm *vmm = dev->vram_mm; + struct ttm_bo_device *bdev; int ret; + size_t acc_size; + + if (WARN_ONCE(!vmm, "VRAM MM not initialized")) + return ERR_PTR(-EINVAL);
if (dev->driver->gem_create_object) { struct drm_gem_object *gem = @@ -242,8 +202,29 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, return ERR_PTR(-ENOMEM); }
- ret = drm_gem_vram_init(dev, gbo, size, pg_align); - if (ret < 0) + gbo->bo.base.funcs = &drm_gem_vram_object_funcs; + + ret = drm_gem_object_init(dev, &gbo->bo.base, size); + if (ret) { + kfree(gbo); + return ERR_PTR(ret); + } + + bdev = &vmm->bdev; + acc_size = ttm_bo_dma_acc_size(bdev, size, sizeof(*gbo)); + + gbo->bo.bdev = bdev; + drm_gem_vram_placement(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM | + DRM_GEM_VRAM_PL_FLAG_SYSTEM); + + /* + * A failing ttm_bo_init will call ttm_buffer_object_destroy + * to release gbo->bo.base and kfree gbo. + */ + ret = ttm_bo_init(bdev, &gbo->bo, size, ttm_bo_type_device, + &gbo->placement, pg_align, false, acc_size, + NULL, NULL, ttm_buffer_object_destroy); + if (ret) return ERR_PTR(ret);
return gbo;
Don't override the GEM object functions unconditionally. If the driver sets the GEM functions, VRAM helpers will not set them. The idea has been taken from SHMEM helpers.
v2: * updated the commit message (Sam) * document the new feature for drm_gem_vram_create()
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_gem_vram_helper.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 9fa7f0ec9308..11315b9e8c03 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -173,6 +173,12 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo, * @size: the buffer size in bytes * @pg_align: the buffer's alignment in multiples of the page size * + * GEM objects are allocated by calling struct drm_driver.gem_create_object, + * if set. Otherwise kzalloc() will be used. Drivers can set their own GEM + * object functions in struct drm_driver.gem_create_object. If no functions + * are set, the new GEM object will use the default functions from GEM VRAM + * helpers. + * * Returns: * A new instance of &struct drm_gem_vram_object on success, or * an ERR_PTR()-encoded error code otherwise. @@ -182,6 +188,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, unsigned long pg_align) { struct drm_gem_vram_object *gbo; + struct drm_gem_object *gem; struct drm_vram_mm *vmm = dev->vram_mm; struct ttm_bo_device *bdev; int ret; @@ -191,8 +198,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, return ERR_PTR(-EINVAL);
if (dev->driver->gem_create_object) { - struct drm_gem_object *gem = - dev->driver->gem_create_object(dev, size); + gem = dev->driver->gem_create_object(dev, size); if (!gem) return ERR_PTR(-ENOMEM); gbo = drm_gem_vram_of_gem(gem); @@ -200,11 +206,13 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, gbo = kzalloc(sizeof(*gbo), GFP_KERNEL); if (!gbo) return ERR_PTR(-ENOMEM); + gem = &gbo->bo.base; }
- gbo->bo.base.funcs = &drm_gem_vram_object_funcs; + if (!gem->funcs) + gem->funcs = &drm_gem_vram_object_funcs;
- ret = drm_gem_object_init(dev, &gbo->bo.base, size); + ret = drm_gem_object_init(dev, gem, size); if (ret) { kfree(gbo); return ERR_PTR(ret);
Most drivers that use VRAM helpers have only a few MiB of framebuffer memory available. To reduce fragmentation, new BOs are now put into system memory by default. Only pin operations are allowed to move BOs into VRAM.
v2: * rebased onto latest drm-tip
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de --- drivers/gpu/drm/drm_gem_vram_helper.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 11315b9e8c03..9d85790e0c07 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -222,8 +222,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, acc_size = ttm_bo_dma_acc_size(bdev, size, sizeof(*gbo));
gbo->bo.bdev = bdev; - drm_gem_vram_placement(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM | - DRM_GEM_VRAM_PL_FLAG_SYSTEM); + drm_gem_vram_placement(gbo, DRM_GEM_VRAM_PL_FLAG_SYSTEM);
/* * A failing ttm_bo_init will call ttm_buffer_object_destroy
On Tue, Sep 22, 2020 at 04:56:57PM +0200, Thomas Zimmermann wrote:
This is a collection of improvements for GEM VRAM helpers. The patches were part of other patchsets that didn't make it into upstream, but the changes are nevertheless useful.
On the series:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Thomas Zimmermann (3): drm/vram-helper: Integrate drm_gem_vram_init() into drm_gem_vram_create() drm/vram-helper: Set object function iff they are not provided by driver drm/vram-helper: Don't put new BOs into VRAM
drivers/gpu/drm/drm_gem_vram_helper.c | 88 ++++++++++++--------------- 1 file changed, 38 insertions(+), 50 deletions(-)
-- 2.28.0
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Thomas Zimmermann