On Sun, Mar 03, 2013 at 07:01:12PM +0100, Daniel Vetter wrote: > On Fri, Feb 22, 2013 at 03:51:15PM +0200, Ville Syrj?l? wrote: > > On Fri, Feb 22, 2013 at 12:56:47AM +0100, Daniel Vetter wrote: > > > Only used by the lvds encoder. Note that we shouldn't do the same > > > simple conversion with the FORCE_6BPC flag, since that's much better > > > handled by moving all the pipe_bpc computation around. > > > > > > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++- > > > drivers/gpu/drm/i915/intel_drv.h | 10 ++++++---- > > > drivers/gpu/drm/i915/intel_lvds.c | 19 +++++++++---------- > > > 3 files changed, 26 insertions(+), 15 deletions(-) > > > > > <snip> > > > @@ -359,6 +357,7 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder, > > > I915_WRITE(BCLRPAT(pipe), 0); > > > > > > drm_mode_set_crtcinfo(adjusted_mode, 0); > > > + pipe_config->timings_set = true; > > > > This changes the behaviour a bit. Previously the flag was only set from > > the centering funcs, but now it's set always. Is that intentional? > > The crtc code also calls drm_mode_set_crtcinfo(adjusted_mode, 0) if > ->timings_set is false, so doesn't result in any behaviour change. Hence > I've figure that fewer lines of code should be better. Want me to change > it back or just add a bit of text to the commit message? E.g. > > "Note that since the lvds code unconditionally sets the crtc timings, we > can also unconditionally set the respective flag and not just when we set > special timings like the old code did." Right, I should have actually read the code with some thought instead of blindly looking at the changes. I think amending the commit message with the proposed text would be enough. -- Ville Syrj?l? Intel OTC