[Intel-gfx] [PATCH] drm/i915/display: Communicate display configuration to pcode

[Andi Shyti]

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

[...]