> -----Original Message----- > From: Manna, Animesh > Sent: Monday, April 29, 2024 5:04 PM > To: Hogander, Jouni <jouni.hogan...@intel.com>; intel- > g...@lists.freedesktop.org > Subject: RE: [PATCH v7 06/11] drm/i915/psr: Modify > intel_dp_get_su_granularity to support panel replay > > > > > -----Original Message----- > > From: Hogander, Jouni <jouni.hogan...@intel.com> > > Sent: Monday, April 29, 2024 4:46 PM > > To: Manna, Animesh <animesh.ma...@intel.com>; intel- > > g...@lists.freedesktop.org > > Subject: Re: [PATCH v7 06/11] drm/i915/psr: Modify > > intel_dp_get_su_granularity to support panel replay > > > > On Mon, 2024-04-29 at 11:02 +0000, Manna, Animesh wrote: > > > > > > > > > > -----Original Message----- > > > > From: Hogander, Jouni <jouni.hogan...@intel.com> > > > > Sent: Friday, April 19, 2024 5:42 PM > > > > To: intel-gfx@lists.freedesktop.org > > > > Cc: Manna, Animesh <animesh.ma...@intel.com>; Hogander, Jouni > > > > <jouni.hogan...@intel.com> > > > > Subject: [PATCH v7 06/11] drm/i915/psr: Modify > > > > intel_dp_get_su_granularity to support panel replay > > > > > > > > Currently intel_dp_get_su_granularity doesn't support panel replay. > > > > This fix modifies it to support panel replay as well. > > > > > > > > v2: rely on PSR definitions on common bits > > > > > > > > Signed-off-by: Jouni Högander <jouni.hogan...@intel.com> > > > > Reviewed-by: Animesh Manna <animesh.ma...@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_psr.c | 62 > > > > +++++++++++++++++++++--- > > > > 1 file changed, 55 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > > index b94f8e33ed1f..29400fac13c2 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > @@ -466,6 +466,40 @@ static u8 > > > > intel_dp_get_sink_sync_latency(struct > > > > intel_dp *intel_dp) > > > > return val; > > > > } > > > > > > > > +static u8 intel_dp_get_su_capability(struct intel_dp *intel_dp) { > > > > + u8 su_capability; > > > > + > > > > + if (intel_dp->psr.sink_panel_replay_su_support) > > > > > > While relooking found that good to add a check for > > > DP_PANEL_PANEL_REPLAY_SU_GRANULARITY_REQUIRED (6th bit of dpcd > > 0xb1). > > > What if it is zero means granularity not needed but will continue to > > > use x-granularity and y-granularity. > > > > Please note it is 5th bit in 0xb1. See my further comment below... > > Same I was referring, 6th bit index 5 as it starts from 0. > > > > > > Regards, > > > Animesh > > > > > > > + drm_dp_dpcd_read(&intel_dp->aux, > > > > + > > > > DP_PANEL_PANEL_REPLAY_X_GRANULARITY, > > > > + &su_capability, 1);
I think instead of DP_PANEL_PANEL_REPLAY_X_GRANULARITY it should be DP_PANEL_PANEL_REPLAY_CAPABILITY, everything else will be fine. Regards, Animesh > > > > + else > > > > + su_capability = intel_dp->psr_dpcd[1]; > > > > + > > > > + return su_capability; > > > > +} > > > > + > > > > +static unsigned int > > > > +intel_dp_get_su_x_granularity_offset(struct intel_dp *intel_dp) { > > > > + return intel_dp->psr.sink_panel_replay_su_support ? > > > > + DP_PANEL_PANEL_REPLAY_X_GRANULARITY : > > > > + DP_PSR2_SU_X_GRANULARITY; } > > > > + > > > > +static unsigned int > > > > +intel_dp_get_su_y_granularity_offset(struct intel_dp *intel_dp) { > > > > + return intel_dp->psr.sink_panel_replay_su_support ? > > > > + DP_PANEL_PANEL_REPLAY_Y_GRANULARITY : > > > > + DP_PSR2_SU_Y_GRANULARITY; } > > > > + > > > > +/* > > > > + * Note: Bits related to granularity are same in panel replay and > > > > psr > > > > + * registers. Rely on PSR definitions on these "common" bits. > > > > + */ > > > > Check this comment. See my further comment below... > > Agree bit position is same. > > > > > > > static void intel_dp_get_su_granularity(struct intel_dp > > > > *intel_dp) { > > > > struct drm_i915_private *i915 = dp_to_i915(intel_dp); @@ - > > > > 473,18 > > > > +507,29 @@ static void intel_dp_get_su_granularity(struct intel_dp > > > > *intel_dp) > > > > u16 w; > > > > u8 y; > > > > > > > > - /* If sink don't have specific granularity requirements > > > > set legacy ones */ > > > > - if (!(intel_dp->psr_dpcd[1] & > > > > DP_PSR2_SU_GRANULARITY_REQUIRED)) { > > > > + /* > > > > + * TODO: Do we need to take into account panel supporting > > > > both PSR > > > > and > > > > + * Panel replay? > > > > + */ > > > > + > > > > + /* > > > > + * If sink don't have specific granularity requirements > > > > +set > > > > legacy > > > > + * ones. > > > > + */ > > > > + if (!(intel_dp_get_su_capability(intel_dp) & > > > > + DP_PSR2_SU_GRANULARITY_REQUIRED)) { > > > > /* As PSR2 HW sends full lines, we do not care > > > > about x granularity */ > > > > w = 4; > > > > y = 4; > > > > goto exit; > > > > } > > > > This block is taking care of checking bit 5 in 0xb1 and using legacy > > ones if no requirements. > > intel_dp_get_su_capability is reading from > DP_PANEL_PANEL_REPLAY_X_GRANULARITY which Is dpcd 0xb2. Am I > missing something? > > Regards, > Animesh > > > > > BR, > > > > Jouni Högander > > > > > > > > > > - r = drm_dp_dpcd_read(&intel_dp->aux, > > > > DP_PSR2_SU_X_GRANULARITY, &w, 2); > > > > + r = drm_dp_dpcd_read(&intel_dp->aux, > > > > + > > > > intel_dp_get_su_x_granularity_offset(intel_dp), > > > > + &w, 2); > > > > if (r != 2) > > > > drm_dbg_kms(&i915->drm, > > > > - "Unable to read > > > > DP_PSR2_SU_X_GRANULARITY\n"); > > > > + "Unable to read selective update x > > > > granularity\n"); > > > > /* > > > > * Spec says that if the value read is 0 the default > > > > granularity should > > > > * be used instead. > > > > @@ -492,10 +537,12 @@ static void > > > > intel_dp_get_su_granularity(struct > > > > intel_dp *intel_dp) > > > > if (r != 2 || w == 0) > > > > w = 4; > > > > > > > > - r = drm_dp_dpcd_read(&intel_dp->aux, > > > > DP_PSR2_SU_Y_GRANULARITY, &y, 1); > > > > + r = drm_dp_dpcd_read(&intel_dp->aux, > > > > + > > > > intel_dp_get_su_y_granularity_offset(intel_dp), > > > > + &y, 1); > > > > if (r != 1) { > > > > drm_dbg_kms(&i915->drm, > > > > - "Unable to read > > > > DP_PSR2_SU_Y_GRANULARITY\n"); > > > > + "Unable to read selective update y > > > > granularity\n"); > > > > y = 4; > > > > } > > > > if (y == 0) > > > > @@ -588,7 +635,8 @@ void intel_psr_init_dpcd(struct intel_dp > > > > *intel_dp) > > > > if (intel_dp->psr_dpcd[0]) > > > > _psr_init_dpcd(intel_dp); > > > > > > > > - if (intel_dp->psr.sink_psr2_support) > > > > + if (intel_dp->psr.sink_psr2_support || > > > > + intel_dp->psr.sink_panel_replay_su_support) > > > > intel_dp_get_su_granularity(intel_dp); > > > > } > > > > > > > > -- > > > > 2.34.1 > > >