diff mbox series

[v14,14/14] drm/msm/dp: set self refresh aware based on PSR support

Message ID 1677774797-31063-15-git-send-email-quic_vpolimer@quicinc.com
State Accepted
Commit 1844e680d56bb0c4e0489138f2b7ba2dc1c988e3
Headers show
Series Add PSR support for eDP | expand

Commit Message

Vinod Polimera March 2, 2023, 4:33 p.m. UTC
For the PSR to kick in, self_refresh_aware has to be set.
Initialize it based on the PSR support for the eDP interface.

Signed-off-by: Vinod Polimera <quic_vpolimer@quicinc.com>
---
 drivers/gpu/drm/msm/dp/dp_drm.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Dmitry Baryshkov March 2, 2023, 4:40 p.m. UTC | #1
On 02/03/2023 18:33, Vinod Polimera wrote:
> For the PSR to kick in, self_refresh_aware has to be set.
> Initialize it based on the PSR support for the eDP interface.
> 
> Signed-off-by: Vinod Polimera <quic_vpolimer@quicinc.com>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Bjorn Andersson March 26, 2023, 4:35 p.m. UTC | #2
On Sun, Mar 26, 2023 at 09:27:23AM -0700, Bjorn Andersson wrote:
> On Thu, Mar 02, 2023 at 10:03:17PM +0530, Vinod Polimera wrote:
> > For the PSR to kick in, self_refresh_aware has to be set.
> > Initialize it based on the PSR support for the eDP interface.
> > 
> 
> When I boot my sc8280xp devices (CRD and X13s) to console with this
> patch included I get a login prompt, and then there are no more screen
> updates.
> 
> Switching virtual terminal (ctrl+alt+fN) causes the screen to redraw.
> 
> Blindly login in and launching Wayland works and from then on screen
> updates works as expected.
> 
> Switching from Wayland to another virtual terminal causes the problem to
> re-appear, no updates after the initial refresh, switching back go the
> Wayland-terminal crashed the machine.
> 

Also, trying to bring the eDP-screen back from DPMS gives me:

<3>[ 2355.218099] [drm:dp_catalog_ctrl_set_pattern_state_bit [msm]] *ERROR* set state_bit for link_train=1 failed
<3>[ 2355.218926] [drm:dp_ctrl_setup_main_link [msm]] *ERROR* link training #1 failed. ret=-110
<3>[ 2355.262859] [drm:dp_catalog_ctrl_set_pattern_state_bit [msm]] *ERROR* set state_bit for link_train=1 failed
<3>[ 2355.263600] [drm:dp_ctrl_setup_main_link [msm]] *ERROR* link training #1 failed. ret=-110
<3>[ 2355.305211] [drm:dp_catalog_ctrl_set_pattern_state_bit [msm]] *ERROR* set state_bit for link_train=1 failed
<3>[ 2355.305955] [drm:dp_ctrl_setup_main_link [msm]] *ERROR* link training #1 failed. ret=-110
<3>[ 2355.345250] [drm:dp_catalog_ctrl_set_pattern_state_bit [msm]] *ERROR* set state_bit for link_train=1 failed
<3>[ 2355.346026] [drm:dp_ctrl_setup_main_link [msm]] *ERROR* link training #1 failed. ret=-110
<3>[ 2355.405650] [drm:dp_display_process_hpd_high [msm]] *ERROR* failed to complete DP link training
<3>[ 2355.668988] [drm:dpu_encoder_phys_vid_wait_for_commit_done:488] [dpu error]vblank timeout
<3>[ 2355.669030] [drm:dpu_kms_wait_for_commit_done:510] [dpu error]wait for commit done returned -110
<3>[ 2355.699989] [drm:dpu_encoder_frame_done_timeout:2398] [dpu error]enc35 frame done timeout

And then the machine just resets.

Regards,
Bjorn

> 
> 
> Reverting this single patch resolves both the issue with the console
> updating as exected and flipping between the virtual terminal with
> Wayland and the others no longer crashes my machine.
> 
> Regards,
> Bjorn
> 
> > Signed-off-by: Vinod Polimera <quic_vpolimer@quicinc.com>
> > ---
> >  drivers/gpu/drm/msm/dp/dp_drm.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
> > index 029e08c..785d766 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> > @@ -117,6 +117,8 @@ static int edp_bridge_atomic_check(struct drm_bridge *drm_bridge,
> >  	if (WARN_ON(!conn_state))
> >  		return -ENODEV;
> >  
> > +	conn_state->self_refresh_aware = dp->psr_supported;
> > +
> >  	if (!conn_state->crtc || !crtc_state)
> >  		return 0;
> >  
> > -- 
> > 2.7.4
> >
Vinod Polimera March 30, 2023, 2:27 p.m. UTC | #3
> -----Original Message-----
> From: Doug Anderson <dianders@chromium.org>
> Sent: Thursday, March 30, 2023 7:54 PM
> To: Vinod Polimera <vpolimer@qti.qualcomm.com>
> Cc: Stephen Boyd <swboyd@chromium.org>; Bjorn Andersson
> <andersson@kernel.org>; Vinod Polimera (QUIC)
> <quic_vpolimer@quicinc.com>; robdclark@gmail.com; dri-
> devel@lists.freedesktop.org; linux-arm-msm@vger.kernel.org;
> freedreno@lists.freedesktop.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Kalyan Thota (QUIC) <quic_kalyant@quicinc.com>;
> dmitry.baryshkov@linaro.org; Kuogee Hsieh (QUIC)
> <quic_khsieh@quicinc.com>; Vishnuvardhan Prodduturi (QUIC)
> <quic_vproddut@quicinc.com>; Bjorn Andersson (QUIC)
> <quic_bjorande@quicinc.com>; Abhinav Kumar (QUIC)
> <quic_abhinavk@quicinc.com>; Sankeerth Billakanti (QUIC)
> <quic_sbillaka@quicinc.com>
> Subject: Re: [PATCH v14 14/14] drm/msm/dp: set self refresh aware based
> on PSR support
> 
> 
> Hi,
> 
> On Wed, Mar 29, 2023 at 8:47 AM Doug Anderson
> <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Wed, Mar 29, 2023 at 8:16 AM Vinod Polimera
> > <vpolimer@qti.qualcomm.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Stephen Boyd <swboyd@chromium.org>
> > > > Sent: Monday, March 27, 2023 9:58 PM
> > > > To: Bjorn Andersson <andersson@kernel.org>; Vinod Polimera (QUIC)
> > > > <quic_vpolimer@quicinc.com>
> > > > Cc: dri-devel@lists.freedesktop.org; linux-arm-msm@vger.kernel.org;
> > > > freedreno@lists.freedesktop.org; devicetree@vger.kernel.org; linux-
> > > > kernel@vger.kernel.org; robdclark@gmail.com;
> dianders@chromium.org;
> > > > Kalyan Thota (QUIC) <quic_kalyant@quicinc.com>;
> > > > dmitry.baryshkov@linaro.org; Kuogee Hsieh (QUIC)
> > > > <quic_khsieh@quicinc.com>; Vishnuvardhan Prodduturi (QUIC)
> > > > <quic_vproddut@quicinc.com>; Bjorn Andersson (QUIC)
> > > > <quic_bjorande@quicinc.com>; Abhinav Kumar (QUIC)
> > > > <quic_abhinavk@quicinc.com>; Sankeerth Billakanti (QUIC)
> > > > <quic_sbillaka@quicinc.com>
> > > > Subject: Re: [PATCH v14 14/14] drm/msm/dp: set self refresh aware
> based
> > > > on PSR support
> > > >
> > > > Quoting Bjorn Andersson (2023-03-26 09:35:56)
> > > > > On Sun, Mar 26, 2023 at 09:27:23AM -0700, Bjorn Andersson wrote:
> > > > > > On Thu, Mar 02, 2023 at 10:03:17PM +0530, Vinod Polimera wrote:
> > > > > > > For the PSR to kick in, self_refresh_aware has to be set.
> > > > > > > Initialize it based on the PSR support for the eDP interface.
> > > > > > >
> > > > > >
> > > > > > When I boot my sc8280xp devices (CRD and X13s) to console with
> this
> > > > > > patch included I get a login prompt, and then there are no more
> screen
> > > > > > updates.
> > > > > >
> > > > > > Switching virtual terminal (ctrl+alt+fN) causes the screen to redraw.
> > > > > >
> > > > > > Blindly login in and launching Wayland works and from then on
> screen
> > > > > > updates works as expected.
> > > > > >
> > > > > > Switching from Wayland to another virtual terminal causes the
> problem
> > > > to
> > > > > > re-appear, no updates after the initial refresh, switching back go the
> > > > > > Wayland-terminal crashed the machine.
> > > > > >
> > > > >
> > > > > Also, trying to bring the eDP-screen back from DPMS gives me:
> > > > >
> > > > > <3>[ 2355.218099] [drm:dp_catalog_ctrl_set_pattern_state_bit
> [msm]]
> > > > *ERROR* set state_bit for link_train=1 failed
> > > > > <3>[ 2355.218926] [drm:dp_ctrl_setup_main_link [msm]] *ERROR*
> link
> > > > training #1 failed. ret=-110
> > > > > <3>[ 2355.262859] [drm:dp_catalog_ctrl_set_pattern_state_bit
> [msm]]
> > > > *ERROR* set state_bit for link_train=1 failed
> > > > > <3>[ 2355.263600] [drm:dp_ctrl_setup_main_link [msm]] *ERROR*
> link
> > > > training #1 failed. ret=-110
> > > > > <3>[ 2355.305211] [drm:dp_catalog_ctrl_set_pattern_state_bit
> [msm]]
> > > > *ERROR* set state_bit for link_train=1 failed
> > > > > <3>[ 2355.305955] [drm:dp_ctrl_setup_main_link [msm]] *ERROR*
> link
> > > > training #1 failed. ret=-110
> > > > > <3>[ 2355.345250] [drm:dp_catalog_ctrl_set_pattern_state_bit
> [msm]]
> > > > *ERROR* set state_bit for link_train=1 failed
> > > > > <3>[ 2355.346026] [drm:dp_ctrl_setup_main_link [msm]] *ERROR*
> link
> > > > training #1 failed. ret=-110
> > > > > <3>[ 2355.405650] [drm:dp_display_process_hpd_high [msm]]
> *ERROR*
> > > > failed to complete DP link training
> > > > > <3>[ 2355.668988]
> > > > [drm:dpu_encoder_phys_vid_wait_for_commit_done:488] [dpu
> > > > error]vblank timeout
> > > > > <3>[ 2355.669030] [drm:dpu_kms_wait_for_commit_done:510] [dpu
> > > > error]wait for commit done returned -110
> > > > > <3>[ 2355.699989] [drm:dpu_encoder_frame_done_timeout:2398]
> [dpu
> > > > error]enc35 frame done timeout
> > > > >
> > > > > And then the machine just resets.
> > > > >
> > > >
> > > > I saw similar behavior on ChromeOS after we picked the PSR patches
> into
> > > > our kernel. I suspect it's the same problem. I switched back and forth
> > > > between VT2 and the OOBE screen with ctrl+alt+forward and that
> showed
> > > > what I typed on the virtual terminal after switching back and forth.
> > > > It's like the redraw only happens once on the switch and never again. I
> > > > switched back and forth enough times that it eventually crashed the
> > > > kernel and rebooted. This was on CRD (sc7280-herobrine-crd.dts).
> > > >
> > > > There's an animation on the OOBE screen that is working though, so
> > > > perhaps PSR is working with the chrome compositor but not the virtual
> > > > terminal? I haven't investigated.
> > >
> > > I was able to reproduce the issue where in virtual terminal, I don't see
> any screen refresh despite typing in.
> > > In the VT mode, I see that PSR is entered, but despite typing in there are
> no atomic commits triggered, hence the last buffer was always refreshed.
> > >
> > > Queries from my side to Rob & Doug:
> > > 1) In VT mode, does the framework operates in single buffer mode
> without any commit for new updates ?
> > > 2) if above is true then, how does driver know if the framework operates
> in single buffer mode, to make any appropriate action
> > > 3) what is the expected behavior with the driver here ? should it return
> atomic_check failure, for single buffer mode operation or, it should exit PSR ?
> > > 4) is there any HINT passed down to the driver so that we can bank on it
> and act accordingly?
> >
> > I haven't looked at this detail about PSR before, and I left my
> > PSR-enabled device at home so I can't easily test this right now. That
> > being said, from a bit of searching I would guess that
> > msm_framebuffer_dirtyfb() is somehow involved here. Are things better
> > if you get rid of the test against 'msm_fb->dirtyfb'?
> >
> > I at least used ftrace to confirm that on a different device
> > msm_framebuffer_dirtyfb() is not called during normal ChromeOS usage
> > but it _is_ called in VT2 usage.
> 
> Indeed, I can confirm that if I comment out the test in
> msm_framebuffer_dirtyfb() and just call straight through to
> drm_atomic_helper_dirtyfb() that typing on VT2 works fine.
> 
> ...so presumably you need to figure out how to get "dirtyfb" set
> properly when you have a PSR-enabled panel or maybe whenever the panel
> enters PSR mode?
> 
I have also observed the same and I am working on fixing it.

Thanks,
Vinod P.

> -Doug
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 029e08c..785d766 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -117,6 +117,8 @@  static int edp_bridge_atomic_check(struct drm_bridge *drm_bridge,
 	if (WARN_ON(!conn_state))
 		return -ENODEV;
 
+	conn_state->self_refresh_aware = dp->psr_supported;
+
 	if (!conn_state->crtc || !crtc_state)
 		return 0;