From: Enric Balletbo Serra <eballetbo@gmail.com> To: Rob Clark <robdclark@gmail.com> Cc: dri-devel <dri-devel@lists.freedesktop.org>, Rob Clark <robdclark@chromium.org>, David Airlie <airlied@linux.ie>, open list <linux-kernel@vger.kernel.org>, Sean Paul <seanpaul@chromium.org>, Sean Paul <sean@poorly.run> Subject: Re: [PATCH 2/2 v2] drm/atomic: clear new_state pointers at hw_done Date: Tue, 12 Nov 2019 16:51:27 +0100 [thread overview] Message-ID: <CAFqH_52nFGmG-sMb03ByGSJGckT1D_TKWuy1DpkJt2usxHWHpA@mail.gmail.com> (raw) In-Reply-To: <20191104173737.142558-2-robdclark@gmail.com> Hi Rob, Missatge de Rob Clark <robdclark@gmail.com> del dia dl., 4 de nov. 2019 a les 18:42: > > From: Rob Clark <robdclark@chromium.org> > > The new state should not be accessed after this point. Clear the > pointers to make that explicit. > > Signed-off-by: Rob Clark <robdclark@chromium.org> While looking to another issue I applied this patch on top of 5.4-rc7 and my display stopped working. The system gets stuck with the messages below ... [ 17.558689] rockchip-vop ff8f0000.vop: Adding to iommu group 1 [ 17.566014] rk3399-gru-sound sound: ASoC: failed to init link DP: -517 [ 17.567618] rockchip-vop ff900000.vop: Adding to iommu group 2 [ 17.580671] rk3399-gru-sound sound: ASoC: failed to init link DP: -517 [ 17.585996] rockchip-drm display-subsystem: bound ff8f0000.vop (ops vop_component_ops [rockchipdrm]) [ 17.589294] rk3399-gru-sound sound: ASoC: failed to init link DP: -517 [ 17.599899] rockchip-drm display-subsystem: bound ff900000.vop (ops vop_component_ops [rockchipdrm]) [ 17.615846] rockchip-dp ff970000.edp: no DP phy configured [ 17.622495] rockchip-drm display-subsystem: bound ff970000.edp (ops rockchip_dp_component_ops [rockchipdrm]) [ 17.633688] rockchip-drm display-subsystem: bound fec00000.dp (ops cdn_dp_component_ops [rockchipdrm]) [ 17.644141] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [ 17.651548] [drm] No driver support for vblank timestamp query. Not really useful information at this point, but I am wondering if could be that the rockchip driver is doing something wrong more than this patch is wrong? Thanks, Enric > --- > drivers/gpu/drm/drm_atomic_helper.c | 30 +++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 648494c813e5..aec9759d9df2 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2246,12 +2246,42 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank); > */ > void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state) > { > + struct drm_connector *connector; > + struct drm_connector_state *old_conn_state, *new_conn_state; > struct drm_crtc *crtc; > struct drm_crtc_state *old_crtc_state, *new_crtc_state; > + struct drm_plane *plane; > + struct drm_plane_state *old_plane_state, *new_plane_state; > struct drm_crtc_commit *commit; > + struct drm_private_obj *obj; > + struct drm_private_state *old_obj_state, *new_obj_state; > int i; > > + /* > + * After this point, drivers should not access the permanent modeset > + * state, so we also clear the new_state pointers to make this > + * restriction explicit. > + * > + * For the CRTC state, we do this in the same loop where we signal > + * hw_done, since we still need to new_crtc_state to fish out the > + * commit. > + */ > + > + for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) { > + old_state->connectors[i].new_state = NULL; > + } > + > + for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) { > + old_state->planes[i].new_state = NULL; > + } > + > + for_each_oldnew_private_obj_in_state(old_state, obj, old_obj_state, new_obj_state, i) { > + old_state->private_objs[i].new_state = NULL; > + } > + > for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { > + old_state->crtcs[i].new_state = NULL; > + > commit = new_crtc_state->commit; > if (!commit) > continue; > -- > 2.23.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Enric Balletbo Serra <eballetbo@gmail.com> To: Rob Clark <robdclark@gmail.com> Cc: Rob Clark <robdclark@chromium.org>, David Airlie <airlied@linux.ie>, open list <linux-kernel@vger.kernel.org>, dri-devel <dri-devel@lists.freedesktop.org>, Sean Paul <seanpaul@chromium.org>, Sean Paul <sean@poorly.run> Subject: Re: [PATCH 2/2 v2] drm/atomic: clear new_state pointers at hw_done Date: Tue, 12 Nov 2019 16:51:27 +0100 [thread overview] Message-ID: <CAFqH_52nFGmG-sMb03ByGSJGckT1D_TKWuy1DpkJt2usxHWHpA@mail.gmail.com> (raw) In-Reply-To: <20191104173737.142558-2-robdclark@gmail.com> Hi Rob, Missatge de Rob Clark <robdclark@gmail.com> del dia dl., 4 de nov. 2019 a les 18:42: > > From: Rob Clark <robdclark@chromium.org> > > The new state should not be accessed after this point. Clear the > pointers to make that explicit. > > Signed-off-by: Rob Clark <robdclark@chromium.org> While looking to another issue I applied this patch on top of 5.4-rc7 and my display stopped working. The system gets stuck with the messages below ... [ 17.558689] rockchip-vop ff8f0000.vop: Adding to iommu group 1 [ 17.566014] rk3399-gru-sound sound: ASoC: failed to init link DP: -517 [ 17.567618] rockchip-vop ff900000.vop: Adding to iommu group 2 [ 17.580671] rk3399-gru-sound sound: ASoC: failed to init link DP: -517 [ 17.585996] rockchip-drm display-subsystem: bound ff8f0000.vop (ops vop_component_ops [rockchipdrm]) [ 17.589294] rk3399-gru-sound sound: ASoC: failed to init link DP: -517 [ 17.599899] rockchip-drm display-subsystem: bound ff900000.vop (ops vop_component_ops [rockchipdrm]) [ 17.615846] rockchip-dp ff970000.edp: no DP phy configured [ 17.622495] rockchip-drm display-subsystem: bound ff970000.edp (ops rockchip_dp_component_ops [rockchipdrm]) [ 17.633688] rockchip-drm display-subsystem: bound fec00000.dp (ops cdn_dp_component_ops [rockchipdrm]) [ 17.644141] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [ 17.651548] [drm] No driver support for vblank timestamp query. Not really useful information at this point, but I am wondering if could be that the rockchip driver is doing something wrong more than this patch is wrong? Thanks, Enric > --- > drivers/gpu/drm/drm_atomic_helper.c | 30 +++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 648494c813e5..aec9759d9df2 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2246,12 +2246,42 @@ EXPORT_SYMBOL(drm_atomic_helper_fake_vblank); > */ > void drm_atomic_helper_commit_hw_done(struct drm_atomic_state *old_state) > { > + struct drm_connector *connector; > + struct drm_connector_state *old_conn_state, *new_conn_state; > struct drm_crtc *crtc; > struct drm_crtc_state *old_crtc_state, *new_crtc_state; > + struct drm_plane *plane; > + struct drm_plane_state *old_plane_state, *new_plane_state; > struct drm_crtc_commit *commit; > + struct drm_private_obj *obj; > + struct drm_private_state *old_obj_state, *new_obj_state; > int i; > > + /* > + * After this point, drivers should not access the permanent modeset > + * state, so we also clear the new_state pointers to make this > + * restriction explicit. > + * > + * For the CRTC state, we do this in the same loop where we signal > + * hw_done, since we still need to new_crtc_state to fish out the > + * commit. > + */ > + > + for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) { > + old_state->connectors[i].new_state = NULL; > + } > + > + for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) { > + old_state->planes[i].new_state = NULL; > + } > + > + for_each_oldnew_private_obj_in_state(old_state, obj, old_obj_state, new_obj_state, i) { > + old_state->private_objs[i].new_state = NULL; > + } > + > for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { > + old_state->crtcs[i].new_state = NULL; > + > commit = new_crtc_state->commit; > if (!commit) > continue; > -- > 2.23.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-11-12 15:51 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-04 17:37 [PATCH 1/2 v2] drm/atomic: fix self-refresh helpers crtc state dereference Rob Clark 2019-11-04 17:37 ` Rob Clark 2019-11-04 17:37 ` [PATCH 2/2 v2] drm/atomic: clear new_state pointers at hw_done Rob Clark 2019-11-04 17:37 ` Rob Clark 2019-11-07 8:29 ` [drm/atomic] 1c3cd2f4f9: BUG:kernel_NULL_pointer_dereference,address kernel test robot 2019-11-07 8:29 ` [drm/atomic] 1c3cd2f4f9: BUG:kernel_NULL_pointer_dereference, address kernel test robot 2019-11-07 8:29 ` [drm/atomic] 1c3cd2f4f9: BUG:kernel_NULL_pointer_dereference,address kernel test robot 2019-11-12 15:51 ` Enric Balletbo Serra [this message] 2019-11-12 15:51 ` [PATCH 2/2 v2] drm/atomic: clear new_state pointers at hw_done Enric Balletbo Serra 2019-11-12 16:27 ` Rob Clark 2019-11-12 16:27 ` Rob Clark 2019-11-06 3:46 ` [PATCH 1/2 v2] drm/atomic: fix self-refresh helpers crtc state dereference Rob Clark 2019-11-06 3:46 ` Rob Clark 2019-11-06 18:58 ` Sean Paul 2019-11-06 18:58 ` Sean Paul
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAFqH_52nFGmG-sMb03ByGSJGckT1D_TKWuy1DpkJt2usxHWHpA@mail.gmail.com \ --to=eballetbo@gmail.com \ --cc=airlied@linux.ie \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-kernel@vger.kernel.org \ --cc=robdclark@chromium.org \ --cc=robdclark@gmail.com \ --cc=sean@poorly.run \ --cc=seanpaul@chromium.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.