> -----Original Message----- > From: Intel-gfx <[email protected]> On Behalf Of Ville > Syrjala > Sent: Wednesday, November 24, 2021 1:37 PM > To: [email protected] > Subject: [Intel-gfx] [PATCH 05/20] drm/i915/fbc: Nuke more FBC state > > From: Ville Syrjälä <[email protected]> > > There isn't a good reason why we'd have to cache all this plane state stuff > in the > FBC state. Instead we can just pre-calculate what FBC will really need. > > Signed-off-by: Ville Syrjälä <[email protected]>
Reviewed-by: Mika Kahola <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_fbc.c | 132 +++++++++++------------ > drivers/gpu/drm/i915/i915_drv.h | 20 +--- > 2 files changed, 67 insertions(+), 85 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > b/drivers/gpu/drm/i915/display/intel_fbc.c > index 6368dddf977c..1e8b75cdfad8 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > @@ -73,25 +73,25 @@ static unsigned int intel_fbc_plane_stride(const struct > intel_plane_state *plane } > > /* plane stride based cfb stride in bytes, assuming 1:1 compression limit */ > - > static unsigned int _intel_fbc_cfb_stride(const struct intel_fbc_state_cache > *cache) > +static unsigned int _intel_fbc_cfb_stride(const struct > +intel_plane_state *plane_state) > { > unsigned int cpp = 4; /* FBC always 4 bytes per pixel */ > > - return cache->fb.stride * cpp; > + return intel_fbc_plane_stride(plane_state) * cpp; > } > > /* minimum acceptable cfb stride in bytes, assuming 1:1 compression limit */ > - > static unsigned int skl_fbc_min_cfb_stride(struct intel_fbc *fbc, > - const struct intel_fbc_state_cache > *cache) > +static unsigned int skl_fbc_min_cfb_stride(const struct > +intel_plane_state *plane_state) > { > - struct drm_i915_private *i915 = fbc->i915; > + struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev); > unsigned int limit = 4; /* 1:4 compression limit is the worst case */ > unsigned int cpp = 4; /* FBC always 4 bytes per pixel */ > + unsigned int width = drm_rect_width(&plane_state->uapi.src) >> 16; > unsigned int height = 4; /* FBC segment is 4 lines */ > unsigned int stride; > > /* minimum segment stride we can use */ > - stride = cache->plane.src_w * cpp * height / limit; > + stride = width * cpp * height / limit; > > /* > * Wa_16011863758: icl+ > @@ -111,11 +111,10 @@ static unsigned int skl_fbc_min_cfb_stride(struct > intel_fbc *fbc, } > > /* properly aligned cfb stride in bytes, assuming 1:1 compression limit */ > -static > unsigned int intel_fbc_cfb_stride(struct intel_fbc *fbc, > - const struct intel_fbc_state_cache > *cache) > +static unsigned int intel_fbc_cfb_stride(const struct intel_plane_state > +*plane_state) > { > - struct drm_i915_private *i915 = fbc->i915; > - unsigned int stride = _intel_fbc_cfb_stride(cache); > + struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev); > + unsigned int stride = _intel_fbc_cfb_stride(plane_state); > > /* > * At least some of the platforms require each 4 line segment to @@ - > 123,30 +122,30 @@ static unsigned int intel_fbc_cfb_stride(struct intel_fbc > *fbc, > * that regardless of the compression limit we choose later. > */ > if (DISPLAY_VER(i915) >= 9) > - return max(ALIGN(stride, 512), skl_fbc_min_cfb_stride(fbc, > cache)); > + return max(ALIGN(stride, 512), > skl_fbc_min_cfb_stride(plane_state)); > else > return stride; > } > > -static unsigned int intel_fbc_cfb_size(struct intel_fbc *fbc, > - const struct intel_fbc_state_cache > *cache) > +static unsigned int intel_fbc_cfb_size(const struct intel_plane_state > +*plane_state) > { > - struct drm_i915_private *i915 = fbc->i915; > - int lines = cache->plane.src_h; > + struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev); > + int lines = drm_rect_height(&plane_state->uapi.src) >> 16; > > if (DISPLAY_VER(i915) == 7) > lines = min(lines, 2048); > else if (DISPLAY_VER(i915) >= 8) > lines = min(lines, 2560); > > - return lines * intel_fbc_cfb_stride(fbc, cache); > + return lines * intel_fbc_cfb_stride(plane_state); > } > > -static u16 intel_fbc_override_cfb_stride(struct intel_fbc *fbc, > - const struct intel_fbc_state_cache > *cache) > +static u16 intel_fbc_override_cfb_stride(const struct intel_plane_state > +*plane_state) > { > - unsigned int stride = _intel_fbc_cfb_stride(cache); > - unsigned int stride_aligned = intel_fbc_cfb_stride(fbc, cache); > + struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev); > + unsigned int stride_aligned = intel_fbc_cfb_stride(plane_state); > + unsigned int stride = _intel_fbc_cfb_stride(plane_state); > + const struct drm_framebuffer *fb = plane_state->hw.fb; > > /* > * Override stride in 64 byte units per 4 line segment. > @@ -156,8 +155,7 @@ static u16 intel_fbc_override_cfb_stride(struct intel_fbc > *fbc, > * we always need to use the override there. > */ > if (stride != stride_aligned || > - (DISPLAY_VER(fbc->i915) == 9 && > - cache->fb.modifier == DRM_FORMAT_MOD_LINEAR)) > + (DISPLAY_VER(i915) == 9 && fb->modifier == > DRM_FORMAT_MOD_LINEAR)) > return stride_aligned * 4 / 64; > > return 0; > @@ -925,31 +923,22 @@ static bool tiling_is_valid(const struct > intel_plane_state *plane_state) > } > } > > -static void intel_fbc_update_state_cache(struct intel_crtc *crtc, > - const struct intel_crtc_state > *crtc_state, > - const struct intel_plane_state > *plane_state) > +static void intel_fbc_update_state_cache(struct intel_atomic_state *state, > + struct intel_crtc *crtc, > + struct intel_plane *plane) > { > - struct drm_i915_private *i915 = to_i915(crtc->base.dev); > - struct intel_fbc *fbc = &i915->fbc; > + struct drm_i915_private *i915 = to_i915(state->base.dev); > + const struct intel_crtc_state *crtc_state = > + intel_atomic_get_new_crtc_state(state, crtc); > + const struct intel_plane_state *plane_state = > + intel_atomic_get_new_plane_state(state, plane); > + struct intel_fbc *fbc = plane->fbc; > struct intel_fbc_state_cache *cache = &fbc->state_cache; > - struct drm_framebuffer *fb = plane_state->hw.fb; > > cache->no_fbc_reason = plane_state->no_fbc_reason; > if (cache->no_fbc_reason) > return; > > - /* > - * Src coordinates are already rotated by 270 degrees for > - * the 90/270 degree plane rotation cases (to match the > - * GTT mapping), hence no need to account for rotation here. > - */ > - cache->plane.src_w = drm_rect_width(&plane_state->uapi.src) >> 16; > - cache->plane.src_h = drm_rect_height(&plane_state->uapi.src) >> 16; > - > - cache->fb.format = fb->format; > - cache->fb.modifier = fb->modifier; > - cache->fb.stride = intel_fbc_plane_stride(plane_state); > - > /* FBC1 compression interval: arbitrary choice of 1 second */ > cache->interval = drm_mode_vrefresh(&crtc_state- > >hw.adjusted_mode); > > @@ -963,12 +952,15 @@ static void intel_fbc_update_state_cache(struct > intel_crtc *crtc, > cache->fence_id = plane_state->ggtt_vma->fence->id; > else > cache->fence_id = -1; > + > + cache->cfb_stride = intel_fbc_cfb_stride(plane_state); > + cache->cfb_size = intel_fbc_cfb_size(plane_state); > + cache->override_cfb_stride = > +intel_fbc_override_cfb_stride(plane_state); > } > > static bool intel_fbc_cfb_size_changed(struct intel_fbc *fbc) { > - return intel_fbc_cfb_size(fbc, &fbc->state_cache) > > - fbc->compressed_fb.size * fbc->limit; > + return fbc->state_cache.cfb_size > fbc->compressed_fb.size * > +fbc->limit; > } > > static bool intel_fbc_can_enable(struct intel_fbc *fbc) @@ -1178,45 +1170,53 > @@ static void intel_fbc_get_reg_params(struct intel_fbc *fbc, > params->interval = cache->interval; > params->crtc.i9xx_plane = to_intel_plane(crtc->base.primary)- > >i9xx_plane; > > - params->fb.format = cache->fb.format; > - params->fb.modifier = cache->fb.modifier; > - params->fb.stride = cache->fb.stride; > - > - params->cfb_stride = intel_fbc_cfb_stride(fbc, cache); > - params->cfb_size = intel_fbc_cfb_size(fbc, cache); > - params->override_cfb_stride = intel_fbc_override_cfb_stride(fbc, > cache); > + params->cfb_stride = cache->cfb_stride; > + params->cfb_size = cache->cfb_size; > + params->override_cfb_stride = cache->override_cfb_stride; > } > > -static bool intel_fbc_can_flip_nuke(const struct intel_crtc_state > *crtc_state) > +static bool intel_fbc_can_flip_nuke(struct intel_atomic_state *state, > + struct intel_crtc *crtc, > + struct intel_plane *plane) > { > - struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > - struct drm_i915_private *i915 = to_i915(crtc->base.dev); > - struct intel_fbc *fbc = &i915->fbc; > + struct intel_fbc *fbc = plane->fbc; > + const struct intel_crtc_state *new_crtc_state = > + intel_atomic_get_new_crtc_state(state, crtc); > + const struct intel_plane_state *old_plane_state = > + intel_atomic_get_old_plane_state(state, plane); > + const struct intel_plane_state *new_plane_state = > + intel_atomic_get_new_plane_state(state, plane); > + const struct drm_framebuffer *old_fb = old_plane_state->hw.fb; > + const struct drm_framebuffer *new_fb = new_plane_state->hw.fb; > const struct intel_fbc_state_cache *cache = &fbc->state_cache; > const struct intel_fbc_reg_params *params = &fbc->params; > > - if (drm_atomic_crtc_needs_modeset(&crtc_state->uapi)) > + if (drm_atomic_crtc_needs_modeset(&new_crtc_state->uapi)) > return false; > > if (!intel_fbc_can_activate(crtc)) > return false; > > - if (params->fb.format != cache->fb.format) > + if (!old_fb || !new_fb) > return false; > > - if (params->fb.modifier != cache->fb.modifier) > + if (old_fb->format->format != new_fb->format->format) > return false; > > - if (params->fb.stride != cache->fb.stride) > + if (old_fb->modifier != new_fb->modifier) > return false; > > - if (params->cfb_stride != intel_fbc_cfb_stride(fbc, cache)) > + if (intel_fbc_plane_stride(old_plane_state) != > + intel_fbc_plane_stride(new_plane_state)) > return false; > > - if (params->cfb_size != intel_fbc_cfb_size(fbc, cache)) > + if (params->cfb_stride != cache->cfb_stride) > return false; > > - if (params->override_cfb_stride != intel_fbc_override_cfb_stride(fbc, > cache)) > + if (params->cfb_size != cache->cfb_size) > + return false; > + > + if (params->override_cfb_stride != cache->override_cfb_stride) > return false; > > return true; > @@ -1226,8 +1226,6 @@ bool intel_fbc_pre_update(struct intel_atomic_state > *state, > struct intel_crtc *crtc) > { > struct intel_plane *plane = to_intel_plane(crtc->base.primary); > - const struct intel_crtc_state *crtc_state = > - intel_atomic_get_new_crtc_state(state, crtc); > const struct intel_plane_state *plane_state = > intel_atomic_get_new_plane_state(state, plane); > struct drm_i915_private *i915 = to_i915(crtc->base.dev); @@ -1243,10 > +1241,10 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state, > if (fbc->crtc != crtc) > goto unlock; > > - intel_fbc_update_state_cache(crtc, crtc_state, plane_state); > + intel_fbc_update_state_cache(state, crtc, plane); > fbc->flip_pending = true; > > - if (!intel_fbc_can_flip_nuke(crtc_state)) { > + if (!intel_fbc_can_flip_nuke(state, crtc, plane)) { > intel_fbc_deactivate(fbc, reason); > > /* > @@ -1425,8 +1423,6 @@ static void intel_fbc_enable(struct intel_atomic_state > *state, { > struct drm_i915_private *i915 = to_i915(crtc->base.dev); > struct intel_plane *plane = to_intel_plane(crtc->base.primary); > - const struct intel_crtc_state *crtc_state = > - intel_atomic_get_new_crtc_state(state, crtc); > const struct intel_plane_state *plane_state = > intel_atomic_get_new_plane_state(state, plane); > struct intel_fbc *fbc = plane->fbc; > @@ -1455,12 +1451,12 @@ static void intel_fbc_enable(struct > intel_atomic_state *state, > > drm_WARN_ON(&i915->drm, fbc->active); > > - intel_fbc_update_state_cache(crtc, crtc_state, plane_state); > + intel_fbc_update_state_cache(state, crtc, plane); > > if (cache->no_fbc_reason) > goto out; > > - if (intel_fbc_alloc_cfb(fbc, intel_fbc_cfb_size(fbc, cache), > min_limit)) { > + if (intel_fbc_alloc_cfb(fbc, intel_fbc_cfb_size(plane_state), > +min_limit)) { > fbc->no_fbc_reason = "not enough stolen memory"; > goto out; > } > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index d79aa827d937..66fa46d41fa5 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -436,18 +436,10 @@ struct intel_fbc { > struct intel_fbc_state_cache { > const char *no_fbc_reason; > > - struct { > - int src_w; > - int src_h; > - } plane; > - > - struct { > - const struct drm_format_info *format; > - unsigned int stride; > - u64 modifier; > - } fb; > - > + unsigned int cfb_stride; > + unsigned int cfb_size; > unsigned int fence_y_offset; > + u16 override_cfb_stride; > u16 interval; > s8 fence_id; > } state_cache; > @@ -464,12 +456,6 @@ struct intel_fbc { > enum i9xx_plane_id i9xx_plane; > } crtc; > > - struct { > - const struct drm_format_info *format; > - unsigned int stride; > - u64 modifier; > - } fb; > - > unsigned int cfb_stride; > unsigned int cfb_size; > unsigned int fence_y_offset; > -- > 2.32.0
