From b6e201f5f13bd61ab8e5187daa0e149826cda154 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:34 +0200 Subject: drm/i915/fbc: Pass whole plane state to intel_fbc_min_limit() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No reason to burden the caller with the details on how the minimum compression limit is calculated, so just pass in the whole plane state instead of just the cpp value. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-3-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index d0c34bc3af6c..083c0cab4847 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -679,8 +679,10 @@ static u64 intel_fbc_stolen_end(struct drm_i915_private *i915) return min(end, intel_fbc_cfb_base_max(i915)); } -static int intel_fbc_min_limit(int fb_cpp) +static int intel_fbc_min_limit(const struct intel_plane_state *plane_state) { + int fb_cpp = plane_state->hw.fb ? plane_state->hw.fb->format->cpp[0] : 0; + return fb_cpp == 2 ? 2 : 1; } @@ -1466,8 +1468,7 @@ static void intel_fbc_enable(struct intel_atomic_state *state, cache = &fbc->state_cache; - min_limit = intel_fbc_min_limit(plane_state->hw.fb ? - plane_state->hw.fb->format->cpp[0] : 0); + min_limit = intel_fbc_min_limit(plane_state); mutex_lock(&fbc->lock); -- cgit From 2e6c99f88679121eacc75196bdf6da8b0e513066 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:35 +0200 Subject: drm/i915/fbc: Nuke lots of crap from intel_fbc_state_cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There's no need to store all this stuff in intel_fbc_state_cache. Just check it all against the plane/crtc states and store only what we need. Probably more should get nuked still, but this is a start. So what we'll do is: - each plane will check its own state and update its local no_fbc_reason - the per-plane no_fbc_reason (if any) then gets propagated to the cache->no_fbc_reason while doing the actual update - fbc->no_fbc_reason gets updated in the end with either the value from the cache or directly from frontbuffer tracking It's still a bit messy, but should hopefuly get cleaned up more in the future. At least now we can observe each plane's reasons for rejecting FBC now more consistently, and we don't have so mcuh redundant state store all over the place. v2: store no_fbc_reason per-plane instead of per-pipe Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-4-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 341 ++++++++++++++----------------- 1 file changed, 158 insertions(+), 183 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 083c0cab4847..8bde3681b96e 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -43,6 +43,7 @@ #include "i915_drv.h" #include "i915_trace.h" #include "i915_vgpu.h" +#include "intel_cdclk.h" #include "intel_de.h" #include "intel_display_types.h" #include "intel_fbc.h" @@ -58,20 +59,6 @@ struct intel_fbc_funcs { void (*set_false_color)(struct intel_fbc *fbc, bool enable); }; -/* - * For SKL+, the plane source size used by the hardware is based on the value we - * write to the PLANE_SIZE register. For BDW-, the hardware looks at the value - * we wrote to PIPESRC. - */ -static void intel_fbc_get_plane_source_size(const struct intel_fbc_state_cache *cache, - int *width, int *height) -{ - if (width) - *width = cache->plane.src_w; - if (height) - *height = cache->plane.src_h; -} - /* plane stride in pixels */ static unsigned int intel_fbc_plane_stride(const struct intel_plane_state *plane_state) { @@ -796,9 +783,13 @@ void intel_fbc_cleanup(struct drm_i915_private *i915) mutex_unlock(&fbc->lock); } -static bool stride_is_valid(struct drm_i915_private *i915, - u64 modifier, unsigned int stride) +static bool stride_is_valid(const struct intel_plane_state *plane_state) { + struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev); + const struct drm_framebuffer *fb = plane_state->hw.fb; + unsigned int stride = intel_fbc_plane_stride(plane_state) * + fb->format->cpp[0]; + /* This should have been caught earlier. */ if (drm_WARN_ON_ONCE(&i915->drm, (stride & (64 - 1)) != 0)) return false; @@ -815,7 +806,7 @@ static bool stride_is_valid(struct drm_i915_private *i915, /* Display WA #1105: skl,bxt,kbl,cfl,glk */ if ((DISPLAY_VER(i915) == 9 || IS_GEMINILAKE(i915)) && - modifier == DRM_FORMAT_MOD_LINEAR && stride & 511) + fb->modifier == DRM_FORMAT_MOD_LINEAR && stride & 511) return false; if (stride > 16384) @@ -824,10 +815,12 @@ static bool stride_is_valid(struct drm_i915_private *i915, return true; } -static bool pixel_format_is_valid(struct drm_i915_private *i915, - u32 pixel_format) +static bool pixel_format_is_valid(const struct intel_plane_state *plane_state) { - switch (pixel_format) { + struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev); + const struct drm_framebuffer *fb = plane_state->hw.fb; + + switch (fb->format->format) { case DRM_FORMAT_XRGB8888: case DRM_FORMAT_XBGR8888: return true; @@ -845,10 +838,13 @@ static bool pixel_format_is_valid(struct drm_i915_private *i915, } } -static bool rotation_is_valid(struct drm_i915_private *i915, - u32 pixel_format, unsigned int rotation) +static bool rotation_is_valid(const struct intel_plane_state *plane_state) { - if (DISPLAY_VER(i915) >= 9 && pixel_format == DRM_FORMAT_RGB565 && + struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev); + const struct drm_framebuffer *fb = plane_state->hw.fb; + unsigned int rotation = plane_state->hw.rotation; + + if (DISPLAY_VER(i915) >= 9 && fb->format->format == DRM_FORMAT_RGB565 && drm_rotation_90_or_270(rotation)) return false; else if (DISPLAY_VER(i915) <= 4 && !IS_G4X(i915) && @@ -864,10 +860,9 @@ static bool rotation_is_valid(struct drm_i915_private *i915, * the X and Y offset registers. That's why we include the src x/y offsets * instead of just looking at the plane size. */ -static bool intel_fbc_hw_tracking_covers_screen(struct intel_fbc *fbc, - struct intel_crtc *crtc) +static bool intel_fbc_hw_tracking_covers_screen(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 effective_w, effective_h, max_w, max_h; if (DISPLAY_VER(i915) >= 10) { @@ -884,18 +879,20 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_fbc *fbc, max_h = 1536; } - intel_fbc_get_plane_source_size(&fbc->state_cache, &effective_w, - &effective_h); - effective_w += fbc->state_cache.plane.adjusted_x; - effective_h += fbc->state_cache.plane.adjusted_y; + effective_w = plane_state->view.color_plane[0].x + + (drm_rect_width(&plane_state->uapi.src) >> 16); + effective_h = plane_state->view.color_plane[0].y + + (drm_rect_height(&plane_state->uapi.src) >> 16); return effective_w <= max_w && effective_h <= max_h; } -static bool tiling_is_valid(struct drm_i915_private *i915, - u64 modifier) +static bool tiling_is_valid(const struct intel_plane_state *plane_state) { - switch (modifier) { + struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev); + const struct drm_framebuffer *fb = plane_state->hw.fb; + + switch (fb->modifier) { case DRM_FORMAT_MOD_LINEAR: case I915_FORMAT_MOD_Y_TILED: case I915_FORMAT_MOD_Yf_TILED: @@ -916,15 +913,10 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc, struct intel_fbc_state_cache *cache = &fbc->state_cache; struct drm_framebuffer *fb = plane_state->hw.fb; - cache->plane.visible = plane_state->uapi.visible; - if (!cache->plane.visible) + cache->no_fbc_reason = plane_state->no_fbc_reason; + if (cache->no_fbc_reason) return; - cache->crtc.mode_flags = crtc_state->hw.adjusted_mode.flags; - if (IS_HASWELL(i915) || IS_BROADWELL(i915)) - cache->crtc.hsw_bdw_pixel_rate = crtc_state->pixel_rate; - - cache->plane.rotation = plane_state->hw.rotation; /* * Src coordinates are already rotated by 270 degrees for * the 90/270 degree plane rotation cases (to match the @@ -932,10 +924,6 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc, */ 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->plane.adjusted_x = plane_state->view.color_plane[0].x; - cache->plane.adjusted_y = plane_state->view.color_plane[0].y; - - cache->plane.pixel_blend_mode = plane_state->hw.pixel_blend_mode; cache->fb.format = fb->format; cache->fb.modifier = fb->modifier; @@ -954,8 +942,6 @@ 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->psr2_active = crtc_state->has_psr2; } static bool intel_fbc_cfb_size_changed(struct intel_fbc *fbc) @@ -1007,6 +993,110 @@ static bool intel_fbc_can_enable(struct intel_fbc *fbc) return true; } +static int intel_fbc_check_plane(struct intel_atomic_state *state, + struct intel_plane *plane) +{ + struct drm_i915_private *i915 = to_i915(state->base.dev); + struct intel_plane_state *plane_state = + intel_atomic_get_new_plane_state(state, plane); + const struct drm_framebuffer *fb = plane_state->hw.fb; + struct intel_crtc *crtc = to_intel_crtc(plane_state->uapi.crtc); + const struct intel_crtc_state *crtc_state; + struct intel_fbc *fbc = plane->fbc; + + if (!fbc) + return 0; + + if (!plane_state->uapi.visible) { + plane_state->no_fbc_reason = "plane not visible"; + return 0; + } + + crtc_state = intel_atomic_get_new_crtc_state(state, crtc); + + if (crtc_state->hw.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) { + plane_state->no_fbc_reason = "interlaced mode not supported"; + return 0; + } + + /* + * Display 12+ is not supporting FBC with PSR2. + * Recommendation is to keep this combination disabled + * Bspec: 50422 HSD: 14010260002 + */ + if (DISPLAY_VER(i915) >= 12 && crtc_state->has_psr2) { + plane_state->no_fbc_reason = "PSR2 enabled"; + return false; + } + + if (!pixel_format_is_valid(plane_state)) { + plane_state->no_fbc_reason = "pixel format not supported"; + return 0; + } + + if (!tiling_is_valid(plane_state)) { + plane_state->no_fbc_reason = "tiling not supported"; + return 0; + } + + if (!rotation_is_valid(plane_state)) { + plane_state->no_fbc_reason = "rotation not supported"; + return 0; + } + + if (!stride_is_valid(plane_state)) { + plane_state->no_fbc_reason = "stride not supported"; + return 0; + } + + if (plane_state->hw.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE && + fb->format->has_alpha) { + plane_state->no_fbc_reason = "per-pixel alpha not supported"; + return false; + } + + if (!intel_fbc_hw_tracking_covers_screen(plane_state)) { + plane_state->no_fbc_reason = "plane size too big"; + return 0; + } + + /* + * Work around a problem on GEN9+ HW, where enabling FBC on a plane + * having a Y offset that isn't divisible by 4 causes FIFO underrun + * and screen flicker. + */ + if (DISPLAY_VER(i915) >= 9 && + plane_state->view.color_plane[0].y & 3) { + plane_state->no_fbc_reason = "plane start Y offset misaligned"; + return false; + } + + /* Wa_22010751166: icl, ehl, tgl, dg1, rkl */ + if (DISPLAY_VER(i915) >= 11 && + (plane_state->view.color_plane[0].y + drm_rect_height(&plane_state->uapi.src)) & 3) { + plane_state->no_fbc_reason = "plane end Y offset misaligned"; + return false; + } + + /* WaFbcExceedCdClockThreshold:hsw,bdw */ + if (IS_HASWELL(i915) || IS_BROADWELL(i915)) { + const struct intel_cdclk_state *cdclk_state; + + cdclk_state = intel_atomic_get_cdclk_state(state); + if (IS_ERR(cdclk_state)) + return PTR_ERR(cdclk_state); + + if (crtc_state->pixel_rate >= cdclk_state->logical.cdclk * 95 / 100) { + plane_state->no_fbc_reason = "pixel rate too high"; + return 0; + } + } + + plane_state->no_fbc_reason = NULL; + + return 0; +} + static bool intel_fbc_can_activate(struct intel_crtc *crtc) { struct drm_i915_private *i915 = to_i915(crtc->base.dev); @@ -1016,8 +1106,8 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) if (!intel_fbc_can_enable(fbc)) return false; - if (!cache->plane.visible) { - fbc->no_fbc_reason = "primary plane not visible"; + if (cache->no_fbc_reason) { + fbc->no_fbc_reason = cache->no_fbc_reason; return false; } @@ -1029,16 +1119,6 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) return false; } - if (cache->crtc.mode_flags & DRM_MODE_FLAG_INTERLACE) { - fbc->no_fbc_reason = "incompatible mode"; - return false; - } - - if (!intel_fbc_hw_tracking_covers_screen(fbc, crtc)) { - fbc->no_fbc_reason = "mode too large for compression"; - return false; - } - /* The use of a CPU fence is one of two ways to detect writes by the * CPU to the scanout and trigger updates to the FBC. * @@ -1061,42 +1141,8 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) return false; } - if (!pixel_format_is_valid(i915, cache->fb.format->format)) { - fbc->no_fbc_reason = "pixel format is invalid"; - return false; - } - - if (!rotation_is_valid(i915, cache->fb.format->format, - cache->plane.rotation)) { - fbc->no_fbc_reason = "rotation unsupported"; - return false; - } - - if (!tiling_is_valid(i915, cache->fb.modifier)) { - fbc->no_fbc_reason = "tiling unsupported"; - return false; - } - - if (!stride_is_valid(i915, cache->fb.modifier, - cache->fb.stride * cache->fb.format->cpp[0])) { - fbc->no_fbc_reason = "framebuffer stride not supported"; - return false; - } - - if (cache->plane.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE && - cache->fb.format->has_alpha) { - fbc->no_fbc_reason = "per-pixel alpha blending is incompatible with FBC"; - return false; - } - - /* WaFbcExceedCdClockThreshold:hsw,bdw */ - if ((IS_HASWELL(i915) || IS_BROADWELL(i915)) && - cache->crtc.hsw_bdw_pixel_rate >= i915->cdclk.hw.cdclk * 95 / 100) { - fbc->no_fbc_reason = "pixel rate is too big"; - return false; - } - - /* It is possible for the required CFB size change without a + /* + * It is possible for the required CFB size change without a * crtc->disable + crtc->enable since it is possible to change the * stride without triggering a full modeset. Since we try to * over-allocate the CFB, there's a chance we may keep FBC enabled even @@ -1105,40 +1151,13 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) * for a frame, free the stolen node, then try to reenable FBC in case * we didn't get any invalidate/deactivate calls, but this would require * a lot of tracking just for a specific case. If we conclude it's an - * important case, we can implement it later. */ + * important case, we can implement it later. + */ if (intel_fbc_cfb_size_changed(fbc)) { fbc->no_fbc_reason = "CFB requirements changed"; return false; } - /* - * Work around a problem on GEN9+ HW, where enabling FBC on a plane - * having a Y offset that isn't divisible by 4 causes FIFO underrun - * and screen flicker. - */ - if (DISPLAY_VER(i915) >= 9 && - (fbc->state_cache.plane.adjusted_y & 3)) { - fbc->no_fbc_reason = "plane Y offset is misaligned"; - return false; - } - - /* Wa_22010751166: icl, ehl, tgl, dg1, rkl */ - if (DISPLAY_VER(i915) >= 11 && - (cache->plane.src_h + cache->plane.adjusted_y) % 4) { - fbc->no_fbc_reason = "plane height + offset is non-modulo of 4"; - return false; - } - - /* - * Display 12+ is not supporting FBC with PSR2. - * Recommendation is to keep this combination disabled - * Bspec: 50422 HSD: 14010260002 - */ - if (fbc->state_cache.psr2_active && DISPLAY_VER(i915) >= 12) { - fbc->no_fbc_reason = "not supported with PSR2"; - return false; - } - return true; } @@ -1157,8 +1176,6 @@ static void intel_fbc_get_reg_params(struct intel_fbc *fbc, params->fence_y_offset = cache->fence_y_offset; params->interval = cache->interval; - - params->crtc.pipe = crtc->pipe; params->crtc.i9xx_plane = to_intel_plane(crtc->base.primary)->i9xx_plane; params->fb.format = cache->fb.format; @@ -1168,8 +1185,6 @@ static void intel_fbc_get_reg_params(struct intel_fbc *fbc, 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->plane_visible = cache->plane.visible; } static bool intel_fbc_can_flip_nuke(const struct intel_crtc_state *crtc_state) @@ -1183,9 +1198,6 @@ static bool intel_fbc_can_flip_nuke(const struct intel_crtc_state *crtc_state) if (drm_atomic_crtc_needs_modeset(&crtc_state->uapi)) return false; - if (!params->plane_visible) - return false; - if (!intel_fbc_can_activate(crtc)) return false; @@ -1381,63 +1393,21 @@ out: mutex_unlock(&fbc->lock); } -/** - * intel_fbc_choose_crtc - select a CRTC to enable FBC on - * @i915: i915 device instance - * @state: the atomic state structure - * - * This function looks at the proposed state for CRTCs and planes, then chooses - * which pipe is going to have FBC by setting intel_crtc_state->enable_fbc to - * true. - * - * Later, intel_fbc_enable is going to look for state->enable_fbc and then maybe - * enable FBC for the chosen CRTC. If it does, it will set i915->fbc.crtc. - */ -void intel_fbc_choose_crtc(struct drm_i915_private *i915, - struct intel_atomic_state *state) +int intel_fbc_atomic_check(struct intel_atomic_state *state) { - struct intel_fbc *fbc = &i915->fbc; - struct intel_plane *plane; struct intel_plane_state *plane_state; - bool crtc_chosen = false; + struct intel_plane *plane; int i; - mutex_lock(&fbc->lock); - - /* Does this atomic commit involve the CRTC currently tied to FBC? */ - if (fbc->crtc && - !intel_atomic_get_new_crtc_state(state, fbc->crtc)) - goto out; - - if (!intel_fbc_can_enable(fbc)) - goto out; - - /* Simply choose the first CRTC that is compatible and has a visible - * plane. We could go for fancier schemes such as checking the plane - * size, but this would just affect the few platforms that don't tie FBC - * to pipe or plane A. */ for_each_new_intel_plane_in_state(state, plane, plane_state, i) { - struct intel_crtc_state *crtc_state; - struct intel_crtc *crtc = to_intel_crtc(plane_state->hw.crtc); + int ret; - if (plane->fbc != fbc) - continue; - - if (!plane_state->uapi.visible) - continue; - - crtc_state = intel_atomic_get_new_crtc_state(state, crtc); - - crtc_state->enable_fbc = true; - crtc_chosen = true; - break; + ret = intel_fbc_check_plane(state, plane); + if (ret) + return ret; } - if (!crtc_chosen) - fbc->no_fbc_reason = "no suitable CRTC for FBC"; - -out: - mutex_unlock(&fbc->lock); + return 0; } /** @@ -1487,12 +1457,10 @@ static void intel_fbc_enable(struct intel_atomic_state *state, intel_fbc_update_state_cache(crtc, crtc_state, plane_state); - /* FIXME crtc_state->enable_fbc lies :( */ - if (!cache->plane.visible) + if (cache->no_fbc_reason) goto out; if (intel_fbc_alloc_cfb(fbc, intel_fbc_cfb_size(fbc, cache), min_limit)) { - cache->plane.visible = false; fbc->no_fbc_reason = "not enough stolen memory"; goto out; } @@ -1541,10 +1509,17 @@ void intel_fbc_disable(struct intel_crtc *crtc) void intel_fbc_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 intel_fbc *fbc = plane->fbc; + + if (!fbc || !plane_state) + return; - if (crtc_state->update_pipe && !crtc_state->enable_fbc) + if (crtc_state->update_pipe && plane_state->no_fbc_reason) intel_fbc_disable(crtc); else intel_fbc_enable(state, crtc); -- cgit From 266790871e8d20d6074c1cf3ede7ae92efc61bea Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:36 +0200 Subject: drm/i915/fbc: Relocate intel_fbc_override_cfb_stride() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move intel_fbc_override_cfb_stride() next to its cousins. Helps with later patches. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-5-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 42 ++++++++++++++++---------------- 1 file changed, 21 insertions(+), 21 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 8bde3681b96e..6368dddf977c 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -142,6 +142,27 @@ static unsigned int intel_fbc_cfb_size(struct intel_fbc *fbc, return lines * intel_fbc_cfb_stride(fbc, cache); } +static u16 intel_fbc_override_cfb_stride(struct intel_fbc *fbc, + const struct intel_fbc_state_cache *cache) +{ + unsigned int stride = _intel_fbc_cfb_stride(cache); + unsigned int stride_aligned = intel_fbc_cfb_stride(fbc, cache); + + /* + * Override stride in 64 byte units per 4 line segment. + * + * Gen9 hw miscalculates cfb stride for linear as + * PLANE_STRIDE*512 instead of PLANE_STRIDE*64, so + * we always need to use the override there. + */ + if (stride != stride_aligned || + (DISPLAY_VER(fbc->i915) == 9 && + cache->fb.modifier == DRM_FORMAT_MOD_LINEAR)) + return stride_aligned * 4 / 64; + + return 0; +} + static u32 i8xx_fbc_ctl(struct intel_fbc *fbc) { const struct intel_fbc_reg_params *params = &fbc->params; @@ -950,27 +971,6 @@ static bool intel_fbc_cfb_size_changed(struct intel_fbc *fbc) fbc->compressed_fb.size * fbc->limit; } -static u16 intel_fbc_override_cfb_stride(struct intel_fbc *fbc, - const struct intel_fbc_state_cache *cache) -{ - unsigned int stride = _intel_fbc_cfb_stride(cache); - unsigned int stride_aligned = intel_fbc_cfb_stride(fbc, cache); - - /* - * Override stride in 64 byte units per 4 line segment. - * - * Gen9 hw miscalculates cfb stride for linear as - * PLANE_STRIDE*512 instead of PLANE_STRIDE*64, so - * we always need to use the override there. - */ - if (stride != stride_aligned || - (DISPLAY_VER(fbc->i915) == 9 && - cache->fb.modifier == DRM_FORMAT_MOD_LINEAR)) - return stride_aligned * 4 / 64; - - return 0; -} - static bool intel_fbc_can_enable(struct intel_fbc *fbc) { struct drm_i915_private *i915 = fbc->i915; -- cgit From 873c995a40a5c2324a5d1e890604066b74914b3c Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:37 +0200 Subject: drm/i915/fbc: Nuke more FBC state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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ä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-6-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 132 +++++++++++++++---------------- 1 file changed, 64 insertions(+), 68 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') 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 (old_fb->format->format != new_fb->format->format) return false; - if (params->fb.modifier != cache->fb.modifier) + if (old_fb->modifier != new_fb->modifier) return false; - if (params->fb.stride != cache->fb.stride) + if (intel_fbc_plane_stride(old_plane_state) != + intel_fbc_plane_stride(new_plane_state)) return false; - if (params->cfb_stride != intel_fbc_cfb_stride(fbc, cache)) + if (params->cfb_stride != cache->cfb_stride) return false; - if (params->cfb_size != intel_fbc_cfb_size(fbc, cache)) + if (params->cfb_size != cache->cfb_size) return false; - if (params->override_cfb_stride != intel_fbc_override_cfb_stride(fbc, cache)) + 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; } -- cgit From e1521cbd27aa100a86b54094cfa4387a9bcc2f63 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:38 +0200 Subject: drm/i915/fbc: Reuse the same struct for the cache and params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The FBC state cache and params are now nearly identical. Just use the same structure for both. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-7-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 62 ++++++++++++++------------------ 1 file changed, 27 insertions(+), 35 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 1e8b75cdfad8..8625825cbee8 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -163,7 +163,7 @@ static u16 intel_fbc_override_cfb_stride(const struct intel_plane_state *plane_s static u32 i8xx_fbc_ctl(struct intel_fbc *fbc) { - const struct intel_fbc_reg_params *params = &fbc->params; + const struct intel_fbc_state *params = &fbc->params; struct drm_i915_private *i915 = fbc->i915; unsigned int cfb_stride; u32 fbc_ctl; @@ -191,11 +191,11 @@ static u32 i8xx_fbc_ctl(struct intel_fbc *fbc) static u32 i965_fbc_ctl2(struct intel_fbc *fbc) { - const struct intel_fbc_reg_params *params = &fbc->params; + const struct intel_fbc_state *params = &fbc->params; u32 fbc_ctl2; fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | - FBC_CTL_PLANE(params->crtc.i9xx_plane); + FBC_CTL_PLANE(params->i9xx_plane); if (params->fence_id >= 0) fbc_ctl2 |= FBC_CTL_CPU_FENCE_EN; @@ -226,7 +226,7 @@ static void i8xx_fbc_deactivate(struct intel_fbc *fbc) static void i8xx_fbc_activate(struct intel_fbc *fbc) { - const struct intel_fbc_reg_params *params = &fbc->params; + const struct intel_fbc_state *params = &fbc->params; struct drm_i915_private *i915 = fbc->i915; int i; @@ -258,8 +258,8 @@ static bool i8xx_fbc_is_compressing(struct intel_fbc *fbc) static void i8xx_fbc_nuke(struct intel_fbc *fbc) { - struct intel_fbc_reg_params *params = &fbc->params; - enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane; + struct intel_fbc_state *params = &fbc->params; + enum i9xx_plane_id i9xx_plane = params->i9xx_plane; struct drm_i915_private *dev_priv = fbc->i915; spin_lock_irq(&dev_priv->uncore.lock); @@ -294,8 +294,8 @@ static const struct intel_fbc_funcs i8xx_fbc_funcs = { static void i965_fbc_nuke(struct intel_fbc *fbc) { - struct intel_fbc_reg_params *params = &fbc->params; - enum i9xx_plane_id i9xx_plane = params->crtc.i9xx_plane; + struct intel_fbc_state *params = &fbc->params; + enum i9xx_plane_id i9xx_plane = params->i9xx_plane; struct drm_i915_private *dev_priv = fbc->i915; spin_lock_irq(&dev_priv->uncore.lock); @@ -330,12 +330,12 @@ static u32 g4x_dpfc_ctl_limit(struct intel_fbc *fbc) static u32 g4x_dpfc_ctl(struct intel_fbc *fbc) { - const struct intel_fbc_reg_params *params = &fbc->params; + const struct intel_fbc_state *params = &fbc->params; struct drm_i915_private *i915 = fbc->i915; u32 dpfc_ctl; dpfc_ctl = g4x_dpfc_ctl_limit(fbc) | - DPFC_CTL_PLANE_G4X(params->crtc.i9xx_plane); + DPFC_CTL_PLANE_G4X(params->i9xx_plane); if (IS_G4X(i915)) dpfc_ctl |= DPFC_CTL_SR_EN; @@ -352,7 +352,7 @@ static u32 g4x_dpfc_ctl(struct intel_fbc *fbc) static void g4x_fbc_activate(struct intel_fbc *fbc) { - const struct intel_fbc_reg_params *params = &fbc->params; + const struct intel_fbc_state *params = &fbc->params; struct drm_i915_private *i915 = fbc->i915; intel_de_write(i915, DPFC_FENCE_YOFF, @@ -403,7 +403,7 @@ static const struct intel_fbc_funcs g4x_fbc_funcs = { static void ilk_fbc_activate(struct intel_fbc *fbc) { - struct intel_fbc_reg_params *params = &fbc->params; + struct intel_fbc_state *params = &fbc->params; struct drm_i915_private *i915 = fbc->i915; intel_de_write(i915, ILK_DPFC_FENCE_YOFF, @@ -454,7 +454,7 @@ static const struct intel_fbc_funcs ilk_fbc_funcs = { static void snb_fbc_program_fence(struct intel_fbc *fbc) { - const struct intel_fbc_reg_params *params = &fbc->params; + const struct intel_fbc_state *params = &fbc->params; struct drm_i915_private *i915 = fbc->i915; u32 ctl = 0; @@ -491,7 +491,7 @@ static const struct intel_fbc_funcs snb_fbc_funcs = { static void glk_fbc_program_cfb_stride(struct intel_fbc *fbc) { - const struct intel_fbc_reg_params *params = &fbc->params; + const struct intel_fbc_state *params = &fbc->params; struct drm_i915_private *i915 = fbc->i915; u32 val = 0; @@ -504,7 +504,7 @@ static void glk_fbc_program_cfb_stride(struct intel_fbc *fbc) static void skl_fbc_program_cfb_stride(struct intel_fbc *fbc) { - const struct intel_fbc_reg_params *params = &fbc->params; + const struct intel_fbc_state *params = &fbc->params; struct drm_i915_private *i915 = fbc->i915; u32 val = 0; @@ -520,14 +520,14 @@ static void skl_fbc_program_cfb_stride(struct intel_fbc *fbc) static u32 ivb_dpfc_ctl(struct intel_fbc *fbc) { - const struct intel_fbc_reg_params *params = &fbc->params; + const struct intel_fbc_state *params = &fbc->params; struct drm_i915_private *i915 = fbc->i915; u32 dpfc_ctl; dpfc_ctl = g4x_dpfc_ctl_limit(fbc); if (IS_IVYBRIDGE(i915)) - dpfc_ctl |= DPFC_CTL_PLANE_IVB(params->crtc.i9xx_plane); + dpfc_ctl |= DPFC_CTL_PLANE_IVB(params->i9xx_plane); if (params->fence_id >= 0) dpfc_ctl |= DPFC_CTL_FENCE_EN_IVB; @@ -933,12 +933,14 @@ static void intel_fbc_update_state_cache(struct intel_atomic_state *state, 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 intel_fbc_state *cache = &fbc->state_cache; cache->no_fbc_reason = plane_state->no_fbc_reason; if (cache->no_fbc_reason) return; + cache->i9xx_plane = plane->i9xx_plane; + /* FBC1 compression interval: arbitrary choice of 1 second */ cache->interval = drm_mode_vrefresh(&crtc_state->hw.adjusted_mode); @@ -1093,7 +1095,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) { struct drm_i915_private *i915 = to_i915(crtc->base.dev); struct intel_fbc *fbc = &i915->fbc; - struct intel_fbc_state_cache *cache = &fbc->state_cache; + struct intel_fbc_state *cache = &fbc->state_cache; if (!intel_fbc_can_enable(fbc)) return false; @@ -1156,23 +1158,13 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc) static void intel_fbc_get_reg_params(struct intel_fbc *fbc, struct intel_crtc *crtc) { - const struct intel_fbc_state_cache *cache = &fbc->state_cache; - struct intel_fbc_reg_params *params = &fbc->params; + const struct intel_fbc_state *cache = &fbc->state_cache; + struct intel_fbc_state *params = &fbc->params; /* Since all our fields are integer types, use memset here so the * comparison function can rely on memcmp because the padding will be * zero. */ - memset(params, 0, sizeof(*params)); - - params->fence_id = cache->fence_id; - params->fence_y_offset = cache->fence_y_offset; - - params->interval = cache->interval; - params->crtc.i9xx_plane = to_intel_plane(crtc->base.primary)->i9xx_plane; - - params->cfb_stride = cache->cfb_stride; - params->cfb_size = cache->cfb_size; - params->override_cfb_stride = cache->override_cfb_stride; + *params = *cache; } static bool intel_fbc_can_flip_nuke(struct intel_atomic_state *state, @@ -1188,8 +1180,8 @@ static bool intel_fbc_can_flip_nuke(struct intel_atomic_state *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; + const struct intel_fbc_state *cache = &fbc->state_cache; + const struct intel_fbc_state *params = &fbc->params; if (drm_atomic_crtc_needs_modeset(&new_crtc_state->uapi)) return false; @@ -1426,7 +1418,7 @@ static void intel_fbc_enable(struct intel_atomic_state *state, 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; + struct intel_fbc_state *cache; int min_limit; if (!fbc || !plane_state) -- cgit From 6e4d2e45ef3eff90e2ee2dcbc29e356158c75f0a Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:39 +0200 Subject: drm/i915/fbc: Pass around FBC instance instead of crtc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass the FBC instance instead of the crtc to a bunch of places. We also adjust intel_fbc_post_update() to do the intel_fbc_get_reg_params() things instead of doing it from the lower level function (which also gets called for front buffer tracking). Nothing in there will change during front buffer updates. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-8-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 8625825cbee8..db390c29c665 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1091,10 +1091,9 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, return 0; } -static bool intel_fbc_can_activate(struct intel_crtc *crtc) +static bool intel_fbc_can_activate(struct intel_fbc *fbc) { - struct drm_i915_private *i915 = to_i915(crtc->base.dev); - struct intel_fbc *fbc = &i915->fbc; + struct drm_i915_private *i915 = fbc->i915; struct intel_fbc_state *cache = &fbc->state_cache; if (!intel_fbc_can_enable(fbc)) @@ -1186,7 +1185,7 @@ static bool intel_fbc_can_flip_nuke(struct intel_atomic_state *state, if (drm_atomic_crtc_needs_modeset(&new_crtc_state->uapi)) return false; - if (!intel_fbc_can_activate(crtc)) + if (!intel_fbc_can_activate(fbc)) return false; if (!old_fb || !new_fb) @@ -1280,18 +1279,12 @@ static void __intel_fbc_disable(struct intel_fbc *fbc) fbc->crtc = NULL; } -static void __intel_fbc_post_update(struct intel_crtc *crtc) +static void __intel_fbc_post_update(struct intel_fbc *fbc) { - struct drm_i915_private *i915 = to_i915(crtc->base.dev); - struct intel_fbc *fbc = &i915->fbc; + struct drm_i915_private *i915 = fbc->i915; drm_WARN_ON(&i915->drm, !mutex_is_locked(&fbc->lock)); - if (fbc->crtc != crtc) - return; - - fbc->flip_pending = false; - if (!i915->params.enable_fbc) { intel_fbc_deactivate(fbc, "disabled at runtime per module param"); __intel_fbc_disable(fbc); @@ -1299,9 +1292,7 @@ static void __intel_fbc_post_update(struct intel_crtc *crtc) return; } - intel_fbc_get_reg_params(fbc, crtc); - - if (!intel_fbc_can_activate(crtc)) + if (!intel_fbc_can_activate(fbc)) return; if (!fbc->busy_bits) @@ -1322,7 +1313,11 @@ void intel_fbc_post_update(struct intel_atomic_state *state, return; mutex_lock(&fbc->lock); - __intel_fbc_post_update(crtc); + if (fbc->crtc == crtc) { + fbc->flip_pending = false; + intel_fbc_get_reg_params(fbc, crtc); + __intel_fbc_post_update(fbc); + } mutex_unlock(&fbc->lock); } @@ -1376,7 +1371,7 @@ void intel_fbc_flush(struct drm_i915_private *i915, if (fbc->active) intel_fbc_nuke(fbc); else if (!fbc->flip_pending) - __intel_fbc_post_update(fbc->crtc); + __intel_fbc_post_update(fbc); } out: -- cgit From 004f80f91a7831cd32970e1078bb00594d042089 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:40 +0200 Subject: drm/i915/fbc: Track FBC usage per-plane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the future we may have multiple planes on the same pipe capable of using FBC. Prepare for that by tracking FBC usage per-plane rather than per-crtc. v2: s/intel_get_crtc_for_pipe/intel_crtc_for_pipe/ Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-9-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 220 +++++++++++++++---------------- 1 file changed, 108 insertions(+), 112 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index db390c29c665..cf7fc0de6081 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -583,7 +583,7 @@ static bool intel_fbc_hw_is_active(struct intel_fbc *fbc) static void intel_fbc_hw_activate(struct intel_fbc *fbc) { - trace_intel_fbc_activate(fbc->crtc); + trace_intel_fbc_activate(fbc->plane); fbc->active = true; fbc->activated = true; @@ -593,7 +593,7 @@ static void intel_fbc_hw_activate(struct intel_fbc *fbc) static void intel_fbc_hw_deactivate(struct intel_fbc *fbc) { - trace_intel_fbc_deactivate(fbc->crtc); + trace_intel_fbc_deactivate(fbc->plane); fbc->active = false; @@ -607,7 +607,7 @@ bool intel_fbc_is_compressing(struct intel_fbc *fbc) static void intel_fbc_nuke(struct intel_fbc *fbc) { - trace_intel_fbc_nuke(fbc->crtc); + trace_intel_fbc_nuke(fbc->plane); fbc->funcs->nuke(fbc); } @@ -1154,8 +1154,7 @@ static bool intel_fbc_can_activate(struct intel_fbc *fbc) return true; } -static void intel_fbc_get_reg_params(struct intel_fbc *fbc, - struct intel_crtc *crtc) +static void intel_fbc_get_reg_params(struct intel_fbc *fbc) { const struct intel_fbc_state *cache = &fbc->state_cache; struct intel_fbc_state *params = &fbc->params; @@ -1213,30 +1212,19 @@ static bool intel_fbc_can_flip_nuke(struct intel_atomic_state *state, return true; } -bool intel_fbc_pre_update(struct intel_atomic_state *state, - struct intel_crtc *crtc) +static bool __intel_fbc_pre_update(struct intel_atomic_state *state, + struct intel_crtc *crtc, + struct intel_plane *plane) { - struct intel_plane *plane = to_intel_plane(crtc->base.primary); - 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); + struct drm_i915_private *i915 = to_i915(state->base.dev); struct intel_fbc *fbc = plane->fbc; - const char *reason = "update pending"; bool need_vblank_wait = false; - if (!fbc || !plane_state) - return need_vblank_wait; - - mutex_lock(&fbc->lock); - - if (fbc->crtc != crtc) - goto unlock; - intel_fbc_update_state_cache(state, crtc, plane); fbc->flip_pending = true; if (!intel_fbc_can_flip_nuke(state, crtc, plane)) { - intel_fbc_deactivate(fbc, reason); + intel_fbc_deactivate(fbc, "update pending"); /* * Display WA #1198: glk+ @@ -1256,8 +1244,31 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state, need_vblank_wait = true; fbc->activated = false; } -unlock: - mutex_unlock(&fbc->lock); + + return need_vblank_wait; +} + +bool intel_fbc_pre_update(struct intel_atomic_state *state, + struct intel_crtc *crtc) +{ + const struct intel_plane_state *plane_state; + bool need_vblank_wait = false; + struct intel_plane *plane; + int i; + + for_each_new_intel_plane_in_state(state, plane, plane_state, i) { + struct intel_fbc *fbc = plane->fbc; + + if (!fbc || plane->pipe != crtc->pipe) + continue; + + mutex_lock(&fbc->lock); + + if (fbc->plane == plane) + need_vblank_wait |= __intel_fbc_pre_update(state, crtc, plane); + + mutex_unlock(&fbc->lock); + } return need_vblank_wait; } @@ -1265,18 +1276,18 @@ unlock: static void __intel_fbc_disable(struct intel_fbc *fbc) { struct drm_i915_private *i915 = fbc->i915; - struct intel_crtc *crtc = fbc->crtc; + struct intel_plane *plane = fbc->plane; drm_WARN_ON(&i915->drm, !mutex_is_locked(&fbc->lock)); - drm_WARN_ON(&i915->drm, !fbc->crtc); + drm_WARN_ON(&i915->drm, !fbc->plane); drm_WARN_ON(&i915->drm, fbc->active); - drm_dbg_kms(&i915->drm, "Disabling FBC on pipe %c\n", - pipe_name(crtc->pipe)); + drm_dbg_kms(&i915->drm, "Disabling FBC on [PLANE:%d:%s]\n", + plane->base.base.id, plane->base.name); __intel_fbc_cleanup_cfb(fbc); - fbc->crtc = NULL; + fbc->plane = NULL; } static void __intel_fbc_post_update(struct intel_fbc *fbc) @@ -1304,27 +1315,32 @@ static void __intel_fbc_post_update(struct intel_fbc *fbc) void intel_fbc_post_update(struct intel_atomic_state *state, struct intel_crtc *crtc) { - struct intel_plane *plane = to_intel_plane(crtc->base.primary); - const struct intel_plane_state *plane_state = - intel_atomic_get_new_plane_state(state, plane); - struct intel_fbc *fbc = plane->fbc; + const struct intel_plane_state *plane_state; + struct intel_plane *plane; + int i; - if (!fbc || !plane_state) - return; + for_each_new_intel_plane_in_state(state, plane, plane_state, i) { + struct intel_fbc *fbc = plane->fbc; - mutex_lock(&fbc->lock); - if (fbc->crtc == crtc) { - fbc->flip_pending = false; - intel_fbc_get_reg_params(fbc, crtc); - __intel_fbc_post_update(fbc); + if (!fbc || plane->pipe != crtc->pipe) + continue; + + mutex_lock(&fbc->lock); + + if (fbc->plane == plane) { + fbc->flip_pending = false; + intel_fbc_get_reg_params(fbc); + __intel_fbc_post_update(fbc); + } + + mutex_unlock(&fbc->lock); } - mutex_unlock(&fbc->lock); } static unsigned int intel_fbc_get_frontbuffer_bit(struct intel_fbc *fbc) { - if (fbc->crtc) - return to_intel_plane(fbc->crtc->base.primary)->frontbuffer_bit; + if (fbc->plane) + return fbc->plane->frontbuffer_bit; else return fbc->possible_framebuffer_bits; } @@ -1345,7 +1361,7 @@ void intel_fbc_invalidate(struct drm_i915_private *i915, fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits; - if (fbc->crtc && fbc->busy_bits) + if (fbc->plane && fbc->busy_bits) intel_fbc_deactivate(fbc, "frontbuffer write"); mutex_unlock(&fbc->lock); @@ -1366,7 +1382,7 @@ void intel_fbc_flush(struct drm_i915_private *i915, if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE) goto out; - if (!fbc->busy_bits && fbc->crtc && + if (!fbc->busy_bits && fbc->plane && (frontbuffer_bits & intel_fbc_get_frontbuffer_bit(fbc))) { if (fbc->active) intel_fbc_nuke(fbc); @@ -1395,43 +1411,24 @@ int intel_fbc_atomic_check(struct intel_atomic_state *state) return 0; } -/** - * intel_fbc_enable: tries to enable FBC on the CRTC - * @crtc: the CRTC - * @state: corresponding &drm_crtc_state for @crtc - * - * This function checks if the given CRTC was chosen for FBC, then enables it if - * possible. Notice that it doesn't activate FBC. It is valid to call - * intel_fbc_enable multiple times for the same pipe without an - * intel_fbc_disable in the middle, as long as it is deactivated. - */ -static void intel_fbc_enable(struct intel_atomic_state *state, - struct intel_crtc *crtc) +static void __intel_fbc_enable(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_plane *plane = to_intel_plane(crtc->base.primary); + struct drm_i915_private *i915 = to_i915(state->base.dev); 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; - int min_limit; - - if (!fbc || !plane_state) - return; - - cache = &fbc->state_cache; - - min_limit = intel_fbc_min_limit(plane_state); - - mutex_lock(&fbc->lock); + struct intel_fbc_state *cache = &fbc->state_cache; + int min_limit = intel_fbc_min_limit(plane_state); - if (fbc->crtc) { - if (fbc->crtc != crtc) - goto out; + if (fbc->plane) { + if (fbc->plane != plane) + return; if (fbc->limit >= min_limit && !intel_fbc_cfb_size_changed(fbc)) - goto out; + return; __intel_fbc_disable(fbc); } @@ -1441,22 +1438,20 @@ static void intel_fbc_enable(struct intel_atomic_state *state, intel_fbc_update_state_cache(state, crtc, plane); if (cache->no_fbc_reason) - goto out; + return; if (intel_fbc_alloc_cfb(fbc, intel_fbc_cfb_size(plane_state), min_limit)) { fbc->no_fbc_reason = "not enough stolen memory"; - goto out; + return; } - drm_dbg_kms(&i915->drm, "Enabling FBC on pipe %c\n", - pipe_name(crtc->pipe)); + drm_dbg_kms(&i915->drm, "Enabling FBC on [PLANE:%d:%s]\n", + plane->base.base.id, plane->base.name); fbc->no_fbc_reason = "FBC enabled but not active yet\n"; - fbc->crtc = crtc; + fbc->plane = plane; intel_fbc_program_cfb(fbc); -out: - mutex_unlock(&fbc->lock); } /** @@ -1467,45 +1462,48 @@ out: */ void intel_fbc_disable(struct intel_crtc *crtc) { - struct intel_plane *plane = to_intel_plane(crtc->base.primary); - struct intel_fbc *fbc = plane->fbc; + struct drm_i915_private *i915 = to_i915(crtc->base.dev); + struct intel_plane *plane; - if (!fbc) - return; + for_each_intel_plane(&i915->drm, plane) { + struct intel_fbc *fbc = plane->fbc; - mutex_lock(&fbc->lock); - if (fbc->crtc == crtc) - __intel_fbc_disable(fbc); - mutex_unlock(&fbc->lock); + if (!fbc || plane->pipe != crtc->pipe) + continue; + + mutex_lock(&fbc->lock); + if (fbc->plane == plane) + __intel_fbc_disable(fbc); + mutex_unlock(&fbc->lock); + } } -/** - * intel_fbc_update: enable/disable FBC on the CRTC - * @state: atomic state - * @crtc: the CRTC - * - * This function checks if the given CRTC was chosen for FBC, then enables it if - * possible. Notice that it doesn't activate FBC. It is valid to call - * intel_fbc_update multiple times for the same pipe without an - * intel_fbc_disable in the middle. - */ void intel_fbc_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 intel_fbc *fbc = plane->fbc; + const struct intel_plane_state *plane_state; + struct intel_plane *plane; + int i; - if (!fbc || !plane_state) - return; + for_each_new_intel_plane_in_state(state, plane, plane_state, i) { + struct intel_fbc *fbc = plane->fbc; - if (crtc_state->update_pipe && plane_state->no_fbc_reason) - intel_fbc_disable(crtc); - else - intel_fbc_enable(state, crtc); + if (!fbc || plane->pipe != crtc->pipe) + continue; + + mutex_lock(&fbc->lock); + + if (crtc_state->update_pipe && plane_state->no_fbc_reason) { + if (fbc->plane == plane) + __intel_fbc_disable(fbc); + } else { + __intel_fbc_enable(state, crtc, plane); + } + + mutex_unlock(&fbc->lock); + } } /** @@ -1522,10 +1520,8 @@ void intel_fbc_global_disable(struct drm_i915_private *i915) return; mutex_lock(&fbc->lock); - if (fbc->crtc) { - drm_WARN_ON(&i915->drm, fbc->crtc->active); + if (fbc->plane) __intel_fbc_disable(fbc); - } mutex_unlock(&fbc->lock); } @@ -1538,7 +1534,7 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work) mutex_lock(&fbc->lock); /* Maybe we were scheduled twice. */ - if (fbc->underrun_detected || !fbc->crtc) + if (fbc->underrun_detected || !fbc->plane) goto out; drm_dbg_kms(&i915->drm, "Disabling FBC due to FIFO underrun.\n"); -- cgit From 62d4874bee61d971b74dfd5fcd8032ff33746885 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:41 +0200 Subject: drm/i915/fbc: Flatten __intel_fbc_pre_update() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use an early return to flatten most of __intel_fbc_pre_update(). Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-10-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 42 ++++++++++++++++---------------- 1 file changed, 21 insertions(+), 21 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index cf7fc0de6081..0bef3b948670 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1223,27 +1223,27 @@ static bool __intel_fbc_pre_update(struct intel_atomic_state *state, intel_fbc_update_state_cache(state, crtc, plane); fbc->flip_pending = true; - if (!intel_fbc_can_flip_nuke(state, crtc, plane)) { - intel_fbc_deactivate(fbc, "update pending"); - - /* - * Display WA #1198: glk+ - * Need an extra vblank wait between FBC disable and most plane - * updates. Bspec says this is only needed for plane disable, but - * that is not true. Touching most plane registers will cause the - * corruption to appear. Also SKL/derivatives do not seem to be - * affected. - * - * TODO: could optimize this a bit by sampling the frame - * counter when we disable FBC (if it was already done earlier) - * and skipping the extra vblank wait before the plane update - * if at least one frame has already passed. - */ - if (fbc->activated && - DISPLAY_VER(i915) >= 10) - need_vblank_wait = true; - fbc->activated = false; - } + if (intel_fbc_can_flip_nuke(state, crtc, plane)) + return need_vblank_wait; + + intel_fbc_deactivate(fbc, "update pending"); + + /* + * Display WA #1198: glk+ + * Need an extra vblank wait between FBC disable and most plane + * updates. Bspec says this is only needed for plane disable, but + * that is not true. Touching most plane registers will cause the + * corruption to appear. Also SKL/derivatives do not seem to be + * affected. + * + * TODO: could optimize this a bit by sampling the frame + * counter when we disable FBC (if it was already done earlier) + * and skipping the extra vblank wait before the plane update + * if at least one frame has already passed. + */ + if (fbc->activated && DISPLAY_VER(i915) >= 10) + need_vblank_wait = true; + fbc->activated = false; return need_vblank_wait; } -- cgit From 32024bb85ec2a8475b89282726121b922caebad9 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:42 +0200 Subject: drm/i915/fbc: Pass i915 instead of FBC instance to FBC underrun stuff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The underrun code doesn't need to know any details about FBC, so just pass in the whole device rather than a specific FBC instance. We could make this a bit more fine grained by also passing in the pipe to intel_fbc_handle_fifo_underrun_irq() and letting the FBC code figure which FBC instance (if any) is active on said pipe. But that seems a bit overkill for this so don't bother. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-11-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 0bef3b948670..00c93040529e 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1547,21 +1547,21 @@ out: /* * intel_fbc_reset_underrun - reset FBC fifo underrun status. - * @fbc: The FBC instance + * @i915: the i915 device * * See intel_fbc_handle_fifo_underrun_irq(). For automated testing we * want to re-enable FBC after an underrun to increase test coverage. */ -int intel_fbc_reset_underrun(struct intel_fbc *fbc) +void intel_fbc_reset_underrun(struct drm_i915_private *i915) { - struct drm_i915_private *i915 = fbc->i915; - int ret; + struct intel_fbc *fbc = &i915->fbc; + + if (!HAS_FBC(i915)) + return; cancel_work_sync(&fbc->underrun_work); - ret = mutex_lock_interruptible(&fbc->lock); - if (ret) - return ret; + mutex_lock(&fbc->lock); if (fbc->underrun_detected) { drm_dbg_kms(&i915->drm, @@ -1571,13 +1571,11 @@ int intel_fbc_reset_underrun(struct intel_fbc *fbc) fbc->underrun_detected = false; mutex_unlock(&fbc->lock); - - return 0; } /** * intel_fbc_handle_fifo_underrun_irq - disable FBC when we get a FIFO underrun - * @fbc: The FBC instance + * @i915: i915 device * * Without FBC, most underruns are harmless and don't really cause too many * problems, except for an annoying message on dmesg. With FBC, underruns can @@ -1589,9 +1587,11 @@ int intel_fbc_reset_underrun(struct intel_fbc *fbc) * * This function is called from the IRQ handler. */ -void intel_fbc_handle_fifo_underrun_irq(struct intel_fbc *fbc) +void intel_fbc_handle_fifo_underrun_irq(struct drm_i915_private *i915) { - if (!HAS_FBC(fbc->i915)) + struct intel_fbc *fbc = &i915->fbc; + + if (!HAS_FBC(i915)) return; /* There's no guarantee that underrun_detected won't be set to true -- cgit From d2de8ccfb29909272fce4eb5cb2bca4fd878df39 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:43 +0200 Subject: drm/i915/fbc: Move FBC debugfs stuff into intel_fbc.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to encapsulate FBC harder let's just move the debugfs stuff into intel_fbc.c. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-12-ville.syrjala@linux.intel.com Acked-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_fbc.c | 110 ++++++++++++++++++++++--------- 1 file changed, 79 insertions(+), 31 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 00c93040529e..ee4e3186cc9c 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -600,7 +600,7 @@ static void intel_fbc_hw_deactivate(struct intel_fbc *fbc) fbc->funcs->deactivate(fbc); } -bool intel_fbc_is_compressing(struct intel_fbc *fbc) +static bool intel_fbc_is_compressing(struct intel_fbc *fbc) { return fbc->funcs->is_compressing(fbc); } @@ -612,36 +612,6 @@ static void intel_fbc_nuke(struct intel_fbc *fbc) fbc->funcs->nuke(fbc); } -int intel_fbc_set_false_color(struct intel_fbc *fbc, bool enable) -{ - if (!fbc->funcs || !fbc->funcs->set_false_color) - return -ENODEV; - - mutex_lock(&fbc->lock); - - fbc->false_color = enable; - - fbc->funcs->set_false_color(fbc, enable); - - mutex_unlock(&fbc->lock); - - return 0; -} - -/** - * intel_fbc_is_active - Is FBC active? - * @fbc: The FBC instance - * - * This function is used to verify the current state of FBC. - * - * FIXME: This should be tracked in the plane config eventually - * instead of queried at runtime for most callers. - */ -bool intel_fbc_is_active(struct intel_fbc *fbc) -{ - return fbc->active; -} - static void intel_fbc_activate(struct intel_fbc *fbc) { intel_fbc_hw_activate(fbc); @@ -1691,3 +1661,81 @@ void intel_fbc_init(struct drm_i915_private *i915) if (intel_fbc_hw_is_active(fbc)) intel_fbc_hw_deactivate(fbc); } + +static int intel_fbc_debugfs_status_show(struct seq_file *m, void *unused) +{ + struct intel_fbc *fbc = m->private; + struct drm_i915_private *i915 = fbc->i915; + intel_wakeref_t wakeref; + + wakeref = intel_runtime_pm_get(&i915->runtime_pm); + mutex_lock(&fbc->lock); + + if (fbc->active) { + seq_puts(m, "FBC enabled\n"); + seq_printf(m, "Compressing: %s\n", + yesno(intel_fbc_is_compressing(fbc))); + } else { + seq_printf(m, "FBC disabled: %s\n", fbc->no_fbc_reason); + } + + mutex_unlock(&fbc->lock); + intel_runtime_pm_put(&i915->runtime_pm, wakeref); + + return 0; +} + +DEFINE_SHOW_ATTRIBUTE(intel_fbc_debugfs_status); + +static int intel_fbc_debugfs_false_color_get(void *data, u64 *val) +{ + struct intel_fbc *fbc = data; + + *val = fbc->false_color; + + return 0; +} + +static int intel_fbc_debugfs_false_color_set(void *data, u64 val) +{ + struct intel_fbc *fbc = data; + + mutex_lock(&fbc->lock); + + fbc->false_color = val; + + if (fbc->active) + fbc->funcs->set_false_color(fbc, fbc->false_color); + + mutex_unlock(&fbc->lock); + + return 0; +} + +DEFINE_SIMPLE_ATTRIBUTE(intel_fbc_debugfs_false_color_fops, + intel_fbc_debugfs_false_color_get, + intel_fbc_debugfs_false_color_set, + "%llu\n"); + +static void intel_fbc_debugfs_add(struct intel_fbc *fbc) +{ + struct drm_i915_private *i915 = fbc->i915; + struct drm_minor *minor = i915->drm.primary; + + debugfs_create_file("i915_fbc_status", 0444, + minor->debugfs_root, fbc, + &intel_fbc_debugfs_status_fops); + + if (fbc->funcs->set_false_color) + debugfs_create_file("i915_fbc_false_color", 0644, + minor->debugfs_root, fbc, + &intel_fbc_debugfs_false_color_fops); +} + +void intel_fbc_debugfs_register(struct drm_i915_private *i915) +{ + struct intel_fbc *fbc = &i915->fbc; + + if (HAS_FBC(i915)) + intel_fbc_debugfs_add(fbc); +} -- cgit From 825bd8335e4e9fccf33b93813693409b4484ea68 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:44 +0200 Subject: drm/i915/fbc: Introduce intel_fbc_add_plane() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to better encapsulate the FBC implementation introduce a small helper to do the plane<->FBC instance association. We'll also try to structure the plane init code such that introducing multiple FBC instances will be easier down the line. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-13-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index ee4e3186cc9c..9be8e7dcaab6 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1612,6 +1612,15 @@ static bool need_fbc_vtd_wa(struct drm_i915_private *i915) return false; } +void intel_fbc_add_plane(struct intel_fbc *fbc, struct intel_plane *plane) +{ + if (!fbc) + return; + + plane->fbc = fbc; + fbc->possible_framebuffer_bits |= plane->frontbuffer_bit; +} + /** * intel_fbc_init - Initialize FBC * @i915: the i915 device -- cgit From 606754fdcb20f781774a279d62bb0852fcb2b79d Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:45 +0200 Subject: drm/i915/fbc: Allocate intel_fbc dynamically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the future we may have more than one FBC instance on some platforms. So let's just allocate it dynamically. This also lets us fully hide the implementation from prying eyes. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-14-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 148 +++++++++++++++++++++++-------- 1 file changed, 110 insertions(+), 38 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 9be8e7dcaab6..1daf4f7b5d80 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -59,6 +59,63 @@ struct intel_fbc_funcs { void (*set_false_color)(struct intel_fbc *fbc, bool enable); }; +struct intel_fbc_state { + const char *no_fbc_reason; + enum i9xx_plane_id i9xx_plane; + unsigned int cfb_stride; + unsigned int cfb_size; + unsigned int fence_y_offset; + u16 override_cfb_stride; + u16 interval; + s8 fence_id; +}; + +struct intel_fbc { + struct drm_i915_private *i915; + const struct intel_fbc_funcs *funcs; + + /* + * This is always the inner lock when overlapping with + * struct_mutex and it's the outer lock when overlapping + * with stolen_lock. + */ + struct mutex lock; + unsigned int possible_framebuffer_bits; + unsigned int busy_bits; + struct intel_plane *plane; + + struct drm_mm_node compressed_fb; + struct drm_mm_node compressed_llb; + + u8 limit; + + bool false_color; + + bool active; + bool activated; + bool flip_pending; + + bool underrun_detected; + struct work_struct underrun_work; + + /* + * Due to the atomic rules we can't access some structures without the + * appropriate locking, so we cache information here in order to avoid + * these problems. + */ + struct intel_fbc_state state_cache; + + /* + * This structure contains everything that's relevant to program the + * hardware registers. When we want to figure out if we need to disable + * and re-enable FBC for a new configuration we just check if there's + * something different in the struct. The genx_fbc_activate functions + * are supposed to read from it in order to program the registers. + */ + struct intel_fbc_state params; + const char *no_fbc_reason; +}; + /* plane stride in pixels */ static unsigned int intel_fbc_plane_stride(const struct intel_plane_state *plane_state) { @@ -762,14 +819,16 @@ static void __intel_fbc_cleanup_cfb(struct intel_fbc *fbc) void intel_fbc_cleanup(struct drm_i915_private *i915) { - struct intel_fbc *fbc = &i915->fbc; + struct intel_fbc *fbc = i915->fbc; - if (!HAS_FBC(i915)) + if (!fbc) return; mutex_lock(&fbc->lock); __intel_fbc_cleanup_cfb(fbc); mutex_unlock(&fbc->lock); + + kfree(fbc); } static bool stride_is_valid(const struct intel_plane_state *plane_state) @@ -1319,9 +1378,9 @@ void intel_fbc_invalidate(struct drm_i915_private *i915, unsigned int frontbuffer_bits, enum fb_op_origin origin) { - struct intel_fbc *fbc = &i915->fbc; + struct intel_fbc *fbc = i915->fbc; - if (!HAS_FBC(i915)) + if (!fbc) return; if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE) @@ -1340,9 +1399,9 @@ void intel_fbc_invalidate(struct drm_i915_private *i915, void intel_fbc_flush(struct drm_i915_private *i915, unsigned int frontbuffer_bits, enum fb_op_origin origin) { - struct intel_fbc *fbc = &i915->fbc; + struct intel_fbc *fbc = i915->fbc; - if (!HAS_FBC(i915)) + if (!fbc) return; mutex_lock(&fbc->lock); @@ -1484,9 +1543,9 @@ void intel_fbc_update(struct intel_atomic_state *state, */ void intel_fbc_global_disable(struct drm_i915_private *i915) { - struct intel_fbc *fbc = &i915->fbc; + struct intel_fbc *fbc = i915->fbc; - if (!HAS_FBC(i915)) + if (!fbc) return; mutex_lock(&fbc->lock); @@ -1497,9 +1556,8 @@ void intel_fbc_global_disable(struct drm_i915_private *i915) static void intel_fbc_underrun_work_fn(struct work_struct *work) { - struct drm_i915_private *i915 = - container_of(work, struct drm_i915_private, fbc.underrun_work); - struct intel_fbc *fbc = &i915->fbc; + struct intel_fbc *fbc = container_of(work, typeof(*fbc), underrun_work); + struct drm_i915_private *i915 = fbc->i915; mutex_lock(&fbc->lock); @@ -1524,9 +1582,9 @@ out: */ void intel_fbc_reset_underrun(struct drm_i915_private *i915) { - struct intel_fbc *fbc = &i915->fbc; + struct intel_fbc *fbc = i915->fbc; - if (!HAS_FBC(i915)) + if (!fbc) return; cancel_work_sync(&fbc->underrun_work); @@ -1559,9 +1617,9 @@ void intel_fbc_reset_underrun(struct drm_i915_private *i915) */ void intel_fbc_handle_fifo_underrun_irq(struct drm_i915_private *i915) { - struct intel_fbc *fbc = &i915->fbc; + struct intel_fbc *fbc = i915->fbc; - if (!HAS_FBC(i915)) + if (!fbc) return; /* There's no guarantee that underrun_detected won't be set to true @@ -1621,6 +1679,34 @@ void intel_fbc_add_plane(struct intel_fbc *fbc, struct intel_plane *plane) fbc->possible_framebuffer_bits |= plane->frontbuffer_bit; } +static struct intel_fbc *intel_fbc_create(struct drm_i915_private *i915) +{ + struct intel_fbc *fbc; + + fbc = kzalloc(sizeof(*fbc), GFP_KERNEL); + if (!fbc) + return NULL; + + fbc->i915 = i915; + INIT_WORK(&fbc->underrun_work, intel_fbc_underrun_work_fn); + mutex_init(&fbc->lock); + + if (DISPLAY_VER(i915) >= 7) + fbc->funcs = &ivb_fbc_funcs; + else if (DISPLAY_VER(i915) == 6) + fbc->funcs = &snb_fbc_funcs; + else if (DISPLAY_VER(i915) == 5) + fbc->funcs = &ilk_fbc_funcs; + else if (IS_G4X(i915)) + fbc->funcs = &g4x_fbc_funcs; + else if (DISPLAY_VER(i915) == 4) + fbc->funcs = &i965_fbc_funcs; + else + fbc->funcs = &i8xx_fbc_funcs; + + return fbc; +} + /** * intel_fbc_init - Initialize FBC * @i915: the i915 device @@ -1629,12 +1715,7 @@ void intel_fbc_add_plane(struct intel_fbc *fbc, struct intel_plane *plane) */ void intel_fbc_init(struct drm_i915_private *i915) { - struct intel_fbc *fbc = &i915->fbc; - - fbc->i915 = i915; - INIT_WORK(&fbc->underrun_work, intel_fbc_underrun_work_fn); - mutex_init(&fbc->lock); - fbc->active = false; + struct intel_fbc *fbc; if (!drm_mm_initialized(&i915->mm.stolen)) mkwrite_device_info(i915)->display.has_fbc = false; @@ -1646,29 +1727,20 @@ void intel_fbc_init(struct drm_i915_private *i915) drm_dbg_kms(&i915->drm, "Sanitized enable_fbc value: %d\n", i915->params.enable_fbc); - if (!HAS_FBC(i915)) { - fbc->no_fbc_reason = "unsupported by this chipset"; + if (!HAS_FBC(i915)) return; - } - if (DISPLAY_VER(i915) >= 7) - fbc->funcs = &ivb_fbc_funcs; - else if (DISPLAY_VER(i915) == 6) - fbc->funcs = &snb_fbc_funcs; - else if (DISPLAY_VER(i915) == 5) - fbc->funcs = &ilk_fbc_funcs; - else if (IS_G4X(i915)) - fbc->funcs = &g4x_fbc_funcs; - else if (DISPLAY_VER(i915) == 4) - fbc->funcs = &i965_fbc_funcs; - else - fbc->funcs = &i8xx_fbc_funcs; + fbc = intel_fbc_create(i915); + if (!fbc) + return; /* We still don't have any sort of hardware state readout for FBC, so * deactivate it in case the BIOS activated it to make sure software * matches the hardware state. */ if (intel_fbc_hw_is_active(fbc)) intel_fbc_hw_deactivate(fbc); + + i915->fbc = fbc; } static int intel_fbc_debugfs_status_show(struct seq_file *m, void *unused) @@ -1743,8 +1815,8 @@ static void intel_fbc_debugfs_add(struct intel_fbc *fbc) void intel_fbc_debugfs_register(struct drm_i915_private *i915) { - struct intel_fbc *fbc = &i915->fbc; + struct intel_fbc *fbc = i915->fbc; - if (HAS_FBC(i915)) + if (fbc) intel_fbc_debugfs_add(fbc); } -- cgit From 98009fd73bde2d66fb449cd277f69932fd12051d Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:46 +0200 Subject: drm/i915/fbc: Move stuff from intel_fbc_can_enable() into intel_fbc_check_plane() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't really see a good reason why we can't just do the vgpu and modparam checks already in intel_fbc_check_plane(). Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-15-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 1daf4f7b5d80..616ab95766b2 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -996,18 +996,6 @@ static bool intel_fbc_cfb_size_changed(struct intel_fbc *fbc) static bool intel_fbc_can_enable(struct intel_fbc *fbc) { - struct drm_i915_private *i915 = fbc->i915; - - if (intel_vgpu_active(i915)) { - fbc->no_fbc_reason = "VGPU is active"; - return false; - } - - if (!i915->params.enable_fbc) { - fbc->no_fbc_reason = "disabled per module param or by default"; - return false; - } - if (fbc->underrun_detected) { fbc->no_fbc_reason = "underrun detected"; return false; @@ -1030,6 +1018,16 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, if (!fbc) return 0; + if (intel_vgpu_active(i915)) { + plane_state->no_fbc_reason = "VGPU active"; + return 0; + } + + if (!i915->params.enable_fbc) { + plane_state->no_fbc_reason = "disabled per module param or by default"; + return 0; + } + if (!plane_state->uapi.visible) { plane_state->no_fbc_reason = "plane not visible"; return 0; -- cgit From b156def9912fe6d9fd7679c9843f80cfcd9d1429 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:47 +0200 Subject: drm/i915/fbc: Disable FBC fully on FIFO underrun MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently a FIFO underrun just causes FBC to be deactivated, and later checks then prevent it from being reactivated. We can simpify our lives a bit by logically disabling FBC on FIFO underruns. This avoids the funny intermediate state where FBC is logically enabled but can't actually be activated. v2: intel_wait_for_vblank() is no more Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-16-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 616ab95766b2..1f8ff7ab2dec 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -994,16 +994,6 @@ static bool intel_fbc_cfb_size_changed(struct intel_fbc *fbc) return fbc->state_cache.cfb_size > fbc->compressed_fb.size * fbc->limit; } -static bool intel_fbc_can_enable(struct intel_fbc *fbc) -{ - if (fbc->underrun_detected) { - fbc->no_fbc_reason = "underrun detected"; - return false; - } - - return true; -} - static int intel_fbc_check_plane(struct intel_atomic_state *state, struct intel_plane *plane) { @@ -1123,22 +1113,11 @@ static bool intel_fbc_can_activate(struct intel_fbc *fbc) struct drm_i915_private *i915 = fbc->i915; struct intel_fbc_state *cache = &fbc->state_cache; - if (!intel_fbc_can_enable(fbc)) - return false; - if (cache->no_fbc_reason) { fbc->no_fbc_reason = cache->no_fbc_reason; return false; } - /* We don't need to use a state cache here since this information is - * global for all CRTC. - */ - if (fbc->underrun_detected) { - fbc->no_fbc_reason = "underrun detected"; - return false; - } - /* The use of a CPU fence is one of two ways to detect writes by the * CPU to the scanout and trigger updates to the FBC. * @@ -1467,6 +1446,11 @@ static void __intel_fbc_enable(struct intel_atomic_state *state, if (cache->no_fbc_reason) return; + if (fbc->underrun_detected) { + fbc->no_fbc_reason = "FIFO underrun"; + return; + } + if (intel_fbc_alloc_cfb(fbc, intel_fbc_cfb_size(plane_state), min_limit)) { fbc->no_fbc_reason = "not enough stolen memory"; return; @@ -1567,6 +1551,9 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work) fbc->underrun_detected = true; intel_fbc_deactivate(fbc, "FIFO underrun"); + if (!fbc->flip_pending) + intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(i915, fbc->plane->pipe)); + __intel_fbc_disable(fbc); out: mutex_unlock(&fbc->lock); } -- cgit From f4cfdbb02ca8227cf4de454071f20cdd09c37cf2 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:48 +0200 Subject: drm/i915/fbc: Nuke state_cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fbc->state_cache has now become useless. We can simply update the reg params directly from the plane/crtc states during __intel_fbc_enable(). Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-17-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 169 ++++++++++++------------------- 1 file changed, 62 insertions(+), 107 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 1f8ff7ab2dec..badffe409814 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -60,7 +60,6 @@ struct intel_fbc_funcs { }; struct intel_fbc_state { - const char *no_fbc_reason; enum i9xx_plane_id i9xx_plane; unsigned int cfb_stride; unsigned int cfb_size; @@ -98,13 +97,6 @@ struct intel_fbc { bool underrun_detected; struct work_struct underrun_work; - /* - * Due to the atomic rules we can't access some structures without the - * appropriate locking, so we cache information here in order to avoid - * these problems. - */ - struct intel_fbc_state state_cache; - /* * This structure contains everything that's relevant to program the * hardware registers. When we want to figure out if we need to disable @@ -673,6 +665,8 @@ static void intel_fbc_activate(struct intel_fbc *fbc) { intel_fbc_hw_activate(fbc); intel_fbc_nuke(fbc); + + fbc->no_fbc_reason = NULL; } static void intel_fbc_deactivate(struct intel_fbc *fbc, const char *reason) @@ -714,9 +708,7 @@ static u64 intel_fbc_stolen_end(struct drm_i915_private *i915) static int intel_fbc_min_limit(const struct intel_plane_state *plane_state) { - int fb_cpp = plane_state->hw.fb ? plane_state->hw.fb->format->cpp[0] : 0; - - return fb_cpp == 2 ? 2 : 1; + return plane_state->hw.fb->format->cpp[0] == 2 ? 2 : 1; } static int intel_fbc_max_limit(struct drm_i915_private *i915) @@ -962,10 +954,9 @@ static void intel_fbc_update_state_cache(struct intel_atomic_state *state, 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 = &fbc->state_cache; + struct intel_fbc_state *cache = &fbc->params; - cache->no_fbc_reason = plane_state->no_fbc_reason; - if (cache->no_fbc_reason) + if (plane_state->no_fbc_reason) return; cache->i9xx_plane = plane->i9xx_plane; @@ -989,9 +980,46 @@ static void intel_fbc_update_state_cache(struct intel_atomic_state *state, cache->override_cfb_stride = intel_fbc_override_cfb_stride(plane_state); } -static bool intel_fbc_cfb_size_changed(struct intel_fbc *fbc) +static bool intel_fbc_is_fence_ok(const struct intel_plane_state *plane_state) +{ + struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev); + + /* The use of a CPU fence is one of two ways to detect writes by the + * CPU to the scanout and trigger updates to the FBC. + * + * The other method is by software tracking (see + * intel_fbc_invalidate/flush()), it will manually notify FBC and nuke + * the current compressed buffer and recompress it. + * + * Note that is possible for a tiled surface to be unmappable (and + * so have no fence associated with it) due to aperture constraints + * at the time of pinning. + * + * FIXME with 90/270 degree rotation we should use the fence on + * the normal GTT view (the rotated view doesn't even have a + * fence). Would need changes to the FBC fence Y offset as well. + * For now this will effectively disable FBC with 90/270 degree + * rotation. + */ + return DISPLAY_VER(i915) >= 9 || + (plane_state->flags & PLANE_HAS_FENCE && + plane_state->ggtt_vma->fence); +} + +static bool intel_fbc_is_cfb_ok(const struct intel_plane_state *plane_state) +{ + struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane); + struct intel_fbc *fbc = plane->fbc; + + return intel_fbc_min_limit(plane_state) <= fbc->limit && + intel_fbc_cfb_size(plane_state) <= fbc->compressed_fb.size * fbc->limit; +} + +static bool intel_fbc_is_ok(const struct intel_plane_state *plane_state) { - return fbc->state_cache.cfb_size > fbc->compressed_fb.size * fbc->limit; + return !plane_state->no_fbc_reason && + intel_fbc_is_fence_ok(plane_state) && + intel_fbc_is_cfb_ok(plane_state); } static int intel_fbc_check_plane(struct intel_atomic_state *state, @@ -1108,74 +1136,11 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, return 0; } -static bool intel_fbc_can_activate(struct intel_fbc *fbc) -{ - struct drm_i915_private *i915 = fbc->i915; - struct intel_fbc_state *cache = &fbc->state_cache; - - if (cache->no_fbc_reason) { - fbc->no_fbc_reason = cache->no_fbc_reason; - return false; - } - - /* The use of a CPU fence is one of two ways to detect writes by the - * CPU to the scanout and trigger updates to the FBC. - * - * The other method is by software tracking (see - * intel_fbc_invalidate/flush()), it will manually notify FBC and nuke - * the current compressed buffer and recompress it. - * - * Note that is possible for a tiled surface to be unmappable (and - * so have no fence associated with it) due to aperture constraints - * at the time of pinning. - * - * FIXME with 90/270 degree rotation we should use the fence on - * the normal GTT view (the rotated view doesn't even have a - * fence). Would need changes to the FBC fence Y offset as well. - * For now this will effectively disable FBC with 90/270 degree - * rotation. - */ - if (DISPLAY_VER(i915) < 9 && cache->fence_id < 0) { - fbc->no_fbc_reason = "framebuffer not tiled or fenced"; - return false; - } - - /* - * It is possible for the required CFB size change without a - * crtc->disable + crtc->enable since it is possible to change the - * stride without triggering a full modeset. Since we try to - * over-allocate the CFB, there's a chance we may keep FBC enabled even - * if this happens, but if we exceed the current CFB size we'll have to - * disable FBC. Notice that it would be possible to disable FBC, wait - * for a frame, free the stolen node, then try to reenable FBC in case - * we didn't get any invalidate/deactivate calls, but this would require - * a lot of tracking just for a specific case. If we conclude it's an - * important case, we can implement it later. - */ - if (intel_fbc_cfb_size_changed(fbc)) { - fbc->no_fbc_reason = "CFB requirements changed"; - return false; - } - - return true; -} - -static void intel_fbc_get_reg_params(struct intel_fbc *fbc) -{ - const struct intel_fbc_state *cache = &fbc->state_cache; - struct intel_fbc_state *params = &fbc->params; - - /* Since all our fields are integer types, use memset here so the - * comparison function can rely on memcmp because the padding will be - * zero. */ - *params = *cache; -} static bool intel_fbc_can_flip_nuke(struct intel_atomic_state *state, struct intel_crtc *crtc, struct intel_plane *plane) { - 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 = @@ -1184,16 +1149,12 @@ static bool intel_fbc_can_flip_nuke(struct intel_atomic_state *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 = &fbc->state_cache; - const struct intel_fbc_state *params = &fbc->params; if (drm_atomic_crtc_needs_modeset(&new_crtc_state->uapi)) return false; - if (!intel_fbc_can_activate(fbc)) - return false; - - if (!old_fb || !new_fb) + if (!intel_fbc_is_ok(old_plane_state) || + !intel_fbc_is_ok(new_plane_state)) return false; if (old_fb->format->format != new_fb->format->format) @@ -1206,13 +1167,16 @@ static bool intel_fbc_can_flip_nuke(struct intel_atomic_state *state, intel_fbc_plane_stride(new_plane_state)) return false; - if (params->cfb_stride != cache->cfb_stride) + if (intel_fbc_cfb_stride(old_plane_state) != + intel_fbc_cfb_stride(new_plane_state)) return false; - if (params->cfb_size != cache->cfb_size) + if (intel_fbc_cfb_size(old_plane_state) != + intel_fbc_cfb_size(new_plane_state)) return false; - if (params->override_cfb_stride != cache->override_cfb_stride) + if (intel_fbc_override_cfb_stride(old_plane_state) != + intel_fbc_override_cfb_stride(new_plane_state)) return false; return true; @@ -1226,7 +1190,6 @@ static bool __intel_fbc_pre_update(struct intel_atomic_state *state, struct intel_fbc *fbc = plane->fbc; bool need_vblank_wait = false; - intel_fbc_update_state_cache(state, crtc, plane); fbc->flip_pending = true; if (intel_fbc_can_flip_nuke(state, crtc, plane)) @@ -1302,16 +1265,6 @@ static void __intel_fbc_post_update(struct intel_fbc *fbc) drm_WARN_ON(&i915->drm, !mutex_is_locked(&fbc->lock)); - if (!i915->params.enable_fbc) { - intel_fbc_deactivate(fbc, "disabled at runtime per module param"); - __intel_fbc_disable(fbc); - - return; - } - - if (!intel_fbc_can_activate(fbc)) - return; - if (!fbc->busy_bits) intel_fbc_activate(fbc); else @@ -1335,7 +1288,6 @@ void intel_fbc_post_update(struct intel_atomic_state *state, if (fbc->plane == plane) { fbc->flip_pending = false; - intel_fbc_get_reg_params(fbc); __intel_fbc_post_update(fbc); } @@ -1425,15 +1377,12 @@ static void __intel_fbc_enable(struct intel_atomic_state *state, 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 = &fbc->state_cache; - int min_limit = intel_fbc_min_limit(plane_state); if (fbc->plane) { if (fbc->plane != plane) return; - if (fbc->limit >= min_limit && - !intel_fbc_cfb_size_changed(fbc)) + if (intel_fbc_is_ok(plane_state)) return; __intel_fbc_disable(fbc); @@ -1441,17 +1390,22 @@ static void __intel_fbc_enable(struct intel_atomic_state *state, drm_WARN_ON(&i915->drm, fbc->active); - intel_fbc_update_state_cache(state, crtc, plane); + fbc->no_fbc_reason = plane_state->no_fbc_reason; + if (fbc->no_fbc_reason) + return; - if (cache->no_fbc_reason) + if (!intel_fbc_is_fence_ok(plane_state)) { + fbc->no_fbc_reason = "framebuffer not fenced"; return; + } if (fbc->underrun_detected) { fbc->no_fbc_reason = "FIFO underrun"; return; } - if (intel_fbc_alloc_cfb(fbc, intel_fbc_cfb_size(plane_state), min_limit)) { + if (intel_fbc_alloc_cfb(fbc, intel_fbc_cfb_size(plane_state), + intel_fbc_min_limit(plane_state))) { fbc->no_fbc_reason = "not enough stolen memory"; return; } @@ -1460,6 +1414,7 @@ static void __intel_fbc_enable(struct intel_atomic_state *state, plane->base.base.id, plane->base.name); fbc->no_fbc_reason = "FBC enabled but not active yet\n"; + intel_fbc_update_state_cache(state, crtc, plane); fbc->plane = plane; intel_fbc_program_cfb(fbc); -- cgit From 0cb9f228bc2b3871fd1fcef87897f0a5af959343 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:49 +0200 Subject: drm/i915/fbc: Move plane pointer into intel_fbc_state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we track the FBC plane as a pointer under intel_fbc and also as a i9xx_plane_id under intel_fbc_state. Just store the pointer once in the fbc state. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-18-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 54 +++++++++++++++----------------- 1 file changed, 26 insertions(+), 28 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index badffe409814..695240eb0bbe 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -60,7 +60,7 @@ struct intel_fbc_funcs { }; struct intel_fbc_state { - enum i9xx_plane_id i9xx_plane; + struct intel_plane *plane; unsigned int cfb_stride; unsigned int cfb_size; unsigned int fence_y_offset; @@ -81,7 +81,6 @@ struct intel_fbc { struct mutex lock; unsigned int possible_framebuffer_bits; unsigned int busy_bits; - struct intel_plane *plane; struct drm_mm_node compressed_fb; struct drm_mm_node compressed_llb; @@ -244,7 +243,7 @@ static u32 i965_fbc_ctl2(struct intel_fbc *fbc) u32 fbc_ctl2; fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | - FBC_CTL_PLANE(params->i9xx_plane); + FBC_CTL_PLANE(params->plane->i9xx_plane); if (params->fence_id >= 0) fbc_ctl2 |= FBC_CTL_CPU_FENCE_EN; @@ -308,7 +307,7 @@ static bool i8xx_fbc_is_compressing(struct intel_fbc *fbc) static void i8xx_fbc_nuke(struct intel_fbc *fbc) { struct intel_fbc_state *params = &fbc->params; - enum i9xx_plane_id i9xx_plane = params->i9xx_plane; + enum i9xx_plane_id i9xx_plane = params->plane->i9xx_plane; struct drm_i915_private *dev_priv = fbc->i915; spin_lock_irq(&dev_priv->uncore.lock); @@ -344,7 +343,7 @@ static const struct intel_fbc_funcs i8xx_fbc_funcs = { static void i965_fbc_nuke(struct intel_fbc *fbc) { struct intel_fbc_state *params = &fbc->params; - enum i9xx_plane_id i9xx_plane = params->i9xx_plane; + enum i9xx_plane_id i9xx_plane = params->plane->i9xx_plane; struct drm_i915_private *dev_priv = fbc->i915; spin_lock_irq(&dev_priv->uncore.lock); @@ -384,7 +383,7 @@ static u32 g4x_dpfc_ctl(struct intel_fbc *fbc) u32 dpfc_ctl; dpfc_ctl = g4x_dpfc_ctl_limit(fbc) | - DPFC_CTL_PLANE_G4X(params->i9xx_plane); + DPFC_CTL_PLANE_G4X(params->plane->i9xx_plane); if (IS_G4X(i915)) dpfc_ctl |= DPFC_CTL_SR_EN; @@ -576,7 +575,7 @@ static u32 ivb_dpfc_ctl(struct intel_fbc *fbc) dpfc_ctl = g4x_dpfc_ctl_limit(fbc); if (IS_IVYBRIDGE(i915)) - dpfc_ctl |= DPFC_CTL_PLANE_IVB(params->i9xx_plane); + dpfc_ctl |= DPFC_CTL_PLANE_IVB(params->plane->i9xx_plane); if (params->fence_id >= 0) dpfc_ctl |= DPFC_CTL_FENCE_EN_IVB; @@ -632,7 +631,7 @@ static bool intel_fbc_hw_is_active(struct intel_fbc *fbc) static void intel_fbc_hw_activate(struct intel_fbc *fbc) { - trace_intel_fbc_activate(fbc->plane); + trace_intel_fbc_activate(fbc->params.plane); fbc->active = true; fbc->activated = true; @@ -642,7 +641,7 @@ static void intel_fbc_hw_activate(struct intel_fbc *fbc) static void intel_fbc_hw_deactivate(struct intel_fbc *fbc) { - trace_intel_fbc_deactivate(fbc->plane); + trace_intel_fbc_deactivate(fbc->params.plane); fbc->active = false; @@ -656,7 +655,7 @@ static bool intel_fbc_is_compressing(struct intel_fbc *fbc) static void intel_fbc_nuke(struct intel_fbc *fbc) { - trace_intel_fbc_nuke(fbc->plane); + trace_intel_fbc_nuke(fbc->params.plane); fbc->funcs->nuke(fbc); } @@ -959,7 +958,7 @@ static void intel_fbc_update_state_cache(struct intel_atomic_state *state, if (plane_state->no_fbc_reason) return; - cache->i9xx_plane = plane->i9xx_plane; + cache->plane = plane; /* FBC1 compression interval: arbitrary choice of 1 second */ cache->interval = drm_mode_vrefresh(&crtc_state->hw.adjusted_mode); @@ -1233,7 +1232,7 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state, mutex_lock(&fbc->lock); - if (fbc->plane == plane) + if (fbc->params.plane == plane) need_vblank_wait |= __intel_fbc_pre_update(state, crtc, plane); mutex_unlock(&fbc->lock); @@ -1245,10 +1244,10 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state, static void __intel_fbc_disable(struct intel_fbc *fbc) { struct drm_i915_private *i915 = fbc->i915; - struct intel_plane *plane = fbc->plane; + struct intel_plane *plane = fbc->params.plane; drm_WARN_ON(&i915->drm, !mutex_is_locked(&fbc->lock)); - drm_WARN_ON(&i915->drm, !fbc->plane); + drm_WARN_ON(&i915->drm, !fbc->params.plane); drm_WARN_ON(&i915->drm, fbc->active); drm_dbg_kms(&i915->drm, "Disabling FBC on [PLANE:%d:%s]\n", @@ -1256,7 +1255,7 @@ static void __intel_fbc_disable(struct intel_fbc *fbc) __intel_fbc_cleanup_cfb(fbc); - fbc->plane = NULL; + fbc->params.plane = NULL; } static void __intel_fbc_post_update(struct intel_fbc *fbc) @@ -1286,7 +1285,7 @@ void intel_fbc_post_update(struct intel_atomic_state *state, mutex_lock(&fbc->lock); - if (fbc->plane == plane) { + if (fbc->params.plane == plane) { fbc->flip_pending = false; __intel_fbc_post_update(fbc); } @@ -1297,8 +1296,8 @@ void intel_fbc_post_update(struct intel_atomic_state *state, static unsigned int intel_fbc_get_frontbuffer_bit(struct intel_fbc *fbc) { - if (fbc->plane) - return fbc->plane->frontbuffer_bit; + if (fbc->params.plane) + return fbc->params.plane->frontbuffer_bit; else return fbc->possible_framebuffer_bits; } @@ -1319,7 +1318,7 @@ void intel_fbc_invalidate(struct drm_i915_private *i915, fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits; - if (fbc->plane && fbc->busy_bits) + if (fbc->params.plane && fbc->busy_bits) intel_fbc_deactivate(fbc, "frontbuffer write"); mutex_unlock(&fbc->lock); @@ -1340,7 +1339,7 @@ void intel_fbc_flush(struct drm_i915_private *i915, if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE) goto out; - if (!fbc->busy_bits && fbc->plane && + if (!fbc->busy_bits && fbc->params.plane && (frontbuffer_bits & intel_fbc_get_frontbuffer_bit(fbc))) { if (fbc->active) intel_fbc_nuke(fbc); @@ -1378,8 +1377,8 @@ static void __intel_fbc_enable(struct intel_atomic_state *state, intel_atomic_get_new_plane_state(state, plane); struct intel_fbc *fbc = plane->fbc; - if (fbc->plane) { - if (fbc->plane != plane) + if (fbc->params.plane) { + if (fbc->params.plane != plane) return; if (intel_fbc_is_ok(plane_state)) @@ -1415,7 +1414,6 @@ static void __intel_fbc_enable(struct intel_atomic_state *state, fbc->no_fbc_reason = "FBC enabled but not active yet\n"; intel_fbc_update_state_cache(state, crtc, plane); - fbc->plane = plane; intel_fbc_program_cfb(fbc); } @@ -1438,7 +1436,7 @@ void intel_fbc_disable(struct intel_crtc *crtc) continue; mutex_lock(&fbc->lock); - if (fbc->plane == plane) + if (fbc->params.plane == plane) __intel_fbc_disable(fbc); mutex_unlock(&fbc->lock); } @@ -1462,7 +1460,7 @@ void intel_fbc_update(struct intel_atomic_state *state, mutex_lock(&fbc->lock); if (crtc_state->update_pipe && plane_state->no_fbc_reason) { - if (fbc->plane == plane) + if (fbc->params.plane == plane) __intel_fbc_disable(fbc); } else { __intel_fbc_enable(state, crtc, plane); @@ -1486,7 +1484,7 @@ void intel_fbc_global_disable(struct drm_i915_private *i915) return; mutex_lock(&fbc->lock); - if (fbc->plane) + if (fbc->params.plane) __intel_fbc_disable(fbc); mutex_unlock(&fbc->lock); } @@ -1499,7 +1497,7 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work) mutex_lock(&fbc->lock); /* Maybe we were scheduled twice. */ - if (fbc->underrun_detected || !fbc->plane) + if (fbc->underrun_detected || !fbc->params.plane) goto out; drm_dbg_kms(&i915->drm, "Disabling FBC due to FIFO underrun.\n"); @@ -1507,7 +1505,7 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work) intel_fbc_deactivate(fbc, "FIFO underrun"); if (!fbc->flip_pending) - intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(i915, fbc->plane->pipe)); + intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(i915, fbc->params.plane->pipe)); __intel_fbc_disable(fbc); out: mutex_unlock(&fbc->lock); -- cgit From d3e27f7c511044c65b27d087e55b092a3d97e8d7 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:50 +0200 Subject: drm/i915/fbc: s/parms/fbc_state/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename the 'params' to just fbc state. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-19-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 138 +++++++++++++++---------------- 1 file changed, 68 insertions(+), 70 deletions(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 695240eb0bbe..ba994c11eddd 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -103,7 +103,7 @@ struct intel_fbc { * something different in the struct. The genx_fbc_activate functions * are supposed to read from it in order to program the registers. */ - struct intel_fbc_state params; + struct intel_fbc_state state; const char *no_fbc_reason; }; @@ -211,12 +211,12 @@ static u16 intel_fbc_override_cfb_stride(const struct intel_plane_state *plane_s static u32 i8xx_fbc_ctl(struct intel_fbc *fbc) { - const struct intel_fbc_state *params = &fbc->params; + const struct intel_fbc_state *fbc_state = &fbc->state; struct drm_i915_private *i915 = fbc->i915; unsigned int cfb_stride; u32 fbc_ctl; - cfb_stride = params->cfb_stride / fbc->limit; + cfb_stride = fbc_state->cfb_stride / fbc->limit; /* FBC_CTL wants 32B or 64B units */ if (DISPLAY_VER(i915) == 2) @@ -225,27 +225,27 @@ static u32 i8xx_fbc_ctl(struct intel_fbc *fbc) cfb_stride = (cfb_stride / 64) - 1; fbc_ctl = FBC_CTL_PERIODIC | - FBC_CTL_INTERVAL(params->interval) | + FBC_CTL_INTERVAL(fbc_state->interval) | FBC_CTL_STRIDE(cfb_stride); if (IS_I945GM(i915)) fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */ - if (params->fence_id >= 0) - fbc_ctl |= FBC_CTL_FENCENO(params->fence_id); + if (fbc_state->fence_id >= 0) + fbc_ctl |= FBC_CTL_FENCENO(fbc_state->fence_id); return fbc_ctl; } static u32 i965_fbc_ctl2(struct intel_fbc *fbc) { - const struct intel_fbc_state *params = &fbc->params; + const struct intel_fbc_state *fbc_state = &fbc->state; u32 fbc_ctl2; fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | - FBC_CTL_PLANE(params->plane->i9xx_plane); + FBC_CTL_PLANE(fbc_state->plane->i9xx_plane); - if (params->fence_id >= 0) + if (fbc_state->fence_id >= 0) fbc_ctl2 |= FBC_CTL_CPU_FENCE_EN; return fbc_ctl2; @@ -274,7 +274,7 @@ static void i8xx_fbc_deactivate(struct intel_fbc *fbc) static void i8xx_fbc_activate(struct intel_fbc *fbc) { - const struct intel_fbc_state *params = &fbc->params; + const struct intel_fbc_state *fbc_state = &fbc->state; struct drm_i915_private *i915 = fbc->i915; int i; @@ -286,7 +286,7 @@ static void i8xx_fbc_activate(struct intel_fbc *fbc) intel_de_write(i915, FBC_CONTROL2, i965_fbc_ctl2(fbc)); intel_de_write(i915, FBC_FENCE_OFF, - params->fence_y_offset); + fbc_state->fence_y_offset); } intel_de_write(i915, FBC_CONTROL, @@ -306,8 +306,8 @@ static bool i8xx_fbc_is_compressing(struct intel_fbc *fbc) static void i8xx_fbc_nuke(struct intel_fbc *fbc) { - struct intel_fbc_state *params = &fbc->params; - enum i9xx_plane_id i9xx_plane = params->plane->i9xx_plane; + struct intel_fbc_state *fbc_state = &fbc->state; + enum i9xx_plane_id i9xx_plane = fbc_state->plane->i9xx_plane; struct drm_i915_private *dev_priv = fbc->i915; spin_lock_irq(&dev_priv->uncore.lock); @@ -342,8 +342,8 @@ static const struct intel_fbc_funcs i8xx_fbc_funcs = { static void i965_fbc_nuke(struct intel_fbc *fbc) { - struct intel_fbc_state *params = &fbc->params; - enum i9xx_plane_id i9xx_plane = params->plane->i9xx_plane; + struct intel_fbc_state *fbc_state = &fbc->state; + enum i9xx_plane_id i9xx_plane = fbc_state->plane->i9xx_plane; struct drm_i915_private *dev_priv = fbc->i915; spin_lock_irq(&dev_priv->uncore.lock); @@ -378,21 +378,21 @@ static u32 g4x_dpfc_ctl_limit(struct intel_fbc *fbc) static u32 g4x_dpfc_ctl(struct intel_fbc *fbc) { - const struct intel_fbc_state *params = &fbc->params; + const struct intel_fbc_state *fbc_state = &fbc->state; struct drm_i915_private *i915 = fbc->i915; u32 dpfc_ctl; dpfc_ctl = g4x_dpfc_ctl_limit(fbc) | - DPFC_CTL_PLANE_G4X(params->plane->i9xx_plane); + DPFC_CTL_PLANE_G4X(fbc_state->plane->i9xx_plane); if (IS_G4X(i915)) dpfc_ctl |= DPFC_CTL_SR_EN; - if (params->fence_id >= 0) { + if (fbc_state->fence_id >= 0) { dpfc_ctl |= DPFC_CTL_FENCE_EN_G4X; if (DISPLAY_VER(i915) < 6) - dpfc_ctl |= DPFC_CTL_FENCENO(params->fence_id); + dpfc_ctl |= DPFC_CTL_FENCENO(fbc_state->fence_id); } return dpfc_ctl; @@ -400,11 +400,11 @@ static u32 g4x_dpfc_ctl(struct intel_fbc *fbc) static void g4x_fbc_activate(struct intel_fbc *fbc) { - const struct intel_fbc_state *params = &fbc->params; + const struct intel_fbc_state *fbc_state = &fbc->state; struct drm_i915_private *i915 = fbc->i915; intel_de_write(i915, DPFC_FENCE_YOFF, - params->fence_y_offset); + fbc_state->fence_y_offset); intel_de_write(i915, DPFC_CONTROL, DPFC_CTL_EN | g4x_dpfc_ctl(fbc)); @@ -451,11 +451,11 @@ static const struct intel_fbc_funcs g4x_fbc_funcs = { static void ilk_fbc_activate(struct intel_fbc *fbc) { - struct intel_fbc_state *params = &fbc->params; + struct intel_fbc_state *fbc_state = &fbc->state; struct drm_i915_private *i915 = fbc->i915; intel_de_write(i915, ILK_DPFC_FENCE_YOFF, - params->fence_y_offset); + fbc_state->fence_y_offset); intel_de_write(i915, ILK_DPFC_CONTROL, DPFC_CTL_EN | g4x_dpfc_ctl(fbc)); @@ -502,15 +502,15 @@ static const struct intel_fbc_funcs ilk_fbc_funcs = { static void snb_fbc_program_fence(struct intel_fbc *fbc) { - const struct intel_fbc_state *params = &fbc->params; + const struct intel_fbc_state *fbc_state = &fbc->state; struct drm_i915_private *i915 = fbc->i915; u32 ctl = 0; - if (params->fence_id >= 0) - ctl = SNB_DPFC_FENCE_EN | SNB_DPFC_FENCENO(params->fence_id); + if (fbc_state->fence_id >= 0) + ctl = SNB_DPFC_FENCE_EN | SNB_DPFC_FENCENO(fbc_state->fence_id); intel_de_write(i915, SNB_DPFC_CTL_SA, ctl); - intel_de_write(i915, SNB_DPFC_CPU_FENCE_OFFSET, params->fence_y_offset); + intel_de_write(i915, SNB_DPFC_CPU_FENCE_OFFSET, fbc_state->fence_y_offset); } static void snb_fbc_activate(struct intel_fbc *fbc) @@ -539,27 +539,27 @@ static const struct intel_fbc_funcs snb_fbc_funcs = { static void glk_fbc_program_cfb_stride(struct intel_fbc *fbc) { - const struct intel_fbc_state *params = &fbc->params; + const struct intel_fbc_state *fbc_state = &fbc->state; struct drm_i915_private *i915 = fbc->i915; u32 val = 0; - if (params->override_cfb_stride) + if (fbc_state->override_cfb_stride) val |= FBC_STRIDE_OVERRIDE | - FBC_STRIDE(params->override_cfb_stride / fbc->limit); + FBC_STRIDE(fbc_state->override_cfb_stride / fbc->limit); intel_de_write(i915, GLK_FBC_STRIDE, val); } static void skl_fbc_program_cfb_stride(struct intel_fbc *fbc) { - const struct intel_fbc_state *params = &fbc->params; + const struct intel_fbc_state *fbc_state = &fbc->state; struct drm_i915_private *i915 = fbc->i915; u32 val = 0; /* Display WA #0529: skl, kbl, bxt. */ - if (params->override_cfb_stride) + if (fbc_state->override_cfb_stride) val |= CHICKEN_FBC_STRIDE_OVERRIDE | - CHICKEN_FBC_STRIDE(params->override_cfb_stride / fbc->limit); + CHICKEN_FBC_STRIDE(fbc_state->override_cfb_stride / fbc->limit); intel_de_rmw(i915, CHICKEN_MISC_4, CHICKEN_FBC_STRIDE_OVERRIDE | @@ -568,16 +568,16 @@ static void skl_fbc_program_cfb_stride(struct intel_fbc *fbc) static u32 ivb_dpfc_ctl(struct intel_fbc *fbc) { - const struct intel_fbc_state *params = &fbc->params; + const struct intel_fbc_state *fbc_state = &fbc->state; struct drm_i915_private *i915 = fbc->i915; u32 dpfc_ctl; dpfc_ctl = g4x_dpfc_ctl_limit(fbc); if (IS_IVYBRIDGE(i915)) - dpfc_ctl |= DPFC_CTL_PLANE_IVB(params->plane->i9xx_plane); + dpfc_ctl |= DPFC_CTL_PLANE_IVB(fbc_state->plane->i9xx_plane); - if (params->fence_id >= 0) + if (fbc_state->fence_id >= 0) dpfc_ctl |= DPFC_CTL_FENCE_EN_IVB; if (fbc->false_color) @@ -631,7 +631,7 @@ static bool intel_fbc_hw_is_active(struct intel_fbc *fbc) static void intel_fbc_hw_activate(struct intel_fbc *fbc) { - trace_intel_fbc_activate(fbc->params.plane); + trace_intel_fbc_activate(fbc->state.plane); fbc->active = true; fbc->activated = true; @@ -641,7 +641,7 @@ static void intel_fbc_hw_activate(struct intel_fbc *fbc) static void intel_fbc_hw_deactivate(struct intel_fbc *fbc) { - trace_intel_fbc_deactivate(fbc->params.plane); + trace_intel_fbc_deactivate(fbc->state.plane); fbc->active = false; @@ -655,7 +655,7 @@ static bool intel_fbc_is_compressing(struct intel_fbc *fbc) static void intel_fbc_nuke(struct intel_fbc *fbc) { - trace_intel_fbc_nuke(fbc->params.plane); + trace_intel_fbc_nuke(fbc->state.plane); fbc->funcs->nuke(fbc); } @@ -943,9 +943,9 @@ static bool tiling_is_valid(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) +static void intel_fbc_update_state(struct intel_atomic_state *state, + struct intel_crtc *crtc, + struct intel_plane *plane) { struct drm_i915_private *i915 = to_i915(state->base.dev); const struct intel_crtc_state *crtc_state = @@ -953,30 +953,29 @@ static void intel_fbc_update_state_cache(struct intel_atomic_state *state, 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 = &fbc->params; + struct intel_fbc_state *fbc_state = &fbc->state; - if (plane_state->no_fbc_reason) - return; + WARN_ON(plane_state->no_fbc_reason); - cache->plane = plane; + fbc_state->plane = plane; /* FBC1 compression interval: arbitrary choice of 1 second */ - cache->interval = drm_mode_vrefresh(&crtc_state->hw.adjusted_mode); + fbc_state->interval = drm_mode_vrefresh(&crtc_state->hw.adjusted_mode); - cache->fence_y_offset = intel_plane_fence_y_offset(plane_state); + fbc_state->fence_y_offset = intel_plane_fence_y_offset(plane_state); drm_WARN_ON(&i915->drm, plane_state->flags & PLANE_HAS_FENCE && !plane_state->ggtt_vma->fence); if (plane_state->flags & PLANE_HAS_FENCE && plane_state->ggtt_vma->fence) - cache->fence_id = plane_state->ggtt_vma->fence->id; + fbc_state->fence_id = plane_state->ggtt_vma->fence->id; else - cache->fence_id = -1; + fbc_state->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); + fbc_state->cfb_stride = intel_fbc_cfb_stride(plane_state); + fbc_state->cfb_size = intel_fbc_cfb_size(plane_state); + fbc_state->override_cfb_stride = intel_fbc_override_cfb_stride(plane_state); } static bool intel_fbc_is_fence_ok(const struct intel_plane_state *plane_state) @@ -1232,7 +1231,7 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state, mutex_lock(&fbc->lock); - if (fbc->params.plane == plane) + if (fbc->state.plane == plane) need_vblank_wait |= __intel_fbc_pre_update(state, crtc, plane); mutex_unlock(&fbc->lock); @@ -1244,10 +1243,9 @@ bool intel_fbc_pre_update(struct intel_atomic_state *state, static void __intel_fbc_disable(struct intel_fbc *fbc) { struct drm_i915_private *i915 = fbc->i915; - struct intel_plane *plane = fbc->params.plane; + struct intel_plane *plane = fbc->state.plane; drm_WARN_ON(&i915->drm, !mutex_is_locked(&fbc->lock)); - drm_WARN_ON(&i915->drm, !fbc->params.plane); drm_WARN_ON(&i915->drm, fbc->active); drm_dbg_kms(&i915->drm, "Disabling FBC on [PLANE:%d:%s]\n", @@ -1255,7 +1253,7 @@ static void __intel_fbc_disable(struct intel_fbc *fbc) __intel_fbc_cleanup_cfb(fbc); - fbc->params.plane = NULL; + fbc->state.plane = NULL; } static void __intel_fbc_post_update(struct intel_fbc *fbc) @@ -1285,7 +1283,7 @@ void intel_fbc_post_update(struct intel_atomic_state *state, mutex_lock(&fbc->lock); - if (fbc->params.plane == plane) { + if (fbc->state.plane == plane) { fbc->flip_pending = false; __intel_fbc_post_update(fbc); } @@ -1296,8 +1294,8 @@ void intel_fbc_post_update(struct intel_atomic_state *state, static unsigned int intel_fbc_get_frontbuffer_bit(struct intel_fbc *fbc) { - if (fbc->params.plane) - return fbc->params.plane->frontbuffer_bit; + if (fbc->state.plane) + return fbc->state.plane->frontbuffer_bit; else return fbc->possible_framebuffer_bits; } @@ -1318,7 +1316,7 @@ void intel_fbc_invalidate(struct drm_i915_private *i915, fbc->busy_bits |= intel_fbc_get_frontbuffer_bit(fbc) & frontbuffer_bits; - if (fbc->params.plane && fbc->busy_bits) + if (fbc->state.plane && fbc->busy_bits) intel_fbc_deactivate(fbc, "frontbuffer write"); mutex_unlock(&fbc->lock); @@ -1339,7 +1337,7 @@ void intel_fbc_flush(struct drm_i915_private *i915, if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE) goto out; - if (!fbc->busy_bits && fbc->params.plane && + if (!fbc->busy_bits && fbc->state.plane && (frontbuffer_bits & intel_fbc_get_frontbuffer_bit(fbc))) { if (fbc->active) intel_fbc_nuke(fbc); @@ -1377,8 +1375,8 @@ static void __intel_fbc_enable(struct intel_atomic_state *state, intel_atomic_get_new_plane_state(state, plane); struct intel_fbc *fbc = plane->fbc; - if (fbc->params.plane) { - if (fbc->params.plane != plane) + if (fbc->state.plane) { + if (fbc->state.plane != plane) return; if (intel_fbc_is_ok(plane_state)) @@ -1413,7 +1411,7 @@ static void __intel_fbc_enable(struct intel_atomic_state *state, plane->base.base.id, plane->base.name); fbc->no_fbc_reason = "FBC enabled but not active yet\n"; - intel_fbc_update_state_cache(state, crtc, plane); + intel_fbc_update_state(state, crtc, plane); intel_fbc_program_cfb(fbc); } @@ -1436,7 +1434,7 @@ void intel_fbc_disable(struct intel_crtc *crtc) continue; mutex_lock(&fbc->lock); - if (fbc->params.plane == plane) + if (fbc->state.plane == plane) __intel_fbc_disable(fbc); mutex_unlock(&fbc->lock); } @@ -1460,7 +1458,7 @@ void intel_fbc_update(struct intel_atomic_state *state, mutex_lock(&fbc->lock); if (crtc_state->update_pipe && plane_state->no_fbc_reason) { - if (fbc->params.plane == plane) + if (fbc->state.plane == plane) __intel_fbc_disable(fbc); } else { __intel_fbc_enable(state, crtc, plane); @@ -1484,7 +1482,7 @@ void intel_fbc_global_disable(struct drm_i915_private *i915) return; mutex_lock(&fbc->lock); - if (fbc->params.plane) + if (fbc->state.plane) __intel_fbc_disable(fbc); mutex_unlock(&fbc->lock); } @@ -1497,7 +1495,7 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work) mutex_lock(&fbc->lock); /* Maybe we were scheduled twice. */ - if (fbc->underrun_detected || !fbc->params.plane) + if (fbc->underrun_detected || !fbc->state.plane) goto out; drm_dbg_kms(&i915->drm, "Disabling FBC due to FIFO underrun.\n"); @@ -1505,7 +1503,7 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work) intel_fbc_deactivate(fbc, "FIFO underrun"); if (!fbc->flip_pending) - intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(i915, fbc->params.plane->pipe)); + intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(i915, fbc->state.plane->pipe)); __intel_fbc_disable(fbc); out: mutex_unlock(&fbc->lock); -- cgit From d5ba72f3c18e4556d99bb0360279d0b1e9544359 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:51 +0200 Subject: drm/i915/fbc: No FBC+double wide pipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FBC and double wide pipe are mutually exclusive. Disable FBC when we have to resort to double wide. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-20-ville.syrjala@linux.intel.com Reviewed-by: Mika Kahola --- drivers/gpu/drm/i915/display/intel_fbc.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index ba994c11eddd..58a5d4430fa6 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1056,6 +1056,11 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, return 0; } + if (crtc_state->double_wide) { + plane_state->no_fbc_reason = "double wide pipe not supported"; + return 0; + } + /* * Display 12+ is not supporting FBC with PSR2. * Recommendation is to keep this combination disabled -- cgit From 812e338619f166d3ab864123b2572523f6e4916a Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 24 Nov 2021 13:36:52 +0200 Subject: drm/i915/fbc: Pimp the FBC debugfs output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that each plane tracks its own no_fbc_reason we can print that out in debugfs, and we can also show which plane is currently selected for FBC duty. Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-21-ville.syrjala@linux.intel.com Reviewed-by: Jani Nikula --- drivers/gpu/drm/i915/display/intel_fbc.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 58a5d4430fa6..f5d16c1b5b3c 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1688,8 +1688,11 @@ static int intel_fbc_debugfs_status_show(struct seq_file *m, void *unused) { struct intel_fbc *fbc = m->private; struct drm_i915_private *i915 = fbc->i915; + struct intel_plane *plane; intel_wakeref_t wakeref; + drm_modeset_lock_all(&i915->drm); + wakeref = intel_runtime_pm_get(&i915->runtime_pm); mutex_lock(&fbc->lock); @@ -1701,9 +1704,24 @@ static int intel_fbc_debugfs_status_show(struct seq_file *m, void *unused) seq_printf(m, "FBC disabled: %s\n", fbc->no_fbc_reason); } + for_each_intel_plane(&i915->drm, plane) { + const struct intel_plane_state *plane_state = + to_intel_plane_state(plane->base.state); + + if (plane->fbc != fbc) + continue; + + seq_printf(m, "%c [PLANE:%d:%s]: %s\n", + fbc->state.plane == plane ? '*' : ' ', + plane->base.base.id, plane->base.name, + plane_state->no_fbc_reason ?: "FBC possible"); + } + mutex_unlock(&fbc->lock); intel_runtime_pm_put(&i915->runtime_pm, wakeref); + drm_modeset_unlock_all(&i915->drm); + return 0; } -- cgit From fd2b94a5cb0ff4bb163cdc4afaede6527eec5f7e Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Wed, 8 Dec 2021 13:05:17 +0200 Subject: drm/i915/trace: split out display trace to a separate file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add display/intel_display_trace.[ch] for defining display tracepoints. The main goal is to reduce cross-includes between gem and display. It would be possible split up tracing even further, but that would lead to more boilerplate. We end up having to include intel_crtc.h in a few places because it was pulled in implicitly via intel_de.h -> i915_trace.h -> intel_crtc.h, and that's no longer the case. There should be no changes to tracepoints. v3: - Rebase v2: - Define TRACE_INCLUDE_PATH relative to define_trace.h (Chris) - Remove useless comments (Ville) Cc: Ville Syrjälä Signed-off-by: Jani Nikula Reviewed-by: Ville Syrjälä Reviewed-by: Lucas De Marchi Link: https://patchwork.freedesktop.org/patch/msgid/7862ad764fbd0748d903c76bc632d3d277874e5b.1638961423.git.jani.nikula@intel.com --- drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/i915/display/intel_fbc.c') diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index f5d16c1b5b3c..b33941c9e089 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -41,10 +41,10 @@ #include #include "i915_drv.h" -#include "i915_trace.h" #include "i915_vgpu.h" #include "intel_cdclk.h" #include "intel_de.h" +#include "intel_display_trace.h" #include "intel_display_types.h" #include "intel_fbc.h" #include "intel_frontbuffer.h" -- cgit