2014-09-04 8:27 GMT-03:00 Damien Lespiau <damien.lespiau@xxxxxxxxx>:
> From: Satheeshakrishna M <satheeshakrishna.m@xxxxxxxxx>
>
> Skylake deprecates the usage of PORT_CLK_SEL and we are advised to use
> the new DPLL_CRTL2 for the DDI->PLL mapping.
>
> Signed-off-by: Satheeshakrishna M <satheeshakrishna.m@xxxxxxxxx>
> Signed-off-by: Damien Lespiau <damien.lespiau@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 30 +++++++++++++++++++++++++-----
> 1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 53ac23d..439cd50 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1223,7 +1223,8 @@ void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc)
> static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> {
> struct drm_encoder *encoder = &intel_encoder->base;
> - struct drm_i915_private *dev_priv = encoder->dev->dev_private;
> + struct drm_device *dev = encoder->dev;
Bikeshed: this could be avoided if you just used dev_priv for the
IS_SKYLAKE macro, now that it accepts it :)
> + struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> enum port port = intel_ddi_get_encoder_port(intel_encoder);
> int type = intel_encoder->type;
> @@ -1242,8 +1243,22 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> intel_edp_panel_on(intel_dp);
> }
>
> - WARN_ON(crtc->config.ddi_pll_sel == PORT_CLK_SEL_NONE);
> - I915_WRITE(PORT_CLK_SEL(port), crtc->config.ddi_pll_sel);
> + if (IS_SKYLAKE(dev)) {
Bikeshed: invert the check so future gens are on the "else".
> + enum skl_dpll dpll = crtc->config.ddi_pll_sel;
> + uint32_t val;
> +
> + val = I915_READ(DPLL_CTRL2);
> +
> + val &= ~(DPLL_CTRL2_DDI_CLK_OFF(port) |
> + DPLL_CTRL2_DDI_CLK_SEL_MASK(port));
> + val |= (DPLL_CTRL2_DDI_CLK_SEL(dpll, port) |
> + DPLL_CTRL2_DDI_SEL_OVERRIDE(port));
> +
> + I915_WRITE(DPLL_CTRL2, val);
> + } else {
> + WARN_ON(crtc->config.ddi_pll_sel == PORT_CLK_SEL_NONE);
> + I915_WRITE(PORT_CLK_SEL(port), crtc->config.ddi_pll_sel);
> + }
Bikeshed: extract this chunk to intel_ddi_set_port_clock() or some other name.
>
> if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> @@ -1267,7 +1282,8 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> {
> struct drm_encoder *encoder = &intel_encoder->base;
> - struct drm_i915_private *dev_priv = encoder->dev->dev_private;
> + struct drm_device *dev = encoder->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> enum port port = intel_ddi_get_encoder_port(intel_encoder);
> int type = intel_encoder->type;
> uint32_t val;
> @@ -1295,7 +1311,11 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> intel_edp_panel_off(intel_dp);
> }
>
> - I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE);
> + if (IS_SKYLAKE(dev))
Same here.
With or without any changes: Reviewed-by: Paulo Zanoni
<paulo.r.zanoni@xxxxxxxxx>
> + I915_WRITE(DPLL_CTRL2, (I915_READ(DPLL_CTRL2) |
> + DPLL_CTRL2_DDI_CLK_OFF(port)));
> + else
> + I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE);
> }
>
> static void intel_enable_ddi(struct intel_encoder *intel_encoder)
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
