From: Jani Nikula <jani.nikula@linux.intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/i915/cdclk: Plumb the full atomic state deeper
Date: Thu, 30 May 2024 15:54:42 +0300 [thread overview]
Message-ID: <87y17rh33h.fsf@intel.com> (raw)
In-Reply-To: <20240528184945.24083-1-ville.syrjala@linux.intel.com>
On Tue, 28 May 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Various parts of the cdclk code need access the full atomic
> state. Currently it's being dug out via the cdclk_state->base.state
> pointer, which is not great as that pointer isn't always valid.
> Instead plumb the full atomic state from the top so that it's
> clear that it is in fact valid.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 60 +++++++++++++---------
> 1 file changed, 35 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index b78154c82a71..7ef8dcb1601a 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -113,7 +113,7 @@ struct intel_cdclk_funcs {
> void (*set_cdclk)(struct drm_i915_private *i915,
> const struct intel_cdclk_config *cdclk_config,
> enum pipe pipe);
> - int (*modeset_calc_cdclk)(struct intel_cdclk_state *state);
> + int (*modeset_calc_cdclk)(struct intel_atomic_state *state);
> u8 (*calc_voltage_level)(int cdclk);
> };
>
> @@ -130,10 +130,11 @@ static void intel_cdclk_set_cdclk(struct drm_i915_private *dev_priv,
> dev_priv->display.funcs.cdclk->set_cdclk(dev_priv, cdclk_config, pipe);
> }
>
> -static int intel_cdclk_modeset_calc_cdclk(struct drm_i915_private *dev_priv,
> - struct intel_cdclk_state *cdclk_config)
> +static int intel_cdclk_modeset_calc_cdclk(struct intel_atomic_state *state)
> {
> - return dev_priv->display.funcs.cdclk->modeset_calc_cdclk(cdclk_config);
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +
> + return dev_priv->display.funcs.cdclk->modeset_calc_cdclk(state);
The dev_priv is an eyesore. Could already start doing:
const struct intel_display *display = to_intel_display(state->base.dev);
return display->funcs.cdclk->modeset_calc_cdclk(state);
And if you wanted to, could also make to_intel_display() handle struct
intel_atomic_state so it would only need to_intel_display(state).
Regardless,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> }
>
> static u8 intel_cdclk_calc_voltage_level(struct drm_i915_private *dev_priv,
> @@ -2834,10 +2835,11 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> return min_cdclk;
> }
>
> -static int intel_compute_min_cdclk(struct intel_cdclk_state *cdclk_state)
> +static int intel_compute_min_cdclk(struct intel_atomic_state *state)
> {
> - struct intel_atomic_state *state = cdclk_state->base.state;
> struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_cdclk_state *cdclk_state =
> + intel_atomic_get_new_cdclk_state(state);
> const struct intel_bw_state *bw_state;
> struct intel_crtc *crtc;
> struct intel_crtc_state *crtc_state;
> @@ -2916,10 +2918,11 @@ static int intel_compute_min_cdclk(struct intel_cdclk_state *cdclk_state)
> * future platforms this code will need to be
> * adjusted.
> */
> -static int bxt_compute_min_voltage_level(struct intel_cdclk_state *cdclk_state)
> +static int bxt_compute_min_voltage_level(struct intel_atomic_state *state)
> {
> - struct intel_atomic_state *state = cdclk_state->base.state;
> struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_cdclk_state *cdclk_state =
> + intel_atomic_get_new_cdclk_state(state);
> struct intel_crtc *crtc;
> struct intel_crtc_state *crtc_state;
> u8 min_voltage_level;
> @@ -2952,13 +2955,14 @@ static int bxt_compute_min_voltage_level(struct intel_cdclk_state *cdclk_state)
> return min_voltage_level;
> }
>
> -static int vlv_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> +static int vlv_modeset_calc_cdclk(struct intel_atomic_state *state)
> {
> - struct intel_atomic_state *state = cdclk_state->base.state;
> struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_cdclk_state *cdclk_state =
> + intel_atomic_get_new_cdclk_state(state);
> int min_cdclk, cdclk;
>
> - min_cdclk = intel_compute_min_cdclk(cdclk_state);
> + min_cdclk = intel_compute_min_cdclk(state);
> if (min_cdclk < 0)
> return min_cdclk;
>
> @@ -2981,11 +2985,13 @@ static int vlv_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> return 0;
> }
>
> -static int bdw_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> +static int bdw_modeset_calc_cdclk(struct intel_atomic_state *state)
> {
> + struct intel_cdclk_state *cdclk_state =
> + intel_atomic_get_new_cdclk_state(state);
> int min_cdclk, cdclk;
>
> - min_cdclk = intel_compute_min_cdclk(cdclk_state);
> + min_cdclk = intel_compute_min_cdclk(state);
> if (min_cdclk < 0)
> return min_cdclk;
>
> @@ -3008,10 +3014,11 @@ static int bdw_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> return 0;
> }
>
> -static int skl_dpll0_vco(struct intel_cdclk_state *cdclk_state)
> +static int skl_dpll0_vco(struct intel_atomic_state *state)
> {
> - struct intel_atomic_state *state = cdclk_state->base.state;
> struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_cdclk_state *cdclk_state =
> + intel_atomic_get_new_cdclk_state(state);
> struct intel_crtc *crtc;
> struct intel_crtc_state *crtc_state;
> int vco, i;
> @@ -3045,15 +3052,17 @@ static int skl_dpll0_vco(struct intel_cdclk_state *cdclk_state)
> return vco;
> }
>
> -static int skl_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> +static int skl_modeset_calc_cdclk(struct intel_atomic_state *state)
> {
> + struct intel_cdclk_state *cdclk_state =
> + intel_atomic_get_new_cdclk_state(state);
> int min_cdclk, cdclk, vco;
>
> - min_cdclk = intel_compute_min_cdclk(cdclk_state);
> + min_cdclk = intel_compute_min_cdclk(state);
> if (min_cdclk < 0)
> return min_cdclk;
>
> - vco = skl_dpll0_vco(cdclk_state);
> + vco = skl_dpll0_vco(state);
>
> cdclk = skl_calc_cdclk(min_cdclk, vco);
>
> @@ -3076,17 +3085,18 @@ static int skl_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> return 0;
> }
>
> -static int bxt_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> +static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
> {
> - struct intel_atomic_state *state = cdclk_state->base.state;
> struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_cdclk_state *cdclk_state =
> + intel_atomic_get_new_cdclk_state(state);
> int min_cdclk, min_voltage_level, cdclk, vco;
>
> - min_cdclk = intel_compute_min_cdclk(cdclk_state);
> + min_cdclk = intel_compute_min_cdclk(state);
> if (min_cdclk < 0)
> return min_cdclk;
>
> - min_voltage_level = bxt_compute_min_voltage_level(cdclk_state);
> + min_voltage_level = bxt_compute_min_voltage_level(state);
> if (min_voltage_level < 0)
> return min_voltage_level;
>
> @@ -3114,7 +3124,7 @@ static int bxt_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> return 0;
> }
>
> -static int fixed_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> +static int fixed_modeset_calc_cdclk(struct intel_atomic_state *state)
> {
> int min_cdclk;
>
> @@ -3123,7 +3133,7 @@ static int fixed_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> * check that the required minimum frequency doesn't exceed
> * the actual cdclk frequency.
> */
> - min_cdclk = intel_compute_min_cdclk(cdclk_state);
> + min_cdclk = intel_compute_min_cdclk(state);
> if (min_cdclk < 0)
> return min_cdclk;
>
> @@ -3263,7 +3273,7 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
> new_cdclk_state->active_pipes =
> intel_calc_active_pipes(state, old_cdclk_state->active_pipes);
>
> - ret = intel_cdclk_modeset_calc_cdclk(dev_priv, new_cdclk_state);
> + ret = intel_cdclk_modeset_calc_cdclk(state);
> if (ret)
> return ret;
--
Jani Nikula, Intel
prev parent reply other threads:[~2024-05-30 12:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-28 18:49 [PATCH 1/3] drm/i915/cdclk: Plumb the full atomic state deeper Ville Syrjala
2024-05-28 18:49 ` [PATCH 2/3] drm/i915: Plumb the full atomic state into icl_check_nv12_planes() Ville Syrjala
2024-05-30 12:57 ` Jani Nikula
2024-05-31 7:16 ` Ville Syrjälä
2024-05-28 18:49 ` [PATCH 3/3] drm/i915: Plumb the full atomic state into skl_ddb_add_affected_planes() Ville Syrjala
2024-05-30 13:00 ` Jani Nikula
2024-05-28 19:25 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/cdclk: Plumb the full atomic state deeper Patchwork
2024-05-28 19:33 ` ✓ Fi.CI.BAT: success " Patchwork
2024-05-29 13:15 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-05-30 12:54 ` Jani Nikula [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87y17rh33h.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.