Re: [Intel-gfx] [PATCH 2/9] drm/i915/psr: Don't tell sink that main link will be active while is active PSR2
On Wed, Nov 28, 2018 at 12:13:30PM -0800, Souza, Jose wrote: > On Wed, 2018-11-28 at 11:02 -0800, Rodrigo Vivi wrote: > > On Mon, Nov 26, 2018 at 04:37:03PM -0800, José Roberto de Souza > > wrote: > > > For PSR2 there is no register to tell HW to keep main link enabled > > > while PSR2 is active, so don't configure sink DPCD with a > > > misleading value. > > > > > > Cc: Dhinakaran Pandiyan > > > Cc: Rodrigo Vivi > > > Signed-off-by: José Roberto de Souza > > > --- > > > drivers/gpu/drm/i915/intel_psr.c | 10 ++ > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > > b/drivers/gpu/drm/i915/intel_psr.c > > > index f5d27a02eb28..888e348cc1b4 100644 > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > @@ -391,12 +391,14 @@ static void intel_psr_enable_sink(struct > > > intel_dp *intel_dp) > > > drm_dp_dpcd_writeb(&intel_dp->aux, > > > DP_RECEIVER_ALPM_CONFIG, > > > DP_ALPM_ENABLE); > > > dpcd_val |= DP_PSR_ENABLE_PSR2; > > > + } else { > > > + if (dev_priv->psr.link_standby) > > > + dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; > > > + > > > + if (INTEL_GEN(dev_priv) >= 8) > > > + dpcd_val |= DP_PSR_CRC_VERIFICATION; > > > > commit message only mention the link stand-by... > > could you please do this in a separated patch? > > We were already doing it for PSR1, I just grouped all the PSR1 checks > inside of this else block, so there is no functional change in CRC > verification but I can move it to a separated patch if you want. I prefer the separated patch to make things clear. The other option would be change the commit message and subject to mention this clean-up here. but up to you. > > > > > > > } > > > > > > - if (dev_priv->psr.link_standby) > > > - dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; > > > - if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8) > > > - dpcd_val |= DP_PSR_CRC_VERIFICATION; > > > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val); > > > > > > drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, > > > DP_SET_POWER_D0); > > > -- > > > 2.19.2 > > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/9] drm/i915/psr: Don't tell sink that main link will be active while is active PSR2
On Wed, 2018-11-28 at 11:02 -0800, Rodrigo Vivi wrote: > On Mon, Nov 26, 2018 at 04:37:03PM -0800, José Roberto de Souza > wrote: > > For PSR2 there is no register to tell HW to keep main link enabled > > while PSR2 is active, so don't configure sink DPCD with a > > misleading value. > > > > Cc: Dhinakaran Pandiyan > > Cc: Rodrigo Vivi > > Signed-off-by: José Roberto de Souza > > --- > > drivers/gpu/drm/i915/intel_psr.c | 10 ++ > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > b/drivers/gpu/drm/i915/intel_psr.c > > index f5d27a02eb28..888e348cc1b4 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -391,12 +391,14 @@ static void intel_psr_enable_sink(struct > > intel_dp *intel_dp) > > drm_dp_dpcd_writeb(&intel_dp->aux, > > DP_RECEIVER_ALPM_CONFIG, > >DP_ALPM_ENABLE); > > dpcd_val |= DP_PSR_ENABLE_PSR2; > > + } else { > > + if (dev_priv->psr.link_standby) > > + dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; > > + > > + if (INTEL_GEN(dev_priv) >= 8) > > + dpcd_val |= DP_PSR_CRC_VERIFICATION; > > commit message only mention the link stand-by... > could you please do this in a separated patch? We were already doing it for PSR1, I just grouped all the PSR1 checks inside of this else block, so there is no functional change in CRC verification but I can move it to a separated patch if you want. > > > } > > > > - if (dev_priv->psr.link_standby) > > - dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; > > - if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8) > > - dpcd_val |= DP_PSR_CRC_VERIFICATION; > > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val); > > > > drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, > > DP_SET_POWER_D0); > > -- > > 2.19.2 > > signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/9] drm/i915/psr: Don't tell sink that main link will be active while is active PSR2
On Mon, Nov 26, 2018 at 04:37:03PM -0800, José Roberto de Souza wrote: > For PSR2 there is no register to tell HW to keep main link enabled > while PSR2 is active, so don't configure sink DPCD with a > misleading value. > > Cc: Dhinakaran Pandiyan > Cc: Rodrigo Vivi > Signed-off-by: José Roberto de Souza > --- > drivers/gpu/drm/i915/intel_psr.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > b/drivers/gpu/drm/i915/intel_psr.c > index f5d27a02eb28..888e348cc1b4 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -391,12 +391,14 @@ static void intel_psr_enable_sink(struct intel_dp > *intel_dp) > drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG, > DP_ALPM_ENABLE); > dpcd_val |= DP_PSR_ENABLE_PSR2; > + } else { > + if (dev_priv->psr.link_standby) > + dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; > + > + if (INTEL_GEN(dev_priv) >= 8) > + dpcd_val |= DP_PSR_CRC_VERIFICATION; commit message only mention the link stand-by... could you please do this in a separated patch? > } > > - if (dev_priv->psr.link_standby) > - dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; > - if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8) > - dpcd_val |= DP_PSR_CRC_VERIFICATION; > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val); > > drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, DP_SET_POWER_D0); > -- > 2.19.2 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/9] drm/i915/psr: Don't tell sink that main link will be active while is active PSR2
For PSR2 there is no register to tell HW to keep main link enabled while PSR2 is active, so don't configure sink DPCD with a misleading value. Cc: Dhinakaran Pandiyan Cc: Rodrigo Vivi Signed-off-by: José Roberto de Souza --- drivers/gpu/drm/i915/intel_psr.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index f5d27a02eb28..888e348cc1b4 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -391,12 +391,14 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp) drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG, DP_ALPM_ENABLE); dpcd_val |= DP_PSR_ENABLE_PSR2; + } else { + if (dev_priv->psr.link_standby) + dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; + + if (INTEL_GEN(dev_priv) >= 8) + dpcd_val |= DP_PSR_CRC_VERIFICATION; } - if (dev_priv->psr.link_standby) - dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; - if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8) - dpcd_val |= DP_PSR_CRC_VERIFICATION; drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val); drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, DP_SET_POWER_D0); -- 2.19.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx