Re: [PATCH 1/2] drm/i915: vlv: fix cdclk setting during modeset while suspended

Ville Syrjälä
2014-11-20T14:49:26Z

On Wed, Nov 19, 2014 at 04:25:37PM +0200, Imre Deak wrote:
> Currently after doing DPMS-OFF on all outputs CDCLK won't be set to its
> minimum value as it should. A subsequent modeset to turn off all outputs
> will thus run with all power domains disabled, and notice that it needs
> to change CDCLK to its minimum value. Since the power domains are
> disabled this will emit a register-access-while-suspended WARN and fail
> to set the minimum freq.
>
> The proper solution for this is to set the minimum frequency during
> DPMS-OFF. That needs a bigger rework that would take into account the
> user DPMS setting too during the calculation of the new modesetting
> configuration. Until that's done this stop-gap solution gets the PIPE-A
> power domain during setting the CDCLK; this domain covers the HW blocks
> needed for this.
>
> Idea to use PIPE-A domain from Ville.
>
> Testcase: igt/pm_rpm
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=82939
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Yeah, not exactly pretty but rearranging the DPMS code seems like a
fairly sizeable task, so I think adding a kludge in the meantime is
acceptable.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
> drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9de2f69..6f29a72 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4939,10 +4939,23 @@ static void valleyview_modeset_global_resources(struct drm_device *dev)
> int req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
>
> if (req_cdclk != dev_priv->vlv_cdclk_freq) {
> + /*
> + * FIXME: We can end up here with all power domains off, yet
> + * with a CDCLK frequency other than the minimum. To account
> + * for this take the PIPE-A power domain, which covers the HW
> + * blocks needed for the following programming. This can be
> + * removed once it's guaranteed that we get here either with
> + * the minimum CDCLK set, or the required power domains
> + * enabled.
> + */
> + intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A);
> +
> if (IS_CHERRYVIEW(dev))
> cherryview_set_cdclk(dev, req_cdclk);
> else
> valleyview_set_cdclk(dev, req_cdclk);
> +
> + intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
> }
> }
>
> --
> 1.8.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC