On 2023-06-21 10:11, Sascha Hauer wrote: > On Tue, Jun 20, 2023 at 06:47:39AM +0000, Jonas Karlman wrote: >> Add missing call to crtc reset helper to properly vblank reset. >> >> Also move vop2_crtc_reset and call vop2_crtc_destroy_state to simplify >> and remove duplicated code. >> >> Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver") >> Signed-off-by: Jonas Karlman <jonas@xxxxxxxxx> >> --- >> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 28 ++++++++------------ >> 1 file changed, 11 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >> index f725487d02ef..1be84fe0208f 100644 >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >> @@ -2080,23 +2080,6 @@ static const struct drm_crtc_helper_funcs vop2_crtc_helper_funcs = { >> .atomic_disable = vop2_crtc_atomic_disable, >> }; >> >> -static void vop2_crtc_reset(struct drm_crtc *crtc) >> -{ >> - struct rockchip_crtc_state *vcstate = to_rockchip_crtc_state(crtc->state); >> - >> - if (crtc->state) { >> - __drm_atomic_helper_crtc_destroy_state(crtc->state); >> - kfree(vcstate); >> - } >> - >> - vcstate = kzalloc(sizeof(*vcstate), GFP_KERNEL); >> - if (!vcstate) >> - return; >> - >> - crtc->state = &vcstate->base; >> - crtc->state->crtc = crtc; >> -} >> - >> static struct drm_crtc_state *vop2_crtc_duplicate_state(struct drm_crtc *crtc) >> { >> struct rockchip_crtc_state *vcstate; >> @@ -2123,6 +2106,17 @@ static void vop2_crtc_destroy_state(struct drm_crtc *crtc, >> kfree(vcstate); >> } >> >> +static void vop2_crtc_reset(struct drm_crtc *crtc) >> +{ >> + struct rockchip_crtc_state *vcstate = >> + kzalloc(sizeof(*vcstate), GFP_KERNEL); >> + >> + if (crtc->state) >> + vop2_crtc_destroy_state(crtc, crtc->state); >> + >> + __drm_atomic_helper_crtc_reset(crtc, &vcstate->base); >> +} > > You missed to check for allocation failures before using vcstate. Good catch, I will fix for both vop and vop2 driver in v2. Regards, Jonas > > Sascha >