Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix up pixel_rate vs. clock confusion in wm calculations
On Thu, 09 Dec 2021, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Use pixel_rate rather than crtc_clock in the watermark calculations.
> These are actually identical on gmch platforms for now since
> we don't adjust the pixel rate based on pfit downscaling. But
> pixel_rate is the thing we are actually interested here so use
> the proper name for it.
>
> Signed-off-by: Ville Syrjälä
On the series,
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_pm.c | 52 ++---
> 1 file changed, 22 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 434b1f8b7fe3..b5d5b625a321 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -915,15 +915,13 @@ static void pnv_update_wm(struct drm_i915_private
> *dev_priv)
>
> crtc = single_enabled_crtc(dev_priv);
> if (crtc) {
> - const struct drm_display_mode *pipe_mode =
> - >config->hw.pipe_mode;
> const struct drm_framebuffer *fb =
> crtc->base.primary->state->fb;
> + int pixel_rate = crtc->config->pixel_rate;
> int cpp = fb->format->cpp[0];
> - int clock = pipe_mode->crtc_clock;
>
> /* Display SR */
> - wm = intel_calculate_wm(clock, _display_wm,
> + wm = intel_calculate_wm(pixel_rate, _display_wm,
> pnv_display_wm.fifo_size,
> cpp, latency->display_sr);
> reg = intel_uncore_read(_priv->uncore, DSPFW1);
> @@ -933,7 +931,7 @@ static void pnv_update_wm(struct drm_i915_private
> *dev_priv)
> drm_dbg_kms(_priv->drm, "DSPFW1 register is %x\n", reg);
>
> /* cursor SR */
> - wm = intel_calculate_wm(clock, _cursor_wm,
> + wm = intel_calculate_wm(pixel_rate, _cursor_wm,
> pnv_display_wm.fifo_size,
> 4, latency->cursor_sr);
> reg = intel_uncore_read(_priv->uncore, DSPFW3);
> @@ -942,7 +940,7 @@ static void pnv_update_wm(struct drm_i915_private
> *dev_priv)
> intel_uncore_write(_priv->uncore, DSPFW3, reg);
>
> /* Display HPLL off SR */
> - wm = intel_calculate_wm(clock, _display_hplloff_wm,
> + wm = intel_calculate_wm(pixel_rate, _display_hplloff_wm,
> pnv_display_hplloff_wm.fifo_size,
> cpp, latency->display_hpll_disable);
> reg = intel_uncore_read(_priv->uncore, DSPFW3);
> @@ -951,7 +949,7 @@ static void pnv_update_wm(struct drm_i915_private
> *dev_priv)
> intel_uncore_write(_priv->uncore, DSPFW3, reg);
>
> /* cursor HPLL off SR */
> - wm = intel_calculate_wm(clock, _cursor_hplloff_wm,
> + wm = intel_calculate_wm(pixel_rate, _cursor_hplloff_wm,
> pnv_display_hplloff_wm.fifo_size,
> 4, latency->cursor_hpll_disable);
> reg = intel_uncore_read(_priv->uncore, DSPFW3);
> @@ -1154,7 +1152,7 @@ static u16 g4x_compute_wm(const struct intel_crtc_state
> *crtc_state,
> const struct drm_display_mode *pipe_mode =
> _state->hw.pipe_mode;
> unsigned int latency = dev_priv->wm.pri_latency[level] * 10;
> - unsigned int clock, htotal, cpp, width, wm;
> + unsigned int pixel_rate, htotal, cpp, width, wm;
>
> if (latency == 0)
> return USHRT_MAX;
> @@ -1175,21 +1173,21 @@ static u16 g4x_compute_wm(const struct
> intel_crtc_state *crtc_state,
> level != G4X_WM_LEVEL_NORMAL)
> cpp = max(cpp, 4u);
>
> - clock = pipe_mode->crtc_clock;
> + pixel_rate = crtc_state->pixel_rate;
> htotal = pipe_mode->crtc_htotal;
>
> width = drm_rect_width(_state->uapi.dst);
>
> if (plane->id == PLANE_CURSOR) {
> - wm = intel_wm_method2(clock, htotal, width, cpp, latency);
> + wm = intel_wm_method2(pixel_rate, htotal, width, cpp, latency);
> } else if (plane->id == PLANE_PRIMARY &&
> level == G4X_WM_LEVEL_NORMAL) {
> - wm = intel_wm_method1(clock, cpp, latency);
> + wm = intel_wm_method1(pixel_rate, cpp, latency);
> } else {
> unsigned int small, large;
>
> - small = intel_wm_method1(clock, cpp, latency);
> - large = intel_wm_method2(clock, htotal, width, cpp, latency);
> + small = intel_wm_method1(pixel_rate, cpp, latency);
> + large = intel_wm_method2(pixel_rate, htotal, width, cpp,
> latency);
>
> wm = min(small, large);
> }
> @@ -1674,7 +1672,7 @@ static u16 vlv_compute_wm_level(const struct
> intel_crtc_state *crtc_state,
>
[Intel-gfx] [PATCH 1/3] drm/i915: Fix up pixel_rate vs. clock confusion in wm calculations
From: Ville Syrjälä
Use pixel_rate rather than crtc_clock in the watermark calculations.
These are actually identical on gmch platforms for now since
we don't adjust the pixel rate based on pfit downscaling. But
pixel_rate is the thing we are actually interested here so use
the proper name for it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 52 ++---
1 file changed, 22 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 434b1f8b7fe3..b5d5b625a321 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -915,15 +915,13 @@ static void pnv_update_wm(struct drm_i915_private
*dev_priv)
crtc = single_enabled_crtc(dev_priv);
if (crtc) {
- const struct drm_display_mode *pipe_mode =
- >config->hw.pipe_mode;
const struct drm_framebuffer *fb =
crtc->base.primary->state->fb;
+ int pixel_rate = crtc->config->pixel_rate;
int cpp = fb->format->cpp[0];
- int clock = pipe_mode->crtc_clock;
/* Display SR */
- wm = intel_calculate_wm(clock, _display_wm,
+ wm = intel_calculate_wm(pixel_rate, _display_wm,
pnv_display_wm.fifo_size,
cpp, latency->display_sr);
reg = intel_uncore_read(_priv->uncore, DSPFW1);
@@ -933,7 +931,7 @@ static void pnv_update_wm(struct drm_i915_private *dev_priv)
drm_dbg_kms(_priv->drm, "DSPFW1 register is %x\n", reg);
/* cursor SR */
- wm = intel_calculate_wm(clock, _cursor_wm,
+ wm = intel_calculate_wm(pixel_rate, _cursor_wm,
pnv_display_wm.fifo_size,
4, latency->cursor_sr);
reg = intel_uncore_read(_priv->uncore, DSPFW3);
@@ -942,7 +940,7 @@ static void pnv_update_wm(struct drm_i915_private *dev_priv)
intel_uncore_write(_priv->uncore, DSPFW3, reg);
/* Display HPLL off SR */
- wm = intel_calculate_wm(clock, _display_hplloff_wm,
+ wm = intel_calculate_wm(pixel_rate, _display_hplloff_wm,
pnv_display_hplloff_wm.fifo_size,
cpp, latency->display_hpll_disable);
reg = intel_uncore_read(_priv->uncore, DSPFW3);
@@ -951,7 +949,7 @@ static void pnv_update_wm(struct drm_i915_private *dev_priv)
intel_uncore_write(_priv->uncore, DSPFW3, reg);
/* cursor HPLL off SR */
- wm = intel_calculate_wm(clock, _cursor_hplloff_wm,
+ wm = intel_calculate_wm(pixel_rate, _cursor_hplloff_wm,
pnv_display_hplloff_wm.fifo_size,
4, latency->cursor_hpll_disable);
reg = intel_uncore_read(_priv->uncore, DSPFW3);
@@ -1154,7 +1152,7 @@ static u16 g4x_compute_wm(const struct intel_crtc_state
*crtc_state,
const struct drm_display_mode *pipe_mode =
_state->hw.pipe_mode;
unsigned int latency = dev_priv->wm.pri_latency[level] * 10;
- unsigned int clock, htotal, cpp, width, wm;
+ unsigned int pixel_rate, htotal, cpp, width, wm;
if (latency == 0)
return USHRT_MAX;
@@ -1175,21 +1173,21 @@ static u16 g4x_compute_wm(const struct intel_crtc_state
*crtc_state,
level != G4X_WM_LEVEL_NORMAL)
cpp = max(cpp, 4u);
- clock = pipe_mode->crtc_clock;
+ pixel_rate = crtc_state->pixel_rate;
htotal = pipe_mode->crtc_htotal;
width = drm_rect_width(_state->uapi.dst);
if (plane->id == PLANE_CURSOR) {
- wm = intel_wm_method2(clock, htotal, width, cpp, latency);
+ wm = intel_wm_method2(pixel_rate, htotal, width, cpp, latency);
} else if (plane->id == PLANE_PRIMARY &&
level == G4X_WM_LEVEL_NORMAL) {
- wm = intel_wm_method1(clock, cpp, latency);
+ wm = intel_wm_method1(pixel_rate, cpp, latency);
} else {
unsigned int small, large;
- small = intel_wm_method1(clock, cpp, latency);
- large = intel_wm_method2(clock, htotal, width, cpp, latency);
+ small = intel_wm_method1(pixel_rate, cpp, latency);
+ large = intel_wm_method2(pixel_rate, htotal, width, cpp,
latency);
wm = min(small, large);
}
@@ -1674,7 +1672,7 @@ static u16 vlv_compute_wm_level(const struct
intel_crtc_state *crtc_state,
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
const struct drm_display_mode *pipe_mode =
_state->hw.pipe_mode;
- unsigned
