| Message ID | 20240626083624.4042544-1-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dp: Wait more before rearming FIFO underrun during retrain | expand |
On Wed, Jun 26, 2024 at 02:06:24PM +0530, Ankit Nautiyal wrote: > During Link re-training reporting underrun is disabled and then > renabled after re-training is completed. For BMG its seen that we get > FIFO underrun just after the retraining is completed and the underrun > reporting is re-enabled. > Add one more intel_crtc_wait_for_next_vblank before re-arming the > underruns. Cc: Arthur Runyan <arthur.j.runyan@intel.com> Art, any new workaround in BMG for this issue? > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 3903f6ead6e6..25af51499383 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -5325,7 +5325,12 @@ static int intel_dp_retrain_link(struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state = > to_intel_crtc_state(crtc->base.state); > > - /* Keep underrun reporting disabled until things are stable */ > + /* > + * Keep underrun reporting disabled until things are stable. > + * Wait for some more time, as we see (at least on BMG) that > + * underrun gets reported just after the reporting is enabled. > + */ > + intel_crtc_wait_for_next_vblank(crtc); > intel_crtc_wait_for_next_vblank(crtc); > > intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true); > -- > 2.40.1 >
There is no underrun expected (that I know of) when coming out of training on recent product generations. You should undo this masking and debug. -----Original Message----- From: Vivi, Rodrigo <rodrigo.vivi@intel.com> Sent: Wednesday, June 26, 2024 8:13 AM To: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Runyan, Arthur J <arthur.j.runyan@intel.com> Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org Subject: Re: [PATCH] drm/i915/dp: Wait more before rearming FIFO underrun during retrain On Wed, Jun 26, 2024 at 02:06:24PM +0530, Ankit Nautiyal wrote: > During Link re-training reporting underrun is disabled and then > renabled after re-training is completed. For BMG its seen that we get > FIFO underrun just after the retraining is completed and the underrun > reporting is re-enabled. > Add one more intel_crtc_wait_for_next_vblank before re-arming the > underruns. Cc: Arthur Runyan <arthur.j.runyan@intel.com> Art, any new workaround in BMG for this issue? > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 3903f6ead6e6..25af51499383 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -5325,7 +5325,12 @@ static int intel_dp_retrain_link(struct intel_encoder *encoder, > const struct intel_crtc_state *crtc_state = > to_intel_crtc_state(crtc->base.state); > > - /* Keep underrun reporting disabled until things are stable */ > + /* > + * Keep underrun reporting disabled until things are stable. > + * Wait for some more time, as we see (at least on BMG) that > + * underrun gets reported just after the reporting is enabled. > + */ > + intel_crtc_wait_for_next_vblank(crtc); > intel_crtc_wait_for_next_vblank(crtc); > > intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true); > -- > 2.40.1 >
Atm the link on DP-SST is retrained without a full modeset, which probably causes an underrun. This was originally done for simplicity, the underrun not causing further known issues. The plan is to retrain the link with a full modeset instead on DP-SST as well, similarly to how this is done on DP-MST already: https://github.com/ideak/linux/commits/sst-modeset-retrain On Wed, Jun 26, 2024 at 03:18:32PM +0000, Runyan, Arthur J wrote: > There is no underrun expected (that I know of) when coming out of > training on recent product generations. You should undo this masking > and debug. > > -----Original Message----- > From: Vivi, Rodrigo <rodrigo.vivi@intel.com> > Sent: Wednesday, June 26, 2024 8:13 AM > To: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Runyan, Arthur J <arthur.j.runyan@intel.com> > Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org > Subject: Re: [PATCH] drm/i915/dp: Wait more before rearming FIFO underrun during retrain > > On Wed, Jun 26, 2024 at 02:06:24PM +0530, Ankit Nautiyal wrote: > > During Link re-training reporting underrun is disabled and then > > renabled after re-training is completed. For BMG its seen that we get > > FIFO underrun just after the retraining is completed and the underrun > > reporting is re-enabled. > > Add one more intel_crtc_wait_for_next_vblank before re-arming the > > underruns. > > Cc: Arthur Runyan <arthur.j.runyan@intel.com> > > Art, any new workaround in BMG for this issue? > > > > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > > > > --- > > drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 3903f6ead6e6..25af51499383 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -5325,7 +5325,12 @@ static int intel_dp_retrain_link(struct intel_encoder *encoder, > > const struct intel_crtc_state *crtc_state = > > to_intel_crtc_state(crtc->base.state); > > > > - /* Keep underrun reporting disabled until things are stable */ > > + /* > > + * Keep underrun reporting disabled until things are stable. > > + * Wait for some more time, as we see (at least on BMG) that > > + * underrun gets reported just after the reporting is enabled. > > + */ > > + intel_crtc_wait_for_next_vblank(crtc); > > intel_crtc_wait_for_next_vblank(crtc); > > > > intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true); > > -- > > 2.40.1 > >
On 6/26/2024 9:13 PM, Imre Deak wrote: > Atm the link on DP-SST is retrained without a full modeset, which > probably causes an underrun. This was originally done for simplicity, > the underrun not causing further known issues. The plan is to retrain > the link with a full modeset instead on DP-SST as well, similarly to how > this is done on DP-MST already: > > https://github.com/ideak/linux/commits/sst-modeset-retrain Thanks Imre, I validated with the given patches from link above, we are not seeing the underruns with these. We can drop my patch and perhaps you can float your changes for review. Regards, Ankit > > On Wed, Jun 26, 2024 at 03:18:32PM +0000, Runyan, Arthur J wrote: >> There is no underrun expected (that I know of) when coming out of >> training on recent product generations. You should undo this masking >> and debug. >> >> -----Original Message----- >> From: Vivi, Rodrigo <rodrigo.vivi@intel.com> >> Sent: Wednesday, June 26, 2024 8:13 AM >> To: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Runyan, Arthur J <arthur.j.runyan@intel.com> >> Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org >> Subject: Re: [PATCH] drm/i915/dp: Wait more before rearming FIFO underrun during retrain >> >> On Wed, Jun 26, 2024 at 02:06:24PM +0530, Ankit Nautiyal wrote: >>> During Link re-training reporting underrun is disabled and then >>> renabled after re-training is completed. For BMG its seen that we get >>> FIFO underrun just after the retraining is completed and the underrun >>> reporting is re-enabled. >>> Add one more intel_crtc_wait_for_next_vblank before re-arming the >>> underruns. >> Cc: Arthur Runyan <arthur.j.runyan@intel.com> >> >> Art, any new workaround in BMG for this issue? >> >>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> >>> --- >>> drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c >>> b/drivers/gpu/drm/i915/display/intel_dp.c >>> index 3903f6ead6e6..25af51499383 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_dp.c >>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c >>> @@ -5325,7 +5325,12 @@ static int intel_dp_retrain_link(struct intel_encoder *encoder, >>> const struct intel_crtc_state *crtc_state = >>> to_intel_crtc_state(crtc->base.state); >>> >>> - /* Keep underrun reporting disabled until things are stable */ >>> + /* >>> + * Keep underrun reporting disabled until things are stable. >>> + * Wait for some more time, as we see (at least on BMG) that >>> + * underrun gets reported just after the reporting is enabled. >>> + */ >>> + intel_crtc_wait_for_next_vblank(crtc); >>> intel_crtc_wait_for_next_vblank(crtc); >>> >>> intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true); >>> -- >>> 2.40.1 >>>
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 3903f6ead6e6..25af51499383 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5325,7 +5325,12 @@ static int intel_dp_retrain_link(struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); - /* Keep underrun reporting disabled until things are stable */ + /* + * Keep underrun reporting disabled until things are stable. + * Wait for some more time, as we see (at least on BMG) that + * underrun gets reported just after the reporting is enabled. + */ + intel_crtc_wait_for_next_vblank(crtc); intel_crtc_wait_for_next_vblank(crtc); intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true);
During Link re-training reporting underrun is disabled and then renabled after re-training is completed. For BMG its seen that we get FIFO underrun just after the retraining is completed and the underrun reporting is re-enabled. Add one more intel_crtc_wait_for_next_vblank before re-arming the underruns. Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)