[Intel-gfx] [PATCH] drm/i915: HSW/BDW PSR Set idle_frames=2 since sometimes it can be off by 1.

[Rodrigo Vivi]

On Mon, Nov 3, 2014 at 9:25 AM, Runyan, Arthur J
<arthur.j.runyan at intel.com> wrote:
>>From: Daniel Vetter
>>
>>On Thu, Oct 30, 2014 at 05:35:55AM -0700, Rodrigo Vivi wrote:
>>> It was identified that in some cases when moving cursor Hardware can do
>>> mistake with idle_frame count. So Spec is being updated to use
>>> 2 as minimum idle_frames.
>>>
>>> Reference:
>>https://hsdhsw.intel.com/hsd/haswell_platform/default.aspx#sighting/default.as
>>px?sighting_id=4394433
>>> Cc: Arthur Runyan <arthur.j.runyan at intel.com>
>>> Cc: Jani Nikula <jani.nikula at intel.com>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>
>>Since we have full-blown sw frontbuffer tracking: Do we care?
>>
>>I.e. is the effect of idle_frames = 1 that the hw will go bananas (bad, we
>>want this patch) or that it might miss a cursor movement (no problem, the
>>kernel will catch it anyway)?

Even with the frontbuffer tracking HW still controls when to really
entry PSR based on this count. So I prefer to be on the safe side and
do the propper setup. If we
I had faced some cases here where even with propper frontbuffer
tracking idle_frames=2 let it more stable. I had also sent this same
patch in the past. But at that point I didn't know we had this miss
calculation issue on HW.

>>
>
> The behavior will depend on the panel.  Going bananas is a possibility.
> It also looks like VBT is being used to indicate that some panels have minimum idle frame requirements, so you really should be using VBT + 1 to ensure the hardware always outputs at least the VBT specified number of frames.

After think a bit I still think this is the patch to go to -fixes.
On the reorg series with 4 patches I just sent I add VBT parse and get
this idle_frame from there. But I'd prefer that regorg going to dinq
and this patch here to -fixes.

>
>
>>Cheers, Daniel
>>
>>> ---
>>>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 47e9d71..8cfbbc2 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -2178,7 +2178,7 @@ static void intel_edp_psr_enable_source(struct
>>intel_dp *intel_dp)
>>>      struct drm_device *dev = dig_port->base.base.dev;
>>>      struct drm_i915_private *dev_priv = dev->dev_private;
>>>      uint32_t max_sleep_time = 0x1f;
>>> -    uint32_t idle_frames = 1;
>>> +    uint32_t idle_frames = 2; /* 2 is the minimum allowed */
>>>      uint32_t val = 0x0;
>>>      const uint32_t link_entry_time =
>>EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
>>>      bool only_standby = false;
>>> --
>>> 1.9.3
>>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Thanks,
Rodrigo.

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