On Mon, Jan 20, 2020 at 07:47:28PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> Let's add a copy of the active_pipes bitmask into the cdclk_state.
> While this is duplicating a bit of information we may already
> have elsewhere, I think it's worth it to decopule the cdclk stuff
> from whatever else wants to use that bitmask. Also we want to get
> rid of all the old ad-hoc global state which is what the current
> bitmask is, so this removes one obstacle.
>
> The one extra thing we have to remember is write locking the cdclk
> state whenever the bitmask changes.
>
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
Reviewed-by: Imre Deak <imre.d...@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 20 +++++++++++---------
> drivers/gpu/drm/i915/display/intel_cdclk.h | 3 +++
> drivers/gpu/drm/i915/display/intel_display.c | 8 +++++---
> 3 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index e14bda2bec71..f8e4510f4bd9 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2125,7 +2125,7 @@ static int vlv_modeset_calc_cdclk(struct
> intel_cdclk_state *cdclk_state)
> cdclk_state->logical.voltage_level =
> vlv_calc_voltage_level(dev_priv, cdclk);
>
> - if (!state->active_pipes) {
> + if (!cdclk_state->active_pipes) {
> cdclk = vlv_calc_cdclk(dev_priv, cdclk_state->force_min_cdclk);
>
> cdclk_state->actual.cdclk = cdclk;
> @@ -2140,7 +2140,6 @@ static int vlv_modeset_calc_cdclk(struct
> intel_cdclk_state *cdclk_state)
>
> static int bdw_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> {
> - struct intel_atomic_state *state = cdclk_state->base.state;
> int min_cdclk, cdclk;
>
> min_cdclk = intel_compute_min_cdclk(cdclk_state);
> @@ -2157,7 +2156,7 @@ static int bdw_modeset_calc_cdclk(struct
> intel_cdclk_state *cdclk_state)
> cdclk_state->logical.voltage_level =
> bdw_calc_voltage_level(cdclk);
>
> - if (!state->active_pipes) {
> + if (!cdclk_state->active_pipes) {
> cdclk = bdw_calc_cdclk(cdclk_state->force_min_cdclk);
>
> cdclk_state->actual.cdclk = cdclk;
> @@ -2209,7 +2208,6 @@ static int skl_dpll0_vco(struct intel_cdclk_state
> *cdclk_state)
>
> static int skl_modeset_calc_cdclk(struct intel_cdclk_state *cdclk_state)
> {
> - struct intel_atomic_state *state = cdclk_state->base.state;
> int min_cdclk, cdclk, vco;
>
> min_cdclk = intel_compute_min_cdclk(cdclk_state);
> @@ -2229,7 +2227,7 @@ static int skl_modeset_calc_cdclk(struct
> intel_cdclk_state *cdclk_state)
> cdclk_state->logical.voltage_level =
> skl_calc_voltage_level(cdclk);
>
> - if (!state->active_pipes) {
> + if (!cdclk_state->active_pipes) {
> cdclk = skl_calc_cdclk(cdclk_state->force_min_cdclk, vco);
>
> cdclk_state->actual.vco = vco;
> @@ -2266,7 +2264,7 @@ static int bxt_modeset_calc_cdclk(struct
> intel_cdclk_state *cdclk_state)
> max_t(int, min_voltage_level,
> dev_priv->display.calc_voltage_level(cdclk));
>
> - if (!state->active_pipes) {
> + if (!cdclk_state->active_pipes) {
> cdclk = bxt_calc_cdclk(dev_priv, cdclk_state->force_min_cdclk);
> vco = bxt_calc_cdclk_pll_vco(dev_priv, cdclk);
>
> @@ -2402,6 +2400,9 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state
> *state)
>
> old_cdclk_state = intel_atomic_get_old_cdclk_state(state);
>
> + new_cdclk_state->active_pipes =
> + intel_calc_active_pipes(state, old_cdclk_state->active_pipes);
> +
> ret = dev_priv->display.modeset_calc_cdclk(new_cdclk_state);
> if (ret)
> return ret;
> @@ -2415,7 +2416,8 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state
> *state)
> ret =
> intel_atomic_serialize_global_state(&new_cdclk_state->base);
> if (ret)
> return ret;
> - } else if (intel_cdclk_changed(&old_cdclk_state->logical,
> + } else if (old_cdclk_state->active_pipes !=
> new_cdclk_state->active_pipes ||
> + intel_cdclk_changed(&old_cdclk_state->logical,
> &new_cdclk_state->logical)) {
> ret = intel_atomic_lock_global_state(&new_cdclk_state->base);
> if (ret)
> @@ -2424,14 +2426,14 @@ int intel_modeset_calc_cdclk(struct
> intel_atomic_state *state)
> return 0;
> }
>
> - if (is_power_of_2(state->active_pipes) &&
> + if (is_power_of_2(new_cdclk_state->active_pipes) &&
> intel_cdclk_can_cd2x_update(dev_priv,
> &old_cdclk_state->actual,
> &new_cdclk_state->actual)) {
> struct intel_crtc *crtc;
> struct intel_crtc_state *crtc_state;
>
> - pipe = ilog2(state->active_pipes);
> + pipe = ilog2(new_cdclk_state->active_pipes);
> crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
>
> crtc_state = intel_atomic_get_crtc_state(&state->base, crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
> b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index 195fca70bfcb..df21dbdcc575 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -50,6 +50,9 @@ struct intel_cdclk_state {
> /* forced minimum cdclk for glk+ audio w/a */
> int force_min_cdclk;
> bool force_min_cdclk_changed;
> +
> + /* bitmask of active pipes */
> + u8 active_pipes;
> };
>
> int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index dca100546be8..27878c484307 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7551,6 +7551,7 @@ static void intel_crtc_disable_noatomic(struct
> intel_crtc *crtc,
> dev_priv->active_pipes &= ~BIT(pipe);
> cdclk_state->min_cdclk[pipe] = 0;
> cdclk_state->min_voltage_level[pipe] = 0;
> + cdclk_state->active_pipes &= ~BIT(pipe);
>
> bw_state->data_rate[pipe] = 0;
> bw_state->num_active_planes[pipe] = 0;
> @@ -18120,10 +18121,9 @@ static void intel_modeset_readout_hw_state(struct
> drm_device *dev)
> struct intel_encoder *encoder;
> struct intel_connector *connector;
> struct drm_connector_list_iter conn_iter;
> + u8 active_pipes = 0;
> int i;
>
> - dev_priv->active_pipes = 0;
> -
> for_each_intel_crtc(dev, crtc) {
> struct intel_crtc_state *crtc_state =
> to_intel_crtc_state(crtc->base.state);
> @@ -18139,13 +18139,15 @@ static void intel_modeset_readout_hw_state(struct
> drm_device *dev)
> crtc->active = crtc_state->hw.active;
>
> if (crtc_state->hw.active)
> - dev_priv->active_pipes |= BIT(crtc->pipe);
> + active_pipes |= BIT(crtc->pipe);
>
> DRM_DEBUG_KMS("[CRTC:%d:%s] hw state readout: %s\n",
> crtc->base.base.id, crtc->base.name,
> enableddisabled(crtc_state->hw.active));
> }
>
> + dev_priv->active_pipes = cdclk_state->active_pipes = active_pipes;
> +
> readout_plane_state(dev_priv);
>
> for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> --
> 2.24.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
