[PATCH v2 00/13] drm/crtc: Convert all drivers to atomic_create_state and remove reset

[PATCH v2 00/13] drm/crtc: Convert all drivers to atomic_create_state and remove reset

[Maxime Ripard]

This is a follow-up to the plane reset removal series, and part of a
larger effort to remove the reset hook from all KMS objects.

The CRTC reset hook is overloaded: it is called both at probe time to
create the initial software state and during suspend/resume to reset
hardware and software state. These two roles have different
requirements, and the reset hook is not fallible, making error
handling difficult for the initial state allocation path.

While reset has the semantics to reset both the software and hardware
state, the vast majority of implementations and all the helpers only
reset the software state, making them equivalent to
atomic_create_state in practice. The atomic_create_state hook makes
this explicit: it only allocates and initializes a pristine state
without any side effect, and returns the state pointer or an ERR_PTR
on failure.

This series converts all CRTC drivers tree-wide from the reset hook to
atomic_create_state, then removes the reset hook from struct
drm_crtc_funcs entirely. Two drivers (tilcdc and loongson) had actual
hardware reset logic mixed into their reset hook; those are moved to
CRTC creation instead. The conversions were done using a combination of
Coccinelle semantic patches and manual adjustments.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Changes in v2:
- Fix indentation in logicvc
- Fix hardware reset at probe and suspend for loonsoon and tilcldc
- Fix state creation for amdgpu
- Rebase on current drm-misc-next
- Fix commit message for st7920
- Fix merge conflict for mediatek
- Link to v1: https://lore.kernel.org/r/20260821-drm-no-more-crtc-reset-v1-0-fb793475c05a@kernel.org

---
Maxime Ripard (13):
      drm/crtc: Introduce hw_reset helper hook
      drm/amdgpu: vkms: Switch to drm_atomic_helper_crtc_create_state
      drm/logicvc: Switch to drm_atomic_helper_crtc_create_state
      drm/tilcdc: Move hardware reset to CRTC creation
      drm/tilcdc: Switch to drm_atomic_helper_crtc_create_state
      drm/atomic-helper: Remove drm_atomic_helper_crtc_reset
      drm/amdgpu: dm: Convert to atomic_create_state
      drm/loongson: Move hardware reset to CRTC creation
      drm/loongson: Convert to atomic_create_state
      drm/mediatek: Convert to atomic_create_state
      drm/sitronix: st7920: Convert to atomic_create_state
      drm/atomic-helper: Remove __drm_atomic_helper_crtc_reset
      drm/crtc: Remove reset

 drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c           |  2 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 28 +++++++++-----
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.h |  2 +-
 .../display/amdgpu_dm/tests/amdgpu_dm_crtc_test.c  | 20 +++++-----
 drivers/gpu/drm/drm_atomic_state_helper.c          | 45 ----------------------
 drivers/gpu/drm/drm_mode_config.c                  | 17 ++++++--
 drivers/gpu/drm/logicvc/logicvc_crtc.c             |  2 +-
 drivers/gpu/drm/loongson/lsdc_crtc.c               | 30 ++++++++-------
 drivers/gpu/drm/mediatek/mtk_crtc.c                | 18 ++++-----
 drivers/gpu/drm/sitronix/st7920.c                  | 12 +++---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c               |  7 ++--
 drivers/gpu/drm/tilcdc/tilcdc_drv.c                |  2 +
 include/drm/drm_atomic_state_helper.h              |  3 --
 include/drm/drm_crtc.h                             | 12 ------
 include/drm/drm_modeset_helper_vtables.h           | 14 +++++++
 15 files changed, 94 insertions(+), 120 deletions(-)
---
base-commit: 07c66ce1e6446f5cc6a4ace99a8d46880f45c527
change-id: 20260709-drm-no-more-crtc-reset-14a4d5bc8d41

Best regards,
-- 
Maxime Ripard <mripard@kernel.org>

[PATCH v2 01/13] drm/crtc: Introduce hw_reset helper hook

[Maxime Ripard]

The CRTC reset hook conflates initial software state allocation with
hardware reset. The atomic_create_state hook addresses the software
state side, but drivers may still need to reset hardware to a known good
state during drm_mode_config_reset(), for example during suspend/resume.

Separating hardware reset from state allocation is also useful for the
pending atomic state readout and userspace atomic reset flag series,
which need to create pristine software state without affecting the
hardware.

Introduce a hw_reset hook in struct drm_crtc_helper_funcs that only
resets the hardware, without touching the software state at all. Call it
from drm_mode_config_crtc_reset_with_create_state() after the state has
been successfully created.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_mode_config.c        | 13 ++++++++++++-
 include/drm/drm_modeset_helper_vtables.h | 14 ++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 366f6d821242..bb2efc274323 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -28,10 +28,11 @@
 #include <drm/drm_encoder.h>
 #include <drm/drm_file.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_mode_config.h>
+#include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_print.h>
 #include <drm/drm_colorop.h>
 #include <linux/dma-resv.h>
 
 #include "drm_crtc_internal.h"
