| Message ID | 20250410235546.43736-2-dakr@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | DRM Rust abstractions | expand |
Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io> Le Fri , Apr 11, 2025 at 01:55:20AM +0200, Danilo Krummrich a écrit : > In the Rust DRM device abstraction we need to allocate a struct > drm_device. > > Currently, there are two options, the deprecated drm_dev_alloc() (which > does not support subclassing) and devm_drm_dev_alloc(). The latter > supports subclassing, but also manages the initial reference through > devres for the parent device. > > In Rust we want to conform with the subclassing pattern, but do not want > to get the initial reference managed for us, since Rust has its own, > idiomatic ways to properly deal with it. > > There are two options to achieve this. > > 1) Allocate the memory ourselves with a KBox. > 2) Implement __drm_dev_alloc(), which supports subclassing, but is > unmanged. > > While (1) would be possible, it would be cumbersome, since it would > require exporting drm_dev_init() and drmm_add_final_kfree(). > > Hence, go with option (2) and implement __drm_dev_alloc(). > > Reviewed-by: Maxime Ripard <mripard@kernel.org> > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > --- > drivers/gpu/drm/drm_drv.c | 58 ++++++++++++++++++++++++++++----------- > include/drm/drm_drv.h | 5 ++++ > 2 files changed, 47 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 17fc5dc708f4..ebb648f1c7a9 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -808,36 +808,62 @@ void *__devm_drm_dev_alloc(struct device *parent, > EXPORT_SYMBOL(__devm_drm_dev_alloc); > > /** > - * drm_dev_alloc - Allocate new DRM device > - * @driver: DRM driver to allocate device for > + * __drm_dev_alloc - Allocation of a &drm_device instance > * @parent: Parent device object > + * @driver: DRM driver > + * @size: the size of the struct which contains struct drm_device > + * @offset: the offset of the &drm_device within the container. > * > - * This is the deprecated version of devm_drm_dev_alloc(), which does not support > - * subclassing through embedding the struct &drm_device in a driver private > - * structure, and which does not support automatic cleanup through devres. > + * This should *NOT* be by any drivers, but is a dedicated interface for the > + * corresponding Rust abstraction. > * > - * RETURNS: > - * Pointer to new DRM device, or ERR_PTR on failure. > + * This is the same as devm_drm_dev_alloc(), but without the corresponding > + * resource management through the parent device, but not the same as > + * drm_dev_alloc(), since the latter is the deprecated version, which does not > + * support subclassing. > + * > + * Returns: A pointer to new DRM device, or an ERR_PTR on failure. > */ > -struct drm_device *drm_dev_alloc(const struct drm_driver *driver, > - struct device *parent) > +void *__drm_dev_alloc(struct device *parent, > + const struct drm_driver *driver, > + size_t size, size_t offset) > { > - struct drm_device *dev; > + void *container; > + struct drm_device *drm; > int ret; > > - dev = kzalloc(sizeof(*dev), GFP_KERNEL); > - if (!dev) > + container = kzalloc(size, GFP_KERNEL); > + if (!container) > return ERR_PTR(-ENOMEM); > > - ret = drm_dev_init(dev, driver, parent); > + drm = container + offset; > + ret = drm_dev_init(drm, driver, parent); > if (ret) { > - kfree(dev); > + kfree(container); > return ERR_PTR(ret); > } > + drmm_add_final_kfree(drm, container); > > - drmm_add_final_kfree(dev, dev); > + return container; > +} > +EXPORT_SYMBOL(__drm_dev_alloc); > > - return dev; > +/** > + * drm_dev_alloc - Allocate new DRM device > + * @driver: DRM driver to allocate device for > + * @parent: Parent device object > + * > + * This is the deprecated version of devm_drm_dev_alloc(), which does not support > + * subclassing through embedding the struct &drm_device in a driver private > + * structure, and which does not support automatic cleanup through devres. > + * > + * RETURNS: > + * Pointer to new DRM device, or ERR_PTR on failure. > + */ > +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, > + struct device *parent) > +{ > + return __drm_dev_alloc(parent, driver, sizeof(struct drm_device), 0); > } > EXPORT_SYMBOL(drm_dev_alloc); > > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index a43d707b5f36..63b51942d606 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -473,6 +473,11 @@ drmm_cgroup_register_region(struct drm_device *dev, > > struct drm_device *drm_dev_alloc(const struct drm_driver *driver, > struct device *parent); > + > +void *__drm_dev_alloc(struct device *parent, > + const struct drm_driver *driver, > + size_t size, size_t offset); > + > int drm_dev_register(struct drm_device *dev, unsigned long flags); > void drm_dev_unregister(struct drm_device *dev); > > -- > 2.49.0 >
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 17fc5dc708f4..ebb648f1c7a9 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -808,36 +808,62 @@ void *__devm_drm_dev_alloc(struct device *parent, EXPORT_SYMBOL(__devm_drm_dev_alloc); /** - * drm_dev_alloc - Allocate new DRM device - * @driver: DRM driver to allocate device for + * __drm_dev_alloc - Allocation of a &drm_device instance * @parent: Parent device object + * @driver: DRM driver + * @size: the size of the struct which contains struct drm_device + * @offset: the offset of the &drm_device within the container. * - * This is the deprecated version of devm_drm_dev_alloc(), which does not support - * subclassing through embedding the struct &drm_device in a driver private - * structure, and which does not support automatic cleanup through devres. + * This should *NOT* be by any drivers, but is a dedicated interface for the + * corresponding Rust abstraction. * - * RETURNS: - * Pointer to new DRM device, or ERR_PTR on failure. + * This is the same as devm_drm_dev_alloc(), but without the corresponding + * resource management through the parent device, but not the same as + * drm_dev_alloc(), since the latter is the deprecated version, which does not + * support subclassing. + * + * Returns: A pointer to new DRM device, or an ERR_PTR on failure. */ -struct drm_device *drm_dev_alloc(const struct drm_driver *driver, - struct device *parent) +void *__drm_dev_alloc(struct device *parent, + const struct drm_driver *driver, + size_t size, size_t offset) { - struct drm_device *dev; + void *container; + struct drm_device *drm; int ret; - dev = kzalloc(sizeof(*dev), GFP_KERNEL); - if (!dev) + container = kzalloc(size, GFP_KERNEL); + if (!container) return ERR_PTR(-ENOMEM); - ret = drm_dev_init(dev, driver, parent); + drm = container + offset; + ret = drm_dev_init(drm, driver, parent); if (ret) { - kfree(dev); + kfree(container); return ERR_PTR(ret); } + drmm_add_final_kfree(drm, container); - drmm_add_final_kfree(dev, dev); + return container; +} +EXPORT_SYMBOL(__drm_dev_alloc); - return dev; +/** + * drm_dev_alloc - Allocate new DRM device + * @driver: DRM driver to allocate device for + * @parent: Parent device object + * + * This is the deprecated version of devm_drm_dev_alloc(), which does not support + * subclassing through embedding the struct &drm_device in a driver private + * structure, and which does not support automatic cleanup through devres. + * + * RETURNS: + * Pointer to new DRM device, or ERR_PTR on failure. + */ +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, + struct device *parent) +{ + return __drm_dev_alloc(parent, driver, sizeof(struct drm_device), 0); } EXPORT_SYMBOL(drm_dev_alloc); diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index a43d707b5f36..63b51942d606 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -473,6 +473,11 @@ drmm_cgroup_register_region(struct drm_device *dev, struct drm_device *drm_dev_alloc(const struct drm_driver *driver, struct device *parent); + +void *__drm_dev_alloc(struct device *parent, + const struct drm_driver *driver, + size_t size, size_t offset); + int drm_dev_register(struct drm_device *dev, unsigned long flags); void drm_dev_unregister(struct drm_device *dev);
In the Rust DRM device abstraction we need to allocate a struct drm_device. Currently, there are two options, the deprecated drm_dev_alloc() (which does not support subclassing) and devm_drm_dev_alloc(). The latter supports subclassing, but also manages the initial reference through devres for the parent device. In Rust we want to conform with the subclassing pattern, but do not want to get the initial reference managed for us, since Rust has its own, idiomatic ways to properly deal with it. There are two options to achieve this. 1) Allocate the memory ourselves with a KBox. 2) Implement __drm_dev_alloc(), which supports subclassing, but is unmanged. While (1) would be possible, it would be cumbersome, since it would require exporting drm_dev_init() and drmm_add_final_kfree(). Hence, go with option (2) and implement __drm_dev_alloc(). Reviewed-by: Maxime Ripard <mripard@kernel.org> Signed-off-by: Danilo Krummrich <dakr@kernel.org> --- drivers/gpu/drm/drm_drv.c | 58 ++++++++++++++++++++++++++++----------- include/drm/drm_drv.h | 5 ++++ 2 files changed, 47 insertions(+), 16 deletions(-)