diff mbox series

[2/4] drm/i915/lnl: update FBC debugfs to include plane information

Message ID 20230828062035.6906-3-vinod.govindapillai@intel.com (mailing list archive)
State New, archived
Headers show
Series fbc on any plane | expand

Commit Message

Vinod Govindapillai Aug. 28, 2023, 6:20 a.m. UTC
In future platforms, FBC can be supported on planes other than
the primary plane. So update the debugfs entry for FBC status
to have the plane ID included.

Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Matt Roper Aug. 29, 2023, 12:01 a.m. UTC | #1
On Mon, Aug 28, 2023 at 09:20:33AM +0300, Vinod Govindapillai wrote:
> In future platforms, FBC can be supported on planes other than

"future platforms" on a patch labelled "drm/i915/lnl" makes it sound
like this is something that shows up beyond LNL, which isn't really the
case.  The "future" is already here, so I'd drop the "lnl" part of the
subject and just say "With Xe2_LPD and beyond..."

> the primary plane. So update the debugfs entry for FBC status
> to have the plane ID included.
> 
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index d36499d7e0be..45e205a0f740 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -1837,7 +1837,9 @@ static int intel_fbc_debugfs_status_show(struct seq_file *m, void *unused)
>  	mutex_lock(&fbc->lock);
>  
>  	if (fbc->active) {
> -		seq_puts(m, "FBC enabled\n");
> +		seq_printf(m, "FBC enabled: [PLANE:%d:%s]\n",
> +			   fbc->state.plane->base.base.id,
> +			   fbc->state.plane->base.name);
>  		seq_printf(m, "Compressing: %s\n",
>  			   str_yes_no(intel_fbc_is_compressing(fbc)));
>  	} else {
> @@ -1910,10 +1912,16 @@ static void intel_fbc_debugfs_add(struct intel_fbc *fbc,
>  
>  void intel_fbc_crtc_debugfs_add(struct intel_crtc *crtc)
>  {
> -	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct intel_plane *plane;
> +
> +	for_each_intel_plane(&i915->drm, plane) {

You can use for_each_intel_plane_on_crtc here to avoid the pipe check
below.


Matt

> +		if (!plane->fbc || plane->pipe != crtc->pipe)
> +			continue;
>  
> -	if (plane->fbc)
>  		intel_fbc_debugfs_add(plane->fbc, crtc->base.debugfs_entry);
> +		break;
> +	}
>  }
>  
>  /* FIXME: remove this once igt is on board with per-crtc stuff */
> -- 
> 2.34.1
>
Ville Syrjala Aug. 29, 2023, 7:46 a.m. UTC | #2
On Mon, Aug 28, 2023 at 09:20:33AM +0300, Vinod Govindapillai wrote:
> In future platforms, FBC can be supported on planes other than
> the primary plane. So update the debugfs entry for FBC status
> to have the plane ID included.
> 
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index d36499d7e0be..45e205a0f740 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -1837,7 +1837,9 @@ static int intel_fbc_debugfs_status_show(struct seq_file *m, void *unused)
>  	mutex_lock(&fbc->lock);
>  
>  	if (fbc->active) {
> -		seq_puts(m, "FBC enabled\n");
> +		seq_printf(m, "FBC enabled: [PLANE:%d:%s]\n",
> +			   fbc->state.plane->base.base.id,
> +			   fbc->state.plane->base.name);

That informtion is already part of the plane loop below.

>  		seq_printf(m, "Compressing: %s\n",
>  			   str_yes_no(intel_fbc_is_compressing(fbc)));
>  	} else {
> @@ -1910,10 +1912,16 @@ static void intel_fbc_debugfs_add(struct intel_fbc *fbc,
>  
>  void intel_fbc_crtc_debugfs_add(struct intel_crtc *crtc)
>  {
> -	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct intel_plane *plane;
> +
> +	for_each_intel_plane(&i915->drm, plane) {
> +		if (!plane->fbc || plane->pipe != crtc->pipe)
> +			continue;
>  
> -	if (plane->fbc)
>  		intel_fbc_debugfs_add(plane->fbc, crtc->base.debugfs_entry);
> +		break;
> +	}

Do we have a case where the first plane wouldn't support FBC? If not
this wouldn't really be needed. But I guess no harm in iterating here
too.

>  }
>  
>  /* FIXME: remove this once igt is on board with per-crtc stuff */
> -- 
> 2.34.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index d36499d7e0be..45e205a0f740 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1837,7 +1837,9 @@  static int intel_fbc_debugfs_status_show(struct seq_file *m, void *unused)
 	mutex_lock(&fbc->lock);
 
 	if (fbc->active) {
-		seq_puts(m, "FBC enabled\n");
+		seq_printf(m, "FBC enabled: [PLANE:%d:%s]\n",
+			   fbc->state.plane->base.base.id,
+			   fbc->state.plane->base.name);
 		seq_printf(m, "Compressing: %s\n",
 			   str_yes_no(intel_fbc_is_compressing(fbc)));
 	} else {
@@ -1910,10 +1912,16 @@  static void intel_fbc_debugfs_add(struct intel_fbc *fbc,
 
 void intel_fbc_crtc_debugfs_add(struct intel_crtc *crtc)
 {
-	struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+	struct intel_plane *plane;
+
+	for_each_intel_plane(&i915->drm, plane) {
+		if (!plane->fbc || plane->pipe != crtc->pipe)
+			continue;
 
-	if (plane->fbc)
 		intel_fbc_debugfs_add(plane->fbc, crtc->base.debugfs_entry);
+		break;
+	}
 }
 
 /* FIXME: remove this once igt is on board with per-crtc stuff */