[2/2] drm/atomic: Create and use __drm_atomic_helper_crtc_reset() everywhere

Commit Message

Maarten Lankhorst Nov. 12, 2018, 3:01 p.m. UTC

We already have __drm_atomic_helper_connector_reset() and
__drm_atomic_helper_plane_reset(), extend this to crtc as well.

Most drivers already have a gpu reset hook, correct it.
Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
convert it to the common one.

Signed-off-by: Maarten Lankhorst <[e-mail]>
Cc: Harry Wentland <[e-mail]>
Cc: Leo Li <[e-mail]>
Cc: Alex Deucher <[e-mail]>
Cc: "Christian König" <[e-mail]>
Cc: "David (ChunMing) Zhou" <[e-mail]>
Cc: David Airlie <[e-mail]>
Cc: Liviu Dudau <[e-mail]>
Cc: Brian Starkey <[e-mail]>
Cc: Mali DP Maintainers <[e-mail]>
Cc: Boris Brezillon <[e-mail]>
Cc: Nicolas Ferre <[e-mail]>
Cc: Alexandre Belloni <[e-mail]>
Cc: Ludovic Desroches <[e-mail]>
Cc: Maarten Lankhorst <[e-mail]>
Cc: Maxime Ripard <[e-mail]>
Cc: Sean Paul <[e-mail]>
Cc: Jani Nikula <[e-mail]>
Cc: Joonas Lahtinen <[e-mail]>
Cc: Rodrigo Vivi <[e-mail]>
Cc: Philipp Zabel <[e-mail]>
Cc: CK Hu <[e-mail]>
Cc: Matthias Brugger <[e-mail]>
Cc: Rob Clark <[e-mail]>
Cc: Ben Skeggs <[e-mail]>
Cc: Tomi Valkeinen <[e-mail]>
Cc: Laurent Pinchart <[e-mail]>
Cc: Kieran Bingham <[e-mail]>
Cc: Sandy Huang <[e-mail]>
Cc: "Heiko Stübner" <[e-mail]>
Cc: Thierry Reding <[e-mail]>
Cc: Jonathan Hunter <[e-mail]>
Cc: Eric Anholt <[e-mail]>
Cc: VMware Graphics <[e-mail]>
Cc: Sinclair Yeh <[e-mail]>
Cc: Thomas Hellstrom <[e-mail]>
Cc: Tony Cheng <[e-mail]>
Cc: Shirish S <[e-mail]>
Cc: Mikita Lipski <[e-mail]>
Cc: Bhawanpreet Lakha <[e-mail]>
Cc: David Francis <[e-mail]>
Cc: Anthony Koo <[e-mail]>
Cc: Jeykumar Sankaran <[e-mail]>
Cc: Jordan Crouse <[e-mail]>
Cc: Bruce Wang <[e-mail]>
Cc: Sravanthi Kollukuduru <[e-mail]>
Cc: Archit Taneja <[e-mail]>
Cc: Steve Kowalik <[e-mail]>
Cc: Carsten Behling <[e-mail]>
Cc: Haneen Mohammed <[e-mail]>
Cc: Daniel Vetter <[e-mail]>
Cc: Rodrigo Siqueira <[e-mail]>
Cc: Mahesh Kumar <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
 drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
 drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
 drivers/gpu/drm/i915/intel_display.c          |  2 +-
 drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
 drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
 drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
 drivers/gpu/drm/tegra/dc.c                    |  5 +--
 drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
 drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
 include/drm/drm_atomic_state_helper.h         |  2 ++
 18 files changed, 56 insertions(+), 81 deletions(-)

Comments

Boris Brezillon Nov. 12, 2018, 3:11 p.m. UTC | #1

On Mon, 12 Nov 2018 16:01:14 +0100
Maarten Lankhorst <[e-mail]> wrote:

> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
> Cc: Harry Wentland <[e-mail]>
> Cc: Leo Li <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: "David (ChunMing) Zhou" <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Liviu Dudau <[e-mail]>
> Cc: Brian Starkey <[e-mail]>
> Cc: Mali DP Maintainers <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Nicolas Ferre <[e-mail]>
> Cc: Alexandre Belloni <[e-mail]>
> Cc: Ludovic Desroches <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: Philipp Zabel <[e-mail]>
> Cc: CK Hu <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> Cc: Rob Clark <[e-mail]>
> Cc: Ben Skeggs <[e-mail]>
> Cc: Tomi Valkeinen <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kieran Bingham <[e-mail]>
> Cc: Sandy Huang <[e-mail]>
> Cc: "Heiko Stübner" <[e-mail]>
> Cc: Thierry Reding <[e-mail]>
> Cc: Jonathan Hunter <[e-mail]>
> Cc: Eric Anholt <[e-mail]>
> Cc: VMware Graphics <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Tony Cheng <[e-mail]>
> Cc: Shirish S <[e-mail]>
> Cc: Mikita Lipski <[e-mail]>
> Cc: Bhawanpreet Lakha <[e-mail]>
> Cc: David Francis <[e-mail]>
> Cc: Anthony Koo <[e-mail]>
> Cc: Jeykumar Sankaran <[e-mail]>
> Cc: Jordan Crouse <[e-mail]>
> Cc: Bruce Wang <[e-mail]>
> Cc: Sravanthi Kollukuduru <[e-mail]>
> Cc: Archit Taneja <[e-mail]>
> Cc: Steve Kowalik <[e-mail]>
> Cc: Carsten Behling <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Rodrigo Siqueira <[e-mail]>
> Cc: Mahesh Kumar <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>  drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--

For the atmel-hlcdc driver

Acked-by: Boris Brezillon <[e-mail]>

Leo Li Nov. 12, 2018, 3:23 p.m. UTC | #2

On 2018-11-12 10:01 AM, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
> Cc: Harry Wentland <[e-mail]>
> Cc: Leo Li <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: "David (ChunMing) Zhou" <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Liviu Dudau <[e-mail]>
> Cc: Brian Starkey <[e-mail]>
> Cc: Mali DP Maintainers <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Nicolas Ferre <[e-mail]>
> Cc: Alexandre Belloni <[e-mail]>
> Cc: Ludovic Desroches <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: Philipp Zabel <[e-mail]>
> Cc: CK Hu <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> Cc: Rob Clark <[e-mail]>
> Cc: Ben Skeggs <[e-mail]>
> Cc: Tomi Valkeinen <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kieran Bingham <[e-mail]>
> Cc: Sandy Huang <[e-mail]>
> Cc: "Heiko Stübner" <[e-mail]>
> Cc: Thierry Reding <[e-mail]>
> Cc: Jonathan Hunter <[e-mail]>
> Cc: Eric Anholt <[e-mail]>
> Cc: VMware Graphics <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Tony Cheng <[e-mail]>
> Cc: Shirish S <[e-mail]>
> Cc: Mikita Lipski <[e-mail]>
> Cc: Bhawanpreet Lakha <[e-mail]>
> Cc: David Francis <[e-mail]>
> Cc: Anthony Koo <[e-mail]>
> Cc: Jeykumar Sankaran <[e-mail]>
> Cc: Jordan Crouse <[e-mail]>
> Cc: Bruce Wang <[e-mail]>
> Cc: Sravanthi Kollukuduru <[e-mail]>
> Cc: Archit Taneja <[e-mail]>
> Cc: Steve Kowalik <[e-mail]>
> Cc: Carsten Behling <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Rodrigo Siqueira <[e-mail]>
> Cc: Mahesh Kumar <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>   drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>   .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
>   drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
>   drivers/gpu/drm/i915/intel_display.c          |  2 +-
>   drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
>   drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
>   drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
>   drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
>   drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>   drivers/gpu/drm/tegra/dc.c                    |  5 +--
>   drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
>   drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
>   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
>   include/drm/drm_atomic_state_helper.h         |  2 ++
>   18 files changed, 56 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5064768642f3..770a71726cd1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2802,9 +2802,7 @@ static void dm_crtc_reset_state(struct drm_crtc *crtc)
>   	if (WARN_ON(!state))
>   		return;
>   
> -	crtc->state = &state->base;
> -	crtc->state->crtc = crtc;
> -
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>   }
>   
>   static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
> index e1b72782848c..9a924ff27148 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -474,10 +474,7 @@ static void malidp_crtc_reset(struct drm_crtc *crtc)
>   
>   	kfree(state);
>   	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>   }
>   
>   static void malidp_crtc_destroy_state(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 96f4082671fe..8084d549c7d1 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -412,10 +412,7 @@ static void atmel_hlcdc_crtc_reset(struct drm_crtc *crtc)
>   	}
>   
>   	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>   }
>   
>   static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 3ba996069d69..3eee1ca33fc5 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -55,6 +55,29 @@
>    * for these functions.
>    */
>   
> +/**
> + * __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_conector->state pointer of @crtc, usually required when
            ^ Do you mean drm_crtc->state?

For amdgpu_dm,
Acked-by: Leo Li <[e-mail]>

Leo

> + * 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)
> +		crtc_state->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
> @@ -64,14 +87,14 @@
>    */
>   void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
>   {
> +	struct drm_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> +
>   	if (crtc->state)
>   		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>   
>   	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
>   }
>   EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
>   
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f383417571ec..907ffeb64781 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15457,7 +15457,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>   
>   		__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
>   		memset(crtc_state, 0, sizeof(*crtc_state));
> -		crtc_state->base.crtc = &crtc->base;
> +		__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->base);
>   
>   		crtc_state->base.active = crtc_state->base.enable =
>   			dev_priv->display.get_pipe_config(crtc, crtc_state);
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 7d4b710b837a..8bc36f0d2b6b 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -120,12 +120,9 @@ static void imx_drm_crtc_reset(struct drm_crtc *crtc)
>   		memset(state, 0, sizeof(*state));
>   	} else {
>   		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>   	}
>   
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>   }
>   
>   static struct drm_crtc_state *imx_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 92ecb9bf982c..a743e5ed1177 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -119,12 +119,9 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
>   		memset(state, 0, sizeof(*state));
>   	} else {
>   		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>   	}
>   
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>   }
>   
>   static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4530d60767b..c86e603f486a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -894,20 +894,12 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
>   		_dpu_crtc_set_suspend(crtc, false);
>   
>   	/* remove previous state, if present */
> -	if (crtc->state) {
> +	if (crtc->state)
>   		dpu_crtc_destroy_state(crtc, crtc->state);
> -		crtc->state = 0;
> -	}
>   
>   	dpu_crtc = to_dpu_crtc(crtc);
>   	cstate = kzalloc(sizeof(*cstate), GFP_KERNEL);
> -	if (!cstate) {
> -		DPU_ERROR("failed to allocate state\n");
> -		return;
> -	}
> -
> -	cstate->base.crtc = crtc;
> -	crtc->state = &cstate->base;
> +	__drm_atomic_helper_crtc_reset(crtc, &cstate->base);
>   }
>   
>   static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index b1da9ce54379..138da66b1530 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -1019,11 +1019,7 @@ static void mdp5_crtc_reset(struct drm_crtc *crtc)
>   	}
>   
>   	mdp5_cstate = kzalloc(sizeof(*mdp5_cstate), GFP_KERNEL);
> -
> -	if (mdp5_cstate) {
> -		mdp5_cstate->base.crtc = crtc;
> -		crtc->state = &mdp5_cstate->base;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &mdp5_cstate->base);
>   }
>   
>   static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 4f57e5379796..341600406589 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -418,16 +418,6 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc)
>   	return &asyh->state;
>   }
>   
> -static void
> -__drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
> -			       struct drm_crtc_state *state)
> -{
> -	if (crtc->state)
> -		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> -	crtc->state = state;
> -	crtc->state->crtc = crtc;
> -}
> -
>   static void
>   nv50_head_reset(struct drm_crtc *crtc)
>   {
> @@ -436,6 +426,9 @@ nv50_head_reset(struct drm_crtc *crtc)
>   	if (WARN_ON(!(asyh = kzalloc(sizeof(*asyh), GFP_KERNEL))))
>   		return;
>   
> +	if (crtc->state)
> +		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> +
>   	__drm_atomic_helper_crtc_reset(crtc, &asyh->state);
>   }
>   
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index caffc547ef97..9c5f8109913c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -554,14 +554,13 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
>   
>   static void omap_crtc_reset(struct drm_crtc *crtc)
>   {
> +	struct omap_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>   	if (crtc->state)
>   		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>   
>   	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(struct omap_crtc_state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>   }
>   
>   static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 17741843cf51..7beab22b4674 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -857,14 +857,12 @@ static void rcar_du_crtc_reset(struct drm_crtc *crtc)
>   	}
>   
>   	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	__drm_atomic_helper_crtc_reset(crtc, &state->state);
>   	if (state == NULL)
>   		return;
>   
>   	state->crc.source = VSP1_DU_CRC_NONE;
>   	state->crc.index = 0;
> -
> -	crtc->state = &state->state;
> -	crtc->state->crtc = crtc;
>   }
>   
>   static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..d2d5aa676084 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1071,13 +1071,14 @@ static void vop_crtc_destroy(struct drm_crtc *crtc)
>   
>   static void vop_crtc_reset(struct drm_crtc *crtc)
>   {
> +	struct rockchip_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> +
>   	if (crtc->state)
>   		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>   	kfree(crtc->state);
>   
> -	crtc->state = kzalloc(sizeof(struct rockchip_crtc_state), GFP_KERNEL);
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>   }
>   
>   static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index f80e82e16475..140d6191d8d2 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1164,10 +1164,7 @@ static void tegra_crtc_reset(struct drm_crtc *crtc)
>   	crtc->state = NULL;
>   
>   	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>   
>   	drm_crtc_vblank_reset(crtc);
>   }
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index cd226e63d557..76beeda157fc 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -998,12 +998,12 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
>   static void
>   vc4_crtc_reset(struct drm_crtc *crtc)
>   {
> -	if (crtc->state)
> -		vc4_crtc_destroy_state(crtc->state);
> +	struct vc4_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>   
> -	crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
>   	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +		vc4_crtc_destroy_state(crtc, crtc->state);
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>   }
>   
>   static const struct drm_crtc_funcs vc4_crtc_funcs = {
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 177bbcb38306..7b018f6ee971 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -98,15 +98,10 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
>   		vkms_state = to_vkms_crtc_state(crtc->state);
>   		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>   		kfree(vkms_state);
> -		crtc->state = NULL;
>   	}
>   
>   	vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL);
> -	if (!vkms_state)
> -		return;
> -
> -	crtc->state = &vkms_state->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vkms_state->base);
>   }
>   
>   static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e6b11f6ae2e4..2878bc0a4718 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -633,14 +633,7 @@ void vmw_du_crtc_reset(struct drm_crtc *crtc)
>   	}
>   
>   	vcs = kzalloc(sizeof(*vcs), GFP_KERNEL);
> -
> -	if (!vcs) {
> -		DRM_ERROR("Cannot allocate vmw_crtc_state\n");
> -		return;
> -	}
> -
> -	crtc->state = &vcs->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vcs->base);
>   }
>   
>   
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index 5b82ccfdb502..ab2b5a49948c 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -37,6 +37,8 @@ struct drm_private_state;
>   struct drm_modeset_acquire_ctx;
>   struct drm_device;
>   
> +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);
>   void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>   					      struct drm_crtc_state *state);
>

