diff mbox series

[2/8] drm/i9i5/display: use intel_display in intel_de_read calls of skl_watermark.c

Message ID 20241105071600.235338-3-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:15 a.m. UTC 
Convert all intel_de_read() to use intel_display instead of
struct drm_i915_private object. This is in preparation for
the rest of the patches in this series where hw support for
the minimum and interim ddb allocations for async flip is
added.

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

Comments

Jani Nikula Nov. 5, 2024, 8:58 a.m. UTC | #1 
On Tue, 05 Nov 2024, Vinod Govindapillai <vinod.govindapillai@intel.com> wrote:
> Convert all intel_de_read() to use intel_display instead of
> struct drm_i915_private object. This is in preparation for
> the rest of the patches in this series where hw support for
> the minimum and interim ddb allocations for async flip is
> added.
>
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  drivers/gpu/drm/i915/display/skl_watermark.c | 48 +++++++++++---------
>  1 file changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> index d9d7238f0fb4..2afc95e7533c 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -82,12 +82,14 @@ intel_has_sagv(struct drm_i915_private *i915)
>  }
>  
>  static u32
> -intel_sagv_block_time(struct drm_i915_private *i915)
> +intel_sagv_block_time(struct intel_display *display)
>  {
> +	struct drm_i915_private *i915 = to_i915(display->drm);
> +
>  	if (DISPLAY_VER(i915) >= 14) {

Please don't limit to changing just the intel_de_* per function. Convert
all you can.

>  		u32 val;
>  
> -		val = intel_de_read(i915, MTL_LATENCY_SAGV);
> +		val = intel_de_read(display, MTL_LATENCY_SAGV);
>  
>  		return REG_FIELD_GET(MTL_LATENCY_QCLK_SAGV, val);
>  	} else if (DISPLAY_VER(i915) >= 12) {
> @@ -126,7 +128,7 @@ static void intel_sagv_init(struct drm_i915_private *i915)
>  
>  	drm_WARN_ON(&i915->drm, i915->display.sagv.status == I915_SAGV_UNKNOWN);
>  
> -	i915->display.sagv.block_time_us = intel_sagv_block_time(i915);
> +	i915->display.sagv.block_time_us = intel_sagv_block_time(&i915->display);

Please add struct intel_display *display local variable.

You don't need to change everything here (in the caller side) in one go,
but quite obviously the function would benefit from further changes.

>  
>  	drm_dbg_kms(&i915->drm, "SAGV supported: %s, original SAGV block time: %u us\n",
>  		    str_yes_no(intel_has_sagv(i915)), i915->display.sagv.block_time_us);
> @@ -791,7 +793,7 @@ static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry *entry, u32 reg)
>  }
>  
>  static void
> -skl_ddb_get_hw_plane_state(struct drm_i915_private *i915,
> +skl_ddb_get_hw_plane_state(struct intel_display *display,
>  			   const enum pipe pipe,
>  			   const enum plane_id plane_id,
>  			   struct skl_ddb_entry *ddb,
> @@ -801,18 +803,18 @@ skl_ddb_get_hw_plane_state(struct drm_i915_private *i915,
>  
>  	/* Cursor doesn't support NV12/planar, so no extra calculation needed */
>  	if (plane_id == PLANE_CURSOR) {
> -		val = intel_de_read(i915, CUR_BUF_CFG(pipe));
> +		val = intel_de_read(display, CUR_BUF_CFG(pipe));
>  		skl_ddb_entry_init_from_hw(ddb, val);
>  		return;
>  	}
>  
> -	val = intel_de_read(i915, PLANE_BUF_CFG(pipe, plane_id));
> +	val = intel_de_read(display, PLANE_BUF_CFG(pipe, plane_id));
>  	skl_ddb_entry_init_from_hw(ddb, val);
>  
> -	if (DISPLAY_VER(i915) >= 11)
> +	if (DISPLAY_VER(display) >= 11)
>  		return;
>  
> -	val = intel_de_read(i915, PLANE_NV12_BUF_CFG(pipe, plane_id));
> +	val = intel_de_read(display, PLANE_NV12_BUF_CFG(pipe, plane_id));
>  	skl_ddb_entry_init_from_hw(ddb_y, val);
>  }
>  
> @@ -832,7 +834,7 @@ static void skl_pipe_ddb_get_hw_state(struct intel_crtc *crtc,
>  		return;
>  
>  	for_each_plane_id_on_crtc(crtc, plane_id)
> -		skl_ddb_get_hw_plane_state(i915, pipe,
> +		skl_ddb_get_hw_plane_state(&i915->display, pipe,

Please add the local variable.

>  					   plane_id,
>  					   &ddb[plane_id],
>  					   &ddb_y[plane_id]);
> @@ -2932,6 +2934,7 @@ static void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
>  				     struct skl_pipe_wm *out)
>  {
>  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct intel_display *display = &i915->display;

Please initialize struct intel_display *display using
to_intel_display(...) when possible instead of i915. Here, it would be:

	struct intel_display *display = to_intel_display(crtc);

That doesn't need to be changed anymore, &i915->display does.

And please put the display variable first.

>  	enum pipe pipe = crtc->pipe;
>  	enum plane_id plane_id;
>  	int level;
> @@ -2942,32 +2945,32 @@ static void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
>  
>  		for (level = 0; level < i915->display.wm.num_levels; level++) {

Please don't limit to changing just intel_de_*. This should be
display->wm.num_levels. Etc.

>  			if (plane_id != PLANE_CURSOR)
> -				val = intel_de_read(i915, PLANE_WM(pipe, plane_id, level));
> +				val = intel_de_read(display, PLANE_WM(pipe, plane_id, level));
>  			else
> -				val = intel_de_read(i915, CUR_WM(pipe, level));
> +				val = intel_de_read(display, CUR_WM(pipe, level));
>  
>  			skl_wm_level_from_reg_val(val, &wm->wm[level]);
>  		}
>  
>  		if (plane_id != PLANE_CURSOR)
> -			val = intel_de_read(i915, PLANE_WM_TRANS(pipe, plane_id));
> +			val = intel_de_read(display, PLANE_WM_TRANS(pipe, plane_id));
>  		else
> -			val = intel_de_read(i915, CUR_WM_TRANS(pipe));
> +			val = intel_de_read(display, CUR_WM_TRANS(pipe));
>  
>  		skl_wm_level_from_reg_val(val, &wm->trans_wm);
>  
>  		if (HAS_HW_SAGV_WM(i915)) {
>  			if (plane_id != PLANE_CURSOR)
> -				val = intel_de_read(i915, PLANE_WM_SAGV(pipe, plane_id));
> +				val = intel_de_read(display, PLANE_WM_SAGV(pipe, plane_id));
>  			else
> -				val = intel_de_read(i915, CUR_WM_SAGV(pipe));
> +				val = intel_de_read(display, CUR_WM_SAGV(pipe));
>  
>  			skl_wm_level_from_reg_val(val, &wm->sagv.wm0);
>  
>  			if (plane_id != PLANE_CURSOR)
> -				val = intel_de_read(i915, PLANE_WM_SAGV_TRANS(pipe, plane_id));
> +				val = intel_de_read(display, PLANE_WM_SAGV_TRANS(pipe, plane_id));
>  			else
> -				val = intel_de_read(i915, CUR_WM_SAGV_TRANS(pipe));
> +				val = intel_de_read(display, CUR_WM_SAGV_TRANS(pipe));
>  
>  			skl_wm_level_from_reg_val(val, &wm->sagv.trans_wm);
>  		} else if (DISPLAY_VER(i915) >= 12) {
> @@ -2985,7 +2988,7 @@ static void skl_wm_get_hw_state(struct drm_i915_private *i915)
>  	struct intel_crtc *crtc;
>  
>  	if (HAS_MBUS_JOINING(i915))
> -		dbuf_state->joined_mbus = intel_de_read(i915, MBUS_CTL) & MBUS_JOIN;
> +		dbuf_state->joined_mbus = intel_de_read(display, MBUS_CTL) & MBUS_JOIN;
>  
>  	dbuf_state->mdclk_cdclk_ratio = intel_mdclk_cdclk_ratio(display, &display->cdclk.hw);
>  
> @@ -3014,7 +3017,7 @@ static void skl_wm_get_hw_state(struct drm_i915_private *i915)
>  			if (!crtc_state->hw.active)
>  				continue;
>  
> -			skl_ddb_get_hw_plane_state(i915, crtc->pipe,
> +			skl_ddb_get_hw_plane_state(display, crtc->pipe,
>  						   plane_id, ddb, ddb_y);
>  
>  			skl_ddb_entry_union(&dbuf_state->ddb[pipe], ddb);
> @@ -3330,18 +3333,19 @@ adjust_wm_latency(struct drm_i915_private *i915,
>  
>  static void mtl_read_wm_latency(struct drm_i915_private *i915, u16 wm[])
>  {
> +	struct intel_display *display = &i915->display;
>  	int num_levels = i915->display.wm.num_levels;

display->wm.num_levels

>  	u32 val;
>  
> -	val = intel_de_read(i915, MTL_LATENCY_LP0_LP1);
> +	val = intel_de_read(display, MTL_LATENCY_LP0_LP1);
>  	wm[0] = REG_FIELD_GET(MTL_LATENCY_LEVEL_EVEN_MASK, val);
>  	wm[1] = REG_FIELD_GET(MTL_LATENCY_LEVEL_ODD_MASK, val);
>  
> -	val = intel_de_read(i915, MTL_LATENCY_LP2_LP3);
> +	val = intel_de_read(display, MTL_LATENCY_LP2_LP3);
>  	wm[2] = REG_FIELD_GET(MTL_LATENCY_LEVEL_EVEN_MASK, val);
>  	wm[3] = REG_FIELD_GET(MTL_LATENCY_LEVEL_ODD_MASK, val);
>  
> -	val = intel_de_read(i915, MTL_LATENCY_LP4_LP5);
> +	val = intel_de_read(display, MTL_LATENCY_LP4_LP5);
>  	wm[4] = REG_FIELD_GET(MTL_LATENCY_LEVEL_EVEN_MASK, val);
>  	wm[5] = REG_FIELD_GET(MTL_LATENCY_LEVEL_ODD_MASK, val);
Jani Nikula Nov. 5, 2024, 9:03 a.m. UTC | #2 
On Tue, 05 Nov 2024, Vinod Govindapillai <vinod.govindapillai@intel.com> wrote:
> Convert all intel_de_read() to use intel_display instead of
> struct drm_i915_private object. This is in preparation for
> the rest of the patches in this series where hw support for
> the minimum and interim ddb allocations for async flip is
> added.
>
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>

Typo in subject prefix.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index d9d7238f0fb4..2afc95e7533c 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -82,12 +82,14 @@  intel_has_sagv(struct drm_i915_private *i915)
 }
 
 static u32
-intel_sagv_block_time(struct drm_i915_private *i915)
+intel_sagv_block_time(struct intel_display *display)
 {
+	struct drm_i915_private *i915 = to_i915(display->drm);
+
 	if (DISPLAY_VER(i915) >= 14) {
 		u32 val;
 
-		val = intel_de_read(i915, MTL_LATENCY_SAGV);
+		val = intel_de_read(display, MTL_LATENCY_SAGV);
 
 		return REG_FIELD_GET(MTL_LATENCY_QCLK_SAGV, val);
 	} else if (DISPLAY_VER(i915) >= 12) {
@@ -126,7 +128,7 @@  static void intel_sagv_init(struct drm_i915_private *i915)
 
 	drm_WARN_ON(&i915->drm, i915->display.sagv.status == I915_SAGV_UNKNOWN);
 
-	i915->display.sagv.block_time_us = intel_sagv_block_time(i915);
+	i915->display.sagv.block_time_us = intel_sagv_block_time(&i915->display);
 
 	drm_dbg_kms(&i915->drm, "SAGV supported: %s, original SAGV block time: %u us\n",
 		    str_yes_no(intel_has_sagv(i915)), i915->display.sagv.block_time_us);
@@ -791,7 +793,7 @@  static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry *entry, u32 reg)
 }
 
 static void
-skl_ddb_get_hw_plane_state(struct drm_i915_private *i915,
+skl_ddb_get_hw_plane_state(struct intel_display *display,
 			   const enum pipe pipe,
 			   const enum plane_id plane_id,
 			   struct skl_ddb_entry *ddb,
@@ -801,18 +803,18 @@  skl_ddb_get_hw_plane_state(struct drm_i915_private *i915,
 
 	/* Cursor doesn't support NV12/planar, so no extra calculation needed */
 	if (plane_id == PLANE_CURSOR) {
-		val = intel_de_read(i915, CUR_BUF_CFG(pipe));
+		val = intel_de_read(display, CUR_BUF_CFG(pipe));
 		skl_ddb_entry_init_from_hw(ddb, val);
 		return;
 	}
 
-	val = intel_de_read(i915, PLANE_BUF_CFG(pipe, plane_id));
+	val = intel_de_read(display, PLANE_BUF_CFG(pipe, plane_id));
 	skl_ddb_entry_init_from_hw(ddb, val);
 
-	if (DISPLAY_VER(i915) >= 11)
+	if (DISPLAY_VER(display) >= 11)
 		return;
 
-	val = intel_de_read(i915, PLANE_NV12_BUF_CFG(pipe, plane_id));
+	val = intel_de_read(display, PLANE_NV12_BUF_CFG(pipe, plane_id));
 	skl_ddb_entry_init_from_hw(ddb_y, val);
 }
 
@@ -832,7 +834,7 @@  static void skl_pipe_ddb_get_hw_state(struct intel_crtc *crtc,
 		return;
 
 	for_each_plane_id_on_crtc(crtc, plane_id)
-		skl_ddb_get_hw_plane_state(i915, pipe,
+		skl_ddb_get_hw_plane_state(&i915->display, pipe,
 					   plane_id,
 					   &ddb[plane_id],
 					   &ddb_y[plane_id]);
@@ -2932,6 +2934,7 @@  static void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
 				     struct skl_pipe_wm *out)
 {
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+	struct intel_display *display = &i915->display;
 	enum pipe pipe = crtc->pipe;
 	enum plane_id plane_id;
 	int level;
@@ -2942,32 +2945,32 @@  static void skl_pipe_wm_get_hw_state(struct intel_crtc *crtc,
 
 		for (level = 0; level < i915->display.wm.num_levels; level++) {
 			if (plane_id != PLANE_CURSOR)
-				val = intel_de_read(i915, PLANE_WM(pipe, plane_id, level));
+				val = intel_de_read(display, PLANE_WM(pipe, plane_id, level));
 			else
-				val = intel_de_read(i915, CUR_WM(pipe, level));
+				val = intel_de_read(display, CUR_WM(pipe, level));
 
 			skl_wm_level_from_reg_val(val, &wm->wm[level]);
 		}
 
 		if (plane_id != PLANE_CURSOR)
-			val = intel_de_read(i915, PLANE_WM_TRANS(pipe, plane_id));
+			val = intel_de_read(display, PLANE_WM_TRANS(pipe, plane_id));
 		else
-			val = intel_de_read(i915, CUR_WM_TRANS(pipe));
+			val = intel_de_read(display, CUR_WM_TRANS(pipe));
 
 		skl_wm_level_from_reg_val(val, &wm->trans_wm);
 
 		if (HAS_HW_SAGV_WM(i915)) {
 			if (plane_id != PLANE_CURSOR)
-				val = intel_de_read(i915, PLANE_WM_SAGV(pipe, plane_id));
+				val = intel_de_read(display, PLANE_WM_SAGV(pipe, plane_id));
 			else
-				val = intel_de_read(i915, CUR_WM_SAGV(pipe));
+				val = intel_de_read(display, CUR_WM_SAGV(pipe));
 
 			skl_wm_level_from_reg_val(val, &wm->sagv.wm0);
 
 			if (plane_id != PLANE_CURSOR)
-				val = intel_de_read(i915, PLANE_WM_SAGV_TRANS(pipe, plane_id));
+				val = intel_de_read(display, PLANE_WM_SAGV_TRANS(pipe, plane_id));
 			else
-				val = intel_de_read(i915, CUR_WM_SAGV_TRANS(pipe));
+				val = intel_de_read(display, CUR_WM_SAGV_TRANS(pipe));
 
 			skl_wm_level_from_reg_val(val, &wm->sagv.trans_wm);
 		} else if (DISPLAY_VER(i915) >= 12) {
@@ -2985,7 +2988,7 @@  static void skl_wm_get_hw_state(struct drm_i915_private *i915)
 	struct intel_crtc *crtc;
 
 	if (HAS_MBUS_JOINING(i915))
-		dbuf_state->joined_mbus = intel_de_read(i915, MBUS_CTL) & MBUS_JOIN;
+		dbuf_state->joined_mbus = intel_de_read(display, MBUS_CTL) & MBUS_JOIN;
 
 	dbuf_state->mdclk_cdclk_ratio = intel_mdclk_cdclk_ratio(display, &display->cdclk.hw);
 
@@ -3014,7 +3017,7 @@  static void skl_wm_get_hw_state(struct drm_i915_private *i915)
 			if (!crtc_state->hw.active)
 				continue;
 
-			skl_ddb_get_hw_plane_state(i915, crtc->pipe,
+			skl_ddb_get_hw_plane_state(display, crtc->pipe,
 						   plane_id, ddb, ddb_y);
 
 			skl_ddb_entry_union(&dbuf_state->ddb[pipe], ddb);
@@ -3330,18 +3333,19 @@  adjust_wm_latency(struct drm_i915_private *i915,
 
 static void mtl_read_wm_latency(struct drm_i915_private *i915, u16 wm[])
 {
+	struct intel_display *display = &i915->display;
 	int num_levels = i915->display.wm.num_levels;
 	u32 val;
 
-	val = intel_de_read(i915, MTL_LATENCY_LP0_LP1);
+	val = intel_de_read(display, MTL_LATENCY_LP0_LP1);
 	wm[0] = REG_FIELD_GET(MTL_LATENCY_LEVEL_EVEN_MASK, val);
 	wm[1] = REG_FIELD_GET(MTL_LATENCY_LEVEL_ODD_MASK, val);
 
-	val = intel_de_read(i915, MTL_LATENCY_LP2_LP3);
+	val = intel_de_read(display, MTL_LATENCY_LP2_LP3);
 	wm[2] = REG_FIELD_GET(MTL_LATENCY_LEVEL_EVEN_MASK, val);
 	wm[3] = REG_FIELD_GET(MTL_LATENCY_LEVEL_ODD_MASK, val);
 
-	val = intel_de_read(i915, MTL_LATENCY_LP4_LP5);
+	val = intel_de_read(display, MTL_LATENCY_LP4_LP5);
 	wm[4] = REG_FIELD_GET(MTL_LATENCY_LEVEL_EVEN_MASK, val);
 	wm[5] = REG_FIELD_GET(MTL_LATENCY_LEVEL_ODD_MASK, val);