On Fri, Oct 30, 2020 at 06:26:06PM +0200, Souza, Jose wrote:
> On Thu, 2020-10-29 at 18:12 +0200, Imre Deak wrote:
> > On Wed, Oct 28, 2020 at 02:07:12PM -0700, José Roberto de Souza wrote:
> > > After commit 00e5deb5c4f5 ("drm/i915: Fix encoder lookup during PSR
> > > atomic check") dig_port was not being used but while fixing it I
> > > realized that would be better to mark all CRTCs that has a eDP
> > > connector as needing to have their state computed.
> > > The principal reason is that in future we will support PSR in
> > > multiple panels.
> > > And this is only forcing the state compute if no register change is
> > > need our atomic handling will just ignore this CRTC + connector
> > > during the atomic commit phase.
> > >
> > > Cc: Imre Deak <imre.deak@xxxxxxxxx>
> > > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_psr.c | 10 ++--------
> > > 1 file changed, 2 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 1576c3722d0b..b5441f0b5b58 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1875,17 +1875,11 @@ void intel_psr_atomic_check(struct drm_connector *connector,
> > > struct drm_connector_state *new_state)
> > > {
> > > struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > > - struct intel_connector *intel_connector;
> >
> > Arg, compiler didn't warn for this.
> >
> > > - struct intel_digital_port *dig_port;
> > > struct drm_crtc_state *crtc_state;
> > >
> > >
> > >
> > >
> > > if (!CAN_PSR(dev_priv) || !new_state->crtc ||
> > > - !dev_priv->psr.force_mode_changed)
> > > - return;
> > > -
> > > - intel_connector = to_intel_connector(connector);
> > > - dig_port = enc_to_dig_port(to_intel_encoder(new_state->best_encoder));
> > > - if (dev_priv->psr.dp != &dig_port->dp)
> > > + !dev_priv->psr.force_mode_changed ||
> > > + connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> > > return;
> >
> > Can't you simplify even more by an is_edp && CAN_PSR check in
> > intel_dp_initial_fastset_check() instead of the psr.force_mode_changed
> > logic?
>
> In the past we ha problems forcing a modeset from the initial commit
> but with this new callback I guess it was fixed.
Ok. Note that now a forced initial modeset can anyway happen due to
other reasons (DSC, wrong DP link rate). Also imo the only way to
guarantee enabling PSR (the purpose of psr.force_mode_changed afaiu) is
to do it in the initial modeset, (no subsequent modeset is guaranteed).
> I sent a test to try-bot, if that work we simply and remove some more
> code around it.
Ok.
> thanks for the headsup.
The fastset hook was just recently added, so thought it's better to have
all similar modeset forcing at one place. Btw, if
intel_pipe_config_compare() wouldn't show a mismatch for the PSR state
(I suppose it doesn't) then intel_crtc_check_fastset() will reset
mode_changed, so probably better to use connectors_changed.
--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
