| Message ID | 20231122093137.1509-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/i915/psr: Don't send a NULL VSC SDP | expand |
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 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);
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); >
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);