From: Jani Nikula <jani.nikula@linux.intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix up pixel_rate vs. clock confusion in wm calculations
Date: Wed, 26 Jan 2022 16:50:57 +0200 [thread overview]
Message-ID: <877damzh2m.fsf@intel.com> (raw)
In-Reply-To: <20211209144311.3221-1-ville.syrjala@linux.intel.com>
On Thu, 09 Dec 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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ä <ville.syrjala@linux.intel.com>
On the series,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> 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 =
> - &crtc->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, &pnv_display_wm,
> + wm = intel_calculate_wm(pixel_rate, &pnv_display_wm,
> pnv_display_wm.fifo_size,
> cpp, latency->display_sr);
> reg = intel_uncore_read(&dev_priv->uncore, DSPFW1);
> @@ -933,7 +931,7 @@ static void pnv_update_wm(struct drm_i915_private *dev_priv)
> drm_dbg_kms(&dev_priv->drm, "DSPFW1 register is %x\n", reg);
>
> /* cursor SR */
> - wm = intel_calculate_wm(clock, &pnv_cursor_wm,
> + wm = intel_calculate_wm(pixel_rate, &pnv_cursor_wm,
> pnv_display_wm.fifo_size,
> 4, latency->cursor_sr);
> reg = intel_uncore_read(&dev_priv->uncore, DSPFW3);
> @@ -942,7 +940,7 @@ static void pnv_update_wm(struct drm_i915_private *dev_priv)
> intel_uncore_write(&dev_priv->uncore, DSPFW3, reg);
>
> /* Display HPLL off SR */
> - wm = intel_calculate_wm(clock, &pnv_display_hplloff_wm,
> + wm = intel_calculate_wm(pixel_rate, &pnv_display_hplloff_wm,
> pnv_display_hplloff_wm.fifo_size,
> cpp, latency->display_hpll_disable);
> reg = intel_uncore_read(&dev_priv->uncore, DSPFW3);
> @@ -951,7 +949,7 @@ static void pnv_update_wm(struct drm_i915_private *dev_priv)
> intel_uncore_write(&dev_priv->uncore, DSPFW3, reg);
>
> /* cursor HPLL off SR */
> - wm = intel_calculate_wm(clock, &pnv_cursor_hplloff_wm,
> + wm = intel_calculate_wm(pixel_rate, &pnv_cursor_hplloff_wm,
> pnv_display_hplloff_wm.fifo_size,
> 4, latency->cursor_hpll_disable);
> reg = intel_uncore_read(&dev_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 =
> &crtc_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(&plane_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 =
> &crtc_state->hw.pipe_mode;
> - unsigned int clock, htotal, cpp, width, wm;
> + unsigned int pixel_rate, htotal, cpp, width, wm;
>
> if (dev_priv->wm.pri_latency[level] == 0)
> return USHRT_MAX;
> @@ -1683,7 +1681,7 @@ static u16 vlv_compute_wm_level(const struct intel_crtc_state *crtc_state,
> return 0;
>
> cpp = plane_state->hw.fb->format->cpp[0];
> - clock = pipe_mode->crtc_clock;
> + pixel_rate = crtc_state->pixel_rate;
> htotal = pipe_mode->crtc_htotal;
> width = crtc_state->pipe_src_w;
>
> @@ -1696,7 +1694,7 @@ static u16 vlv_compute_wm_level(const struct intel_crtc_state *crtc_state,
> */
> wm = 63;
> } else {
> - wm = vlv_wm_method2(clock, htotal, width, cpp,
> + wm = vlv_wm_method2(pixel_rate, htotal, width, cpp,
> dev_priv->wm.pri_latency[level] * 10);
> }
>
> @@ -2277,13 +2275,13 @@ static void i965_update_wm(struct drm_i915_private *dev_priv)
> &crtc->config->hw.pipe_mode;
> const struct drm_framebuffer *fb =
> crtc->base.primary->state->fb;
> - int clock = pipe_mode->crtc_clock;
> + int pixel_rate = crtc->config->pixel_rate;
> int htotal = pipe_mode->crtc_htotal;
> int hdisplay = crtc->config->pipe_src_w;
> int cpp = fb->format->cpp[0];
> int entries;
>
> - entries = intel_wm_method2(clock, htotal,
> + entries = intel_wm_method2(pixel_rate, htotal,
> hdisplay, cpp, sr_latency_ns / 100);
> entries = DIV_ROUND_UP(entries, I915_FIFO_LINE_SIZE);
> srwm = I965_FIFO_SIZE - entries;
> @@ -2294,7 +2292,7 @@ static void i965_update_wm(struct drm_i915_private *dev_priv)
> "self-refresh entries: %d, wm: %d\n",
> entries, srwm);
>
> - entries = intel_wm_method2(clock, htotal,
> + entries = intel_wm_method2(pixel_rate, htotal,
> crtc->base.cursor->state->crtc_w, 4,
> sr_latency_ns / 100);
> entries = DIV_ROUND_UP(entries,
> @@ -2373,8 +2371,6 @@ static void i9xx_update_wm(struct drm_i915_private *dev_priv)
> fifo_size = i9xx_get_fifo_size(dev_priv, PLANE_A);
> crtc = intel_crtc_for_plane(dev_priv, PLANE_A);
> if (intel_crtc_active(crtc)) {
> - const struct drm_display_mode *pipe_mode =
> - &crtc->config->hw.pipe_mode;
> const struct drm_framebuffer *fb =
> crtc->base.primary->state->fb;
> int cpp;
> @@ -2384,7 +2380,7 @@ static void i9xx_update_wm(struct drm_i915_private *dev_priv)
> else
> cpp = fb->format->cpp[0];
>
> - planea_wm = intel_calculate_wm(pipe_mode->crtc_clock,
> + planea_wm = intel_calculate_wm(crtc->config->pixel_rate,
> wm_info, fifo_size, cpp,
> pessimal_latency_ns);
> enabled = crtc;
> @@ -2403,8 +2399,6 @@ static void i9xx_update_wm(struct drm_i915_private *dev_priv)
> fifo_size = i9xx_get_fifo_size(dev_priv, PLANE_B);
> crtc = intel_crtc_for_plane(dev_priv, PLANE_B);
> if (intel_crtc_active(crtc)) {
> - const struct drm_display_mode *pipe_mode =
> - &crtc->config->hw.pipe_mode;
> const struct drm_framebuffer *fb =
> crtc->base.primary->state->fb;
> int cpp;
> @@ -2414,7 +2408,7 @@ static void i9xx_update_wm(struct drm_i915_private *dev_priv)
> else
> cpp = fb->format->cpp[0];
>
> - planeb_wm = intel_calculate_wm(pipe_mode->crtc_clock,
> + planeb_wm = intel_calculate_wm(crtc->config->pixel_rate,
> wm_info, fifo_size, cpp,
> pessimal_latency_ns);
> if (enabled == NULL)
> @@ -2456,7 +2450,7 @@ static void i9xx_update_wm(struct drm_i915_private *dev_priv)
> &enabled->config->hw.pipe_mode;
> const struct drm_framebuffer *fb =
> enabled->base.primary->state->fb;
> - int clock = pipe_mode->crtc_clock;
> + int pixel_rate = enabled->config->pixel_rate;
> int htotal = pipe_mode->crtc_htotal;
> int hdisplay = enabled->config->pipe_src_w;
> int cpp;
> @@ -2467,7 +2461,7 @@ static void i9xx_update_wm(struct drm_i915_private *dev_priv)
> else
> cpp = fb->format->cpp[0];
>
> - entries = intel_wm_method2(clock, htotal, hdisplay, cpp,
> + entries = intel_wm_method2(pixel_rate, htotal, hdisplay, cpp,
> sr_latency_ns / 100);
> entries = DIV_ROUND_UP(entries, wm_info->cacheline_size);
> drm_dbg_kms(&dev_priv->drm,
> @@ -2504,7 +2498,6 @@ static void i9xx_update_wm(struct drm_i915_private *dev_priv)
> static void i845_update_wm(struct drm_i915_private *dev_priv)
> {
> struct intel_crtc *crtc;
> - const struct drm_display_mode *pipe_mode;
> u32 fwater_lo;
> int planea_wm;
>
> @@ -2512,8 +2505,7 @@ static void i845_update_wm(struct drm_i915_private *dev_priv)
> if (crtc == NULL)
> return;
>
> - pipe_mode = &crtc->config->hw.pipe_mode;
> - planea_wm = intel_calculate_wm(pipe_mode->crtc_clock,
> + planea_wm = intel_calculate_wm(crtc->config->pixel_rate,
> &i845_wm_info,
> i845_get_fifo_size(dev_priv, PLANE_A),
> 4, pessimal_latency_ns);
--
Jani Nikula, Intel Open Source Graphics Center
prev parent reply other threads:[~2022-01-26 14:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-09 14:43 [Intel-gfx] [PATCH 1/3] drm/i915: Fix up pixel_rate vs. clock confusion in wm calculations Ville Syrjala
2021-12-09 14:43 ` [Intel-gfx] [PATCH 2/3] drm/i915: Use the correct plane source width in watermark calculations Ville Syrjala
2021-12-09 14:43 ` [Intel-gfx] [PATCH 3/3] drm/i915: Use single_enabled_crtc() in i9xx_update_wm() Ville Syrjala
2021-12-10 3:23 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Fix up pixel_rate vs. clock confusion in wm calculations Patchwork
2021-12-10 12:37 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2022-01-26 14:50 ` Jani Nikula [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877damzh2m.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.