Addressed comments from Jose and Paul in version 3. On Thu, 2022-02-24 at 15:02 -0500, Lyude Paul wrote: > Also - I realized this is missing an appropriate Fixes: tag for the > commit > that enabled PSR2 selective fetch on tigerlake in the first place > > On Wed, 2022-02-23 at 17:32 +0000, Souza, Jose wrote: > > On Wed, 2022-02-23 at 14:48 +0200, Jouni Högander wrote: > > > Currently we are observing occasional screen flickering when > > > PSR2 selective fetch is enabled. More specifically glitch seems > > > to happen on full frame update when cursor moves to coords > > > x = -1 or y = -1. > > > > > > According to Bspec SF Single full frame should not be set if > > > SF Partial Frame Enable is not set. This happened to be true for > > > ADLP as PSR2_MAN_TRK_CTL_ENABLE is always set and for ADLP it's > > > actually "SF Partial Frame Enable" (Bit 31). > > > > > > Setting "SF Partial Frame Enable" bit also on full update seems > > > to > > > fix screen flickering. > > > > > > Also make code more clear by setting PSR2_MAN_TRK_CTL_ENABLE > > > only if not on ADLP as this bit doesn't exist in ADLP. > > > > Bit exist but has another name. > > > > > Bspec: 49274 > > > > > > v2: Fix Mihai Harpau email address > > > > > > Reported-by: Lyude Paul <lyude@xxxxxxxxxx> > > > Cc: Mihai Harpau <mharpau@xxxxxxxxx> > > > Cc: José Roberto de Souza <jose.souza@xxxxxxxxx> > > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/5077 > > > Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/display/intel_psr.c | 20 > > > ++++++++++++++++++-- > > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > > 2 files changed, 19 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > index 2e0b092f4b6b..90aca75e05e0 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > @@ -1439,6 +1439,13 @@ static inline u32 > > > man_trk_ctl_single_full_frame_bit_get(struct drm_i915_private > > > PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME; > > > } > > > > > > +static inline u32 man_trk_ctl_partial_frame_bit_get(struct > > > drm_i915_private *dev_priv) > > > +{ > > > + return IS_ALDERLAKE_P(dev_priv) ? > > > + ADLP_PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE : > > > + PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE; > > > +} > > > + > > > static void psr_force_hw_tracking_exit(struct intel_dp > > > *intel_dp) > > > { > > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > @@ -1543,7 +1550,17 @@ static void psr2_man_trk_ctl_calc(struct > > > intel_crtc_state *crtc_state, > > > { > > > struct intel_crtc *crtc = to_intel_crtc(crtc_state- > > > >uapi.crtc); > > > struct drm_i915_private *dev_priv = to_i915(crtc- > > > >base.dev); > > > - u32 val = PSR2_MAN_TRK_CTL_ENABLE; > > > + u32 val = 0; > > > + > > > + /* > > > + * ADL_P doesn't have HW tracking nor manual tracking > > > enable > > > + * bit > > > + */ > > > > Nit: Unnecessary comment. > > > > Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > > > > > + if (!IS_ALDERLAKE_P(dev_priv)) > > > + val = PSR2_MAN_TRK_CTL_ENABLE; > > > + > > > + /* SF partial frame enable has to be set even on full > > > update */ > > > + val |= man_trk_ctl_partial_frame_bit_get(dev_priv); > > > > > > if (full_update) { > > > /* > > > @@ -1563,7 +1580,6 @@ static void psr2_man_trk_ctl_calc(struct > > > intel_crtc_state *crtc_state, > > > } else { > > > drm_WARN_ON(crtc_state->uapi.crtc->dev, clip->y1 > > > % 4 || > > > clip->y2 % 4); > > > > > > - val |= PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE; > > > val |= > > > PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(clip->y1 / 4 > > > + 1); > > > val |= PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(clip- > > > >y2 / 4 + > > > 1); > > > } > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index 2b8a3086ed35..89bbb64e520d 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -2316,6 +2316,7 @@ > > > #define > > > ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR(val) REG_FIELD_P > > > REP(ADLP_ > > > PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK, val) > > > #define > > > ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK REG_GENMASK > > > (12, 0) > > > #define > > > ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR(val) REG_FIELD_P > > > REP(ADLP_ > > > PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK, val) > > > +#define > > > ADLP_PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE REG_BIT(31) > > > #define > > > ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME REG_BIT(14) > > > #define > > > ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME REG_BIT(13) > > > BR, Jouni Högander
