Messages in this thread | | | Date | Tue, 24 Jan 2023 02:21:31 +0200 | Subject | Re: [PATCH Resend v11 05/15] drm/msm/dp: disable self_refresh_aware after entering psr | From | Dmitry Baryshkov <> |
| |
On 19/01/2023 16:26, Vinod Polimera wrote: > From: Sankeerth Billakanti <quic_sbillaka@quicinc.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?
> > Signed-off-by: Sankeerth Billakanti <quic_sbillaka@quicinc.com> > Signed-off-by: Vinod Polimera <quic_vpolimer@quicinc.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
| |