Heiko Stuebner Nov. 12, 2018, 3:36 p.m. UTC | #3

Am Montag, 12. November 2018, 16:01:14 CET schrieb Maarten Lankhorst:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>


> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..d2d5aa676084 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1071,13 +1071,14 @@ static void vop_crtc_destroy(struct drm_crtc *crtc)
>  
>  static void vop_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct rockchip_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  	kfree(crtc->state);
>  
> -	crtc->state = kzalloc(sizeof(struct rockchip_crtc_state), GFP_KERNEL);
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)

for the Rockchip-part
Acked-by: Heiko Stuebner <[e-mail]>

Thierry Reding Nov. 12, 2018, 3:36 p.m. UTC | #4

On Mon, Nov 12, 2018 at 04:01:14PM +0100, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
> Cc: Harry Wentland <[e-mail]>
> Cc: Leo Li <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: "David (ChunMing) Zhou" <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Liviu Dudau <[e-mail]>
> Cc: Brian Starkey <[e-mail]>
> Cc: Mali DP Maintainers <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Nicolas Ferre <[e-mail]>
> Cc: Alexandre Belloni <[e-mail]>
> Cc: Ludovic Desroches <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: Philipp Zabel <[e-mail]>
> Cc: CK Hu <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> Cc: Rob Clark <[e-mail]>
> Cc: Ben Skeggs <[e-mail]>
> Cc: Tomi Valkeinen <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kieran Bingham <[e-mail]>
> Cc: Sandy Huang <[e-mail]>
> Cc: "Heiko Stübner" <[e-mail]>
> Cc: Thierry Reding <[e-mail]>
> Cc: Jonathan Hunter <[e-mail]>
> Cc: Eric Anholt <[e-mail]>
> Cc: VMware Graphics <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Tony Cheng <[e-mail]>
> Cc: Shirish S <[e-mail]>
> Cc: Mikita Lipski <[e-mail]>
> Cc: Bhawanpreet Lakha <[e-mail]>
> Cc: David Francis <[e-mail]>
> Cc: Anthony Koo <[e-mail]>
> Cc: Jeykumar Sankaran <[e-mail]>
> Cc: Jordan Crouse <[e-mail]>
> Cc: Bruce Wang <[e-mail]>
> Cc: Sravanthi Kollukuduru <[e-mail]>
> Cc: Archit Taneja <[e-mail]>
> Cc: Steve Kowalik <[e-mail]>
> Cc: Carsten Behling <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Rodrigo Siqueira <[e-mail]>
> Cc: Mahesh Kumar <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>  drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
>  drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
>  drivers/gpu/drm/i915/intel_display.c          |  2 +-
>  drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
>  drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
>  drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>  drivers/gpu/drm/tegra/dc.c                    |  5 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
>  drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
>  include/drm/drm_atomic_state_helper.h         |  2 ++
>  18 files changed, 56 insertions(+), 81 deletions(-)

Looks good to me:

Acked-by: Thierry Reding <[e-mail]>

Harry Wentland Nov. 12, 2018, 4:02 p.m. UTC | #5

On 2018-11-12 10:01 a.m., Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
> Cc: Harry Wentland <[e-mail]>
> Cc: Leo Li <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: "David (ChunMing) Zhou" <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Liviu Dudau <[e-mail]>
> Cc: Brian Starkey <[e-mail]>
> Cc: Mali DP Maintainers <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Nicolas Ferre <[e-mail]>
> Cc: Alexandre Belloni <[e-mail]>
> Cc: Ludovic Desroches <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: Philipp Zabel <[e-mail]>
> Cc: CK Hu <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> Cc: Rob Clark <[e-mail]>
> Cc: Ben Skeggs <[e-mail]>
> Cc: Tomi Valkeinen <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kieran Bingham <[e-mail]>
> Cc: Sandy Huang <[e-mail]>
> Cc: "Heiko Stübner" <[e-mail]>
> Cc: Thierry Reding <[e-mail]>
> Cc: Jonathan Hunter <[e-mail]>
> Cc: Eric Anholt <[e-mail]>
> Cc: VMware Graphics <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Tony Cheng <[e-mail]>
> Cc: Shirish S <[e-mail]>
> Cc: Mikita Lipski <[e-mail]>
> Cc: Bhawanpreet Lakha <[e-mail]>
> Cc: David Francis <[e-mail]>
> Cc: Anthony Koo <[e-mail]>
> Cc: Jeykumar Sankaran <[e-mail]>
> Cc: Jordan Crouse <[e-mail]>
> Cc: Bruce Wang <[e-mail]>
> Cc: Sravanthi Kollukuduru <[e-mail]>
> Cc: Archit Taneja <[e-mail]>
> Cc: Steve Kowalik <[e-mail]>
> Cc: Carsten Behling <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Rodrigo Siqueira <[e-mail]>
> Cc: Mahesh Kumar <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]

For amdgpu_dm and core changes
Reviewed-by: Harry Wentland <[e-mail]>

Harry

> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>  drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
>  drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
>  drivers/gpu/drm/i915/intel_display.c          |  2 +-
>  drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
>  drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
>  drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>  drivers/gpu/drm/tegra/dc.c                    |  5 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
>  drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
>  include/drm/drm_atomic_state_helper.h         |  2 ++
>  18 files changed, 56 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5064768642f3..770a71726cd1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2802,9 +2802,7 @@ static void dm_crtc_reset_state(struct drm_crtc *crtc)
>  	if (WARN_ON(!state))
>  		return;
>  
> -	crtc->state = &state->base;
> -	crtc->state->crtc = crtc;
> -
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
> index e1b72782848c..9a924ff27148 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -474,10 +474,7 @@ static void malidp_crtc_reset(struct drm_crtc *crtc)
>  
>  	kfree(state);
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static void malidp_crtc_destroy_state(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 96f4082671fe..8084d549c7d1 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -412,10 +412,7 @@ static void atmel_hlcdc_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 3ba996069d69..3eee1ca33fc5 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -55,6 +55,29 @@
>   * for these functions.
>   */
>  
> +/**
> + * __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_conector->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)
> +		crtc_state->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
> @@ -64,14 +87,14 @@
>   */
>  void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct drm_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f383417571ec..907ffeb64781 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15457,7 +15457,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
>  		memset(crtc_state, 0, sizeof(*crtc_state));
> -		crtc_state->base.crtc = &crtc->base;
> +		__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->base);
>  
>  		crtc_state->base.active = crtc_state->base.enable =
>  			dev_priv->display.get_pipe_config(crtc, crtc_state);
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 7d4b710b837a..8bc36f0d2b6b 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -120,12 +120,9 @@ static void imx_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *imx_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 92ecb9bf982c..a743e5ed1177 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -119,12 +119,9 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4530d60767b..c86e603f486a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -894,20 +894,12 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
>  		_dpu_crtc_set_suspend(crtc, false);
>  
>  	/* remove previous state, if present */
> -	if (crtc->state) {
> +	if (crtc->state)
>  		dpu_crtc_destroy_state(crtc, crtc->state);
> -		crtc->state = 0;
> -	}
>  
>  	dpu_crtc = to_dpu_crtc(crtc);
>  	cstate = kzalloc(sizeof(*cstate), GFP_KERNEL);
> -	if (!cstate) {
> -		DPU_ERROR("failed to allocate state\n");
> -		return;
> -	}
> -
> -	cstate->base.crtc = crtc;
> -	crtc->state = &cstate->base;
> +	__drm_atomic_helper_crtc_reset(crtc, &cstate->base);
>  }
>  
>  static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index b1da9ce54379..138da66b1530 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -1019,11 +1019,7 @@ static void mdp5_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	mdp5_cstate = kzalloc(sizeof(*mdp5_cstate), GFP_KERNEL);
> -
> -	if (mdp5_cstate) {
> -		mdp5_cstate->base.crtc = crtc;
> -		crtc->state = &mdp5_cstate->base;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &mdp5_cstate->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 4f57e5379796..341600406589 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -418,16 +418,6 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc)
>  	return &asyh->state;
>  }
>  
> -static void
> -__drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
> -			       struct drm_crtc_state *state)
> -{
> -	if (crtc->state)
> -		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> -	crtc->state = state;
> -	crtc->state->crtc = crtc;
> -}
> -
>  static void
>  nv50_head_reset(struct drm_crtc *crtc)
>  {
> @@ -436,6 +426,9 @@ nv50_head_reset(struct drm_crtc *crtc)
>  	if (WARN_ON(!(asyh = kzalloc(sizeof(*asyh), GFP_KERNEL))))
>  		return;
>  
> +	if (crtc->state)
> +		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> +
>  	__drm_atomic_helper_crtc_reset(crtc, &asyh->state);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index caffc547ef97..9c5f8109913c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -554,14 +554,13 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
>  
>  static void omap_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct omap_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(struct omap_crtc_state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 17741843cf51..7beab22b4674 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -857,14 +857,12 @@ static void rcar_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	__drm_atomic_helper_crtc_reset(crtc, &state->state);
>  	if (state == NULL)
>  		return;
>  
>  	state->crc.source = VSP1_DU_CRC_NONE;
>  	state->crc.index = 0;
> -
> -	crtc->state = &state->state;
> -	crtc->state->crtc = crtc;
>  }
>  
>  static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..d2d5aa676084 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1071,13 +1071,14 @@ static void vop_crtc_destroy(struct drm_crtc *crtc)
>  
>  static void vop_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct rockchip_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  	kfree(crtc->state);
>  
> -	crtc->state = kzalloc(sizeof(struct rockchip_crtc_state), GFP_KERNEL);
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index f80e82e16475..140d6191d8d2 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1164,10 +1164,7 @@ static void tegra_crtc_reset(struct drm_crtc *crtc)
>  	crtc->state = NULL;
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  
>  	drm_crtc_vblank_reset(crtc);
>  }
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index cd226e63d557..76beeda157fc 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -998,12 +998,12 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
>  static void
>  vc4_crtc_reset(struct drm_crtc *crtc)
>  {
> -	if (crtc->state)
> -		vc4_crtc_destroy_state(crtc->state);
> +	struct vc4_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  
> -	crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
>  	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +		vc4_crtc_destroy_state(crtc, crtc->state);
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static const struct drm_crtc_funcs vc4_crtc_funcs = {
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 177bbcb38306..7b018f6ee971 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -98,15 +98,10 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
>  		vkms_state = to_vkms_crtc_state(crtc->state);
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  		kfree(vkms_state);
> -		crtc->state = NULL;
>  	}
>  
>  	vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL);
> -	if (!vkms_state)
> -		return;
> -
> -	crtc->state = &vkms_state->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vkms_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e6b11f6ae2e4..2878bc0a4718 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -633,14 +633,7 @@ void vmw_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	vcs = kzalloc(sizeof(*vcs), GFP_KERNEL);
> -
> -	if (!vcs) {
> -		DRM_ERROR("Cannot allocate vmw_crtc_state\n");
> -		return;
> -	}
> -
> -	crtc->state = &vcs->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vcs->base);
>  }
>  
>  
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index 5b82ccfdb502..ab2b5a49948c 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -37,6 +37,8 @@ struct drm_private_state;
>  struct drm_modeset_acquire_ctx;
>  struct drm_device;
>  
> +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);
>  void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  					      struct drm_crtc_state *state);
>

Sean Paul Nov. 12, 2018, 4:11 p.m. UTC | #6

On Mon, Nov 12, 2018 at 04:01:14PM +0100, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
> Cc: Harry Wentland <[e-mail]>
> Cc: Leo Li <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: "David (ChunMing) Zhou" <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Liviu Dudau <[e-mail]>
> Cc: Brian Starkey <[e-mail]>
> Cc: Mali DP Maintainers <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Nicolas Ferre <[e-mail]>
> Cc: Alexandre Belloni <[e-mail]>
> Cc: Ludovic Desroches <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: Philipp Zabel <[e-mail]>
> Cc: CK Hu <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> Cc: Rob Clark <[e-mail]>
> Cc: Ben Skeggs <[e-mail]>
> Cc: Tomi Valkeinen <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kieran Bingham <[e-mail]>
> Cc: Sandy Huang <[e-mail]>
> Cc: "Heiko Stübner" <[e-mail]>
> Cc: Thierry Reding <[e-mail]>
> Cc: Jonathan Hunter <[e-mail]>
> Cc: Eric Anholt <[e-mail]>
> Cc: VMware Graphics <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Tony Cheng <[e-mail]>
> Cc: Shirish S <[e-mail]>
> Cc: Mikita Lipski <[e-mail]>
> Cc: Bhawanpreet Lakha <[e-mail]>
> Cc: David Francis <[e-mail]>
> Cc: Anthony Koo <[e-mail]>
> Cc: Jeykumar Sankaran <[e-mail]>
> Cc: Jordan Crouse <[e-mail]>
> Cc: Bruce Wang <[e-mail]>
> Cc: Sravanthi Kollukuduru <[e-mail]>
> Cc: Archit Taneja <[e-mail]>
> Cc: Steve Kowalik <[e-mail]>
> Cc: Carsten Behling <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Rodrigo Siqueira <[e-mail]>
> Cc: Mahesh Kumar <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>  drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
>  drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
>  drivers/gpu/drm/i915/intel_display.c          |  2 +-
>  drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
>  drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
>  drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>  drivers/gpu/drm/tegra/dc.c                    |  5 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
>  drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
>  include/drm/drm_atomic_state_helper.h         |  2 ++
>  18 files changed, 56 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5064768642f3..770a71726cd1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2802,9 +2802,7 @@ static void dm_crtc_reset_state(struct drm_crtc *crtc)
>  	if (WARN_ON(!state))

Can you give this the same treatment as the other allocation checks?

>  		return;
>  
> -	crtc->state = &state->base;
> -	crtc->state->crtc = crtc;
> -
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
> index e1b72782848c..9a924ff27148 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -474,10 +474,7 @@ static void malidp_crtc_reset(struct drm_crtc *crtc)
>  
>  	kfree(state);
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}

You're changing behavior slightly here. If the allocation fails in the old code,
you just continue on (and presumably use-after-free on the next crtc->state
access). Whereas now you're going to just deref NULL. Neither one are really
desireable :)

So you probably want to continue checking the allocation and clear crtc->state
if it fails. After that you can call helper_crtc_reset with &state->base.

> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static void malidp_crtc_destroy_state(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 96f4082671fe..8084d549c7d1 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -412,10 +412,7 @@ static void atmel_hlcdc_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 3ba996069d69..3eee1ca33fc5 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -55,6 +55,29 @@
>   * for these functions.
>   */
>  
> +/**
> + * __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_conector->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)
> +		crtc_state->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
> @@ -64,14 +87,14 @@
>   */
>  void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct drm_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f383417571ec..907ffeb64781 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15457,7 +15457,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
>  		memset(crtc_state, 0, sizeof(*crtc_state));
> -		crtc_state->base.crtc = &crtc->base;
> +		__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->base);
>  
>  		crtc_state->base.active = crtc_state->base.enable =
>  			dev_priv->display.get_pipe_config(crtc, crtc_state);
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 7d4b710b837a..8bc36f0d2b6b 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -120,12 +120,9 @@ static void imx_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *imx_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 92ecb9bf982c..a743e5ed1177 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -119,12 +119,9 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4530d60767b..c86e603f486a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -894,20 +894,12 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
>  		_dpu_crtc_set_suspend(crtc, false);
>  
>  	/* remove previous state, if present */
> -	if (crtc->state) {
> +	if (crtc->state)
>  		dpu_crtc_destroy_state(crtc, crtc->state);
> -		crtc->state = 0;
> -	}
>  
>  	dpu_crtc = to_dpu_crtc(crtc);
>  	cstate = kzalloc(sizeof(*cstate), GFP_KERNEL);
> -	if (!cstate) {
> -		DPU_ERROR("failed to allocate state\n");
> -		return;
> -	}
> -
> -	cstate->base.crtc = crtc;
> -	crtc->state = &cstate->base;
> +	__drm_atomic_helper_crtc_reset(crtc, &cstate->base);
>  }
>  
>  static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index b1da9ce54379..138da66b1530 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -1019,11 +1019,7 @@ static void mdp5_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	mdp5_cstate = kzalloc(sizeof(*mdp5_cstate), GFP_KERNEL);
> -
> -	if (mdp5_cstate) {
> -		mdp5_cstate->base.crtc = crtc;
> -		crtc->state = &mdp5_cstate->base;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &mdp5_cstate->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 4f57e5379796..341600406589 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -418,16 +418,6 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc)
>  	return &asyh->state;
>  }
>  
> -static void
> -__drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
> -			       struct drm_crtc_state *state)
> -{
> -	if (crtc->state)
> -		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> -	crtc->state = state;
> -	crtc->state->crtc = crtc;
> -}
> -
>  static void
>  nv50_head_reset(struct drm_crtc *crtc)
>  {
> @@ -436,6 +426,9 @@ nv50_head_reset(struct drm_crtc *crtc)
>  	if (WARN_ON(!(asyh = kzalloc(sizeof(*asyh), GFP_KERNEL))))
>  		return;
>  
> +	if (crtc->state)
> +		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> +
>  	__drm_atomic_helper_crtc_reset(crtc, &asyh->state);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index caffc547ef97..9c5f8109913c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -554,14 +554,13 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
>  
>  static void omap_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct omap_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(struct omap_crtc_state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 17741843cf51..7beab22b4674 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -857,14 +857,12 @@ static void rcar_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	__drm_atomic_helper_crtc_reset(crtc, &state->state);
>  	if (state == NULL)
>  		return;
>  
>  	state->crc.source = VSP1_DU_CRC_NONE;
>  	state->crc.index = 0;
> -
> -	crtc->state = &state->state;
> -	crtc->state->crtc = crtc;
>  }
>  
>  static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..d2d5aa676084 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1071,13 +1071,14 @@ static void vop_crtc_destroy(struct drm_crtc *crtc)
>  
>  static void vop_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct rockchip_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  	kfree(crtc->state);
>  
> -	crtc->state = kzalloc(sizeof(struct rockchip_crtc_state), GFP_KERNEL);
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index f80e82e16475..140d6191d8d2 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1164,10 +1164,7 @@ static void tegra_crtc_reset(struct drm_crtc *crtc)
>  	crtc->state = NULL;
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  
>  	drm_crtc_vblank_reset(crtc);
>  }
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index cd226e63d557..76beeda157fc 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -998,12 +998,12 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
>  static void
>  vc4_crtc_reset(struct drm_crtc *crtc)
>  {
> -	if (crtc->state)
> -		vc4_crtc_destroy_state(crtc->state);
> +	struct vc4_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  
> -	crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
>  	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +		vc4_crtc_destroy_state(crtc, crtc->state);
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static const struct drm_crtc_funcs vc4_crtc_funcs = {
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 177bbcb38306..7b018f6ee971 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -98,15 +98,10 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
>  		vkms_state = to_vkms_crtc_state(crtc->state);
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  		kfree(vkms_state);
> -		crtc->state = NULL;
>  	}
>  
>  	vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL);
> -	if (!vkms_state)
> -		return;
> -
> -	crtc->state = &vkms_state->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vkms_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e6b11f6ae2e4..2878bc0a4718 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -633,14 +633,7 @@ void vmw_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	vcs = kzalloc(sizeof(*vcs), GFP_KERNEL);
> -
> -	if (!vcs) {
> -		DRM_ERROR("Cannot allocate vmw_crtc_state\n");
> -		return;
> -	}
> -
> -	crtc->state = &vcs->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vcs->base);
>  }
>  
>  
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index 5b82ccfdb502..ab2b5a49948c 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -37,6 +37,8 @@ struct drm_private_state;
>  struct drm_modeset_acquire_ctx;
>  struct drm_device;
>  
> +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);
>  void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  					      struct drm_crtc_state *state);
> -- 
> 2.19.1
>

Maarten Lankhorst Nov. 12, 2018, 5:33 p.m. UTC | #7

Op 12-11-18 om 17:11 schreef Sean Paul:
> On Mon, Nov 12, 2018 at 04:01:14PM +0100, Maarten Lankhorst wrote:
>> We already have __drm_atomic_helper_connector_reset() and
>> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
>>
>> Most drivers already have a gpu reset hook, correct it.
>> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
>> convert it to the common one.
>>
>> Signed-off-by: Maarten Lankhorst <[e-mail]>
>> Cc: Harry Wentland <[e-mail]>
>> Cc: Leo Li <[e-mail]>
>> Cc: Alex Deucher <[e-mail]>
>> Cc: "Christian König" <[e-mail]>
>> Cc: "David (ChunMing) Zhou" <[e-mail]>
>> Cc: David Airlie <[e-mail]>
>> Cc: Liviu Dudau <[e-mail]>
>> Cc: Brian Starkey <[e-mail]>
>> Cc: Mali DP Maintainers <[e-mail]>
>> Cc: Boris Brezillon <[e-mail]>
>> Cc: Nicolas Ferre <[e-mail]>
>> Cc: Alexandre Belloni <[e-mail]>
>> Cc: Ludovic Desroches <[e-mail]>
>> Cc: Maarten Lankhorst <[e-mail]>
>> Cc: Maxime Ripard <[e-mail]>
>> Cc: Sean Paul <[e-mail]>
>> Cc: Jani Nikula <[e-mail]>
>> Cc: Joonas Lahtinen <[e-mail]>
>> Cc: Rodrigo Vivi <[e-mail]>
>> Cc: Philipp Zabel <[e-mail]>
>> Cc: CK Hu <[e-mail]>
>> Cc: Matthias Brugger <[e-mail]>
>> Cc: Rob Clark <[e-mail]>
>> Cc: Ben Skeggs <[e-mail]>
>> Cc: Tomi Valkeinen <[e-mail]>
>> Cc: Laurent Pinchart <[e-mail]>
>> Cc: Kieran Bingham <[e-mail]>
>> Cc: Sandy Huang <[e-mail]>
>> Cc: "Heiko Stübner" <[e-mail]>
>> Cc: Thierry Reding <[e-mail]>
>> Cc: Jonathan Hunter <[e-mail]>
>> Cc: Eric Anholt <[e-mail]>
>> Cc: VMware Graphics <[e-mail]>
>> Cc: Sinclair Yeh <[e-mail]>
>> Cc: Thomas Hellstrom <[e-mail]>
>> Cc: Tony Cheng <[e-mail]>
>> Cc: Shirish S <[e-mail]>
>> Cc: Mikita Lipski <[e-mail]>
>> Cc: Bhawanpreet Lakha <[e-mail]>
>> Cc: David Francis <[e-mail]>
>> Cc: Anthony Koo <[e-mail]>
>> Cc: Jeykumar Sankaran <[e-mail]>
>> Cc: Jordan Crouse <[e-mail]>
>> Cc: Bruce Wang <[e-mail]>
>> Cc: Sravanthi Kollukuduru <[e-mail]>
>> Cc: Archit Taneja <[e-mail]>
>> Cc: Steve Kowalik <[e-mail]>
>> Cc: Carsten Behling <[e-mail]>
>> Cc: Haneen Mohammed <[e-mail]>
>> Cc: Daniel Vetter <[e-mail]>
>> Cc: Rodrigo Siqueira <[e-mail]>
>> Cc: Mahesh Kumar <[e-mail]>
>> Cc: [e-mail]
>> Cc: [e-mail]
>> Cc: [e-mail]
>> Cc: [e-mail]
>> Cc: [e-mail]
>> Cc: [e-mail]
>> Cc: [e-mail]
>> Cc: [e-mail]
>> Cc: [e-mail]
>> Cc: [e-mail]
>> Cc: [e-mail]
>> Cc: [e-mail]
>> ---
>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>>  drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
>>  drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
>>  drivers/gpu/drm/i915/intel_display.c          |  2 +-
>>  drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
>>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
>>  drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
>>  drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
>>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>>  drivers/gpu/drm/tegra/dc.c                    |  5 +--
>>  drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
>>  drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
>>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
>>  include/drm/drm_atomic_state_helper.h         |  2 ++
>>  18 files changed, 56 insertions(+), 81 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 5064768642f3..770a71726cd1 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -2802,9 +2802,7 @@ static void dm_crtc_reset_state(struct drm_crtc *crtc)
>>  	if (WARN_ON(!state))
> Can you give this the same treatment as the other allocation checks?

Yeah sure, the reason I didn't was because all of the amdgpu  reset functions worked like that. I'll send a preparation patch to fix that up.

~Maarten

Ville Syrjälä Nov. 12, 2018, 6:21 p.m. UTC | #8

On Mon, Nov 12, 2018 at 04:01:14PM +0100, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
> Cc: Harry Wentland <[e-mail]>
> Cc: Leo Li <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: "David (ChunMing) Zhou" <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Liviu Dudau <[e-mail]>
> Cc: Brian Starkey <[e-mail]>
> Cc: Mali DP Maintainers <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Nicolas Ferre <[e-mail]>
> Cc: Alexandre Belloni <[e-mail]>
> Cc: Ludovic Desroches <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: Philipp Zabel <[e-mail]>
> Cc: CK Hu <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> Cc: Rob Clark <[e-mail]>
> Cc: Ben Skeggs <[e-mail]>
> Cc: Tomi Valkeinen <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kieran Bingham <[e-mail]>
> Cc: Sandy Huang <[e-mail]>
> Cc: "Heiko Stübner" <[e-mail]>
> Cc: Thierry Reding <[e-mail]>
> Cc: Jonathan Hunter <[e-mail]>
> Cc: Eric Anholt <[e-mail]>
> Cc: VMware Graphics <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Tony Cheng <[e-mail]>
> Cc: Shirish S <[e-mail]>
> Cc: Mikita Lipski <[e-mail]>
> Cc: Bhawanpreet Lakha <[e-mail]>
> Cc: David Francis <[e-mail]>
> Cc: Anthony Koo <[e-mail]>
> Cc: Jeykumar Sankaran <[e-mail]>
> Cc: Jordan Crouse <[e-mail]>
> Cc: Bruce Wang <[e-mail]>
> Cc: Sravanthi Kollukuduru <[e-mail]>
> Cc: Archit Taneja <[e-mail]>
> Cc: Steve Kowalik <[e-mail]>
> Cc: Carsten Behling <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Rodrigo Siqueira <[e-mail]>
> Cc: Mahesh Kumar <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>  drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
>  drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
>  drivers/gpu/drm/i915/intel_display.c          |  2 +-
>  drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
>  drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
>  drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>  drivers/gpu/drm/tegra/dc.c                    |  5 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
>  drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
>  include/drm/drm_atomic_state_helper.h         |  2 ++
>  18 files changed, 56 insertions(+), 81 deletions(-)
> 
<snip>
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 3ba996069d69..3eee1ca33fc5 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -55,6 +55,29 @@
>   * for these functions.
>   */
>  
> +/**
> + * __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_conector->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)
> +		crtc_state->crtc = crtc;

So apparently crtc and connector reset expect a NULL state, but plane
reset does not. That seems a bit inconsistent. Migth be nice to get
them all to agree on what is the correct behaviour.

> +
> +	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
> @@ -64,14 +87,14 @@
>   */
>  void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct drm_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f383417571ec..907ffeb64781 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15457,7 +15457,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
>  		memset(crtc_state, 0, sizeof(*crtc_state));
> -		crtc_state->base.crtc = &crtc->base;
> +		__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->base);

intel_crtc_init() could use the same treatment.

And intel_create_plane_state() could use the plane reset. In fact it
looks like we aren't intializing plane constant alpha at all. So it'll
start out as zero which probably isn't what most people would expect.

I also wonder if this patch shouldn't be split up more. Just in case
there's some unforseen regression somewhere I'd hate to see the
entire thing get reverted.

Liviu Dudau Nov. 12, 2018, 8:01 p.m. UTC | #9

On Mon, Nov 12, 2018 at 04:01:14PM +0100, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
> Cc: Harry Wentland <[e-mail]>
> Cc: Leo Li <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: "David (ChunMing) Zhou" <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Liviu Dudau <[e-mail]>
> Cc: Brian Starkey <[e-mail]>
> Cc: Mali DP Maintainers <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Nicolas Ferre <[e-mail]>
> Cc: Alexandre Belloni <[e-mail]>
> Cc: Ludovic Desroches <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: Philipp Zabel <[e-mail]>
> Cc: CK Hu <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> Cc: Rob Clark <[e-mail]>
> Cc: Ben Skeggs <[e-mail]>
> Cc: Tomi Valkeinen <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kieran Bingham <[e-mail]>
> Cc: Sandy Huang <[e-mail]>
> Cc: "Heiko Stübner" <[e-mail]>
> Cc: Thierry Reding <[e-mail]>
> Cc: Jonathan Hunter <[e-mail]>
> Cc: Eric Anholt <[e-mail]>
> Cc: VMware Graphics <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Tony Cheng <[e-mail]>
> Cc: Shirish S <[e-mail]>
> Cc: Mikita Lipski <[e-mail]>
> Cc: Bhawanpreet Lakha <[e-mail]>
> Cc: David Francis <[e-mail]>
> Cc: Anthony Koo <[e-mail]>
> Cc: Jeykumar Sankaran <[e-mail]>
> Cc: Jordan Crouse <[e-mail]>
> Cc: Bruce Wang <[e-mail]>
> Cc: Sravanthi Kollukuduru <[e-mail]>
> Cc: Archit Taneja <[e-mail]>
> Cc: Steve Kowalik <[e-mail]>
> Cc: Carsten Behling <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Rodrigo Siqueira <[e-mail]>
> Cc: Mahesh Kumar <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>  drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
>  drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
>  drivers/gpu/drm/i915/intel_display.c          |  2 +-
>  drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
>  drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
>  drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>  drivers/gpu/drm/tegra/dc.c                    |  5 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
>  drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
>  include/drm/drm_atomic_state_helper.h         |  2 ++
>  18 files changed, 56 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5064768642f3..770a71726cd1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2802,9 +2802,7 @@ static void dm_crtc_reset_state(struct drm_crtc *crtc)
>  	if (WARN_ON(!state))
>  		return;
>  
> -	crtc->state = &state->base;
> -	crtc->state->crtc = crtc;
> -
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
> index e1b72782848c..9a924ff27148 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -474,10 +474,7 @@ static void malidp_crtc_reset(struct drm_crtc *crtc)
>  
>  	kfree(state);
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static void malidp_crtc_destroy_state(struct drm_crtc *crtc,

For the malidp changes:
Acked-by: Liviu Dudau <[e-mail]>

Best regards,
Liviu

> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 96f4082671fe..8084d549c7d1 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -412,10 +412,7 @@ static void atmel_hlcdc_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 3ba996069d69..3eee1ca33fc5 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -55,6 +55,29 @@
>   * for these functions.
>   */
>  
> +/**
> + * __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_conector->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)
> +		crtc_state->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
> @@ -64,14 +87,14 @@
>   */
>  void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct drm_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f383417571ec..907ffeb64781 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15457,7 +15457,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
>  		memset(crtc_state, 0, sizeof(*crtc_state));
> -		crtc_state->base.crtc = &crtc->base;
> +		__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->base);
>  
>  		crtc_state->base.active = crtc_state->base.enable =
>  			dev_priv->display.get_pipe_config(crtc, crtc_state);
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 7d4b710b837a..8bc36f0d2b6b 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -120,12 +120,9 @@ static void imx_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *imx_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 92ecb9bf982c..a743e5ed1177 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -119,12 +119,9 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4530d60767b..c86e603f486a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -894,20 +894,12 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
>  		_dpu_crtc_set_suspend(crtc, false);
>  
>  	/* remove previous state, if present */
> -	if (crtc->state) {
> +	if (crtc->state)
>  		dpu_crtc_destroy_state(crtc, crtc->state);
> -		crtc->state = 0;
> -	}
>  
>  	dpu_crtc = to_dpu_crtc(crtc);
>  	cstate = kzalloc(sizeof(*cstate), GFP_KERNEL);
> -	if (!cstate) {
> -		DPU_ERROR("failed to allocate state\n");
> -		return;
> -	}
> -
> -	cstate->base.crtc = crtc;
> -	crtc->state = &cstate->base;
> +	__drm_atomic_helper_crtc_reset(crtc, &cstate->base);
>  }
>  
>  static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index b1da9ce54379..138da66b1530 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -1019,11 +1019,7 @@ static void mdp5_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	mdp5_cstate = kzalloc(sizeof(*mdp5_cstate), GFP_KERNEL);
> -
> -	if (mdp5_cstate) {
> -		mdp5_cstate->base.crtc = crtc;
> -		crtc->state = &mdp5_cstate->base;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &mdp5_cstate->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 4f57e5379796..341600406589 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -418,16 +418,6 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc)
>  	return &asyh->state;
>  }
>  
> -static void
> -__drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
> -			       struct drm_crtc_state *state)
> -{
> -	if (crtc->state)
> -		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> -	crtc->state = state;
> -	crtc->state->crtc = crtc;
> -}
> -
>  static void
>  nv50_head_reset(struct drm_crtc *crtc)
>  {
> @@ -436,6 +426,9 @@ nv50_head_reset(struct drm_crtc *crtc)
>  	if (WARN_ON(!(asyh = kzalloc(sizeof(*asyh), GFP_KERNEL))))
>  		return;
>  
> +	if (crtc->state)
> +		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> +
>  	__drm_atomic_helper_crtc_reset(crtc, &asyh->state);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index caffc547ef97..9c5f8109913c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -554,14 +554,13 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
>  
>  static void omap_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct omap_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(struct omap_crtc_state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 17741843cf51..7beab22b4674 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -857,14 +857,12 @@ static void rcar_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	__drm_atomic_helper_crtc_reset(crtc, &state->state);
>  	if (state == NULL)
>  		return;
>  
>  	state->crc.source = VSP1_DU_CRC_NONE;
>  	state->crc.index = 0;
> -
> -	crtc->state = &state->state;
> -	crtc->state->crtc = crtc;
>  }
>  
>  static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..d2d5aa676084 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1071,13 +1071,14 @@ static void vop_crtc_destroy(struct drm_crtc *crtc)
>  
>  static void vop_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct rockchip_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  	kfree(crtc->state);
>  
> -	crtc->state = kzalloc(sizeof(struct rockchip_crtc_state), GFP_KERNEL);
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index f80e82e16475..140d6191d8d2 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1164,10 +1164,7 @@ static void tegra_crtc_reset(struct drm_crtc *crtc)
>  	crtc->state = NULL;
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  
>  	drm_crtc_vblank_reset(crtc);
>  }
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index cd226e63d557..76beeda157fc 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -998,12 +998,12 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
>  static void
>  vc4_crtc_reset(struct drm_crtc *crtc)
>  {
> -	if (crtc->state)
> -		vc4_crtc_destroy_state(crtc->state);
> +	struct vc4_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  
> -	crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
>  	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +		vc4_crtc_destroy_state(crtc, crtc->state);
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static const struct drm_crtc_funcs vc4_crtc_funcs = {
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 177bbcb38306..7b018f6ee971 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -98,15 +98,10 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
>  		vkms_state = to_vkms_crtc_state(crtc->state);
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  		kfree(vkms_state);
> -		crtc->state = NULL;
>  	}
>  
>  	vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL);
> -	if (!vkms_state)
> -		return;
> -
> -	crtc->state = &vkms_state->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vkms_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e6b11f6ae2e4..2878bc0a4718 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -633,14 +633,7 @@ void vmw_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	vcs = kzalloc(sizeof(*vcs), GFP_KERNEL);
> -
> -	if (!vcs) {
> -		DRM_ERROR("Cannot allocate vmw_crtc_state\n");
> -		return;
> -	}
> -
> -	crtc->state = &vcs->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vcs->base);
>  }
>  
>  
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index 5b82ccfdb502..ab2b5a49948c 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -37,6 +37,8 @@ struct drm_private_state;
>  struct drm_modeset_acquire_ctx;
>  struct drm_device;
>  
> +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);
>  void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  					      struct drm_crtc_state *state);
> -- 
> 2.19.1
> 
> _______________________________________________
> dri-devel mailing list
> [e-mail]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Rodrigo Siqueira Nov. 18, 2018, 1:31 p.m. UTC | #10

