Re: [Intel-gfx] [PATCH 1/4] drm/i915/display/tgl+: Dispatch atomic commits instead of front buffer modifications

[Daniel Vetter <daniel@ffwll.ch>]

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);
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch