On Tue, Dec 01, 2015 at 07:00:19AM +0000, Scot Doyle wrote:
> > Index: i915/intel_dp.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_dp.c,v
> > retrieving revision 1.26
> > diff -u -p -r1.26 intel_dp.c
> > --- i915/intel_dp.c 23 Sep 2015 23:12:12 -0000 1.26
> > +++ i915/intel_dp.c 1 Dec 2015 05:38:09 -0000
> > @@ -94,13 +94,19 @@ static int
> > intel_dp_max_link_bw(struct intel_dp *intel_dp)
> > {
> > int max_link_bw = intel_dp->dpcd[DP_MAX_LINK_RATE];
> > + struct drm_device *dev = intel_dp->attached_connector->base.dev;
> >
> > switch (max_link_bw) {
> > case DP_LINK_BW_1_62:
> > case DP_LINK_BW_2_7:
> > break;
> > case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */
> > - max_link_bw = DP_LINK_BW_2_7;
> > + if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) ||
> > + INTEL_INFO(dev)->gen >= 8) &&
>
> Should we include "INTEL_INFO(dev)->gen >= 8"? There is at least one known
> regression (booting with blank screen) described in commit
> 8749be86a8f0c8a956d20f7d3c768ed6d1e356a1.
I think we can safely skip that one as it concerns skylake and
we don't have any code for skylake at the moment.
>
> > + intel_dp->dpcd[DP_DPCD_REV] >= 0x12)
> > + max_link_bw = DP_LINK_BW_5_4;
>
> No need to set max_link_bw to same value? Negate previous condition,
> assign DP_LINK_BW_2_7 and delete following else clause?
It's harmless though, I'd prefer to leave it as then it is potentially
one less conflict to deal with later.
>
> > + else
> > + max_link_bw = DP_LINK_BW_2_7;
> > break;
> > default:
> > WARN(1, "invalid max DP link bw val %x, using 1.62Gbps\n",
> > @@ -847,9 +853,10 @@ intel_dp_compute_config(struct intel_enc
> > struct intel_connector *intel_connector = intel_dp->attached_connector;
> > int lane_count, clock;
> > int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> > - int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 :
> > 0;
> > + /* Conveniently, the link BW constants become indices with a shift...*/
> > + int max_clock = intel_dp_max_link_bw(intel_dp) >> 3;
> > int bpp, mode_rate;
> > - static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
> > + static int bws[] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7, DP_LINK_BW_5_4 };
> > int link_avail, link_clock;
> >
> > if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A)
> > @@ -2673,10 +2680,15 @@ intel_dp_complete_link_train(struct inte
> > bool channel_eq = false;
> > int tries, cr_tries;
> > uint32_t DP = intel_dp->DP;
> > + uint32_t training_pattern = DP_TRAINING_PATTERN_2;
> > +
> > + /* Training Pattern 3 for HBR2 ot 1.2 devices that support it*/
> > + if (intel_dp->link_bw == DP_LINK_BW_5_4 || intel_dp->use_tps3)
> > + training_pattern = DP_TRAINING_PATTERN_3;
> >
> > /* channel equalization */
> > if (!intel_dp_set_link_train(intel_dp, &DP,
> > - DP_TRAINING_PATTERN_2 |
> > + training_pattern |
> > DP_LINK_SCRAMBLING_DISABLE)) {
> > DRM_ERROR("failed to start channel equalization\n");
> > return;
> > @@ -2703,7 +2715,7 @@ intel_dp_complete_link_train(struct inte
> > if (!drm_dp_clock_recovery_ok(link_status,
> > intel_dp->lane_count)) {
> > intel_dp_start_link_train(intel_dp);
> > intel_dp_set_link_train(intel_dp, &DP,
> > - DP_TRAINING_PATTERN_2 |
> > + training_pattern |
> > DP_LINK_SCRAMBLING_DISABLE);
> > cr_tries++;
> > continue;
> > @@ -2719,7 +2731,7 @@ intel_dp_complete_link_train(struct inte
> > intel_dp_link_down(intel_dp);
> > intel_dp_start_link_train(intel_dp);
> > intel_dp_set_link_train(intel_dp, &DP,
> > - DP_TRAINING_PATTERN_2 |
> > + training_pattern |
> > DP_LINK_SCRAMBLING_DISABLE);
> > tries = 0;
> > cr_tries++;
> > @@ -2864,6 +2876,15 @@ intel_dp_get_dpcd(struct intel_dp *intel
> > DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
> > }
> > }
> > +
> > + /* Training Pattern 3 support, both source and sink */
> > + if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
> > + intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED &&
> > + (IS_HASWELL(dev) || INTEL_INFO(dev)->gen >= 8)) {
> > + intel_dp->use_tps3 = true;
> > + DRM_DEBUG_KMS("Displayport TPS3 supported\n");
> > + } else
> > + intel_dp->use_tps3 = false;
> >
> > if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > DP_DWN_STRM_PORT_PRESENT))
> > Index: i915/intel_drv.h
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_drv.h,v
> > retrieving revision 1.8
> > diff -u -p -r1.8 intel_drv.h
> > --- i915/intel_drv.h 23 Sep 2015 23:12:12 -0000 1.8
> > +++ i915/intel_drv.h 1 Dec 2015 05:31:47 -0000
> > @@ -497,6 +497,7 @@ struct intel_dp {
> > struct delayed_work panel_vdd_work;
> > bool want_panel_vdd;
> > bool psr_setup_done;
> > + bool use_tps3;
> > struct intel_connector *attached_connector;
> > };
> >
> >