diff mbox series

[v4,01/14] drm/i915: Fix PIPEDMC disabling for a bigjoiner configuration

Message ID 20230510103131.1618266-2-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/tc: Add a workaround for an IOM/TCSS firmware hang issue | expand

Commit Message

Imre Deak May 10, 2023, 10:31 a.m. UTC 
For a bigjoiner configuration display->crtc_disable() will be called
first for the slave CRTCs and then for the master CRTC. However slave
CRTCs will be actually disabled only after the master CRTC is disabled
(from the encoder disable hooks called with the master CRTC state).
Hence the slave PIPEDMCs can be disabled only after the master CRTC is
disabled, make this so.

intel_encoders_post_pll_disable() must be called only for the master
CRTC, as for the other two encoder disable hooks. While at it fix this
up as well. This didn't cause a problem, since
intel_encoders_post_pll_disable() will call the corresponding hook only
for an encoder/connector connected to the given CRTC, however slave
CRTCs will have no associated encoder/connector.

Fixes: 3af2ff0840be ("drm/i915: Enable a PIPEDMC whenever its corresponding pipe is enabled")
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Rudi Heitbaum May 30, 2023, 1:49 p.m. UTC | #1 
Hi Imre/Dave,

Ref: [v4,01/14] drm/i915: Fix PIPEDMC disabling for a bigjoiner configuration
     [git pull] drm fixes for 6.4-rc4
     drm-fixes-2023-05-26:
     drm fixes for 6.4-rc4

This patch has caused a regression between 6.4-rc3 and 6.4-rc4. Other
tested kernels include 6.3.4 work fine. Dropping the patch allows the decode
playback of media via Kodi. Without dropping the patch - the media
starts and stutters then ceases to play.

There is an additional issue that 6.4-rc4 audio playback is also failing
(where 6.4-rc3 was fine), I have not yet tracked this down.

This is all on:
DMI: Intel(R) Client Systems NUC12WSKi7/NUC12WSBi7, BIOS WSADL357.0087.2023.0306.1931 03/06/2023
12th Gen Intel(R) Core(TM) i7-1260P (family: 0x6, model: 0x9a, stepping: 0x3)
microcode: updated early: 0x429 -> 0x42a, date = 2023-02-14

Regards

Rudi

On Wed, May 10, 2023 at 01:31:18PM +0300, Imre Deak wrote:
> For a bigjoiner configuration display->crtc_disable() will be called
> first for the slave CRTCs and then for the master CRTC. However slave
> CRTCs will be actually disabled only after the master CRTC is disabled
> (from the encoder disable hooks called with the master CRTC state).
> Hence the slave PIPEDMCs can be disabled only after the master CRTC is
> disabled, make this so.
> 
> intel_encoders_post_pll_disable() must be called only for the master
> CRTC, as for the other two encoder disable hooks. While at it fix this
> up as well. This didn't cause a problem, since
> intel_encoders_post_pll_disable() will call the corresponding hook only
> for an encoder/connector connected to the given CRTC, however slave
> CRTCs will have no associated encoder/connector.
> 
> Fixes: 3af2ff0840be ("drm/i915: Enable a PIPEDMC whenever its corresponding pipe is enabled")
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 1d5d42a408035..116fa52290b84 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1702,9 +1702,17 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
>  
>  	intel_disable_shared_dpll(old_crtc_state);
>  
> -	intel_encoders_post_pll_disable(state, crtc);
> +	if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) {
> +		struct intel_crtc *slave_crtc;
> +
> +		intel_encoders_post_pll_disable(state, crtc);
>  
> -	intel_dmc_disable_pipe(i915, crtc->pipe);
> +		intel_dmc_disable_pipe(i915, crtc->pipe);
> +
> +		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> +						 intel_crtc_bigjoiner_slave_pipes(old_crtc_state))
> +			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
> +	}
>  }
>  
>  static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
Imre Deak May 31, 2023, 8:47 a.m. UTC | #2 
On Tue, May 30, 2023 at 01:49:07PM +0000, Rudi Heitbaum wrote:
Hi Rudi,

Could you open a ticket at
https://gitlab.freedesktop.org/drm/intel/-/issues/new

attaching a dmesg log after booting with drm.debug=0xe, with the
messages from boot-up until the issue happens?

Thanks,
Imre

> Hi Imre/Dave,
> 
> Ref: [v4,01/14] drm/i915: Fix PIPEDMC disabling for a bigjoiner configuration
>      [git pull] drm fixes for 6.4-rc4
>      drm-fixes-2023-05-26:
>      drm fixes for 6.4-rc4
> 
> This patch has caused a regression between 6.4-rc3 and 6.4-rc4. Other
> tested kernels include 6.3.4 work fine. Dropping the patch allows the decode
> playback of media via Kodi. Without dropping the patch - the media
> starts and stutters then ceases to play.
> 
> There is an additional issue that 6.4-rc4 audio playback is also failing
> (where 6.4-rc3 was fine), I have not yet tracked this down.
> 
> This is all on:
> DMI: Intel(R) Client Systems NUC12WSKi7/NUC12WSBi7, BIOS WSADL357.0087.2023.0306.1931 03/06/2023
> 12th Gen Intel(R) Core(TM) i7-1260P (family: 0x6, model: 0x9a, stepping: 0x3)
> microcode: updated early: 0x429 -> 0x42a, date = 2023-02-14
> 
> Regards
> 
> Rudi
> 
> On Wed, May 10, 2023 at 01:31:18PM +0300, Imre Deak wrote:
> > For a bigjoiner configuration display->crtc_disable() will be called
> > first for the slave CRTCs and then for the master CRTC. However slave
> > CRTCs will be actually disabled only after the master CRTC is disabled
> > (from the encoder disable hooks called with the master CRTC state).
> > Hence the slave PIPEDMCs can be disabled only after the master CRTC is
> > disabled, make this so.
> > 
> > intel_encoders_post_pll_disable() must be called only for the master
> > CRTC, as for the other two encoder disable hooks. While at it fix this
> > up as well. This didn't cause a problem, since
> > intel_encoders_post_pll_disable() will call the corresponding hook only
> > for an encoder/connector connected to the given CRTC, however slave
> > CRTCs will have no associated encoder/connector.
> > 
> > Fixes: 3af2ff0840be ("drm/i915: Enable a PIPEDMC whenever its corresponding pipe is enabled")
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 1d5d42a408035..116fa52290b84 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -1702,9 +1702,17 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
> >  
> >  	intel_disable_shared_dpll(old_crtc_state);
> >  
> > -	intel_encoders_post_pll_disable(state, crtc);
> > +	if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) {
> > +		struct intel_crtc *slave_crtc;
> > +
> > +		intel_encoders_post_pll_disable(state, crtc);
> >  
> > -	intel_dmc_disable_pipe(i915, crtc->pipe);
> > +		intel_dmc_disable_pipe(i915, crtc->pipe);
> > +
> > +		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> > +						 intel_crtc_bigjoiner_slave_pipes(old_crtc_state))
> > +			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
> > +	}
> >  }
> >  
> >  static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
Rudi Heitbaum May 31, 2023, 11:31 a.m. UTC | #3 
Hi Imre,

Raised the ticket and was able to capture the logs for you.

https://gitlab.freedesktop.org/drm/intel/-/issues/8559

Thanks
Rudi

