On Fri, Feb 7, 2014 at 5:14 PM, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > On Fri, Feb 07, 2014 at 04:09:47PM -0200, Rodrigo Vivi wrote: >> As pointed out by Ville we were using inverted logic here. >> According to spec: >> For link standby mode set 170h[1] = 1. >> For full link disabling set 170h[1] = 0. >> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 50381f7..4ecda72 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -1661,12 +1661,12 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp) >> /* Enable PSR in sink */ >> if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) >> intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, >> - DP_PSR_ENABLE & >> - ~DP_PSR_MAIN_LINK_ACTIVE); >> - else >> - intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, >> DP_PSR_ENABLE | >> DP_PSR_MAIN_LINK_ACTIVE); >> + else >> + intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, >> + DP_PSR_ENABLE & >> + ~DP_PSR_MAIN_LINK_ACTIVE); > > I think this is now the opposite of what we want. Ie. if the sink > doesn't require training we should disable the main link. Otherwise we > should keep the main link on, and that way avoid the need to train on > PSR exit. To be honest, I think I agree with you, but apparently performance counter inc improved on this way... > > Actually I'm not sure that's really what we want. I think the hardware > can do the training on its own, so in theory we should just always disable > the main link. Although the PM guide has a comment indicating that the > hardware training can fail, in which case software must repeat it. We > don't have code to do that, so I guess leaving the main link on is the > safer option. Would be nice to have a comment in the code stating as > much, if this is indeed the reason why the code was written this way. I'll do a carefull check and local tests and send new version fixed or with good comments. > >> >> /* Setup AUX registers */ >> I915_WRITE(EDP_PSR_AUX_DATA1(dev), EDP_PSR_DPCD_COMMAND); >> -- >> 1.7.11.7 > > -- > Ville Syrjälä > Intel OTC -- Rodrigo Vivi Blog: http://blog.vivi.eng.br _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
