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 The g4x docs are a bit confusing though. They seem to indicate the the PIPECONF dither controls only affect DP. > + > + /* Bspec claims that we can't use dithering for 30bpp pipes. */ > + if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30) > + pipeconf |= PIPECONF_DITHER_EN | > PIPECONF_DITHER_TYPE_SP; > - } > - } > > - if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(&intel_crtc->base, > - INTEL_OUTPUT_EDP)) { > - if (intel_crtc->config.dither) { > - pipeconf |= PIPECONF_6BPC | > - PIPECONF_ENABLE | > - I965_PIPECONF_ACTIVE; > + pipeconf &= ~PIPECONF_BPC_MASK; > + switch (intel_crtc->config.pipe_bpp) { > + case 18: > + pipeconf |= PIPECONF_6BPC; > + break; > + case 24: > + pipeconf |= PIPECONF_8BPC; > + break; > + case 30: > + pipeconf |= PIPECONF_10BPC; > + break; > + default: > + /* Case prevented by intel_choose_pipe_bpp_dither. */ > + BUG(); > } > } > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index 58a98ff..094f3c5 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -135,7 +135,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder) > /* Set the dithering flag on LVDS as needed, note that there is no > * special lvds dither control bit on pch-split platforms, dithering is > * only controlled through the PIPECONF reg. */ > - if (INTEL_INFO(dev)->gen == 4) { > + if (INTEL_INFO(dev)->gen == 4 && !IS_G4X(dev)) { > if (intel_crtc->config.dither) > temp |= LVDS_ENABLE_DITHER; > else > -- > 1.7.11.7 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrj?l? Intel OTC