[Intel-gfx] [Intel-xe] [PATCH 3/4] drm/i915/lnl: support FBC on any plane

[Ville Syrjälä]

On Tue, Aug 29, 2023 at 09:04:28AM -0700, Matt Roper wrote:
> On Tue, Aug 29, 2023 at 06:50:02AM -0700, Govindapillai, Vinod wrote:
> > On Mon, 2023-08-28 at 17:16 -0700, Matt Roper wrote:
> > > On Mon, Aug 28, 2023 at 09:20:34AM +0300, Vinod Govindapillai wrote:
> > > > In LNL onwards, FBC can be associated to the first three planes.
> > > 
> > > The title of this patch shouldn't say "any plane" when the reality is
> > > that only the first three support FBC (the upper two do not).
> > > 
> > > > The FBC will be enabled for first FBC capable visible plane
> > > > until the userspace can select one of these FBC capable plane
> > > > explicitly
> > > 
> > > Even if we add new Intel-specific uapi to select this explicitly, is any
> > > userspace actually going to use it?  Would it make more sense to try to
> > > come up with a heuristic to guess which plane would benefit the most and
> > > switch to that automatically without userspace needing to be involved in
> > > the decision?  For that matter, do we have a real-world use case lined
> > > up where we can see that switching FBC to a higher plane is beneficial?
> > > Even if the hardware suddenly has this capability, it isn't necessarily
> > > worth adding the extra complexity to the driver if we don't expect to
> > > get real-world benefit from it.
> > > 
> > > BTW, I'm not super familiar with all the FBC-specific details, but it
> > > feels like if we do go forward with this, the decision to select a
> > > specific plane for FBC usage should be handled more deliberately during
> > > the atomic check phase.  Right now it doesn't seem like we're really
> > > making a firm decision on which plane to use, and we're probably not
> > > handling all the cases where the register needs to be reprogrammed
> > > if/when the FBC moves from one plane to another (potentially on a
> > > per-frame basis).
> > 
> > Hi Matt,
> > 
> > Agree with the comments about the subject line and also about your suggestions about some code
> > changes, will update accordingly.
> > 
> > Yes.. the logic for how to choose a plane for FBC is still open.
> > 
> > While discussing with Uma, it was decided that we split this task into two where we first have this
> > possibility to associate FBC to first three planes and then decide on this logic to select the plane
> > for fbc after discussing with Ville. ( Jira VLK-19372 / VLK-21508)
> > 
> > In this patch, "fbc" pointer is associated with first three plane implying those three are FBC
> > capable. And because there is no selection logic there, I choose the first available fbc capable
> > plane for FBC and the rest two to have a no_fbc_reason as "fbc enabled on another plane" which will
> > prevent them from being configured for FBC.
> > 
> > Ville has done refactoring to FBC code earlier and I think the current code flow should handle
> > disable/enable/register handling etc. Ville had some comments about the approach I explained above,
> > I have asked for some clarifications
> > 
> > BR
> > Vinod
> 
> I also just noticed that there's a workaround (Wa_14019417088) that
> requires us to always bind FBC to plane 3.
> 
>         "Plane - FBC binding is fixed to Plane 3."
> 
> And it's marked as a permanent workaround for Xe2_LPD.  So it sounds
> like even though the hardware was designed to support FBC on any of the
> three planes, the reality is that we can only use it on one specific
> plane for this platform; moving it around will have to wait for the next
> platform down the road.

That w/a is a mess. Trawling the HSD confirmed my hunch that it's
"plane 3" because of Windows compositor behaviour. So for us that
is just nonsense.

And looks like we can avoid the whole mess by simply programming
FBC_CTL twice; first without the enable bit set so the plane
binding gets updated, and then again with the with enable bit
set. That is in fact documented as the "alternate w/a".

-- 
Ville Syrjälä
Intel