dri-devel Archive mirror
 help / color / mirror / Atom feed
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
Cc: <dri-devel@lists.freedesktop.org>, <daniel@ffwll.ch>,
	<maarten.lankhorst@linux.intel.com>, <airlied@gmail.com>,
	<mripard@kernel.org>, <tzimmermann@suse.de>,
	<intel-xe@lists.freedesktop.org>,
	Thomas Hellstr_m <thomas.hellstrom@linux.intel.com>
Subject: Re: [PATCH v3 1/4] drm: add devm release action
Date: Mon, 22 Apr 2024 16:54:49 -0400	[thread overview]
Message-ID: <ZibOmWPr3pZXdoNM@intel.com> (raw)
In-Reply-To: <20240422065756.294679-2-aravind.iddamsetty@linux.intel.com>

On Mon, Apr 22, 2024 at 12:27:53PM +0530, Aravind Iddamsetty wrote:
> In scenarios where drm_dev_put is directly called by driver we want to
> release devm_drm_dev_init_release action associated with struct
> drm_device.
> 
> v2: Directly expose the original function, instead of introducing a
> helper (Rodrigo)
> 
> v3: add kernel-doc (Maxime Ripard)
> 
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Hellstr_m <thomas.hellstrom@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 

please avoid these empty lines here.... cc, rv-b, sign-offs, links,
etc are all in the same block.

> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_drv.c | 13 +++++++++++++
>  include/drm/drm_drv.h     |  2 ++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 243cacb3575c..9d0409165f1e 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -714,6 +714,19 @@ static int devm_drm_dev_init(struct device *parent,
>  					devm_drm_dev_init_release, dev);
>  }
>  
> +/**
> + * devm_drm_dev_release_action - Call the final release action of the device

Seeing the doc here gave me a second thought....

the original release should be renamed to _devm_drm_dev_release
and this should be called devm_drm_dev_release without the 'action' word.

> + * @dev: DRM device
> + *
> + * In scenarios where drm_dev_put is directly called by driver we want to release
> + * devm_drm_dev_init_release action associated with struct drm_device.

But also, this made me more confusing on why this is needed.
Why can't we call put and get back?

The next needs to make it clear on why we need to release in these
scenarios regarless of the number of counters. But do we really
want this?

Can we block the flr if we have users? Perhaps this is the reason
that on my side the flr was not that clean? beucase I had display
so I had clients connected?

> + */
> +void devm_drm_dev_release_action(struct drm_device *dev)
> +{
> +	devm_release_action(dev->dev, devm_drm_dev_init_release, dev);
> +}
> +EXPORT_SYMBOL(devm_drm_dev_release_action);
> +
>  void *__devm_drm_dev_alloc(struct device *parent,
>  			   const struct drm_driver *driver,
>  			   size_t size, size_t offset)
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 8878260d7529..fa9123684874 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -444,6 +444,8 @@ struct drm_driver {
>  	const struct file_operations *fops;
>  };
>  
> +void devm_drm_dev_release_action(struct drm_device *dev);
> +
>  void *__devm_drm_dev_alloc(struct device *parent,
>  			   const struct drm_driver *driver,
>  			   size_t size, size_t offset);
> -- 
> 2.25.1
> 

  reply	other threads:[~2024-04-22 20:55 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-22  6:57 [PATCH v4 0/4] drm/xe: Support PCIe FLR Aravind Iddamsetty
2024-04-22  6:57 ` [PATCH v3 1/4] drm: add devm release action Aravind Iddamsetty
2024-04-22 20:54   ` Rodrigo Vivi [this message]
2024-04-23  8:55     ` Aravind Iddamsetty
2024-04-23 17:42       ` Rodrigo Vivi
2024-04-24 11:30         ` Aravind Iddamsetty
2024-04-24 11:49         ` Maxime Ripard
2024-04-24 12:20           ` Rodrigo Vivi
2024-04-25 12:52             ` Maxime Ripard
2024-04-25 14:42               ` Aravind Iddamsetty
2024-04-24 11:51   ` Maxime Ripard
2024-04-24 12:36     ` Aravind Iddamsetty
2024-05-02 13:42       ` Maxime Ripard
2024-04-22  6:57 ` [PATCH 2/4] drm/xe: Save and restore PCI state Aravind Iddamsetty
2024-04-22  6:57 ` [PATCH v2 3/4] drm/xe: Extract xe_gt_idle() helper Aravind Iddamsetty
2024-04-22  6:57 ` [PATCH v3 4/4] drm/xe/FLR: Support PCIe FLR Aravind Iddamsetty
2024-04-23 15:04   ` Nilawar, Badal
2024-04-24  3:12     ` Aravind Iddamsetty
2024-04-24 11:12       ` Nilawar, Badal
2024-04-25  4:07         ` Aravind Iddamsetty
2024-04-23 23:49   ` Michał Winiarski
2024-04-24  5:12     ` Aravind Iddamsetty
2024-04-24 23:29       ` Michał Winiarski
2024-04-25  6:17         ` Aravind Iddamsetty
2024-04-26  0:53           ` Michał Winiarski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZibOmWPr3pZXdoNM@intel.com \
    --to=rodrigo.vivi@intel.com \
    --cc=airlied@gmail.com \
    --cc=aravind.iddamsetty@linux.intel.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=thomas.hellstrom@linux.intel.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).