On Wed, Sep 12, 2012 at 10:06:29AM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zan...@intel.com>
>
> Because ironlake_crtc_mode_set is a giant function that used to have
> 404 lines. Let's try to make it less complex/confusing.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zan...@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 88
> ++++++++++++++++++++--------------
> 1 file changed, 52 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index e061acd..244bce6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4646,6 +4646,50 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
> return 120000;
> }
>
> +static void ironlake_set_pipeconf(struct drm_crtc *crtc,
> + struct drm_display_mode *adjusted_mode,
> + bool dither)
> +{
> + struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + int pipe = intel_crtc->pipe;
> + uint32_t val;
> +
> + val = I915_READ(PIPECONF(pipe));
> +
> + val &= ~PIPE_BPC_MASK;
> + switch (intel_crtc->bpp) {
> + case 18:
> + val |= PIPE_6BPC;
> + break;
> + case 24:
> + val |= PIPE_8BPC;
> + break;
> + case 30:
> + val |= PIPE_10BPC;
> + break;
> + case 36:
> + val |= PIPE_12BPC;
> + break;
> + default:
> + val |= PIPE_8BPC;
> + break;
> + }
> +
> + val &= ~(PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
> + if (dither)
> + val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
> +
> + val &= ~PIPECONF_INTERLACE_MASK;
> + if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
> + val |= PIPECONF_INTERLACED_ILK;
> + else
> + val |= PIPECONF_PROGRESSIVE;
> +
> + I915_WRITE(PIPECONF(pipe), val);
> + POSTING_READ(PIPECONF(pipe));
> +}
> +
> static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> struct drm_display_mode *mode,
> struct drm_display_mode *adjusted_mode,
> @@ -4659,7 +4703,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> int plane = intel_crtc->plane;
> int refclk, num_connectors = 0;
> intel_clock_t clock, reduced_clock;
> - u32 dpll, fp = 0, fp2 = 0, dspcntr, pipeconf;
> + u32 dpll, fp = 0, fp2 = 0, dspcntr;
> bool ok, has_reduced_clock = false, is_sdvo = false;
> bool is_crt = false, is_lvds = false, is_tv = false, is_dp = false;
> struct intel_encoder *encoder, *edp_encoder = NULL;
> @@ -4768,32 +4812,17 @@ static int ironlake_crtc_mode_set(struct drm_crtc
> *crtc,
> target_clock = adjusted_mode->clock;
>
> /* determine panel color depth */
> - temp = I915_READ(PIPECONF(pipe));
> - temp &= ~PIPE_BPC_MASK;
> dither = intel_choose_pipe_bpp_dither(crtc, fb, &pipe_bpp, mode);
> - switch (pipe_bpp) {
> - case 18:
> - temp |= PIPE_6BPC;
> - break;
> - case 24:
> - temp |= PIPE_8BPC;
> - break;
> - case 30:
> - temp |= PIPE_10BPC;
> - break;
> - case 36:
> - temp |= PIPE_12BPC;
> - break;
> - default:
> + if (is_lvds && dev_priv->lvds_dither)
> + dither = true;
> +
> + if (pipe_bpp != 18 && pipe_bpp != 24 && pipe_bpp != 30 &&
> + pipe_bpp != 36) {
> WARN(1, "intel_choose_pipe_bpp returned invalid value %d\n",
> - pipe_bpp);
> - temp |= PIPE_8BPC;
> + pipe_bpp);
> pipe_bpp = 24;
> - break;
choose_pipe_bpp_dither already has this fallback code and we duplicate
things here, again. Can you follow up with a 2nd patch to rip this out and
add a BUG(); to the new ilk_set_pipeconf function for the default: bpp
case and directly store the bpp in intel_crtc->bpp?
Patch looks good, applied.
-Daniel
> }
> -
> intel_crtc->bpp = pipe_bpp;
> - I915_WRITE(PIPECONF(pipe), temp);
>
> if (!lane) {
> /*
> @@ -4877,9 +4906,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> else
> dpll |= PLL_REF_INPUT_DREFCLK;
>
> - /* setup pipeconf */
> - pipeconf = I915_READ(PIPECONF(pipe));
> -
> /* Set up the display plane register */
> dspcntr = DISPPLANE_GAMMA_ENABLE;
>
> @@ -4942,12 +4968,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc
> *crtc,
> I915_WRITE(PCH_LVDS, temp);
> }
>
> - pipeconf &= ~PIPECONF_DITHER_EN;
> - pipeconf &= ~PIPECONF_DITHER_TYPE_MASK;
> - if ((is_lvds && dev_priv->lvds_dither) || dither) {
> - pipeconf |= PIPECONF_DITHER_EN;
> - pipeconf |= PIPECONF_DITHER_TYPE_SP;
> - }
> if (is_dp && !is_cpu_edp) {
> intel_dp_set_m_n(crtc, mode, adjusted_mode);
> } else {
> @@ -4983,9 +5003,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> }
> }
>
> - pipeconf &= ~PIPECONF_INTERLACE_MASK;
> if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> - pipeconf |= PIPECONF_INTERLACED_ILK;
> /* the chip adds 2 halflines automatically */
> adjusted_mode->crtc_vtotal -= 1;
> adjusted_mode->crtc_vblank_end -= 1;
> @@ -4993,7 +5011,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> adjusted_mode->crtc_hsync_start
> - adjusted_mode->crtc_htotal/2);
> } else {
> - pipeconf |= PIPECONF_PROGRESSIVE;
> I915_WRITE(VSYNCSHIFT(pipe), 0);
> }
>
> @@ -5031,8 +5048,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> if (is_cpu_edp)
> ironlake_set_pll_edp(crtc, adjusted_mode->clock);
>
> - I915_WRITE(PIPECONF(pipe), pipeconf);
> - POSTING_READ(PIPECONF(pipe));
> + ironlake_set_pipeconf(crtc, adjusted_mode, dither);
>
> intel_wait_for_vblank(dev, pipe);
>
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
