On Fri, May 11, 2018 at 12:51:45PM -0700, Dhinakaran Pandiyan wrote:
> While touching the code around this, I noticed that absence of ALPM
> capability does not stop us from enabling PSR2. But, the spec
> unambiguously states that ALPM is required for PSR2 and so does this
> commit that introduced this code
>
> drm/i915/psr: enable ALPM for psr2
>
> As per edp1.4 spec , alpm is required for psr2 operation as it's
> used for all psr2 main link power down management and alpm enable
> bit must be set for psr2 operation.
>
Since, the code introduced by "drm/i915/psr: enable ALPM for psr2" enables PSR2 even if ALPM isn't supported, can we add the "Fixes" tag here ? Rest looks good.
Reviewed-by: Tarun Vyas <tarun.vyas@xxxxxxxxx>
> Cc: Jose Roberto de Souza <jose.souza@xxxxxxxxx>
> Cc: Vathsala Nagaraju <vathsala.nagaraju@xxxxxxxxx>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_psr.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index b4a4f5d3a2bb..92abf61e234c 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -254,6 +254,10 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>
> if (INTEL_GEN(dev_priv) >= 9 &&
> (intel_dp->psr_dpcd[0] == DP_PSR2_WITH_Y_COORD_IS_SUPPORTED)) {
> + bool y_req = intel_dp->psr_dpcd[1] &
> + DP_PSR2_SU_Y_COORDINATE_REQUIRED;
> + bool alpm = intel_dp_get_alpm_status(intel_dp);
> +
> /*
> * All panels that supports PSR version 03h (PSR2 +
> * Y-coordinate) can handle Y-coordinates in VSC but we are
> @@ -265,16 +269,13 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
> * Y-coordinate requirement panels we would need to enable
> * GTC first.
> */
> - dev_priv->psr.sink_psr2_support =
> - intel_dp->psr_dpcd[1] & DP_PSR2_SU_Y_COORDINATE_REQUIRED;
> + dev_priv->psr.sink_psr2_support = y_req && alpm;
> DRM_DEBUG_KMS("PSR2 %ssupported\n",
> dev_priv->psr.sink_psr2_support ? "" : "not ");
>
> if (dev_priv->psr.sink_psr2_support) {
> dev_priv->psr.colorimetry_support =
> intel_dp_get_colorimetry_status(intel_dp);
> - dev_priv->psr.alpm =
> - intel_dp_get_alpm_status(intel_dp);
> dev_priv->psr.sink_sync_latency =
> intel_dp_get_sink_sync_latency(intel_dp);
> }
> @@ -386,13 +387,12 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
> u8 dpcd_val = DP_PSR_ENABLE;
>
> /* Enable ALPM at sink for psr2 */
> - if (dev_priv->psr.psr2_enabled && dev_priv->psr.alpm)
> - drm_dp_dpcd_writeb(&intel_dp->aux,
> - DP_RECEIVER_ALPM_CONFIG,
> - DP_ALPM_ENABLE);
> -
> - if (dev_priv->psr.psr2_enabled)
> + if (dev_priv->psr.psr2_enabled) {
> + drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
> + DP_ALPM_ENABLE);
> dpcd_val |= DP_PSR_ENABLE_PSR2;
> + }
> +
> if (dev_priv->psr.link_standby)
> dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
> --
> 2.14.1
>
> _______________________________________________
> 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