On 11/12, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
> Cc: Harry Wentland <[e-mail]>
> Cc: Leo Li <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: "David (ChunMing) Zhou" <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Liviu Dudau <[e-mail]>
> Cc: Brian Starkey <[e-mail]>
> Cc: Mali DP Maintainers <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Nicolas Ferre <[e-mail]>
> Cc: Alexandre Belloni <[e-mail]>
> Cc: Ludovic Desroches <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: Philipp Zabel <[e-mail]>
> Cc: CK Hu <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> Cc: Rob Clark <[e-mail]>
> Cc: Ben Skeggs <[e-mail]>
> Cc: Tomi Valkeinen <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kieran Bingham <[e-mail]>
> Cc: Sandy Huang <[e-mail]>
> Cc: "Heiko Stübner" <[e-mail]>
> Cc: Thierry Reding <[e-mail]>
> Cc: Jonathan Hunter <[e-mail]>
> Cc: Eric Anholt <[e-mail]>
> Cc: VMware Graphics <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Tony Cheng <[e-mail]>
> Cc: Shirish S <[e-mail]>
> Cc: Mikita Lipski <[e-mail]>
> Cc: Bhawanpreet Lakha <[e-mail]>
> Cc: David Francis <[e-mail]>
> Cc: Anthony Koo <[e-mail]>
> Cc: Jeykumar Sankaran <[e-mail]>
> Cc: Jordan Crouse <[e-mail]>
> Cc: Bruce Wang <[e-mail]>
> Cc: Sravanthi Kollukuduru <[e-mail]>
> Cc: Archit Taneja <[e-mail]>
> Cc: Steve Kowalik <[e-mail]>
> Cc: Carsten Behling <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Rodrigo Siqueira <[e-mail]>
> Cc: Mahesh Kumar <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>  drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
>  drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
>  drivers/gpu/drm/i915/intel_display.c          |  2 +-
>  drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
>  drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
>  drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>  drivers/gpu/drm/tegra/dc.c                    |  5 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
>  drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
>  include/drm/drm_atomic_state_helper.h         |  2 ++
>  18 files changed, 56 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5064768642f3..770a71726cd1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2802,9 +2802,7 @@ static void dm_crtc_reset_state(struct drm_crtc *crtc)
>  	if (WARN_ON(!state))
>  		return;
>  
> -	crtc->state = &state->base;
> -	crtc->state->crtc = crtc;
> -
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
> index e1b72782848c..9a924ff27148 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -474,10 +474,7 @@ static void malidp_crtc_reset(struct drm_crtc *crtc)
>  
>  	kfree(state);
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static void malidp_crtc_destroy_state(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 96f4082671fe..8084d549c7d1 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -412,10 +412,7 @@ static void atmel_hlcdc_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 3ba996069d69..3eee1ca33fc5 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -55,6 +55,29 @@
>   * for these functions.
>   */
>  
> +/**
> + * __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_conector->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)
> +		crtc_state->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
> @@ -64,14 +87,14 @@
>   */
>  void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct drm_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f383417571ec..907ffeb64781 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15457,7 +15457,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
>  		memset(crtc_state, 0, sizeof(*crtc_state));
> -		crtc_state->base.crtc = &crtc->base;
> +		__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->base);
>  
>  		crtc_state->base.active = crtc_state->base.enable =
>  			dev_priv->display.get_pipe_config(crtc, crtc_state);
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 7d4b710b837a..8bc36f0d2b6b 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -120,12 +120,9 @@ static void imx_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *imx_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 92ecb9bf982c..a743e5ed1177 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -119,12 +119,9 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4530d60767b..c86e603f486a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -894,20 +894,12 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
>  		_dpu_crtc_set_suspend(crtc, false);
>  
>  	/* remove previous state, if present */
> -	if (crtc->state) {
> +	if (crtc->state)
>  		dpu_crtc_destroy_state(crtc, crtc->state);
> -		crtc->state = 0;
> -	}
>  
>  	dpu_crtc = to_dpu_crtc(crtc);
>  	cstate = kzalloc(sizeof(*cstate), GFP_KERNEL);
> -	if (!cstate) {
> -		DPU_ERROR("failed to allocate state\n");
> -		return;
> -	}
> -
> -	cstate->base.crtc = crtc;
> -	crtc->state = &cstate->base;
> +	__drm_atomic_helper_crtc_reset(crtc, &cstate->base);
>  }
>  
>  static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index b1da9ce54379..138da66b1530 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -1019,11 +1019,7 @@ static void mdp5_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	mdp5_cstate = kzalloc(sizeof(*mdp5_cstate), GFP_KERNEL);
> -
> -	if (mdp5_cstate) {
> -		mdp5_cstate->base.crtc = crtc;
> -		crtc->state = &mdp5_cstate->base;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &mdp5_cstate->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 4f57e5379796..341600406589 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -418,16 +418,6 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc)
>  	return &asyh->state;
>  }
>  
> -static void
> -__drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
> -			       struct drm_crtc_state *state)
> -{
> -	if (crtc->state)
> -		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> -	crtc->state = state;
> -	crtc->state->crtc = crtc;
> -}
> -
>  static void
>  nv50_head_reset(struct drm_crtc *crtc)
>  {
> @@ -436,6 +426,9 @@ nv50_head_reset(struct drm_crtc *crtc)
>  	if (WARN_ON(!(asyh = kzalloc(sizeof(*asyh), GFP_KERNEL))))
>  		return;
>  
> +	if (crtc->state)
> +		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> +
>  	__drm_atomic_helper_crtc_reset(crtc, &asyh->state);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index caffc547ef97..9c5f8109913c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -554,14 +554,13 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
>  
>  static void omap_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct omap_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(struct omap_crtc_state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 17741843cf51..7beab22b4674 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -857,14 +857,12 @@ static void rcar_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	__drm_atomic_helper_crtc_reset(crtc, &state->state);
>  	if (state == NULL)
>  		return;
>  
>  	state->crc.source = VSP1_DU_CRC_NONE;
>  	state->crc.index = 0;
> -
> -	crtc->state = &state->state;
> -	crtc->state->crtc = crtc;
>  }
>  
>  static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..d2d5aa676084 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1071,13 +1071,14 @@ static void vop_crtc_destroy(struct drm_crtc *crtc)
>  
>  static void vop_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct rockchip_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  	kfree(crtc->state);
>  
> -	crtc->state = kzalloc(sizeof(struct rockchip_crtc_state), GFP_KERNEL);
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index f80e82e16475..140d6191d8d2 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1164,10 +1164,7 @@ static void tegra_crtc_reset(struct drm_crtc *crtc)
>  	crtc->state = NULL;
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  
>  	drm_crtc_vblank_reset(crtc);
>  }
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index cd226e63d557..76beeda157fc 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -998,12 +998,12 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
>  static void
>  vc4_crtc_reset(struct drm_crtc *crtc)
>  {
> -	if (crtc->state)
> -		vc4_crtc_destroy_state(crtc->state);
> +	struct vc4_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  
> -	crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
>  	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +		vc4_crtc_destroy_state(crtc, crtc->state);
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static const struct drm_crtc_funcs vc4_crtc_funcs = {
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 177bbcb38306..7b018f6ee971 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -98,15 +98,10 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
>  		vkms_state = to_vkms_crtc_state(crtc->state);
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  		kfree(vkms_state);
> -		crtc->state = NULL;
>  	}
>  
>  	vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL);
> -	if (!vkms_state)
> -		return;
> -
> -	crtc->state = &vkms_state->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vkms_state->base);
>  }

I tested the changes with IGT, and everything looks fine for VKMS.

Reviewed-by: Rodrigo Siqueira <[e-mail]>
  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e6b11f6ae2e4..2878bc0a4718 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -633,14 +633,7 @@ void vmw_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	vcs = kzalloc(sizeof(*vcs), GFP_KERNEL);
> -
> -	if (!vcs) {
> -		DRM_ERROR("Cannot allocate vmw_crtc_state\n");
> -		return;
> -	}
> -
> -	crtc->state = &vcs->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vcs->base);
>  }
>  
>  
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index 5b82ccfdb502..ab2b5a49948c 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -37,6 +37,8 @@ struct drm_private_state;
>  struct drm_modeset_acquire_ctx;
>  struct drm_device;
>  
> +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);
>  void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  					      struct drm_crtc_state *state);
> -- 
> 2.19.1
>

CK Hu (胡俊光) Nov. 20, 2018, 7:08 a.m. UTC | #11

On Mon, 2018-11-12 at 16:01 +0100, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
> Cc: Harry Wentland <[e-mail]>
> Cc: Leo Li <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: "David (ChunMing) Zhou" <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Liviu Dudau <[e-mail]>
> Cc: Brian Starkey <[e-mail]>
> Cc: Mali DP Maintainers <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Nicolas Ferre <[e-mail]>
> Cc: Alexandre Belloni <[e-mail]>
> Cc: Ludovic Desroches <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: Philipp Zabel <[e-mail]>
> Cc: CK Hu <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> Cc: Rob Clark <[e-mail]>
> Cc: Ben Skeggs <[e-mail]>
> Cc: Tomi Valkeinen <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kieran Bingham <[e-mail]>
> Cc: Sandy Huang <[e-mail]>
> Cc: "Heiko Stübner" <[e-mail]>
> Cc: Thierry Reding <[e-mail]>
> Cc: Jonathan Hunter <[e-mail]>
> Cc: Eric Anholt <[e-mail]>
> Cc: VMware Graphics <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Tony Cheng <[e-mail]>
> Cc: Shirish S <[e-mail]>
> Cc: Mikita Lipski <[e-mail]>
> Cc: Bhawanpreet Lakha <[e-mail]>
> Cc: David Francis <[e-mail]>
> Cc: Anthony Koo <[e-mail]>
> Cc: Jeykumar Sankaran <[e-mail]>
> Cc: Jordan Crouse <[e-mail]>
> Cc: Bruce Wang <[e-mail]>
> Cc: Sravanthi Kollukuduru <[e-mail]>
> Cc: Archit Taneja <[e-mail]>
> Cc: Steve Kowalik <[e-mail]>
> Cc: Carsten Behling <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Rodrigo Siqueira <[e-mail]>
> Cc: Mahesh Kumar <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>  drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
>  drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
>  drivers/gpu/drm/i915/intel_display.c          |  2 +-
>  drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
>  drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
>  drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>  drivers/gpu/drm/tegra/dc.c                    |  5 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
>  drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
>  include/drm/drm_atomic_state_helper.h         |  2 ++
>  18 files changed, 56 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5064768642f3..770a71726cd1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2802,9 +2802,7 @@ static void dm_crtc_reset_state(struct drm_crtc *crtc)
>  	if (WARN_ON(!state))
>  		return;
>  
> -	crtc->state = &state->base;
> -	crtc->state->crtc = crtc;
> -
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
> index e1b72782848c..9a924ff27148 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -474,10 +474,7 @@ static void malidp_crtc_reset(struct drm_crtc *crtc)
>  
>  	kfree(state);
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static void malidp_crtc_destroy_state(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 96f4082671fe..8084d549c7d1 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -412,10 +412,7 @@ static void atmel_hlcdc_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 3ba996069d69..3eee1ca33fc5 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -55,6 +55,29 @@
>   * for these functions.
>   */
>  
> +/**
> + * __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_conector->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)
> +		crtc_state->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
> @@ -64,14 +87,14 @@
>   */
>  void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct drm_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f383417571ec..907ffeb64781 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15457,7 +15457,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
>  		memset(crtc_state, 0, sizeof(*crtc_state));
> -		crtc_state->base.crtc = &crtc->base;
> +		__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->base);
>  
>  		crtc_state->base.active = crtc_state->base.enable =
>  			dev_priv->display.get_pipe_config(crtc, crtc_state);
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 7d4b710b837a..8bc36f0d2b6b 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -120,12 +120,9 @@ static void imx_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *imx_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 92ecb9bf982c..a743e5ed1177 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -119,12 +119,9 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4530d60767b..c86e603f486a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -894,20 +894,12 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
>  		_dpu_crtc_set_suspend(crtc, false);
>  
>  	/* remove previous state, if present */
> -	if (crtc->state) {
> +	if (crtc->state)
>  		dpu_crtc_destroy_state(crtc, crtc->state);
> -		crtc->state = 0;
> -	}
>  
>  	dpu_crtc = to_dpu_crtc(crtc);
>  	cstate = kzalloc(sizeof(*cstate), GFP_KERNEL);
> -	if (!cstate) {
> -		DPU_ERROR("failed to allocate state\n");
> -		return;
> -	}
> -
> -	cstate->base.crtc = crtc;
> -	crtc->state = &cstate->base;
> +	__drm_atomic_helper_crtc_reset(crtc, &cstate->base);
>  }

For the mediatek drm driver,

Reviewed-by: CK Hu <[e-mail]>

>  
>  static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index b1da9ce54379..138da66b1530 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -1019,11 +1019,7 @@ static void mdp5_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	mdp5_cstate = kzalloc(sizeof(*mdp5_cstate), GFP_KERNEL);
> -
> -	if (mdp5_cstate) {
> -		mdp5_cstate->base.crtc = crtc;
> -		crtc->state = &mdp5_cstate->base;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &mdp5_cstate->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 4f57e5379796..341600406589 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -418,16 +418,6 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc)
>  	return &asyh->state;
>  }
>  
> -static void
> -__drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
> -			       struct drm_crtc_state *state)
> -{
> -	if (crtc->state)
> -		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> -	crtc->state = state;
> -	crtc->state->crtc = crtc;
> -}
> -
>  static void
>  nv50_head_reset(struct drm_crtc *crtc)
>  {
> @@ -436,6 +426,9 @@ nv50_head_reset(struct drm_crtc *crtc)
>  	if (WARN_ON(!(asyh = kzalloc(sizeof(*asyh), GFP_KERNEL))))
>  		return;
>  
> +	if (crtc->state)
> +		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> +
>  	__drm_atomic_helper_crtc_reset(crtc, &asyh->state);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index caffc547ef97..9c5f8109913c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -554,14 +554,13 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
>  
>  static void omap_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct omap_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(struct omap_crtc_state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 17741843cf51..7beab22b4674 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -857,14 +857,12 @@ static void rcar_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	__drm_atomic_helper_crtc_reset(crtc, &state->state);
>  	if (state == NULL)
>  		return;
>  
>  	state->crc.source = VSP1_DU_CRC_NONE;
>  	state->crc.index = 0;
> -
> -	crtc->state = &state->state;
> -	crtc->state->crtc = crtc;
>  }
>  
>  static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..d2d5aa676084 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1071,13 +1071,14 @@ static void vop_crtc_destroy(struct drm_crtc *crtc)
>  
>  static void vop_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct rockchip_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  	kfree(crtc->state);
>  
> -	crtc->state = kzalloc(sizeof(struct rockchip_crtc_state), GFP_KERNEL);
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index f80e82e16475..140d6191d8d2 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1164,10 +1164,7 @@ static void tegra_crtc_reset(struct drm_crtc *crtc)
>  	crtc->state = NULL;
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  
>  	drm_crtc_vblank_reset(crtc);
>  }
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index cd226e63d557..76beeda157fc 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -998,12 +998,12 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
>  static void
>  vc4_crtc_reset(struct drm_crtc *crtc)
>  {
> -	if (crtc->state)
> -		vc4_crtc_destroy_state(crtc->state);
> +	struct vc4_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  
> -	crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
>  	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +		vc4_crtc_destroy_state(crtc, crtc->state);
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static const struct drm_crtc_funcs vc4_crtc_funcs = {
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 177bbcb38306..7b018f6ee971 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -98,15 +98,10 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
>  		vkms_state = to_vkms_crtc_state(crtc->state);
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  		kfree(vkms_state);
> -		crtc->state = NULL;
>  	}
>  
>  	vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL);
> -	if (!vkms_state)
> -		return;
> -
> -	crtc->state = &vkms_state->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vkms_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e6b11f6ae2e4..2878bc0a4718 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -633,14 +633,7 @@ void vmw_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	vcs = kzalloc(sizeof(*vcs), GFP_KERNEL);
> -
> -	if (!vcs) {
> -		DRM_ERROR("Cannot allocate vmw_crtc_state\n");
> -		return;
> -	}
> -
> -	crtc->state = &vcs->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vcs->base);
>  }
>  
> 
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index 5b82ccfdb502..ab2b5a49948c 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -37,6 +37,8 @@ struct drm_private_state;
>  struct drm_modeset_acquire_ctx;
>  struct drm_device;
>  
> +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);
>  void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  					      struct drm_crtc_state *state);

Philipp Zabel Nov. 20, 2018, 9:30 a.m. UTC | #12

On Mon, 2018-11-12 at 16:01 +0100, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
[...]
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 7d4b710b837a..8bc36f0d2b6b 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -120,12 +120,9 @@ static void imx_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *imx_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 92ecb9bf982c..a743e5ed1177 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -119,12 +119,9 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc *crtc)

For the imx-drm and mediatek drivers,

Acked-by: Philipp Zabel <[e-mail]>

regards
Philipp

Lyude Paul Nov. 21, 2018, 6:41 p.m. UTC | #13

For the nouveau and drm core changes

Reviewed-by: Lyude Paul <[e-mail]>

On Mon, 2018-11-12 at 16:01 +0100, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
> Cc: Harry Wentland <[e-mail]>
> Cc: Leo Li <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: "David (ChunMing) Zhou" <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Liviu Dudau <[e-mail]>
> Cc: Brian Starkey <[e-mail]>
> Cc: Mali DP Maintainers <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Nicolas Ferre <[e-mail]>
> Cc: Alexandre Belloni <[e-mail]>
> Cc: Ludovic Desroches <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: Philipp Zabel <[e-mail]>
> Cc: CK Hu <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> Cc: Rob Clark <[e-mail]>
> Cc: Ben Skeggs <[e-mail]>
> Cc: Tomi Valkeinen <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kieran Bingham <[e-mail]>
> Cc: Sandy Huang <[e-mail]>
> Cc: "Heiko Stübner" <[e-mail]>
> Cc: Thierry Reding <[e-mail]>
> Cc: Jonathan Hunter <[e-mail]>
> Cc: Eric Anholt <[e-mail]>
> Cc: VMware Graphics <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Tony Cheng <[e-mail]>
> Cc: Shirish S <[e-mail]>
> Cc: Mikita Lipski <[e-mail]>
> Cc: Bhawanpreet Lakha <[e-mail]>
> Cc: David Francis <[e-mail]>
> Cc: Anthony Koo <[e-mail]>
> Cc: Jeykumar Sankaran <[e-mail]>
> Cc: Jordan Crouse <[e-mail]>
> Cc: Bruce Wang <[e-mail]>
> Cc: Sravanthi Kollukuduru <[e-mail]>
> Cc: Archit Taneja <[e-mail]>
> Cc: Steve Kowalik <[e-mail]>
> Cc: Carsten Behling <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Rodrigo Siqueira <[e-mail]>
> Cc: Mahesh Kumar <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>  drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
>  drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
>  drivers/gpu/drm/i915/intel_display.c          |  2 +-
>  drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
>  drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
>  drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>  drivers/gpu/drm/tegra/dc.c                    |  5 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
>  drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
>  include/drm/drm_atomic_state_helper.h         |  2 ++
>  18 files changed, 56 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5064768642f3..770a71726cd1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2802,9 +2802,7 @@ static void dm_crtc_reset_state(struct drm_crtc *crtc)
>  	if (WARN_ON(!state))
>  		return;
>  
> -	crtc->state = &state->base;
> -	crtc->state->crtc = crtc;
> -
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
> b/drivers/gpu/drm/arm/malidp_crtc.c
> index e1b72782848c..9a924ff27148 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -474,10 +474,7 @@ static void malidp_crtc_reset(struct drm_crtc *crtc)
>  
>  	kfree(state);
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static void malidp_crtc_destroy_state(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 96f4082671fe..8084d549c7d1 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -412,10 +412,7 @@ static void atmel_hlcdc_crtc_reset(struct drm_crtc
> *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
> b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 3ba996069d69..3eee1ca33fc5 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -55,6 +55,29 @@
>   * for these functions.
>   */
>  
> +/**
> + * __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_conector->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)
> +		crtc_state->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
> @@ -64,14 +87,14 @@
>   */
>  void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct drm_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f383417571ec..907ffeb64781 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15457,7 +15457,7 @@ static void intel_modeset_readout_hw_state(struct
> drm_device *dev)
>  
>  		__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
>  		memset(crtc_state, 0, sizeof(*crtc_state));
> -		crtc_state->base.crtc = &crtc->base;
> +		__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state-
> >base);
>  
>  		crtc_state->base.active = crtc_state->base.enable =
>  			dev_priv->display.get_pipe_config(crtc, crtc_state);
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-
> crtc.c
> index 7d4b710b837a..8bc36f0d2b6b 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -120,12 +120,9 @@ static void imx_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *imx_drm_crtc_duplicate_state(struct drm_crtc
> *crtc)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 92ecb9bf982c..a743e5ed1177 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -119,12 +119,9 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc
> *crtc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4530d60767b..c86e603f486a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -894,20 +894,12 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
>  		_dpu_crtc_set_suspend(crtc, false);
>  
>  	/* remove previous state, if present */
> -	if (crtc->state) {
> +	if (crtc->state)
>  		dpu_crtc_destroy_state(crtc, crtc->state);
> -		crtc->state = 0;
> -	}
>  
>  	dpu_crtc = to_dpu_crtc(crtc);
>  	cstate = kzalloc(sizeof(*cstate), GFP_KERNEL);
> -	if (!cstate) {
> -		DPU_ERROR("failed to allocate state\n");
> -		return;
> -	}
> -
> -	cstate->base.crtc = crtc;
> -	crtc->state = &cstate->base;
> +	__drm_atomic_helper_crtc_reset(crtc, &cstate->base);
>  }
>  
>  static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index b1da9ce54379..138da66b1530 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -1019,11 +1019,7 @@ static void mdp5_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	mdp5_cstate = kzalloc(sizeof(*mdp5_cstate), GFP_KERNEL);
> -
> -	if (mdp5_cstate) {
> -		mdp5_cstate->base.crtc = crtc;
> -		crtc->state = &mdp5_cstate->base;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &mdp5_cstate->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
> b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 4f57e5379796..341600406589 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -418,16 +418,6 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc)
>  	return &asyh->state;
>  }
>  
> -static void
> -__drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
> -			       struct drm_crtc_state *state)
> -{
> -	if (crtc->state)
> -		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> -	crtc->state = state;
> -	crtc->state->crtc = crtc;
> -}
> -
>  static void
>  nv50_head_reset(struct drm_crtc *crtc)
>  {
> @@ -436,6 +426,9 @@ nv50_head_reset(struct drm_crtc *crtc)
>  	if (WARN_ON(!(asyh = kzalloc(sizeof(*asyh), GFP_KERNEL))))
>  		return;
>  
> +	if (crtc->state)
> +		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> +
>  	__drm_atomic_helper_crtc_reset(crtc, &asyh->state);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index caffc547ef97..9c5f8109913c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -554,14 +554,13 @@ static int omap_crtc_atomic_get_property(struct
> drm_crtc *crtc,
>  
>  static void omap_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct omap_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(struct omap_crtc_state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-
> du/rcar_du_crtc.c
> index 17741843cf51..7beab22b4674 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -857,14 +857,12 @@ static void rcar_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	__drm_atomic_helper_crtc_reset(crtc, &state->state);
>  	if (state == NULL)
>  		return;
>  
>  	state->crc.source = VSP1_DU_CRC_NONE;
>  	state->crc.index = 0;
> -
> -	crtc->state = &state->state;
> -	crtc->state->crtc = crtc;
>  }
>  
>  static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..d2d5aa676084 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1071,13 +1071,14 @@ static void vop_crtc_destroy(struct drm_crtc *crtc)
>  
>  static void vop_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct rockchip_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  	kfree(crtc->state);
>  
> -	crtc->state = kzalloc(sizeof(struct rockchip_crtc_state), GFP_KERNEL);
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc
> *crtc)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index f80e82e16475..140d6191d8d2 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1164,10 +1164,7 @@ static void tegra_crtc_reset(struct drm_crtc *crtc)
>  	crtc->state = NULL;
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  
>  	drm_crtc_vblank_reset(crtc);
>  }
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index cd226e63d557..76beeda157fc 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -998,12 +998,12 @@ static void vc4_crtc_destroy_state(struct drm_crtc
> *crtc,
>  static void
>  vc4_crtc_reset(struct drm_crtc *crtc)
>  {
> -	if (crtc->state)
> -		vc4_crtc_destroy_state(crtc->state);
> +	struct vc4_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  
> -	crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
>  	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +		vc4_crtc_destroy_state(crtc, crtc->state);
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static const struct drm_crtc_funcs vc4_crtc_funcs = {
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 177bbcb38306..7b018f6ee971 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -98,15 +98,10 @@ static void vkms_atomic_crtc_reset(struct drm_crtc
> *crtc)
>  		vkms_state = to_vkms_crtc_state(crtc->state);
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  		kfree(vkms_state);
> -		crtc->state = NULL;
>  	}
>  
>  	vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL);
> -	if (!vkms_state)
> -		return;
> -
> -	crtc->state = &vkms_state->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vkms_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e6b11f6ae2e4..2878bc0a4718 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -633,14 +633,7 @@ void vmw_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	vcs = kzalloc(sizeof(*vcs), GFP_KERNEL);
> -
> -	if (!vcs) {
> -		DRM_ERROR("Cannot allocate vmw_crtc_state\n");
> -		return;
> -	}
> -
> -	crtc->state = &vcs->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vcs->base);
>  }
>  
>  
> diff --git a/include/drm/drm_atomic_state_helper.h
> b/include/drm/drm_atomic_state_helper.h
> index 5b82ccfdb502..ab2b5a49948c 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -37,6 +37,8 @@ struct drm_private_state;
>  struct drm_modeset_acquire_ctx;
>  struct drm_device;
>  
> +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);
>  void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  					      struct drm_crtc_state *state);

