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.
> 
> > 
> > 
> > If so I would suggest we try someting like this:
> > 
> > psr2_block_count_lines()
> > {
> >         return ...wake_lines... ? 12 : 8;

I guess we could even make that 'roundup(max(wake_lines), 4)'
to be more future proof.

Hmm, except that might not be all that future proof if the
hardware didn't support all block size between the min/max.
Eg. if it only supported 2,3,5 blocks.

So I guess we might want this thing to return only actually
supported numbers.

> > }
> > 
> > psr2_block_count()
> > {
> >         return psr2_block_count_lines() / 4;
> > }
> > 
> > if (vblank_lengh < psr2_block_count_lines())
> >         fail;
> > 
> > if (psr_block_count() > 2)
> >         val |= BLOCK_COUNT_NUM_3;
> > else
> >         val |= BLOCK_COUNT_NUM_2;
> > 
> > > +               drm_dbg_kms(&dev_priv->drm,
> > > +                           "PSR2 not enabled, too short vblank
> > > time\n");
> > > +               return false;
> > > +       }
> > > +
> > >         if (HAS_PSR2_SEL_FETCH(dev_priv)) {
> > >                 if (!intel_psr2_sel_fetch_config_valid(intel_dp,
> > > crtc_state) &&
> > >                     !HAS_PSR_HW_TRACKING(dev_priv)) {
> > > -- 
> > > 2.34.1
> > 
> 

-- 
Ville Syrjälä
Intel