On Mon, 2016-12-19 at 19:28 +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> With the cdclk state, all the .modeset_commit_cdclk() hooks are
> now pointless wrappers. Let's replace them with just a .set_cdclk()
> function pointer. However let's wrap that in a small helper that
> does the state comparison and prints a unified debug message across
> all platforms. We didn't even have the debug print on all platforms
> previously. This reduces the clutter in intel_atomic_commit_tail() a
> little bit.
>
> v2: Wrap .set_cdclk() in intel_set_cdclk()
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Nice cleanup!
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_cdclk.c | 71 +++++++++++++--------------------
> ---
> drivers/gpu/drm/i915/intel_display.c | 5 +--
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 4 files changed, 30 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b5a8d0f4cfbd..faf753570a9a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -614,6 +614,8 @@ struct intel_cdclk_state;
> struct drm_i915_display_funcs {
> void (*get_cdclk)(struct drm_i915_private *dev_priv,
> struct intel_cdclk_state *cdclk_state);
> + void (*set_cdclk)(struct drm_i915_private *dev_priv,
> + const struct intel_cdclk_state *cdclk_state);
> int (*get_fifo_size)(struct drm_i915_private *dev_priv, int plane);
> int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
> int (*compute_intermediate_wm)(struct drm_device *dev,
> @@ -628,7 +630,6 @@ struct drm_i915_display_funcs {
> 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 (*modeset_commit_cdclk)(struct drm_atomic_state *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 bba077c4bd56..c2d995169528 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -855,9 +855,6 @@ static void skl_set_cdclk(struct drm_i915_private
> *dev_priv,
>
> WARN_ON((cdclk == 24000) != (vco == 0));
>
> - DRM_DEBUG_DRIVER("Changing CDCLK to %d kHz (VCO %d kHz)\n",
> - cdclk, vco);
> -
> mutex_lock(&dev_priv->rps.hw_lock);
> ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> SKL_CDCLK_PREPARE_FOR_CHANGE,
> @@ -1158,9 +1155,6 @@ static void bxt_set_cdclk(struct drm_i915_private
> *dev_priv,
> u32 val, divider;
> int ret;
>
> - DRM_DEBUG_DRIVER("Changing CDCLK to %d kHz (VCO %d kHz)\n",
> - cdclk, vco);
> -
> /* cdclk = vco / 2 / div{1,1.5,2,4} */
> switch (DIV_ROUND_CLOSEST(vco, cdclk)) {
> case 8:
> @@ -1323,6 +1317,22 @@ bool intel_cdclk_state_compare(const struct
> intel_cdclk_state *a,
> return memcmp(a, b, sizeof(*a)) == 0;
> }
>
> +void intel_set_cdclk(struct drm_i915_private *dev_priv,
> + const struct intel_cdclk_state *cdclk_state)
> +{
> + if (intel_cdclk_state_compare(&dev_priv->cdclk.hw, cdclk_state))
> + return;
> +
> + if (WARN_ON_ONCE(!dev_priv->display.set_cdclk))
> + return;
> +
> + DRM_DEBUG_DRIVER("Changing CDCLK to %d kHz, VCO %d kHz, ref %d
> kHz\n",
> + cdclk_state->cdclk, cdclk_state->vco,
> + cdclk_state->ref);
> +
> + dev_priv->display.set_cdclk(dev_priv, cdclk_state);
> +}
> +
> static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state
> *crtc_state,
> int pixel_rate)
> {
> @@ -1417,16 +1427,6 @@ static int vlv_modeset_calc_cdclk(struct
> drm_atomic_state *state)
> return 0;
> }
>
> -static void vlv_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> -{
> - struct drm_i915_private *dev_priv = to_i915(old_state->dev);
> -
> - if (IS_CHERRYVIEW(dev_priv))
> - chv_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> - else
> - vlv_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> -}
> -
> static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
> {
> struct drm_i915_private *dev_priv = to_i915(state->dev);
> @@ -1460,13 +1460,6 @@ static int bdw_modeset_calc_cdclk(struct
> drm_atomic_state *state)
> return 0;
> }
>
> -static void bdw_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> -{
> - struct drm_i915_private *dev_priv = to_i915(old_state->dev);
> -
> - bdw_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> -}
> -
> static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
> {
> struct intel_atomic_state *intel_state =
> to_intel_atomic_state(state);
> @@ -1506,13 +1499,6 @@ static int skl_modeset_calc_cdclk(struct
> drm_atomic_state *state)
> return 0;
> }
>
> -static void skl_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> -{
> - struct drm_i915_private *dev_priv = to_i915(old_state->dev);
> -
> - skl_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> -}
> -
> static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
> {
> struct drm_i915_private *dev_priv = to_i915(state->dev);
> @@ -1557,13 +1543,6 @@ static int bxt_modeset_calc_cdclk(struct
> drm_atomic_state *state)
> return 0;
> }
>
> -static void bxt_modeset_commit_cdclk(struct drm_atomic_state *old_state)
> -{
> - struct drm_i915_private *dev_priv = to_i915(old_state->dev);
> -
> - bxt_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> -}
> -
> static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> {
> int max_cdclk_freq = dev_priv->max_cdclk_freq;
> @@ -1718,24 +1697,24 @@ void intel_update_rawclk(struct drm_i915_private
> *dev_priv)
>
> void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> {
> - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> - dev_priv->display.modeset_commit_cdclk =
> - vlv_modeset_commit_cdclk;
> + if (IS_CHERRYVIEW(dev_priv)) {
> + dev_priv->display.set_cdclk = chv_set_cdclk;
> + dev_priv->display.modeset_calc_cdclk =
> + vlv_modeset_calc_cdclk;
> + } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> + dev_priv->display.set_cdclk = vlv_set_cdclk;
> dev_priv->display.modeset_calc_cdclk =
> vlv_modeset_calc_cdclk;
> } else if (IS_BROADWELL(dev_priv)) {
> - dev_priv->display.modeset_commit_cdclk =
> - bdw_modeset_commit_cdclk;
> + 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.modeset_commit_cdclk =
> - bxt_modeset_commit_cdclk;
> + dev_priv->display.set_cdclk = bxt_set_cdclk;
> dev_priv->display.modeset_calc_cdclk =
> bxt_modeset_calc_cdclk;
> } else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> - dev_priv->display.modeset_commit_cdclk =
> - skl_modeset_commit_cdclk;
> + dev_priv->display.set_cdclk = skl_set_cdclk;
> dev_priv->display.modeset_calc_cdclk =
> skl_modeset_calc_cdclk;
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index fb3abb58b6ca..409537011b28 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12872,10 +12872,7 @@ static void intel_atomic_commit_tail(struct
> drm_atomic_state *state)
> if (intel_state->modeset) {
> drm_atomic_helper_update_legacy_modeset_state(state->dev,
> state);
>
> - if (dev_priv->display.modeset_commit_cdclk &&
> - !intel_cdclk_state_compare(&dev_priv->cdclk.hw,
> - &dev_priv->cdclk.actual))
> - dev_priv->display.modeset_commit_cdclk(state);
> + intel_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
>
> /*
> * SKL workaround: bspec recommends we disable the SAGV when
> we
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index d167ee458d63..40074827f01d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1231,6 +1231,8 @@ void intel_update_cdclk(struct drm_i915_private
> *dev_priv);
> void intel_update_rawclk(struct drm_i915_private *dev_priv);
> 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);
>
> /* intel_display.c */
> enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
