Imre Deak <imre.deak@xxxxxxxxx> writes:
> The assumption when adding the intel_display_power_is_enabled() checks
> was that if it returns success the power can't be turned off afterwards
> during the HW access, which is guaranteed by modeset locks. This isn't
> always true, so make sure we hold a dedicated reference for the time of
> the access.
>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
Was concerned on domain mask overflows but there was already
BUILD_BUG_ON for it.
Revieved-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 67 ++++++++++++++++++++++++++----------
> 1 file changed, 48 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 836bbdc..6abfc54 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8181,18 +8181,22 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + enum intel_display_power_domain power_domain;
> uint32_t tmp;
> + bool ret;
>
> - if (!intel_display_power_is_enabled(dev_priv,
> - POWER_DOMAIN_PIPE(crtc->pipe)))
> + power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
> + if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> return false;
>
> pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> pipe_config->shared_dpll = DPLL_ID_PRIVATE;
>
> + ret = false;
> +
> tmp = I915_READ(PIPECONF(crtc->pipe));
> if (!(tmp & PIPECONF_ENABLE))
> - return false;
> + goto out;
>
> if (IS_G4X(dev) || IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> switch (tmp & PIPECONF_BPC_MASK) {
> @@ -8272,7 +8276,12 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->base.adjusted_mode.crtc_clock =
> pipe_config->port_clock / pipe_config->pixel_multiplier;
>
> - return true;
> + ret = true;
> +
> +out:
> + intel_display_power_put(dev_priv, power_domain);
> +
> + return ret;
> }
>
> static void ironlake_init_pch_refclk(struct drm_device *dev)
> @@ -9376,18 +9385,21 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + enum intel_display_power_domain power_domain;
> uint32_t tmp;
> + bool ret;
>
> - if (!intel_display_power_is_enabled(dev_priv,
> - POWER_DOMAIN_PIPE(crtc->pipe)))
> + power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
> + if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> return false;
>
> pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> pipe_config->shared_dpll = DPLL_ID_PRIVATE;
>
> + ret = false;
> tmp = I915_READ(PIPECONF(crtc->pipe));
> if (!(tmp & PIPECONF_ENABLE))
> - return false;
> + goto out;
>
> switch (tmp & PIPECONF_BPC_MASK) {
> case PIPECONF_6BPC:
> @@ -9450,7 +9462,12 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>
> ironlake_get_pfit_config(crtc, pipe_config);
>
> - return true;
> + ret = true;
> +
> +out:
> + intel_display_power_put(dev_priv, power_domain);
> +
> + return ret;
> }
>
> static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
> @@ -9982,12 +9999,17 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - enum intel_display_power_domain pfit_domain;
> + enum intel_display_power_domain power_domain;
> + unsigned long power_domain_mask;
> uint32_t tmp;
> + bool ret;
>
> - if (!intel_display_power_is_enabled(dev_priv,
> - POWER_DOMAIN_PIPE(crtc->pipe)))
> + power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
> + if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> return false;
> + power_domain_mask = BIT(power_domain);
> +
> + ret = false;
>
> pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> pipe_config->shared_dpll = DPLL_ID_PRIVATE;
> @@ -10014,13 +10036,14 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->cpu_transcoder = TRANSCODER_EDP;
> }
>
> - if (!intel_display_power_is_enabled(dev_priv,
> - POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
> - return false;
> + power_domain = POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder);
> + if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> + goto out;
> + power_domain_mask |= BIT(power_domain);
>
> tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> if (!(tmp & PIPECONF_ENABLE))
> - return false;
> + goto out;
>
> haswell_get_ddi_port_state(crtc, pipe_config);
>
> @@ -10030,14 +10053,14 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> skl_init_scalers(dev, crtc, pipe_config);
> }
>
> - pfit_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> -
> if (INTEL_INFO(dev)->gen >= 9) {
> pipe_config->scaler_state.scaler_id = -1;
> pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> }
>
> - if (intel_display_power_is_enabled(dev_priv, pfit_domain)) {
> + power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> + if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
> + power_domain_mask |= BIT(power_domain);
> if (INTEL_INFO(dev)->gen >= 9)
> skylake_get_pfit_config(crtc, pipe_config);
> else
> @@ -10055,7 +10078,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->pixel_multiplier = 1;
> }
>
> - return true;
> + ret = true;
> +
> +out:
> + for_each_power_domain(power_domain, power_domain_mask)
> + intel_display_power_put(dev_priv, power_domain);
> +
> + return ret;
> }
>
> static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
> --
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
