Re: [Intel-gfx] [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

2023-11-03 Thread Manna, Animesh


> -Original Message-
> From: Hogander, Jouni 
> Sent: Friday, November 3, 2023 12:32 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org; Murthy, Arun R
> ; Nikula, Jani 
> Subject: Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for
> panel replay
> 
> On Fri, 2023-11-03 at 06:10 +, Manna, Animesh wrote:
> >
> >
> > > -Original Message-
> > > From: Hogander, Jouni 
> > > Sent: Thursday, November 2, 2023 1:08 PM
> > > To: Manna, Animesh ; intel-
> > > g...@lists.freedesktop.org
> > > Cc: dri-de...@lists.freedesktop.org; Murthy, Arun R
> > > ; Nikula, Jani 
> > > Subject: Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support
> > > for panel replay
> > >
> > > On Wed, 2023-10-11 at 16:39 +0530, Animesh Manna wrote:
> > > > Add debugfs support which will print source and sink status per
> > > > connector basis.
> > >
> > > Sorry for late review. Noticed only by now that you have added this
> > > patch into you set.
> >
> > Added from v5.
> >
> > >
> > > Can you please describe in commit message how you see the output of
> > > debugfs interface will look like after your changes?
> >
> > Sure.
> >
> > >
> > > >
> > > > v1: Initial version. [rb-ed by Arun]
> > > > v2: Added check for DP 2.0 and connector type in
> > > > connector_debugfs_add().
> > > >
> > > > Cc: Jouni Högander 
> > > > Cc: Arun R Murthy 
> > > > Cc: Jani Nikula 
> > > > Signed-off-by: Animesh Manna 
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_psr.c | 136
> > > > +
> > > > --
> > > >  1 file changed, 102 insertions(+), 34 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 80de831c2f60..399fc0a8e636 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -2823,6 +2823,25 @@ static int
> > > > psr_get_status_and_error_status(struct intel_dp *intel_dp,
> > > > return 0;
> > > >  }
> > > >
> > > > +static int panel_replay_get_status_and_error_status(struct
> > > > intel_dp
> > > > *intel_dp,
> > > > +   u8 *status,
> > > > u8
> > > > *error_status)
> > > > +{
> > > > +   struct drm_dp_aux *aux = _dp->aux;
> > > > +   int ret;
> > > > +
> > > > +   ret = drm_dp_dpcd_readb(aux,
> > > > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> > > > +   if (ret != 1)
> > > > +   return ret;
> > > > +
> > > > +   ret = drm_dp_dpcd_readb(aux,
> > > > DP_PANEL_REPLAY_ERROR_STATUS,
> > > > error_status);
> > > > +   if (ret != 1)
> > > > +   return ret;
> > > > +
> > > > +   *status = *status & DP_PSR_SINK_STATE_MASK;
> > > > +
> > > > +   return 0;
> > > > +}
> > > > +
> > >
> > > I think you should modify  psr_get_status_and_error_status instead
> > > of duplicating most of it.
> >
> > DPCD addresses are different for panel replay, I did not get the need
> > of it.
> 
> I would like to see:
> 
> unsigned int 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);
> 
> rather than duplicating it.

Added in v8.

> 
> >
> > >
> > > >  static void psr_alpm_check(struct intel_dp *intel_dp)
> > > >  {
> > > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > > @@
> > > > -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp,
> > > > struct seq_file *m)
> > > > status = live_status[status_val];
> > > > }
> > > >
> > > > -   seq_printf(m, "Source PSR status: %s [0x%08x]\n", status,
> > > > val);
> > > > +   seq_printf(m, "Source PSR/PanelReplay status: %s
> > > > [0x%08x]\n",
> > > > status, val);
> > > >  }
> > > >
> > > >  static int intel_psr_status(struct seq_file *m, struct intel_dp
> > > > *intel_dp)
> > > > @@ -3048,18 +3067,23 @@ static int intel_psr_status(struct
> > > > seq_file *m, struct intel_dp *intel_dp)
> > > > bool enabled;
> > > > u32 val;
> > > >
> > > > -   seq_printf(m, "Sink support: %s", str_yes_no(psr-
> > > > > sink_support));
> > > > -   if (psr->sink_support)
> > > > +   seq_printf(m, "Sink support: PSR = %s, Panel Replay =
> > > > %s",
> > > > +  str_yes_no(psr->sink_support),
> > > > +  str_yes_no(psr->sink_panel_replay_support));
> > > > +
> > > > +   if (psr->sink_support || psr->sink_panel_replay_support)
> > > > seq_printf(m, " [0x%02x]", intel_dp-
> > > > >psr_dpcd[0]);
> > > > seq_puts(m, "\n");
> > > >
> > > > -   if (!psr->sink_support)
> > > > +   if (!(psr->sink_support || psr-
> > > > >sink_panel_replay_support))
> > > > return 0;
> > > >
> > > > wakeref = intel_runtime_pm_get(_priv->runtime_pm);
> > > > mutex_lock(>lock);
> > > >
> > > > -   

Re: [Intel-gfx] [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

2023-11-03 Thread Hogander, Jouni
On Fri, 2023-11-03 at 06:10 +, Manna, Animesh wrote:
> 
> 
> > -Original Message-
> > From: Hogander, Jouni 
> > Sent: Thursday, November 2, 2023 1:08 PM
> > To: Manna, Animesh ; intel-
> > g...@lists.freedesktop.org
> > Cc: dri-de...@lists.freedesktop.org; Murthy, Arun R
> > ; Nikula, Jani 
> > Subject: Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support
> > for
> > panel replay
> > 
> > On Wed, 2023-10-11 at 16:39 +0530, Animesh Manna wrote:
> > > Add debugfs support which will print source and sink status per
> > > connector basis.
> > 
> > Sorry for late review. Noticed only by now that you have added this
> > patch
> > into you set.
> 
> Added from v5.
> 
> > 
> > Can you please describe in commit message how you see the output of
> > debugfs interface will look like after your changes?
> 
> Sure.
> 
> > 
> > > 
> > > v1: Initial version. [rb-ed by Arun]
> > > v2: Added check for DP 2.0 and connector type in
> > > connector_debugfs_add().
> > > 
> > > Cc: Jouni Högander 
> > > Cc: Arun R Murthy 
> > > Cc: Jani Nikula 
> > > Signed-off-by: Animesh Manna 
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 136
> > > +
> > > --
> > >  1 file changed, 102 insertions(+), 34 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 80de831c2f60..399fc0a8e636 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -2823,6 +2823,25 @@ static int
> > > psr_get_status_and_error_status(struct intel_dp *intel_dp,
> > > return 0;
> > >  }
> > > 
> > > +static int panel_replay_get_status_and_error_status(struct
> > > intel_dp
> > > *intel_dp,
> > > +   u8 *status,
> > > u8
> > > *error_status)
> > > +{
> > > +   struct drm_dp_aux *aux = _dp->aux;
> > > +   int ret;
> > > +
> > > +   ret = drm_dp_dpcd_readb(aux,
> > > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> > > +   if (ret != 1)
> > > +   return ret;
> > > +
> > > +   ret = drm_dp_dpcd_readb(aux,
> > > DP_PANEL_REPLAY_ERROR_STATUS,
> > > error_status);
> > > +   if (ret != 1)
> > > +   return ret;
> > > +
> > > +   *status = *status & DP_PSR_SINK_STATE_MASK;
> > > +
> > > +   return 0;
> > > +}
> > > +
> > 
> > I think you should modify  psr_get_status_and_error_status instead
> > of
> > duplicating most of it.
> 
> DPCD addresses are different for panel replay, I did not get the need
> of it. 

I would like to see:

unsigned int 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);

rather than duplicating it.

>  
> > 
> > >  static void psr_alpm_check(struct intel_dp *intel_dp)
> > >  {
> > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > @@
> > > -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp,
> > > struct
> > > seq_file *m)
> > > status = live_status[status_val];
> > > }
> > > 
> > > -   seq_printf(m, "Source PSR status: %s [0x%08x]\n", status,
> > > val);
> > > +   seq_printf(m, "Source PSR/PanelReplay status: %s
> > > [0x%08x]\n",
> > > status, val);
> > >  }
> > > 
> > >  static int intel_psr_status(struct seq_file *m, struct intel_dp
> > > *intel_dp)
> > > @@ -3048,18 +3067,23 @@ static int intel_psr_status(struct
> > > seq_file
> > > *m, struct intel_dp *intel_dp)
> > > bool enabled;
> > > u32 val;
> > > 
> > > -   seq_printf(m, "Sink support: %s", str_yes_no(psr-
> > > > sink_support));
> > > -   if (psr->sink_support)
> > > +   seq_printf(m, "Sink support: PSR = %s, Panel Replay =
> > > %s",
> > > +  str_yes_no(psr->sink_support),
> > > +  str_yes_no(psr->sink_panel_replay_support));
> > > +
> > > +   if (psr->sink_support || psr->sink_panel_replay_support)
> > > seq_printf(m, " [0x%02x]", intel_dp-
> > > >psr_dpcd[0]);
> > > seq_puts(m, "\n");
> > > 
> > > -   if (!psr->sink_support)
> > > +   if (!(psr->sink_support || psr-
> > > >sink_panel_replay_support))
> > > return 0;
> > > 
> > > wakeref = intel_runtime_pm_get(_priv->runtime_pm);
> > > mutex_lock(>lock);
> > > 
> > > -   if (psr->enabled)
> > > +   if (psr->panel_replay_enabled)
> > > +   status = "Panel Replay Enabled";
> > > +   else if (psr->enabled)
> > > status = psr->psr2_enabled ? "PSR2 enabled" :
> > > "PSR1
> > > enabled";
> > > else
> > > status = "disabled";
> > > @@ -3072,14 +3096,17 @@ static int intel_psr_status(struct
> > > seq_file
> > > *m, struct intel_dp *intel_dp)
> > > goto unlock;
> > > }
> > > 
> > > -   if (psr->psr2_enabled) {
> > > +   if 

Re: [Intel-gfx] [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

2023-11-03 Thread Manna, Animesh


> -Original Message-
> From: Hogander, Jouni 
> Sent: Thursday, November 2, 2023 1:08 PM
> To: Manna, Animesh ; intel-
> g...@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org; Murthy, Arun R
> ; Nikula, Jani 
> Subject: Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for
> panel replay
> 
> On Wed, 2023-10-11 at 16:39 +0530, Animesh Manna wrote:
> > Add debugfs support which will print source and sink status per
> > connector basis.
> 
> Sorry for late review. Noticed only by now that you have added this patch
> into you set.

Added from v5.

> 
> Can you please describe in commit message how you see the output of
> debugfs interface will look like after your changes?

Sure.

> 
> >
> > v1: Initial version. [rb-ed by Arun]
> > v2: Added check for DP 2.0 and connector type in
> > connector_debugfs_add().
> >
> > Cc: Jouni Högander 
> > Cc: Arun R Murthy 
> > Cc: Jani Nikula 
> > Signed-off-by: Animesh Manna 
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 136 +
> > --
> >  1 file changed, 102 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 80de831c2f60..399fc0a8e636 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2823,6 +2823,25 @@ static int
> > psr_get_status_and_error_status(struct intel_dp *intel_dp,
> > return 0;
> >  }
> >
> > +static int panel_replay_get_status_and_error_status(struct intel_dp
> > *intel_dp,
> > +   u8 *status, u8
> > *error_status)
> > +{
> > +   struct drm_dp_aux *aux = _dp->aux;
> > +   int ret;
> > +
> > +   ret = drm_dp_dpcd_readb(aux,
> > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> > +   if (ret != 1)
> > +   return ret;
> > +
> > +   ret = drm_dp_dpcd_readb(aux, DP_PANEL_REPLAY_ERROR_STATUS,
> > error_status);
> > +   if (ret != 1)
> > +   return ret;
> > +
> > +   *status = *status & DP_PSR_SINK_STATE_MASK;
> > +
> > +   return 0;
> > +}
> > +
> 
> I think you should modify  psr_get_status_and_error_status instead of
> duplicating most of it.

DPCD addresses are different for panel replay, I did not get the need of it. 
 
> 
> >  static void psr_alpm_check(struct intel_dp *intel_dp)
> >  {
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@
> > -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp, struct
> > seq_file *m)
> > status = live_status[status_val];
> > }
> >
> > -   seq_printf(m, "Source PSR status: %s [0x%08x]\n", status,
> > val);
> > +   seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n",
> > status, val);
> >  }
> >
> >  static int intel_psr_status(struct seq_file *m, struct intel_dp
> > *intel_dp)
> > @@ -3048,18 +3067,23 @@ static int intel_psr_status(struct seq_file
> > *m, struct intel_dp *intel_dp)
> > bool enabled;
> > u32 val;
> >
> > -   seq_printf(m, "Sink support: %s", str_yes_no(psr-
> > >sink_support));
> > -   if (psr->sink_support)
> > +   seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
> > +  str_yes_no(psr->sink_support),
> > +  str_yes_no(psr->sink_panel_replay_support));
> > +
> > +   if (psr->sink_support || psr->sink_panel_replay_support)
> > seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
> > seq_puts(m, "\n");
> >
> > -   if (!psr->sink_support)
> > +   if (!(psr->sink_support || psr->sink_panel_replay_support))
> > return 0;
> >
> > wakeref = intel_runtime_pm_get(_priv->runtime_pm);
> > mutex_lock(>lock);
> >
> > -   if (psr->enabled)
> > +   if (psr->panel_replay_enabled)
> > +   status = "Panel Replay Enabled";
> > +   else if (psr->enabled)
> > status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1
> > enabled";
> > else
> > status = "disabled";
> > @@ -3072,14 +3096,17 @@ static int intel_psr_status(struct seq_file
> > *m, struct intel_dp *intel_dp)
> > goto unlock;
> > }
> >
> > -   if (psr->psr2_enabled) {
> > +   if (psr->panel_replay_enabled) {
> > +   val = intel_de_read(dev_priv,
> > TRANS_DP2_CTL(cpu_transcoder));
> > +   enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
> > +   } else if (psr->psr2_enabled) {
> > val = intel_de_read(dev_priv,
> > EDP_PSR2_CTL(cpu_transcoder));
> >
> > enabled = val & EDP_PSR2_ENABLE;
> > } else {
> > val = intel_de_read(dev_priv, psr_ctl_reg(dev_priv,
> > cpu_transcoder));
> > enabled = val & EDP_PSR_ENABLE;
> > }
> > -   seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
> > +   seq_printf(m, "Source PSR/PanelReplay ctl: %s 

Re: [Intel-gfx] [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

2023-11-02 Thread Hogander, Jouni
On Wed, 2023-10-11 at 16:39 +0530, Animesh Manna wrote:
> Add debugfs support which will print source and sink status
> per connector basis.

Sorry for late review. Noticed only by now that you have added this
patch into you set.

Can you please describe in commit message how you see the output of
debugfs interface will look like after your changes?

> 
> v1: Initial version. [rb-ed by Arun]
> v2: Added check for DP 2.0 and connector type in
> connector_debugfs_add().
> 
> Cc: Jouni Högander 
> Cc: Arun R Murthy 
> Cc: Jani Nikula 
> Signed-off-by: Animesh Manna 
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 136 +
> --
>  1 file changed, 102 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 80de831c2f60..399fc0a8e636 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2823,6 +2823,25 @@ static int
> psr_get_status_and_error_status(struct intel_dp *intel_dp,
> return 0;
>  }
>  
> +static int panel_replay_get_status_and_error_status(struct intel_dp
> *intel_dp,
> +   u8 *status, u8
> *error_status)
> +{
> +   struct drm_dp_aux *aux = _dp->aux;
> +   int ret;
> +
> +   ret = drm_dp_dpcd_readb(aux,
> DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> +   if (ret != 1)
> +   return ret;
> +
> +   ret = drm_dp_dpcd_readb(aux, DP_PANEL_REPLAY_ERROR_STATUS,
> error_status);
> +   if (ret != 1)
> +   return ret;
> +
> +   *status = *status & DP_PSR_SINK_STATE_MASK;
> +
> +   return 0;
> +}
> +

I think you should modify  psr_get_status_and_error_status instead of
duplicating most of it.

>  static void psr_alpm_check(struct intel_dp *intel_dp)
>  {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> @@ -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp,
> struct seq_file *m)
> status = live_status[status_val];
> }
>  
> -   seq_printf(m, "Source PSR status: %s [0x%08x]\n", status,
> val);
> +   seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n",
> status, val);
>  }
>  
>  static int intel_psr_status(struct seq_file *m, struct intel_dp
> *intel_dp)
> @@ -3048,18 +3067,23 @@ static int intel_psr_status(struct seq_file
> *m, struct intel_dp *intel_dp)
> bool enabled;
> u32 val;
>  
> -   seq_printf(m, "Sink support: %s", str_yes_no(psr-
> >sink_support));
> -   if (psr->sink_support)
> +   seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
> +  str_yes_no(psr->sink_support),
> +  str_yes_no(psr->sink_panel_replay_support));
> +
> +   if (psr->sink_support || psr->sink_panel_replay_support)
> seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
> seq_puts(m, "\n");
>  
> -   if (!psr->sink_support)
> +   if (!(psr->sink_support || psr->sink_panel_replay_support))
> return 0;
>  
> wakeref = intel_runtime_pm_get(_priv->runtime_pm);
> mutex_lock(>lock);
>  
> -   if (psr->enabled)
> +   if (psr->panel_replay_enabled)
> +   status = "Panel Replay Enabled";
> +   else if (psr->enabled)
> status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1
> enabled";
> else
> status = "disabled";
> @@ -3072,14 +3096,17 @@ static int intel_psr_status(struct seq_file
> *m, struct intel_dp *intel_dp)
> goto unlock;
> }
>  
> -   if (psr->psr2_enabled) {
> +   if (psr->panel_replay_enabled) {
> +   val = intel_de_read(dev_priv,
> TRANS_DP2_CTL(cpu_transcoder));
> +   enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
> +   } else if (psr->psr2_enabled) {
> val = intel_de_read(dev_priv,
> EDP_PSR2_CTL(cpu_transcoder));
> 
> enabled = val & EDP_PSR2_ENABLE;
> } else {
> val = intel_de_read(dev_priv, psr_ctl_reg(dev_priv,
> cpu_transcoder));
> enabled = val & EDP_PSR_ENABLE;
> }
> -   seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
> +   seq_printf(m, "Source PSR/PanelReplay ctl: %s [0x%08x]\n",
>    str_enabled_disabled(enabled), val);
> psr_source_status(intel_dp, m);
> seq_printf(m, "Busy frontbuffer bits: 0x%08x\n",
> @@ -3221,6 +3248,7 @@ static int i915_psr_sink_status_show(struct
> seq_file *m, void *data)
>  {
> struct intel_connector *connector = m->private;
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> +   struct intel_psr *psr = _dp->psr;
> static const char * const sink_status[] = {
> "inactive",
> "transition to active, capture and display",
> @@ -3231,45 +3259,82 @@ static int i915_psr_sink_status_show(struct
> seq_file *m, 

Re: [Intel-gfx] [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

2023-10-30 Thread Murthy, Arun R


> -Original Message-
> From: Manna, Animesh 
> Sent: Wednesday, October 11, 2023 4:40 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org; Manna, Animesh
> ; Hogander, Jouni ;
> Murthy, Arun R ; Nikula, Jani
> 
> Subject: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel
> replay
> 
> Add debugfs support which will print source and sink status per connector
> basis.
> 
> v1: Initial version. [rb-ed by Arun]
> v2: Added check for DP 2.0 and connector type in connector_debugfs_add().
> 
> Cc: Jouni Högander 
> Cc: Arun R Murthy 
> Cc: Jani Nikula 
> Signed-off-by: Animesh Manna 
> ---
Reviewed-by: Arun R Murthy 

Thanks and Regards,
Arun R Murthy


>  drivers/gpu/drm/i915/display/intel_psr.c | 136 +--
>  1 file changed, 102 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 80de831c2f60..399fc0a8e636 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2823,6 +2823,25 @@ static int psr_get_status_and_error_status(struct
> intel_dp *intel_dp,
>   return 0;
>  }
> 
> +static int panel_replay_get_status_and_error_status(struct intel_dp 
> *intel_dp,
> + u8 *status, u8
> *error_status) {
> + struct drm_dp_aux *aux = _dp->aux;
> + int ret;
> +
> + ret = drm_dp_dpcd_readb(aux,
> DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> + if (ret != 1)
> + return ret;
> +
> + ret = drm_dp_dpcd_readb(aux, DP_PANEL_REPLAY_ERROR_STATUS,
> error_status);
> + if (ret != 1)
> + return ret;
> +
> + *status = *status & DP_PSR_SINK_STATE_MASK;
> +
> + return 0;
> +}
> +
>  static void psr_alpm_check(struct intel_dp *intel_dp)  {
>   struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ -
> 3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp, struct seq_file
> *m)
>   status = live_status[status_val];
>   }
> 
> - seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val);
> + seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n", status,
> +val);
>  }
> 
>  static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp) 
> @@ -
> 3048,18 +3067,23 @@ static int intel_psr_status(struct seq_file *m, struct
> intel_dp *intel_dp)
>   bool enabled;
>   u32 val;
> 
> - seq_printf(m, "Sink support: %s", str_yes_no(psr->sink_support));
> - if (psr->sink_support)
> + seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
> +str_yes_no(psr->sink_support),
> +str_yes_no(psr->sink_panel_replay_support));
> +
> + if (psr->sink_support || psr->sink_panel_replay_support)
>   seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
>   seq_puts(m, "\n");
> 
> - if (!psr->sink_support)
> + if (!(psr->sink_support || psr->sink_panel_replay_support))
>   return 0;
> 
>   wakeref = intel_runtime_pm_get(_priv->runtime_pm);
>   mutex_lock(>lock);
> 
> - if (psr->enabled)
> + if (psr->panel_replay_enabled)
> + status = "Panel Replay Enabled";
> + else if (psr->enabled)
>   status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1 enabled";
>   else
>   status = "disabled";
> @@ -3072,14 +3096,17 @@ static int intel_psr_status(struct seq_file *m,
> struct intel_dp *intel_dp)
>   goto unlock;
>   }
> 
> - if (psr->psr2_enabled) {
> + if (psr->panel_replay_enabled) {
> + val = intel_de_read(dev_priv,
> TRANS_DP2_CTL(cpu_transcoder));
> + enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
> + } else if (psr->psr2_enabled) {
>   val = intel_de_read(dev_priv, EDP_PSR2_CTL(cpu_transcoder));
>   enabled = val & EDP_PSR2_ENABLE;
>   } else {
>   val = intel_de_read(dev_priv, psr_ctl_reg(dev_priv,
> cpu_transcoder));
>   enabled = val & EDP_PSR_ENABLE;
>   }
> - seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
> + seq_printf(m, "Source PSR/PanelReplay ctl: %s [0x%08x]\n",
>  str_enabled_disabled(enabled), val);
>   psr_source_status(intel_dp, m);
>   seq_printf(m, "Busy frontbuffer bits: 0x%08x\n", @@ -3221,6 +3248,7
> @@ static int i915_psr_sink_status_show(struct seq_file *m, void *data)  {
>   struct intel_connector *connector = m->private;
>   struct intel_dp *intel_dp = intel_attached_dp(connector);
> + struct intel_psr *psr = _dp->psr;
>   static const char * const sink_status[] = {
>   "inactive",
>   "transition to active, capture and display", @@ -3231,45
> +3259,82 @@ static int i915_psr_sink_status_show(struct seq_file *m, void
> *data)
>   "reserved",
>   "sink internal error",
>   };
> + 

Re: [Intel-gfx] [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

2023-10-15 Thread Murthy, Arun R


> -Original Message-
> From: Manna, Animesh 
> Sent: Wednesday, October 11, 2023 4:40 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org; Manna, Animesh
> ; Hogander, Jouni ;
> Murthy, Arun R ; Nikula, Jani
> 
> Subject: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel
> replay
> 
> Add debugfs support which will print source and sink status per connector
> basis.
> 
> v1: Initial version. [rb-ed by Arun]
> v2: Added check for DP 2.0 and connector type in connector_debugfs_add().
> 
> Cc: Jouni Högander 
> Cc: Arun R Murthy 
> Cc: Jani Nikula 
> Signed-off-by: Animesh Manna 
> ---

Reviewed-:by: Arun R Murthy 

Thanks and Regards,
Arun R Murthy
---
>  drivers/gpu/drm/i915/display/intel_psr.c | 136 +--
>  1 file changed, 102 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 80de831c2f60..399fc0a8e636 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2823,6 +2823,25 @@ static int psr_get_status_and_error_status(struct
> intel_dp *intel_dp,
>   return 0;
>  }
> 
> +static int panel_replay_get_status_and_error_status(struct intel_dp 
> *intel_dp,
> + u8 *status, u8
> *error_status) {
> + struct drm_dp_aux *aux = _dp->aux;
> + int ret;
> +
> + ret = drm_dp_dpcd_readb(aux,
> DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> + if (ret != 1)
> + return ret;
> +
> + ret = drm_dp_dpcd_readb(aux, DP_PANEL_REPLAY_ERROR_STATUS,
> error_status);
> + if (ret != 1)
> + return ret;
> +
> + *status = *status & DP_PSR_SINK_STATE_MASK;
> +
> + return 0;
> +}
> +
>  static void psr_alpm_check(struct intel_dp *intel_dp)  {
>   struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ -
> 3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp, struct seq_file
> *m)
>   status = live_status[status_val];
>   }
> 
> - seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val);
> + seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n", status,
> +val);
>  }
> 
>  static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp) 
> @@ -
> 3048,18 +3067,23 @@ static int intel_psr_status(struct seq_file *m, struct
> intel_dp *intel_dp)
>   bool enabled;
>   u32 val;
> 
> - seq_printf(m, "Sink support: %s", str_yes_no(psr->sink_support));
> - if (psr->sink_support)
> + seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
> +str_yes_no(psr->sink_support),
> +str_yes_no(psr->sink_panel_replay_support));
> +
> + if (psr->sink_support || psr->sink_panel_replay_support)
>   seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
>   seq_puts(m, "\n");
> 
> - if (!psr->sink_support)
> + if (!(psr->sink_support || psr->sink_panel_replay_support))
>   return 0;
> 
>   wakeref = intel_runtime_pm_get(_priv->runtime_pm);
>   mutex_lock(>lock);
> 
> - if (psr->enabled)
> + if (psr->panel_replay_enabled)
> + status = "Panel Replay Enabled";
> + else if (psr->enabled)
>   status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1 enabled";
>   else
>   status = "disabled";
> @@ -3072,14 +3096,17 @@ static int intel_psr_status(struct seq_file *m,
> struct intel_dp *intel_dp)
>   goto unlock;
>   }
> 
> - if (psr->psr2_enabled) {
> + if (psr->panel_replay_enabled) {
> + val = intel_de_read(dev_priv,
> TRANS_DP2_CTL(cpu_transcoder));
> + enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
> + } else if (psr->psr2_enabled) {
>   val = intel_de_read(dev_priv, EDP_PSR2_CTL(cpu_transcoder));
>   enabled = val & EDP_PSR2_ENABLE;
>   } else {
>   val = intel_de_read(dev_priv, psr_ctl_reg(dev_priv,
> cpu_transcoder));
>   enabled = val & EDP_PSR_ENABLE;
>   }
> - seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
> + seq_printf(m, "Source PSR/PanelReplay ctl: %s [0x%08x]\n",
>  str_enabled_disabled(enabled), val);
>   psr_source_status(intel_dp, m);
>   seq_printf(m, "Busy frontbuffer bits: 0x%08x\n", @@ -3221,6 +3248,7
> @@ static int i915_psr_sink_status_show(struct seq_file *m, void *data)  {
>   struct intel_connector *connector = m->private;
>   struct intel_dp *intel_dp = intel_attached_dp(connector);
> + struct intel_psr *psr = _dp->psr;
>   static const char * const sink_status[] = {
>   "inactive",
>   "transition to active, capture and display", @@ -3231,45
> +3259,82 @@ static int i915_psr_sink_status_show(struct seq_file *m, void
> *data)
>   "reserved",
>   "sink internal error",
>   };
> + 

[Intel-gfx] [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

2023-10-11 Thread Animesh Manna
Add debugfs support which will print source and sink status
per connector basis.

v1: Initial version. [rb-ed by Arun]
v2: Added check for DP 2.0 and connector type in connector_debugfs_add().

Cc: Jouni Högander 
Cc: Arun R Murthy 
Cc: Jani Nikula 
Signed-off-by: Animesh Manna 
---
 drivers/gpu/drm/i915/display/intel_psr.c | 136 +--
 1 file changed, 102 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c 
b/drivers/gpu/drm/i915/display/intel_psr.c
index 80de831c2f60..399fc0a8e636 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2823,6 +2823,25 @@ static int psr_get_status_and_error_status(struct 
intel_dp *intel_dp,
return 0;
 }
 
+static int panel_replay_get_status_and_error_status(struct intel_dp *intel_dp,
+   u8 *status, u8 
*error_status)
+{
+   struct drm_dp_aux *aux = _dp->aux;
+   int ret;
+
+   ret = drm_dp_dpcd_readb(aux, DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, 
status);
+   if (ret != 1)
+   return ret;
+
+   ret = drm_dp_dpcd_readb(aux, DP_PANEL_REPLAY_ERROR_STATUS, 
error_status);
+   if (ret != 1)
+   return ret;
+
+   *status = *status & DP_PSR_SINK_STATE_MASK;
+
+   return 0;
+}
+
 static void psr_alpm_check(struct intel_dp *intel_dp)
 {
struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
@@ -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp, struct 
seq_file *m)
status = live_status[status_val];
}
 
-   seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val);
+   seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n", status, 
val);
 }
 
 static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp)
@@ -3048,18 +3067,23 @@ static int intel_psr_status(struct seq_file *m, struct 
intel_dp *intel_dp)
bool enabled;
u32 val;
 
-   seq_printf(m, "Sink support: %s", str_yes_no(psr->sink_support));
-   if (psr->sink_support)
+   seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
+  str_yes_no(psr->sink_support),
+  str_yes_no(psr->sink_panel_replay_support));
+
+   if (psr->sink_support || psr->sink_panel_replay_support)
seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
seq_puts(m, "\n");
 
-   if (!psr->sink_support)
+   if (!(psr->sink_support || psr->sink_panel_replay_support))
return 0;
 
wakeref = intel_runtime_pm_get(_priv->runtime_pm);
mutex_lock(>lock);
 
-   if (psr->enabled)
+   if (psr->panel_replay_enabled)
+   status = "Panel Replay Enabled";
+   else if (psr->enabled)
status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1 enabled";
else
status = "disabled";
@@ -3072,14 +3096,17 @@ static int intel_psr_status(struct seq_file *m, struct 
intel_dp *intel_dp)
goto unlock;
}
 
-   if (psr->psr2_enabled) {
+   if (psr->panel_replay_enabled) {
+   val = intel_de_read(dev_priv, TRANS_DP2_CTL(cpu_transcoder));
+   enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
+   } else if (psr->psr2_enabled) {
val = intel_de_read(dev_priv, EDP_PSR2_CTL(cpu_transcoder));
enabled = val & EDP_PSR2_ENABLE;
} else {
val = intel_de_read(dev_priv, psr_ctl_reg(dev_priv, 
cpu_transcoder));
enabled = val & EDP_PSR_ENABLE;
}
-   seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
+   seq_printf(m, "Source PSR/PanelReplay ctl: %s [0x%08x]\n",
   str_enabled_disabled(enabled), val);
psr_source_status(intel_dp, m);
seq_printf(m, "Busy frontbuffer bits: 0x%08x\n",
@@ -3221,6 +3248,7 @@ static int i915_psr_sink_status_show(struct seq_file *m, 
void *data)
 {
struct intel_connector *connector = m->private;
struct intel_dp *intel_dp = intel_attached_dp(connector);
+   struct intel_psr *psr = _dp->psr;
static const char * const sink_status[] = {
"inactive",
"transition to active, capture and display",
@@ -3231,45 +3259,82 @@ 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;
 
-   if (!CAN_PSR(intel_dp)) {
-   seq_puts(m, "PSR