Re: [Intel-gfx] [PATCH 1/4] drm/i915/psr: Don't send a NULL VSC SDP

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

On Thu, Nov 23, 2023 at 07:14:29AM +0000, Hogander, Jouni wrote:
> On Wed, 2023-11-22 at 11:31 +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The PSR code is unconditionally enabling the VSC SDP whether or not
> > PSR
> > itself is enabled. This means if the DP code decided not to use a VSC
> > SDP we're always transmitting a zeroed SDP. Not sure what the
> > hardware
> > will even do in that case. We also see a "Failed to unpack DP VSC
> > SDP"
> > message on every readout since the DIP buffer is just full of zeroes.
> 
> This is already taken care by this patch :
> 
> https://patchwork.freedesktop.org/patch/568234/?series=126651&rev=1

Yeah, I suppose that takes care of it.

On a slight tangent, we should see about nuking crtc_state->psr_vsc
and just switch to using the normal crtc_state->infoframes.vsc,
including full readout/state check/etc. I suppose the only open question
is whether the hardware mutates the VSC DIP buffer when it does its
PSR magic, and if so we'd need to sanitize the results from the
readout to not include those mutable parts (or ignore those parts
in the state check).

> 
> I'm about to merge it.
> 
> BR,
> 
> Jouni Högander
> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 8d180132a74b..931295934659 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1373,6 +1373,9 @@ void intel_psr_compute_config(struct intel_dp
> > *intel_dp,
> >         else
> >                 crtc_state->has_psr = _psr_compute_config(intel_dp,
> > crtc_state);
> >  
> > +       if (!crtc_state->has_psr)
> > +               return;
> > +
> >         crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp,
> > crtc_state);
> >  
> >         crtc_state->infoframes.enable |=
> > intel_hdmi_infoframe_enable(DP_SDP_VSC);
> 

-- 
Ville Syrjälä
Intel