Re: [PATCH 1/2] drm/i915: adjusted_mode->clock in the dp mode_fixup

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, 30 May 2012 13:52:02 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> ... instead of changing mode->clock, which we should leave as-is.
> 
> We only touch that if it's a panel, and then adjusted mode->clock
> equals adjusted_mode->clock. Outside of intel_dp.c we only use
> ajusted_mode->clock in the mode_set functions.
> 
> Within intel_dp.c we only use it to calculate the dp dithering
> and link bw parameters, so that's the only thing we need to fix
> up.
> 
> As a temporary ugliness (until the cleanup in the next patch) we
> pass the adjusted_mode into dp_dither for both parameters (because
> that one still looks at mode->clock).
> 
> Note that we do overwrite adjusted_mode->clock with the selected dp
> link clock, but that only happens after we've calculated everything we
> need based on the dotclock of the adjusted output configuration.
> 
> v2: Adjust the debug message to also use adjusted_mode->clock.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 296cfc2..a9dffa6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -698,11 +698,6 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *mode,
>  		intel_fixed_panel_mode(intel_dp->panel_fixed_mode, adjusted_mode);
>  		intel_pch_panel_fitting(dev, DRM_MODE_SCALE_FULLSCREEN,
>  					mode, adjusted_mode);
> -		/*
> -		 * the mode->clock is used to calculate the Data&Link M/N
> -		 * of the pipe. For the eDP the fixed clock should be used.
> -		 */
> -		mode->clock = intel_dp->panel_fixed_mode->clock;

But in ironlake_crtc_mode_set() we have:

 if (is_cpu_edp) {
     target_clock = mode->clock;
 } else {
   if (is_dp)
     target_clock = mode->clock;
   else
     target_clock = adjusted_mode->clock;
 }

It would seem like eDP would have had mode->clock adjusted previously.
Similarly PCH eDP uses the adjusted mode->clock in intel_dp_set_m_n().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre