| Message ID | 1502953104-25562-1-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 16, 2017 at 11:51 PM Manasi Navare <manasi.d.navare@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?! > > 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? > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Jim Bride <jim.bride@linux.intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@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->base.base.id, > intel_connector->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 > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Wed, 16 Aug 2017, Manasi Navare <manasi.d.navare@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. > Also since its a panel, link training should not fail dynamically > based on cable conditions like in case of DP. > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Jim Bride <jim.bride@linux.intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@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) Make that intel_dp_is_edp when you expose it outside of intel_dp.c BR, Jani > { > 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->base.base.id, > intel_connector->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,
On Thu, 2017-08-17 at 12:29 -0700, Manasi Navare wrote: > On Thu, Aug 17, 2017 at 12:51:27PM +0300, Jani Nikula wrote: > > On Wed, 16 Aug 2017, Manasi Navare <manasi.d.navare@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. > > > Also since its a panel, link training should not fail dynamically > > > based on cable conditions like in case of DP. > > > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > Cc: Jim Bride <jim.bride@linux.intel.com> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > > Signed-off-by: Manasi Navare <manasi.d.navare@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) > > > > Make that intel_dp_is_edp when you expose it outside of intel_dp.c > > > > BR, > > Jani > > > > Yea renaming it as intel_dp_is_edp() makes sense after making it a non static function. > So should I make a seperate patch for this change and changing the name at all places > that call is_edp() currently? > > Regards > Manasi > Don't we already have a intel_dp_is_edp() that checks VBT? That'll need to be renamed if is_edp() is going to become intel_dp_is_edp() Btw I remember seeing Jim's psr patch making this function global(non-static?) too. > > > { > > > 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->base.base.id, > > > intel_connector->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, > > > > -- > > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Aug 17, 2017 at 12:51:27PM +0300, Jani Nikula wrote: > On Wed, 16 Aug 2017, Manasi Navare <manasi.d.navare@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. > > Also since its a panel, link training should not fail dynamically > > based on cable conditions like in case of DP. > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Jim Bride <jim.bride@linux.intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@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) > > Make that intel_dp_is_edp when you expose it outside of intel_dp.c > > BR, > Jani > Yea renaming it as intel_dp_is_edp() makes sense after making it a non static function. So should I make a seperate patch for this change and changing the name at all places that call is_edp() currently? Regards Manasi > > { > > 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->base.base.id, > > intel_connector->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, > > -- > Jani Nikula, Intel Open Source Technology Center
On Thu, Aug 17, 2017 at 12:28:56PM -0700, Pandiyan, Dhinakaran wrote: > On Thu, 2017-08-17 at 12:29 -0700, Manasi Navare wrote: > > On Thu, Aug 17, 2017 at 12:51:27PM +0300, Jani Nikula wrote: > > > On Wed, 16 Aug 2017, Manasi Navare <manasi.d.navare@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. > > > > Also since its a panel, link training should not fail dynamically > > > > based on cable conditions like in case of DP. > > > > > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > > Cc: Jim Bride <jim.bride@linux.intel.com> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > > > Signed-off-by: Manasi Navare <manasi.d.navare@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) > > > > > > Make that intel_dp_is_edp when you expose it outside of intel_dp.c > > > > > > BR, > > > Jani > > > > > > > Yea renaming it as intel_dp_is_edp() makes sense after making it a non static function. > > So should I make a seperate patch for this change and changing the name at all places > > that call is_edp() currently? > > > > Regards > > Manasi > > > > Don't we already have a intel_dp_is_edp() that checks VBT? That'll need > to be renamed if is_edp() is going to become intel_dp_is_edp() > Yes just noticed we do have that intel_dp_is_edp(). But I agree that we need to add intel_dp prefix if we are going to expose the is_edp() function. Do you have any suggestions for the name? > Btw I remember seeing Jim's psr patch making this function > global(non-static?) too. > Yes I have looked at it and that patch is still in review, so this review comment applies to his patch as well. So either he makes this change in his or it happens here. I plan to add this comment to Jim's PSR patch review too. Regards Manasi > > > > > { > > > > 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->base.base.id, > > > > intel_connector->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, > > > > > > -- > > > Jani Nikula, Intel Open Source Technology Center > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@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" > > 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 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@linux.intel.com> > Cc: Jim Bride <[3]jim.bride@linux.intel.com> > Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <[5]daniel.vetter@intel.com> > Signed-off-by: Manasi Navare <[6]manasi.d.navare@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@lists.freedesktop.org > [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > References > > 1. mailto:manasi.d.navare@intel.com > 2. mailto:jani.nikula@linux.intel.com > 3. mailto:jim.bride@linux.intel.com > 4. mailto:ville.syrjala@linux.intel.com > 5. mailto:daniel.vetter@intel.com > 6. mailto:manasi.d.navare@intel.com > 7. http://base.base.id/ > 8. http://base.name/ > 9. mailto:Intel-gfx@lists.freedesktop.org > 10. https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@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. > > > > 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 > 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@linux.intel.com> > > Cc: Jim Bride <[3]jim.bride@linux.intel.com> > > Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com> > > Cc: Daniel Vetter <[5]daniel.vetter@intel.com> > > Signed-off-by: Manasi Navare <[6]manasi.d.navare@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@lists.freedesktop.org > > [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > References > > > > 1. mailto:manasi.d.navare@intel.com > > 2. mailto:jani.nikula@linux.intel.com > > 3. mailto:jim.bride@linux.intel.com > > 4. mailto:ville.syrjala@linux.intel.com > > 5. mailto:daniel.vetter@intel.com > > 6. mailto:manasi.d.navare@intel.com > > 7. http://base.base.id/ > > 8. http://base.name/ > > 9. mailto:Intel-gfx@lists.freedesktop.org > > 10. https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Aug 17, 2017 at 1:11 PM, Pandiyan, Dhinakaran <dhinakaran.pandiyan@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@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.... > >> > >> > 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@linux.intel.com> >> > Cc: Jim Bride <[3]jim.bride@linux.intel.com> >> > Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com> >> > Cc: Daniel Vetter <[5]daniel.vetter@intel.com> >> > Signed-off-by: Manasi Navare <[6]manasi.d.navare@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@lists.freedesktop.org >> > [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> > >> > References >> > >> > 1. mailto:manasi.d.navare@intel.com >> > 2. mailto:jani.nikula@linux.intel.com >> > 3. mailto:jim.bride@linux.intel.com >> > 4. mailto:ville.syrjala@linux.intel.com >> > 5. mailto:daniel.vetter@intel.com >> > 6. mailto:manasi.d.navare@intel.com >> > 7. http://base.base.id/ >> > 8. http://base.name/ >> > 9. mailto:Intel-gfx@lists.freedesktop.org >> > 10. https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Aug 17, 2017 at 01:11:00PM -0700, Pandiyan, Dhinakaran 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@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. > Yes but link training is not supposed to fail for eDP since its a fixed connection and so after discussing with Daniel and Jani, this is the consensus that we shouldnt be falling back and retraining in case of eDP since most eDP panels are going to have 1 or 2 modes. 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 > > 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@linux.intel.com> > > > Cc: Jim Bride <[3]jim.bride@linux.intel.com> > > > Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com> > > > Cc: Daniel Vetter <[5]daniel.vetter@intel.com> > > > Signed-off-by: Manasi Navare <[6]manasi.d.navare@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@lists.freedesktop.org > > > [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > References > > > > > > 1. mailto:manasi.d.navare@intel.com > > > 2. mailto:jani.nikula@linux.intel.com > > > 3. mailto:jim.bride@linux.intel.com > > > 4. mailto:ville.syrjala@linux.intel.com > > > 5. mailto:daniel.vetter@intel.com > > > 6. mailto:manasi.d.navare@intel.com > > > 7. http://base.base.id/ > > > 8. http://base.name/ > > > 9. mailto:Intel-gfx@lists.freedesktop.org > > > 10. https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Aug 17, 2017 at 02:01:05PM -0700, Manasi Navare wrote: > On Thu, Aug 17, 2017 at 01:11:00PM -0700, Pandiyan, Dhinakaran 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@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. > > Hmm, I have always dealt with laptops with eDPs with a fixed mode and so are the systems in CI which are giving this "no mode left on the connector error". Manasi > > Yes but link training is not supposed to fail for eDP since its a > fixed connection and so after discussing with Daniel and Jani, this is > the consensus that we shouldnt be falling back and retraining in case > of eDP since most eDP panels are going to have 1 or 2 modes. > > 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 > > > 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@linux.intel.com> > > > > Cc: Jim Bride <[3]jim.bride@linux.intel.com> > > > > Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com> > > > > Cc: Daniel Vetter <[5]daniel.vetter@intel.com> > > > > Signed-off-by: Manasi Navare <[6]manasi.d.navare@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@lists.freedesktop.org > > > > [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > > > References > > > > > > > > 1. mailto:manasi.d.navare@intel.com > > > > 2. mailto:jani.nikula@linux.intel.com > > > > 3. mailto:jim.bride@linux.intel.com > > > > 4. mailto:ville.syrjala@linux.intel.com > > > > 5. mailto:daniel.vetter@intel.com > > > > 6. mailto:manasi.d.navare@intel.com > > > > 7. http://base.base.id/ > > > > 8. http://base.name/ > > > > 9. mailto:Intel-gfx@lists.freedesktop.org > > > > 10. https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@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@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@linux.intel.com> > >> > Cc: Jim Bride <[3]jim.bride@linux.intel.com> > >> > Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com> > >> > Cc: Daniel Vetter <[5]daniel.vetter@intel.com> > >> > Signed-off-by: Manasi Navare <[6]manasi.d.navare@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@lists.freedesktop.org > >> > [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >> > > >> > References > >> > > >> > 1. mailto:manasi.d.navare@intel.com > >> > 2. mailto:jani.nikula@linux.intel.com > >> > 3. mailto:jim.bride@linux.intel.com > >> > 4. mailto:ville.syrjala@linux.intel.com > >> > 5. mailto:daniel.vetter@intel.com > >> > 6. mailto:manasi.d.navare@intel.com > >> > 7. http://base.base.id/ > >> > 8. http://base.name/ > >> > 9. mailto:Intel-gfx@lists.freedesktop.org > >> > 10. https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br
On Thursday, August 17, 2017 1:44:14 PM PDT Rodrigo Vivi wrote: > On Thu, Aug 17, 2017 at 1:11 PM, Pandiyan, Dhinakaran > > <dhinakaran.pandiyan@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@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?! Yeah, Clint very graciously explained that to me after I sent the email.
On Thu, Aug 17, 2017 at 12:46:53PM -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@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" > > > > > 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 > 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. We use References: for that. Fixes: is for when you fix a bug in a previous patch, and since this fixes a regression you need to add a Fixes: line that references your original link training failure fallback. -Daniel > > Regards > Manasi > > > > > Cc: Jani Nikula <[2]jani.nikula@linux.intel.com> > > Cc: Jim Bride <[3]jim.bride@linux.intel.com> > > Cc: Ville Syrjälä <[4]ville.syrjala@linux.intel.com> > > Cc: Daniel Vetter <[5]daniel.vetter@intel.com> > > Signed-off-by: Manasi Navare <[6]manasi.d.navare@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@lists.freedesktop.org > > [10]https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > References > > > > 1. mailto:manasi.d.navare@intel.com > > 2. mailto:jani.nikula@linux.intel.com > > 3. mailto:jim.bride@linux.intel.com > > 4. mailto:ville.syrjala@linux.intel.com > > 5. mailto:daniel.vetter@intel.com > > 6. mailto:manasi.d.navare@intel.com > > 7. http://base.base.id/ > > 8. http://base.name/ > > 9. mailto:Intel-gfx@lists.freedesktop.org > > 10. https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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->base.base.id, intel_connector->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,
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. Also since its a panel, link training should not fail dynamically based on cable conditions like in case of DP. Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Jim Bride <jim.bride@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Signed-off-by: Manasi Navare <manasi.d.navare@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(-)