> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@xxxxxxxxx> > Sent: Friday, June 7, 2024 3:41 PM > To: Manna, Animesh <animesh.manna@xxxxxxxxx>; intel- > gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Kahola, Mika <mika.kahola@xxxxxxxxx> > Subject: Re: [PATCH v6 10/26] drm/i915/psr: Print Panel Replay status instead > of frame lock status > > On Fri, 2024-06-07 at 10:09 +0000, Manna, Animesh wrote: > > > > > > > -----Original Message----- > > > From: Hogander, Jouni <jouni.hogander@xxxxxxxxx> > > > Sent: Friday, June 7, 2024 3:34 PM > > > To: Manna, Animesh <animesh.manna@xxxxxxxxx>; intel- > > > gfx@xxxxxxxxxxxxxxxxxxxxx > > > Cc: Kahola, Mika <mika.kahola@xxxxxxxxx> > > > Subject: Re: [PATCH v6 10/26] drm/i915/psr: Print Panel Replay > > > status instead of frame lock status > > > > > > On Fri, 2024-06-07 at 09:59 +0000, Manna, Animesh wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Hogander, Jouni <jouni.hogander@xxxxxxxxx> > > > > > Sent: Thursday, June 6, 2024 9:08 PM > > > > > To: Manna, Animesh <animesh.manna@xxxxxxxxx>; intel- > > > > > gfx@xxxxxxxxxxxxxxxxxxxxx > > > > > Cc: Kahola, Mika <mika.kahola@xxxxxxxxx> > > > > > Subject: Re: [PATCH v6 10/26] drm/i915/psr: Print Panel Replay > > > > > status instead of frame lock status > > > > > > > > > > On Thu, 2024-06-06 at 14:35 +0000, Manna, Animesh wrote: > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Hogander, Jouni <jouni.hogander@xxxxxxxxx> > > > > > > > Sent: Wednesday, June 5, 2024 3:56 PM > > > > > > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > > > > > Cc: Manna, Animesh <animesh.manna@xxxxxxxxx>; Kahola, Mika > > > > > > > <mika.kahola@xxxxxxxxx>; Hogander, Jouni > > > > > > > <jouni.hogander@xxxxxxxxx> > > > > > > > Subject: [PATCH v6 10/26] drm/i915/psr: Print Panel Replay > > > > > > > status instead of frame lock status > > > > > > > > > > > > > > Currently Panel Replay status printout is printing frame > > > > > > > lock status. It should print Panel Replay status instead. > > > > > > > Panel Replay status register field follows PSR status > > > > > > > register field. > > > > > > > Use existing PSR code for that. > > > > > > > > > > > > > > Fixes: ef75c25e8fed ("drm/i915/panelreplay: Debugfs support > > > > > > > for panel > > > > > > > replay") > > > > > > > Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx> > > > > > > > --- > > > > > > > drivers/gpu/drm/i915/display/intel_psr.c | 22 +++++------- > > > > > > > ---- > > > > > > > ---- > > > > > > > -- > > > > > > > 1 file changed, 5 insertions(+), 17 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > > index 7bdae0d0ea45..3530e5f44096 100644 > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > > @@ -3579,16 +3579,9 @@ static int > > > > > > > i915_psr_sink_status_show(struct seq_file *m, void *data) > > > > > > > "reserved", > > > > > > > "sink internal error", > > > > > > > }; > > > > > > > - static const char * const panel_replay_status[] = { > > > > > > > - "Sink device frame is locked to the Source > > > > > > > device", > > > > > > > - "Sink device is coasting, using the VTotal > > > > > > > target", > > > > > > > - "Sink device is governing the frame rate > > > > > > > (frame rate unlock is granted)", > > > > > > > - "Sink device in the process of re-locking > > > > > > > with the Source device", > > > > > > > - }; > > > > > > > const char *str; > > > > > > > int ret; > > > > > > > u8 status, error_status; > > > > > > > - u32 idx; > > > > > > > > > > > > > > if (!(CAN_PSR(intel_dp) || > > > > > > > CAN_PANEL_REPLAY(intel_dp))) { > > > > > > > seq_puts(m, "PSR/Panel-Replay > > > > > > > Unsupported\n"); @@ > > > > > > > -3602,16 +3595,11 @@ static int > > > > > > > i915_psr_sink_status_show(struct seq_file *m, void *data) > > > > > > > if (ret) > > > > > > > return ret; > > > > > > > > > > > > > > - str = "unknown"; > > > > > > > - if (intel_dp->psr.panel_replay_enabled) { > > > > > > > - idx = (status & DP_SINK_FRAME_LOCKED_MASK) > > > > > > > >> > > > > > > > DP_SINK_FRAME_LOCKED_SHIFT; > > > > > > > - if (idx < ARRAY_SIZE(panel_replay_status)) > > > > > > > - str = panel_replay_status[idx]; > > > > > > > - } else if (intel_dp->psr.enabled) { > > > > > > > - idx = status & DP_PSR_SINK_STATE_MASK; > > > > > > > - if (idx < ARRAY_SIZE(sink_status)) > > > > > > > - str = sink_status[idx]; > > > > > > > - } > > > > > > > + status &= DP_PSR_SINK_STATE_MASK; > > > > > > > + if (status < ARRAY_SIZE(sink_status)) > > > > > > > + str = sink_status[status]; > > > > > > > + else > > > > > > > + str = "unknown"; > > > > > > > > > > > > psr_get_status_and_error_status() is returning frame-locked- > > > > > > status for panel replay, Its different dpcd > > > > > > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, not same like > psr. > > > > > > > > > > Panel Replay STATUS ~= PSR STATUS if you look at description of > > > > > the registers. Frame lock status is completely different thing. > > > > > I don't understand why psr sink status debugfs interface should > > > > > print frame lock status for Panel Replay? > > > > > > > > If we do not want to print > > > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS > > > > the psr_get_status_and_error_status() need to be modified. Do you > > > > agree? > > > > > > Yes, and this what I'm doing in this patch? Or can you elaborate a > > > bit what do you mean? > > > > I do not see any change in psr_get_status_and_error_status() in this > > patch. > > Just adding below the code-snippet where based on panel_replay_enabled > > flag offset is assigned to DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS. > > > > static int psr_get_status_and_error_status(struct intel_dp *intel_dp, > > u8 *status, u8 > > *error_status) > > { > > struct drm_dp_aux *aux = &intel_dp->aux; > > int ret; > > unsigned int offset; > > > > offset = intel_dp->psr.panel_replay_enabled ? > > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS : > > DP_PSR_STATUS; > > > > ret = drm_dp_dpcd_readb(aux, offset, status); > > if (ret != 1) > > return ret; > > ... > > ... > > ... > > > > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS contains two fields. "Sink > Device Panel Replay Status" and "SINK FRAME LOCKED". Currently we are > printing latter. "SINK FRAME LOCKED" is not actually that much > related to psr status debugfs interface. This patch is changing the interface to > print out "Sink Device Panel Replay Status". Thanks for clarifying, the name of DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS confused me. No objection from myside. Reviewed-by: Animesh Manna <animesh.manna@xxxxxxxxx> Regards, Animesh > > BR, > > Jouni Högander > > > Regards, > > Animesh > > > > > > > > BR, > > > > > > Jouni Högander > > > > > > > > Regards, > > > > Animesh > > > > > > > > > > BR, > > > > > > > > > > Jouni Högander > > > > > > > > > > > > > > > > > Regards, > > > > > > Animesh > > > > > > > > > > > > > > > > > > > > seq_printf(m, "Sink %s status: 0x%x [%s]\n", > > > > > > > psr_mode_str(intel_dp), status, str); > > > > > > > > > > > > > > -- > > > > > > > 2.34.1 > > > > > > > > > > > >