Hi
2012/11/5 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
>
> Both need to happen before we enable the pll.
Not true, at least for the docs I checked (gen6+), setting/computing
the m/n registers can be done anytime before enabling the CPU pipe.
Please change the commit message :)
> 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).
>
> 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 callbakc. But for now this will allow us to clean
> things up a bit.
>
> 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 2ecc7f8..1ad6d34 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4465,6 +4465,7 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
Don't we also need to patch vlv_update_pll?
> 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;
We kinda have a "naming standard" where variables of type "struct
intel_xxx" are called "intel_xxx" and variables of type "struct
drm_xxx" are called "xxx". I I'd vote to call this intel_encoder.
> int pipe = intel_crtc->pipe;
> u32 dpll;
> bool is_sdvo;
> @@ -4533,6 +4534,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.
> @@ -4577,6 +4582,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;
Same here.
> int pipe = intel_crtc->pipe;
> u32 dpll;
>
> @@ -4610,6 +4616,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.
> @@ -5537,6 +5547,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
