From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: <intel-gfx@lists.freedesktop.org>, <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH 5/6] drm/i915/power: Convert "i830 power well" code to intel_display
Date: Fri, 6 Sep 2024 11:13:30 -0400 [thread overview]
Message-ID: <ZtscGjH68EuueDMQ@intel.com> (raw)
In-Reply-To: <20240906143306.15937-6-ville.syrjala@linux.intel.com>
On Fri, Sep 06, 2024 at 05:33:05PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> struct intel_display will replace struct drm_i915_private as
> the main thing for display code. Convert the "i830 power well"
> code to use it (as much as possible at this stage).
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 79 +++++++++----------
> drivers/gpu/drm/i915/display/intel_display.h | 5 +-
> .../i915/display/intel_display_power_well.c | 22 ++++--
> 3 files changed, 56 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index b4ec9bf12aa7..0ec78b06ca80 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2226,9 +2226,10 @@ static void i9xx_pfit_disable(const struct intel_crtc_state *old_crtc_state)
> static void i9xx_crtc_disable(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> {
> + struct intel_display *display = to_intel_display(state);
> + struct drm_i915_private *dev_priv = to_i915(display->drm);
> struct intel_crtc_state *old_crtc_state =
> intel_atomic_get_old_crtc_state(state, crtc);
> - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum pipe pipe = crtc->pipe;
>
> /*
> @@ -2267,7 +2268,7 @@ static void i9xx_crtc_disable(struct intel_atomic_state *state,
>
> /* clock the pipe down to 640x480@60 to potentially save power */
> if (IS_I830(dev_priv))
> - i830_enable_pipe(dev_priv, pipe);
> + i830_enable_pipe(display, pipe);
> }
>
> void intel_encoder_destroy(struct drm_encoder *encoder)
> @@ -8257,9 +8258,8 @@ int intel_initial_commit(struct drm_device *dev)
> return ret;
> }
>
> -void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> +void i830_enable_pipe(struct intel_display *display, enum pipe pipe)
> {
> - struct intel_display *display = &dev_priv->display;
> struct intel_crtc *crtc = intel_crtc_for_pipe(display, pipe);
> enum transcoder cpu_transcoder = (enum transcoder)pipe;
> /* 640x480@60Hz, ~25175 kHz */
> @@ -8273,10 +8273,10 @@ void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> u32 dpll, fp;
> int i;
>
> - drm_WARN_ON(&dev_priv->drm,
> + drm_WARN_ON(display->drm,
> i9xx_calc_dpll_params(48000, &clock) != 25154);
>
> - drm_dbg_kms(&dev_priv->drm,
> + drm_dbg_kms(display->drm,
> "enabling pipe %c due to force quirk (vco=%d dot=%d)\n",
> pipe_name(pipe), clock.vco, clock.dot);
>
> @@ -8288,35 +8288,35 @@ void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> PLL_REF_INPUT_DREFCLK |
> DPLL_VCO_ENABLE;
>
> - intel_de_write(dev_priv, TRANS_HTOTAL(dev_priv, cpu_transcoder),
> + intel_de_write(display, TRANS_HTOTAL(display, cpu_transcoder),
> HACTIVE(640 - 1) | HTOTAL(800 - 1));
> - intel_de_write(dev_priv, TRANS_HBLANK(dev_priv, cpu_transcoder),
> + intel_de_write(display, TRANS_HBLANK(display, cpu_transcoder),
> HBLANK_START(640 - 1) | HBLANK_END(800 - 1));
> - intel_de_write(dev_priv, TRANS_HSYNC(dev_priv, cpu_transcoder),
> + intel_de_write(display, TRANS_HSYNC(display, cpu_transcoder),
> HSYNC_START(656 - 1) | HSYNC_END(752 - 1));
> - intel_de_write(dev_priv, TRANS_VTOTAL(dev_priv, cpu_transcoder),
> + intel_de_write(display, TRANS_VTOTAL(display, cpu_transcoder),
> VACTIVE(480 - 1) | VTOTAL(525 - 1));
> - intel_de_write(dev_priv, TRANS_VBLANK(dev_priv, cpu_transcoder),
> + intel_de_write(display, TRANS_VBLANK(display, cpu_transcoder),
> VBLANK_START(480 - 1) | VBLANK_END(525 - 1));
> - intel_de_write(dev_priv, TRANS_VSYNC(dev_priv, cpu_transcoder),
> + intel_de_write(display, TRANS_VSYNC(display, cpu_transcoder),
> VSYNC_START(490 - 1) | VSYNC_END(492 - 1));
> - intel_de_write(dev_priv, PIPESRC(dev_priv, pipe),
> + intel_de_write(display, PIPESRC(display, pipe),
> PIPESRC_WIDTH(640 - 1) | PIPESRC_HEIGHT(480 - 1));
>
> - intel_de_write(dev_priv, FP0(pipe), fp);
> - intel_de_write(dev_priv, FP1(pipe), fp);
> + intel_de_write(display, FP0(pipe), fp);
> + intel_de_write(display, FP1(pipe), fp);
>
> /*
> * Apparently we need to have VGA mode enabled prior to changing
> * the P1/P2 dividers. Otherwise the DPLL will keep using the old
> * dividers, even though the register value does change.
> */
> - intel_de_write(dev_priv, DPLL(dev_priv, pipe),
> + intel_de_write(display, DPLL(display, pipe),
> dpll & ~DPLL_VGA_MODE_DIS);
> - intel_de_write(dev_priv, DPLL(dev_priv, pipe), dpll);
> + intel_de_write(display, DPLL(display, pipe), dpll);
>
> /* Wait for the clocks to stabilize. */
> - intel_de_posting_read(dev_priv, DPLL(dev_priv, pipe));
> + intel_de_posting_read(display, DPLL(display, pipe));
> udelay(150);
>
> /* The pixel multiplier can only be updated once the
> @@ -8324,47 +8324,46 @@ void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> *
> * So write it again.
> */
> - intel_de_write(dev_priv, DPLL(dev_priv, pipe), dpll);
> + intel_de_write(display, DPLL(display, pipe), dpll);
>
> /* We do this three times for luck */
> for (i = 0; i < 3 ; i++) {
> - intel_de_write(dev_priv, DPLL(dev_priv, pipe), dpll);
> - intel_de_posting_read(dev_priv, DPLL(dev_priv, pipe));
> + intel_de_write(display, DPLL(display, pipe), dpll);
> + intel_de_posting_read(display, DPLL(display, pipe));
> udelay(150); /* wait for warmup */
> }
>
> - intel_de_write(dev_priv, TRANSCONF(dev_priv, pipe), TRANSCONF_ENABLE);
> - intel_de_posting_read(dev_priv, TRANSCONF(dev_priv, pipe));
> + intel_de_write(display, TRANSCONF(display, pipe), TRANSCONF_ENABLE);
> + intel_de_posting_read(display, TRANSCONF(display, pipe));
>
> intel_wait_for_pipe_scanline_moving(crtc);
> }
>
> -void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> +void i830_disable_pipe(struct intel_display *display, enum pipe pipe)
> {
> - struct intel_display *display = &dev_priv->display;
> struct intel_crtc *crtc = intel_crtc_for_pipe(display, pipe);
>
> - drm_dbg_kms(&dev_priv->drm, "disabling pipe %c due to force quirk\n",
> + drm_dbg_kms(display->drm, "disabling pipe %c due to force quirk\n",
> pipe_name(pipe));
>
> - drm_WARN_ON(&dev_priv->drm,
> - intel_de_read(dev_priv, DSPCNTR(dev_priv, PLANE_A)) & DISP_ENABLE);
> - drm_WARN_ON(&dev_priv->drm,
> - intel_de_read(dev_priv, DSPCNTR(dev_priv, PLANE_B)) & DISP_ENABLE);
> - drm_WARN_ON(&dev_priv->drm,
> - intel_de_read(dev_priv, DSPCNTR(dev_priv, PLANE_C)) & DISP_ENABLE);
> - drm_WARN_ON(&dev_priv->drm,
> - intel_de_read(dev_priv, CURCNTR(dev_priv, PIPE_A)) & MCURSOR_MODE_MASK);
> - drm_WARN_ON(&dev_priv->drm,
> - intel_de_read(dev_priv, CURCNTR(dev_priv, PIPE_B)) & MCURSOR_MODE_MASK);
> + drm_WARN_ON(display->drm,
> + intel_de_read(display, DSPCNTR(display, PLANE_A)) & DISP_ENABLE);
> + drm_WARN_ON(display->drm,
> + intel_de_read(display, DSPCNTR(display, PLANE_B)) & DISP_ENABLE);
> + drm_WARN_ON(display->drm,
> + intel_de_read(display, DSPCNTR(display, PLANE_C)) & DISP_ENABLE);
> + drm_WARN_ON(display->drm,
> + intel_de_read(display, CURCNTR(display, PIPE_A)) & MCURSOR_MODE_MASK);
> + drm_WARN_ON(display->drm,
> + intel_de_read(display, CURCNTR(display, PIPE_B)) & MCURSOR_MODE_MASK);
>
> - intel_de_write(dev_priv, TRANSCONF(dev_priv, pipe), 0);
> - intel_de_posting_read(dev_priv, TRANSCONF(dev_priv, pipe));
> + intel_de_write(display, TRANSCONF(display, pipe), 0);
> + intel_de_posting_read(display, TRANSCONF(display, pipe));
>
> intel_wait_for_pipe_scanline_stopped(crtc);
>
> - intel_de_write(dev_priv, DPLL(dev_priv, pipe), DPLL_VGA_MODE_DIS);
> - intel_de_posting_read(dev_priv, DPLL(dev_priv, pipe));
> + intel_de_write(display, DPLL(display, pipe), DPLL_VGA_MODE_DIS);
> + intel_de_posting_read(display, DPLL(display, pipe));
> }
>
> void intel_hpd_poll_fini(struct drm_i915_private *i915)
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index b21d9578d5db..7ca26e5cb20e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -52,6 +52,7 @@ struct intel_atomic_state;
> struct intel_crtc;
> struct intel_crtc_state;
> struct intel_digital_port;
> +struct intel_display;
> struct intel_dp;
> struct intel_encoder;
> struct intel_initial_plane_config;
> @@ -437,8 +438,8 @@ void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state);
> void ilk_set_pipeconf(const struct intel_crtc_state *crtc_state);
> void intel_enable_transcoder(const struct intel_crtc_state *new_crtc_state);
> void intel_disable_transcoder(const struct intel_crtc_state *old_crtc_state);
> -void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
> -void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
> +void i830_enable_pipe(struct intel_display *display, enum pipe pipe);
> +void i830_disable_pipe(struct intel_display *display, enum pipe pipe);
> int vlv_get_hpll_vco(struct drm_i915_private *dev_priv);
> int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> const char *name, u32 reg, int ref_freq);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> index a5d9b17e03a2..9f275a6674a1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> @@ -1066,24 +1066,30 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
> static void i830_pipes_power_well_enable(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> - if ((intel_de_read(dev_priv, TRANSCONF(dev_priv, PIPE_A)) & TRANSCONF_ENABLE) == 0)
> - i830_enable_pipe(dev_priv, PIPE_A);
> - if ((intel_de_read(dev_priv, TRANSCONF(dev_priv, PIPE_B)) & TRANSCONF_ENABLE) == 0)
> - i830_enable_pipe(dev_priv, PIPE_B);
> + struct intel_display *display = &dev_priv->display;
> +
> + if ((intel_de_read(display, TRANSCONF(dev_priv, PIPE_A)) & TRANSCONF_ENABLE) == 0)
> + i830_enable_pipe(display, PIPE_A);
> + if ((intel_de_read(display, TRANSCONF(dev_priv, PIPE_B)) & TRANSCONF_ENABLE) == 0)
> + i830_enable_pipe(display, PIPE_B);
> }
>
> static void i830_pipes_power_well_disable(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> - i830_disable_pipe(dev_priv, PIPE_B);
> - i830_disable_pipe(dev_priv, PIPE_A);
> + struct intel_display *display = &dev_priv->display;
> +
> + i830_disable_pipe(display, PIPE_B);
> + i830_disable_pipe(display, PIPE_A);
> }
>
> static bool i830_pipes_power_well_enabled(struct drm_i915_private *dev_priv,
> struct i915_power_well *power_well)
> {
> - return intel_de_read(dev_priv, TRANSCONF(dev_priv, PIPE_A)) & TRANSCONF_ENABLE &&
> - intel_de_read(dev_priv, TRANSCONF(dev_priv, PIPE_B)) & TRANSCONF_ENABLE;
> + struct intel_display *display = &dev_priv->display;
> +
> + return intel_de_read(display, TRANSCONF(dev_priv, PIPE_A)) & TRANSCONF_ENABLE &&
> + intel_de_read(display, TRANSCONF(dev_priv, PIPE_B)) & TRANSCONF_ENABLE;
> }
>
> static void i830_pipes_power_well_sync_hw(struct drm_i915_private *dev_priv,
> --
> 2.44.2
>
next prev parent reply other threads:[~2024-09-06 15:13 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-06 14:33 [PATCH 0/6] drm/i915: Some intel_display conversions Ville Syrjala
2024-09-06 14:33 ` [PATCH 1/6] drm/i915/cdclk: Add missing braces Ville Syrjala
2024-09-06 15:10 ` Rodrigo Vivi
2024-09-06 15:19 ` Jani Nikula
2024-09-06 14:33 ` [PATCH 2/6] drm/i915/cdclk: Convert CDCLK code to intel_display Ville Syrjala
2024-09-06 15:09 ` Rodrigo Vivi
2024-09-06 15:18 ` Jani Nikula
2024-09-06 16:17 ` Ville Syrjälä
2024-09-06 14:33 ` [PATCH 3/6] drm/i915/power: Convert low level DC state " Ville Syrjala
2024-09-06 15:10 ` Rodrigo Vivi
2024-09-06 14:33 ` [PATCH 4/6] drm/i915/vga: Convert VGA " Ville Syrjala
2024-09-06 15:12 ` Rodrigo Vivi
2024-09-06 14:33 ` [PATCH 5/6] drm/i915/power: Convert "i830 power well" " Ville Syrjala
2024-09-06 15:13 ` Rodrigo Vivi [this message]
2024-09-06 14:33 ` [PATCH 6/6] drm/i915/dmc: Convert DMC " Ville Syrjala
2024-09-06 15:16 ` Rodrigo Vivi
2024-09-06 15:43 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Some intel_display conversions Patchwork
2024-09-06 15:43 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-09-06 16:08 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-09-09 14:15 ` Ville Syrjälä
2024-09-10 8:14 ` Illipilli, TejasreeX
2024-09-10 5:34 ` ✓ Fi.CI.BAT: success " Patchwork
2024-09-11 0:54 ` ✗ Fi.CI.IGT: failure " Patchwork
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=ZtscGjH68EuueDMQ@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox