diff mbox series

drm/xe/display: Add intel_plane_initial_vblank_wait

Message ID 20241107113617.349916-1-maarten.lankhorst@linux.intel.com (mailing list archive)
State New
Headers show
Series drm/xe/display: Add intel_plane_initial_vblank_wait | expand

Commit Message

Maarten Lankhorst Nov. 7, 2024, 11:36 a.m. UTC 
We're changing the driver to have no interrupts during early init for
Xe, so we poll the PIPE_FRMSTMSMP counter instead.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241105121857.17389-2-maarten.lankhorst@linux.intel.com
Signed-off-by: Maarten Lankhorst,,, <dev@lankhorst.se>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  6 +++---
 .../drm/i915/display/intel_plane_initial.c    |  9 ++++++++-
 .../drm/i915/display/intel_plane_initial.h    |  2 ++
 drivers/gpu/drm/xe/display/xe_plane_initial.c | 19 ++++++++++++++++++-
 4 files changed, 31 insertions(+), 5 deletions(-)

Comments

Ville Syrjala Nov. 7, 2024, 12:21 p.m. UTC | #1 
On Thu, Nov 07, 2024 at 12:36:17PM +0100, Maarten Lankhorst wrote:
> We're changing the driver to have no interrupts during early init for
> Xe, so we poll the PIPE_FRMSTMSMP counter instead.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20241105121857.17389-2-maarten.lankhorst@linux.intel.com
> Signed-off-by: Maarten Lankhorst,,, <dev@lankhorst.se>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  6 +++---
>  .../drm/i915/display/intel_plane_initial.c    |  9 ++++++++-
>  .../drm/i915/display/intel_plane_initial.h    |  2 ++
>  drivers/gpu/drm/xe/display/xe_plane_initial.c | 19 ++++++++++++++++++-
>  4 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index d7f92dc56d1e9..7bd6da2bb36f7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -795,7 +795,7 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc,
>  	if ((crtc_state->active_planes & ~BIT(PLANE_CURSOR)) == 0 &&
>  	    hsw_ips_disable(crtc_state)) {
>  		crtc_state->ips_enabled = false;
> -		intel_crtc_wait_for_next_vblank(crtc);
> +		intel_plane_initial_vblank_wait(plane);
>  	}
>  
>  	/*
> @@ -809,7 +809,7 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc,
>  	 */
>  	if (HAS_GMCH(dev_priv) &&
>  	    intel_set_memory_cxsr(dev_priv, false))
> -		intel_crtc_wait_for_next_vblank(crtc);
> +		intel_plane_initial_vblank_wait(plane);
>  
>  	/*
>  	 * Gen2 reports pipe underruns whenever all planes are disabled.
> @@ -819,7 +819,7 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc,
>  		intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false);
>  
>  	intel_plane_disable_arm(NULL, plane, crtc_state);
> -	intel_crtc_wait_for_next_vblank(crtc);
> +	intel_plane_initial_vblank_wait(plane);
>  }
>  
>  unsigned int
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> index 62401f6a04e4f..77eb2b763be5e 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> @@ -14,6 +14,13 @@
>  #include "intel_frontbuffer.h"
>  #include "intel_plane_initial.h"
>  
> +void intel_plane_initial_vblank_wait(struct intel_plane *plane)
> +{
> +	struct intel_crtc *crtc = intel_crtc_for_pipe(to_intel_display(plane->base.dev), plane->pipe);
> +
> +	intel_crtc_wait_for_next_vblank(crtc);
> +}
> +
>  static bool
>  intel_reuse_initial_plane_obj(struct intel_crtc *this,
>  			      const struct intel_initial_plane_config plane_configs[],
> @@ -438,7 +445,7 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
>  		intel_find_initial_plane_obj(crtc, plane_configs);
>  
>  		if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
> -			intel_crtc_wait_for_next_vblank(crtc);
> +			intel_plane_initial_vblank_wait(to_intel_plane(crtc->base.primary));
>  
>  		plane_config_fini(plane_config);
>  	}
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.h b/drivers/gpu/drm/i915/display/intel_plane_initial.h
> index 64ab95239cd45..149b1bf1bc1f8 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.h
> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.h
> @@ -7,7 +7,9 @@
>  #define __INTEL_PLANE_INITIAL_H__
>  
>  struct drm_i915_private;
> +struct intel_plane;
>  
> +void intel_plane_initial_vblank_wait(struct intel_plane *plane);
>  void intel_initial_plane_config(struct drm_i915_private *i915);
>  
>  #endif
> diff --git a/drivers/gpu/drm/xe/display/xe_plane_initial.c b/drivers/gpu/drm/xe/display/xe_plane_initial.c
> index 668f544a7ac80..9252cfeef0a1f 100644
> --- a/drivers/gpu/drm/xe/display/xe_plane_initial.c
> +++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c
> @@ -8,7 +8,9 @@
>  
>  #include "regs/xe_gtt_defs.h"
>  #include "xe_ggtt.h"
> +#include "xe_mmio.h"
>  
> +#include "i915_reg.h"
>  #include "intel_atomic_plane.h"
>  #include "intel_crtc.h"
>  #include "intel_display.h"
> @@ -22,6 +24,21 @@
>  
>  #include <generated/xe_wa_oob.h>
>  
> +void intel_plane_initial_vblank_wait(struct intel_plane *plane)
> +{
> +	/* Early xe has no irq */
> +	struct xe_device *xe = to_xe_device(plane->base.dev);
> +	struct xe_reg pipe_frmtmstmp = XE_REG(i915_mmio_reg_offset(PIPE_FRMTMSTMP(plane->pipe)));
> +	u32 timestamp;
> +	int ret;
> +
> +	timestamp = xe_mmio_read32(xe_root_tile_mmio(xe), pipe_frmtmstmp);
> +
> +	ret = xe_mmio_wait32_not(xe_root_tile_mmio(xe), pipe_frmtmstmp, ~0U, timestamp, 40000U, &timestamp, false);
> +	if (ret < 0)
> +		drm_warn(&xe->drm, "waiting for early vblank failed with %i\n", ret);

Why did you change to this from the earlier SURFLIVE approach?

> +}
> +
>  static bool
>  intel_reuse_initial_plane_obj(struct intel_crtc *this,
>  			      const struct intel_initial_plane_config plane_configs[],
> @@ -293,7 +310,7 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
>  		intel_find_initial_plane_obj(crtc, plane_configs);
>  
>  		if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
> -			intel_crtc_wait_for_next_vblank(crtc);
> +			intel_plane_initial_vblank_wait(to_intel_plane(crtc->base.primary));
>  
>  		plane_config_fini(plane_config);
>  	}
> -- 
> 2.45.2
Maarten Lankhorst Nov. 7, 2024, 5:45 p.m. UTC | #2 
Hey,

Den 2024-11-07 kl. 13:21, skrev Ville Syrjälä:
> On Thu, Nov 07, 2024 at 12:36:17PM +0100, Maarten Lankhorst wrote:
>> We're changing the driver to have no interrupts during early init for
>> Xe, so we poll the PIPE_FRMSTMSMP counter instead.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Link: https://patchwork.freedesktop.org/patch/msgid/20241105121857.17389-2-maarten.lankhorst@linux.intel.com
>> Signed-off-by: Maarten Lankhorst,,, <dev@lankhorst.se>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display.c  |  6 +++---
>>  .../drm/i915/display/intel_plane_initial.c    |  9 ++++++++-
>>  .../drm/i915/display/intel_plane_initial.h    |  2 ++
>>  drivers/gpu/drm/xe/display/xe_plane_initial.c | 19 ++++++++++++++++++-
>>  4 files changed, 31 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index d7f92dc56d1e9..7bd6da2bb36f7 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -795,7 +795,7 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc,
>>  	if ((crtc_state->active_planes & ~BIT(PLANE_CURSOR)) == 0 &&
>>  	    hsw_ips_disable(crtc_state)) {
>>  		crtc_state->ips_enabled = false;
>> -		intel_crtc_wait_for_next_vblank(crtc);
>> +		intel_plane_initial_vblank_wait(plane);
>>  	}
>>  
>>  	/*
>> @@ -809,7 +809,7 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc,
>>  	 */
>>  	if (HAS_GMCH(dev_priv) &&
>>  	    intel_set_memory_cxsr(dev_priv, false))
>> -		intel_crtc_wait_for_next_vblank(crtc);
>> +		intel_plane_initial_vblank_wait(plane);
>>  
>>  	/*
>>  	 * Gen2 reports pipe underruns whenever all planes are disabled.
>> @@ -819,7 +819,7 @@ void intel_plane_disable_noatomic(struct intel_crtc *crtc,
>>  		intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false);
>>  
>>  	intel_plane_disable_arm(NULL, plane, crtc_state);
>> -	intel_crtc_wait_for_next_vblank(crtc);
>> +	intel_plane_initial_vblank_wait(plane);
>>  }
>>  
>>  unsigned int
>> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
>> index 62401f6a04e4f..77eb2b763be5e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
>> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
>> @@ -14,6 +14,13 @@
>>  #include "intel_frontbuffer.h"
>>  #include "intel_plane_initial.h"
>>  
>> +void intel_plane_initial_vblank_wait(struct intel_plane *plane)
>> +{
>> +	struct intel_crtc *crtc = intel_crtc_for_pipe(to_intel_display(plane->base.dev), plane->pipe);
>> +
>> +	intel_crtc_wait_for_next_vblank(crtc);
>> +}
>> +
>>  static bool
>>  intel_reuse_initial_plane_obj(struct intel_crtc *this,
>>  			      const struct intel_initial_plane_config plane_configs[],
>> @@ -438,7 +445,7 @@ void intel_initial_plane_config(struct drm_i915_private *i915)
>>  		intel_find_initial_plane_obj(crtc, plane_configs);
>>  
>>  		if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
>> -			intel_crtc_wait_for_next_vblank(crtc);
>> +			intel_plane_initial_vblank_wait(to_intel_plane(crtc->base.primary));
>>  
>>  		plane_config_fini(plane_config);
>>  	}
>> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.h b/drivers/gpu/drm/i915/display/intel_plane_initial.h
>> index 64ab95239cd45..149b1bf1bc1f8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.h
>> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.h
>> @@ -7,7 +7,9 @@
>>  #define __INTEL_PLANE_INITIAL_H__
>>  
>>  struct drm_i915_private;
>> +struct intel_plane;
>>  
>> +void intel_plane_initial_vblank_wait(struct intel_plane *plane);
>>  void intel_initial_plane_config(struct drm_i915_private *i915);
>>  
>>  #endif
>> diff --git a/drivers/gpu/drm/xe/display/xe_plane_initial.c b/drivers/gpu/drm/xe/display/xe_plane_initial.c
>> index 668f544a7ac80..9252cfeef0a1f 100644
>> --- a/drivers/gpu/drm/xe/display/xe_plane_initial.c
>> +++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c
>> @@ -8,7 +8,9 @@
>>  
>>  #include "regs/xe_gtt_defs.h"
>>  #include "xe_ggtt.h"
>> +#include "xe_mmio.h"
>>  
>> +#include "i915_reg.h"
>>  #include "intel_atomic_plane.h"
>>  #include "intel_crtc.h"
>>  #include "intel_display.h"
>> @@ -22,6 +24,21 @@
>>  
>>  #include <generated/xe_wa_oob.h>
>>  
>> +void intel_plane_initial_vblank_wait(struct intel_plane *plane)
>> +{
>> +	/* Early xe has no irq */
>> +	struct xe_device *xe = to_xe_device(plane->base.dev);
>> +	struct xe_reg pipe_frmtmstmp = XE_REG(i915_mmio_reg_offset(PIPE_FRMTMSTMP(plane->pipe)));
>> +	u32 timestamp;
>> +	int ret;
>> +
>> +	timestamp = xe_mmio_read32(xe_root_tile_mmio(xe), pipe_frmtmstmp);
>> +
>> +	ret = xe_mmio_wait32_not(xe_root_tile_mmio(xe), pipe_frmtmstmp, ~0U, timestamp, 40000U, &timestamp, false);
>> +	if (ret < 0)
>> +		drm_warn(&xe->drm, "waiting for early vblank failed with %i\n", ret);
> 
> Why did you change to this from the earlier SURFLIVE approach?This works correctly when calling disable_plane too. When I tested on lunarlake, this gave me an issue.
When failing to construct initial fb, intel_plane_disable_noatomic() is called,
and then calls drm_crtc_wait_one_vblank if not fixed like above.

SURFLIVE works great for the specific case of flipping, not that great for disabling. :)

Cheers,
~Maarten
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index d7f92dc56d1e9..7bd6da2bb36f7 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -795,7 +795,7 @@  void intel_plane_disable_noatomic(struct intel_crtc *crtc,
 	if ((crtc_state->active_planes & ~BIT(PLANE_CURSOR)) == 0 &&
 	    hsw_ips_disable(crtc_state)) {
 		crtc_state->ips_enabled = false;
-		intel_crtc_wait_for_next_vblank(crtc);
+		intel_plane_initial_vblank_wait(plane);
 	}
 
 	/*
@@ -809,7 +809,7 @@  void intel_plane_disable_noatomic(struct intel_crtc *crtc,
 	 */
 	if (HAS_GMCH(dev_priv) &&
 	    intel_set_memory_cxsr(dev_priv, false))
-		intel_crtc_wait_for_next_vblank(crtc);
+		intel_plane_initial_vblank_wait(plane);
 
 	/*
 	 * Gen2 reports pipe underruns whenever all planes are disabled.
@@ -819,7 +819,7 @@  void intel_plane_disable_noatomic(struct intel_crtc *crtc,
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false);
 
 	intel_plane_disable_arm(NULL, plane, crtc_state);
-	intel_crtc_wait_for_next_vblank(crtc);
+	intel_plane_initial_vblank_wait(plane);
 }
 
 unsigned int
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
index 62401f6a04e4f..77eb2b763be5e 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
@@ -14,6 +14,13 @@ 
 #include "intel_frontbuffer.h"
 #include "intel_plane_initial.h"
 
+void intel_plane_initial_vblank_wait(struct intel_plane *plane)
+{
+	struct intel_crtc *crtc = intel_crtc_for_pipe(to_intel_display(plane->base.dev), plane->pipe);
+
+	intel_crtc_wait_for_next_vblank(crtc);
+}
+
 static bool
 intel_reuse_initial_plane_obj(struct intel_crtc *this,
 			      const struct intel_initial_plane_config plane_configs[],
@@ -438,7 +445,7 @@  void intel_initial_plane_config(struct drm_i915_private *i915)
 		intel_find_initial_plane_obj(crtc, plane_configs);
 
 		if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
-			intel_crtc_wait_for_next_vblank(crtc);
+			intel_plane_initial_vblank_wait(to_intel_plane(crtc->base.primary));
 
 		plane_config_fini(plane_config);
 	}
diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.h b/drivers/gpu/drm/i915/display/intel_plane_initial.h
index 64ab95239cd45..149b1bf1bc1f8 100644
--- a/drivers/gpu/drm/i915/display/intel_plane_initial.h
+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.h
@@ -7,7 +7,9 @@ 
 #define __INTEL_PLANE_INITIAL_H__
 
 struct drm_i915_private;
+struct intel_plane;
 
+void intel_plane_initial_vblank_wait(struct intel_plane *plane);
 void intel_initial_plane_config(struct drm_i915_private *i915);
 
 #endif
diff --git a/drivers/gpu/drm/xe/display/xe_plane_initial.c b/drivers/gpu/drm/xe/display/xe_plane_initial.c
index 668f544a7ac80..9252cfeef0a1f 100644
--- a/drivers/gpu/drm/xe/display/xe_plane_initial.c
+++ b/drivers/gpu/drm/xe/display/xe_plane_initial.c
@@ -8,7 +8,9 @@ 
 
 #include "regs/xe_gtt_defs.h"
 #include "xe_ggtt.h"
+#include "xe_mmio.h"
 
+#include "i915_reg.h"
 #include "intel_atomic_plane.h"
 #include "intel_crtc.h"
 #include "intel_display.h"
@@ -22,6 +24,21 @@ 
 
 #include <generated/xe_wa_oob.h>
 
+void intel_plane_initial_vblank_wait(struct intel_plane *plane)
+{
+	/* Early xe has no irq */
+	struct xe_device *xe = to_xe_device(plane->base.dev);
+	struct xe_reg pipe_frmtmstmp = XE_REG(i915_mmio_reg_offset(PIPE_FRMTMSTMP(plane->pipe)));
+	u32 timestamp;
+	int ret;
+
+	timestamp = xe_mmio_read32(xe_root_tile_mmio(xe), pipe_frmtmstmp);
+
+	ret = xe_mmio_wait32_not(xe_root_tile_mmio(xe), pipe_frmtmstmp, ~0U, timestamp, 40000U, &timestamp, false);
+	if (ret < 0)
+		drm_warn(&xe->drm, "waiting for early vblank failed with %i\n", ret);
+}
+
 static bool
 intel_reuse_initial_plane_obj(struct intel_crtc *this,
 			      const struct intel_initial_plane_config plane_configs[],
@@ -293,7 +310,7 @@  void intel_initial_plane_config(struct drm_i915_private *i915)
 		intel_find_initial_plane_obj(crtc, plane_configs);
 
 		if (i915->display.funcs.display->fixup_initial_plane_config(crtc, plane_config))
-			intel_crtc_wait_for_next_vblank(crtc);
+			intel_plane_initial_vblank_wait(to_intel_plane(crtc->base.primary));
 
 		plane_config_fini(plane_config);
 	}