Re: [PATCH i-g-t 2/2] tests/kms_setmode: Disable CPU deep sleep for accurate vblank timestamps

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, May 27, 2026 at 03:49:36PM +0000, Jason-JH Lin (林睿祥) wrote:
> On Mon, 2026-05-25 at 09:07 -0400, Mark Yacoub wrote:
> > 
> > 
> > On Sat, May 23, 2026 at 2:02 AM Jason-JH Lin (林睿祥)
> > <Jason-JH.Lin@mediatek.com> wrote:
> > > > 
> > > [snip]
> > > 
> > > > > Hi Ville,
> > > > > 
> > > > > Thank you for the feedback.
> > > > > 
> > > > > You're right - the patch was purely to pass IGT tests, not to
> > > > > fix a
> > > > > real problem (although we haven't encountered any issue within
> > > > > randomly ~200us jitter on our current platform).
> > > > > 
> > > > > I've investigated the real-world impact of this ~200us
> > > > > timestamp
> > > > > jitter. In our current Android SurfaceFlinger implementation,
> > > > > real-world usage is stable with CPU deep sleep enabled.
> > > > > It seems this issue only appears in IGT single-CRTC testing
> > > > > where
> > > > > CPU
> > > > > enters deep sleep during idle periods between frames.
> > > > > Therefore, we prefer not to disable CPU deep sleep system-wide
> > > > > in
> > > > > kernel due to power consumption concerns.
> > > > > 
> > > > > However, I'd like to understand how the 1-scanline standard was
> > > > > originally defined for this test. Do real-world use cases
> > > > > actually
> > > > > require this level of precision?
> > > > 
> > > > I don't have specific examples, but people cared about precision
> > > > enough to add the whole timestamp correction thingy into the
> > > > kernel.
> > > > And in i915 we also sometimes use the timestamps to cook up a
> > > > frame
> > > > counter (when the hardware lacks one) and I wouldn't want to do
> > > > that
> > > > without adjusting the timestamps based on the scanout position.
> > > > 
> > > > Anyways, since this is usually implemented via a scanline counter
> > > > (sometimes even a pixel counter) that's the kind of accuracy one
> > > > would expect to get, and so that's what one tests for to make
> > > > sure
> > > > it's working correctly.
> > > > 
> > > > > 
> > > > > Not all platforms support or implement get_scanout_position() -
> > > > > could the standard be relaxed for such platforms?
> > > > 
> > > > If you don't implement accurate timestamps then the test is
> > > > expected to fail. I see nothing wrong in that.
> > > > 
> > > > Relaxing the test doesn't make sense to me because then it can
> > > > no longer tell you whether your timestamps are accurate or not.
> > > > Although I guess you could have another similar test that
> > > > makes sure that the timestamps are at least somewhat sane,
> > > > even if not very accurate.
> > > > 
> > > 
> > > Hi Ville,
> > > 
> > > I understand your point, and I agree that implementing
> > > get_scanout_position() is the proper long-term fix.
> > > 
> > > However, I'd like to explain our reasoning for this patch.
> > > The purpose of this test is to verify the accuracy of the kernel
> > > vblank
> > > timestamp itself. We have identified that disabling CPU idle
> > > eliminates
> > > the jitter and the test passes, which confirms that our timestamp
> > > implementation is correct - the ~200us jitter is caused by CPU idle
> > > wakeup latency as an external interference, not by the timestamp
> > > mechanism itself.
> > > 
> > > Therefore, before we complete the get_scanout_position()
> > > implementation, we believe this patch still has value - it allows
> > > the
> > > test to effectively verify our timestamp accuracy by removing the
> > > external CPU idle interference, while also preventing other
> > > potential
> > > issues from affecting timestamp precision on MediaTek platforms.
> > > 
> > 
> > here is my 2c: 
> > As this test uncovered another bug (external CPU Idle interference),
> > it MUST not be removed. Otherwise, we hide a bug, even if it wasn't
> > the intended test.
> > Therefore, if you decide with Ville that there is value in your
> > workaround, it must be ANOTHER test.
> > Test#1 shows that you have an accurate timestamp without interference
> > (this shall pass for MTK)
> > another test showing that CPU idleness with throw off the accuracy of
> > the timestamp (will fail)
> > This will give you 2 results: accurate timestamps that deviates
> > sometime. It's up to you and the platform owner to decide if this is
> > an acceptable behavior. 
> > But, we shall not change a test to hide a bug.
> > 
> > -Mark
> > > 
> 
> Hi Ville and Mark,
> 
> Thank you both for the valuable feedback.
> I investigated implementing get_scanout_position() on MediaTek, and I
> found a hardware limitation that prevents us from achieving accurate
> vblank timestamps on DSI.
> Unlike Intel, which typically has an output-side scanline counter that
> tracks the real panel scanout position across the full frame (including
> blanking), our DSI provides an input-side line counter. It only eflects
> the DSI input processing position during the active region, and it
> becomes non-informative during vblank (often reading as 0), so we
> cannot determine the exact scanline position within the vblank
> interval.
> 
> A simplified diagram of the difference:
> 
> DISP -> DSI -> Panel
>      |      |
>      |      +-- Intel scanline counter (OUTPUT side)
>      |      e.g. PIPEDSL
>      |      - Counts the real panel scanout position
>      |      - Covers active + vblank (0 ~ vtotal-1)
>      |      - Still meaningful during vblank
>      |
>      +-- MediaTek DSI line counter (INPUT side)
>      e.g. DSI_INPUT_DEBUG
>      - Counts DSI input processing position
>      - Only covers active region (0 ~ vdisplay-1)
>      - During vblank there is no input data, so the
>        counter may stop/reset and return as 0
>      - Therefore it cannot indicate the exact position
>        within the vblank interval (vdisplay ~ vtotal-1)
> 
> I attempted a get_scanout_position() based approach, but due to the
> above limitation, the 1-scanline accuracy requirement cannot be met on
> DSI (I got +-140 ~ 280us jitters). As a result, this test cannot pass
> on MTK DSI unless we eliminate CPU idle wakeup latency effects.
> 
> Regarding Mark's suggestion to split into 2 tests:
> In our case, a “Test #2” that expects accurate timestamps with CPU idle
> enabled would effectively be a permanent fail (or would have to be
> skipped) on DSI due to the hardware counter limitation.
> Therefore, I believe keeping a single test with a clear comment on the
> limitation and the disabling CPU idle workaround is more practical.
> 
> Given this hardware limitation on DSI, would you consider the
> workaround justified for MTK DSI platforms? Or do you have any
> suggestions/ideas on how we could avoid or mitigate this limitation
> and still provide accurate timestamps?

If your hardware can't pass the accurate timestamp test
then just let it fail.

-- 
Ville Syrjälä
Intel