i915 yet don't support PSR in Apple panels, so lets keep it disabled while we work on that.
v2: Renamed DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED to DP_DPCD_QUIRK_NO_PSR (Ville)
Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW) Cc: Ville Syrjälä ville.syrjala@linux.intel.com 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 2d6c491a0542..b00fd5ced0a0 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_NO_PSR) } };
#undef OUI diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 2084784f320d..40ca6cc43cc4 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -278,6 +278,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_NO_PSR)) { + 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 5736c942c85b..047314ce25d6 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1365,6 +1365,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_NO_PSR, };
/**
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.
v2: Moving the set of DP_PSR_CRC_VERIFICATION to the else block of 'if (dev_priv->psr.psr2_enabled)' to another patch. (Rodrigo)
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 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 40ca6cc43cc4..8515f4a6f4f1 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -395,10 +395,11 @@ 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 (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);
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
For PSR2 there is no register to tell HW to keep main link enabled
Right, there is no bit in PSR2_CTL Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
while PSR2 is active, so don't configure sink DPCD with a misleading value.
v2: Moving the set of DP_PSR_CRC_VERIFICATION to the else block of 'if (dev_priv->psr.psr2_enabled)' to another patch. (Rodrigo)
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 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 40ca6cc43cc4..8515f4a6f4f1 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -395,10 +395,11 @@ 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 (dev_priv->psr.link_standby)
dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
As we have a else block for the 'if (dev_priv->psr.psr2_enabled) {' and this bit is only set for PSR1 move it to that block to make it more easy to read.
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 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 8515f4a6f4f1..b04472e637c8 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -398,10 +398,11 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp) } 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.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 Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
As we have a else block for the 'if (dev_priv->psr.psr2_enabled) {' and this bit is only set for PSR1 move it to that block to make it more easy to read.
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 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 8515f4a6f4f1..b04472e637c8 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -398,10 +398,11 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp) } 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.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);
Do we need this DPCD write? The panel should already be awake by this point, I think it's worth removing it if there's no regression.
Your change in this patch looks good, so Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
On Fri, 2018-11-30 at 15:54 -0800, Dhinakaran Pandiyan wrote:
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
As we have a else block for the 'if (dev_priv->psr.psr2_enabled) {' and this bit is only set for PSR1 move it to that block to make it more easy to read.
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 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 8515f4a6f4f1..b04472e637c8 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -398,10 +398,11 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp) } 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.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);
Do we need this DPCD write? The panel should already be awake by this point, I think it's worth removing it if there's no regression.
Your change in this patch looks good, so Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
Added to the list to things to TODO.
Thanks for the review.
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 Reviewed-by: 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 b04472e637c8..77162c469079 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -394,7 +394,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 Thu, 2018-11-29 at 18:25 -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.
With PSR short pulse handling implemented, I think we are ready for this.
Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
Cc: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Reviewed-by: 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 b04472e637c8..77162c469079 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -394,7 +394,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;
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 Reviewed-by: 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 77162c469079..c4a8f476eea9 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -649,17 +649,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); }
Source is required to comply to sink SU granularity when DP_PSR2_SU_GRANULARITY_REQUIRED is set in DP_PSR_CAPS, so adding the registers offsets.
v2: Also adding DP_PSR2_SU_Y_GRANULARITY(Rodrigo)
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 | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 047314ce25d6..0e04b2db3dde 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -314,6 +314,10 @@ # 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 */ +#define DP_PSR2_SU_Y_GRANULARITY 0x074 /* 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 Thu, 2018-11-29 at 18:25 -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 registers offsets.
v2: Also adding DP_PSR2_SU_Y_GRANULARITY(Rodrigo)
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 | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 047314ce25d6..0e04b2db3dde 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -314,6 +314,10 @@ # 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 */ +#define DP_PSR2_SU_Y_GRANULARITY 0x074 /* eDP 1.4b */
Definitions above use spaces instead of tabs, so it'd have been good to be consistent. But, there are places in the file where tabs are used too, so will leave it to you if you want to switch.
Verified against eDP spec 1.4b Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
/*
- 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,
Selective updates have a default granularity requirements as stated by eDP spec, so check if HW can match those requirements before enable PSR2.
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 | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index c4a8f476eea9..282ff1bc68a7 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -539,6 +539,18 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, return false; }
+ /* 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 % 4) { + DRM_DEBUG_KMS("PSR2 not enabled, default SU granularity not match\n"); + return false; + } + return true; }
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
Selective updates have a default granularity requirements as stated by eDP spec
Needs reference to the location in the spec.
, so check if HW can match those requirements before enable PSR2.
typo: enabling*
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 | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index c4a8f476eea9..282ff1bc68a7 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -539,6 +539,18 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, return false; }
- /* HW will always send full lines in SU blocks, so X will
s/X/starting X coordinate
* 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.
How about rewriting this as - "HW sends SU blocks of size four scan lines, which means the starting X coordinate and Y granularity requirements will always be met. We only need to validate the SU block width is a multiple of 4."?
*/- if (crtc_hdisplay % 4) {
DRM_DEBUG_KMS("PSR2 not enabled, default SU granularitynot match\n");
"PSR2 not enabled, hdisplay(%d) not multiple of 4\n"
With nits addressed,
Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
return false;- }
- return true;
}
On Fri, 2018-11-30 at 16:37 -0800, Dhinakaran Pandiyan wrote:
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
Selective updates have a default granularity requirements as stated by eDP spec
Needs reference to the location in the spec.
Done
, so check if HW can match those requirements before enable PSR2.
typo: enabling*
Done
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 | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index c4a8f476eea9..282ff1bc68a7 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -539,6 +539,18 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, return false; }
- /* HW will always send full lines in SU blocks, so X will
s/X/starting X coordinate
* 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.How about rewriting this as - "HW sends SU blocks of size four scan lines, which means the starting X coordinate and Y granularity requirements will always be met. We only need to validate the SU block width is a multiple of 4."?
Sounds better, thanks
*/- if (crtc_hdisplay % 4) {
DRM_DEBUG_KMS("PSR2 not enabled, default SU granularitynot match\n");
"PSR2 not enabled, hdisplay(%d) not multiple of 4\n"
Done.
With nits addressed,
Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
Thanks
return false;- }
- return true;
}
According to eDP spec, sink can required specific selective update granularity that source must comply. Here caching the value if required and checking if 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 | 21 ++++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 43ac6873a2bb..0727d8051dd3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -507,6 +507,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 282ff1bc68a7..f9eccaac850a 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -261,6 +261,23 @@ 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; + ssize_t r; + + if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED)) { + /* Returning the default X granularity */ + return 4; + } + + 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 = @@ -315,6 +332,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); } } } @@ -546,7 +565,7 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, * at each 4 lines with height of 4 lines, what eDP states * that sink should support. */ - if (crtc_hdisplay % 4) { + if (crtc_hdisplay % dev_priv->psr.su_x_granularity) { DRM_DEBUG_KMS("PSR2 not enabled, default SU granularity not match\n"); return false; }
On Thu, 2018-11-29 at 18:25 -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 if 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 | 21 ++++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 43ac6873a2bb..0727d8051dd3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -507,6 +507,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 282ff1bc68a7..f9eccaac850a 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -261,6 +261,23 @@ 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;
- ssize_t r;
- if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED))
{
/* Returning the default X granularity */return 4;- }
nit: Braces not needed, you could move the comment a line above.
A value of 0 in this DPCD indicates there is no granularity requirement, why assume 4?
- 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");
Please change this to the warning level that we use elsewhere for aux failures.
If I'm reading the spec correctly, a value of 0 in this DPCD means the sink expects a granularity of 4, so returning 0 would be incorrect.
- return val;
Assume the default value of 4 if aux read fails (after printing an error)
+}
void intel_psr_init_dpcd(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = @@ -315,6 +332,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); } } } @@ -546,7 +565,7 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, * at each 4 lines with height of 4 lines, what eDP states * that sink should support. */
- if (crtc_hdisplay % 4) {
- if (crtc_hdisplay % dev_priv->psr.su_x_granularity) { DRM_DEBUG_KMS("PSR2 not enabled, default SU granularity
not match\n"); return false; }
On Mon, 2018-12-03 at 12:59 -0800, Dhinakaran Pandiyan wrote:
On Thu, 2018-11-29 at 18:25 -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 if 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 | 21 ++++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 43ac6873a2bb..0727d8051dd3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -507,6 +507,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 282ff1bc68a7..f9eccaac850a 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -261,6 +261,23 @@ 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;
- ssize_t r;
- if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED))
{
/* Returning the default X granularity */return 4;- }
nit: Braces not needed, you could move the comment a line above.
A value of 0 in this DPCD indicates there is no granularity requirement, why assume 4?
Like you said bellow, 4 is the default granularity if this is not set.
- 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");Please change this to the warning level that we use elsewhere for aux failures.
So changing to DRM_DEBUG_KMS()
If I'm reading the spec correctly, a value of 0 in this DPCD means the sink expects a granularity of 4, so returning 0 would be incorrect.
- return val;
Assume the default value of 4 if aux read fails (after printing an error)
Done
+}
void intel_psr_init_dpcd(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = @@ -315,6 +332,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); } } } @@ -546,7 +565,7 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, * at each 4 lines with height of 4 lines, what eDP states * that sink should support. */
- if (crtc_hdisplay % 4) {
- if (crtc_hdisplay % dev_priv->psr.su_x_granularity) { DRM_DEBUG_KMS("PSR2 not enabled, default SU granularity
not match\n"); return false; }
On Mon, 2018-12-03 at 14:45 -0800, Souza, Jose wrote:
On Mon, 2018-12-03 at 12:59 -0800, Dhinakaran Pandiyan wrote:
On Thu, 2018-11-29 at 18:25 -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 if 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 | 21 ++++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 43ac6873a2bb..0727d8051dd3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -507,6 +507,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 282ff1bc68a7..f9eccaac850a 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -261,6 +261,23 @@ 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;
- ssize_t r;
- if (!(intel_dp->psr_dpcd[1] & DP_PSR2_SU_GRANULARITY_REQUIRED))
{
/* Returning the default X granularity */return 4;- }
nit: Braces not needed, you could move the comment a line above.
A value of 0 in this DPCD indicates there is no granularity requirement, why assume 4?
Like you said bellow, 4 is the default granularity if this is not set.
The spec states 0 means no granularity requirements, return 1 here?
- 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");Please change this to the warning level that we use elsewhere for aux failures.
So changing to DRM_DEBUG_KMS()
If I'm reading the spec correctly, a value of 0 in this DPCD means the sink expects a granularity of 4, so returning 0 would be incorrect.
- return val;
Assume the default value of 4 if aux read fails (after printing an error)
Done
+}
void intel_psr_init_dpcd(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = @@ -315,6 +332,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); } } } @@ -546,7 +565,7 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, * at each 4 lines with height of 4 lines, what eDP states * that sink should support. */
- if (crtc_hdisplay % 4) {
- if (crtc_hdisplay % dev_priv->psr.su_x_granularity) { DRM_DEBUG_KMS("PSR2 not enabled, default SU granularity
not match\n"); return false; }
On Mon, 2018-12-03 at 15:12 -0800, Pandiyan, Dhinakaran wrote:
On Mon, 2018-12-03 at 14:45 -0800, Souza, Jose wrote:
On Mon, 2018-12-03 at 12:59 -0800, Dhinakaran Pandiyan wrote:
On Thu, 2018-11-29 at 18:25 -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 if 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 | 21 ++++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 43ac6873a2bb..0727d8051dd3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -507,6 +507,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 282ff1bc68a7..f9eccaac850a 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -261,6 +261,23 @@ 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;
- ssize_t r;
- if (!(intel_dp->psr_dpcd[1] &
DP_PSR2_SU_GRANULARITY_REQUIRED)) {
/* Returning the default X granularity */return 4;- }
nit: Braces not needed, you could move the comment a line above.
A value of 0 in this DPCD indicates there is no granularity requirement, why assume 4?
Like you said bellow, 4 is the default granularity if this is not set.
The spec states 0 means no granularity requirements, return 1 here?
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
- r = drm_dp_dpcd_read(&intel_dp->aux,
DP_PSR2_SU_X_GRANULARITY, &val, 2);
- if (r != 2)
DRM_WARN("Unable to readDP_PSR2_SU_X_GRANULARITY\n");
Please change this to the warning level that we use elsewhere for aux failures.
So changing to DRM_DEBUG_KMS()
If I'm reading the spec correctly, a value of 0 in this DPCD means the sink expects a granularity of 4, so returning 0 would be incorrect.
- return val;
Assume the default value of 4 if aux read fails (after printing an error)
Done
+}
void intel_psr_init_dpcd(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = @@ -315,6 +332,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) ; } } } @@ -546,7 +565,7 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp, * at each 4 lines with height of 4 lines, what eDP states * that sink should support. */
- if (crtc_hdisplay % 4) {
- if (crtc_hdisplay % dev_priv->psr.su_x_granularity) { DRM_DEBUG_KMS("PSR2 not enabled, default SU
granularity not match\n"); return false; }
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.
Acked-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com Reviewed-by: 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 f9eccaac850a..0257dbcf9384 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -495,9 +495,6 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1); val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT;
- /* 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;
- Reusing the EDP_PSR2_FRAME_BEFORE_SU_SHIFT in EDP_PSR2_FRAME_BEFORE_SU - Removing unused EDP_PSR2_FRAME_BEFORE_SU_MASK - Adding EDP_PSR2_FRAME_BEFORE_SU_MAX - Adding EDP_PSR2_IDLE_FRAME() - Adding EDP_PSR2_IDLE_FRAME_MAX
In the next patch the new macros will be used.
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/i915_reg.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d3ef97915455..9e46da5032c0 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4216,10 +4216,11 @@ enum { #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_FRAME_BEFORE_SU_MAX 0xf +#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT) #define EDP_PSR2_IDLE_FRAME_SHIFT 0 +#define EDP_PSR2_IDLE_FRAME_MAX 0xf +#define EDP_PSR2_IDLE_FRAME(a) ((a) << EDP_PSR2_IDLE_FRAME_SHIFT)
#define _PSR_EVENT_TRANS_A 0x60848 #define _PSR_EVENT_TRANS_B 0x61848
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
- Reusing the EDP_PSR2_FRAME_BEFORE_SU_SHIFT in
EDP_PSR2_FRAME_BEFORE_SU
- Removing unused EDP_PSR2_FRAME_BEFORE_SU_MASK
- Adding EDP_PSR2_FRAME_BEFORE_SU_MAX
- Adding EDP_PSR2_IDLE_FRAME()
- Adding EDP_PSR2_IDLE_FRAME_MAX
In the next patch the new macros will be used.
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/i915_reg.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d3ef97915455..9e46da5032c0 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4216,10 +4216,11 @@ enum { #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)
_MASKs are useful when we want to read back the register for debugging. So, I'm not fully convinced this is an improvement.
Rodrigo, your thoughts on this?
-#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << 4) -#define EDP_PSR2_IDLE_FRAME_MASK 0xf +#define EDP_PSR2_FRAME_BEFORE_SU_MAX 0xf +#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT) #define EDP_PSR2_IDLE_FRAME_SHIFT 0 +#define EDP_PSR2_IDLE_FRAME_MAX 0xf
Not sure if this is better than re-using _MASK here. I'm sure there are places in the driver where we already do that.
+#define EDP_PSR2_IDLE_FRAME(a) ((a) << EDP_PSR2_IDLE_FRAME_SHIFT)
#define _PSR_EVENT_TRANS_A 0x60848 #define _PSR_EVENT_TRANS_B 0x61848
On Mon, Dec 03, 2018 at 03:03:52PM -0800, Dhinakaran Pandiyan wrote:
On Thu, 2018-11-29 at 18:25 -0800, José Roberto de Souza wrote:
- Reusing the EDP_PSR2_FRAME_BEFORE_SU_SHIFT in
EDP_PSR2_FRAME_BEFORE_SU
- Removing unused EDP_PSR2_FRAME_BEFORE_SU_MASK
- Adding EDP_PSR2_FRAME_BEFORE_SU_MAX
- Adding EDP_PSR2_IDLE_FRAME()
- Adding EDP_PSR2_IDLE_FRAME_MAX
In the next patch the new macros will be used.
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/i915_reg.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d3ef97915455..9e46da5032c0 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4216,10 +4216,11 @@ enum { #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)
_MASKs are useful when we want to read back the register for debugging. So, I'm not fully convinced this is an improvement.
Rodrigo, your thoughts on this?
I agree with DK.
But I also don't mind removing if we aren't using.
-#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << 4) -#define EDP_PSR2_IDLE_FRAME_MASK 0xf +#define EDP_PSR2_FRAME_BEFORE_SU_MAX 0xf +#define EDP_PSR2_FRAME_BEFORE_SU(a) ((a) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT) #define EDP_PSR2_IDLE_FRAME_SHIFT 0 +#define EDP_PSR2_IDLE_FRAME_MAX 0xf
Not sure if this is better than re-using _MASK here. I'm sure there are places in the driver where we already do that.
I think it depends on the use actually...
+#define EDP_PSR2_IDLE_FRAME(a) ((a) << EDP_PSR2_IDLE_FRAME_SHIFT)
#define _PSR_EVENT_TRANS_A 0x60848 #define _PSR_EVENT_TRANS_B 0x61848
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
EDP_PSR2_FRAME_BEFORE_SU() is the number of frames that PSR2 HW will wait before enter in PSR2 activation state, important to note here is that it will wait for X frames not X idle frames. So lets reuse the previous approch to get the maximum number of frames between 6 and sink_sync_latency to enter in PSR2 activation state and just remove the VBT idle_frames.
And EDP_PSR2_FRAME_BEFORE_SU() is the number of idle frames that PSR2 HW will wait before enter in PSR2 deep sleep when PSR2 is active. Important note here is that HW will need to go to PSR2 idle state every time it exits PSR2 deep sleep, so avoid as much as possible deep sleep will provide in overal more power savings as PSR2 sleep will save some power as memory will not be read in the idle frames and screen will be partialy updated without exit PSR2.
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, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 0257dbcf9384..36c2eb27ed8d 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -489,18 +489,20 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
/* 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, so sink_sync_latency + 1. */ - int idle_frames = max(6, dev_priv->vbt.psr.idle_frames); + val = max(6, dev_priv->psr.sink_sync_latency + 1); + val = min_t(u32, val, EDP_PSR2_FRAME_BEFORE_SU_MAX); + val = EDP_PSR2_FRAME_BEFORE_SU(val);
- idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1); - val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT; + /* Avoid deep sleep as much as possible to avoid PSR2 idle state */ + val |= EDP_PSR2_IDLE_FRAME(EDP_PSR2_IDLE_FRAME_MAX);
val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE; 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); - 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 Thu, 2018-11-29 at 18:25 -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.
v2: Renamed DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED to DP_DPCD_QUIRK_NO_PSR (Ville)
Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW) Cc: Ville Syrjälä ville.syrjala@linux.intel.com 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 2d6c491a0542..b00fd5ced0a0 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_NO_PSR) } };
#undef OUI diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 2084784f320d..40ca6cc43cc4 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -278,6 +278,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_NO_PSR)) {
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 5736c942c85b..047314ce25d6 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1365,6 +1365,7 @@ enum drm_dp_quirk { * to 16 bits. So will give a constant value (0x8000) for compatability. */ DP_DPCD_QUIRK_CONSTANT_N,
nit: Documentation missing here. I guess we need something along the lines of "PSR not supported" without referring to the specific DP device. With that, Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
- DP_DPCD_QUIRK_NO_PSR,
};
/**
On Fri, 2018-11-30 at 15:35 -0800, Dhinakaran Pandiyan wrote:
On Thu, 2018-11-29 at 18:25 -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.
v2: Renamed DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED to DP_DPCD_QUIRK_NO_PSR (Ville)
Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW) Cc: Ville Syrjälä ville.syrjala@linux.intel.com 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 2d6c491a0542..b00fd5ced0a0 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_NO_PSR) } };
#undef OUI diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 2084784f320d..40ca6cc43cc4 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -278,6 +278,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_NO_PSR)) {
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 5736c942c85b..047314ce25d6 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1365,6 +1365,7 @@ enum drm_dp_quirk { * to 16 bits. So will give a constant value (0x8000) for compatability. */ DP_DPCD_QUIRK_CONSTANT_N,
nit: Documentation missing here. I guess we need something along the lines of "PSR not supported" without referring to the specific DP device. With that, Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
Adding here:
/** * @DP_DPCD_QUIRK_NO_PSR * * The device don't properly support PSR even if reports that it * supports or driver still need to implement proper handling for such * device. */
- DP_DPCD_QUIRK_NO_PSR,
};
/**
On Mon, 2018-12-03 at 12:14 -0800, Souza, Jose wrote:
On Fri, 2018-11-30 at 15:35 -0800, Dhinakaran Pandiyan wrote:
On Thu, 2018-11-29 at 18:25 -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.
v2: Renamed DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED to DP_DPCD_QUIRK_NO_PSR (Ville)
Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW) Cc: Ville Syrjälä ville.syrjala@linux.intel.com 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 2d6c491a0542..b00fd5ced0a0 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_NO_PSR) } };
#undef OUI diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 2084784f320d..40ca6cc43cc4 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -278,6 +278,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_NO_PSR)) {
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 5736c942c85b..047314ce25d6 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1365,6 +1365,7 @@ enum drm_dp_quirk { * to 16 bits. So will give a constant value (0x8000) for compatability. */ DP_DPCD_QUIRK_CONSTANT_N,
nit: Documentation missing here. I guess we need something along the lines of "PSR not supported" without referring to the specific DP device. With that, Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
Adding here:
/** * @DP_DPCD_QUIRK_NO_PSR * * The device don't properly support PSR even if reports that
nit: "device does not support"
it * supports or driver still need to implement proper handling
^needs
for such * device. */
- DP_DPCD_QUIRK_NO_PSR,
};
/**
On Mon, 2018-12-03 at 14:45 -0800, Dhinakaran Pandiyan wrote:
On Mon, 2018-12-03 at 12:14 -0800, Souza, Jose wrote:
On Fri, 2018-11-30 at 15:35 -0800, Dhinakaran Pandiyan wrote:
On Thu, 2018-11-29 at 18:25 -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.
v2: Renamed DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED to DP_DPCD_QUIRK_NO_PSR (Ville)
Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW) Cc: Ville Syrjälä ville.syrjala@linux.intel.com 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 2d6c491a0542..b00fd5ced0a0 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_NO_PSR) } };
#undef OUI diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 2084784f320d..40ca6cc43cc4 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -278,6 +278,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_NO_PSR)) {
DRM_DEBUG_KMS("PSR support not currentlyavailable 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 5736c942c85b..047314ce25d6 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1365,6 +1365,7 @@ enum drm_dp_quirk { * to 16 bits. So will give a constant value (0x8000) for compatability. */ DP_DPCD_QUIRK_CONSTANT_N,
nit: Documentation missing here. I guess we need something along the lines of "PSR not supported" without referring to the specific DP device. With that, Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
Adding here:
/** * @DP_DPCD_QUIRK_NO_PSR * * The device don't properly support PSR even if reports that
nit: "device does not support"
Thanks
it * supports or driver still need to implement proper handling
^needsfor such * device. */
- DP_DPCD_QUIRK_NO_PSR,
};
/**
On Fri, 2018-11-30 at 15:35 -0800, Dhinakaran Pandiyan wrote:
On Thu, 2018-11-29 at 18:25 -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.
v2: Renamed DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED to DP_DPCD_QUIRK_NO_PSR (Ville)
Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW) Cc: Ville Syrjälä ville.syrjala@linux.intel.com 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 2d6c491a0542..b00fd5ced0a0 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_NO_PSR) } };
#undef OUI diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 2084784f320d..40ca6cc43cc4 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -278,6 +278,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_NO_PSR)) {
DRM_DEBUG_KMS("PSR support not currently available forthis panel\n");
return;- }
Another nitpick: While you make other changes, please also move this above the power state check. Checking for power state control is not very useful if we are never going to enable PSR on this panel.
- 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 5736c942c85b..047314ce25d6 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1365,6 +1365,7 @@ enum drm_dp_quirk { * to 16 bits. So will give a constant value (0x8000) for compatability. */ DP_DPCD_QUIRK_CONSTANT_N,
nit: Documentation missing here. I guess we need something along the lines of "PSR not supported" without referring to the specific DP device. With that, Reviewed-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com
- DP_DPCD_QUIRK_NO_PSR,
};
/**
On Thu, 29 Nov 2018, José Roberto de Souza jose.souza@intel.com wrote:
i915 yet don't support PSR in Apple panels, so lets keep it disabled while we work on that.
v2: Renamed DP_DPCD_QUIRK_PSR_NOT_CURRENTLY_SUPPORTED to DP_DPCD_QUIRK_NO_PSR (Ville)
Fixes: 598c6cfe0690 (drm/i915/psr: Enable PSR1 on gen-9+ HW) Cc: Ville Syrjälä ville.syrjala@linux.intel.com 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 2d6c491a0542..b00fd5ced0a0 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 */
nitpick "panels needs"
- { OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_NO_PSR) }
};
#undef OUI diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 2084784f320d..40ca6cc43cc4 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -278,6 +278,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_NO_PSR)) {
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 5736c942c85b..047314ce25d6 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1365,6 +1365,7 @@ enum drm_dp_quirk { * to 16 bits. So will give a constant value (0x8000) for compatability. */ DP_DPCD_QUIRK_CONSTANT_N,
Please add a proper comment here, similar to above DP_DPCD_QUIRK_CONSTANT_N.
BR, Jani.
- DP_DPCD_QUIRK_NO_PSR,
};
/**
dri-devel@lists.freedesktop.org
-
Dhinakaran Pandiyan -
Jani Nikula -
José Roberto de Souza -
Pandiyan, Dhinakaran
-
Rodrigo Vivi -
Souza, Jose