From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: <intel-gfx@lists.freedesktop.org>,
<intel-xe@lists.freedesktop.org>,
"Luca Coelho" <luciano.coelho@intel.com>
Subject: Re: [PATCH v4 7/9] drm/i915/dmc: convert dmc wakelock interface to struct intel_display
Date: Wed, 17 Apr 2024 10:31:28 -0400 [thread overview]
Message-ID: <Zh_dQKwAERyclZYy@intel.com> (raw)
In-Reply-To: <3c260bbbce0af8714b07157dc032b038efa3bf1c.1713358679.git.jani.nikula@intel.com>
On Wed, Apr 17, 2024 at 04:02:45PM +0300, Jani Nikula wrote:
> Convert the dmc wakelock interface to struct intel_display instead of
> struct drm_i915_private. We'll want to convert the intel_de interfaces,
> and there's a bit of coupling between the two, so start here.
>
> Cc: Luca Coelho <luciano.coelho@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_de.h | 40 ++++++++--------
> .../drm/i915/display/intel_display_driver.c | 2 +-
> .../i915/display/intel_display_power_well.c | 6 +--
> drivers/gpu/drm/i915/display/intel_dmc.c | 4 +-
> drivers/gpu/drm/i915/display/intel_dmc_wl.c | 48 +++++++++++--------
> drivers/gpu/drm/i915/display/intel_dmc_wl.h | 12 ++---
> 6 files changed, 59 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_de.h b/drivers/gpu/drm/i915/display/intel_de.h
> index 4b51388c6041..15440058ad2a 100644
> --- a/drivers/gpu/drm/i915/display/intel_de.h
> +++ b/drivers/gpu/drm/i915/display/intel_de.h
> @@ -15,11 +15,11 @@ intel_de_read(struct drm_i915_private *i915, i915_reg_t reg)
> {
> u32 val;
>
> - intel_dmc_wl_get(i915, reg);
> + intel_dmc_wl_get(&i915->display, reg);
>
> val = intel_uncore_read(&i915->uncore, reg);
>
> - intel_dmc_wl_put(i915, reg);
> + intel_dmc_wl_put(&i915->display, reg);
>
> return val;
> }
> @@ -29,11 +29,11 @@ intel_de_read8(struct drm_i915_private *i915, i915_reg_t reg)
> {
> u8 val;
>
> - intel_dmc_wl_get(i915, reg);
> + intel_dmc_wl_get(&i915->display, reg);
>
> val = intel_uncore_read8(&i915->uncore, reg);
>
> - intel_dmc_wl_put(i915, reg);
> + intel_dmc_wl_put(&i915->display, reg);
>
> return val;
> }
> @@ -44,13 +44,13 @@ intel_de_read64_2x32(struct drm_i915_private *i915,
> {
> u64 val;
>
> - intel_dmc_wl_get(i915, lower_reg);
> - intel_dmc_wl_get(i915, upper_reg);
> + intel_dmc_wl_get(&i915->display, lower_reg);
> + intel_dmc_wl_get(&i915->display, upper_reg);
>
> val = intel_uncore_read64_2x32(&i915->uncore, lower_reg, upper_reg);
>
> - intel_dmc_wl_put(i915, upper_reg);
> - intel_dmc_wl_put(i915, lower_reg);
> + intel_dmc_wl_put(&i915->display, upper_reg);
> + intel_dmc_wl_put(&i915->display, lower_reg);
>
> return val;
> }
> @@ -58,21 +58,21 @@ intel_de_read64_2x32(struct drm_i915_private *i915,
> static inline void
> intel_de_posting_read(struct drm_i915_private *i915, i915_reg_t reg)
> {
> - intel_dmc_wl_get(i915, reg);
> + intel_dmc_wl_get(&i915->display, reg);
>
> intel_uncore_posting_read(&i915->uncore, reg);
>
> - intel_dmc_wl_put(i915, reg);
> + intel_dmc_wl_put(&i915->display, reg);
> }
>
> static inline void
> intel_de_write(struct drm_i915_private *i915, i915_reg_t reg, u32 val)
> {
> - intel_dmc_wl_get(i915, reg);
> + intel_dmc_wl_get(&i915->display, reg);
>
> intel_uncore_write(&i915->uncore, reg, val);
>
> - intel_dmc_wl_put(i915, reg);
> + intel_dmc_wl_put(&i915->display, reg);
> }
>
> static inline u32
> @@ -87,11 +87,11 @@ intel_de_rmw(struct drm_i915_private *i915, i915_reg_t reg, u32 clear, u32 set)
> {
> u32 val;
>
> - intel_dmc_wl_get(i915, reg);
> + intel_dmc_wl_get(&i915->display, reg);
>
> val = __intel_de_rmw_nowl(i915, reg, clear, set);
>
> - intel_dmc_wl_put(i915, reg);
> + intel_dmc_wl_put(&i915->display, reg);
>
> return val;
> }
> @@ -110,11 +110,11 @@ intel_de_wait(struct drm_i915_private *i915, i915_reg_t reg,
> {
> int ret;
>
> - intel_dmc_wl_get(i915, reg);
> + intel_dmc_wl_get(&i915->display, reg);
>
> ret = __intel_de_wait_for_register_nowl(i915, reg, mask, value, timeout);
>
> - intel_dmc_wl_put(i915, reg);
> + intel_dmc_wl_put(&i915->display, reg);
>
> return ret;
> }
> @@ -125,11 +125,11 @@ intel_de_wait_fw(struct drm_i915_private *i915, i915_reg_t reg,
> {
> int ret;
>
> - intel_dmc_wl_get(i915, reg);
> + intel_dmc_wl_get(&i915->display, reg);
>
> ret = intel_wait_for_register_fw(&i915->uncore, reg, mask, value, timeout);
>
> - intel_dmc_wl_put(i915, reg);
> + intel_dmc_wl_put(&i915->display, reg);
>
> return ret;
> }
> @@ -142,12 +142,12 @@ intel_de_wait_custom(struct drm_i915_private *i915, i915_reg_t reg,
> {
> int ret;
>
> - intel_dmc_wl_get(i915, reg);
> + intel_dmc_wl_get(&i915->display, reg);
>
> ret = __intel_wait_for_register(&i915->uncore, reg, mask, value,
> fast_timeout_us, slow_timeout_ms, out_value);
>
> - intel_dmc_wl_put(i915, reg);
> + intel_dmc_wl_put(&i915->display, reg);
>
> return ret;
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 4f112a69dea8..1b24339e4ab6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -198,7 +198,7 @@ void intel_display_driver_early_probe(struct drm_i915_private *i915)
> intel_dpll_init_clock_hook(i915);
> intel_init_display_hooks(i915);
> intel_fdi_init_hook(i915);
> - intel_dmc_wl_init(i915);
> + intel_dmc_wl_init(&i915->display);
> }
>
> /* part #1: call before irq install */
> 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 7f4b7602cf02..a28e61130b81 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> @@ -822,7 +822,7 @@ void gen9_enable_dc5(struct drm_i915_private *dev_priv)
> intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1,
> 0, SKL_SELECT_ALTERNATE_DC_EXIT);
>
> - intel_dmc_wl_enable(dev_priv);
> + intel_dmc_wl_enable(&dev_priv->display);
>
> gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5);
> }
> @@ -853,7 +853,7 @@ void skl_enable_dc6(struct drm_i915_private *dev_priv)
> intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1,
> 0, SKL_SELECT_ALTERNATE_DC_EXIT);
>
> - intel_dmc_wl_enable(dev_priv);
> + intel_dmc_wl_enable(&dev_priv->display);
>
> gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6);
> }
> @@ -975,7 +975,7 @@ void gen9_disable_dc_states(struct drm_i915_private *dev_priv)
> if (!HAS_DISPLAY(dev_priv))
> return;
>
> - intel_dmc_wl_disable(dev_priv);
> + intel_dmc_wl_disable(&dev_priv->display);
>
> intel_cdclk_get_cdclk(dev_priv, &cdclk_config);
> /* Can't read out voltage_level so can't use intel_cdclk_changed() */
> diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
> index a34ff3383fd3..3697a02b51b6 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> @@ -553,7 +553,7 @@ void intel_dmc_disable_program(struct drm_i915_private *i915)
> disable_all_event_handlers(i915);
> pipedmc_clock_gating_wa(i915, false);
>
> - intel_dmc_wl_disable(i915);
> + intel_dmc_wl_disable(&i915->display);
> }
>
> void assert_dmc_loaded(struct drm_i915_private *i915)
> @@ -1083,7 +1083,7 @@ void intel_dmc_suspend(struct drm_i915_private *i915)
> if (dmc)
> flush_work(&dmc->work);
>
> - intel_dmc_wl_disable(i915);
> + intel_dmc_wl_disable(&i915->display);
>
> /* Drop the reference held in case DMC isn't loaded. */
> if (!intel_dmc_has_payload(i915))
> diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> index 162de0d20554..e79c45e36722 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> @@ -51,9 +51,10 @@ static struct intel_dmc_wl_range lnl_wl_range[] = {
> { .start = 0x60000, .end = 0x7ffff },
> };
>
> -static void __intel_dmc_wl_release(struct drm_i915_private *i915)
> +static void __intel_dmc_wl_release(struct intel_display *display)
> {
> - struct intel_dmc_wl *wl = &i915->display.wl;
> + struct drm_i915_private *i915 = to_i915(display->drm);
> + struct intel_dmc_wl *wl = &display->wl;
>
> WARN_ON(refcount_read(&wl->refcount));
>
> @@ -106,23 +107,25 @@ static bool intel_dmc_wl_check_range(u32 address)
> return wl_needed;
> }
>
> -static bool __intel_dmc_wl_supported(struct drm_i915_private *i915)
> +static bool __intel_dmc_wl_supported(struct intel_display *display)
> {
> + struct drm_i915_private *i915 = to_i915(display->drm);
> +
> if (DISPLAY_VER(i915) < 20 ||
> !intel_dmc_has_payload(i915) ||
> - !i915->display.params.enable_dmc_wl)
> + !display->params.enable_dmc_wl)
> return false;
>
> return true;
> }
>
> -void intel_dmc_wl_init(struct drm_i915_private *i915)
> +void intel_dmc_wl_init(struct intel_display *display)
> {
> - struct intel_dmc_wl *wl = &i915->display.wl;
> + struct drm_i915_private *i915 = to_i915(display->drm);
> + struct intel_dmc_wl *wl = &display->wl;
>
> /* don't call __intel_dmc_wl_supported(), DMC is not loaded yet */
> - if (DISPLAY_VER(i915) < 20 ||
> - !i915->display.params.enable_dmc_wl)
> + if (DISPLAY_VER(i915) < 20 || !display->params.enable_dmc_wl)
> return;
>
> INIT_DELAYED_WORK(&wl->work, intel_dmc_wl_work);
> @@ -130,12 +133,13 @@ void intel_dmc_wl_init(struct drm_i915_private *i915)
> refcount_set(&wl->refcount, 0);
> }
>
> -void intel_dmc_wl_enable(struct drm_i915_private *i915)
> +void intel_dmc_wl_enable(struct intel_display *display)
> {
> - struct intel_dmc_wl *wl = &i915->display.wl;
> + struct drm_i915_private *i915 = to_i915(display->drm);
> + struct intel_dmc_wl *wl = &display->wl;
> unsigned long flags;
>
> - if (!__intel_dmc_wl_supported(i915))
> + if (!__intel_dmc_wl_supported(display))
> return;
>
> spin_lock_irqsave(&wl->lock, flags);
> @@ -157,12 +161,13 @@ void intel_dmc_wl_enable(struct drm_i915_private *i915)
> spin_unlock_irqrestore(&wl->lock, flags);
> }
>
> -void intel_dmc_wl_disable(struct drm_i915_private *i915)
> +void intel_dmc_wl_disable(struct intel_display *display)
> {
> - struct intel_dmc_wl *wl = &i915->display.wl;
> + struct drm_i915_private *i915 = to_i915(display->drm);
> + struct intel_dmc_wl *wl = &display->wl;
> unsigned long flags;
>
> - if (!__intel_dmc_wl_supported(i915))
> + if (!__intel_dmc_wl_supported(display))
> return;
>
> flush_delayed_work(&wl->work);
> @@ -183,12 +188,13 @@ void intel_dmc_wl_disable(struct drm_i915_private *i915)
> spin_unlock_irqrestore(&wl->lock, flags);
> }
>
> -void intel_dmc_wl_get(struct drm_i915_private *i915, i915_reg_t reg)
> +void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg)
> {
> - struct intel_dmc_wl *wl = &i915->display.wl;
> + struct drm_i915_private *i915 = to_i915(display->drm);
> + struct intel_dmc_wl *wl = &display->wl;
> unsigned long flags;
>
> - if (!__intel_dmc_wl_supported(i915))
> + if (!__intel_dmc_wl_supported(display))
> return;
>
> if (!intel_dmc_wl_check_range(reg.reg))
> @@ -231,12 +237,12 @@ void intel_dmc_wl_get(struct drm_i915_private *i915, i915_reg_t reg)
> spin_unlock_irqrestore(&wl->lock, flags);
> }
>
> -void intel_dmc_wl_put(struct drm_i915_private *i915, i915_reg_t reg)
> +void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg)
> {
> - struct intel_dmc_wl *wl = &i915->display.wl;
> + struct intel_dmc_wl *wl = &display->wl;
> unsigned long flags;
>
> - if (!__intel_dmc_wl_supported(i915))
> + if (!__intel_dmc_wl_supported(display))
> return;
>
> if (!intel_dmc_wl_check_range(reg.reg))
> @@ -252,7 +258,7 @@ void intel_dmc_wl_put(struct drm_i915_private *i915, i915_reg_t reg)
> goto out_unlock;
>
> if (refcount_dec_and_test(&wl->refcount)) {
> - __intel_dmc_wl_release(i915);
> + __intel_dmc_wl_release(display);
>
> goto out_unlock;
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.h b/drivers/gpu/drm/i915/display/intel_dmc_wl.h
> index 6fb86b05b437..adab51208d0a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.h
> +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.h
> @@ -12,7 +12,7 @@
>
> #include "i915_reg_defs.h"
>
> -struct drm_i915_private;
> +struct intel_display;
>
> struct intel_dmc_wl {
> spinlock_t lock; /* protects enabled, taken and refcount */
> @@ -22,10 +22,10 @@ struct intel_dmc_wl {
> struct delayed_work work;
> };
>
> -void intel_dmc_wl_init(struct drm_i915_private *i915);
> -void intel_dmc_wl_enable(struct drm_i915_private *i915);
> -void intel_dmc_wl_disable(struct drm_i915_private *i915);
> -void intel_dmc_wl_get(struct drm_i915_private *i915, i915_reg_t reg);
> -void intel_dmc_wl_put(struct drm_i915_private *i915, i915_reg_t reg);
> +void intel_dmc_wl_init(struct intel_display *display);
> +void intel_dmc_wl_enable(struct intel_display *display);
> +void intel_dmc_wl_disable(struct intel_display *display);
> +void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg);
> +void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg);
>
> #endif /* __INTEL_WAKELOCK_H__ */
> --
> 2.39.2
>
next prev parent reply other threads:[~2024-04-17 14:31 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-17 13:02 [PATCH v4 0/9] drm/i915: better high level abstraction for display Jani Nikula
2024-04-17 13:02 ` [PATCH v4 1/9] drm/i915/display: add intel_display -> drm_device backpointer Jani Nikula
2024-04-17 13:02 ` [PATCH v4 2/9] drm/i915/display: add generic to_intel_display() macro Jani Nikula
2024-04-17 13:02 ` [PATCH v4 3/9] drm/i915: add generic __to_intel_display() Jani Nikula
2024-04-17 13:02 ` [PATCH v4 4/9] drm/i915/display: accept either i915 or display for feature tests Jani Nikula
2024-04-17 13:58 ` Rodrigo Vivi
2024-04-17 13:02 ` [PATCH v4 5/9] drm/i915/quirks: convert struct drm_i915_private to struct intel_display Jani Nikula
2024-04-17 13:02 ` [PATCH v4 6/9] drm/i915/display: rename __intel_wait_for_register_nowl() to indicate intel_de_ Jani Nikula
2024-04-17 13:59 ` Rodrigo Vivi
2024-04-17 13:02 ` [PATCH v4 7/9] drm/i915/dmc: convert dmc wakelock interface to struct intel_display Jani Nikula
2024-04-17 14:31 ` Rodrigo Vivi [this message]
2024-04-17 13:02 ` [PATCH v4 8/9] drm/i915/de: allow intel_display and drm_i915_private for de functions Jani Nikula
2024-04-17 14:00 ` Rodrigo Vivi
2024-04-17 13:02 ` [PATCH v4 9/9] drm/i915/dmc: use struct intel_display more Jani Nikula
2024-04-17 14:31 ` Rodrigo Vivi
2024-04-18 18:21 ` Jani Nikula
2024-04-17 13:26 ` ✓ CI.Patch_applied: success for drm/i915: better high level abstraction for display (rev3) Patchwork
2024-04-17 13:26 ` ✗ CI.checkpatch: warning " Patchwork
2024-04-17 13:27 ` ✓ CI.KUnit: success " Patchwork
2024-04-17 13:39 ` ✓ CI.Build: " Patchwork
2024-04-17 13:42 ` ✓ CI.Hooks: " Patchwork
2024-04-17 14:00 ` ✗ CI.checksparse: warning " Patchwork
2024-04-17 14:25 ` ✓ CI.BAT: success " Patchwork
2024-04-17 14:25 ` ✗ Fi.CI.CHECKPATCH: warning " Patchwork
2024-04-17 14:25 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-04-17 14:38 ` ✓ Fi.CI.BAT: success " Patchwork
2024-04-18 11:25 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-04-18 23:43 ` ✗ CI.FULL: " 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=Zh_dQKwAERyclZYy@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
--cc=luciano.coelho@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.