Kieran Bingham Nov. 21, 2018, 9:06 p.m. UTC | #14

On 12/11/2018 15:01, Maarten Lankhorst wrote:
> We already have __drm_atomic_helper_connector_reset() and
> __drm_atomic_helper_plane_reset(), extend this to crtc as well.
> 
> Most drivers already have a gpu reset hook, correct it.
> Nouveau already implemented its own __drm_atomic_helper_crtc_reset(),
> convert it to the common one.
> 
> Signed-off-by: Maarten Lankhorst <[e-mail]>
> Cc: Harry Wentland <[e-mail]>
> Cc: Leo Li <[e-mail]>
> Cc: Alex Deucher <[e-mail]>
> Cc: "Christian König" <[e-mail]>
> Cc: "David (ChunMing) Zhou" <[e-mail]>
> Cc: David Airlie <[e-mail]>
> Cc: Liviu Dudau <[e-mail]>
> Cc: Brian Starkey <[e-mail]>
> Cc: Mali DP Maintainers <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Nicolas Ferre <[e-mail]>
> Cc: Alexandre Belloni <[e-mail]>
> Cc: Ludovic Desroches <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Sean Paul <[e-mail]>
> Cc: Jani Nikula <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Rodrigo Vivi <[e-mail]>
> Cc: Philipp Zabel <[e-mail]>
> Cc: CK Hu <[e-mail]>
> Cc: Matthias Brugger <[e-mail]>
> Cc: Rob Clark <[e-mail]>
> Cc: Ben Skeggs <[e-mail]>
> Cc: Tomi Valkeinen <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kieran Bingham <[e-mail]>
> Cc: Sandy Huang <[e-mail]>
> Cc: "Heiko Stübner" <[e-mail]>
> Cc: Thierry Reding <[e-mail]>
> Cc: Jonathan Hunter <[e-mail]>
> Cc: Eric Anholt <[e-mail]>
> Cc: VMware Graphics <[e-mail]>
> Cc: Sinclair Yeh <[e-mail]>
> Cc: Thomas Hellstrom <[e-mail]>
> Cc: Tony Cheng <[e-mail]>
> Cc: Shirish S <[e-mail]>
> Cc: Mikita Lipski <[e-mail]>
> Cc: Bhawanpreet Lakha <[e-mail]>
> Cc: David Francis <[e-mail]>
> Cc: Anthony Koo <[e-mail]>
> Cc: Jeykumar Sankaran <[e-mail]>
> Cc: Jordan Crouse <[e-mail]>
> Cc: Bruce Wang <[e-mail]>
> Cc: Sravanthi Kollukuduru <[e-mail]>
> Cc: Archit Taneja <[e-mail]>
> Cc: Steve Kowalik <[e-mail]>
> Cc: Carsten Behling <[e-mail]>
> Cc: Haneen Mohammed <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>
> Cc: Rodrigo Siqueira <[e-mail]>
> Cc: Mahesh Kumar <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
>  drivers/gpu/drm/arm/malidp_crtc.c             |  5 +--
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |  5 +--
>  drivers/gpu/drm/drm_atomic_state_helper.c     | 31 ++++++++++++++++---
>  drivers/gpu/drm/i915/intel_display.c          |  2 +-
>  drivers/gpu/drm/imx/ipuv3-crtc.c              |  5 +--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  5 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 ++-----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  6 +---
>  drivers/gpu/drm/nouveau/dispnv50/head.c       | 13 ++------
>  drivers/gpu/drm/omapdrm/omap_crtc.c           |  7 ++---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |  4 +--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  7 +++--
>  drivers/gpu/drm/tegra/dc.c                    |  5 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c                |  8 ++---
>  drivers/gpu/drm/vkms/vkms_crtc.c              |  7 +----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |  9 +-----
>  include/drm/drm_atomic_state_helper.h         |  2 ++
>  18 files changed, 56 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 5064768642f3..770a71726cd1 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2802,9 +2802,7 @@ static void dm_crtc_reset_state(struct drm_crtc *crtc)
>  	if (WARN_ON(!state))
>  		return;
>  
> -	crtc->state = &state->base;
> -	crtc->state->crtc = crtc;
> -
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
> index e1b72782848c..9a924ff27148 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -474,10 +474,7 @@ static void malidp_crtc_reset(struct drm_crtc *crtc)
>  
>  	kfree(state);
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static void malidp_crtc_destroy_state(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 96f4082671fe..8084d549c7d1 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -412,10 +412,7 @@ static void atmel_hlcdc_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 3ba996069d69..3eee1ca33fc5 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -55,6 +55,29 @@
>   * for these functions.
>   */
>  
> +/**
> + * __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_conector->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)
> +		crtc_state->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
> @@ -64,14 +87,14 @@
>   */
>  void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct drm_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f383417571ec..907ffeb64781 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15457,7 +15457,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
>  		memset(crtc_state, 0, sizeof(*crtc_state));
> -		crtc_state->base.crtc = &crtc->base;
> +		__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->base);
>  
>  		crtc_state->base.active = crtc_state->base.enable =
>  			dev_priv->display.get_pipe_config(crtc, crtc_state);
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index 7d4b710b837a..8bc36f0d2b6b 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -120,12 +120,9 @@ static void imx_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *imx_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 92ecb9bf982c..a743e5ed1177 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -119,12 +119,9 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
>  		memset(state, 0, sizeof(*state));
>  	} else {
>  		state = kzalloc(sizeof(*state), GFP_KERNEL);
> -		if (!state)
> -			return;
> -		crtc->state = &state->base;
>  	}
>  
> -	state->base.crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  }
>  
>  static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d4530d60767b..c86e603f486a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -894,20 +894,12 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
>  		_dpu_crtc_set_suspend(crtc, false);
>  
>  	/* remove previous state, if present */
> -	if (crtc->state) {
> +	if (crtc->state)
>  		dpu_crtc_destroy_state(crtc, crtc->state);
> -		crtc->state = 0;
> -	}
>  
>  	dpu_crtc = to_dpu_crtc(crtc);
>  	cstate = kzalloc(sizeof(*cstate), GFP_KERNEL);
> -	if (!cstate) {
> -		DPU_ERROR("failed to allocate state\n");
> -		return;
> -	}
> -
> -	cstate->base.crtc = crtc;
> -	crtc->state = &cstate->base;
> +	__drm_atomic_helper_crtc_reset(crtc, &cstate->base);
>  }
>  
>  static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index b1da9ce54379..138da66b1530 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -1019,11 +1019,7 @@ static void mdp5_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	mdp5_cstate = kzalloc(sizeof(*mdp5_cstate), GFP_KERNEL);
> -
> -	if (mdp5_cstate) {
> -		mdp5_cstate->base.crtc = crtc;
> -		crtc->state = &mdp5_cstate->base;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &mdp5_cstate->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 4f57e5379796..341600406589 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -418,16 +418,6 @@ nv50_head_atomic_duplicate_state(struct drm_crtc *crtc)
>  	return &asyh->state;
>  }
>  
> -static void
> -__drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
> -			       struct drm_crtc_state *state)
> -{
> -	if (crtc->state)
> -		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> -	crtc->state = state;
> -	crtc->state->crtc = crtc;
> -}
> -
>  static void
>  nv50_head_reset(struct drm_crtc *crtc)
>  {
> @@ -436,6 +426,9 @@ nv50_head_reset(struct drm_crtc *crtc)
>  	if (WARN_ON(!(asyh = kzalloc(sizeof(*asyh), GFP_KERNEL))))
>  		return;
>  
> +	if (crtc->state)
> +		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
> +
>  	__drm_atomic_helper_crtc_reset(crtc, &asyh->state);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index caffc547ef97..9c5f8109913c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -554,14 +554,13 @@ static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
>  
>  static void omap_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct omap_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  
>  	kfree(crtc->state);
> -	crtc->state = kzalloc(sizeof(struct omap_crtc_state), GFP_KERNEL);
> -
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 17741843cf51..7beab22b4674 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -857,14 +857,12 @@ static void rcar_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	__drm_atomic_helper_crtc_reset(crtc, &state->state);
>  	if (state == NULL)
>  		return;
>  
>  	state->crc.source = VSP1_DU_CRC_NONE;
>  	state->crc.index = 0;
> -
> -	crtc->state = &state->state;
> -	crtc->state->crtc = crtc;
>  }

For rcar-du:

Acked-by: Kieran Bingham <[e-mail]>


