On Tue, Apr 23, 2013 at 06:27:54PM +0300, Ville Syrj?l? wrote: > On Fri, Apr 19, 2013 at 08:17:10PM +0200, Daniel Vetter wrote: > > The current code is rather ... ugly. The only thing it managed to pull > > off is getting 6bpc on DP working on g4x. Then someone added another > > custom hack for 6bpc eDP on vlv. Fix up this entire mess by properly > > implementing the PIPECONF-based dither/bpc controls on g4x/vlv. > > > > Note that compared to pch based platforms g4x/vlv don't support 12bpc > > modes. g4x is already caught, extend the check for vlv. > > > > The other fixup is to restrict the lvds-specific dithering to early > > gen4 devices - g4x should use the pipeconf dither controls. Note that > > on gen2/3 the dither control is in the panel fitter even. > > > > v2: Don't enable dithering when the pipe is in 10 bpc mode. Quoting > > from Bspec "PIPEACONF - Pipe A Configuration Register, bit 4": > > > > "Programming note: Dithering should only be enabled for 8 bpc or 6 > > bpc." > > > > v3: Actually drop the old ugly dither code. > > > > v4: Explain in a short comment why g4x/vlv shouldn't dither for 30 bpp > > pipes (Jesse). > > > > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch> > > --- > > drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++-------------- > > drivers/gpu/drm/i915/intel_lvds.c | 2 +- > > 2 files changed, 22 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 8c36376..7f1ab8b 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -4605,22 +4605,29 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc) > > pipeconf &= ~PIPECONF_DOUBLE_WIDE; > > } > > > > - /* default to 8bpc */ > > - pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN); > > - if (intel_crtc->config.has_dp_encoder) { > > - if (intel_crtc->config.dither) { > > - pipeconf |= PIPECONF_6BPC | > > - PIPECONF_DITHER_EN | > > + /* only g4x and later have fancy bpc/dither controls */ > > + if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) { > > + pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN); > ^^^^^^^^^^^^^^^^^ > > PIPECONF_DITHER_TYPE_MASK Will fix up, thansk for spotting this. > The g4x docs are a bit confusing though. They seem to indicate the the > PIPECONF dither controls only affect DP. Hm, this could put a pending question from Jesse at ease whether we should still enable dithering in the lvds port register on g4x. Can you point me at the relevant bspec language? I didn't spot anything when hunting around in the docs ... Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch