| Message ID | 20210731001019.150373-1-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications | expand |
On Fri, 2021-07-30 at 17:10 -0700, José Roberto de Souza wrote: > PSR2 selective fetch requires plane and transcoder registers to > be programed during the vblank to properly update the display and > there is no way around it. > > We could disable PSR2 at every notification of dirty front buffer from > user space but that would hurt the power savings and it would still > cause some race conditions between PSR2 exit sequence and atomic > commits that causes underruns and glitches. > > So from display 12 and newer we will start to do atomic commits > every time user space notify that front buffer is dirty and ignore > all frontbuffer flushes and invalidates on the PSR side. Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> > > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_cursor.c | 3 ++- > drivers/gpu/drm/i915/display/intel_display.c | 7 ++++++- > drivers/gpu/drm/i915/display/intel_psr.c | 6 ++++++ > 3 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c > index c7618fef01439..d44022cb46a65 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 = > @@ -638,7 +639,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane, > */ > if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) || > crtc_state->update_pipe || crtc_state->bigjoiner || > - crtc_state->enable_psr2_sel_fetch) > + DISPLAY_VER(i915) >= 12) > goto slow; > > /* > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 5ff0a011b28eb..4a936e1e7fa82 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -11720,10 +11720,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 (DISPLAY_VER(i915) >= 12) > + 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_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index 1b0daf649e823..caf92f414a6e7 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -2039,6 +2039,9 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv, > { > struct intel_encoder *encoder; > > + if (DISPLAY_VER(dev_priv) >= 12) > + return; > + > if (origin == ORIGIN_FLIP) > return; > > @@ -2123,6 +2126,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv, > continue; > } > > + if (DISPLAY_VER(dev_priv) >= 12) > + continue; > + > mutex_lock(&intel_dp->psr.lock); > if (!intel_dp->psr.enabled) { > mutex_unlock(&intel_dp->psr.lock);
On Thu, Aug 12, 2021 at 7:24 PM Souza, Jose <jose.souza@intel.com> wrote: > > On Fri, 2021-07-30 at 17:10 -0700, José Roberto de Souza wrote: > > PSR2 selective fetch requires plane and transcoder registers to > > be programed during the vblank to properly update the display and > > there is no way around it. > > > > We could disable PSR2 at every notification of dirty front buffer from > > user space but that would hurt the power savings and it would still > > cause some race conditions between PSR2 exit sequence and atomic > > commits that causes underruns and glitches. > > > > So from display 12 and newer we will start to do atomic commits > > every time user space notify that front buffer is dirty and ignore > > all frontbuffer flushes and invalidates on the PSR side. So I filed a JIRA about this a while ago, and: - This should be done on gen9 (or maybe gen10+), because we stopped having userspace that was busted. Would be good to check whether we can't push this down further even. - We need to disable the entire intel_frontbuffer.c interface/uapi for _all_ things. It's a horrendous uapi, and it needs to die. You're already touching intel_user_framebuffer_dirty, so the other pieces really should be done too. Maybe make a macro or function or something so that we have a consistent check for which platforms still have to support frontbuffer tracking. -Daniel > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_cursor.c | 3 ++- > > drivers/gpu/drm/i915/display/intel_display.c | 7 ++++++- > > drivers/gpu/drm/i915/display/intel_psr.c | 6 ++++++ > > 3 files changed, 14 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c > > index c7618fef01439..d44022cb46a65 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 = > > @@ -638,7 +639,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane, > > */ > > if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) || > > crtc_state->update_pipe || crtc_state->bigjoiner || > > - crtc_state->enable_psr2_sel_fetch) > > + DISPLAY_VER(i915) >= 12) > > goto slow; > > > > /* > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index 5ff0a011b28eb..4a936e1e7fa82 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -11720,10 +11720,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 (DISPLAY_VER(i915) >= 12) > > + 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_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > > index 1b0daf649e823..caf92f414a6e7 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -2039,6 +2039,9 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv, > > { > > struct intel_encoder *encoder; > > > > + if (DISPLAY_VER(dev_priv) >= 12) > > + return; > > + > > if (origin == ORIGIN_FLIP) > > return; > > > > @@ -2123,6 +2126,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv, > > continue; > > } > > > > + if (DISPLAY_VER(dev_priv) >= 12) > > + continue; > > + > > mutex_lock(&intel_dp->psr.lock); > > if (!intel_dp->psr.enabled) { > > mutex_unlock(&intel_dp->psr.lock); >
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c index c7618fef01439..d44022cb46a65 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 = @@ -638,7 +639,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane, */ if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) || crtc_state->update_pipe || crtc_state->bigjoiner || - crtc_state->enable_psr2_sel_fetch) + DISPLAY_VER(i915) >= 12) goto slow; /* diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 5ff0a011b28eb..4a936e1e7fa82 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -11720,10 +11720,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 (DISPLAY_VER(i915) >= 12) + 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_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 1b0daf649e823..caf92f414a6e7 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -2039,6 +2039,9 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv, { struct intel_encoder *encoder; + if (DISPLAY_VER(dev_priv) >= 12) + return; + if (origin == ORIGIN_FLIP) return; @@ -2123,6 +2126,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv, continue; } + if (DISPLAY_VER(dev_priv) >= 12) + continue; + mutex_lock(&intel_dp->psr.lock); if (!intel_dp->psr.enabled) { mutex_unlock(&intel_dp->psr.lock);
PSR2 selective fetch requires plane and transcoder registers to be programed during the vblank to properly update the display and there is no way around it. We could disable PSR2 at every notification of dirty front buffer from user space but that would hurt the power savings and it would still cause some race conditions between PSR2 exit sequence and atomic commits that causes underruns and glitches. So from display 12 and newer we will start to do atomic commits every time user space notify that front buffer is dirty and ignore all frontbuffer flushes and invalidates on the PSR side. Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_cursor.c | 3 ++- drivers/gpu/drm/i915/display/intel_display.c | 7 ++++++- drivers/gpu/drm/i915/display/intel_psr.c | 6 ++++++ 3 files changed, 14 insertions(+), 2 deletions(-)