| Message ID | 20210930001409.254817-3-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/9] drm/i915/display/psr: Handle plane and pipe restrictions at every page flip | expand |
On Wed, Sep 29, 2021 at 05:14:03PM -0700, José Roberto de Souza wrote: > This unnecessary flushes are hurting power-savings are it causes > features like PSR, FBC and DRRS to disable it self to handle > frontbuffer rendering, below some explanation of why each removed > call is not necessary. > > The flush in intel_prepare_plane_fb() is not required as framebuffer > will be flipped and power-saving features do the proper flip handling > in hardware. > > intel_find_initial_plane_obj() flush is not required because it is > only executed during driver load and at this point the power-saving > features are not even enabled. > > And the last one intelfb_create(), is also not required as at this > point the fbdev was just allocated, userspace will draw on > it what will trigger frontbuffer invalidates and flushes later on. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> Yeah, I think these are not right. We might have to think a bit more about page flips vs. frontbuffer tracking at some point. But at least FBC doesn't want these, and for drrs I think you added something to the flip path (also drrs is kinda semi borked anyway atm and needs some real fixing). So lgtm Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 3 --- > drivers/gpu/drm/i915/display/intel_fbdev.c | 2 -- > 2 files changed, 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 4ce80ca7751f0..9e9db1b0a907b 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -1711,8 +1711,6 @@ intel_find_initial_plane_obj(struct intel_crtc *crtc, > plane_state->uapi.crtc = &crtc->base; > intel_plane_copy_uapi_to_hw_state(plane_state, plane_state, crtc); > > - intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB); > - > atomic_or(plane->frontbuffer_bit, &to_intel_frontbuffer(fb)->bits); > } > > @@ -10755,7 +10753,6 @@ intel_prepare_plane_fb(struct drm_plane *_plane, > return ret; > > i915_gem_object_wait_priority(obj, 0, &attr); > - i915_gem_object_flush_frontbuffer(obj, ORIGIN_DIRTYFB); > > if (!new_plane_state->uapi.fence) { /* implicit fencing */ > struct dma_fence *fence; > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c > index 60d3ded270476..53484267b2a4a 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c > @@ -230,8 +230,6 @@ static int intelfb_create(struct drm_fb_helper *helper, > goto out_unlock; > } > > - intel_frontbuffer_flush(to_frontbuffer(ifbdev), ORIGIN_DIRTYFB); > - > info = drm_fb_helper_alloc_fbi(helper); > if (IS_ERR(info)) { > drm_err(&dev_priv->drm, "Failed to allocate fb_info\n"); > -- > 2.33.0
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 4ce80ca7751f0..9e9db1b0a907b 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -1711,8 +1711,6 @@ intel_find_initial_plane_obj(struct intel_crtc *crtc, plane_state->uapi.crtc = &crtc->base; intel_plane_copy_uapi_to_hw_state(plane_state, plane_state, crtc); - intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB); - atomic_or(plane->frontbuffer_bit, &to_intel_frontbuffer(fb)->bits); } @@ -10755,7 +10753,6 @@ intel_prepare_plane_fb(struct drm_plane *_plane, return ret; i915_gem_object_wait_priority(obj, 0, &attr); - i915_gem_object_flush_frontbuffer(obj, ORIGIN_DIRTYFB); if (!new_plane_state->uapi.fence) { /* implicit fencing */ struct dma_fence *fence; diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c index 60d3ded270476..53484267b2a4a 100644 --- a/drivers/gpu/drm/i915/display/intel_fbdev.c +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c @@ -230,8 +230,6 @@ static int intelfb_create(struct drm_fb_helper *helper, goto out_unlock; } - intel_frontbuffer_flush(to_frontbuffer(ifbdev), ORIGIN_DIRTYFB); - info = drm_fb_helper_alloc_fbi(helper); if (IS_ERR(info)) { drm_err(&dev_priv->drm, "Failed to allocate fb_info\n");
This unnecessary flushes are hurting power-savings are it causes features like PSR, FBC and DRRS to disable it self to handle frontbuffer rendering, below some explanation of why each removed call is not necessary. The flush in intel_prepare_plane_fb() is not required as framebuffer will be flipped and power-saving features do the proper flip handling in hardware. intel_find_initial_plane_obj() flush is not required because it is only executed during driver load and at this point the power-saving features are not even enabled. And the last one intelfb_create(), is also not required as at this point the fbdev was just allocated, userspace will draw on it what will trigger frontbuffer invalidates and flushes later on. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 3 --- drivers/gpu/drm/i915/display/intel_fbdev.c | 2 -- 2 files changed, 5 deletions(-)