On Mon, Nov 3, 2014 at 9:25 AM, Runyan, Arthur J <arthur.j.runyan@xxxxxxxxx> 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@xxxxxxxxx> >>> Cc: Jani Nikula <jani.nikula@xxxxxxxxx> >>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> >> >>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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx Thanks, Rodrigo. -- Rodrigo Vivi Blog: http://blog.vivi.eng.br _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
