[PATCH 0/7] Use trans push mechanism to generate frame change event

[PATCH 0/7] Use trans push mechanism to generate frame change event

[Jouni Högander]

Currently we are using "automatic" frame change event generation. The
event is generated by any access to plane or pipe registers.

We have option to use "PSR PR Frame Change Enable" bit in TRANS_PUSH
register to enable frame change event generation on trans push. When
this bit is set "automatic" frame change event generation doesn't work
anymore.

This patch set is taking trans push mechanism into use.

Jouni Högander (7):
  drm/i915/psr: Add TRANS_PUSH register bit definition for PSR
  drm/i915/vrr: Do not overwrite TRANS_PUSH PSR Frame Change Enable
  drm/i915/vrr: Use TRANS_PUSH mechanism for PSR frame change
  drm/i915/psr: Rename psr_force_hw_tracking_exit as psr_force_exit
  drm/i915/psr: Simplify frontbuffer invalidate/flush callbacks
  drm/i915/psr: Add VRR send push interface for PSR usage
  drm/i915/display: Generate PSR frame change event on cursor update

 drivers/gpu/drm/i915/display/intel_cursor.c   |  5 ++
 drivers/gpu/drm/i915/display/intel_psr.c      | 83 ++++++-------------
 drivers/gpu/drm/i915/display/intel_vrr.c      | 71 ++++++++++++++--
 drivers/gpu/drm/i915/display/intel_vrr.h      |  6 ++
 drivers/gpu/drm/i915/display/intel_vrr_regs.h |  1 +
 5 files changed, 103 insertions(+), 63 deletions(-)

-- 
2.34.1

[PATCH 1/7] drm/i915/psr: Add TRANS_PUSH register bit definition for PSR

[Jouni Högander]

Add TRANS_PUSH register bit LNL_TRANS_PUSH_PSR_PR_EN definition for PSR
usage.

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_vrr_regs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/display/intel_vrr_regs.h b/drivers/gpu/drm/i915/display/intel_vrr_regs.h
index 6ed0e0dc97e76..50540eac61a31 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr_regs.h
@@ -100,6 +100,7 @@
 #define TRANS_PUSH(dev_priv, trans)		_MMIO_TRANS2(dev_priv, trans, _TRANS_PUSH_A)
 #define  TRANS_PUSH_EN				REG_BIT(31)
 #define  TRANS_PUSH_SEND			REG_BIT(30)
+#define  LNL_TRANS_PUSH_PSR_PR_EN		REG_BIT(16)
 
 #define _TRANS_VRR_VSYNC_A			0x60078
 #define TRANS_VRR_VSYNC(dev_priv, trans)	_MMIO_TRANS2(dev_priv, trans, _TRANS_VRR_VSYNC_A)
-- 
2.34.1

[PATCH 2/7] drm/i915/vrr: Do not overwrite TRANS_PUSH PSR Frame Change Enable

[Jouni Högander]

Currently vrr code is overwriting possibly set PSR PR Frame Change Enable
bit in TRANS_PUSH register. Avoid this by using rmw instead of write.

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

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 9a51f5bac3071..8b4e3f938efea 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -339,8 +339,8 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
 	if (!crtc_state->vrr.enable)
 		return;
 
-	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
-		       TRANS_PUSH_EN);
+	intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder), 0,
+		     TRANS_PUSH_EN);
 
 	if (HAS_AS_SDP(display))
 		intel_de_write(display,
@@ -371,7 +371,9 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
 	intel_de_wait_for_clear(display,
 				TRANS_VRR_STATUS(display, cpu_transcoder),
 				VRR_STATUS_VRR_EN_LIVE, 1000);
-	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0);
+
+	intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder), TRANS_PUSH_EN,
+		     0);
 
 	if (HAS_AS_SDP(display))
 		intel_de_write(display,
-- 
2.34.1

Re: [PATCH 2/7] drm/i915/vrr: Do not overwrite TRANS_PUSH PSR Frame Change Enable

[Ville Syrjälä]

On Thu, Oct 10, 2024 at 08:33:11AM +0300, Jouni Högander wrote:
> Currently vrr code is overwriting possibly set PSR PR Frame Change Enable
> bit in TRANS_PUSH register. Avoid this by using rmw instead of write.

RMWs are not good if we ever want to do this from DSB/etc. We should
know what the state should be and just program that in.

Does the PSR bit here have to match the PSR_CTL enable bit or
could we just always set the bit here based on has_psr whether
PSR is currently active or not?

> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 9a51f5bac3071..8b4e3f938efea 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -339,8 +339,8 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
>  	if (!crtc_state->vrr.enable)
>  		return;
>  
> -	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
> -		       TRANS_PUSH_EN);
> +	intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder), 0,
> +		     TRANS_PUSH_EN);
>  
>  	if (HAS_AS_SDP(display))
>  		intel_de_write(display,
> @@ -371,7 +371,9 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
>  	intel_de_wait_for_clear(display,
>  				TRANS_VRR_STATUS(display, cpu_transcoder),
>  				VRR_STATUS_VRR_EN_LIVE, 1000);
> -	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0);
> +
> +	intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder), TRANS_PUSH_EN,
> +		     0);
>  
>  	if (HAS_AS_SDP(display))
>  		intel_de_write(display,
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

Re: [PATCH 2/7] drm/i915/vrr: Do not overwrite TRANS_PUSH PSR Frame Change Enable

[Hogander, Jouni]

On Thu, 2024-10-10 at 15:06 +0300, Ville Syrjälä wrote:
> On Thu, Oct 10, 2024 at 08:33:11AM +0300, Jouni Högander wrote:
> > Currently vrr code is overwriting possibly set PSR PR Frame Change
> > Enable
> > bit in TRANS_PUSH register. Avoid this by using rmw instead of
> > write.
> 
> RMWs are not good if we ever want to do this from DSB/etc. We should
> know what the state should be and just program that in.
> 
> Does the PSR bit here have to match the PSR_CTL enable bit or
> could we just always set the bit here based on has_psr whether
> PSR is currently active or not?

I will try that out. I'll guess this should be ok.

BR,

Jouni Högander
> 
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_vrr.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> > b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index 9a51f5bac3071..8b4e3f938efea 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -339,8 +339,8 @@ void intel_vrr_enable(const struct
> > intel_crtc_state *crtc_state)
> >         if (!crtc_state->vrr.enable)
> >                 return;
> >  
> > -       intel_de_write(display, TRANS_PUSH(display,
> > cpu_transcoder),
> > -                      TRANS_PUSH_EN);
> > +       intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder),
> > 0,
> > +                    TRANS_PUSH_EN);
> >  
> >         if (HAS_AS_SDP(display))
> >                 intel_de_write(display,
> > @@ -371,7 +371,9 @@ void intel_vrr_disable(const struct
> > intel_crtc_state *old_crtc_state)
> >         intel_de_wait_for_clear(display,
> >                                 TRANS_VRR_STATUS(display,
> > cpu_transcoder),
> >                                 VRR_STATUS_VRR_EN_LIVE, 1000);
> > -       intel_de_write(display, TRANS_PUSH(display,
> > cpu_transcoder), 0);
> > +
> > +       intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder),
> > TRANS_PUSH_EN,
> > +                    0);
> >  
> >         if (HAS_AS_SDP(display))
> >                 intel_de_write(display,
> > -- 
> > 2.34.1
> 

[PATCH 3/7] drm/i915/vrr: Use TRANS_PUSH mechanism for PSR frame change

[Jouni Högander]

In Lunarlake and onwards it is possible to generate "PSR frame change"
event using TRANS_PUSH mechanism. Implement function to enable this and
take PSR into account in intel_vrr_send_push.

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c |  6 ++++
 drivers/gpu/drm/i915/display/intel_vrr.c | 45 ++++++++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_vrr.h |  2 ++
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index e3357f3b5c705..be73c1aaf1ee2 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -44,6 +44,7 @@
 #include "intel_psr.h"
 #include "intel_psr_regs.h"
 #include "intel_snps_phy.h"
+#include "intel_vrr.h"
 #include "skl_universal_plane.h"
 
 /**
@@ -1928,6 +1929,9 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
 			intel_de_rmw(display, CLKGATE_DIS_MISC, 0,
 				     CLKGATE_DIS_MISC_DMASC_GATING_DIS);
 	}
+
+	if (DISPLAY_VER(dev_priv) >= 20)
+		intel_vrr_psr_frame_change_enable(crtc_state);
 }
 
 static bool psr_interrupt_error_check(struct intel_dp *intel_dp)
@@ -2164,6 +2168,8 @@ void intel_psr_disable(struct intel_dp *intel_dp,
 
 	mutex_lock(&intel_dp->psr.lock);
 
+	if (DISPLAY_VER(display) >= 20)
+		intel_vrr_psr_frame_change_disable(old_crtc_state);
 	intel_psr_disable_locked(intel_dp);
 
 	mutex_unlock(&intel_dp->psr.lock);
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 8b4e3f938efea..5925ade4591d4 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -312,12 +312,20 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_display *display = to_intel_display(crtc_state);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+	u32 trans_push_val = TRANS_PUSH_SEND;
 
-	if (!crtc_state->vrr.enable)
+	if (!crtc_state->vrr.enable && (DISPLAY_VER(display) < 20 ||
+					!crtc_state->has_psr))
 		return;
 
+	if (crtc_state->vrr.enable)
+		trans_push_val |= TRANS_PUSH_EN;
+
+	if (DISPLAY_VER(display) >= 20 && crtc_state->has_psr)
+		trans_push_val |= LNL_TRANS_PUSH_PSR_PR_EN;
+
 	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
-		       TRANS_PUSH_EN | TRANS_PUSH_SEND);
+		       trans_push_val);
 }
 
 bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
@@ -325,7 +333,8 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
 	struct intel_display *display = to_intel_display(crtc_state);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 
-	if (!crtc_state->vrr.enable)
+	if (!crtc_state->vrr.enable && (DISPLAY_VER(display) < 20 ||
+					!crtc_state->has_psr))
 		return false;
 
 	return intel_de_read(display, TRANS_PUSH(display, cpu_transcoder)) & TRANS_PUSH_SEND;
@@ -358,6 +367,36 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
 	}
 }
 
+/**
+ * intel_vrr_psr_frame_change_enable - Enable PSR frame change mechanism
+ * @crtc_state: Intel crtc state
+ *
+ * This function is for PSR to enable PSR frame change mechanism which is more
+ * controlled way to generate frame change event.
+ */
+void intel_vrr_psr_frame_change_enable(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+
+	intel_de_rmw(display, TRANS_PUSH(display, crtc_state->cpu_transcoder),
+		     0, LNL_TRANS_PUSH_PSR_PR_EN);
+}
+
+/**
+ * intel_vrr_psr_frame_change_disable - Disable PSR frame change mechanism
+ * @crtc_state: Intel crtc state
+ *
+ * This function is for PSR to disable PSR frame change mechanism.
+ */
+void intel_vrr_psr_frame_change_disable(const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+
+	intel_de_rmw(dev_priv, TRANS_PUSH(dev_priv, cpu_transcoder),
+		     LNL_TRANS_PUSH_PSR_PR_EN, 0);
+}
+
 void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
 {
 	struct intel_display *display = to_intel_display(old_crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
index 89937858200d3..a75f159168c11 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr.h
@@ -23,6 +23,8 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state);
 void intel_vrr_send_push(const struct intel_crtc_state *crtc_state);
 bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state);
 void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state);
+void intel_vrr_psr_frame_change_enable(const struct intel_crtc_state *crtc_state);
+void intel_vrr_psr_frame_change_disable(const struct intel_crtc_state *crtc_state);
 void intel_vrr_get_config(struct intel_crtc_state *crtc_state);
 int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state);
 int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state);
-- 
2.34.1

[PATCH 4/7] drm/i915/psr: Rename psr_force_hw_tracking_exit as psr_force_exit

[Jouni Högander]

psr_force_hw_tracking_exit is misleading name as it is used for PSR1, PSR2
HW tracking and PSR2 selective fetch. Due to this rename it as
psr_force_exit.

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

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index be73c1aaf1ee2..b32020321ca7a 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2301,7 +2301,7 @@ static u32 man_trk_ctl_continuos_full_frame(struct intel_display *display)
 	       PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME;
 }
 
-static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
+static void psr_force_exit(struct intel_dp *intel_dp)
 {
 	struct intel_display *display = to_intel_display(intel_dp);
 	enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
@@ -2805,7 +2805,7 @@ void intel_psr_post_plane_update(struct intel_atomic_state *state,
 
 		/* Force a PSR exit when enabling CRC to avoid CRC timeouts */
 		if (crtc_state->crc_enabled && psr->enabled)
-			psr_force_hw_tracking_exit(intel_dp);
+			psr_force_exit(intel_dp);
 
 		/*
 		 * Clear possible busy bits in case we have
@@ -3202,10 +3202,10 @@ static void _psr_flush_handle(struct intel_dp *intel_dp)
 			 * continuous full frame is disabled, only a single full
 			 * frame is required
 			 */
-			psr_force_hw_tracking_exit(intel_dp);
+			psr_force_exit(intel_dp);
 		}
 	} else {
-		psr_force_hw_tracking_exit(intel_dp);
+		psr_force_exit(intel_dp);
 
 		if (!intel_dp->psr.active && !intel_dp->psr.busy_frontbuffer_bits)
 			queue_work(dev_priv->unordered_wq, &intel_dp->psr.work);
-- 
2.34.1

[PATCH 5/7] drm/i915/psr: Simplify frontbuffer invalidate/flush callbacks

[Jouni Högander]

There is unnecessary complexity in frontbuffer tracking invalidate and
flush callbacks. Simplify them a bit with some minor changes to sequences:

Invalidate:

1. Additionally write single full frame bit when selective fetch is
enabled. This should be ok as continuous full frame bit is already set.
2. Rewrite bits in PSR2_MAN_TRK_CTL if two invalidate calls in row without
flush in between (psr.psr2_sel_fetch_cff_enabled == true).

Flush:

1. intel_dp->psr.psr2_sel_fetch_cff_enabled is clearn also when it is
already false.

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

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index b32020321ca7a..5be8076475f0b 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -3069,28 +3069,8 @@ static void intel_psr_work(struct work_struct *work)
 
 static void _psr_invalidate_handle(struct intel_dp *intel_dp)
 {
-	struct intel_display *display = to_intel_display(intel_dp);
-	enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
-
 	if (intel_dp->psr.psr2_sel_fetch_enabled) {
-		u32 val;
-
-		if (intel_dp->psr.psr2_sel_fetch_cff_enabled) {
-			/* Send one update otherwise lag is observed in screen */
-			intel_de_write(display,
-				       CURSURFLIVE(display, intel_dp->psr.pipe),
-				       0);
-			return;
-		}
-
-		val = man_trk_ctl_enable_bit_get(display) |
-		      man_trk_ctl_partial_frame_bit_get(display) |
-		      man_trk_ctl_continuos_full_frame(display);
-		intel_de_write(display,
-			       PSR2_MAN_TRK_CTL(display, cpu_transcoder),
-			       val);
-		intel_de_write(display,
-			       CURSURFLIVE(display, intel_dp->psr.pipe), 0);
+		psr_force_exit(intel_dp);
 		intel_dp->psr.psr2_sel_fetch_cff_enabled = true;
 	} else {
 		intel_psr_exit(intel_dp);
@@ -3172,43 +3152,21 @@ static void _psr_flush_handle(struct intel_dp *intel_dp)
 {
 	struct intel_display *display = to_intel_display(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(display->drm);
-	enum transcoder cpu_transcoder = intel_dp->psr.transcoder;
+
+	psr_force_exit(intel_dp);
 
 	if (intel_dp->psr.psr2_sel_fetch_enabled) {
-		if (intel_dp->psr.psr2_sel_fetch_cff_enabled) {
-			/* can we turn CFF off? */
-			if (intel_dp->psr.busy_frontbuffer_bits == 0) {
-				u32 val = man_trk_ctl_enable_bit_get(display) |
-					man_trk_ctl_partial_frame_bit_get(display) |
-					man_trk_ctl_single_full_frame_bit_get(display) |
-					man_trk_ctl_continuos_full_frame(display);
-
-				/*
-				 * Set psr2_sel_fetch_cff_enabled as false to allow selective
-				 * updates. Still keep cff bit enabled as we don't have proper
-				 * SU configuration in case update is sent for any reason after
-				 * sff bit gets cleared by the HW on next vblank.
-				 */
-				intel_de_write(display,
-					       PSR2_MAN_TRK_CTL(display, cpu_transcoder),
-					       val);
-				intel_de_write(display,
-					       CURSURFLIVE(display, intel_dp->psr.pipe),
-					       0);
-				intel_dp->psr.psr2_sel_fetch_cff_enabled = false;
-			}
-		} else {
+		/* can we turn CFF off? */
+		if (intel_dp->psr.busy_frontbuffer_bits == 0)
 			/*
-			 * continuous full frame is disabled, only a single full
-			 * frame is required
+			 * Set psr2_sel_fetch_cff_enabled as false to allow selective
+			 * updates. Still keep cff bit enabled as we don't have proper
+			 * SU configuration in case update is sent for any reason after
+			 * sff bit gets cleared by the HW on next vblank.
 			 */
-			psr_force_exit(intel_dp);
-		}
-	} else {
-		psr_force_exit(intel_dp);
-
-		if (!intel_dp->psr.active && !intel_dp->psr.busy_frontbuffer_bits)
-			queue_work(dev_priv->unordered_wq, &intel_dp->psr.work);
+			intel_dp->psr.psr2_sel_fetch_cff_enabled = false;
+	} else if (!intel_dp->psr.active && !intel_dp->psr.busy_frontbuffer_bits) {
+		queue_work(dev_priv->unordered_wq, &intel_dp->psr.work);
 	}
 }
 
-- 
2.34.1

[PATCH 6/7] drm/i915/psr: Add VRR send push interface for PSR usage

[Jouni Högander]

Add own interface for PSR usage to perform push on frontbuffer tracking
invalidate and flush call backs. Use this new interface from PSR code.

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c |  7 ++++++-
 drivers/gpu/drm/i915/display/intel_vrr.c | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_vrr.h |  4 ++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 5be8076475f0b..7959a33771b13 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2326,8 +2326,13 @@ static void psr_force_exit(struct intel_dp *intel_dp)
 	 * This workaround do not exist for platforms with display 10 or newer
 	 * but testing proved that it works for up display 13, for newer
 	 * than that testing will be needed.
+	 *
+	 * In Lunarlake we can use TRANS_PUSH mechanism to force sending update
+	 * to sink.
 	 */
-	intel_de_write(display, CURSURFLIVE(display, intel_dp->psr.pipe), 0);
+	DISPLAY_VER(display) >= 20 ?
+		intel_vrr_psr_send_push(display, cpu_transcoder) :
+		intel_de_write(display, CURSURFLIVE(display, intel_dp->psr.pipe), 0);
 }
 
 void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state)
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 5925ade4591d4..d51830d173b61 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -328,6 +328,24 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
 		       trans_push_val);
 }
 
+/**
+ * intel_vrr_psr_send_push - Send push interface for PSR code
+ * @display: Intel display
+ * @cpu_transcoder: cpu_transcode
+ *
+ * This is for PSR usage to perform push on frontbuffer tracking invalidate and
+ * flush call back. PSR mutex should be taken by caller.
+ */
+void intel_vrr_psr_send_push(struct intel_display *display,
+			     enum transcoder cpu_transcoder)
+{
+	if (DISPLAY_VER(display) < 20)
+		return;
+
+	intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder), 0,
+		     TRANS_PUSH_SEND | LNL_TRANS_PUSH_PSR_PR_EN);
+}
+
 bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_display *display = to_intel_display(crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
index a75f159168c11..3da7ba12697ff 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr.h
@@ -12,6 +12,8 @@ struct drm_connector_state;
 struct intel_atomic_state;
 struct intel_connector;
 struct intel_crtc_state;
+struct intel_display;
+enum transcoder;
 
 bool intel_vrr_is_capable(struct intel_connector *connector);
 bool intel_vrr_is_in_range(struct intel_connector *connector, int vrefresh);
@@ -25,6 +27,8 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state);
 void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state);
 void intel_vrr_psr_frame_change_enable(const struct intel_crtc_state *crtc_state);
 void intel_vrr_psr_frame_change_disable(const struct intel_crtc_state *crtc_state);
+void intel_vrr_psr_send_push(struct intel_display *display,
+			     enum transcoder cpu_transcoder);
 void intel_vrr_get_config(struct intel_crtc_state *crtc_state);
 int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state);
 int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state);
-- 
2.34.1

Re: [PATCH 6/7] drm/i915/psr: Add VRR send push interface for PSR usage

[Jani Nikula]

On Thu, 10 Oct 2024, Jouni Högander <jouni.hogander@intel.com> wrote:
> Add own interface for PSR usage to perform push on frontbuffer tracking
> invalidate and flush call backs. Use this new interface from PSR code.
>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c |  7 ++++++-
>  drivers/gpu/drm/i915/display/intel_vrr.c | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_vrr.h |  4 ++++
>  3 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 5be8076475f0b..7959a33771b13 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2326,8 +2326,13 @@ static void psr_force_exit(struct intel_dp *intel_dp)
>  	 * This workaround do not exist for platforms with display 10 or newer
>  	 * but testing proved that it works for up display 13, for newer
>  	 * than that testing will be needed.
> +	 *
> +	 * In Lunarlake we can use TRANS_PUSH mechanism to force sending update
> +	 * to sink.
>  	 */
> -	intel_de_write(display, CURSURFLIVE(display, intel_dp->psr.pipe), 0);
> +	DISPLAY_VER(display) >= 20 ?
> +		intel_vrr_psr_send_push(display, cpu_transcoder) :
> +		intel_de_write(display, CURSURFLIVE(display, intel_dp->psr.pipe), 0);
>  }
>  
>  void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 5925ade4591d4..d51830d173b61 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -328,6 +328,24 @@ void intel_vrr_send_push(const struct intel_crtc_state *crtc_state)
>  		       trans_push_val);
>  }
>  
> +/**
> + * intel_vrr_psr_send_push - Send push interface for PSR code
> + * @display: Intel display
> + * @cpu_transcoder: cpu_transcode
> + *
> + * This is for PSR usage to perform push on frontbuffer tracking invalidate and
> + * flush call back. PSR mutex should be taken by caller.
> + */
> +void intel_vrr_psr_send_push(struct intel_display *display,
> +			     enum transcoder cpu_transcoder)
> +{
> +	if (DISPLAY_VER(display) < 20)
> +		return;
> +
> +	intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder), 0,
> +		     TRANS_PUSH_SEND | LNL_TRANS_PUSH_PSR_PR_EN);
> +}
> +
>  bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> index a75f159168c11..3da7ba12697ff 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -12,6 +12,8 @@ struct drm_connector_state;
>  struct intel_atomic_state;
>  struct intel_connector;
>  struct intel_crtc_state;
> +struct intel_display;
> +enum transcoder;
>  
>  bool intel_vrr_is_capable(struct intel_connector *connector);
>  bool intel_vrr_is_in_range(struct intel_connector *connector, int vrefresh);
> @@ -25,6 +27,8 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state);
>  void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state);
>  void intel_vrr_psr_frame_change_enable(const struct intel_crtc_state *crtc_state);
>  void intel_vrr_psr_frame_change_disable(const struct intel_crtc_state *crtc_state);
> +void intel_vrr_psr_send_push(struct intel_display *display,
> +			     enum transcoder cpu_transcoder);

Nitpick, why not just make that parameter crtc_state like for all the
other functions?

BR,
Jani.

>  void intel_vrr_get_config(struct intel_crtc_state *crtc_state);
>  int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state);
>  int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state);

-- 
Jani Nikula, Intel

Re: [PATCH 6/7] drm/i915/psr: Add VRR send push interface for PSR usage

[Hogander, Jouni]

On Thu, 2024-10-10 at 11:03 +0300, Jani Nikula wrote:
> On Thu, 10 Oct 2024, Jouni Högander <jouni.hogander@intel.com> wrote:
> > Add own interface for PSR usage to perform push on frontbuffer
> > tracking
> > invalidate and flush call backs. Use this new interface from PSR
> > code.
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c |  7 ++++++-
> >  drivers/gpu/drm/i915/display/intel_vrr.c | 18 ++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_vrr.h |  4 ++++
> >  3 files changed, 28 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 5be8076475f0b..7959a33771b13 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2326,8 +2326,13 @@ static void psr_force_exit(struct intel_dp
> > *intel_dp)
> >          * This workaround do not exist for platforms with display
> > 10 or newer
> >          * but testing proved that it works for up display 13, for
> > newer
> >          * than that testing will be needed.
> > +        *
> > +        * In Lunarlake we can use TRANS_PUSH mechanism to force
> > sending update
> > +        * to sink.
> >          */
> > -       intel_de_write(display, CURSURFLIVE(display, intel_dp-
> > >psr.pipe), 0);
> > +       DISPLAY_VER(display) >= 20 ?
> > +               intel_vrr_psr_send_push(display, cpu_transcoder) :
> > +               intel_de_write(display, CURSURFLIVE(display,
> > intel_dp->psr.pipe), 0);
> >  }
> >  
> >  void intel_psr2_program_trans_man_trk_ctl(const struct
> > intel_crtc_state *crtc_state)
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> > b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index 5925ade4591d4..d51830d173b61 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -328,6 +328,24 @@ void intel_vrr_send_push(const struct
> > intel_crtc_state *crtc_state)
> >                        trans_push_val);
> >  }
> >  
> > +/**
> > + * intel_vrr_psr_send_push - Send push interface for PSR code
> > + * @display: Intel display
> > + * @cpu_transcoder: cpu_transcode
> > + *
> > + * This is for PSR usage to perform push on frontbuffer tracking
> > invalidate and
> > + * flush call back. PSR mutex should be taken by caller.
> > + */
> > +void intel_vrr_psr_send_push(struct intel_display *display,
> > +                            enum transcoder cpu_transcoder)
> > +{
> > +       if (DISPLAY_VER(display) < 20)
> > +               return;
> > +
> > +       intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder),
> > 0,
> > +                    TRANS_PUSH_SEND | LNL_TRANS_PUSH_PSR_PR_EN);
> > +}
> > +
> >  bool intel_vrr_is_push_sent(const struct intel_crtc_state
> > *crtc_state)
> >  {
> >         struct intel_display *display =
> > to_intel_display(crtc_state);
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h
> > b/drivers/gpu/drm/i915/display/intel_vrr.h
> > index a75f159168c11..3da7ba12697ff 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> > @@ -12,6 +12,8 @@ struct drm_connector_state;
> >  struct intel_atomic_state;
> >  struct intel_connector;
> >  struct intel_crtc_state;
> > +struct intel_display;
> > +enum transcoder;
> >  
> >  bool intel_vrr_is_capable(struct intel_connector *connector);
> >  bool intel_vrr_is_in_range(struct intel_connector *connector, int
> > vrefresh);
> > @@ -25,6 +27,8 @@ bool intel_vrr_is_push_sent(const struct
> > intel_crtc_state *crtc_state);
> >  void intel_vrr_disable(const struct intel_crtc_state
> > *old_crtc_state);
> >  void intel_vrr_psr_frame_change_enable(const struct
> > intel_crtc_state *crtc_state);
> >  void intel_vrr_psr_frame_change_disable(const struct
> > intel_crtc_state *crtc_state);
> > +void intel_vrr_psr_send_push(struct intel_display *display,
> > +                            enum transcoder cpu_transcoder);
> 
> Nitpick, why not just make that parameter crtc_state like for all the
> other functions?

This is about to be used from frontbuffer flush/invalidate path as
well. There we do not have crtc_state directly. I'll guess we could use
current state of crtc. Do you think that would be better? 

BR,

Jouni Högander

> 
> BR,
> Jani.
> 
> >  void intel_vrr_get_config(struct intel_crtc_state *crtc_state);
> >  int intel_vrr_vmax_vblank_start(const struct intel_crtc_state
> > *crtc_state);
> >  int intel_vrr_vmin_vblank_start(const struct intel_crtc_state
> > *crtc_state);
> 

Re: [PATCH 6/7] drm/i915/psr: Add VRR send push interface for PSR usage

[Jani Nikula]

On Thu, 10 Oct 2024, "Hogander, Jouni" <jouni.hogander@intel.com> wrote:
> On Thu, 2024-10-10 at 11:03 +0300, Jani Nikula wrote:
>> On Thu, 10 Oct 2024, Jouni Högander <jouni.hogander@intel.com> wrote:
>> > Add own interface for PSR usage to perform push on frontbuffer
>> > tracking
>> > invalidate and flush call backs. Use this new interface from PSR
>> > code.
>> > 
>> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_psr.c |  7 ++++++-
>> >  drivers/gpu/drm/i915/display/intel_vrr.c | 18 ++++++++++++++++++
>> >  drivers/gpu/drm/i915/display/intel_vrr.h |  4 ++++
>> >  3 files changed, 28 insertions(+), 1 deletion(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
>> > b/drivers/gpu/drm/i915/display/intel_psr.c
>> > index 5be8076475f0b..7959a33771b13 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>> > @@ -2326,8 +2326,13 @@ static void psr_force_exit(struct intel_dp
>> > *intel_dp)
>> >          * This workaround do not exist for platforms with display
>> > 10 or newer
>> >          * but testing proved that it works for up display 13, for
>> > newer
>> >          * than that testing will be needed.
>> > +        *
>> > +        * In Lunarlake we can use TRANS_PUSH mechanism to force
>> > sending update
>> > +        * to sink.
>> >          */
>> > -       intel_de_write(display, CURSURFLIVE(display, intel_dp-
>> > >psr.pipe), 0);
>> > +       DISPLAY_VER(display) >= 20 ?
>> > +               intel_vrr_psr_send_push(display, cpu_transcoder) :
>> > +               intel_de_write(display, CURSURFLIVE(display,
>> > intel_dp->psr.pipe), 0);
>> >  }
>> >  
>> >  void intel_psr2_program_trans_man_trk_ctl(const struct
>> > intel_crtc_state *crtc_state)
>> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
>> > b/drivers/gpu/drm/i915/display/intel_vrr.c
>> > index 5925ade4591d4..d51830d173b61 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> > @@ -328,6 +328,24 @@ void intel_vrr_send_push(const struct
>> > intel_crtc_state *crtc_state)
>> >                        trans_push_val);
>> >  }
>> >  
>> > +/**
>> > + * intel_vrr_psr_send_push - Send push interface for PSR code
>> > + * @display: Intel display
>> > + * @cpu_transcoder: cpu_transcode
>> > + *
>> > + * This is for PSR usage to perform push on frontbuffer tracking
>> > invalidate and
>> > + * flush call back. PSR mutex should be taken by caller.
>> > + */
>> > +void intel_vrr_psr_send_push(struct intel_display *display,
>> > +                            enum transcoder cpu_transcoder)
>> > +{
>> > +       if (DISPLAY_VER(display) < 20)
>> > +               return;
>> > +
>> > +       intel_de_rmw(display, TRANS_PUSH(display, cpu_transcoder),
>> > 0,
>> > +                    TRANS_PUSH_SEND | LNL_TRANS_PUSH_PSR_PR_EN);
>> > +}
>> > +
>> >  bool intel_vrr_is_push_sent(const struct intel_crtc_state
>> > *crtc_state)
>> >  {
>> >         struct intel_display *display =
>> > to_intel_display(crtc_state);
>> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h
>> > b/drivers/gpu/drm/i915/display/intel_vrr.h
>> > index a75f159168c11..3da7ba12697ff 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_vrr.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
>> > @@ -12,6 +12,8 @@ struct drm_connector_state;
>> >  struct intel_atomic_state;
>> >  struct intel_connector;
>> >  struct intel_crtc_state;
>> > +struct intel_display;
>> > +enum transcoder;
>> >  
>> >  bool intel_vrr_is_capable(struct intel_connector *connector);
>> >  bool intel_vrr_is_in_range(struct intel_connector *connector, int
>> > vrefresh);
>> > @@ -25,6 +27,8 @@ bool intel_vrr_is_push_sent(const struct
>> > intel_crtc_state *crtc_state);
>> >  void intel_vrr_disable(const struct intel_crtc_state
>> > *old_crtc_state);
>> >  void intel_vrr_psr_frame_change_enable(const struct
>> > intel_crtc_state *crtc_state);
>> >  void intel_vrr_psr_frame_change_disable(const struct
>> > intel_crtc_state *crtc_state);
>> > +void intel_vrr_psr_send_push(struct intel_display *display,
>> > +                            enum transcoder cpu_transcoder);
>> 
>> Nitpick, why not just make that parameter crtc_state like for all the
>> other functions?
>
> This is about to be used from frontbuffer flush/invalidate path as
> well. There we do not have crtc_state directly. I'll guess we could use
> current state of crtc. Do you think that would be better? 

I wasn't aware of the potential future use. Just stick with this, at
least for now.

BR,
Jani.


>
> BR,
>
> Jouni Högander
>
>> 
>> BR,
>> Jani.
>> 
>> >  void intel_vrr_get_config(struct intel_crtc_state *crtc_state);
>> >  int intel_vrr_vmax_vblank_start(const struct intel_crtc_state
>> > *crtc_state);
>> >  int intel_vrr_vmin_vblank_start(const struct intel_crtc_state
>> > *crtc_state);
>> 
>

-- 
Jani Nikula, Intel

[PATCH 7/7] drm/i915/display: Generate PSR frame change event on cursor update

[Jouni Högander]

On LunarLake and onwards we are using vrr send push mechanism to trigger
frame change event. Due to this we need to trigger it using
intel_vrr_psr_send_push provided by VRR code on legacy cursor update.

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

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 050eacc709cc1..dc8629f843662 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -25,6 +25,7 @@
 #include "intel_psr.h"
 #include "intel_psr_regs.h"
 #include "intel_vblank.h"
+#include "intel_vrr.h"
 #include "skl_watermark.h"
 
 /* Cursor formats */
@@ -790,6 +791,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
 	struct intel_crtc_state *crtc_state =
 		to_intel_crtc_state(crtc->base.state);
 	struct intel_crtc_state *new_crtc_state;
+	struct intel_display *display = to_intel_display(crtc);
 	struct intel_vblank_evade_ctx evade;
 	int ret;
 
@@ -910,6 +912,9 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
 		intel_plane_disable_arm(plane, crtc_state);
 	}
 
+	if (crtc_state->has_psr)
+		intel_vrr_psr_send_push(display, crtc_state->cpu_transcoder);
+
 	local_irq_enable();
 
 	intel_psr_unlock(crtc_state);
-- 
2.34.1

✗ CI.Patch_applied: failure for Use trans push mechanism to generate frame change event

[Patchwork]

== Series Details ==

Series: Use trans push mechanism to generate frame change event
URL   : https://patchwork.freedesktop.org/series/139831/
State : failure

== Summary ==

=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: e1aba2bf4f79 drm-tip: 2024y-10m-09d-20h-21m-55s UTC integration manifest
=== git am output follows ===
error: patch failed: drivers/gpu/drm/i915/display/intel_cursor.c:910
error: drivers/gpu/drm/i915/display/intel_cursor.c: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: drm/i915/psr: Add TRANS_PUSH register bit definition for PSR
Applying: drm/i915/vrr: Do not overwrite TRANS_PUSH PSR Frame Change Enable
Applying: drm/i915/vrr: Use TRANS_PUSH mechanism for PSR frame change
Applying: drm/i915/psr: Rename psr_force_hw_tracking_exit as psr_force_exit
Applying: drm/i915/psr: Simplify frontbuffer invalidate/flush callbacks
Applying: drm/i915/psr: Add VRR send push interface for PSR usage
Applying: drm/i915/display: Generate PSR frame change event on cursor update
Patch failed at 0007 drm/i915/display: Generate PSR frame change event on cursor update
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".