On Fri, Apr 19, 2013 at 11:14:34AM +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.
> 
> Signed-off-by: Daniel Vetter <daniel.vet...@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 d91bad8..2a82bd8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5160,8 +5160,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);
> @@ -5190,7 +5189,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;
> @@ -5273,8 +5272,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);
> @@ -5284,7 +5282,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;
> @@ -5645,7 +5643,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) {
> @@ -5680,11 +5678,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);
>  
> @@ -5752,7 +5745,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);
> @@ -5829,7 +5822,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) {
> @@ -5865,9 +5857,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);
>  
> @@ -5881,7 +5870,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 c20201d..e3ca7e7 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -201,6 +201,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..58a98ff 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)
>                       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 */
> +             if (INTEL_INFO(dev)->gen < 4)
> +                     pfit_control |= PANEL_8TO6_DITHER_ENABLE;

I'm not quite sure about the gen4 and earlier stuff. Isn't the pipe
always 8bpc, and then we should enable dithering on the port/pfit
when lvds is 6bpc.

Right now I think we'll start with pipe_bpp=18 when the primary plane
surface is 16bpp, and then we wouldn't enable dithering here for 6bpc
lvds.

> +
>       }
> +
>       /*
>        * 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
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to