On Thu, 20 Feb 2020, "Souza, Jose" <jose.souza@xxxxxxxxx> wrote:
> On Thu, 2020-02-20 at 09:41 +0530, Anshuman Gupta wrote:
>> On 2020-02-18 at 23:53:28 +0530, 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.
>> Is it possible to having a psr state in intel crtc state to do this,
>> this can be used in future when will have dual psr display?
>
> When we implement that we would probably move i915_psr to intel_crtc,
> just not sure how we would handle when the connector changes its CRTC
> but that is something to handle in the future.
Likely not intel_crtc, but rather intel_encoder or intel_dp I think.
BR,
Jani.
>
>>
>> intel_psr_fastset_force() also forcing the fastset when psr mode
>> change from debugfs, may be intel_psr_force_mode_changed_set(true)
>> can get rid of that.
>
> They are different, intel_psr_fastset_force() will do a atomic commit
> and change the PSR state while intel_psr_force_mode_changed_set() will
> just make sure that the pipe attached to the eDP panel will have its
> state computed and checked in the next atomic commit.
>
>
>> > v2: Check if intel_dp is null in intel_psr_force_mode_changed_set()
>> >
>> > Fixes: 60c6a14b489b ("drm/i915/display: Force the state compute
>> > phase once to enable PSR")
>> > Closes: https://gitlab.freedesktop.org/drm/intel/issues/1151
>> > Reported-by: Ross Zwisler <zwisler@xxxxxxxxxx>
>> > Tested-by: Ross Zwisler <zwisler@xxxxxxxxxx>
>> > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
>> > Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
>> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_psr.c | 18 ++++++++++++++++--
>> > 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, 21 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..35bafd281deb 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);
>> > +
>> > 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,15 @@ 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)
>> > +{
>> > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>> > +
>> > + if (!CAN_PSR(dev_priv) || !intel_dp ||
>> > !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_mode_changed_set(dev_priv->psr.dp, true);
>> > +
>> > /*
>> > * Some ports require correctly set-up hpd registers for
>> > detection to
>> > * work properly (leading to ghost connected connector status),
>> > e.g. VGA
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> > b/drivers/gpu/drm/i915/i915_drv.h
>> > index 3330b538d379..a546655072bd 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -505,7 +505,7 @@ struct i915_psr {
>> > bool dc3co_enabled;
>> > u32 dc3co_exit_delay;
>> > struct delayed_work dc3co_work;
>> > - bool initially_probed;
>> > + bool force_mode_changed;
>> > };
>> >
>> > #define QUIRK_LVDS_SSC_DISABLE (1<<1)
>> > --
>> > 2.25.1
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
