Commit 2e6c99f8 authored by Ville Syrjälä's avatar Ville Syrjälä

drm/i915/fbc: Nuke lots of crap from intel_fbc_state_cache

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: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20211124113652.22090-4-ville.syrjala@linux.intel.comReviewed-by: default avatarMika Kahola <mika.kahola@intel.com>
parent b6e201f5
......@@ -8047,7 +8047,6 @@ static int intel_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
intel_fbc_choose_crtc(dev_priv, state);
ret = intel_compute_global_watermarks(state);
if (ret)
goto fail;
......@@ -8079,6 +8078,10 @@ static int intel_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
ret = intel_fbc_atomic_check(state);
if (ret)
goto fail;
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
if (new_crtc_state->uapi.async_flip) {
......
......@@ -695,6 +695,8 @@ struct intel_plane_state {
/* Clear Color Value */
u64 ccval;
const char *no_fbc_reason;
};
struct intel_initial_plane_config {
......@@ -1125,8 +1127,6 @@ struct intel_crtc_state {
bool crc_enabled;
bool enable_fbc;
bool double_wide;
int pbn;
......
......@@ -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 @@ void intel_fbc_flush(struct drm_i915_private *i915,
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);
......
......@@ -17,8 +17,7 @@ struct intel_crtc_state;
struct intel_fbc;
struct intel_plane_state;
void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
struct intel_atomic_state *state);
int intel_fbc_atomic_check(struct intel_atomic_state *state);
bool intel_fbc_is_active(struct intel_fbc *fbc);
bool intel_fbc_is_compressing(struct intel_fbc *fbc);
bool intel_fbc_pre_update(struct intel_atomic_state *state,
......
......@@ -413,26 +413,11 @@ struct intel_fbc {
* these problems.
*/
struct intel_fbc_state_cache {
struct {
unsigned int mode_flags;
u32 hsw_bdw_pixel_rate;
} crtc;
const char *no_fbc_reason;
struct {
unsigned int rotation;
int src_w;
int src_h;
bool visible;
/*
* Display surface base address adjustement for
* pageflips. Note that on gen4+ this only adjusts up
* to a tile, offsets within a tile are handled in
* the hw itself (with the TILEOFF register).
*/
int adjusted_x;
int adjusted_y;
u16 pixel_blend_mode;
} plane;
struct {
......@@ -444,7 +429,6 @@ struct intel_fbc {
unsigned int fence_y_offset;
u16 interval;
s8 fence_id;
bool psr2_active;
} state_cache;
/*
......@@ -456,7 +440,6 @@ struct intel_fbc {
*/
struct intel_fbc_reg_params {
struct {
enum pipe pipe;
enum i9xx_plane_id i9xx_plane;
} crtc;
......@@ -472,7 +455,6 @@ struct intel_fbc {
u16 override_cfb_stride;
u16 interval;
s8 fence_id;
bool plane_visible;
} params;
const char *no_fbc_reason;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment