| Message ID | 1654538139-7450-1-git-send-email-quic_khsieh@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/msm/dp: check core_initialized before disable interrupts at dp_display_unbind() | expand |
Quoting Kuogee Hsieh (2022-06-06 10:55:39) > During msm initialize phase, dp_display_unbind() will be called to undo > initializations had been done by dp_display_bind() previously if there is > error happen at msm_drm_bind. In this case, core_initialized flag had to > be check to make sure clocks is on before update DP controller register > to disable HPD interrupts. Otherwise system will crash due to below NOC > fatal error. > > QTISECLIB [01f01a7ad]CNOC2 ERROR: ERRLOG0_LOW = 0x00061007 > QTISECLIB [01f01a7ad]GEM_NOC ERROR: ERRLOG0_LOW = 0x00001007 > QTISECLIB [01f0371a0]CNOC2 ERROR: ERRLOG0_HIGH = 0x00000003 > QTISECLIB [01f055297]GEM_NOC ERROR: ERRLOG0_HIGH = 0x00000003 > QTISECLIB [01f072beb]CNOC2 ERROR: ERRLOG1_LOW = 0x00000024 > QTISECLIB [01f0914b8]GEM_NOC ERROR: ERRLOG1_LOW = 0x00000042 > QTISECLIB [01f0ae639]CNOC2 ERROR: ERRLOG1_HIGH = 0x00004002 > QTISECLIB [01f0cc73f]GEM_NOC ERROR: ERRLOG1_HIGH = 0x00004002 > QTISECLIB [01f0ea092]CNOC2 ERROR: ERRLOG2_LOW = 0x0009020c > QTISECLIB [01f10895f]GEM_NOC ERROR: ERRLOG2_LOW = 0x0ae9020c > QTISECLIB [01f125ae1]CNOC2 ERROR: ERRLOG2_HIGH = 0x00000000 > QTISECLIB [01f143be7]GEM_NOC ERROR: ERRLOG2_HIGH = 0x00000000 > QTISECLIB [01f16153a]CNOC2 ERROR: ERRLOG3_LOW = 0x00000000 > QTISECLIB [01f17fe07]GEM_NOC ERROR: ERRLOG3_LOW = 0x00000000 > QTISECLIB [01f19cf89]CNOC2 ERROR: ERRLOG3_HIGH = 0x00000000 > QTISECLIB [01f1bb08e]GEM_NOC ERROR: ERRLOG3_HIGH = 0x00000000 > QTISECLIB [01f1d8a31]CNOC2 ERROR: SBM1 FAULTINSTATUS0_LOW = 0x00000002 > QTISECLIB [01f1f72a4]GEM_NOC ERROR: SBM0 FAULTINSTATUS0_LOW = 0x00000001 > QTISECLIB [01f21a217]CNOC3 ERROR: ERRLOG0_LOW = 0x00000006 > QTISECLIB [01f23dfd3]NOC error fatal Don't think this pile of lines really helps anyone, but OK. > > changes in v2: > -- drop the first patch (drm/msm: enable msm irq after all initializations are done successfully at msm_drm_init()) since the problem had been fixed by other patch > > Fixes: a65c95ff88f2 ("drm/msm/dp: stop event kernel thread when DP unbind") > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
On 06/06/2022 20:55, Kuogee Hsieh wrote: > During msm initialize phase, dp_display_unbind() will be called to undo > initializations had been done by dp_display_bind() previously if there is > error happen at msm_drm_bind. In this case, core_initialized flag had to > be check to make sure clocks is on before update DP controller register > to disable HPD interrupts. Otherwise system will crash due to below NOC > fatal error. > > QTISECLIB [01f01a7ad]CNOC2 ERROR: ERRLOG0_LOW = 0x00061007 > QTISECLIB [01f01a7ad]GEM_NOC ERROR: ERRLOG0_LOW = 0x00001007 > QTISECLIB [01f0371a0]CNOC2 ERROR: ERRLOG0_HIGH = 0x00000003 > QTISECLIB [01f055297]GEM_NOC ERROR: ERRLOG0_HIGH = 0x00000003 > QTISECLIB [01f072beb]CNOC2 ERROR: ERRLOG1_LOW = 0x00000024 > QTISECLIB [01f0914b8]GEM_NOC ERROR: ERRLOG1_LOW = 0x00000042 > QTISECLIB [01f0ae639]CNOC2 ERROR: ERRLOG1_HIGH = 0x00004002 > QTISECLIB [01f0cc73f]GEM_NOC ERROR: ERRLOG1_HIGH = 0x00004002 > QTISECLIB [01f0ea092]CNOC2 ERROR: ERRLOG2_LOW = 0x0009020c > QTISECLIB [01f10895f]GEM_NOC ERROR: ERRLOG2_LOW = 0x0ae9020c > QTISECLIB [01f125ae1]CNOC2 ERROR: ERRLOG2_HIGH = 0x00000000 > QTISECLIB [01f143be7]GEM_NOC ERROR: ERRLOG2_HIGH = 0x00000000 > QTISECLIB [01f16153a]CNOC2 ERROR: ERRLOG3_LOW = 0x00000000 > QTISECLIB [01f17fe07]GEM_NOC ERROR: ERRLOG3_LOW = 0x00000000 > QTISECLIB [01f19cf89]CNOC2 ERROR: ERRLOG3_HIGH = 0x00000000 > QTISECLIB [01f1bb08e]GEM_NOC ERROR: ERRLOG3_HIGH = 0x00000000 > QTISECLIB [01f1d8a31]CNOC2 ERROR: SBM1 FAULTINSTATUS0_LOW = 0x00000002 > QTISECLIB [01f1f72a4]GEM_NOC ERROR: SBM0 FAULTINSTATUS0_LOW = 0x00000001 > QTISECLIB [01f21a217]CNOC3 ERROR: ERRLOG0_LOW = 0x00000006 > QTISECLIB [01f23dfd3]NOC error fatal > > changes in v2: > -- drop the first patch (drm/msm: enable msm irq after all initializations are done successfully at msm_drm_init()) since the problem had been fixed by other patch > > Fixes: a65c95ff88f2 ("drm/msm/dp: stop event kernel thread when DP unbind") Please. Stop using commit IDs from the private git tree. I thought we have stumbled upon this during the last cycle, thus we won't see rogue commit IDs anymore. But I was wrong. I've got another complaint from Stephen. I'm going to fix this one manually, but please make sure to stop sending wrong Fixes tags in the future. Generally I feel like this is a sign of an incorrect process on your side. The patches should be developed and tested against one of upstream trees (linux-next, msm-next or msm-next-lumag) and then backported to your integration/backports/stable/whatever. Since today the msm-next-lumag is included into the linux-next, so you can use linux-next as a basis. > Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> > --- > drivers/gpu/drm/msm/dp/dp_display.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > index da5c03a..2b72639 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -309,7 +309,8 @@ static void dp_display_unbind(struct device *dev, struct device *master, > struct msm_drm_private *priv = dev_get_drvdata(master); > > /* disable all HPD interrupts */ > - dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false); > + if (dp->core_initialized) > + dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false); > > kthread_stop(dp->ev_tsk); >
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index da5c03a..2b72639 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -309,7 +309,8 @@ static void dp_display_unbind(struct device *dev, struct device *master, struct msm_drm_private *priv = dev_get_drvdata(master); /* disable all HPD interrupts */ - dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false); + if (dp->core_initialized) + dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false); kthread_stop(dp->ev_tsk);
During msm initialize phase, dp_display_unbind() will be called to undo initializations had been done by dp_display_bind() previously if there is error happen at msm_drm_bind. In this case, core_initialized flag had to be check to make sure clocks is on before update DP controller register to disable HPD interrupts. Otherwise system will crash due to below NOC fatal error. QTISECLIB [01f01a7ad]CNOC2 ERROR: ERRLOG0_LOW = 0x00061007 QTISECLIB [01f01a7ad]GEM_NOC ERROR: ERRLOG0_LOW = 0x00001007 QTISECLIB [01f0371a0]CNOC2 ERROR: ERRLOG0_HIGH = 0x00000003 QTISECLIB [01f055297]GEM_NOC ERROR: ERRLOG0_HIGH = 0x00000003 QTISECLIB [01f072beb]CNOC2 ERROR: ERRLOG1_LOW = 0x00000024 QTISECLIB [01f0914b8]GEM_NOC ERROR: ERRLOG1_LOW = 0x00000042 QTISECLIB [01f0ae639]CNOC2 ERROR: ERRLOG1_HIGH = 0x00004002 QTISECLIB [01f0cc73f]GEM_NOC ERROR: ERRLOG1_HIGH = 0x00004002 QTISECLIB [01f0ea092]CNOC2 ERROR: ERRLOG2_LOW = 0x0009020c QTISECLIB [01f10895f]GEM_NOC ERROR: ERRLOG2_LOW = 0x0ae9020c QTISECLIB [01f125ae1]CNOC2 ERROR: ERRLOG2_HIGH = 0x00000000 QTISECLIB [01f143be7]GEM_NOC ERROR: ERRLOG2_HIGH = 0x00000000 QTISECLIB [01f16153a]CNOC2 ERROR: ERRLOG3_LOW = 0x00000000 QTISECLIB [01f17fe07]GEM_NOC ERROR: ERRLOG3_LOW = 0x00000000 QTISECLIB [01f19cf89]CNOC2 ERROR: ERRLOG3_HIGH = 0x00000000 QTISECLIB [01f1bb08e]GEM_NOC ERROR: ERRLOG3_HIGH = 0x00000000 QTISECLIB [01f1d8a31]CNOC2 ERROR: SBM1 FAULTINSTATUS0_LOW = 0x00000002 QTISECLIB [01f1f72a4]GEM_NOC ERROR: SBM0 FAULTINSTATUS0_LOW = 0x00000001 QTISECLIB [01f21a217]CNOC3 ERROR: ERRLOG0_LOW = 0x00000006 QTISECLIB [01f23dfd3]NOC error fatal changes in v2: -- drop the first patch (drm/msm: enable msm irq after all initializations are done successfully at msm_drm_init()) since the problem had been fixed by other patch Fixes: a65c95ff88f2 ("drm/msm/dp: stop event kernel thread when DP unbind") Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com> --- drivers/gpu/drm/msm/dp/dp_display.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)