diff mbox series

[v9,08/12] drm/i915/psr: Panel replay uses SRD_STATUS to track it's status

Message ID 20240503063413.1008135-9-jouni.hogander@intel.com (mailing list archive)
State New, archived
Headers show
Series Panel replay selective update support | expand

Commit Message

Hogander, Jouni May 3, 2024, 6:34 a.m. UTC
DP Panel replay uses SRD_STATUS to track it's status despite selective
update mode.

Bspec: 53370, 68920

v3:
  - do not use PSR2_STATUS for PSR1
v2:
  - use intel_dp_is_edp to differentiate
  - modify debugfs status as well

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Manna, Animesh May 9, 2024, 3:13 p.m. UTC | #1
> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Friday, May 3, 2024 12:04 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Manna, Animesh <animesh.manna@intel.com>; Hogander, Jouni
> <jouni.hogander@intel.com>
> Subject: [PATCH v9 08/12] drm/i915/psr: Panel replay uses SRD_STATUS to
> track it's status
> 
> DP Panel replay uses SRD_STATUS to track it's status despite selective update
> mode.
> 
> Bspec: 53370, 68920
> 
> v3:
>   - do not use PSR2_STATUS for PSR1
> v2:
>   - use intel_dp_is_edp to differentiate
>   - modify debugfs status as well
> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 5e5ef432b931..8f23df5cd5a8 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2860,7 +2860,8 @@ void intel_psr_wait_for_idle_locked(const struct
> intel_crtc_state *new_crtc_stat
>  		if (!intel_dp->psr.enabled)
>  			continue;
> 
> -		if (intel_dp->psr.sel_update_enabled)
> +		if (intel_dp_is_edp(intel_dp) &&
> +		    intel_dp->psr.sel_update_enabled)

Why not using panel_replay_enabled check here? For edp panel replay selective update the above condition will be true, rt?

>  			ret =
> _psr2_ready_for_pipe_update_locked(intel_dp);
>  		else
>  			ret =
> _psr1_ready_for_pipe_update_locked(intel_dp);
> @@ -2881,7 +2882,8 @@ static bool __psr_wait_for_idle_locked(struct
> intel_dp *intel_dp)
>  	if (!intel_dp->psr.enabled)
>  		return false;
> 
> -	if (intel_dp->psr.sel_update_enabled) {
> +	if (!intel_dp->psr.panel_replay_enabled &&
> +	    intel_dp->psr.sel_update_enabled) {
>  		reg = EDP_PSR2_STATUS(cpu_transcoder);
>  		mask = EDP_PSR2_STATUS_STATE_MASK;
>  	} else {
> @@ -3500,7 +3502,7 @@ psr_source_status(struct intel_dp *intel_dp, struct
> seq_file *m)
>  	const char *status = "unknown";
>  	u32 val, status_val;
> 
> -	if (intel_dp->psr.sel_update_enabled) {
> +	if (intel_dp_is_edp(intel_dp) && intel_dp->psr.sel_update_enabled) {

Same as above.

Regards,
Animesh

>  		static const char * const live_status[] = {
>  			"IDLE",
>  			"CAPTURE",
> --
> 2.34.1
Hogander, Jouni May 10, 2024, 5:31 a.m. UTC | #2
On Thu, 2024-05-09 at 15:13 +0000, Manna, Animesh wrote:
> 
> 
> > -----Original Message-----
> > From: Hogander, Jouni <jouni.hogander@intel.com>
> > Sent: Friday, May 3, 2024 12:04 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Manna, Animesh <animesh.manna@intel.com>; Hogander, Jouni
> > <jouni.hogander@intel.com>
> > Subject: [PATCH v9 08/12] drm/i915/psr: Panel replay uses
> > SRD_STATUS to
> > track it's status
> > 
> > DP Panel replay uses SRD_STATUS to track it's status despite
> > selective update
> > mode.
> > 
> > Bspec: 53370, 68920
> > 
> > v3:
> >   - do not use PSR2_STATUS for PSR1
> > v2:
> >   - use intel_dp_is_edp to differentiate
> >   - modify debugfs status as well
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 5e5ef432b931..8f23df5cd5a8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2860,7 +2860,8 @@ void intel_psr_wait_for_idle_locked(const
> > struct
> > intel_crtc_state *new_crtc_stat
> >                 if (!intel_dp->psr.enabled)
> >                         continue;
> > 
> > -               if (intel_dp->psr.sel_update_enabled)
> > +               if (intel_dp_is_edp(intel_dp) &&
> > +                   intel_dp->psr.sel_update_enabled)
> 
> Why not using panel_replay_enabled check here? For edp panel replay
> selective update the above condition will be true, rt?

We want to use PSR2_STATUS for PSR2 and Panel Replay with ALPM. I
somehow read/thought Panel Replay full frame update would use
SRD_STATUS on eDP, but now after re-checking Bspec correct check would
be:

/* PSR2 and Panel Replay with ALPM */
if (intel_dp_is_edp(intel_dp) && (intel_dp->psr.sel_update || intel_dp-
>psr.panel_replay_enabled))

Do you agree?

BR,

Jouni Högander


> 
> >                         ret =
> > _psr2_ready_for_pipe_update_locked(intel_dp);
> >                 else
> >                         ret =
> > _psr1_ready_for_pipe_update_locked(intel_dp);
> > @@ -2881,7 +2882,8 @@ static bool __psr_wait_for_idle_locked(struct
> > intel_dp *intel_dp)
> >         if (!intel_dp->psr.enabled)
> >                 return false;
> > 
> > -       if (intel_dp->psr.sel_update_enabled) {
> > +       if (!intel_dp->psr.panel_replay_enabled &&
> > +           intel_dp->psr.sel_update_enabled) {
> >                 reg = EDP_PSR2_STATUS(cpu_transcoder);
> >                 mask = EDP_PSR2_STATUS_STATE_MASK;
> >         } else {
> > @@ -3500,7 +3502,7 @@ psr_source_status(struct intel_dp *intel_dp,
> > struct
> > seq_file *m)
> >         const char *status = "unknown";
> >         u32 val, status_val;
> > 
> > -       if (intel_dp->psr.sel_update_enabled) {
> > +       if (intel_dp_is_edp(intel_dp) && intel_dp-
> > >psr.sel_update_enabled) {
> 
> Same as above.
> 
> Regards,
> Animesh
> 
> >                 static const char * const live_status[] = {
> >                         "IDLE",
> >                         "CAPTURE",
> > --
> > 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 5e5ef432b931..8f23df5cd5a8 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2860,7 +2860,8 @@  void intel_psr_wait_for_idle_locked(const struct intel_crtc_state *new_crtc_stat
 		if (!intel_dp->psr.enabled)
 			continue;
 
-		if (intel_dp->psr.sel_update_enabled)
+		if (intel_dp_is_edp(intel_dp) &&
+		    intel_dp->psr.sel_update_enabled)
 			ret = _psr2_ready_for_pipe_update_locked(intel_dp);
 		else
 			ret = _psr1_ready_for_pipe_update_locked(intel_dp);
@@ -2881,7 +2882,8 @@  static bool __psr_wait_for_idle_locked(struct intel_dp *intel_dp)
 	if (!intel_dp->psr.enabled)
 		return false;
 
-	if (intel_dp->psr.sel_update_enabled) {
+	if (!intel_dp->psr.panel_replay_enabled &&
+	    intel_dp->psr.sel_update_enabled) {
 		reg = EDP_PSR2_STATUS(cpu_transcoder);
 		mask = EDP_PSR2_STATUS_STATE_MASK;
 	} else {
@@ -3500,7 +3502,7 @@  psr_source_status(struct intel_dp *intel_dp, struct seq_file *m)
 	const char *status = "unknown";
 	u32 val, status_val;
 
-	if (intel_dp->psr.sel_update_enabled) {
+	if (intel_dp_is_edp(intel_dp) && intel_dp->psr.sel_update_enabled) {
 		static const char * const live_status[] = {
 			"IDLE",
 			"CAPTURE",