On Wed, 31 May 2023 at 18:47, Imre Deak <imre.deak@intel.com> wrote:
>
> On Tue, May 30, 2023 at 01:49:07PM +0000, Rudi Heitbaum wrote:
> Hi Rudi,
>
> Could you open a ticket at
> https://gitlab.freedesktop.org/drm/intel/-/issues/new
>
> attaching a dmesg log after booting with drm.debug=0xe, with the
> messages from boot-up until the issue happens?
>
> Thanks,
> Imre
>
> > Hi Imre/Dave,
> >
> > Ref: [v4,01/14] drm/i915: Fix PIPEDMC disabling for a bigjoiner configuration
> >      [git pull] drm fixes for 6.4-rc4
> >      drm-fixes-2023-05-26:
> >      drm fixes for 6.4-rc4
> >
> > This patch has caused a regression between 6.4-rc3 and 6.4-rc4. Other
> > tested kernels include 6.3.4 work fine. Dropping the patch allows the decode
> > playback of media via Kodi. Without dropping the patch - the media
> > starts and stutters then ceases to play.
> >
> > There is an additional issue that 6.4-rc4 audio playback is also failing
> > (where 6.4-rc3 was fine), I have not yet tracked this down.
> >
> > This is all on:
> > DMI: Intel(R) Client Systems NUC12WSKi7/NUC12WSBi7, BIOS WSADL357.0087.2023.0306.1931 03/06/2023
> > 12th Gen Intel(R) Core(TM) i7-1260P (family: 0x6, model: 0x9a, stepping: 0x3)
> > microcode: updated early: 0x429 -> 0x42a, date = 2023-02-14
> >
> > Regards
> >
> > Rudi
> >
> > On Wed, May 10, 2023 at 01:31:18PM +0300, Imre Deak wrote:
> > > For a bigjoiner configuration display->crtc_disable() will be called
> > > first for the slave CRTCs and then for the master CRTC. However slave
> > > CRTCs will be actually disabled only after the master CRTC is disabled
> > > (from the encoder disable hooks called with the master CRTC state).
> > > Hence the slave PIPEDMCs can be disabled only after the master CRTC is
> > > disabled, make this so.
> > >
> > > intel_encoders_post_pll_disable() must be called only for the master
> > > CRTC, as for the other two encoder disable hooks. While at it fix this
> > > up as well. This didn't cause a problem, since
> > > intel_encoders_post_pll_disable() will call the corresponding hook only
> > > for an encoder/connector connected to the given CRTC, however slave
> > > CRTCs will have no associated encoder/connector.
> > >
> > > Fixes: 3af2ff0840be ("drm/i915: Enable a PIPEDMC whenever its corresponding pipe is enabled")
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 1d5d42a408035..116fa52290b84 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -1702,9 +1702,17 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
> > >
> > >     intel_disable_shared_dpll(old_crtc_state);
> > >
> > > -   intel_encoders_post_pll_disable(state, crtc);
> > > +   if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) {
> > > +           struct intel_crtc *slave_crtc;
> > > +
> > > +           intel_encoders_post_pll_disable(state, crtc);
> > >
> > > -   intel_dmc_disable_pipe(i915, crtc->pipe);
> > > +           intel_dmc_disable_pipe(i915, crtc->pipe);
> > > +
> > > +           for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> > > +                                            intel_crtc_bigjoiner_slave_pipes(old_crtc_state))
> > > +                   intel_dmc_disable_pipe(i915, slave_crtc->pipe);
> > > +   }
> > >  }
> > >
> > >  static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 1d5d42a408035..116fa52290b84 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1702,9 +1702,17 @@  static void hsw_crtc_disable(struct intel_atomic_state *state,
 
 	intel_disable_shared_dpll(old_crtc_state);
 
-	intel_encoders_post_pll_disable(state, crtc);
+	if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) {
+		struct intel_crtc *slave_crtc;
+
+		intel_encoders_post_pll_disable(state, crtc);
 
-	intel_dmc_disable_pipe(i915, crtc->pipe);
+		intel_dmc_disable_pipe(i915, crtc->pipe);
+
+		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
+						 intel_crtc_bigjoiner_slave_pipes(old_crtc_state))
+			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
+	}
 }
 
 static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)