Re: [PATCH 3/7] drm/i915: PSR: deprecate link_standby support for core platforms.

["Jindal, Sonika" <sonika.jindal@intel.com>]

On 2/28/2015 6:56 AM, Rodrigo Vivi wrote:
> On Haswell and Broadwell with link in standby when exit event happens
> between vblank and VSC packet, PSR exit on panel but DPA transmitter
> still sends black pixel. hen this condition hits, panel will intermittently
> display black frame.
>
> The known W/A for this case involve the of single_frame update
> that isn't supported on Haswell and to be supported on Broadwell
> 3 other workarounds would be required. So it is better and safe to
> just deprecate link_standby for now.
>
> Also, link fully off saves more power than link_standby and afwk
> no OEM is requesting link standby on VBT. There is no reason for that.
>
> For Skylake let's just consider it behaves like Broadwell until
> we prove otherwise.
>
Hmm, on SKL I am able to read the sink crc only when I keep this 
link_standby true. Not sure if we'l have to pull it back or need to look 
for another solution.
> Reference: HSD: bdwgfx/1912559
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c |  3 ---
>   drivers/gpu/drm/i915/i915_drv.h     |  1 -
>   drivers/gpu/drm/i915/intel_psr.c    | 25 ++++++++-----------------
>   3 files changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 94b3984..756bf4e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2281,9 +2281,6 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
>   		}
>   	seq_puts(m, "\n");
>
> -	seq_printf(m, "Link standby: %s\n",
> -		   yesno((bool)dev_priv->psr.link_standby));
> -
>   	/* CHV PSR has no kind of performance counter */
>   	if (HAS_PSR(dev) && HAS_DDI(dev)) {
>   		psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) &
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a1dd8bc..dfd044f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -863,7 +863,6 @@ struct i915_psr {
>   	bool active;
>   	struct delayed_work work;
>   	unsigned busy_frontbuffer_bits;
> -	bool link_standby;
>   };
>
>   enum intel_pch {
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 6c8e9e0..d2ff87d 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -157,14 +157,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>
>   	aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
>
> -	/* Enable PSR in sink */
> -	if (dev_priv->psr.link_standby)
> -		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> -				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
> -	else
> -		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> -				   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
> -
>   	aux_data_reg = (INTEL_INFO(dev)->gen >= 9) ?
>   				DPA_AUX_CH_DATA1 : EDP_PSR_AUX_DATA1(dev);
>   	aux_ctl_reg = (INTEL_INFO(dev)->gen >= 9) ?
> @@ -193,6 +185,8 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>   		   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
>   		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
>   	}
> +
> +	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, DP_PSR_ENABLE);
>   }
>
>   static void vlv_psr_enable_source(struct intel_dp *intel_dp)
> @@ -242,9 +236,6 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>   	uint32_t val = 0x0;
>   	const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
>
> -	if (dev_priv->psr.link_standby)
> -		val |= EDP_PSR_LINK_STANDBY;
> -
>   	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
>   		val |= EDP_PSR_TP2_TP3_TIME_0us;
>   		val |= EDP_PSR_TP1_TIME_0us;
> @@ -295,6 +286,12 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
>   		return false;
>   	}
>
> +	if (!IS_VALLEYVIEW(dev) && ((dev_priv->vbt.psr.full_link) ||
> +				    (dig_port->port != PORT_A))){
> +		DRM_DEBUG_KMS("PSR condition failed: Link Standby requested/needed but not supported on this platform\n");
> +		return false;
> +	}
> +
>   	dev_priv->psr.source_ok = true;
>   	return true;
>   }
> @@ -353,12 +350,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   	if (!intel_psr_match_conditions(intel_dp))
>   		goto unlock;
>
> -	/* First we check VBT, but we must respect sink and source
> -	 * known restrictions */
> -	dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link;
> -	if (IS_BROADWELL(dev) && intel_dig_port->port != PORT_A)
> -		dev_priv->psr.link_standby = true;
> -
>   	dev_priv->psr.busy_frontbuffer_bits = 0;
>
>   	if (HAS_DDI(dev)) {
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx