From: Stephen Boyd <swboyd@chromium.org> To: Rob Clark <robdclark@gmail.com>, dri-devel@lists.freedesktop.org Cc: "Maxime Ripard" <maxime@cerno.tech>, "Rob Clark" <robdclark@chromium.org>, "John Stultz" <john.stultz@linaro.org>, "Sean Paul" <sean@poorly.run>, "David Airlie" <airlied@linux.ie>, "Daniel Vetter" <daniel@ffwll.ch>, "Abhinav Kumar" <abhinavk@codeaurora.org>, "Thomas Zimmermann" <tzimmermann@suse.de>, "Stephen Boyd" <swboyd@chromium.org>, "Kalyan Thota" <kalyan_t@codeaurora.org>, "Hongbo Yao" <yaohongbo@huawei.com>, "Qinglang Miao" <miaoqinglang@huawei.com>, "Laurent Pinchart" <laurent.pinchart@ideasonboard.com>, "Lee Jones" <lee.jones@linaro.org>, "Ville Syrjälä" <ville.syrjala@linux.intel.com>, linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/msm/dpu: Delete bonkers code Date: Fri, 30 Apr 2021 10:44:53 -0700 [thread overview] Message-ID: <CAE-0n513cwqs1c89PZpn0ojuDQ44nwxbRfaYssKHcGwKxK8JdA@mail.gmail.com> (raw) In-Reply-To: <20210430171744.1721408-1-robdclark@gmail.com> Quoting Rob Clark (2021-04-30 10:17:39) > From: Rob Clark <robdclark@chromium.org> > > dpu_crtc_atomic_flush() was directly poking it's attached planes in a > code path that ended up in dpu_plane_atomic_update(), even if the plane > was not involved in the current atomic update. While a bit dubious, > this worked before because plane->state would always point to something > valid. But now using drm_atomic_get_new_plane_state() we could get a > NULL state pointer instead, leading to: > > [ 20.873273] Call trace: > [ 20.875740] dpu_plane_atomic_update+0x5c/0xed0 > [ 20.880311] dpu_plane_restore+0x40/0x88 > [ 20.884266] dpu_crtc_atomic_flush+0xf4/0x208 > [ 20.888660] drm_atomic_helper_commit_planes+0x150/0x238 > [ 20.894014] msm_atomic_commit_tail+0x1d4/0x7a0 > [ 20.898579] commit_tail+0xa4/0x168 > [ 20.902102] drm_atomic_helper_commit+0x164/0x178 > [ 20.906841] drm_atomic_commit+0x54/0x60 > [ 20.910798] drm_atomic_connector_commit_dpms+0x10c/0x118 > [ 20.916236] drm_mode_obj_set_property_ioctl+0x1e4/0x440 > [ 20.921588] drm_connector_property_set_ioctl+0x60/0x88 > [ 20.926852] drm_ioctl_kernel+0xd0/0x120 > [ 20.930807] drm_ioctl+0x21c/0x478 > [ 20.934235] __arm64_sys_ioctl+0xa8/0xe0 > [ 20.938193] invoke_syscall+0x64/0x130 > [ 20.941977] el0_svc_common.constprop.3+0x5c/0xe0 > [ 20.946716] do_el0_svc+0x80/0xa0 > [ 20.950058] el0_svc+0x20/0x30 > [ 20.953145] el0_sync_handler+0x88/0xb0 > [ 20.957014] el0_sync+0x13c/0x140 > > The reason for the codepath seems dubious, the atomic suspend/resume > heplers should handle the power-collapse case. If not, the CRTC's > atomic_check() should be adding the planes to the atomic update. > > Reported-by: Stephen Boyd <sboyd@kernel.org> Maybe better to use swboyd@chromium.org for this one. > Reported-by: John Stultz <john.stultz@linaro.org> > Fixes: 37418bf14c13 drm: Use state helper instead of the plane state pointer Should be Fixes: 37418bf14c13 ("drm: Use state helper instead of the plane state pointer") to match the preferred format. > Signed-off-by: Rob Clark <robdclark@chromium.org> Otherwise looks good, thanks.
WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <swboyd@chromium.org> To: Rob Clark <robdclark@gmail.com>, dri-devel@lists.freedesktop.org Cc: Rob Clark <robdclark@chromium.org>, freedreno@lists.freedesktop.org, Thomas Zimmermann <tzimmermann@suse.de>, David Airlie <airlied@linux.ie>, Lee Jones <lee.jones@linaro.org>, linux-arm-msm@vger.kernel.org, Hongbo Yao <yaohongbo@huawei.com>, linux-kernel@vger.kernel.org, Abhinav Kumar <abhinavk@codeaurora.org>, Stephen Boyd <swboyd@chromium.org>, Qinglang Miao <miaoqinglang@huawei.com>, Maxime Ripard <maxime@cerno.tech>, Kalyan Thota <kalyan_t@codeaurora.org>, Sean Paul <sean@poorly.run>, Laurent Pinchart <laurent.pinchart@ideasonboard.com> Subject: Re: [PATCH] drm/msm/dpu: Delete bonkers code Date: Fri, 30 Apr 2021 10:44:53 -0700 [thread overview] Message-ID: <CAE-0n513cwqs1c89PZpn0ojuDQ44nwxbRfaYssKHcGwKxK8JdA@mail.gmail.com> (raw) In-Reply-To: <20210430171744.1721408-1-robdclark@gmail.com> Quoting Rob Clark (2021-04-30 10:17:39) > From: Rob Clark <robdclark@chromium.org> > > dpu_crtc_atomic_flush() was directly poking it's attached planes in a > code path that ended up in dpu_plane_atomic_update(), even if the plane > was not involved in the current atomic update. While a bit dubious, > this worked before because plane->state would always point to something > valid. But now using drm_atomic_get_new_plane_state() we could get a > NULL state pointer instead, leading to: > > [ 20.873273] Call trace: > [ 20.875740] dpu_plane_atomic_update+0x5c/0xed0 > [ 20.880311] dpu_plane_restore+0x40/0x88 > [ 20.884266] dpu_crtc_atomic_flush+0xf4/0x208 > [ 20.888660] drm_atomic_helper_commit_planes+0x150/0x238 > [ 20.894014] msm_atomic_commit_tail+0x1d4/0x7a0 > [ 20.898579] commit_tail+0xa4/0x168 > [ 20.902102] drm_atomic_helper_commit+0x164/0x178 > [ 20.906841] drm_atomic_commit+0x54/0x60 > [ 20.910798] drm_atomic_connector_commit_dpms+0x10c/0x118 > [ 20.916236] drm_mode_obj_set_property_ioctl+0x1e4/0x440 > [ 20.921588] drm_connector_property_set_ioctl+0x60/0x88 > [ 20.926852] drm_ioctl_kernel+0xd0/0x120 > [ 20.930807] drm_ioctl+0x21c/0x478 > [ 20.934235] __arm64_sys_ioctl+0xa8/0xe0 > [ 20.938193] invoke_syscall+0x64/0x130 > [ 20.941977] el0_svc_common.constprop.3+0x5c/0xe0 > [ 20.946716] do_el0_svc+0x80/0xa0 > [ 20.950058] el0_svc+0x20/0x30 > [ 20.953145] el0_sync_handler+0x88/0xb0 > [ 20.957014] el0_sync+0x13c/0x140 > > The reason for the codepath seems dubious, the atomic suspend/resume > heplers should handle the power-collapse case. If not, the CRTC's > atomic_check() should be adding the planes to the atomic update. > > Reported-by: Stephen Boyd <sboyd@kernel.org> Maybe better to use swboyd@chromium.org for this one. > Reported-by: John Stultz <john.stultz@linaro.org> > Fixes: 37418bf14c13 drm: Use state helper instead of the plane state pointer Should be Fixes: 37418bf14c13 ("drm: Use state helper instead of the plane state pointer") to match the preferred format. > Signed-off-by: Rob Clark <robdclark@chromium.org> Otherwise looks good, thanks. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-04-30 17:44 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-30 17:17 [PATCH] drm/msm/dpu: Delete bonkers code Rob Clark 2021-04-30 17:17 ` Rob Clark 2021-04-30 17:37 ` John Stultz 2021-04-30 17:37 ` John Stultz 2021-04-30 17:44 ` Stephen Boyd [this message] 2021-04-30 17:44 ` Stephen Boyd 2021-05-03 8:19 ` Maxime Ripard 2021-05-03 8:19 ` Maxime Ripard 2021-05-26 19:03 ` patchwork-bot+linux-arm-msm 2021-06-01 22:47 [PATCH v4 0/6] iommu/arm-smmu: adreno-smmu page fault handling Rob Clark 2021-06-01 22:47 ` [PATCH] drm/msm/dpu: Delete bonkers code Rob Clark 2021-06-01 22:47 ` Rob Clark
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=CAE-0n513cwqs1c89PZpn0ojuDQ44nwxbRfaYssKHcGwKxK8JdA@mail.gmail.com \ --to=swboyd@chromium.org \ --cc=abhinavk@codeaurora.org \ --cc=airlied@linux.ie \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=freedreno@lists.freedesktop.org \ --cc=john.stultz@linaro.org \ --cc=kalyan_t@codeaurora.org \ --cc=laurent.pinchart@ideasonboard.com \ --cc=lee.jones@linaro.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=maxime@cerno.tech \ --cc=miaoqinglang@huawei.com \ --cc=robdclark@chromium.org \ --cc=robdclark@gmail.com \ --cc=sean@poorly.run \ --cc=tzimmermann@suse.de \ --cc=ville.syrjala@linux.intel.com \ --cc=yaohongbo@huawei.com \ /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.