* [PATCH 5.1 stable v2] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
@ 2019-06-03 10:09 Jian-Hong Pan
2019-06-10 6:01 ` [PATCH stable-5.1 0/3] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-03 10:09 UTC (permalink / raw)
To: stable
Cc: tiwai, drake, linux, Ville Syrjälä,
Abhay Kumar, Imre Deak, Jian-Hong Pan
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
commit 905801fe72377b4dc53c6e13eea1a91c6a4aa0c4 upstream.
CDCLK has to be at least twice the BLCK regardless of audio. Audio
driver has to probe using this hook and increase the clock even in
absence of any display.
v2: Use atomic refcount for get_power, put_power so that we can
call each once(Abhay).
v3: Reset power well 2 to avoid any transaction on iDisp link
during cdclk change(Abhay).
v4: Remove Power well 2 reset workaround(Ville).
v5: Remove unwanted Power well 2 register defined in v4(Abhay).
v6:
- Use a dedicated flag instead of state->modeset for min CDCLK changes
- Make get/put audio power domain symmetric
- Rebased on top of intel_wakeref tracking changes.
[jian-hong@endlessm.com: Rediff to keep i915_audio_component_get_power
and i915_audio_component_put_power for Linux stable 5.1.x]
Cc: <stable@vger.kernel.org> # 5.1.x
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
Tested-by: Abhay Kumar <abhay.kumar@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190320135439.12201-1-imre.deak@intel.com
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
---
v2: Use intel_display_power_put_unchecked() instead of
intel_display_power_put_power() in i915_audio_component_put_power() for
balance.
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/intel_audio.c | 62 ++++++++++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_cdclk.c | 30 ++++++-----------
drivers/gpu/drm/i915/intel_display.c | 9 +++++-
drivers/gpu/drm/i915/intel_drv.h | 3 ++
5 files changed, 83 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a67a63b5aa84..5c9bb1b2d1f3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1622,6 +1622,8 @@ struct drm_i915_private {
struct intel_cdclk_state actual;
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+
+ int force_min_cdclk;
} cdclk;
/**
@@ -1741,6 +1743,7 @@ struct drm_i915_private {
*
*/
struct mutex av_mutex;
+ int audio_power_refcount;
struct {
struct mutex mutex;
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 5104c6bbd66f..2f3fd5bf0f11 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -741,15 +741,71 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
}
}
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+ bool enable)
+{
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ int ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+ state = drm_atomic_state_alloc(&dev_priv->drm);
+ if (WARN_ON(!state))
+ return;
+
+ state->acquire_ctx = &ctx;
+
+retry:
+ to_intel_atomic_state(state)->cdclk.force_min_cdclk_changed = true;
+ to_intel_atomic_state(state)->cdclk.force_min_cdclk =
+ enable ? 2 * 96000 : 0;
+
+ /*
+ * Protects dev_priv->cdclk.force_min_cdclk
+ * Need to lock this here in case we have no active pipes
+ * and thus wouldn't lock it during the commit otherwise.
+ */
+ ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
+ &ctx);
+ if (!ret)
+ ret = drm_atomic_commit(state);
+
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ drm_modeset_backoff(&ctx);
+ goto retry;
+ }
+
+ WARN_ON(ret);
+
+ drm_atomic_state_put(state);
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
static void i915_audio_component_get_power(struct device *kdev)
{
- intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
+ struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+ intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
+
+ /* Force CDCLK to 2*BCLK as long as we need audio to be powered. */
+ if (dev_priv->audio_power_refcount++ == 0)
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ glk_force_audio_cdclk(dev_priv, true);
}
static void i915_audio_component_put_power(struct device *kdev)
{
- intel_display_power_put_unchecked(kdev_to_i915(kdev),
- POWER_DOMAIN_AUDIO);
+ struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+ /* Stop forcing CDCLK to 2*BCLK if no need for audio to be powered. */
+ if (--dev_priv->audio_power_refcount == 0)
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ glk_force_audio_cdclk(dev_priv, false);
+
+ intel_display_power_put_unchecked(dev_priv, POWER_DOMAIN_AUDIO);
}
static void i915_audio_component_codec_wake_override(struct device *kdev,
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 15ba950dee00..553f57ff60f4 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2187,19 +2187,8 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
/*
* According to BSpec, "The CD clock frequency must be at least twice
* the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
- *
- * FIXME: Check the actual, not default, BCLK being used.
- *
- * FIXME: This does not depend on ->has_audio because the higher CDCLK
- * is required for audio probe, also when there are no audio capable
- * displays connected at probe time. This leads to unnecessarily high
- * CDCLK when audio is not required.
- *
- * FIXME: This limit is only applied when there are displays connected
- * at probe time. If we probe without displays, we'll still end up using
- * the platform minimum CDCLK, failing audio probe.
*/
- if (INTEL_GEN(dev_priv) >= 9)
+ if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
min_cdclk = max(2 * 96000, min_cdclk);
/*
@@ -2239,7 +2228,7 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
intel_state->min_cdclk[i] = min_cdclk;
}
- min_cdclk = 0;
+ min_cdclk = intel_state->cdclk.force_min_cdclk;
for_each_pipe(dev_priv, pipe)
min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
@@ -2300,7 +2289,8 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
vlv_calc_voltage_level(dev_priv, cdclk);
if (!intel_state->active_crtcs) {
- cdclk = vlv_calc_cdclk(dev_priv, 0);
+ cdclk = vlv_calc_cdclk(dev_priv,
+ intel_state->cdclk.force_min_cdclk);
intel_state->cdclk.actual.cdclk = cdclk;
intel_state->cdclk.actual.voltage_level =
@@ -2333,7 +2323,7 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
bdw_calc_voltage_level(cdclk);
if (!intel_state->active_crtcs) {
- cdclk = bdw_calc_cdclk(0);
+ cdclk = bdw_calc_cdclk(intel_state->cdclk.force_min_cdclk);
intel_state->cdclk.actual.cdclk = cdclk;
intel_state->cdclk.actual.voltage_level =
@@ -2405,7 +2395,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
skl_calc_voltage_level(cdclk);
if (!intel_state->active_crtcs) {
- cdclk = skl_calc_cdclk(0, vco);
+ cdclk = skl_calc_cdclk(intel_state->cdclk.force_min_cdclk, vco);
intel_state->cdclk.actual.vco = vco;
intel_state->cdclk.actual.cdclk = cdclk;
@@ -2444,10 +2434,10 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
if (!intel_state->active_crtcs) {
if (IS_GEMINILAKE(dev_priv)) {
- cdclk = glk_calc_cdclk(0);
+ cdclk = glk_calc_cdclk(intel_state->cdclk.force_min_cdclk);
vco = glk_de_pll_vco(dev_priv, cdclk);
} else {
- cdclk = bxt_calc_cdclk(0);
+ cdclk = bxt_calc_cdclk(intel_state->cdclk.force_min_cdclk);
vco = bxt_de_pll_vco(dev_priv, cdclk);
}
@@ -2483,7 +2473,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
cnl_compute_min_voltage_level(intel_state));
if (!intel_state->active_crtcs) {
- cdclk = cnl_calc_cdclk(0);
+ cdclk = cnl_calc_cdclk(intel_state->cdclk.force_min_cdclk);
vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
intel_state->cdclk.actual.vco = vco;
@@ -2519,7 +2509,7 @@ static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
cnl_compute_min_voltage_level(intel_state));
if (!intel_state->active_crtcs) {
- cdclk = icl_calc_cdclk(0, ref);
+ cdclk = icl_calc_cdclk(intel_state->cdclk.force_min_cdclk, ref);
vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
intel_state->cdclk.actual.vco = vco;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 421aac80a838..ebbac873b8f4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12769,6 +12769,11 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
return -EINVAL;
}
+ /* keep the current setting */
+ if (!intel_state->cdclk.force_min_cdclk_changed)
+ intel_state->cdclk.force_min_cdclk =
+ dev_priv->cdclk.force_min_cdclk;
+
intel_state->modeset = true;
intel_state->active_crtcs = dev_priv->active_crtcs;
intel_state->cdclk.logical = dev_priv->cdclk.logical;
@@ -12864,7 +12869,7 @@ static int intel_atomic_check(struct drm_device *dev,
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state, *crtc_state;
int ret, i;
- bool any_ms = false;
+ bool any_ms = intel_state->cdclk.force_min_cdclk_changed;
/* Catch I915_MODE_FLAG_INHERITED */
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
@@ -13456,6 +13461,8 @@ static int intel_atomic_commit(struct drm_device *dev,
dev_priv->active_crtcs = intel_state->active_crtcs;
dev_priv->cdclk.logical = intel_state->cdclk.logical;
dev_priv->cdclk.actual = intel_state->cdclk.actual;
+ dev_priv->cdclk.force_min_cdclk =
+ intel_state->cdclk.force_min_cdclk;
}
drm_atomic_state_get(state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d5660ac1b0d6..539caca05da4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -480,6 +480,9 @@ struct intel_atomic_state {
* state only when all crtc's are DPMS off.
*/
struct intel_cdclk_state actual;
+
+ int force_min_cdclk;
+ bool force_min_cdclk_changed;
} cdclk;
bool dpll_set, modeset;
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH stable-5.1 0/3] drm/i915: Prevent screen from flickering when the CDCLK changes
2019-06-03 10:09 [PATCH 5.1 stable v2] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled Jian-Hong Pan
@ 2019-06-10 6:01 ` Jian-Hong Pan
2019-06-13 7:52 ` Greg KH
2019-06-10 6:01 ` [PATCH stable-5.1 1/3] drm/i915: Save the old CDCLK atomic state Jian-Hong Pan
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-10 6:01 UTC (permalink / raw)
To: stable; +Cc: linux, hui.wang, Jian-Hong Pan
Hi,
After apply the commit "drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio
power is enabled", it induces the screen to flicker when the CDCLK changes on
the laptop like ASUS E406MA. [1]
So, we need these commits to prevent that:
commit 48d9f87ddd21 drm/i915: Save the old CDCLK atomic state
commit 2b21dfbeee72 drm/i915: Remove redundant store of logical CDCLK state
commit 59f9e9cab3a1 drm/i915: Skip modeset for cdclk changes if possible
[1]: https://bugzilla.kernel.org/show_bug.cgi?id=203623#c12
Jian-Hong Pan
Imre Deak (2):
drm/i915: Save the old CDCLK atomic state
drm/i915: Remove redundant store of logical CDCLK state
Ville Syrjälä (1):
drm/i915: Skip modeset for cdclk changes if possible
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/intel_cdclk.c | 155 ++++++++++++++++++++++-----
drivers/gpu/drm/i915/intel_display.c | 48 +++++++--
drivers/gpu/drm/i915/intel_drv.h | 18 +++-
4 files changed, 186 insertions(+), 38 deletions(-)
--
2.22.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH stable-5.1 1/3] drm/i915: Save the old CDCLK atomic state
2019-06-03 10:09 [PATCH 5.1 stable v2] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled Jian-Hong Pan
2019-06-10 6:01 ` [PATCH stable-5.1 0/3] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
@ 2019-06-10 6:01 ` Jian-Hong Pan
2019-06-20 14:19 ` Greg KH
2019-06-10 6:01 ` [PATCH stable-5.1 2/3] drm/i915: Remove redundant store of logical CDCLK state Jian-Hong Pan
2019-06-10 6:01 ` [PATCH stable-5.1 3/3] drm/i915: Skip modeset for cdclk changes if possible Jian-Hong Pan
3 siblings, 1 reply; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-10 6:01 UTC (permalink / raw)
To: stable; +Cc: linux, hui.wang, Imre Deak, Ville Syrjälä, Jian-Hong Pan
From: Imre Deak <imre.deak@intel.com>
commit 48d9f87ddd2108663fd866b254e05d422243cc56 upstream.
The old state will be needed by an upcoming patch to determine if the
commit increases or decreases CDCLK, so move the old state to the atomic
state (while keeping the new one in dev_priv). cdclk.logical and
cdclk.actual in the atomic state isn't used atm anywhere after the
atomic check phase, so this should be safe.
v2:
- Use swap() instead of opencoding it. (Ville)
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190320135439.12201-2-imre.deak@intel.com
Cc: <stable@vger.kernel.org> # 5.1.x
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
---
drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++++++++
drivers/gpu/drm/i915/intel_display.c | 4 ++--
drivers/gpu/drm/i915/intel_drv.h | 1 +
3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 553f57ff60f4..9814a6f2b3c4 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2100,6 +2100,26 @@ bool intel_cdclk_changed(const struct intel_cdclk_state *a,
a->voltage_level != b->voltage_level;
}
+/**
+ * intel_cdclk_swap_state - make atomic CDCLK configuration effective
+ * @state: atomic state
+ *
+ * This is the CDCLK version of drm_atomic_helper_swap_state() since the
+ * helper does not handle driver-specific global state.
+ *
+ * Similarly to the atomic helpers this function does a complete swap,
+ * i.e. it also puts the old state into @state. This is used by the commit
+ * code to determine how CDCLK has changed (for instance did it increase or
+ * decrease).
+ */
+void intel_cdclk_swap_state(struct intel_atomic_state *state)
+{
+ struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+
+ swap(state->cdclk.logical, dev_priv->cdclk.logical);
+ swap(state->cdclk.actual, dev_priv->cdclk.actual);
+}
+
void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
const char *context)
{
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ebbac873b8f4..dd1a059cf850 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13459,10 +13459,10 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_state->min_voltage_level,
sizeof(intel_state->min_voltage_level));
dev_priv->active_crtcs = intel_state->active_crtcs;
- dev_priv->cdclk.logical = intel_state->cdclk.logical;
- dev_priv->cdclk.actual = intel_state->cdclk.actual;
dev_priv->cdclk.force_min_cdclk =
intel_state->cdclk.force_min_cdclk;
+
+ intel_cdclk_swap_state(intel_state);
}
drm_atomic_state_get(state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 539caca05da4..a9183579700d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1597,6 +1597,7 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
const struct intel_cdclk_state *b);
bool intel_cdclk_changed(const struct intel_cdclk_state *a,
const struct intel_cdclk_state *b);
+void intel_cdclk_swap_state(struct intel_atomic_state *state);
void intel_set_cdclk(struct drm_i915_private *dev_priv,
const struct intel_cdclk_state *cdclk_state);
void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
--
2.22.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH stable-5.1 2/3] drm/i915: Remove redundant store of logical CDCLK state
2019-06-03 10:09 [PATCH 5.1 stable v2] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled Jian-Hong Pan
2019-06-10 6:01 ` [PATCH stable-5.1 0/3] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
2019-06-10 6:01 ` [PATCH stable-5.1 1/3] drm/i915: Save the old CDCLK atomic state Jian-Hong Pan
@ 2019-06-10 6:01 ` Jian-Hong Pan
2019-06-10 6:01 ` [PATCH stable-5.1 3/3] drm/i915: Skip modeset for cdclk changes if possible Jian-Hong Pan
3 siblings, 0 replies; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-10 6:01 UTC (permalink / raw)
To: stable; +Cc: linux, hui.wang, Imre Deak, Ville Syrjälä, Jian-Hong Pan
From: Imre Deak <imre.deak@intel.com>
commit 2b21dfbeee725778daed2c3dd45a3fc808176feb upstream.
We copied the original state into the atomic state already earlier in
the function, so no need to do it a second time.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190320135439.12201-3-imre.deak@intel.com
Cc: <stable@vger.kernel.org> # 5.1.x
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
---
drivers/gpu/drm/i915/intel_display.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dd1a059cf850..aa16804687c2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12827,8 +12827,6 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
DRM_DEBUG_KMS("New voltage level calculated to be logical %u, actual %u\n",
intel_state->cdclk.logical.voltage_level,
intel_state->cdclk.actual.voltage_level);
- } else {
- to_intel_atomic_state(state)->cdclk.logical = dev_priv->cdclk.logical;
}
intel_modeset_clear_plls(state);
--
2.22.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH stable-5.1 3/3] drm/i915: Skip modeset for cdclk changes if possible
2019-06-03 10:09 [PATCH 5.1 stable v2] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled Jian-Hong Pan
` (2 preceding siblings ...)
2019-06-10 6:01 ` [PATCH stable-5.1 2/3] drm/i915: Remove redundant store of logical CDCLK state Jian-Hong Pan
@ 2019-06-10 6:01 ` Jian-Hong Pan
3 siblings, 0 replies; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-10 6:01 UTC (permalink / raw)
To: stable
Cc: linux, hui.wang, Ville Syrjälä,
Abhay Kumar, Imre Deak, Clint Taylor, Jian-Hong Pan
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
commit 59f9e9cab3a1e6762fb707d0d829b982930f1349 upstream.
If we have only a single active pipe and the cdclk change only requires
the cd2x divider to be updated bxt+ can do the update with forcing a full
modeset on the pipe. Try to hook that up.
v2:
- Wait for vblank after an optimized CDCLK change.
- Avoid optimization if the pipe needs a modeset (or was disabled).
- Split CDCLK change to a pre/post plane update step.
v3:
- Use correct version of CDCLK state as old state. (Ville)
- Remove unused intel_cdclk_can_skip_modeset()
v4:
- For consistency call intel_set_cdclk_post_plane_update() only during
modesets (and not fastsets).
v5:
- Remove the logic to update the CD2X divider on-the-fly on ICL, since
only a divider of 1 is supported there. Clint also noticed that the
pipe select bits in CDCLK_CTL are oddly defined on ICL, it's not clear
yet whether that's only an error in the specification.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
Tested-by: Abhay Kumar <abhay.kumar@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190327101321.3095-1-imre.deak@intel.com
Cc: <stable@vger.kernel.org> # 5.1.x
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/intel_cdclk.c | 135 +++++++++++++++++++++------
drivers/gpu/drm/i915/intel_display.c | 42 ++++++++-
drivers/gpu/drm/i915/intel_drv.h | 17 +++-
4 files changed, 163 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5c9bb1b2d1f3..0c4a76bca5c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -280,7 +280,8 @@ struct drm_i915_display_funcs {
void (*get_cdclk)(struct drm_i915_private *dev_priv,
struct intel_cdclk_state *cdclk_state);
void (*set_cdclk)(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state);
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe);
int (*get_fifo_size)(struct drm_i915_private *dev_priv,
enum i9xx_plane_id i9xx_plane);
int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 9814a6f2b3c4..00f76261924c 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -516,7 +516,8 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
}
static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
u32 val, cmd = cdclk_state->voltage_level;
@@ -598,7 +599,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
}
static void chv_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
u32 val, cmd = cdclk_state->voltage_level;
@@ -697,7 +699,8 @@ static void bdw_get_cdclk(struct drm_i915_private *dev_priv,
}
static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
u32 val;
@@ -987,7 +990,8 @@ static void skl_dpll0_disable(struct drm_i915_private *dev_priv)
}
static void skl_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
int vco = cdclk_state->vco;
@@ -1158,7 +1162,7 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.cdclk = skl_calc_cdclk(0, cdclk_state.vco);
cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
- skl_set_cdclk(dev_priv, &cdclk_state);
+ skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
/**
@@ -1176,7 +1180,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = 0;
cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
- skl_set_cdclk(dev_priv, &cdclk_state);
+ skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
static int bxt_calc_cdclk(int min_cdclk)
@@ -1355,7 +1359,8 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
}
static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
int vco = cdclk_state->vco;
@@ -1408,11 +1413,10 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
bxt_de_pll_enable(dev_priv, vco);
val = divider | skl_cdclk_decimal(cdclk);
- /*
- * FIXME if only the cd2x divider needs changing, it could be done
- * without shutting off the pipe (if only one pipe is active).
- */
- val |= BXT_CDCLK_CD2X_PIPE_NONE;
+ if (pipe == INVALID_PIPE)
+ val |= BXT_CDCLK_CD2X_PIPE_NONE;
+ else
+ val |= BXT_CDCLK_CD2X_PIPE(pipe);
/*
* Disable SSA Precharge when CD clock frequency < 500 MHz,
* enable otherwise.
@@ -1421,6 +1425,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
I915_WRITE(CDCLK_CTL, val);
+ if (pipe != INVALID_PIPE)
+ intel_wait_for_vblank(dev_priv, pipe);
+
mutex_lock(&dev_priv->pcu_lock);
/*
* The timeout isn't specified, the 2ms used here is based on
@@ -1525,7 +1532,7 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
}
cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
- bxt_set_cdclk(dev_priv, &cdclk_state);
+ bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
/**
@@ -1543,7 +1550,7 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = 0;
cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
- bxt_set_cdclk(dev_priv, &cdclk_state);
+ bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
static int cnl_calc_cdclk(int min_cdclk)
@@ -1663,7 +1670,8 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
}
static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
int vco = cdclk_state->vco;
@@ -1704,13 +1712,15 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
cnl_cdclk_pll_enable(dev_priv, vco);
val = divider | skl_cdclk_decimal(cdclk);
- /*
- * FIXME if only the cd2x divider needs changing, it could be done
- * without shutting off the pipe (if only one pipe is active).
- */
- val |= BXT_CDCLK_CD2X_PIPE_NONE;
+ if (pipe == INVALID_PIPE)
+ val |= BXT_CDCLK_CD2X_PIPE_NONE;
+ else
+ val |= BXT_CDCLK_CD2X_PIPE(pipe);
I915_WRITE(CDCLK_CTL, val);
+ if (pipe != INVALID_PIPE)
+ intel_wait_for_vblank(dev_priv, pipe);
+
/* inform PCU of the change */
mutex_lock(&dev_priv->pcu_lock);
sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
@@ -1847,7 +1857,8 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
}
static void icl_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
unsigned int cdclk = cdclk_state->cdclk;
unsigned int vco = cdclk_state->vco;
@@ -1872,6 +1883,11 @@ static void icl_set_cdclk(struct drm_i915_private *dev_priv,
if (dev_priv->cdclk.hw.vco != vco)
cnl_cdclk_pll_enable(dev_priv, vco);
+ /*
+ * On ICL CD2X_DIV can only be 1, so we'll never end up changing the
+ * divider here synchronized to a pipe while CDCLK is on, nor will we
+ * need the corresponding vblank wait.
+ */
I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
skl_cdclk_decimal(cdclk));
@@ -2002,7 +2018,7 @@ void icl_init_cdclk(struct drm_i915_private *dev_priv)
sanitized_state.voltage_level =
icl_calc_voltage_level(sanitized_state.cdclk);
- icl_set_cdclk(dev_priv, &sanitized_state);
+ icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
}
/**
@@ -2020,7 +2036,7 @@ void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = 0;
cdclk_state.voltage_level = icl_calc_voltage_level(cdclk_state.cdclk);
- icl_set_cdclk(dev_priv, &cdclk_state);
+ icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
/**
@@ -2048,7 +2064,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
- cnl_set_cdclk(dev_priv, &cdclk_state);
+ cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
/**
@@ -2066,7 +2082,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = 0;
cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
- cnl_set_cdclk(dev_priv, &cdclk_state);
+ cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
/**
@@ -2085,6 +2101,27 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
a->ref != b->ref;
}
+/**
+ * intel_cdclk_needs_cd2x_update - Determine if two CDCLK states require a cd2x divider update
+ * @a: first CDCLK state
+ * @b: second CDCLK state
+ *
+ * Returns:
+ * True if the CDCLK states require just a cd2x divider update, false if not.
+ */
+bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *a,
+ const struct intel_cdclk_state *b)
+{
+ /* Older hw doesn't have the capability */
+ if (INTEL_GEN(dev_priv) < 10 && !IS_GEN9_LP(dev_priv))
+ return false;
+
+ return a->cdclk != b->cdclk &&
+ a->vco == b->vco &&
+ a->ref == b->ref;
+}
+
/**
* intel_cdclk_changed - Determine if two CDCLK states are different
* @a: first CDCLK state
@@ -2133,12 +2170,14 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
* intel_set_cdclk - Push the CDCLK state to the hardware
* @dev_priv: i915 device
* @cdclk_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
*
* Program the hardware based on the passed in CDCLK state,
* if necessary.
*/
-void intel_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+static void intel_set_cdclk(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
if (!intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state))
return;
@@ -2148,7 +2187,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
intel_dump_cdclk_state(cdclk_state, "Changing CDCLK to");
- dev_priv->display.set_cdclk(dev_priv, cdclk_state);
+ dev_priv->display.set_cdclk(dev_priv, cdclk_state, pipe);
if (WARN(intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state),
"cdclk state doesn't match!\n")) {
@@ -2157,6 +2196,46 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
}
}
+/**
+ * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware
+ * @dev_priv: i915 device
+ * @old_state: old CDCLK state
+ * @new_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
+ *
+ * Program the hardware before updating the HW plane state based on the passed
+ * in CDCLK state, if necessary.
+ */
+void
+intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *old_state,
+ const struct intel_cdclk_state *new_state,
+ enum pipe pipe)
+{
+ if (pipe == INVALID_PIPE || old_state->cdclk <= new_state->cdclk)
+ intel_set_cdclk(dev_priv, new_state, pipe);
+}
+
+/**
+ * intel_set_cdclk_post_plane_update - Push the CDCLK state to the hardware
+ * @dev_priv: i915 device
+ * @old_state: old CDCLK state
+ * @new_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
+ *
+ * Program the hardware after updating the HW plane state based on the passed
+ * in CDCLK state, if necessary.
+ */
+void
+intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *old_state,
+ const struct intel_cdclk_state *new_state,
+ enum pipe pipe)
+{
+ if (pipe != INVALID_PIPE && old_state->cdclk > new_state->cdclk)
+ intel_set_cdclk(dev_priv, new_state, pipe);
+}
+
static int intel_pixel_rate_to_cdclk(struct drm_i915_private *dev_priv,
int pixel_rate)
{
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index aa16804687c2..044c9235a86d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12778,6 +12778,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
intel_state->active_crtcs = dev_priv->active_crtcs;
intel_state->cdclk.logical = dev_priv->cdclk.logical;
intel_state->cdclk.actual = dev_priv->cdclk.actual;
+ intel_state->cdclk.pipe = INVALID_PIPE;
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
if (new_crtc_state->active)
@@ -12797,6 +12798,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
* adjusted_mode bits in the crtc directly.
*/
if (dev_priv->display.modeset_calc_cdclk) {
+ enum pipe pipe;
+
ret = dev_priv->display.modeset_calc_cdclk(state);
if (ret < 0)
return ret;
@@ -12813,12 +12816,36 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
return ret;
}
+ if (is_power_of_2(intel_state->active_crtcs)) {
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+
+ pipe = ilog2(intel_state->active_crtcs);
+ crtc = &intel_get_crtc_for_pipe(dev_priv, pipe)->base;
+ crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ if (crtc_state && needs_modeset(crtc_state))
+ pipe = INVALID_PIPE;
+ } else {
+ pipe = INVALID_PIPE;
+ }
+
/* All pipes must be switched off while we change the cdclk. */
- if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
- &intel_state->cdclk.actual)) {
+ if (pipe != INVALID_PIPE &&
+ intel_cdclk_needs_cd2x_update(dev_priv,
+ &dev_priv->cdclk.actual,
+ &intel_state->cdclk.actual)) {
+ ret = intel_lock_all_pipes(state);
+ if (ret < 0)
+ return ret;
+
+ intel_state->cdclk.pipe = pipe;
+ } else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
+ &intel_state->cdclk.actual)) {
ret = intel_modeset_all_pipes(state);
if (ret < 0)
return ret;
+
+ intel_state->cdclk.pipe = INVALID_PIPE;
}
DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
@@ -13227,7 +13254,10 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
if (intel_state->modeset) {
drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
- intel_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
+ intel_set_cdclk_pre_plane_update(dev_priv,
+ &intel_state->cdclk.actual,
+ &dev_priv->cdclk.actual,
+ intel_state->cdclk.pipe);
/*
* SKL workaround: bspec recommends we disable the SAGV when we
@@ -13256,6 +13286,12 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
dev_priv->display.update_crtcs(state);
+ if (intel_state->modeset)
+ intel_set_cdclk_post_plane_update(dev_priv,
+ &intel_state->cdclk.actual,
+ &dev_priv->cdclk.actual,
+ intel_state->cdclk.pipe);
+
/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
* already, but still need the state for the delayed optimization. To
* fix this:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a9183579700d..18b60016f0ed 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -483,6 +483,8 @@ struct intel_atomic_state {
int force_min_cdclk;
bool force_min_cdclk_changed;
+ /* pipe to which cd2x update is synchronized */
+ enum pipe pipe;
} cdclk;
bool dpll_set, modeset;
@@ -1593,13 +1595,24 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
void intel_update_max_cdclk(struct drm_i915_private *dev_priv);
void intel_update_cdclk(struct drm_i915_private *dev_priv);
void intel_update_rawclk(struct drm_i915_private *dev_priv);
+bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *a,
+ const struct intel_cdclk_state *b);
bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
const struct intel_cdclk_state *b);
bool intel_cdclk_changed(const struct intel_cdclk_state *a,
const struct intel_cdclk_state *b);
void intel_cdclk_swap_state(struct intel_atomic_state *state);
-void intel_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state);
+void
+intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *old_state,
+ const struct intel_cdclk_state *new_state,
+ enum pipe pipe);
+void
+intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *old_state,
+ const struct intel_cdclk_state *new_state,
+ enum pipe pipe);
void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
const char *context);
--
2.22.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH stable-5.1 0/3] drm/i915: Prevent screen from flickering when the CDCLK changes
2019-06-10 6:01 ` [PATCH stable-5.1 0/3] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
@ 2019-06-13 7:52 ` Greg KH
2019-06-13 8:37 ` Jian-Hong Pan
0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2019-06-13 7:52 UTC (permalink / raw)
To: Jian-Hong Pan; +Cc: stable, linux, hui.wang
On Mon, Jun 10, 2019 at 02:01:39PM +0800, Jian-Hong Pan wrote:
> Hi,
>
> After apply the commit "drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio
> power is enabled", it induces the screen to flicker when the CDCLK changes on
> the laptop like ASUS E406MA. [1]
>
> So, we need these commits to prevent that:
> commit 48d9f87ddd21 drm/i915: Save the old CDCLK atomic state
> commit 2b21dfbeee72 drm/i915: Remove redundant store of logical CDCLK state
> commit 59f9e9cab3a1 drm/i915: Skip modeset for cdclk changes if possible
>
> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=203623#c12
>
> Jian-Hong Pan
>
> Imre Deak (2):
> drm/i915: Save the old CDCLK atomic state
> drm/i915: Remove redundant store of logical CDCLK state
>
> Ville Syrjälä (1):
> drm/i915: Skip modeset for cdclk changes if possible
>
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_cdclk.c | 155 ++++++++++++++++++++++-----
> drivers/gpu/drm/i915/intel_display.c | 48 +++++++--
> drivers/gpu/drm/i915/intel_drv.h | 18 +++-
> 4 files changed, 186 insertions(+), 38 deletions(-)
These are all big patches, I would like to get an ack from the i915
developer(s) that these are acceptable for the stable tree before
applying them...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH stable-5.1 0/3] drm/i915: Prevent screen from flickering when the CDCLK changes
2019-06-13 7:52 ` Greg KH
@ 2019-06-13 8:37 ` Jian-Hong Pan
2019-06-13 10:05 ` Ville Syrjälä
0 siblings, 1 reply; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-13 8:37 UTC (permalink / raw)
To: Ville Syrjälä, Abhay Kumar, Imre Deak, Clint Taylor
Cc: stable, Linux Upstreaming Team, Hui Wang, Greg KH
Greg KH <gregkh@linuxfoundation.org> 於 2019年6月13日 週四 下午3:52寫道：
>
> On Mon, Jun 10, 2019 at 02:01:39PM +0800, Jian-Hong Pan wrote:
> > Hi,
> >
> > After apply the commit "drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio
> > power is enabled", it induces the screen to flicker when the CDCLK changes on
> > the laptop like ASUS E406MA. [1]
> >
> > So, we need these commits to prevent that:
> > commit 48d9f87ddd21 drm/i915: Save the old CDCLK atomic state
> > commit 2b21dfbeee72 drm/i915: Remove redundant store of logical CDCLK state
> > commit 59f9e9cab3a1 drm/i915: Skip modeset for cdclk changes if possible
> >
> > [1]: https://bugzilla.kernel.org/show_bug.cgi?id=203623#c12
> >
> > Jian-Hong Pan
> >
> > Imre Deak (2):
> > drm/i915: Save the old CDCLK atomic state
> > drm/i915: Remove redundant store of logical CDCLK state
> >
> > Ville Syrjälä (1):
> > drm/i915: Skip modeset for cdclk changes if possible
> >
> > drivers/gpu/drm/i915/i915_drv.h | 3 +-
> > drivers/gpu/drm/i915/intel_cdclk.c | 155 ++++++++++++++++++++++-----
> > drivers/gpu/drm/i915/intel_display.c | 48 +++++++--
> > drivers/gpu/drm/i915/intel_drv.h | 18 +++-
> > 4 files changed, 186 insertions(+), 38 deletions(-)
>
> These are all big patches, I would like to get an ack from the i915
> developer(s) that these are acceptable for the stable tree before
> applying them...
>
> thanks,
>
> greg k-h
Hi Intel friends,
We have laptops like ASUS E406MA, which hits the issue: The audio
playback does not work anymore after suspend & resume [1]
Thanks for your contribution. We found the commit "drm/i915: Force
2*96 MHz cdclk on glk/cnl when audio power is enabled" can fix it.
But, it induces the screen to flicker when the CDCLK changes on. So,
we need these commits to be back ported to Linux stable 5.1.x tree to
prevent flickering:
59f9e9cab3a1 drm/i915: Skip modeset for cdclk changes if possible
2b21dfbeee72 drm/i915: Remove redundant store of logical CDCLK state
48d9f87ddd21 drm/i915: Save the old CDCLK atomic state
905801fe7237 drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio
power is enabled
[1] https://bugzilla.kernel.org/show_bug.cgi?id=203623
May we have your comment or ack for the back port patches?
https://www.spinics.net/lists/stable/msg308491.html
https://www.spinics.net/lists/stable/msg310121.html
https://www.spinics.net/lists/stable/msg310122.html
https://www.spinics.net/lists/stable/msg310124.html
https://www.spinics.net/lists/stable/msg310125.html
Thank you,
Jian-Hong Pan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH stable-5.1 0/3] drm/i915: Prevent screen from flickering when the CDCLK changes
2019-06-13 8:37 ` Jian-Hong Pan
@ 2019-06-13 10:05 ` Ville Syrjälä
2019-06-14 5:59 ` Jian-Hong Pan
0 siblings, 1 reply; 17+ messages in thread
From: Ville Syrjälä @ 2019-06-13 10:05 UTC (permalink / raw)
To: Jian-Hong Pan
Cc: Abhay Kumar, Imre Deak, Clint Taylor, stable,
Linux Upstreaming Team, Hui Wang, Greg KH
On Thu, Jun 13, 2019 at 04:37:48PM +0800, Jian-Hong Pan wrote:
> Greg KH <gregkh@linuxfoundation.org> 於 2019年6月13日 週四 下午3:52寫道：
> >
> > On Mon, Jun 10, 2019 at 02:01:39PM +0800, Jian-Hong Pan wrote:
> > > Hi,
> > >
> > > After apply the commit "drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio
> > > power is enabled", it induces the screen to flicker when the CDCLK changes on
> > > the laptop like ASUS E406MA. [1]
> > >
> > > So, we need these commits to prevent that:
> > > commit 48d9f87ddd21 drm/i915: Save the old CDCLK atomic state
> > > commit 2b21dfbeee72 drm/i915: Remove redundant store of logical CDCLK state
> > > commit 59f9e9cab3a1 drm/i915: Skip modeset for cdclk changes if possible
> > >
> > > [1]: https://bugzilla.kernel.org/show_bug.cgi?id=203623#c12
> > >
> > > Jian-Hong Pan
> > >
> > > Imre Deak (2):
> > > drm/i915: Save the old CDCLK atomic state
> > > drm/i915: Remove redundant store of logical CDCLK state
> > >
> > > Ville Syrjälä (1):
> > > drm/i915: Skip modeset for cdclk changes if possible
> > >
> > > drivers/gpu/drm/i915/i915_drv.h | 3 +-
> > > drivers/gpu/drm/i915/intel_cdclk.c | 155 ++++++++++++++++++++++-----
> > > drivers/gpu/drm/i915/intel_display.c | 48 +++++++--
> > > drivers/gpu/drm/i915/intel_drv.h | 18 +++-
> > > 4 files changed, 186 insertions(+), 38 deletions(-)
> >
> > These are all big patches, I would like to get an ack from the i915
> > developer(s) that these are acceptable for the stable tree before
> > applying them...
> >
> > thanks,
> >
> > greg k-h
>
> Hi Intel friends,
>
> We have laptops like ASUS E406MA, which hits the issue: The audio
> playback does not work anymore after suspend & resume [1]
> Thanks for your contribution. We found the commit "drm/i915: Force
> 2*96 MHz cdclk on glk/cnl when audio power is enabled" can fix it.
> But, it induces the screen to flicker when the CDCLK changes on. So,
> we need these commits to be back ported to Linux stable 5.1.x tree to
> prevent flickering:
Pardon the delay.
Now that I refreshed my memory a bit I can't really see how
this could fix anything, except by luck. Before these patches
we always forced cdclk to be >=2*96 MHz so audio should never
have hit this particular problem.
The reason for adding this extra complexity was to claw back
a few milliwatts of power by allowing cdclk to drop below that
magic limit when audio isn't needed.
I *think* these patches should probably work in 5.1, but for
now I don't see this as anything more than bandaid for an
unknown issue somewhere else. So I'd rather like a new bug
filed at https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel
with a full dmesg with drm.debug=0xe (+ some audio debug
knob(s) which show when it's trying to poke the hw during
suspend/resume) attached.
>
> 59f9e9cab3a1 drm/i915: Skip modeset for cdclk changes if possible
> 2b21dfbeee72 drm/i915: Remove redundant store of logical CDCLK state
> 48d9f87ddd21 drm/i915: Save the old CDCLK atomic state
> 905801fe7237 drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio
> power is enabled
>
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=203623
>
> May we have your comment or ack for the back port patches?
> https://www.spinics.net/lists/stable/msg308491.html
> https://www.spinics.net/lists/stable/msg310121.html
> https://www.spinics.net/lists/stable/msg310122.html
> https://www.spinics.net/lists/stable/msg310124.html
> https://www.spinics.net/lists/stable/msg310125.html
>
> Thank you,
> Jian-Hong Pan
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH stable-5.1 0/3] drm/i915: Prevent screen from flickering when the CDCLK changes
2019-06-13 10:05 ` Ville Syrjälä
@ 2019-06-14 5:59 ` Jian-Hong Pan
2019-06-14 14:11 ` Ville Syrjälä
0 siblings, 1 reply; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-14 5:59 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Abhay Kumar, Imre Deak, Clint Taylor, stable,
Linux Upstreaming Team, Hui Wang, Greg KH, Takashi Iwai
Ville Syrjälä <ville.syrjala@linux.intel.com> 於 2019年6月13日 週四 下午6:05寫道：
>
> On Thu, Jun 13, 2019 at 04:37:48PM +0800, Jian-Hong Pan wrote:
> > Greg KH <gregkh@linuxfoundation.org> 於 2019年6月13日 週四 下午3:52寫道：
> > >
> > > On Mon, Jun 10, 2019 at 02:01:39PM +0800, Jian-Hong Pan wrote:
> > > > Hi,
> > > >
> > > > After apply the commit "drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio
> > > > power is enabled", it induces the screen to flicker when the CDCLK changes on
> > > > the laptop like ASUS E406MA. [1]
> > > >
> > > > So, we need these commits to prevent that:
> > > > commit 48d9f87ddd21 drm/i915: Save the old CDCLK atomic state
> > > > commit 2b21dfbeee72 drm/i915: Remove redundant store of logical CDCLK state
> > > > commit 59f9e9cab3a1 drm/i915: Skip modeset for cdclk changes if possible
> > > >
> > > > [1]: https://bugzilla.kernel.org/show_bug.cgi?id=203623#c12
> > > >
> > > > Jian-Hong Pan
> > > >
> > > > Imre Deak (2):
> > > > drm/i915: Save the old CDCLK atomic state
> > > > drm/i915: Remove redundant store of logical CDCLK state
> > > >
> > > > Ville Syrjälä (1):
> > > > drm/i915: Skip modeset for cdclk changes if possible
> > > >
> > > > drivers/gpu/drm/i915/i915_drv.h | 3 +-
> > > > drivers/gpu/drm/i915/intel_cdclk.c | 155 ++++++++++++++++++++++-----
> > > > drivers/gpu/drm/i915/intel_display.c | 48 +++++++--
> > > > drivers/gpu/drm/i915/intel_drv.h | 18 +++-
> > > > 4 files changed, 186 insertions(+), 38 deletions(-)
> > >
> > > These are all big patches, I would like to get an ack from the i915
> > > developer(s) that these are acceptable for the stable tree before
> > > applying them...
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > Hi Intel friends,
> >
> > We have laptops like ASUS E406MA, which hits the issue: The audio
> > playback does not work anymore after suspend & resume [1]
> > Thanks for your contribution. We found the commit "drm/i915: Force
> > 2*96 MHz cdclk on glk/cnl when audio power is enabled" can fix it.
> > But, it induces the screen to flicker when the CDCLK changes on. So,
> > we need these commits to be back ported to Linux stable 5.1.x tree to
> > prevent flickering:
>
> Pardon the delay.
>
> Now that I refreshed my memory a bit I can't really see how
> this could fix anything, except by luck. Before these patches
> we always forced cdclk to be >=2*96 MHz so audio should never
> have hit this particular problem.
>
> The reason for adding this extra complexity was to claw back
> a few milliwatts of power by allowing cdclk to drop below that
> magic limit when audio isn't needed.
>
> I *think* these patches should probably work in 5.1, but for
> now I don't see this as anything more than bandaid for an
> unknown issue somewhere else. So I'd rather like a new bug
> filed at https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel
> with a full dmesg with drm.debug=0xe (+ some audio debug
> knob(s) which show when it's trying to poke the hw during
> suspend/resume) attached.
Thanks for your attention!
I have filed the bug on freedesktop's bugzilla.
https://bugs.freedesktop.org/show_bug.cgi?id=110916
I can upload more log information with guidance.
Jian-Hong Pan
> >
> > 59f9e9cab3a1 drm/i915: Skip modeset for cdclk changes if possible
> > 2b21dfbeee72 drm/i915: Remove redundant store of logical CDCLK state
> > 48d9f87ddd21 drm/i915: Save the old CDCLK atomic state
> > 905801fe7237 drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio
> > power is enabled
> >
> > [1] https://bugzilla.kernel.org/show_bug.cgi?id=203623
> >
> > May we have your comment or ack for the back port patches?
> > https://www.spinics.net/lists/stable/msg308491.html
> > https://www.spinics.net/lists/stable/msg310121.html
> > https://www.spinics.net/lists/stable/msg310122.html
> > https://www.spinics.net/lists/stable/msg310124.html
> > https://www.spinics.net/lists/stable/msg310125.html
> >
> > Thank you,
> > Jian-Hong Pan
>
> --
> Ville Syrjälä
> Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH stable-5.1 0/3] drm/i915: Prevent screen from flickering when the CDCLK changes
2019-06-14 5:59 ` Jian-Hong Pan
@ 2019-06-14 14:11 ` Ville Syrjälä
0 siblings, 0 replies; 17+ messages in thread
From: Ville Syrjälä @ 2019-06-14 14:11 UTC (permalink / raw)
To: Jian-Hong Pan
Cc: Abhay Kumar, Imre Deak, Clint Taylor, stable,
Linux Upstreaming Team, Hui Wang, Greg KH, Takashi Iwai
On Fri, Jun 14, 2019 at 01:59:51PM +0800, Jian-Hong Pan wrote:
> Ville Syrjälä <ville.syrjala@linux.intel.com> 於 2019年6月13日 週四 下午6:05寫道：
> >
> > On Thu, Jun 13, 2019 at 04:37:48PM +0800, Jian-Hong Pan wrote:
> > > Greg KH <gregkh@linuxfoundation.org> 於 2019年6月13日 週四 下午3:52寫道：
> > > >
> > > > On Mon, Jun 10, 2019 at 02:01:39PM +0800, Jian-Hong Pan wrote:
> > > > > Hi,
> > > > >
> > > > > After apply the commit "drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio
> > > > > power is enabled", it induces the screen to flicker when the CDCLK changes on
> > > > > the laptop like ASUS E406MA. [1]
> > > > >
> > > > > So, we need these commits to prevent that:
> > > > > commit 48d9f87ddd21 drm/i915: Save the old CDCLK atomic state
> > > > > commit 2b21dfbeee72 drm/i915: Remove redundant store of logical CDCLK state
> > > > > commit 59f9e9cab3a1 drm/i915: Skip modeset for cdclk changes if possible
> > > > >
> > > > > [1]: https://bugzilla.kernel.org/show_bug.cgi?id=203623#c12
> > > > >
> > > > > Jian-Hong Pan
> > > > >
> > > > > Imre Deak (2):
> > > > > drm/i915: Save the old CDCLK atomic state
> > > > > drm/i915: Remove redundant store of logical CDCLK state
> > > > >
> > > > > Ville Syrjälä (1):
> > > > > drm/i915: Skip modeset for cdclk changes if possible
> > > > >
> > > > > drivers/gpu/drm/i915/i915_drv.h | 3 +-
> > > > > drivers/gpu/drm/i915/intel_cdclk.c | 155 ++++++++++++++++++++++-----
> > > > > drivers/gpu/drm/i915/intel_display.c | 48 +++++++--
> > > > > drivers/gpu/drm/i915/intel_drv.h | 18 +++-
> > > > > 4 files changed, 186 insertions(+), 38 deletions(-)
> > > >
> > > > These are all big patches, I would like to get an ack from the i915
> > > > developer(s) that these are acceptable for the stable tree before
> > > > applying them...
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > >
> > > Hi Intel friends,
> > >
> > > We have laptops like ASUS E406MA, which hits the issue: The audio
> > > playback does not work anymore after suspend & resume [1]
> > > Thanks for your contribution. We found the commit "drm/i915: Force
> > > 2*96 MHz cdclk on glk/cnl when audio power is enabled" can fix it.
> > > But, it induces the screen to flicker when the CDCLK changes on. So,
> > > we need these commits to be back ported to Linux stable 5.1.x tree to
> > > prevent flickering:
> >
> > Pardon the delay.
> >
> > Now that I refreshed my memory a bit I can't really see how
> > this could fix anything, except by luck. Before these patches
> > we always forced cdclk to be >=2*96 MHz so audio should never
> > have hit this particular problem.
> >
> > The reason for adding this extra complexity was to claw back
> > a few milliwatts of power by allowing cdclk to drop below that
> > magic limit when audio isn't needed.
> >
> > I *think* these patches should probably work in 5.1, but for
> > now I don't see this as anything more than bandaid for an
> > unknown issue somewhere else. So I'd rather like a new bug
> > filed at https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel
> > with a full dmesg with drm.debug=0xe (+ some audio debug
> > knob(s) which show when it's trying to poke the hw during
> > suspend/resume) attached.
>
> Thanks for your attention!
> I have filed the bug on freedesktop's bugzilla.
> https://bugs.freedesktop.org/show_bug.cgi?id=110916
>
> I can upload more log information with guidance.
OK, I think I see what's going on. We would just need to change a few
lines of code to fix that (basically just s/0/2*96000/ in two places)
but we don't want that upstream anymore due to these patches.
So I guess we need to go ahead with the backport. I read through them
and checked the 5.1 baseline and couldn't spot any problems. So for
the backport to 5.1:
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH stable-5.1 1/3] drm/i915: Save the old CDCLK atomic state
2019-06-10 6:01 ` [PATCH stable-5.1 1/3] drm/i915: Save the old CDCLK atomic state Jian-Hong Pan
@ 2019-06-20 14:19 ` Greg KH
2019-06-21 10:07 ` [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2019-06-20 14:19 UTC (permalink / raw)
To: Jian-Hong Pan; +Cc: stable, linux, hui.wang, Imre Deak, Ville Syrjälä
On Mon, Jun 10, 2019 at 02:01:41PM +0800, Jian-Hong Pan wrote:
> From: Imre Deak <imre.deak@intel.com>
>
> commit 48d9f87ddd2108663fd866b254e05d422243cc56 upstream.
>
> The old state will be needed by an upcoming patch to determine if the
> commit increases or decreases CDCLK, so move the old state to the atomic
> state (while keeping the new one in dev_priv). cdclk.logical and
> cdclk.actual in the atomic state isn't used atm anywhere after the
> atomic check phase, so this should be safe.
>
> v2:
> - Use swap() instead of opencoding it. (Ville)
>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20190320135439.12201-2-imre.deak@intel.com
> Cc: <stable@vger.kernel.org> # 5.1.x
> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> ---
> drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++++++++
> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 3 files changed, 23 insertions(+), 2 deletions(-)
Does not apply against the latest 5.1.y tree. Can you rebase this
series and resend please?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes
2019-06-20 14:19 ` Greg KH
@ 2019-06-21 10:07 ` Jian-Hong Pan
2019-06-21 10:07 ` [PATCH stable-5.1 v2 1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled Jian-Hong Pan
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-21 10:07 UTC (permalink / raw)
To: Greg KH, stable
Cc: Imre Deak, Ville Syrjälä,
Abhay Kumar, tiwai, hui.wang, linux, Jian-Hong Pan
Hi,
To make it more clearly, I re-send this patch series again.
To fix "the audio playback does not work anymore after suspend & resume"
on ASUS E406MA, we need rediffed commit "drm/i915: Force 2*96 MHz cdclk
on glk/cnl when audio power is enabled". However, after apply the
commit "drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is
enabled", it induces the screen to flicker when the CDCLK changes on the
laptop. [1]
So, we need these commits to prevent that:
commit 48d9f87ddd21 drm/i915: Save the old CDCLK atomic state
commit 2b21dfbeee72 drm/i915: Remove redundant store of logical CDCLK state
commit 59f9e9cab3a1 drm/i915: Skip modeset for cdclk changes if possible
This issue is also reconfirmed on the list. [2][3]
[1] https://bugzilla.kernel.org/show_bug.cgi?id=203623
[2] https://bugs.freedesktop.org/show_bug.cgi?id=110916
[3] https://www.spinics.net/lists/stable/msg310910.html
Thank you,
Jian-Hong Pan
Imre Deak (2):
drm/i915: Save the old CDCLK atomic state
drm/i915: Remove redundant store of logical CDCLK state
Ville Syrjälä (2):
drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
drm/i915: Skip modeset for cdclk changes if possible
drivers/gpu/drm/i915/i915_drv.h | 6 +-
drivers/gpu/drm/i915/intel_audio.c | 62 ++++++++-
drivers/gpu/drm/i915/intel_cdclk.c | 185 ++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_display.c | 57 +++++++--
drivers/gpu/drm/i915/intel_drv.h | 21 ++-
5 files changed, 269 insertions(+), 62 deletions(-)
--
2.22.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH stable-5.1 v2 1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
2019-06-21 10:07 ` [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
@ 2019-06-21 10:07 ` Jian-Hong Pan
2019-06-21 10:07 ` [PATCH stable-5.1 v2 2/4] drm/i915: Save the old CDCLK atomic state Jian-Hong Pan
` (3 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-21 10:07 UTC (permalink / raw)
To: Greg KH, stable
Cc: Imre Deak, Ville Syrjälä,
Abhay Kumar, tiwai, hui.wang, linux, Clint Taylor, Jian-Hong Pan
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
commit 905801fe72377b4dc53c6e13eea1a91c6a4aa0c4 upstream.
CDCLK has to be at least twice the BLCK regardless of audio. Audio
driver has to probe using this hook and increase the clock even in
absence of any display.
v2: Use atomic refcount for get_power, put_power so that we can
call each once(Abhay).
v3: Reset power well 2 to avoid any transaction on iDisp link
during cdclk change(Abhay).
v4: Remove Power well 2 reset workaround(Ville).
v5: Remove unwanted Power well 2 register defined in v4(Abhay).
v6:
- Use a dedicated flag instead of state->modeset for min CDCLK changes
- Make get/put audio power domain symmetric
- Rebased on top of intel_wakeref tracking changes.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
Tested-by: Abhay Kumar <abhay.kumar@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190320135439.12201-1-imre.deak@intel.com
[jian-hong@endlessm.com: Rediff to keep i915_audio_component_get_power
and i915_audio_component_put_power for Linux stable 5.1.x]
Cc: <stable@vger.kernel.org> # 5.1.x
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=203623
Buglink: https://bugs.freedesktop.org/show_bug.cgi?id=110916
Link: https://www.spinics.net/lists/stable/msg310910.html
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/intel_audio.c | 62 ++++++++++++++++++++++++++--
drivers/gpu/drm/i915/intel_cdclk.c | 30 +++++---------
drivers/gpu/drm/i915/intel_display.c | 9 +++-
drivers/gpu/drm/i915/intel_drv.h | 3 ++
5 files changed, 83 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a67a63b5aa84..5c9bb1b2d1f3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1622,6 +1622,8 @@ struct drm_i915_private {
struct intel_cdclk_state actual;
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+
+ int force_min_cdclk;
} cdclk;
/**
@@ -1741,6 +1743,7 @@ struct drm_i915_private {
*
*/
struct mutex av_mutex;
+ int audio_power_refcount;
struct {
struct mutex mutex;
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 5104c6bbd66f..2f3fd5bf0f11 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -741,15 +741,71 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
}
}
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+ bool enable)
+{
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ int ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+ state = drm_atomic_state_alloc(&dev_priv->drm);
+ if (WARN_ON(!state))
+ return;
+
+ state->acquire_ctx = &ctx;
+
+retry:
+ to_intel_atomic_state(state)->cdclk.force_min_cdclk_changed = true;
+ to_intel_atomic_state(state)->cdclk.force_min_cdclk =
+ enable ? 2 * 96000 : 0;
+
+ /*
+ * Protects dev_priv->cdclk.force_min_cdclk
+ * Need to lock this here in case we have no active pipes
+ * and thus wouldn't lock it during the commit otherwise.
+ */
+ ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
+ &ctx);
+ if (!ret)
+ ret = drm_atomic_commit(state);
+
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ drm_modeset_backoff(&ctx);
+ goto retry;
+ }
+
+ WARN_ON(ret);
+
+ drm_atomic_state_put(state);
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
static void i915_audio_component_get_power(struct device *kdev)
{
- intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
+ struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+ intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
+
+ /* Force CDCLK to 2*BCLK as long as we need audio to be powered. */
+ if (dev_priv->audio_power_refcount++ == 0)
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ glk_force_audio_cdclk(dev_priv, true);
}
static void i915_audio_component_put_power(struct device *kdev)
{
- intel_display_power_put_unchecked(kdev_to_i915(kdev),
- POWER_DOMAIN_AUDIO);
+ struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+ /* Stop forcing CDCLK to 2*BCLK if no need for audio to be powered. */
+ if (--dev_priv->audio_power_refcount == 0)
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ glk_force_audio_cdclk(dev_priv, false);
+
+ intel_display_power_put_unchecked(dev_priv, POWER_DOMAIN_AUDIO);
}
static void i915_audio_component_codec_wake_override(struct device *kdev,
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 15ba950dee00..553f57ff60f4 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2187,19 +2187,8 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
/*
* According to BSpec, "The CD clock frequency must be at least twice
* the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
- *
- * FIXME: Check the actual, not default, BCLK being used.
- *
- * FIXME: This does not depend on ->has_audio because the higher CDCLK
- * is required for audio probe, also when there are no audio capable
- * displays connected at probe time. This leads to unnecessarily high
- * CDCLK when audio is not required.
- *
- * FIXME: This limit is only applied when there are displays connected
- * at probe time. If we probe without displays, we'll still end up using
- * the platform minimum CDCLK, failing audio probe.
*/
- if (INTEL_GEN(dev_priv) >= 9)
+ if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
min_cdclk = max(2 * 96000, min_cdclk);
/*
@@ -2239,7 +2228,7 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
intel_state->min_cdclk[i] = min_cdclk;
}
- min_cdclk = 0;
+ min_cdclk = intel_state->cdclk.force_min_cdclk;
for_each_pipe(dev_priv, pipe)
min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
@@ -2300,7 +2289,8 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
vlv_calc_voltage_level(dev_priv, cdclk);
if (!intel_state->active_crtcs) {
- cdclk = vlv_calc_cdclk(dev_priv, 0);
+ cdclk = vlv_calc_cdclk(dev_priv,
+ intel_state->cdclk.force_min_cdclk);
intel_state->cdclk.actual.cdclk = cdclk;
intel_state->cdclk.actual.voltage_level =
@@ -2333,7 +2323,7 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
bdw_calc_voltage_level(cdclk);
if (!intel_state->active_crtcs) {
- cdclk = bdw_calc_cdclk(0);
+ cdclk = bdw_calc_cdclk(intel_state->cdclk.force_min_cdclk);
intel_state->cdclk.actual.cdclk = cdclk;
intel_state->cdclk.actual.voltage_level =
@@ -2405,7 +2395,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
skl_calc_voltage_level(cdclk);
if (!intel_state->active_crtcs) {
- cdclk = skl_calc_cdclk(0, vco);
+ cdclk = skl_calc_cdclk(intel_state->cdclk.force_min_cdclk, vco);
intel_state->cdclk.actual.vco = vco;
intel_state->cdclk.actual.cdclk = cdclk;
@@ -2444,10 +2434,10 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
if (!intel_state->active_crtcs) {
if (IS_GEMINILAKE(dev_priv)) {
- cdclk = glk_calc_cdclk(0);
+ cdclk = glk_calc_cdclk(intel_state->cdclk.force_min_cdclk);
vco = glk_de_pll_vco(dev_priv, cdclk);
} else {
- cdclk = bxt_calc_cdclk(0);
+ cdclk = bxt_calc_cdclk(intel_state->cdclk.force_min_cdclk);
vco = bxt_de_pll_vco(dev_priv, cdclk);
}
@@ -2483,7 +2473,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
cnl_compute_min_voltage_level(intel_state));
if (!intel_state->active_crtcs) {
- cdclk = cnl_calc_cdclk(0);
+ cdclk = cnl_calc_cdclk(intel_state->cdclk.force_min_cdclk);
vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
intel_state->cdclk.actual.vco = vco;
@@ -2519,7 +2509,7 @@ static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
cnl_compute_min_voltage_level(intel_state));
if (!intel_state->active_crtcs) {
- cdclk = icl_calc_cdclk(0, ref);
+ cdclk = icl_calc_cdclk(intel_state->cdclk.force_min_cdclk, ref);
vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
intel_state->cdclk.actual.vco = vco;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cd8a22d6370e..1d7f750ad809 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12773,6 +12773,11 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
return -EINVAL;
}
+ /* keep the current setting */
+ if (!intel_state->cdclk.force_min_cdclk_changed)
+ intel_state->cdclk.force_min_cdclk =
+ dev_priv->cdclk.force_min_cdclk;
+
intel_state->modeset = true;
intel_state->active_crtcs = dev_priv->active_crtcs;
intel_state->cdclk.logical = dev_priv->cdclk.logical;
@@ -12868,7 +12873,7 @@ static int intel_atomic_check(struct drm_device *dev,
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state, *crtc_state;
int ret, i;
- bool any_ms = false;
+ bool any_ms = intel_state->cdclk.force_min_cdclk_changed;
/* Catch I915_MODE_FLAG_INHERITED */
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
@@ -13460,6 +13465,8 @@ static int intel_atomic_commit(struct drm_device *dev,
dev_priv->active_crtcs = intel_state->active_crtcs;
dev_priv->cdclk.logical = intel_state->cdclk.logical;
dev_priv->cdclk.actual = intel_state->cdclk.actual;
+ dev_priv->cdclk.force_min_cdclk =
+ intel_state->cdclk.force_min_cdclk;
}
drm_atomic_state_get(state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 18e17b422701..824935b87a12 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -480,6 +480,9 @@ struct intel_atomic_state {
* state only when all crtc's are DPMS off.
*/
struct intel_cdclk_state actual;
+
+ int force_min_cdclk;
+ bool force_min_cdclk_changed;
} cdclk;
bool dpll_set, modeset;
--
2.22.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH stable-5.1 v2 2/4] drm/i915: Save the old CDCLK atomic state
2019-06-21 10:07 ` [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
2019-06-21 10:07 ` [PATCH stable-5.1 v2 1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled Jian-Hong Pan
@ 2019-06-21 10:07 ` Jian-Hong Pan
2019-06-21 10:07 ` [PATCH stable-5.1 v2 3/4] drm/i915: Remove redundant store of logical CDCLK state Jian-Hong Pan
` (2 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-21 10:07 UTC (permalink / raw)
To: Greg KH, stable
Cc: Imre Deak, Ville Syrjälä,
Abhay Kumar, tiwai, hui.wang, linux, Jian-Hong Pan
From: Imre Deak <imre.deak@intel.com>
commit 48d9f87ddd2108663fd866b254e05d422243cc56 upstream.
The old state will be needed by an upcoming patch to determine if the
commit increases or decreases CDCLK, so move the old state to the atomic
state (while keeping the new one in dev_priv). cdclk.logical and
cdclk.actual in the atomic state isn't used atm anywhere after the
atomic check phase, so this should be safe.
v2:
- Use swap() instead of opencoding it. (Ville)
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190320135439.12201-2-imre.deak@intel.com
Cc: <stable@vger.kernel.org> # 5.1.x
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
---
drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++++++++
drivers/gpu/drm/i915/intel_display.c | 4 ++--
drivers/gpu/drm/i915/intel_drv.h | 1 +
3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 553f57ff60f4..9814a6f2b3c4 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2100,6 +2100,26 @@ bool intel_cdclk_changed(const struct intel_cdclk_state *a,
a->voltage_level != b->voltage_level;
}
+/**
+ * intel_cdclk_swap_state - make atomic CDCLK configuration effective
+ * @state: atomic state
+ *
+ * This is the CDCLK version of drm_atomic_helper_swap_state() since the
+ * helper does not handle driver-specific global state.
+ *
+ * Similarly to the atomic helpers this function does a complete swap,
+ * i.e. it also puts the old state into @state. This is used by the commit
+ * code to determine how CDCLK has changed (for instance did it increase or
+ * decrease).
+ */
+void intel_cdclk_swap_state(struct intel_atomic_state *state)
+{
+ struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+
+ swap(state->cdclk.logical, dev_priv->cdclk.logical);
+ swap(state->cdclk.actual, dev_priv->cdclk.actual);
+}
+
void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
const char *context)
{
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1d7f750ad809..fb2a23830325 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13463,10 +13463,10 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_state->min_voltage_level,
sizeof(intel_state->min_voltage_level));
dev_priv->active_crtcs = intel_state->active_crtcs;
- dev_priv->cdclk.logical = intel_state->cdclk.logical;
- dev_priv->cdclk.actual = intel_state->cdclk.actual;
dev_priv->cdclk.force_min_cdclk =
intel_state->cdclk.force_min_cdclk;
+
+ intel_cdclk_swap_state(intel_state);
}
drm_atomic_state_get(state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 824935b87a12..2a11dfc28a2a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1597,6 +1597,7 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
const struct intel_cdclk_state *b);
bool intel_cdclk_changed(const struct intel_cdclk_state *a,
const struct intel_cdclk_state *b);
+void intel_cdclk_swap_state(struct intel_atomic_state *state);
void intel_set_cdclk(struct drm_i915_private *dev_priv,
const struct intel_cdclk_state *cdclk_state);
void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
--
2.22.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH stable-5.1 v2 3/4] drm/i915: Remove redundant store of logical CDCLK state
2019-06-21 10:07 ` [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
2019-06-21 10:07 ` [PATCH stable-5.1 v2 1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled Jian-Hong Pan
2019-06-21 10:07 ` [PATCH stable-5.1 v2 2/4] drm/i915: Save the old CDCLK atomic state Jian-Hong Pan
@ 2019-06-21 10:07 ` Jian-Hong Pan
2019-06-21 10:07 ` [PATCH stable-5.1 v2 4/4] drm/i915: Skip modeset for cdclk changes if possible Jian-Hong Pan
2019-07-01 15:14 ` [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes Greg KH
4 siblings, 0 replies; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-21 10:07 UTC (permalink / raw)
To: Greg KH, stable
Cc: Imre Deak, Ville Syrjälä,
Abhay Kumar, tiwai, hui.wang, linux, Jian-Hong Pan
From: Imre Deak <imre.deak@intel.com>
commit 2b21dfbeee725778daed2c3dd45a3fc808176feb upstream.
We copied the original state into the atomic state already earlier in
the function, so no need to do it a second time.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190320135439.12201-3-imre.deak@intel.com
Cc: <stable@vger.kernel.org> # 5.1.x
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
---
drivers/gpu/drm/i915/intel_display.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fb2a23830325..d8fabe4643d9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12831,8 +12831,6 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
DRM_DEBUG_KMS("New voltage level calculated to be logical %u, actual %u\n",
intel_state->cdclk.logical.voltage_level,
intel_state->cdclk.actual.voltage_level);
- } else {
- to_intel_atomic_state(state)->cdclk.logical = dev_priv->cdclk.logical;
}
intel_modeset_clear_plls(state);
--
2.22.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH stable-5.1 v2 4/4] drm/i915: Skip modeset for cdclk changes if possible
2019-06-21 10:07 ` [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
` (2 preceding siblings ...)
2019-06-21 10:07 ` [PATCH stable-5.1 v2 3/4] drm/i915: Remove redundant store of logical CDCLK state Jian-Hong Pan
@ 2019-06-21 10:07 ` Jian-Hong Pan
2019-07-01 15:14 ` [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes Greg KH
4 siblings, 0 replies; 17+ messages in thread
From: Jian-Hong Pan @ 2019-06-21 10:07 UTC (permalink / raw)
To: Greg KH, stable
Cc: Imre Deak, Ville Syrjälä,
Abhay Kumar, tiwai, hui.wang, linux, Clint Taylor, Jian-Hong Pan
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
commit 59f9e9cab3a1e6762fb707d0d829b982930f1349 upstream.
If we have only a single active pipe and the cdclk change only requires
the cd2x divider to be updated bxt+ can do the update with forcing a full
modeset on the pipe. Try to hook that up.
v2:
- Wait for vblank after an optimized CDCLK change.
- Avoid optimization if the pipe needs a modeset (or was disabled).
- Split CDCLK change to a pre/post plane update step.
v3:
- Use correct version of CDCLK state as old state. (Ville)
- Remove unused intel_cdclk_can_skip_modeset()
v4:
- For consistency call intel_set_cdclk_post_plane_update() only during
modesets (and not fastsets).
v5:
- Remove the logic to update the CD2X divider on-the-fly on ICL, since
only a divider of 1 is supported there. Clint also noticed that the
pipe select bits in CDCLK_CTL are oddly defined on ICL, it's not clear
yet whether that's only an error in the specification.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
Tested-by: Abhay Kumar <abhay.kumar@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190327101321.3095-1-imre.deak@intel.com
Cc: <stable@vger.kernel.org> # 5.1.x
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/intel_cdclk.c | 135 +++++++++++++++++++++------
drivers/gpu/drm/i915/intel_display.c | 42 ++++++++-
drivers/gpu/drm/i915/intel_drv.h | 17 +++-
4 files changed, 163 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5c9bb1b2d1f3..0c4a76bca5c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -280,7 +280,8 @@ struct drm_i915_display_funcs {
void (*get_cdclk)(struct drm_i915_private *dev_priv,
struct intel_cdclk_state *cdclk_state);
void (*set_cdclk)(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state);
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe);
int (*get_fifo_size)(struct drm_i915_private *dev_priv,
enum i9xx_plane_id i9xx_plane);
int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 9814a6f2b3c4..00f76261924c 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -516,7 +516,8 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
}
static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
u32 val, cmd = cdclk_state->voltage_level;
@@ -598,7 +599,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
}
static void chv_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
u32 val, cmd = cdclk_state->voltage_level;
@@ -697,7 +699,8 @@ static void bdw_get_cdclk(struct drm_i915_private *dev_priv,
}
static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
u32 val;
@@ -987,7 +990,8 @@ static void skl_dpll0_disable(struct drm_i915_private *dev_priv)
}
static void skl_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
int vco = cdclk_state->vco;
@@ -1158,7 +1162,7 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.cdclk = skl_calc_cdclk(0, cdclk_state.vco);
cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
- skl_set_cdclk(dev_priv, &cdclk_state);
+ skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
/**
@@ -1176,7 +1180,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = 0;
cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
- skl_set_cdclk(dev_priv, &cdclk_state);
+ skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
static int bxt_calc_cdclk(int min_cdclk)
@@ -1355,7 +1359,8 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
}
static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
int vco = cdclk_state->vco;
@@ -1408,11 +1413,10 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
bxt_de_pll_enable(dev_priv, vco);
val = divider | skl_cdclk_decimal(cdclk);
- /*
- * FIXME if only the cd2x divider needs changing, it could be done
- * without shutting off the pipe (if only one pipe is active).
- */
- val |= BXT_CDCLK_CD2X_PIPE_NONE;
+ if (pipe == INVALID_PIPE)
+ val |= BXT_CDCLK_CD2X_PIPE_NONE;
+ else
+ val |= BXT_CDCLK_CD2X_PIPE(pipe);
/*
* Disable SSA Precharge when CD clock frequency < 500 MHz,
* enable otherwise.
@@ -1421,6 +1425,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
I915_WRITE(CDCLK_CTL, val);
+ if (pipe != INVALID_PIPE)
+ intel_wait_for_vblank(dev_priv, pipe);
+
mutex_lock(&dev_priv->pcu_lock);
/*
* The timeout isn't specified, the 2ms used here is based on
@@ -1525,7 +1532,7 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
}
cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
- bxt_set_cdclk(dev_priv, &cdclk_state);
+ bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
/**
@@ -1543,7 +1550,7 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = 0;
cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
- bxt_set_cdclk(dev_priv, &cdclk_state);
+ bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
static int cnl_calc_cdclk(int min_cdclk)
@@ -1663,7 +1670,8 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
}
static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
int cdclk = cdclk_state->cdclk;
int vco = cdclk_state->vco;
@@ -1704,13 +1712,15 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
cnl_cdclk_pll_enable(dev_priv, vco);
val = divider | skl_cdclk_decimal(cdclk);
- /*
- * FIXME if only the cd2x divider needs changing, it could be done
- * without shutting off the pipe (if only one pipe is active).
- */
- val |= BXT_CDCLK_CD2X_PIPE_NONE;
+ if (pipe == INVALID_PIPE)
+ val |= BXT_CDCLK_CD2X_PIPE_NONE;
+ else
+ val |= BXT_CDCLK_CD2X_PIPE(pipe);
I915_WRITE(CDCLK_CTL, val);
+ if (pipe != INVALID_PIPE)
+ intel_wait_for_vblank(dev_priv, pipe);
+
/* inform PCU of the change */
mutex_lock(&dev_priv->pcu_lock);
sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
@@ -1847,7 +1857,8 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
}
static void icl_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
unsigned int cdclk = cdclk_state->cdclk;
unsigned int vco = cdclk_state->vco;
@@ -1872,6 +1883,11 @@ static void icl_set_cdclk(struct drm_i915_private *dev_priv,
if (dev_priv->cdclk.hw.vco != vco)
cnl_cdclk_pll_enable(dev_priv, vco);
+ /*
+ * On ICL CD2X_DIV can only be 1, so we'll never end up changing the
+ * divider here synchronized to a pipe while CDCLK is on, nor will we
+ * need the corresponding vblank wait.
+ */
I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
skl_cdclk_decimal(cdclk));
@@ -2002,7 +2018,7 @@ void icl_init_cdclk(struct drm_i915_private *dev_priv)
sanitized_state.voltage_level =
icl_calc_voltage_level(sanitized_state.cdclk);
- icl_set_cdclk(dev_priv, &sanitized_state);
+ icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
}
/**
@@ -2020,7 +2036,7 @@ void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = 0;
cdclk_state.voltage_level = icl_calc_voltage_level(cdclk_state.cdclk);
- icl_set_cdclk(dev_priv, &cdclk_state);
+ icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
/**
@@ -2048,7 +2064,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
- cnl_set_cdclk(dev_priv, &cdclk_state);
+ cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
/**
@@ -2066,7 +2082,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = 0;
cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
- cnl_set_cdclk(dev_priv, &cdclk_state);
+ cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
/**
@@ -2085,6 +2101,27 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
a->ref != b->ref;
}
+/**
+ * intel_cdclk_needs_cd2x_update - Determine if two CDCLK states require a cd2x divider update
+ * @a: first CDCLK state
+ * @b: second CDCLK state
+ *
+ * Returns:
+ * True if the CDCLK states require just a cd2x divider update, false if not.
+ */
+bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *a,
+ const struct intel_cdclk_state *b)
+{
+ /* Older hw doesn't have the capability */
+ if (INTEL_GEN(dev_priv) < 10 && !IS_GEN9_LP(dev_priv))
+ return false;
+
+ return a->cdclk != b->cdclk &&
+ a->vco == b->vco &&
+ a->ref == b->ref;
+}
+
/**
* intel_cdclk_changed - Determine if two CDCLK states are different
* @a: first CDCLK state
@@ -2133,12 +2170,14 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
* intel_set_cdclk - Push the CDCLK state to the hardware
* @dev_priv: i915 device
* @cdclk_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
*
* Program the hardware based on the passed in CDCLK state,
* if necessary.
*/
-void intel_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state)
+static void intel_set_cdclk(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *cdclk_state,
+ enum pipe pipe)
{
if (!intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state))
return;
@@ -2148,7 +2187,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
intel_dump_cdclk_state(cdclk_state, "Changing CDCLK to");
- dev_priv->display.set_cdclk(dev_priv, cdclk_state);
+ dev_priv->display.set_cdclk(dev_priv, cdclk_state, pipe);
if (WARN(intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state),
"cdclk state doesn't match!\n")) {
@@ -2157,6 +2196,46 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
}
}
+/**
+ * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware
+ * @dev_priv: i915 device
+ * @old_state: old CDCLK state
+ * @new_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
+ *
+ * Program the hardware before updating the HW plane state based on the passed
+ * in CDCLK state, if necessary.
+ */
+void
+intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *old_state,
+ const struct intel_cdclk_state *new_state,
+ enum pipe pipe)
+{
+ if (pipe == INVALID_PIPE || old_state->cdclk <= new_state->cdclk)
+ intel_set_cdclk(dev_priv, new_state, pipe);
+}
+
+/**
+ * intel_set_cdclk_post_plane_update - Push the CDCLK state to the hardware
+ * @dev_priv: i915 device
+ * @old_state: old CDCLK state
+ * @new_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
+ *
+ * Program the hardware after updating the HW plane state based on the passed
+ * in CDCLK state, if necessary.
+ */
+void
+intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *old_state,
+ const struct intel_cdclk_state *new_state,
+ enum pipe pipe)
+{
+ if (pipe != INVALID_PIPE && old_state->cdclk > new_state->cdclk)
+ intel_set_cdclk(dev_priv, new_state, pipe);
+}
+
static int intel_pixel_rate_to_cdclk(struct drm_i915_private *dev_priv,
int pixel_rate)
{
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d8fabe4643d9..d7c05c7750ae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12782,6 +12782,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
intel_state->active_crtcs = dev_priv->active_crtcs;
intel_state->cdclk.logical = dev_priv->cdclk.logical;
intel_state->cdclk.actual = dev_priv->cdclk.actual;
+ intel_state->cdclk.pipe = INVALID_PIPE;
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
if (new_crtc_state->active)
@@ -12801,6 +12802,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
* adjusted_mode bits in the crtc directly.
*/
if (dev_priv->display.modeset_calc_cdclk) {
+ enum pipe pipe;
+
ret = dev_priv->display.modeset_calc_cdclk(state);
if (ret < 0)
return ret;
@@ -12817,12 +12820,36 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
return ret;
}
+ if (is_power_of_2(intel_state->active_crtcs)) {
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+
+ pipe = ilog2(intel_state->active_crtcs);
+ crtc = &intel_get_crtc_for_pipe(dev_priv, pipe)->base;
+ crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ if (crtc_state && needs_modeset(crtc_state))
+ pipe = INVALID_PIPE;
+ } else {
+ pipe = INVALID_PIPE;
+ }
+
/* All pipes must be switched off while we change the cdclk. */
- if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
- &intel_state->cdclk.actual)) {
+ if (pipe != INVALID_PIPE &&
+ intel_cdclk_needs_cd2x_update(dev_priv,
+ &dev_priv->cdclk.actual,
+ &intel_state->cdclk.actual)) {
+ ret = intel_lock_all_pipes(state);
+ if (ret < 0)
+ return ret;
+
+ intel_state->cdclk.pipe = pipe;
+ } else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
+ &intel_state->cdclk.actual)) {
ret = intel_modeset_all_pipes(state);
if (ret < 0)
return ret;
+
+ intel_state->cdclk.pipe = INVALID_PIPE;
}
DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
@@ -13231,7 +13258,10 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
if (intel_state->modeset) {
drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
- intel_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
+ intel_set_cdclk_pre_plane_update(dev_priv,
+ &intel_state->cdclk.actual,
+ &dev_priv->cdclk.actual,
+ intel_state->cdclk.pipe);
/*
* SKL workaround: bspec recommends we disable the SAGV when we
@@ -13260,6 +13290,12 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
dev_priv->display.update_crtcs(state);
+ if (intel_state->modeset)
+ intel_set_cdclk_post_plane_update(dev_priv,
+ &intel_state->cdclk.actual,
+ &dev_priv->cdclk.actual,
+ intel_state->cdclk.pipe);
+
/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
* already, but still need the state for the delayed optimization. To
* fix this:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2a11dfc28a2a..ceaa05dfa0d0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -483,6 +483,8 @@ struct intel_atomic_state {
int force_min_cdclk;
bool force_min_cdclk_changed;
+ /* pipe to which cd2x update is synchronized */
+ enum pipe pipe;
} cdclk;
bool dpll_set, modeset;
@@ -1593,13 +1595,24 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
void intel_update_max_cdclk(struct drm_i915_private *dev_priv);
void intel_update_cdclk(struct drm_i915_private *dev_priv);
void intel_update_rawclk(struct drm_i915_private *dev_priv);
+bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *a,
+ const struct intel_cdclk_state *b);
bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
const struct intel_cdclk_state *b);
bool intel_cdclk_changed(const struct intel_cdclk_state *a,
const struct intel_cdclk_state *b);
void intel_cdclk_swap_state(struct intel_atomic_state *state);
-void intel_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state);
+void
+intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *old_state,
+ const struct intel_cdclk_state *new_state,
+ enum pipe pipe);
+void
+intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
+ const struct intel_cdclk_state *old_state,
+ const struct intel_cdclk_state *new_state,
+ enum pipe pipe);
void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
const char *context);
--
2.22.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes
2019-06-21 10:07 ` [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
` (3 preceding siblings ...)
2019-06-21 10:07 ` [PATCH stable-5.1 v2 4/4] drm/i915: Skip modeset for cdclk changes if possible Jian-Hong Pan
@ 2019-07-01 15:14 ` Greg KH
4 siblings, 0 replies; 17+ messages in thread
From: Greg KH @ 2019-07-01 15:14 UTC (permalink / raw)
To: Jian-Hong Pan
Cc: stable, Imre Deak, Ville Syrjälä,
Abhay Kumar, tiwai, hui.wang, linux
On Fri, Jun 21, 2019 at 06:07:12PM +0800, Jian-Hong Pan wrote:
> Hi,
>
> To make it more clearly, I re-send this patch series again.
>
> To fix "the audio playback does not work anymore after suspend & resume"
> on ASUS E406MA, we need rediffed commit "drm/i915: Force 2*96 MHz cdclk
> on glk/cnl when audio power is enabled". However, after apply the
> commit "drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is
> enabled", it induces the screen to flicker when the CDCLK changes on the
> laptop. [1]
>
> So, we need these commits to prevent that:
>
> commit 48d9f87ddd21 drm/i915: Save the old CDCLK atomic state
> commit 2b21dfbeee72 drm/i915: Remove redundant store of logical CDCLK state
> commit 59f9e9cab3a1 drm/i915: Skip modeset for cdclk changes if possible
>
> This issue is also reconfirmed on the list. [2][3]
>
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=203623
> [2] https://bugs.freedesktop.org/show_bug.cgi?id=110916
> [3] https://www.spinics.net/lists/stable/msg310910.html
Now queued up, thanks.
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-07-01 15:15 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-03 10:09 [PATCH 5.1 stable v2] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled Jian-Hong Pan
2019-06-10 6:01 ` [PATCH stable-5.1 0/3] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
2019-06-13 7:52 ` Greg KH
2019-06-13 8:37 ` Jian-Hong Pan
2019-06-13 10:05 ` Ville Syrjälä
2019-06-14 5:59 ` Jian-Hong Pan
2019-06-14 14:11 ` Ville Syrjälä
2019-06-10 6:01 ` [PATCH stable-5.1 1/3] drm/i915: Save the old CDCLK atomic state Jian-Hong Pan
2019-06-20 14:19 ` Greg KH
2019-06-21 10:07 ` [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes Jian-Hong Pan
2019-06-21 10:07 ` [PATCH stable-5.1 v2 1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled Jian-Hong Pan
2019-06-21 10:07 ` [PATCH stable-5.1 v2 2/4] drm/i915: Save the old CDCLK atomic state Jian-Hong Pan
2019-06-21 10:07 ` [PATCH stable-5.1 v2 3/4] drm/i915: Remove redundant store of logical CDCLK state Jian-Hong Pan
2019-06-21 10:07 ` [PATCH stable-5.1 v2 4/4] drm/i915: Skip modeset for cdclk changes if possible Jian-Hong Pan
2019-07-01 15:14 ` [PATCH stable-5.1 v2 0/4] drm/i915: Prevent screen from flickering when the CDCLK changes Greg KH
2019-06-10 6:01 ` [PATCH stable-5.1 2/3] drm/i915: Remove redundant store of logical CDCLK state Jian-Hong Pan
2019-06-10 6:01 ` [PATCH stable-5.1 3/3] drm/i915: Skip modeset for cdclk changes if possible Jian-Hong Pan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).