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@xxxxxxxxx>
> Cc: Tarun Vyas <tarun.vyas@xxxxxxxxx>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
