Hi Stanislav,
[...]
> +/**
> + * intel_display_to_pcode- inform pcode for display config
> + * @cdclk: current cdclk as per new or old state
> + * @voltage_level: current voltage_level send to Pcode
> + * @active_pipes: active pipes for more accurate power accounting
> + */
> +static void intel_display_to_pcode(struct drm_i915_private *dev_priv,
> + unsigned int cdclk, u8 voltage_level,
> + u8 active_pipes)
> +{
> + int ret;
> +
if (DISPLAY_VER(dev_priv) < 12)
return;
the rest can go outside the if and we save one indentation leve.
BTW... /dev_priv/i915/ ?
> + if (DISPLAY_VER(dev_priv) >= 12) {
> + ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
> + SKL_CDCLK_PREPARE_FOR_CHANGE |
> + DISPLAY_TO_PCODE_MASK
> + (cdclk, active_pipes, voltage_level),
> + SKL_CDCLK_READY_FOR_CHANGE,
> + SKL_CDCLK_READY_FOR_CHANGE, 3);
> + if (ret) {
> + drm_err(&dev_priv->drm,
> + "Failed to inform PCU about display config (err %d)\n",
> + ret);
> + return;
> + }
> + }
> +}
[...]
> +void intel_display_to_pcode_pre_notification(struct intel_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + const struct intel_cdclk_state *old_cdclk_state =
> + intel_atomic_get_old_cdclk_state(state);
> + const struct intel_cdclk_state *new_cdclk_state =
> + intel_atomic_get_new_cdclk_state(state);
> + if (!intel_cdclk_changed(&old_cdclk_state->actual,
> + &new_cdclk_state->actual) &&
> + (new_cdclk_state->active_pipes ==
> + old_cdclk_state->active_pipes))
> + return;
> + if (old_cdclk_state->actual.cdclk <= new_cdclk_state->actual.cdclk) {
> + intel_display_to_pcode(dev_priv, new_cdclk_state->actual.cdclk,
> + new_cdclk_state->actual.voltage_level,
> + new_cdclk_state->active_pipes);
> + return;
> + }
> + if (old_cdclk_state->actual.cdclk >= new_cdclk_state->actual.cdclk) {
> + intel_display_to_pcode(dev_priv, old_cdclk_state->actual.cdclk,
> + old_cdclk_state->actual.voltage_level,
> + old_cdclk_state->active_pipes);
> + return;
> + }
> + if (old_cdclk_state->active_pipes != new_cdclk_state->active_pipes) {
> + intel_display_to_pcode(dev_priv, new_cdclk_state->actual.cdclk,
> + new_cdclk_state->actual.voltage_level,
> + new_cdclk_state->active_pipes);
> + return;
> + }
> + intel_display_to_pcode(dev_priv, DISPLAY_TO_PCODE_CDCLK_MAX,
> + new_cdclk_state->actual.voltage_level,
> + new_cdclk_state->active_pipes);
if you replace all the ifs with "else if"s you can have only one
return and remove all the brackets.
> +}
> +
> +/*intel_display_to_pcode_post_notification: after frequency change sending
> + * voltage_level, active pipes, current CDCLK frequency.
> + * @state: intel atomic state
> + */
> +void intel_display_to_pcode_post_notification(struct intel_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + const struct intel_cdclk_state *new_cdclk_state =
> + intel_atomic_get_new_cdclk_state(state);
> + intel_display_to_pcode(dev_priv, new_cdclk_state->actual.cdclk,
> + new_cdclk_state->actual.voltage_level,
> + new_cdclk_state->active_pipes);
the indentation here is off
> +}
Andi
[...]
