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

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

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.

>  
>  	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?

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

-- 
Ville Syrjälä
Intel