linux-kernel - Re: [PATCH v2] drm/mediatek: Call drm_atomic_helper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAAOTY_8pSGOhuRt=swb7o8zm6swnuXqyzPeASRppGCfi032xbQ@mail.gmail.com>
Date: Thu, 27 Jun 2024 22:02:27 +0800
From: Chun-Kuang Hu <chunkuang.hu@...nel.org>
To: Douglas Anderson <dianders@...omium.org>
Cc: Chun-Kuang Hu <chunkuang.hu@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>, 
	Linus Walleij <linus.walleij@...aro.org>, Neil Armstrong <neil.armstrong@...aro.org>, 
	Fei Shao <fshao@...omium.org>, Maxime Ripard <mripard@...nel.org>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Daniel Vetter <daniel@...ll.ch>, 
	David Airlie <airlied@...il.com>, Matthias Brugger <matthias.bgg@...il.com>, 
	dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v2] drm/mediatek: Call drm_atomic_helper_shutdown() at
 shutdown time

Hi, Douglas

Douglas Anderson <dianders@...omium.org> 於 2024年6月12日 週三 上午1:28寫道：
>
> 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 users the component model and shutdown happens in the base
> driver. The "drvdata" for this driver will always be valid if
> shutdown() is called and as of commit 2a073968289d
> ("drm/atomic-helper: drm_atomic_helper_shutdown(NULL) should be a
> noop") we don't need to confirm that "drm" is non-NULL.

Applied to mediatek-drm-next [1], thanks.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-next

>
> Suggested-by: Maxime Ripard <mripard@...nel.org>
> Reviewed-by: Maxime Ripard <mripard@...nel.org>
> Reviewed-by: Fei Shao <fshao@...omium.org>
> Tested-by: Fei Shao <fshao@...omium.org>
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> ---
> v1 of this patch was part of a series [1]. It got tested and reviewed
> but never landed. Reposting separately in the hopes that Mediatek DRM
> folks will land it. If, instead, Mediatek DRM folks want to Ack it I'm
> happy to land through drm-misc.
>
> I noticed that this was missing when I failed to add "mediatek" to my
> patch series IDing which DRM modeset drivers did this properly [2].
> Assuming my patch lands, that means that Mediatek devices will start
> getting more warnings printed.
>
> [1] https://lore.kernel.org/r/20230901164111.RFT.5.I2b014f90afc4729b6ecc7b5ddd1f6dedcea4625b@changeid
> [2] https://lore.kernel.org/r/20240611074846.1.Ieb287c2c3ee3f6d3b0d5f49b29f746b93621749c@changeid
>
> Changes in v2:
> - Removed NULL check since it's not needed since 6.7
> - Rebased on ToT.
>
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index b5f605751b0a..de811e2265da 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -952,6 +952,13 @@ static void mtk_drm_remove(struct platform_device *pdev)
>                 of_node_put(private->comp_node[i]);
>  }
>
> +static void mtk_drm_shutdown(struct platform_device *pdev)
> +{
> +       struct mtk_drm_private *private = platform_get_drvdata(pdev);
> +
> +       drm_atomic_helper_shutdown(private->drm);
> +}
> +
>  static int mtk_drm_sys_prepare(struct device *dev)
>  {
>         struct mtk_drm_private *private = dev_get_drvdata(dev);
> @@ -983,6 +990,7 @@ static const struct dev_pm_ops mtk_drm_pm_ops = {
>  static struct platform_driver mtk_drm_platform_driver = {
>         .probe  = mtk_drm_probe,
>         .remove_new = mtk_drm_remove,
> +       .shutdown = mtk_drm_shutdown,
>         .driver = {
>                 .name   = "mediatek-drm",
>                 .pm     = &mtk_drm_pm_ops,
> --
> 2.45.2.505.gda0bf45e8d-goog
>