Re: [Intel-gfx] [PATCH 1/3] drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure

From: Manasi Navare <manasi.d.navare@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: Matteo Iervasi <matteoiervasi@gmail.com>,
	Jani Nikula <jani.nikula@intel.com>,
	Albert Astals Cid <aacid@kde.org>,
	intel-gfx@lists.freedesktop.org,
	Emanuele Panigati <ilpanich@gmail.com>
Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure
Date: Fri, 20 Mar 2020 16:17:27 -0700	[thread overview]
Message-ID: <20200320231727.GD15035@intel.com> (raw)
In-Reply-To: <20200320190831.GQ13686@intel.com>

On Fri, Mar 20, 2020 at 09:08:31PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 19, 2020 at 03:20:50PM -0700, Manasi Navare wrote:
> > On Thu, Mar 19, 2020 at 06:38:42PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Some new eDP panels don't like to operate at the max parameters, and
> > > instead we need to go for an optimal confiugration. That unfortunately
> > > doesn't work with older eDP panels which are generally only guaranteed
> > > to work at the max parameters.
> > > 
> > > To solve these two conflicting requirements let's start with the optimal
> > > setup, and if that fails we start again with the max parameters. The
> > > downside is probably an extra modeset when we switch strategies but
> > > I don't see a good way to avoid that.
> > > 
> > > For a bit of history we first tried to go for the fast+narrow in
> > > commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config
> > > fast and narrow"). but that had to be reverted due to regression
> > > on older panels in commit f11cb1c19ad0 ("drm/i915/dp: revert back
> > > to max link rate and lane count on eDP"). So now we try to get
> > > the best of both worlds by using both strategies.
> > > 
> > > v2: Deal with output_bpp and uapi vs. hw state split
> > >     Reword some comments
> > > 
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Cc: Albert Astals Cid <aacid@kde.org> # v5.0 backport
> > > Cc: Emanuele Panigati <ilpanich@gmail.com> # v5.0 backport
> > > Cc: Matteo Iervasi <matteoiervasi@gmail.com> # v5.0 backport
> > > Cc: Timo Aaltonen <tjaalton@ubuntu.com>
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=105267
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=109959
> > > References: https://gitlab.freedesktop.org/drm/intel/issues/272
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > This approach looks good to me to fallback to max parameters if
> > it fails the first time.
> > 
> > > ---
> > >  .../drm/i915/display/intel_display_types.h    |  1 +
> > >  drivers/gpu/drm/i915/display/intel_dp.c       | 74 ++++++++++++++++---
> > >  2 files changed, 66 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index 5e00e611f077..ffde0d4af23c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -1258,6 +1258,7 @@ struct intel_dp {
> > >  	bool link_trained;
> > >  	bool has_audio;
> > >  	bool reset_link_params;
> > > +	bool use_max_params;
> > >  	u8 dpcd[DP_RECEIVER_CAP_SIZE];
> > >  	u8 psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
> > >  	u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index ef2e06e292d5..85abcce492ca 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -465,6 +465,12 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > >  {
> > >  	int index;
> > >  
> > > +	if (intel_dp_is_edp(intel_dp) && !intel_dp->use_max_params) {
> > > +		DRM_DEBUG_KMS("Retrying Link training for eDP with max parameters\n");
> > > +		intel_dp->use_max_params = true;
> > > +		return 0;
> > > +	}
> > 
> > We need to remove the current check for intel_dp_can_link_train_fallback_for_edp() right?
> 
> No. Why do you think it needs to be removed?
>

Okay so if trying max params link training again fails on eDP, then it fallsback from max to lower values
and fallback link training continues until it can handle the fixed mode with the params or
the lowest params?

So if I understand it correctly we first try to use the optimum approach, if that fails then
we try with max params so in this iteration if it fails again then max params is still true
then it will fallback and call intel_dp_can_link_train_fallback_for_edp() and then
again keep retrying?

Manasi

> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx