On Wed, May 30, 2012 at 01:18:35PM +0100, Chris Wilson wrote:
> On Wed, 30 May 2012 13:52:02 +0200, Daniel Vetter <daniel.vetter@xxxxxxxx> 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@xxxxxxxx>
> > ---
> > 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().
Well, adjusted_mode->clock after we've run intel_dp_mode_fixup should be
the same in all cases, because we overwrite it with the fixed dp link
clock we're selecting. The target_clock otoh looks more worrisome. I guess
I've managed to not hit this because I don't have an eDP panel (where we
change mode->clock), but still I guess we need to clean this up a bit.
I'll try to come up with a way to avoid this maze.
-Daniel
--
Daniel Vetter
Mail: daniel@xxxxxxxx
Mobile: +41 (0)79 365 57 48
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
