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

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 <[e-mail]>
---
 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 <[e-mail]>

Reviewed-by: Dmitry Baryshkov <[e-mail]>

Bjorn Andersson March 26, 2023, 4:27 p.m. UTC | #2

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.



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 <[e-mail]>
> ---
>  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
>

Bjorn Andersson March 26, 2023, 4:35 p.m. UTC | #3

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 <[e-mail]>
> > ---
> >  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
> >

Dmitry Baryshkov March 26, 2023, 10:02 p.m. UTC | #4

On Sun, 26 Mar 2023 at 19:24, Bjorn Andersson <[e-mail]> 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.
>
>
>
> 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.

I hope Vinod Polimera can assist in solving the issue. In the worst
case we will have to revert this commit, shortcutting the PSR until it
is properly debugged.

>
> Regards,
> Bjorn
>
> > Signed-off-by: Vinod Polimera <[e-mail]>
> > ---
> >  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
> >

Stephen Boyd March 27, 2023, 4:27 p.m. UTC | #5

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.

Vinod Polimera March 29, 2023, 3:16 p.m. UTC | #6

> -----Original Message-----
> From: Stephen Boyd <[e-mail]>
> Sent: Monday, March 27, 2023 9:58 PM
> To: Bjorn Andersson <[e-mail]>; Vinod Polimera (QUIC)
> <[e-mail]>
> Cc: [e-mail]; [e-mail];
> [e-mail]; [e-mail]; linux-
> [e-mail]; [e-mail]; [e-mail];
> Kalyan Thota (QUIC) <[e-mail]>;
> [e-mail]; Kuogee Hsieh (QUIC)
> <[e-mail]>; Vishnuvardhan Prodduturi (QUIC)
> <[e-mail]>; Bjorn Andersson (QUIC)
> <[e-mail]>; Abhinav Kumar (QUIC)
> <[e-mail]>; Sankeerth Billakanti (QUIC)
> <[e-mail]>
> 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?

Thanks,
Vinod P.

Doug Anderson March 29, 2023, 3:47 p.m. UTC | #7

Hi,

On Wed, Mar 29, 2023 at 8:16 AM Vinod Polimera
<[e-mail]> wrote:
>
>
>
> > -----Original Message-----
> > From: Stephen Boyd <[e-mail]>
> > Sent: Monday, March 27, 2023 9:58 PM
> > To: Bjorn Andersson <[e-mail]>; Vinod Polimera (QUIC)
> > <[e-mail]>
> > Cc: [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; linux-
> > [e-mail]; [e-mail]; [e-mail];
> > Kalyan Thota (QUIC) <[e-mail]>;
> > [e-mail]; Kuogee Hsieh (QUIC)
> > <[e-mail]>; Vishnuvardhan Prodduturi (QUIC)
> > <[e-mail]>; Bjorn Andersson (QUIC)
> > <[e-mail]>; Abhinav Kumar (QUIC)
> > <[e-mail]>; Sankeerth Billakanti (QUIC)
> > <[e-mail]>
> > 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.

-Doug

Doug Anderson March 30, 2023, 2:23 p.m. UTC | #8

Hi,

On Wed, Mar 29, 2023 at 8:47 AM Doug Anderson <[e-mail]> wrote:
>
> Hi,
>
> On Wed, Mar 29, 2023 at 8:16 AM Vinod Polimera
> <[e-mail]> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Stephen Boyd <[e-mail]>
> > > Sent: Monday, March 27, 2023 9:58 PM
> > > To: Bjorn Andersson <[e-mail]>; Vinod Polimera (QUIC)
> > > <[e-mail]>
> > > Cc: [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]; linux-
> > > [e-mail]; [e-mail]; [e-mail];
> > > Kalyan Thota (QUIC) <[e-mail]>;
> > > [e-mail]; Kuogee Hsieh (QUIC)
> > > <[e-mail]>; Vishnuvardhan Prodduturi (QUIC)
> > > <[e-mail]>; Bjorn Andersson (QUIC)
> > > <[e-mail]>; Abhinav Kumar (QUIC)
> > > <[e-mail]>; Sankeerth Billakanti (QUIC)
> > > <[e-mail]>
> > > 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?

-Doug

Vinod Polimera March 30, 2023, 2:27 p.m. UTC | #9

> -----Original Message-----
> From: Doug Anderson <[e-mail]>
> Sent: Thursday, March 30, 2023 7:54 PM
> To: Vinod Polimera <[e-mail]>
> Cc: Stephen Boyd <[e-mail]>; Bjorn Andersson
> <[e-mail]>; Vinod Polimera (QUIC)
> <[e-mail]>; [e-mail]; dri-
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; linux-
> [e-mail]; Kalyan Thota (QUIC) <[e-mail]>;
> [e-mail]; Kuogee Hsieh (QUIC)
> <[e-mail]>; Vishnuvardhan Prodduturi (QUIC)
> <[e-mail]>; Bjorn Andersson (QUIC)
> <[e-mail]>; Abhinav Kumar (QUIC)
> <[e-mail]>; Sankeerth Billakanti (QUIC)
> <[e-mail]>
> 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
> <[e-mail]> wrote:
> >
> > Hi,
> >
> > On Wed, Mar 29, 2023 at 8:16 AM Vinod Polimera
> > <[e-mail]> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Stephen Boyd <[e-mail]>
> > > > Sent: Monday, March 27, 2023 9:58 PM
> > > > To: Bjorn Andersson <[e-mail]>; Vinod Polimera (QUIC)
> > > > <[e-mail]>
> > > > Cc: [e-mail]; [e-mail];
> > > > [e-mail]; [e-mail]; linux-
> > > > [e-mail]; [e-mail];
> [e-mail];
> > > > Kalyan Thota (QUIC) <[e-mail]>;
> > > > [e-mail]; Kuogee Hsieh (QUIC)
> > > > <[e-mail]>; Vishnuvardhan Prodduturi (QUIC)
> > > > <[e-mail]>; Bjorn Andersson (QUIC)
> > > > <[e-mail]>; Abhinav Kumar (QUIC)
> > > > <[e-mail]>; Sankeerth Billakanti (QUIC)
> > > > <[e-mail]>
> > > > 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

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;