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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Mar 20, 2020 at 04:17:27PM -0700, Manasi Navare wrote:
> 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?

Yep.

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