[Intel-gfx] [PATCH 4/4] drm/i915: Introduce PSR Block Power Domains.

[Chris Wilson]

On Wed, Aug 03, 2016 at 02:44:38PM -0700, Rodrigo Vivi wrote:
> Issue: By the spec panels can emmit HPD when find some erros.
> Some of the errors we see are on reserved bits of spec so we
> end up not handling any of them.
> 
> Our handling functions uses aux communications. However if the
> link is in a deep off state like in PSR2 for gen9 we will have
> trouble and link is never restablished. Also even with PSR1 case
> we see flickering screens or hard lock depending on the panel.
> 
> Solution: Prevent PSR in some situations where we know link
> shouldn't be off.
> 
> Propose: Reuse the runtime PM domains to block PSR. PSR is not
> a Power Well but it is a power saving runtime component.
> Also it is better reuse something that is stablish and safe
> then get back to old times where we had psr_exits and
> psr_disables spread all over our code.
> 
> Cc: Imre Deak <imre.deak at intel.com>
> Cc: Tarun Vyas <tarun.vyas at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |  1 +
>  drivers/gpu/drm/i915/i915_reg.h         |  1 +
>  drivers/gpu/drm/i915/intel_display.c    |  2 --
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 56 +++++++++++++++++++++++++++++++++
>  4 files changed, 58 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 83afdd0..d1bf060 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -840,6 +840,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>  
>  	intel_pm_setup(&dev_priv->drm);
>  	intel_init_dpio(dev_priv);
> +	intel_psr_init(dev_priv);
>  	intel_power_domains_init(dev_priv);
>  	intel_irq_init(dev_priv);
>  	intel_init_display_hooks(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2f93d4a..1cd939f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -718,6 +718,7 @@ enum skl_disp_power_wells {
>  	/* Not actual bit groups. Used as IDs for lookup_power_well() */
>  	SKL_DISP_PW_ALWAYS_ON,
>  	SKL_DISP_PW_DC_OFF,
> +	SKL_DISP_PW_PSR_BLOCK,

PSR_BLOCK, psr_blk are not great. I would lose the BLOCK here and
instead use
gen9_psr_wakelock_is_enabled
gen9_psr_wakelock_enable
gen9_psr_wakelock_disable

> +static bool gen9_psr_blk_power_well_enabled(struct drm_i915_private *dev_priv,
> +					    struct i915_power_well *power_well)
> +{
> +	return (dev_priv->psr.rpm_block);

Looks like rpm_block only exists to duplicate rpm state.

> +}
> +
> +static void gen9_psr_blk_power_well_enable(struct drm_i915_private *dev_priv,
> +					   struct i915_power_well *power_well)
> +{
> +	intel_psr_rpm_block(dev_priv);

intel_psr_rpm_block -> intel_psr_block()

> +
> +	WARN_ON(dev_priv->psr.active);
> +}
> +
> +static void gen9_psr_blk_power_well_disable(struct drm_i915_private *dev_priv,
> +					    struct i915_power_well *power_well)
> +{
> +	intel_psr_rpm_unblock(dev_priv);

intel_psr_rpm_unblock -> intel_psr_unblock()

Design looks fine to me.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre