On Thu, 28 Mar 2013 10:41:58 +0100
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> We need it in the fdi m_n computation, which nicely kills almost
> all ugly special cases in there.
>
> It looks like we also need this to handle 12bpc hdmi correctly.
>
> Eventually it might be better to switch things around and put the
> target clock into adjusted_mode->clock and create a new pipe_config
> parameter for the port link clock.
>
> v2: Add a massive comment in the code to explain this mess.
>
> v3: s/dp_target_clock/pixel_target_clock in anticipation of the hdmi
> use-case.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/intel_display.c | 25 +++----------------------
> drivers/gpu/drm/i915/intel_dp.c | 1 +
> drivers/gpu/drm/i915/intel_drv.h | 7 ++++++-
> 3 files changed, 10 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dfa8919..464eb90 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5356,25 +5356,9 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct drm_display_mode *adjusted_mode =
> &intel_crtc->config.adjusted_mode;
> - struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
> - struct intel_encoder *intel_encoder, *edp_encoder = NULL;
> struct intel_link_m_n m_n = {0};
> int target_clock, lane, link_bw;
> - bool is_dp = false, is_cpu_edp = false;
> -
> - for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> - switch (intel_encoder->type) {
> - case INTEL_OUTPUT_DISPLAYPORT:
> - is_dp = true;
> - break;
> - case INTEL_OUTPUT_EDP:
> - is_dp = true;
> - if (!intel_encoder_is_pch_edp(&intel_encoder->base))
> - is_cpu_edp = true;
> - edp_encoder = intel_encoder;
> - break;
> - }
> - }
> + uint32_t bps;
>
> /* FDI is a binary signal running at ~2.7GHz, encoding
> * each output octet as 10 bits. The actual frequency
> @@ -5385,11 +5369,8 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
> */
> link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
>
> - /* [e]DP over FDI requires target mode clock instead of link clock. */
> - if (edp_encoder)
> - target_clock = intel_edp_target_clock(edp_encoder, mode);
> - else if (is_dp)
> - target_clock = mode->clock;
> + if (intel_crtc->config.pixel_target_clock)
> + target_clock = intel_crtc->config.pixel_target_clock;
> else
> target_clock = adjusted_mode->clock;
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ef680d5..b1bf00b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -753,6 +753,7 @@ found:
> 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;
>
> DRM_DEBUG_KMS("DP link bw %02x lane count %d clock %d bpp %d\n",
> intel_dp->link_bw, intel_dp->lane_count,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 41fabcb..2a86a12 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -198,7 +198,12 @@ struct intel_crtc_config {
> bool dither;
> 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).
> + */
> + int pixel_target_clock;
> /* Used by SDVO (and if we ever fix it, HDMI). */
> unsigned pixel_multiplier;
> };
Since you already explained there are other callers of
edp_target_clock():
Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
