| Message ID | 1370030389-7860-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2013/5/31 Daniel Vetter <daniel.vetter@ffwll.ch>: > ... not the port clock. This allows us to kill the funny semantics > around pixel_target_clock. > > Since the dpll code still needs the real port clock, add a new > port_clock field to the pipe configuration. Handling the default case > for that one is a bit tricky, since encoders might not consistently > overwrite it when retrying the crtc/encoder bw arbitrage step in the > compute config stage. Hence we need to always clear port_clock and > update it again if the encoder hasn't put in something more specific. > This can't be done in one step since the encoder might want to adjust > the mode first. > > I was a bit on the fence whether I should subsume the pixel multiplier > handling into the port_clock, too. But then I decided against this > since it's on an abstract level still the dotclock of the adjusted > mode, and only our hw makes it a bit special due to the separate pixel > mulitplier setting (which requires that the dpll runs at the > non-multiplied dotclock). > > So after this patch the adjusted_mode accurately describes the mode we > feed into the port, after the panel fitter and pixel multiplier (or > line doubling, if we ever bother with that) have done their job. > Since the fdi link is between the pfit and the pixel multiplier steps > we need to be careful with calculating the fdi link config. > > The ironlake cpu edp port is the only encoder interested in the > portclock (outside of compute_config), refactor the flow a bit to > avoid duplicated code. I've broken this part in an early version of > the patch, hence why I've changed the code a bit more than strictly > required. > > v2: Fix up ilk cpu pll handling. > > v3: Introduce an fdi_dotclock variable in ironlake_fdi_compute_config > to make it clearer that we transmit the adjusted_mode without the > pixel multiplier taken into account. The old code multiplied the the > available link bw with the pixel multiplier, which results in the same > fdi configuration, but is much more confusing. > > v4: Rebase on top of Imre's is_cpu_edp removal. > > v5: Rebase on top of Paulo's haswell watermark fixes, which introduce > a new place which looked at the pixel_clock and so needed conversion. > > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v3) > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/intel_ddi.c | 3 ++- > drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++------------ > drivers/gpu/drm/i915/intel_dp.c | 50 +++++++++++++----------------------- > drivers/gpu/drm/i915/intel_drv.h | 13 +++++----- > drivers/gpu/drm/i915/intel_hdmi.c | 4 +-- > drivers/gpu/drm/i915/intel_pm.c | 5 +--- > 6 files changed, 49 insertions(+), 63 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 9649df8..486c46b 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -624,7 +624,7 @@ intel_ddi_calculate_wrpll(int clock /* in Hz */, > clock, *p_out, *n2_out, *r2_out); > } > > -bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock) > +bool intel_ddi_pll_mode_set(struct drm_crtc *crtc) > { > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc); > @@ -634,6 +634,7 @@ bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock) > int type = intel_encoder->type; > enum pipe pipe = intel_crtc->pipe; > uint32_t reg, val; > + int clock = intel_crtc->config.port_clock; > > /* TODO: reuse PLLs when possible (compare values) */ > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 8c3722d..2772b61 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3974,7 +3974,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, > { > struct drm_device *dev = intel_crtc->base.dev; > struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode; > - int target_clock, lane, link_bw; > + int lane, link_bw, fdi_dotclock; > bool setup_ok, needs_recompute = false; > > retry: > @@ -3987,19 +3987,15 @@ retry: > */ > link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10; > > - if (pipe_config->pixel_target_clock) > - target_clock = pipe_config->pixel_target_clock; > - else > - target_clock = adjusted_mode->clock; > - > - lane = ironlake_get_lanes_required(target_clock, link_bw, > + lane = ironlake_get_lanes_required(adjusted_mode->clock, link_bw, > pipe_config->pipe_bpp); > > pipe_config->fdi_lanes = lane; > > + fdi_dotclock = adjusted_mode->clock; > if (pipe_config->pixel_multiplier > 1) > - link_bw *= pipe_config->pixel_multiplier; > - intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock, > + fdi_dotclock /= pipe_config->pixel_multiplier; > + intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock, > link_bw, &pipe_config->fdi_m_n); This seems to change how we set m_n values, which is not the goal of the patch. Can't we try to do this in a separate patch? I've spent a lot of time in the last months bisecting display code, and the big patches were the ones I spent most time debugging... > > setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev, > @@ -4340,8 +4336,6 @@ static void vlv_update_pll(struct intel_crtc *crtc) > { > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - struct drm_display_mode *adjusted_mode = > - &crtc->config.adjusted_mode; > struct intel_encoder *encoder; > int pipe = crtc->pipe; > u32 dpll, mdiv; > @@ -4394,7 +4388,7 @@ static void vlv_update_pll(struct intel_crtc *crtc) > vlv_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv); > > /* Set HBR and RBR LPF coefficients */ > - if (adjusted_mode->clock == 162000 || > + if (crtc->config.port_clock == 162000 || > intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI)) > vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe), > 0x005f0021); > @@ -4839,7 +4833,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, > * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2. > */ > limit = intel_limit(crtc, refclk); > - ok = dev_priv->display.find_dpll(limit, crtc, adjusted_mode->clock, > + ok = dev_priv->display.find_dpll(limit, crtc, > + intel_crtc->config.port_clock, > refclk, NULL, &clock); > if (!ok) { > DRM_ERROR("Couldn't find PLL settings for mode!\n"); > @@ -5447,7 +5442,6 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc) > } > > static bool ironlake_compute_clocks(struct drm_crtc *crtc, > - struct drm_display_mode *adjusted_mode, > intel_clock_t *clock, > bool *has_reduced_clock, > intel_clock_t *reduced_clock) > @@ -5475,7 +5469,8 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc, > * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2. > */ > limit = intel_limit(crtc, refclk); > - ret = dev_priv->display.find_dpll(limit, crtc, adjusted_mode->clock, > + ret = dev_priv->display.find_dpll(limit, crtc, > + to_intel_crtc(crtc)->config.port_clock, > refclk, NULL, clock); > if (!ret) > return false; > @@ -5675,7 +5670,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)), > "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev)); > > - ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock, > + ok = ironlake_compute_clocks(crtc, &clock, > &has_reduced_clock, &reduced_clock); > if (!ok) { > DRM_ERROR("Couldn't find PLL settings for mode!\n"); > @@ -5878,7 +5873,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, > WARN(num_connectors != 1, "%d connectors attached to pipe %c\n", > num_connectors, pipe_name(pipe)); > > - if (!intel_ddi_pll_mode_set(crtc, adjusted_mode->clock)) > + if (!intel_ddi_pll_mode_set(crtc)) > return -EINVAL; > > /* Ensure that the cursor is valid for the new mode before changing... */ > @@ -7769,6 +7764,9 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, > goto fail; > > encoder_retry: > + /* Ensure the port clock default is reset when retrying. */ > + pipe_config->port_clock = 0; > + > /* Pass our mode to the connectors and the CRTC to give them a chance to > * adjust it according to limitations or connector properties, and also > * a chance to reject the mode entirely. > @@ -7797,6 +7795,11 @@ encoder_retry: > } > } > > + /* Set default port clock if not overwritten by the encoder. Needs to be > + * done afterwards in case the encoder adjusts the mode. */ > + if (!pipe_config->port_clock) > + pipe_config->port_clock = pipe_config->adjusted_mode.clock; > + > ret = intel_crtc_compute_config(crtc, pipe_config); > if (ret < 0) { > DRM_DEBUG_KMS("CRTC fixup failed\n"); > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 91a31b3..c92eeeb 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -677,7 +677,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, > int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0; > int bpp, mode_rate; > static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 }; > - int target_clock, link_avail, link_clock; > + int link_avail, link_clock; > > if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A) > pipe_config->has_pch_encoder = true; > @@ -694,8 +694,6 @@ intel_dp_compute_config(struct intel_encoder *encoder, > intel_pch_panel_fitting(intel_crtc, pipe_config, > intel_connector->panel.fitting_mode); > } > - /* We need to take the panel's fixed mode into account. */ > - target_clock = adjusted_mode->clock; > > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > return false; > @@ -711,7 +709,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, > bpp = min_t(int, bpp, dev_priv->vbt.edp_bpp); > > for (; bpp >= 6*3; bpp -= 2*3) { > - mode_rate = intel_dp_link_required(target_clock, bpp); > + mode_rate = intel_dp_link_required(adjusted_mode->clock, bpp); > > for (clock = 0; clock <= max_clock; clock++) { > for (lane_count = 1; lane_count <= max_lane_count; lane_count <<= 1) { > @@ -746,18 +744,17 @@ found: > > intel_dp->link_bw = bws[clock]; > intel_dp->lane_count = lane_count; > - adjusted_mode->clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw); > pipe_config->pipe_bpp = bpp; > - pipe_config->pixel_target_clock = target_clock; > + pipe_config->port_clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw); > > DRM_DEBUG_KMS("DP link bw %02x lane count %d clock %d bpp %d\n", > intel_dp->link_bw, intel_dp->lane_count, > - adjusted_mode->clock, bpp); > + pipe_config->port_clock, bpp); > DRM_DEBUG_KMS("DP link bw required %i available %i\n", > mode_rate, link_avail); > > intel_link_compute_m_n(bpp, lane_count, > - target_clock, adjusted_mode->clock, > + adjusted_mode->clock, pipe_config->port_clock, > &pipe_config->dp_m_n); > > intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw); > @@ -780,24 +777,28 @@ void intel_dp_init_link_config(struct intel_dp *intel_dp) > } > } > > -static void ironlake_set_pll_edp(struct drm_crtc *crtc, int clock) > +static void ironlake_set_pll_cpu_edp(struct intel_dp *intel_dp) > { > - struct drm_device *dev = crtc->dev; > + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > + struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc); > + struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > u32 dpa_ctl; > > - DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", clock); > + DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", crtc->config.port_clock); > dpa_ctl = I915_READ(DP_A); > dpa_ctl &= ~DP_PLL_FREQ_MASK; > > - if (clock < 200000) { > + if (crtc->config.port_clock == 162000) { > /* For a long time we've carried around a ILK-DevA w/a for the > * 160MHz clock. If we're really unlucky, it's still required. > */ > DRM_DEBUG_KMS("160MHz cpu eDP clock, might need ilk devA w/a\n"); > dpa_ctl |= DP_PLL_FREQ_160MHZ; > + intel_dp->DP |= DP_PLL_FREQ_160MHZ; > } else { > dpa_ctl |= DP_PLL_FREQ_270MHZ; > + intel_dp->DP |= DP_PLL_FREQ_270MHZ; I really think that all the code that moves these lines above belong to a separate patch. This patch is already too big and if we bisect eDP regressions to it we'll have a lot to debug. It looks correct, but it's better to be safe, especially considering all those complicated gen+port+pch checks inside the intel_dp_mode_set. > } > > I915_WRITE(DP_A, dpa_ctl); > @@ -814,8 +815,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > enum port port = dp_to_dig_port(intel_dp)->port; > - struct drm_crtc *crtc = encoder->crtc; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); > > /* > * There are four kinds of DP registers: > @@ -845,7 +845,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, > > if (intel_dp->has_audio) { > DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n", > - pipe_name(intel_crtc->pipe)); > + pipe_name(crtc->pipe)); > intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE; > intel_write_eld(encoder, adjusted_mode); > } > @@ -864,13 +864,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, > if (intel_dp->link_configuration[1] & DP_LANE_COUNT_ENHANCED_FRAME_EN) > intel_dp->DP |= DP_ENHANCED_FRAMING; > > - intel_dp->DP |= intel_crtc->pipe << 29; > - > - /* don't miss out required setting for eDP */ > - if (adjusted_mode->clock < 200000) > - intel_dp->DP |= DP_PLL_FREQ_160MHZ; > - else > - intel_dp->DP |= DP_PLL_FREQ_270MHZ; > + intel_dp->DP |= crtc->pipe << 29; > } else if (!HAS_PCH_CPT(dev) || port == PORT_A) { > if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev)) > intel_dp->DP |= intel_dp->color_range; > @@ -884,22 +878,14 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, > if (intel_dp->link_configuration[1] & DP_LANE_COUNT_ENHANCED_FRAME_EN) > intel_dp->DP |= DP_ENHANCED_FRAMING; > > - if (intel_crtc->pipe == 1) > + if (crtc->pipe == 1) > intel_dp->DP |= DP_PIPEB_SELECT; > - > - if (port == PORT_A && !IS_VALLEYVIEW(dev)) { > - /* don't miss out required setting for eDP */ > - if (adjusted_mode->clock < 200000) > - intel_dp->DP |= DP_PLL_FREQ_160MHZ; > - else > - intel_dp->DP |= DP_PLL_FREQ_270MHZ; > - } > } else { > intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT; > } > > if (port == PORT_A && !IS_VALLEYVIEW(dev)) > - ironlake_set_pll_edp(crtc, adjusted_mode->clock); > + ironlake_set_pll_cpu_edp(intel_dp); > } > > #define IDLE_ON_MASK (PP_ON | 0 | PP_SEQUENCE_MASK | 0 | PP_SEQUENCE_STATE_MASK) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index fdf6303..afda71f 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -243,12 +243,13 @@ struct intel_crtc_config { > > int pipe_bpp; > struct intel_link_m_n dp_m_n; > - /** > - * This is currently used by DP and HDMI encoders since those can have a > - * target pixel clock != the port link clock (which is currently stored > - * in adjusted_mode->clock). > + > + /* > + * Frequence the dpll for the port should run at. Differs from the > + * adjusted dotclock e.g. for DP or 12bpc hdmi mode. > */ > - int pixel_target_clock; > + int port_clock; > + > /* Used by SDVO (and if we ever fix it, HDMI). */ > unsigned pixel_multiplier; > > @@ -786,7 +787,7 @@ extern void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv, > extern void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc); > extern void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc); > extern void intel_ddi_setup_hw_pll_state(struct drm_device *dev); > -extern bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock); > +extern bool intel_ddi_pll_mode_set(struct drm_crtc *crtc); > extern void intel_ddi_put_crtc_pll(struct drm_crtc *crtc); > extern void intel_ddi_set_pipe_settings(struct drm_crtc *crtc); > extern void intel_ddi_prepare_link_retrain(struct drm_encoder *encoder); > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index 8062a92..bc12518 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -835,9 +835,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, > desired_bpp = 12*3; > > /* Need to adjust the port link by 1.5x for 12bpc. */ > - adjusted_mode->clock = clock_12bpc; > - pipe_config->pixel_target_clock = > - pipe_config->requested_mode.clock; > + pipe_config->port_clock = clock_12bpc; > } else { > DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n"); > desired_bpp = 8*3; > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 49a1887..4126fb1 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2078,10 +2078,7 @@ static uint32_t hsw_wm_get_pixel_rate(struct drm_device *dev, > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > uint32_t pixel_rate, pfit_size; > > - if (intel_crtc->config.pixel_target_clock) > - pixel_rate = intel_crtc->config.pixel_target_clock; > - else > - pixel_rate = intel_crtc->config.adjusted_mode.clock; > + pixel_rate = intel_crtc->config.adjusted_mode.clock; > > /* We only use IF-ID interlacing. If we ever use PF-ID we'll need to > * adjust the pixel_rate here. */ > -- > 1.7.11.7 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
2013/5/31 Paulo Zanoni <przanoni@gmail.com>: > 2013/5/31 Daniel Vetter <daniel.vetter@ffwll.ch>: >> ... not the port clock. This allows us to kill the funny semantics >> around pixel_target_clock. >> >> Since the dpll code still needs the real port clock, add a new >> port_clock field to the pipe configuration. Handling the default case >> for that one is a bit tricky, since encoders might not consistently >> overwrite it when retrying the crtc/encoder bw arbitrage step in the >> compute config stage. Hence we need to always clear port_clock and >> update it again if the encoder hasn't put in something more specific. >> This can't be done in one step since the encoder might want to adjust >> the mode first. >> >> I was a bit on the fence whether I should subsume the pixel multiplier >> handling into the port_clock, too. But then I decided against this >> since it's on an abstract level still the dotclock of the adjusted >> mode, and only our hw makes it a bit special due to the separate pixel >> mulitplier setting (which requires that the dpll runs at the >> non-multiplied dotclock). >> >> So after this patch the adjusted_mode accurately describes the mode we >> feed into the port, after the panel fitter and pixel multiplier (or >> line doubling, if we ever bother with that) have done their job. >> Since the fdi link is between the pfit and the pixel multiplier steps >> we need to be careful with calculating the fdi link config. >> >> The ironlake cpu edp port is the only encoder interested in the >> portclock (outside of compute_config), refactor the flow a bit to >> avoid duplicated code. I've broken this part in an early version of >> the patch, hence why I've changed the code a bit more than strictly >> required. >> >> v2: Fix up ilk cpu pll handling. >> >> v3: Introduce an fdi_dotclock variable in ironlake_fdi_compute_config >> to make it clearer that we transmit the adjusted_mode without the >> pixel multiplier taken into account. The old code multiplied the the >> available link bw with the pixel multiplier, which results in the same >> fdi configuration, but is much more confusing. >> >> v4: Rebase on top of Imre's is_cpu_edp removal. >> >> v5: Rebase on top of Paulo's haswell watermark fixes, which introduce >> a new place which looked at the pixel_clock and so needed conversion. >> >> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v3) >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> --- >> drivers/gpu/drm/i915/intel_ddi.c | 3 ++- >> drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++------------ >> drivers/gpu/drm/i915/intel_dp.c | 50 +++++++++++++----------------------- >> drivers/gpu/drm/i915/intel_drv.h | 13 +++++----- >> drivers/gpu/drm/i915/intel_hdmi.c | 4 +-- >> drivers/gpu/drm/i915/intel_pm.c | 5 +--- >> 6 files changed, 49 insertions(+), 63 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c >> index 9649df8..486c46b 100644 >> --- a/drivers/gpu/drm/i915/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/intel_ddi.c >> @@ -624,7 +624,7 @@ intel_ddi_calculate_wrpll(int clock /* in Hz */, >> clock, *p_out, *n2_out, *r2_out); >> } >> >> -bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock) >> +bool intel_ddi_pll_mode_set(struct drm_crtc *crtc) >> { >> struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc); >> @@ -634,6 +634,7 @@ bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock) >> int type = intel_encoder->type; >> enum pipe pipe = intel_crtc->pipe; >> uint32_t reg, val; >> + int clock = intel_crtc->config.port_clock; >> >> /* TODO: reuse PLLs when possible (compare values) */ >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 8c3722d..2772b61 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -3974,7 +3974,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, >> { >> struct drm_device *dev = intel_crtc->base.dev; >> struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode; >> - int target_clock, lane, link_bw; >> + int lane, link_bw, fdi_dotclock; >> bool setup_ok, needs_recompute = false; >> >> retry: >> @@ -3987,19 +3987,15 @@ retry: >> */ >> link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10; >> >> - if (pipe_config->pixel_target_clock) >> - target_clock = pipe_config->pixel_target_clock; >> - else >> - target_clock = adjusted_mode->clock; >> - >> - lane = ironlake_get_lanes_required(target_clock, link_bw, >> + lane = ironlake_get_lanes_required(adjusted_mode->clock, link_bw, >> pipe_config->pipe_bpp); >> >> pipe_config->fdi_lanes = lane; >> >> + fdi_dotclock = adjusted_mode->clock; >> if (pipe_config->pixel_multiplier > 1) >> - link_bw *= pipe_config->pixel_multiplier; >> - intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock, >> + fdi_dotclock /= pipe_config->pixel_multiplier; >> + intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock, >> link_bw, &pipe_config->fdi_m_n); > > This seems to change how we set m_n values, which is not the goal of > the patch. Can't we try to do this in a separate patch? I've spent a > lot of time in the last months bisecting display code, and the big > patches were the ones I spent most time debugging... > > >> >> setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev, >> @@ -4340,8 +4336,6 @@ static void vlv_update_pll(struct intel_crtc *crtc) >> { >> struct drm_device *dev = crtc->base.dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> - struct drm_display_mode *adjusted_mode = >> - &crtc->config.adjusted_mode; >> struct intel_encoder *encoder; >> int pipe = crtc->pipe; >> u32 dpll, mdiv; >> @@ -4394,7 +4388,7 @@ static void vlv_update_pll(struct intel_crtc *crtc) >> vlv_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv); >> >> /* Set HBR and RBR LPF coefficients */ >> - if (adjusted_mode->clock == 162000 || >> + if (crtc->config.port_clock == 162000 || >> intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI)) >> vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe), >> 0x005f0021); >> @@ -4839,7 +4833,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, >> * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2. >> */ >> limit = intel_limit(crtc, refclk); >> - ok = dev_priv->display.find_dpll(limit, crtc, adjusted_mode->clock, >> + ok = dev_priv->display.find_dpll(limit, crtc, >> + intel_crtc->config.port_clock, >> refclk, NULL, &clock); >> if (!ok) { >> DRM_ERROR("Couldn't find PLL settings for mode!\n"); >> @@ -5447,7 +5442,6 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc) >> } >> >> static bool ironlake_compute_clocks(struct drm_crtc *crtc, >> - struct drm_display_mode *adjusted_mode, >> intel_clock_t *clock, >> bool *has_reduced_clock, >> intel_clock_t *reduced_clock) >> @@ -5475,7 +5469,8 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc, >> * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2. >> */ >> limit = intel_limit(crtc, refclk); >> - ret = dev_priv->display.find_dpll(limit, crtc, adjusted_mode->clock, >> + ret = dev_priv->display.find_dpll(limit, crtc, >> + to_intel_crtc(crtc)->config.port_clock, >> refclk, NULL, clock); >> if (!ret) >> return false; >> @@ -5675,7 +5670,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, >> WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)), >> "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev)); >> >> - ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock, >> + ok = ironlake_compute_clocks(crtc, &clock, >> &has_reduced_clock, &reduced_clock); >> if (!ok) { >> DRM_ERROR("Couldn't find PLL settings for mode!\n"); >> @@ -5878,7 +5873,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, >> WARN(num_connectors != 1, "%d connectors attached to pipe %c\n", >> num_connectors, pipe_name(pipe)); >> >> - if (!intel_ddi_pll_mode_set(crtc, adjusted_mode->clock)) >> + if (!intel_ddi_pll_mode_set(crtc)) >> return -EINVAL; >> >> /* Ensure that the cursor is valid for the new mode before changing... */ >> @@ -7769,6 +7764,9 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, >> goto fail; >> >> encoder_retry: >> + /* Ensure the port clock default is reset when retrying. */ >> + pipe_config->port_clock = 0; >> + >> /* Pass our mode to the connectors and the CRTC to give them a chance to >> * adjust it according to limitations or connector properties, and also >> * a chance to reject the mode entirely. >> @@ -7797,6 +7795,11 @@ encoder_retry: >> } >> } >> >> + /* Set default port clock if not overwritten by the encoder. Needs to be >> + * done afterwards in case the encoder adjusts the mode. */ >> + if (!pipe_config->port_clock) >> + pipe_config->port_clock = pipe_config->adjusted_mode.clock; >> + >> ret = intel_crtc_compute_config(crtc, pipe_config); >> if (ret < 0) { >> DRM_DEBUG_KMS("CRTC fixup failed\n"); >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 91a31b3..c92eeeb 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -677,7 +677,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, >> int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0; >> int bpp, mode_rate; >> static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 }; >> - int target_clock, link_avail, link_clock; >> + int link_avail, link_clock; >> >> if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A) >> pipe_config->has_pch_encoder = true; >> @@ -694,8 +694,6 @@ intel_dp_compute_config(struct intel_encoder *encoder, >> intel_pch_panel_fitting(intel_crtc, pipe_config, >> intel_connector->panel.fitting_mode); >> } >> - /* We need to take the panel's fixed mode into account. */ >> - target_clock = adjusted_mode->clock; >> >> if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) >> return false; >> @@ -711,7 +709,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, >> bpp = min_t(int, bpp, dev_priv->vbt.edp_bpp); >> >> for (; bpp >= 6*3; bpp -= 2*3) { >> - mode_rate = intel_dp_link_required(target_clock, bpp); >> + mode_rate = intel_dp_link_required(adjusted_mode->clock, bpp); >> >> for (clock = 0; clock <= max_clock; clock++) { >> for (lane_count = 1; lane_count <= max_lane_count; lane_count <<= 1) { >> @@ -746,18 +744,17 @@ found: >> >> intel_dp->link_bw = bws[clock]; >> intel_dp->lane_count = lane_count; >> - adjusted_mode->clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw); >> pipe_config->pipe_bpp = bpp; >> - pipe_config->pixel_target_clock = target_clock; >> + pipe_config->port_clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw); >> >> DRM_DEBUG_KMS("DP link bw %02x lane count %d clock %d bpp %d\n", >> intel_dp->link_bw, intel_dp->lane_count, >> - adjusted_mode->clock, bpp); >> + pipe_config->port_clock, bpp); >> DRM_DEBUG_KMS("DP link bw required %i available %i\n", >> mode_rate, link_avail); >> >> intel_link_compute_m_n(bpp, lane_count, >> - target_clock, adjusted_mode->clock, >> + adjusted_mode->clock, pipe_config->port_clock, >> &pipe_config->dp_m_n); >> >> intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw); >> @@ -780,24 +777,28 @@ void intel_dp_init_link_config(struct intel_dp *intel_dp) >> } >> } >> >> -static void ironlake_set_pll_edp(struct drm_crtc *crtc, int clock) >> +static void ironlake_set_pll_cpu_edp(struct intel_dp *intel_dp) >> { >> - struct drm_device *dev = crtc->dev; >> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); >> + struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc); >> + struct drm_device *dev = crtc->base.dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> u32 dpa_ctl; >> >> - DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", clock); >> + DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", crtc->config.port_clock); >> dpa_ctl = I915_READ(DP_A); >> dpa_ctl &= ~DP_PLL_FREQ_MASK; >> >> - if (clock < 200000) { >> + if (crtc->config.port_clock == 162000) { >> /* For a long time we've carried around a ILK-DevA w/a for the >> * 160MHz clock. If we're really unlucky, it's still required. >> */ >> DRM_DEBUG_KMS("160MHz cpu eDP clock, might need ilk devA w/a\n"); >> dpa_ctl |= DP_PLL_FREQ_160MHZ; >> + intel_dp->DP |= DP_PLL_FREQ_160MHZ; >> } else { >> dpa_ctl |= DP_PLL_FREQ_270MHZ; >> + intel_dp->DP |= DP_PLL_FREQ_270MHZ; > > I really think that all the code that moves these lines above belong > to a separate patch. This patch is already too big and if we bisect > eDP regressions to it we'll have a lot to debug. It looks correct, but > it's better to be safe, especially considering all those complicated > gen+port+pch checks inside the intel_dp_mode_set. Also, I can't find a way to apply this patch to dinq. The patch alone doesn't apply. It used to be the 7th patch of the PLL series, but that series also needs some rebasing. > > >> } >> >> I915_WRITE(DP_A, dpa_ctl); >> @@ -814,8 +815,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_dp *intel_dp = enc_to_intel_dp(encoder); >> enum port port = dp_to_dig_port(intel_dp)->port; >> - struct drm_crtc *crtc = encoder->crtc; >> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); >> >> /* >> * There are four kinds of DP registers: >> @@ -845,7 +845,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, >> >> if (intel_dp->has_audio) { >> DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n", >> - pipe_name(intel_crtc->pipe)); >> + pipe_name(crtc->pipe)); >> intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE; >> intel_write_eld(encoder, adjusted_mode); >> } >> @@ -864,13 +864,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, >> if (intel_dp->link_configuration[1] & DP_LANE_COUNT_ENHANCED_FRAME_EN) >> intel_dp->DP |= DP_ENHANCED_FRAMING; >> >> - intel_dp->DP |= intel_crtc->pipe << 29; >> - >> - /* don't miss out required setting for eDP */ >> - if (adjusted_mode->clock < 200000) >> - intel_dp->DP |= DP_PLL_FREQ_160MHZ; >> - else >> - intel_dp->DP |= DP_PLL_FREQ_270MHZ; >> + intel_dp->DP |= crtc->pipe << 29; >> } else if (!HAS_PCH_CPT(dev) || port == PORT_A) { >> if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev)) >> intel_dp->DP |= intel_dp->color_range; >> @@ -884,22 +878,14 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, >> if (intel_dp->link_configuration[1] & DP_LANE_COUNT_ENHANCED_FRAME_EN) >> intel_dp->DP |= DP_ENHANCED_FRAMING; >> >> - if (intel_crtc->pipe == 1) >> + if (crtc->pipe == 1) >> intel_dp->DP |= DP_PIPEB_SELECT; >> - >> - if (port == PORT_A && !IS_VALLEYVIEW(dev)) { >> - /* don't miss out required setting for eDP */ >> - if (adjusted_mode->clock < 200000) >> - intel_dp->DP |= DP_PLL_FREQ_160MHZ; >> - else >> - intel_dp->DP |= DP_PLL_FREQ_270MHZ; >> - } >> } else { >> intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT; >> } >> >> if (port == PORT_A && !IS_VALLEYVIEW(dev)) >> - ironlake_set_pll_edp(crtc, adjusted_mode->clock); >> + ironlake_set_pll_cpu_edp(intel_dp); >> } >> >> #define IDLE_ON_MASK (PP_ON | 0 | PP_SEQUENCE_MASK | 0 | PP_SEQUENCE_STATE_MASK) >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index fdf6303..afda71f 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -243,12 +243,13 @@ struct intel_crtc_config { >> >> int pipe_bpp; >> struct intel_link_m_n dp_m_n; >> - /** >> - * This is currently used by DP and HDMI encoders since those can have a >> - * target pixel clock != the port link clock (which is currently stored >> - * in adjusted_mode->clock). >> + >> + /* >> + * Frequence the dpll for the port should run at. Differs from the >> + * adjusted dotclock e.g. for DP or 12bpc hdmi mode. >> */ >> - int pixel_target_clock; >> + int port_clock; >> + >> /* Used by SDVO (and if we ever fix it, HDMI). */ >> unsigned pixel_multiplier; >> >> @@ -786,7 +787,7 @@ extern void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv, >> extern void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc); >> extern void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc); >> extern void intel_ddi_setup_hw_pll_state(struct drm_device *dev); >> -extern bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock); >> +extern bool intel_ddi_pll_mode_set(struct drm_crtc *crtc); >> extern void intel_ddi_put_crtc_pll(struct drm_crtc *crtc); >> extern void intel_ddi_set_pipe_settings(struct drm_crtc *crtc); >> extern void intel_ddi_prepare_link_retrain(struct drm_encoder *encoder); >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c >> index 8062a92..bc12518 100644 >> --- a/drivers/gpu/drm/i915/intel_hdmi.c >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c >> @@ -835,9 +835,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, >> desired_bpp = 12*3; >> >> /* Need to adjust the port link by 1.5x for 12bpc. */ >> - adjusted_mode->clock = clock_12bpc; >> - pipe_config->pixel_target_clock = >> - pipe_config->requested_mode.clock; >> + pipe_config->port_clock = clock_12bpc; >> } else { >> DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n"); >> desired_bpp = 8*3; >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index 49a1887..4126fb1 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -2078,10 +2078,7 @@ static uint32_t hsw_wm_get_pixel_rate(struct drm_device *dev, >> struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> uint32_t pixel_rate, pfit_size; >> >> - if (intel_crtc->config.pixel_target_clock) >> - pixel_rate = intel_crtc->config.pixel_target_clock; >> - else >> - pixel_rate = intel_crtc->config.adjusted_mode.clock; >> + pixel_rate = intel_crtc->config.adjusted_mode.clock; >> >> /* We only use IF-ID interlacing. If we ever use PF-ID we'll need to >> * adjust the pixel_rate here. */ >> -- >> 1.7.11.7 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 9649df8..486c46b 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -624,7 +624,7 @@ intel_ddi_calculate_wrpll(int clock /* in Hz */, clock, *p_out, *n2_out, *r2_out); } -bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock) +bool intel_ddi_pll_mode_set(struct drm_crtc *crtc) { struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc); @@ -634,6 +634,7 @@ bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock) int type = intel_encoder->type; enum pipe pipe = intel_crtc->pipe; uint32_t reg, val; + int clock = intel_crtc->config.port_clock; /* TODO: reuse PLLs when possible (compare values) */ diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8c3722d..2772b61 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3974,7 +3974,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, { struct drm_device *dev = intel_crtc->base.dev; struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode; - int target_clock, lane, link_bw; + int lane, link_bw, fdi_dotclock; bool setup_ok, needs_recompute = false; retry: @@ -3987,19 +3987,15 @@ retry: */ link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10; - if (pipe_config->pixel_target_clock) - target_clock = pipe_config->pixel_target_clock; - else - target_clock = adjusted_mode->clock; - - lane = ironlake_get_lanes_required(target_clock, link_bw, + lane = ironlake_get_lanes_required(adjusted_mode->clock, link_bw, pipe_config->pipe_bpp); pipe_config->fdi_lanes = lane; + fdi_dotclock = adjusted_mode->clock; if (pipe_config->pixel_multiplier > 1) - link_bw *= pipe_config->pixel_multiplier; - intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock, + fdi_dotclock /= pipe_config->pixel_multiplier; + intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock, link_bw, &pipe_config->fdi_m_n); setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev, @@ -4340,8 +4336,6 @@ static void vlv_update_pll(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_display_mode *adjusted_mode = - &crtc->config.adjusted_mode; struct intel_encoder *encoder; int pipe = crtc->pipe; u32 dpll, mdiv; @@ -4394,7 +4388,7 @@ static void vlv_update_pll(struct intel_crtc *crtc) vlv_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv); /* Set HBR and RBR LPF coefficients */ - if (adjusted_mode->clock == 162000 || + if (crtc->config.port_clock == 162000 || intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI)) vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe), 0x005f0021); @@ -4839,7 +4833,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2. */ limit = intel_limit(crtc, refclk); - ok = dev_priv->display.find_dpll(limit, crtc, adjusted_mode->clock, + ok = dev_priv->display.find_dpll(limit, crtc, + intel_crtc->config.port_clock, refclk, NULL, &clock); if (!ok) { DRM_ERROR("Couldn't find PLL settings for mode!\n"); @@ -5447,7 +5442,6 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc) } static bool ironlake_compute_clocks(struct drm_crtc *crtc, - struct drm_display_mode *adjusted_mode, intel_clock_t *clock, bool *has_reduced_clock, intel_clock_t *reduced_clock) @@ -5475,7 +5469,8 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc, * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2. */ limit = intel_limit(crtc, refclk); - ret = dev_priv->display.find_dpll(limit, crtc, adjusted_mode->clock, + ret = dev_priv->display.find_dpll(limit, crtc, + to_intel_crtc(crtc)->config.port_clock, refclk, NULL, clock); if (!ret) return false; @@ -5675,7 +5670,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)), "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev)); - ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock, + ok = ironlake_compute_clocks(crtc, &clock, &has_reduced_clock, &reduced_clock); if (!ok) { DRM_ERROR("Couldn't find PLL settings for mode!\n"); @@ -5878,7 +5873,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, WARN(num_connectors != 1, "%d connectors attached to pipe %c\n", num_connectors, pipe_name(pipe)); - if (!intel_ddi_pll_mode_set(crtc, adjusted_mode->clock)) + if (!intel_ddi_pll_mode_set(crtc)) return -EINVAL; /* Ensure that the cursor is valid for the new mode before changing... */ @@ -7769,6 +7764,9 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, goto fail; encoder_retry: + /* Ensure the port clock default is reset when retrying. */ + pipe_config->port_clock = 0; + /* Pass our mode to the connectors and the CRTC to give them a chance to * adjust it according to limitations or connector properties, and also * a chance to reject the mode entirely. @@ -7797,6 +7795,11 @@ encoder_retry: } } + /* Set default port clock if not overwritten by the encoder. Needs to be + * done afterwards in case the encoder adjusts the mode. */ + if (!pipe_config->port_clock) + pipe_config->port_clock = pipe_config->adjusted_mode.clock; + ret = intel_crtc_compute_config(crtc, pipe_config); if (ret < 0) { DRM_DEBUG_KMS("CRTC fixup failed\n"); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 91a31b3..c92eeeb 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -677,7 +677,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0; int bpp, mode_rate; static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 }; - int target_clock, link_avail, link_clock; + int link_avail, link_clock; if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A) pipe_config->has_pch_encoder = true; @@ -694,8 +694,6 @@ intel_dp_compute_config(struct intel_encoder *encoder, intel_pch_panel_fitting(intel_crtc, pipe_config, intel_connector->panel.fitting_mode); } - /* We need to take the panel's fixed mode into account. */ - target_clock = adjusted_mode->clock; if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) return false; @@ -711,7 +709,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, bpp = min_t(int, bpp, dev_priv->vbt.edp_bpp); for (; bpp >= 6*3; bpp -= 2*3) { - mode_rate = intel_dp_link_required(target_clock, bpp); + mode_rate = intel_dp_link_required(adjusted_mode->clock, bpp); for (clock = 0; clock <= max_clock; clock++) { for (lane_count = 1; lane_count <= max_lane_count; lane_count <<= 1) { @@ -746,18 +744,17 @@ found: intel_dp->link_bw = bws[clock]; intel_dp->lane_count = lane_count; - adjusted_mode->clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw); pipe_config->pipe_bpp = bpp; - pipe_config->pixel_target_clock = target_clock; + pipe_config->port_clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw); DRM_DEBUG_KMS("DP link bw %02x lane count %d clock %d bpp %d\n", intel_dp->link_bw, intel_dp->lane_count, - adjusted_mode->clock, bpp); + pipe_config->port_clock, bpp); DRM_DEBUG_KMS("DP link bw required %i available %i\n", mode_rate, link_avail); intel_link_compute_m_n(bpp, lane_count, - target_clock, adjusted_mode->clock, + adjusted_mode->clock, pipe_config->port_clock, &pipe_config->dp_m_n); intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw); @@ -780,24 +777,28 @@ void intel_dp_init_link_config(struct intel_dp *intel_dp) } } -static void ironlake_set_pll_edp(struct drm_crtc *crtc, int clock) +static void ironlake_set_pll_cpu_edp(struct intel_dp *intel_dp) { - struct drm_device *dev = crtc->dev; + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); + struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc); + struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; u32 dpa_ctl; - DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", clock); + DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", crtc->config.port_clock); dpa_ctl = I915_READ(DP_A); dpa_ctl &= ~DP_PLL_FREQ_MASK; - if (clock < 200000) { + if (crtc->config.port_clock == 162000) { /* For a long time we've carried around a ILK-DevA w/a for the * 160MHz clock. If we're really unlucky, it's still required. */ DRM_DEBUG_KMS("160MHz cpu eDP clock, might need ilk devA w/a\n"); dpa_ctl |= DP_PLL_FREQ_160MHZ; + intel_dp->DP |= DP_PLL_FREQ_160MHZ; } else { dpa_ctl |= DP_PLL_FREQ_270MHZ; + intel_dp->DP |= DP_PLL_FREQ_270MHZ; } I915_WRITE(DP_A, dpa_ctl); @@ -814,8 +815,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_i915_private *dev_priv = dev->dev_private; struct intel_dp *intel_dp = enc_to_intel_dp(encoder); enum port port = dp_to_dig_port(intel_dp)->port; - struct drm_crtc *crtc = encoder->crtc; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); /* * There are four kinds of DP registers: @@ -845,7 +845,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, if (intel_dp->has_audio) { DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n", - pipe_name(intel_crtc->pipe)); + pipe_name(crtc->pipe)); intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE; intel_write_eld(encoder, adjusted_mode); } @@ -864,13 +864,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, if (intel_dp->link_configuration[1] & DP_LANE_COUNT_ENHANCED_FRAME_EN) intel_dp->DP |= DP_ENHANCED_FRAMING; - intel_dp->DP |= intel_crtc->pipe << 29; - - /* don't miss out required setting for eDP */ - if (adjusted_mode->clock < 200000) - intel_dp->DP |= DP_PLL_FREQ_160MHZ; - else - intel_dp->DP |= DP_PLL_FREQ_270MHZ; + intel_dp->DP |= crtc->pipe << 29; } else if (!HAS_PCH_CPT(dev) || port == PORT_A) { if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev)) intel_dp->DP |= intel_dp->color_range; @@ -884,22 +878,14 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, if (intel_dp->link_configuration[1] & DP_LANE_COUNT_ENHANCED_FRAME_EN) intel_dp->DP |= DP_ENHANCED_FRAMING; - if (intel_crtc->pipe == 1) + if (crtc->pipe == 1) intel_dp->DP |= DP_PIPEB_SELECT; - - if (port == PORT_A && !IS_VALLEYVIEW(dev)) { - /* don't miss out required setting for eDP */ - if (adjusted_mode->clock < 200000) - intel_dp->DP |= DP_PLL_FREQ_160MHZ; - else - intel_dp->DP |= DP_PLL_FREQ_270MHZ; - } } else { intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT; } if (port == PORT_A && !IS_VALLEYVIEW(dev)) - ironlake_set_pll_edp(crtc, adjusted_mode->clock); + ironlake_set_pll_cpu_edp(intel_dp); } #define IDLE_ON_MASK (PP_ON | 0 | PP_SEQUENCE_MASK | 0 | PP_SEQUENCE_STATE_MASK) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index fdf6303..afda71f 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -243,12 +243,13 @@ struct intel_crtc_config { int pipe_bpp; struct intel_link_m_n dp_m_n; - /** - * This is currently used by DP and HDMI encoders since those can have a - * target pixel clock != the port link clock (which is currently stored - * in adjusted_mode->clock). + + /* + * Frequence the dpll for the port should run at. Differs from the + * adjusted dotclock e.g. for DP or 12bpc hdmi mode. */ - int pixel_target_clock; + int port_clock; + /* Used by SDVO (and if we ever fix it, HDMI). */ unsigned pixel_multiplier; @@ -786,7 +787,7 @@ extern void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv, extern void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc); extern void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc); extern void intel_ddi_setup_hw_pll_state(struct drm_device *dev); -extern bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock); +extern bool intel_ddi_pll_mode_set(struct drm_crtc *crtc); extern void intel_ddi_put_crtc_pll(struct drm_crtc *crtc); extern void intel_ddi_set_pipe_settings(struct drm_crtc *crtc); extern void intel_ddi_prepare_link_retrain(struct drm_encoder *encoder); diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 8062a92..bc12518 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -835,9 +835,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, desired_bpp = 12*3; /* Need to adjust the port link by 1.5x for 12bpc. */ - adjusted_mode->clock = clock_12bpc; - pipe_config->pixel_target_clock = - pipe_config->requested_mode.clock; + pipe_config->port_clock = clock_12bpc; } else { DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n"); desired_bpp = 8*3; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 49a1887..4126fb1 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2078,10 +2078,7 @@ static uint32_t hsw_wm_get_pixel_rate(struct drm_device *dev, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); uint32_t pixel_rate, pfit_size; - if (intel_crtc->config.pixel_target_clock) - pixel_rate = intel_crtc->config.pixel_target_clock; - else - pixel_rate = intel_crtc->config.adjusted_mode.clock; + pixel_rate = intel_crtc->config.adjusted_mode.clock; /* We only use IF-ID interlacing. If we ever use PF-ID we'll need to * adjust the pixel_rate here. */