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@xxxxxxxxxxxxxxx>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
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@xxxxxxxxxxxxxxx>
> ---
> 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
--
Ville Syrjälä
Intel
