Re: [PATCH 8/8] drm/i915/debugfs: add dbuf alloc status as part of i915_ddb_info

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

On Thu, 2024-11-07 at 00:46 +0200, Ville Syrjälä wrote:
> On Tue, Nov 05, 2024 at 09:16:00AM +0200, Vinod Govindapillai wrote:
> > > From xe3 onwards, there is a provision to define and
> > use min ddb and interim ddb allocations for async flip
> > use case. Add the dbuf allocation status as part of
> > i915_ddb_info as well to show if min or interim ddb
> > is being used.
> > 
> > Bspec: 72053
> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_debugfs.c  | 23 ++++++++++++++++---
> >  .../i915/display/skl_universal_plane_regs.h   |  1 +
> >  2 files changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index 11aff485d8fa..bce4a1ab05c0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -39,6 +39,7 @@
> >  #include "intel_pps.h"
> >  #include "intel_psr.h"
> >  #include "intel_psr_regs.h"
> > +#include "skl_universal_plane_regs.h"
> >  #include "intel_vdsc.h"
> >  #include "intel_wm.h"
> >  
> > @@ -688,9 +689,24 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
> >         return 0;
> >  }
> >  
> > +static u32 dbuf_alloc_status(struct intel_display *display,
> > +                            enum pipe pipe, enum plane_id plane_id)
> > +{
> > +       u32 val = 0;
> > +
> > +       if (DISPLAY_VER(display) >= 30) {
> > +               u32 reg = intel_de_read(display,
> > +                                       PLANE_MIN_BUF_CFG(pipe, plane_id));
> > +               val = REG_FIELD_GET(PLANE_DBUF_ALLOC_STATUS_MASK, reg);
> > +       }
> > +
> > +       return val;
> > +}
> > +
> >  static int i915_ddb_info(struct seq_file *m, void *unused)
> >  {
> >         struct drm_i915_private *dev_priv = node_to_i915(m->private);
> > +       struct intel_display *display = &dev_priv->display;
> >         struct skl_ddb_entry *entry;
> >         struct intel_crtc *crtc;
> >  
> > @@ -699,7 +715,7 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
> >  
> >         drm_modeset_lock_all(&dev_priv->drm);
> >  
> > -       seq_printf(m, "%-15s%8s%8s%8s\n", "", "Start", "End", "Size");
> > +       seq_printf(m, "%-15s%8s%8s%8s%16s\n", "", "Start", "End", "Size", "Alloc Status");
> 
> This guy is meant ot just print the software state. The hardware might
> not even be awake here. So this doesn't belong here. It would be better
> to add it to the intel_watermark tool instead.

Okay. I was using this to verify this feature. With async flip IGT test cases, I can check this
ddb_info in a loop to see if this dbuf allocation status is being changed.

So could there be any use if this be part of a new debugfs entry?

> 
> >  
> >         for_each_intel_crtc(&dev_priv->drm, crtc) {
> >                 struct intel_crtc_state *crtc_state =
> > @@ -711,9 +727,10 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
> >  
> >                 for_each_plane_id_on_crtc(crtc, plane_id) {
> >                         entry = &crtc_state->wm.skl.plane_ddb[plane_id];
> > -                       seq_printf(m, "  Plane%-8d%8u%8u%8u\n", plane_id + 1,
> > +                       seq_printf(m, "  Plane%-8d%8u%8u%8u%8u\n", plane_id + 1,
> >                                    entry->start, entry->end,
> > -                                  skl_ddb_entry_size(entry));
> > +                                  skl_ddb_entry_size(entry),
> > +                                  dbuf_alloc_status(display, pipe, plane_id));
> >                 }
> >  
> >                 entry = &crtc_state->wm.skl.plane_ddb[PLANE_CURSOR];
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane_regs.h
> > b/drivers/gpu/drm/i915/display/skl_universal_plane_regs.h
> > index 65a5482fae60..53550356430d 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane_regs.h
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane_regs.h
> > @@ -390,6 +390,7 @@
> >  
> >  #define PLANE_AUTO_MIN_DBUF_EN                 REG_BIT(31)
> >  #define PLANE_MIN_DDB_BLOCKS_MASK              REG_GENMASK(27, 16)
> > +#define PLANE_DBUF_ALLOC_STATUS_MASK           REG_GENMASK(15, 14)
> 
> Do these bits actually mean something?
This gives the dbuf allocation status. Whenever HW uses the min_ddb configuration this bit is
updated and I had verified this.

BR
Vinod
> 
> >  #define PLANE_INTERIM_DDB_BLOCKS_MASK          REG_GENMASK(11, 0)
> >  
> >  /* tgl+ */
> > -- 
> > 2.34.1
>