lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 24 Jan 2023 15:09:56 +0000
From:   Vinod Polimera <vpolimer@....qualcomm.com>
To:     "dmitry.baryshkov@...aro.org" <dmitry.baryshkov@...aro.org>,
        "Vinod Polimera (QUIC)" <quic_vpolimer@...cinc.com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        "freedreno@...ts.freedesktop.org" <freedreno@...ts.freedesktop.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
CC:     "Sankeerth Billakanti (QUIC)" <quic_sbillaka@...cinc.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "robdclark@...il.com" <robdclark@...il.com>,
        "dianders@...omium.org" <dianders@...omium.org>,
        "swboyd@...omium.org" <swboyd@...omium.org>,
        "Kalyan Thota (QUIC)" <quic_kalyant@...cinc.com>,
        "Kuogee Hsieh (QUIC)" <quic_khsieh@...cinc.com>,
        "Vishnuvardhan Prodduturi (QUIC)" <quic_vproddut@...cinc.com>,
        "Bjorn Andersson (QUIC)" <quic_bjorande@...cinc.com>,
        "Abhinav Kumar (QUIC)" <quic_abhinavk@...cinc.com>
Subject: RE: [PATCH Resend v11 05/15] drm/msm/dp: disable self_refresh_aware
 after entering psr



> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> Sent: Tuesday, January 24, 2023 5:52 AM
> To: Vinod Polimera (QUIC) <quic_vpolimer@...cinc.com>; dri-
> devel@...ts.freedesktop.org; linux-arm-msm@...r.kernel.org;
> freedreno@...ts.freedesktop.org; devicetree@...r.kernel.org
> Cc: Sankeerth Billakanti (QUIC) <quic_sbillaka@...cinc.com>; linux-
> kernel@...r.kernel.org; robdclark@...il.com; dianders@...omium.org;
> swboyd@...omium.org; Kalyan Thota (QUIC) <quic_kalyant@...cinc.com>;
> Kuogee Hsieh (QUIC) <quic_khsieh@...cinc.com>; Vishnuvardhan
> Prodduturi (QUIC) <quic_vproddut@...cinc.com>; Bjorn Andersson (QUIC)
> <quic_bjorande@...cinc.com>; Abhinav Kumar (QUIC)
> <quic_abhinavk@...cinc.com>
> Subject: Re: [PATCH Resend v11 05/15] drm/msm/dp: disable
> self_refresh_aware after entering psr
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> On 19/01/2023 16:26, Vinod Polimera wrote:
> > From: Sankeerth Billakanti <quic_sbillaka@...cinc.com>
> >
> > Updated frames get queued if self_refresh_aware is set when the
> > sink is in psr. To support bridge enable and avoid queuing of update
> > frames, reset the self_refresh_aware state after entering psr.
> 
> I'm not convinced by this change. E.g. analogix code doesn't do this.
> Could you please clarify, why do you need to toggle the
> self_refresh_aware flag?
> 
This was done to fix a bug reported by google. The use case is as follows:
	CPU was running in a low frequency with debug build.
	When self refresh was triggered by the library, due to system latency, the queued work was not scheduled.
	There in another commit came and reinitialized the timer for the next PSR trigger.
	This sequence happened multiple times  and we found there were multiple works which are stuck and yet to be run.
	As PSR trigger is guarded by self_refresh_aware, we initialized the variable such that, if we are in PSR then until PSR exit, there cannot be any further PSR entry again.
		https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/tags/v5.15.90/drivers/gpu/drm/drm_self_refresh_helper.c#105
	This has solved few flicker issues during the stress testing. 
> >
> > Signed-off-by: Sankeerth Billakanti <quic_sbillaka@...cinc.com>
> > Signed-off-by: Vinod Polimera <quic_vpolimer@...cinc.com>
> > ---
> >   drivers/gpu/drm/msm/dp/dp_drm.c | 27
> ++++++++++++++++++++++++++-
> >   1 file changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c
> b/drivers/gpu/drm/msm/dp/dp_drm.c
> > index 029e08c..92d1a1b 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_drm.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c
> > @@ -134,6 +134,8 @@ static void edp_bridge_atomic_enable(struct
> drm_bridge *drm_bridge,
> >       struct drm_crtc_state *old_crtc_state;
> >       struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge);
> >       struct msm_dp *dp = dp_bridge->dp_display;
> > +     struct drm_connector *connector;
> > +     struct drm_connector_state *conn_state = NULL;
> >
> >       /*
> >        * Check the old state of the crtc to determine if the panel
> > @@ -150,10 +152,22 @@ static void edp_bridge_atomic_enable(struct
> drm_bridge *drm_bridge,
> >
> >       if (old_crtc_state && old_crtc_state->self_refresh_active) {
> >               dp_display_set_psr(dp, false);
> > -             return;
> > +             goto psr_aware;
> >       }
> >
> >       dp_bridge_atomic_enable(drm_bridge, old_bridge_state);
> > +
> > +psr_aware:
> > +     connector =
> drm_atomic_get_new_connector_for_encoder(atomic_state,
> > +                                                     drm_bridge->encoder);
> > +     if (connector)
> > +             conn_state =
> drm_atomic_get_new_connector_state(atomic_state,
> > +                                                             connector);
> > +
> > +     if (conn_state) {
> > +             conn_state->self_refresh_aware = dp->psr_supported;
> > +     }
> 
> No need to wrap a single line statement in brackets.
> 
> > +
> >   }
> >
> >   static void edp_bridge_atomic_disable(struct drm_bridge *drm_bridge,
> > @@ -164,6 +178,14 @@ static void edp_bridge_atomic_disable(struct
> drm_bridge *drm_bridge,
> >       struct drm_crtc_state *new_crtc_state = NULL, *old_crtc_state = NULL;
> >       struct msm_dp_bridge *dp_bridge = to_dp_bridge(drm_bridge);
> >       struct msm_dp *dp = dp_bridge->dp_display;
> > +     struct drm_connector *connector;
> > +     struct drm_connector_state *conn_state = NULL;
> > +
> > +     connector =
> drm_atomic_get_old_connector_for_encoder(atomic_state,
> > +                                                     drm_bridge->encoder);
> > +     if (connector)
> > +             conn_state =
> drm_atomic_get_new_connector_state(atomic_state,
> > +                                                             connector);
> >
> >       crtc = drm_atomic_get_old_crtc_for_encoder(atomic_state,
> >                                                  drm_bridge->encoder);
> > @@ -190,6 +212,9 @@ static void edp_bridge_atomic_disable(struct
> drm_bridge *drm_bridge,
> >        * when display disable occurs while the sink is in psr state.
> >        */
> >       if (new_crtc_state->self_refresh_active) {
> > +             if (conn_state)
> > +                     conn_state->self_refresh_aware = false;
> > +
> >               dp_display_set_psr(dp, true);
> >               return;
> >       } else if (old_crtc_state->self_refresh_active) {
> 
> --
> With best wishes
> Dmitry

Thanks,
Vinod P.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