[Intel-gfx] [PATCH 01/17] drm/i915: add encoder->pre_pll_enable callback

[Paulo Zanoni]

Hi

2012/11/26 Daniel Vetter <daniel.vetter at ffwll.ch>:
> Currently we have two encoder specific bits in the common mode_set
> functions:
> - lvds pin pair enabling
> - dp m/n setting and computation
>
> Now the lvds stuff needs to happen before the pll is enabled. Since
> that is done in the crtc_mode_set functions, we need to add a new
> callback to be able to move them to the encoder code (where they
> belong). The dp m/n stuff is a giant mess anyway (since it also
> confuses itself with the fdi link m/n handling), so that needs to be
> handled separately.
>
> I think that we can move the pll enabling down quite a bit, which
> might allow us to eventually merge encoder->pre_enable with this new
> pre_pll_enable callback. But for now this will allow us to clean
> things up a bit.
>
> Note that vlv doesn't support lvds, hence we don't need to change
> anything in there.

My only worry is that in the future we might use the pre_pll_enable
callback for some non-LVDS encoder, and then we'll forget about
calling this on VLV.

Even if you don't think this is necessary, the patch looks correct, so:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

Although I'd prefer to see "v3".

>
> v2: Fixup commit message, both suggested from Paulo Zanoni.
> - dp m/n doesn't need to happen before pll enabling
> - lvds doesn't exist on vlv, hence no changes required in the vlv pll
>   function.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  2 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 769fc8f..647f16e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4462,6 +4462,7 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>         struct drm_device *dev = crtc->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +       struct intel_encoder *encoder;
>         int pipe = intel_crtc->pipe;
>         u32 dpll;
>         bool is_sdvo;
> @@ -4530,6 +4531,10 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>         POSTING_READ(DPLL(pipe));
>         udelay(150);
>
> +       for_each_encoder_on_crtc(dev, crtc, encoder)
> +               if (encoder->pre_pll_enable)
> +                       encoder->pre_pll_enable(encoder);
> +
>         /* The LVDS pin pair needs to be on before the DPLLs are enabled.
>          * This is an exception to the general rule that mode_set doesn't turn
>          * things on.
> @@ -4574,6 +4579,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
>         struct drm_device *dev = crtc->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +       struct intel_encoder *encoder;
>         int pipe = intel_crtc->pipe;
>         u32 dpll;
>
> @@ -4607,6 +4613,10 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
>         POSTING_READ(DPLL(pipe));
>         udelay(150);
>
> +       for_each_encoder_on_crtc(dev, crtc, encoder)
> +               if (encoder->pre_pll_enable)
> +                       encoder->pre_pll_enable(encoder);
> +
>         /* The LVDS pin pair needs to be on before the DPLLs are enabled.
>          * This is an exception to the general rule that mode_set doesn't turn
>          * things on.
> @@ -5535,6 +5545,10 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>                 I915_WRITE(TRANSDPLINK_N1(pipe), 0);
>         }
>
> +       for_each_encoder_on_crtc(dev, crtc, encoder)
> +               if (encoder->pre_pll_enable)
> +                       encoder->pre_pll_enable(encoder);
> +
>         if (intel_crtc->pch_pll) {
>                 I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bcc5241..42a40a1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -153,6 +153,7 @@ struct intel_encoder {
>         bool cloneable;
>         bool connectors_active;
>         void (*hot_plug)(struct intel_encoder *);
> +       void (*pre_pll_enable)(struct intel_encoder *);
>         void (*pre_enable)(struct intel_encoder *);
>         void (*enable)(struct intel_encoder *);
>         void (*disable)(struct intel_encoder *);
> --
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni