Re: [PATCH v2 01/17] drm/i915: Update pipes in reverse order for bigjoiner

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

On Fri, Apr 05, 2024 at 01:42:49PM +0000, Kulkarni, Vandita wrote:
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> > Syrjala
> > Sent: Friday, April 5, 2024 3:04 AM
> > To: intel-gfx@lists.freedesktop.org
> > Subject: [PATCH v2 01/17] drm/i915: Update pipes in reverse order for
> > bigjoiner
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > With bigjoiner the master crtc is the one that will send out the uapi
> > event/etc. We want that to happen after all the slaves are done, so let's try
> > to do the commits in reverse order so that the master comes last.
> > 
> > Even worse, the modeset helper will simply complete the commit on the
> > slave pipe immediately as it consider the crtc to be inactive (it can't see our
> > crtc_state->hw.active/etc.).
> > 
> > With regular sync updates this generally doesn't matter all that much as the
> > slave pipe should typically finish its work during the same frame as the
> > master pipe. However in case the slave pipe's commit slips into the next
> > frame we end up in a bit of trouble. This is most visible with either async flips
> > (currently disabled with bigjoiner exactly for this reason), and DSB gamma
> > updates. With DSB the problem happens because the DSB itself will wait until
> > the next start vblank before starting to execute. So if the master pipe already
> > finished its commit and the DSB on the slave pipe is still waiting for the next
> > vblank we will assume the DSB as gotten stuck and terminate it.
> > 
> > Reversing the commit order should ameliarate this for the most part as the
> > master pipe is guaranteed to start its commit after the slave pipe started. The
> > one thing that can still screw us over is the fact that we aren't necessarily
> > going to commit the pipes in the reverse order as the actual order is dictated
> > by the DDB overlap avoidance.
> > But that can only happen while other pipes are being enabled/disabled, and
> > so in the normal steady state we should be safe.
> > 
> > The full fix will involve making the commit machinery aware of the slave
> > pipes and not finish their commits prematurely. But that will involve a bit
> > more work than this. And this commit order reversal will still be beneficial to
> > avoid userspace getting an -EBUSY from the following page flip if the second
> > pipe's commit does stretch into the next frame.
> > 
> 
> LGTM.
> Reviewed-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> 
> I had just one query though,
> Will there be a case where we can get vblank between slave update and master update,
> if so do you think there will be any problem due to that?

It can happen, in which case you may observe a vertical tear.
Since we've disabled all the fancy transcoder level stuff 
(vrr/lrr/etc.) that should be the worst of it.

-- 
Ville Syrjälä
Intel