@@ -228,16 +229,26 @@ static int drm_mode_config_crtc_create_state(struct drm_crtc *crtc)
 	return 0;
 }
 
 static int drm_mode_config_crtc_reset_with_create_state(struct drm_crtc *crtc)
 {
+	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
+	int ret;
+
 	if (crtc->state) {
 		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
 		crtc->state = NULL;
 	}
 
-	return drm_mode_config_crtc_create_state(crtc);
+	ret = drm_mode_config_crtc_create_state(crtc);
+	if (ret)
+		return ret;
+
+	if (crtc_funcs->hw_reset)
+		crtc_funcs->hw_reset(crtc);
+
+	return 0;
 }
 
 static int drm_mode_config_connector_create_state(struct drm_connector *connector)
 {
 	struct drm_connector_state *conn_state;
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index ca6268945c28..4965356a6314 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -279,10 +279,24 @@ struct drm_crtc_helper_funcs {
 	 * drivers still using legacy CRTC helpers, which is different from the
 	 * rules under atomic.
 	 */
 	void (*disable)(struct drm_crtc *crtc);
 
+	/**
+	 * @hw_reset:
+	 *
+	 * Optional hook for CRTC hardware reset.
+	 *
+	 * Unlike @drm_crtc_funcs.reset, which both resets hardware and
+	 * creates new software state, this hook only resets the
+	 * hardware to a known good state without touching the software
+	 * state at all.
+	 *
+	 * This hook is called by drm_mode_config_reset().
+	 */
+	void (*hw_reset)(struct drm_crtc *crtc);
+
 	/**
 	 * @atomic_check:
 	 *
 	 * Drivers should check plane-update related CRTC constraints in this
 	 * hook. They can also check mode related limitations but need to be

-- 
2.55.0

Re: [PATCH v2 01/13] drm/crtc: Introduce hw_reset helper hook

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing NULL pointer check for optional `crtc_funcs` before checking for `hw_reset`
--

commit cd389080636467064c5a9461f52395b91014fd73
Author: Maxime Ripard <mripard@kernel.org>

drm/crtc: Introduce hw_reset helper hook

This commit introduces a new hw_reset hook in struct drm_crtc_helper_funcs to
allow resetting hardware to a known good state without affecting software state.
It updates drm_mode_config_crtc_reset_with_create_state() to call this hook
after the software state is successfully created.

> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c

[ ... ]

> @@ -230,12 +231,22 @@ static int drm_mode_config_crtc_create_state(struct drm_crtc *crtc)
>  
>  static int drm_mode_config_crtc_reset_with_create_state(struct drm_crtc *crtc)
>  {
> +	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
> +	int ret;
> +
>  	if (crtc->state) {
>  		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
>  		crtc->state = NULL;
>  	}
>  
> -	return drm_mode_config_crtc_create_state(crtc);
> +	ret = drm_mode_config_crtc_create_state(crtc);
> +	if (ret)
> +		return ret;
> +
> +	if (crtc_funcs->hw_reset)
> +		crtc_funcs->hw_reset(crtc);

[Severity: High]
Could this lead to a NULL pointer dereference for drivers that do not use the
CRTC helpers?

If a driver implements atomic_create_state but doesn't use the CRTC helpers,
crtc->helper_private will be NULL. When drm_mode_config_reset() iterates
over all CRTCs and calls drm_mode_config_crtc_reset_with_create_state(),
crtc_funcs will evaluate to NULL.

Since crtc_funcs is unconditionally dereferenced here to check for hw_reset,
would this cause a panic during initialization or suspend/resume paths?

Should there be an additional check to ensure crtc_funcs is valid?

	if (crtc_funcs && crtc_funcs->hw_reset)
		crtc_funcs->hw_reset(crtc);

> +
> +	return 0;
>  }
>  
>  static int drm_mode_config_connector_create_state(struct drm_connector *connector)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-drm-no-more-crtc-reset-v2-0-4500d49a337f@kernel.org?part=1

Re: [PATCH v2 01/13] drm/crtc: Introduce hw_reset helper hook

[Thomas Zimmermann]

Hi Maxime

Am 31.08.26 um 17:34 schrieb Maxime Ripard:
> The CRTC reset hook conflates initial software state allocation with
> hardware reset. The atomic_create_state hook addresses the software
> state side, but drivers may still need to reset hardware to a known good
> state during drm_mode_config_reset(), for example during suspend/resume.
>
> Separating hardware reset from state allocation is also useful for the
> pending atomic state readout and userspace atomic reset flag series,
> which need to create pristine software state without affecting the
> hardware.
>
> Introduce a hw_reset hook in struct drm_crtc_helper_funcs that only
> resets the hardware, without touching the software state at all. Call it
> from drm_mode_config_crtc_reset_with_create_state() after the state has
> been successfully created.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

with comments below.

> ---
>   drivers/gpu/drm/drm_mode_config.c        | 13 ++++++++++++-
>   include/drm/drm_modeset_helper_vtables.h | 14 ++++++++++++++
>   2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 366f6d821242..bb2efc274323 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -28,10 +28,11 @@
>   #include <drm/drm_encoder.h>
>   #include <drm/drm_file.h>
>   #include <drm/drm_framebuffer.h>
>   #include <drm/drm_managed.h>
>   #include <drm/drm_mode_config.h>
> +#include <drm/drm_modeset_helper_vtables.h>
>   #include <drm/drm_print.h>
>   #include <drm/drm_colorop.h>
>   #include <linux/dma-resv.h>
>   
>   #include "drm_crtc_internal.h"
> @@ -228,16 +229,26 @@ static int drm_mode_config_crtc_create_state(struct drm_crtc *crtc)
>   	return 0;
>   }
>   
>   static int drm_mode_config_crtc_reset_with_create_state(struct drm_crtc *crtc)
>   {
> +	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;

Please see the Sahsiko comment about this being NULL. And I suggest to 
call the variable 'helpers' because 'funcs' sounds like regular 
drm_crtc_funcs. But that's really just nitpicking.

Best regards
Thomas

> +	int ret;
> +
>   	if (crtc->state) {
>   		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
>   		crtc->state = NULL;
>   	}
>   
> -	return drm_mode_config_crtc_create_state(crtc);
> +	ret = drm_mode_config_crtc_create_state(crtc);
> +	if (ret)
> +		return ret;
> +
> +	if (crtc_funcs->hw_reset)
> +		crtc_funcs->hw_reset(crtc);
> +
> +	return 0;
>   }
>   
>   static int drm_mode_config_connector_create_state(struct drm_connector *connector)
>   {
>   	struct drm_connector_state *conn_state;
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index ca6268945c28..4965356a6314 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -279,10 +279,24 @@ struct drm_crtc_helper_funcs {
>   	 * drivers still using legacy CRTC helpers, which is different from the
>   	 * rules under atomic.
>   	 */
>   	void (*disable)(struct drm_crtc *crtc);
>   
> +	/**
> +	 * @hw_reset:
> +	 *
> +	 * Optional hook for CRTC hardware reset.
> +	 *
> +	 * Unlike @drm_crtc_funcs.reset, which both resets hardware and
> +	 * creates new software state, this hook only resets the
> +	 * hardware to a known good state without touching the software
> +	 * state at all.
> +	 *
> +	 * This hook is called by drm_mode_config_reset().
> +	 */
> +	void (*hw_reset)(struct drm_crtc *crtc);
> +
>   	/**
>   	 * @atomic_check:
>   	 *
>   	 * Drivers should check plane-update related CRTC constraints in this
>   	 * hook. They can also check mode related limitations but need to be
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)

[PATCH v2 02/13] drm/amdgpu: vkms: Switch to drm_atomic_helper_crtc_create_state

[Maxime Ripard]

The amdgpu vkms crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset() as its reset hook.

Switch to drm_atomic_helper_crtc_create_state() instead.

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
index c835504fdf2b..592c5c253938 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
@@ -45,11 +45,11 @@ static const u32 amdgpu_vkms_formats[] = {
 
 static const struct drm_crtc_funcs amdgpu_vkms_crtc_funcs = {
 	.set_config             = drm_atomic_helper_set_config,
 	.destroy                = drm_crtc_cleanup,
 	.page_flip              = drm_atomic_helper_page_flip,
-	.reset                  = drm_atomic_helper_crtc_reset,
+	.atomic_create_state = drm_atomic_helper_crtc_create_state,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
 	DRM_CRTC_VBLANK_TIMER_FUNCS,
 };
 

-- 
2.55.0

[PATCH v2 03/13] drm/logicvc: Switch to drm_atomic_helper_crtc_create_state

[Maxime Ripard]

The logicvc crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset() as its reset hook.

Switch to drm_atomic_helper_crtc_create_state() instead.

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: Paul Kocialkowski <paulk@sys-base.io>
---
 drivers/gpu/drm/logicvc/logicvc_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/logicvc/logicvc_crtc.c b/drivers/gpu/drm/logicvc/logicvc_crtc.c
index 81e5b80736fa..af92a573c3f2 100644
--- a/drivers/gpu/drm/logicvc/logicvc_crtc.c
+++ b/drivers/gpu/drm/logicvc/logicvc_crtc.c
@@ -211,14 +211,14 @@ static void logicvc_crtc_disable_vblank(struct drm_crtc *drm_crtc)
 	regmap_write_bits(logicvc->regmap, LOGICVC_INT_MASK_REG,
 			  LOGICVC_INT_MASK_V_SYNC, LOGICVC_INT_MASK_V_SYNC);
 }
 
 static const struct drm_crtc_funcs logicvc_crtc_funcs = {
-	.reset			= drm_atomic_helper_crtc_reset,
 	.destroy		= drm_crtc_cleanup,
 	.set_config		= drm_atomic_helper_set_config,
 	.page_flip		= drm_atomic_helper_page_flip,
+	.atomic_create_state	= drm_atomic_helper_crtc_create_state,
 	.atomic_duplicate_state	= drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state	= drm_atomic_helper_crtc_destroy_state,
 	.enable_vblank		= logicvc_crtc_enable_vblank,
 	.disable_vblank		= logicvc_crtc_disable_vblank,
 };

-- 
2.55.0

[PATCH v2 04/13] drm/tilcdc: Move hardware reset to CRTC creation

[Maxime Ripard]

The tilcdc crtc reset hook performs both atomic state initialization
using drm_atomic_helper_crtc_reset() and hardware-level register
writes to disable the raster and clear IRQ status.

The hardware reset is not related to atomic state initialization, and
the reset hook is being converted to atomic_create_state which only
deals with state allocation. Move the hardware reset to the CRTC
creation path instead.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: Jyri Sarha <jyri.sarha@iki.fi>
Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 +++----
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 2 ++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index e07ecb36a27a..b299d73ce385 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -683,18 +683,16 @@ static void tilcdc_crtc_disable_vblank(struct drm_crtc *crtc)
 			     LCDC_V2_END_OF_FRAME0_INT_ENA);
 
 	spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags);
 }
 
-static void tilcdc_crtc_reset(struct drm_crtc *crtc)
+static void tilcdc_crtc_hw_reset(struct drm_crtc *crtc)
 {
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	int ret;
 
-	drm_atomic_helper_crtc_reset(crtc);
-
 	/* Turn the raster off if it for some reason is on. */
 	pm_runtime_get_sync(dev->dev);
 	if (tilcdc_read(dev, LCDC_RASTER_CTRL_REG) & LCDC_RASTER_ENABLE) {
 		/* Enable DMA Frame Done Interrupt */
 		tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG, LCDC_FRAME_DONE);
@@ -714,11 +712,11 @@ static void tilcdc_crtc_reset(struct drm_crtc *crtc)
 }
 
 static const struct drm_crtc_funcs tilcdc_crtc_funcs = {
 	.set_config     = drm_atomic_helper_set_config,
 	.page_flip      = drm_atomic_helper_page_flip,
-	.reset		= tilcdc_crtc_reset,
+	.reset		= drm_atomic_helper_crtc_reset,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 	.enable_vblank	= tilcdc_crtc_enable_vblank,
 	.disable_vblank	= tilcdc_crtc_disable_vblank,
 };
@@ -818,10 +816,11 @@ static const struct drm_crtc_helper_funcs tilcdc_crtc_helper_funcs = {
 	.mode_fixup	= tilcdc_crtc_mode_fixup,
 	.atomic_check	= tilcdc_crtc_atomic_check,
 	.atomic_enable	= tilcdc_crtc_atomic_enable,
 	.atomic_disable	= tilcdc_crtc_atomic_disable,
 	.atomic_flush	= tilcdc_crtc_atomic_flush,
+	.hw_reset	= tilcdc_crtc_hw_reset,
 };
 
 void tilcdc_crtc_update_clk(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 1d6c9a423a41..827f15d14fad 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -247,13 +247,15 @@ static int tilcdc_pm_suspend(struct device *dev)
 }
 
 static int tilcdc_pm_resume(struct device *dev)
 {
 	struct drm_device *ddev = dev_get_drvdata(dev);
+	struct tilcdc_drm_private *priv = ddev_to_tilcdc_priv(ddev);
 
 	/* Select default pin state */
 	pinctrl_pm_select_default_state(dev);
+
 	return  drm_mode_config_helper_resume(ddev);
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(tilcdc_pm_ops,
 				tilcdc_pm_suspend, tilcdc_pm_resume);

-- 
2.55.0

Re: [PATCH v2 04/13] drm/tilcdc: Move hardware reset to CRTC creation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] Unused variable `priv` causes a compiler warning/error.

Pre-existing issues:
- [High] Missing explicit cancellation of `recover_work` leads to a potential Use-After-Free.
- [High] Missing return value check for `pm_runtime_get_sync()` before accessing hardware registers.
--

commit d111f5c86cb625a96553bb44ee58889cee8c25aa
Author: Maxime Ripard <mripard@kernel.org>

drm/tilcdc: Move hardware reset to CRTC creation

The tilcdc crtc reset hook performs both atomic state initialization
using drm_atomic_helper_crtc_reset() and hardware-level register
writes to disable the raster and clear IRQ status.

The hardware reset is not related to atomic state initialization, and
the reset hook is being converted to atomic_create_state which only
deals with state allocation. Move the hardware reset to the CRTC
creation path instead.

> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index e07ecb36a27a8..b299d73ce3856 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
[ ... ]
> @@ -685,14 +685,12 @@ static void tilcdc_crtc_disable_vblank(struct drm_crtc *crtc)
>  	spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags);
>  }
>  
> -static void tilcdc_crtc_reset(struct drm_crtc *crtc)
> +static void tilcdc_crtc_hw_reset(struct drm_crtc *crtc)
>  {
>  	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
>  	struct drm_device *dev = crtc->dev;
>  	int ret;
>  
> -	drm_atomic_helper_crtc_reset(crtc);
> -
>  	/* Turn the raster off if it for some reason is on. */
>  	pm_runtime_get_sync(dev->dev);
>  	if (tilcdc_read(dev, LCDC_RASTER_CTRL_REG) & LCDC_RASTER_ENABLE) {

[Severity: High]
This is a pre-existing issue, but is it safe to proceed with reading
LCDC_RASTER_CTRL_REG if pm_runtime_get_sync() fails?

If pm_runtime_get_sync() returns a negative error code (e.g. -EACCES) due to
a resume failure, the hardware may remain powered off. Accessing MMIO
registers on a powered-off or suspended device can cause a bus fault on ARM
platforms, leading to a synchronous external abort kernel panic.

Should we check the return value of pm_runtime_get_sync() here before
accessing the hardware?

[ ... ]
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index 1d6c9a423a410..827f15d14fad9 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -249,9 +249,11 @@ static int tilcdc_pm_suspend(struct device *dev)
>  static int tilcdc_pm_resume(struct device *dev)
>  {
>  	struct drm_device *ddev = dev_get_drvdata(dev);
> +	struct tilcdc_drm_private *priv = ddev_to_tilcdc_priv(ddev);

[Severity: Low]
Does this code leave the priv variable unused?

It appears priv is declared and assigned here but never referenced,
which can cause a compiler warning.

>  
>  	/* Select default pin state */
>  	pinctrl_pm_select_default_state(dev);
> +
>  	return  drm_mode_config_helper_resume(ddev);
>  }

I also noticed a separate regression outside the patch context in the driver
teardown path:

[Severity: High]
This isn't a bug introduced by this patch, but is it possible that the driver
teardown fails to cancel the recover_work?

Looking at tilcdc_crtc_destroy() in tilcdc_crtc.c, the workqueue is flushed:

drivers/gpu/drm/tilcdc/tilcdc_crtc.c:tilcdc_crtc_destroy() {
    ...
	tilcdc_crtc_shutdown(priv->crtc);

	flush_workqueue(priv->wq);

	of_node_put(priv->crtc->port);
}

However, recover_work is queued on system_percpu_wq during sync loss events 
in tilcdc_crtc_irq(), not on priv->wq. Since there is no explicit 
cancel_work_sync(&tilcdc_crtc->recover_work) before the tilcdc_crtc 
structure is ultimately freed, could this lead to a use-after-free if the 
work executes after the CRTC state is destroyed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-drm-no-more-crtc-reset-v2-0-4500d49a337f@kernel.org?part=4

Re: [PATCH v2 04/13] drm/tilcdc: Move hardware reset to CRTC creation

[Thomas Zimmermann]

Am 31.08.26 um 17:34 schrieb Maxime Ripard:
> The tilcdc crtc reset hook performs both atomic state initialization
> using drm_atomic_helper_crtc_reset() and hardware-level register
> writes to disable the raster and clear IRQ status.
>
> The hardware reset is not related to atomic state initialization, and
> the reset hook is being converted to atomic_create_state which only
> deals with state allocation. Move the hardware reset to the CRTC
> creation path instead.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
> Cc: Jyri Sarha <jyri.sarha@iki.fi>
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>   drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 7 +++----
>   drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 2 ++
>   2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index e07ecb36a27a..b299d73ce385 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -683,18 +683,16 @@ static void tilcdc_crtc_disable_vblank(struct drm_crtc *crtc)
>   			     LCDC_V2_END_OF_FRAME0_INT_ENA);
>   
>   	spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags);
>   }
>   
> -static void tilcdc_crtc_reset(struct drm_crtc *crtc)
> +static void tilcdc_crtc_hw_reset(struct drm_crtc *crtc)
>   {
>   	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
>   	struct drm_device *dev = crtc->dev;
>   	int ret;
>   
> -	drm_atomic_helper_crtc_reset(crtc);
> -
>   	/* Turn the raster off if it for some reason is on. */
>   	pm_runtime_get_sync(dev->dev);
>   	if (tilcdc_read(dev, LCDC_RASTER_CTRL_REG) & LCDC_RASTER_ENABLE) {
>   		/* Enable DMA Frame Done Interrupt */
>   		tilcdc_write(dev, LCDC_INT_ENABLE_SET_REG, LCDC_FRAME_DONE);
> @@ -714,11 +712,11 @@ static void tilcdc_crtc_reset(struct drm_crtc *crtc)
>   }
>   
>   static const struct drm_crtc_funcs tilcdc_crtc_funcs = {
>   	.set_config     = drm_atomic_helper_set_config,
>   	.page_flip      = drm_atomic_helper_page_flip,
> -	.reset		= tilcdc_crtc_reset,
> +	.reset		= drm_atomic_helper_crtc_reset,
>   	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>   	.enable_vblank	= tilcdc_crtc_enable_vblank,
>   	.disable_vblank	= tilcdc_crtc_disable_vblank,
>   };
> @@ -818,10 +816,11 @@ static const struct drm_crtc_helper_funcs tilcdc_crtc_helper_funcs = {
>   	.mode_fixup	= tilcdc_crtc_mode_fixup,
>   	.atomic_check	= tilcdc_crtc_atomic_check,
>   	.atomic_enable	= tilcdc_crtc_atomic_enable,
>   	.atomic_disable	= tilcdc_crtc_atomic_disable,
>   	.atomic_flush	= tilcdc_crtc_atomic_flush,
> +	.hw_reset	= tilcdc_crtc_hw_reset,
>   };
>   
>   void tilcdc_crtc_update_clk(struct drm_crtc *crtc)
>   {
>   	struct drm_device *dev = crtc->dev;
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index 1d6c9a423a41..827f15d14fad 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -247,13 +247,15 @@ static int tilcdc_pm_suspend(struct device *dev)
>   }
>   
>   static int tilcdc_pm_resume(struct device *dev)
>   {
>   	struct drm_device *ddev = dev_get_drvdata(dev);
> +	struct tilcdc_drm_private *priv = ddev_to_tilcdc_priv(ddev);
>   
>   	/* Select default pin state */
>   	pinctrl_pm_select_default_state(dev);
> +
>   	return  drm_mode_config_helper_resume(ddev);
>   }
>   
>   static DEFINE_SIMPLE_DEV_PM_OPS(tilcdc_pm_ops,
>   				tilcdc_pm_suspend, tilcdc_pm_resume);
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)

[PATCH v2 05/13] drm/tilcdc: Switch to drm_atomic_helper_crtc_create_state

[Maxime Ripard]

The tilcdc crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset() as its reset hook.

Switch to drm_atomic_helper_crtc_create_state() instead.

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: Jyri Sarha <jyri.sarha@iki.fi>
Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index b299d73ce385..f619b7a6e70d 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -712,11 +712,11 @@ static void tilcdc_crtc_hw_reset(struct drm_crtc *crtc)
 }
 
 static const struct drm_crtc_funcs tilcdc_crtc_funcs = {
 	.set_config     = drm_atomic_helper_set_config,
 	.page_flip      = drm_atomic_helper_page_flip,
-	.reset		= drm_atomic_helper_crtc_reset,
+	.atomic_create_state = drm_atomic_helper_crtc_create_state,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 	.enable_vblank	= tilcdc_crtc_enable_vblank,
 	.disable_vblank	= tilcdc_crtc_disable_vblank,
 };

-- 
2.55.0

Re: [PATCH v2 05/13] drm/tilcdc: Switch to drm_atomic_helper_crtc_create_state

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional dereference of potentially NULL crtc->helper_private pointer when checking for the hw_reset hook.
--

commit 64d66423bd9e9cda8a92c70fb8fec4016d121ea0
Author: Maxime Ripard <mripard@kernel.org>

drm/tilcdc: Switch to drm_atomic_helper_crtc_create_state

The tilcdc crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset() as its reset hook.

Switch to drm_atomic_helper_crtc_create_state() instead.

> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index b299d73ce3856..f619b7a6e70d9 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -714,7 +714,7 @@ static void tilcdc_crtc_hw_reset(struct drm_crtc *crtc)
>  static const struct drm_crtc_funcs tilcdc_crtc_funcs = {
>  	.set_config     = drm_atomic_helper_set_config,
>  	.page_flip      = drm_atomic_helper_page_flip,
> -	.reset		= drm_atomic_helper_crtc_reset,
> +	.atomic_create_state = drm_atomic_helper_crtc_create_state,

[Severity: High]
Does this change expose a NULL pointer dereference in the core DRM API?

By switching to .atomic_create_state, drm_mode_config_reset() will now call
drm_mode_config_crtc_reset_with_create_state() during driver probe or
system resume.

Looking at drm_mode_config_crtc_reset_with_create_state() in
drivers/gpu/drm/drm_mode_config.c:

drm_mode_config_crtc_reset_with_create_state() {
	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
	...
	if (crtc_funcs->hw_reset)
		crtc_funcs->hw_reset(crtc);

	return 0;
}

The crtc_funcs pointer is dereferenced here without a preceding NULL guard. 
If a driver does not use CRTC helpers, crtc->helper_private will be NULL. 
Wouldn't this unconditional dereference to check for hw_reset lead to a 
crash for any driver leaving helper_private unpopulated?

>  	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>  	.enable_vblank	= tilcdc_crtc_enable_vblank,
>  	.disable_vblank	= tilcdc_crtc_disable_vblank,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-drm-no-more-crtc-reset-v2-0-4500d49a337f@kernel.org?part=5

[PATCH v2 06/13] drm/atomic-helper: Remove drm_atomic_helper_crtc_reset

[Maxime Ripard]

All users of drm_atomic_helper_crtc_reset() have been converted to
drm_atomic_helper_crtc_create_state().

Remove the now unused helper.

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 19 -------------------
 include/drm/drm_atomic_state_helper.h     |  1 -
 include/drm/drm_crtc.h                    |  2 --
 3 files changed, 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index d90d1d7c9cf9..b592b7f5f264 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -103,29 +103,10 @@ __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
 
 	crtc->state = crtc_state;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_reset);
 
-/**
- * drm_atomic_helper_crtc_reset - default &drm_crtc_funcs.reset hook for CRTCs
- * @crtc: drm CRTC
- *
- * Resets the atomic state for @crtc by freeing the state pointer (which might
- * be NULL, e.g. at driver load time) and allocating a new empty state object.
- */
-void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
-{
-	struct drm_crtc_state *crtc_state =
-		kzalloc_obj(*crtc->state);
-
-	if (crtc->state)
-		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
-
-	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
-}
-EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
-
 /**
  * drm_atomic_helper_crtc_create_state - default &drm_crtc_funcs.atomic_create_state hook for crtcs
  * @crtc: crtc object
  *
  * Allocates and  initializes pristine @drm_crtc_state.
diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
index 34a599c3d86d..df70817c94fc 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -42,11 +42,10 @@ struct drm_device;
 
 void __drm_atomic_helper_crtc_state_init(struct drm_crtc_state *state,
 					  struct drm_crtc *crtc);
 void __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
 				    struct drm_crtc_state *state);
-void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc);
 struct drm_crtc_state *
 drm_atomic_helper_crtc_create_state(struct drm_crtc *crtc);
 void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 					      struct drm_crtc_state *state);
 struct drm_crtc_state *
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 152349f973e3..12f43253f059 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -428,12 +428,10 @@ struct drm_crtc_funcs {
 	 *
 	 * Reset CRTC hardware and software state to off. This function isn't
 	 * called by the core directly, only through drm_mode_config_reset().
 	 * It's not a helper hook only for historical reasons.
 	 *
-	 * Atomic drivers can use drm_atomic_helper_crtc_reset() to reset
-	 * atomic state using this hook.
 	 */
 	void (*reset)(struct drm_crtc *crtc);
 
 	/**
 	 * @cursor_set:

-- 
2.55.0

[PATCH v2 07/13] drm/amdgpu: dm: Convert to atomic_create_state

[Maxime Ripard]

The amdgpu display manager crtc implementation provides a custom reset
hook. However, this hook only allocates the state, initializes it with
__drm_atomic_helper_crtc_reset(), and frees the previous state. It
does not perform any hardware reset.

Since this is exactly what the atomic_create_state hook is meant to
do, minus the old state cleanup which the caller handles, convert the
implementation to use atomic_create_state with
__drm_atomic_helper_crtc_state_init() instead.

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Rodrigo Siqueira <siqueira@igalia.com>
Cc: amd-gfx@lists.freedesktop.org
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 28 ++++++++++++++--------
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.h |  2 +-
 .../display/amdgpu_dm/tests/amdgpu_dm_crtc_test.c  | 20 ++++++++--------
 3 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index 62eac6e65334..cfe1fbfbab8e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -473,24 +473,23 @@ static void amdgpu_dm_crtc_destroy(struct drm_crtc *crtc)
 
 	drm_crtc_cleanup(crtc);
 	kfree(crtc);
 }
 
-STATIC_IFN_KUNIT void amdgpu_dm_crtc_reset_state(struct drm_crtc *crtc)
+STATIC_IFN_KUNIT struct drm_crtc_state *amdgpu_dm_crtc_create_state(struct drm_crtc *crtc)
 {
 	struct dm_crtc_state *state;
 
 	state = kzalloc_obj(*state);
 	if (!state)
-		return;
+		return ERR_PTR(-ENOMEM);
 
-	if (crtc->state)
-		amdgpu_dm_crtc_destroy_state(crtc, crtc->state);
+	__drm_atomic_helper_crtc_state_init(&state->base, crtc);
 
-	__drm_atomic_helper_crtc_reset(crtc, &state->base);
+	return &state->base;
 }
-EXPORT_IF_KUNIT(amdgpu_dm_crtc_reset_state);
+EXPORT_IF_KUNIT(amdgpu_dm_crtc_create_state);
 
 #ifdef CONFIG_DEBUG_FS
 static int amdgpu_dm_crtc_late_register(struct drm_crtc *crtc)
 {
 	crtc_debugfs_init(crtc);
@@ -563,11 +562,11 @@ amdgpu_dm_atomic_crtc_get_property(struct drm_crtc *crtc,
 }
 #endif
 
 /* Implemented only the options currently available for the driver */
 static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = {
-	.reset = amdgpu_dm_crtc_reset_state,
+	.atomic_create_state = amdgpu_dm_crtc_create_state,
 	.destroy = amdgpu_dm_crtc_destroy,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
 	.atomic_duplicate_state = amdgpu_dm_crtc_duplicate_state,
 	.atomic_destroy_state = amdgpu_dm_crtc_destroy_state,
@@ -779,13 +778,22 @@ int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm,
 
 	amdgpu_dm_ism_init(&acrtc->ism, &default_ism_config);
 
 	drm_crtc_helper_add(&acrtc->base, &amdgpu_dm_crtc_helper_funcs);
 
-	/* Create (reset) the plane state */
-	if (acrtc->base.funcs->reset)
-		acrtc->base.funcs->reset(&acrtc->base);
+	/* Create the plane state */
+	if (acrtc->base.funcs->atomic_create_state) {
+		struct drm_crtc_state *crtc_state;
+
+		crtc_state = acrtc->base.funcs->atomic_create_state(&acrtc->base);
+		if (IS_ERR(crtc_state)) {
+			res = PTR_ERR(crtc_state);
+			goto fail;
+		}
+
+		acrtc->base.state = crtc_state;
+	}
 
 	acrtc->max_cursor_width = dm->adev->dm.dc->caps.max_cursor_size;
 	acrtc->max_cursor_height = dm->adev->dm.dc->caps.max_cursor_size;
 
 	acrtc->crtc_id = crtc_index;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.h
index 93c6d0d8d7fd..ad516aeb9798 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.h
@@ -47,11 +47,11 @@ bool amdgpu_dm_crtc_helper_mode_fixup(struct drm_crtc *crtc,
 				      const struct drm_display_mode *mode,
 				      struct drm_display_mode *adjusted_mode);
 void amdgpu_dm_crtc_destroy_state(struct drm_crtc *crtc,
 				  struct drm_crtc_state *state);
 struct drm_crtc_state *amdgpu_dm_crtc_duplicate_state(struct drm_crtc *crtc);
-void amdgpu_dm_crtc_reset_state(struct drm_crtc *crtc);
+struct drm_crtc_state *amdgpu_dm_crtc_create_state(struct drm_crtc *crtc);
 int amdgpu_dm_crtc_count_crtc_active_planes(struct drm_crtc_state *new_crtc_state);
 void amdgpu_dm_crtc_update_crtc_active_planes(struct drm_crtc *crtc,
 					      struct drm_crtc_state *new_crtc_state);
 void amdgpu_dm_crtc_vblank_control_worker(struct work_struct *work);
 void amdgpu_dm_idle_worker(struct work_struct *work);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_crtc_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_crtc_test.c
index 4dacddd23878..13deafdefdcc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_crtc_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_crtc_test.c
@@ -1402,35 +1402,35 @@ static void dm_test_crtc_duplicate_state_copies_fields(struct kunit *test)
 	KUNIT_EXPECT_TRUE(test, dm_dup->mpo_requested);
 
 	amdgpu_dm_crtc_destroy_state(crtc, dup);
 }
 
-/* Tests for amdgpu_dm_crtc_reset_state() */
+/* Tests for amdgpu_dm_crtc_create_state() */
 
 /**
- * dm_test_crtc_reset_state_allocates_state - Test reset installs a fresh state
+ * dm_test_crtc_create_state_allocates_state - Test reset installs a fresh state
  * @test: The KUnit test context
  *
  * Resetting a CRTC with no existing state must allocate and install a new
  * drm_crtc_state.
  */
-static void dm_test_crtc_reset_state_allocates_state(struct kunit *test)
+static void dm_test_crtc_create_state_allocates_state(struct kunit *test)
 {
 	struct amdgpu_device *adev = dm_kunit_alloc_adev(test);
+	struct drm_crtc_state *crtc_state;
 	struct drm_crtc *crtc;
 
 	crtc = kunit_kzalloc(test, sizeof(*crtc), GFP_KERNEL);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc);
 	crtc->dev = &adev->ddev;
 	crtc->state = NULL;
 
-	amdgpu_dm_crtc_reset_state(crtc);
+	crtc_state = amdgpu_dm_crtc_create_state(crtc);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, crtc_state);
 
-	KUNIT_EXPECT_NOT_NULL(test, crtc->state);
-
-	if (crtc->state)
-		amdgpu_dm_crtc_destroy_state(crtc, crtc->state);
+	if (!IS_ERR(crtc_state))
+		amdgpu_dm_crtc_destroy_state(crtc, crtc_state);
 }
 
 /* Tests for amdgpu_dm_crtc_destroy_state() */
 
 /**
@@ -1905,12 +1905,12 @@ static struct kunit_case amdgpu_dm_crtc_tests[] = {
 	/* amdgpu_dm_crtc_count_crtc_active_planes */
 	KUNIT_CASE(dm_test_count_crtc_active_planes_none),
 	KUNIT_CASE(dm_test_count_crtc_active_planes_mixed),
 	/* amdgpu_dm_crtc_duplicate_state */
 	KUNIT_CASE(dm_test_crtc_duplicate_state_copies_fields),
-	/* amdgpu_dm_crtc_reset_state */
-	KUNIT_CASE(dm_test_crtc_reset_state_allocates_state),
+	/* amdgpu_dm_crtc_create_state */
+	KUNIT_CASE(dm_test_crtc_create_state_allocates_state),
 	/* amdgpu_dm_crtc_destroy_state */
 	KUNIT_CASE(dm_test_crtc_destroy_state_no_stream),
 	KUNIT_CASE(dm_test_crtc_destroy_state_releases_stream),
 	/* amdgpu_dm_crtc_handle_vblank */
 	KUNIT_CASE(dm_test_crtc_handle_vblank_no_event),

-- 
2.55.0

Re: [PATCH v2 07/13] drm/amdgpu: dm: Convert to atomic_create_state

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Use-after-free and list corruption in `amdgpu_dm_crtc_init()` error path.
--

commit 0347668ca054c8ea5682a0562b13ea3c763f5fc2
Author: Maxime Ripard <mripard@kernel.org>

drm/amdgpu: dm: Convert to atomic_create_state

This commit converts the AMDGPU DM CRTC implementation to use the
atomic_create_state hook rather than a custom reset hook. It initializes
the state with __drm_atomic_helper_crtc_state_init() and handles the
newly created state during CRTC initialization.

[ ... ]
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index 62eac6e65334b..cfe1fbfbab8ea 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> @@ -781,9 +780,18 @@ int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm,
>  
>  	drm_crtc_helper_add(&acrtc->base, &amdgpu_dm_crtc_helper_funcs);
>  
> -	/* Create (reset) the plane state */
> -	if (acrtc->base.funcs->reset)
> -		acrtc->base.funcs->reset(&acrtc->base);
> +	/* Create the plane state */
> +	if (acrtc->base.funcs->atomic_create_state) {
> +		struct drm_crtc_state *crtc_state;
> +
> +		crtc_state = acrtc->base.funcs->atomic_create_state(&acrtc->base);
> +		if (IS_ERR(crtc_state)) {
> +			res = PTR_ERR(crtc_state);
> +			goto fail;

[Severity: High]
Does this error path leave dangling pointers in the DRM core lists?

This patch introduces a new failure point that jumps to the fail block after
the CRTC and plane have already been successfully initialized and linked via
drm_crtc_init_with_planes() earlier in amdgpu_dm_crtc_init().

Looking at the fail block in amdgpu_dm_crtc_init():

fail:
        kfree(acrtc);
        kfree(cursor_plane);
        return res;

Freeing acrtc and cursor_plane directly without unregistering them via
drm_crtc_cleanup() and drm_plane_cleanup() leaves freed memory linked in the
DRM mode configuration lists. Subsequent traversals of these lists will
dereference the freed memory, which can cause a kernel panic. 

While the plane cleanup omission existed previously, the CRTC list corruption
was newly introduced in this patch.

> +		}
> +
> +		acrtc->base.state = crtc_state;
> +	}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-drm-no-more-crtc-reset-v2-0-4500d49a337f@kernel.org?part=7

[PATCH v2 08/13] drm/loongson: Move hardware reset to CRTC creation

[Maxime Ripard]

The loongson crtc reset hook performs both atomic state initialization
using __drm_atomic_helper_crtc_reset() and hardware-level register
writes to reset the CRTC.

The hardware reset is not related to atomic state initialization, and
the reset hook is being converted to atomic_create_state which only
deals with state allocation. Move the hardware reset to the CRTC
creation path instead.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: Huacai Chen <chenhuacai@kernel.org>
Cc: Icenowy Zheng <zhengxingda@iscas.ac.cn>
Cc: Jianmin Lv <lvjianmin@loongson.cn>
Cc: Mingcong Bai <jeffbai@aosc.io>
Cc: Qianhai Wu <wuqianhai@loongson.cn>
Cc: Xi Ruoyao <xry111@xry111.site>
---
 drivers/gpu/drm/loongson/lsdc_crtc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/loongson/lsdc_crtc.c b/drivers/gpu/drm/loongson/lsdc_crtc.c
index 16b8f36a4071..7cca574e3b6a 100644
--- a/drivers/gpu/drm/loongson/lsdc_crtc.c
+++ b/drivers/gpu/drm/loongson/lsdc_crtc.c
@@ -388,12 +388,10 @@ static const struct lsdc_crtc_hw_ops ls7a2000_crtc_hw_ops[2] = {
 	},
 };
 
 static void lsdc_crtc_reset(struct drm_crtc *crtc)
 {
-	struct lsdc_crtc *lcrtc = to_lsdc_crtc(crtc);
-	const struct lsdc_crtc_hw_ops *ops = lcrtc->hw_ops;
 	struct lsdc_crtc_state *priv_crtc_state;
 
 	if (crtc->state)
 		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
 
@@ -401,13 +399,10 @@ static void lsdc_crtc_reset(struct drm_crtc *crtc)
 
 	if (!priv_crtc_state)
 		__drm_atomic_helper_crtc_reset(crtc, NULL);
 	else
 		__drm_atomic_helper_crtc_reset(crtc, &priv_crtc_state->base);
-
-	/* Reset the CRTC hardware, this is required for S3 support */
-	ops->reset(lcrtc);
 }
 
 static void lsdc_crtc_atomic_destroy_state(struct drm_crtc *crtc,
 					   struct drm_crtc_state *state)
 {
@@ -935,18 +930,28 @@ static bool lsdc_crtc_get_scanout_position(struct drm_crtc *crtc,
 		*etime = ktime_get();
 
 	return true;
 }
 
+static void lsdc_crtc_hw_reset(struct drm_crtc *crtc)
+{
+	struct lsdc_crtc *lcrtc = to_lsdc_crtc(crtc);
+
+	/* Reset the CRTC hardware, this is required for S3 support */
+	if (lcrtc->hw_ops->reset)
+		lcrtc->hw_ops->reset(lcrtc);
+}
+
 static const struct drm_crtc_helper_funcs lsdc_crtc_helper_funcs = {
 	.mode_valid = lsdc_crtc_mode_valid,
 	.mode_set_nofb = lsdc_crtc_mode_set_nofb,
 	.atomic_enable = lsdc_crtc_atomic_enable,
 	.atomic_disable = lsdc_crtc_atomic_disable,
 	.atomic_check = lsdc_crtc_helper_atomic_check,
 	.atomic_flush = lsdc_crtc_atomic_flush,
 	.get_scanout_position = lsdc_crtc_get_scanout_position,
+	.hw_reset = lsdc_crtc_hw_reset,
 };
 
 int ls7a1000_crtc_init(struct drm_device *ddev,
 		       struct drm_crtc *crtc,
 		       struct drm_plane *primary,

-- 
2.55.0

Re: [PATCH v2 08/13] drm/loongson: Move hardware reset to CRTC creation

[Icenowy Zheng]

在 2026-08-31一的 17:34 +0200，Maxime Ripard写道：
> The loongson crtc reset hook performs both atomic state
> initialization
> using __drm_atomic_helper_crtc_reset() and hardware-level register
> writes to reset the CRTC.
> 
> The hardware reset is not related to atomic state initialization, and
> the reset hook is being converted to atomic_create_state which only
> deals with state allocation. Move the hardware reset to the CRTC
> creation path instead.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
> Cc: Huacai Chen <chenhuacai@kernel.org>
> Cc: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> Cc: Jianmin Lv <lvjianmin@loongson.cn>
> Cc: Mingcong Bai <jeffbai@aosc.io>
> Cc: Qianhai Wu <wuqianhai@loongson.cn>
> Cc: Xi Ruoyao <xry111@xry111.site>
> ---
>  drivers/gpu/drm/loongson/lsdc_crtc.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/loongson/lsdc_crtc.c
> b/drivers/gpu/drm/loongson/lsdc_crtc.c
> index 16b8f36a4071..7cca574e3b6a 100644
> --- a/drivers/gpu/drm/loongson/lsdc_crtc.c
> +++ b/drivers/gpu/drm/loongson/lsdc_crtc.c
> @@ -388,12 +388,10 @@ static const struct lsdc_crtc_hw_ops
> ls7a2000_crtc_hw_ops[2] = {
>  	},
>  };
>  
>  static void lsdc_crtc_reset(struct drm_crtc *crtc)
>  {
> -	struct lsdc_crtc *lcrtc = to_lsdc_crtc(crtc);
> -	const struct lsdc_crtc_hw_ops *ops = lcrtc->hw_ops;
>  	struct lsdc_crtc_state *priv_crtc_state;
>  
>  	if (crtc->state)
>  		crtc->funcs->atomic_destroy_state(crtc, crtc-
> >state);
>  
> @@ -401,13 +399,10 @@ static void lsdc_crtc_reset(struct drm_crtc
> *crtc)
>  
>  	if (!priv_crtc_state)
>  		__drm_atomic_helper_crtc_reset(crtc, NULL);
>  	else
>  		__drm_atomic_helper_crtc_reset(crtc,
> &priv_crtc_state->base);
> -
> -	/* Reset the CRTC hardware, this is required for S3 support
> */
> -	ops->reset(lcrtc);
>  }
>  
>  static void lsdc_crtc_atomic_destroy_state(struct drm_crtc *crtc,
>  					   struct drm_crtc_state
> *state)
>  {
> @@ -935,18 +930,28 @@ static bool
> lsdc_crtc_get_scanout_position(struct drm_crtc *crtc,
>  		*etime = ktime_get();
>  
>  	return true;
>  }
>  
> +static void lsdc_crtc_hw_reset(struct drm_crtc *crtc)
> +{
> +	struct lsdc_crtc *lcrtc = to_lsdc_crtc(crtc);
> +
> +	/* Reset the CRTC hardware, this is required for S3 support
> */
> +	if (lcrtc->hw_ops->reset)
> +		lcrtc->hw_ops->reset(lcrtc);
> +}
> +
>  static const struct drm_crtc_helper_funcs lsdc_crtc_helper_funcs = {
>  	.mode_valid = lsdc_crtc_mode_valid,
>  	.mode_set_nofb = lsdc_crtc_mode_set_nofb,
>  	.atomic_enable = lsdc_crtc_atomic_enable,
>  	.atomic_disable = lsdc_crtc_atomic_disable,
>  	.atomic_check = lsdc_crtc_helper_atomic_check,
>  	.atomic_flush = lsdc_crtc_atomic_flush,
>  	.get_scanout_position = lsdc_crtc_get_scanout_position,
> +	.hw_reset = lsdc_crtc_hw_reset,

This seems good but I don't know whether I have a device to test this -
- I don't know whether MIPS-based Loongson devices can do S3 on the
mainline kernel.

Thanks,
Icenowy

>  };
>  
>  int ls7a1000_crtc_init(struct drm_device *ddev,
>  		       struct drm_crtc *crtc,
>  		       struct drm_plane *primary,

Re: [PATCH v2 08/13] drm/loongson: Move hardware reset to CRTC creation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `lsdc_crtc1_soft_reset` calls `msleep(20)`, which can sleep, but it is reachable from the CRTC atomic enable callback (`lsdc_crtc_atomic_enable`), which runs in atomic context.
--

commit 4cb085e0fbbbf2cb0c2550f2c69cc0175ba5ecf8
Author: Maxime Ripard <mripard@kernel.org>

drm/loongson: Move hardware reset to CRTC creation

This patch moves the hardware-level CRTC reset from the atomic state
initialization hook to the CRTC creation path. It adds a new hw_reset
callback to handle the register writes independently.

> diff --git a/drivers/gpu/drm/loongson/lsdc_crtc.c b/drivers/gpu/drm/loongson/lsdc_crtc.c
> index 16b8f36a40710..7cca574e3b6af 100644
> --- a/drivers/gpu/drm/loongson/lsdc_crtc.c
> +++ b/drivers/gpu/drm/loongson/lsdc_crtc.c

[ ... ]

> @@ -937,6 +932,15 @@ static bool lsdc_crtc_get_scanout_position(struct drm_crtc *crtc,
>  	return true;
>  }
>  
> +static void lsdc_crtc_hw_reset(struct drm_crtc *crtc)
> +{
> +	struct lsdc_crtc *lcrtc = to_lsdc_crtc(crtc);
> +
> +	/* Reset the CRTC hardware, this is required for S3 support */
> +	if (lcrtc->hw_ops->reset)
> +		lcrtc->hw_ops->reset(lcrtc);
> +}
> +

While reviewing the hardware reset paths in this file, I noticed a pre-existing
issue nearby:

drivers/gpu/drm/loongson/lsdc_crtc.c:
> static void lsdc_crtc1_soft_reset(struct lsdc_crtc *lcrtc)
> {
[ ... ]
> 	/* Wait about a vblank time */
> 	msleep(20);
> }

[Severity: High]
This isn't a bug introduced by this patch, but can this msleep() be called
during atomic context?

The DRM core calls the CRTC .atomic_enable callback, which executes in atomic
context:

lsdc_crtc_atomic_enable()
  lcrtc->hw_ops->enable(lcrtc) // resolves to lsdc_crtc1_enable()

If a hardware stall is detected, it attempts error recovery:

lsdc_crtc1_enable() {
    ...
    if (val & CRTC_ANCHORED) {
        drm_warn(&ldev->base, "%s stall\n", lcrtc->base.name);
        return lsdc_crtc1_soft_reset(lcrtc);
    }
}

This calls lsdc_crtc1_soft_reset(), which erroneously calls msleep(20).

Sleeping in atomic context violates core kernel locking rules. This triggers
might_sleep() warnings and can lead to deadlocks or panics. (Note that
lsdc_crtc0_soft_reset() correctly uses mdelay(20) instead).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-drm-no-more-crtc-reset-v2-0-4500d49a337f@kernel.org?part=8

Re: [PATCH v2 08/13] drm/loongson: Move hardware reset to CRTC creation

[Thomas Zimmermann]

Am 31.08.26 um 17:34 schrieb Maxime Ripard:
> The loongson crtc reset hook performs both atomic state initialization
> using __drm_atomic_helper_crtc_reset() and hardware-level register
> writes to reset the CRTC.
>
> The hardware reset is not related to atomic state initialization, and
> the reset hook is being converted to atomic_create_state which only
> deals with state allocation. Move the hardware reset to the CRTC
> creation path instead.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Looks good now.

> ---
> Cc: Huacai Chen <chenhuacai@kernel.org>
> Cc: Icenowy Zheng <zhengxingda@iscas.ac.cn>
> Cc: Jianmin Lv <lvjianmin@loongson.cn>
> Cc: Mingcong Bai <jeffbai@aosc.io>
> Cc: Qianhai Wu <wuqianhai@loongson.cn>
> Cc: Xi Ruoyao <xry111@xry111.site>
> ---
>   drivers/gpu/drm/loongson/lsdc_crtc.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/loongson/lsdc_crtc.c b/drivers/gpu/drm/loongson/lsdc_crtc.c
> index 16b8f36a4071..7cca574e3b6a 100644
> --- a/drivers/gpu/drm/loongson/lsdc_crtc.c
> +++ b/drivers/gpu/drm/loongson/lsdc_crtc.c
> @@ -388,12 +388,10 @@ static const struct lsdc_crtc_hw_ops ls7a2000_crtc_hw_ops[2] = {
>   	},
>   };
>   
>   static void lsdc_crtc_reset(struct drm_crtc *crtc)
>   {
> -	struct lsdc_crtc *lcrtc = to_lsdc_crtc(crtc);
> -	const struct lsdc_crtc_hw_ops *ops = lcrtc->hw_ops;
>   	struct lsdc_crtc_state *priv_crtc_state;
>   
>   	if (crtc->state)
>   		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
>   
> @@ -401,13 +399,10 @@ static void lsdc_crtc_reset(struct drm_crtc *crtc)
>   
>   	if (!priv_crtc_state)
>   		__drm_atomic_helper_crtc_reset(crtc, NULL);
>   	else
>   		__drm_atomic_helper_crtc_reset(crtc, &priv_crtc_state->base);
> -
> -	/* Reset the CRTC hardware, this is required for S3 support */
> -	ops->reset(lcrtc);
>   }
>   
>   static void lsdc_crtc_atomic_destroy_state(struct drm_crtc *crtc,
>   					   struct drm_crtc_state *state)
>   {
> @@ -935,18 +930,28 @@ static bool lsdc_crtc_get_scanout_position(struct drm_crtc *crtc,
>   		*etime = ktime_get();
>   
>   	return true;
>   }
>   
> +static void lsdc_crtc_hw_reset(struct drm_crtc *crtc)
> +{
> +	struct lsdc_crtc *lcrtc = to_lsdc_crtc(crtc);
> +
> +	/* Reset the CRTC hardware, this is required for S3 support */
> +	if (lcrtc->hw_ops->reset)
> +		lcrtc->hw_ops->reset(lcrtc);
> +}
> +
>   static const struct drm_crtc_helper_funcs lsdc_crtc_helper_funcs = {
>   	.mode_valid = lsdc_crtc_mode_valid,
>   	.mode_set_nofb = lsdc_crtc_mode_set_nofb,
>   	.atomic_enable = lsdc_crtc_atomic_enable,
>   	.atomic_disable = lsdc_crtc_atomic_disable,
>   	.atomic_check = lsdc_crtc_helper_atomic_check,
>   	.atomic_flush = lsdc_crtc_atomic_flush,
>   	.get_scanout_position = lsdc_crtc_get_scanout_position,
> +	.hw_reset = lsdc_crtc_hw_reset,
>   };
>   
>   int ls7a1000_crtc_init(struct drm_device *ddev,
>   		       struct drm_crtc *crtc,
>   		       struct drm_plane *primary,
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)

Re: [PATCH v2 08/13] drm/loongson: Move hardware reset to CRTC creation

[wuqianhai]

Looks good.

Reviewed-by: wuqianhai <wuqianhai@loongson.cn>

在 2026/9/2 15:10, Thomas Zimmermann 写道:
> 
> 
> Am 31.08.26 um 17:34 schrieb Maxime Ripard:
>> The loongson crtc reset hook performs both atomic state initialization
>> using __drm_atomic_helper_crtc_reset() and hardware-level register
>> writes to reset the CRTC.
>>
>> The hardware reset is not related to atomic state initialization, and
>> the reset hook is being converted to atomic_create_state which only
>> deals with state allocation. Move the hardware reset to the CRTC
>> creation path instead.
>>
>> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Looks good now.
> 
>> ---
>> Cc: Huacai Chen <chenhuacai@kernel.org>
>> Cc: Icenowy Zheng <zhengxingda@iscas.ac.cn>
>> Cc: Jianmin Lv <lvjianmin@loongson.cn>
>> Cc: Mingcong Bai <jeffbai@aosc.io>
>> Cc: Qianhai Wu <wuqianhai@loongson.cn>
>> Cc: Xi Ruoyao <xry111@xry111.site>
>> ---
>>   drivers/gpu/drm/loongson/lsdc_crtc.c | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/loongson/lsdc_crtc.c b/drivers/gpu/drm/ 
>> loongson/lsdc_crtc.c
>> index 16b8f36a4071..7cca574e3b6a 100644
>> --- a/drivers/gpu/drm/loongson/lsdc_crtc.c
>> +++ b/drivers/gpu/drm/loongson/lsdc_crtc.c
>> @@ -388,12 +388,10 @@ static const struct lsdc_crtc_hw_ops 
>> ls7a2000_crtc_hw_ops[2] = {
>>       },
>>   };
>>   static void lsdc_crtc_reset(struct drm_crtc *crtc)
>>   {
>> -    struct lsdc_crtc *lcrtc = to_lsdc_crtc(crtc);
>> -    const struct lsdc_crtc_hw_ops *ops = lcrtc->hw_ops;
>>       struct lsdc_crtc_state *priv_crtc_state;
>>       if (crtc->state)
>>           crtc->funcs->atomic_destroy_state(crtc, crtc->state);
>> @@ -401,13 +399,10 @@ static void lsdc_crtc_reset(struct drm_crtc *crtc)
>>       if (!priv_crtc_state)
>>           __drm_atomic_helper_crtc_reset(crtc, NULL);
>>       else
>>           __drm_atomic_helper_crtc_reset(crtc, &priv_crtc_state->base);
>> -
>> -    /* Reset the CRTC hardware, this is required for S3 support */
>> -    ops->reset(lcrtc);
>>   }
>>   static void lsdc_crtc_atomic_destroy_state(struct drm_crtc *crtc,
>>                          struct drm_crtc_state *state)
>>   {
>> @@ -935,18 +930,28 @@ static bool 
>> lsdc_crtc_get_scanout_position(struct drm_crtc *crtc,
>>           *etime = ktime_get();
>>       return true;
>>   }
>> +static void lsdc_crtc_hw_reset(struct drm_crtc *crtc)
>> +{
>> +    struct lsdc_crtc *lcrtc = to_lsdc_crtc(crtc);
>> +
>> +    /* Reset the CRTC hardware, this is required for S3 support */
>> +    if (lcrtc->hw_ops->reset)
>> +        lcrtc->hw_ops->reset(lcrtc);
>> +}
>> +
>>   static const struct drm_crtc_helper_funcs lsdc_crtc_helper_funcs = {
>>       .mode_valid = lsdc_crtc_mode_valid,
>>       .mode_set_nofb = lsdc_crtc_mode_set_nofb,
>>       .atomic_enable = lsdc_crtc_atomic_enable,
>>       .atomic_disable = lsdc_crtc_atomic_disable,
>>       .atomic_check = lsdc_crtc_helper_atomic_check,
>>       .atomic_flush = lsdc_crtc_atomic_flush,
>>       .get_scanout_position = lsdc_crtc_get_scanout_position,
>> +    .hw_reset = lsdc_crtc_hw_reset,
>>   };
>>   int ls7a1000_crtc_init(struct drm_device *ddev,
>>                  struct drm_crtc *crtc,
>>                  struct drm_plane *primary,
>>
> 

[PATCH v2 09/13] drm/loongson: Convert to atomic_create_state

[Maxime Ripard]

The loongsoon crtc implementation provides a custom reset hook. However,
this hook only allocates the state, initializes it with
__drm_atomic_helper_crtc_reset(), and frees the previous state. It does
not perform any hardware reset.

Since the atomic_create_state hook is only meant to deal with state
allocation and initialization, convert it accordingly. The hardware
reset through ops->reset() is kept as part of the state creation
since the core currently calls atomic_create_state in the same
context where the reset hook was called.

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: Huacai Chen <chenhuacai@kernel.org>
Cc: Icenowy Zheng <zhengxingda@iscas.ac.cn>
Cc: Jianmin Lv <lvjianmin@loongson.cn>
Cc: Mingcong Bai <jeffbai@aosc.io>
Cc: Qianhai Wu <wuqianhai@loongson.cn>
Cc: Xi Ruoyao <xry111@xry111.site>
---
 drivers/gpu/drm/loongson/lsdc_crtc.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/loongson/lsdc_crtc.c b/drivers/gpu/drm/loongson/lsdc_crtc.c
index 7cca574e3b6a..a47816e3fa81 100644
--- a/drivers/gpu/drm/loongson/lsdc_crtc.c
+++ b/drivers/gpu/drm/loongson/lsdc_crtc.c
@@ -386,23 +386,22 @@ static const struct lsdc_crtc_hw_ops ls7a2000_crtc_hw_ops[2] = {
 		.get_vblank_counter = lsdc_crtc1_get_vblank_count,
 		.reset = lsdc_crtc1_reset,
 	},
 };
 
-static void lsdc_crtc_reset(struct drm_crtc *crtc)
+static struct drm_crtc_state *lsdc_crtc_create_state(struct drm_crtc *crtc)
 {
 	struct lsdc_crtc_state *priv_crtc_state;
 
-	if (crtc->state)
-		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
-
 	priv_crtc_state = kzalloc_obj(*priv_crtc_state);
 
 	if (!priv_crtc_state)
-		__drm_atomic_helper_crtc_reset(crtc, NULL);
-	else
-		__drm_atomic_helper_crtc_reset(crtc, &priv_crtc_state->base);
+		return ERR_PTR(-ENOMEM);
+
+	__drm_atomic_helper_crtc_state_init(&priv_crtc_state->base, crtc);
+
+	return &priv_crtc_state->base;
 }
 
 static void lsdc_crtc_atomic_destroy_state(struct drm_crtc *crtc,
 					   struct drm_crtc_state *state)
 {
@@ -705,11 +704,11 @@ static void lsdc_crtc_atomic_print_state(struct drm_printer *p,
 	drm_printf(p, "\tMedium clock multiplier = %u\n", pparms->loopc);
 	drm_printf(p, "\tOutput clock divider = %u\n", pparms->div_out);
 }
 
 static const struct drm_crtc_funcs ls7a1000_crtc_funcs = {
-	.reset = lsdc_crtc_reset,
+	.atomic_create_state = lsdc_crtc_create_state,
 	.destroy = drm_crtc_cleanup,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
 	.atomic_duplicate_state = lsdc_crtc_atomic_duplicate_state,
 	.atomic_destroy_state = lsdc_crtc_atomic_destroy_state,
@@ -719,11 +718,11 @@ static const struct drm_crtc_funcs ls7a1000_crtc_funcs = {
 	.get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
 	.atomic_print_state = lsdc_crtc_atomic_print_state,
 };
 
 static const struct drm_crtc_funcs ls7a2000_crtc_funcs = {
-	.reset = lsdc_crtc_reset,
+	.atomic_create_state = lsdc_crtc_create_state,
 	.destroy = drm_crtc_cleanup,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
 	.atomic_duplicate_state = lsdc_crtc_atomic_duplicate_state,
 	.atomic_destroy_state = lsdc_crtc_atomic_destroy_state,

-- 
2.55.0

[PATCH v2 10/13] drm/mediatek: Convert to atomic_create_state

[Maxime Ripard]

The mediatek crtc implementation provides a custom reset hook.
However, this hook only allocates the state, initializes it with
__drm_atomic_helper_crtc_reset(), and frees the previous state. It
does not perform any hardware reset.

Since this is exactly what the atomic_create_state hook is meant to
do, minus the old state cleanup which the caller handles, convert the
implementation to use atomic_create_state with
__drm_atomic_helper_crtc_state_init() instead.

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Chun-Kuang Hu <chunkuang.hu@kernel.org>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/drm/mediatek/mtk_crtc.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
index 97e3ff412e6e..2920f3198ef6 100644
--- a/drivers/gpu/drm/mediatek/mtk_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
@@ -148,23 +148,21 @@ static void mtk_crtc_destroy(struct drm_crtc *crtc)
 	}
 
 	drm_crtc_cleanup(crtc);
 }
 
-static void mtk_crtc_reset(struct drm_crtc *crtc)
+static struct drm_crtc_state *mtk_crtc_create_state(struct drm_crtc *crtc)
 {
 	struct mtk_crtc_state *state;
 
-	if (crtc->state) {
-		__drm_atomic_helper_crtc_destroy_state(crtc->state);
-		kfree(to_mtk_crtc_state(crtc->state));
-	}
-	crtc->state = NULL;
-
 	state = kzalloc_obj(*state);
-	if (state)
-		__drm_atomic_helper_crtc_reset(crtc, &state->base);
+	if (!state)
+		return ERR_PTR(-ENOMEM);
+
+	__drm_atomic_helper_crtc_state_init(&state->base, crtc);
+
+	return &state->base;
 }
 
 static struct drm_crtc_state *mtk_crtc_duplicate_state(struct drm_crtc *crtc)
 {
 	struct mtk_crtc_state *state;
@@ -884,11 +882,11 @@ static void mtk_crtc_atomic_flush(struct drm_crtc *crtc,
 
 static const struct drm_crtc_funcs mtk_crtc_funcs = {
 	.set_config		= drm_atomic_helper_set_config,
 	.page_flip		= drm_atomic_helper_page_flip,
 	.destroy		= mtk_crtc_destroy,
-	.reset			= mtk_crtc_reset,
+	.atomic_create_state	= mtk_crtc_create_state,
 	.atomic_duplicate_state	= mtk_crtc_duplicate_state,
 	.atomic_destroy_state	= mtk_crtc_destroy_state,
 	.enable_vblank		= mtk_crtc_enable_vblank,
 	.disable_vblank		= mtk_crtc_disable_vblank,
 };

-- 
2.55.0

[PATCH v2 11/13] drm/sitronix: st7920: Convert to atomic_create_state

[Maxime Ripard]

The st7920 crtc implementation provides a custom reset hook. However,
this hook only allocates the state, initializes it with
__drm_atomic_helper_crtc_reset(), and warns if there's a previous state.
It does not perform any hardware reset.

Since this is exactly what the atomic_create_state hook is meant to
do, minus the old state cleanup which the caller handles, convert the
implementation to use atomic_create_state with
__drm_atomic_helper_crtc_state_init() instead.

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: Iker Pedrosa <ikerpedrosam@gmail.com>
---
 drivers/gpu/drm/sitronix/st7920.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/sitronix/st7920.c b/drivers/gpu/drm/sitronix/st7920.c
index 0a75e9319080..9887ed746ceb 100644
--- a/drivers/gpu/drm/sitronix/st7920.c
+++ b/drivers/gpu/drm/sitronix/st7920.c
@@ -573,21 +573,21 @@ static void st7920_crtc_atomic_disable(struct drm_crtc *crtc,
 
 	drm_dev_exit(idx);
 }
 
 /* Called during init to allocate the CRTC's atomic state. */
-static void st7920_crtc_reset(struct drm_crtc *crtc)
+static struct drm_crtc_state *st7920_crtc_create_state(struct drm_crtc *crtc)
 {
 	struct st7920_crtc_state *st7920_state;
 
-	drm_WARN_ON_ONCE(crtc->dev, crtc->state);
-
 	st7920_state = kzalloc_obj(*st7920_state);
 	if (!st7920_state)
-		return;
+		return ERR_PTR(-ENOMEM);
 
-	__drm_atomic_helper_crtc_reset(crtc, &st7920_state->base);
+	__drm_atomic_helper_crtc_state_init(&st7920_state->base, crtc);
+
+	return &st7920_state->base;
 }
 
 static struct drm_crtc_state *st7920_crtc_duplicate_state(struct drm_crtc *crtc)
 {
 	struct st7920_crtc_state *st7920_state;
@@ -627,11 +627,11 @@ static const struct drm_crtc_helper_funcs st7920_crtc_helper_funcs = {
 	.atomic_enable = st7920_crtc_atomic_enable,
 	.atomic_disable = st7920_crtc_atomic_disable,
 };
 
 static const struct drm_crtc_funcs st7920_crtc_funcs = {
-	.reset = st7920_crtc_reset,
+	.atomic_create_state = st7920_crtc_create_state,
 	.destroy = drm_crtc_cleanup,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
 	.atomic_duplicate_state = st7920_crtc_duplicate_state,
 	.atomic_destroy_state = st7920_crtc_destroy_state,

-- 
2.55.0

Re: [PATCH v2 11/13] drm/sitronix: st7920: Convert to atomic_create_state

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver calls sleeping functions (usleep_range, msleep, spi_write) from CRTC atomic_enable/atomic_disable and Plane atomic_update callbacks, which are designated as atomic context paths by the DRM subsystem guidelines where sleeping is strictly forbidden.
--

commit a9d5c4bd0a5df0b653500203f8b4425d59185b78
Author: Maxime Ripard <mripard@kernel.org>

drm/sitronix: st7920: Convert to atomic_create_state

The st7920 crtc implementation provides a custom reset hook. However,
this hook only allocates the state, initializes it with
__drm_atomic_helper_crtc_reset(), and warns if there's a previous state.
It does not perform any hardware reset.

Since this is exactly what the atomic_create_state hook is meant to
do, minus the old state cleanup which the caller handles, convert the
implementation to use atomic_create_state with
__drm_atomic_helper_crtc_state_init() instead.

> diff --git a/drivers/gpu/drm/sitronix/st7920.c b/drivers/gpu/drm/sitronix/st7920.c
> index 0a75e9319080f..9887ed746cebd 100644
> --- a/drivers/gpu/drm/sitronix/st7920.c
> +++ b/drivers/gpu/drm/sitronix/st7920.c
> @@ -575,17 +575,17 @@ static void st7920_crtc_atomic_disable(struct drm_crtc *crtc,
>  }

[Severity: High]
This is a pre-existing issue, but does this driver incorrectly call sleeping
functions from atomic context paths?

A display modeset or screen update triggers DRM callbacks like
st7920_crtc_atomic_enable(), st7920_crtc_atomic_disable(), and
st7920_primary_plane_atomic_update() in drivers/gpu/drm/sitronix/st7920.c,
which are designated as atomic contexts where sleeping is strictly forbidden.

However, they internally call functions that can sleep:

st7920_crtc_atomic_enable() (at line 553):
  st7920_hw_reset()
    usleep_range(15, 20);
    msleep(40);

st7920_crtc_atomic_disable():
  st7920_power_off()
    spi_write()

st7920_primary_plane_atomic_update():
  st7920_fb_blit_rect()
    spi_write()

Could this lead to "scheduling while atomic" kernel warnings or deadlocks
when userspace triggers screen updates?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-drm-no-more-crtc-reset-v2-0-4500d49a337f@kernel.org?part=11

Re: [PATCH v2 11/13] drm/sitronix: st7920: Convert to atomic_create_state

[Iker Pedrosa]

Hi Maxime,

The ST7920 driver changes look correct to me.

Reviewed-by: Iker Pedrosa <ikerpedrosam@gmail.com>

El lun, 31 ago 2026 a las 17:35, Maxime Ripard (<mripard@kernel.org>) escribió:
>
> The st7920 crtc implementation provides a custom reset hook. However,
> this hook only allocates the state, initializes it with
> __drm_atomic_helper_crtc_reset(), and warns if there's a previous state.
> It does not perform any hardware reset.
>
> Since this is exactly what the atomic_create_state hook is meant to
> do, minus the old state cleanup which the caller handles, convert the
> implementation to use atomic_create_state with
> __drm_atomic_helper_crtc_state_init() instead.
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
> Cc: Iker Pedrosa <ikerpedrosam@gmail.com>
> ---
>  drivers/gpu/drm/sitronix/st7920.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/sitronix/st7920.c b/drivers/gpu/drm/sitronix/st7920.c
> index 0a75e9319080..9887ed746ceb 100644
> --- a/drivers/gpu/drm/sitronix/st7920.c
> +++ b/drivers/gpu/drm/sitronix/st7920.c
> @@ -573,21 +573,21 @@ static void st7920_crtc_atomic_disable(struct drm_crtc *crtc,
>
>         drm_dev_exit(idx);
>  }
>
>  /* Called during init to allocate the CRTC's atomic state. */
> -static void st7920_crtc_reset(struct drm_crtc *crtc)
> +static struct drm_crtc_state *st7920_crtc_create_state(struct drm_crtc *crtc)
>  {
>         struct st7920_crtc_state *st7920_state;
>
> -       drm_WARN_ON_ONCE(crtc->dev, crtc->state);
> -
>         st7920_state = kzalloc_obj(*st7920_state);
>         if (!st7920_state)
> -               return;
> +               return ERR_PTR(-ENOMEM);
>
> -       __drm_atomic_helper_crtc_reset(crtc, &st7920_state->base);
> +       __drm_atomic_helper_crtc_state_init(&st7920_state->base, crtc);
> +
> +       return &st7920_state->base;
>  }
>
>  static struct drm_crtc_state *st7920_crtc_duplicate_state(struct drm_crtc *crtc)
>  {
>         struct st7920_crtc_state *st7920_state;
> @@ -627,11 +627,11 @@ static const struct drm_crtc_helper_funcs st7920_crtc_helper_funcs = {
>         .atomic_enable = st7920_crtc_atomic_enable,
>         .atomic_disable = st7920_crtc_atomic_disable,
>  };
>
>  static const struct drm_crtc_funcs st7920_crtc_funcs = {
> -       .reset = st7920_crtc_reset,
> +       .atomic_create_state = st7920_crtc_create_state,
>         .destroy = drm_crtc_cleanup,
>         .set_config = drm_atomic_helper_set_config,
>         .page_flip = drm_atomic_helper_page_flip,
>         .atomic_duplicate_state = st7920_crtc_duplicate_state,
>         .atomic_destroy_state = st7920_crtc_destroy_state,
>
> --
> 2.55.0
>

[PATCH v2 12/13] drm/atomic-helper: Remove __drm_atomic_helper_crtc_reset

[Maxime Ripard]

All users of __drm_atomic_helper_crtc_reset() have been converted to use
drm_atomic_helper_crtc_state_init() and atomic_create_state.

Remove the now unused helper.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 26 --------------------------
 include/drm/drm_atomic_state_helper.h     |  2 --
 2 files changed, 28 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index b592b7f5f264..354fa9c6aca8 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -77,36 +77,10 @@ __drm_atomic_helper_crtc_state_init(struct drm_crtc_state *crtc_state,
 	crtc_state->crtc = crtc;
 	crtc_state->background_color = DRM_ARGB64_PREP(0xffff, 0, 0, 0);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_state_init);
 
-/**
- * __drm_atomic_helper_crtc_reset - reset state on CRTC
- * @crtc: drm CRTC
- * @crtc_state: CRTC state to assign
- *
- * Initializes the newly allocated @crtc_state and assigns it to
- * the &drm_crtc->state pointer of @crtc, usually required when
- * initializing the drivers or when called from the &drm_crtc_funcs.reset
- * hook.
- *
- * This is useful for drivers that subclass the CRTC state.
- */
-void
-__drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
-			       struct drm_crtc_state *crtc_state)
-{
-	if (crtc_state)
-		__drm_atomic_helper_crtc_state_init(crtc_state, crtc);
-
-	if (drm_dev_has_vblank(crtc->dev))
-		drm_crtc_vblank_reset(crtc);
-
-	crtc->state = crtc_state;
-}
-EXPORT_SYMBOL(__drm_atomic_helper_crtc_reset);
-
 /**
  * drm_atomic_helper_crtc_create_state - default &drm_crtc_funcs.atomic_create_state hook for crtcs
  * @crtc: crtc object
  *
  * Allocates and  initializes pristine @drm_crtc_state.
diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
index df70817c94fc..480eaae42f45 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -40,12 +40,10 @@ struct drm_private_state;
 struct drm_modeset_acquire_ctx;
 struct drm_device;
 
 void __drm_atomic_helper_crtc_state_init(struct drm_crtc_state *state,
 					  struct drm_crtc *crtc);
-void __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
-				    struct drm_crtc_state *state);
 struct drm_crtc_state *
 drm_atomic_helper_crtc_create_state(struct drm_crtc *crtc);
 void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 					      struct drm_crtc_state *state);
 struct drm_crtc_state *

-- 
2.55.0

[PATCH v2 13/13] drm/crtc: Remove reset

[Maxime Ripard]

All crtc drivers now implement the atomic_create_state hook instead of
the reset hook for initial state creation. The reset hook conflated
initial state allocation at probe time with hardware and software reset
during suspend/resume, making error handling difficult since it is not
fallible.

Remove the reset hook from struct drm_crtc_funcs and the associated
call in drm_mode_config_reset().

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_mode_config.c |  4 +---
 include/drm/drm_crtc.h            | 10 ----------
 2 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index bb2efc274323..5eed5c194b66 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -305,13 +305,11 @@ void drm_mode_config_reset(struct drm_device *dev)
 		else if (plane->funcs->atomic_create_state)
 			drm_mode_config_plane_reset_with_create_state(plane);
 	}
 
 	drm_for_each_crtc(crtc, dev) {
-		if (crtc->funcs->reset)
-			crtc->funcs->reset(crtc);
-		else if (crtc->funcs->atomic_create_state)
+		if (crtc->funcs->atomic_create_state)
 			drm_mode_config_crtc_reset_with_create_state(crtc);
 	}
 
 	drm_for_each_encoder(encoder, dev)
 		if (encoder->funcs && encoder->funcs->reset)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 12f43253f059..be5dca538d87 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -421,20 +421,10 @@ struct drm_crtc_state {
  * Each driver is responsible for filling out this structure at startup time,
  * in addition to providing other modesetting features, like i2c and DDC
  * bus accessors.
  */
 struct drm_crtc_funcs {
-	/**
-	 * @reset:
-	 *
-	 * Reset CRTC hardware and software state to off. This function isn't
-	 * called by the core directly, only through drm_mode_config_reset().
-	 * It's not a helper hook only for historical reasons.
-	 *
-	 */
-	void (*reset)(struct drm_crtc *crtc);
-
 	/**
 	 * @cursor_set:
 	 *
 	 * Update the cursor image. The cursor position is relative to the CRTC
 	 * and can be partially or fully outside of the visible area.

-- 
2.55.0

Re: [PATCH v2 13/13] drm/crtc: Remove reset

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked dereference of optional `crtc->helper_private` in `drm_mode_config_crtc_reset_with_create_state()` before calling `hw_reset`.
--

commit c111ccec175b0bcef9dba73f9b4a2e31404bd779
Author: Maxime Ripard <mripard@kernel.org>
drm/crtc: Remove reset

This commit removes the reset hook from struct drm_crtc_funcs. As a result,
all crtc drivers now rely on the atomic_create_state hook for initial state
creation, and drm_mode_config_reset() is updated to reflect this.

> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index bb2efc274323e..5eed5c194b661 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -307,9 +307,7 @@ void drm_mode_config_reset(struct drm_device *dev)
>  	}
>  
>  	drm_for_each_crtc(crtc, dev) {
> -		if (crtc->funcs->reset)
> -			crtc->funcs->reset(crtc);
> -		else if (crtc->funcs->atomic_create_state)
> +		if (crtc->funcs->atomic_create_state)
>  			drm_mode_config_crtc_reset_with_create_state(crtc);

[Severity: High]
This is a pre-existing issue, but does removing the fallback to the reset
hook expose a kernel panic during initialization or resume?

By forcing all CRTCs that implement atomic_create_state through
drm_mode_config_crtc_reset_with_create_state(), we unconditionally hit a path
that dereferences the optional helper_private pointer without checking if it
exists.

Looking at drm_mode_config_crtc_reset_with_create_state() in
drivers/gpu/drm/drm_mode_config.c:

drm_mode_config_crtc_reset_with_create_state() {
	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
	...
	if (crtc_funcs->hw_reset)
		crtc_funcs->hw_reset(crtc);
	...
}

If an atomic DRM driver provides its own atomic implementations and does not
use the CRTC helpers, helper_private will be NULL. In that case, crtc_funcs
is NULL and the check for crtc_funcs->hw_reset will cause a NULL pointer
dereference.

>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-drm-no-more-crtc-reset-v2-0-4500d49a337f@kernel.org?part=13

Re: [PATCH v2 00/13] drm/crtc: Convert all drivers to atomic_create_state and remove reset

[Thomas Zimmermann]

Hi Maxime,

I've gone over the remaining patches again and it all looks good now.

Best regards
Thomas

Am 31.08.26 um 17:34 schrieb Maxime Ripard:
> This is a follow-up to the plane reset removal series, and part of a
> larger effort to remove the reset hook from all KMS objects.
>
> The CRTC reset hook is overloaded: it is called both at probe time to
> create the initial software state and during suspend/resume to reset
> hardware and software state. These two roles have different
> requirements, and the reset hook is not fallible, making error
> handling difficult for the initial state allocation path.
>
> While reset has the semantics to reset both the software and hardware
> state, the vast majority of implementations and all the helpers only
> reset the software state, making them equivalent to
> atomic_create_state in practice. The atomic_create_state hook makes
> this explicit: it only allocates and initializes a pristine state
> without any side effect, and returns the state pointer or an ERR_PTR
> on failure.
>
> This series converts all CRTC drivers tree-wide from the reset hook to
> atomic_create_state, then removes the reset hook from struct
> drm_crtc_funcs entirely. Two drivers (tilcdc and loongson) had actual
> hardware reset logic mixed into their reset hook; those are moved to
> CRTC creation instead. The conversions were done using a combination of
> Coccinelle semantic patches and manual adjustments.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
> Changes in v2:
> - Fix indentation in logicvc
> - Fix hardware reset at probe and suspend for loonsoon and tilcldc
> - Fix state creation for amdgpu
> - Rebase on current drm-misc-next
> - Fix commit message for st7920
> - Fix merge conflict for mediatek
> - Link to v1: https://lore.kernel.org/r/20260821-drm-no-more-crtc-reset-v1-0-fb793475c05a@kernel.org
>
> ---
> Maxime Ripard (13):
>        drm/crtc: Introduce hw_reset helper hook
>        drm/amdgpu: vkms: Switch to drm_atomic_helper_crtc_create_state
>        drm/logicvc: Switch to drm_atomic_helper_crtc_create_state
>        drm/tilcdc: Move hardware reset to CRTC creation
>        drm/tilcdc: Switch to drm_atomic_helper_crtc_create_state
>        drm/atomic-helper: Remove drm_atomic_helper_crtc_reset
>        drm/amdgpu: dm: Convert to atomic_create_state
>        drm/loongson: Move hardware reset to CRTC creation
>        drm/loongson: Convert to atomic_create_state
>        drm/mediatek: Convert to atomic_create_state
>        drm/sitronix: st7920: Convert to atomic_create_state
>        drm/atomic-helper: Remove __drm_atomic_helper_crtc_reset
>        drm/crtc: Remove reset
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c           |  2 +-
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 28 +++++++++-----
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.h |  2 +-
>   .../display/amdgpu_dm/tests/amdgpu_dm_crtc_test.c  | 20 +++++-----
>   drivers/gpu/drm/drm_atomic_state_helper.c          | 45 ----------------------
>   drivers/gpu/drm/drm_mode_config.c                  | 17 ++++++--
>   drivers/gpu/drm/logicvc/logicvc_crtc.c             |  2 +-
>   drivers/gpu/drm/loongson/lsdc_crtc.c               | 30 ++++++++-------
>   drivers/gpu/drm/mediatek/mtk_crtc.c                | 18 ++++-----
>   drivers/gpu/drm/sitronix/st7920.c                  | 12 +++---
>   drivers/gpu/drm/tilcdc/tilcdc_crtc.c               |  7 ++--
>   drivers/gpu/drm/tilcdc/tilcdc_drv.c                |  2 +
>   include/drm/drm_atomic_state_helper.h              |  3 --
>   include/drm/drm_crtc.h                             | 12 ------
>   include/drm/drm_modeset_helper_vtables.h           | 14 +++++++
>   15 files changed, 94 insertions(+), 120 deletions(-)
> ---
> base-commit: 07c66ce1e6446f5cc6a4ace99a8d46880f45c527
> change-id: 20260709-drm-no-more-crtc-reset-14a4d5bc8d41
>
> Best regards,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)