Re: [PATCH v3 10/10] drm/i915/psr: Allow DSB usage when PSR is enabled

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

-- 
Ville Syrjälä
Intel