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). v5: Also clear the dither type correctly as spotted by Ville. v6: As Ville pointed out we need to indeed set the dithering both in the pipeconf register (for DP outputs) and in the LVDS port register (for LVDS ouputs). Otherwise LVDS panel will not get properly dithered. The old patch got away with this since it forgot to clear the LVDS dither bit ... Cc: Ville Syrj?l? <ville.syrjala at linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 505c2fc..3cdb921 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4591,22 +4591,30 @@ 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); + + /* 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(); } } -- 1.7.11.7