[Intel-gfx] [PATCH] drm/i915: Simplify i830 DVO 2x clock handling

[Chris Wilson]

Quoting Ville Syrjala (2019-03-04 13:41:13)
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Let's just always enable the DVO 2x clock on i830. This way we don't
> have to track if DVO is being used or not. The spec does suggest we
> should disable the clock when it isn't needed, but this does appear
> to work just fine.

And after i830, 2X_MODE seems to be the default? Whereas the only reason
for i830 being special is that both pipes must be using the same mode,
ergo we didn't do either (or something like that).

> This removes another crtc->config usage.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> @@ -1468,26 +1455,12 @@ static void i9xx_enable_pll(struct intel_crtc *crtc,
>         /*
>          * Apparently we need to have VGA mode enabled prior to changing
>          * the P1/P2 dividers. Otherwise the DPLL will keep using the old
>          * dividers, even though the register value does change.
>          */
> -       I915_WRITE(reg, 0);
> -
> +       I915_WRITE(reg, dpll & ~DPLL_VGA_MODE_DIS);

This looks to be a separate tweak?

>         I915_WRITE(reg, dpll);
>  
>         /* Wait for the clocks to stabilize. */

> @@ -7494,7 +7457,19 @@ static void i8xx_compute_dpll(struct intel_crtc *crtc,
>                         dpll |= PLL_P2_DIVIDE_BY_4;
>         }
>  
> -       if (!IS_I830(dev_priv) &&
> +       /*
> +        * Bspec:
> +        * "[Almador Errata}: For the correct operation of the muxed DVO pins
> +        *  (GDEVSELB/ I 2 Cdata, GIRDBY/I 2 CClk) and (GFRAMEB/DVI_Data,
> +        *  GTRDYB/DVI_Clk): Bit 31 (DPLL VCO Enable) and Bit 30 (2X Clock
> +        *  Enable) must be set to “1” in both the DPLL A Control Register
> +        *  (06014h-06017h) and DPLL B Control Register (06018h-0601Bh)."
> +        *
> +        * For simplicity We simply keep both bits always enabled in
> +        * both DPLLS. The spec says we should disable the DVO 2X clock
> +        * when not needed, but this seems to work fine in practice.
> +        */
> +       if (IS_I830(dev_priv) ||
>             intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DVO))
>                 dpll |= DPLL_DVO_2X_MODE;

Is VGA/CRT a native encoder? That might be a useful test to check that
still works.

I couldn't see anything you missed for DPLL_DVO_2X_MODE, so happy that
this does what it says on the tin, and I trust that you actually ran
this on an i830...

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris