On Fri, May 10, 2024 at 07:12:14PM +0100, Matthew Auld wrote:
> Hopefully make it clearer when to use devm vs drmm.
>
> Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> ---
> drivers/gpu/drm/drm_managed.c | 42 +++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> index 7646f67bda4e..20d705bbc0a3 100644
> --- a/drivers/gpu/drm/drm_managed.c
> +++ b/drivers/gpu/drm/drm_managed.c
> @@ -34,6 +34,48 @@
> * during the lifetime of the driver, all the functions are fully concurrent
> * safe. But it is recommended to use managed resources only for resources that
> * change rarely, if ever, during the lifetime of the &drm_device instance.
> + *
> + * Note that the distinction between devm and drmm is important to get right.
> + * Consider some hotunplug scenarios, where it is valid for there to be multiple
> + * unplugged struct &drm_device instances each being kept alive by an open
> + * driver fd. The driver needs a clean separation between what needs to happen
> + * when the struct &device is removed and what needs to happen when a given
> + * struct &drm_device instance is released, as well as in some cases a more
> + * finer grained marking of critical sections that require hardware interaction.
> + * See below.
> + *
> + * devm
> + * ~~~~
> + * In general use devm for cleaning up anything hardware related. So removing
> + * pci mmaps, releasing interrupt handlers, basically anything hw related. The
^
Extra space.
> + * devm release actions are called when the struct &device is removed, shortly
> + * after calling into the drivers struct &pci_driver.remove() callback, if this
> + * is a pci device.
> + *
> + * devm can be thought of as an alternative to putting all the hw related
nit: perhaps s/thought/seen ?
> + * cleanup directly in the struct &pci_driver.remove() callback, where the
> + * correct ordering of the unwind steps needs to be manually done in the error
> + * path of the struct &pci_driver.probe() and again on the remove side. With
> + * devm this is all done automatically.
> + *
> + * drmm
> + * ~~~~
> + * In general use this for cleaning up anything software related. So data
> + * structures and the like which are tied to the lifetime of a particular struct
> + * &drm_device instance.
> + *
> + * drmm can be thought of as an alternative to putting all the software related
nit: perhaps s/thought/seen ?
> + * cleanup directly in the struct &drm_driver.release() callback, where again
> + * the correct ordering of the unwind steps needs to be done manually. As with
> + * devm this is instead done automatically.
> + *
> + * Sometimes there is no clean separation between software and hardware, which
> + * is where drm_dev_enter() comes in. For example, a driver might have some
> + * state tied to a struct &drm_device instance, for which the same cleanup path
> + * is called for both a plugged and unplugged device, and the cleanup itself
> + * might require talking to the device if it's still attached to this particular
> + * struct &drm_device. For that we instead mark the device sections. See
> + * drm_dev_enter(), drm_dev_exit() and drm_dev_unplug().
perhaps open up a bit more here?
anyway, everything looks good to me.
Sima, thoughts?
> */
>
> struct drmres_node {
> --
> 2.45.0
>
