On Tue, 27 Dec 2022 11:39:25 -0500 Rodrigo Vivi rodrigo.vivi@intel.com wrote:
On Sun, Dec 25, 2022 at 09:55:08PM +0300, Alexey Lukyanchuk wrote:
dell wyse 3040 doesn't peform poweroff properly, but instead remains in turned power on state.
okay, the motivation is explained in the commit msg..
Additional mutex_lock and intel_crtc_wait_for_next_vblank feature 6.2 kernel resolve this trouble.
but this why is not very clear... seems that by magic it was found, without explaining what race we are really protecting here.
but even worse is: what about those many random vblank waits in the code? what's the reasoning?
cc: stable@vger.kernel.org original commit Link: https://patchwork.freedesktop.org/patch/508926/ fixes: fe0f1e3bfdfeb53e18f1206aea4f40b9bd1f291c Signed-off-by: Alexey Lukyanchuk skif@skif-web.ru
I got some troubles with this device (dell wyse 3040) since kernel 5.11 started to use i915_driver_shutdown function. I found solution here:
https://lore.kernel.org/dri-devel/Y1wd6ZJ8LdJpCfZL@intel.com/#r
drivers/gpu/drm/i915/display/intel_audio.c | 37 +++++++++++++++------- 1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c index aacbc6da8..44344ecdf 100644 --- a/drivers/gpu/drm/i915/display/intel_audio.c +++ b/drivers/gpu/drm/i915/display/intel_audio.c @@ -336,6 +336,7 @@ static void g4x_audio_codec_disable(struct intel_encoder *encoder, const struct drm_connector_state *old_conn_state) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
- struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); u32 eldv, tmp;
tmp = intel_de_read(dev_priv, G4X_AUD_VID_DID); @@ -348,6 +349,9 @@ static void g4x_audio_codec_disable(struct intel_encoder *encoder, tmp = intel_de_read(dev_priv, G4X_AUD_CNTL_ST); tmp &= ~eldv; intel_de_write(dev_priv, G4X_AUD_CNTL_ST, tmp);
- intel_crtc_wait_for_next_vblank(crtc);
- intel_crtc_wait_for_next_vblank(crtc);
} static void g4x_audio_codec_enable(struct intel_encoder *encoder, @@ -355,12 +359,15 @@ static void g4x_audio_codec_enable(struct intel_encoder *encoder, const struct drm_connector_state *conn_state) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
- struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); struct drm_connector *connector = conn_state->connector; const u8 *eld = connector->eld; u32 eldv; u32 tmp; int len, i;
- intel_crtc_wait_for_next_vblank(crtc);
- tmp = intel_de_read(dev_priv, G4X_AUD_VID_DID); if (tmp == INTEL_AUDIO_DEVBLC || tmp == INTEL_AUDIO_DEVCL) eldv = G4X_ELDV_DEVCL_DEVBLC;
@@ -493,6 +500,7 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder, const struct drm_connector_state *old_conn_state) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
- struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc); enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder; u32 tmp;
@@ -508,6 +516,10 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder, tmp |= AUD_CONFIG_N_VALUE_INDEX; intel_de_write(dev_priv, HSW_AUD_CFG(cpu_transcoder), tmp);
- intel_crtc_wait_for_next_vblank(crtc);
- intel_crtc_wait_for_next_vblank(crtc);
- /* Invalidate ELD */ tmp = intel_de_read(dev_priv, HSW_AUD_PIN_ELD_CP_VLD); tmp &= ~AUDIO_ELD_VALID(cpu_transcoder);
@@ -633,6 +645,7 @@ static void hsw_audio_codec_enable(struct intel_encoder *encoder, const struct drm_connector_state *conn_state) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
- struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); struct drm_connector *connector = conn_state->connector; enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; const u8 *eld = connector->eld;
@@ -651,12 +664,7 @@ static void hsw_audio_codec_enable(struct intel_encoder *encoder, tmp &= ~AUDIO_ELD_VALID(cpu_transcoder); intel_de_write(dev_priv, HSW_AUD_PIN_ELD_CP_VLD, tmp);
- /*
* FIXME: We're supposed to wait for vblank here, but we have vblanks* disabled during the mode set. The proper fix would be to push the* rest of the setup into a vblank work item, queued here, but the* infrastructure is not there yet.*/
- intel_crtc_wait_for_next_vblank(crtc);
/* Reset ELD write address */ tmp = intel_de_read(dev_priv, HSW_AUD_DIP_ELD_CTRL(cpu_transcoder)); @@ -705,6 +713,8 @@ static void ilk_audio_codec_disable(struct intel_encoder *encoder, aud_cntrl_st2 = CPT_AUD_CNTRL_ST2; }
- mutex_lock(&dev_priv->display.audio.mutex);
- /* Disable timestamps */ tmp = intel_de_read(dev_priv, aud_config); tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
@@ -721,6 +731,10 @@ static void ilk_audio_codec_disable(struct intel_encoder *encoder, tmp = intel_de_read(dev_priv, aud_cntrl_st2); tmp &= ~eldv; intel_de_write(dev_priv, aud_cntrl_st2, tmp);
- mutex_unlock(&dev_priv->display.audio.mutex);
- intel_crtc_wait_for_next_vblank(crtc);
- intel_crtc_wait_for_next_vblank(crtc);
} static void ilk_audio_codec_enable(struct intel_encoder *encoder, @@ -740,12 +754,7 @@ static void ilk_audio_codec_enable(struct intel_encoder *encoder, if (drm_WARN_ON(&dev_priv->drm, port == PORT_A)) return;
- /*
* FIXME: We're supposed to wait for vblank here, but we have vblanks* disabled during the mode set. The proper fix would be to push the* rest of the setup into a vblank work item, queued here, but the* infrastructure is not there yet.*/
- intel_crtc_wait_for_next_vblank(crtc);
if (HAS_PCH_IBX(dev_priv)) { hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe); @@ -767,6 +776,8 @@ static void ilk_audio_codec_enable(struct intel_encoder *encoder, eldv = IBX_ELD_VALID(port);
- mutex_lock(&dev_priv->display.audio.mutex);
- /* Invalidate ELD */ tmp = intel_de_read(dev_priv, aud_cntrl_st2); tmp &= ~eldv;
@@ -798,6 +809,8 @@ static void ilk_audio_codec_enable(struct intel_encoder *encoder, else tmp |= audio_config_hdmi_pixel_clock(crtc_state); intel_de_write(dev_priv, aud_config, tmp);
- mutex_unlock(&dev_priv->display.audio.mutex);
} /** -- 2.25.1
I would like to say, that this solution was found in drm-tip repository: link: git://anongit.freedesktop.org/drm-tip I will quotate original commit message from Ville Syrjälä ville.syrjala@linux.intel.com: "The spec tells us to do a bunch of vblank waits in the audio enable/disable sequences. Make it so." So it's just a backport of accepted patch. Which i wanna to propagate to stable versions