On Thu, Apr 25, 2013 at 03:20:12PM +0200, Daniel Vetter wrote:
> Up to now we've relied on the bios to get this right for us. Let's try
> out whether our code has improved a bit, since we should dither
> always when the output bpp doesn't match the plane bpp.
> - gen5+ should be fine, since we only use the bios hint as an upgrade.
> - gen4 changes, since here dithering is still controlled in the lvds
> register.
> - gen2/3 has implicit dithering depeding upon whether you use 2 or 3
> lvds pairs (which makes sense, since it only supports 8bpc pipe
> outpu configurations).
> - hsw doesn't support lvds.
>
> v2: Remove redudant dither setting.
>
> v3: Completly drop reliance on dev_priv->lvds_dither.
>
> v4: Enable dithering on gen2/3 only when we have a 18bpp panel, since
> up-dithering to a 24bpp panel is not supported by the hw. Spotted by
> Ville.
>
> 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 | 25 +++++++------------------
> drivers/gpu/drm/i915/intel_drv.h | 5 +++++
> drivers/gpu/drm/i915/intel_lvds.c | 12 +++++++-----
> 3 files changed, 19 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 16106fe..f26a867 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5156,8 +5156,7 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
> }
>
> static void ironlake_set_pipeconf(struct drm_crtc *crtc,
> - struct drm_display_mode *adjusted_mode,
> - bool dither)
> + struct drm_display_mode *adjusted_mode)
> {
> struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -5186,7 +5185,7 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
> }
>
> val &= ~(PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
> - if (dither)
> + if (intel_crtc->config.dither)
> val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
>
> val &= ~PIPECONF_INTERLACE_MASK;
> @@ -5269,8 +5268,7 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc)
> }
>
> static void haswell_set_pipeconf(struct drm_crtc *crtc,
> - struct drm_display_mode *adjusted_mode,
> - bool dither)
> + struct drm_display_mode *adjusted_mode)
> {
> struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -5280,7 +5278,7 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc,
> val = I915_READ(PIPECONF(cpu_transcoder));
>
> val &= ~(PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
> - if (dither)
> + if (intel_crtc->config.dither)
> val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
>
> val &= ~PIPECONF_INTERLACE_MASK_HSW;
> @@ -5641,7 +5639,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> bool is_lvds = false;
> struct intel_encoder *encoder;
> int ret;
> - bool dither, fdi_config_ok;
> + bool fdi_config_ok;
>
> for_each_encoder_on_crtc(dev, crtc, encoder) {
> switch (encoder->type) {
> @@ -5676,11 +5674,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> /* Ensure that the cursor is valid for the new mode before changing... */
> intel_crtc_update_cursor(crtc, true);
>
> - /* determine panel color depth */
> - dither = intel_crtc->config.dither;
> - if (is_lvds && dev_priv->lvds_dither)
> - dither = true;
> -
> DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe_name(pipe));
> drm_mode_debug_printmodeline(mode);
>
> @@ -5747,7 +5740,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>
> fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
>
> - ironlake_set_pipeconf(crtc, adjusted_mode, dither);
> + ironlake_set_pipeconf(crtc, adjusted_mode);
>
> /* Set up the display plane register */
> I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
> @@ -5824,7 +5817,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> bool is_cpu_edp = false;
> struct intel_encoder *encoder;
> int ret;
> - bool dither;
>
> for_each_encoder_on_crtc(dev, crtc, encoder) {
> switch (encoder->type) {
> @@ -5860,9 +5852,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> /* Ensure that the cursor is valid for the new mode before changing... */
> intel_crtc_update_cursor(crtc, true);
>
> - /* determine panel color depth */
> - dither = intel_crtc->config.dither;
> -
> DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe_name(pipe));
> drm_mode_debug_printmodeline(mode);
>
> @@ -5876,7 +5865,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> if (intel_crtc->config.has_pch_encoder)
> ironlake_fdi_set_m_n(crtc);
>
> - haswell_set_pipeconf(crtc, adjusted_mode, dither);
> + haswell_set_pipeconf(crtc, adjusted_mode);
>
> intel_set_pipe_csc(crtc);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 66922f8..723cac9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -213,6 +213,11 @@ struct intel_crtc_config {
> /* DP has a bunch of special case unfortunately, so mark the pipe
> * accordingly. */
> bool has_dp_encoder;
> +
> + /*
> + * Enable dithering, used when the selected pipe bpp doesn't match the
> + * plane bpp.
> + */
> bool dither;
>
> /* Controls for the clock computation, to override various stages. */
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 563f505..fdfc820 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -136,7 +136,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
> * special lvds dither control bit on pch-split platforms, dithering is
> * only controlled through the PIPECONF reg. */
> if (INTEL_INFO(dev)->gen == 4) {
> - if (dev_priv->lvds_dither)
> + if (intel_crtc->config.dither)
The description of the lvds port dither bit for gen4 also seems to
suggest it should only be enabled for 18bpp lvds.
> temp |= LVDS_ENABLE_DITHER;
> else
> temp &= ~LVDS_ENABLE_DITHER;
> @@ -335,7 +335,13 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
> DRM_DEBUG_KMS("forcing display bpp (was %d) to LVDS (%d)\n",
> pipe_config->pipe_bpp, lvds_bpp);
> pipe_config->pipe_bpp = lvds_bpp;
> +
> + /* Make sure pre-965 set dither correctly for 18bpp panels. */
> + if (INTEL_INFO(dev)->gen < 4 && lvds_bpp == 18)
> + pfit_control |= PANEL_8TO6_DITHER_ENABLE;
> +
> }
> +
> /*
> * We have timings from the BIOS for the panel, put them in
> * to the adjusted mode. The CRTC will be set up for this mode,
> @@ -470,10 +476,6 @@ out:
> pfit_pgm_ratios = 0;
> }
>
> - /* Make sure pre-965 set dither correctly */
> - if (INTEL_INFO(dev)->gen < 4 && dev_priv->lvds_dither)
> - pfit_control |= PANEL_8TO6_DITHER_ENABLE;
> -
> if (pfit_control != lvds_encoder->pfit_control ||
> pfit_pgm_ratios != lvds_encoder->pfit_pgm_ratios) {
> lvds_encoder->pfit_control = pfit_control;
> --
> 1.7.11.7
--
Ville Syrj?l?
Intel OTC
