| Message ID | 1370099783-20328-6-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2013/6/1 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. > > 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. > > v6: Split out prep patches as requested by Paulo Zanoni. Also rebase > on top of the fdi dotclock handling fix in the fdi lanes/bw > computation code. > > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v3) > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Due to my comments on patch 3, there's a possibility you may need to rebase this patch. My only optional bikeshed is to print port_clock inside intel_dump_pipe_config. If you rebase, you may consider doing it :) Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v6) > --- > drivers/gpu/drm/i915/intel_ddi.c | 3 ++- > drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++--------------- > drivers/gpu/drm/i915/intel_dp.c | 18 +++++++----------- > 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, 35 insertions(+), 40 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 761254d..103f4e9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3992,7 +3992,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, fdi_dotclock; > + int lane, link_bw, fdi_dotclock; > bool setup_ok, needs_recompute = false; > > retry: > @@ -4005,12 +4005,7 @@ 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; > - > - fdi_dotclock = target_clock; > + fdi_dotclock = adjusted_mode->clock; In the patch you sent Friday, you used adjusted_mode->clock instead of fdi_dotclock as the parameter to ironlake_get_lanes_required, you only initialized fdi_dotclock before calling intel_link_compute_m_n. This doesn't change the code behavior, I'm just pointing in case you changed without noticing and prefer the older version. > if (pipe_config->pixel_multiplier > 1) > fdi_dotclock /= pipe_config->pixel_multiplier; > > @@ -4360,8 +4355,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; > @@ -4414,7 +4407,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); > @@ -4859,7 +4852,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"); > @@ -5467,7 +5461,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) > @@ -5495,7 +5488,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; > @@ -5695,7 +5689,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"); > @@ -5898,7 +5892,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... */ > @@ -7789,6 +7783,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. > @@ -7817,6 +7814,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 647cc2b..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); > @@ -788,12 +785,11 @@ static void ironlake_set_pll_cpu_edp(struct intel_dp *intel_dp) > struct drm_i915_private *dev_priv = dev->dev_private; > u32 dpa_ctl; > > - DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", > - crtc->config.adjusted_mode.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 (crtc->config.adjusted_mode.clock == 162000) { > + 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. > */ > 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 >
2013/6/3 Paulo Zanoni <przanoni@gmail.com>: > 2013/6/1 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. >> >> 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. >> >> v6: Split out prep patches as requested by Paulo Zanoni. Also rebase >> on top of the fdi dotclock handling fix in the fdi lanes/bw >> computation code. >> >> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v3) >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Due to my comments on patch 3, there's a possibility you may need to > rebase this patch. > > My only optional bikeshed is to print port_clock inside > intel_dump_pipe_config. If you rebase, you may consider doing it :) > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v6) > >> --- >> drivers/gpu/drm/i915/intel_ddi.c | 3 ++- >> drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++--------------- >> drivers/gpu/drm/i915/intel_dp.c | 18 +++++++----------- >> 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, 35 insertions(+), 40 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 761254d..103f4e9 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -3992,7 +3992,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, fdi_dotclock; >> + int lane, link_bw, fdi_dotclock; >> bool setup_ok, needs_recompute = false; >> >> retry: >> @@ -4005,12 +4005,7 @@ 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; >> - >> - fdi_dotclock = target_clock; >> + fdi_dotclock = adjusted_mode->clock; > > In the patch you sent Friday, you used adjusted_mode->clock instead of > fdi_dotclock as the parameter to ironlake_get_lanes_required, you only > initialized fdi_dotclock before calling intel_link_compute_m_n. This > doesn't change the code behavior, I'm just pointing in case you > changed without noticing and prefer the older version. Completely ignore this paragraph and read the comment inside patch 3. I forgot to remove the paragraph before sending the email. Sorry. > > >> if (pipe_config->pixel_multiplier > 1) >> fdi_dotclock /= pipe_config->pixel_multiplier; >> >> @@ -4360,8 +4355,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; >> @@ -4414,7 +4407,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); >> @@ -4859,7 +4852,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"); >> @@ -5467,7 +5461,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) >> @@ -5495,7 +5488,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; >> @@ -5695,7 +5689,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"); >> @@ -5898,7 +5892,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... */ >> @@ -7789,6 +7783,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. >> @@ -7817,6 +7814,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 647cc2b..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); >> @@ -788,12 +785,11 @@ static void ironlake_set_pll_cpu_edp(struct intel_dp *intel_dp) >> struct drm_i915_private *dev_priv = dev->dev_private; >> u32 dpa_ctl; >> >> - DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", >> - crtc->config.adjusted_mode.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 (crtc->config.adjusted_mode.clock == 162000) { >> + 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. >> */ >> 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 >> > > > > -- > Paulo Zanoni
On Mon, Jun 03, 2013 at 01:54:40PM -0300, Paulo Zanoni wrote: > 2013/6/1 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. > > > > 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. > > > > v6: Split out prep patches as requested by Paulo Zanoni. Also rebase > > on top of the fdi dotclock handling fix in the fdi lanes/bw > > computation code. > > > > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v3) > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Due to my comments on patch 3, there's a possibility you may need to > rebase this patch. > > My only optional bikeshed is to print port_clock inside > intel_dump_pipe_config. If you rebase, you may consider doing it :) Yeah, I think I'll follow up with another pipe config dump patch to add lots more stuff. Already screamed around because it wasn't as complete as I've wished for ;-) -Daniel > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v6) > > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 3 ++- > > drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++--------------- > > drivers/gpu/drm/i915/intel_dp.c | 18 +++++++----------- > > 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, 35 insertions(+), 40 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 761254d..103f4e9 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3992,7 +3992,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, fdi_dotclock; > > + int lane, link_bw, fdi_dotclock; > > bool setup_ok, needs_recompute = false; > > > > retry: > > @@ -4005,12 +4005,7 @@ 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; > > - > > - fdi_dotclock = target_clock; > > + fdi_dotclock = adjusted_mode->clock; > > In the patch you sent Friday, you used adjusted_mode->clock instead of > fdi_dotclock as the parameter to ironlake_get_lanes_required, you only > initialized fdi_dotclock before calling intel_link_compute_m_n. This > doesn't change the code behavior, I'm just pointing in case you > changed without noticing and prefer the older version. > > > > if (pipe_config->pixel_multiplier > 1) > > fdi_dotclock /= pipe_config->pixel_multiplier; > > > > @@ -4360,8 +4355,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; > > @@ -4414,7 +4407,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); > > @@ -4859,7 +4852,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"); > > @@ -5467,7 +5461,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) > > @@ -5495,7 +5488,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; > > @@ -5695,7 +5689,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"); > > @@ -5898,7 +5892,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... */ > > @@ -7789,6 +7783,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. > > @@ -7817,6 +7814,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 647cc2b..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); > > @@ -788,12 +785,11 @@ static void ironlake_set_pll_cpu_edp(struct intel_dp *intel_dp) > > struct drm_i915_private *dev_priv = dev->dev_private; > > u32 dpa_ctl; > > > > - DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", > > - crtc->config.adjusted_mode.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 (crtc->config.adjusted_mode.clock == 162000) { > > + 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. > > */ > > 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 > > > > > > -- > 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 761254d..103f4e9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3992,7 +3992,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, fdi_dotclock; + int lane, link_bw, fdi_dotclock; bool setup_ok, needs_recompute = false; retry: @@ -4005,12 +4005,7 @@ 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; - - fdi_dotclock = target_clock; + fdi_dotclock = adjusted_mode->clock; if (pipe_config->pixel_multiplier > 1) fdi_dotclock /= pipe_config->pixel_multiplier; @@ -4360,8 +4355,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; @@ -4414,7 +4407,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); @@ -4859,7 +4852,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"); @@ -5467,7 +5461,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) @@ -5495,7 +5488,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; @@ -5695,7 +5689,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"); @@ -5898,7 +5892,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... */ @@ -7789,6 +7783,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. @@ -7817,6 +7814,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 647cc2b..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); @@ -788,12 +785,11 @@ static void ironlake_set_pll_cpu_edp(struct intel_dp *intel_dp) struct drm_i915_private *dev_priv = dev->dev_private; u32 dpa_ctl; - DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", - crtc->config.adjusted_mode.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 (crtc->config.adjusted_mode.clock == 162000) { + 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. */ 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. */