Re: [Intel-gfx] [PATCH v2 1/1] drm/i915/lnl: possibility to enable FBC on first three planes

["Govindapillai, Vinod" <vinod.govindapillai@intel.com>]

Thanks Ville. Updated the patch as per your comments.

About the WA, I am planning to send that as two separate patches one were we enabled FBC + PSR2
based on some conditions and the next one this WA

BR
Vinod

On Fri, 2023-09-01 at 16:10 +0300, Ville Syrjälä wrote:
> On Fri, Sep 01, 2023 at 03:59:50PM +0300, Vinod Govindapillai wrote:
> > In LNL onwards, FBC can be associated to the first three planes.
> > FBC will be enabled on planes first come first served basis
> > until the userspace can select one of these FBC capable plane
> > explicitly. FBC can be supported in planes with per pixel alpha
> > 
> > v2:
> >  - avoid fbc->state.plane check in intel_fbc_check_plane (Ville)
> >  - simplify plane binding register writes (Matt)
> >  - Update the subject to reflect that fbc can be enabled only in
> >    the first three planes (Matt)
> > 
> > Bspec: 69560
> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_fbc.c           | 7 ++++++-
> >  drivers/gpu/drm/i915/display/skl_universal_plane.c | 5 ++++-
> >  drivers/gpu/drm/i915/i915_reg.h                    | 1 +
> >  3 files changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index 66c8aed07bbc..f1537bb63775 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -660,6 +660,10 @@ static u32 ivb_dpfc_ctl(struct intel_fbc *fbc)
> >         if (IS_IVYBRIDGE(i915))
> >                 dpfc_ctl |= DPFC_CTL_PLANE_IVB(fbc_state->plane->i9xx_plane);
> >  
> > +       if (DISPLAY_VER(i915) >= 20)
> > +               dpfc_ctl |= REG_FIELD_PREP(DPFC_CTL_PLANE_BINDING_MASK,
> > +                                          fbc_state->plane->id);
> 
> Please add the customary wrapped macro for this.
> 
> We'll also need to deal with that annoying plane switching w/a.
> Easiest might be to just write DPCF_CTL without the enable bit
> in .program_cfb(). But that can be a separate patch for clarity.
> 
> > +
> >         if (fbc_state->fence_id >= 0)
> >                 dpfc_ctl |= DPFC_CTL_FENCE_EN_IVB;
> >  
> > @@ -1206,7 +1210,8 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state,
> >                 return 0;
> >         }
> >  
> > -       if (plane_state->hw.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE &&
> > +       if (DISPLAY_VER(i915) < 20 &&
> 
> One patch per logical change please.
> 
> > +           plane_state->hw.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE &&
> >             fb->format->has_alpha) {
> >                 plane_state->no_fbc_reason = "per-pixel alpha not supported";
> >                 return 0;
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 4d01c7ae4485..1291351c9941 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -1962,7 +1962,10 @@ static bool skl_plane_has_fbc(struct drm_i915_private *dev_priv,
> >         if ((DISPLAY_RUNTIME_INFO(dev_priv)->fbc_mask & BIT(fbc_id)) == 0)
> >                 return false;
> >  
> > -       return plane_id == PLANE_PRIMARY;
> > +       if (DISPLAY_VER(dev_priv) >= 20)
> > +               return plane_id <= PLANE_SPRITE1;
> 
> I think we have some kind of is_hdr_plane() helper somewhere.
> Probably should use that.
> 
> > +       else
> > +               return plane_id == PLANE_PRIMARY;
> >  }
> >  
> >  static struct intel_fbc *skl_plane_fbc(struct drm_i915_private *dev_priv,
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index aefad14ab27a..aadcc630cb52 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1327,6 +1327,7 @@
> >  #define   DPFC_CTL_PLANE_IVB(i9xx_plane)       REG_FIELD_PREP(DPFC_CTL_PLANE_MASK_IVB,
> > (i9xx_plane))
> >  #define   DPFC_CTL_FENCE_EN_IVB                        REG_BIT(28) /* ivb+ */
> >  #define   DPFC_CTL_PERSISTENT_MODE             REG_BIT(25) /* g4x-snb */
> > +#define   DPFC_CTL_PLANE_BINDING_MASK          REG_GENMASK(12, 11) /* lnl */
> 
> Presumably lnl+
> 
> >  #define   DPFC_CTL_FALSE_COLOR                 REG_BIT(10) /* ivb+ */
> >  #define   DPFC_CTL_SR_EN                       REG_BIT(10) /* g4x only */
> >  #define   DPFC_CTL_SR_EXIT_DIS                 REG_BIT(9) /* g4x only */
> > -- 
> > 2.34.1
>