From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Bunch of stuff in intel_color_check() needs to look at both the
old and new crtc states. Currently we do that by digging the
full atomic state via the crtc_state->state pointer. That thing
is a total footgun if I ever saw one, as it's only valid during
specific parts of the atomic flow. A lot of people have been
bitten by this thing in the past when trying to use it after
it's no longer valid.
Take a small step towards elimination of the footgun by not
using it in the inte_color_check(). Instead we plumb in the
entire atomic state all the way from the top.
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/display/intel_color.c | 111 +++++++++++--------
drivers/gpu/drm/i915/display/intel_color.h | 4 +-
drivers/gpu/drm/i915/display/intel_display.c | 2 +-
3 files changed, 69 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 82b155708422..ede628b58a5c 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -30,7 +30,8 @@
#include "intel_dsb.h"
struct intel_color_funcs {
- int (*color_check)(struct intel_crtc_state *crtc_state);
+ int (*color_check)(struct intel_atomic_state *state,
+ struct intel_crtc *crtc);
/*
* Program non-arming double buffered color management registers
* before vblank evasion. The registers should then latch after
@@ -1942,11 +1943,9 @@ bool intel_color_uses_dsb(const struct intel_crtc_state *crtc_state)
return crtc_state->dsb;
}
-static bool intel_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
+static bool intel_can_preload_luts(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
- struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
- struct intel_atomic_state *state =
- to_intel_atomic_state(new_crtc_state->uapi.state);
const struct intel_crtc_state *old_crtc_state =
intel_atomic_get_old_crtc_state(state, crtc);
@@ -1954,11 +1953,9 @@ static bool intel_can_preload_luts(const struct intel_crtc_state *new_crtc_state
!old_crtc_state->pre_csc_lut;
}
-static bool vlv_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
+static bool vlv_can_preload_luts(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
- struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
- struct intel_atomic_state *state =
- to_intel_atomic_state(new_crtc_state->uapi.state);
const struct intel_crtc_state *old_crtc_state =
intel_atomic_get_old_crtc_state(state, crtc);
@@ -1966,13 +1963,13 @@ static bool vlv_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
!old_crtc_state->post_csc_lut;
}
-static bool chv_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
+static bool chv_can_preload_luts(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
- struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
- struct intel_atomic_state *state =
- to_intel_atomic_state(new_crtc_state->uapi.state);
const struct intel_crtc_state *old_crtc_state =
intel_atomic_get_old_crtc_state(state, crtc);
+ const struct intel_crtc_state *new_crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
/*
* CGM_PIPE_MODE is itself single buffered. We'd have to
@@ -1982,14 +1979,15 @@ static bool chv_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
if (old_crtc_state->cgm_mode || new_crtc_state->cgm_mode)
return false;
- return vlv_can_preload_luts(new_crtc_state);
+ return vlv_can_preload_luts(state, crtc);
}
-int intel_color_check(struct intel_crtc_state *crtc_state)
+int intel_color_check(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
- struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+ struct drm_i915_private *i915 = to_i915(state->base.dev);
- return i915->display.funcs.color->color_check(crtc_state);
+ return i915->display.funcs.color->color_check(state, crtc);
}
void intel_color_get_config(struct intel_crtc_state *crtc_state)
@@ -2039,14 +2037,14 @@ static bool need_plane_update(struct intel_plane *plane,
}
static int
-intel_color_add_affected_planes(struct intel_crtc_state *new_crtc_state)
+intel_color_add_affected_planes(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
- struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
- struct drm_i915_private *i915 = to_i915(crtc->base.dev);
- struct intel_atomic_state *state =
- to_intel_atomic_state(new_crtc_state->uapi.state);
+ struct drm_i915_private *i915 = to_i915(state->base.dev);
const struct intel_crtc_state *old_crtc_state =
intel_atomic_get_old_crtc_state(state, crtc);
+ struct intel_crtc_state *new_crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
struct intel_plane *plane;
if (!new_crtc_state->hw.active ||
@@ -2240,9 +2238,12 @@ static void intel_assign_luts(struct intel_crtc_state *crtc_state)
crtc_state->hw.gamma_lut);
}
-static int i9xx_color_check(struct intel_crtc_state *crtc_state)
+static int i9xx_color_check(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
- struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+ struct drm_i915_private *i915 = to_i915(state->base.dev);
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
int ret;
ret = check_luts(crtc_state);
@@ -2262,13 +2263,13 @@ static int i9xx_color_check(struct intel_crtc_state *crtc_state)
return ret;
}
- ret = intel_color_add_affected_planes(crtc_state);
+ ret = intel_color_add_affected_planes(state, crtc);
if (ret)
return ret;
intel_assign_luts(crtc_state);
- crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
+ crtc_state->preload_luts = intel_can_preload_luts(state, crtc);
return 0;
}
@@ -2277,8 +2278,11 @@ static int i9xx_color_check(struct intel_crtc_state *crtc_state)
* VLV color pipeline:
* u0.10 -> WGC csc -> u0.10 -> pipe gamma -> u0.10
*/
-static int vlv_color_check(struct intel_crtc_state *crtc_state)
+static int vlv_color_check(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
int ret;
ret = check_luts(crtc_state);
@@ -2293,7 +2297,7 @@ static int vlv_color_check(struct intel_crtc_state *crtc_state)
crtc_state->wgc_enable = crtc_state->hw.ctm;
- ret = intel_color_add_affected_planes(crtc_state);
+ ret = intel_color_add_affected_planes(state, crtc);
if (ret)
return ret;
@@ -2301,7 +2305,7 @@ static int vlv_color_check(struct intel_crtc_state *crtc_state)
vlv_assign_csc(crtc_state);
- crtc_state->preload_luts = vlv_can_preload_luts(crtc_state);
+ crtc_state->preload_luts = vlv_can_preload_luts(state, crtc);
return 0;
}
@@ -2336,8 +2340,11 @@ static u32 chv_cgm_mode(const struct intel_crtc_state *crtc_state)
* We always bypass the WGC csc and use the CGM csc
* instead since it has degamma and better precision.
*/
-static int chv_color_check(struct intel_crtc_state *crtc_state)
+static int chv_color_check(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
int ret;
ret = check_luts(crtc_state);
@@ -2362,7 +2369,7 @@ static int chv_color_check(struct intel_crtc_state *crtc_state)
*/
crtc_state->wgc_enable = false;
- ret = intel_color_add_affected_planes(crtc_state);
+ ret = intel_color_add_affected_planes(state, crtc);
if (ret)
return ret;
@@ -2370,7 +2377,7 @@ static int chv_color_check(struct intel_crtc_state *crtc_state)
chv_assign_csc(crtc_state);
- crtc_state->preload_luts = chv_can_preload_luts(crtc_state);
+ crtc_state->preload_luts = chv_can_preload_luts(state, crtc);
return 0;
}
@@ -2454,9 +2461,12 @@ static int ilk_assign_luts(struct intel_crtc_state *crtc_state)
return 0;
}
-static int ilk_color_check(struct intel_crtc_state *crtc_state)
+static int ilk_color_check(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
- struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+ struct drm_i915_private *i915 = to_i915(state->base.dev);
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
int ret;
ret = check_luts(crtc_state);
@@ -2484,7 +2494,7 @@ static int ilk_color_check(struct intel_crtc_state *crtc_state)
crtc_state->csc_mode = ilk_csc_mode(crtc_state);
- ret = intel_color_add_affected_planes(crtc_state);
+ ret = intel_color_add_affected_planes(state, crtc);
if (ret)
return ret;
@@ -2494,7 +2504,7 @@ static int ilk_color_check(struct intel_crtc_state *crtc_state)
ilk_assign_csc(crtc_state);
- crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
+ crtc_state->preload_luts = intel_can_preload_luts(state, crtc);
return 0;
}
@@ -2555,9 +2565,12 @@ static int ivb_assign_luts(struct intel_crtc_state *crtc_state)
return 0;
}
-static int ivb_color_check(struct intel_crtc_state *crtc_state)
+static int ivb_color_check(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
- struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+ struct drm_i915_private *i915 = to_i915(state->base.dev);
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
int ret;
ret = check_luts(crtc_state);
@@ -2592,7 +2605,7 @@ static int ivb_color_check(struct intel_crtc_state *crtc_state)
crtc_state->csc_mode = ivb_csc_mode(crtc_state);
- ret = intel_color_add_affected_planes(crtc_state);
+ ret = intel_color_add_affected_planes(state, crtc);
if (ret)
return ret;
@@ -2602,7 +2615,7 @@ static int ivb_color_check(struct intel_crtc_state *crtc_state)
ilk_assign_csc(crtc_state);
- crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
+ crtc_state->preload_luts = intel_can_preload_luts(state, crtc);
return 0;
}
@@ -2686,9 +2699,12 @@ static int glk_check_luts(const struct intel_crtc_state *crtc_state)
return _check_luts(crtc_state, degamma_tests, gamma_tests);
}
-static int glk_color_check(struct intel_crtc_state *crtc_state)
+static int glk_color_check(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
- struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+ struct drm_i915_private *i915 = to_i915(state->base.dev);
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
int ret;
ret = glk_check_luts(crtc_state);
@@ -2725,7 +2741,7 @@ static int glk_color_check(struct intel_crtc_state *crtc_state)
crtc_state->csc_mode = 0;
- ret = intel_color_add_affected_planes(crtc_state);
+ ret = intel_color_add_affected_planes(state, crtc);
if (ret)
return ret;
@@ -2735,7 +2751,7 @@ static int glk_color_check(struct intel_crtc_state *crtc_state)
ilk_assign_csc(crtc_state);
- crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
+ crtc_state->preload_luts = intel_can_preload_luts(state, crtc);
return 0;
}
@@ -2783,8 +2799,11 @@ static u32 icl_csc_mode(const struct intel_crtc_state *crtc_state)
return csc_mode;
}
-static int icl_color_check(struct intel_crtc_state *crtc_state)
+static int icl_color_check(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
{
+ struct intel_crtc_state *crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
int ret;
ret = check_luts(crtc_state);
@@ -2799,7 +2818,7 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
icl_assign_csc(crtc_state);
- crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
+ crtc_state->preload_luts = intel_can_preload_luts(state, crtc);
return 0;
}
diff --git a/drivers/gpu/drm/i915/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h
index 8ecd36149def..21ba4aa02e7b 100644
--- a/drivers/gpu/drm/i915/display/intel_color.h
+++ b/drivers/gpu/drm/i915/display/intel_color.h
@@ -8,6 +8,7 @@
#include <linux/types.h>
+struct intel_atomic_state;
struct intel_crtc_state;
struct intel_crtc;
struct drm_i915_private;
@@ -16,7 +17,8 @@ struct drm_property_blob;
void intel_color_init_hooks(struct drm_i915_private *i915);
int intel_color_init(struct drm_i915_private *i915);
void intel_color_crtc_init(struct intel_crtc *crtc);
-int intel_color_check(struct intel_crtc_state *crtc_state);
+int intel_color_check(struct intel_atomic_state *state,
+ struct intel_crtc *crtc);
void intel_color_prepare_commit(struct intel_crtc_state *crtc_state);
void intel_color_cleanup_commit(struct intel_crtc_state *crtc_state);
bool intel_color_uses_dsb(const struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 1e8e2fd52cf6..3b2765b371f8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4255,7 +4255,7 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
crtc_state->uapi.color_mgmt_changed = true;
if (intel_crtc_needs_color_update(crtc_state)) {
- ret = intel_color_check(crtc_state);
+ ret = intel_color_check(state, crtc);
if (ret)
return ret;
}
--
2.44.1
