| Message ID | 20200714083238.28479-4-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vram-helper: Update GEM VRAM object creation | expand |
On Tue, Jul 14, 2020 at 10:32:38AM +0200, Thomas Zimmermann wrote: > Don't override the GEM object functions unconditionally. If the driver > sets the GEM functions, VRAM helpers will now them. The idea has been s/now/own > taken from SHMEM helpers. If drivers need special versions of some of > the GEM functions, they can now override them. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/drm_gem_vram_helper.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c > index af767d3da5da..7194144610cb 100644 > --- a/drivers/gpu/drm/drm_gem_vram_helper.c > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c > @@ -190,6 +190,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, > unsigned long pg_align) The documentation of drm_gem_vram_create() could really use some love here. It should document the behavior around gem_create_object(), and the default allocation of drm_gem_vram_object with no drm_gem_object assigned etc. Sam > { > 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; > @@ -199,8 +200,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); > @@ -208,11 +208,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); > -- > 2.27.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Sam Am 16.07.20 um 22:11 schrieb Sam Ravnborg: > On Tue, Jul 14, 2020 at 10:32:38AM +0200, Thomas Zimmermann wrote: >> Don't override the GEM object functions unconditionally. If the driver >> sets the GEM functions, VRAM helpers will now them. The idea has been > s/now/own Ooops, I forgot a word. This should have been 'will now use them'. >> taken from SHMEM helpers. If drivers need special versions of some of >> the GEM functions, they can now override them. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/drm_gem_vram_helper.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c >> index af767d3da5da..7194144610cb 100644 >> --- a/drivers/gpu/drm/drm_gem_vram_helper.c >> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c >> @@ -190,6 +190,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, >> unsigned long pg_align) > > The documentation of drm_gem_vram_create() could really use some love > here. It should document the behavior around gem_create_object(), and > the default allocation of drm_gem_vram_object with no drm_gem_object > assigned etc. Sure Best regards Thomas > > Sam > > >> { >> 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; >> @@ -199,8 +200,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); >> @@ -208,11 +208,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); >> -- >> 2.27.0 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index af767d3da5da..7194144610cb 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -190,6 +190,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; @@ -199,8 +200,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); @@ -208,11 +208,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);
Don't override the GEM object functions unconditionally. If the driver sets the GEM functions, VRAM helpers will now them. The idea has been taken from SHMEM helpers. If drivers need special versions of some of the GEM functions, they can now override them. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_gem_vram_helper.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)