| Message ID | 20250109073137.1977494-11-jouni.hogander@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | PSR DSB support | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jouni > Högander > Sent: Thursday, January 9, 2025 1:02 PM > To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org > Cc: Hogander, Jouni <jouni.hogander@intel.com> > Subject: [PATCH v3 10/10] drm/i915/psr: Allow DSB usage when PSR is > enabled > > Now as we have correct PSR2_MAN_TRK_CTL handling in place we can allow > DSB usage also when PSR is enabled for LunarLake onwards. > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> Reviewed-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index e448ff64660a..58575800fad2 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -7631,6 +7631,7 @@ static void intel_atomic_dsb_finish(struct > intel_atomic_state *state, > intel_atomic_get_old_crtc_state(state, crtc); > struct intel_crtc_state *new_crtc_state = > intel_atomic_get_new_crtc_state(state, crtc); > + struct intel_display *display = to_intel_display(crtc); > > if (!new_crtc_state->hw.active) > return; > @@ -7643,7 +7644,7 @@ static void intel_atomic_dsb_finish(struct > intel_atomic_state *state, > new_crtc_state->update_planes && > !new_crtc_state->vrr.enable && > !new_crtc_state->do_async_flip && > - !new_crtc_state->has_psr && > + (DISPLAY_VER(display) >= 20 || !new_crtc_state->has_psr) > && > !new_crtc_state->scaler_state.scaler_users && > !old_crtc_state->scaler_state.scaler_users && > !intel_crtc_needs_modeset(new_crtc_state) && > -- > 2.43.0
On Thu, Jan 09, 2025 at 09:31:37AM +0200, Jouni Högander wrote: > Now as we have correct PSR2_MAN_TRK_CTL handling in place we can allow DSB > usage also when PSR is enabled for LunarLake onwards. We seem to still lack an answer as to when the PSR wakes, when it latches the update, and how does all that guarantee that the DSB interrupt fires after the update has been latched? Some thoughts as to how to figure this out: 1. make sure we're in PSR 2. sample TIMESTAMP_CTR 3. start DSB in which write PLANE_SURF with a new value send push wait for vblank poll PLANE_SURFLIVE == new value fire interrupt in the interrupt handler: sample TIMESTAMP_CTR again And then compare flip timestmap vs. frame timestamp vs. the manually sampled timestamps. And then repeat without the SURFLIVE poll to make sure nothing has changed. You'll need to be careful to make sure it will actually poll for long enough to make a real difference (if the poll actuall is needed), but tweaking the poll interval+count suitably. I don't remeber what the max poll count was, but IIRC it wasn't too high so the duration will have to get bumped for long polls. I guess one could also try to poll for the actual PSR status, but dunno how well that'll work. And we could also try to come up with different ideas on where to sample timestamps. Unfortunately we only have the single pipe flip timestamp register so we can only sample one timestamp from the DSB itself per frame. If we had more we could much more easily figure things out :/ I pushed my latest DSB selftest stuff to https://github.com/vsyrjala/linux.git dsb_selftests_7 which has a bunch of stuff for this kind of experimentation. It's in a somewhat sorry state at the moment since I last used to hunt for various DSB bugs, but at least it still builds :) The way I use that is that I run igt 'testdisplay -o ...' to make sure nothing else is actively poking the hardware and then I trigger the DSB tests via debugfs. > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index e448ff64660a..58575800fad2 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -7631,6 +7631,7 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, > intel_atomic_get_old_crtc_state(state, crtc); > struct intel_crtc_state *new_crtc_state = > intel_atomic_get_new_crtc_state(state, crtc); > + struct intel_display *display = to_intel_display(crtc); > > if (!new_crtc_state->hw.active) > return; > @@ -7643,7 +7644,7 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, > new_crtc_state->update_planes && > !new_crtc_state->vrr.enable && > !new_crtc_state->do_async_flip && > - !new_crtc_state->has_psr && > + (DISPLAY_VER(display) >= 20 || !new_crtc_state->has_psr) && > !new_crtc_state->scaler_state.scaler_users && > !old_crtc_state->scaler_state.scaler_users && > !intel_crtc_needs_modeset(new_crtc_state) && > -- > 2.43.0
On Fri, Jan 17, 2025 at 10:20:17PM +0200, Ville Syrjälä wrote: > On Thu, Jan 09, 2025 at 09:31:37AM +0200, Jouni Högander wrote: > > Now as we have correct PSR2_MAN_TRK_CTL handling in place we can allow DSB > > usage also when PSR is enabled for LunarLake onwards. > > We seem to still lack an answer as to when the PSR wakes, when it > latches the update, and how does all that guarantee that the DSB > interrupt fires after the update has been latched? > > Some thoughts as to how to figure this out: > 1. make sure we're in PSR > 2. sample TIMESTAMP_CTR > 3. start DSB in which > write PLANE_SURF with a new value > send push > wait for vblank > poll PLANE_SURFLIVE == new value > fire interrupt > > in the interrupt handler: > sample TIMESTAMP_CTR again > > And then compare flip timestmap vs. frame timestamp vs. the > manually sampled timestamps. And then repeat without the SURFLIVE > poll to make sure nothing has changed. You'll need to be careful > to make sure it will actually poll for long enough to make a real > difference (if the poll actuall is needed), but tweaking the poll > interval+count suitably. I don't remeber what the max poll > count was, but IIRC it wasn't too high so the duration will have > to get bumped for long polls. > > I guess one could also try to poll for the actual PSR status, > but dunno how well that'll work. > > And we could also try to come up with different ideas on where > to sample timestamps. Unfortunately we only have the single > pipe flip timestamp register so we can only sample one timestamp > from the DSB itself per frame. If we had more we could much more > easily figure things out :/ > > I pushed my latest DSB selftest stuff to > https://github.com/vsyrjala/linux.git dsb_selftests_7 > which has a bunch of stuff for this kind of experimentation. > It's in a somewhat sorry state at the moment since I last used > to hunt for various DSB bugs, but at least it still builds :) > > The way I use that is that I run igt 'testdisplay -o ...' > to make sure nothing else is actively poking the hardware > and then I trigger the DSB tests via debugfs. I poked around a bit, though only on a TGL+PSR1 system (what I had at hand), so some of this might not apply to PSR2 and/or more modern platforms. General notes: - PSR1 exit is triggered by any pipe/plane register write (even the non-arming ones) We basically have three cases to consider here: 1. PSR1 is currently inactive Obviously everything should be with the current code, vblank evasion works, wait for vblank+interrupt scheme for flip completion works 2. PSR1 is active, but DC states are not The wakeup latency here is super quick (it's < 1 scanline, how much below? I've not yet measured), and arming registers do latch nearly immediately. The scanline counter starts counting accordingly from vblank_start-1. However the hardware still considers PSR to be active for that short duration so DSB_SKIP_WAITS_EN _will_ skip the waits. Unfortunately being this quick I'm not convinced we have enough time to write all the registers atomically before the hardware latches something. So I'm thinking we may need to remove DSB_SKIP_WAITS_EN, in which case the vblank evasion will push the arming register writes into the next frame. This will mean the wakeup will take one full frame. 3. PSR1 is active and so are DC states The wakeup latency is ~5ms. During that time scanline counter reads 0, PSR is active for the purposes of DSB_SKIP_WAITS_EN. Again we pretty much need DSB_SKIP_WAITS_EN=0 here to make sure the interrupt gets signalled after the wait for vblank. vblank evasion will get skipped on account the scanline being 0. Somewhat ironically this would give us ~5ms total wakeup latency which is now faster than the previous case. So everything here would be fine if we know that the wakeup has just started since we have all of that 5ms to write all the registers. But I guess we can't really know when the wakeup started, so we might be doing the vblank evasion just before the scanline counter starts to read its proper value, and then we have that < 1 scanline to write all the arming registers. Not sure if its enough. If not, then we could also explicitly evade scanline 0 as well, which would again force all arming register writes into the next frame giving us the same kind of single frame wakeup latency as in case 2. IIRC you said that you had stuff get stuck with DSB_SKIP_WAITS_EN=0. Was that with hardware that has TRANS_PUSH for PSR? I suppose that could happen if the scanline counter already reads something (eg. again vblank_start-1) before we've done the push, that would cause the vblank evasion to wait forever. I could see two ways to perhaps handle that: - if DSB_SKIP_WAITS_EN becomes inactive immediately after TRANS_PUSH then we could just keep DSB_SKIP_WAITS_EN=1 all the time and things should be fine - if DSB_SKIP_WAITS_EN stays active for some time after TRANS_PUSH then we'll perhaps need to poke that bit from the DSB itself dynamically so that we will skip the vblank evasion, but not the wait for blank prior to generating the interrupt. But this needs a bit more reverse engineering for sure. > > > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index e448ff64660a..58575800fad2 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -7631,6 +7631,7 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, > > intel_atomic_get_old_crtc_state(state, crtc); > > struct intel_crtc_state *new_crtc_state = > > intel_atomic_get_new_crtc_state(state, crtc); > > + struct intel_display *display = to_intel_display(crtc); > > > > if (!new_crtc_state->hw.active) > > return; > > @@ -7643,7 +7644,7 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, > > new_crtc_state->update_planes && > > !new_crtc_state->vrr.enable && > > !new_crtc_state->do_async_flip && > > - !new_crtc_state->has_psr && > > + (DISPLAY_VER(display) >= 20 || !new_crtc_state->has_psr) && > > !new_crtc_state->scaler_state.scaler_users && > > !old_crtc_state->scaler_state.scaler_users && > > !intel_crtc_needs_modeset(new_crtc_state) && > > -- > > 2.43.0 > > -- > Ville Syrjälä > Intel
On Sat, 2025-01-18 at 01:07 +0200, Ville Syrjälä wrote: > On Fri, Jan 17, 2025 at 10:20:17PM +0200, Ville Syrjälä wrote: > > On Thu, Jan 09, 2025 at 09:31:37AM +0200, Jouni Högander wrote: > > > Now as we have correct PSR2_MAN_TRK_CTL handling in place we can > > > allow DSB > > > usage also when PSR is enabled for LunarLake onwards. > > > > We seem to still lack an answer as to when the PSR wakes, when it > > latches the update, and how does all that guarantee that the DSB > > interrupt fires after the update has been latched? > > > > Some thoughts as to how to figure this out: > > 1. make sure we're in PSR > > 2. sample TIMESTAMP_CTR > > 3. start DSB in which > > write PLANE_SURF with a new value > > send push > > wait for vblank > > poll PLANE_SURFLIVE == new value > > fire interrupt > > > > in the interrupt handler: > > sample TIMESTAMP_CTR again > > > > And then compare flip timestmap vs. frame timestamp vs. the > > manually sampled timestamps. And then repeat without the SURFLIVE > > poll to make sure nothing has changed. You'll need to be careful > > to make sure it will actually poll for long enough to make a real > > difference (if the poll actuall is needed), but tweaking the poll > > interval+count suitably. I don't remeber what the max poll > > count was, but IIRC it wasn't too high so the duration will have > > to get bumped for long polls. > > > > I guess one could also try to poll for the actual PSR status, > > but dunno how well that'll work. > > > > And we could also try to come up with different ideas on where > > to sample timestamps. Unfortunately we only have the single > > pipe flip timestamp register so we can only sample one timestamp > > from the DSB itself per frame. If we had more we could much more > > easily figure things out :/ > > > > I pushed my latest DSB selftest stuff to > > https://github.com/vsyrjala/linux.git dsb_selftests_7 > > which has a bunch of stuff for this kind of experimentation. > > It's in a somewhat sorry state at the moment since I last used > > to hunt for various DSB bugs, but at least it still builds :) > > > > The way I use that is that I run igt 'testdisplay -o ...' > > to make sure nothing else is actively poking the hardware > > and then I trigger the DSB tests via debugfs. > > I poked around a bit, though only on a TGL+PSR1 system (what I had > at hand), so some of this might not apply to PSR2 and/or more > modern platforms. > > General notes: > - PSR1 exit is triggered by any pipe/plane register write (even the > non-arming ones) This is same for PSR2 as well. > > We basically have three cases to consider here: > 1. PSR1 is currently inactive > Obviously everything should be with the current code, > vblank evasion works, wait for vblank+interrupt scheme > for flip completion works > > 2. PSR1 is active, but DC states are not > The wakeup latency here is super quick (it's < 1 scanline, how > much below? I've not yet measured), and arming registers do latch > nearly immediately. The scanline counter starts counting > accordingly > from vblank_start-1. However the hardware still considers PSR to be > active for that short duration so DSB_SKIP_WAITS_EN _will_ skip the > waits. > > Unfortunately being this quick I'm not convinced we have enough > time > to write all the registers atomically before the hardware latches > something. So I'm thinking we may need to remove DSB_SKIP_WAITS_EN, > in which case the vblank evasion will push the arming register > writes into the next frame. This will mean the wakeup will take > one full frame. To my understanding DSB_SKIP_WAITS_EN have impact only when in SRD (PSR)/DEEP_SLEEP(PSR2). I.e. In this scenario we still do have all waits as in commit without PSR. > > 3. PSR1 is active and so are DC states > The wakeup latency is ~5ms. During that time scanline counter reads > 0, PSR is active for the purposes of DSB_SKIP_WAITS_EN. Again we > pretty much need DSB_SKIP_WAITS_EN=0 here to make sure the > interrupt > gets signalled after the wait for vblank. vblank evasion will get > skipped on account the scanline being 0. Somewhat ironically this > would give us ~5ms total wakeup latency which is now faster than > the > previous case. Again my understanding is that DSB_SKIP_WAITS_EN=0/1 have impact only when in SRD/DEEP_SLEEP. There is SRD_CTRL/PSR2_CTRL[Idle Frames] to control when entering SRD/DEEP_SLEEP. So wait for vblank is supposed to work normally in this scenario as well because there has to be at least one idle frame before entering sleep. > > So everything here would be fine if we know that the wakeup has > just > started since we have all of that 5ms to write all the registers. > But I guess we can't really know when the wakeup started, so we > might be doing the vblank evasion just before the scanline counter > starts to read its proper value, and then we have that < 1 scanline > to write all the arming registers. Not sure if its enough. If not, > then we could also explicitly evade scanline 0 as well, which would > again force all arming register writes into the next frame giving > us the same kind of single frame wakeup latency as in case 2. hmm, what else could trigger the wakeup than the commit that is on hand ? Frontbuffer flush/legacy cursor update? It begins to look like we still need to take that mutex over DSB commit... > > IIRC you said that you had stuff get stuck with DSB_SKIP_WAITS_EN=0. > Was that with hardware that has TRANS_PUSH for PSR? It has TRANS_PUSH, but not using it for PSR. I.e. On LunarLake and still using register writes as a trigger for Frame Change event towards PSR. It happened with DSB commit where scanline wait was the first thing. I.e. only cursor plane updating. > I suppose that > could happen if the scanline counter already reads something > (eg. again vblank_start-1) before we've done the push, that would > cause the vblank evasion to wait forever. I could see two ways to > perhaps handle that: > - if DSB_SKIP_WAITS_EN becomes inactive immediately after > TRANS_PUSH then we could just keep DSB_SKIP_WAITS_EN=1 all > the time and things should be fine > - if DSB_SKIP_WAITS_EN stays active for some time after TRANS_PUSH > then we'll perhaps need to poke that bit from the DSB itself > dynamically so that we will skip the vblank evasion, but > not the wait for blank prior to generating the interrupt. > But this needs a bit more reverse engineering for sure. TRAN_PUSH is still not enabled in this patch set. BR, Jouni Högander > > > > > > > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > index e448ff64660a..58575800fad2 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -7631,6 +7631,7 @@ static void intel_atomic_dsb_finish(struct > > > intel_atomic_state *state, > > > intel_atomic_get_old_crtc_state(state, crtc); > > > struct intel_crtc_state *new_crtc_state = > > > intel_atomic_get_new_crtc_state(state, crtc); > > > + struct intel_display *display = to_intel_display(crtc); > > > > > > if (!new_crtc_state->hw.active) > > > return; > > > @@ -7643,7 +7644,7 @@ static void intel_atomic_dsb_finish(struct > > > intel_atomic_state *state, > > > new_crtc_state->update_planes && > > > !new_crtc_state->vrr.enable && > > > !new_crtc_state->do_async_flip && > > > - !new_crtc_state->has_psr && > > > + (DISPLAY_VER(display) >= 20 || !new_crtc_state- > > > >has_psr) && > > > !new_crtc_state->scaler_state.scaler_users && > > > !old_crtc_state->scaler_state.scaler_users && > > > !intel_crtc_needs_modeset(new_crtc_state) && > > > -- > > > 2.43.0 > > > > -- > > Ville Syrjälä > > Intel >
On Mon, Jan 20, 2025 at 07:28:52AM +0000, Hogander, Jouni wrote: > On Sat, 2025-01-18 at 01:07 +0200, Ville Syrjälä wrote: > > On Fri, Jan 17, 2025 at 10:20:17PM +0200, Ville Syrjälä wrote: > > > On Thu, Jan 09, 2025 at 09:31:37AM +0200, Jouni Högander wrote: > > > > Now as we have correct PSR2_MAN_TRK_CTL handling in place we can > > > > allow DSB > > > > usage also when PSR is enabled for LunarLake onwards. > > > > > > We seem to still lack an answer as to when the PSR wakes, when it > > > latches the update, and how does all that guarantee that the DSB > > > interrupt fires after the update has been latched? > > > > > > Some thoughts as to how to figure this out: > > > 1. make sure we're in PSR > > > 2. sample TIMESTAMP_CTR > > > 3. start DSB in which > > > write PLANE_SURF with a new value > > > send push > > > wait for vblank > > > poll PLANE_SURFLIVE == new value > > > fire interrupt > > > > > > in the interrupt handler: > > > sample TIMESTAMP_CTR again > > > > > > And then compare flip timestmap vs. frame timestamp vs. the > > > manually sampled timestamps. And then repeat without the SURFLIVE > > > poll to make sure nothing has changed. You'll need to be careful > > > to make sure it will actually poll for long enough to make a real > > > difference (if the poll actuall is needed), but tweaking the poll > > > interval+count suitably. I don't remeber what the max poll > > > count was, but IIRC it wasn't too high so the duration will have > > > to get bumped for long polls. > > > > > > I guess one could also try to poll for the actual PSR status, > > > but dunno how well that'll work. > > > > > > And we could also try to come up with different ideas on where > > > to sample timestamps. Unfortunately we only have the single > > > pipe flip timestamp register so we can only sample one timestamp > > > from the DSB itself per frame. If we had more we could much more > > > easily figure things out :/ > > > > > > I pushed my latest DSB selftest stuff to > > > https://github.com/vsyrjala/linux.git dsb_selftests_7 > > > which has a bunch of stuff for this kind of experimentation. > > > It's in a somewhat sorry state at the moment since I last used > > > to hunt for various DSB bugs, but at least it still builds :) > > > > > > The way I use that is that I run igt 'testdisplay -o ...' > > > to make sure nothing else is actively poking the hardware > > > and then I trigger the DSB tests via debugfs. > > > > I poked around a bit, though only on a TGL+PSR1 system (what I had > > at hand), so some of this might not apply to PSR2 and/or more > > modern platforms. > > > > General notes: > > - PSR1 exit is triggered by any pipe/plane register write (even the > > non-arming ones) > > This is same for PSR2 as well. > > > > > We basically have three cases to consider here: > > 1. PSR1 is currently inactive > > Obviously everything should be with the current code, > > vblank evasion works, wait for vblank+interrupt scheme > > for flip completion works > > > > 2. PSR1 is active, but DC states are not > > The wakeup latency here is super quick (it's < 1 scanline, how > > much below? I've not yet measured), and arming registers do latch > > nearly immediately. Actually it's only ~1usec (based on the timestamps). I also used the following DSB batch to test how many registers we can write there: for (i = 0; ...; i++) dsb_write(PLANE_SURF, i << 12) dsb_interrupt() and then in the interrupt handler I read PLANE_SURFLIVE, and it always shows 0xa000, meaning we only have time to write ten registers. So definitely not enough to guarantee that all arming registers get written. So, for PSR1 at least I think we'd have two options: 1) do a manual PSR wake around the whole commit, which doesn't sound very nice 2) evade hw_scaline==0 so that we wait until we've woken up from the DC state, and then proceed with the normal vblank evasion and arming register writes. And obviously all that only works in DSB_SKIP_WAITS_EN is disabled. Also if all the pipes are doing the full update using the DSB then we could perhaps also remove the explicit DC_OFF dance around the whole commit. > The scanline counter starts counting > > accordingly > > from vblank_start-1. However the hardware still considers PSR to be > > active for that short duration so DSB_SKIP_WAITS_EN _will_ skip the > > waits. > > > > Unfortunately being this quick I'm not convinced we have enough > > time > > to write all the registers atomically before the hardware latches > > something. So I'm thinking we may need to remove DSB_SKIP_WAITS_EN, > > in which case the vblank evasion will push the arming register > > writes into the next frame. This will mean the wakeup will take > > one full frame. > > To my understanding DSB_SKIP_WAITS_EN have impact only when in SRD > (PSR)/DEEP_SLEEP(PSR2). I.e. In this scenario we still do have all > waits as in commit without PSR. The PSR state machine is already in link off mode in this case, meaning the pipe has been halted, and the display has been signalled to scan out from its RFB (confirmed by checking the PSR status register in DPCD), and DSB_SKIP_WAITS_EN is already active. But the link is still actually up (I'm guessing it might be transmitting the idle pattern, but I've not confirmed that. Can't remeber if we even have any kind of status register that could show this...). So looks like the link only gets actually turned off by the DMC when entering the DC state. > > > > > 3. PSR1 is active and so are DC states > > The wakeup latency is ~5ms. During that time scanline counter reads > > 0, PSR is active for the purposes of DSB_SKIP_WAITS_EN. Again we > > pretty much need DSB_SKIP_WAITS_EN=0 here to make sure the > > interrupt > > gets signalled after the wait for vblank. vblank evasion will get > > skipped on account the scanline being 0. Somewhat ironically this > > would give us ~5ms total wakeup latency which is now faster than > > the > > previous case. > > Again my understanding is that DSB_SKIP_WAITS_EN=0/1 have impact only > when in SRD/DEEP_SLEEP. There is SRD_CTRL/PSR2_CTRL[Idle Frames] to > control when entering SRD/DEEP_SLEEP. The PSR state machine is what matters. That stops the pipe, and causes DSP_SKIP_WAITS_EN to become active. And the PSR state machine is also what is affected by the idle frames stuff/etc. AFAIK the only stuff that is actually done by the DMC is turning the link/PLL/etc on and off. And obviously link training I guess gets triggered from there somehow when waking up from PSR, hence why the wakeup takes that 5ms longer than when DC States are disabled. > So wait for vblank is supposed to > work normally in this scenario as well because there has to be at least > one idle frame before entering sleep. > > > > > So everything here would be fine if we know that the wakeup has > > just > > started since we have all of that 5ms to write all the registers. > > But I guess we can't really know when the wakeup started, so we > > might be doing the vblank evasion just before the scanline counter > > starts to read its proper value, and then we have that < 1 scanline > > to write all the arming registers. Not sure if its enough. If not, > > then we could also explicitly evade scanline 0 as well, which would > > again force all arming register writes into the next frame giving > > us the same kind of single frame wakeup latency as in case 2. > > hmm, what else could trigger the wakeup than the commit that is on hand > ? Frontbuffer flush/legacy cursor update? Could be any register access really. So yeah those, interrupts, sysfs/debugfs accesses. Ie. all kinds of stuff we can't possibly keep track of IMO. > It begins to look like we > still need to take that mutex over DSB commit... I don't think we need that mutex unless we go with the "let's manually wake from PSR and wait for the exit" approach. > > > > > IIRC you said that you had stuff get stuck with DSB_SKIP_WAITS_EN=0. > > Was that with hardware that has TRANS_PUSH for PSR? > > It has TRANS_PUSH, but not using it for PSR. I.e. On LunarLake and > still using register writes as a trigger for Frame Change event towards > PSR. > > It happened with DSB commit where scanline wait was the first thing. > I.e. only cursor plane updating. > > > I suppose that > > could happen if the scanline counter already reads something > > (eg. again vblank_start-1) before we've done the push, that would > > cause the vblank evasion to wait forever. I could see two ways to > > perhaps handle that: > > - if DSB_SKIP_WAITS_EN becomes inactive immediately after > > TRANS_PUSH then we could just keep DSB_SKIP_WAITS_EN=1 all > > the time and things should be fine > > - if DSB_SKIP_WAITS_EN stays active for some time after TRANS_PUSH > > then we'll perhaps need to poke that bit from the DSB itself > > dynamically so that we will skip the vblank evasion, but > > not the wait for blank prior to generating the interrupt. > > But this needs a bit more reverse engineering for sure. > > TRAN_PUSH is still not enabled in this patch set. > > BR, > > Jouni Högander > > > > > > > > > > > > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_display.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > index e448ff64660a..58575800fad2 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > @@ -7631,6 +7631,7 @@ static void intel_atomic_dsb_finish(struct > > > > intel_atomic_state *state, > > > > intel_atomic_get_old_crtc_state(state, crtc); > > > > struct intel_crtc_state *new_crtc_state = > > > > intel_atomic_get_new_crtc_state(state, crtc); > > > > + struct intel_display *display = to_intel_display(crtc); > > > > > > > > if (!new_crtc_state->hw.active) > > > > return; > > > > @@ -7643,7 +7644,7 @@ static void intel_atomic_dsb_finish(struct > > > > intel_atomic_state *state, > > > > new_crtc_state->update_planes && > > > > !new_crtc_state->vrr.enable && > > > > !new_crtc_state->do_async_flip && > > > > - !new_crtc_state->has_psr && > > > > + (DISPLAY_VER(display) >= 20 || !new_crtc_state- > > > > >has_psr) && > > > > !new_crtc_state->scaler_state.scaler_users && > > > > !old_crtc_state->scaler_state.scaler_users && > > > > !intel_crtc_needs_modeset(new_crtc_state) && > > > > -- > > > > 2.43.0 > > > > > > -- > > > Ville Syrjälä > > > Intel > > >
On Mon, Jan 20, 2025 at 04:39:24PM +0200, Ville Syrjälä wrote: > On Mon, Jan 20, 2025 at 07:28:52AM +0000, Hogander, Jouni wrote: > > On Sat, 2025-01-18 at 01:07 +0200, Ville Syrjälä wrote: > > > On Fri, Jan 17, 2025 at 10:20:17PM +0200, Ville Syrjälä wrote: > > > > On Thu, Jan 09, 2025 at 09:31:37AM +0200, Jouni Högander wrote: > > > > > Now as we have correct PSR2_MAN_TRK_CTL handling in place we can > > > > > allow DSB > > > > > usage also when PSR is enabled for LunarLake onwards. > > > > > > > > We seem to still lack an answer as to when the PSR wakes, when it > > > > latches the update, and how does all that guarantee that the DSB > > > > interrupt fires after the update has been latched? > > > > > > > > Some thoughts as to how to figure this out: > > > > 1. make sure we're in PSR > > > > 2. sample TIMESTAMP_CTR > > > > 3. start DSB in which > > > > write PLANE_SURF with a new value > > > > send push > > > > wait for vblank > > > > poll PLANE_SURFLIVE == new value > > > > fire interrupt > > > > > > > > in the interrupt handler: > > > > sample TIMESTAMP_CTR again > > > > > > > > And then compare flip timestmap vs. frame timestamp vs. the > > > > manually sampled timestamps. And then repeat without the SURFLIVE > > > > poll to make sure nothing has changed. You'll need to be careful > > > > to make sure it will actually poll for long enough to make a real > > > > difference (if the poll actuall is needed), but tweaking the poll > > > > interval+count suitably. I don't remeber what the max poll > > > > count was, but IIRC it wasn't too high so the duration will have > > > > to get bumped for long polls. > > > > > > > > I guess one could also try to poll for the actual PSR status, > > > > but dunno how well that'll work. > > > > > > > > And we could also try to come up with different ideas on where > > > > to sample timestamps. Unfortunately we only have the single > > > > pipe flip timestamp register so we can only sample one timestamp > > > > from the DSB itself per frame. If we had more we could much more > > > > easily figure things out :/ > > > > > > > > I pushed my latest DSB selftest stuff to > > > > https://github.com/vsyrjala/linux.git dsb_selftests_7 > > > > which has a bunch of stuff for this kind of experimentation. > > > > It's in a somewhat sorry state at the moment since I last used > > > > to hunt for various DSB bugs, but at least it still builds :) > > > > > > > > The way I use that is that I run igt 'testdisplay -o ...' > > > > to make sure nothing else is actively poking the hardware > > > > and then I trigger the DSB tests via debugfs. > > > > > > I poked around a bit, though only on a TGL+PSR1 system (what I had > > > at hand), so some of this might not apply to PSR2 and/or more > > > modern platforms. > > > > > > General notes: > > > - PSR1 exit is triggered by any pipe/plane register write (even the > > > non-arming ones) > > > > This is same for PSR2 as well. > > > > > > > > We basically have three cases to consider here: > > > 1. PSR1 is currently inactive > > > Obviously everything should be with the current code, > > > vblank evasion works, wait for vblank+interrupt scheme > > > for flip completion works > > > > > > 2. PSR1 is active, but DC states are not > > > The wakeup latency here is super quick (it's < 1 scanline, how > > > much below? I've not yet measured), and arming registers do latch > > > nearly immediately. > > Actually it's only ~1usec (based on the timestamps). I also used the > following DSB batch to test how many registers we can write there: > > for (i = 0; ...; i++) > dsb_write(PLANE_SURF, i << 12) > dsb_interrupt() > > and then in the interrupt handler I read PLANE_SURFLIVE, and it always > shows 0xa000, meaning we only have time to write ten registers. So > definitely not enough to guarantee that all arming registers get > written. Oh, and I also checked whether increasing the vblank delay would give us more time here, but unfortunately it does not. So looks like the timing of this "fake vblank" doesn't take that into account :/ > > So, for PSR1 at least I think we'd have two options: > 1) do a manual PSR wake around the whole commit, which doesn't > sound very nice > 2) evade hw_scaline==0 so that we wait until we've woken up > from the DC state, and then proceed with the normal vblank > evasion and arming register writes. And obviously all that > only works in DSB_SKIP_WAITS_EN is disabled. Also if all > the pipes are doing the full update using the DSB then we > could perhaps also remove the explicit DC_OFF dance around > the whole commit. > > > The scanline counter starts counting > > > accordingly > > > from vblank_start-1. However the hardware still considers PSR to be > > > active for that short duration so DSB_SKIP_WAITS_EN _will_ skip the > > > waits. > > > > > > Unfortunately being this quick I'm not convinced we have enough > > > time > > > to write all the registers atomically before the hardware latches > > > something. So I'm thinking we may need to remove DSB_SKIP_WAITS_EN, > > > in which case the vblank evasion will push the arming register > > > writes into the next frame. This will mean the wakeup will take > > > one full frame. > > > > To my understanding DSB_SKIP_WAITS_EN have impact only when in SRD > > (PSR)/DEEP_SLEEP(PSR2). I.e. In this scenario we still do have all > > waits as in commit without PSR. > > The PSR state machine is already in link off mode in this case, > meaning the pipe has been halted, and the display has been > signalled to scan out from its RFB (confirmed by checking the > PSR status register in DPCD), and DSB_SKIP_WAITS_EN is already > active. But the link is still actually up (I'm guessing it might > be transmitting the idle pattern, but I've not confirmed that. > Can't remeber if we even have any kind of status register that > could show this...). So looks like the link only gets actually > turned off by the DMC when entering the DC state. DP_TP_STATUS to the rescue. Monitoring that while in PSR (with DC states still disabled) I see: when active: DP_TP_STATUS: 0x00700000 DP Stream Status: 1 DP Init Status: Active SST when in PSR: DP_TP_STATUS: 0x00000000 DP Stream Status: 0 DP Init Status: Pattern1 when transitioning between the two: DP_TP_STATUS: 0x00200000 DP Stream Status: 0 DP Init Status: Idle SST DP_TP_STATUS: 0x00100000 DP Stream Status: 0 DP Init Status: Pattern3 So looks like it's sort of doing link training always. But since the link doesn't get turned off when DC states aren't enabled the it's just practically immediate. And I now also confirmed that the 5ms wakeup time I was seeing with DC states enabled corresponds to the TP1/TP3 times configured in SRD_CTL. Reducing those also reduces the wakeup time accordingly.
On Mon, 2025-01-20 at 16:39 +0200, Ville Syrjälä wrote: > On Mon, Jan 20, 2025 at 07:28:52AM +0000, Hogander, Jouni wrote: > > On Sat, 2025-01-18 at 01:07 +0200, Ville Syrjälä wrote: > > > On Fri, Jan 17, 2025 at 10:20:17PM +0200, Ville Syrjälä wrote: > > > > On Thu, Jan 09, 2025 at 09:31:37AM +0200, Jouni Högander wrote: > > > > > Now as we have correct PSR2_MAN_TRK_CTL handling in place we > > > > > can > > > > > allow DSB > > > > > usage also when PSR is enabled for LunarLake onwards. > > > > > > > > We seem to still lack an answer as to when the PSR wakes, when > > > > it > > > > latches the update, and how does all that guarantee that the > > > > DSB > > > > interrupt fires after the update has been latched? > > > > > > > > Some thoughts as to how to figure this out: > > > > 1. make sure we're in PSR > > > > 2. sample TIMESTAMP_CTR > > > > 3. start DSB in which > > > > write PLANE_SURF with a new value > > > > send push > > > > wait for vblank > > > > poll PLANE_SURFLIVE == new value > > > > fire interrupt > > > > > > > > in the interrupt handler: > > > > sample TIMESTAMP_CTR again > > > > > > > > And then compare flip timestmap vs. frame timestamp vs. the > > > > manually sampled timestamps. And then repeat without the > > > > SURFLIVE > > > > poll to make sure nothing has changed. You'll need to be > > > > careful > > > > to make sure it will actually poll for long enough to make a > > > > real > > > > difference (if the poll actuall is needed), but tweaking the > > > > poll > > > > interval+count suitably. I don't remeber what the max poll > > > > count was, but IIRC it wasn't too high so the duration will > > > > have > > > > to get bumped for long polls. > > > > > > > > I guess one could also try to poll for the actual PSR status, > > > > but dunno how well that'll work. > > > > > > > > And we could also try to come up with different ideas on where > > > > to sample timestamps. Unfortunately we only have the single > > > > pipe flip timestamp register so we can only sample one > > > > timestamp > > > > from the DSB itself per frame. If we had more we could much > > > > more > > > > easily figure things out :/ > > > > > > > > I pushed my latest DSB selftest stuff to > > > > https://github.com/vsyrjala/linux.git dsb_selftests_7 > > > > which has a bunch of stuff for this kind of experimentation. > > > > It's in a somewhat sorry state at the moment since I last used > > > > to hunt for various DSB bugs, but at least it still builds :) > > > > > > > > The way I use that is that I run igt 'testdisplay -o ...' > > > > to make sure nothing else is actively poking the hardware > > > > and then I trigger the DSB tests via debugfs. > > > > > > I poked around a bit, though only on a TGL+PSR1 system (what I > > > had > > > at hand), so some of this might not apply to PSR2 and/or more > > > modern platforms. > > > > > > General notes: > > > - PSR1 exit is triggered by any pipe/plane register write (even > > > the > > > non-arming ones) > > > > This is same for PSR2 as well. > > > > > > > > We basically have three cases to consider here: > > > 1. PSR1 is currently inactive > > > Obviously everything should be with the current code, > > > vblank evasion works, wait for vblank+interrupt scheme > > > for flip completion works > > > > > > 2. PSR1 is active, but DC states are not > > > The wakeup latency here is super quick (it's < 1 scanline, how > > > much below? I've not yet measured), and arming registers do > > > latch > > > nearly immediately. > > Actually it's only ~1usec (based on the timestamps). I also used the > following DSB batch to test how many registers we can write there: > > for (i = 0; ...; i++) > dsb_write(PLANE_SURF, i << 12) > dsb_interrupt() > > and then in the interrupt handler I read PLANE_SURFLIVE, and it > always > shows 0xa000, meaning we only have time to write ten registers. So > definitely not enough to guarantee that all arming registers get > written. > > So, for PSR1 at least I think we'd have two options: > 1) do a manual PSR wake around the whole commit, which doesn't > sound very nice > 2) evade hw_scaline==0 so that we wait until we've woken up > from the DC state, and then proceed with the normal vblank > evasion and arming register writes. And obviously all that > only works in DSB_SKIP_WAITS_EN is disabled. Also if all > the pipes are doing the full update using the DSB then we > could perhaps also remove the explicit DC_OFF dance around > the whole commit. > > > The scanline counter starts counting > > > accordingly > > > from vblank_start-1. However the hardware still considers PSR > > > to be > > > active for that short duration so DSB_SKIP_WAITS_EN _will_ skip > > > the > > > waits. > > > > > > Unfortunately being this quick I'm not convinced we have enough > > > time > > > to write all the registers atomically before the hardware > > > latches > > > something. So I'm thinking we may need to remove > > > DSB_SKIP_WAITS_EN, > > > in which case the vblank evasion will push the arming register > > > writes into the next frame. This will mean the wakeup will take > > > one full frame. > > > > To my understanding DSB_SKIP_WAITS_EN have impact only when in SRD > > (PSR)/DEEP_SLEEP(PSR2). I.e. In this scenario we still do have all > > waits as in commit without PSR. > > The PSR state machine is already in link off mode in this case, > meaning the pipe has been halted, and the display has been > signalled to scan out from its RFB (confirmed by checking the > PSR status register in DPCD), and DSB_SKIP_WAITS_EN is already > active. But the link is still actually up (I'm guessing it might > be transmitting the idle pattern, but I've not confirmed that. > Can't remeber if we even have any kind of status register that > could show this...). So looks like the link only gets actually > turned off by the DMC when entering the DC state. > > > > > > > > > 3. PSR1 is active and so are DC states > > > The wakeup latency is ~5ms. During that time scanline counter > > > reads > > > 0, PSR is active for the purposes of DSB_SKIP_WAITS_EN. Again > > > we > > > pretty much need DSB_SKIP_WAITS_EN=0 here to make sure the > > > interrupt > > > gets signalled after the wait for vblank. vblank evasion will > > > get > > > skipped on account the scanline being 0. Somewhat ironically > > > this > > > would give us ~5ms total wakeup latency which is now faster > > > than > > > the > > > previous case. > > > > Again my understanding is that DSB_SKIP_WAITS_EN=0/1 have impact > > only > > when in SRD/DEEP_SLEEP. There is SRD_CTRL/PSR2_CTRL[Idle Frames] to > > control when entering SRD/DEEP_SLEEP. > > The PSR state machine is what matters. That stops the pipe, and > causes > DSP_SKIP_WAITS_EN to become active. And the PSR state machine is also > what is affected by the idle frames stuff/etc. AFAIK the only stuff > that is actually done by the DMC is turning the link/PLL/etc on and > off. And obviously link training I guess gets triggered from there > somehow when waking up from PSR, hence why the wakeup takes that 5ms > longer than when DC States are disabled. Ok, I need do some investigation on this. The failure I was referring was this one: https://intel-gfx-ci.01.org/tree/intel-xe/xe-pw-142521v2/shard-lnl-8/igt@kms_psr2_sf@psr2-cursor-plane-move-continuous-exceed-fully-sf.html I.e. there is that scanline wait at the begin in DSB buffer. It was hanging in waiting the scanline. I verified this by checking if preceding write to 0x700300 completes and it never did. This was with PSR2 but not with Panel Replay. Also if I disable DEEP_SLEEP by setting PSR2_CTL[idle frames] as 0 it didn't hang. Also waking it up by writing e.g. CURSURFLIVE helped. BR, Jouni Högander > > > So wait for vblank is supposed to > > work normally in this scenario as well because there has to be at > > least > > one idle frame before entering sleep. > > > > > > > > So everything here would be fine if we know that the wakeup has > > > just > > > started since we have all of that 5ms to write all the > > > registers. > > > But I guess we can't really know when the wakeup started, so we > > > might be doing the vblank evasion just before the scanline > > > counter > > > starts to read its proper value, and then we have that < 1 > > > scanline > > > to write all the arming registers. Not sure if its enough. If > > > not, > > > then we could also explicitly evade scanline 0 as well, which > > > would > > > again force all arming register writes into the next frame > > > giving > > > us the same kind of single frame wakeup latency as in case 2. > > > > hmm, what else could trigger the wakeup than the commit that is on > > hand > > ? Frontbuffer flush/legacy cursor update? > > Could be any register access really. So yeah those, interrupts, > sysfs/debugfs accesses. Ie. all kinds of stuff we can't possibly > keep track of IMO. > > > It begins to look like we > > still need to take that mutex over DSB commit... > > I don't think we need that mutex unless we go with the > "let's manually wake from PSR and wait for the exit" approach. > > > > > > > > > IIRC you said that you had stuff get stuck with > > > DSB_SKIP_WAITS_EN=0. > > > Was that with hardware that has TRANS_PUSH for PSR? > > > > It has TRANS_PUSH, but not using it for PSR. I.e. On LunarLake and > > still using register writes as a trigger for Frame Change event > > towards > > PSR. > > > > It happened with DSB commit where scanline wait was the first > > thing. > > I.e. only cursor plane updating. > > > > > I suppose that > > > could happen if the scanline counter already reads something > > > (eg. again vblank_start-1) before we've done the push, that would > > > cause the vblank evasion to wait forever. I could see two ways to > > > perhaps handle that: > > > - if DSB_SKIP_WAITS_EN becomes inactive immediately after > > > TRANS_PUSH then we could just keep DSB_SKIP_WAITS_EN=1 all > > > the time and things should be fine > > > - if DSB_SKIP_WAITS_EN stays active for some time after > > > TRANS_PUSH > > > then we'll perhaps need to poke that bit from the DSB itself > > > dynamically so that we will skip the vblank evasion, but > > > not the wait for blank prior to generating the interrupt. > > > But this needs a bit more reverse engineering for sure. > > > > TRAN_PUSH is still not enabled in this patch set. > > > > BR, > > > > Jouni Högander > > > > > > > > > > > > > > > > > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_display.c | 3 ++- > > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > > > > b/drivers/gpu/drm/i915/display/intel_display.c > > > > > index e448ff64660a..58575800fad2 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > @@ -7631,6 +7631,7 @@ static void > > > > > intel_atomic_dsb_finish(struct > > > > > intel_atomic_state *state, > > > > > intel_atomic_get_old_crtc_state(state, > > > > > crtc); > > > > > struct intel_crtc_state *new_crtc_state = > > > > > intel_atomic_get_new_crtc_state(state, > > > > > crtc); > > > > > + struct intel_display *display = > > > > > to_intel_display(crtc); > > > > > > > > > > if (!new_crtc_state->hw.active) > > > > > return; > > > > > @@ -7643,7 +7644,7 @@ static void > > > > > intel_atomic_dsb_finish(struct > > > > > intel_atomic_state *state, > > > > > new_crtc_state->update_planes && > > > > > !new_crtc_state->vrr.enable && > > > > > !new_crtc_state->do_async_flip && > > > > > - !new_crtc_state->has_psr && > > > > > + (DISPLAY_VER(display) >= 20 || > > > > > !new_crtc_state- > > > > > > has_psr) && > > > > > !new_crtc_state->scaler_state.scaler_users > > > > > && > > > > > !old_crtc_state->scaler_state.scaler_users > > > > > && > > > > > !intel_crtc_needs_modeset(new_crtc_state) && > > > > > -- > > > > > 2.43.0 > > > > > > > > -- > > > > Ville Syrjälä > > > > Intel > > > > > >
On Tue, Jan 21, 2025 at 10:29:25AM +0000, Hogander, Jouni wrote: > On Mon, 2025-01-20 at 16:39 +0200, Ville Syrjälä wrote: > > On Mon, Jan 20, 2025 at 07:28:52AM +0000, Hogander, Jouni wrote: > > > On Sat, 2025-01-18 at 01:07 +0200, Ville Syrjälä wrote: > > > > On Fri, Jan 17, 2025 at 10:20:17PM +0200, Ville Syrjälä wrote: > > > > > On Thu, Jan 09, 2025 at 09:31:37AM +0200, Jouni Högander wrote: > > > > > > Now as we have correct PSR2_MAN_TRK_CTL handling in place we > > > > > > can > > > > > > allow DSB > > > > > > usage also when PSR is enabled for LunarLake onwards. > > > > > > > > > > We seem to still lack an answer as to when the PSR wakes, when > > > > > it > > > > > latches the update, and how does all that guarantee that the > > > > > DSB > > > > > interrupt fires after the update has been latched? > > > > > > > > > > Some thoughts as to how to figure this out: > > > > > 1. make sure we're in PSR > > > > > 2. sample TIMESTAMP_CTR > > > > > 3. start DSB in which > > > > > write PLANE_SURF with a new value > > > > > send push > > > > > wait for vblank > > > > > poll PLANE_SURFLIVE == new value > > > > > fire interrupt > > > > > > > > > > in the interrupt handler: > > > > > sample TIMESTAMP_CTR again > > > > > > > > > > And then compare flip timestmap vs. frame timestamp vs. the > > > > > manually sampled timestamps. And then repeat without the > > > > > SURFLIVE > > > > > poll to make sure nothing has changed. You'll need to be > > > > > careful > > > > > to make sure it will actually poll for long enough to make a > > > > > real > > > > > difference (if the poll actuall is needed), but tweaking the > > > > > poll > > > > > interval+count suitably. I don't remeber what the max poll > > > > > count was, but IIRC it wasn't too high so the duration will > > > > > have > > > > > to get bumped for long polls. > > > > > > > > > > I guess one could also try to poll for the actual PSR status, > > > > > but dunno how well that'll work. > > > > > > > > > > And we could also try to come up with different ideas on where > > > > > to sample timestamps. Unfortunately we only have the single > > > > > pipe flip timestamp register so we can only sample one > > > > > timestamp > > > > > from the DSB itself per frame. If we had more we could much > > > > > more > > > > > easily figure things out :/ > > > > > > > > > > I pushed my latest DSB selftest stuff to > > > > > https://github.com/vsyrjala/linux.git dsb_selftests_7 > > > > > which has a bunch of stuff for this kind of experimentation. > > > > > It's in a somewhat sorry state at the moment since I last used > > > > > to hunt for various DSB bugs, but at least it still builds :) > > > > > > > > > > The way I use that is that I run igt 'testdisplay -o ...' > > > > > to make sure nothing else is actively poking the hardware > > > > > and then I trigger the DSB tests via debugfs. > > > > > > > > I poked around a bit, though only on a TGL+PSR1 system (what I > > > > had > > > > at hand), so some of this might not apply to PSR2 and/or more > > > > modern platforms. > > > > > > > > General notes: > > > > - PSR1 exit is triggered by any pipe/plane register write (even > > > > the > > > > non-arming ones) > > > > > > This is same for PSR2 as well. > > > > > > > > > > > We basically have three cases to consider here: > > > > 1. PSR1 is currently inactive > > > > Obviously everything should be with the current code, > > > > vblank evasion works, wait for vblank+interrupt scheme > > > > for flip completion works > > > > > > > > 2. PSR1 is active, but DC states are not > > > > The wakeup latency here is super quick (it's < 1 scanline, how > > > > much below? I've not yet measured), and arming registers do > > > > latch > > > > nearly immediately. > > > > Actually it's only ~1usec (based on the timestamps). I also used the > > following DSB batch to test how many registers we can write there: > > > > for (i = 0; ...; i++) > > dsb_write(PLANE_SURF, i << 12) > > dsb_interrupt() > > > > and then in the interrupt handler I read PLANE_SURFLIVE, and it > > always > > shows 0xa000, meaning we only have time to write ten registers. So > > definitely not enough to guarantee that all arming registers get > > written. > > > > So, for PSR1 at least I think we'd have two options: > > 1) do a manual PSR wake around the whole commit, which doesn't > > sound very nice > > 2) evade hw_scaline==0 so that we wait until we've woken up > > from the DC state, and then proceed with the normal vblank > > evasion and arming register writes. And obviously all that > > only works in DSB_SKIP_WAITS_EN is disabled. Also if all > > the pipes are doing the full update using the DSB then we > > could perhaps also remove the explicit DC_OFF dance around > > the whole commit. > > > > > The scanline counter starts counting > > > > accordingly > > > > from vblank_start-1. However the hardware still considers PSR > > > > to be > > > > active for that short duration so DSB_SKIP_WAITS_EN _will_ skip > > > > the > > > > waits. > > > > > > > > Unfortunately being this quick I'm not convinced we have enough > > > > time > > > > to write all the registers atomically before the hardware > > > > latches > > > > something. So I'm thinking we may need to remove > > > > DSB_SKIP_WAITS_EN, > > > > in which case the vblank evasion will push the arming register > > > > writes into the next frame. This will mean the wakeup will take > > > > one full frame. > > > > > > To my understanding DSB_SKIP_WAITS_EN have impact only when in SRD > > > (PSR)/DEEP_SLEEP(PSR2). I.e. In this scenario we still do have all > > > waits as in commit without PSR. > > > > The PSR state machine is already in link off mode in this case, > > meaning the pipe has been halted, and the display has been > > signalled to scan out from its RFB (confirmed by checking the > > PSR status register in DPCD), and DSB_SKIP_WAITS_EN is already > > active. But the link is still actually up (I'm guessing it might > > be transmitting the idle pattern, but I've not confirmed that. > > Can't remeber if we even have any kind of status register that > > could show this...). So looks like the link only gets actually > > turned off by the DMC when entering the DC state. > > > > > > > > > > > > > 3. PSR1 is active and so are DC states > > > > The wakeup latency is ~5ms. During that time scanline counter > > > > reads > > > > 0, PSR is active for the purposes of DSB_SKIP_WAITS_EN. Again > > > > we > > > > pretty much need DSB_SKIP_WAITS_EN=0 here to make sure the > > > > interrupt > > > > gets signalled after the wait for vblank. vblank evasion will > > > > get > > > > skipped on account the scanline being 0. Somewhat ironically > > > > this > > > > would give us ~5ms total wakeup latency which is now faster > > > > than > > > > the > > > > previous case. > > > > > > Again my understanding is that DSB_SKIP_WAITS_EN=0/1 have impact > > > only > > > when in SRD/DEEP_SLEEP. There is SRD_CTRL/PSR2_CTRL[Idle Frames] to > > > control when entering SRD/DEEP_SLEEP. > > > > The PSR state machine is what matters. That stops the pipe, and > > causes > > DSP_SKIP_WAITS_EN to become active. And the PSR state machine is also > > what is affected by the idle frames stuff/etc. AFAIK the only stuff > > that is actually done by the DMC is turning the link/PLL/etc on and > > off. And obviously link training I guess gets triggered from there > > somehow when waking up from PSR, hence why the wakeup takes that 5ms > > longer than when DC States are disabled. > > Ok, I need do some investigation on this. The failure I was referring > was this one: > > https://intel-gfx-ci.01.org/tree/intel-xe/xe-pw-142521v2/shard-lnl-8/igt@kms_psr2_sf@psr2-cursor-plane-move-continuous-exceed-fully-sf.html > > I.e. there is that scanline wait at the begin in DSB buffer. It was > hanging in waiting the scanline. I verified this by checking if > preceding write to 0x700300 completes and it never did. > > This was with PSR2 but not with Panel Replay. Also if I disable > DEEP_SLEEP by setting PSR2_CTL[idle frames] as 0 it didn't hang. Also > waking it up by writing e.g. CURSURFLIVE helped. Right, so the problem there really looks to be that we don't have any non-arming register writes to trigger the wakeup (because we don't have the noarm/arm split done for cursors). So yeah, we probably need to always make sure we put some kind of dummy write to the noarm section. CURSURFLIVE should be fine for that I guess.
On Tue, Jan 21, 2025 at 10:29:25AM +0000, Hogander, Jouni wrote: > On Mon, 2025-01-20 at 16:39 +0200, Ville Syrjälä wrote: > > On Mon, Jan 20, 2025 at 07:28:52AM +0000, Hogander, Jouni wrote: > > > On Sat, 2025-01-18 at 01:07 +0200, Ville Syrjälä wrote: > > > > On Fri, Jan 17, 2025 at 10:20:17PM +0200, Ville Syrjälä wrote: > > > > > On Thu, Jan 09, 2025 at 09:31:37AM +0200, Jouni Högander wrote: > > > > > > Now as we have correct PSR2_MAN_TRK_CTL handling in place we > > > > > > can > > > > > > allow DSB > > > > > > usage also when PSR is enabled for LunarLake onwards. > > > > > > > > > > We seem to still lack an answer as to when the PSR wakes, when > > > > > it > > > > > latches the update, and how does all that guarantee that the > > > > > DSB > > > > > interrupt fires after the update has been latched? > > > > > > > > > > Some thoughts as to how to figure this out: > > > > > 1. make sure we're in PSR > > > > > 2. sample TIMESTAMP_CTR > > > > > 3. start DSB in which > > > > > write PLANE_SURF with a new value > > > > > send push > > > > > wait for vblank > > > > > poll PLANE_SURFLIVE == new value > > > > > fire interrupt > > > > > > > > > > in the interrupt handler: > > > > > sample TIMESTAMP_CTR again > > > > > > > > > > And then compare flip timestmap vs. frame timestamp vs. the > > > > > manually sampled timestamps. And then repeat without the > > > > > SURFLIVE > > > > > poll to make sure nothing has changed. You'll need to be > > > > > careful > > > > > to make sure it will actually poll for long enough to make a > > > > > real > > > > > difference (if the poll actuall is needed), but tweaking the > > > > > poll > > > > > interval+count suitably. I don't remeber what the max poll > > > > > count was, but IIRC it wasn't too high so the duration will > > > > > have > > > > > to get bumped for long polls. > > > > > > > > > > I guess one could also try to poll for the actual PSR status, > > > > > but dunno how well that'll work. > > > > > > > > > > And we could also try to come up with different ideas on where > > > > > to sample timestamps. Unfortunately we only have the single > > > > > pipe flip timestamp register so we can only sample one > > > > > timestamp > > > > > from the DSB itself per frame. If we had more we could much > > > > > more > > > > > easily figure things out :/ > > > > > > > > > > I pushed my latest DSB selftest stuff to > > > > > https://github.com/vsyrjala/linux.git dsb_selftests_7 > > > > > which has a bunch of stuff for this kind of experimentation. > > > > > It's in a somewhat sorry state at the moment since I last used > > > > > to hunt for various DSB bugs, but at least it still builds :) > > > > > > > > > > The way I use that is that I run igt 'testdisplay -o ...' > > > > > to make sure nothing else is actively poking the hardware > > > > > and then I trigger the DSB tests via debugfs. > > > > > > > > I poked around a bit, though only on a TGL+PSR1 system (what I > > > > had > > > > at hand), so some of this might not apply to PSR2 and/or more > > > > modern platforms. > > > > > > > > General notes: > > > > - PSR1 exit is triggered by any pipe/plane register write (even > > > > the > > > > non-arming ones) > > > > > > This is same for PSR2 as well. > > > > > > > > > > > We basically have three cases to consider here: > > > > 1. PSR1 is currently inactive > > > > Obviously everything should be with the current code, > > > > vblank evasion works, wait for vblank+interrupt scheme > > > > for flip completion works > > > > > > > > 2. PSR1 is active, but DC states are not > > > > The wakeup latency here is super quick (it's < 1 scanline, how > > > > much below? I've not yet measured), and arming registers do > > > > latch > > > > nearly immediately. > > > > Actually it's only ~1usec (based on the timestamps). I also used the > > following DSB batch to test how many registers we can write there: > > > > for (i = 0; ...; i++) > > dsb_write(PLANE_SURF, i << 12) > > dsb_interrupt() > > > > and then in the interrupt handler I read PLANE_SURFLIVE, and it > > always > > shows 0xa000, meaning we only have time to write ten registers. So > > definitely not enough to guarantee that all arming registers get > > written. > > > > So, for PSR1 at least I think we'd have two options: > > 1) do a manual PSR wake around the whole commit, which doesn't > > sound very nice > > 2) evade hw_scaline==0 so that we wait until we've woken up > > from the DC state, and then proceed with the normal vblank > > evasion and arming register writes. And obviously all that > > only works in DSB_SKIP_WAITS_EN is disabled. Also if all > > the pipes are doing the full update using the DSB then we > > could perhaps also remove the explicit DC_OFF dance around > > the whole commit. > > > > > The scanline counter starts counting > > > > accordingly > > > > from vblank_start-1. However the hardware still considers PSR > > > > to be > > > > active for that short duration so DSB_SKIP_WAITS_EN _will_ skip > > > > the > > > > waits. > > > > > > > > Unfortunately being this quick I'm not convinced we have enough > > > > time > > > > to write all the registers atomically before the hardware > > > > latches > > > > something. So I'm thinking we may need to remove > > > > DSB_SKIP_WAITS_EN, > > > > in which case the vblank evasion will push the arming register > > > > writes into the next frame. This will mean the wakeup will take > > > > one full frame. > > > > > > To my understanding DSB_SKIP_WAITS_EN have impact only when in SRD > > > (PSR)/DEEP_SLEEP(PSR2). I.e. In this scenario we still do have all > > > waits as in commit without PSR. > > > > The PSR state machine is already in link off mode in this case, > > meaning the pipe has been halted, and the display has been > > signalled to scan out from its RFB (confirmed by checking the > > PSR status register in DPCD), and DSB_SKIP_WAITS_EN is already > > active. But the link is still actually up (I'm guessing it might > > be transmitting the idle pattern, but I've not confirmed that. > > Can't remeber if we even have any kind of status register that > > could show this...). So looks like the link only gets actually > > turned off by the DMC when entering the DC state. > > > > > > > > > > > > > 3. PSR1 is active and so are DC states > > > > The wakeup latency is ~5ms. During that time scanline counter > > > > reads > > > > 0, PSR is active for the purposes of DSB_SKIP_WAITS_EN. Again > > > > we > > > > pretty much need DSB_SKIP_WAITS_EN=0 here to make sure the > > > > interrupt > > > > gets signalled after the wait for vblank. vblank evasion will > > > > get > > > > skipped on account the scanline being 0. Somewhat ironically > > > > this > > > > would give us ~5ms total wakeup latency which is now faster > > > > than > > > > the > > > > previous case. > > > > > > Again my understanding is that DSB_SKIP_WAITS_EN=0/1 have impact > > > only > > > when in SRD/DEEP_SLEEP. There is SRD_CTRL/PSR2_CTRL[Idle Frames] to > > > control when entering SRD/DEEP_SLEEP. > > > > The PSR state machine is what matters. That stops the pipe, and > > causes > > DSP_SKIP_WAITS_EN to become active. And the PSR state machine is also > > what is affected by the idle frames stuff/etc. AFAIK the only stuff > > that is actually done by the DMC is turning the link/PLL/etc on and > > off. And obviously link training I guess gets triggered from there > > somehow when waking up from PSR, hence why the wakeup takes that 5ms > > longer than when DC States are disabled. > > Ok, I need do some investigation on this. The failure I was referring > was this one: > > https://intel-gfx-ci.01.org/tree/intel-xe/xe-pw-142521v2/shard-lnl-8/igt@kms_psr2_sf@psr2-cursor-plane-move-continuous-exceed-fully-sf.html > > I.e. there is that scanline wait at the begin in DSB buffer. It was > hanging in waiting the scanline. I verified this by checking if > preceding write to 0x700300 completes and it never did. > > This was with PSR2 but not with Panel Replay. Also if I disable > DEEP_SLEEP by setting PSR2_CTL[idle frames] as 0 it didn't hang. I guess that idle frames==0 thing only applies to PSR2. With PSR1 it doesn't seem to do anything (well, apart from what it says on the tin). OTOH with PSR1 there is a dedicated link off vs. standby bit in SRD_CTL, so I guess there is no need for anything else.
On Tue, 2025-01-21 at 17:11 +0200, Ville Syrjälä wrote: > On Tue, Jan 21, 2025 at 10:29:25AM +0000, Hogander, Jouni wrote: > > On Mon, 2025-01-20 at 16:39 +0200, Ville Syrjälä wrote: > > > On Mon, Jan 20, 2025 at 07:28:52AM +0000, Hogander, Jouni wrote: > > > > On Sat, 2025-01-18 at 01:07 +0200, Ville Syrjälä wrote: > > > > > On Fri, Jan 17, 2025 at 10:20:17PM +0200, Ville Syrjälä > > > > > wrote: > > > > > > On Thu, Jan 09, 2025 at 09:31:37AM +0200, Jouni Högander > > > > > > wrote: > > > > > > > Now as we have correct PSR2_MAN_TRK_CTL handling in place > > > > > > > we > > > > > > > can > > > > > > > allow DSB > > > > > > > usage also when PSR is enabled for LunarLake onwards. > > > > > > > > > > > > We seem to still lack an answer as to when the PSR wakes, > > > > > > when > > > > > > it > > > > > > latches the update, and how does all that guarantee that > > > > > > the > > > > > > DSB > > > > > > interrupt fires after the update has been latched? > > > > > > > > > > > > Some thoughts as to how to figure this out: > > > > > > 1. make sure we're in PSR > > > > > > 2. sample TIMESTAMP_CTR > > > > > > 3. start DSB in which > > > > > > write PLANE_SURF with a new value > > > > > > send push > > > > > > wait for vblank > > > > > > poll PLANE_SURFLIVE == new value > > > > > > fire interrupt > > > > > > > > > > > > in the interrupt handler: > > > > > > sample TIMESTAMP_CTR again > > > > > > > > > > > > And then compare flip timestmap vs. frame timestamp vs. the > > > > > > manually sampled timestamps. And then repeat without the > > > > > > SURFLIVE > > > > > > poll to make sure nothing has changed. You'll need to be > > > > > > careful > > > > > > to make sure it will actually poll for long enough to make > > > > > > a > > > > > > real > > > > > > difference (if the poll actuall is needed), but tweaking > > > > > > the > > > > > > poll > > > > > > interval+count suitably. I don't remeber what the max poll > > > > > > count was, but IIRC it wasn't too high so the duration will > > > > > > have > > > > > > to get bumped for long polls. > > > > > > > > > > > > I guess one could also try to poll for the actual PSR > > > > > > status, > > > > > > but dunno how well that'll work. > > > > > > > > > > > > And we could also try to come up with different ideas on > > > > > > where > > > > > > to sample timestamps. Unfortunately we only have the single > > > > > > pipe flip timestamp register so we can only sample one > > > > > > timestamp > > > > > > from the DSB itself per frame. If we had more we could much > > > > > > more > > > > > > easily figure things out :/ > > > > > > > > > > > > I pushed my latest DSB selftest stuff to > > > > > > https://github.com/vsyrjala/linux.git dsb_selftests_7 > > > > > > which has a bunch of stuff for this kind of > > > > > > experimentation. > > > > > > It's in a somewhat sorry state at the moment since I last > > > > > > used > > > > > > to hunt for various DSB bugs, but at least it still builds > > > > > > :) > > > > > > > > > > > > The way I use that is that I run igt 'testdisplay -o ...' > > > > > > to make sure nothing else is actively poking the hardware > > > > > > and then I trigger the DSB tests via debugfs. > > > > > > > > > > I poked around a bit, though only on a TGL+PSR1 system (what > > > > > I > > > > > had > > > > > at hand), so some of this might not apply to PSR2 and/or more > > > > > modern platforms. > > > > > > > > > > General notes: > > > > > - PSR1 exit is triggered by any pipe/plane register write > > > > > (even > > > > > the > > > > > non-arming ones) > > > > > > > > This is same for PSR2 as well. > > > > > > > > > > > > > > We basically have three cases to consider here: > > > > > 1. PSR1 is currently inactive > > > > > Obviously everything should be with the current code, > > > > > vblank evasion works, wait for vblank+interrupt scheme > > > > > for flip completion works > > > > > > > > > > 2. PSR1 is active, but DC states are not > > > > > The wakeup latency here is super quick (it's < 1 scanline, > > > > > how > > > > > much below? I've not yet measured), and arming registers do > > > > > latch > > > > > nearly immediately. > > > > > > Actually it's only ~1usec (based on the timestamps). I also used > > > the > > > following DSB batch to test how many registers we can write > > > there: > > > > > > for (i = 0; ...; i++) > > > dsb_write(PLANE_SURF, i << 12) > > > dsb_interrupt() > > > > > > and then in the interrupt handler I read PLANE_SURFLIVE, and it > > > always > > > shows 0xa000, meaning we only have time to write ten registers. > > > So > > > definitely not enough to guarantee that all arming registers get > > > written. > > > > > > So, for PSR1 at least I think we'd have two options: > > > 1) do a manual PSR wake around the whole commit, which doesn't > > > sound very nice > > > 2) evade hw_scaline==0 so that we wait until we've woken up > > > from the DC state, and then proceed with the normal vblank > > > evasion and arming register writes. And obviously all that > > > only works in DSB_SKIP_WAITS_EN is disabled. Also if all > > > the pipes are doing the full update using the DSB then we > > > could perhaps also remove the explicit DC_OFF dance around > > > the whole commit. > > > > > > > The scanline counter starts counting > > > > > accordingly > > > > > from vblank_start-1. However the hardware still considers > > > > > PSR > > > > > to be > > > > > active for that short duration so DSB_SKIP_WAITS_EN _will_ > > > > > skip > > > > > the > > > > > waits. > > > > > > > > > > Unfortunately being this quick I'm not convinced we have > > > > > enough > > > > > time > > > > > to write all the registers atomically before the hardware > > > > > latches > > > > > something. So I'm thinking we may need to remove > > > > > DSB_SKIP_WAITS_EN, > > > > > in which case the vblank evasion will push the arming > > > > > register > > > > > writes into the next frame. This will mean the wakeup will > > > > > take > > > > > one full frame. > > > > > > > > To my understanding DSB_SKIP_WAITS_EN have impact only when in > > > > SRD > > > > (PSR)/DEEP_SLEEP(PSR2). I.e. In this scenario we still do have > > > > all > > > > waits as in commit without PSR. > > > > > > The PSR state machine is already in link off mode in this case, > > > meaning the pipe has been halted, and the display has been > > > signalled to scan out from its RFB (confirmed by checking the > > > PSR status register in DPCD), and DSB_SKIP_WAITS_EN is already > > > active. But the link is still actually up (I'm guessing it might > > > be transmitting the idle pattern, but I've not confirmed that. > > > Can't remeber if we even have any kind of status register that > > > could show this...). So looks like the link only gets actually > > > turned off by the DMC when entering the DC state. > > > > > > > > > > > > > > > > > 3. PSR1 is active and so are DC states > > > > > The wakeup latency is ~5ms. During that time scanline > > > > > counter > > > > > reads > > > > > 0, PSR is active for the purposes of DSB_SKIP_WAITS_EN. > > > > > Again > > > > > we > > > > > pretty much need DSB_SKIP_WAITS_EN=0 here to make sure the > > > > > interrupt > > > > > gets signalled after the wait for vblank. vblank evasion > > > > > will > > > > > get > > > > > skipped on account the scanline being 0. Somewhat > > > > > ironically > > > > > this > > > > > would give us ~5ms total wakeup latency which is now faster > > > > > than > > > > > the > > > > > previous case. > > > > > > > > Again my understanding is that DSB_SKIP_WAITS_EN=0/1 have > > > > impact > > > > only > > > > when in SRD/DEEP_SLEEP. There is SRD_CTRL/PSR2_CTRL[Idle > > > > Frames] to > > > > control when entering SRD/DEEP_SLEEP. > > > > > > The PSR state machine is what matters. That stops the pipe, and > > > causes > > > DSP_SKIP_WAITS_EN to become active. And the PSR state machine is > > > also > > > what is affected by the idle frames stuff/etc. AFAIK the only > > > stuff > > > that is actually done by the DMC is turning the link/PLL/etc on > > > and > > > off. And obviously link training I guess gets triggered from > > > there > > > somehow when waking up from PSR, hence why the wakeup takes that > > > 5ms > > > longer than when DC States are disabled. > > > > Ok, I need do some investigation on this. The failure I was > > referring > > was this one: > > > > https://intel-gfx-ci.01.org/tree/intel-xe/xe-pw-142521v2/shard-lnl-8/igt@kms_psr2_sf@psr2-cursor-plane-move-continuous-exceed-fully-sf.html > > > > I.e. there is that scanline wait at the begin in DSB buffer. It was > > hanging in waiting the scanline. I verified this by checking if > > preceding write to 0x700300 completes and it never did. > > > > This was with PSR2 but not with Panel Replay. Also if I disable > > DEEP_SLEEP by setting PSR2_CTL[idle frames] as 0 it didn't hang. > > I guess that idle frames==0 thing only applies to PSR2. With > PSR1 it doesn't seem to do anything (well, apart from what it > says on the tin). OTOH with PSR1 there is a dedicated link off > vs. standby bit in SRD_CTL, so I guess there is no need for > anything else. > Setting PSR2_CTL[idle frames] as 0 doesn't trigger wake-up. It just disables DEEP_SLEEP. So we can't use that to solve this. I'll guess adding extra CURSURFLIVE write is the way to go with this. BR, Jouni Högander
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index e448ff64660a..58575800fad2 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -7631,6 +7631,7 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, intel_atomic_get_old_crtc_state(state, crtc); struct intel_crtc_state *new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc); + struct intel_display *display = to_intel_display(crtc); if (!new_crtc_state->hw.active) return; @@ -7643,7 +7644,7 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, new_crtc_state->update_planes && !new_crtc_state->vrr.enable && !new_crtc_state->do_async_flip && - !new_crtc_state->has_psr && + (DISPLAY_VER(display) >= 20 || !new_crtc_state->has_psr) && !new_crtc_state->scaler_state.scaler_users && !old_crtc_state->scaler_state.scaler_users && !intel_crtc_needs_modeset(new_crtc_state) &&
Now as we have correct PSR2_MAN_TRK_CTL handling in place we can allow DSB usage also when PSR is enabled for LunarLake onwards. Signed-off-by: Jouni Högander <jouni.hogander@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)