drm/crtc: Convert all drivers to atomic_create_state and remove reset
Subsystems (75)
Maxime Ripard
maintainercover drm/crtc: Convert all drivers to atomic_create_state and remove reset
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.
---
Maxime Ripard (70):
drm/adp: Switch to drm_atomic_helper_crtc_create_state drm/amdgpu: vkms: Switch to drm_atomic_helper_crtc_create_state drm/arm: hdlcd: Switch to drm_atomic_helper_crtc_create_state drm/armada: Switch to drm_atomic_helper_crtc_create_state drm/exynos: Switch to drm_atomic_helper_crtc_create_state drm/fsl-dcu: Switch to drm_atomic_helper_crtc_create_state drm/gud: Switch to drm_atomic_helper_crtc_create_state drm/hisilicon: hibmc: Switch to drm_atomic_helper_crtc_create_state drm/hisilicon: kirin: Switch to drm_atomic_helper_crtc_create_state drm/hyperv: Switch to drm_atomic_helper_crtc_create_state drm/imx: dc: Switch to drm_atomic_helper_crtc_create_state drm/imx: dcss: Switch to drm_atomic_helper_crtc_create_state drm/ingenic: Switch to drm_atomic_helper_crtc_create_state drm/kmb: Switch to drm_atomic_helper_crtc_create_state drm/logicvc: Switch to drm_atomic_helper_crtc_create_state drm/meson: Switch to drm_atomic_helper_crtc_create_state drm/msm: mdp4: Switch to drm_atomic_helper_crtc_create_state drm/mxs: mxsfb: Switch to drm_atomic_helper_crtc_create_state drm/qxl: Switch to drm_atomic_helper_crtc_create_state drm/renesas: shmobile: Switch to drm_atomic_helper_crtc_create_state drm/simple-kms: Remove unused reset_crtc hook drm/simple-kms: Switch to drm_atomic_helper_crtc_create_state drm/sitronix: st7571: Switch to drm_atomic_helper_crtc_create_state drm/sprd: Switch to drm_atomic_helper_crtc_create_state drm/sti: Switch to drm_atomic_helper_crtc_create_state drm/stm: Switch to drm_atomic_helper_crtc_create_state drm/sun4i: Switch to drm_atomic_helper_crtc_create_state drm/tests: 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/tiny: appletbdrm: Switch to drm_atomic_helper_crtc_create_state drm/tiny: bochs: Switch to drm_atomic_helper_crtc_create_state drm/tiny: cirrus: Switch to drm_atomic_helper_crtc_create_state drm/tiny: pixpaper: Switch to drm_atomic_helper_crtc_create_state drm/tiny: sharp: Switch to drm_atomic_helper_crtc_create_state drm/udl: Switch to drm_atomic_helper_crtc_create_state drm/vbox: Switch to drm_atomic_helper_crtc_create_state drm/verisilicon: Switch to drm_atomic_helper_crtc_create_state drm/virtio: Switch to drm_atomic_helper_crtc_create_state drm/xlnx: Switch to drm_atomic_helper_crtc_create_state drm/mipi-dbi: Switch to drm_atomic_helper_crtc_create_state drm/atomic-helper: Remove drm_atomic_helper_crtc_reset
sysfb: Convert to create_state
drm/amdgpu: dm: Convert to atomic_create_state drm/komeda: Convert to atomic_create_state
drm/malidp: Convert to atomic_create_state
drm/ast: Convert to atomic_create_state
drm/atmel-hlcdc: Convert to atomic_create_state
drm/imx: ipuv3: Convert to atomic_create_state drm/loongsoon: Move hardware reset to CRTC creation drm/loongson: Convert to atomic_create_state drm/mediatek: Convert to atomic_create_state drm/mgag200: Convert to atomic_create_state
drm/msm: dpu1: Convert to atomic_create_state
drm/msm: mdp5: Convert to atomic_create_state
drm/mxsfb: lcdif: Convert to atomic_create_state drm/nouveau: Convert to atomic_create_state
drm/omapdrm: Convert to atomic_create_state
drm/rcar-du: Convert to atomic_create_state drm/rzg2l-du: Convert to atomic_create_state drm/rockchip: vop: Convert to atomic_create_state drm/rockchip: vop2: Convert to atomic_create_state drm/sitronix: st7920: Convert to atomic_create_state drm/solomon: ssd130x: Convert to atomic_create_state drm/tegra: Convert to atomic_create_state
drm/vc4: Convert to atomic_create_state
drm/vkms: Convert to atomic_create_state
drm/vmwgfx: Convert to atomic_create_state drm/atomic-helper: Remove __drm_atomic_helper_crtc_reset drm/crtc: Remove reset
drivers/gpu/drm/adp/adp_drv.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 17 ++++---- drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 18 ++++---- drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +- drivers/gpu/drm/arm/malidp_crtc.c | 18 ++++---- drivers/gpu/drm/armada/armada_crtc.c | 2 +- drivers/gpu/drm/ast/ast_mode.c | 18 ++++---- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 19 ++++----- drivers/gpu/drm/drm_atomic_state_helper.c | 45 -------------------- drivers/gpu/drm/drm_mode_config.c | 4 +- drivers/gpu/drm/drm_simple_kms_helper.c | 13 +----- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 +- drivers/gpu/drm/gud/gud_drv.c | 2 +- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 2 +- drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +- drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 2 +- drivers/gpu/drm/imx/dc/dc-crtc.c | 2 +- drivers/gpu/drm/imx/dcss/dcss-crtc.c | 2 +- drivers/gpu/drm/imx/ipuv3/ipuv3-crtc.c | 18 ++++---- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +- drivers/gpu/drm/kmb/kmb_crtc.c | 2 +- drivers/gpu/drm/logicvc/logicvc_crtc.c | 2 +- drivers/gpu/drm/loongson/lsdc_crtc.c | 26 ++++++------ drivers/gpu/drm/mediatek/mtk_crtc.c | 18 ++++---- drivers/gpu/drm/meson/meson_crtc.c | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 4 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 15 ++++--- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 18 ++++---- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 2 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 20 ++++----- drivers/gpu/drm/mxsfb/lcdif_kms.c | 17 ++++---- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 4 +- drivers/gpu/drm/nouveau/dispnv50/head.c | 14 +++---- drivers/gpu/drm/omapdrm/omap_crtc.c | 17 ++++---- drivers/gpu/drm/qxl/qxl_display.c | 2 +- drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c | 17 ++++---- drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c | 15 +++---- drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 2 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 18 ++++---- drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 18 ++++---- drivers/gpu/drm/sitronix/st7571.c | 2 +- drivers/gpu/drm/sitronix/st7920.c | 10 +++-- drivers/gpu/drm/solomon/ssd130x.c | 10 +++-- drivers/gpu/drm/sprd/sprd_dpu.c | 2 +- drivers/gpu/drm/sti/sti_crtc.c | 2 +- drivers/gpu/drm/stm/ltdc.c | 4 +- drivers/gpu/drm/sun4i/sun4i_crtc.c | 2 +- drivers/gpu/drm/sysfb/drm_sysfb_helper.h | 4 +- drivers/gpu/drm/sysfb/drm_sysfb_modeset.c | 20 ++++----- drivers/gpu/drm/tegra/dc.c | 18 ++++---- drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 49 +++++++++------------- drivers/gpu/drm/tiny/appletbdrm.c | 2 +- drivers/gpu/drm/tiny/bochs.c | 2 +- drivers/gpu/drm/tiny/cirrus-qemu.c | 2 +- drivers/gpu/drm/tiny/pixpaper.c | 2 +- drivers/gpu/drm/tiny/sharp-memory.c | 2 +- drivers/gpu/drm/udl/udl_modeset.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_mode.c | 2 +- drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c | 2 +- drivers/gpu/drm/vc4/vc4_crtc.c | 14 +++---- drivers/gpu/drm/vc4/vc4_drv.h | 2 +- drivers/gpu/drm/vc4/vc4_txp.c | 2 +- drivers/gpu/drm/verisilicon/vs_crtc.c | 2 +- drivers/gpu/drm/virtio/virtgpu_display.c | 2 +- drivers/gpu/drm/vkms/vkms_crtc.c | 18 ++++---- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 ++++------ drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +- drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +- include/drm/drm_atomic_state_helper.h | 3 -- include/drm/drm_crtc.h | 12 ------ include/drm/drm_mipi_dbi.h | 2 +- include/drm/drm_simple_kms_helper.h | 1 - 78 files changed, 275 insertions(+), 385 deletions(-)
---
base-commit: 7ea58b9ddb52264a4ed36bccb975062ada32c62d
change-id: 20260709-drm-no-more-crtc-reset-14a4d5bc8d41
Best regards,
Signature · 1 line
Maxime Ripard <[email protected]>
Maxime Ripard
maintainerPATCH 1/70 drm/adp: Switch to drm_atomic_helper_crtc_create_state
The adp crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Janne Grunau <[email protected]>
Cc: Sasha Finkelstein <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/adp/adp_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/adp/adp_drv.c +1 -1
@@ -349,11 +349,11 @@ static void adp_crtc_atomic_flush(struct drm_crtc *crtc,static const struct drm_crtc_funcs adp_crtc_funcs = { .destroy = drm_crtc_cleanup, .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 = adp_crtc_enable_vblank, .disable_vblank = adp_crtc_disable_vblank, };
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 01/70] drm/adp: Switch to drm_atomic_helper_crtc_create_state
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing NULL check for dma_alloc_coherent and potential integer overflow in new_size calculation.
- [High] Calling dma_alloc_coherent() with GFP_KERNEL during atomic_flush violates DRM atomic context constraints. --
--- Patch [1]: [PATCH 01/70] drm/adp: Switch to drm_atomic_helper_crtc_create_state --- commit 3f2217690a441eaa2502ebbe2a7e5840385ac1e3
Author: Maxime Ripard <[email protected]>
drm/adp: Switch to drm_atomic_helper_crtc_create_state
The adp crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
drivers/gpu/drm/adp/adp_drv.c static const struct drm_crtc_funcs adp_crtc_funcs = { · line 357
Quoted 13 lines
drivers/gpu/drm/adp/adp_drv.c +1
-1@@ -351,7 +351,7 @@ static const struct drm_crtc_funcs adp_crtc_funcs = { .destroy = drm_crtc_cleanup, .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 = adp_crtc_enable_vblank,
[Severity: High]
This is a pre-existing issue, but does calling dma_alloc_coherent() with GFP_KERNEL in adp_crtc_atomic_flush() violate DRM atomic context constraints?
drivers/gpu/drm/adp/adp_drv.c:adp_crtc_atomic_flush() {
...
if (new_size != 0) {
adp->mask_buf = dma_alloc_coherent(crtc->dev->dev, new_size,
&adp->mask_iova, GFP_KERNEL);adp_crtc_atomic_flush() executes during the hardware programming phase of drm_atomic_helper_commit_tail(), which is an atomic context where sleeping is forbidden.
dma_alloc_coherent() with GFP_KERNEL can sleep, potentially causing kernel warnings and deadlocks.
[Severity: High]
This isn't a bug introduced by this patch, but what happens if dma_alloc_coherent() returns NULL?
drivers/gpu/drm/adp/adp_drv.c:adp_crtc_atomic_flush() {
...
if (new_size != 0) {
adp->mask_buf = dma_alloc_coherent(crtc->dev->dev, new_size,
&adp->mask_iova, GFP_KERNEL);
memset(adp->mask_buf, 0xFF, new_size);The immediate memset() would dereference the NULL pointer.
Additionally, does the calculation of new_size in adp_crtc_atomic_flush() run the risk of a 32-bit integer overflow for maliciously large resolutions?
u64 new_size = ALIGN(new_state->mode.hdisplay * new_state->mode.vdisplay * 4, PAGE_SIZE);
Is there a chance hdisplay * vdisplay * 4 could overflow before being cast to u64?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Maxime Ripard
maintainerPATCH 2/70 drm/amdgpu: vkms: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe 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.
---
Cc: "Christian König" <[email protected]>
Cc: Alex Deucher <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c +1 -1
@@ -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 , };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 3/70 drm/arm: hdlcd: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe hdlcd crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Liviu Dudau <[email protected]>
--- drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/arm/hdlcd_crtc.c +1 -1
@@ -65,11 +65,11 @@ static void hdlcd_crtc_disable_vblank(struct drm_crtc *crtc)static const struct drm_crtc_funcs hdlcd_crtc_funcs = { .destroy = hdlcd_crtc_cleanup, .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 = hdlcd_crtc_enable_vblank, .disable_vblank = hdlcd_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Re: [PATCH 03/70] drm/arm: hdlcd: Switch to drm_atomic_helper_crtc_create_state
Quoted 6 lines · Maxime Ripard
The hdlcd crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset()as its reset hook.Switch to
drm_atomic_helper_crtc_create_state()instead.Signed-off-by Maxime Ripard <[email protected]>
Best regards,
Liviu
Quoted 27 lines
---
Cc: Liviu Dudau <[email protected]>--- drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/arm/hdlcd_crtc.c +1
-1@@ -65,11 +65,11 @@ static void hdlcd_crtc_disable_vblank(struct drm_crtc *crtc)static const struct drm_crtc_funcs hdlcd_crtc_funcs = { .destroy = hdlcd_crtc_cleanup, .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 = hdlcd_crtc_enable_vblank, .disable_vblank = hdlcd_crtc_disable_vblank, };Signature · 2 lines
-- 2.55.0
Signature · 8 lines
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯Maxime Ripard
maintainerPATCH 4/70 drm/armada: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe armada crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Russell King <[email protected]>
--- drivers/gpu/drm/armada/armada_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/armada/armada_crtc.c +1 -1
@@ -815,11 +815,11 @@ static void armada_drm_crtc_disable_vblank(struct drm_crtc *crtc)armada_drm_crtc_disable_irq (dcrtc,VSYNC_IRQ_ENA );spin_unlock_irqrestore (&dcrtc->irq_lock, flags); }static const struct drm_crtc_funcs armada_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .cursor_set = armada_drm_crtc_cursor_set, .cursor_move = armada_drm_crtc_cursor_move, .destroy = armada_drm_crtc_destroy, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 5/70 drm/exynos: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe exynos crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Alim Akhtar <[email protected]>
Cc: Inki Dae <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Kyungmin Park <[email protected]>
Cc: Peter Griffin <[email protected]>
Cc: Seung-Woo Kim <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/exynos/exynos_drm_crtc.c +1 -1
@@ -161,11 +161,11 @@ static void exynos_drm_crtc_disable_vblank(struct drm_crtc *crtc)static const struct drm_crtc_funcs exynos_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .destroy = exynos_drm_crtc_destroy, - .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 = exynos_drm_crtc_enable_vblank, .disable_vblank = exynos_drm_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 6/70 drm/fsl-dcu: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe fsl-dcu crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Alison Wang <[email protected]>
Cc: Stefan Agner <[email protected]>
--- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c +1 -1
@@ -159,11 +159,11 @@ static void fsl_dcu_drm_crtc_disable_vblank(struct drm_crtc *crtc)static const struct drm_crtc_funcs fsl_dcu_drm_crtc_funcs = { .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, .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, .set_config = drm_atomic_helper_set_config, .enable_vblank = fsl_dcu_drm_crtc_enable_vblank, .disable_vblank = fsl_dcu_drm_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 7/70 drm/gud: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe gud crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Ruben Wauters <[email protected]>
--- drivers/gpu/drm/gud/gud_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/gud/gud_drv.c +1 -1
@@ -343,11 +343,11 @@ static const struct drm_crtc_helper_funcs gud_crtc_helper_funcs = { .atomic_enable = gud_crtc_atomic_enable, .atomic_disable = gud_crtc_atomic_disable, };static const struct drm_crtc_funcs gud_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
Re: [PATCH 07/70] drm/gud: Switch to drm_atomic_helper_crtc_create_state
Quoted 6 lines · Maxime Ripard
The gud crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset()as its reset hook.Switch to
drm_atomic_helper_crtc_create_state()instead.Signed-off-by Maxime Ripard <[email protected]>
Quoted 23 lines
---
Cc: Ruben Wauters <[email protected]>--- drivers/gpu/drm/gud/gud_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/gud/gud_drv.c +1
-1@@ -343,11 +343,11 @@ static const struct drm_crtc_helper_funcs gud_crtc_helper_funcs = { .atomic_enable = gud_crtc_atomic_enable, .atomic_disable = gud_crtc_atomic_disable, };static const struct drm_crtc_funcs gud_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Maxime Ripard
maintainerPATCH 8/70 drm/hisilicon: hibmc: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe hibmc crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Tian Tao <[email protected]>
Cc: Xinwei Kong <[email protected]>
Cc: Yongbang Shi <[email protected]>
--- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c +1 -1
@@ -489,11 +489,11 @@ static int hibmc_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,static const struct drm_crtc_funcs hibmc_crtc_funcs = { .page_flip = drm_atomic_helper_page_flip, .set_config = drm_atomic_helper_set_config, .destroy = drm_crtc_cleanup, - .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 = hibmc_crtc_enable_vblank, .disable_vblank = hibmc_crtc_disable_vblank, .gamma_set = hibmc_crtc_gamma_set,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 9/70 drm/hisilicon: kirin: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe kirin crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: John Stultz <[email protected]>
Cc: Sumit Semwal <[email protected]>
Cc: Xinliang Liu <[email protected]>
Cc: Yongqin Liu <[email protected]>
--- drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c +1 -1
@@ -538,11 +538,11 @@ static const struct drm_crtc_helper_funcs ade_crtc_helper_funcs = {static const struct drm_crtc_funcs ade_crtc_funcs = { .destroy = drm_crtc_cleanup, .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 = ade_crtc_enable_vblank, .disable_vblank = ade_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Re: [PATCH 09/70] drm/hisilicon: kirin: Switch to drm_atomic_helper_crtc_create_state
Quoted 7 lines · Maxime Ripard
The kirin crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset()as its reset hook.Switch to
drm_atomic_helper_crtc_create_state()instead.Signed-off-by Maxime Ripard <[email protected]>
I no longer have hardware access, but looks ok.
Maxime Ripard
maintainerPATCH 10/70 drm/hyperv: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe hyperv crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Dexuan Cui <[email protected]>
Cc: Long Li <[email protected]>
Cc: Saurabh Sengar <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c +1 -1
@@ -124,11 +124,11 @@ static const struct drm_crtc_helper_funcs hv_drm_crtc_helper_funcs = { .atomic_enable = hv_drm_crtc_helper_atomic_enable, .atomic_disable = drm_crtc_vblank_atomic_disable, };static const struct drm_crtc_funcs hv_drm_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 11/70 drm/imx: dc: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe imx dc crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Fabio Estevam <[email protected]>
Cc: Frank Li <[email protected]>
Cc: Liu Ying <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/imx/dc/dc-crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/imx/dc/dc-crtc.c +1 -1
@@ -96,11 +96,11 @@ static void dc_crtc_disable_vblank(struct drm_crtc *crtc)/* nosync due to atomic context */ disable_irq_nosync (dc_crtc->irq_dec_framecomplete); }static const struct drm_crtc_funcs dc_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 11/70] drm/imx: dc: Switch to drm_atomic_helper_crtc_create_state
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] CRTC atomic callbacks call sleeping functions in atomic context. --
--- Patch [11]: [PATCH 11/70] drm/imx: dc: Switch to drm_atomic_helper_crtc_create_state --- commit 81ac70d774e55256bc7f69e9538e45aed1e198c8
Author: Maxime Ripard <[email protected]>
drm/imx: dc: Switch to drm_atomic_helper_crtc_create_state
The imx dc crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook. Switch to drm_atomic_helper_crtc_create_state() instead.
drivers/gpu/drm/imx/dc/dc-crtc.c static void dc_crtc_disable_vblank(struct drm_crtc *crtc) · line 101
Quoted 10 lines
drivers/gpu/drm/imx/dc/dc-crtc.c +1
-1@@ -98,7 +98,7 @@ static void dc_crtc_disable_vblank(struct drm_crtc *crtc) }static const struct drm_crtc_funcs dc_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state,
[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the CRTC implementation in this file, I noticed that several atomic callbacks might be calling sleeping functions.
For example, does dc_crtc_atomic_enable() call pm_runtime_resume_and_get(), which can sleep?
Additionally, it looks like dc_crtc_atomic_enable(), dc_crtc_atomic_flush(), and dc_crtc_atomic_disable() all use DC_CRTC_WAIT_FOR_COMPLETION_TIMEOUT(), which waits for a completion and puts the task to sleep.
According to DRM subsystem guidelines, CRTC atomic enable, disable, and update callbacks run in an atomic context where sleeping is forbidden. Could this cause scheduling while atomic panics or warnings during non-blocking commits and page flips?
Quoted 3 lines
drivers/gpu/drm/imx/dc/dc-crtc.c +0
-0.destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip,
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
Maxime Ripard
maintainerPATCH 12/70 drm/imx: dcss: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe imx dcss crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Fabio Estevam <[email protected]>
Cc: Frank Li <[email protected]>
Cc: Laurentiu Palcu <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/imx/dcss/dcss-crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/imx/dcss/dcss-crtc.c +1 -1
@@ -43,11 +43,11 @@ static void dcss_disable_vblank(struct drm_crtc *crtc)static const struct drm_crtc_funcs dcss_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, .enable_vblank = dcss_enable_vblank, .disable_vblank = dcss_disable_vblank, };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 13/70 drm/ingenic: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe ingenic crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Paul Cercueil <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/ingenic/ingenic-drm-drv.c +1 -1
@@ -995,11 +995,11 @@ static const struct drm_plane_funcs ingenic_drm_primary_plane_funcs = { };static const struct drm_crtc_funcs ingenic_drm_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, .destroy = drm_crtc_cleanup, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
Re: [PATCH 13/70] drm/ingenic: Switch to drm_atomic_helper_crtc_create_state
Hi Maxime,
Le vendredi 21 août 2026 à 16:38 +0200, Maxime Ripard a écrit :
Quoted 6 lines
The ingenic crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset()as its reset hook.Switch to
drm_atomic_helper_crtc_create_state()instead.Signed-off-by Maxime Ripard <[email protected]>
Cheers,
-Paul
Quoted 28 lines
---
Cc: Paul Cercueil <[email protected]>
Cc: [email protected]
---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +-1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/ingenic/ingenic-drm-drv.c +0
-0@@ -995,11 +995,11 @@ static const struct drm_plane_funcs ingenic_drm_primary_plane_funcs = { };
static const structdrm_crtc_funcsingenic_drm_crtc_funcs= {
.set_config =drm_atomic_helper_set_config,
.page_flip =drm_atomic_helper_page_flip,drivers/gpu/drm/ingenic/ingenic-drm-drv.c +1
-1- .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state,.destroy =
drm_crtc_cleanup,
.atomic_duplicate_state =drm_atomic_helper_crtc_duplicate_state,
.atomic_destroy_state =drm_atomic_helper_crtc_destroy_state,
Maxime Ripard
maintainerPATCH 14/70 drm/kmb: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe kmb crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Anitha Chrisanthus <[email protected]>
--- drivers/gpu/drm/kmb/kmb_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/kmb/kmb_crtc.c +1 -1
@@ -56,11 +56,11 @@ static void kmb_crtc_disable_vblank(struct drm_crtc *crtc)static const struct drm_crtc_funcs kmb_crtc_funcs = { .destroy = drm_crtc_cleanup, .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 = kmb_crtc_enable_vblank, .disable_vblank = kmb_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 15/70 drm/logicvc: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe logicvc crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Paul Kocialkowski <[email protected]>
--- drivers/gpu/drm/logicvc/logicvc_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/logicvc/logicvc_crtc.c +1 -1
@@ -211,11 +211,11 @@ 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, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
Paul Kocialkowski
maintainerRe: [PATCH 15/70] drm/logicvc: Switch to drm_atomic_helper_crtc_create_state
Hi Maxime,
Quoted 17 lines · 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.Signed-off-by Maxime Ripard <[email protected]>---
Cc: Paul Kocialkowski <[email protected]>--- drivers/gpu/drm/logicvc/logicvc_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/logicvc/logicvc_crtc.c +0
-0
drivers/gpu/drm/logicvc/logicvc_crtc.c static void logicvc_crtc_disable_vblank(struct drm_crtc *drm_crtc) · line 216
Quoted 7 lines
drivers/gpu/drm/logicvc/logicvc_crtc.c +1
-1@@ -211,11 +211,11 @@ 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, + .atomic_create_state = drm_atomic_helper_crtc_create_state,
It looks like you are breaking the tab indent and replacing it with a single space here. Could you keep the tab alignment instead?
Thanks,
Paul
Quoted 9 lines
drivers/gpu/drm/logicvc/logicvc_crtc.c +0
-0.destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,Signature · 2 lines
-- 2.55.0
Signature · 6 lines
Paul Kocialkowski, Independent contractor - sys-base - https://www.sys-base.io/ Free software developer - https://www.paulk.fr/ Expert in multimedia, graphics and embedded hardware support with Linux.
Maxime Ripard
maintainerPATCH 16/70 drm/meson: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe meson crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Jerome Brunet <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: Martin Blumenstingl <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/meson/meson_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/meson/meson_crtc.c +1 -1
@@ -72,11 +72,11 @@ static void meson_crtc_disable_vblank(struct drm_crtc *crtc)static const struct drm_crtc_funcs meson_crtc_funcs = { .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .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, .set_config = drm_atomic_helper_set_config, .enable_vblank = meson_crtc_enable_vblank, .disable_vblank = meson_crtc_disable_vblank, };
Signature · 7 lines
2.55.0 _______________________________________________ linux-amlogic mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-amlogic
Maxime Ripard
maintainerPATCH 17/70 drm/msm: mdp4: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe mdp4 crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Abhinav Kumar <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Cc: Jessica Zhang <[email protected]>
Cc: Marijn Suijten <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Sean Paul <[email protected]>
Cc: [email protected]
Cc: [email protected]
--- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c +1 -1
@@ -475,11 +475,11 @@ static int mdp4_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)static const struct drm_crtc_funcs mdp4_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .cursor_set = mdp4_crtc_cursor_set, .cursor_move = mdp4_crtc_cursor_move, - .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 = msm_crtc_enable_vblank, .disable_vblank = msm_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Re: [PATCH 17/70] drm/msm: mdp4: Switch to drm_atomic_helper_crtc_create_state
Quoted 15 lines · Maxime Ripard
The mdp4 crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset()as its reset hook.Switch to
drm_atomic_helper_crtc_create_state()instead.Signed-off-by Maxime Ripard <[email protected]>---
Cc: Abhinav Kumar <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Cc: Jessica Zhang <[email protected]>
Cc: Marijn Suijten <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Sean Paul <[email protected]>
Cc: [email protected]
Cc: [email protected]
Quoted 4 lines
--- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
For this and other msm commits, Ack to merge them through drm-misc
Signature · 2 lines
With best wishes Dmitry
Maxime Ripard
maintainerPATCH 18/70 drm/mxs: mxsfb: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe mxsfb crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Fabio Estevam <[email protected]>
Cc: Frank Li <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Stefan Agner <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/mxsfb/mxsfb_kms.c +2 -2
@@ -482,22 +482,22 @@ static const struct drm_crtc_helper_funcs mxsfb_crtc_helper_funcs = { .atomic_enable = mxsfb_crtc_atomic_enable, .atomic_disable = mxsfb_crtc_atomic_disable, };static const struct drm_crtc_funcs mxsfb_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, .enable_vblank = mxsfb_crtc_enable_vblank, .disable_vblank = mxsfb_crtc_disable_vblank, };static const struct drm_crtc_funcs mxsfb_crtc_with_crc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 18/70] drm/mxs: mxsfb: Switch to drm_atomic_helper_crtc_create_state
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free in DRM cleanup paths on driver unbind due to devm_kzalloc allocated driver private data. --
--- Patch [18]: [PATCH 18/70] drm/mxs: mxsfb: Switch to drm_atomic_helper_crtc_create_state --- commit 176787b6391787b0d414456f33f183b472ea7c41
Author: Maxime Ripard <[email protected]>
drm/mxs: mxsfb: Switch to drm_atomic_helper_crtc_create_state
This commit replaces the deprecated drm_atomic_helper_crtc_reset() hook with drm_atomic_helper_crtc_create_state() for the mxsfb CRTC implementation.
drivers/gpu/drm/mxsfb/mxsfb_kms.c static const struct drm_crtc_helper_funcs mxsfb_crtc_helper_funcs = { · line 490
Quoted 13 lines
drivers/gpu/drm/mxsfb/mxsfb_kms.c +1
-1@@ -484,7 +484,7 @@ static const struct drm_crtc_helper_funcs mxsfb_crtc_helper_funcs = { };static const struct drm_crtc_funcs mxsfb_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip,
[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the mxsfb driver, I noticed a potential use-after-free during driver unbind.
The driver private structure is allocated with devm_kzalloc() in mxsfb_load() in drivers/gpu/drm/mxsfb/mxsfb_drv.c:
mxsfb = devm_kzalloc(&pdev->dev, sizeof(*mxsfb), GFP_KERNEL);
And the DRM device is allocated independently via drm_dev_alloc() in mxsfb_probe():
drm = drm_dev_alloc(&mxsfb_driver, &pdev->dev);
During driver unbind, mxsfb_remove() calls drm_dev_put(drm). If user space currently holds an open file descriptor for the DRM device, the DRM device's refcount is > 0 and it will not be freed immediately.
When mxsfb_remove() returns, devres cleanup will free the mxsfb_drm_private structure. Later, when user space closes the file descriptor, drm_dev_release() will trigger drmm_mode_config_cleanup().
Since drmm_mode_config_cleanup() will access CRTCs and planes embedded within the already-freed mxsfb_drm_private structure, could this result in a use-after-free?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=18
Maxime Ripard
maintainerPATCH 19/70 drm/qxl: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe qxl crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Dave Airlie <[email protected]>
Cc: Gerd Hoffmann <[email protected]>
Cc: [email protected]
Cc: [email protected]
--- drivers/gpu/drm/qxl/qxl_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/qxl/qxl_display.c +1 -1
@@ -416,11 +416,11 @@ static void qxl_crtc_destroy(struct drm_crtc *crtc)static const struct drm_crtc_funcs qxl_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .destroy = qxl_crtc_destroy, .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 , };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 20/70 drm/renesas: shmobile: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe shmobile crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Geert Uytterhoeven <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c +1 -1
@@ -362,11 +362,11 @@ static void shmob_drm_disable_vblank(struct drm_crtc *crtc)shmob_drm_crtc_enable_vblank (sdev, false); }static const struct drm_crtc_funcs crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = shmob_drm_crtc_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 21/70 drm/simple-kms: Remove unused reset_crtc hook
In reply to Maxime RipardThe reset_crtc hook in drm_simple_display_pipe_funcs allows simple KMS drivers to provide a custom CRTC reset implementation. However, no driver ever implemented it.
Remove the hook and the wrapper function, and usedrm_atomic_helper_crtc_reset() directly.
--- drivers/gpu/drm/drm_simple_kms_helper.c | 13 +------------ include/drm/drm_simple_kms_helper.h | 1 - 2 files changed, 1 insertion(+), 13 deletions(-)
drivers/gpu/drm/drm_simple_kms_helper.c +1 -12
@@ -100,21 +100,10 @@ static const struct drm_crtc_helper_funcs drm_simple_kms_crtc_helper_funcs = { .atomic_check = drm_simple_kms_crtc_check, .atomic_enable = drm_simple_kms_crtc_enable, .atomic_disable = drm_simple_kms_crtc_disable, }; -static void drm_simple_kms_crtc_reset (struct drm_crtc *crtc) -{ -struct drm_simple_display_pipe *pipe; - - pipe = container_of(crtc, struct drm_simple_display_pipe, crtc); -if (!pipe->funcs || !pipe->funcs->reset_crtc) -return drm_atomic_helper_crtc_reset (crtc); - -return pipe->funcs->reset_crtc (pipe); -} -static struct drm_crtc_state *drm_simple_kms_crtc_duplicate_state (struct drm_crtc *crtc) {struct drm_simple_display_pipe *pipe; pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
@@ -156,11 +145,11 @@ static void drm_simple_kms_crtc_disable_vblank(struct drm_crtc *crtc) pipe->funcs->disable_vblank (pipe); }static const struct drm_crtc_funcs drm_simple_kms_crtc_funcs = { - .reset = drm_simple_kms_crtc_reset, + .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_duplicate_state = drm_simple_kms_crtc_duplicate_state, .atomic_destroy_state = drm_simple_kms_crtc_destroy_state,
include/drm/drm_simple_kms_helper.h +0 -1
@@ -37,11 +37,10 @@ struct drm_simple_display_pipe_funcs { struct drm_plane_state *new_plane_state);void (*end_fb_access)(struct drm_simple_display_pipe *pipe,struct drm_plane_state *plane_state);int (*enable_vblank)(struct drm_simple_display_pipe *pipe);void (*disable_vblank)(struct drm_simple_display_pipe *pipe); -void (*reset_crtc)(struct drm_simple_display_pipe *pipe);struct drm_crtc_state * (*duplicate_crtc_state)(struct drm_simple_display_pipe *pipe);void (*destroy_crtc_state)(struct drm_simple_display_pipe *pipe,struct drm_crtc_state *crtc_state);void (*reset_plane)(struct drm_simple_display_pipe *pipe);struct drm_plane_state * (*duplicate_plane_state)(struct drm_simple_display_pipe *pipe);
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 22/70 drm/simple-kms: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe simple KMS crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
--- drivers/gpu/drm/drm_simple_kms_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/drm_simple_kms_helper.c +1 -1
@@ -145,11 +145,11 @@ static void drm_simple_kms_crtc_disable_vblank(struct drm_crtc *crtc) pipe->funcs->disable_vblank (pipe); }static const struct drm_crtc_funcs drm_simple_kms_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_simple_kms_crtc_duplicate_state, .atomic_destroy_state = drm_simple_kms_crtc_destroy_state,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 23/70 drm/sitronix: st7571: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe st7571 crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Marcus Folkesson <[email protected]>
--- drivers/gpu/drm/sitronix/st7571.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/sitronix/st7571.c +1 -1
@@ -422,11 +422,11 @@ static const struct drm_crtc_helper_funcs st7571_crtc_helper_funcs = { .atomic_check = drm_crtc_helper_atomic_check, .mode_valid = st7571_crtc_mode_valid, };static const struct drm_crtc_funcs st7571_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 23/70] drm/sitronix: st7571: Switch to drm_atomic_helper_crtc_create_state
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Unconditional 8-line page iterations in st7571_transform_xy cause an out-of-bounds read when nlines is not a multiple of 8.
- [High] Mismatch between framebuffer pitch during conversion and reading causes display corruption and invalid reads from hwbuf. --
--- Patch [23]: [PATCH 23/70] drm/sitronix: st7571: Switch to drm_atomic_helper_crtc_create_state --- commit cf528fe94124a1e49e93edb3680754d3976e2864
Author: Maxime Ripard <[email protected]>
drm/sitronix: st7571: Switch to drm_atomic_helper_crtc_create_state
The st7571 crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook. Switch to drm_atomic_helper_crtc_create_state() instead.
drivers/gpu/drm/sitronix/st7571.c static const struct drm_crtc_helper_funcs st7571_crtc_helper_funcs = { · line 430
Quoted 13 lines
drivers/gpu/drm/sitronix/st7571.c +1
-1@@ -424,7 +424,7 @@ static const struct drm_crtc_helper_funcs st7571_crtc_helper_funcs = { };static const struct drm_crtc_funcs st7571_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip,
[Severity: High]
This isn't a bug introduced by this patch, but does this code cause an out-of-bounds read when nlines is not a multiple of 8?
In drivers/gpu/drm/sitronix/st7571.c:st7571_probe():
st7571->hwbuf = devm_kzalloc(st7571->dev, (st7571->nlines * st7571->ncols * st7571->bpp) / 8, GFP_KERNEL);
If nlines is not a multiple of 8, hwbuf is allocated strictly based on the unaligned nlines value.
During a display update:
drivers/gpu/drm/sitronix/st7571.c:st7571_fb_update_rect_monochrome():
for (int y = rect->y1; y < rect->y2; y += ST7571_PAGE_HEIGHT) {drivers/gpu/drm/sitronix/st7571.c:st7571_transform_xy():
for (int i = 0; i < 8; i++) {
int row_idx = y + i;
u8 byte = p[row_idx * row_len + x];This iterates 8 times vertically, which could read beyond the allocated buffer if nlines is smaller than the assumed 8-line page height.
[Severity: High]
This is a pre-existing issue, but does a mismatch between the framebuffer pitch during conversion and reading cause invalid reads from hwbuf?
In drivers/gpu/drm/sitronix/st7571.c:st7571_prepare_buffer_monochrome():
dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8); iosys_map_set_vaddr(&dst, st7571->hwbuf); drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
The converted partial damage is tightly packed at offset 0 of hwbuf with a dynamic pitch.
However, when reading the data:
drivers/gpu/drm/sitronix/st7571.c:st7571_fb_update_rect_monochrome():
row[x] = st7571_transform_xy(st7571->hwbuf, x, y, 1);
drivers/gpu/drm/sitronix/st7571.c:st7571_transform_xy():
u8 row_len = 16 * bpp; ... int row_idx = y + i; u8 byte = p[row_idx * row_len + x];
st7571_transform_xy() accesses hwbuf using absolute coordinates and assumes a full-screen layout with a fixed pitch, which seems to completely miss the packed damage data.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=23
Maxime Ripard
maintainerPATCH 24/70 drm/sprd: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe sprd crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Baolin Wang <[email protected]>
Cc: Chunyan Zhang <[email protected]>
Cc: Orson Zhai <[email protected]>
--- drivers/gpu/drm/sprd/sprd_dpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/sprd/sprd_dpu.c +1 -1
@@ -705,11 +705,11 @@ static const struct drm_crtc_helper_funcs sprd_crtc_helper_funcs = {static const struct drm_crtc_funcs sprd_crtc_funcs = { .destroy = drm_crtc_cleanup, .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 = sprd_crtc_enable_vblank, .disable_vblank = sprd_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 25/70 drm/sti: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe sti crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Alain Volmat <[email protected]>
Cc: Raphael Gallais-Pou <[email protected]>
--- drivers/gpu/drm/sti/sti_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/sti/sti_crtc.c +1 -1
@@ -326,11 +326,11 @@ static int sti_crtc_late_register(struct drm_crtc *crtc)static const struct drm_crtc_funcs sti_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .destroy = sti_crtc_destroy, .set_property = sti_crtc_set_property, - .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, .late_register = sti_crtc_late_register, .enable_vblank = sti_crtc_enable_vblank, .disable_vblank = sti_crtc_disable_vblank,
Signature · 1 line
2.55.0
Re: [PATCH 25/70] drm/sti: Switch to drm_atomic_helper_crtc_create_state
Le 21/08/2026 à 16:39, Maxime Ripard a écrit :
Quoted 5 lines
The sti crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset()as its reset hook.Switch to
drm_atomic_helper_crtc_create_state()instead.
Quoted 8 lines
Signed-off-by Maxime Ripard <[email protected]>---
Cc: Alain Volmat <[email protected]>
Cc: Raphael Gallais-Pou <[email protected]>--- drivers/gpu/drm/sti/sti_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Thank you,
Best regards,
Raphaël
Maxime Ripard
maintainerPATCH 26/70 drm/stm: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe stm crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Alexandre Torgue <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: Philippe Cornu <[email protected]>
Cc: Raphael Gallais-Pou <[email protected]>
Cc: Yannick Fertre <[email protected]>
--- drivers/gpu/drm/stm/ltdc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/stm/ltdc.c +2 -2
@@ -1207,11 +1207,11 @@ static void ltdc_crtc_atomic_print_state(struct drm_printer *p, }static const struct drm_crtc_funcs ltdc_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 = ltdc_crtc_enable_vblank, .disable_vblank = ltdc_crtc_disable_vblank, .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
@@ -1219,11 +1219,11 @@ static const struct drm_crtc_funcs ltdc_crtc_funcs = { };static const struct drm_crtc_funcs ltdc_crtc_with_crc_support_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 = ltdc_crtc_enable_vblank, .disable_vblank = ltdc_crtc_disable_vblank, .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
Signature · 1 line
2.55.0
Re: [PATCH 26/70] drm/stm: Switch to drm_atomic_helper_crtc_create_state
Le 21/08/2026 à 16:39, Maxime Ripard a écrit :
Quoted 12 lines
The stm crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset()as its reset hook.Switch to
drm_atomic_helper_crtc_create_state()instead.Signed-off-by Maxime Ripard <[email protected]>---
Cc: Alexandre Torgue <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: Philippe Cornu <[email protected]>
Cc: Raphael Gallais-Pou <[email protected]>
Cc: Yannick Fertre <[email protected]>
Quoted 3 lines
--- drivers/gpu/drm/stm/ltdc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Thank you,
Best regards,
Raphaël
Maxime Ripard
maintainerPATCH 27/70 drm/sun4i: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe sun4i crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Chen-Yu Tsai <[email protected]>
Cc: Samuel Holland <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/sun4i/sun4i_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/sun4i/sun4i_crtc.c +1 -1
@@ -181,11 +181,11 @@ static void sun4i_crtc_disable_vblank(struct drm_crtc *crtc)static const struct drm_crtc_funcs sun4i_crtc_funcs = { .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .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, .set_config = drm_atomic_helper_set_config, .enable_vblank = sun4i_crtc_enable_vblank, .disable_vblank = sun4i_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 28/70 drm/tests: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe KUnit test helpers use the deprecated drm_atomic_helper_crtc_reset() as their reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
--- drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/tests/drm_kunit_helpers.c +1 -1
@@ -215,11 +215,11 @@ static const struct drm_crtc_helper_funcs default_crtc_helper_funcs = { };static const struct drm_crtc_funcs default_crtc_funcs = { .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, };/** * drm_kunit_helper_create_crtc - Creates a mock CRTC for a KUnit test * @test: The test context object
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 29/70 drm/tilcdc: Move hardware reset to CRTC creation
In reply to Maxime RipardThe 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.
---
Cc: Jyri Sarha <[email protected]>
Cc: Tomi Valkeinen <[email protected]>
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 49 +++++++++++++++--------------------- 1 file changed, 20 insertions(+), 29 deletions(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +20 -29
@@ -683,42 +683,14 @@ 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) -{ -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 ); -tilcdc_clear_irqstatus (dev,0xffffffff ); - - tilcdc_crtc->frame_done = false; -tilcdc_clear (dev,LCDC_RASTER_CTRL_REG ,LCDC_RASTER_ENABLE ); - - ret =wait_event_timeout (tilcdc_crtc->frame_done_wq, - tilcdc_crtc->frame_done, -msecs_to_jiffies (500 )); -if (ret ==0 ) -drm_err (dev,"%s: timeout waiting for framedone\n" , - __func__); - } -pm_runtime_put_sync (dev->dev); -} -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, };
@@ -1003,7 +975,26 @@ int tilcdc_crtc_create(struct drm_device *dev)spin_lock_init (&tilcdc_crtc->irq_lock);INIT_WORK (&tilcdc_crtc->recover_work, tilcdc_crtc_recover_work);drm_crtc_helper_add (crtc, &tilcdc_crtc_helper_funcs); +/* 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 ); +tilcdc_clear_irqstatus (dev,0xffffffff ); + + tilcdc_crtc->frame_done = false; +tilcdc_clear (dev,LCDC_RASTER_CTRL_REG ,LCDC_RASTER_ENABLE ); + + ret =wait_event_timeout (tilcdc_crtc->frame_done_wq, + tilcdc_crtc->frame_done, +msecs_to_jiffies (500 )); +if (ret ==0 ) +drm_err (dev,"%s: timeout waiting for framedone\n" , + __func__); + } +pm_runtime_put_sync (dev->dev); +return 0 ; }
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 29/70] drm/tilcdc: Move hardware reset to CRTC creation
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Moving the hardware reset logic to tilcdc_crtc_create() causes an unconditional 500ms timeout and a potential fatal bus fault if the bootloader left the display enabled, because it waits for an interrupt before the IRQ handler is registered.
Pre-existing issues:
- [High] The hardware reset sequence unconditionally writes to a V2-only interrupt enable register, which fails to enable the framedone interrupt on V1 hardware and can cause register corruption. --
--- Patch [29]: [PATCH 29/70] drm/tilcdc: Move hardware reset to CRTC creation --- commit d53692ddb744ac620f653bb0b29134bf757b3e82
Author: Maxime Ripard <[email protected]>
drm/tilcdc: Move hardware reset to CRTC creation
The tilcdc crtc reset hook performs both atomic state initialization and hardware-level register writes. This patch moves the hardware reset sequence to the CRTC creation path since it is not related to atomic state initialization.
[ ... ]
Quoted 4 lines
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +0
-0
[ ... ]
drivers/gpu/drm/tilcdc/tilcdc_crtc.c int tilcdc_crtc_create(struct drm_device *dev) · line 984
Quoted 9 lines
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +5
-0@@ -1005,5 +977,24 @@ int tilcdc_crtc_create(struct drm_device *dev)drm_crtc_helper_add (crtc, &tilcdc_crtc_helper_funcs); +/* 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 );
[Severity: High]
This is a pre-existing issue, but does this write corrupt V1 hardware?
The LCDC_INT_ENABLE_SET_REG is specific to V2 hardware. On V1 hardware, writing to this register accesses an undefined register address.
For V1, shouldn't the framedone interrupt be enabled via LCDC_V1_FRAME_DONE_INT_ENA within LCDC_RASTER_CTRL_REG instead? This unconditional write might fail to enable the interrupt on V1 controllers, leading to a guaranteed timeout in the wait_event_timeout() call below.
drivers/gpu/drm/tilcdc/tilcdc_crtc.c int tilcdc_crtc_create(struct drm_device *dev) · line 992
Quoted 8 lines
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +8
-0+tilcdc_clear_irqstatus (dev,0xffffffff ); + + tilcdc_crtc->frame_done = false; +tilcdc_clear (dev,LCDC_RASTER_CTRL_REG ,LCDC_RASTER_ENABLE ); + + ret =wait_event_timeout (tilcdc_crtc->frame_done_wq, + tilcdc_crtc->frame_done, +msecs_to_jiffies (500 ));
[Severity: High]
Will this unconditionally time out during boot if the bootloader left the display enabled?
In the driver initialization sequence, tilcdc_crtc_create() executes before the IRQ handler is registered in tilcdc_pdev_probe():
tilcdc_pdev_probe()
...
ret = tilcdc_crtc_create(ddev);
...
ret = tilcdc_irq_install(ddev, priv->irq);
Since the IRQ handler isn't registered yet when this code runs, wait_event_timeout() will wait for an interrupt that cannot be handled, causing a 500ms boot delay.
Could this also cause a kernel panic once the IRQ is finally registered?
When the timeout finishes, pm_runtime_put_sync() disables the device clocks, leaving the unhandled interrupt pending in hardware. When tilcdc_irq_install() later requests the IRQ, the handler will fire immediately and attempt to read the status register while the IP's clocks are gated, leading to an imprecise external abort.
Quoted 8 lines
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +6
-0+if (ret ==0 ) +drm_err (dev,"%s: timeout waiting for framedone\n" , + __func__); + } +pm_runtime_put_sync (dev->dev); +return 0 ; }
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=29
Maxime Ripard
maintainerPATCH 30/70 drm/tilcdc: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe tilcdc crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Jyri Sarha <[email protected]>
Cc: Tomi Valkeinen <[email protected]>
--- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/tilcdc/tilcdc_crtc.c +1 -1
@@ -686,11 +686,11 @@ static void tilcdc_crtc_disable_vblank(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, };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 31/70 drm/tiny: appletbdrm: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe appletbdrm crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Aditya Garg <[email protected]>
Cc: Aun-Ali Zaidi <[email protected]>
--- drivers/gpu/drm/tiny/appletbdrm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/tiny/appletbdrm.c +1 -1
@@ -596,11 +596,11 @@ static const struct drm_connector_helper_funcs appletbdrm_connector_helper_funcsstatic const struct drm_crtc_helper_funcs appletbdrm_crtc_helper_funcs = { .mode_valid = appletbdrm_crtc_helper_mode_valid, };static const struct drm_crtc_funcs appletbdrm_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 31/70] drm/tiny: appletbdrm: Switch to drm_atomic_helper_crtc_create_state
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] appletbdrm_flush_damage constructs malformed USB packets and uses mismatched coordinate spaces for clipping. --
--- Patch [31]: [PATCH 31/70] drm/tiny: appletbdrm: Switch to drm_atomic_helper_crtc_create_state --- commit c5a19cb22088517bba7b18adab2a528f848a790f
Author: Maxime Ripard <[email protected]>
drm/tiny: appletbdrm: Switch to drm_atomic_helper_crtc_create_state
The appletbdrm crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
drivers/gpu/drm/tiny/appletbdrm.c static const struct drm_crtc_helper_funcs appletbdrm_crtc_helper_funcs = { · line 604
Quoted 13 lines
drivers/gpu/drm/tiny/appletbdrm.c +1
-1@@ -598,7 +598,7 @@ static const struct drm_crtc_helper_funcs appletbdrm_crtc_helper_funcs = { };static const struct drm_crtc_funcs appletbdrm_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip,
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in appletbdrm_flush_damage() in this same file.
When damage (in framebuffer coordinates) is intersected with state->dst (in CRTC coordinates), could this produce incorrect clipping results if the plane is not placed at (0,0)?
Also, because buf_size is calculated before the intersection, but the frame dimensions use the mutated damage:
drivers/gpu/drm/tiny/appletbdrm.c:appletbdrm_flush_damage() {
...
u32 buf_size = rect_size(&damage);
if (!drm_rect_intersect(&dst_clip, &damage))
continue;
/* ... */
frame->width = cpu_to_le16(drm_rect_height(&damage));
frame->height = cpu_to_le16(drm_rect_width(&damage));
frame->buf_size = cpu_to_le32(buf_size);
...
}If the damage rectangle is partially outside the destination bounds, will this cause buf_size to be larger than the actual frame dimensions? Does this lead to malformed USB frames with uninitialized memory gaps before the footer, violating the hardware USB protocol?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=31
Re: [PATCH 31/70] drm/tiny: appletbdrm: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardPlease use my linux.dev email
-------- Original Message --------
On Friday, 08/21/26 at 20:10 Maxime Ripard <[email protected]> wrote:
The appletbdrm crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Aditya Garg <[email protected]>
Cc: Aun-Ali Zaidi <[email protected]>
--- drivers/gpu/drm/tiny/appletbdrm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/tiny/appletbdrm.c +1 -1
@@ -596,11 +596,11 @@ static const struct drm_connector_helper_funcs appletbdrm_connector_helper_funcsstatic const struct drm_crtc_helper_funcs appletbdrm_crtc_helper_funcs = { .mode_valid = appletbdrm_crtc_helper_mode_valid, };static const struct drm_crtc_funcs appletbdrm_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
--
2.55.0
Maxime Ripard
maintainerPATCH 32/70 drm/tiny: bochs: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe bochs crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Gerd Hoffmann <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/tiny/bochs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/tiny/bochs.c +1 -1
@@ -548,11 +548,11 @@ static const struct drm_crtc_helper_funcs bochs_crtc_helper_funcs = { .atomic_enable = bochs_crtc_helper_atomic_enable, .atomic_disable = bochs_crtc_helper_atomic_disable, };static const struct drm_crtc_funcs bochs_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 33/70 drm/tiny: cirrus: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe cirrus crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Dave Airlie <[email protected]>
Cc: Gerd Hoffmann <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/tiny/cirrus-qemu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/tiny/cirrus-qemu.c +1 -1
@@ -417,11 +417,11 @@ static const struct drm_crtc_helper_funcs cirrus_crtc_helper_funcs = { .atomic_enable = cirrus_crtc_helper_atomic_enable, .atomic_disable = drm_crtc_vblank_atomic_disable, };static const struct drm_crtc_funcs cirrus_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 34/70 drm/tiny: pixpaper: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe pixpaper crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: LiangCheng Wang <[email protected]>
--- drivers/gpu/drm/tiny/pixpaper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/tiny/pixpaper.c +1 -1
@@ -961,11 +961,11 @@ static const struct drm_plane_helper_funcs pixpaper_plane_helper_funcs = { };static const struct drm_crtc_funcs pixpaper_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, .destroy = drm_crtc_cleanup, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, };
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 34/70] drm/tiny: pixpaper: Switch to drm_atomic_helper_crtc_create_state
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the plane and CRTC atomic callbacks call sleeping functions in atomic context, violating DRM subsystem rules. --
--- Patch [34]: [PATCH 34/70] drm/tiny: pixpaper: Switch to drm_atomic_helper_crtc_create_state --- commit 0c05f2d46ff22e1262b7a7c99b4dd00d2c48ebfe
Author: Maxime Ripard <[email protected]>
drm/tiny: pixpaper: Switch to drm_atomic_helper_crtc_create_state
This commit switches the pixpaper crtc implementation from using the deprecated drm_atomic_helper_crtc_reset() hook to using drm_atomic_helper_crtc_create_state().
drivers/gpu/drm/tiny/pixpaper.c static const struct drm_plane_helper_funcs pixpaper_plane_helper_funcs = { · line 966
Quoted 10 lines
drivers/gpu/drm/tiny/pixpaper.c +1
-1@@ -963,7 +963,7 @@ static const struct drm_plane_helper_funcs pixpaper_plane_helper_funcs = {static const struct drm_crtc_funcs pixpaper_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]
This isn't a bug introduced by this patch, but while reviewing the atomic state changes I noticed a pre-existing issue. Does pixpaper_wait_for_panel() call sleeping functions while in an atomic context?
Looking at pixpaper_crtc_atomic_enable() and pixpaper_plane_atomic_update(), these are DRM atomic callbacks and execute in atomic context where sleeping is forbidden. However, both of them call pixpaper_wait_for_panel():
pixpaper_wait_for_panel() {
...
usleep_range(1000, 1500);
while (gpiod_get_value_cansleep(panel->busy) != 1) {
...
}Could this trigger scheduling while atomic oopses or deadlocks during non-blocking commits? Should this be modified to use non-sleeping delays and GPIO reads instead?
Quoted 4 lines
drivers/gpu/drm/tiny/pixpaper.c +0
-0.destroy = drm_crtc_cleanup, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,};
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=34
Maxime Ripard
maintainerPATCH 35/70 drm/tiny: sharp: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe sharp crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Alex Lanzano <[email protected]>
--- drivers/gpu/drm/tiny/sharp-memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/tiny/sharp-memory.c +1 -1
@@ -343,11 +343,11 @@ static const struct drm_crtc_helper_funcs sharp_memory_crtc_helper_funcs = { .atomic_enable = sharp_memory_crtc_enable, .atomic_disable = sharp_memory_crtc_disable, };static const struct drm_crtc_funcs sharp_memory_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 35/70] drm/tiny: sharp: Switch to drm_atomic_helper_crtc_create_state
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The background kthread or PWM signal is not stopped on probe error paths, leading to Use-After-Free when the device memory is freed.
- [High] The driver does not check the return value of kthread_run(), which causes a kernel panic in the remove path if thread creation fails.
- [High] Plane atomic update callback calls sleeping functions (mutex_lock and spi_write) in an atomic context.
--
--- Patch [35]: [PATCH 35/70] drm/tiny: sharp: Switch to drm_atomic_helper_crtc_create_state --- commit d365eb0228322a04306eb8553f60a438b3220d14
Author: Maxime Ripard <[email protected]>
drm/tiny: sharp: Switch to drm_atomic_helper_crtc_create_state
The sharp crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook. Switch to drm_atomic_helper_crtc_create_state() instead.
drivers/gpu/drm/tiny/sharp-memory.c static const struct drm_crtc_helper_funcs sharp_memory_crtc_helper_funcs = { · line 348
Quoted 10 lines
drivers/gpu/drm/tiny/sharp-memory.c +1
-1@@ -345,7 +345,7 @@ static const struct drm_crtc_helper_funcs sharp_memory_crtc_helper_funcs = { };static const struct drm_crtc_funcs sharp_memory_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state,
[Severity: High]
This isn't a bug introduced by this patch, but does the plane atomic update callback call sleeping functions in an atomic context?
The DRM subsystem executes plane atomic update callbacks in an atomic context where sleeping is forbidden, but sharp_memory_plane_atomic_update() invokes the display update flow:
drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_plane_atomic_update() {
...
sharp_memory_fb_dirty(plane_state->fb, shadow_plane_state->data,
&rect, &shadow_plane_state->fmtcnv_state);
...
}which then calls sleeping functions unconditionally:
drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_update_display() {
...
mutex_lock(&smd->tx_mutex);
/* Populate the transmit buffer with frame data */
...
ret = sharp_memory_spi_write(smd->spi, tx_buffer, tx_buffer_size);
...
}Could this lead to kernel splats and potential deadlocks on any atomic plane update from userspace?
[Severity: High]
This is a pre-existing issue, but does the probe error path return directly without stopping the kthread or PWM?
In sharp_memory_probe(), the background kthread is started, but if subsequent initialization steps fail, the function returns directly:
drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_probe() {
...
smd->sw_vcom_signal = kthread_run(sharp_memory_sw_vcom_signal_thread,
smd, "sw_vcom_signal");
...
if (ret)
return dev_err_probe(dev, ret, "Failed to initialize display pipeline.\n");
...
}Can this lead to a Use-After-Free memory corruption if the smd structure is freed by devres while the running kthread continues executing and dereferencing the freed memory?
[Severity: High]
This is also a pre-existing issue, but is the return value of kthread_run() used without checking IS_ERR()?
If kthread creation fails in sharp_memory_probe(), smd->sw_vcom_signal would hold an ERR_PTR:
drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_probe() {
...
smd->sw_vcom_signal = kthread_run(sharp_memory_sw_vcom_signal_thread,
smd, "sw_vcom_signal");
...
}Would this cause a kernel panic due to an invalid pointer dereference when the driver is removed and kthread_stop() is unconditionally called?
drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_remove() {
...
kthread_stop(smd->sw_vcom_signal);
...
}Quoted 6 lines
drivers/gpu/drm/tiny/sharp-memory.c +0
-0.destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip,drivers/gpu/drm/tiny/sharp-memory.c +0
-0.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, };
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=35
Maxime Ripard
maintainerPATCH 36/70 drm/udl: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe udl crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Dave Airlie <[email protected]>
Cc: Sean Paul <[email protected]>
--- drivers/gpu/drm/udl/udl_modeset.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/udl/udl_modeset.c +1 -1
@@ -398,11 +398,11 @@ static const struct drm_crtc_helper_funcs udl_crtc_helper_funcs = { .atomic_enable = udl_crtc_helper_atomic_enable, .atomic_disable = udl_crtc_helper_atomic_disable, };static const struct drm_crtc_funcs udl_crtc_funcs = { - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 37/70 drm/vbox: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe vbox crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Hans de Goede <[email protected]>
--- drivers/gpu/drm/vboxvideo/vbox_mode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/vboxvideo/vbox_mode.c +1 -1
@@ -248,11 +248,11 @@ static void vbox_crtc_destroy(struct drm_crtc *crtc)static const struct drm_crtc_funcs vbox_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip,/* .gamma_set = vbox_crtc_gamma_set, */ .destroy = vbox_crtc_destroy, - .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, }; static int vbox_primary_atomic_check(struct drm_plane *plane,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 38/70 drm/verisilicon: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe verisilicon crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Icenowy Zheng <[email protected]>
--- drivers/gpu/drm/verisilicon/vs_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/verisilicon/vs_crtc.c +1 -1
@@ -147,11 +147,11 @@ static void vs_crtc_disable_vblank(struct drm_crtc *crtc)static const struct drm_crtc_funcs vs_crtc_funcs = { .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .page_flip = drm_atomic_helper_page_flip, - .reset = drm_atomic_helper_crtc_reset, + .atomic_create_state = drm_atomic_helper_crtc_create_state, .set_config = drm_atomic_helper_set_config, .enable_vblank = vs_crtc_enable_vblank, .disable_vblank = vs_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 39/70 drm/virtio: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe virtio crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Chia-I Wu <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Dmitry Osipenko <[email protected]>
Cc: Gerd Hoffmann <[email protected]>
Cc: Gurchetan Singh <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/virtio/virtgpu_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/virtio/virtgpu_display.c +1 -1
@@ -53,11 +53,11 @@static const struct drm_crtc_funcs virtio_gpu_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 , };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 40/70 drm/xlnx: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe xlnx crtc implementation uses the deprecateddrm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
---
Cc: Michal Simek <[email protected]>
Cc: Tomi Valkeinen <[email protected]>
--- drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/xlnx/zynqmp_kms.c +1 -1
@@ -306,11 +306,11 @@ static void zynqmp_dpsub_crtc_disable_vblank(struct drm_crtc *crtc)static const struct drm_crtc_funcs zynqmp_dpsub_crtc_funcs = { .destroy = drm_crtc_cleanup, .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 = zynqmp_dpsub_crtc_enable_vblank, .disable_vblank = zynqmp_dpsub_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 41/70 drm/mipi-dbi: Switch to drm_atomic_helper_crtc_create_state
In reply to Maxime RipardThe mipi-dbi crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook.
Switch to drm_atomic_helper_crtc_create_state() instead.
--- include/drm/drm_mipi_dbi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/drm/drm_mipi_dbi.h +1 -1
@@ -232,11 +232,11 @@ void drm_mipi_dbi_plane_helper_atomic_update(struct drm_plane *plane,/* * CRTC */ #define DRM_MIPI_DBI_CRTC_FUNCS \ - .reset = drm_atomic_helper_crtc_reset, \ + .atomic_create_state = drm_atomic_helper_crtc_create_state, \ .set_config = drm_atomic_helper_set_config, \ .page_flip = drm_atomic_helper_page_flip, \ .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, \ .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 42/70 drm/atomic-helper: Remove drm_atomic_helper_crtc_reset
In reply to Maxime RipardAll users of drm_atomic_helper_crtc_reset() have been converted to drm_atomic_helper_crtc_create_state().
Remove the now unused helper.
--- 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(-)
drivers/gpu/drm/drm_atomic_state_helper.c +0 -19
@@ -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.
include/drm/drm_atomic_state_helper.h +0 -1
@@ -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 *
include/drm/drm_crtc.h +0 -2
@@ -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:
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 43/70 sysfb: Convert to create_state
In reply to Maxime Ripard---
Cc: Javier Martinez Canillas <[email protected]>
--- drivers/gpu/drm/sysfb/drm_sysfb_helper.h | 4 ++-- drivers/gpu/drm/sysfb/drm_sysfb_modeset.c | 20 +++++++++----------- 2 files changed, 11 insertions(+), 13 deletions(-)
drivers/gpu/drm/sysfb/drm_sysfb_helper.h +2 -2
@@ -169,16 +169,16 @@ int drm_sysfb_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_#define DRM_SYSFB_CRTC_HELPER_FUNCS \ .mode_valid = drm_sysfb_crtc_helper_mode_valid, \ .atomic_check = drm_sysfb_crtc_helper_atomic_check -void drm_sysfb_crtc_reset (struct drm_crtc *crtc); +struct drm_crtc_state *drm_sysfb_crtc_create_state (struct drm_crtc *crtc);struct drm_crtc_state *drm_sysfb_crtc_atomic_duplicate_state (struct drm_crtc *crtc);void drm_sysfb_crtc_atomic_destroy_state (struct drm_crtc *crtc,struct drm_crtc_state *crtc_state);#define DRM_SYSFB_CRTC_FUNCS \ - .reset = drm_sysfb_crtc_reset, \ + .atomic_create_state = drm_sysfb_crtc_create_state, \ .set_config = drm_atomic_helper_set_config, \ .page_flip = drm_atomic_helper_page_flip, \ .atomic_duplicate_state = drm_sysfb_crtc_atomic_duplicate_state, \ .atomic_destroy_state = drm_sysfb_crtc_atomic_destroy_state
drivers/gpu/drm/sysfb/drm_sysfb_modeset.c +9 -11
@@ -513,27 +513,25 @@ int drm_sysfb_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_return 0 ; }EXPORT_SYMBOL (drm_sysfb_crtc_helper_atomic_check); -void drm_sysfb_crtc_reset (struct drm_crtc *crtc) +struct drm_crtc_state *drm_sysfb_crtc_create_state (struct drm_crtc *crtc) {struct drm_sysfb_device *sysfb =to_drm_sysfb_device (crtc->dev);struct drm_sysfb_crtc_state *sysfb_crtc_state; -if (crtc->state) -drm_sysfb_crtc_state_destroy (to_drm_sysfb_crtc_state (crtc->state)); - sysfb_crtc_state =kzalloc_obj (*sysfb_crtc_state); -if (sysfb_crtc_state) { - sysfb_crtc_state->format = sysfb->fb_format; -__drm_atomic_helper_crtc_reset (crtc, &sysfb_crtc_state->base); - }else { -__drm_atomic_helper_crtc_reset (crtc,NULL ); - } +if (!sysfb_crtc_state) +return ERR_PTR (-ENOMEM ); + + sysfb_crtc_state->format = sysfb->fb_format; +__drm_atomic_helper_crtc_state_init (&sysfb_crtc_state->base, crtc); + +return &sysfb_crtc_state->base; } -EXPORT_SYMBOL (drm_sysfb_crtc_reset); +EXPORT_SYMBOL (drm_sysfb_crtc_create_state);struct drm_crtc_state *drm_sysfb_crtc_atomic_duplicate_state (struct drm_crtc *crtc) {struct drm_device *dev = crtc->dev;struct drm_crtc_state *crtc_state = crtc->state;
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 44/70 drm/amdgpu: dm: Convert to atomic_create_state
In reply to Maxime RipardThe 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.
---
Cc: "Christian König" <[email protected]>
Cc: Alex Deucher <[email protected]>
Cc: Harry Wentland <[email protected]>
Cc: Leo Li <[email protected]>
Cc: Rodrigo Siqueira <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c +8 -9
@@ -438,22 +438,21 @@ static void amdgpu_dm_crtc_destroy(struct drm_crtc *crtc)drm_crtc_cleanup (crtc);kfree (crtc); } -static void amdgpu_dm_crtc_reset_state (struct drm_crtc *crtc) +static 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; }#ifdef CONFIG_DEBUG_FS static int amdgpu_dm_crtc_late_register (struct drm_crtc *crtc) {
@@ -527,11 +526,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,
@@ -741,13 +740,13 @@ 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) + acrtc->base.funcs->atomic_create_state (&acrtc->base); 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;
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 44/70] drm/amdgpu: dm: Convert to atomic_create_state
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The return value of `atomic_create_state()` is completely discarded during CRTC initialization…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The return value of atomic_create_state() is completely discarded during CRTC initialization, causing a memory leak and leaving crtc->state uninitialized (NULL). --
--- Patch [44]: [PATCH 44/70] drm/amdgpu: dm: Convert to atomic_create_state --- commit 680651b77d5bb2911f7b94020aa95ffc0ffde185
Author: Maxime Ripard <[email protected]>
drm/amdgpu: dm: Convert to atomic_create_state
The amdgpu display manager crtc implementation provides a custom reset hook. This converts the implementation to use atomic_create_state with __drm_atomic_helper_crtc_state_init() instead.
Quoted 4 lines
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c +0
-0
[ ... ]
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm, · line 761
Quoted 10 lines
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c +3
-3@@ -757,9 +756,9 @@ 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) + acrtc->base.funcs->atomic_create_state (&acrtc->base);
[Severity: High]
Does this drop the newly allocated state and leave acrtc->base.state uninitialized (NULL)?
Previously, amdgpu_dm_crtc_reset_state() called __drm_atomic_helper_crtc_reset(), which internally assigns the newly allocated state to the crtc:
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);
crtc->state = crtc_state;
}With the new conversion, amdgpu_dm_crtc_create_state() allocates and returns the state pointer:
static 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 ERR_PTR(-ENOMEM);
__drm_atomic_helper_crtc_state_init(&state->base, crtc);
return &state->base;
}However, amdgpu_dm_crtc_init() ignores the return value of atomic_create_state() and does not assign it to the crtc.
Could this result in a memory leak of the returned dm_crtc_state object, as well as a regression due to NULL pointer dereferences when crtc->state is accessed later during driver initialization?
Quoted 3 lines
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c +0
-0acrtc->max_cursor_width = dm->adev->dm.dc->caps.max_cursor_size; acrtc->max_cursor_height = dm->adev->dm.dc->caps.max_cursor_size;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=44
Maxime Ripard
maintainerPATCH 45/70 drm/komeda: Convert to atomic_create_state
In reply to Maxime RipardThe komeda 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.
---
Cc: Liviu Dudau <[email protected]>
--- drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/gpu/drm/arm/display/komeda/komeda_crtc.c +8 -10
@@ -491,23 +491,21 @@ static const struct drm_crtc_helper_funcs komeda_crtc_helper_funcs = { .atomic_disable = komeda_crtc_atomic_disable, .mode_valid = komeda_crtc_mode_valid, .mode_fixup = komeda_crtc_mode_fixup, }; -static void komeda_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *komeda_crtc_create_state (struct drm_crtc *crtc) {struct komeda_crtc_state *state; -if (crtc->state) -__drm_atomic_helper_crtc_destroy_state (crtc->state); - -kfree (to_kcrtc_st (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 *komeda_crtc_atomic_duplicate_state (struct drm_crtc *crtc) {
@@ -553,11 +551,11 @@ static void komeda_crtc_vblank_disable(struct drm_crtc *crtc)static const struct drm_crtc_funcs komeda_crtc_funcs = { .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, - .reset = komeda_crtc_reset, + .atomic_create_state = komeda_crtc_create_state, .atomic_duplicate_state = komeda_crtc_atomic_duplicate_state, .atomic_destroy_state = komeda_crtc_atomic_destroy_state, .enable_vblank = komeda_crtc_vblank_enable, .disable_vblank = komeda_crtc_vblank_disable, };
Signature · 1 line
2.55.0
Re: [PATCH 45/70] drm/komeda: Convert to atomic_create_state
Quoted 11 lines · Maxime Ripard
The komeda 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_statehook is meant to do, minus the old state cleanup which the caller handles, convert the implementation to useatomic_create_statewith__drm_atomic_helper_crtc_state_init()instead.Signed-off-by Maxime Ripard <[email protected]>
Best regards,
Liviu
Quoted 58 lines
---
Cc: Liviu Dudau <[email protected]>--- drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)drivers/gpu/drm/arm/display/komeda/komeda_crtc.c +8
-10@@ -491,23 +491,21 @@ static const struct drm_crtc_helper_funcs komeda_crtc_helper_funcs = { .atomic_disable = komeda_crtc_atomic_disable, .mode_valid = komeda_crtc_mode_valid, .mode_fixup = komeda_crtc_mode_fixup, }; -static void komeda_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *komeda_crtc_create_state (struct drm_crtc *crtc) {struct komeda_crtc_state *state; -if (crtc->state) -__drm_atomic_helper_crtc_destroy_state (crtc->state); - -kfree (to_kcrtc_st (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 *komeda_crtc_atomic_duplicate_state (struct drm_crtc *crtc) {@@ -553,11 +551,11 @@ static void komeda_crtc_vblank_disable(struct drm_crtc *crtc)static const struct drm_crtc_funcs komeda_crtc_funcs = { .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, - .reset = komeda_crtc_reset, + .atomic_create_state = komeda_crtc_create_state, .atomic_duplicate_state = komeda_crtc_atomic_duplicate_state, .atomic_destroy_state = komeda_crtc_atomic_destroy_state, .enable_vblank = komeda_crtc_vblank_enable, .disable_vblank = komeda_crtc_vblank_disable, };Signature · 2 lines
-- 2.55.0
Signature · 8 lines
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯Maxime Ripard
maintainerPATCH 46/70 drm/malidp: Convert to atomic_create_state
In reply to Maxime RipardThe malidp 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.
---
Cc: Liviu Dudau <[email protected]>
--- drivers/gpu/drm/arm/malidp_crtc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/gpu/drm/arm/malidp_crtc.c +9 -9
@@ -474,21 +474,21 @@ static void malidp_crtc_destroy_state(struct drm_crtc *crtc, }kfree (mali_state); } -static void malidp_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *malidp_crtc_create_state (struct drm_crtc *crtc) { -struct malidp_crtc_state *state =kzalloc_obj (*state); +struct malidp_crtc_state *state; -if (crtc->state) -malidp_crtc_destroy_state (crtc, crtc->state); + state =kzalloc_obj (*state); +if (!state) +return ERR_PTR (-ENOMEM ); -if (state) -__drm_atomic_helper_crtc_reset (crtc, &state->base); -else -__drm_atomic_helper_crtc_reset (crtc,NULL ); +__drm_atomic_helper_crtc_state_init (&state->base, crtc); + +return &state->base; }static int malidp_crtc_enable_vblank (struct drm_crtc *crtc) {struct malidp_drm *malidp =crtc_to_malidp_device (crtc);
@@ -509,11 +509,11 @@ static void malidp_crtc_disable_vblank(struct drm_crtc *crtc) }static const struct drm_crtc_funcs malidp_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, - .reset = malidp_crtc_reset, + .atomic_create_state = malidp_crtc_create_state, .atomic_duplicate_state = malidp_crtc_duplicate_state, .atomic_destroy_state = malidp_crtc_destroy_state, .enable_vblank = malidp_crtc_enable_vblank, .disable_vblank = malidp_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Re: [PATCH 46/70] drm/malidp: Convert to atomic_create_state
Quoted 11 lines · Maxime Ripard
The malidp 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_statehook is meant to do, minus the old state cleanup which the caller handles, convert the implementation to useatomic_create_statewith__drm_atomic_helper_crtc_state_init()instead.Signed-off-by Maxime Ripard <[email protected]>
Best regards,
Liviu
Quoted 57 lines
---
Cc: Liviu Dudau <[email protected]>--- drivers/gpu/drm/arm/malidp_crtc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)drivers/gpu/drm/arm/malidp_crtc.c +9
-9@@ -474,21 +474,21 @@ static void malidp_crtc_destroy_state(struct drm_crtc *crtc, }kfree (mali_state); } -static void malidp_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *malidp_crtc_create_state (struct drm_crtc *crtc) { -struct malidp_crtc_state *state =kzalloc_obj (*state); +struct malidp_crtc_state *state; -if (crtc->state) -malidp_crtc_destroy_state (crtc, crtc->state); + state =kzalloc_obj (*state); +if (!state) +return ERR_PTR (-ENOMEM ); -if (state) -__drm_atomic_helper_crtc_reset (crtc, &state->base); -else -__drm_atomic_helper_crtc_reset (crtc,NULL ); +__drm_atomic_helper_crtc_state_init (&state->base, crtc); + +return &state->base; }static int malidp_crtc_enable_vblank (struct drm_crtc *crtc) {struct malidp_drm *malidp =crtc_to_malidp_device (crtc);@@ -509,11 +509,11 @@ static void malidp_crtc_disable_vblank(struct drm_crtc *crtc) }static const struct drm_crtc_funcs malidp_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, - .reset = malidp_crtc_reset, + .atomic_create_state = malidp_crtc_create_state, .atomic_duplicate_state = malidp_crtc_duplicate_state, .atomic_destroy_state = malidp_crtc_destroy_state, .enable_vblank = malidp_crtc_enable_vblank, .disable_vblank = malidp_crtc_disable_vblank, };Signature · 2 lines
-- 2.55.0
Signature · 8 lines
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯Maxime Ripard
maintainerPATCH 47/70 drm/ast: Convert to atomic_create_state
In reply to Maxime RipardThe ast 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.
---
Cc: Dave Airlie <[email protected]>
Cc: Jocelyn Falempe <[email protected]>
--- drivers/gpu/drm/ast/ast_mode.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/gpu/drm/ast/ast_mode.c +9 -9
@@ -886,21 +886,21 @@ static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = { .atomic_flush = ast_crtc_helper_atomic_flush, .atomic_enable = ast_crtc_helper_atomic_enable, .atomic_disable = ast_crtc_helper_atomic_disable, }; -static void ast_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *ast_crtc_create_state (struct drm_crtc *crtc) { -struct ast_crtc_state *ast_state =kzalloc_obj (*ast_state); +struct ast_crtc_state *ast_state; -if (crtc->state) - crtc->funcs->atomic_destroy_state (crtc, crtc->state); + ast_state =kzalloc_obj (*ast_state); +if (!ast_state) +return ERR_PTR (-ENOMEM ); -if (ast_state) -__drm_atomic_helper_crtc_reset (crtc, &ast_state->base); -else -__drm_atomic_helper_crtc_reset (crtc,NULL ); +__drm_atomic_helper_crtc_state_init (&ast_state->base, crtc); + +return &ast_state->base; }static struct drm_crtc_state *ast_crtc_atomic_duplicate_state (struct drm_crtc *crtc) {
@@ -932,11 +932,11 @@ static void ast_crtc_atomic_destroy_state(struct drm_crtc *crtc,__drm_atomic_helper_crtc_destroy_state (&ast_state->base);kfree (ast_state); }static const struct drm_crtc_funcs ast_crtc_funcs = { - .reset = ast_crtc_reset, + .atomic_create_state = ast_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = ast_crtc_atomic_duplicate_state, .atomic_destroy_state = ast_crtc_atomic_destroy_state,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 48/70 drm/atmel-hlcdc: Convert to atomic_create_state
In reply to Maxime RipardThe atmel-hlcdc 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.
---
Cc: Alexandre Belloni <[email protected]>
Cc: Claudiu Beznea <[email protected]>
Cc: Dharma Balasubiramani <[email protected]>
Cc: Manikandan Muralidharan <[email protected]>
Cc: Nicolas Ferre <[email protected]>
--- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +8 -11
@@ -527,24 +527,21 @@ void atmel_hlcdc_crtc_irq(struct drm_crtc *c) {drm_crtc_handle_vblank (c);atmel_hlcdc_crtc_finish_page_flip (drm_crtc_to_atmel_hlcdc_crtc (c)); } -static void atmel_hlcdc_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *atmel_hlcdc_crtc_create_state (struct drm_crtc *crtc) {struct atmel_hlcdc_crtc_state *state; -if (crtc->state) { -__drm_atomic_helper_crtc_destroy_state (crtc->state); - state =drm_crtc_state_to_atmel_hlcdc_crtc_state (crtc->state); -kfree (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 *atmel_hlcdc_crtc_duplicate_state (struct drm_crtc *crtc) {
@@ -597,11 +594,11 @@ static void atmel_hlcdc_crtc_disable_vblank(struct drm_crtc *c) }static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = { .page_flip = drm_atomic_helper_page_flip, .set_config = drm_atomic_helper_set_config, - .reset = atmel_hlcdc_crtc_reset, + .atomic_create_state = atmel_hlcdc_crtc_create_state, .atomic_duplicate_state = atmel_hlcdc_crtc_duplicate_state, .atomic_destroy_state = atmel_hlcdc_crtc_destroy_state, .enable_vblank = atmel_hlcdc_crtc_enable_vblank, .disable_vblank = atmel_hlcdc_crtc_disable_vblank, };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 49/70 drm/imx: ipuv3: Convert to atomic_create_state
In reply to Maxime RipardThe ipuv3 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.
---
Cc: Fabio Estevam <[email protected]>
Cc: Frank Li <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Philipp Zabel <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/imx/ipuv3/ipuv3-crtc.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/gpu/drm/imx/ipuv3/ipuv3-crtc.c +8 -10
@@ -105,23 +105,21 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc, crtc->state->event =NULL ; }spin_unlock_irq (&crtc->dev->event_lock); } -static void imx_drm_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *imx_drm_crtc_create_state (struct drm_crtc *crtc) {struct imx_crtc_state *state; -if (crtc->state) -__drm_atomic_helper_crtc_destroy_state (crtc->state); - -kfree (to_imx_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 *imx_drm_crtc_duplicate_state (struct drm_crtc *crtc) {struct imx_crtc_state *state;
@@ -162,11 +160,11 @@ static void ipu_disable_vblank(struct drm_crtc *crtc) }static const struct drm_crtc_funcs ipu_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, - .reset = imx_drm_crtc_reset, + .atomic_create_state = imx_drm_crtc_create_state, .atomic_duplicate_state = imx_drm_crtc_duplicate_state, .atomic_destroy_state = imx_drm_crtc_destroy_state, .enable_vblank = ipu_enable_vblank, .disable_vblank = ipu_disable_vblank, };
Signature · 1 line
2.55.0
Re: [PATCH 49/70] drm/imx: ipuv3: Convert to atomic_create_state
Quoted 11 lines · Maxime Ripard
The ipuv3 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_statehook is meant to do, minus the old state cleanup which the caller handles, convert the implementation to useatomic_create_statewith__drm_atomic_helper_crtc_state_init()instead.Signed-off-by Maxime Ripard <[email protected]>
regards
Philipp
Maxime Ripard
maintainerPATCH 50/70 drm/loongsoon: Move hardware reset to CRTC creation
In reply to Maxime RipardThe loongsoon 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.
---
Cc: Huacai Chen <[email protected]>
Cc: Icenowy Zheng <[email protected]>
Cc: Jianmin Lv <[email protected]>
Cc: Mingcong Bai <[email protected]>
Cc: Qianhai Wu <[email protected]>
Cc: Xi Ruoyao <[email protected]>
--- drivers/gpu/drm/loongson/lsdc_crtc.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/gpu/drm/loongson/lsdc_crtc.c +6 -5
@@ -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) {
@@ -981,10 +976,13 @@ int ls7a1000_crtc_init(struct drm_device *ddev,if (ret)return ret;drm_crtc_enable_color_mgmt (crtc,0 , false,256 ); +/* Reset the CRTC hardware, this is required for S3 support */ + lcrtc->hw_ops->reset (lcrtc); +return 0 ; } int ls7a2000_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc,
@@ -1020,7 +1018,10 @@ int ls7a2000_crtc_init(struct drm_device *ddev,if (ret)return ret;drm_crtc_enable_color_mgmt (crtc,0 , false,256 ); +/* Reset the CRTC hardware, this is required for S3 support */ + lcrtc->hw_ops->reset (lcrtc); +return 0 ; }
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 50/70] drm/loongsoon: Move hardware reset to CRTC creation
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Broken S3 Resume due to missing CRTC hardware reset --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Broken S3 Resume due to missing CRTC hardware reset
--
--- Patch [50]: [PATCH 50/70] drm/loongsoon: Move hardware reset to CRTC creation --- commit 841a0290c125afb8611341788938204f1fca139c
Author: Maxime Ripard <[email protected]>
drm/loongsoon: Move hardware reset to CRTC creation
This commit moves the hardware-level CRTC reset out of the atomic reset hook and into the CRTC initialization functions.
drivers/gpu/drm/loongson/lsdc_crtc.c static void lsdc_crtc_reset(struct drm_crtc *crtc) · line 404
Quoted 11 lines
drivers/gpu/drm/loongson/lsdc_crtc.c +0
-3@@ -403,9 +401,6 @@ static void lsdc_crtc_reset(struct drm_crtc *crtc)__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); }
[Severity: High]
Since drm_mode_config_reset() is typically called via drm_atomic_helper_resume() during system resume, does removing the hardware reset from this hook prevent the CRTC from being correctly reinitialized after waking from sleep?
[ ... ]
drivers/gpu/drm/loongson/lsdc_crtc.c int ls7a1000_crtc_init(struct drm_device *ddev, · line 985
Quoted 9 lines
drivers/gpu/drm/loongson/lsdc_crtc.c +3
-0@@ -983,6 +978,9 @@ int ls7a1000_crtc_init(struct drm_device *ddev,drm_crtc_enable_color_mgmt (crtc,0 , false,256 ); +/* Reset the CRTC hardware, this is required for S3 support */ + lcrtc->hw_ops->reset (lcrtc); +return 0 ; }
[Severity: High]
The comment explicitly notes this reset is required for S3 support, but ls7a1000_crtc_init() only executes during device probe.
During S3 resume, the hardware registers will likely contain garbage values. If the hardware reset is no longer called during resume, and the atomic commit path merely performs a read-modify-write to enable the output, won't this preserve those garbage bits and cause the display hardware to hang or fail to resume?
(The same question applies to the identical change in ls7a2000_crtc_init below).
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=50
Thomas Zimmermann
maintainerRe: [PATCH 50/70] drm/loongsoon: Move hardware reset to CRTC creation
In reply to Maxime RipardHi Maxime
Am 21.08.26 um 16:39 schrieb Maxime Ripard:
Quoted 39 lines
The loongsoon 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_statewhich only deals with state allocation. Move the hardware reset to the CRTC creation path instead.Signed-off-by Maxime Ripard <[email protected]>---
Cc: Huacai Chen <[email protected]>
Cc: Icenowy Zheng <[email protected]>
Cc: Jianmin Lv <[email protected]>
Cc: Mingcong Bai <[email protected]>
Cc: Qianhai Wu <[email protected]>
Cc: Xi Ruoyao <[email protected]>--- drivers/gpu/drm/loongson/lsdc_crtc.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)drivers/gpu/drm/loongson/lsdc_crtc.c +0
-2@@ -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);
drivers/gpu/drm/loongson/lsdc_crtc.c static void lsdc_crtc_reset(struct drm_crtc *crtc) · line 408
Quoted 8 lines
drivers/gpu/drm/loongson/lsdc_crtc.c +0
-3@@ -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);
As the Sashiko bot noted, removing this here will likely break S3 resume. IIUC you could instead add it before resuming DRM state at [1]
[1] https://elixir.bootlin.com/linux/v7.2/source/drivers/gpu/drm/loongson/lsdc_drv.c#L390
Best regards
Thomas
Quoted 31 lines
drivers/gpu/drm/loongson/lsdc_crtc.c +6
-0}static void lsdc_crtc_atomic_destroy_state (struct drm_crtc *crtc,struct drm_crtc_state *state) {@@ -981,10 +976,13 @@ int ls7a1000_crtc_init(struct drm_device *ddev,if (ret)return ret;drm_crtc_enable_color_mgmt (crtc,0 , false,256 ); +/* Reset the CRTC hardware, this is required for S3 support */ + lcrtc->hw_ops->reset (lcrtc); +return 0 ; } int ls7a2000_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc,@@ -1020,7 +1018,10 @@ int ls7a2000_crtc_init(struct drm_device *ddev,if (ret)return ret;drm_crtc_enable_color_mgmt (crtc,0 , false,256 ); +/* Reset the CRTC hardware, this is required for S3 support */ + lcrtc->hw_ops->reset (lcrtc); +return 0 ; }
Signature · 6 lines
-- 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)
Maxime Ripard
maintainerPATCH 51/70 drm/loongson: Convert to atomic_create_state
In reply to Maxime RipardThe 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.
---
Cc: Huacai Chen <[email protected]>
Cc: Icenowy Zheng <[email protected]>
Cc: Jianmin Lv <[email protected]>
Cc: Mingcong Bai <[email protected]>
Cc: Qianhai Wu <[email protected]>
Cc: Xi Ruoyao <[email protected]>
--- drivers/gpu/drm/loongson/lsdc_crtc.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
drivers/gpu/drm/loongson/lsdc_crtc.c +8 -9
@@ -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,
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 51/70] drm/loongson: Convert to atomic_create_state
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Meaningful code and commit message mismatch regarding hardware reset. --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Meaningful code and commit message mismatch regarding hardware reset. --
--- Patch [51]: [PATCH 51/70] drm/loongson: Convert to atomic_create_state --- commit 242c1e9da80c08d14806505d193ccb1acbb378d8
Author: Maxime Ripard <[email protected]>
drm/loongson: Convert to atomic_create_state
This commit converts the drm_crtc_funcs.reset hook to atomic_create_state for the loongson driver. The change is made because the original hook only handled state allocation and initialization without performing a hardware reset.
Quoted 10 lines
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_statehook is only meant to deal with state allocation and initialization, convert it accordingly. The hardware reset throughops->reset()is kept as part of the state creation since the core currently callsatomic_create_statein the same context where the reset hook was called.
[Severity: Medium]
Is the second paragraph of the commit message accurate for the loongson driver?
The first paragraph correctly notes that the original reset hook "does not perform any hardware reset", but the second paragraph claims that "The hardware reset through ops->reset() is kept". Since neither the old code nor the new lsdc_crtc_create_state() function performs a hardware reset, could this be a copy-paste error from another patch in the series?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=51
Maxime Ripard
maintainerPATCH 52/70 drm/mediatek: Convert to atomic_create_state
In reply to Maxime RipardThe 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.
---
Cc: AngeloGioacchino Del Regno <[email protected]>
Cc: Chun-Kuang Hu <[email protected]>
Cc: Matthias Brugger <[email protected]>
Cc: Philipp Zabel <[email protected]>
--- drivers/gpu/drm/mediatek/mtk_crtc.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/gpu/drm/mediatek/mtk_crtc.c +8 -10
@@ -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, };
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 53/70 drm/mgag200: Convert to atomic_create_state
In reply to Maxime RipardThe mgag200 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.
---
Cc: Dave Airlie <[email protected]>
Cc: Jocelyn Falempe <[email protected]>
--- drivers/gpu/drm/mgag200/mgag200_drv.h | 4 ++-- drivers/gpu/drm/mgag200/mgag200_mode.c | 15 +++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-)
drivers/gpu/drm/mgag200/mgag200_drv.h +2 -2
@@ -405,19 +405,19 @@ void mgag200_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic .atomic_check = mgag200_crtc_helper_atomic_check, \ .atomic_flush = mgag200_crtc_helper_atomic_flush, \ .atomic_enable = mgag200_crtc_helper_atomic_enable, \ .atomic_disable = mgag200_crtc_helper_atomic_disable -void mgag200_crtc_reset (struct drm_crtc *crtc); +struct drm_crtc_state *mgag200_crtc_atomic_create_state (struct drm_crtc *crtc);struct drm_crtc_state *mgag200_crtc_atomic_duplicate_state (struct drm_crtc *crtc);void mgag200_crtc_atomic_destroy_state (struct drm_crtc *crtc,struct drm_crtc_state *crtc_state);#define MGAG200_CRTC_FUNCS \ - .reset = mgag200_crtc_reset, \ .destroy = drm_crtc_cleanup, \ .set_config = drm_atomic_helper_set_config, \ .page_flip = drm_atomic_helper_page_flip, \ + .atomic_create_state = mgag200_crtc_atomic_create_state, \ .atomic_duplicate_state = mgag200_crtc_atomic_duplicate_state, \ .atomic_destroy_state = mgag200_crtc_atomic_destroy_statevoid mgag200_set_mode_regs (struct mga_device *mdev,const struct drm_display_mode *mode,bool set_vidrst);
drivers/gpu/drm/mgag200/mgag200_mode.c +7 -8
@@ -677,22 +677,21 @@ void mgag200_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomicstruct mga_device *mdev =to_mga_device (crtc->dev);mgag200_disable_display (mdev); } -void mgag200_crtc_reset (struct drm_crtc *crtc) +struct drm_crtc_state *mgag200_crtc_atomic_create_state (struct drm_crtc *crtc) {struct mgag200_crtc_state *mgag200_crtc_state; -if (crtc->state) - crtc->funcs->atomic_destroy_state (crtc, crtc->state); - mgag200_crtc_state =kzalloc_obj (*mgag200_crtc_state); -if (mgag200_crtc_state) -__drm_atomic_helper_crtc_reset (crtc, &mgag200_crtc_state->base); -else -__drm_atomic_helper_crtc_reset (crtc,NULL ); +if (!mgag200_crtc_state) +return ERR_PTR (-ENOMEM ); + +__drm_atomic_helper_crtc_state_init (&mgag200_crtc_state->base, crtc); + +return &mgag200_crtc_state->base; }struct drm_crtc_state *mgag200_crtc_atomic_duplicate_state (struct drm_crtc *crtc) {struct drm_crtc_state *crtc_state = crtc->state;
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 54/70 drm/msm: dpu1: Convert to atomic_create_state
In reply to Maxime RipardThe dpu1 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.
---
Cc: Abhinav Kumar <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Cc: Jessica Zhang <[email protected]>
Cc: Marijn Suijten <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Sean Paul <[email protected]>
Cc: [email protected]
Cc: [email protected]
--- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +9 -9
@@ -1138,21 +1138,21 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) end:DPU_ATRACE_END ("crtc_commit" ); } -static void dpu_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *dpu_crtc_create_state (struct drm_crtc *crtc) { -struct dpu_crtc_state *cstate =kzalloc_obj (*cstate); +struct dpu_crtc_state *cstate; -if (crtc->state) -dpu_crtc_destroy_state (crtc, crtc->state); + cstate =kzalloc_obj (*cstate); +if (!cstate) +return ERR_PTR (-ENOMEM ); -if (cstate) -__drm_atomic_helper_crtc_reset (crtc, &cstate->base); -else -__drm_atomic_helper_crtc_reset (crtc,NULL ); +__drm_atomic_helper_crtc_state_init (&cstate->base, crtc); + +return &cstate->base; }/** * dpu_crtc_duplicate_state - state duplicate hook * @crtc: Pointer to drm crtc structure
@@ -1841,11 +1841,11 @@ static int dpu_crtc_late_register(struct drm_crtc *crtc) }static const struct drm_crtc_funcs dpu_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, - .reset = dpu_crtc_reset, + .atomic_create_state = dpu_crtc_create_state, .atomic_duplicate_state = dpu_crtc_duplicate_state, .atomic_destroy_state = dpu_crtc_destroy_state, .atomic_print_state = dpu_crtc_atomic_print_state, .late_register = dpu_crtc_late_register, .verify_crc_source = dpu_crtc_verify_crc_source,
Signature · 1 line
2.55.0
Re: [PATCH 54/70] drm/msm: dpu1: Convert to atomic_create_state
Quoted 20 lines · Maxime Ripard
The dpu1 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_statehook is meant to do, minus the old state cleanup which the caller handles, convert the implementation to useatomic_create_statewith__drm_atomic_helper_crtc_state_init()instead.Signed-off-by Maxime Ripard <[email protected]>---
Cc: Abhinav Kumar <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Cc: Jessica Zhang <[email protected]>
Cc: Marijn Suijten <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Sean Paul <[email protected]>
Cc: [email protected]
Cc: [email protected]
Quoted 4 lines
--- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
Signature · 2 lines
With best wishes Dmitry
Maxime Ripard
maintainerPATCH 55/70 drm/msm: mdp5: Convert to atomic_create_state
In reply to Maxime RipardThe mdp5 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.
---
Cc: Abhinav Kumar <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Cc: Jessica Zhang <[email protected]>
Cc: Marijn Suijten <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Sean Paul <[email protected]>
Cc: [email protected]
Cc: [email protected]
--- drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c +10 -10
@@ -1126,27 +1126,27 @@ static void mdp5_crtc_destroy_state(struct drm_crtc *crtc, struct drm_crtc_state__drm_atomic_helper_crtc_destroy_state (state);kfree (mdp5_cstate); } -static void mdp5_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *mdp5_crtc_create_state (struct drm_crtc *crtc) { -struct mdp5_crtc_state *mdp5_cstate =kzalloc_obj (*mdp5_cstate); +struct mdp5_crtc_state *mdp5_cstate; -if (crtc->state) -mdp5_crtc_destroy_state (crtc, crtc->state); + mdp5_cstate =kzalloc_obj (*mdp5_cstate); +if (!mdp5_cstate) +return ERR_PTR (-ENOMEM ); -if (mdp5_cstate) -__drm_atomic_helper_crtc_reset (crtc, &mdp5_cstate->base); -else -__drm_atomic_helper_crtc_reset (crtc,NULL ); +__drm_atomic_helper_crtc_state_init (&mdp5_cstate->base, crtc); + +return &mdp5_cstate->base; }static const struct drm_crtc_funcs mdp5_crtc_no_lm_cursor_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, - .reset = mdp5_crtc_reset, + .atomic_create_state = mdp5_crtc_create_state, .atomic_duplicate_state = mdp5_crtc_duplicate_state, .atomic_destroy_state = mdp5_crtc_destroy_state, .atomic_print_state = mdp5_crtc_atomic_print_state, .get_vblank_counter = mdp5_crtc_get_vblank_counter, .enable_vblank = msm_crtc_enable_vblank,
@@ -1155,11 +1155,11 @@ static const struct drm_crtc_funcs mdp5_crtc_no_lm_cursor_funcs = { };static const struct drm_crtc_funcs mdp5_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, - .reset = mdp5_crtc_reset, + .atomic_create_state = mdp5_crtc_create_state, .atomic_duplicate_state = mdp5_crtc_duplicate_state, .atomic_destroy_state = mdp5_crtc_destroy_state, .cursor_set = mdp5_crtc_cursor_set, .cursor_move = mdp5_crtc_cursor_move, .atomic_print_state = mdp5_crtc_atomic_print_state,
Signature · 1 line
2.55.0
Re: [PATCH 55/70] drm/msm: mdp5: Convert to atomic_create_state
Quoted 20 lines · Maxime Ripard
The mdp5 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_statehook is meant to do, minus the old state cleanup which the caller handles, convert the implementation to useatomic_create_statewith__drm_atomic_helper_crtc_state_init()instead.Signed-off-by Maxime Ripard <[email protected]>---
Cc: Abhinav Kumar <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Cc: Jessica Zhang <[email protected]>
Cc: Marijn Suijten <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Sean Paul <[email protected]>
Cc: [email protected]
Cc: [email protected]
Quoted 4 lines
--- drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
Signature · 2 lines
With best wishes Dmitry
Maxime Ripard
maintainerPATCH 56/70 drm/mxsfb: lcdif: Convert to atomic_create_state
In reply to Maxime RipardThe lcdif 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.
---
Cc: Fabio Estevam <[email protected]>
Cc: Frank Li <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Stefan Agner <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/mxsfb/lcdif_kms.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
drivers/gpu/drm/mxsfb/lcdif_kms.c +8 -9
@@ -594,22 +594,21 @@ static void lcdif_crtc_atomic_destroy_state(struct drm_crtc *crtc, {__drm_atomic_helper_crtc_destroy_state (state);kfree (to_lcdif_crtc_state (state)); } -static void lcdif_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *lcdif_crtc_create_state (struct drm_crtc *crtc) {struct lcdif_crtc_state *state; -if (crtc->state) -lcdif_crtc_atomic_destroy_state (crtc, 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 *lcdif_crtc_atomic_duplicate_state (struct drm_crtc *crtc) {
@@ -657,11 +656,11 @@ static const struct drm_crtc_helper_funcs lcdif_crtc_helper_funcs = { .atomic_enable = lcdif_crtc_atomic_enable, .atomic_disable = lcdif_crtc_atomic_disable, };static const struct drm_crtc_funcs lcdif_crtc_funcs = { - .reset = lcdif_crtc_reset, + .atomic_create_state = lcdif_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = lcdif_crtc_atomic_duplicate_state, .atomic_destroy_state = lcdif_crtc_atomic_destroy_state,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 57/70 drm/nouveau: Convert to atomic_create_state
In reply to Maxime RipardThe nouveau nv50 head 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.
---
Cc: Danilo Krummrich <[email protected]>
Cc: Lyude Paul <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/nouveau/dispnv50/head.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/gpu/drm/nouveau/dispnv50/head.c +6 -8
@@ -489,22 +489,20 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc) asyh->clr.mask =0 ; asyh->set.mask =0 ;return &asyh->state; } -static void -nv50_head_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *nv50_head_create_state (struct drm_crtc *crtc) {struct nv50_head_atom *asyh;if (WARN_ON (!(asyh =kzalloc_obj (*asyh)))) -return ; +return ERR_PTR (-ENOMEM ); -if (crtc->state) -nv50_head_atomic_destroy_state (crtc, crtc->state); +__drm_atomic_helper_crtc_state_init (&asyh->state, crtc); -__drm_atomic_helper_crtc_reset (crtc, &asyh->state); +return &asyh->state; }static int nv50_head_late_register (struct drm_crtc *crtc) {
@@ -523,11 +521,11 @@ nv50_head_destroy(struct drm_crtc *crtc)kfree (head); }static const struct drm_crtc_funcs nv50_head_func = { - .reset = nv50_head_reset, + .atomic_create_state = nv50_head_create_state, .destroy = nv50_head_destroy, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = nv50_head_atomic_duplicate_state, .atomic_destroy_state = nv50_head_atomic_destroy_state,
@@ -537,11 +535,11 @@ nv50_head_func = { .late_register = nv50_head_late_register, };static const struct drm_crtc_funcs nvd9_head_func = { - .reset = nv50_head_reset, + .atomic_create_state = nv50_head_create_state, .destroy = nv50_head_destroy, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = nv50_head_atomic_duplicate_state, .atomic_destroy_state = nv50_head_atomic_destroy_state,
Signature · 1 line
2.55.0
Re: [PATCH 57/70] drm/nouveau: Convert to atomic_create_state
Quoted 79 lines · Maxime Ripard
The nouveau nv50 head 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_statehook is meant to do, minus the old state cleanup which the caller handles, convert the implementation to useatomic_create_statewith__drm_atomic_helper_crtc_state_init()instead.Signed-off-by Maxime Ripard <[email protected]>---
Cc: Danilo Krummrich <[email protected]>
Cc: Lyude Paul <[email protected]>
Cc: [email protected]
---
drivers/gpu/drm/nouveau/dispnv50/head.c | 14 ++++++--------1 file changed, 6 insertions(+), 8 deletions(-)drivers/gpu/drm/nouveau/dispnv50/head.c +0
-0@@ -489,22 +489,20 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc) asyh->clr.mask = 0; asyh->set.mask = 0; return &asyh->state; }drivers/gpu/drm/nouveau/dispnv50/head.c +1
-2-static void -nv50_head_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *nv50_head_create_state(struct drm_crtc*crtc)
{
structnv50_head_atom*asyh;
if (WARN_ON(!(asyh = kzalloc_obj(*asyh))))drivers/gpu/drm/nouveau/dispnv50/head.c +1
-1-return ; +return ERR_PTR (-ENOMEM );drivers/gpu/drm/nouveau/dispnv50/head.c +1
-2-if (crtc->state) -nv50_head_atomic_destroy_state (crtc, crtc->state); +__drm_atomic_helper_crtc_state_init (&asyh->state, crtc);drivers/gpu/drm/nouveau/dispnv50/head.c +1
-1-__drm_atomic_helper_crtc_reset (crtc, &asyh->state); +return &asyh->state;}
static int
nv50_head_late_register(structdrm_crtc*crtc)
{drivers/gpu/drm/nouveau/dispnv50/head.c +0
-0@@ -523,11 +521,11 @@ nv50_head_destroy(struct drm_crtc *crtc) kfree(head); }
static const structdrm_crtc_funcs
nv50_head_func = {drivers/gpu/drm/nouveau/dispnv50/head.c +1
-1- .reset = nv50_head_reset, + .atomic_create_state = nv50_head_create_state,.destroy =
nv50_head_destroy,
.set_config =drm_atomic_helper_set_config, .page_flip =drm_atomic_helper_page_flip, .atomic_duplicate_state =nv50_head_atomic_duplicate_state, .atomic_destroy_state =nv50_head_atomic_destroy_state,drivers/gpu/drm/nouveau/dispnv50/head.c +0
-0@@ -537,11 +535,11 @@ nv50_head_func = { .late_register = nv50_head_late_register, };
static const structdrm_crtc_funcs
nvd9_head_func = {drivers/gpu/drm/nouveau/dispnv50/head.c +1
-1- .reset = nv50_head_reset, + .atomic_create_state = nv50_head_create_state,.destroy =
nv50_head_destroy,
.set_config =drm_atomic_helper_set_config, .page_flip =drm_atomic_helper_page_flip, .atomic_duplicate_state =nv50_head_atomic_duplicate_state, .atomic_destroy_state =nv50_head_atomic_destroy_state,
Maxime Ripard
maintainerPATCH 58/70 drm/omapdrm: Convert to atomic_create_state
In reply to Maxime RipardThe omap 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.
---
Cc: Tomi Valkeinen <[email protected]>
--- drivers/gpu/drm/omapdrm/omap_crtc.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
drivers/gpu/drm/omapdrm/omap_crtc.c +8 -9
@@ -705,22 +705,21 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,return -EINVAL ;return 0 ; } -static void omap_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *omap_crtc_create_state (struct drm_crtc *crtc) {struct omap_crtc_state *state; -if (crtc->state) -__drm_atomic_helper_crtc_destroy_state (crtc->state); - -kfree (crtc->state); - 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 *omap_crtc_duplicate_state (struct drm_crtc *crtc) {
@@ -743,11 +742,11 @@ omap_crtc_duplicate_state(struct drm_crtc *crtc)return &state->base; }static const struct drm_crtc_funcs omap_crtc_funcs = { - .reset = omap_crtc_reset, + .atomic_create_state = omap_crtc_create_state, .set_config = drm_atomic_helper_set_config, .destroy = omap_crtc_destroy, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = omap_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 59/70 drm/rcar-du: Convert to atomic_create_state
In reply to Maxime RipardThe rcar-du crtc implementation provides a custom reset hook. However, this hook only allocates the state, initializes it with __drm_atomic_helper_crtc_reset(), sets driver-specific default values for the CRC source and index, 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.
---
Cc: Geert Uytterhoeven <[email protected]>
Cc: Kieran Bingham <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Magnus Damm <[email protected]>
Cc: Tomi Valkeinen <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)
drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c +7 -10
@@ -995,27 +995,24 @@ static void rcar_du_crtc_cleanup(struct drm_crtc *crtc)rcar_du_crtc_crc_cleanup (rcrtc);drm_crtc_cleanup (crtc); } -static void rcar_du_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *rcar_du_crtc_create_state (struct drm_crtc *crtc) {struct rcar_du_crtc_state *state; -if (crtc->state) { -rcar_du_crtc_atomic_destroy_state (crtc, crtc->state); - crtc->state =NULL ; - } - state =kzalloc_obj (*state);if (state ==NULL ) -return ; +return ERR_PTR (-ENOMEM ); state->crc.source =VSP1_DU_CRC_NONE ; state->crc.index =0 ; -__drm_atomic_helper_crtc_reset (crtc, &state->state); +__drm_atomic_helper_crtc_state_init (&state->state, crtc); + +return &state->state; }static int rcar_du_crtc_enable_vblank (struct drm_crtc *crtc) {struct rcar_du_crtc *rcrtc =to_rcar_crtc (crtc);
@@ -1153,22 +1150,22 @@ static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc,return ret; }static const struct drm_crtc_funcs crtc_funcs_gen2 = { - .reset = rcar_du_crtc_reset, + .atomic_create_state = rcar_du_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = rcar_du_crtc_atomic_duplicate_state, .atomic_destroy_state = rcar_du_crtc_atomic_destroy_state, .enable_vblank = rcar_du_crtc_enable_vblank, .disable_vblank = rcar_du_crtc_disable_vblank, };static const struct drm_crtc_funcs crtc_funcs_gen3 = { - .reset = rcar_du_crtc_reset, + .atomic_create_state = rcar_du_crtc_create_state, .destroy = rcar_du_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = rcar_du_crtc_atomic_duplicate_state, .atomic_destroy_state = rcar_du_crtc_atomic_destroy_state,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 60/70 drm/rzg2l-du: Convert to atomic_create_state
In reply to Maxime RipardThe rzg2l-du 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.
---
Cc: Biju Das <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c +6 -9
@@ -330,24 +330,21 @@ static void rzg2l_du_crtc_atomic_destroy_state(struct drm_crtc *crtc, {__drm_atomic_helper_crtc_destroy_state (state);kfree (to_rzg2l_crtc_state (state)); } -static void rzg2l_du_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *rzg2l_du_crtc_create_state (struct drm_crtc *crtc) {struct rzg2l_du_crtc_state *state; -if (crtc->state) { -rzg2l_du_crtc_atomic_destroy_state (crtc, crtc->state); - crtc->state =NULL ; - } - state =kzalloc_obj (*state);if (!state) -return ; +return ERR_PTR (-ENOMEM ); -__drm_atomic_helper_crtc_reset (crtc, &state->state); +__drm_atomic_helper_crtc_state_init (&state->state, crtc); + +return &state->state; }static int rzg2l_du_crtc_enable_vblank (struct drm_crtc *crtc) {struct rzg2l_du_crtc *rcrtc =to_rzg2l_crtc (crtc);
@@ -363,11 +360,11 @@ static void rzg2l_du_crtc_disable_vblank(struct drm_crtc *crtc) rcrtc->vblank_enable = false; }static const struct drm_crtc_funcs crtc_funcs_rz = { - .reset = rzg2l_du_crtc_reset, + .atomic_create_state = rzg2l_du_crtc_create_state, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = rzg2l_du_crtc_atomic_duplicate_state, .atomic_destroy_state = rzg2l_du_crtc_atomic_destroy_state, .enable_vblank = rzg2l_du_crtc_enable_vblank,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 61/70 drm/rockchip: vop: Convert to atomic_create_state
In reply to Maxime RipardThe rockchip vop 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.
---
Cc: "Heiko Stübner" <[email protected]>
Cc: Andy Yan <[email protected]>
Cc: Sandy Huang <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/gpu/drm/rockchip/rockchip_drm_vop.c +9 -9
@@ -1654,21 +1654,21 @@ static void vop_crtc_destroy_state(struct drm_crtc *crtc,__drm_atomic_helper_crtc_destroy_state (&s->base);kfree (s); } -static void vop_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *vop_crtc_create_state (struct drm_crtc *crtc) { -struct rockchip_crtc_state *crtc_state =kzalloc_obj (*crtc_state); +struct rockchip_crtc_state *crtc_state; -if (crtc->state) -vop_crtc_destroy_state (crtc, crtc->state); + crtc_state =kzalloc_obj (*crtc_state); +if (!crtc_state) +return ERR_PTR (-ENOMEM ); -if (crtc_state) -__drm_atomic_helper_crtc_reset (crtc, &crtc_state->base); -else -__drm_atomic_helper_crtc_reset (crtc,NULL ); +__drm_atomic_helper_crtc_state_init (&crtc_state->base, crtc); + +return &crtc_state->base; }#ifdef CONFIG_DRM_ANALOGIX_DP static struct drm_connector *vop_get_edp_connector (struct vop *vop) {
@@ -1736,11 +1736,11 @@ vop_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name,static const struct drm_crtc_funcs vop_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .destroy = drm_crtc_cleanup, - .reset = vop_crtc_reset, + .atomic_create_state = vop_crtc_create_state, .atomic_duplicate_state = vop_crtc_duplicate_state, .atomic_destroy_state = vop_crtc_destroy_state, .enable_vblank = vop_crtc_enable_vblank, .disable_vblank = vop_crtc_disable_vblank, .set_crc_source = vop_crtc_set_crc_source,
Signature · 7 lines
2.55.0 _______________________________________________ Linux-rockchip mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH 61/70] drm/rockchip: vop: Convert to atomic_create_state
Am Freitag, 21. August 2026, 16:39:37 Mitteleuropäische Sommerzeit schrieb Maxime Ripard:
Quoted 11 lines
The rockchip vop 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_statehook is meant to do, minus the old state cleanup which the caller handles, convert the implementation to useatomic_create_statewith__drm_atomic_helper_crtc_state_init()instead.Signed-off-by Maxime Ripard <[email protected]>
Maxime Ripard
maintainerPATCH 62/70 drm/rockchip: vop2: Convert to atomic_create_state
In reply to Maxime RipardThe rockchip vop2 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.
---
Cc: "Heiko Stübner" <[email protected]>
Cc: Andy Yan <[email protected]>
Cc: Sandy Huang <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c +9 -9
@@ -2298,28 +2298,28 @@ static void vop2_crtc_destroy_state(struct drm_crtc *crtc,__drm_atomic_helper_crtc_destroy_state (&vcstate->base);kfree (vcstate); } -static void vop2_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *vop2_crtc_create_state (struct drm_crtc *crtc) { -struct rockchip_crtc_state *vcstate =kzalloc_obj (*vcstate); +struct rockchip_crtc_state *vcstate; -if (crtc->state) -vop2_crtc_destroy_state (crtc, crtc->state); + vcstate =kzalloc_obj (*vcstate); +if (!vcstate) +return ERR_PTR (-ENOMEM ); -if (vcstate) -__drm_atomic_helper_crtc_reset (crtc, &vcstate->base); -else -__drm_atomic_helper_crtc_reset (crtc,NULL ); +__drm_atomic_helper_crtc_state_init (&vcstate->base, crtc); + +return &vcstate->base; }static const struct drm_crtc_funcs vop2_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .destroy = drm_crtc_cleanup, - .reset = vop2_crtc_reset, + .atomic_create_state = vop2_crtc_create_state, .atomic_duplicate_state = vop2_crtc_duplicate_state, .atomic_destroy_state = vop2_crtc_destroy_state, .enable_vblank = vop2_crtc_enable_vblank, .disable_vblank = vop2_crtc_disable_vblank, .late_register = vop2_crtc_late_register,
Signature · 7 lines
2.55.0 _______________________________________________ Linux-rockchip mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH 62/70] drm/rockchip: vop2: Convert to atomic_create_state
Am Freitag, 21. August 2026, 16:39:38 Mitteleuropäische Sommerzeit schrieb Maxime Ripard:
Quoted 11 lines
The rockchip vop2 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_statehook is meant to do, minus the old state cleanup which the caller handles, convert the implementation to useatomic_create_statewith__drm_atomic_helper_crtc_state_init()instead.Signed-off-by Maxime Ripard <[email protected]>
Maxime Ripard
maintainerPATCH 63/70 drm/sitronix: st7920: Convert to atomic_create_state
In reply to Maxime RipardThe st7920 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.
---
Cc: Iker Pedrosa <[email protected]>
--- drivers/gpu/drm/sitronix/st7920.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/gpu/drm/sitronix/st7920.c +6 -4
@@ -573,21 +573,23 @@ 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 +629,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,
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 63/70] drm/sitronix: st7920: Convert to atomic_create_state
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The commit message falsely claims that the old `st7920_crtc_reset` freed the previous state. --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message falsely claims that the old st7920_crtc_reset freed the previous state. --
--- Patch [63]: [PATCH 63/70] drm/sitronix: st7920: Convert to atomic_create_state --- commit 51126b7e2261e0176cf52ba6e5a5273dcbb0592e
Author: Maxime Ripard <[email protected]>
drm/sitronix: st7920: Convert to atomic_create_state
This commit converts the st7920 CRTC implementation to use the atomic_create_state hook rather than a custom reset hook.
Quoted 4 lines
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 frees the previous state. It does not perform any hardware reset.
[Severity: Medium]
Is this description accurate regarding the previous state being freed?
Looking at the original st7920_crtc_reset() implementation before this patch, it warns if crtc->state is non-NULL but does not appear to free the old state memory:
drm_WARN_ON_ONCE(crtc->dev, crtc->state); st7920_state = kzalloc_obj(*st7920_state); if (!st7920_state) return; __drm_atomic_helper_crtc_reset(crtc, &st7920_state->base);
Could the commit message be updated to clarify that the old hook was not actually freeing the previous state?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=63
Maxime Ripard
maintainerPATCH 64/70 drm/solomon: ssd130x: Convert to atomic_create_state
In reply to Maxime RipardThe ssd130x 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.
---
Cc: Javier Martinez Canillas <[email protected]>
--- drivers/gpu/drm/solomon/ssd130x.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/gpu/drm/solomon/ssd130x.c +6 -4
@@ -1465,21 +1465,23 @@ static int ssd133x_crtc_atomic_check(struct drm_crtc *crtc,return 0 ; }/* Called during init to allocate the CRTC's atomic state. */ -static void ssd130x_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *ssd130x_crtc_create_state (struct drm_crtc *crtc) {struct ssd130x_crtc_state *ssd130x_state;drm_WARN_ON_ONCE (crtc->dev, crtc->state); ssd130x_state =kzalloc_obj (*ssd130x_state);if (!ssd130x_state) -return ; +return ERR_PTR (-ENOMEM ); -__drm_atomic_helper_crtc_reset (crtc, &ssd130x_state->base); +__drm_atomic_helper_crtc_state_init (&ssd130x_state->base, crtc); + +return &ssd130x_state->base; }static struct drm_crtc_state *ssd130x_crtc_duplicate_state (struct drm_crtc *crtc) {struct ssd130x_crtc_state *old_ssd130x_state;
@@ -1532,11 +1534,11 @@ static const struct drm_crtc_helper_funcs ssd130x_crtc_helper_funcs[] = { .atomic_check = ssd133x_crtc_atomic_check, }, };static const struct drm_crtc_funcs ssd130x_crtc_funcs = { - .reset = ssd130x_crtc_reset, + .atomic_create_state = ssd130x_crtc_create_state, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .atomic_duplicate_state = ssd130x_crtc_duplicate_state, .atomic_destroy_state = ssd130x_crtc_destroy_state,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 65/70 drm/tegra: Convert to atomic_create_state
In reply to Maxime RipardThe tegra 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.
---
Cc: Jonathan Hunter <[email protected]>
Cc: Mikko Perttunen <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: [email protected]
--- drivers/gpu/drm/tegra/dc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/gpu/drm/tegra/dc.c +9 -9
@@ -1484,21 +1484,21 @@ static struct drm_plane *tegra_dc_add_planes(struct drm_device *drm,static void tegra_dc_destroy (struct drm_crtc *crtc) {drm_crtc_cleanup (crtc); } -static void tegra_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *tegra_crtc_create_state (struct drm_crtc *crtc) { -struct tegra_dc_state *state =kzalloc_obj (*state); +struct tegra_dc_state *state; -if (crtc->state) -tegra_crtc_atomic_destroy_state (crtc, crtc->state); + state =kzalloc_obj (*state); +if (!state) +return ERR_PTR (-ENOMEM ); -if (state) -__drm_atomic_helper_crtc_reset (crtc, &state->base); -else -__drm_atomic_helper_crtc_reset (crtc,NULL ); +__drm_atomic_helper_crtc_state_init (&state->base, crtc); + +return &state->base; }static struct drm_crtc_state *tegra_crtc_atomic_duplicate_state (struct drm_crtc *crtc) {
@@ -1903,11 +1903,11 @@ static void tegra_dc_disable_vblank(struct drm_crtc *crtc)static const struct drm_crtc_funcs tegra_crtc_funcs = { .page_flip = drm_atomic_helper_page_flip, .set_config = drm_atomic_helper_set_config, .destroy = tegra_dc_destroy, - .reset = tegra_crtc_reset, + .atomic_create_state = tegra_crtc_create_state, .atomic_duplicate_state = tegra_crtc_atomic_duplicate_state, .atomic_destroy_state = tegra_crtc_atomic_destroy_state, .late_register = tegra_dc_late_register, .early_unregister = tegra_dc_early_unregister, .get_vblank_counter = tegra_dc_get_vblank_counter,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 66/70 drm/vc4: Convert to atomic_create_state
In reply to Maxime RipardThe vc4 crtc implementation provides a custom reset hook. However, this hook only allocates the state, initializes it with __drm_atomic_helper_crtc_reset(), sets the assigned_channel field to VC4_HVS_CHANNEL_DISABLED, 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.
---
Cc: "Maíra Canal" <[email protected]>
Cc: Dave Stevenson <[email protected]>
Cc: Raspberry Pi Kernel Maintenance <[email protected]>
--- drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c | 2 +- drivers/gpu/drm/vc4/vc4_crtc.c | 14 ++++++-------- drivers/gpu/drm/vc4/vc4_drv.h | 2 +- drivers/gpu/drm/vc4/vc4_txp.c | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-)
drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c +1 -1
@@ -12,11 +12,11 @@ static const struct drm_crtc_helper_funcs vc4_dummy_crtc_helper_funcs = { };static const struct drm_crtc_funcs vc4_dummy_crtc_funcs = { .atomic_destroy_state = vc4_crtc_destroy_state, .atomic_duplicate_state = vc4_crtc_duplicate_state, - .reset = vc4_crtc_reset, + .atomic_create_state = vc4_crtc_create_state, }; struct vc4_dummy_crtc *vc4_mock_pv(struct kunit *test, struct drm_device *drm, struct drm_plane *plane,
drivers/gpu/drm/vc4/vc4_crtc.c +6 -8
@@ -1133,25 +1133,23 @@ void vc4_crtc_destroy_state(struct drm_crtc *crtc, }drm_atomic_helper_crtc_destroy_state (crtc, state); } -void vc4_crtc_reset (struct drm_crtc *crtc) +struct drm_crtc_state *vc4_crtc_create_state (struct drm_crtc *crtc) {struct vc4_crtc_state *vc4_crtc_state; -if (crtc->state) -vc4_crtc_destroy_state (crtc, crtc->state); - vc4_crtc_state =kzalloc_obj (*vc4_crtc_state);if (!vc4_crtc_state) { - crtc->state =NULL ; -return ; +return ERR_PTR (-ENOMEM ); } vc4_crtc_state->assigned_channel =VC4_HVS_CHANNEL_DISABLED ; -__drm_atomic_helper_crtc_reset (crtc, &vc4_crtc_state->base); +__drm_atomic_helper_crtc_state_init (&vc4_crtc_state->base, crtc); + +return &vc4_crtc_state->base; }int vc4_crtc_late_register (struct drm_crtc *crtc) {struct drm_device *drm = crtc->dev;
@@ -1168,11 +1166,11 @@ static const struct drm_crtc_funcs vc4_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = vc4_page_flip, .set_property = NULL, .cursor_set = NULL, /* handled by drm_mode_cursor_universal */ .cursor_move = NULL, /* handled by drm_mode_cursor_universal */ - .reset = vc4_crtc_reset, + .atomic_create_state = vc4_crtc_create_state, .atomic_duplicate_state = vc4_crtc_duplicate_state, .atomic_destroy_state = vc4_crtc_destroy_state, .enable_vblank = vc4_enable_vblank, .disable_vblank = vc4_disable_vblank, .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
drivers/gpu/drm/vc4/vc4_drv.h +1 -1
@@ -947,11 +947,11 @@ int vc4_page_flip(struct drm_crtc *crtc,int vc4_crtc_atomic_check (struct drm_crtc *crtc,struct drm_atomic_commit *state);struct drm_crtc_state *vc4_crtc_duplicate_state (struct drm_crtc *crtc);void vc4_crtc_destroy_state (struct drm_crtc *crtc,struct drm_crtc_state *state); -void vc4_crtc_reset (struct drm_crtc *crtc); +struct drm_crtc_state *vc4_crtc_create_state (struct drm_crtc *crtc);void vc4_crtc_handle_vblank (struct vc4_crtc *crtc);void vc4_crtc_send_vblank (struct drm_crtc *crtc);int vc4_crtc_late_register (struct drm_crtc *crtc); void vc4_crtc_get_margins(struct drm_crtc_state *state, unsigned int *left, unsigned int *right,
drivers/gpu/drm/vc4/vc4_txp.c +1 -1
@@ -423,11 +423,11 @@ static int vc4_txp_enable_vblank(struct drm_crtc *crtc)static void vc4_txp_disable_vblank (struct drm_crtc *crtc) {}static const struct drm_crtc_funcs vc4_txp_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = vc4_page_flip, - .reset = vc4_crtc_reset, + .atomic_create_state = vc4_crtc_create_state, .atomic_duplicate_state = vc4_crtc_duplicate_state, .atomic_destroy_state = vc4_crtc_destroy_state, .enable_vblank = vc4_txp_enable_vblank, .disable_vblank = vc4_txp_disable_vblank, .late_register = vc4_crtc_late_register,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 67/70 drm/vkms: Convert to atomic_create_state
In reply to Maxime RipardThe vkms crtc implementation provides a custom reset hook. However, this hook only allocates the state, initializes it with __drm_atomic_helper_crtc_reset(), initializes the composer work struct, 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.
---
Cc: Haneen Mohammed <[email protected]>
Cc: Louis Chauvet <[email protected]>
Cc: Melissa Wen <[email protected]>
--- drivers/gpu/drm/vkms/vkms_crtc.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
drivers/gpu/drm/vkms/vkms_crtc.c +10 -8
@@ -84,26 +84,28 @@ static void vkms_atomic_crtc_destroy_state(struct drm_crtc *crtc,WARN_ON (work_pending (&vkms_state->composer_work));kfree (vkms_state->active_planes);kfree (vkms_state); } -static void vkms_atomic_crtc_reset (struct drm_crtc *crtc) +static struct drm_crtc_state *vkms_atomic_crtc_create_state (struct drm_crtc *crtc) { -struct vkms_crtc_state *vkms_state =kzalloc_obj (*vkms_state); +struct vkms_crtc_state *vkms_state; -if (crtc->state) -vkms_atomic_crtc_destroy_state (crtc, crtc->state); + vkms_state =kzalloc_obj (*vkms_state); +if (!vkms_state) +return ERR_PTR (-ENOMEM ); -__drm_atomic_helper_crtc_reset (crtc, &vkms_state->base); -if (vkms_state) -INIT_WORK (&vkms_state->composer_work, vkms_composer_worker); +__drm_atomic_helper_crtc_state_init (&vkms_state->base, crtc); +INIT_WORK (&vkms_state->composer_work, vkms_composer_worker); + +return &vkms_state->base; }static const struct drm_crtc_funcs vkms_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, - .reset = vkms_atomic_crtc_reset, + .atomic_create_state = vkms_atomic_crtc_create_state, .atomic_duplicate_state = vkms_atomic_crtc_duplicate_state, .atomic_destroy_state = vkms_atomic_crtc_destroy_state,DRM_CRTC_VBLANK_TIMER_FUNCS , .get_crc_sources = vkms_get_crc_sources, .set_crc_source = vkms_set_crc_source,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 68/70 drm/vmwgfx: Convert to atomic_create_state
In reply to Maxime RipardThe vmwgfx 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.
---
Cc: Broadcom internal kernel review list <[email protected]>
Cc: Zack Rusin <[email protected]>
--- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 +++++++--------------- drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +- 5 files changed, 11 insertions(+), 19 deletions(-)
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +7 -15
@@ -204,36 +204,28 @@ vmw_du_crtc_duplicate_state(struct drm_crtc *crtc)return state; }/** -* vmw_du_crtc_reset - creates a blank vmw crtc state +* vmw_du_crtc_create_state - creates a blank vmw crtc state * @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. +* Allocates a new empty state object for @crtc. */ -void vmw_du_crtc_reset (struct drm_crtc *crtc) +struct drm_crtc_state *vmw_du_crtc_create_state (struct drm_crtc *crtc) {struct vmw_crtc_state *vcs; - -if (crtc->state) { -__drm_atomic_helper_crtc_destroy_state (crtc->state); - -kfree (vmw_crtc_state_to_vcs (crtc->state)); - } - vcs =kzalloc_obj (*vcs); -if (!vcs) {DRM_ERROR ("Cannot allocate vmw_crtc_state\n" ); -return ; +return ERR_PTR (-ENOMEM ); } -__drm_atomic_helper_crtc_reset (crtc, &vcs->base); +__drm_atomic_helper_crtc_state_init (&vcs->base, crtc); + +return &vcs->base; }/** * vmw_du_crtc_destroy_state - destroy crtc state
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h +1 -1
@@ -419,11 +419,11 @@ void vmw_du_plane_unpin_surf(struct vmw_plane_state *vps);int vmw_du_crtc_atomic_check (struct drm_crtc *crtc,struct drm_atomic_commit *state);void vmw_du_crtc_atomic_begin (struct drm_crtc *crtc,struct drm_atomic_commit *state); -void vmw_du_crtc_reset (struct drm_crtc *crtc); +struct drm_crtc_state *vmw_du_crtc_create_state (struct drm_crtc *crtc);struct drm_crtc_state *vmw_du_crtc_duplicate_state (struct drm_crtc *crtc);void vmw_du_crtc_destroy_state (struct drm_crtc *crtc,struct drm_crtc_state *state);void vmw_du_connector_reset (struct drm_connector *connector); struct drm_connector_state *
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c +1 -1
@@ -247,11 +247,11 @@ static void vmw_ldu_crtc_mode_set_nofb(struct drm_crtc *crtc) }static const struct drm_crtc_funcs vmw_legacy_crtc_funcs = { .gamma_set = vmw_du_crtc_gamma_set, .destroy = vmw_ldu_crtc_destroy, - .reset = vmw_du_crtc_reset, + .atomic_create_state = vmw_du_crtc_create_state, .atomic_duplicate_state = vmw_du_crtc_duplicate_state, .atomic_destroy_state = vmw_du_crtc_destroy_state, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .enable_vblank = vmw_vkms_enable_vblank,
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c +1 -1
@@ -303,11 +303,11 @@ static void vmw_sou_crtc_atomic_disable(struct drm_crtc *crtc, }static const struct drm_crtc_funcs vmw_screen_object_crtc_funcs = { .gamma_set = vmw_du_crtc_gamma_set, .destroy = vmw_sou_crtc_destroy, - .reset = vmw_du_crtc_reset, + .atomic_create_state = vmw_du_crtc_create_state, .atomic_duplicate_state = vmw_du_crtc_duplicate_state, .atomic_destroy_state = vmw_du_crtc_destroy_state, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .enable_vblank = vmw_vkms_enable_vblank,
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c +1 -1
@@ -776,11 +776,11 @@ int vmw_kms_stdu_surface_dirty(struct vmw_private *dev_priv,* Screen Target CRTC dispatch table */ static const struct drm_crtc_funcs vmw_stdu_crtc_funcs = { .gamma_set = vmw_du_crtc_gamma_set, .destroy = vmw_stdu_crtc_destroy, - .reset = vmw_du_crtc_reset, + .atomic_create_state = vmw_du_crtc_create_state, .atomic_duplicate_state = vmw_du_crtc_duplicate_state, .atomic_destroy_state = vmw_du_crtc_destroy_state, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .enable_vblank = vmw_vkms_enable_vblank,
Signature · 1 line
2.55.0
Maxime Ripard
maintainerPATCH 69/70 drm/atomic-helper: Remove __drm_atomic_helper_crtc_reset
In reply to Maxime RipardAll 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.
--- drivers/gpu/drm/drm_atomic_state_helper.c | 26 -------------------------- include/drm/drm_atomic_state_helper.h | 2 -- 2 files changed, 28 deletions(-)
drivers/gpu/drm/drm_atomic_state_helper.c +0 -26
@@ -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.
include/drm/drm_atomic_state_helper.h +0 -2
@@ -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 *
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 69/70] drm/atomic-helper: Remove __drm_atomic_helper_crtc_reset
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Memory leak and unassigned CRTC state due to ignored return value from `atomic_create_state`. --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Memory leak and unassigned CRTC state due to ignored return value from atomic_create_state. --
--- Patch [69]: [PATCH 69/70] drm/atomic-helper: Remove __drm_atomic_helper_crtc_reset --- commit 9dc0ccd39531b9bf90d43caf9838f04ca43ed26d
Author: Maxime Ripard <[email protected]>
drm/atomic-helper: Remove __drm_atomic_helper_crtc_reset
This patch series converts users of __drm_atomic_helper_crtc_reset() to use drm_atomic_helper_crtc_state_init() and atomic_create_state(). It then removes the now unused __drm_atomic_helper_crtc_reset() helper entirely.
Quoted 4 lines
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c +0
-0
[ ... ]
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm, · line 761
Quoted 10 lines
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c +3
-3@@ -757,9 +756,9 @@ 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) + acrtc->base.funcs->atomic_create_state (&acrtc->base);
[Severity: Medium]
Does this code leak the newly allocated state?
The old .reset() hook called __drm_atomic_helper_crtc_reset(), which internally assigned crtc->state. The new .atomic_create_state() hook returns the allocated state pointer, but the return value is ignored here.
Could this leave acrtc->base.state unassigned and leak the allocated struct dm_crtc_state during driver initialization?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=69
Re: [PATCH 69/70] drm/atomic-helper: Remove __drm_atomic_helper_crtc_reset
In reply to Maxime RipardQuoted 11 lines · Maxime Ripard
All users of
__drm_atomic_helper_crtc_reset()have been converted to usedrm_atomic_helper_crtc_state_init()andatomic_create_state.Remove the now unused helper.
Signed-off-by Maxime Ripard <[email protected]>--- drivers/gpu/drm/drm_atomic_state_helper.c | 26 -------------------------- include/drm/drm_atomic_state_helper.h | 2 -- 2 files changed, 28 deletions(-)
Signature · 2 lines
With best wishes Dmitry
Maxime Ripard
maintainerPATCH 70/70 drm/crtc: Remove reset
In reply to Maxime RipardAll 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().
--- drivers/gpu/drm/drm_mode_config.c | 4 +--- include/drm/drm_crtc.h | 10 ---------- 2 files changed, 1 insertion(+), 13 deletions(-)
drivers/gpu/drm/drm_mode_config.c +1 -3
@@ -294,13 +294,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)
include/drm/drm_crtc.h +0 -10
@@ -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.
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH 70/70] drm/crtc: Remove reset
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The return value of the newly introduced `atomic_create_state` hook is ignored in…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The return value of the newly introduced atomic_create_state hook is ignored in amdgpu_dm_crtc_init(), causing a memory leak and leaving the CRTC state uninitialized. --
--- Patch [70]: [PATCH 70/70] drm/crtc: Remove reset --- commit de86d4e5c49ea36efff6debb8f1603e3e3c819b9
Author: Maxime Ripard <[email protected]>
drm/crtc: Remove reset
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().
Quoted 5 lines
drivers/gpu/drm/drm_mode_config.c +0
-0
drivers/gpu/drm/drm_mode_config.c void drm_mode_config_reset(struct drm_device *dev) · line 301
Quoted 9 lines
drivers/gpu/drm/drm_mode_config.c +1
-3@@ -296,9 +296,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]
Does this transition leave the CRTC state uninitialized for AMD GPUs?
An earlier patch in this series switched amdgpu_dm_crtc_init() to call atomic_create_state() instead of reset(), but the returned state pointer is now ignored:
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c:amdgpu_dm_crtc_init() {
...
/* Create the plane state */
if (acrtc->base.funcs->atomic_create_state)
acrtc->base.funcs->atomic_create_state(&acrtc->base);
...
}Since the returned struct drm_crtc_state pointer is not assigned to crtc->state, and the AMDGPU driver doesn't call drm_mode_config_reset() during probe, could this lead to a memory leak and a NULL pointer dereference when the DRM core later accesses the uninitialized CRTC state?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=70
Re: [PATCH 70/70] drm/crtc: Remove reset
In reply to Maxime RipardQuoted 6 lines · Maxime Ripard
All crtc drivers now implement the
atomic_create_statehook 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.
Quoted 9 lines
Remove the reset hook from struct
drm_crtc_funcsand the associated call indrm_mode_config_reset().Signed-off-by Maxime Ripard <[email protected]>--- drivers/gpu/drm/drm_mode_config.c | 4 +--- include/drm/drm_crtc.h | 10 ---------- 2 files changed, 1 insertion(+), 13 deletions(-)
Signature · 2 lines
With best wishes Dmitry
Hi Maxime
Am 21.08.26 um 16:38 schrieb Maxime Ripard:
Quoted 17 lines
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_statein practice. Theatomic_create_statehook makes this explicit: it only allocates and initializes a pristine state without any side effect, and returns the state pointer or anERR_PTRon failure.
Quoted 8 lines
This series converts all CRTC drivers tree-wide from the reset hook to
atomic_create_state, then removes the reset hook from structdrm_crtc_funcsentirely. 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 <[email protected]>
I've gone through the series and it looks good to me; except for patch 50.
Feel free to add
to the series with the exception of patch 50.
Best regards
Thomas
Quoted 157 lines
---
Maxime Ripard (70):
drm/adp: Switch todrm_atomic_helper_crtc_create_statedrm/amdgpu: vkms: Switch todrm_atomic_helper_crtc_create_statedrm/arm: hdlcd: Switch todrm_atomic_helper_crtc_create_statedrm/armada: Switch todrm_atomic_helper_crtc_create_statedrm/exynos: Switch todrm_atomic_helper_crtc_create_statedrm/fsl-dcu: Switch todrm_atomic_helper_crtc_create_statedrm/gud: Switch todrm_atomic_helper_crtc_create_statedrm/hisilicon: hibmc: Switch todrm_atomic_helper_crtc_create_statedrm/hisilicon: kirin: Switch todrm_atomic_helper_crtc_create_statedrm/hyperv: Switch todrm_atomic_helper_crtc_create_statedrm/imx: dc: Switch todrm_atomic_helper_crtc_create_statedrm/imx: dcss: Switch todrm_atomic_helper_crtc_create_statedrm/ingenic: Switch todrm_atomic_helper_crtc_create_statedrm/kmb: Switch todrm_atomic_helper_crtc_create_statedrm/logicvc: Switch todrm_atomic_helper_crtc_create_statedrm/meson: Switch todrm_atomic_helper_crtc_create_statedrm/msm: mdp4: Switch todrm_atomic_helper_crtc_create_statedrm/mxs: mxsfb: Switch todrm_atomic_helper_crtc_create_statedrm/qxl: Switch todrm_atomic_helper_crtc_create_statedrm/renesas: shmobile: Switch todrm_atomic_helper_crtc_create_statedrm/simple-kms: Remove unusedreset_crtchook drm/simple-kms: Switch todrm_atomic_helper_crtc_create_statedrm/sitronix: st7571: Switch todrm_atomic_helper_crtc_create_statedrm/sprd: Switch todrm_atomic_helper_crtc_create_statedrm/sti: Switch todrm_atomic_helper_crtc_create_statedrm/stm: Switch todrm_atomic_helper_crtc_create_statedrm/sun4i: Switch todrm_atomic_helper_crtc_create_statedrm/tests: Switch todrm_atomic_helper_crtc_create_statedrm/tilcdc: Move hardware reset to CRTC creation drm/tilcdc: Switch todrm_atomic_helper_crtc_create_statedrm/tiny: appletbdrm: Switch todrm_atomic_helper_crtc_create_statedrm/tiny: bochs: Switch todrm_atomic_helper_crtc_create_statedrm/tiny: cirrus: Switch todrm_atomic_helper_crtc_create_statedrm/tiny: pixpaper: Switch todrm_atomic_helper_crtc_create_statedrm/tiny: sharp: Switch todrm_atomic_helper_crtc_create_statedrm/udl: Switch todrm_atomic_helper_crtc_create_statedrm/vbox: Switch todrm_atomic_helper_crtc_create_statedrm/verisilicon: Switch todrm_atomic_helper_crtc_create_statedrm/virtio: Switch todrm_atomic_helper_crtc_create_statedrm/xlnx: Switch todrm_atomic_helper_crtc_create_statedrm/mipi-dbi: Switch todrm_atomic_helper_crtc_create_statedrm/atomic-helper: Removedrm_atomic_helper_crtc_reset
sysfb: Convert tocreate_state
drm/amdgpu: dm: Convert toatomic_create_statedrm/komeda: Convert toatomic_create_state
drm/malidp: Convert toatomic_create_state
drm/ast: Convert toatomic_create_state
drm/atmel-hlcdc: Convert toatomic_create_state
drm/imx: ipuv3: Convert toatomic_create_statedrm/loongsoon: Move hardware reset to CRTC creation drm/loongson: Convert toatomic_create_statedrm/mediatek: Convert toatomic_create_statedrm/mgag200: Convert toatomic_create_state
drm/msm: dpu1: Convert toatomic_create_state
drm/msm: mdp5: Convert toatomic_create_state
drm/mxsfb: lcdif: Convert toatomic_create_statedrm/nouveau: Convert toatomic_create_state
drm/omapdrm: Convert toatomic_create_state
drm/rcar-du: Convert toatomic_create_statedrm/rzg2l-du: Convert toatomic_create_statedrm/rockchip: vop: Convert toatomic_create_statedrm/rockchip: vop2: Convert toatomic_create_statedrm/sitronix: st7920: Convert toatomic_create_statedrm/solomon: ssd130x: Convert toatomic_create_statedrm/tegra: Convert toatomic_create_state
drm/vc4: Convert toatomic_create_state
drm/vkms: Convert toatomic_create_state
drm/vmwgfx: Convert toatomic_create_statedrm/atomic-helper: Remove__drm_atomic_helper_crtc_resetdrm/crtc: Remove resetdrivers/gpu/drm/adp/adp_drv.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 2 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 17 ++++---- drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 18 ++++---- drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +- drivers/gpu/drm/arm/malidp_crtc.c | 18 ++++---- drivers/gpu/drm/armada/armada_crtc.c | 2 +- drivers/gpu/drm/ast/ast_mode.c | 18 ++++---- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 19 ++++----- drivers/gpu/drm/drm_atomic_state_helper.c | 45 -------------------- drivers/gpu/drm/drm_mode_config.c | 4 +- drivers/gpu/drm/drm_simple_kms_helper.c | 13 +----- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 2 +- drivers/gpu/drm/gud/gud_drv.c | 2 +- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 2 +- drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +- drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 2 +- drivers/gpu/drm/imx/dc/dc-crtc.c | 2 +- drivers/gpu/drm/imx/dcss/dcss-crtc.c | 2 +- drivers/gpu/drm/imx/ipuv3/ipuv3-crtc.c | 18 ++++---- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +- drivers/gpu/drm/kmb/kmb_crtc.c | 2 +- drivers/gpu/drm/logicvc/logicvc_crtc.c | 2 +- drivers/gpu/drm/loongson/lsdc_crtc.c | 26 ++++++------ drivers/gpu/drm/mediatek/mtk_crtc.c | 18 ++++---- drivers/gpu/drm/meson/meson_crtc.c | 2 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 4 +- drivers/gpu/drm/mgag200/mgag200_mode.c | 15 ++++--- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 18 ++++---- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 2 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 20 ++++----- drivers/gpu/drm/mxsfb/lcdif_kms.c | 17 ++++---- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 4 +- drivers/gpu/drm/nouveau/dispnv50/head.c | 14 +++---- drivers/gpu/drm/omapdrm/omap_crtc.c | 17 ++++---- drivers/gpu/drm/qxl/qxl_display.c | 2 +- drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c | 17 ++++---- drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c | 15 +++---- drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 2 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 18 ++++---- drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 18 ++++---- drivers/gpu/drm/sitronix/st7571.c | 2 +- drivers/gpu/drm/sitronix/st7920.c | 10 +++-- drivers/gpu/drm/solomon/ssd130x.c | 10 +++-- drivers/gpu/drm/sprd/sprd_dpu.c | 2 +- drivers/gpu/drm/sti/sti_crtc.c | 2 +- drivers/gpu/drm/stm/ltdc.c | 4 +- drivers/gpu/drm/sun4i/sun4i_crtc.c | 2 +- drivers/gpu/drm/sysfb/drm_sysfb_helper.h | 4 +- drivers/gpu/drm/sysfb/drm_sysfb_modeset.c | 20 ++++----- drivers/gpu/drm/tegra/dc.c | 18 ++++---- drivers/gpu/drm/tests/drm_kunit_helpers.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 49 +++++++++------------- drivers/gpu/drm/tiny/appletbdrm.c | 2 +- drivers/gpu/drm/tiny/bochs.c | 2 +- drivers/gpu/drm/tiny/cirrus-qemu.c | 2 +- drivers/gpu/drm/tiny/pixpaper.c | 2 +- drivers/gpu/drm/tiny/sharp-memory.c | 2 +- drivers/gpu/drm/udl/udl_modeset.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_mode.c | 2 +- drivers/gpu/drm/vc4/tests/vc4_mock_crtc.c | 2 +- drivers/gpu/drm/vc4/vc4_crtc.c | 14 +++---- drivers/gpu/drm/vc4/vc4_drv.h | 2 +- drivers/gpu/drm/vc4/vc4_txp.c | 2 +- drivers/gpu/drm/verisilicon/vs_crtc.c | 2 +- drivers/gpu/drm/virtio/virtgpu_display.c | 2 +- drivers/gpu/drm/vkms/vkms_crtc.c | 18 ++++---- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 ++++------ drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 +- drivers/gpu/drm/xlnx/zynqmp_kms.c | 2 +- include/drm/drm_atomic_state_helper.h | 3 -- include/drm/drm_crtc.h | 12 ------ include/drm/drm_mipi_dbi.h | 2 +- include/drm/drm_simple_kms_helper.h | 1 - 78 files changed, 275 insertions(+), 385 deletions(-)---
base-commit:7ea58b9ddb52264a4ed36bccb975062ada32c62d
change-id: 20260709-drm-no-more-crtc-reset-14a4d5bc8d41Best regards,
Signature · 6 lines
-- 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)
Quoted 17 lines · 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_statein practice. Theatomic_create_statehook makes this explicit: it only allocates and initializes a pristine state without any side effect, and returns the state pointer or anERR_PTRon failure.
Quoted 8 lines
This series converts all CRTC drivers tree-wide from the reset hook to
atomic_create_state, then removes the reset hook from structdrm_crtc_funcsentirely. 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 <[email protected]>
Applied patches 1,3-14,16-28,31-41,43,45-49,52-62,64-68 to drm-misc-next
Maxime