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 <[email protected]>
> Cc: Harry Wentland <[email protected]>
> Cc: Leo Li <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: "Christian König" <[email protected]>
> Cc: "David (ChunMing) Zhou" <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Liviu Dudau <[email protected]>
> Cc: Brian Starkey <[email protected]>
> Cc: Mali DP Maintainers <[email protected]>
> Cc: Boris Brezillon <[email protected]>
> Cc: Nicolas Ferre <[email protected]>
> Cc: Alexandre Belloni <[email protected]>
> Cc: Ludovic Desroches <[email protected]>
> Cc: Maarten Lankhorst <[email protected]>
> Cc: Maxime Ripard <[email protected]>
> Cc: Sean Paul <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Joonas Lahtinen <[email protected]>
> Cc: Rodrigo Vivi <[email protected]>
> Cc: Philipp Zabel <[email protected]>
> Cc: CK Hu <[email protected]>
> Cc: Matthias Brugger <[email protected]>
> Cc: Rob Clark <[email protected]>
> Cc: Ben Skeggs <[email protected]>
> Cc: Tomi Valkeinen <[email protected]>
> Cc: Laurent Pinchart <[email protected]>
> Cc: Kieran Bingham <[email protected]>
> Cc: Sandy Huang <[email protected]>
> Cc: "Heiko Stübner" <[email protected]>
> Cc: Thierry Reding <[email protected]>
> Cc: Jonathan Hunter <[email protected]>
> Cc: Eric Anholt <[email protected]>
> Cc: VMware Graphics <[email protected]>
> Cc: Sinclair Yeh <[email protected]>
> Cc: Thomas Hellstrom <[email protected]>
> Cc: Tony Cheng <[email protected]>
> Cc: Shirish S <[email protected]>
> Cc: Mikita Lipski <[email protected]>
> Cc: Bhawanpreet Lakha <[email protected]>
> Cc: David Francis <[email protected]>
> Cc: Anthony Koo <[email protected]>
> Cc: Jeykumar Sankaran <[email protected]>
> Cc: Jordan Crouse <[email protected]>
> Cc: Bruce Wang <[email protected]>
> Cc: Sravanthi Kollukuduru <[email protected]>
> Cc: Archit Taneja <[email protected]>
> Cc: Steve Kowalik <[email protected]>
> Cc: Carsten Behling <[email protected]>
> Cc: Haneen Mohammed <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Rodrigo Siqueira <[email protected]>
> Cc: Mahesh Kumar <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> .../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.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
