* [PATCH 0/3] drm/vram-helper: Various cleanups
@ 2019-12-11 18:08 Thomas Zimmermann
2019-12-11 18:08 ` [PATCH 1/3] drm/vram-helper: Remove interruptible flag from public interface Thomas Zimmermann
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Thomas Zimmermann @ 2019-12-11 18:08 UTC (permalink / raw)
To: airlied, daniel, maarten.lankhorst, mripard, z.liuxinliang,
zourongrong, kong.kongxinwei, puck.chen, kraxel, sam
Cc: Thomas Zimmermann, dri-devel
A number of cleanups that I wanted to apply for some time. The first
two patches simplify the public interface. The third patch adds support
for struct drm_driver.gem_create_object. All tested by running fbdev,
X11 and Weston on ast HW.
Thomas Zimmermann (3):
drm/vram-helper: Remove interruptible flag from public interface
drm/vram-helper: Remove BO device from public interface
drm/vram-helper: Support struct drm_driver.gem_create_object
drivers/gpu/drm/ast/ast_mode.c | 3 +-
drivers/gpu/drm/drm_gem_vram_helper.c | 43 ++++++++++-----------
drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 2 +-
drivers/gpu/drm/mgag200/mgag200_cursor.c | 3 +-
drivers/gpu/drm/mgag200/mgag200_drv.c | 3 +-
include/drm/drm_gem_vram_helper.h | 6 +--
6 files changed, 26 insertions(+), 34 deletions(-)
--
2.24.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] drm/vram-helper: Remove interruptible flag from public interface 2019-12-11 18:08 [PATCH 0/3] drm/vram-helper: Various cleanups Thomas Zimmermann @ 2019-12-11 18:08 ` Thomas Zimmermann 2019-12-13 10:05 ` Daniel Vetter 2019-12-11 18:08 ` [PATCH 2/3] drm/vram-helper: Remove BO device " Thomas Zimmermann 2019-12-11 18:08 ` [PATCH 3/3] drm/vram-helper: Support struct drm_driver.gem_create_object Thomas Zimmermann 2 siblings, 1 reply; 9+ messages in thread From: Thomas Zimmermann @ 2019-12-11 18:08 UTC (permalink / raw) To: airlied, daniel, maarten.lankhorst, mripard, z.liuxinliang, zourongrong, kong.kongxinwei, puck.chen, kraxel, sam Cc: Thomas Zimmermann, dri-devel The flag 'interruptible', which is passed to various functions, is always set to be false. Remove it and hard-code the value. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Suggested-by: Daniel Vetter <daniel@ffwll.ch> --- drivers/gpu/drm/ast/ast_mode.c | 2 +- drivers/gpu/drm/drm_gem_vram_helper.c | 17 ++++++----------- drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 2 +- drivers/gpu/drm/mgag200/mgag200_cursor.c | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.c | 2 +- include/drm/drm_gem_vram_helper.h | 4 +--- 6 files changed, 11 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index b879bd666e35..26336642dd59 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1145,7 +1145,7 @@ static int ast_cursor_init(struct drm_device *dev) for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) { gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, - size, 0, false); + size, 0); if (IS_ERR(gbo)) { ret = PTR_ERR(gbo); goto err_drm_gem_vram_put; diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 666cb4c22bb9..4908f1281002 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -94,8 +94,7 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo, static int drm_gem_vram_init(struct drm_device *dev, struct ttm_bo_device *bdev, struct drm_gem_vram_object *gbo, - size_t size, unsigned long pg_align, - bool interruptible) + size_t size, unsigned long pg_align) { int ret; size_t acc_size; @@ -112,7 +111,7 @@ static int drm_gem_vram_init(struct drm_device *dev, drm_gem_vram_placement(gbo, TTM_PL_FLAG_VRAM | TTM_PL_FLAG_SYSTEM); ret = ttm_bo_init(bdev, &gbo->bo, size, ttm_bo_type_device, - &gbo->placement, pg_align, interruptible, acc_size, + &gbo->placement, pg_align, false, acc_size, NULL, NULL, ttm_buffer_object_destroy); if (ret) goto err_drm_gem_object_release; @@ -130,7 +129,6 @@ static int drm_gem_vram_init(struct drm_device *dev, * @bdev: the TTM BO device backing the object * @size: the buffer size in bytes * @pg_align: the buffer's alignment in multiples of the page size - * @interruptible: sleep interruptible if waiting for memory * * Returns: * A new instance of &struct drm_gem_vram_object on success, or @@ -139,8 +137,7 @@ static int drm_gem_vram_init(struct drm_device *dev, struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, struct ttm_bo_device *bdev, size_t size, - unsigned long pg_align, - bool interruptible) + unsigned long pg_align) { struct drm_gem_vram_object *gbo; int ret; @@ -149,7 +146,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, if (!gbo) return ERR_PTR(-ENOMEM); - ret = drm_gem_vram_init(dev, bdev, gbo, size, pg_align, interruptible); + ret = drm_gem_vram_init(dev, bdev, gbo, size, pg_align); if (ret < 0) goto err_kfree; @@ -485,7 +482,6 @@ EXPORT_SYMBOL(drm_gem_vram_vunmap); * @dev: the DRM device * @bdev: the TTM BO device managing the buffer object * @pg_align: the buffer's alignment in multiples of the page size - * @interruptible: sleep interruptible if waiting for memory * @args: the arguments as provided to \ &struct drm_driver.dumb_create * @@ -502,7 +498,6 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file, struct drm_device *dev, struct ttm_bo_device *bdev, unsigned long pg_align, - bool interruptible, struct drm_mode_create_dumb *args) { size_t pitch, size; @@ -517,7 +512,7 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file, if (!size) return -EINVAL; - gbo = drm_gem_vram_create(dev, bdev, size, pg_align, interruptible); + gbo = drm_gem_vram_create(dev, bdev, size, pg_align); if (IS_ERR(gbo)) return PTR_ERR(gbo); @@ -613,7 +608,7 @@ int drm_gem_vram_driver_dumb_create(struct drm_file *file, return -EINVAL; return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev, 0, - false, args); + args); } EXPORT_SYMBOL(drm_gem_vram_driver_dumb_create); diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c index 21b684eab5c9..16f5cd2c1b3b 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c @@ -58,7 +58,7 @@ int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel, if (size == 0) return -EINVAL; - gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, size, 0, false); + gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, size, 0); if (IS_ERR(gbo)) { ret = PTR_ERR(gbo); if (ret != -ERESTARTSYS) diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c index 5444cf1573a3..dd54fd507e13 100644 --- a/drivers/gpu/drm/mgag200/mgag200_cursor.c +++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c @@ -209,7 +209,7 @@ int mgag200_cursor_init(struct mga_device *mdev) for (i = 0; i < ncursors; ++i) { gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, - size, 0, false); + size, 0); if (IS_ERR(gbo)) { ret = PTR_ERR(gbo); goto err_drm_gem_vram_put; diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 9f4f5f071add..dc125838f5d1 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -121,7 +121,7 @@ int mgag200_driver_dumb_create(struct drm_file *file, pg_align = PFN_UP(mdev->mc.vram_size); return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev, - pg_align, false, args); + pg_align, args); } static struct drm_driver driver = { diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h index 08adaf3695ea..90736427285e 100644 --- a/include/drm/drm_gem_vram_helper.h +++ b/include/drm/drm_gem_vram_helper.h @@ -95,8 +95,7 @@ static inline struct drm_gem_vram_object *drm_gem_vram_of_gem( struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, struct ttm_bo_device *bdev, size_t size, - unsigned long pg_align, - bool interruptible); + unsigned long pg_align); void drm_gem_vram_put(struct drm_gem_vram_object *gbo); u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo); s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo); @@ -112,7 +111,6 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file, struct drm_device *dev, struct ttm_bo_device *bdev, unsigned long pg_align, - bool interruptible, struct drm_mode_create_dumb *args); /* -- 2.24.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] drm/vram-helper: Remove interruptible flag from public interface 2019-12-11 18:08 ` [PATCH 1/3] drm/vram-helper: Remove interruptible flag from public interface Thomas Zimmermann @ 2019-12-13 10:05 ` Daniel Vetter 0 siblings, 0 replies; 9+ messages in thread From: Daniel Vetter @ 2019-12-13 10:05 UTC (permalink / raw) To: Thomas Zimmermann Cc: puck.chen, dri-devel, z.liuxinliang, kong.kongxinwei, kraxel, zourongrong, airlied, sam On Wed, Dec 11, 2019 at 07:08:30PM +0100, Thomas Zimmermann wrote: > The flag 'interruptible', which is passed to various functions, > is always set to be false. Remove it and hard-code the value. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Suggested-by: Daniel Vetter <daniel@ffwll.ch> Yeah generally we try to make everything interruptible when we except that we might stall waiting for hw. So display flip code, or when you have an actual render engine. For vram helpers I don't see a need for this, everything here should completely briskly. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Maybe throw a doc patch on top to explain this with a sentence or so? -Daniel > --- > drivers/gpu/drm/ast/ast_mode.c | 2 +- > drivers/gpu/drm/drm_gem_vram_helper.c | 17 ++++++----------- > drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 2 +- > drivers/gpu/drm/mgag200/mgag200_cursor.c | 2 +- > drivers/gpu/drm/mgag200/mgag200_drv.c | 2 +- > include/drm/drm_gem_vram_helper.h | 4 +--- > 6 files changed, 11 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c > index b879bd666e35..26336642dd59 100644 > --- a/drivers/gpu/drm/ast/ast_mode.c > +++ b/drivers/gpu/drm/ast/ast_mode.c > @@ -1145,7 +1145,7 @@ static int ast_cursor_init(struct drm_device *dev) > > for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) { > gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, > - size, 0, false); > + size, 0); > if (IS_ERR(gbo)) { > ret = PTR_ERR(gbo); > goto err_drm_gem_vram_put; > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c > index 666cb4c22bb9..4908f1281002 100644 > --- a/drivers/gpu/drm/drm_gem_vram_helper.c > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c > @@ -94,8 +94,7 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo, > static int drm_gem_vram_init(struct drm_device *dev, > struct ttm_bo_device *bdev, > struct drm_gem_vram_object *gbo, > - size_t size, unsigned long pg_align, > - bool interruptible) > + size_t size, unsigned long pg_align) > { > int ret; > size_t acc_size; > @@ -112,7 +111,7 @@ static int drm_gem_vram_init(struct drm_device *dev, > drm_gem_vram_placement(gbo, TTM_PL_FLAG_VRAM | TTM_PL_FLAG_SYSTEM); > > ret = ttm_bo_init(bdev, &gbo->bo, size, ttm_bo_type_device, > - &gbo->placement, pg_align, interruptible, acc_size, > + &gbo->placement, pg_align, false, acc_size, > NULL, NULL, ttm_buffer_object_destroy); > if (ret) > goto err_drm_gem_object_release; > @@ -130,7 +129,6 @@ static int drm_gem_vram_init(struct drm_device *dev, > * @bdev: the TTM BO device backing the object > * @size: the buffer size in bytes > * @pg_align: the buffer's alignment in multiples of the page size > - * @interruptible: sleep interruptible if waiting for memory > * > * Returns: > * A new instance of &struct drm_gem_vram_object on success, or > @@ -139,8 +137,7 @@ static int drm_gem_vram_init(struct drm_device *dev, > struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, > struct ttm_bo_device *bdev, > size_t size, > - unsigned long pg_align, > - bool interruptible) > + unsigned long pg_align) > { > struct drm_gem_vram_object *gbo; > int ret; > @@ -149,7 +146,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, > if (!gbo) > return ERR_PTR(-ENOMEM); > > - ret = drm_gem_vram_init(dev, bdev, gbo, size, pg_align, interruptible); > + ret = drm_gem_vram_init(dev, bdev, gbo, size, pg_align); > if (ret < 0) > goto err_kfree; > > @@ -485,7 +482,6 @@ EXPORT_SYMBOL(drm_gem_vram_vunmap); > * @dev: the DRM device > * @bdev: the TTM BO device managing the buffer object > * @pg_align: the buffer's alignment in multiples of the page size > - * @interruptible: sleep interruptible if waiting for memory > * @args: the arguments as provided to \ > &struct drm_driver.dumb_create > * > @@ -502,7 +498,6 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file, > struct drm_device *dev, > struct ttm_bo_device *bdev, > unsigned long pg_align, > - bool interruptible, > struct drm_mode_create_dumb *args) > { > size_t pitch, size; > @@ -517,7 +512,7 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file, > if (!size) > return -EINVAL; > > - gbo = drm_gem_vram_create(dev, bdev, size, pg_align, interruptible); > + gbo = drm_gem_vram_create(dev, bdev, size, pg_align); > if (IS_ERR(gbo)) > return PTR_ERR(gbo); > > @@ -613,7 +608,7 @@ int drm_gem_vram_driver_dumb_create(struct drm_file *file, > return -EINVAL; > > return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev, 0, > - false, args); > + args); > } > EXPORT_SYMBOL(drm_gem_vram_driver_dumb_create); > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c > index 21b684eab5c9..16f5cd2c1b3b 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c > @@ -58,7 +58,7 @@ int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel, > if (size == 0) > return -EINVAL; > > - gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, size, 0, false); > + gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, size, 0); > if (IS_ERR(gbo)) { > ret = PTR_ERR(gbo); > if (ret != -ERESTARTSYS) > diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c > index 5444cf1573a3..dd54fd507e13 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_cursor.c > +++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c > @@ -209,7 +209,7 @@ int mgag200_cursor_init(struct mga_device *mdev) > > for (i = 0; i < ncursors; ++i) { > gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, > - size, 0, false); > + size, 0); > if (IS_ERR(gbo)) { > ret = PTR_ERR(gbo); > goto err_drm_gem_vram_put; > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c > index 9f4f5f071add..dc125838f5d1 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.c > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c > @@ -121,7 +121,7 @@ int mgag200_driver_dumb_create(struct drm_file *file, > pg_align = PFN_UP(mdev->mc.vram_size); > > return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev, > - pg_align, false, args); > + pg_align, args); > } > > static struct drm_driver driver = { > diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h > index 08adaf3695ea..90736427285e 100644 > --- a/include/drm/drm_gem_vram_helper.h > +++ b/include/drm/drm_gem_vram_helper.h > @@ -95,8 +95,7 @@ static inline struct drm_gem_vram_object *drm_gem_vram_of_gem( > struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, > struct ttm_bo_device *bdev, > size_t size, > - unsigned long pg_align, > - bool interruptible); > + unsigned long pg_align); > void drm_gem_vram_put(struct drm_gem_vram_object *gbo); > u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo); > s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo); > @@ -112,7 +111,6 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file, > struct drm_device *dev, > struct ttm_bo_device *bdev, > unsigned long pg_align, > - bool interruptible, > struct drm_mode_create_dumb *args); > > /* > -- > 2.24.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] drm/vram-helper: Remove BO device from public interface 2019-12-11 18:08 [PATCH 0/3] drm/vram-helper: Various cleanups Thomas Zimmermann 2019-12-11 18:08 ` [PATCH 1/3] drm/vram-helper: Remove interruptible flag from public interface Thomas Zimmermann @ 2019-12-11 18:08 ` Thomas Zimmermann 2019-12-13 10:08 ` Daniel Vetter 2019-12-11 18:08 ` [PATCH 3/3] drm/vram-helper: Support struct drm_driver.gem_create_object Thomas Zimmermann 2 siblings, 1 reply; 9+ messages in thread From: Thomas Zimmermann @ 2019-12-11 18:08 UTC (permalink / raw) To: airlied, daniel, maarten.lankhorst, mripard, z.liuxinliang, zourongrong, kong.kongxinwei, puck.chen, kraxel, sam Cc: Thomas Zimmermann, dri-devel TTM is an implementation detail of the VRAM helpers and therefore shouldn't be exposed to the callers. There's only one correct value for the BO device anyway, which is the one stored in the DRM device. So remove struct ttm_bo_device from the VRAM-helper interface and use the device's VRAM manager unconditionally. The GEM initializer function fails if the VRAM manager has not been initialized. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/ast/ast_mode.c | 3 +-- drivers/gpu/drm/drm_gem_vram_helper.c | 21 +++++++++------------ drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 2 +- drivers/gpu/drm/mgag200/mgag200_cursor.c | 3 +-- drivers/gpu/drm/mgag200/mgag200_drv.c | 3 +-- include/drm/drm_gem_vram_helper.h | 2 -- 6 files changed, 13 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 26336642dd59..f4fbdab29bb7 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1144,8 +1144,7 @@ static int ast_cursor_init(struct drm_device *dev) size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE); for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) { - gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, - size, 0); + gbo = drm_gem_vram_create(dev, size, 0); if (IS_ERR(gbo)) { ret = PTR_ERR(gbo); goto err_drm_gem_vram_put; diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 4908f1281002..b760fd27f3c0 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -92,13 +92,18 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo, } static int drm_gem_vram_init(struct drm_device *dev, - struct ttm_bo_device *bdev, 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")) + return -EINVAL; + bdev = &vmm->bdev; + gbo->bo.base.funcs = &drm_gem_vram_object_funcs; ret = drm_gem_object_init(dev, &gbo->bo.base, size); @@ -126,7 +131,6 @@ static int drm_gem_vram_init(struct drm_device *dev, /** * drm_gem_vram_create() - Creates a VRAM-backed GEM object * @dev: the DRM device - * @bdev: the TTM BO device backing the object * @size: the buffer size in bytes * @pg_align: the buffer's alignment in multiples of the page size * @@ -135,7 +139,6 @@ static int drm_gem_vram_init(struct drm_device *dev, * an ERR_PTR()-encoded error code otherwise. */ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, - struct ttm_bo_device *bdev, size_t size, unsigned long pg_align) { @@ -146,7 +149,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, if (!gbo) return ERR_PTR(-ENOMEM); - ret = drm_gem_vram_init(dev, bdev, gbo, size, pg_align); + ret = drm_gem_vram_init(dev, gbo, size, pg_align); if (ret < 0) goto err_kfree; @@ -480,7 +483,6 @@ EXPORT_SYMBOL(drm_gem_vram_vunmap); Helper for implementing &struct drm_driver.dumb_create * @file: the DRM file * @dev: the DRM device - * @bdev: the TTM BO device managing the buffer object * @pg_align: the buffer's alignment in multiples of the page size * @args: the arguments as provided to \ &struct drm_driver.dumb_create @@ -496,7 +498,6 @@ EXPORT_SYMBOL(drm_gem_vram_vunmap); */ int drm_gem_vram_fill_create_dumb(struct drm_file *file, struct drm_device *dev, - struct ttm_bo_device *bdev, unsigned long pg_align, struct drm_mode_create_dumb *args) { @@ -512,7 +513,7 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file, if (!size) return -EINVAL; - gbo = drm_gem_vram_create(dev, bdev, size, pg_align); + gbo = drm_gem_vram_create(dev, size, pg_align); if (IS_ERR(gbo)) return PTR_ERR(gbo); @@ -604,11 +605,7 @@ int drm_gem_vram_driver_dumb_create(struct drm_file *file, struct drm_device *dev, struct drm_mode_create_dumb *args) { - if (WARN_ONCE(!dev->vram_mm, "VRAM MM not initialized")) - return -EINVAL; - - return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev, 0, - args); + return drm_gem_vram_fill_create_dumb(file, dev, 0, args); } EXPORT_SYMBOL(drm_gem_vram_driver_dumb_create); diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c index 16f5cd2c1b3b..4a5bde80045a 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c @@ -58,7 +58,7 @@ int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel, if (size == 0) return -EINVAL; - gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, size, 0); + gbo = drm_gem_vram_create(dev, size, 0); if (IS_ERR(gbo)) { ret = PTR_ERR(gbo); if (ret != -ERESTARTSYS) diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c index dd54fd507e13..d491edd317ff 100644 --- a/drivers/gpu/drm/mgag200/mgag200_cursor.c +++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c @@ -208,8 +208,7 @@ int mgag200_cursor_init(struct mga_device *mdev) return -ENOMEM; for (i = 0; i < ncursors; ++i) { - gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, - size, 0); + gbo = drm_gem_vram_create(dev, size, 0); if (IS_ERR(gbo)) { ret = PTR_ERR(gbo); goto err_drm_gem_vram_put; diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index dc125838f5d1..1b5b60da2e62 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -120,8 +120,7 @@ int mgag200_driver_dumb_create(struct drm_file *file, if (mgag200_pin_bo_at_0(mdev)) pg_align = PFN_UP(mdev->mc.vram_size); - return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev, - pg_align, args); + return drm_gem_vram_fill_create_dumb(file, dev, pg_align, args); } static struct drm_driver driver = { diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h index 90736427285e..9341f54383b3 100644 --- a/include/drm/drm_gem_vram_helper.h +++ b/include/drm/drm_gem_vram_helper.h @@ -93,7 +93,6 @@ static inline struct drm_gem_vram_object *drm_gem_vram_of_gem( } struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, - struct ttm_bo_device *bdev, size_t size, unsigned long pg_align); void drm_gem_vram_put(struct drm_gem_vram_object *gbo); @@ -109,7 +108,6 @@ void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, void *vaddr); int drm_gem_vram_fill_create_dumb(struct drm_file *file, struct drm_device *dev, - struct ttm_bo_device *bdev, unsigned long pg_align, struct drm_mode_create_dumb *args); -- 2.24.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] drm/vram-helper: Remove BO device from public interface 2019-12-11 18:08 ` [PATCH 2/3] drm/vram-helper: Remove BO device " Thomas Zimmermann @ 2019-12-13 10:08 ` Daniel Vetter 0 siblings, 0 replies; 9+ messages in thread From: Daniel Vetter @ 2019-12-13 10:08 UTC (permalink / raw) To: Thomas Zimmermann Cc: puck.chen, dri-devel, z.liuxinliang, kong.kongxinwei, kraxel, zourongrong, airlied, sam On Wed, Dec 11, 2019 at 07:08:31PM +0100, Thomas Zimmermann wrote: > TTM is an implementation detail of the VRAM helpers and therefore > shouldn't be exposed to the callers. There's only one correct value > for the BO device anyway, which is the one stored in the DRM device. > > So remove struct ttm_bo_device from the VRAM-helper interface and > use the device's VRAM manager unconditionally. The GEM initializer > function fails if the VRAM manager has not been initialized. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> If/when someone is really bored I think might be really nice to create something like drm_device.gem (or .mm) like we have for drm_device.mode_config ... drm_device is an unwieldy monster. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/ast/ast_mode.c | 3 +-- > drivers/gpu/drm/drm_gem_vram_helper.c | 21 +++++++++------------ > drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 2 +- > drivers/gpu/drm/mgag200/mgag200_cursor.c | 3 +-- > drivers/gpu/drm/mgag200/mgag200_drv.c | 3 +-- > include/drm/drm_gem_vram_helper.h | 2 -- > 6 files changed, 13 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c > index 26336642dd59..f4fbdab29bb7 100644 > --- a/drivers/gpu/drm/ast/ast_mode.c > +++ b/drivers/gpu/drm/ast/ast_mode.c > @@ -1144,8 +1144,7 @@ static int ast_cursor_init(struct drm_device *dev) > size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE); > > for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) { > - gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, > - size, 0); > + gbo = drm_gem_vram_create(dev, size, 0); > if (IS_ERR(gbo)) { > ret = PTR_ERR(gbo); > goto err_drm_gem_vram_put; > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c > index 4908f1281002..b760fd27f3c0 100644 > --- a/drivers/gpu/drm/drm_gem_vram_helper.c > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c > @@ -92,13 +92,18 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo, > } > > static int drm_gem_vram_init(struct drm_device *dev, > - struct ttm_bo_device *bdev, > 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")) > + return -EINVAL; > + bdev = &vmm->bdev; > + > gbo->bo.base.funcs = &drm_gem_vram_object_funcs; > > ret = drm_gem_object_init(dev, &gbo->bo.base, size); > @@ -126,7 +131,6 @@ static int drm_gem_vram_init(struct drm_device *dev, > /** > * drm_gem_vram_create() - Creates a VRAM-backed GEM object > * @dev: the DRM device > - * @bdev: the TTM BO device backing the object > * @size: the buffer size in bytes > * @pg_align: the buffer's alignment in multiples of the page size > * > @@ -135,7 +139,6 @@ static int drm_gem_vram_init(struct drm_device *dev, > * an ERR_PTR()-encoded error code otherwise. > */ > struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, > - struct ttm_bo_device *bdev, > size_t size, > unsigned long pg_align) > { > @@ -146,7 +149,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, > if (!gbo) > return ERR_PTR(-ENOMEM); > > - ret = drm_gem_vram_init(dev, bdev, gbo, size, pg_align); > + ret = drm_gem_vram_init(dev, gbo, size, pg_align); > if (ret < 0) > goto err_kfree; > > @@ -480,7 +483,6 @@ EXPORT_SYMBOL(drm_gem_vram_vunmap); > Helper for implementing &struct drm_driver.dumb_create > * @file: the DRM file > * @dev: the DRM device > - * @bdev: the TTM BO device managing the buffer object > * @pg_align: the buffer's alignment in multiples of the page size > * @args: the arguments as provided to \ > &struct drm_driver.dumb_create > @@ -496,7 +498,6 @@ EXPORT_SYMBOL(drm_gem_vram_vunmap); > */ > int drm_gem_vram_fill_create_dumb(struct drm_file *file, > struct drm_device *dev, > - struct ttm_bo_device *bdev, > unsigned long pg_align, > struct drm_mode_create_dumb *args) > { > @@ -512,7 +513,7 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file, > if (!size) > return -EINVAL; > > - gbo = drm_gem_vram_create(dev, bdev, size, pg_align); > + gbo = drm_gem_vram_create(dev, size, pg_align); > if (IS_ERR(gbo)) > return PTR_ERR(gbo); > > @@ -604,11 +605,7 @@ int drm_gem_vram_driver_dumb_create(struct drm_file *file, > struct drm_device *dev, > struct drm_mode_create_dumb *args) > { > - if (WARN_ONCE(!dev->vram_mm, "VRAM MM not initialized")) > - return -EINVAL; > - > - return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev, 0, > - args); > + return drm_gem_vram_fill_create_dumb(file, dev, 0, args); > } > EXPORT_SYMBOL(drm_gem_vram_driver_dumb_create); > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c > index 16f5cd2c1b3b..4a5bde80045a 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c > @@ -58,7 +58,7 @@ int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel, > if (size == 0) > return -EINVAL; > > - gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, size, 0); > + gbo = drm_gem_vram_create(dev, size, 0); > if (IS_ERR(gbo)) { > ret = PTR_ERR(gbo); > if (ret != -ERESTARTSYS) > diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c > index dd54fd507e13..d491edd317ff 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_cursor.c > +++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c > @@ -208,8 +208,7 @@ int mgag200_cursor_init(struct mga_device *mdev) > return -ENOMEM; > > for (i = 0; i < ncursors; ++i) { > - gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, > - size, 0); > + gbo = drm_gem_vram_create(dev, size, 0); > if (IS_ERR(gbo)) { > ret = PTR_ERR(gbo); > goto err_drm_gem_vram_put; > diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c > index dc125838f5d1..1b5b60da2e62 100644 > --- a/drivers/gpu/drm/mgag200/mgag200_drv.c > +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c > @@ -120,8 +120,7 @@ int mgag200_driver_dumb_create(struct drm_file *file, > if (mgag200_pin_bo_at_0(mdev)) > pg_align = PFN_UP(mdev->mc.vram_size); > > - return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev, > - pg_align, args); > + return drm_gem_vram_fill_create_dumb(file, dev, pg_align, args); > } > > static struct drm_driver driver = { > diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h > index 90736427285e..9341f54383b3 100644 > --- a/include/drm/drm_gem_vram_helper.h > +++ b/include/drm/drm_gem_vram_helper.h > @@ -93,7 +93,6 @@ static inline struct drm_gem_vram_object *drm_gem_vram_of_gem( > } > > struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, > - struct ttm_bo_device *bdev, > size_t size, > unsigned long pg_align); > void drm_gem_vram_put(struct drm_gem_vram_object *gbo); > @@ -109,7 +108,6 @@ void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, void *vaddr); > > int drm_gem_vram_fill_create_dumb(struct drm_file *file, > struct drm_device *dev, > - struct ttm_bo_device *bdev, > unsigned long pg_align, > struct drm_mode_create_dumb *args); > > -- > 2.24.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] drm/vram-helper: Support struct drm_driver.gem_create_object 2019-12-11 18:08 [PATCH 0/3] drm/vram-helper: Various cleanups Thomas Zimmermann 2019-12-11 18:08 ` [PATCH 1/3] drm/vram-helper: Remove interruptible flag from public interface Thomas Zimmermann 2019-12-11 18:08 ` [PATCH 2/3] drm/vram-helper: Remove BO device " Thomas Zimmermann @ 2019-12-11 18:08 ` Thomas Zimmermann 2019-12-11 19:11 ` Sam Ravnborg 2 siblings, 1 reply; 9+ messages in thread From: Thomas Zimmermann @ 2019-12-11 18:08 UTC (permalink / raw) To: airlied, daniel, maarten.lankhorst, mripard, z.liuxinliang, zourongrong, kong.kongxinwei, puck.chen, kraxel, sam Cc: Thomas Zimmermann, dri-devel Drivers that what to allocate VRAM GEM objects with additional fields can now do this by implementing struct drm_driver.gem_create_object. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_gem_vram_helper.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index b760fd27f3c0..d475d94e2e3e 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -2,6 +2,7 @@ #include <drm/drm_debugfs.h> #include <drm/drm_device.h> +#include <drm/drm_drv.h> #include <drm/drm_file.h> #include <drm/drm_framebuffer.h> #include <drm/drm_gem_ttm_helper.h> @@ -142,13 +143,19 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, size_t size, unsigned long pg_align) { + struct drm_gem_object *gem; struct drm_gem_vram_object *gbo; int ret; - gbo = kzalloc(sizeof(*gbo), GFP_KERNEL); - if (!gbo) + if (dev->driver->gem_create_object) + gem = dev->driver->gem_create_object(dev, size); + else + gem = kzalloc(sizeof(*gbo), GFP_KERNEL); + if (!gem) return ERR_PTR(-ENOMEM); + gbo = drm_gem_vram_of_gem(gem); + ret = drm_gem_vram_init(dev, gbo, size, pg_align); if (ret < 0) goto err_kfree; -- 2.24.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] drm/vram-helper: Support struct drm_driver.gem_create_object 2019-12-11 18:08 ` [PATCH 3/3] drm/vram-helper: Support struct drm_driver.gem_create_object Thomas Zimmermann @ 2019-12-11 19:11 ` Sam Ravnborg 2019-12-12 5:39 ` Thomas Zimmermann 0 siblings, 1 reply; 9+ messages in thread From: Sam Ravnborg @ 2019-12-11 19:11 UTC (permalink / raw) To: Thomas Zimmermann Cc: puck.chen, dri-devel, z.liuxinliang, kong.kongxinwei, kraxel, zourongrong, airlied Hi Thomas, On Wed, Dec 11, 2019 at 07:08:32PM +0100, Thomas Zimmermann wrote: > Drivers that what to allocate VRAM GEM objects with additional fields > can now do this by implementing struct drm_driver.gem_create_object. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/drm_gem_vram_helper.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c > index b760fd27f3c0..d475d94e2e3e 100644 > --- a/drivers/gpu/drm/drm_gem_vram_helper.c > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c > @@ -2,6 +2,7 @@ > > #include <drm/drm_debugfs.h> > #include <drm/drm_device.h> > +#include <drm/drm_drv.h> > #include <drm/drm_file.h> > #include <drm/drm_framebuffer.h> > #include <drm/drm_gem_ttm_helper.h> > @@ -142,13 +143,19 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, > size_t size, > unsigned long pg_align) > { > + struct drm_gem_object *gem; > struct drm_gem_vram_object *gbo; > int ret; > > - gbo = kzalloc(sizeof(*gbo), GFP_KERNEL); > - if (!gbo) > + if (dev->driver->gem_create_object) > + gem = dev->driver->gem_create_object(dev, size); > + else > + gem = kzalloc(sizeof(*gbo), GFP_KERNEL); The size is (*gbo) but you assume it is a gem. Looks wrong at first glance??? Sam > + if (!gem) > return ERR_PTR(-ENOMEM); > > + gbo = drm_gem_vram_of_gem(gem); > + > ret = drm_gem_vram_init(dev, gbo, size, pg_align); > if (ret < 0) > goto err_kfree; > -- > 2.24.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] drm/vram-helper: Support struct drm_driver.gem_create_object 2019-12-11 19:11 ` Sam Ravnborg @ 2019-12-12 5:39 ` Thomas Zimmermann 2019-12-13 10:23 ` Daniel Vetter 0 siblings, 1 reply; 9+ messages in thread From: Thomas Zimmermann @ 2019-12-12 5:39 UTC (permalink / raw) To: Sam Ravnborg Cc: puck.chen, dri-devel, z.liuxinliang, kong.kongxinwei, kraxel, zourongrong, airlied [-- Attachment #1.1.1: Type: text/plain, Size: 2296 bytes --] Hi Sam Am 11.12.19 um 20:11 schrieb Sam Ravnborg: > Hi Thomas, > > On Wed, Dec 11, 2019 at 07:08:32PM +0100, Thomas Zimmermann wrote: >> Drivers that what to allocate VRAM GEM objects with additional fields >> can now do this by implementing struct drm_driver.gem_create_object. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/drm_gem_vram_helper.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c >> index b760fd27f3c0..d475d94e2e3e 100644 >> --- a/drivers/gpu/drm/drm_gem_vram_helper.c >> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c >> @@ -2,6 +2,7 @@ >> >> #include <drm/drm_debugfs.h> >> #include <drm/drm_device.h> >> +#include <drm/drm_drv.h> >> #include <drm/drm_file.h> >> #include <drm/drm_framebuffer.h> >> #include <drm/drm_gem_ttm_helper.h> >> @@ -142,13 +143,19 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, >> size_t size, >> unsigned long pg_align) >> { >> + struct drm_gem_object *gem; >> struct drm_gem_vram_object *gbo; >> int ret; >> >> - gbo = kzalloc(sizeof(*gbo), GFP_KERNEL); >> - if (!gbo) >> + if (dev->driver->gem_create_object) >> + gem = dev->driver->gem_create_object(dev, size); >> + else >> + gem = kzalloc(sizeof(*gbo), GFP_KERNEL); > The size is (*gbo) but you assume it is a gem. > Looks wrong at first glance??? The conversion to gbo is... > > Sam > > >> + if (!gem) >> return ERR_PTR(-ENOMEM); >> >> + gbo = drm_gem_vram_of_gem(gem); >> + ...here. This could be pushed into the if branch, but I found it more readable to put the statement here. For the else branch, it doesn't matter as 'gem' goes first in the structure. I'll move it into the if branch, so it's more obvious what's going on and doesn't break accidentally. Best regards Thomas >> ret = drm_gem_vram_init(dev, gbo, size, pg_align); >> if (ret < 0) >> goto err_kfree; >> -- >> 2.24.0 -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] drm/vram-helper: Support struct drm_driver.gem_create_object 2019-12-12 5:39 ` Thomas Zimmermann @ 2019-12-13 10:23 ` Daniel Vetter 0 siblings, 0 replies; 9+ messages in thread From: Daniel Vetter @ 2019-12-13 10:23 UTC (permalink / raw) To: Thomas Zimmermann Cc: puck.chen, dri-devel, z.liuxinliang, kong.kongxinwei, kraxel, zourongrong, airlied, Sam Ravnborg On Thu, Dec 12, 2019 at 06:39:04AM +0100, Thomas Zimmermann wrote: > Hi Sam > > Am 11.12.19 um 20:11 schrieb Sam Ravnborg: > > Hi Thomas, > > > > On Wed, Dec 11, 2019 at 07:08:32PM +0100, Thomas Zimmermann wrote: > >> Drivers that what to allocate VRAM GEM objects with additional fields > >> can now do this by implementing struct drm_driver.gem_create_object. > >> > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > >> --- > >> drivers/gpu/drm/drm_gem_vram_helper.c | 11 +++++++++-- > >> 1 file changed, 9 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c > >> index b760fd27f3c0..d475d94e2e3e 100644 > >> --- a/drivers/gpu/drm/drm_gem_vram_helper.c > >> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c > >> @@ -2,6 +2,7 @@ > >> > >> #include <drm/drm_debugfs.h> > >> #include <drm/drm_device.h> > >> +#include <drm/drm_drv.h> > >> #include <drm/drm_file.h> > >> #include <drm/drm_framebuffer.h> > >> #include <drm/drm_gem_ttm_helper.h> > >> @@ -142,13 +143,19 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, > >> size_t size, > >> unsigned long pg_align) > >> { > >> + struct drm_gem_object *gem; > >> struct drm_gem_vram_object *gbo; > >> int ret; > >> > >> - gbo = kzalloc(sizeof(*gbo), GFP_KERNEL); > >> - if (!gbo) > >> + if (dev->driver->gem_create_object) > >> + gem = dev->driver->gem_create_object(dev, size); > >> + else > >> + gem = kzalloc(sizeof(*gbo), GFP_KERNEL); > > The size is (*gbo) but you assume it is a gem. > > Looks wrong at first glance??? > > The conversion to gbo is... > > > > > Sam > > > > > >> + if (!gem) > >> return ERR_PTR(-ENOMEM); > >> > >> + gbo = drm_gem_vram_of_gem(gem); > >> + > > ...here. This could be pushed into the if branch, but I found it more > readable to put the statement here. For the else branch, it doesn't > matter as 'gem' goes first in the structure. > > I'll move it into the if branch, so it's more obvious what's going on > and doesn't break accidentally. If you push this into the if() branch them imo also push the gem temp variable there. I think that would be quite readable again. Also, probably means you need to push the if (!gem) check too, and make it if (!gbo) in the else branch. Either way: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Best regards > Thomas > > >> ret = drm_gem_vram_init(dev, gbo, size, pg_align); > >> if (ret < 0) > >> goto err_kfree; > >> -- > >> 2.24.0 > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-12-13 10:23 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-12-11 18:08 [PATCH 0/3] drm/vram-helper: Various cleanups Thomas Zimmermann 2019-12-11 18:08 ` [PATCH 1/3] drm/vram-helper: Remove interruptible flag from public interface Thomas Zimmermann 2019-12-13 10:05 ` Daniel Vetter 2019-12-11 18:08 ` [PATCH 2/3] drm/vram-helper: Remove BO device " Thomas Zimmermann 2019-12-13 10:08 ` Daniel Vetter 2019-12-11 18:08 ` [PATCH 3/3] drm/vram-helper: Support struct drm_driver.gem_create_object Thomas Zimmermann 2019-12-11 19:11 ` Sam Ravnborg 2019-12-12 5:39 ` Thomas Zimmermann 2019-12-13 10:23 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.