| Message ID | 1520579339-14745-1-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 08 Mar 2018, Manasi Navare <manasi.d.navare@intel.com> wrote: > The panels are generally designed to support only a single > clock and lane configuration, and typically these values > correspond to the native resolution of the panel. But some > panels advertise the MAX_LINK_RATE in DPCD higher than what > is required to support the native resolution. > So optimize and set the link rate and lane count to the > least values required by the panel to support the native > resolution. This will also be an effective power saving > for such eDP panels. I'm afraid this will lead to flickering on plenty of laptops. It will just get reverted when it hits end users. We've been down this path before. If we decide to try to do this again, we need to figure out how *not* to regress those machines. We can't just talk our way out of this. As a tip, it's often useful to have a look at git blame and git log when you're considering changes to code that seems odd or contentious or something. In this case that leads to commit 344c5bbcb7a2 ("drm/i915/edp: use lane count and link rate from DPCD for eDP"), which in turn leads to commit 56071a207602 ("drm/i915: use lane count and link rate from VBT as minimums for eDP") and a bunch of bugzilla links. That stuff was done by yours truly years ago, so I'd rather not repeat my mistakes now. ;) BR, Jani. > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Matt Atwood <matthew.s.atwood@intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 8d1d7af..ba1114b 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1812,15 +1812,14 @@ intel_dp_compute_config(struct intel_encoder *encoder, > bpp = dev_priv->vbt.edp.bpp; > } > > - /* > - * Use the maximum clock and number of lanes the eDP panel > - * advertizes being capable of. The panels are generally > - * designed to support only a single clock and lane > - * configuration, and typically these values correspond to the > - * native resolution of the panel. > + /* The panels are generally designed to support only a single > + * clock and lane configuration, and typically these values > + * correspond to the native resolution of the panel. But some > + * panels advertise higher link rates that might not be required > + * for the native resolution of the panel. So use the least > + * required link rate/lane count for the panel's native > + * resolution. > */ > - min_lane_count = max_lane_count; > - min_clock = max_clock; > } > > for (; bpp >= 6*3; bpp -= 2*3) {
Quoting Jani Nikula (2018-03-09 10:20:37) > On Thu, 08 Mar 2018, Manasi Navare <manasi.d.navare@intel.com> wrote: > > The panels are generally designed to support only a single > > clock and lane configuration, and typically these values > > correspond to the native resolution of the panel. But some > > panels advertise the MAX_LINK_RATE in DPCD higher than what > > is required to support the native resolution. > > So optimize and set the link rate and lane count to the > > least values required by the panel to support the native > > resolution. This will also be an effective power saving > > for such eDP panels. > > I'm afraid this will lead to flickering on plenty of laptops. It will > just get reverted when it hits end users. We've been down this path > before. If we decide to try to do this again, we need to figure out how > *not* to regress those machines. We can't just talk our way out of this. > > As a tip, it's often useful to have a look at git blame and git log when > you're considering changes to code that seems odd or contentious or > something. In this case that leads to commit 344c5bbcb7a2 > ("drm/i915/edp: use lane count and link rate from DPCD for eDP"), which > in turn leads to commit 56071a207602 ("drm/i915: use lane count and link > rate from VBT as minimums for eDP") and a bunch of bugzilla links. Also include a bit of rationale in the comment about what happens if we don't have that block. * Use the maximum clock and number of lanes the eDP panel * advertizes being capable of. The panels are generally * designed to support only a single clock and lane * configuration, and typically these values correspond to the * native resolution of the panel. + On some? many? panels, failure to force the maximum bandwidth results + in flickering, hence we unconditionally apply this w/a. -Chris
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 8d1d7af..ba1114b 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1812,15 +1812,14 @@ intel_dp_compute_config(struct intel_encoder *encoder, bpp = dev_priv->vbt.edp.bpp; } - /* - * Use the maximum clock and number of lanes the eDP panel - * advertizes being capable of. The panels are generally - * designed to support only a single clock and lane - * configuration, and typically these values correspond to the - * native resolution of the panel. + /* The panels are generally designed to support only a single + * clock and lane configuration, and typically these values + * correspond to the native resolution of the panel. But some + * panels advertise higher link rates that might not be required + * for the native resolution of the panel. So use the least + * required link rate/lane count for the panel's native + * resolution. */ - min_lane_count = max_lane_count; - min_clock = max_clock; } for (; bpp >= 6*3; bpp -= 2*3) {
The panels are generally designed to support only a single clock and lane configuration, and typically these values correspond to the native resolution of the panel. But some panels advertise the MAX_LINK_RATE in DPCD higher than what is required to support the native resolution. So optimize and set the link rate and lane count to the least values required by the panel to support the native resolution. This will also be an effective power saving for such eDP panels. Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Matt Atwood <matthew.s.atwood@intel.com> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)