* [PATCH] drm/msm/dpu: Delete bonkers code
@ 2021-04-30 17:17 Rob Clark
2021-04-30 17:37 ` John Stultz
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Rob Clark @ 2021-04-30 17:17 UTC (permalink / raw)
To: dri-devel
Cc: Maxime Ripard, Rob Clark, Stephen Boyd, John Stultz, Rob Clark,
Sean Paul, David Airlie, Daniel Vetter, Abhinav Kumar,
Thomas Zimmermann, Stephen Boyd, Kalyan Thota, Hongbo Yao,
Qinglang Miao, Laurent Pinchart, Lee Jones,
Ville Syrjälä,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVER FOR MSM ADRENO GPU, open list
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>
Reported-by: John Stultz <john.stultz@linaro.org>
Fixes: 37418bf14c13 drm: Use state helper instead of the plane state pointer
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 10 ----------
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 16 ----------------
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 6 ------
3 files changed, 32 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 7c29976be243..18bc76b7f1a3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -648,16 +648,6 @@ static void dpu_crtc_atomic_flush(struct drm_crtc *crtc,
if (unlikely(!cstate->num_mixers))
return;
- /*
- * For planes without commit update, drm framework will not add
- * those planes to current state since hardware update is not
- * required. However, if those planes were power collapsed since
- * last commit cycle, driver has to restore the hardware state
- * of those planes explicitly here prior to plane flush.
- */
- drm_atomic_crtc_for_each_plane(plane, crtc)
- dpu_plane_restore(plane, state);
-
/* update performance setting before crtc kickoff */
dpu_core_perf_crtc_update(crtc, 1, false);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index df7f3d3afd8b..7a993547eb75 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1258,22 +1258,6 @@ static void dpu_plane_atomic_update(struct drm_plane *plane,
}
}
-void dpu_plane_restore(struct drm_plane *plane, struct drm_atomic_state *state)
-{
- struct dpu_plane *pdpu;
-
- if (!plane || !plane->state) {
- DPU_ERROR("invalid plane\n");
- return;
- }
-
- pdpu = to_dpu_plane(plane);
-
- DPU_DEBUG_PLANE(pdpu, "\n");
-
- dpu_plane_atomic_update(plane, state);
-}
-
static void dpu_plane_destroy(struct drm_plane *plane)
{
struct dpu_plane *pdpu = plane ? to_dpu_plane(plane) : NULL;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index 03b6365a750c..34e03ac05f4a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -84,12 +84,6 @@ bool is_dpu_plane_virtual(struct drm_plane *plane);
void dpu_plane_get_ctl_flush(struct drm_plane *plane, struct dpu_hw_ctl *ctl,
u32 *flush_sspp);
-/**
- * dpu_plane_restore - restore hw state if previously power collapsed
- * @plane: Pointer to drm plane structure
- */
-void dpu_plane_restore(struct drm_plane *plane, struct drm_atomic_state *state);
-
/**
* dpu_plane_flush - final plane operations before commit flush
* @plane: Pointer to drm plane structure
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/msm/dpu: Delete bonkers code
2021-04-30 17:17 [PATCH] drm/msm/dpu: Delete bonkers code Rob Clark
@ 2021-04-30 17:37 ` John Stultz
2021-04-30 17:44 ` Stephen Boyd
2021-05-26 19:03 ` patchwork-bot+linux-arm-msm
2 siblings, 0 replies; 6+ messages in thread
From: John Stultz @ 2021-04-30 17:37 UTC (permalink / raw)
To: Rob Clark
Cc: dri-devel, Maxime Ripard, Rob Clark, Stephen Boyd, Sean Paul,
David Airlie, Daniel Vetter, Abhinav Kumar, Thomas Zimmermann,
Stephen Boyd, Kalyan Thota, Hongbo Yao, Qinglang Miao,
Laurent Pinchart, Lee Jones, Ville Syrjälä,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVER FOR MSM ADRENO GPU, open list
On Fri, Apr 30, 2021 at 10:14 AM Rob Clark <robdclark@gmail.com> wrote:
>
> 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.
Thanks! This patch gets things booting again!
Tested-by: John Stultz <john.stultz@linaro.org>
thanks
-john
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/msm/dpu: Delete bonkers code
2021-04-30 17:17 [PATCH] drm/msm/dpu: Delete bonkers code Rob Clark
2021-04-30 17:37 ` John Stultz
@ 2021-04-30 17:44 ` Stephen Boyd
2021-05-03 8:19 ` Maxime Ripard
2021-05-26 19:03 ` patchwork-bot+linux-arm-msm
2 siblings, 1 reply; 6+ messages in thread
From: Stephen Boyd @ 2021-04-30 17:44 UTC (permalink / raw)
To: Rob Clark, dri-devel
Cc: Maxime Ripard, Rob Clark, John Stultz, Sean Paul, David Airlie,
Daniel Vetter, Abhinav Kumar, Thomas Zimmermann, Stephen Boyd,
Kalyan Thota, Hongbo Yao, Qinglang Miao, Laurent Pinchart,
Lee Jones, Ville Syrjälä,
linux-arm-msm, freedreno, linux-kernel
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/msm/dpu: Delete bonkers code
2021-04-30 17:44 ` Stephen Boyd
@ 2021-05-03 8:19 ` Maxime Ripard
0 siblings, 0 replies; 6+ messages in thread
From: Maxime Ripard @ 2021-05-03 8:19 UTC (permalink / raw)
To: Stephen Boyd
Cc: Rob Clark, dri-devel, Rob Clark, John Stultz, Sean Paul,
David Airlie, Daniel Vetter, Abhinav Kumar, Thomas Zimmermann,
Kalyan Thota, Hongbo Yao, Qinglang Miao, Laurent Pinchart,
Lee Jones, Ville Syrjälä,
linux-arm-msm, freedreno, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2563 bytes --]
Hi,
On Fri, Apr 30, 2021 at 10:44:53AM -0700, Stephen Boyd wrote:
> 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.
Thanks for figuring this out, I've applied it with your chromium address
and the proper fixes format.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/msm/dpu: Delete bonkers code
2021-04-30 17:17 [PATCH] drm/msm/dpu: Delete bonkers code Rob Clark
2021-04-30 17:37 ` John Stultz
2021-04-30 17:44 ` Stephen Boyd
@ 2021-05-26 19:03 ` patchwork-bot+linux-arm-msm
2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+linux-arm-msm @ 2021-05-26 19:03 UTC (permalink / raw)
To: Rob Clark; +Cc: linux-arm-msm
Hello:
This patch was applied to qcom/linux.git (refs/heads/for-next):
On Fri, 30 Apr 2021 10:17:39 -0700 you wrote:
> 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:
>
> [...]
Here is the summary with links:
- drm/msm/dpu: Delete bonkers code
https://git.kernel.org/qcom/c/a712b307cfde
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 0/6] iommu/arm-smmu: adreno-smmu page fault handling
@ 2021-06-01 22:47 Rob Clark
2021-06-01 22:47 ` [PATCH] drm/msm/dpu: Delete bonkers code Rob Clark
0 siblings, 1 reply; 6+ messages in thread
From: Rob Clark @ 2021-06-01 22:47 UTC (permalink / raw)
To: dri-devel
Cc: Jordan Crouse, Rob Clark, Abhinav Kumar, Akhil P Oommen,
AngeloGioacchino Del Regno, Bjorn Andersson, Dave Airlie,
Douglas Anderson, Eric Anholt,
open list:DRM DRIVER FOR MSM ADRENO GPU, open list:IOMMU DRIVERS,
Iskren Chernev, Joerg Roedel, Jonathan Marek, Kalyan Thota,
Konrad Dybcio, Krishna Reddy, Kristian H. Kristensen,
Laurent Pinchart, Lee Jones, moderated list:ARM SMMU DRIVERS,
open list:DRM DRIVER FOR MSM ADRENO GPU, open list,
Marijn Suijten, Maxime Ripard, Qinglang Miao, Robin Murphy,
Sai Prakash Ranjan, Sharat Masetty, Stephen Boyd,
Thomas Zimmermann, Ville Syrjälä,
Will Deacon, Zhenzhong Duan
From: Rob Clark <robdclark@chromium.org>
This picks up an earlier series[1] from Jordan, and adds additional
support needed to generate GPU devcore dumps on iova faults. Original
description:
This is a stack to add an Adreno GPU specific handler for pagefaults. The first
patch starts by wiring up report_iommu_fault for arm-smmu. The next patch adds
a adreno-smmu-priv function hook to capture a handful of important debugging
registers such as TTBR0, CONTEXTIDR, FSYNR0 and others. This is used by the
third patch to print more detailed information on page fault such as the TTBR0
for the pagetable that caused the fault and the source of the fault as
determined by a combination of the FSYNR1 register and an internal GPU
register.
This code provides a solid base that we can expand on later for even more
extensive GPU side page fault debugging capabilities.
v4: [Rob] Add support to stall SMMU on fault, and let the GPU driver
resume translation after it has had a chance to snapshot the GPUs
state
v3: Always clear FSR even if the target driver is going to handle resume
v2: Fix comment wording and function pointer check per Rob Clark
[1] https://lore.kernel.org/dri-devel/20210225175135.91922-1-jcrouse@codeaurora.org/
Jordan Crouse (3):
iommu/arm-smmu: Add support for driver IOMMU fault handlers
iommu/arm-smmu-qcom: Add an adreno-smmu-priv callback to get pagefault
info
drm/msm: Improve the a6xx page fault handler
Rob Clark (3):
iommu/arm-smmu-qcom: Add stall support
drm/msm: Add crashdump support for stalled SMMU
drm/msm: devcoredump iommu fault support
drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 2 +-
drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 2 +-
drivers/gpu/drm/msm/adreno/a4xx_gpu.c | 2 +-
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 9 +-
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 101 +++++++++++++++++++-
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 +-
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 43 +++++++--
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 15 +++
drivers/gpu/drm/msm/msm_debugfs.c | 2 +-
drivers/gpu/drm/msm/msm_gem.h | 1 +
drivers/gpu/drm/msm/msm_gem_submit.c | 1 +
drivers/gpu/drm/msm/msm_gpu.c | 55 ++++++++++-
drivers/gpu/drm/msm/msm_gpu.h | 19 +++-
drivers/gpu/drm/msm/msm_gpummu.c | 5 +
drivers/gpu/drm/msm/msm_iommu.c | 22 ++++-
drivers/gpu/drm/msm/msm_mmu.h | 5 +-
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 50 ++++++++++
drivers/iommu/arm/arm-smmu/arm-smmu.c | 9 +-
drivers/iommu/arm/arm-smmu/arm-smmu.h | 2 +
include/linux/adreno-smmu-priv.h | 38 +++++++-
20 files changed, 354 insertions(+), 31 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] drm/msm/dpu: Delete bonkers code
2021-06-01 22:47 [PATCH v4 0/6] iommu/arm-smmu: adreno-smmu page fault handling Rob Clark
@ 2021-06-01 22:47 ` Rob Clark
0 siblings, 0 replies; 6+ messages in thread
From: Rob Clark @ 2021-06-01 22:47 UTC (permalink / raw)
To: dri-devel
Cc: Jordan Crouse, Rob Clark, Stephen Boyd, John Stultz, Rob Clark,
Sean Paul, David Airlie, Daniel Vetter, Abhinav Kumar,
Maxime Ripard, Thomas Zimmermann, Stephen Boyd, Kalyan Thota,
Sakari Ailus, Qinglang Miao, Laurent Pinchart, Lee Jones,
Dmitry Baryshkov, Ville Syrjälä,
open list:DRM DRIVER FOR MSM ADRENO GPU,
open list:DRM DRIVER FOR MSM ADRENO GPU, open list
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>
Reported-by: John Stultz <john.stultz@linaro.org>
Fixes: 37418bf14c13 drm: Use state helper instead of the plane state pointer
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 10 ----------
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 16 ----------------
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 6 ------
3 files changed, 32 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 7c29976be243..18bc76b7f1a3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -648,16 +648,6 @@ static void dpu_crtc_atomic_flush(struct drm_crtc *crtc,
if (unlikely(!cstate->num_mixers))
return;
- /*
- * For planes without commit update, drm framework will not add
- * those planes to current state since hardware update is not
- * required. However, if those planes were power collapsed since
- * last commit cycle, driver has to restore the hardware state
- * of those planes explicitly here prior to plane flush.
- */
- drm_atomic_crtc_for_each_plane(plane, crtc)
- dpu_plane_restore(plane, state);
-
/* update performance setting before crtc kickoff */
dpu_core_perf_crtc_update(crtc, 1, false);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index df7f3d3afd8b..7a993547eb75 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1258,22 +1258,6 @@ static void dpu_plane_atomic_update(struct drm_plane *plane,
}
}
-void dpu_plane_restore(struct drm_plane *plane, struct drm_atomic_state *state)
-{
- struct dpu_plane *pdpu;
-
- if (!plane || !plane->state) {
- DPU_ERROR("invalid plane\n");
- return;
- }
-
- pdpu = to_dpu_plane(plane);
-
- DPU_DEBUG_PLANE(pdpu, "\n");
-
- dpu_plane_atomic_update(plane, state);
-}
-
static void dpu_plane_destroy(struct drm_plane *plane)
{
struct dpu_plane *pdpu = plane ? to_dpu_plane(plane) : NULL;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index 03b6365a750c..34e03ac05f4a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -84,12 +84,6 @@ bool is_dpu_plane_virtual(struct drm_plane *plane);
void dpu_plane_get_ctl_flush(struct drm_plane *plane, struct dpu_hw_ctl *ctl,
u32 *flush_sspp);
-/**
- * dpu_plane_restore - restore hw state if previously power collapsed
- * @plane: Pointer to drm plane structure
- */
-void dpu_plane_restore(struct drm_plane *plane, struct drm_atomic_state *state);
-
/**
* dpu_plane_flush - final plane operations before commit flush
* @plane: Pointer to drm plane structure
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-01 22:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-30 17:17 [PATCH] drm/msm/dpu: Delete bonkers code Rob Clark
2021-04-30 17:37 ` John Stultz
2021-04-30 17:44 ` Stephen Boyd
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).