On Sun, 15 Apr 2012 19:24:34 +0200, Daniel Vetter <daniel.vetter at ffwll.ch> wrote: > This regression has been introduced in > > commit ca9bfa7eed20ea34e862804e62aae10eb159edbb > Author: Daniel Vetter <daniel.vetter at ffwll.ch> > Date: Sat Jan 28 14:49:20 2012 +0100 > > drm/i915: fixup interlaced vertical timings confusion, part 1 > > Unfortunately that commit failed to take into account that the lvds > code does some special adjustements to the crtc timings for upscaling > an centering. > > Fix this by explicitly computing crtc timings in the lvds mode fixup > function and setting a special flag in mode->private_flags if the crtc > timings have been adjusted. > > v2: Add a comment to explain the new mode driver private flag, > suggested by Eugeni Dodonov. > > Reported-and-Tested-by: Hans de Bruin <jmdebruin at xmsnet.nl> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=43071 > Reviewed-by: Eugeni Dodonov <eugeni.dodonov at intel.com> > Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch> > --- > drivers/gpu/drm/i915/intel_display.c | 7 +++++-- > drivers/gpu/drm/i915/intel_drv.h | 4 ++++ > drivers/gpu/drm/i915/intel_lvds.c | 6 ++++++ > 3 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index bae38ac..8be3091 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3478,8 +3478,11 @@ static bool intel_crtc_mode_fixup(struct drm_crtc *crtc, > return false; > } > > - /* All interlaced capable intel hw wants timings in frames. */ > - drm_mode_set_crtcinfo(adjusted_mode, 0); > + /* All interlaced capable intel hw wants timings in frames. Note though > + * that intel_lvds_mode_fixup does some funny tricks with the crtc > + * timings, so we need to be careful not to clobber these.*/ > + if (!(adjusted_mode->private_flags & INTEL_MODE_CRTC_TIMINGS_SET)) > + drm_mode_set_crtcinfo(adjusted_mode, 0); > > return true; > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 5a14149..715afa1 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -105,6 +105,10 @@ > #define INTEL_MODE_PIXEL_MULTIPLIER_SHIFT (0x0) > #define INTEL_MODE_PIXEL_MULTIPLIER_MASK (0xf << INTEL_MODE_PIXEL_MULTIPLIER_SHIFT) > #define INTEL_MODE_DP_FORCE_6BPC (0x10) > +/* This flag must be set by the encoder's mode_fixup if it changes the crtc > + * timings in the mode to prevent the crtc fixup from overwriting them. > + * Currently only lvds needs that. */ > +#define INTEL_MODE_CRTC_TIMINGS_SET (0x20) > > static inline void > intel_mode_set_pixel_multiplier(struct drm_display_mode *mode, > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index 95db2e9..30e2c82 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -187,6 +187,8 @@ centre_horizontally(struct drm_display_mode *mode, > > mode->crtc_hsync_start = mode->crtc_hblank_start + sync_pos; > mode->crtc_hsync_end = mode->crtc_hsync_start + sync_width; > + > + mode->private_flags |= INTEL_MODE_CRTC_TIMINGS_SET; > } > > static void > @@ -208,6 +210,8 @@ centre_vertically(struct drm_display_mode *mode, > > mode->crtc_vsync_start = mode->crtc_vblank_start + sync_pos; > mode->crtc_vsync_end = mode->crtc_vsync_start + sync_width; > + > + mode->private_flags |= INTEL_MODE_CRTC_TIMINGS_SET; > } > > static inline u32 panel_fitter_scaling(u32 source, u32 target) > @@ -283,6 +287,8 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder, > for_each_pipe(pipe) > I915_WRITE(BCLRPAT(pipe), 0); > > + drm_mode_set_crtcinfo(adjusted_mode, 0); This line at least is confusing, since adjusted_mode is already overriden in intel_fixed_panel_mode(). -Chris -- Chris Wilson, Intel Open Source Technology Centre
