Re: [Intel-gfx] [PATCH 7/8] drm/i915/display/skl+: Drop frontbuffer rendering support
On Wed, Aug 25, 2021 at 03:47:12PM +0300, Ville Syrjälä wrote: > On Wed, Aug 25, 2021 at 12:49:25AM +, Souza, Jose wrote: > > On Thu, 2021-08-19 at 19:07 +0300, Ville Syrjälä wrote: > > > On Wed, Aug 18, 2021 at 07:48:03PM +, Souza, Jose wrote: > > > > On Wed, 2021-08-18 at 17:55 +0300, Ville Syrjälä wrote: > > > > > On Tue, Aug 17, 2021 at 05:42:15PM -0700, José Roberto de Souza wrote: > > > > > > By now all the userspace applications should have migrated to atomic > > > > > > or at least be calling DRM_IOCTL_MODE_DIRTYFB. > > > > > > > > > > > > With that we can kill frontbuffer rendering support in i915 for > > > > > > modern platforms. > > > > > > > > > > > > So here converting legacy APIs into atomic commits so it can be > > > > > > properly handled by driver i915. > > > > > > > > > > > > Several IGT tests will fail with this changes, because some tests > > > > > > were stressing those frontbuffer rendering scenarios that no > > > > > > userspace > > > > > > should be using by now, fixes to IGT should be sent soon. > > > > > > > > > > Blocking atomic commits instead of the current lightweight frontbuffer > > > > > interface sounds like a terrible plan. How unusable is X with this > > > > > approach? > > > > > > > > 100% usable, had no issues when running X in TGL and ADL-P. > > > > Added a debug message in intel_user_framebuffer_dirty() and X is not > > > > even using frontbuffer rendering at all. > > > > > > Turn off your compositor if you want to test front buffer rendering. > > > > Worked fine on Plasma with a 4K panel, was not able to find how to do that > > in Gnome. > > I didn't think you can turn off composition with either one of those. > You actually confirmed it's running with everytithing unredirected and > eg. there was no lag moving windows around and wiggling the mouse? > > Avoiding that lag is pretty much the sole reason why the legacy > cursor unsynced update stuff even exists in the driver. Hard to > imagine you wouldn't hit the same issue with the server getting > blocked on dirtyfb all the time. Oh and running x11perf/etc. to see the impact on the raw numbers would probably be good idea. -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 7/8] drm/i915/display/skl+: Drop frontbuffer rendering support
On Wed, Aug 25, 2021 at 12:49:25AM +, Souza, Jose wrote: > On Thu, 2021-08-19 at 19:07 +0300, Ville Syrjälä wrote: > > On Wed, Aug 18, 2021 at 07:48:03PM +, Souza, Jose wrote: > > > On Wed, 2021-08-18 at 17:55 +0300, Ville Syrjälä wrote: > > > > On Tue, Aug 17, 2021 at 05:42:15PM -0700, José Roberto de Souza wrote: > > > > > By now all the userspace applications should have migrated to atomic > > > > > or at least be calling DRM_IOCTL_MODE_DIRTYFB. > > > > > > > > > > With that we can kill frontbuffer rendering support in i915 for > > > > > modern platforms. > > > > > > > > > > So here converting legacy APIs into atomic commits so it can be > > > > > properly handled by driver i915. > > > > > > > > > > Several IGT tests will fail with this changes, because some tests > > > > > were stressing those frontbuffer rendering scenarios that no userspace > > > > > should be using by now, fixes to IGT should be sent soon. > > > > > > > > Blocking atomic commits instead of the current lightweight frontbuffer > > > > interface sounds like a terrible plan. How unusable is X with this > > > > approach? > > > > > > 100% usable, had no issues when running X in TGL and ADL-P. > > > Added a debug message in intel_user_framebuffer_dirty() and X is not even > > > using frontbuffer rendering at all. > > > > Turn off your compositor if you want to test front buffer rendering. > > Worked fine on Plasma with a 4K panel, was not able to find how to do that in > Gnome. I didn't think you can turn off composition with either one of those. You actually confirmed it's running with everytithing unredirected and eg. there was no lag moving windows around and wiggling the mouse? Avoiding that lag is pretty much the sole reason why the legacy cursor unsynced update stuff even exists in the driver. Hard to imagine you wouldn't hit the same issue with the server getting blocked on dirtyfb all the time. -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 7/8] drm/i915/display/skl+: Drop frontbuffer rendering support
On Thu, 2021-08-19 at 19:07 +0300, Ville Syrjälä wrote: > On Wed, Aug 18, 2021 at 07:48:03PM +, Souza, Jose wrote: > > On Wed, 2021-08-18 at 17:55 +0300, Ville Syrjälä wrote: > > > On Tue, Aug 17, 2021 at 05:42:15PM -0700, José Roberto de Souza wrote: > > > > By now all the userspace applications should have migrated to atomic > > > > or at least be calling DRM_IOCTL_MODE_DIRTYFB. > > > > > > > > With that we can kill frontbuffer rendering support in i915 for > > > > modern platforms. > > > > > > > > So here converting legacy APIs into atomic commits so it can be > > > > properly handled by driver i915. > > > > > > > > Several IGT tests will fail with this changes, because some tests > > > > were stressing those frontbuffer rendering scenarios that no userspace > > > > should be using by now, fixes to IGT should be sent soon. > > > > > > Blocking atomic commits instead of the current lightweight frontbuffer > > > interface sounds like a terrible plan. How unusable is X with this > > > approach? > > > > 100% usable, had no issues when running X in TGL and ADL-P. > > Added a debug message in intel_user_framebuffer_dirty() and X is not even > > using frontbuffer rendering at all. > > Turn off your compositor if you want to test front buffer rendering. Worked fine on Plasma with a 4K panel, was not able to find how to do that in Gnome. >
Re: [Intel-gfx] [PATCH 7/8] drm/i915/display/skl+: Drop frontbuffer rendering support
On Wed, Aug 18, 2021 at 2:37 AM José Roberto de Souza
wrote:
>
> By now all the userspace applications should have migrated to atomic
> or at least be calling DRM_IOCTL_MODE_DIRTYFB.
>
> With that we can kill frontbuffer rendering support in i915 for
> modern platforms.
>
> So here converting legacy APIs into atomic commits so it can be
> properly handled by driver i915.
>
> Several IGT tests will fail with this changes, because some tests
> were stressing those frontbuffer rendering scenarios that no userspace
> should be using by now, fixes to IGT should be sent soon.
>
> Cc: Daniel Vetter
> Cc: Gwan-gyeong Mun
> Cc: Ville Syrjälä
> Cc: Jani Nikula
> Cc: Rodrigo Vivi
> Signed-off-by: José Roberto de Souza
Patch looks good in overall direction, but there's more
intel_frontbuffer.c functions to disable, none of the tracking of the
tracking bits should ever be set, maybe even throw some WARN_ON in the
code.
-Daniel
> ---
> drivers/gpu/drm/i915/display/intel_cursor.c | 6 ++
> drivers/gpu/drm/i915/display/intel_display.c | 7 ++-
> drivers/gpu/drm/i915/display/intel_frontbuffer.c | 6 ++
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> 4 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c
> b/drivers/gpu/drm/i915/display/intel_cursor.c
> index c7618fef01439..5aa996c3b7980 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -617,6 +617,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
>u32 src_w, u32 src_h,
>struct drm_modeset_acquire_ctx *ctx)
> {
> + struct drm_i915_private *i915 = to_i915(_crtc->dev);
> struct intel_plane *plane = to_intel_plane(_plane);
> struct intel_crtc *crtc = to_intel_crtc(_crtc);
> struct intel_plane_state *old_plane_state =
> @@ -633,12 +634,9 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
> * PSR2 selective fetch also requires the slow path as
> * PSR2 plane and transcoder registers can only be updated during
> * vblank.
> -*
> -* FIXME bigjoiner fastpath would be good
> */
> if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) ||
> - crtc_state->update_pipe || crtc_state->bigjoiner ||
> - crtc_state->enable_psr2_sel_fetch)
> + crtc_state->update_pipe || !HAS_FRONTBUFFER_RENDERING(i915))
> goto slow;
>
> /*
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index e55c9e2cb254a..f700544454ad5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -11744,10 +11744,15 @@ static int intel_user_framebuffer_dirty(struct
> drm_framebuffer *fb,
> unsigned num_clips)
> {
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> + struct drm_i915_private *i915 = to_i915(obj->base.dev);
>
> i915_gem_object_flush_if_display(obj);
> - intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
>
> + if (!HAS_FRONTBUFFER_RENDERING(i915))
> + return drm_atomic_helper_dirtyfb(fb, file, flags, color,
> clips,
> +num_clips);
> +
> + intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> index e4834d84ce5e3..6be2f767a203c 100644
> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> @@ -91,6 +91,9 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
>
> trace_intel_frontbuffer_flush(frontbuffer_bits, origin);
>
> + if (!HAS_FRONTBUFFER_RENDERING(i915))
> + return;
> +
> might_sleep();
> intel_edp_drrs_flush(i915, frontbuffer_bits);
> intel_psr_flush(i915, frontbuffer_bits, origin);
> @@ -179,6 +182,9 @@ void __intel_fb_invalidate(struct intel_frontbuffer
> *front,
>
> trace_intel_frontbuffer_invalidate(frontbuffer_bits, origin);
>
> + if (!HAS_FRONTBUFFER_RENDERING(i915))
> + return;
> +
> might_sleep();
> intel_psr_invalidate(i915, frontbuffer_bits, origin);
> intel_edp_drrs_invalidate(i915, frontbuffer_bits);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1ea27c4e94a6d..fe1dc8b7871a0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1719,6 +1719,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>
> #define HAS_VRR(i915) (GRAPHICS_VER(i915) >= 12)
>
> +#define HAS_FRONTBUFFER_RENDERING(i915)(GRAPHICS_VER(i915)
Re: [Intel-gfx] [PATCH 7/8] drm/i915/display/skl+: Drop frontbuffer rendering support
On Wed, Aug 18, 2021 at 07:48:03PM +, Souza, Jose wrote: > On Wed, 2021-08-18 at 17:55 +0300, Ville Syrjälä wrote: > > On Tue, Aug 17, 2021 at 05:42:15PM -0700, José Roberto de Souza wrote: > > > By now all the userspace applications should have migrated to atomic > > > or at least be calling DRM_IOCTL_MODE_DIRTYFB. > > > > > > With that we can kill frontbuffer rendering support in i915 for > > > modern platforms. > > > > > > So here converting legacy APIs into atomic commits so it can be > > > properly handled by driver i915. > > > > > > Several IGT tests will fail with this changes, because some tests > > > were stressing those frontbuffer rendering scenarios that no userspace > > > should be using by now, fixes to IGT should be sent soon. > > > > Blocking atomic commits instead of the current lightweight frontbuffer > > interface sounds like a terrible plan. How unusable is X with this > > approach? > > 100% usable, had no issues when running X in TGL and ADL-P. > Added a debug message in intel_user_framebuffer_dirty() and X is not even > using frontbuffer rendering at all. Turn off your compositor if you want to test front buffer rendering. -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 7/8] drm/i915/display/skl+: Drop frontbuffer rendering support
On Wed, 2021-08-18 at 17:55 +0300, Ville Syrjälä wrote: > On Tue, Aug 17, 2021 at 05:42:15PM -0700, José Roberto de Souza wrote: > > By now all the userspace applications should have migrated to atomic > > or at least be calling DRM_IOCTL_MODE_DIRTYFB. > > > > With that we can kill frontbuffer rendering support in i915 for > > modern platforms. > > > > So here converting legacy APIs into atomic commits so it can be > > properly handled by driver i915. > > > > Several IGT tests will fail with this changes, because some tests > > were stressing those frontbuffer rendering scenarios that no userspace > > should be using by now, fixes to IGT should be sent soon. > > Blocking atomic commits instead of the current lightweight frontbuffer > interface sounds like a terrible plan. How unusable is X with this > approach? 100% usable, had no issues when running X in TGL and ADL-P. Added a debug message in intel_user_framebuffer_dirty() and X is not even using frontbuffer rendering at all. >
Re: [Intel-gfx] [PATCH 7/8] drm/i915/display/skl+: Drop frontbuffer rendering support
On Tue, Aug 17, 2021 at 05:42:15PM -0700, José Roberto de Souza wrote: > By now all the userspace applications should have migrated to atomic > or at least be calling DRM_IOCTL_MODE_DIRTYFB. > > With that we can kill frontbuffer rendering support in i915 for > modern platforms. > > So here converting legacy APIs into atomic commits so it can be > properly handled by driver i915. > > Several IGT tests will fail with this changes, because some tests > were stressing those frontbuffer rendering scenarios that no userspace > should be using by now, fixes to IGT should be sent soon. Blocking atomic commits instead of the current lightweight frontbuffer interface sounds like a terrible plan. How unusable is X with this approach? -- Ville Syrjälä Intel
[Intel-gfx] [PATCH 7/8] drm/i915/display/skl+: Drop frontbuffer rendering support
By now all the userspace applications should have migrated to atomic
or at least be calling DRM_IOCTL_MODE_DIRTYFB.
With that we can kill frontbuffer rendering support in i915 for
modern platforms.
So here converting legacy APIs into atomic commits so it can be
properly handled by driver i915.
Several IGT tests will fail with this changes, because some tests
were stressing those frontbuffer rendering scenarios that no userspace
should be using by now, fixes to IGT should be sent soon.
Cc: Daniel Vetter
Cc: Gwan-gyeong Mun
Cc: Ville Syrjälä
Cc: Jani Nikula
Cc: Rodrigo Vivi
Signed-off-by: José Roberto de Souza
---
drivers/gpu/drm/i915/display/intel_cursor.c | 6 ++
drivers/gpu/drm/i915/display/intel_display.c | 7 ++-
drivers/gpu/drm/i915/display/intel_frontbuffer.c | 6 ++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
4 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c
b/drivers/gpu/drm/i915/display/intel_cursor.c
index c7618fef01439..5aa996c3b7980 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -617,6 +617,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
u32 src_w, u32 src_h,
struct drm_modeset_acquire_ctx *ctx)
{
+ struct drm_i915_private *i915 = to_i915(_crtc->dev);
struct intel_plane *plane = to_intel_plane(_plane);
struct intel_crtc *crtc = to_intel_crtc(_crtc);
struct intel_plane_state *old_plane_state =
@@ -633,12 +634,9 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
* PSR2 selective fetch also requires the slow path as
* PSR2 plane and transcoder registers can only be updated during
* vblank.
-*
-* FIXME bigjoiner fastpath would be good
*/
if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) ||
- crtc_state->update_pipe || crtc_state->bigjoiner ||
- crtc_state->enable_psr2_sel_fetch)
+ crtc_state->update_pipe || !HAS_FRONTBUFFER_RENDERING(i915))
goto slow;
/*
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index e55c9e2cb254a..f700544454ad5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -11744,10 +11744,15 @@ static int intel_user_framebuffer_dirty(struct
drm_framebuffer *fb,
unsigned num_clips)
{
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct drm_i915_private *i915 = to_i915(obj->base.dev);
i915_gem_object_flush_if_display(obj);
- intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
+ if (!HAS_FRONTBUFFER_RENDERING(i915))
+ return drm_atomic_helper_dirtyfb(fb, file, flags, color, clips,
+num_clips);
+
+ intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
return 0;
}
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
index e4834d84ce5e3..6be2f767a203c 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
@@ -91,6 +91,9 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
trace_intel_frontbuffer_flush(frontbuffer_bits, origin);
+ if (!HAS_FRONTBUFFER_RENDERING(i915))
+ return;
+
might_sleep();
intel_edp_drrs_flush(i915, frontbuffer_bits);
intel_psr_flush(i915, frontbuffer_bits, origin);
@@ -179,6 +182,9 @@ void __intel_fb_invalidate(struct intel_frontbuffer *front,
trace_intel_frontbuffer_invalidate(frontbuffer_bits, origin);
+ if (!HAS_FRONTBUFFER_RENDERING(i915))
+ return;
+
might_sleep();
intel_psr_invalidate(i915, frontbuffer_bits, origin);
intel_edp_drrs_invalidate(i915, frontbuffer_bits);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1ea27c4e94a6d..fe1dc8b7871a0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1719,6 +1719,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_VRR(i915) (GRAPHICS_VER(i915) >= 12)
+#define HAS_FRONTBUFFER_RENDERING(i915)(GRAPHICS_VER(i915) < 9)
+
/* Only valid when HAS_DISPLAY() is true */
#define INTEL_DISPLAY_ENABLED(dev_priv) \
(drm_WARN_ON(&(dev_priv)->drm, !HAS_DISPLAY(dev_priv)),
!(dev_priv)->params.disable_display)
--
2.32.0
