On Fri, Feb 17, 2017 at 11:22:05AM -0200, Paulo Zanoni wrote:
> Each x_modeset_calc_cdclk() has to do the same platform checks twice,
> so extract them to a single function. This way, the platform checks
> are all in the same place, and the platform-common code gets rid of
> all the platform-specific checks, which IMHO makes the code easier to
> read.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_cdclk.c | 84 ++++++++++++++++++++------------------
> 1 file changed, 45 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index d505ff1..6efc5f4 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1496,6 +1496,47 @@ static int intel_max_pixel_rate(struct drm_atomic_state *state)
> return max_pixel_rate;
> }
>
> +static void intel_calc_cdclk(struct intel_atomic_state *state, int max_pixclk,
> + int *cdclk, int *vco)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +
> + switch (INTEL_INFO(dev_priv)->platform) {
> + case INTEL_VALLEYVIEW:
> + case INTEL_CHERRYVIEW:
> + *cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
> + break;
> + case INTEL_BROADWELL:
> + /*
> + * FIXME: should also account for plane ratio once 64bpp pixel
> + * formats are supported.
> + */
> + *cdclk = bdw_calc_cdclk(max_pixclk);
> + break;
> + case INTEL_SKYLAKE:
> + case INTEL_KABYLAKE:
> + /*
> + * FIXME: should also account for plane ratio once 64bpp pixel
> + * formats are supported.
> + */
> + *vco = state->cdclk.logical.vco;
> + if (!*vco)
> + *vco = dev_priv->skl_preferred_vco_freq;
> + *cdclk = skl_calc_cdclk(max_pixclk, *vco);
> + break;
> + case INTEL_BROXTON:
> + *cdclk = bxt_calc_cdclk(max_pixclk);
> + *vco = bxt_de_pll_vco(dev_priv, *cdclk);
> + break;
> + case INTEL_GEMINILAKE:
> + *cdclk = glk_calc_cdclk(max_pixclk);
> + *vco = glk_de_pll_vco(dev_priv, *cdclk);
> + break;
> + default:
> + MISSING_CASE(INTEL_INFO(dev_priv)->platform);
> + }
> +}
How about just replacing the .modeset_calc_cdclk() vfunc with a slightly
lower level vfunc that just computes the cdclk/vco/whatever without
containing the active_crtcs logic?
Then we should have just
intel_modeset_calc_cdclk()
{
.calc_cdclk(logical, max_pixclk);
/*
* maybe keep the max_cdclk check here, although it that
* happens I think we have a bug somewhere, so perhaps
* just convert it into a WARN, or drop entirely.
*/
if (!active_crtcs)
.calc_cdclk(actual, 0);
else
actual = logical;
}
> +
> static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
> {
> struct drm_i915_private *dev_priv = to_i915(state->dev);
> @@ -1503,14 +1544,7 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
> int max_pixclk = intel_max_pixel_rate(state);
> int cdclk;
>
> - /*
> - * FIXME: Broadwell should also account for plane ratio once 64bpp pixel
> - * formats are supported.
> - */
> - if (IS_BROADWELL(dev_priv))
> - cdclk = bdw_calc_cdclk(max_pixclk);
> - else
> - cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
> + intel_calc_cdclk(intel_state, max_pixclk, &cdclk, NULL);
>
> if (cdclk > dev_priv->max_cdclk_freq) {
> DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> @@ -1521,11 +1555,7 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
> intel_state->cdclk.logical.cdclk = cdclk;
>
> if (!intel_state->active_crtcs) {
> - if (IS_BROADWELL(dev_priv))
> - cdclk = bdw_calc_cdclk(0);
> - else
> - cdclk = vlv_calc_cdclk(dev_priv, 0);
> -
> + intel_calc_cdclk(intel_state, 0, &cdclk, NULL);
> intel_state->cdclk.actual.cdclk = cdclk;
> } else {
> intel_state->cdclk.actual = intel_state->cdclk.logical;
> @@ -1541,22 +1571,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
> int max_pixclk = intel_max_pixel_rate(state);
> int cdclk, vco;
>
> - /*
> - * FIXME: Skylake/Kabylake should also account for plane ratio once
> - * 64bpp pixel formats are supported.
> - */
> - if (IS_GEMINILAKE(dev_priv)) {
> - cdclk = glk_calc_cdclk(max_pixclk);
> - vco = glk_de_pll_vco(dev_priv, cdclk);
> - } else if (IS_BROXTON(dev_priv)) {
> - cdclk = bxt_calc_cdclk(max_pixclk);
> - vco = bxt_de_pll_vco(dev_priv, cdclk);
> - } else {
> - vco = intel_state->cdclk.logical.vco;
> - if (!vco)
> - vco = dev_priv->skl_preferred_vco_freq;
> - cdclk = skl_calc_cdclk(max_pixclk, vco);
> - }
> + intel_calc_cdclk(intel_state, max_pixclk, &cdclk, &vco);
>
> if (cdclk > dev_priv->max_cdclk_freq) {
> DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> @@ -1568,16 +1583,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
> intel_state->cdclk.logical.cdclk = cdclk;
>
> if (!intel_state->active_crtcs) {
> - if (IS_GEMINILAKE(dev_priv)) {
> - cdclk = glk_calc_cdclk(0);
> - vco = glk_de_pll_vco(dev_priv, cdclk);
> - } else if (IS_BROXTON(dev_priv)) {
> - cdclk = bxt_calc_cdclk(0);
> - vco = bxt_de_pll_vco(dev_priv, cdclk);
> - } else {
> - cdclk = skl_calc_cdclk(0, vco);
> - }
> -
> + intel_calc_cdclk(intel_state, 0, &cdclk, &vco);
> intel_state->cdclk.actual.vco = vco;
> intel_state->cdclk.actual.cdclk = cdclk;
> } else {
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
