Re: [PATCH 2/2] drm/i915: Implement vblank synchronized MBUS join changes

[Paz Zcharya <pazz@chromium.org>]

On Wed, Feb 28, 2024 at 10:02:13AM +0200, Stanislav Lisovskiy wrote:
> Currently we can't change MBUS join status without doing a modeset,
> because we are lacking mechanism to synchronize those with vblank.
> However then this means that we can't do a fastset, if there is a need
> to change MBUS join state. Fix that by implementing such change.
> We already call correspondent check and update at pre_plane dbuf update,
> so the only thing left is to have a non-modeset version of that.
> If active pipes stay the same then fastset is possible and only MBUS
> join state/ddb allocation updates would be committed.
> 
> v2: Implement additional changes according to BSpec.
>     Vblank wait is needed after MBus/Dbuf programming in case if
>     no modeset is done and we are switching from single to multiple
>     displays, i.e mbus join state switches from "joined" to  "non-joined"
>     state. Otherwise vblank wait is not needed according to spec.
> 
> v3: Split mbus and dbox programming into to pre/post plane update parts,
>     how it should be done according to BSpec.
> 
> v4: - Place "single display to multiple displays scenario" MBUS/DBOX programming
>       after DDB reallocation, but before crtc enabling(that is where is has
>       to be according to spec).
>     - Check if crtc is still active, not only the old state.
>     - Do a vblank wait if MBUX DBOX register was modified.
>     - And of course do vblank wait only if crtc was active.
>     - Do vblank wait only if we are not doing a modeset, if we are doing
>       something before *commit_modeset_enables, because all crtcs might be
>       disabled at this moment, so we will get WARN if try waiting for vblank
>       then.
>     - Still getting FIFO underrun so try waiting for vblank in pre_plane update
>       as well.
>     - Write also pipe that we need to sync with to MBUS_CTL register.
> 
> v5: - Do vblank wait only for the first pipe, if mbus is joined
>     - Check also if new/old_dbuf_state is not NULL, before getting single pipe
>       and active pipes.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Thank you for this patch, Stanislav!
We tested it on a MTL-U based Chromebook (Screebo),
using different configurations (eDP, eDP + HDMI, HDMI, etc.), and
it worked well -- joined the mbus + no visual issues or i915 errors.

Tested-by: Paz Zcharya <pazz@chromium.org>

Just a small note, checkpatch.pl is complaining about a few things.
I assume you probably saw it, but flagging just in case.