On Tue, 27 Dec 2022 11:39:25 -0500 Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> 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@xxxxxxxxxxxxxxx > > original commit Link: https://patchwork.freedesktop.org/patch/508926/ > > fixes: fe0f1e3bfdfeb53e18f1206aea4f40b9bd1f291c > > Signed-off-by: Alexey Lukyanchuk <skif@xxxxxxxxxxx> > > --- > > 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@xxxxxxxxx/#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@xxxxxxxxxxxxxxx>: "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
