On Tue, Mar 22, 2022 at 03:36:10PM +0200, Souza, Jose wrote:
> On Tue, 2022-03-22 at 09:49 +0200, Lisovskiy, Stanislav wrote:
> > On Mon, Mar 21, 2022 at 07:01:27PM +0200, Souza, Jose wrote:
> > > On Mon, 2022-03-21 at 12:49 +0200, Stanislav Lisovskiy wrote:
> > > > We are currently getting FIFO underruns, in particular
> > > > when PSR2 is enabled. There seem to be no existing workaround
> > > > or patches, which can fix that issue(were expecting some recent
> > > > selective fetch update and DBuf bw/SAGV fixes to help,
> > > > which unfortunately didn't).
> > > > Current idea is that it looks like for some reason the
> > > > DBuf prefill time isn't enough once we exit PSR2, despite its
> > > > theoretically correct.
> > > > So bump it up a bit by 15%(minimum experimental amount required
> > > > to get it working), if PSR2 is enabled.
> > > > For PSR1 there is no need in this hack, so we limit it only
> > > > to PSR2 and Alderlake.
> > > >
> > > > v2: - Added comment(Jose Souza)
> > > > - Fixed 15% calculation(Jose Souza)
> > > >
> > > > Signed-off-by: Stanislav Lisovskiy <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_cdclk.c | 26 ++++++++++++++++++++++
> > > > 1 file changed, 26 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > index 8888fda8b701..92d57869983a 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > @@ -2325,6 +2325,32 @@ int intel_crtc_compute_min_cdclk(const struct
> > > > intel_crtc_state *crtc_state)
> > > > dev_priv->max_cdclk_freq));
> > > > }
> > > >
> > > > +
> > > > + /*
> > > > + * HACK. We are getting FIFO underruns, in particular
> > > > + * when PSR2 is enabled. There seem to be no existing workaround
> > > > + * or patches as of now.
> > > > + * Current idea is that it looks like for some reason the
> > > > + * DBuf prefill time isn't enough once we exit PSR2, despite its
> > > > + * theoretically correct.
> > > > + * So bump it up a bit by 15%(minimum experimental amount
> > > > required
> > > > + * to get it working), if PSR2 is enabled.
> > > > + * For PSR1 there is no need in this hack, so we limit it only
> > > > + * to PSR2 and Alderlake.
> > > > + */
> > > > + if (IS_ALDERLAKE_P(dev_priv)) {
> > >
> > >
> > > And please check if we can only apply this when two or more pipes are
> > > enabled.
> > > Otherwise this will impact power numbers in the case that is matters most.
> >
> > That one I can check. Probably need someone at office to disconnect all the
> > pipes, except eDP to see
> > if its still reproducible, however I think I've seen it already happening.
>
> You can have some hack code in the functions that check if a port is
> connected and return false for all ports other than port A/eDP.
Yes, did that. i915_display_info now reports only eDP connected. Modified
intel_dp_detect to return disconnected state
for all non-eDP connectors, also modified hotplug and digport work routines to
bail out for non PORT_A ports
just in case.
So even with eDP connected only FIFO underruns still happen, just ran
kms_plane_multiple and it appeared immediately.
Stan
>
>
> >
> > Stan
> >
> > >
> > > > + struct intel_encoder *encoder;
> > > > +
> > > > + for_each_intel_encoder_with_psr(&dev_priv->drm,
> > > > encoder) {
> > > > + struct intel_dp *intel_dp =
> > > > enc_to_intel_dp(encoder);
> > > > +
> > > > + if (intel_dp->psr.psr2_enabled) {
> > > > + min_cdclk = DIV_ROUND_UP(min_cdclk *
> > > > 115, 100);
> > > > + break;
> > > > + }
> > > > + }
> > > > + }
> > > > +
> > > > if (min_cdclk > dev_priv->max_cdclk_freq) {
> > > > drm_dbg_kms(&dev_priv->drm,
> > > > "required cdclk (%d kHz) exceeds max (%d
> > > > kHz)\n",
> > >
>
