[Intel-gfx] [PATCH 3/7] drm/i915: PSR: deprecate link_standby support for core platforms.

Rodrigo Vivi rodrigo.vivi at gmail.com
Mon Mar 2 12:27:57 PST 2015


On Mon, Mar 2, 2015 at 3:41 AM, Jindal, Sonika <sonika.jindal at intel.com> wrote:
>
>
> On 2/28/2015 6:56 AM, Rodrigo Vivi wrote:
>>
>> On Haswell and Broadwell with link in standby when exit event happens
>> between vblank and VSC packet, PSR exit on panel but DPA transmitter
>> still sends black pixel. hen this condition hits, panel will
>> intermittently
>> display black frame.
>>
>> The known W/A for this case involve the of single_frame update
>> that isn't supported on Haswell and to be supported on Broadwell
>> 3 other workarounds would be required. So it is better and safe to
>> just deprecate link_standby for now.
>>
>> Also, link fully off saves more power than link_standby and afwk
>> no OEM is requesting link standby on VBT. There is no reason for that.
>>
>> For Skylake let's just consider it behaves like Broadwell until
>> we prove otherwise.
>>
> Hmm, on SKL I am able to read the sink crc only when I keep this
> link_standby true. Not sure if we'l have to pull it back or need to look for
> another solution.

I faced this on BDW as well, but after reworking the test case I could
make it read crc on BDW with link fully off.

Could you please try this on SKL? kms_psr_sink_crc from psr-crc-skl
branch at: http://cgit.freedesktop.org/~vivijim/intel-gpu-tools/log/?h=psr-crc-skl

>
>> Reference: HSD: bdwgfx/1912559
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c |  3 ---
>>   drivers/gpu/drm/i915/i915_drv.h     |  1 -
>>   drivers/gpu/drm/i915/intel_psr.c    | 25 ++++++++-----------------
>>   3 files changed, 8 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 94b3984..756bf4e 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2281,9 +2281,6 @@ static int i915_edp_psr_status(struct seq_file *m,
>> void *data)
>>                 }
>>         seq_puts(m, "\n");
>>
>> -       seq_printf(m, "Link standby: %s\n",
>> -                  yesno((bool)dev_priv->psr.link_standby));
>> -
>>         /* CHV PSR has no kind of performance counter */
>>         if (HAS_PSR(dev) && HAS_DDI(dev)) {
>>                 psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) &
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index a1dd8bc..dfd044f 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -863,7 +863,6 @@ struct i915_psr {
>>         bool active;
>>         struct delayed_work work;
>>         unsigned busy_frontbuffer_bits;
>> -       bool link_standby;
>>   };
>>
>>   enum intel_pch {
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c
>> b/drivers/gpu/drm/i915/intel_psr.c
>> index 6c8e9e0..d2ff87d 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -157,14 +157,6 @@ static void hsw_psr_enable_sink(struct intel_dp
>> *intel_dp)
>>
>>         aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
>>
>> -       /* Enable PSR in sink */
>> -       if (dev_priv->psr.link_standby)
>> -               drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>> -                                  DP_PSR_ENABLE |
>> DP_PSR_MAIN_LINK_ACTIVE);
>> -       else
>> -               drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>> -                                  DP_PSR_ENABLE &
>> ~DP_PSR_MAIN_LINK_ACTIVE);
>> -
>>         aux_data_reg = (INTEL_INFO(dev)->gen >= 9) ?
>>                                 DPA_AUX_CH_DATA1 : EDP_PSR_AUX_DATA1(dev);
>>         aux_ctl_reg = (INTEL_INFO(dev)->gen >= 9) ?
>> @@ -193,6 +185,8 @@ static void hsw_psr_enable_sink(struct intel_dp
>> *intel_dp)
>>                    (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
>>                    (aux_clock_divider <<
>> DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
>>         }
>> +
>> +       drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, DP_PSR_ENABLE);
>>   }
>>
>>   static void vlv_psr_enable_source(struct intel_dp *intel_dp)
>> @@ -242,9 +236,6 @@ static void hsw_psr_enable_source(struct intel_dp
>> *intel_dp)
>>         uint32_t val = 0x0;
>>         const uint32_t link_entry_time =
>> EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
>>
>> -       if (dev_priv->psr.link_standby)
>> -               val |= EDP_PSR_LINK_STANDBY;
>> -
>>         if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
>>                 val |= EDP_PSR_TP2_TP3_TIME_0us;
>>                 val |= EDP_PSR_TP1_TIME_0us;
>> @@ -295,6 +286,12 @@ static bool intel_psr_match_conditions(struct
>> intel_dp *intel_dp)
>>                 return false;
>>         }
>>
>> +       if (!IS_VALLEYVIEW(dev) && ((dev_priv->vbt.psr.full_link) ||
>> +                                   (dig_port->port != PORT_A))){
>> +               DRM_DEBUG_KMS("PSR condition failed: Link Standby
>> requested/needed but not supported on this platform\n");
>> +               return false;
>> +       }
>> +
>>         dev_priv->psr.source_ok = true;
>>         return true;
>>   }
>> @@ -353,12 +350,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>>         if (!intel_psr_match_conditions(intel_dp))
>>                 goto unlock;
>>
>> -       /* First we check VBT, but we must respect sink and source
>> -        * known restrictions */
>> -       dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link;
>> -       if (IS_BROADWELL(dev) && intel_dig_port->port != PORT_A)
>> -               dev_priv->psr.link_standby = true;
>> -
>>         dev_priv->psr.busy_frontbuffer_bits = 0;
>>
>>         if (HAS_DDI(dev)) {
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br


More information about the Intel-gfx mailing list