[Intel-gfx] [PATCH] drm/i915: Do not do link training fallback or prune modes for eDP

Manasi Navare manasi.d.navare at intel.com
Thu Aug 17 22:10:51 UTC 2017


On Thu, Aug 17, 2017 at 01:44:14PM -0700, Rodrigo Vivi wrote:
> On Thu, Aug 17, 2017 at 1:11 PM, Pandiyan, Dhinakaran
> <dhinakaran.pandiyan at intel.com> wrote:
> > On Thu, 2017-08-17 at 12:46 -0700, Manasi Navare wrote:
> >> On Thu, Aug 17, 2017 at 07:01:05AM +0000, Rodrigo Vivi wrote:
> >> >    On Wed, Aug 16, 2017 at 11:51 PM Manasi Navare
> >> >    <[1]manasi.d.navare at intel.com> wrote:
> >> >
> >> >      In case of eDP because the panel has a fixed mode we cannot
> >> >      link train fallback and prune modes since this results in
> >> >      no modes available for eDP connector.
> >> >
> >> >    What about downclock modes?!
> >>
> >> What are the downclock modes? We have seen an issue with pruning modes
> >> in case of eDP panel where we end up pruning the mode due to link train
> >> fallback and then we get an error saying "No modes available for the
> >> connector"
> >>
> >
> > fwiw I've got two laptops with eDP's having more than ten modes.
> 
> those ten are probably a list of modes with scaler right?!
> 
> Anyways, there are eDP panels that supports multiple modes like same
> resolution but different clocks.
> Like the ones with 60Hz and 48Hz where we cannot get PSR on 60 and can
> get on 48 one.
> 
> This made Jim to do this patch:
> https://patchwork.freedesktop.org/patch/msgid/1502308133-26892-1-git-send-email-jim.bride@linux.intel.com
> 
> that now is merged already and allow we select different modes on eDP
> per request.
> 
> this case proves that there are eDP panels out there where this bug
> wouldn't occur.
> 
> So I think we need to find a different way to handle this case where
> there is no other mode
> or fix the link status somehow....
>

Thanks Rodrigo for this explanation. Yes those modes advertised to the
userspace would be the modes with scalar. But the HW would still only
support the fixed mode ot the alternate DRRS mode. Infact in case of link training
fallback, when the userspace triggers a new modeset, it will call mode_valid() hook
where if the requested mode's hdisplay or vdisplay are greater than the fixed mode's
hdisplay or vdisplay then it returns error MODE_PANEL instead of MODE_OK.
Also Jim brought up a point that for some eDP panels only 2 lanes could be wired in
which case we do want link train fallback in order to retry modeset with fewer lanes.
So we need fallback I suppose but handle this no mode situation differently.

Daniel, Jani any thoughts on how this could be handled?

Regards
Manasi

> >
> >> >
> >> >      Also since its a panel, link training should not fail dynamically
> >> >      based on cable conditions like in  case of DP.
> >> >
> >> >    Is there any bug associated with this patch?
> >>
> >> Yes this is the bug:
> >> https://bugs.freedesktop.org/show_bug.cgi?id=101518
> 
> Please add this for reference in a future patch or version:
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101518
> 
> >> But the reason I didnt have this in the Fixes tab is because
> >> this patch will not fix the bug, it will just isolate to it being
> >> a bug with AUX Timeouts warning.
> >>
> >> Regards
> >> Manasi
> >>
> >> >
> >> >      Cc: Jani Nikula <[2]jani.nikula at linux.intel.com>
> >> >      Cc: Jim Bride <[3]jim.bride at linux.intel.com>
> >> >      Cc: Ville Syrjälä <[4]ville.syrjala at linux.intel.com>
> >> >      Cc: Daniel Vetter <[5]daniel.vetter at intel.com>
> >> >      Signed-off-by: Manasi Navare <[6]manasi.d.navare at intel.com>
> >> >      ---
> >> >       drivers/gpu/drm/i915/intel_dp.c               | 2 +-
> >> >       drivers/gpu/drm/i915/intel_dp_link_training.c | 3 ++-
> >> >       drivers/gpu/drm/i915/intel_drv.h              | 1 +
> >> >       3 files changed, 4 insertions(+), 2 deletions(-)
> >> >      diff --git a/drivers/gpu/drm/i915/intel_dp.c
> >> >      b/drivers/gpu/drm/i915/intel_dp.c
> >> >      index 4fd4853..edac0c8 100644
> >> >      --- a/drivers/gpu/drm/i915/intel_dp.c
> >> >      +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> >      @@ -109,7 +109,7 @@ static const int default_rates[] = { 162000,
> >> >      270000, 540000 };
> >> >        * If a CPU or PCH DP output is attached to an eDP panel, this
> >> >      function
> >> >        * will return true, and false otherwise.
> >> >        */
> >> >      -static bool is_edp(struct intel_dp *intel_dp)
> >> >      +bool is_edp(struct intel_dp *intel_dp)
> >> >       {
> >> >              struct intel_digital_port *intel_dig_port =
> >> >      dp_to_dig_port(intel_dp);
> >> >      diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
> >> >      b/drivers/gpu/drm/i915/intel_dp_link_training.c
> >> >      index 05907fa..18ec61f 100644
> >> >      --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> >> >      +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> >> >      @@ -332,7 +332,8 @@ intel_dp_start_link_train(struct intel_dp
> >> >      *intel_dp)
> >> >                            intel_connector->[7]base.base.id,
> >> >                            intel_connector->[8]base.name,
> >> >                            intel_dp->link_rate, intel_dp->lane_count);
> >> >      -       if (!intel_dp_get_link_train_fallback_values(intel_dp,
> >> >      +       /* Dont fallback and prune modes if its eDP */
> >> >      +       if (!is_edp(intel_dp) &&
> >> >      !intel_dp_get_link_train_fallback_values(intel_dp,
> >> >
> >> >      intel_dp->link_rate,
> >> >
> >> >      intel_dp->lane_count))
> >> >                      /* Schedule a Hotplug Uevent to userspace to start
> >> >      modeset */
> >> >      diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >> >      b/drivers/gpu/drm/i915/intel_drv.h
> >> >      index fa47285..9800a15 100644
> >> >      --- a/drivers/gpu/drm/i915/intel_drv.h
> >> >      +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> >      @@ -1548,6 +1548,7 @@ static inline unsigned int
> >> >      intel_dp_unused_lane_mask(int lane_count)
> >> >       }
> >> >       bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> >> >      +bool is_edp(struct intel_dp *intel_dp);
> >> >       int intel_dp_link_required(int pixel_clock, int bpp);
> >> >       int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> >> >       bool intel_digital_port_connected(struct drm_i915_private
> >> >      *dev_priv,
> >> >      --
> >> >      2.1.4
> >> >      _______________________________________________
> >> >      Intel-gfx mailing list
> >> >      [9]Intel-gfx at lists.freedesktop.org
> >> >      [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >
> >> > References
> >> >
> >> >    1. mailto:manasi.d.navare at intel.com
> >> >    2. mailto:jani.nikula at linux.intel.com
> >> >    3. mailto:jim.bride at linux.intel.com
> >> >    4. mailto:ville.syrjala at linux.intel.com
> >> >    5. mailto:daniel.vetter at intel.com
> >> >    6. mailto:manasi.d.navare at intel.com
> >> >    7. http://base.base.id/
> >> >    8. http://base.name/
> >> >    9. mailto:Intel-gfx at lists.freedesktop.org
> >> >   10. https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br


More information about the Intel-gfx mailing list