> -----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;
...
...
...
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
> > > >
> >
