From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Douglas Anderson <dianders@chromium.org>
Cc: Maxime Ripard <mripard@kernel.org>,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [RFT PATCH 01/15] drm/armada: Call drm_atomic_helper_shutdown() at shutdown time
Date: Sun, 3 Sep 2023 16:53:42 +0100 [thread overview]
Message-ID: <ZPSsBhbekKY7VyDg@shell.armlinux.org.uk> (raw)
In-Reply-To: <20230901164111.RFT.1.I3d5598bd73a59b5ded71430736c93f67dc5dea61@changeid>
On Fri, Sep 01, 2023 at 04:41:12PM -0700, Douglas Anderson wrote:
> Based on grepping through the source code this driver appears to be
> missing a call to drm_atomic_helper_shutdown() at system shutdown
> time. Among other things, this means that if a panel is in use that it
> won't be cleanly powered off at system shutdown time.
>
> The fact that we should call drm_atomic_helper_shutdown() in the case
> of OS shutdown/restart comes straight out of the kernel doc "driver
> instance overview" in drm_drv.c.
>
> This driver was fairly easy to update. The drm_device is stored in the
> drvdata so we just have to make sure the drvdata is NULL whenever the
> device is not bound.
... and there I think you have a misunderstanding of the driver model.
Please have a look at device_unbind_cleanup() which will be called if
probe fails, or when the device is removed (in other words, when it is
not bound to a driver.)
Also, devices which aren't bound to a driver won't have their shutdown
method called (because there is no driver currently bound to that
device.) So, ->probe must have completed successfully, and ->remove
must not have been called for that device.
So, I think that all these dev_set_drvdata(dev, NULL) that you're
adding are just asking for a kernel janitor to come along later and
remove them because they serve no purpose... so best not introduce
them in the first place.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
WARNING: multiple messages have this Message-ID (diff)
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Douglas Anderson <dianders@chromium.org>
Cc: dri-devel@lists.freedesktop.org,
Maxime Ripard <mripard@kernel.org>,
airlied@gmail.com, daniel@ffwll.ch, linux-kernel@vger.kernel.org
Subject: Re: [RFT PATCH 01/15] drm/armada: Call drm_atomic_helper_shutdown() at shutdown time
Date: Sun, 3 Sep 2023 16:53:42 +0100 [thread overview]
Message-ID: <ZPSsBhbekKY7VyDg@shell.armlinux.org.uk> (raw)
In-Reply-To: <20230901164111.RFT.1.I3d5598bd73a59b5ded71430736c93f67dc5dea61@changeid>
On Fri, Sep 01, 2023 at 04:41:12PM -0700, Douglas Anderson wrote:
> Based on grepping through the source code this driver appears to be
> missing a call to drm_atomic_helper_shutdown() at system shutdown
> time. Among other things, this means that if a panel is in use that it
> won't be cleanly powered off at system shutdown time.
>
> The fact that we should call drm_atomic_helper_shutdown() in the case
> of OS shutdown/restart comes straight out of the kernel doc "driver
> instance overview" in drm_drv.c.
>
> This driver was fairly easy to update. The drm_device is stored in the
> drvdata so we just have to make sure the drvdata is NULL whenever the
> device is not bound.
... and there I think you have a misunderstanding of the driver model.
Please have a look at device_unbind_cleanup() which will be called if
probe fails, or when the device is removed (in other words, when it is
not bound to a driver.)
Also, devices which aren't bound to a driver won't have their shutdown
method called (because there is no driver currently bound to that
device.) So, ->probe must have completed successfully, and ->remove
must not have been called for that device.
So, I think that all these dev_set_drvdata(dev, NULL) that you're
adding are just asking for a kernel janitor to come along later and
remove them because they serve no purpose... so best not introduce
them in the first place.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2023-09-03 15:53 UTC|newest]
Thread overview: 141+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-01 23:41 [RFT PATCH 00/15] drm: non-drm-misc drivers call drm_atomic_helper_shutdown() at the right times Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` [Nouveau] " Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` [RFT PATCH 01/15] drm/armada: Call drm_atomic_helper_shutdown() at shutdown time Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-03 15:53 ` Russell King (Oracle) [this message]
2023-09-03 15:53 ` Russell King (Oracle)
2023-09-04 7:36 ` Maxime Ripard
2023-09-04 7:36 ` Maxime Ripard
2023-09-04 7:55 ` Russell King (Oracle)
2023-09-04 7:55 ` Russell King (Oracle)
2023-09-04 15:18 ` Maxime Ripard
2023-09-04 15:18 ` Maxime Ripard
2023-09-05 14:23 ` Doug Anderson
2023-09-05 14:23 ` Doug Anderson
2023-09-13 15:34 ` Doug Anderson
2023-09-13 15:34 ` Doug Anderson
2023-09-20 18:03 ` Doug Anderson
2023-09-20 18:03 ` Doug Anderson
2023-09-20 18:58 ` Russell King (Oracle)
2023-09-20 18:58 ` Russell King (Oracle)
2023-09-21 18:46 ` Doug Anderson
2023-09-21 18:46 ` Doug Anderson
2023-09-04 7:53 ` Maxime Ripard
2023-09-04 7:53 ` Maxime Ripard
2023-09-04 7:56 ` Russell King (Oracle)
2023-09-04 7:56 ` Russell King (Oracle)
2023-09-01 23:41 ` [RFT PATCH 02/15] drm/imx/dcss: " Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:36 ` Maxime Ripard
2023-09-04 7:36 ` Maxime Ripard
2023-09-04 7:36 ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 03/15] drm/ingenic: " Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:36 ` Maxime Ripard
2023-09-04 7:36 ` Maxime Ripard
2023-09-04 9:15 ` Paul Cercueil
2023-09-04 9:15 ` Paul Cercueil
2023-09-05 20:16 ` Doug Anderson
2023-09-05 20:16 ` Doug Anderson
2023-09-06 8:39 ` Maxime Ripard
2023-09-06 8:39 ` Maxime Ripard
2023-09-13 16:23 ` Doug Anderson
2023-09-13 16:23 ` Doug Anderson
2023-09-14 8:14 ` Maxime Ripard
2023-09-14 8:14 ` Maxime Ripard
2023-09-14 22:29 ` Doug Anderson
2023-09-14 22:29 ` Doug Anderson
2023-09-19 9:33 ` Maxime Ripard
2023-09-19 9:33 ` Maxime Ripard
2023-09-13 16:25 ` Doug Anderson
2023-09-13 16:25 ` Doug Anderson
2023-09-13 18:00 ` Paul Cercueil
2023-09-13 18:00 ` Paul Cercueil
2023-09-13 18:21 ` Doug Anderson
2023-09-13 18:21 ` Doug Anderson
2023-09-01 23:41 ` [RFT PATCH 04/15] drm/kmb: " Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:26 ` Maxime Ripard
2023-09-04 7:26 ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 05/15] drm/mediatek: " Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:27 ` Maxime Ripard
2023-09-04 7:27 ` Maxime Ripard
2023-09-04 7:27 ` Maxime Ripard
2023-09-08 11:51 ` Fei Shao
2023-09-08 11:51 ` Fei Shao
2023-09-08 11:51 ` Fei Shao
2023-09-11 16:10 ` Doug Anderson
2023-09-11 16:10 ` Doug Anderson
2023-09-11 16:10 ` Doug Anderson
2023-09-12 6:28 ` Fei Shao
2023-09-12 6:28 ` Fei Shao
2023-09-12 6:28 ` Fei Shao
2023-09-01 23:41 ` [Nouveau] [RFT PATCH 06/15] drm/nouveau: Call drm_atomic_helper_shutdown() or equiv " Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:27 ` [Nouveau] " Maxime Ripard
2023-09-04 7:27 ` Maxime Ripard
2023-09-04 7:27 ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 07/15] drm/tegra: Call drm_atomic_helper_shutdown() " Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:28 ` Maxime Ripard
2023-09-04 7:28 ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 08/15] drm/arcpgu: " Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:28 ` Maxime Ripard
2023-09-04 7:28 ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 09/15] drm/amdgpu: " Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` [RFT PATCH 10/15] drm/sprd: Call drm_atomic_helper_shutdown() at remove time Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:54 ` Maxime Ripard
2023-09-04 7:54 ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 11/15] drm/exynos: Call drm_atomic_helper_shutdown() at shutdown/unbind time Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:54 ` Maxime Ripard
2023-09-04 7:54 ` Maxime Ripard
2023-09-04 7:54 ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 12/15] drm/gma500: Call drm_helper_force_disable_all() at shutdown/remove time Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:29 ` Maxime Ripard
2023-09-04 7:29 ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 13/15] drm/imx/ipuv3: Call drm_atomic_helper_shutdown() at shutdown/unbind time Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:55 ` Maxime Ripard
2023-09-04 7:55 ` Maxime Ripard
2023-09-04 7:55 ` Maxime Ripard
2023-09-04 8:30 ` Philipp Zabel
2023-09-04 8:30 ` Philipp Zabel
2023-09-04 8:30 ` Philipp Zabel
2023-09-05 20:29 ` Doug Anderson
2023-09-05 20:29 ` Doug Anderson
2023-09-05 20:29 ` Doug Anderson
2023-09-06 5:47 ` Philipp Zabel
2023-09-06 5:47 ` Philipp Zabel
2023-09-06 5:47 ` Philipp Zabel
2023-09-06 14:30 ` Doug Anderson
2023-09-06 14:30 ` Doug Anderson
2023-09-06 14:30 ` Doug Anderson
2023-09-13 18:21 ` Doug Anderson
2023-09-13 18:21 ` Doug Anderson
2023-09-13 18:21 ` Doug Anderson
2023-09-01 23:41 ` [RFT PATCH 14/15] drm/radeon: Call drm_helper_force_disable_all() at shutdown/remove time Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:30 ` Maxime Ripard
2023-09-04 7:30 ` Maxime Ripard
2023-09-04 7:30 ` Maxime Ripard
2023-09-01 23:41 ` [RFT PATCH 15/15] drm/renesas/shmobile: " Douglas Anderson
2023-09-01 23:41 ` Douglas Anderson
2023-09-04 7:27 ` Geert Uytterhoeven
2023-09-04 7:27 ` Geert Uytterhoeven
2023-09-05 21:10 ` Doug Anderson
2023-09-05 21:10 ` Doug Anderson
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=ZPSsBhbekKY7VyDg@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=dianders@chromium.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mripard@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.