Re: [Intel-gfx] [PATCH v3] drm/i915/psr: Force PSR probe only after full initialization
On Fri, 2020-02-21 at 10:15 -0800, Souza, Jose wrote:
> On Fri, 2020-02-21 at 15:46 +, Mun, Gwan-gyeong wrote:
> > On Thu, 2020-02-20 at 12:55 -0800, Souza, Jose wrote:
> > > On Thu, 2020-02-20 at 12:39 +, Mun, Gwan-gyeong wrote:
> > > > On Tue, 2020-02-18 at 12:39 -0800, José Roberto de Souza wrote:
> > > > > Commit 60c6a14b489b ("drm/i915/display: Force the state
> > > > > compute
> > > > > phase
> > > > > once to enable PSR") was forcing the state compute too
> > > > > earlier
> > > > > causing errors because not everything was initialized, so
> > > > > here
> > > > > moving to i915_driver_register() when everything is ready and
> > > > > driver
> > > > > is registering into the rest of the system.
> > > > >
> > > > > Also fixing the place where it disarm the force probe as
> > > > > during
> > > > > the
> > > > > atomic check phase errors could happen like the ones due
> > > > > locking
> > > > > and
> > > > > it would cause PSR to never be enabled if that happens.
> > > > > Leaving the disarm to the atomic commit phase,
> > > > > intel_psr_enable()
> > > > > or
> > > > > intel_psr_update() will be called even if the current state
> > > > > do
> > > > > not
> > > > > allow PSR to be enabled.
> > > > >
> > > > > v2: Check if intel_dp is null in
> > > > > intel_psr_force_mode_changed_set()
> > > > > v3: Check intel_dp before get dev_priv
> > > > >
> > > > > Fixes: 60c6a14b489b ("drm/i915/display: Force the state
> > > > > compute
> > > > > phase
> > > > > once to enable PSR")
> > > > > Closes: https://gitlab.freedesktop.org/drm/intel/issues/1151
> > > > > Tested-by: Ross Zwisler
> > > > > Reported-by: Ross Zwisler
> > > > > Cc: Gwan-gyeong Mun
> > > > > Cc: Jani Nikula
> > > > > Signed-off-by: José Roberto de Souza
> > > > > ---
> > > > > drivers/gpu/drm/i915/display/intel_psr.c | 22
> > > > > --
> > > > > drivers/gpu/drm/i915/display/intel_psr.h | 1 +
> > > > > drivers/gpu/drm/i915/i915_drv.c | 3 +++
> > > > > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> > > > > 4 files changed, 25 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > index b4942b6445ae..2a0f7354fba5 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > @@ -936,6 +936,8 @@ void intel_psr_enable(struct intel_dp
> > > > > *intel_dp,
> > > > > {
> > > > > struct drm_i915_private *dev_priv =
> > > > > dp_to_i915(intel_dp);
> > > > >
> > > > > + intel_psr_force_mode_changed_set(intel_dp, false);
> > > > > +
> > > > Hi,
> > > > intel_psr_enable() and intel_psr_update already have checking
> > > > routine
> > > > for CAN_PSR and has_psr.
> > > > therefore we don't need to check twice here.
> > >
> > > Minor overhead but if you really want I can remove the function
> > > call
> > > and just do a dev_priv->psr.force_mode_changed = false; for
> > > intel_psr_enable/update
> > >
> > > > And if there are no issues that moving "disarming
> > > > force_mode_changed"
> > > > to intel_psr_compute_config(),
> > > > can we move them to intel_psr_compute_config()?
> > >
> > > atomic check can fail at any point so we could disarm the
> > > mode_changed,
> > > fail, retry(because the return was EAGAIN) and then PSR will not
> > > be
> > > enabled.
> > >
> > If disarming the "force_mode_changed" would be handled on
> > intel_psr_compute_config(),
> > (after failing atomic check and)the retry step will set
> > "crtc_state-
> > > mode_changed = true" on
> > intel_digital_connector_atomic_check(). ( because the
> > force_mode_changed is not disabled yet.)
> >
> > The mode_changed will lead "encoder->compute_config" which will
> > call
> > intel_psr_compute_config().
> > And we can disable "force_mode_changed" on
> > intel_psr_compute_config()
> > which sets "crtc_state->has_psr = true".
> > the "crtc_state->has_psr" enables PSR.
>
> After call encoder->compute_config()->intel_psr_compute_config()
> there
> is a lot of code left to be executed in intel_atomic_check() that can
> cause the atomic check to fail.
> The next pipe in this loop can already cause that.
Hi Jose,
Thank you for explaining in detail.
>
> > > > > if (!crtc_state->has_psr)
> > > > > return;
> > > > >
> > > > > @@ -1096,6 +1098,8 @@ void intel_psr_update(struct intel_dp
> > > > > *intel_dp,
> > > > > struct i915_psr *psr = &dev_priv->psr;
> > > > > bool enable, psr2_enable;
> > > > >
> > > > > + intel_psr_force_mode_changed_set(intel_dp, false);
> > > > > +
> > > > > if (!CAN_PSR(dev_priv) || READ_ONCE(psr->dp) !=
> > > > > intel_dp)
> > > > > return;
> > > > >
> > > > > @@ -1629,7 +1633,7 @@ void intel_psr_atomic_check(struct
> > > > > drm_connector *connector,
> > > > > struct drm_crtc_state *crtc_state;
> > > > >
> > > > > if (!CAN_PSR(dev_priv) |
Re: [Intel-gfx] [PATCH v3] drm/i915/psr: Force PSR probe only after full initialization
On Fri, 2020-02-21 at 15:46 +, Mun, Gwan-gyeong wrote:
> On Thu, 2020-02-20 at 12:55 -0800, Souza, Jose wrote:
> > On Thu, 2020-02-20 at 12:39 +, Mun, Gwan-gyeong wrote:
> > > On Tue, 2020-02-18 at 12:39 -0800, José Roberto de Souza wrote:
> > > > Commit 60c6a14b489b ("drm/i915/display: Force the state compute
> > > > phase
> > > > once to enable PSR") was forcing the state compute too earlier
> > > > causing errors because not everything was initialized, so here
> > > > moving to i915_driver_register() when everything is ready and
> > > > driver
> > > > is registering into the rest of the system.
> > > >
> > > > Also fixing the place where it disarm the force probe as during
> > > > the
> > > > atomic check phase errors could happen like the ones due
> > > > locking
> > > > and
> > > > it would cause PSR to never be enabled if that happens.
> > > > Leaving the disarm to the atomic commit phase,
> > > > intel_psr_enable()
> > > > or
> > > > intel_psr_update() will be called even if the current state do
> > > > not
> > > > allow PSR to be enabled.
> > > >
> > > > v2: Check if intel_dp is null in
> > > > intel_psr_force_mode_changed_set()
> > > > v3: Check intel_dp before get dev_priv
> > > >
> > > > Fixes: 60c6a14b489b ("drm/i915/display: Force the state compute
> > > > phase
> > > > once to enable PSR")
> > > > Closes: https://gitlab.freedesktop.org/drm/intel/issues/1151
> > > > Tested-by: Ross Zwisler
> > > > Reported-by: Ross Zwisler
> > > > Cc: Gwan-gyeong Mun
> > > > Cc: Jani Nikula
> > > > Signed-off-by: José Roberto de Souza
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_psr.c | 22
> > > > --
> > > > drivers/gpu/drm/i915/display/intel_psr.h | 1 +
> > > > drivers/gpu/drm/i915/i915_drv.c | 3 +++
> > > > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> > > > 4 files changed, 25 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index b4942b6445ae..2a0f7354fba5 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -936,6 +936,8 @@ void intel_psr_enable(struct intel_dp
> > > > *intel_dp,
> > > > {
> > > > struct drm_i915_private *dev_priv =
> > > > dp_to_i915(intel_dp);
> > > >
> > > > + intel_psr_force_mode_changed_set(intel_dp, false);
> > > > +
> > > Hi,
> > > intel_psr_enable() and intel_psr_update already have checking
> > > routine
> > > for CAN_PSR and has_psr.
> > > therefore we don't need to check twice here.
> >
> > Minor overhead but if you really want I can remove the function
> > call
> > and just do a dev_priv->psr.force_mode_changed = false; for
> > intel_psr_enable/update
> >
> > > And if there are no issues that moving "disarming
> > > force_mode_changed"
> > > to intel_psr_compute_config(),
> > > can we move them to intel_psr_compute_config()?
> >
> > atomic check can fail at any point so we could disarm the
> > mode_changed,
> > fail, retry(because the return was EAGAIN) and then PSR will not be
> > enabled.
> >
> If disarming the "force_mode_changed" would be handled on
> intel_psr_compute_config(),
> (after failing atomic check and)the retry step will set "crtc_state-
> > mode_changed = true" on
> intel_digital_connector_atomic_check(). ( because the
> force_mode_changed is not disabled yet.)
>
> The mode_changed will lead "encoder->compute_config" which will call
> intel_psr_compute_config().
> And we can disable "force_mode_changed" on intel_psr_compute_config()
> which sets "crtc_state->has_psr = true".
> the "crtc_state->has_psr" enables PSR.
After call encoder->compute_config()->intel_psr_compute_config() there
is a lot of code left to be executed in intel_atomic_check() that can
cause the atomic check to fail.
The next pipe in this loop can already cause that.
>
> > > > if (!crtc_state->has_psr)
> > > > return;
> > > >
> > > > @@ -1096,6 +1098,8 @@ void intel_psr_update(struct intel_dp
> > > > *intel_dp,
> > > > struct i915_psr *psr = &dev_priv->psr;
> > > > bool enable, psr2_enable;
> > > >
> > > > + intel_psr_force_mode_changed_set(intel_dp, false);
> > > > +
> > > > if (!CAN_PSR(dev_priv) || READ_ONCE(psr->dp) !=
> > > > intel_dp)
> > > > return;
> > > >
> > > > @@ -1629,7 +1633,7 @@ void intel_psr_atomic_check(struct
> > > > drm_connector *connector,
> > > > struct drm_crtc_state *crtc_state;
> > > >
> > > > if (!CAN_PSR(dev_priv) || !new_state->crtc ||
> > > > - dev_priv->psr.initially_probed)
> > > > + !dev_priv->psr.force_mode_changed)
> > > > return;
> > > >
> > > > intel_connector = to_intel_connector(connector);
> > > > @@ -1640,5 +1644,19 @@ void intel_psr_atomic_check(struct
> > > > drm_connector *connector,
> > > > crtc_state = drm_atomic_get_new_crtc_s
Re: [Intel-gfx] [PATCH v3] drm/i915/psr: Force PSR probe only after full initialization
On Thu, 2020-02-20 at 12:55 -0800, Souza, Jose wrote:
> On Thu, 2020-02-20 at 12:39 +, Mun, Gwan-gyeong wrote:
> > On Tue, 2020-02-18 at 12:39 -0800, José Roberto de Souza wrote:
> > > Commit 60c6a14b489b ("drm/i915/display: Force the state compute
> > > phase
> > > once to enable PSR") was forcing the state compute too earlier
> > > causing errors because not everything was initialized, so here
> > > moving to i915_driver_register() when everything is ready and
> > > driver
> > > is registering into the rest of the system.
> > >
> > > Also fixing the place where it disarm the force probe as during
> > > the
> > > atomic check phase errors could happen like the ones due locking
> > > and
> > > it would cause PSR to never be enabled if that happens.
> > > Leaving the disarm to the atomic commit phase, intel_psr_enable()
> > > or
> > > intel_psr_update() will be called even if the current state do
> > > not
> > > allow PSR to be enabled.
> > >
> > > v2: Check if intel_dp is null in
> > > intel_psr_force_mode_changed_set()
> > > v3: Check intel_dp before get dev_priv
> > >
> > > Fixes: 60c6a14b489b ("drm/i915/display: Force the state compute
> > > phase
> > > once to enable PSR")
> > > Closes: https://gitlab.freedesktop.org/drm/intel/issues/1151
> > > Tested-by: Ross Zwisler
> > > Reported-by: Ross Zwisler
> > > Cc: Gwan-gyeong Mun
> > > Cc: Jani Nikula
> > > Signed-off-by: José Roberto de Souza
> > > ---
> > > drivers/gpu/drm/i915/display/intel_psr.c | 22
> > > --
> > > drivers/gpu/drm/i915/display/intel_psr.h | 1 +
> > > drivers/gpu/drm/i915/i915_drv.c | 3 +++
> > > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> > > 4 files changed, 25 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index b4942b6445ae..2a0f7354fba5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -936,6 +936,8 @@ void intel_psr_enable(struct intel_dp
> > > *intel_dp,
> > > {
> > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > >
> > > + intel_psr_force_mode_changed_set(intel_dp, false);
> > > +
> > Hi,
> > intel_psr_enable() and intel_psr_update already have checking
> > routine
> > for CAN_PSR and has_psr.
> > therefore we don't need to check twice here.
>
> Minor overhead but if you really want I can remove the function call
> and just do a dev_priv->psr.force_mode_changed = false; for
> intel_psr_enable/update
>
> > And if there are no issues that moving "disarming
> > force_mode_changed"
> > to intel_psr_compute_config(),
> > can we move them to intel_psr_compute_config()?
>
> atomic check can fail at any point so we could disarm the
> mode_changed,
> fail, retry(because the return was EAGAIN) and then PSR will not be
> enabled.
>
If disarming the "force_mode_changed" would be handled on
intel_psr_compute_config(),
(after failing atomic check and)the retry step will set "crtc_state-
>mode_changed = true" on
intel_digital_connector_atomic_check(). ( because the
force_mode_changed is not disabled yet.)
The mode_changed will lead "encoder->compute_config" which will call
intel_psr_compute_config().
And we can disable "force_mode_changed" on intel_psr_compute_config()
which sets "crtc_state->has_psr = true".
the "crtc_state->has_psr" enables PSR.
> > > if (!crtc_state->has_psr)
> > > return;
> > >
> > > @@ -1096,6 +1098,8 @@ void intel_psr_update(struct intel_dp
> > > *intel_dp,
> > > struct i915_psr *psr = &dev_priv->psr;
> > > bool enable, psr2_enable;
> > >
> > > + intel_psr_force_mode_changed_set(intel_dp, false);
> > > +
> > > if (!CAN_PSR(dev_priv) || READ_ONCE(psr->dp) != intel_dp)
> > > return;
> > >
> > > @@ -1629,7 +1633,7 @@ void intel_psr_atomic_check(struct
> > > drm_connector *connector,
> > > struct drm_crtc_state *crtc_state;
> > >
> > > if (!CAN_PSR(dev_priv) || !new_state->crtc ||
> > > - dev_priv->psr.initially_probed)
> > > + !dev_priv->psr.force_mode_changed)
> > > return;
> > >
> > > intel_connector = to_intel_connector(connector);
> > > @@ -1640,5 +1644,19 @@ void intel_psr_atomic_check(struct
> > > drm_connector *connector,
> > > crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> > > new_state->crtc);
> > > crtc_state->mode_changed = true;
> > > - dev_priv->psr.initially_probed = true;
> > > +}
> > > +
> > > +void intel_psr_force_mode_changed_set(struct intel_dp *intel_dp,
> > > bool set)
> > IMHO, it would be better intel_psr_set_force_mode_changed() as a
> > function name.
>
> Okay
>
> > > +{
> > > + struct drm_i915_private *dev_priv;
> > > +
> > > + if (!intel_dp)
> > > + return;
> > > +
> > > + dev_priv = dp_to_i915(intel_dp);
> > > + if (!CAN_PSR(dev_priv) || !intel_dp_is_edp(intel_dp) ||
> > > + intel_dp != dev_priv->psr.dp
Re: [Intel-gfx] [PATCH v3] drm/i915/psr: Force PSR probe only after full initialization
On Thu, 2020-02-20 at 12:39 +, Mun, Gwan-gyeong wrote:
> On Tue, 2020-02-18 at 12:39 -0800, José Roberto de Souza wrote:
> > Commit 60c6a14b489b ("drm/i915/display: Force the state compute
> > phase
> > once to enable PSR") was forcing the state compute too earlier
> > causing errors because not everything was initialized, so here
> > moving to i915_driver_register() when everything is ready and
> > driver
> > is registering into the rest of the system.
> >
> > Also fixing the place where it disarm the force probe as during the
> > atomic check phase errors could happen like the ones due locking
> > and
> > it would cause PSR to never be enabled if that happens.
> > Leaving the disarm to the atomic commit phase, intel_psr_enable()
> > or
> > intel_psr_update() will be called even if the current state do not
> > allow PSR to be enabled.
> >
> > v2: Check if intel_dp is null in intel_psr_force_mode_changed_set()
> > v3: Check intel_dp before get dev_priv
> >
> > Fixes: 60c6a14b489b ("drm/i915/display: Force the state compute
> > phase
> > once to enable PSR")
> > Closes: https://gitlab.freedesktop.org/drm/intel/issues/1151
> > Tested-by: Ross Zwisler
> > Reported-by: Ross Zwisler
> > Cc: Gwan-gyeong Mun
> > Cc: Jani Nikula
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/i915/display/intel_psr.c | 22
> > --
> > drivers/gpu/drm/i915/display/intel_psr.h | 1 +
> > drivers/gpu/drm/i915/i915_drv.c | 3 +++
> > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> > 4 files changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index b4942b6445ae..2a0f7354fba5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -936,6 +936,8 @@ void intel_psr_enable(struct intel_dp
> > *intel_dp,
> > {
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >
> > + intel_psr_force_mode_changed_set(intel_dp, false);
> > +
> Hi,
> intel_psr_enable() and intel_psr_update already have checking routine
> for CAN_PSR and has_psr.
> therefore we don't need to check twice here.
Minor overhead but if you really want I can remove the function call
and just do a dev_priv->psr.force_mode_changed = false; for
intel_psr_enable/update
> And if there are no issues that moving "disarming force_mode_changed"
> to intel_psr_compute_config(),
> can we move them to intel_psr_compute_config()?
atomic check can fail at any point so we could disarm the mode_changed,
fail, retry(because the return was EAGAIN) and then PSR will not be
enabled.
>
> > if (!crtc_state->has_psr)
> > return;
> >
> > @@ -1096,6 +1098,8 @@ void intel_psr_update(struct intel_dp
> > *intel_dp,
> > struct i915_psr *psr = &dev_priv->psr;
> > bool enable, psr2_enable;
> >
> > + intel_psr_force_mode_changed_set(intel_dp, false);
> > +
> > if (!CAN_PSR(dev_priv) || READ_ONCE(psr->dp) != intel_dp)
> > return;
> >
> > @@ -1629,7 +1633,7 @@ void intel_psr_atomic_check(struct
> > drm_connector *connector,
> > struct drm_crtc_state *crtc_state;
> >
> > if (!CAN_PSR(dev_priv) || !new_state->crtc ||
> > - dev_priv->psr.initially_probed)
> > + !dev_priv->psr.force_mode_changed)
> > return;
> >
> > intel_connector = to_intel_connector(connector);
> > @@ -1640,5 +1644,19 @@ void intel_psr_atomic_check(struct
> > drm_connector *connector,
> > crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> >new_state->crtc);
> > crtc_state->mode_changed = true;
> > - dev_priv->psr.initially_probed = true;
> > +}
> > +
> > +void intel_psr_force_mode_changed_set(struct intel_dp *intel_dp,
> > bool set)
> IMHO, it would be better intel_psr_set_force_mode_changed() as a
> function name.
Okay
> > +{
> > + struct drm_i915_private *dev_priv;
> > +
> > + if (!intel_dp)
> > + return;
> > +
> > + dev_priv = dp_to_i915(intel_dp);
> > + if (!CAN_PSR(dev_priv) || !intel_dp_is_edp(intel_dp) ||
> > + intel_dp != dev_priv->psr.dp)
> > + return;
> > +
> > + dev_priv->psr.force_mode_changed = set;
> > }
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > b/drivers/gpu/drm/i915/display/intel_psr.h
> > index c58a1d438808..27a70468e2b9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > @@ -40,5 +40,6 @@ bool intel_psr_enabled(struct intel_dp
> > *intel_dp);
> > void intel_psr_atomic_check(struct drm_connector *connector,
> > struct drm_connector_state *old_state,
> > struct drm_connector_state *new_state);
> > +void intel_psr_force_mode_changed_set(struct intel_dp *intel_dp,
> > bool set);
> >
> > #endif /* __INTEL_PSR_H__ */
> > diff --git a/drivers/gpu/drm/i915/i915_
Re: [Intel-gfx] [PATCH v3] drm/i915/psr: Force PSR probe only after full initialization
On Tue, 2020-02-18 at 12:39 -0800, José Roberto de Souza wrote:
> Commit 60c6a14b489b ("drm/i915/display: Force the state compute phase
> once to enable PSR") was forcing the state compute too earlier
> causing errors because not everything was initialized, so here
> moving to i915_driver_register() when everything is ready and driver
> is registering into the rest of the system.
>
> Also fixing the place where it disarm the force probe as during the
> atomic check phase errors could happen like the ones due locking and
> it would cause PSR to never be enabled if that happens.
> Leaving the disarm to the atomic commit phase, intel_psr_enable() or
> intel_psr_update() will be called even if the current state do not
> allow PSR to be enabled.
>
> v2: Check if intel_dp is null in intel_psr_force_mode_changed_set()
> v3: Check intel_dp before get dev_priv
>
> Fixes: 60c6a14b489b ("drm/i915/display: Force the state compute phase
> once to enable PSR")
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1151
> Tested-by: Ross Zwisler
> Reported-by: Ross Zwisler
> Cc: Gwan-gyeong Mun
> Cc: Jani Nikula
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 22 --
> drivers/gpu/drm/i915/display/intel_psr.h | 1 +
> drivers/gpu/drm/i915/i915_drv.c | 3 +++
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> 4 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index b4942b6445ae..2a0f7354fba5 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -936,6 +936,8 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>
> + intel_psr_force_mode_changed_set(intel_dp, false);
> +
Hi,
intel_psr_enable() and intel_psr_update already have checking routine
for CAN_PSR and has_psr.
therefore we don't need to check twice here.
And if there are no issues that moving "disarming force_mode_changed"
to intel_psr_compute_config(),
can we move them to intel_psr_compute_config()?
> if (!crtc_state->has_psr)
> return;
>
> @@ -1096,6 +1098,8 @@ void intel_psr_update(struct intel_dp
> *intel_dp,
> struct i915_psr *psr = &dev_priv->psr;
> bool enable, psr2_enable;
>
> + intel_psr_force_mode_changed_set(intel_dp, false);
> +
> if (!CAN_PSR(dev_priv) || READ_ONCE(psr->dp) != intel_dp)
> return;
>
> @@ -1629,7 +1633,7 @@ void intel_psr_atomic_check(struct
> drm_connector *connector,
> struct drm_crtc_state *crtc_state;
>
> if (!CAN_PSR(dev_priv) || !new_state->crtc ||
> - dev_priv->psr.initially_probed)
> + !dev_priv->psr.force_mode_changed)
> return;
>
> intel_connector = to_intel_connector(connector);
> @@ -1640,5 +1644,19 @@ void intel_psr_atomic_check(struct
> drm_connector *connector,
> crtc_state = drm_atomic_get_new_crtc_state(new_state->state,
> new_state->crtc);
> crtc_state->mode_changed = true;
> - dev_priv->psr.initially_probed = true;
> +}
> +
> +void intel_psr_force_mode_changed_set(struct intel_dp *intel_dp,
> bool set)
IMHO, it would be better intel_psr_set_force_mode_changed() as a
function name.
> +{
> + struct drm_i915_private *dev_priv;
> +
> + if (!intel_dp)
> + return;
> +
> + dev_priv = dp_to_i915(intel_dp);
> + if (!CAN_PSR(dev_priv) || !intel_dp_is_edp(intel_dp) ||
> + intel_dp != dev_priv->psr.dp)
> + return;
> +
> + dev_priv->psr.force_mode_changed = set;
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> b/drivers/gpu/drm/i915/display/intel_psr.h
> index c58a1d438808..27a70468e2b9 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -40,5 +40,6 @@ bool intel_psr_enabled(struct intel_dp *intel_dp);
> void intel_psr_atomic_check(struct drm_connector *connector,
> struct drm_connector_state *old_state,
> struct drm_connector_state *new_state);
> +void intel_psr_force_mode_changed_set(struct intel_dp *intel_dp,
> bool set);
>
> #endif /* __INTEL_PSR_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index f7a1c33697b7..83791c197611 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -58,6 +58,7 @@
> #include "display/intel_hotplug.h"
> #include "display/intel_overlay.h"
> #include "display/intel_pipe_crc.h"
> +#include "display/intel_psr.h"
> #include "display/intel_sprite.h"
> #include "display/intel_vga.h"
>
> @@ -1256,6 +1257,8 @@ static void i915_driver_register(struct
> drm_i915_private *dev_priv)
>
> intel_audio_init(dev_priv);
>
> + intel_psr_force_mod
