| 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
| ||
|
Message-ID: <aM1w4URkELrmK7No@lizhi-Precision-Tower-5810>
Date: Fri, 19 Sep 2025 11:04:01 -0400
From: Frank Li <Frank.li@....com>
To: Liu Ying <victor.liu@....com>
Cc: Philipp Zabel <p.zabel@...gutronix.de>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
Dmitry Baryshkov <lumag@...nel.org>,
dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/14] drm/imx: dc-crtc: Disable at boot
On Fri, Jul 04, 2025 at 05:03:52PM +0800, Liu Ying wrote:
> CRTC(s) could still be running after the DRM device is unplugged by
> calling drm_dev_unplug(), because the CRTC disablement logic is
> protected and bypassed by the drm_dev_enter()/drm_dev_exit() pair.
> Hence, Pixel Engine's AXI clock use count(managed by Pixel Engine
> driver's runtime PM) and pixel clock use count could be inbalanced
> after removing and re-installing the driver module. To fix this,
> add a helper dc_crtc_disable_at_boot() and call it to properly
> disable all CRTCs before advertising DRM device to user-space by
> calling drm_dev_register().
>
> Fixes: 711a3b878366 ("drm/imx: Add i.MX8qxp Display Controller KMS")
> Signed-off-by: Liu Ying <victor.liu@....com>
> ---
> drivers/gpu/drm/imx/dc/dc-crtc.c | 50 ++++++++++++++++++++++++++++++++++++----
> drivers/gpu/drm/imx/dc/dc-drv.c | 5 ++++
> drivers/gpu/drm/imx/dc/dc-drv.h | 3 +++
> 3 files changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/imx/dc/dc-crtc.c b/drivers/gpu/drm/imx/dc/dc-crtc.c
> index 31d3a982deaf7a0390937285c9d5d00100323181..45a87df1ad6a8bd768aa5ed38d6f03f14052b3d7 100644
> --- a/drivers/gpu/drm/imx/dc/dc-crtc.c
> +++ b/drivers/gpu/drm/imx/dc/dc-crtc.c
> @@ -293,6 +293,16 @@ dc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state)
> dc_crtc_queue_state_event(new_crtc_state);
> }
>
> +static inline void __dc_crtc_disable_fg(struct drm_crtc *crtc)
> +{
> + struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> +
> + enable_irq(dc_crtc->irq_dec_seqcomplete);
> + dc_fg_disable(dc_crtc->fg);
> + DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(dec_seqcomplete_done);
> + disable_irq(dc_crtc->irq_dec_seqcomplete);
> +}
> +
> static void
> dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *state)
> {
> @@ -305,11 +315,7 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *state)
> if (!drm_dev_enter(crtc->dev, &idx))
> goto out;
>
> - enable_irq(dc_crtc->irq_dec_seqcomplete);
> - dc_fg_disable(dc_crtc->fg);
> - DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(dec_seqcomplete_done);
> - disable_irq(dc_crtc->irq_dec_seqcomplete);
> -
> + __dc_crtc_disable_fg(crtc);
> dc_fg_disable_clock(dc_crtc->fg);
>
> /* request pixel engine power-off as plane is off too */
> @@ -337,6 +343,40 @@ dc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *state)
> spin_unlock_irq(&crtc->dev->event_lock);
> }
>
> +void dc_crtc_disable_at_boot(struct drm_crtc *crtc)
> +{
> + struct dc_drm_device *dc_drm = to_dc_drm_device(crtc->dev);
> + struct dc_crtc *dc_crtc = to_dc_crtc(crtc);
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(dc_crtc->de->dev);
> + if (ret < 0) {
> + dc_crtc_err(crtc, "failed to get DC display engine RPM: %d\n",
> + ret);
> + return;
> + }
> +
> + if (!dc_fg_wait_for_frame_index_moving(dc_crtc->fg)) {
> + dc_crtc_dbg(crtc, "FrameGen frame index isn't moving\n");
> + goto out;
> + }
> +
> + dc_crtc_dbg(crtc, "disabling at boot\n");
> + __dc_crtc_disable_fg(crtc);
> + dc_fg_disable_clock(dc_crtc->fg);
> +
> + if (!dc_drm->pe_clk_axi_disabled) {
> + clk_disable_unprepare(dc_drm->pe->clk_axi);
> + dc_drm->pe_clk_axi_disabled = true;
> + }
look like dc_crtc_disable_at_boot() call by dc_drm_bind(), does it call
twice without call unbind()? Most like other place paired function have
not correct.
Frank
> +
> +out:
> + ret = pm_runtime_put(dc_crtc->de->dev);
> + if (ret < 0)
> + dc_crtc_err(crtc, "failed to put DC display engine RPM: %d\n",
> + ret);
> +}
> +
> static bool dc_crtc_get_scanout_position(struct drm_crtc *crtc,
> bool in_vblank_irq,
> int *vpos, int *hpos,
> diff --git a/drivers/gpu/drm/imx/dc/dc-drv.c b/drivers/gpu/drm/imx/dc/dc-drv.c
> index 04f021d2d6cfc93972aa8d9073be24d347152602..f93766b6bfbfae8510db05278d104820ca0719c4 100644
> --- a/drivers/gpu/drm/imx/dc/dc-drv.c
> +++ b/drivers/gpu/drm/imx/dc/dc-drv.c
> @@ -17,6 +17,7 @@
>
> #include <drm/clients/drm_client_setup.h>
> #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_fbdev_dma.h>
> #include <drm/drm_fourcc.h>
> @@ -96,6 +97,7 @@ static int dc_drm_bind(struct device *dev)
> struct dc_priv *priv = dev_get_drvdata(dev);
> struct dc_drm_device *dc_drm;
> struct drm_device *drm;
> + struct drm_crtc *crtc;
> int ret;
>
> dc_drm = devm_drm_dev_alloc(dev, &dc_drm_driver, struct dc_drm_device,
> @@ -118,6 +120,9 @@ static int dc_drm_bind(struct device *dev)
> if (ret)
> return ret;
>
> + drm_for_each_crtc(crtc, drm)
> + dc_crtc_disable_at_boot(crtc);
> +
> ret = drm_dev_register(drm, 0);
> if (ret) {
> dev_err(dev, "failed to register drm device: %d\n", ret);
> diff --git a/drivers/gpu/drm/imx/dc/dc-drv.h b/drivers/gpu/drm/imx/dc/dc-drv.h
> index eb61b8c7626933adc7688f046139e2167665dad1..68e99ba7cedbca1b8bdc0d8ced7a610a1056bfc7 100644
> --- a/drivers/gpu/drm/imx/dc/dc-drv.h
> +++ b/drivers/gpu/drm/imx/dc/dc-drv.h
> @@ -50,6 +50,8 @@ struct dc_drm_device {
> struct dc_pe *pe;
> /** @tc: tcon list */
> struct dc_tc *tc[DC_DISPLAYS];
> + /** @pe_clk_axi_disabled: disablement flag at boot */
> + bool pe_clk_axi_disabled;
> };
>
> struct dc_subdev_info {
> @@ -96,6 +98,7 @@ static inline int dc_subdev_get_id(const struct dc_subdev_info *info,
> return -EINVAL;
> }
>
> +void dc_crtc_disable_at_boot(struct drm_crtc *crtc);
> void dc_de_post_bind(struct dc_drm_device *dc_drm);
> void dc_pe_post_bind(struct dc_drm_device *dc_drm);
>
>
> --
> 2.34.1
>
Powered by blists - more mailing lists