diff mbox series

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

Message ID 20241105071600.235338-9-vinod.govindapillai@intel.com (mailing list archive)
State New
Headers show
Series use hw support for min/interim ddb allocation for async flip | expand

Commit Message

Vinod Govindapillai Nov. 5, 2024, 7:16 a.m. UTC 
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(-)

Comments

Ville Syrjala Nov. 6, 2024, 10:46 p.m. UTC | #1 
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
Vinod Govindapillai Nov. 20, 2024, 10:30 p.m. UTC | #2 
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
>
diff mbox series

Patch

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");
 
 	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)
 #define PLANE_INTERIM_DDB_BLOCKS_MASK		REG_GENMASK(11, 0)
 
 /* tgl+ */