| Message ID | 20240605102553.187309-11-jouni.hogander@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Panel Replay eDP support | expand |
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Wednesday, June 5, 2024 3:56 PM > To: intel-gfx@lists.freedesktop.org > Cc: Manna, Animesh <animesh.manna@intel.com>; Kahola, Mika > <mika.kahola@intel.com>; Hogander, Jouni <jouni.hogander@intel.com> > 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@intel.com> > --- > 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. Regards, Animesh > > seq_printf(m, "Sink %s status: 0x%x [%s]\n", psr_mode_str(intel_dp), > status, str); > > -- > 2.34.1
On Thu, 2024-06-06 at 14:35 +0000, Manna, Animesh wrote: > > > > -----Original Message----- > > From: Hogander, Jouni <jouni.hogander@intel.com> > > Sent: Wednesday, June 5, 2024 3:56 PM > > To: intel-gfx@lists.freedesktop.org > > Cc: Manna, Animesh <animesh.manna@intel.com>; Kahola, Mika > > <mika.kahola@intel.com>; Hogander, Jouni <jouni.hogander@intel.com> > > 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@intel.com> > > --- > > 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? 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 >
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Thursday, June 6, 2024 9:08 PM > To: Manna, Animesh <animesh.manna@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Kahola, Mika <mika.kahola@intel.com> > 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@intel.com> > > > Sent: Wednesday, June 5, 2024 3:56 PM > > > To: intel-gfx@lists.freedesktop.org > > > Cc: Manna, Animesh <animesh.manna@intel.com>; Kahola, Mika > > > <mika.kahola@intel.com>; Hogander, Jouni <jouni.hogander@intel.com> > > > 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@intel.com> > > > --- > > > 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? 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 > >
On Fri, 2024-06-07 at 09:59 +0000, Manna, Animesh wrote: > > > > -----Original Message----- > > From: Hogander, Jouni <jouni.hogander@intel.com> > > Sent: Thursday, June 6, 2024 9:08 PM > > To: Manna, Animesh <animesh.manna@intel.com>; intel- > > gfx@lists.freedesktop.org > > Cc: Kahola, Mika <mika.kahola@intel.com> > > 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@intel.com> > > > > Sent: Wednesday, June 5, 2024 3:56 PM > > > > To: intel-gfx@lists.freedesktop.org > > > > Cc: Manna, Animesh <animesh.manna@intel.com>; Kahola, Mika > > > > <mika.kahola@intel.com>; Hogander, Jouni > > > > <jouni.hogander@intel.com> > > > > 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@intel.com> > > > > --- > > > > 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? 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 > > > >
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Friday, June 7, 2024 3:34 PM > To: Manna, Animesh <animesh.manna@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Kahola, Mika <mika.kahola@intel.com> > 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@intel.com> > > > Sent: Thursday, June 6, 2024 9:08 PM > > > To: Manna, Animesh <animesh.manna@intel.com>; intel- > > > gfx@lists.freedesktop.org > > > Cc: Kahola, Mika <mika.kahola@intel.com> > > > 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@intel.com> > > > > > Sent: Wednesday, June 5, 2024 3:56 PM > > > > > To: intel-gfx@lists.freedesktop.org > > > > > Cc: Manna, Animesh <animesh.manna@intel.com>; Kahola, Mika > > > > > <mika.kahola@intel.com>; Hogander, Jouni > > > > > <jouni.hogander@intel.com> > > > > > 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@intel.com> > > > > > --- > > > > > 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 > > > > > >
On Fri, 2024-06-07 at 10:09 +0000, Manna, Animesh wrote: > > > > -----Original Message----- > > From: Hogander, Jouni <jouni.hogander@intel.com> > > Sent: Friday, June 7, 2024 3:34 PM > > To: Manna, Animesh <animesh.manna@intel.com>; intel- > > gfx@lists.freedesktop.org > > Cc: Kahola, Mika <mika.kahola@intel.com> > > 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@intel.com> > > > > Sent: Thursday, June 6, 2024 9:08 PM > > > > To: Manna, Animesh <animesh.manna@intel.com>; intel- > > > > gfx@lists.freedesktop.org > > > > Cc: Kahola, Mika <mika.kahola@intel.com> > > > > 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@intel.com> > > > > > > Sent: Wednesday, June 5, 2024 3:56 PM > > > > > > To: intel-gfx@lists.freedesktop.org > > > > > > Cc: Manna, Animesh <animesh.manna@intel.com>; Kahola, Mika > > > > > > <mika.kahola@intel.com>; Hogander, Jouni > > > > > > <jouni.hogander@intel.com> > > > > > > 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@intel.com> > > > > > > --- > > > > > > 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". 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 > > > > > > > > >
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Friday, June 7, 2024 3:41 PM > To: Manna, Animesh <animesh.manna@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Kahola, Mika <mika.kahola@intel.com> > 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@intel.com> > > > Sent: Friday, June 7, 2024 3:34 PM > > > To: Manna, Animesh <animesh.manna@intel.com>; intel- > > > gfx@lists.freedesktop.org > > > Cc: Kahola, Mika <mika.kahola@intel.com> > > > 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@intel.com> > > > > > Sent: Thursday, June 6, 2024 9:08 PM > > > > > To: Manna, Animesh <animesh.manna@intel.com>; intel- > > > > > gfx@lists.freedesktop.org > > > > > Cc: Kahola, Mika <mika.kahola@intel.com> > > > > > 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@intel.com> > > > > > > > Sent: Wednesday, June 5, 2024 3:56 PM > > > > > > > To: intel-gfx@lists.freedesktop.org > > > > > > > Cc: Manna, Animesh <animesh.manna@intel.com>; Kahola, Mika > > > > > > > <mika.kahola@intel.com>; Hogander, Jouni > > > > > > > <jouni.hogander@intel.com> > > > > > > > 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@intel.com> > > > > > > > --- > > > > > > > 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@intel.com> 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 > > > > > > > > > > > >
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"; seq_printf(m, "Sink %s status: 0x%x [%s]\n", psr_mode_str(intel_dp), status, str);
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@intel.com> --- drivers/gpu/drm/i915/display/intel_psr.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-)