>  static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..d2d5aa676084 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1071,13 +1071,14 @@ static void vop_crtc_destroy(struct drm_crtc *crtc)
>  
>  static void vop_crtc_reset(struct drm_crtc *crtc)
>  {
> +	struct rockchip_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> +
>  	if (crtc->state)
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  	kfree(crtc->state);
>  
> -	crtc->state = kzalloc(sizeof(struct rockchip_crtc_state), GFP_KERNEL);
> -	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index f80e82e16475..140d6191d8d2 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -1164,10 +1164,7 @@ static void tegra_crtc_reset(struct drm_crtc *crtc)
>  	crtc->state = NULL;
>  
>  	state = kzalloc(sizeof(*state), GFP_KERNEL);
> -	if (state) {
> -		crtc->state = &state->base;
> -		crtc->state->crtc = crtc;
> -	}
> +	__drm_atomic_helper_crtc_reset(crtc, &state->base);
>  
>  	drm_crtc_vblank_reset(crtc);
>  }
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index cd226e63d557..76beeda157fc 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -998,12 +998,12 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
>  static void
>  vc4_crtc_reset(struct drm_crtc *crtc)
>  {
> -	if (crtc->state)
> -		vc4_crtc_destroy_state(crtc->state);
> +	struct vc4_crtc_state *crtc_state =
> +		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
>  
> -	crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
>  	if (crtc->state)
> -		crtc->state->crtc = crtc;
> +		vc4_crtc_destroy_state(crtc, crtc->state);
> +	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
>  }
>  
>  static const struct drm_crtc_funcs vc4_crtc_funcs = {
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 177bbcb38306..7b018f6ee971 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -98,15 +98,10 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
>  		vkms_state = to_vkms_crtc_state(crtc->state);
>  		__drm_atomic_helper_crtc_destroy_state(crtc->state);
>  		kfree(vkms_state);
> -		crtc->state = NULL;
>  	}
>  
>  	vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL);
> -	if (!vkms_state)
> -		return;
> -
> -	crtc->state = &vkms_state->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vkms_state->base);
>  }
>  
>  static struct drm_crtc_state *
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e6b11f6ae2e4..2878bc0a4718 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -633,14 +633,7 @@ void vmw_du_crtc_reset(struct drm_crtc *crtc)
>  	}
>  
>  	vcs = kzalloc(sizeof(*vcs), GFP_KERNEL);
> -
> -	if (!vcs) {
> -		DRM_ERROR("Cannot allocate vmw_crtc_state\n");
> -		return;
> -	}
> -
> -	crtc->state = &vcs->base;
> -	crtc->state->crtc = crtc;
> +	__drm_atomic_helper_crtc_reset(crtc, &vcs->base);
>  }
>  
>  
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index 5b82ccfdb502..ab2b5a49948c 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -37,6 +37,8 @@ struct drm_private_state;
>  struct drm_modeset_acquire_ctx;
>  struct drm_device;
>  
> +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);
>  void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  					      struct drm_crtc_state *state);
>

Patch

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 5064768642f3..770a71726cd1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2802,9 +2802,7 @@  static void dm_crtc_reset_state(struct drm_crtc *crtc)
 	if (WARN_ON(!state))
 		return;
 
-	crtc->state = &state->base;
-	crtc->state->crtc = crtc;
-
+	__drm_atomic_helper_crtc_reset(crtc, &state->base);
 }
 
 static struct drm_crtc_state *
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
index e1b72782848c..9a924ff27148 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -474,10 +474,7 @@  static void malidp_crtc_reset(struct drm_crtc *crtc)
 
 	kfree(state);
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (state) {
-		crtc->state = &state->base;
-		crtc->state->crtc = crtc;
-	}
+	__drm_atomic_helper_crtc_reset(crtc, &state->base);
 }
 
 static void malidp_crtc_destroy_state(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 96f4082671fe..8084d549c7d1 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -412,10 +412,7 @@  static void atmel_hlcdc_crtc_reset(struct drm_crtc *crtc)
 	}
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (state) {
-		crtc->state = &state->base;
-		crtc->state->crtc = crtc;
-	}
+	__drm_atomic_helper_crtc_reset(crtc, &state->base);
 }
 
 static struct drm_crtc_state *
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 3ba996069d69..3eee1ca33fc5 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -55,6 +55,29 @@ 
  * for these functions.
  */
 
+/**
+ * __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_conector->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)
+		crtc_state->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
@@ -64,14 +87,14 @@ 
  */
 void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
 {
+	struct drm_crtc_state *crtc_state =
+		kzalloc(sizeof(*crtc->state), GFP_KERNEL);
+
 	if (crtc->state)
 		__drm_atomic_helper_crtc_destroy_state(crtc->state);
 
 	kfree(crtc->state);
-	crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
-
-	if (crtc->state)
-		crtc->state->crtc = crtc;
+	__drm_atomic_helper_crtc_reset(crtc, crtc_state);
 }
 EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f383417571ec..907ffeb64781 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15457,7 +15457,7 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 		__drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
 		memset(crtc_state, 0, sizeof(*crtc_state));
-		crtc_state->base.crtc = &crtc->base;
+		__drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->base);
 
 		crtc_state->base.active = crtc_state->base.enable =
 			dev_priv->display.get_pipe_config(crtc, crtc_state);
diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
index 7d4b710b837a..8bc36f0d2b6b 100644
--- a/drivers/gpu/drm/imx/ipuv3-crtc.c
+++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
@@ -120,12 +120,9 @@  static void imx_drm_crtc_reset(struct drm_crtc *crtc)
 		memset(state, 0, sizeof(*state));
 	} else {
 		state = kzalloc(sizeof(*state), GFP_KERNEL);
-		if (!state)
-			return;
-		crtc->state = &state->base;
 	}
 
-	state->base.crtc = crtc;
+	__drm_atomic_helper_crtc_reset(crtc, &state->base);
 }
 
 static struct drm_crtc_state *imx_drm_crtc_duplicate_state(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 92ecb9bf982c..a743e5ed1177 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -119,12 +119,9 @@  static void mtk_drm_crtc_reset(struct drm_crtc *crtc)
 		memset(state, 0, sizeof(*state));
 	} else {
 		state = kzalloc(sizeof(*state), GFP_KERNEL);
-		if (!state)
-			return;
-		crtc->state = &state->base;
 	}
 
-	state->base.crtc = crtc;
+	__drm_atomic_helper_crtc_reset(crtc, &state->base);
 }
 
 static struct drm_crtc_state *mtk_drm_crtc_duplicate_state(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index d4530d60767b..c86e603f486a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -894,20 +894,12 @@  static void dpu_crtc_reset(struct drm_crtc *crtc)
 		_dpu_crtc_set_suspend(crtc, false);
 
 	/* remove previous state, if present */
-	if (crtc->state) {
+	if (crtc->state)
 		dpu_crtc_destroy_state(crtc, crtc->state);
-		crtc->state = 0;
-	}
 
 	dpu_crtc = to_dpu_crtc(crtc);
 	cstate = kzalloc(sizeof(*cstate), GFP_KERNEL);
-	if (!cstate) {
-		DPU_ERROR("failed to allocate state\n");
-		return;
-	}
-
-	cstate->base.crtc = crtc;
-	crtc->state = &cstate->base;
+	__drm_atomic_helper_crtc_reset(crtc, &cstate->base);
 }
 
 static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index b1da9ce54379..138da66b1530 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -1019,11 +1019,7 @@  static void mdp5_crtc_reset(struct drm_crtc *crtc)
 	}
 
 	mdp5_cstate = kzalloc(sizeof(*mdp5_cstate), GFP_KERNEL);
-
-	if (mdp5_cstate) {
-		mdp5_cstate->base.crtc = crtc;
-		crtc->state = &mdp5_cstate->base;
-	}
+	__drm_atomic_helper_crtc_reset(crtc, &mdp5_cstate->base);
 }
 
 static struct drm_crtc_state *
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
index 4f57e5379796..341600406589 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -418,16 +418,6 @@  nv50_head_atomic_duplicate_state(struct drm_crtc *crtc)
 	return &asyh->state;
 }
 
-static void
-__drm_atomic_helper_crtc_reset(struct drm_crtc *crtc,
-			       struct drm_crtc_state *state)
-{
-	if (crtc->state)
-		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
-	crtc->state = state;
-	crtc->state->crtc = crtc;
-}
-
 static void
 nv50_head_reset(struct drm_crtc *crtc)
 {
@@ -436,6 +426,9 @@  nv50_head_reset(struct drm_crtc *crtc)
 	if (WARN_ON(!(asyh = kzalloc(sizeof(*asyh), GFP_KERNEL))))
 		return;
 
+	if (crtc->state)
+		crtc->funcs->atomic_destroy_state(crtc, crtc->state);
+
 	__drm_atomic_helper_crtc_reset(crtc, &asyh->state);
 }
 
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index caffc547ef97..9c5f8109913c 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -554,14 +554,13 @@  static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
 
 static void omap_crtc_reset(struct drm_crtc *crtc)
 {
+	struct omap_crtc_state *crtc_state =
+		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
 	if (crtc->state)
 		__drm_atomic_helper_crtc_destroy_state(crtc->state);
 
 	kfree(crtc->state);
-	crtc->state = kzalloc(sizeof(struct omap_crtc_state), GFP_KERNEL);
-
-	if (crtc->state)
-		crtc->state->crtc = crtc;
+	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
 }
 
 static struct drm_crtc_state *
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 17741843cf51..7beab22b4674 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -857,14 +857,12 @@  static void rcar_du_crtc_reset(struct drm_crtc *crtc)
 	}
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	__drm_atomic_helper_crtc_reset(crtc, &state->state);
 	if (state == NULL)
 		return;
 
 	state->crc.source = VSP1_DU_CRC_NONE;
 	state->crc.index = 0;
-
-	crtc->state = &state->state;
-	crtc->state->crtc = crtc;
 }
 
 static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index fb70fb486fbf..d2d5aa676084 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1071,13 +1071,14 @@  static void vop_crtc_destroy(struct drm_crtc *crtc)
 
 static void vop_crtc_reset(struct drm_crtc *crtc)
 {
+	struct rockchip_crtc_state *crtc_state =
+		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
+
 	if (crtc->state)
 		__drm_atomic_helper_crtc_destroy_state(crtc->state);
 	kfree(crtc->state);
 
-	crtc->state = kzalloc(sizeof(struct rockchip_crtc_state), GFP_KERNEL);
-	if (crtc->state)
-		crtc->state->crtc = crtc;
+	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
 }
 
 static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index f80e82e16475..140d6191d8d2 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1164,10 +1164,7 @@  static void tegra_crtc_reset(struct drm_crtc *crtc)
 	crtc->state = NULL;
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (state) {
-		crtc->state = &state->base;
-		crtc->state->crtc = crtc;
-	}
+	__drm_atomic_helper_crtc_reset(crtc, &state->base);
 
 	drm_crtc_vblank_reset(crtc);
 }
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index cd226e63d557..76beeda157fc 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -998,12 +998,12 @@  static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
 static void
 vc4_crtc_reset(struct drm_crtc *crtc)
 {
-	if (crtc->state)
-		vc4_crtc_destroy_state(crtc->state);
+	struct vc4_crtc_state *crtc_state =
+		kzalloc(sizeof(*crtc_state), GFP_KERNEL);
 
-	crtc->state = kzalloc(sizeof(struct vc4_crtc_state), GFP_KERNEL);
 	if (crtc->state)
-		crtc->state->crtc = crtc;
+		vc4_crtc_destroy_state(crtc, crtc->state);
+	__drm_atomic_helper_crtc_reset(crtc, &crtc_state->base);
 }
 
 static const struct drm_crtc_funcs vc4_crtc_funcs = {
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 177bbcb38306..7b018f6ee971 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -98,15 +98,10 @@  static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
 		vkms_state = to_vkms_crtc_state(crtc->state);
 		__drm_atomic_helper_crtc_destroy_state(crtc->state);
 		kfree(vkms_state);
-		crtc->state = NULL;
 	}
 
 	vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL);
-	if (!vkms_state)
-		return;
-
-	crtc->state = &vkms_state->base;
-	crtc->state->crtc = crtc;
+	__drm_atomic_helper_crtc_reset(crtc, &vkms_state->base);
 }
 
 static struct drm_crtc_state *
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index e6b11f6ae2e4..2878bc0a4718 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -633,14 +633,7 @@  void vmw_du_crtc_reset(struct drm_crtc *crtc)
 	}
 
 	vcs = kzalloc(sizeof(*vcs), GFP_KERNEL);
-
-	if (!vcs) {
-		DRM_ERROR("Cannot allocate vmw_crtc_state\n");
-		return;
-	}
-
-	crtc->state = &vcs->base;
-	crtc->state->crtc = crtc;
+	__drm_atomic_helper_crtc_reset(crtc, &vcs->base);
 }
 
 
diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
index 5b82ccfdb502..ab2b5a49948c 100644
--- a/include/drm/drm_atomic_state_helper.h
+++ b/include/drm/drm_atomic_state_helper.h
@@ -37,6 +37,8 @@  struct drm_private_state;
 struct drm_modeset_acquire_ctx;
 struct drm_device;
 
+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);
 void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 					      struct drm_crtc_state *state);