Re: [Intel-gfx] [PATCH v3 3/4] drm/i915/psr: Check that vblank is long enough for psr2

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

On Tue, Mar 21, 2023 at 04:14:57PM +0000, Hogander, Jouni wrote:
> On Tue, 2023-03-21 at 17:43 +0200, Ville Syrjälä wrote:
> > On Mon, Mar 20, 2023 at 06:59:44PM +0200, Jouni Högander wrote:
> > > Ensure vblank >= psr2 vblank
> > > where
> > > Psr2 vblank = PSR2_CTL Block Count Number maximum line count.
> > > 
> > > Bspec: 71580, 49274
> > > 
> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 1050d777a108..1b40d9c73c18 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -958,6 +958,14 @@ static bool intel_psr2_config_valid(struct
> > > intel_dp *intel_dp,
> > >                 return false;
> > >         }
> > >  
> > > +       /* Vblank >= PSR2_CTL Block Count Number maximum line count
> > > */
> > > +       if (crtc_state->hw.adjusted_mode.crtc_vblank_end -
> > > +           crtc_state->hw.adjusted_mode.crtc_vblank_start < 12) {
> > 
> > Why 12? Shouldn't it be based on the wake_lines/BLOCK_COUNT_NUM
> > stuff?
> 
> I took this directly from Bspec. I think your suggestions make sense. I
> will experiment them and come back on this.

BTW the other thing that might be a bit unclear here is whether
we care about the transcoder's full vblank length, or the pipe's
vblank length (as reduced by the delayed vblank stuff).

If you're experimenting with this then changing the vblank
delay can be done live with intel_reg (on tgl just alter
TRANS_VBLANK.vblanl_start, on adl+ alter TRANS_SET_CONTEXT_LATENCY).

When doing stuff like this I always just run eg. 'testdisplay -o <id>,0'
to quiesce the driver as much as possible, and then
'export IGT_NO_FORCEWAKE=1' before poking the registers with
intel_reg avoid the debugfs forcewake stuff from perturbing the
system either.

Using that approach it should be possible to determine which vblank
length actually matters. Though you do need to be careful about
the pkg-c latency/prefill stuff when increasing the vblank delay.
So might also need to disable wm1+ (and maybe also sagv) leaving
only wm0 enabled. That would allow you to push the pipe's delayed
start of vblank very close to the end of vblank without getting
underruns.

-- 
Ville Syrjälä
Intel