i915 yet don't support PSR in Apple panels, so lets keep it disabled while we work on that.
Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW) Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com --- drivers/gpu/drm/drm_dp_helper.c | 2 ++ drivers/gpu/drm/i915/intel_psr.c | 6 ++++++ include/drm/drm_dp_helper.h | 1 + 3 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 6d483487f2b4..6b5a19d3e347 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -1273,6 +1273,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { { OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) }, /* LG LP140WF6-SPM1 eDP panel */ { OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) }, + /* Apple panels needs some additional handling to support PSR */ + { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED) } };
#undef OUI diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 572e626eadff..f5d27a02eb28 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -274,6 +274,12 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) DRM_DEBUG_KMS("Panel lacks power state control, PSR cannot be enabled\n"); return; } + + if (drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED)) { + DRM_DEBUG_KMS("PSR support not currently available for this panel\n"); + return; + } + dev_priv->psr.sink_support = true; dev_priv->psr.sink_sync_latency = intel_dp_get_sink_sync_latency(intel_dp); diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 3314e91f6eb3..db516c48cda3 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1364,6 +1364,7 @@ enum drm_dp_quirk { * to 16 bits. So will give a constant value (0x8000) for compatability. */ DP_DPCD_QUIRK_CONSTANT_N, + DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED, };
/**
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 dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com --- 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);
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 dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
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
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 dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
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
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 dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
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
eDP spec states 2 different bits to enable sink to trigger a interruption when there is a CRC mismatch. DP_PSR_CRC_VERIFICATION is for PSR only and DP_PSR_IRQ_HPD_WITH_CRC_ERRORS is for PSR2 only.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 888e348cc1b4..607c3ec41679 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -390,7 +390,7 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp) 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; + dpcd_val |= DP_PSR_ENABLE_PSR2 | DP_PSR_IRQ_HPD_WITH_CRC_ERRORS; } else { if (dev_priv->psr.link_standby) dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
On Mon, Nov 26, 2018 at 04:37:04PM -0800, José Roberto de Souza wrote:
eDP spec states 2 different bits to enable sink to trigger a interruption when there is a CRC mismatch. DP_PSR_CRC_VERIFICATION is for PSR only and DP_PSR_IRQ_HPD_WITH_CRC_ERRORS is for PSR2 only.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/i915/intel_psr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 888e348cc1b4..607c3ec41679 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -390,7 +390,7 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp) 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;
dpcd_val |= DP_PSR_ENABLE_PSR2 | DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
good catch!
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
} else { if (dev_priv->psr.link_standby) dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; -- 2.19.2
On Thu, 2018-11-29 at 14:04 -0800, Rodrigo Vivi wrote:
On Mon, Nov 26, 2018 at 04:37:04PM -0800, José Roberto de Souza wrote:
eDP spec states 2 different bits to enable sink to trigger a interruption when there is a CRC mismatch. DP_PSR_CRC_VERIFICATION is for PSR only and DP_PSR_IRQ_HPD_WITH_CRC_ERRORS is for PSR2 only.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/i915/intel_psr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 888e348cc1b4..607c3ec41679 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -390,7 +390,7 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp) 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;
dpcd_val |= DP_PSR_ENABLE_PSR2 |DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
good catch!
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
Is there a commit that this patch Fixes?
} else { if (dev_priv->psr.link_standby) dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE; -- 2.19.2
For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is already unset in gen10 + GLK we can just drop it and fix for both gens.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 607c3ec41679..7607a58a6ec0 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) hsw_psr_setup_aux(intel_dp);
- if (dev_priv->psr.psr2_enabled) { + if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) && + !IS_GEMINILAKE(dev_priv))) { i915_reg_t reg = gen9_chicken_trans_reg(dev_priv, cpu_transcoder); u32 chicken = I915_READ(reg);
- if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) - chicken |= (PSR2_VSC_ENABLE_PROG_HEADER - | PSR2_ADD_VERTICAL_LINE_COUNT); - - else - chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL; + chicken |= PSR2_VSC_ENABLE_PROG_HEADER | + PSR2_ADD_VERTICAL_LINE_COUNT; I915_WRITE(reg, chicken); }
On Mon, Nov 26, 2018 at 04:37:05PM -0800, José Roberto de Souza wrote:
For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is already unset in gen10 + GLK we can just drop it and fix for both gens.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
Ok, this patch seems right according to spec.
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
But I wonder now if we need intel_psr_setup_vsc() at all for platforms different than gen9.
Because description of this bit is: This field enables the programmable header for the PSR2 VSC packet.
Without the programmable version I would assume display engine is now responsible for setting header entirely?
Art?
drivers/gpu/drm/i915/intel_psr.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 607c3ec41679..7607a58a6ec0 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) hsw_psr_setup_aux(intel_dp);
- if (dev_priv->psr.psr2_enabled) {
- if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) &&
!IS_GEMINILAKE(dev_priv))) {
if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))chicken |= (PSR2_VSC_ENABLE_PROG_HEADER| PSR2_ADD_VERTICAL_LINE_COUNT);elsechicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
chicken |= PSR2_VSC_ENABLE_PROG_HEADER |PSR2_ADD_VERTICAL_LINE_COUNT;-- 2.19.2
On Thu, 2018-11-29 at 14:15 -0800, Rodrigo Vivi wrote:
On Mon, Nov 26, 2018 at 04:37:05PM -0800, José Roberto de Souza wrote:
For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is already unset in gen10 + GLK we can just drop it and fix for both gens.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
Ok, this patch seems right according to spec.
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
But I wonder now if we need intel_psr_setup_vsc() at all for platforms different than gen9.
Because description of this bit is: This field enables the programmable header for the PSR2 VSC packet.
Without the programmable version I would assume display engine is now responsible for setting header entirely?
As this was in a chicken register in gen 9 my guess is that it was fixed on newer gens and as it is required for PSR2 we don't need to manually enable it in driver but Art could confirm it.
Art?
drivers/gpu/drm/i915/intel_psr.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 607c3ec41679..7607a58a6ec0 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) hsw_psr_setup_aux(intel_dp);
- if (dev_priv->psr.psr2_enabled) {
- if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) &&
!IS_GEMINILAKE(dev_priv))) {; u32 chicken = I915_READ(reg);
if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))chicken |= (PSR2_VSC_ENABLE_PROG_HEADER| PSR2_ADD_VERTICAL_LINE_COUNT);elsechicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
chicken |= PSR2_VSC_ENABLE_PROG_HEADER |PSR2_ADD_VERTICAL_LINE_COUNT;-- 2.19.2
I'll check on it.
-----Original Message----- From: Souza, Jose Sent: Thursday, 29 November, 2018 3:47 PM To: Vivi, Rodrigo rodrigo.vivi@intel.com Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Runyan, Arthur J arthur.j.runyan@intel.com; Pandiyan, Dhinakaran dhinakaran.pandiyan@intel.com Subject: Re: [PATCH 4/9] drm/i915/icl: Do not change reserved registers related to PSR2
On Thu, 2018-11-29 at 14:15 -0800, Rodrigo Vivi wrote:
On Mon, Nov 26, 2018 at 04:37:05PM -0800, José Roberto de Souza wrote:
For ICL the bit 12 of CHICKEN_TRANS is reserved so we should not touch it and as by default VSC_DATA_SEL_SOFTWARE_CONTROL is
already
unset in gen10 + GLK we can just drop it and fix for both gens.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
Ok, this patch seems right according to spec.
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
But I wonder now if we need intel_psr_setup_vsc() at all for platforms different than gen9.
Because description of this bit is: This field enables the programmable header for the PSR2 VSC packet.
Without the programmable version I would assume display engine is now responsible for setting header entirely?
As this was in a chicken register in gen 9 my guess is that it was fixed on newer gens and as it is required for PSR2 we don't need to manually enable it in driver but Art could confirm it.
Art?
drivers/gpu/drm/i915/intel_psr.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 607c3ec41679..7607a58a6ec0 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -635,17 +635,14 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) hsw_psr_setup_aux(intel_dp);
- if (dev_priv->psr.psr2_enabled) {
- if (dev_priv->psr.psr2_enabled && (IS_GEN9(dev_priv) &&
!IS_GEMINILAKE(dev_priv))) {; u32 chicken = I915_READ(reg);
if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv))chicken |= (PSR2_VSC_ENABLE_PROG_HEADER| PSR2_ADD_VERTICAL_LINE_COUNT);elsechicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
chicken |= PSR2_VSC_ENABLE_PROG_HEADER |PSR2_ADD_VERTICAL_LINE_COUNT;-- 2.19.2
Source is required to comply to sink SU granularity when DP_PSR2_SU_GRANULARITY_REQUIRED is set in DP_PSR_CAPS, so adding the register here.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com --- include/drm/drm_dp_helper.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index db516c48cda3..acc7ccfd2044 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -314,6 +314,9 @@ # define DP_PSR_SETUP_TIME_SHIFT 1 # define DP_PSR2_SU_Y_COORDINATE_REQUIRED (1 << 4) /* eDP 1.4a */ # define DP_PSR2_SU_GRANULARITY_REQUIRED (1 << 5) /* eDP 1.4b */ + +#define DP_PSR2_SU_X_GRANULARITY 0x072 /* eDP 1.4b */ + /* * 0x80-0x8f describe downstream port capabilities, but there are two layouts * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set. If it was not,
On Mon, Nov 26, 2018 at 04:37:06PM -0800, José Roberto de Souza wrote:
Source is required to comply to sink SU granularity when DP_PSR2_SU_GRANULARITY_REQUIRED is set in DP_PSR_CAPS, so adding the register here.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
include/drm/drm_dp_helper.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index db516c48cda3..acc7ccfd2044 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -314,6 +314,9 @@ # define DP_PSR_SETUP_TIME_SHIFT 1 # define DP_PSR2_SU_Y_COORDINATE_REQUIRED (1 << 4) /* eDP 1.4a */ # define DP_PSR2_SU_GRANULARITY_REQUIRED (1 << 5) /* eDP 1.4b */
+#define DP_PSR2_SU_X_GRANULARITY 0x072 /* eDP 1.4b */
/*
- 0x80-0x8f describe downstream port capabilities, but there are two layouts
- based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set. If it was not,
-- 2.19.2
According to eDP spec, sink can required specific selective update granularity that source must comply. Here caching the value if required and checking source supports it.
Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_psr.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f763b30f98d9..cbcd85af95bf 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -506,6 +506,7 @@ struct i915_psr { ktime_t last_exit; bool sink_not_reliable; bool irq_aux_error; + u16 su_x_granularity; };
enum intel_pch { diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 7607a58a6ec0..9215c9052381 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -257,6 +257,21 @@ static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp) return val; }
+static u16 intel_dp_get_su_x_granulartiy(struct intel_dp *intel_dp) +{ + u16 val = 0; + ssize_t r; + + if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED)) + return val; + + r = drm_dp_dpcd_read(&intel_dp->aux, DP_PSR2_SU_X_GRANULARITY, &val, 2); + if (r != 2) + DRM_WARN("Unable to read DP_PSR2_SU_X_GRANULARITY\n"); + + return val; +} + void intel_psr_init_dpcd(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = @@ -311,6 +326,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) if (dev_priv->psr.sink_psr2_support) { dev_priv->psr.colorimetry_support = intel_dp_get_colorimetry_status(intel_dp); + dev_priv->psr.su_x_granularity = + intel_dp_get_su_x_granulartiy(intel_dp); } } } @@ -525,6 +542,21 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, return false; }
+ if (dev_priv->psr.su_x_granularity) { + /* + * HW will always send full lines in SU blocks, so X will + * always be 0 and we only need to check the width to validate + * horizontal granularity. + * About vertical granularity HW works by SU blocks starting + * at each 4 lines with height of 4 lines, what eDP states + * that sink should support. + */ + if (crtc_hdisplay % dev_priv->psr.su_x_granularity) { + DRM_DEBUG_KMS("PSR2 not enabled, HW can not match sink SU granularity requirement\n"); + return false; + } + } + return true; }
On Mon, Nov 26, 2018 at 04:37:07PM -0800, José Roberto de Souza wrote:
According to eDP spec, sink can required specific selective update granularity that source must comply. Here caching the value if required and checking source supports it.
Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_psr.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f763b30f98d9..cbcd85af95bf 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -506,6 +506,7 @@ struct i915_psr { ktime_t last_exit; bool sink_not_reliable; bool irq_aux_error;
- u16 su_x_granularity;
};
enum intel_pch { diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 7607a58a6ec0..9215c9052381 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -257,6 +257,21 @@ static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp) return val; }
+static u16 intel_dp_get_su_x_granulartiy(struct intel_dp *intel_dp) +{
- u16 val = 0;
- ssize_t r;
- if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED))
return val;- r = drm_dp_dpcd_read(&intel_dp->aux, DP_PSR2_SU_X_GRANULARITY, &val, 2);
- if (r != 2)
DRM_WARN("Unable to read DP_PSR2_SU_X_GRANULARITY\n");- return val;
+}
void intel_psr_init_dpcd(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = @@ -311,6 +326,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) if (dev_priv->psr.sink_psr2_support) { dev_priv->psr.colorimetry_support = intel_dp_get_colorimetry_status(intel_dp);
dev_priv->psr.su_x_granularity =intel_dp_get_su_x_granulartiy(intel_dp);} @@ -525,6 +542,21 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, return false; }
- if (dev_priv->psr.su_x_granularity) {
/** HW will always send full lines in SU blocks, so X will* always be 0 and we only need to check the width to validate* horizontal granularity.* About vertical granularity HW works by SU blocks starting* at each 4 lines with height of 4 lines, what eDP states* that sink should support.*/if (crtc_hdisplay % dev_priv->psr.su_x_granularity) {DRM_DEBUG_KMS("PSR2 not enabled, HW can not match sink SU granularity requirement\n");return false;
I wonder if regardless this bit we still need to do a sort of check anyway because spec states:
" Sets the grid pattern granularity in the X direction. A value of 0 indicates that no X-coordinate granularity requirement exists other than the standard restrictions, wherein the: • Starting X-coordinate must be evenly divisible by 16 • Rectangle width must be evenly divisible by 4 "
Also, why we are just checking X granularity and not Y? (0074h)
(maybe it would be useful to introduce along with previous patch that I had just reviewed)
}- }
- return true;
}
-- 2.19.2
On Thu, 2018-11-29 at 15:03 -0800, Rodrigo Vivi wrote:
On Mon, Nov 26, 2018 at 04:37:07PM -0800, José Roberto de Souza wrote:
According to eDP spec, sink can required specific selective update granularity that source must comply. Here caching the value if required and checking source supports it.
Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_psr.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f763b30f98d9..cbcd85af95bf 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -506,6 +506,7 @@ struct i915_psr { ktime_t last_exit; bool sink_not_reliable; bool irq_aux_error;
- u16 su_x_granularity;
};
enum intel_pch { diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 7607a58a6ec0..9215c9052381 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -257,6 +257,21 @@ static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp) return val; }
+static u16 intel_dp_get_su_x_granulartiy(struct intel_dp *intel_dp) +{
- u16 val = 0;
- ssize_t r;
- if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED))
return val;- r = drm_dp_dpcd_read(&intel_dp->aux, DP_PSR2_SU_X_GRANULARITY,
&val, 2);
- if (r != 2)
DRM_WARN("Unable to read DP_PSR2_SU_X_GRANULARITY\n");- return val;
+}
void intel_psr_init_dpcd(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = @@ -311,6 +326,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) if (dev_priv->psr.sink_psr2_support) { dev_priv->psr.colorimetry_support = intel_dp_get_colorimetry_status(intel_d p);
dev_priv->psr.su_x_granularity =intel_dp_get_su_x_granulartiy(intel_dp); } } } @@ -525,6 +542,21 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, return false; }
- if (dev_priv->psr.su_x_granularity) {
/** HW will always send full lines in SU blocks, so Xwill
* always be 0 and we only need to check the width tovalidate
* horizontal granularity.* About vertical granularity HW works by SU blocksstarting
* at each 4 lines with height of 4 lines, what eDPstates
* that sink should support.*/if (crtc_hdisplay % dev_priv->psr.su_x_granularity) {DRM_DEBUG_KMS("PSR2 not enabled, HW can notmatch sink SU granularity requirement\n");
return false;I wonder if regardless this bit we still need to do a sort of check anyway because spec states:
" Sets the grid pattern granularity in the X direction. A value of 0 indicates that no X-coordinate granularity requirement exists other than the standard restrictions, wherein the: • Starting X-coordinate must be evenly divisible by 16 • Rectangle width must be evenly divisible by 4 "
Hum, the x will always be 0 so it will always be divisible by 16 but the width could not be divisible by 4 in the odd resolutions. I will add this check, thanks.
Also, why we are just checking X granularity and not Y? (0074h)
(maybe it would be useful to introduce along with previous patch that I had just reviewed)
It is in the comment above but coping from eDP spec:
A value of 00h, 01h, 02h, or 04h should be supported by the Sink device to ensure interoperability with various Source devices. A value of 08h or 10h may be considered for system-specific implementations.
Our height is multiple of 4 so we are safe even if sink have a y granularity set.
}- }
- return true;
}
-- 2.19.2
The first 8 bits of PSR2_CTL have 2 fields to set frames count, the first one is to set how many idle frames PSR2 HW needs to wait before enter in deep sleep and the second one it is how many frames(it don't need to be idle frames) PSR2 HW will wait before start the PSR activation sequence. The previous names was really misleading and caused wrong values being set so better rename to make it clear.
Also taking the oportunity to improve those macros.
Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com --- drivers/gpu/drm/i915/i915_reg.h | 35 ++++++++++++++++---------------- drivers/gpu/drm/i915/intel_psr.c | 7 ++++--- 2 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 47baf2fe8f71..73046bb9ec7c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4203,23 +4203,24 @@ enum { #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* Reserved in ICL+ */ #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
-#define EDP_PSR2_CTL _MMIO(0x6f900) -#define EDP_PSR2_ENABLE (1 << 31) -#define EDP_SU_TRACK_ENABLE (1 << 30) -#define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ -#define EDP_Y_COORDINATE_ENABLE (1 << 25) /* GLK and CNL+ */ -#define EDP_MAX_SU_DISABLE_TIME(t) ((t) << 20) -#define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f << 20) -#define EDP_PSR2_TP2_TIME_500us (0 << 8) -#define EDP_PSR2_TP2_TIME_100us (1 << 8) -#define EDP_PSR2_TP2_TIME_2500us (2 << 8) -#define EDP_PSR2_TP2_TIME_50us (3 << 8) -#define EDP_PSR2_TP2_TIME_MASK (3 << 8) -#define EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4 -#define EDP_PSR2_FRAME_BEFORE_SU_MASK (0xf << 4) -#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << 4) -#define EDP_PSR2_IDLE_FRAME_MASK 0xf -#define EDP_PSR2_IDLE_FRAME_SHIFT 0 +#define EDP_PSR2_CTL _MMIO(0x6f900) +#define EDP_PSR2_ENABLE (1 << 31) +#define EDP_SU_TRACK_ENABLE (1 << 30) +#define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ +#define EDP_Y_COORDINATE_ENABLE (1 << 25) /* GLK and CNL+ */ +#define EDP_MAX_SU_DISABLE_TIME(t) ((t) << 20) +#define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f << 20) +#define EDP_PSR2_TP2_TIME_500us (0 << 8) +#define EDP_PSR2_TP2_TIME_100us (1 << 8) +#define EDP_PSR2_TP2_TIME_2500us (2 << 8) +#define EDP_PSR2_TP2_TIME_50us (3 << 8) +#define EDP_PSR2_TP2_TIME_MASK (3 << 8) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT (4) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK (0xf << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE(n) (((n) << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) & EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK (0xf) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT (0) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(n) (((n) << EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT) & EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK)
#define _PSR_EVENT_TRANS_A 0x60848 #define _PSR_EVENT_TRANS_B 0x61848 diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 9215c9052381..ba7bbe3f8df2 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -479,6 +479,7 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp) static void hsw_activate_psr2(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); + struct i915_psr *psr = &dev_priv->psr; u32 val;
/* Let's use 6 as the minimum to cover all known cases including the @@ -486,8 +487,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) */ int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
- idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1); - val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT; + idle_frames = max(idle_frames, psr->sink_sync_latency + 1); + val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
/* FIXME: selective update is probably totally broken because it doesn't * mesh at all with our frontbuffer tracking. And the hw alone isn't @@ -496,7 +497,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) val |= EDP_Y_COORDINATE_ENABLE;
- val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv->psr.sink_sync_latency + 1); + val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1);
if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 && dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
On Mon, Nov 26, 2018 at 04:37:08PM -0800, José Roberto de Souza wrote:
The first 8 bits of PSR2_CTL have 2 fields to set frames count, the first one is to set how many idle frames PSR2 HW needs to wait before enter in deep sleep and the second one it is how many frames(it don't need to be idle frames) PSR2 HW will wait before start the PSR activation sequence. The previous names was really misleading and caused wrong values being set so better rename to make it clear.
I honestly prefer the old names for 2 reasons:
- they are shorter - they follow the exact name we have on spec
Also taking the oportunity to improve those macros.
Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/i915/i915_reg.h | 35 ++++++++++++++++---------------- drivers/gpu/drm/i915/intel_psr.c | 7 ++++--- 2 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 47baf2fe8f71..73046bb9ec7c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4203,23 +4203,24 @@ enum { #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* Reserved in ICL+ */ #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
-#define EDP_PSR2_CTL _MMIO(0x6f900) -#define EDP_PSR2_ENABLE (1 << 31) -#define EDP_SU_TRACK_ENABLE (1 << 30) -#define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ -#define EDP_Y_COORDINATE_ENABLE (1 << 25) /* GLK and CNL+ */ -#define EDP_MAX_SU_DISABLE_TIME(t) ((t) << 20) -#define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f << 20) -#define EDP_PSR2_TP2_TIME_500us (0 << 8) -#define EDP_PSR2_TP2_TIME_100us (1 << 8) -#define EDP_PSR2_TP2_TIME_2500us (2 << 8) -#define EDP_PSR2_TP2_TIME_50us (3 << 8) -#define EDP_PSR2_TP2_TIME_MASK (3 << 8) -#define EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4 -#define EDP_PSR2_FRAME_BEFORE_SU_MASK (0xf << 4) -#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << 4) -#define EDP_PSR2_IDLE_FRAME_MASK 0xf -#define EDP_PSR2_IDLE_FRAME_SHIFT 0 +#define EDP_PSR2_CTL _MMIO(0x6f900) +#define EDP_PSR2_ENABLE (1 << 31) +#define EDP_SU_TRACK_ENABLE (1 << 30) +#define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ +#define EDP_Y_COORDINATE_ENABLE (1 << 25) /* GLK and CNL+ */ +#define EDP_MAX_SU_DISABLE_TIME(t) ((t) << 20) +#define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f << 20) +#define EDP_PSR2_TP2_TIME_500us (0 << 8) +#define EDP_PSR2_TP2_TIME_100us (1 << 8) +#define EDP_PSR2_TP2_TIME_2500us (2 << 8) +#define EDP_PSR2_TP2_TIME_50us (3 << 8) +#define EDP_PSR2_TP2_TIME_MASK (3 << 8) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT (4) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK (0xf << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE(n) (((n) << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) & EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK (0xf) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT (0) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(n) (((n) << EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT) & EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK)
#define _PSR_EVENT_TRANS_A 0x60848 #define _PSR_EVENT_TRANS_B 0x61848 diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 9215c9052381..ba7bbe3f8df2 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -479,6 +479,7 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp) static void hsw_activate_psr2(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
struct i915_psr *psr = &dev_priv->psr; u32 val;
/* Let's use 6 as the minimum to cover all known cases including the
@@ -486,8 +487,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) */ int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
- idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1);
- val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT;
idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
/* FIXME: selective update is probably totally broken because it doesn't
- mesh at all with our frontbuffer tracking. And the hw alone isn't
@@ -496,7 +497,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) val |= EDP_Y_COORDINATE_ENABLE;
- val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv->psr.sink_sync_latency + 1);
val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1);
if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 && dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50)
-- 2.19.2
On Thu, 2018-11-29 at 15:07 -0800, Rodrigo Vivi wrote:
On Mon, Nov 26, 2018 at 04:37:08PM -0800, José Roberto de Souza wrote:
The first 8 bits of PSR2_CTL have 2 fields to set frames count, the first one is to set how many idle frames PSR2 HW needs to wait before enter in deep sleep and the second one it is how many frames(it don't need to be idle frames) PSR2 HW will wait before start the PSR activation sequence. The previous names was really misleading and caused wrong values
The idea was to setup a conservative configuration for PSR2 until we were ready to enable the feature and some testing was done. Not sure why you think the values are wrong.
being set so better rename to make it clear.
I honestly prefer the old names for 2 reasons:
- they are shorter
- they follow the exact name we have on spec
+1 for the above reason.
Also taking the oportunity to improve those macros.
Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/i915/i915_reg.h | 35 ++++++++++++++++----------
drivers/gpu/drm/i915/intel_psr.c | 7 ++++--- 2 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 47baf2fe8f71..73046bb9ec7c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4203,23 +4203,24 @@ enum { #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* Reserved in ICL+ */ #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
-#define EDP_PSR2_CTL _MMIO(0x6f900) -#define EDP_PSR2_ENABLE (1 << 31) -#define EDP_SU_TRACK_ENABLE (1 << 30) -#define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ -#define EDP_Y_COORDINATE_ENABLE (1 << 25) /* GLK and CNL+ */ -#define EDP_MAX_SU_DISABLE_TIME(t) ((t) << 20) -#define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f << 20) -#define EDP_PSR2_TP2_TIME_500us (0 << 8) -#define EDP_PSR2_TP2_TIME_100us (1 << 8) -#define EDP_PSR2_TP2_TIME_2500us (2 << 8) -#define EDP_PSR2_TP2_TIME_50us (3 << 8) -#define EDP_PSR2_TP2_TIME_MASK (3 << 8) -#define EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4 -#define EDP_PSR2_FRAME_BEFORE_SU_MASK (0xf << 4) -#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << 4) -#define EDP_PSR2_IDLE_FRAME_MASK 0xf -#define EDP_PSR2_IDLE_FRAME_SHIFT 0 +#define EDP_PSR2_CTL _MMIO(0 x6f900) +#define EDP_PSR2_ENABLE (1 << 31) +#define EDP_SU_TRACK_ENABLE (1 << 30) +#define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ +#define EDP_Y_COORDINATE_ENABLE (1 << 25) /* GLK and CNL+ */ +#define EDP_MAX_SU_DISABLE_TIME(t) ((t) << 20) +#define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f << 20) +#define EDP_PSR2_TP2_TIME_500us (0 << 8) +#define EDP_PSR2_TP2_TIME_100us (1 << 8) +#define EDP_PSR2_TP2_TIME_2500us (2 << 8) +#define EDP_PSR2_TP2_TIME_50us (3 << 8) +#define EDP_PSR2_TP2_TIME_MASK (3 << 8) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT (4) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK (0xf << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE(n) (((n) << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) & EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK (0xf) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT (0) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(n) (((n) << EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT) & EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK)
#define _PSR_EVENT_TRANS_A 0x60848 #define _PSR_EVENT_TRANS_B 0x61848 diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 9215c9052381..ba7bbe3f8df2 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -479,6 +479,7 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp) static void hsw_activate_psr2(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
struct i915_psr *psr = &dev_priv->psr; u32 val;
/* Let's use 6 as the minimum to cover all known cases
including the @@ -486,8 +487,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) */ int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
- idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency
- 1);
- val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT;
idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
/* FIXME: selective update is probably totally broken because
it doesn't * mesh at all with our frontbuffer tracking. And the hw alone isn't @@ -496,7 +497,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) val |= EDP_Y_COORDINATE_ENABLE;
- val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv->psr.sink_sync_latency
- 1);
- val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency +
1);
if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 && dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50) -- 2.19.2
On Thu, 2018-11-29 at 15:25 -0800, Dhinakaran Pandiyan wrote:
On Thu, 2018-11-29 at 15:07 -0800, Rodrigo Vivi wrote:
On Mon, Nov 26, 2018 at 04:37:08PM -0800, José Roberto de Souza wrote:
The first 8 bits of PSR2_CTL have 2 fields to set frames count, the first one is to set how many idle frames PSR2 HW needs to wait before enter in deep sleep and the second one it is how many frames(it don't need to be idle frames) PSR2 HW will wait before start the PSR activation sequence. The previous names was really misleading and caused wrong values
The idea was to setup a conservative configuration for PSR2 until we were ready to enable the feature and some testing was done. Not sure why you think the values are wrong.
being set so better rename to make it clear.
I honestly prefer the old names for 2 reasons:
- they are shorter
- they follow the exact name we have on spec
+1 for the above reason.
Okay droping this patch.
Also taking the oportunity to improve those macros.
Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/i915/i915_reg.h | 35 ++++++++++++++++----------
drivers/gpu/drm/i915/intel_psr.c | 7 ++++--- 2 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 47baf2fe8f71..73046bb9ec7c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4203,23 +4203,24 @@ enum { #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* Reserved in ICL+ */ #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
-#define EDP_PSR2_CTL _MMIO(0x6f900) -#define EDP_PSR2_ENABLE (1 << 31) -#define EDP_SU_TRACK_ENABLE (1 << 30) -#define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ -#define EDP_Y_COORDINATE_ENABLE (1 << 25) /* GLK and CNL+ */ -#define EDP_MAX_SU_DISABLE_TIME(t) ((t) << 20) -#define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f << 20) -#define EDP_PSR2_TP2_TIME_500us (0 << 8) -#define EDP_PSR2_TP2_TIME_100us (1 << 8) -#define EDP_PSR2_TP2_TIME_2500us (2 << 8) -#define EDP_PSR2_TP2_TIME_50us (3 << 8) -#define EDP_PSR2_TP2_TIME_MASK (3 << 8) -#define EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4 -#define EDP_PSR2_FRAME_BEFORE_SU_MASK (0xf << 4) -#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << 4) -#define EDP_PSR2_IDLE_FRAME_MASK 0xf -#define EDP_PSR2_IDLE_FRAME_SHIFT 0 +#define EDP_PSR2_CTL _MMIO(0 x6f900) +#define EDP_PSR2_ENABLE (1 << 31) +#define EDP_SU_TRACK_ENABLE (1 << 30) +#define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ +#define EDP_Y_COORDINATE_ENABLE (1 << 25) /* GLK and CNL+ */ +#define EDP_MAX_SU_DISABLE_TIME(t) ((t) << 20) +#define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f << 20) +#define EDP_PSR2_TP2_TIME_500us (0 << 8) +#define EDP_PSR2_TP2_TIME_100us (1 << 8) +#define EDP_PSR2_TP2_TIME_2500us (2 << 8) +#define EDP_PSR2_TP2_TIME_50us (3 << 8) +#define EDP_PSR2_TP2_TIME_MASK (3 << 8) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT (4) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK (0xf << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) +#define EDP_PSR2_FRAMES_BEFORE_ACTIVATE(n) (((n) << EDP_PSR2_FRAMES_BEFORE_ACTIVATE_SHIFT) & EDP_PSR2_FRAMES_BEFORE_ACTIVATE_MASK) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK (0xf) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT (0) +#define EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(n) (((n) << EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_SHIFT) & EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP_MASK)
#define _PSR_EVENT_TRANS_A 0x60848 #define _PSR_EVENT_TRANS_B 0x61848 diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 9215c9052381..ba7bbe3f8df2 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -479,6 +479,7 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp) static void hsw_activate_psr2(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
struct i915_psr *psr = &dev_priv->psr; u32 val;
/* Let's use 6 as the minimum to cover all known cases
including the @@ -486,8 +487,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) */ int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
- idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency
- 1);
- val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT;
idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
/* FIXME: selective update is probably totally broken because
it doesn't * mesh at all with our frontbuffer tracking. And the hw alone isn't @@ -496,7 +497,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) val |= EDP_Y_COORDINATE_ENABLE;
- val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv-
psr.sink_sync_latency
- 1);
- val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency +
1);
if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 && dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50) -- 2.19.2
EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP() was being set with the number of frames that it should wait to enter PSR, what is wrong. Here it is setting this field with the highest value to avoid PSR2 exits frequently, as when HW exit deep sleep it needs to go to idle state causing a PSR exit for then waiting a few frames before activate PSR2 again. This will result in more power saving as the sleep state also provide some power savings by doing selective updates instead of full screen updates.
About EDP_PSR2_FRAMES_BEFORE_ACTIVATE() it is the number of frames (not idle frames) that PSR2 hardware will wait to activate PSR2, so lets keep using the sink sync latency.
Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index ba7bbe3f8df2..6fd793fec5e9 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -482,13 +482,13 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) struct i915_psr *psr = &dev_priv->psr; u32 val;
- /* Let's use 6 as the minimum to cover all known cases including the - * off-by-one issue that HW has in some cases. + /* sink_sync_latency of 8 means source has to wait for more than 8 + * frames, we'll go with 9 frames for now */ - int idle_frames = max(6, dev_priv->vbt.psr.idle_frames); + val = EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1);
- idle_frames = max(idle_frames, psr->sink_sync_latency + 1); - val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames); + /* Avoid deep sleep as much as possible to avoid PSR2 idle state */ + val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
/* FIXME: selective update is probably totally broken because it doesn't * mesh at all with our frontbuffer tracking. And the hw alone isn't @@ -497,8 +497,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) val |= EDP_Y_COORDINATE_ENABLE;
- val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1); - if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 && dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50) val |= EDP_PSR2_TP2_TIME_50us;
On Mon, Nov 26, 2018 at 04:37:09PM -0800, José Roberto de Souza wrote:
EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP() was being set with the number of frames that it should wait to enter PSR, what is wrong. Here it is setting this field with the highest value to avoid PSR2 exits frequently, as when HW exit deep sleep it needs to go to idle state causing a PSR exit for then waiting a few frames before activate PSR2 again. This will result in more power saving as the sleep state also provide some power savings by doing selective updates instead of full screen updates.
About EDP_PSR2_FRAMES_BEFORE_ACTIVATE() it is the number of frames (not idle frames) that PSR2 hardware will wait to activate PSR2, so lets keep using the sink sync latency.
Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/i915/intel_psr.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index ba7bbe3f8df2..6fd793fec5e9 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -482,13 +482,13 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) struct i915_psr *psr = &dev_priv->psr; u32 val;
- /* Let's use 6 as the minimum to cover all known cases including the
* off-by-one issue that HW has in some cases.
- /* sink_sync_latency of 8 means source has to wait for more than 8
* frames, we'll go with 9 frames for now
- int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
Too many changes in a single patch that I couldn't understand why we are removing the minimal of 6 that was our safe net.
- val = EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1);
- idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
- val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
/* Avoid deep sleep as much as possible to avoid PSR2 idle state */
val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
/* FIXME: selective update is probably totally broken because it doesn't
- mesh at all with our frontbuffer tracking. And the hw alone isn't
@@ -497,8 +497,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) val |= EDP_Y_COORDINATE_ENABLE;
- val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency + 1);
- if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 && dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50) val |= EDP_PSR2_TP2_TIME_50us;
-- 2.19.2
On Thu, 2018-11-29 at 15:10 -0800, Rodrigo Vivi wrote:
On Mon, Nov 26, 2018 at 04:37:09PM -0800, José Roberto de Souza wrote:
EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP() was being set with the number of frames that it should wait to enter PSR, what is wrong. Here it is setting this field with the highest value to avoid PSR2 exits frequently, as when HW exit deep sleep it needs to go to idle state causing a PSR exit for then waiting a few frames before activate PSR2 again. This will result in more power saving as the sleep state also provide some power savings by doing selective updates instead of full screen updates.
About EDP_PSR2_FRAMES_BEFORE_ACTIVATE() it is the number of frames (not idle frames) that PSR2 hardware will wait to activate PSR2, so lets keep using the sink sync latency.
Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/i915/intel_psr.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index ba7bbe3f8df2..6fd793fec5e9 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -482,13 +482,13 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) struct i915_psr *psr = &dev_priv->psr; u32 val;
- /* Let's use 6 as the minimum to cover all known cases
including the
* off-by-one issue that HW has in some cases.
- /* sink_sync_latency of 8 means source has to wait for more
than 8
* frames, we'll go with 9 frames for now
- int idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
Too many changes in a single patch that I couldn't understand why we are removing the minimal of 6 that was our safe net.
The vbt idle_frames should not be used for PSR2 as PSR2 just wait for X frames(idle or not) to activate PSR2 so just rely in sink sync_latency should be enough but I can bring it back for safety.
- val = EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency +
1);
- idle_frames = max(idle_frames, psr->sink_sync_latency + 1);
- val = EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(idle_frames);
- /* Avoid deep sleep as much as possible to avoid PSR2 idle
state */
val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
/* FIXME: selective update is probably totally broken because
it doesn't * mesh at all with our frontbuffer tracking. And the hw alone isn't @@ -497,8 +497,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) val |= EDP_Y_COORDINATE_ENABLE;
- val |= EDP_PSR2_FRAMES_BEFORE_ACTIVATE(psr->sink_sync_latency +
1);
- if (dev_priv->vbt.psr.tp2_tp3_wakeup_time_us >= 0 && dev_priv->vbt.psr.tp2_tp3_wakeup_time_us <= 50) val |= EDP_PSR2_TP2_TIME_50us;
-- 2.19.2
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Our frontbuffer tracking improved over the years + the WA #0884 helped us keep PSR2 enabled while triggering screen updates when necessary so this FIXME is not valid anymore.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 6fd793fec5e9..a1bde8bbd85b 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -490,9 +490,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) /* Avoid deep sleep as much as possible to avoid PSR2 idle state */ val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
- /* FIXME: selective update is probably totally broken because it doesn't - * mesh at all with our frontbuffer tracking. And the hw alone isn't - * good enough. */ val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE; if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) val |= EDP_Y_COORDINATE_ENABLE;
On Mon, Nov 26, 2018 at 04:37:10PM -0800, José Roberto de Souza wrote:
Our frontbuffer tracking improved over the years + the WA #0884 helped us keep PSR2 enabled while triggering screen updates when necessary so this FIXME is not valid anymore.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
drivers/gpu/drm/i915/intel_psr.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 6fd793fec5e9..a1bde8bbd85b 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -490,9 +490,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) /* Avoid deep sleep as much as possible to avoid PSR2 idle state */ val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
- /* FIXME: selective update is probably totally broken because it doesn't
* mesh at all with our frontbuffer tracking. And the hw alone isn't* good enough. */-- 2.19.2
On Thu, 2018-11-29 at 15:11 -0800, Rodrigo Vivi wrote:
On Mon, Nov 26, 2018 at 04:37:10PM -0800, José Roberto de Souza wrote:
Our frontbuffer tracking improved over the years + the WA #0884 helped us keep PSR2 enabled while triggering screen updates when necessary so this FIXME is not valid anymore.
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Cc: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
Acked-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
drivers/gpu/drm/i915/intel_psr.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 6fd793fec5e9..a1bde8bbd85b 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -490,9 +490,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) /* Avoid deep sleep as much as possible to avoid PSR2 idle state */ val |= EDP_PSR2_IDLE_FRAMES_TO_DEEP_SLEEP(15);
- /* FIXME: selective update is probably totally broken because
it doesn't
* mesh at all with our frontbuffer tracking. And the hw aloneisn't
* good enough. */-- 2.19.2
On Mon, Nov 26, 2018 at 04:37:02PM -0800, José Roberto de Souza wrote:
i915 yet don't support PSR in Apple panels, so lets keep it disabled while we work on that.
Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW) Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/drm_dp_helper.c | 2 ++ drivers/gpu/drm/i915/intel_psr.c | 6 ++++++ include/drm/drm_dp_helper.h | 1 + 3 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 6d483487f2b4..6b5a19d3e347 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -1273,6 +1273,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { { OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) }, /* LG LP140WF6-SPM1 eDP panel */ { OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
- /* Apple panels needs some additional handling to support PSR */
- { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED) }
};
#undef OUI diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 572e626eadff..f5d27a02eb28 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -274,6 +274,12 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) DRM_DEBUG_KMS("Panel lacks power state control, PSR cannot be enabled\n"); return; }
- if (drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED)) {
DRM_DEBUG_KMS("PSR support not currently available for this panel\n");return;- }
- dev_priv->psr.sink_support = true; dev_priv->psr.sink_sync_latency = intel_dp_get_sink_sync_latency(intel_dp);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 3314e91f6eb3..db516c48cda3 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1364,6 +1364,7 @@ enum drm_dp_quirk { * to 16 bits. So will give a constant value (0x8000) for compatability. */ DP_DPCD_QUIRK_CONSTANT_N,
- DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED,
Why such a convoluted name? DP_DPCD_QUIRK_NO_PSR?
};
/**
2.19.2
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 2018-11-27 at 15:38 +0200, Ville Syrjälä wrote:
On Mon, Nov 26, 2018 at 04:37:02PM -0800, José Roberto de Souza wrote:
i915 yet don't support PSR in Apple panels, so lets keep it disabled while we work on that.
Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW) Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/drm_dp_helper.c | 2 ++ drivers/gpu/drm/i915/intel_psr.c | 6 ++++++ include/drm/drm_dp_helper.h | 1 + 3 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 6d483487f2b4..6b5a19d3e347 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -1273,6 +1273,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { { OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) }, /* LG LP140WF6-SPM1 eDP panel */ { OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
- /* Apple panels needs some additional handling to support PSR
*/
- { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false,
BIT(DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED) } };
#undef OUI diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 572e626eadff..f5d27a02eb28 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -274,6 +274,12 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) DRM_DEBUG_KMS("Panel lacks power state control, PSR cannot be enabled\n"); return; }
- if (drm_dp_has_quirk(&intel_dp->desc,
DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED)) {
DRM_DEBUG_KMS("PSR support not currently available forthis panel\n");
return;- }
- dev_priv->psr.sink_support = true; dev_priv->psr.sink_sync_latency = intel_dp_get_sink_sync_latency(intel_dp);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 3314e91f6eb3..db516c48cda3 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1364,6 +1364,7 @@ enum drm_dp_quirk { * to 16 bits. So will give a constant value (0x8000) for compatability. */ DP_DPCD_QUIRK_CONSTANT_N,
- DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED,
Why such a convoluted name? DP_DPCD_QUIRK_NO_PSR?
Okay changing to DP_DPCD_QUIRK_NO_PSR.
};
/**
2.19.2
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 2018-11-27 at 13:55 -0800, Souza, Jose wrote:
On Tue, 2018-11-27 at 15:38 +0200, Ville Syrjälä wrote:
On Mon, Nov 26, 2018 at 04:37:02PM -0800, José Roberto de Souza wrote:
i915 yet don't support PSR in Apple panels, so lets keep
Replace "Apple" with specific model name?
disabled while we work on that.
Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW)
Bugzilla please. Also Cc the bug reporter?
Cc: Rodrigo Vivi rodrigo.vivi@intel.com Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Signed-off-by: José Roberto de Souza jose.souza@intel.com
drivers/gpu/drm/drm_dp_helper.c | 2 ++ drivers/gpu/drm/i915/intel_psr.c | 6 ++++++ include/drm/drm_dp_helper.h | 1 + 3 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 6d483487f2b4..6b5a19d3e347 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -1273,6 +1273,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = { { OUI(0x00, 0x22, 0xb9), DEVICE_ID_ANY, true, BIT(DP_DPCD_QUIRK_CONSTANT_N) }, /* LG LP140WF6-SPM1 eDP panel */ { OUI(0x00, 0x22, 0xb9), DEVICE_ID('s', 'i', 'v', 'a', 'r', 'T'), false, BIT(DP_DPCD_QUIRK_CONSTANT_N) },
- /* Apple panels needs some additional handling to support PSR
*/
- { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false,
BIT(DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED) } };
#undef OUI diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 572e626eadff..f5d27a02eb28 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -274,6 +274,12 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) DRM_DEBUG_KMS("Panel lacks power state control, PSR cannot be enabled\n"); return; }
- if (drm_dp_has_quirk(&intel_dp->desc,
DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED)) {
DRM_DEBUG_KMS("PSR support not currently available forthis panel\n");
return;- }
- dev_priv->psr.sink_support = true; dev_priv->psr.sink_sync_latency = intel_dp_get_sink_sync_latency(intel_dp);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 3314e91f6eb3..db516c48cda3 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1364,6 +1364,7 @@ enum drm_dp_quirk { * to 16 bits. So will give a constant value (0x8000) for compatability. */ DP_DPCD_QUIRK_CONSTANT_N,
- DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED,
Why such a convoluted name? DP_DPCD_QUIRK_NO_PSR?
Okay changing to DP_DPCD_QUIRK_NO_PSR.
};
/**
2.19.2
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi José,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on v4.20-rc4 next-20181126] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jos-Roberto-de-Souza/drm-i915-Disab... base: git://anongit.freedesktop.org/drm-intel for-linux-next reproduce: make htmldocs
All warnings (new ones prefixed by >>):
include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params ' include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params ' include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params ' include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params ' include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params ' include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params ' include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params ' include/net/mac80211.h:477: warning: cannot understand function prototype: 'struct ieee80211_ftm_responder_params ' net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_retries' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_failed' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.avg_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info' kernel/rcu/tree.c:685: warning: Excess function parameter 'irq' description in 'rcu_nmi_exit' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf' include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array' include/linux/gpio/driver.h:375: warning: Function parameter or member 'init_valid_mask' not described in 'gpio_chip' include/linux/iio/hw-consumer.h:1: warning: no structured comments found include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry' include/linux/regulator/driver.h:227: warning: Function parameter or member 'resume' not described in 'regulator_ops' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb' drivers/slimbus/stream.c:1: warning: no structured comments found include/linux/spi/spi.h:177: warning: Function parameter or member 'driver_override' not described in 'spi_device' drivers/target/target_core_device.c:1: warning: no structured comments found drivers/usb/typec/bus.c:1: warning: no structured comments found drivers/usb/typec/class.c:1: warning: no structured comments found include/linux/w1.h:281: warning: Function parameter or member 'of_match_table' not described in 'w1_family' fs/direct-io.c:257: warning: Excess function parameter 'offset' description in 'dio_complete' fs/file_table.c:1: warning: no structured comments found fs/libfs.c:477: warning: Excess function parameter 'available' description in 'simple_write_end' fs/posix_acl.c:646: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode' fs/posix_acl.c:646: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode' fs/posix_acl.c:646: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:183: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_read_lock' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:254: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_invalidate_range_start_gfx' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:302: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_invalidate_range_start_hsa' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:382: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor ' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:383: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor ' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_leaf' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_leaf' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'start' not described in 'for_each_amdgpu_vm_pt_leaf' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'end' not described in 'for_each_amdgpu_vm_pt_leaf' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:555: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_leaf' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_dfs_safe' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_dfs_safe' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_dfs_safe' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:603: warning: Function parameter or member 'entry' not described in 'for_each_amdgpu_vm_pt_dfs_safe' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:848: warning: Function parameter or member 'level' not described in 'amdgpu_vm_bo_param' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1356: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_func' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'level' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1523: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_huge' drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:3098: warning: Function parameter or member 'pasid' not described in 'amdgpu_vm_make_compute' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:171: warning: Function parameter or member 'backlight_link' not described in 'amdgpu_display_manager' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:171: warning: Function parameter or member 'freesync_module' not described in 'amdgpu_display_manager' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:171: warning: Function parameter or member 'fw_dmcu' not described in 'amdgpu_display_manager' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:171: warning: Function parameter or member 'dmcu_fw_version' not described in 'amdgpu_display_manager' drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:1: warning: no structured comments found include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver' include/drm/drm_drv.h:609: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver' include/drm/drm_atomic_state_helper.h:1: warning: no structured comments found
include/drm/drm_dp_helper.h:1369: warning: Enum value 'DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED' not described in enum 'drm_dp_quirk'
drivers/gpu/drm/drm_dp_helper.c:1364: warning: Function parameter or member 'dsc_dpcd' not described in 'drm_dp_dsc_sink_max_slice_count' drivers/gpu/drm/drm_dp_helper.c:1364: warning: Function parameter or member 'is_edp' not described in 'drm_dp_dsc_sink_max_slice_count' Error: Cannot open file drivers/gpu/drm/drm_global.c Error: Cannot open file drivers/gpu/drm/drm_global.c WARNING: kernel-doc 'scripts/kernel-doc -rst -enable-lineno -export drivers/gpu/drm/drm_global.c' failed with return code 2 drivers/gpu/drm/i915/i915_vma.h:49: warning: cannot understand function prototype: 'struct i915_vma ' drivers/gpu/drm/i915/i915_vma.h:1: warning: no structured comments found drivers/gpu/drm/i915/intel_guc_fwif.h:536: warning: cannot understand function prototype: 'struct guc_log_buffer_state ' drivers/gpu/drm/i915/i915_trace.h:1: warning: no structured comments found include/linux/skbuff.h:862: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'list' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'head_frag' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'encapsulation' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'csum_valid' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'csum_level' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'offload_fwd_mark' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'offload_mr_fwd_mark' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff' include/linux/skbuff.h:862: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff' include/net/sock.h:238: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_portpair' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_cookie' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_listener' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common' include/net/sock.h:238: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common' include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.rmem_alloc' not described in 'sock' include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.len' not described in 'sock' include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.head' not described in 'sock' include/net/sock.h:509: warning: Function parameter or member 'sk_backlog.tail' not described in 'sock' include/net/sock.h:509: warning: Function parameter or member 'sk_wq_raw' not described in 'sock' include/net/sock.h:509: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock' include/net/sock.h:509: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock' include/net/sock.h:509: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock' include/net/sock.h:509: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock' include/linux/netdevice.h:2052: warning: Function parameter or member 'adj_list.upper' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'adj_list.lower' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'gso_partial_features' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'switchdev_ops' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'name_assign_type' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'mpls_ptr' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'xdp_prog' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'qdisc_hash' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device' include/linux/netdevice.h:2052: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device' include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state' include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state' Documentation/admin-guide/cgroup-v2.rst:1507: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/admin-guide/cgroup-v2.rst:1509: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/admin-guide/cgroup-v2.rst:1510: WARNING: Block quote ends without a blank line; unexpected unindent. include/net/mac80211.h:1211: ERROR: Unexpected indentation. include/net/mac80211.h:1218: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/wait.h:110: WARNING: Block quote ends without a blank line; unexpected unindent. include/linux/wait.h:113: ERROR: Unexpected indentation. include/linux/wait.h:115: WARNING: Block quote ends without a blank line; unexpected unindent. kernel/time/hrtimer.c:1129: WARNING: Block quote ends without a blank line; unexpected unindent. kernel/signal.c:344: WARNING: Inline literal start-string without end-string. include/linux/kernel.h:137: WARNING: Inline interpreted text or phrase reference start-string without end-string. include/uapi/linux/firewire-cdev.h:312: WARNING: Inline literal start-string without end-string. Documentation/driver-api/gpio/board.rst:209: ERROR: Unexpected indentation. drivers/ata/libata-core.c:5958: ERROR: Unknown target name: "hw". drivers/message/fusion/mptbase.c:5057: WARNING: Definition list ends without a blank line; unexpected unindent. drivers/tty/serial/serial_core.c:1938: WARNING: Definition list ends without a blank line; unexpected unindent. include/linux/mtd/rawnand.h:1189: WARNING: Inline strong start-string without end-string. include/linux/mtd/rawnand.h:1191: WARNING: Inline strong start-string without end-string. include/linux/regulator/driver.h:286: ERROR: Unknown target name: "regulator_regmap_x_voltage". Documentation/driver-api/soundwire/locking.rst:50: ERROR: Inconsistent literal block quoting. Documentation/driver-api/soundwire/locking.rst:51: WARNING: Line block ends without a blank line. Documentation/driver-api/soundwire/locking.rst:55: WARNING: Inline substitution_reference start-string without end-string. Documentation/driver-api/soundwire/locking.rst:56: WARNING: Line block ends without a blank line. include/linux/spi/spi.h:365: ERROR: Unexpected indentation. Documentation/driver-api/usb/typec_bus.rst:76: WARNING: Definition list ends without a blank line; unexpected unindent. block/bio.c:883: WARNING: Inline emphasis start-string without end-string. fs/posix_acl.c:635: WARNING: Inline emphasis start-string without end-string. drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:1573: WARNING: Inline emphasis start-string without end-string. drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:1575: WARNING: Inline emphasis start-string without end-string. include/drm/drm_drv.h:656: ERROR: Unknown target name: "driver". Documentation/laptops/lg-laptop.rst:2: WARNING: Explicit markup ends without a blank line; unexpected unindent. Documentation/laptops/lg-laptop.rst:16: ERROR: Unexpected indentation. Documentation/laptops/lg-laptop.rst:17: WARNING: Block quote ends without a blank line; unexpected unindent.
vim +1369 include/drm/drm_dp_helper.h
76fa998a Jani Nikula 2017-05-18 @1369
:::::: The code at line 1369 was first introduced by commit :::::: 76fa998acd86b6b40d0217e12af39c2406bdcd2b drm/dp: start a DPCD based DP sink/branch device quirk database
:::::: TO: Jani Nikula jani.nikula@intel.com :::::: CC: Jani Nikula jani.nikula@intel.com
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
dri-devel@lists.freedesktop.org
-
Dhinakaran Pandiyan -
José Roberto de Souza -
kbuild test robot -
Rodrigo Vivi -
Runyan, Arthur J -
Souza, Jose
-
Ville Syrjälä