Hi
Am 15.04.20 um 09:39 schrieb Daniel Vetter:
> Add a new macro helper to combine the usual init sequence in drivers,
> consisting of a kzalloc + devm_drm_dev_init + drmm_add_final_kfree
> triplet. This allows us to remove the rather unsightly
> drmm_add_final_kfree from all currently merged drivers.
>
> The kerneldoc is only added for this new function. Existing kerneldoc
> and examples will be udated at the very end, since once all drivers
> are converted over to devm_drm_dev_alloc we can unexport a lot of
> interim functions and make the documentation for driver authors a lot
> cleaner and less confusing. There will be only one true way to
> initialize a drm_device at the end of this, which is going to be
> devm_drm_dev_alloc.
>
> v2:
> - Actually explain what this is for in the commit message (Sam)
> - Fix checkpatch issues (Sam)
>
> Acked-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> Cc: Noralf Trønnes <noralf@xxxxxxxxxxx>
> Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Cc: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx>
> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
Sorry for being late. A number of nits are listed below. In any case:
Reviewed-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
Best regards
Thomas
> ---
> drivers/gpu/drm/drm_drv.c | 23 +++++++++++++++++++++++
> include/drm/drm_drv.h | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 56 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 1bb4f636b83c..8e1813d2a12e 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -739,6 +739,29 @@ int devm_drm_dev_init(struct device *parent,
> }
> EXPORT_SYMBOL(devm_drm_dev_init);
>
> +void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver,
> + size_t size, size_t offset)
Maybe rename 'offset' of 'dev_offset' to make the relationship clear.
> +{
> + void *container;
> + struct drm_device *drm;
> + int ret;
> +
> + container = kzalloc(size, GFP_KERNEL);
> + if (!container)
> + return ERR_PTR(-ENOMEM);
> +
> + drm = container + offset;
While convenient, I somewhat dislike the use of void* variables. I'd use
unsigned char* for container and do an explicit cast to struct
drm_device* here.
> + ret = devm_drm_dev_init(parent, drm, driver);
> + if (ret) {
> + kfree(container);
> + return ERR_PTR(ret);
> + }
> + drmm_add_final_kfree(drm, container);
> +
> + return container;
> +}
> +EXPORT_SYMBOL(__devm_drm_dev_alloc);
> +
> /**
> * drm_dev_alloc - Allocate new DRM device
> * @driver: DRM driver to allocate device for
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index e7c6ea261ed1..f07f15721254 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -626,6 +626,39 @@ int devm_drm_dev_init(struct device *parent,
> struct drm_device *dev,
> struct drm_driver *driver);
>
> +void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver,
> + size_t size, size_t offset);
> +
> +/**
> + * devm_drm_dev_alloc - Resource managed allocation of a &drm_device instance
> + * @parent: Parent device object
> + * @driver: DRM driver
> + * @type: the type of the struct which contains struct &drm_device
> + * @member: the name of the &drm_device within @type.
> + *
> + * This allocates and initialize a new DRM device. No device registration is done.
> + * Call drm_dev_register() to advertice the device to user space and register it
> + * with other core subsystems. This should be done last in the device
> + * initialization sequence to make sure userspace can't access an inconsistent
> + * state.
> + *
> + * The initial ref-count of the object is 1. Use drm_dev_get() and
> + * drm_dev_put() to take and drop further ref-counts.
> + *
> + * It is recommended that drivers embed &struct drm_device into their own device
> + * structure.
> + *
> + * Note that this manages the lifetime of the resulting &drm_device
> + * automatically using devres. The DRM device initialized with this function is
> + * automatically put on driver detach using drm_dev_put().
> + *
> + * RETURNS:
> + * Pointer to new DRM device, or ERR_PTR on failure.
> + */
> +#define devm_drm_dev_alloc(parent, driver, type, member) \
I'd replace 'member' with 'dev_field' to make the relation ship clear.
> + ((type *) __devm_drm_dev_alloc(parent, driver, sizeof(type), \
> + offsetof(type, member)))
> +
> struct drm_device *drm_dev_alloc(struct drm_driver *driver,
> struct device *parent);
> int drm_dev_register(struct drm_device *dev, unsigned long flags);
>
--
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:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
