On Wed, Apr 24, 2013 at 12:30:14AM +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).
>
> 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;
bpc bits were already cleared. This line can be dropped.
Otherwise:
Reviewed-by: Ville Syrj?l? <ville.syrjala at linux.intel.com>
> + 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
--
Ville Syrj?l?
Intel OTC
