renesas/master bisection: igt-kms-rockchip.kms_vblank.pipe-A-wait-forked on rk3399-gru-kevin

[Brian Norris]

On Wed, Jan 04, 2023 at 10:11:46AM +0100, Michel Dänzer wrote:
> On 1/4/23 03:11, Brian Norris wrote:
> > On Tue, Jan 03, 2023 at 07:04:00PM +0100, Michel Dänzer wrote:
> >> On 12/21/22 23:02, Brian Norris wrote:
> > 
> >>> 3. leave vblank enabled even in the presence of PSR
> > 
> > I'm leaning toward this.
> 
> If this means vblank interrupts will arrive as expected even while in PSR, that may be the ideal solution indeed.

Yes. And I think I have a working patchset for this now. It passes all
the igt-gpu-tools/kms_vblank tests for me now, and I don't think it
causes any other issues. I'll send it out shortly, after a bit more
testing.

Side note: I believe this vblank-disabled issue actually came in as an
upstream regression at commit 6c836d965bad ("drm/rockchip: Use the
helpers for PSR"); before that, we were doing this very differently, and
didn't touch vblank at all for PSR, similar to the "downstream" stuff I
mentioned earlier.

> >> 5. Go/stay out of PSR while vblank interrupts are enabled (probably want to make sure vblankoffdelay is set up such that vblank interrupts are disabled ASAP)
> > 
> > That seems not extremely nice, to waste power. Based on the earlier
> > downstream implementation (which left vblank interrupts enabled), I'd
> > wager there's a much larger power win from PSR (on the display-transport
> > and memory-bandwidth costs), relative to the power cost of vblank
> > interrupts.
> > 
> > Also, messing with vblankoffdelay sounds likely to trigger the races
> > mentioned in the drm_vblank.c kerneldoc.
> 
> Not sure how likely that is, quite a few drivers are setting dev->vblank_disable_immediate = true.
> 
> With that, vblank interrupts should generally be enabled only while there are screen updates as well[0], in which case PSR shouldn't be possible anyway.
> 
> [0] There may be user space which uses the vblank ioctls even while there are no screen updates though, which would prevent PSR in this case.

OK. I'm just reading docs here; definitely not an expert.

> >>> [1] Or is it? I don't really know the DRM_IOCTL_WAIT_VBLANK ABI
> >>>     contract in the presence of PSR, but I believe the upstream PSR
> >>>     support has always worked this way. I could imagine
> >>>     DRM_IOCTL_WAIT_VBLANK users might not love seeing EINVAL here
> >>>     though.
> >>
> >> Yeah, that's pretty likely to cause issues with existing real-world user space.
> > 
> > OK. Any hints as to what kind of user space uses this?
> 
> I don't have any specific example, just thinking about how user space could respond to the vblank ioctls returning an error, and it would seem to be generally either of:
> 
> * Just run non-throttled, which might negate any energy savings from PSR
> * Fail to work altogether
> 
> 
> > I'm in part simply curious, but I'm also wondering what the
> > error-handling and reliability (e.g., what if vblanks don't come?)
> > expectations might be in practice.
> 
> If vblank events never come, user space might freeze.

Thanks. If my patchset works out like I expect, this should no longer be
noticeable to user space, so I don't really have to test out your
educated guesses :)

Thanks for the idea-bouncing.

Brian