There's a lot of duplicated platform-independent logic in the current modeset_calc_cdclk() functions. Adding cdclk support for more platforms will only add more copies of this code.
To solve this problem, in this patch we create a new function called intel_modeset_calc_cdclk() which unifies the platform-independent logic, and we also create a new vfunc called calc_cdclk_state(), which is responsible to contain only the platform-dependent code. The code is now smaller and support for new platforms should be easier and not require duplicated platform-independent code. v2: Previously I had 2 different patches addressing these problems, but wiht Ville's suggestion I think it makes more sense to keep everything in a single patch (Ville). Cc: Ville Syrjälä <[email protected]> Cc: Rodrigo Vivi <[email protected]> Signed-off-by: Paulo Zanoni <[email protected]> --- drivers/gpu/drm/i915/i915_drv.h | 4 +- drivers/gpu/drm/i915/intel_cdclk.c | 187 ++++++++++------------------------- drivers/gpu/drm/i915/intel_display.c | 6 +- drivers/gpu/drm/i915/intel_drv.h | 1 + 4 files changed, 60 insertions(+), 138 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4e7046d..f1c35c7 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -644,7 +644,9 @@ struct drm_i915_display_funcs { struct intel_crtc_state *cstate); int (*compute_global_watermarks)(struct drm_atomic_state *state); void (*update_wm)(struct intel_crtc *crtc); - int (*modeset_calc_cdclk)(struct drm_atomic_state *state); + void (*calc_cdclk_state)(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state); /* Returns the active state of the crtc, and if the crtc is active, * fills out the pipe-config with the hw state. */ bool (*get_pipe_config)(struct intel_crtc *, diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index d643c0c..dd6fe25 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1496,148 +1496,69 @@ static int intel_max_pixel_rate(struct drm_atomic_state *state) return max_pixel_rate; } -static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state) +static void vlv_calc_cdclk_state(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state) { - struct drm_i915_private *dev_priv = to_i915(state->dev); - int max_pixclk = intel_max_pixel_rate(state); - struct intel_atomic_state *intel_state = - to_intel_atomic_state(state); - int cdclk; - - cdclk = vlv_calc_cdclk(dev_priv, max_pixclk); - - if (cdclk > dev_priv->max_cdclk_freq) { - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", - cdclk, dev_priv->max_cdclk_freq); - return -EINVAL; - } - - intel_state->cdclk.logical.cdclk = cdclk; - - if (!intel_state->active_crtcs) { - cdclk = vlv_calc_cdclk(dev_priv, 0); - - intel_state->cdclk.actual.cdclk = cdclk; - } else { - intel_state->cdclk.actual = - intel_state->cdclk.logical; - } - - return 0; + cdclk_state->cdclk = vlv_calc_cdclk(dev_priv, max_pixclk); } -static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state) +static void bdw_calc_cdclk_state(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state) { - struct drm_i915_private *dev_priv = to_i915(state->dev); - struct intel_atomic_state *intel_state = to_intel_atomic_state(state); - int max_pixclk = intel_max_pixel_rate(state); - int cdclk; - - /* - * FIXME should also account for plane ratio - * once 64bpp pixel formats are supported. - */ - cdclk = bdw_calc_cdclk(max_pixclk); - - if (cdclk > dev_priv->max_cdclk_freq) { - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", - cdclk, dev_priv->max_cdclk_freq); - return -EINVAL; - } - - intel_state->cdclk.logical.cdclk = cdclk; - - if (!intel_state->active_crtcs) { - cdclk = bdw_calc_cdclk(0); - - intel_state->cdclk.actual.cdclk = cdclk; - } else { - intel_state->cdclk.actual = - intel_state->cdclk.logical; - } - - return 0; + cdclk_state->cdclk = bdw_calc_cdclk(max_pixclk); } -static int skl_modeset_calc_cdclk(struct drm_atomic_state *state) +static void skl_calc_cdclk_state(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state) { - struct intel_atomic_state *intel_state = to_intel_atomic_state(state); - struct drm_i915_private *dev_priv = to_i915(state->dev); - const int max_pixclk = intel_max_pixel_rate(state); - int cdclk, vco; + if (!cdclk_state->vco) + cdclk_state->vco = dev_priv->skl_preferred_vco_freq; - vco = intel_state->cdclk.logical.vco; - if (!vco) - vco = dev_priv->skl_preferred_vco_freq; - - /* - * FIXME should also account for plane ratio - * once 64bpp pixel formats are supported. - */ - cdclk = skl_calc_cdclk(max_pixclk, vco); - - if (cdclk > dev_priv->max_cdclk_freq) { - DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", - cdclk, dev_priv->max_cdclk_freq); - return -EINVAL; - } - - intel_state->cdclk.logical.vco = vco; - intel_state->cdclk.logical.cdclk = cdclk; - - if (!intel_state->active_crtcs) { - cdclk = skl_calc_cdclk(0, vco); + cdclk_state->cdclk = skl_calc_cdclk(max_pixclk, cdclk_state->vco); +} - intel_state->cdclk.actual.vco = vco; - intel_state->cdclk.actual.cdclk = cdclk; - } else { - intel_state->cdclk.actual = - intel_state->cdclk.logical; - } +static void bxt_calc_cdclk_state(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state) +{ + cdclk_state->cdclk = bxt_calc_cdclk(max_pixclk); + cdclk_state->vco = bxt_de_pll_vco(dev_priv, cdclk_state->cdclk); +} - return 0; +static void glk_calc_cdclk_state(struct drm_i915_private *dev_priv, + int max_pixclk, + struct intel_cdclk_state *cdclk_state) +{ + cdclk_state->cdclk = glk_calc_cdclk(max_pixclk); + cdclk_state->vco = glk_de_pll_vco(dev_priv, cdclk_state->cdclk); } -static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state) +int intel_modeset_calc_cdclk(struct intel_atomic_state *state) { - struct drm_i915_private *dev_priv = to_i915(state->dev); - int max_pixclk = intel_max_pixel_rate(state); - struct intel_atomic_state *intel_state = - to_intel_atomic_state(state); - int cdclk, vco; + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + struct intel_cdclk_state *logical = &state->cdclk.logical; + struct intel_cdclk_state *actual = &state->cdclk.actual; + int max_pixclk = intel_max_pixel_rate(&state->base); - if (IS_GEMINILAKE(dev_priv)) { - cdclk = glk_calc_cdclk(max_pixclk); - vco = glk_de_pll_vco(dev_priv, cdclk); - } else { - cdclk = bxt_calc_cdclk(max_pixclk); - vco = bxt_de_pll_vco(dev_priv, cdclk); - } + /* + * FIXME: should also account for plane ratio once 64bpp pixel formats + * are supported. + */ + dev_priv->display.calc_cdclk_state(dev_priv, max_pixclk, logical); - if (cdclk > dev_priv->max_cdclk_freq) { + if (logical->cdclk > dev_priv->max_cdclk_freq) { DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n", - cdclk, dev_priv->max_cdclk_freq); + logical->cdclk, dev_priv->max_cdclk_freq); return -EINVAL; } - intel_state->cdclk.logical.vco = vco; - 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 { - cdclk = bxt_calc_cdclk(0); - vco = bxt_de_pll_vco(dev_priv, cdclk); - } - - intel_state->cdclk.actual.vco = vco; - intel_state->cdclk.actual.cdclk = cdclk; - } else { - intel_state->cdclk.actual = - intel_state->cdclk.logical; - } + if (!state->active_crtcs) + dev_priv->display.calc_cdclk_state(dev_priv, 0, actual); + else + *actual = *logical; return 0; } @@ -1823,24 +1744,22 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) { if (IS_CHERRYVIEW(dev_priv)) { dev_priv->display.set_cdclk = chv_set_cdclk; - dev_priv->display.modeset_calc_cdclk = - vlv_modeset_calc_cdclk; + dev_priv->display.calc_cdclk_state = vlv_calc_cdclk_state; } else if (IS_VALLEYVIEW(dev_priv)) { dev_priv->display.set_cdclk = vlv_set_cdclk; - dev_priv->display.modeset_calc_cdclk = - vlv_modeset_calc_cdclk; + dev_priv->display.calc_cdclk_state = vlv_calc_cdclk_state; } else if (IS_BROADWELL(dev_priv)) { dev_priv->display.set_cdclk = bdw_set_cdclk; - dev_priv->display.modeset_calc_cdclk = - bdw_modeset_calc_cdclk; - } else if (IS_GEN9_LP(dev_priv)) { + dev_priv->display.calc_cdclk_state = bdw_calc_cdclk_state; + } else if (IS_BROXTON(dev_priv)) { + dev_priv->display.set_cdclk = bxt_set_cdclk; + dev_priv->display.calc_cdclk_state = bxt_calc_cdclk_state; + } else if (IS_GEMINILAKE(dev_priv)) { dev_priv->display.set_cdclk = bxt_set_cdclk; - dev_priv->display.modeset_calc_cdclk = - bxt_modeset_calc_cdclk; + dev_priv->display.calc_cdclk_state = glk_calc_cdclk_state; } else if (IS_GEN9_BC(dev_priv)) { dev_priv->display.set_cdclk = skl_set_cdclk; - dev_priv->display.modeset_calc_cdclk = - skl_modeset_calc_cdclk; + dev_priv->display.calc_cdclk_state = skl_calc_cdclk_state; } if (IS_GEN9_BC(dev_priv)) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f6feb93..1d2cb491 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12414,8 +12414,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state) * mode set on this crtc. For other crtcs we need to use the * adjusted_mode bits in the crtc directly. */ - if (dev_priv->display.modeset_calc_cdclk) { - ret = dev_priv->display.modeset_calc_cdclk(state); + if (dev_priv->display.calc_cdclk_state) { + ret = intel_modeset_calc_cdclk(intel_state); if (ret < 0) return ret; @@ -15468,7 +15468,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) pixclk = crtc_state->pixel_rate; else - WARN_ON(dev_priv->display.modeset_calc_cdclk); + WARN_ON(dev_priv->display.calc_cdclk_state); /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 821c57c..3e112fe 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1260,6 +1260,7 @@ bool intel_cdclk_state_compare(const struct intel_cdclk_state *a, const struct intel_cdclk_state *b); void intel_set_cdclk(struct drm_i915_private *dev_priv, const struct intel_cdclk_state *cdclk_state); +int intel_modeset_calc_cdclk(struct intel_atomic_state *state); /* intel_display.c */ enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc); -- 2.7.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
