* [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8
@ 2019-02-05 16:08 Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 01/13] drm/i915: Split the gamma/csc enable bits from the plane_ctl() function Ville Syrjala
` (16 more replies)
0 siblings, 17 replies; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Rebase of the gamma/csc series. The last patch is new to fix Matt's
observation that we need something to configure the DSPCNTR gamma/csc
bits correctly during modeset.
Ville Syrjälä (13):
drm/i915: Split the gamma/csc enable bits from the plane_ctl()
function
drm/i915: Precompute gamma_mode
drm/i915: Constify the state arguments to the color management stuff
drm/i915: Pull GAMMA_MODE write out from haswell_load_luts()
drm/i915: Split color mgmt based on single vs. double buffered
registers
drm/i915: Move LUT programming to happen after vblank waits
drm/i915: Populate gamma_mode for all platforms
drm/i915: Track pipe gamma enable/disable in crtc state
drm/i915: Track pipe csc enable in crtc state
drm/i915: Turn off pipe gamma when it's not needed
drm/i915: Turn off pipe CSC when it's not needed
drm/i915: Disable pipe gamma when C8 pixel format is used
drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable()
drivers/gpu/drm/i915/i915_drv.h | 16 +-
drivers/gpu/drm/i915/i915_reg.h | 14 +-
drivers/gpu/drm/i915/intel_atomic_plane.c | 5 +
drivers/gpu/drm/i915/intel_color.c | 381 ++++++++++++++--------
drivers/gpu/drm/i915/intel_display.c | 283 ++++++++++++----
drivers/gpu/drm/i915/intel_drv.h | 14 +-
drivers/gpu/drm/i915/intel_sprite.c | 67 +++-
7 files changed, 568 insertions(+), 212 deletions(-)
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 01/13] drm/i915: Split the gamma/csc enable bits from the plane_ctl() function
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 02/13] drm/i915: Precompute gamma_mode Ville Syrjala
` (15 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
On g4x+ the pipe gamma enable bit for the primary plane affects
the pipe bottom color as well. The same for the pipe csc enable
bit on ilk+. Thus we must configure those bits correctly even
when the primary plane is disabled.
To make the feasible let's split those settings from the
plane_ctl() function into a seprate funciton that we can
call from the ->disable_plane() hook as well.
For consistency we'll do that on all the plane types. While
that has no real benefits at this time, it'll become useful
when we start to control the pipe gamma/csc enable bits
dynamically when we overhaul the color management code.
On pre-g4x there doesn't appear to be any way to gamma
correct the pipe bottom color, but sticking to the same
pattern doesn't hurt. And it'll still help us to do
crtc state readout correctly for the pipe gamma enable
bit for the color management overhaul.
An alternative apporach would be to still precompute these
bits into plane_state->ctl, but that would require that we
run through the plane check even when the plane isn't logically
enabled on any crtc. Currently that condition causes us to
short circuit the entire thing and not call ->check_plane().
There would also be some chicken and egg problems with
->check_plane() vs. crtc color state check that would
requite splitting certain things into multiple steps.
So all in all this seems like the easier route.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 128 ++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_sprite.c | 54 ++++++++---
3 files changed, 139 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index df7a7a310f2f..37fdd15c598c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3215,28 +3215,38 @@ i9xx_plane_max_stride(struct intel_plane *plane,
}
}
+static u32 i9xx_plane_ctl_crtc(const struct intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ u32 dspcntr = 0;
+
+ dspcntr |= DISPPLANE_GAMMA_ENABLE;
+
+ if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+ dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
+
+ if (INTEL_GEN(dev_priv) < 5)
+ dspcntr |= DISPPLANE_SEL_PIPE(crtc->pipe);
+
+ return dspcntr;
+}
+
static u32 i9xx_plane_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
struct drm_i915_private *dev_priv =
to_i915(plane_state->base.plane->dev);
- struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
const struct drm_framebuffer *fb = plane_state->base.fb;
unsigned int rotation = plane_state->base.rotation;
u32 dspcntr;
- dspcntr = DISPLAY_PLANE_ENABLE | DISPPLANE_GAMMA_ENABLE;
+ dspcntr = DISPLAY_PLANE_ENABLE;
if (IS_G4X(dev_priv) || IS_GEN(dev_priv, 5) ||
IS_GEN(dev_priv, 6) || IS_IVYBRIDGE(dev_priv))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
- if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
- dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
-
- if (INTEL_GEN(dev_priv) < 5)
- dspcntr |= DISPPLANE_SEL_PIPE(crtc->pipe);
-
switch (fb->format->format) {
case DRM_FORMAT_C8:
dspcntr |= DISPPLANE_8BPP;
@@ -3364,11 +3374,13 @@ static void i9xx_update_plane(struct intel_plane *plane,
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
u32 linear_offset;
- u32 dspcntr = plane_state->ctl;
int x = plane_state->color_plane[0].x;
int y = plane_state->color_plane[0].y;
unsigned long irqflags;
u32 dspaddr_offset;
+ u32 dspcntr;
+
+ dspcntr = plane_state->ctl | i9xx_plane_ctl_crtc(crtc_state);
linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
@@ -3428,10 +3440,23 @@ static void i9xx_disable_plane(struct intel_plane *plane,
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
unsigned long irqflags;
+ u32 dspcntr;
+
+ /*
+ * DSPCNTR pipe gamma enable on g4x+ and pipe csc
+ * enable on ilk+ affect the pipe bottom color as
+ * well, so we must configure them even if the plane
+ * is disabled.
+ *
+ * On pre-g4x there is no way to gamma correct the
+ * pipe bottom color but we'll keep on doing this
+ * anyway.
+ */
+ dspcntr = i9xx_plane_ctl_crtc(crtc_state);
spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
- I915_WRITE_FW(DSPCNTR(i9xx_plane), 0);
+ I915_WRITE_FW(DSPCNTR(i9xx_plane), dspcntr);
if (INTEL_GEN(dev_priv) >= 4)
I915_WRITE_FW(DSPSURF(i9xx_plane), 0);
else
@@ -3668,6 +3693,20 @@ static u32 cnl_plane_ctl_flip(unsigned int reflect)
return 0;
}
+u32 skl_plane_ctl_crtc(const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+ u32 plane_ctl = 0;
+
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+ return plane_ctl;
+
+ plane_ctl |= PLANE_CTL_PIPE_GAMMA_ENABLE;
+ plane_ctl |= PLANE_CTL_PIPE_CSC_ENABLE;
+
+ return plane_ctl;
+}
+
u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
@@ -3682,10 +3721,7 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
if (INTEL_GEN(dev_priv) < 10 && !IS_GEMINILAKE(dev_priv)) {
plane_ctl |= skl_plane_ctl_alpha(plane_state);
- plane_ctl |=
- PLANE_CTL_PIPE_GAMMA_ENABLE |
- PLANE_CTL_PIPE_CSC_ENABLE |
- PLANE_CTL_PLANE_GAMMA_DISABLE;
+ plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
plane_ctl |= PLANE_CTL_YUV_TO_RGB_CSC_FORMAT_BT709;
@@ -3710,19 +3746,27 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
return plane_ctl;
}
+u32 glk_plane_color_ctl_crtc(const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+ u32 plane_color_ctl = 0;
+
+ if (INTEL_GEN(dev_priv) >= 11)
+ return plane_color_ctl;
+
+ plane_color_ctl |= PLANE_COLOR_PIPE_GAMMA_ENABLE;
+ plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE;
+
+ return plane_color_ctl;
+}
+
u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
- struct drm_i915_private *dev_priv =
- to_i915(plane_state->base.plane->dev);
const struct drm_framebuffer *fb = plane_state->base.fb;
struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
u32 plane_color_ctl = 0;
- if (INTEL_GEN(dev_priv) < 11) {
- plane_color_ctl |= PLANE_COLOR_PIPE_GAMMA_ENABLE;
- plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE;
- }
plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
@@ -9927,11 +9971,15 @@ i845_cursor_max_stride(struct intel_plane *plane,
return 2048;
}
+static u32 i845_cursor_ctl_crtc(const struct intel_crtc_state *crtc_state)
+{
+ return CURSOR_GAMMA_ENABLE;
+}
+
static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
return CURSOR_ENABLE |
- CURSOR_GAMMA_ENABLE |
CURSOR_FORMAT_ARGB |
CURSOR_STRIDE(plane_state->color_plane[0].stride);
}
@@ -10001,7 +10049,9 @@ static void i845_update_cursor(struct intel_plane *plane,
unsigned int width = plane_state->base.crtc_w;
unsigned int height = plane_state->base.crtc_h;
- cntl = plane_state->ctl;
+ cntl = plane_state->ctl |
+ i845_cursor_ctl_crtc(crtc_state);
+
size = (height << 12) | width;
base = intel_cursor_base(plane_state);
@@ -10068,27 +10118,36 @@ i9xx_cursor_max_stride(struct intel_plane *plane,
return plane->base.dev->mode_config.cursor_width * 4;
}
-static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
- const struct intel_plane_state *plane_state)
+static u32 i9xx_cursor_ctl_crtc(const struct intel_crtc_state *crtc_state)
{
- struct drm_i915_private *dev_priv =
- to_i915(plane_state->base.plane->dev);
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
u32 cntl = 0;
- if (IS_GEN(dev_priv, 6) || IS_IVYBRIDGE(dev_priv))
- cntl |= MCURSOR_TRICKLE_FEED_DISABLE;
+ if (INTEL_GEN(dev_priv) >= 11)
+ return cntl;
- if (INTEL_GEN(dev_priv) <= 10) {
- cntl |= MCURSOR_GAMMA_ENABLE;
+ cntl |= MCURSOR_GAMMA_ENABLE;
- if (HAS_DDI(dev_priv))
- cntl |= MCURSOR_PIPE_CSC_ENABLE;
- }
+ if (HAS_DDI(dev_priv))
+ cntl |= MCURSOR_PIPE_CSC_ENABLE;
if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv))
cntl |= MCURSOR_PIPE_SELECT(crtc->pipe);
+ return cntl;
+}
+
+static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state *plane_state)
+{
+ struct drm_i915_private *dev_priv =
+ to_i915(plane_state->base.plane->dev);
+ u32 cntl = 0;
+
+ if (IS_GEN(dev_priv, 6) || IS_IVYBRIDGE(dev_priv))
+ cntl |= MCURSOR_TRICKLE_FEED_DISABLE;
+
switch (plane_state->base.crtc_w) {
case 64:
cntl |= MCURSOR_MODE_64_ARGB_AX;
@@ -10213,7 +10272,8 @@ static void i9xx_update_cursor(struct intel_plane *plane,
unsigned long irqflags;
if (plane_state && plane_state->base.visible) {
- cntl = plane_state->ctl;
+ cntl = plane_state->ctl |
+ i9xx_cursor_ctl_crtc(crtc_state);
if (plane_state->base.crtc_h != plane_state->base.crtc_w)
fbc_ctl = CUR_FBC_CTL_EN | (plane_state->base.crtc_h - 1);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 90ba5436370e..d496eff6fdd0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1756,9 +1756,10 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state);
+u32 glk_plane_color_ctl_crtc(const struct intel_crtc_state *crtc_state);
u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state);
-u32 glk_color_ctl(const struct intel_plane_state *plane_state);
+u32 skl_plane_ctl_crtc(const struct intel_crtc_state *crtc_state);
u32 skl_plane_stride(const struct intel_plane_state *plane_state,
int plane);
int skl_check_plane_surface(struct intel_plane_state *plane_state);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index cd42e81f8a90..b56a1a9ad01d 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -484,9 +484,16 @@ skl_program_plane(struct intel_plane *plane,
struct intel_plane *linked = plane_state->linked_plane;
const struct drm_framebuffer *fb = plane_state->base.fb;
u8 alpha = plane_state->base.alpha >> 8;
+ u32 plane_color_ctl = 0;
unsigned long irqflags;
u32 keymsk, keymax;
+ plane_ctl |= skl_plane_ctl_crtc(crtc_state);
+
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+ plane_color_ctl = plane_state->color_ctl |
+ glk_plane_color_ctl_crtc(crtc_state);
+
/* Sizes are 0 based */
src_w--;
src_h--;
@@ -533,8 +540,7 @@ skl_program_plane(struct intel_plane *plane,
}
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
- I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
- plane_state->color_ctl);
+ I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id), plane_color_ctl);
if (fb->format->is_yuv && icl_is_hdr_plane(plane))
icl_program_input_csc(plane, crtc_state, plane_state);
@@ -733,6 +739,11 @@ vlv_update_clrc(const struct intel_plane_state *plane_state)
SP_SH_SIN(sh_sin) | SP_SH_COS(sh_cos));
}
+static u32 vlv_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
+{
+ return SP_GAMMA_ENABLE;
+}
+
static u32 vlv_sprite_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
@@ -741,7 +752,7 @@ static u32 vlv_sprite_ctl(const struct intel_crtc_state *crtc_state,
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 sprctl;
- sprctl = SP_ENABLE | SP_GAMMA_ENABLE;
+ sprctl = SP_ENABLE;
switch (fb->format->format) {
case DRM_FORMAT_YUYV:
@@ -808,7 +819,6 @@ vlv_update_plane(struct intel_plane *plane,
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum pipe pipe = plane->pipe;
enum plane_id plane_id = plane->id;
- u32 sprctl = plane_state->ctl;
u32 sprsurf_offset = plane_state->color_plane[0].offset;
u32 linear_offset;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
@@ -819,6 +829,9 @@ vlv_update_plane(struct intel_plane *plane,
u32 x = plane_state->color_plane[0].x;
u32 y = plane_state->color_plane[0].y;
unsigned long irqflags;
+ u32 sprctl;
+
+ sprctl = plane_state->ctl | vlv_sprite_ctl_crtc(crtc_state);
/* Sizes are 0 based */
crtc_w--;
@@ -901,6 +914,19 @@ vlv_plane_get_hw_state(struct intel_plane *plane,
return ret;
}
+static u32 ivb_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+ u32 sprctl = 0;
+
+ sprctl |= SPRITE_GAMMA_ENABLE;
+
+ if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+ sprctl |= SPRITE_PIPE_CSC_ENABLE;
+
+ return sprctl;
+}
+
static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
@@ -911,14 +937,11 @@ static u32 ivb_sprite_ctl(const struct intel_crtc_state *crtc_state,
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 sprctl;
- sprctl = SPRITE_ENABLE | SPRITE_GAMMA_ENABLE;
+ sprctl = SPRITE_ENABLE;
if (IS_IVYBRIDGE(dev_priv))
sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
- if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
- sprctl |= SPRITE_PIPE_CSC_ENABLE;
-
switch (fb->format->format) {
case DRM_FORMAT_XBGR8888:
sprctl |= SPRITE_FORMAT_RGBX888 | SPRITE_RGB_ORDER_RGBX;
@@ -970,7 +993,6 @@ ivb_update_plane(struct intel_plane *plane,
{
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum pipe pipe = plane->pipe;
- u32 sprctl = plane_state->ctl, sprscale = 0;
u32 sprsurf_offset = plane_state->color_plane[0].offset;
u32 linear_offset;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
@@ -982,8 +1004,11 @@ ivb_update_plane(struct intel_plane *plane,
u32 y = plane_state->color_plane[0].y;
u32 src_w = drm_rect_width(&plane_state->base.src) >> 16;
u32 src_h = drm_rect_height(&plane_state->base.src) >> 16;
+ u32 sprctl, sprscale = 0;
unsigned long irqflags;
+ sprctl = plane_state->ctl | ivb_sprite_ctl_crtc(crtc_state);
+
/* Sizes are 0 based */
src_w--;
src_h--;
@@ -1080,6 +1105,11 @@ g4x_sprite_max_stride(struct intel_plane *plane,
return 16384;
}
+static u32 g4x_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
+{
+ return DVS_GAMMA_ENABLE;
+}
+
static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
@@ -1090,7 +1120,7 @@ static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 dvscntr;
- dvscntr = DVS_ENABLE | DVS_GAMMA_ENABLE;
+ dvscntr = DVS_ENABLE;
if (IS_GEN(dev_priv, 6))
dvscntr |= DVS_TRICKLE_FEED_DISABLE;
@@ -1146,7 +1176,6 @@ g4x_update_plane(struct intel_plane *plane,
{
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum pipe pipe = plane->pipe;
- u32 dvscntr = plane_state->ctl, dvsscale = 0;
u32 dvssurf_offset = plane_state->color_plane[0].offset;
u32 linear_offset;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
@@ -1158,8 +1187,11 @@ g4x_update_plane(struct intel_plane *plane,
u32 y = plane_state->color_plane[0].y;
u32 src_w = drm_rect_width(&plane_state->base.src) >> 16;
u32 src_h = drm_rect_height(&plane_state->base.src) >> 16;
+ u32 dvscntr, dvsscale = 0;
unsigned long irqflags;
+ dvscntr = plane_state->ctl | g4x_sprite_ctl_crtc(crtc_state);
+
/* Sizes are 0 based */
src_w--;
src_h--;
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 02/13] drm/i915: Precompute gamma_mode
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 01/13] drm/i915: Split the gamma/csc enable bits from the plane_ctl() function Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 03/13] drm/i915: Constify the state arguments to the color management stuff Ville Syrjala
` (14 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
We shouldn't be computing gamma mode during the commit phase.
Move it to the check phase.
v2: Reword comments a bit (Matt)
Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 4b0044cdcf1a..3a533143a9ef 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -375,8 +375,7 @@ static void haswell_load_luts(struct intel_crtc_state *crtc_state)
reenable_ips = true;
}
- crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
- I915_WRITE(GAMMA_MODE(crtc->pipe), GAMMA_MODE_MODE_8BIT);
+ I915_WRITE(GAMMA_MODE(crtc->pipe), crtc_state->gamma_mode);
i9xx_load_luts(crtc_state);
@@ -476,9 +475,7 @@ static void broadwell_load_luts(struct intel_crtc_state *crtc_state)
bdw_load_gamma_lut(crtc_state,
INTEL_INFO(dev_priv)->color.degamma_lut_size);
- crtc_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
- I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
- POSTING_READ(GAMMA_MODE(pipe));
+ I915_WRITE(GAMMA_MODE(pipe), crtc_state->gamma_mode);
/*
* Reset the index, otherwise it prevents the legacy palette to be
@@ -532,9 +529,7 @@ static void glk_load_luts(struct intel_crtc_state *crtc_state)
bdw_load_gamma_lut(crtc_state, 0);
- crtc_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
- I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
- POSTING_READ(GAMMA_MODE(pipe));
+ I915_WRITE(GAMMA_MODE(pipe), crtc_state->gamma_mode);
}
/* Loads the palette/gamma unit for the CRTC on CherryView. */
@@ -634,8 +629,10 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
/* Always allow legacy gamma LUT with no further checking. */
- if (crtc_state_is_legacy_gamma(crtc_state))
+ if (crtc_state_is_legacy_gamma(crtc_state)) {
+ crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
return 0;
+ }
if (check_lut_size(crtc_state->base.degamma_lut, degamma_length) ||
check_lut_size(crtc_state->base.gamma_lut, gamma_length))
@@ -645,6 +642,12 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
drm_color_lut_check(crtc_state->base.gamma_lut, gamma_tests))
return -EINVAL;
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+ crtc_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
+ else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
+ crtc_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
+ else
+ crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
return 0;
}
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 03/13] drm/i915: Constify the state arguments to the color management stuff
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 01/13] drm/i915: Split the gamma/csc enable bits from the plane_ctl() function Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 02/13] drm/i915: Precompute gamma_mode Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 04/13] drm/i915: Pull GAMMA_MODE write out from haswell_load_luts() Ville Syrjala
` (13 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Pass the crtc state etc. as const to the color management commit
functions. And while at it polish some of the local variables.
v2: Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 4 +-
drivers/gpu/drm/i915/intel_color.c | 140 ++++++++++++++++-------------
drivers/gpu/drm/i915/intel_drv.h | 4 +-
3 files changed, 80 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 45b837bc8f9e..b18c7d223357 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -323,8 +323,8 @@ struct drm_i915_display_funcs {
/* display clock increase/decrease */
/* pll clock increase/decrease */
- void (*load_csc_matrix)(struct intel_crtc_state *crtc_state);
- void (*load_luts)(struct intel_crtc_state *crtc_state);
+ void (*load_csc_matrix)(const struct intel_crtc_state *crtc_state);
+ void (*load_luts)(const struct intel_crtc_state *crtc_state);
};
#define CSR_VERSION(major, minor) ((major) << 16 | (minor))
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 3a533143a9ef..390ee2878f9d 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -74,12 +74,12 @@
#define ILK_CSC_COEFF_1_0 \
((7 << 12) | ILK_CSC_COEFF_FP(CTM_COEFF_1_0, 8))
-static bool lut_is_legacy(struct drm_property_blob *lut)
+static bool lut_is_legacy(const struct drm_property_blob *lut)
{
return drm_color_lut_size(lut) == LEGACY_LUT_LENGTH;
}
-static bool crtc_state_is_legacy_gamma(struct intel_crtc_state *crtc_state)
+static bool crtc_state_is_legacy_gamma(const struct intel_crtc_state *crtc_state)
{
return !crtc_state->base.degamma_lut &&
!crtc_state->base.ctm &&
@@ -115,8 +115,8 @@ static u64 *ctm_mult_by_limited(u64 *result, const u64 *input)
static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *crtc)
{
- int pipe = crtc->pipe;
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum pipe pipe = crtc->pipe;
I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
@@ -137,13 +137,14 @@ static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *crtc)
I915_WRITE(PIPE_CSC_MODE(pipe), 0);
}
-static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state)
+static void ilk_load_csc_matrix(const struct intel_crtc_state *crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
- int i, pipe = crtc->pipe;
- u16 coeffs[9] = { 0, };
bool limited_color_range = false;
+ enum pipe pipe = crtc->pipe;
+ u16 coeffs[9] = {};
+ int i;
/*
* FIXME if there's a gamma LUT after the CSC, we should
@@ -256,16 +257,16 @@ static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state)
/*
* Set up the pipe CSC unit on CherryView.
*/
-static void cherryview_load_csc_matrix(struct intel_crtc_state *crtc_state)
+static void cherryview_load_csc_matrix(const struct intel_crtc_state *crtc_state)
{
- struct drm_device *dev = crtc_state->base.crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- int pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum pipe pipe = crtc->pipe;
u32 mode;
if (crtc_state->base.ctm) {
- struct drm_color_ctm *ctm = crtc_state->base.ctm->data;
- u16 coeffs[9] = { 0, };
+ const struct drm_color_ctm *ctm = crtc_state->base.ctm->data;
+ u16 coeffs[9] = {};
int i;
for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
@@ -303,18 +304,17 @@ static void cherryview_load_csc_matrix(struct intel_crtc_state *crtc_state)
I915_WRITE(CGM_PIPE_MODE(pipe), mode);
}
-void intel_color_set_csc(struct intel_crtc_state *crtc_state)
+void intel_color_set_csc(const struct intel_crtc_state *crtc_state)
{
- struct drm_device *dev = crtc_state->base.crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
if (dev_priv->display.load_csc_matrix)
dev_priv->display.load_csc_matrix(crtc_state);
}
/* Loads the legacy palette/gamma unit for the CRTC. */
-static void i9xx_load_luts_internal(struct intel_crtc_state *crtc_state,
- struct drm_property_blob *blob)
+static void i9xx_load_luts_internal(const struct intel_crtc_state *crtc_state,
+ const struct drm_property_blob *blob)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -329,7 +329,8 @@ static void i9xx_load_luts_internal(struct intel_crtc_state *crtc_state,
}
if (blob) {
- struct drm_color_lut *lut = blob->data;
+ const struct drm_color_lut *lut = blob->data;
+
for (i = 0; i < 256; i++) {
u32 word =
(drm_color_lut_extract(lut[i].red, 8) << 16) |
@@ -353,13 +354,13 @@ static void i9xx_load_luts_internal(struct intel_crtc_state *crtc_state,
}
}
-static void i9xx_load_luts(struct intel_crtc_state *crtc_state)
+static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
{
i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
}
/* Loads the legacy palette/gamma unit for the CRTC on Haswell. */
-static void haswell_load_luts(struct intel_crtc_state *crtc_state)
+static void haswell_load_luts(const struct intel_crtc_state *crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -383,17 +384,19 @@ static void haswell_load_luts(struct intel_crtc_state *crtc_state)
hsw_enable_ips(crtc_state);
}
-static void bdw_load_degamma_lut(struct intel_crtc_state *crtc_state)
+static void bdw_load_degamma_lut(const struct intel_crtc_state *crtc_state)
{
- struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
- enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ const struct drm_property_blob *degamma_lut = crtc_state->base.degamma_lut;
u32 i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+ enum pipe pipe = crtc->pipe;
I915_WRITE(PREC_PAL_INDEX(pipe),
PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
- if (crtc_state->base.degamma_lut) {
- struct drm_color_lut *lut = crtc_state->base.degamma_lut->data;
+ if (degamma_lut) {
+ const struct drm_color_lut *lut = degamma_lut->data;
for (i = 0; i < lut_size; i++) {
u32 word =
@@ -413,11 +416,13 @@ static void bdw_load_degamma_lut(struct intel_crtc_state *crtc_state)
}
}
-static void bdw_load_gamma_lut(struct intel_crtc_state *crtc_state, u32 offset)
+static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32 offset)
{
- struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
- enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
u32 i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+ enum pipe pipe = crtc->pipe;
WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
@@ -426,8 +431,8 @@ static void bdw_load_gamma_lut(struct intel_crtc_state *crtc_state, u32 offset)
PAL_PREC_AUTO_INCREMENT |
offset);
- if (crtc_state->base.gamma_lut) {
- struct drm_color_lut *lut = crtc_state->base.gamma_lut->data;
+ if (gamma_lut) {
+ const struct drm_color_lut *lut = gamma_lut->data;
for (i = 0; i < lut_size; i++) {
u32 word =
@@ -461,10 +466,11 @@ static void bdw_load_gamma_lut(struct intel_crtc_state *crtc_state, u32 offset)
}
/* Loads the palette/gamma unit for the CRTC on Broadwell+. */
-static void broadwell_load_luts(struct intel_crtc_state *crtc_state)
+static void broadwell_load_luts(const struct intel_crtc_state *crtc_state)
{
- struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
- enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum pipe pipe = crtc->pipe;
if (crtc_state_is_legacy_gamma(crtc_state)) {
haswell_load_luts(crtc_state);
@@ -484,10 +490,11 @@ static void broadwell_load_luts(struct intel_crtc_state *crtc_state)
I915_WRITE(PREC_PAL_INDEX(pipe), 0);
}
-static void glk_load_degamma_lut(struct intel_crtc_state *crtc_state)
+static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state)
{
- struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
- enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum pipe pipe = crtc->pipe;
const u32 lut_size = 33;
u32 i;
@@ -514,11 +521,11 @@ static void glk_load_degamma_lut(struct intel_crtc_state *crtc_state)
I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16));
}
-static void glk_load_luts(struct intel_crtc_state *crtc_state)
+static void glk_load_luts(const struct intel_crtc_state *crtc_state)
{
- struct drm_device *dev = crtc_state->base.crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum pipe pipe = crtc->pipe;
glk_load_degamma_lut(crtc_state);
@@ -532,28 +539,29 @@ static void glk_load_luts(struct intel_crtc_state *crtc_state)
I915_WRITE(GAMMA_MODE(pipe), crtc_state->gamma_mode);
}
-/* Loads the palette/gamma unit for the CRTC on CherryView. */
-static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
+static void cherryview_load_luts(const struct intel_crtc_state *crtc_state)
{
- struct drm_crtc *crtc = crtc_state->base.crtc;
- struct drm_i915_private *dev_priv = to_i915(crtc->dev);
- enum pipe pipe = to_intel_crtc(crtc)->pipe;
- struct drm_color_lut *lut;
- u32 i, lut_size;
- u32 word0, word1;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
+ const struct drm_property_blob *degamma_lut = crtc_state->base.degamma_lut;
+ enum pipe pipe = crtc->pipe;
if (crtc_state_is_legacy_gamma(crtc_state)) {
/* Turn off degamma/gamma on CGM block. */
I915_WRITE(CGM_PIPE_MODE(pipe),
(crtc_state->base.ctm ? CGM_PIPE_MODE_CSC : 0));
- i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
+ i9xx_load_luts_internal(crtc_state, gamma_lut);
return;
}
- if (crtc_state->base.degamma_lut) {
- lut = crtc_state->base.degamma_lut->data;
- lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+ if (degamma_lut) {
+ const struct drm_color_lut *lut = degamma_lut->data;
+ int i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+
for (i = 0; i < lut_size; i++) {
+ u32 word0, word1;
+
/* Write LUT in U0.14 format. */
word0 =
(drm_color_lut_extract(lut[i].green, 14) << 16) |
@@ -565,10 +573,13 @@ static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
}
}
- if (crtc_state->base.gamma_lut) {
- lut = crtc_state->base.gamma_lut->data;
- lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+ if (gamma_lut) {
+ const struct drm_color_lut *lut = gamma_lut->data;
+ int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+
for (i = 0; i < lut_size; i++) {
+ u32 word0, word1;
+
/* Write LUT in U0.10 format. */
word0 =
(drm_color_lut_extract(lut[i].green, 10) << 16) |
@@ -582,8 +593,8 @@ static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
I915_WRITE(CGM_PIPE_MODE(pipe),
(crtc_state->base.ctm ? CGM_PIPE_MODE_CSC : 0) |
- (crtc_state->base.degamma_lut ? CGM_PIPE_MODE_DEGAMMA : 0) |
- (crtc_state->base.gamma_lut ? CGM_PIPE_MODE_GAMMA : 0));
+ (degamma_lut ? CGM_PIPE_MODE_DEGAMMA : 0) |
+ (gamma_lut ? CGM_PIPE_MODE_GAMMA : 0));
/*
* Also program a linear LUT in the legacy block (behind the
@@ -592,10 +603,9 @@ static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
i9xx_load_luts_internal(crtc_state, NULL);
}
-void intel_color_load_luts(struct intel_crtc_state *crtc_state)
+void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
{
- struct drm_device *dev = crtc_state->base.crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
dev_priv->display.load_luts(crtc_state);
}
@@ -620,6 +630,8 @@ static int check_lut_size(const struct drm_property_blob *lut, int expected)
int intel_color_check(struct intel_crtc_state *crtc_state)
{
struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+ const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
+ const struct drm_property_blob *degamma_lut = crtc_state->base.degamma_lut;
int gamma_length, degamma_length;
u32 gamma_tests, degamma_tests;
@@ -634,12 +646,12 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
return 0;
}
- if (check_lut_size(crtc_state->base.degamma_lut, degamma_length) ||
- check_lut_size(crtc_state->base.gamma_lut, gamma_length))
+ if (check_lut_size(degamma_lut, degamma_length) ||
+ check_lut_size(gamma_lut, gamma_length))
return -EINVAL;
- if (drm_color_lut_check(crtc_state->base.degamma_lut, degamma_tests) ||
- drm_color_lut_check(crtc_state->base.gamma_lut, gamma_tests))
+ if (drm_color_lut_check(degamma_lut, degamma_tests) ||
+ drm_color_lut_check(gamma_lut, gamma_tests))
return -EINVAL;
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d496eff6fdd0..5586870fa003 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2380,8 +2380,8 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
/* intel_color.c */
void intel_color_init(struct intel_crtc *crtc);
int intel_color_check(struct intel_crtc_state *crtc_state);
-void intel_color_set_csc(struct intel_crtc_state *crtc_state);
-void intel_color_load_luts(struct intel_crtc_state *crtc_state);
+void intel_color_set_csc(const struct intel_crtc_state *crtc_state);
+void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
/* intel_lspcon.c */
bool lspcon_init(struct intel_digital_port *intel_dig_port);
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 04/13] drm/i915: Pull GAMMA_MODE write out from haswell_load_luts()
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (2 preceding siblings ...)
2019-02-05 16:08 ` [PATCH v2 03/13] drm/i915: Constify the state arguments to the color management stuff Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 05/13] drm/i915: Split color mgmt based on single vs. double buffered registers Ville Syrjala
` (12 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
For bdw+ let's move the GAMMA_MODE write for the legacy LUT
mode into the .load_luts() funciton directly, rather than
relying on haswell_load_luts(). We'll be getting rid of
haswell_load_luts() entirely soon, and it's anyway cleaner
to have the GAMMA_MODE write in a single place.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 36 +++++++++++++++++-------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 390ee2878f9d..227a97ce71b2 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -473,21 +473,20 @@ static void broadwell_load_luts(const struct intel_crtc_state *crtc_state)
enum pipe pipe = crtc->pipe;
if (crtc_state_is_legacy_gamma(crtc_state)) {
- haswell_load_luts(crtc_state);
- return;
- }
+ i9xx_load_luts(crtc_state);
+ } else {
+ bdw_load_degamma_lut(crtc_state);
+ bdw_load_gamma_lut(crtc_state,
+ INTEL_INFO(dev_priv)->color.degamma_lut_size);
- bdw_load_degamma_lut(crtc_state);
- bdw_load_gamma_lut(crtc_state,
- INTEL_INFO(dev_priv)->color.degamma_lut_size);
+ /*
+ * Reset the index, otherwise it prevents the legacy palette to be
+ * written properly.
+ */
+ I915_WRITE(PREC_PAL_INDEX(pipe), 0);
+ }
I915_WRITE(GAMMA_MODE(pipe), crtc_state->gamma_mode);
-
- /*
- * Reset the index, otherwise it prevents the legacy palette to be
- * written properly.
- */
- I915_WRITE(PREC_PAL_INDEX(pipe), 0);
}
static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state)
@@ -530,11 +529,16 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
glk_load_degamma_lut(crtc_state);
if (crtc_state_is_legacy_gamma(crtc_state)) {
- haswell_load_luts(crtc_state);
- return;
- }
+ i9xx_load_luts(crtc_state);
+ } else {
+ bdw_load_gamma_lut(crtc_state, 0);
- bdw_load_gamma_lut(crtc_state, 0);
+ /*
+ * Reset the index, otherwise it prevents the legacy palette to be
+ * written properly.
+ */
+ I915_WRITE(PREC_PAL_INDEX(pipe), 0);
+ }
I915_WRITE(GAMMA_MODE(pipe), crtc_state->gamma_mode);
}
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 05/13] drm/i915: Split color mgmt based on single vs. double buffered registers
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (3 preceding siblings ...)
2019-02-05 16:08 ` [PATCH v2 04/13] drm/i915: Pull GAMMA_MODE write out from haswell_load_luts() Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 06/13] drm/i915: Move LUT programming to happen after vblank waits Ville Syrjala
` (11 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Split the color management hooks along the single vs. double
buffered registers line. Of the currently programmed registers
GAMMA_MODE and the ilk+ pipe CSC are double buffered, the
LUTS and CHV CGM block are single buffered.
The double buffered register will be programmed during the
normal pipe update with evasion, and also during pipe enable
so that the settings will already be correct when the pipe
starts up before the planes are enabled.
The single buffered registers are currently programmed before
the vblank evade. Which is totally wrong, but we'll correct
that later.
v2: Add some docs to explain the two vfuncs (Matt,Uma)
Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 14 +++++++-
drivers/gpu/drm/i915/intel_color.c | 49 +++++++++++++---------------
drivers/gpu/drm/i915/intel_display.c | 16 +++++----
drivers/gpu/drm/i915/intel_drv.h | 2 +-
4 files changed, 46 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b18c7d223357..19b0cb9445ea 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -323,7 +323,19 @@ struct drm_i915_display_funcs {
/* display clock increase/decrease */
/* pll clock increase/decrease */
- void (*load_csc_matrix)(const struct intel_crtc_state *crtc_state);
+ /*
+ * Program double buffered color management registers during
+ * vblank evasion. The registers should then latch during the
+ * next vblank start, alongside any other double buffered registers
+ * involved with the same commit.
+ */
+ void (*color_commit)(const struct intel_crtc_state *crtc_state);
+ /*
+ * Load LUTs (and other single buffered color management
+ * registers). Will (hopefully) be called during the vblank
+ * following the latching of any double buffered registers
+ * involved with the same commit.
+ */
void (*load_luts)(const struct intel_crtc_state *crtc_state);
};
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 227a97ce71b2..c06bf1c687f9 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -304,14 +304,6 @@ static void cherryview_load_csc_matrix(const struct intel_crtc_state *crtc_state
I915_WRITE(CGM_PIPE_MODE(pipe), mode);
}
-void intel_color_set_csc(const struct intel_crtc_state *crtc_state)
-{
- struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
-
- if (dev_priv->display.load_csc_matrix)
- dev_priv->display.load_csc_matrix(crtc_state);
-}
-
/* Loads the legacy palette/gamma unit for the CRTC. */
static void i9xx_load_luts_internal(const struct intel_crtc_state *crtc_state,
const struct drm_property_blob *blob)
@@ -359,6 +351,16 @@ static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
}
+static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
+ I915_WRITE(GAMMA_MODE(crtc->pipe), crtc_state->gamma_mode);
+
+ ilk_load_csc_matrix(crtc_state);
+}
+
/* Loads the legacy palette/gamma unit for the CRTC on Haswell. */
static void haswell_load_luts(const struct intel_crtc_state *crtc_state)
{
@@ -376,8 +378,6 @@ static void haswell_load_luts(const struct intel_crtc_state *crtc_state)
reenable_ips = true;
}
- I915_WRITE(GAMMA_MODE(crtc->pipe), crtc_state->gamma_mode);
-
i9xx_load_luts(crtc_state);
if (reenable_ips)
@@ -485,8 +485,6 @@ static void broadwell_load_luts(const struct intel_crtc_state *crtc_state)
*/
I915_WRITE(PREC_PAL_INDEX(pipe), 0);
}
-
- I915_WRITE(GAMMA_MODE(pipe), crtc_state->gamma_mode);
}
static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state)
@@ -539,8 +537,6 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
*/
I915_WRITE(PREC_PAL_INDEX(pipe), 0);
}
-
- I915_WRITE(GAMMA_MODE(pipe), crtc_state->gamma_mode);
}
static void cherryview_load_luts(const struct intel_crtc_state *crtc_state)
@@ -551,10 +547,9 @@ static void cherryview_load_luts(const struct intel_crtc_state *crtc_state)
const struct drm_property_blob *degamma_lut = crtc_state->base.degamma_lut;
enum pipe pipe = crtc->pipe;
+ cherryview_load_csc_matrix(crtc_state);
+
if (crtc_state_is_legacy_gamma(crtc_state)) {
- /* Turn off degamma/gamma on CGM block. */
- I915_WRITE(CGM_PIPE_MODE(pipe),
- (crtc_state->base.ctm ? CGM_PIPE_MODE_CSC : 0));
i9xx_load_luts_internal(crtc_state, gamma_lut);
return;
}
@@ -595,11 +590,6 @@ static void cherryview_load_luts(const struct intel_crtc_state *crtc_state)
}
}
- I915_WRITE(CGM_PIPE_MODE(pipe),
- (crtc_state->base.ctm ? CGM_PIPE_MODE_CSC : 0) |
- (degamma_lut ? CGM_PIPE_MODE_DEGAMMA : 0) |
- (gamma_lut ? CGM_PIPE_MODE_GAMMA : 0));
-
/*
* Also program a linear LUT in the legacy block (behind the
* CGM block).
@@ -614,6 +604,14 @@ void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
dev_priv->display.load_luts(crtc_state);
}
+void intel_color_commit(const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+
+ if (dev_priv->display.color_commit)
+ dev_priv->display.color_commit(crtc_state);
+}
+
static int check_lut_size(const struct drm_property_blob *lut, int expected)
{
int len;
@@ -675,18 +673,17 @@ void intel_color_init(struct intel_crtc *crtc)
drm_mode_crtc_set_gamma_size(&crtc->base, 256);
if (IS_CHERRYVIEW(dev_priv)) {
- dev_priv->display.load_csc_matrix = cherryview_load_csc_matrix;
dev_priv->display.load_luts = cherryview_load_luts;
} else if (IS_HASWELL(dev_priv)) {
- dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
dev_priv->display.load_luts = haswell_load_luts;
+ dev_priv->display.color_commit = hsw_color_commit;
} else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
IS_BROXTON(dev_priv)) {
- dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
dev_priv->display.load_luts = broadwell_load_luts;
+ dev_priv->display.color_commit = hsw_color_commit;
} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
- dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
dev_priv->display.load_luts = glk_load_luts;
+ dev_priv->display.color_commit = hsw_color_commit;
} else {
dev_priv->display.load_luts = i9xx_load_luts;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 37fdd15c598c..5a8a36a552d4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5752,6 +5752,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
* clocks enabled
*/
intel_color_load_luts(pipe_config);
+ intel_color_commit(pipe_config);
if (dev_priv->display.initial_watermarks != NULL)
dev_priv->display.initial_watermarks(old_intel_state, pipe_config);
@@ -5862,8 +5863,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
haswell_set_pipemisc(pipe_config);
- intel_color_set_csc(pipe_config);
-
intel_crtc->active = true;
/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
@@ -5882,6 +5881,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
* clocks enabled
*/
intel_color_load_luts(pipe_config);
+ intel_color_commit(pipe_config);
/*
* Display WA #1153: enable hardware to bypass the alpha math
@@ -6227,8 +6227,6 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
i9xx_set_pipeconf(pipe_config);
- intel_color_set_csc(pipe_config);
-
intel_crtc->active = true;
intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
@@ -6248,6 +6246,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
i9xx_pfit_enable(pipe_config);
intel_color_load_luts(pipe_config);
+ intel_color_commit(pipe_config);
dev_priv->display.initial_watermarks(old_intel_state,
pipe_config);
@@ -6304,6 +6303,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
i9xx_pfit_enable(pipe_config);
intel_color_load_luts(pipe_config);
+ intel_color_commit(pipe_config);
if (dev_priv->display.initial_watermarks != NULL)
dev_priv->display.initial_watermarks(old_intel_state,
@@ -13727,10 +13727,8 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
if (!modeset &&
(intel_cstate->base.color_mgmt_changed ||
- intel_cstate->update_pipe)) {
- intel_color_set_csc(intel_cstate);
+ intel_cstate->update_pipe))
intel_color_load_luts(intel_cstate);
- }
/* Perform vblank evasion around commit operation */
intel_pipe_update_start(intel_cstate);
@@ -13738,6 +13736,10 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
if (modeset)
goto out;
+ if (intel_cstate->base.color_mgmt_changed ||
+ intel_cstate->update_pipe)
+ intel_color_commit(intel_cstate);
+
if (intel_cstate->update_pipe)
intel_update_pipe_config(old_intel_cstate, intel_cstate);
else if (INTEL_GEN(dev_priv) >= 9)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5586870fa003..b0720ed04aa5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2380,7 +2380,7 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
/* intel_color.c */
void intel_color_init(struct intel_crtc *crtc);
int intel_color_check(struct intel_crtc_state *crtc_state);
-void intel_color_set_csc(const struct intel_crtc_state *crtc_state);
+void intel_color_commit(const struct intel_crtc_state *crtc_state);
void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
/* intel_lspcon.c */
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 06/13] drm/i915: Move LUT programming to happen after vblank waits
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (4 preceding siblings ...)
2019-02-05 16:08 ` [PATCH v2 05/13] drm/i915: Split color mgmt based on single vs. double buffered registers Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-07 15:46 ` Maarten Lankhorst
2019-02-05 16:08 ` [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms Ville Syrjala
` (10 subsequent siblings)
16 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The LUTs are single buffered so we should program them after
the double buffered pipe updates have been latched by the
hardware.
We'll also fix up the IPS vs. split gamma w/a to do the IPS
disable like everyone else. Note that this is currently dead
code as we don't use the split gamma mode on HSW, but that
will be fixed up shortly.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 25 +--------------
drivers/gpu/drm/i915/intel_display.c | 47 ++++++++++++++++++++++++----
2 files changed, 42 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index c06bf1c687f9..2a371eed8061 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -361,29 +361,6 @@ static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
ilk_load_csc_matrix(crtc_state);
}
-/* Loads the legacy palette/gamma unit for the CRTC on Haswell. */
-static void haswell_load_luts(const struct intel_crtc_state *crtc_state)
-{
- struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
- struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
- bool reenable_ips = false;
-
- /*
- * Workaround : Do not read or write the pipe palette/gamma data while
- * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
- */
- if (IS_HASWELL(dev_priv) && crtc_state->ips_enabled &&
- (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)) {
- hsw_disable_ips(crtc_state);
- reenable_ips = true;
- }
-
- i9xx_load_luts(crtc_state);
-
- if (reenable_ips)
- hsw_enable_ips(crtc_state);
-}
-
static void bdw_load_degamma_lut(const struct intel_crtc_state *crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
@@ -675,7 +652,7 @@ void intel_color_init(struct intel_crtc *crtc)
if (IS_CHERRYVIEW(dev_priv)) {
dev_priv->display.load_luts = cherryview_load_luts;
} else if (IS_HASWELL(dev_priv)) {
- dev_priv->display.load_luts = haswell_load_luts;
+ dev_priv->display.load_luts = i9xx_load_luts;
dev_priv->display.color_commit = hsw_color_commit;
} else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
IS_BROXTON(dev_priv)) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5a8a36a552d4..ad5d39d81d6e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5346,24 +5346,54 @@ intel_pre_disable_primary_noatomic(struct drm_crtc *crtc)
static bool hsw_pre_update_disable_ips(const struct intel_crtc_state *old_crtc_state,
const struct intel_crtc_state *new_crtc_state)
{
+ struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
if (!old_crtc_state->ips_enabled)
return false;
if (needs_modeset(&new_crtc_state->base))
return true;
+ /*
+ * Workaround : Do not read or write the pipe palette/gamma data while
+ * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
+ *
+ * Disable IPS before we program the LUT.
+ */
+ if (IS_HASWELL(dev_priv) &&
+ (new_crtc_state->base.color_mgmt_changed ||
+ new_crtc_state->update_pipe) &&
+ new_crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)
+ return true;
+
return !new_crtc_state->ips_enabled;
}
static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_state,
const struct intel_crtc_state *new_crtc_state)
{
+ struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
if (!new_crtc_state->ips_enabled)
return false;
if (needs_modeset(&new_crtc_state->base))
return true;
+ /*
+ * Workaround : Do not read or write the pipe palette/gamma data while
+ * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
+ *
+ * Re-enable IPS after the LUT has been programmed.
+ */
+ if (IS_HASWELL(dev_priv) &&
+ (new_crtc_state->base.color_mgmt_changed ||
+ new_crtc_state->update_pipe) &&
+ new_crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)
+ return true;
+
/*
* We can't read out IPS on broadwell, assume the worst and
* forcibly enable IPS on the first fastset.
@@ -11117,7 +11147,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
return ret;
}
- if (crtc_state->color_mgmt_changed) {
+ if (mode_changed || crtc_state->color_mgmt_changed) {
ret = intel_color_check(pipe_config);
if (ret)
return ret;
@@ -13210,6 +13240,16 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
*/
drm_atomic_helper_wait_for_flip_done(dev, state);
+ for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+ new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
+
+ if (new_crtc_state->active &&
+ !needs_modeset(new_crtc_state) &&
+ (new_intel_crtc_state->base.color_mgmt_changed ||
+ new_intel_crtc_state->update_pipe))
+ intel_color_load_luts(new_intel_crtc_state);
+ }
+
/*
* Now that the vblank has passed, we can go ahead and program the
* optimal watermarks on platforms that need two-step watermark
@@ -13725,11 +13765,6 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
intel_atomic_get_new_crtc_state(old_intel_state, intel_crtc);
bool modeset = needs_modeset(&intel_cstate->base);
- if (!modeset &&
- (intel_cstate->base.color_mgmt_changed ||
- intel_cstate->update_pipe))
- intel_color_load_luts(intel_cstate);
-
/* Perform vblank evasion around commit operation */
intel_pipe_update_start(intel_cstate);
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (5 preceding siblings ...)
2019-02-05 16:08 ` [PATCH v2 06/13] drm/i915: Move LUT programming to happen after vblank waits Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-07 15:49 ` Maarten Lankhorst
2019-02-05 16:08 ` [PATCH v2 08/13] drm/i915: Track pipe gamma enable/disable in crtc state Ville Syrjala
` (9 subsequent siblings)
16 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
On pre-HSW gamma mode is configured via PIPECONF. The bits are
the same except shifted up, so we can reuse just store them in
crtc_state->gamma_mode in the HSW+ way, allowing us to share
some code later.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 10 ++++-
drivers/gpu/drm/i915/intel_color.c | 60 +++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_display.c | 14 ++++++-
3 files changed, 66 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ede54fdc1676..41111a17e519 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5590,9 +5590,15 @@ enum {
#define PIPECONF_SINGLE_WIDE 0
#define PIPECONF_PIPE_UNLOCKED 0
#define PIPECONF_PIPE_LOCKED (1 << 25)
-#define PIPECONF_PALETTE 0
-#define PIPECONF_GAMMA (1 << 24)
#define PIPECONF_FORCE_BORDER (1 << 25)
+#define PIPECONF_GAMMA_MODE_MASK_I9XX (1 << 24) /* gmch */
+#define PIPECONF_GAMMA_MODE_MASK_ILK (3 << 24) /* ilk-ivb */
+#define PIPECONF_GAMMA_MODE_8BIT (0 << 24) /* gmch,ilk-ivb */
+#define PIPECONF_GAMMA_MODE_10BIT (1 << 24) /* gmch,ilk-ivb */
+#define PIPECONF_GAMMA_MODE_12BIT (2 << 24) /* ilk-ivb */
+#define PIPECONF_GAMMA_MODE_SPLIT (3 << 24) /* ivb */
+#define PIPECONF_GAMMA_MODE(x) ((x)<<24) /* pass in GAMMA_MODE_MODE_* */
+#define PIPECONF_GAMMA_MODE_SHIFT 24
#define PIPECONF_INTERLACE_MASK (7 << 21)
#define PIPECONF_INTERLACE_MASK_HSW (3 << 21)
/* Note that pre-gen3 does not support interlaced display directly. Panel
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 2a371eed8061..e7dd07490c68 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -351,6 +351,32 @@ static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
}
+static void i9xx_color_commit(const struct intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum pipe pipe = crtc->pipe;
+ u32 val;
+
+ val = I915_READ(PIPECONF(pipe));
+ val &= ~PIPECONF_GAMMA_MODE_MASK_I9XX;
+ val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
+ I915_WRITE(PIPECONF(pipe), val);
+}
+
+static void ilk_color_commit(const struct intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum pipe pipe = crtc->pipe;
+ u32 val;
+
+ val = I915_READ(PIPECONF(pipe));
+ val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
+ val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
+ I915_WRITE(PIPECONF(pipe), val);
+}
+
static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
@@ -585,8 +611,7 @@ void intel_color_commit(const struct intel_crtc_state *crtc_state)
{
struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
- if (dev_priv->display.color_commit)
- dev_priv->display.color_commit(crtc_state);
+ dev_priv->display.color_commit(crtc_state);
}
static int check_lut_size(const struct drm_property_blob *lut, int expected)
@@ -649,20 +674,25 @@ void intel_color_init(struct intel_crtc *crtc)
drm_mode_crtc_set_gamma_size(&crtc->base, 256);
- if (IS_CHERRYVIEW(dev_priv)) {
- dev_priv->display.load_luts = cherryview_load_luts;
- } else if (IS_HASWELL(dev_priv)) {
- dev_priv->display.load_luts = i9xx_load_luts;
- dev_priv->display.color_commit = hsw_color_commit;
- } else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
- IS_BROXTON(dev_priv)) {
- dev_priv->display.load_luts = broadwell_load_luts;
- dev_priv->display.color_commit = hsw_color_commit;
- } else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
- dev_priv->display.load_luts = glk_load_luts;
- dev_priv->display.color_commit = hsw_color_commit;
+ if (HAS_GMCH_DISPLAY(dev_priv)) {
+ if (IS_CHERRYVIEW(dev_priv))
+ dev_priv->display.load_luts = cherryview_load_luts;
+ else
+ dev_priv->display.load_luts = i9xx_load_luts;
+
+ dev_priv->display.color_commit = i9xx_color_commit;
} else {
- dev_priv->display.load_luts = i9xx_load_luts;
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ dev_priv->display.load_luts = glk_load_luts;
+ else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
+ dev_priv->display.load_luts = broadwell_load_luts;
+ else
+ dev_priv->display.load_luts = i9xx_load_luts;
+
+ if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
+ dev_priv->display.color_commit = hsw_color_commit;
+ else
+ dev_priv->display.color_commit = ilk_color_commit;
}
/* Enable color management support when we have degamma & gamma LUTs. */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ad5d39d81d6e..8e93459f312f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3450,7 +3450,7 @@ static void i9xx_disable_plane(struct intel_plane *plane,
*
* On pre-g4x there is no way to gamma correct the
* pipe bottom color but we'll keep on doing this
- * anyway.
+ * anyway so that the crtc state readout works correctly.
*/
dspcntr = i9xx_plane_ctl_crtc(crtc_state);
@@ -7674,6 +7674,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
crtc_state->limited_color_range)
pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
+ pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
+
I915_WRITE(PIPECONF(crtc->pipe), pipeconf);
POSTING_READ(PIPECONF(crtc->pipe));
}
@@ -8126,6 +8128,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
(tmp & PIPECONF_COLOR_RANGE_SELECT))
pipe_config->limited_color_range = true;
+ pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_I9XX) >>
+ PIPECONF_GAMMA_MODE_SHIFT;
+
if (INTEL_GEN(dev_priv) < 4)
pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
@@ -8665,6 +8670,8 @@ static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state)
if (crtc_state->limited_color_range)
val |= PIPECONF_COLOR_RANGE_SELECT;
+ val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
+
I915_WRITE(PIPECONF(pipe), val);
POSTING_READ(PIPECONF(pipe));
}
@@ -9199,6 +9206,9 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
if (tmp & PIPECONF_COLOR_RANGE_SELECT)
pipe_config->limited_color_range = true;
+ pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >>
+ PIPECONF_GAMMA_MODE_SHIFT;
+
if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
struct intel_shared_dpll *pll;
enum intel_dpll_id pll_id;
@@ -12069,6 +12079,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
PIPE_CONF_CHECK_BOOL(double_wide);
+ PIPE_CONF_CHECK_X(gamma_mode);
+
PIPE_CONF_CHECK_P(shared_dpll);
PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 08/13] drm/i915: Track pipe gamma enable/disable in crtc state
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (6 preceding siblings ...)
2019-02-05 16:08 ` [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 09/13] drm/i915: Track pipe csc enable " Ville Syrjala
` (8 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Track whether pipe gamma is enabled or disabled. For now we
stick to the current behaviour of always enabling gamma. But
we do get working state readout for this now. On SKL+ we use
the pipe bottom color as our hardware state. On pre-SKL we
read the state back from the primary plane control register.
That only really correct for g4x+, as older platforms never
gamma correct pipe bottom color. But doing the readout the
same way on all platforms is fine, and there is no other way
to do it really.
v2: Initialize val at declaration (Uma)
Drop the bogus skl scaler comment change (Uma)
Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 28 +++++++++++-
drivers/gpu/drm/i915/intel_display.c | 65 +++++++++++++++++++---------
drivers/gpu/drm/i915/intel_drv.h | 3 ++
drivers/gpu/drm/i915/intel_sprite.c | 17 ++++++--
4 files changed, 88 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index e7dd07490c68..8acc729925af 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -387,6 +387,28 @@ static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
ilk_load_csc_matrix(crtc_state);
}
+static void skl_color_commit(const struct intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum pipe pipe = crtc->pipe;
+ u32 val = 0;
+
+ /*
+ * We don't (yet) allow userspace to control the pipe background color,
+ * so force it to black, but apply pipe gamma and CSC appropriately
+ * so that its handling will match how we program our planes.
+ */
+ if (crtc_state->gamma_enable)
+ val |= SKL_BOTTOM_COLOR_GAMMA_ENABLE;
+ val |= SKL_BOTTOM_COLOR_CSC_ENABLE;
+ I915_WRITE(SKL_BOTTOM_COLOR(pipe), val);
+
+ I915_WRITE(GAMMA_MODE(crtc->pipe), crtc_state->gamma_mode);
+
+ ilk_load_csc_matrix(crtc_state);
+}
+
static void bdw_load_degamma_lut(const struct intel_crtc_state *crtc_state)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
@@ -644,6 +666,8 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
+ crtc_state->gamma_enable = true;
+
/* Always allow legacy gamma LUT with no further checking. */
if (crtc_state_is_legacy_gamma(crtc_state)) {
crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
@@ -689,7 +713,9 @@ void intel_color_init(struct intel_crtc *crtc)
else
dev_priv->display.load_luts = i9xx_load_luts;
- if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
+ if (INTEL_GEN(dev_priv) >= 9)
+ dev_priv->display.color_commit = skl_color_commit;
+ else if (IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
dev_priv->display.color_commit = hsw_color_commit;
else
dev_priv->display.color_commit = ilk_color_commit;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8e93459f312f..ebf26c0e0a98 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3221,7 +3221,8 @@ static u32 i9xx_plane_ctl_crtc(const struct intel_crtc_state *crtc_state)
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
u32 dspcntr = 0;
- dspcntr |= DISPPLANE_GAMMA_ENABLE;
+ if (crtc_state->gamma_enable)
+ dspcntr |= DISPPLANE_GAMMA_ENABLE;
if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
@@ -3701,7 +3702,9 @@ u32 skl_plane_ctl_crtc(const struct intel_crtc_state *crtc_state)
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
return plane_ctl;
- plane_ctl |= PLANE_CTL_PIPE_GAMMA_ENABLE;
+ if (crtc_state->gamma_enable)
+ plane_ctl |= PLANE_CTL_PIPE_GAMMA_ENABLE;
+
plane_ctl |= PLANE_CTL_PIPE_CSC_ENABLE;
return plane_ctl;
@@ -3754,7 +3757,9 @@ u32 glk_plane_color_ctl_crtc(const struct intel_crtc_state *crtc_state)
if (INTEL_GEN(dev_priv) >= 11)
return plane_color_ctl;
- plane_color_ctl |= PLANE_COLOR_PIPE_GAMMA_ENABLE;
+ if (crtc_state->gamma_enable)
+ plane_color_ctl |= PLANE_COLOR_PIPE_GAMMA_ENABLE;
+
plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE;
return plane_color_ctl;
@@ -3974,16 +3979,6 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
else if (old_crtc_state->pch_pfit.enabled)
ironlake_pfit_disable(old_crtc_state);
}
-
- /*
- * We don't (yet) allow userspace to control the pipe background color,
- * so force it to black, but apply pipe gamma and CSC so that its
- * handling will match how we program our planes.
- */
- if (INTEL_GEN(dev_priv) >= 9)
- I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe),
- SKL_BOTTOM_COLOR_GAMMA_ENABLE |
- SKL_BOTTOM_COLOR_CSC_ENABLE);
}
static void intel_fdi_normal_train(struct intel_crtc *crtc)
@@ -8083,6 +8078,20 @@ static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
pipe_config->output_format = output;
}
+static void i9xx_get_pipe_color_config(struct intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum i9xx_plane_id i9xx_plane = plane->i9xx_plane;
+ u32 tmp;
+
+ tmp = I915_READ(DSPCNTR(i9xx_plane));
+
+ if (tmp & DISPPLANE_GAMMA_ENABLE)
+ crtc_state->gamma_enable = true;
+}
+
static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config)
{
@@ -8131,6 +8140,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_I9XX) >>
PIPECONF_GAMMA_MODE_SHIFT;
+ i9xx_get_pipe_color_config(pipe_config);
+
if (INTEL_GEN(dev_priv) < 4)
pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
@@ -9209,6 +9220,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >>
PIPECONF_GAMMA_MODE_SHIFT;
+ i9xx_get_pipe_color_config(pipe_config);
+
if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
struct intel_shared_dpll *pll;
enum intel_dpll_id pll_id;
@@ -9843,6 +9856,15 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
pipe_config->gamma_mode =
I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
+ if (INTEL_GEN(dev_priv) >= 9) {
+ u32 tmp = I915_READ(SKL_BOTTOM_COLOR(crtc->pipe));
+
+ if (tmp & SKL_BOTTOM_COLOR_GAMMA_ENABLE)
+ pipe_config->gamma_enable = true;
+ } else {
+ i9xx_get_pipe_color_config(pipe_config);
+ }
+
power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
WARN_ON(power_domain_mask & BIT_ULL(power_domain));
@@ -10013,7 +10035,12 @@ i845_cursor_max_stride(struct intel_plane *plane,
static u32 i845_cursor_ctl_crtc(const struct intel_crtc_state *crtc_state)
{
- return CURSOR_GAMMA_ENABLE;
+ u32 cntl = 0;
+
+ if (crtc_state->gamma_enable)
+ cntl |= CURSOR_GAMMA_ENABLE;
+
+ return cntl;
}
static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
@@ -10167,7 +10194,8 @@ static u32 i9xx_cursor_ctl_crtc(const struct intel_crtc_state *crtc_state)
if (INTEL_GEN(dev_priv) >= 11)
return cntl;
- cntl |= MCURSOR_GAMMA_ENABLE;
+ if (crtc_state->gamma_enable)
+ cntl = MCURSOR_GAMMA_ENABLE;
if (HAS_DDI(dev_priv))
cntl |= MCURSOR_PIPE_CSC_ENABLE;
@@ -11161,12 +11189,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
ret = intel_color_check(pipe_config);
if (ret)
return ret;
-
- /*
- * Changing color management on Intel hardware is
- * handled as part of planes update.
- */
- crtc_state->planes_changed = true;
}
ret = 0;
@@ -12080,6 +12102,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
PIPE_CONF_CHECK_BOOL(double_wide);
PIPE_CONF_CHECK_X(gamma_mode);
+ PIPE_CONF_CHECK_BOOL(gamma_enable);
PIPE_CONF_CHECK_P(shared_dpll);
PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b0720ed04aa5..f0db61471209 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -951,6 +951,9 @@ struct intel_crtc_state {
/* Output down scaling is done in LSPCON device */
bool lspcon_downsampling;
+ /* enable pipe gamma? */
+ bool gamma_enable;
+
/* Display Stream compression state */
struct {
bool compression_enable;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index b56a1a9ad01d..db373e3ac601 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -741,7 +741,12 @@ vlv_update_clrc(const struct intel_plane_state *plane_state)
static u32 vlv_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
{
- return SP_GAMMA_ENABLE;
+ u32 sprctl = 0;
+
+ if (crtc_state->gamma_enable)
+ sprctl |= SP_GAMMA_ENABLE;
+
+ return sprctl;
}
static u32 vlv_sprite_ctl(const struct intel_crtc_state *crtc_state,
@@ -919,7 +924,8 @@ static u32 ivb_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
u32 sprctl = 0;
- sprctl |= SPRITE_GAMMA_ENABLE;
+ if (crtc_state->gamma_enable)
+ sprctl |= SPRITE_GAMMA_ENABLE;
if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
sprctl |= SPRITE_PIPE_CSC_ENABLE;
@@ -1107,7 +1113,12 @@ g4x_sprite_max_stride(struct intel_plane *plane,
static u32 g4x_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
{
- return DVS_GAMMA_ENABLE;
+ u32 dvscntr = 0;
+
+ if (crtc_state->gamma_enable)
+ dvscntr |= DVS_GAMMA_ENABLE;
+
+ return dvscntr;
}
static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 09/13] drm/i915: Track pipe csc enable in crtc state
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (7 preceding siblings ...)
2019-02-05 16:08 ` [PATCH v2 08/13] drm/i915: Track pipe gamma enable/disable in crtc state Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 10/13] drm/i915: Turn off pipe gamma when it's not needed Ville Syrjala
` (7 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Just like we did for pipe gamma, let's also track the pipe csc
state. The hardware only exists on ILK+, and currently we always
enable it on hsw+ and never on any other platforms. Just like
with pipe gamma, the primary plane control register is used
for the readout on pre-SKL, and the pipe bottom color register
on SKL+.
v2: Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 4 ++--
drivers/gpu/drm/i915/intel_color.c | 7 ++++++-
drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++++----
drivers/gpu/drm/i915/intel_drv.h | 3 +++
drivers/gpu/drm/i915/intel_sprite.c | 6 ++++--
5 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 41111a17e519..30b3c385e41b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6130,7 +6130,7 @@ enum {
#define MCURSOR_PIPE_SELECT_SHIFT 28
#define MCURSOR_PIPE_SELECT(pipe) ((pipe) << 28)
#define MCURSOR_GAMMA_ENABLE (1 << 26)
-#define MCURSOR_PIPE_CSC_ENABLE (1 << 24)
+#define MCURSOR_PIPE_CSC_ENABLE (1 << 24) /* ilk+ */
#define MCURSOR_ROTATE_180 (1 << 15)
#define MCURSOR_TRICKLE_FEED_DISABLE (1 << 14)
#define _CURABASE 0x70084
@@ -6185,7 +6185,7 @@ enum {
#define DISPPLANE_RGBA888 (0xf << 26)
#define DISPPLANE_STEREO_ENABLE (1 << 25)
#define DISPPLANE_STEREO_DISABLE 0
-#define DISPPLANE_PIPE_CSC_ENABLE (1 << 24)
+#define DISPPLANE_PIPE_CSC_ENABLE (1 << 24) /* ilk+ */
#define DISPPLANE_SEL_PIPE_SHIFT 24
#define DISPPLANE_SEL_PIPE_MASK (3 << DISPPLANE_SEL_PIPE_SHIFT)
#define DISPPLANE_SEL_PIPE(pipe) ((pipe) << DISPPLANE_SEL_PIPE_SHIFT)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 8acc729925af..4e61cf4a911a 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -401,7 +401,8 @@ static void skl_color_commit(const struct intel_crtc_state *crtc_state)
*/
if (crtc_state->gamma_enable)
val |= SKL_BOTTOM_COLOR_GAMMA_ENABLE;
- val |= SKL_BOTTOM_COLOR_CSC_ENABLE;
+ if (crtc_state->csc_enable)
+ val |= SKL_BOTTOM_COLOR_CSC_ENABLE;
I915_WRITE(SKL_BOTTOM_COLOR(pipe), val);
I915_WRITE(GAMMA_MODE(crtc->pipe), crtc_state->gamma_mode);
@@ -668,6 +669,10 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
crtc_state->gamma_enable = true;
+ if (INTEL_GEN(dev_priv) >= 9 ||
+ IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+ crtc_state->csc_enable = true;
+
/* Always allow legacy gamma LUT with no further checking. */
if (crtc_state_is_legacy_gamma(crtc_state)) {
crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ebf26c0e0a98..c802055d23a6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3224,7 +3224,7 @@ static u32 i9xx_plane_ctl_crtc(const struct intel_crtc_state *crtc_state)
if (crtc_state->gamma_enable)
dspcntr |= DISPPLANE_GAMMA_ENABLE;
- if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+ if (crtc_state->csc_enable)
dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
if (INTEL_GEN(dev_priv) < 5)
@@ -3705,7 +3705,8 @@ u32 skl_plane_ctl_crtc(const struct intel_crtc_state *crtc_state)
if (crtc_state->gamma_enable)
plane_ctl |= PLANE_CTL_PIPE_GAMMA_ENABLE;
- plane_ctl |= PLANE_CTL_PIPE_CSC_ENABLE;
+ if (crtc_state->csc_enable)
+ plane_ctl |= PLANE_CTL_PIPE_CSC_ENABLE;
return plane_ctl;
}
@@ -3760,7 +3761,8 @@ u32 glk_plane_color_ctl_crtc(const struct intel_crtc_state *crtc_state)
if (crtc_state->gamma_enable)
plane_color_ctl |= PLANE_COLOR_PIPE_GAMMA_ENABLE;
- plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE;
+ if (crtc_state->csc_enable)
+ plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE;
return plane_color_ctl;
}
@@ -8090,6 +8092,10 @@ static void i9xx_get_pipe_color_config(struct intel_crtc_state *crtc_state)
if (tmp & DISPPLANE_GAMMA_ENABLE)
crtc_state->gamma_enable = true;
+
+ if (!HAS_GMCH_DISPLAY(dev_priv) &&
+ tmp & DISPPLANE_PIPE_CSC_ENABLE)
+ crtc_state->csc_enable = true;
}
static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
@@ -9861,6 +9867,9 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
if (tmp & SKL_BOTTOM_COLOR_GAMMA_ENABLE)
pipe_config->gamma_enable = true;
+
+ if (tmp & SKL_BOTTOM_COLOR_CSC_ENABLE)
+ pipe_config->csc_enable = true;
} else {
i9xx_get_pipe_color_config(pipe_config);
}
@@ -10197,7 +10206,7 @@ static u32 i9xx_cursor_ctl_crtc(const struct intel_crtc_state *crtc_state)
if (crtc_state->gamma_enable)
cntl = MCURSOR_GAMMA_ENABLE;
- if (HAS_DDI(dev_priv))
+ if (crtc_state->csc_enable)
cntl |= MCURSOR_PIPE_CSC_ENABLE;
if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv))
@@ -12103,6 +12112,7 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
PIPE_CONF_CHECK_X(gamma_mode);
PIPE_CONF_CHECK_BOOL(gamma_enable);
+ PIPE_CONF_CHECK_BOOL(csc_enable);
PIPE_CONF_CHECK_P(shared_dpll);
PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f0db61471209..b3e3fa289aed 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -954,6 +954,9 @@ struct intel_crtc_state {
/* enable pipe gamma? */
bool gamma_enable;
+ /* enable pipe csc? */
+ bool csc_enable;
+
/* Display Stream compression state */
struct {
bool compression_enable;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index db373e3ac601..610398607e8e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -921,13 +921,12 @@ vlv_plane_get_hw_state(struct intel_plane *plane,
static u32 ivb_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
{
- struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
u32 sprctl = 0;
if (crtc_state->gamma_enable)
sprctl |= SPRITE_GAMMA_ENABLE;
- if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+ if (crtc_state->csc_enable)
sprctl |= SPRITE_PIPE_CSC_ENABLE;
return sprctl;
@@ -1118,6 +1117,9 @@ static u32 g4x_sprite_ctl_crtc(const struct intel_crtc_state *crtc_state)
if (crtc_state->gamma_enable)
dvscntr |= DVS_GAMMA_ENABLE;
+ if (crtc_state->csc_enable)
+ dvscntr |= DVS_PIPE_CSC_ENABLE;
+
return dvscntr;
}
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 10/13] drm/i915: Turn off pipe gamma when it's not needed
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (8 preceding siblings ...)
2019-02-05 16:08 ` [PATCH v2 09/13] drm/i915: Track pipe csc enable " Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 11/13] drm/i915: Turn off pipe CSC " Ville Syrjala
` (6 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The pipe internal precision is higher than what we currently program to
the degamma/gamma LUTs. We can get a higher quality image by bypassing
the LUTs when they're not needed. Let's do that.
Each plane has its own control bit for this, so we have to update
all active planes. The way we've done this we don't actually have
to run through the whole .check_plane() thing. And we actually
do the .color_check() after .check_plane() so we couldn't even do
that without shuffling the code around.
Additionally on pre-skl we have to update the primary plane regardless
of whether it's active or not on account of the primary plane gamma
enable bit also affecting the pipe bottom color.
v2: Drop the '.' from patch title (Uma)
Fix 'primayr' typo (Uma,Matt)
Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 55 ++++++++++++++++++++++++++++--
1 file changed, 53 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 4e61cf4a911a..17fa2df3ad4d 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -637,6 +637,51 @@ void intel_color_commit(const struct intel_crtc_state *crtc_state)
dev_priv->display.color_commit(crtc_state);
}
+static bool need_plane_update(struct intel_plane *plane,
+ const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+
+ /*
+ * On pre-SKL the pipe gamma enable and pipe csc enable for
+ * the pipe bottom color are configured via the primary plane.
+ * We have to reconfigure that even if the plane is inactive.
+ */
+ return crtc_state->active_planes & BIT(plane->id) ||
+ (INTEL_GEN(dev_priv) < 9 &&
+ plane->id == PLANE_PRIMARY);
+}
+
+static int
+intel_color_add_affected_planes(struct intel_crtc_state *new_crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ struct intel_atomic_state *state =
+ to_intel_atomic_state(new_crtc_state->base.state);
+ const struct intel_crtc_state *old_crtc_state =
+ intel_atomic_get_old_crtc_state(state, crtc);
+ struct intel_plane *plane;
+
+ if (new_crtc_state->gamma_enable == old_crtc_state->gamma_enable)
+ return 0;
+
+ for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
+ struct intel_plane_state *plane_state;
+
+ if (!need_plane_update(plane, new_crtc_state))
+ continue;
+
+ plane_state = intel_atomic_get_plane_state(state, plane);
+ if (IS_ERR(plane_state))
+ return PTR_ERR(plane_state);
+
+ new_crtc_state->update_planes |= BIT(plane->id);
+ }
+
+ return 0;
+}
+
static int check_lut_size(const struct drm_property_blob *lut, int expected)
{
int len;
@@ -661,20 +706,26 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
const struct drm_property_blob *degamma_lut = crtc_state->base.degamma_lut;
int gamma_length, degamma_length;
u32 gamma_tests, degamma_tests;
+ int ret;
degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
- crtc_state->gamma_enable = true;
+ crtc_state->gamma_enable = gamma_lut || degamma_lut;
if (INTEL_GEN(dev_priv) >= 9 ||
IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
crtc_state->csc_enable = true;
+ ret = intel_color_add_affected_planes(crtc_state);
+ if (ret)
+ return ret;
+
/* Always allow legacy gamma LUT with no further checking. */
- if (crtc_state_is_legacy_gamma(crtc_state)) {
+ if (!crtc_state->gamma_enable ||
+ crtc_state_is_legacy_gamma(crtc_state)) {
crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
return 0;
}
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 11/13] drm/i915: Turn off pipe CSC when it's not needed
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (9 preceding siblings ...)
2019-02-05 16:08 ` [PATCH v2 10/13] drm/i915: Turn off pipe gamma when it's not needed Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 12/13] drm/i915: Disable pipe gamma when C8 pixel format is used Ville Syrjala
` (5 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
As with pipe gamma we can avoid the potential precision loss from
the pipe csc unit when there is no need to use it. And again
we need the same logic for updating the planes.
v2: Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 17fa2df3ad4d..df7c212bf68d 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -663,7 +663,8 @@ intel_color_add_affected_planes(struct intel_crtc_state *new_crtc_state)
intel_atomic_get_old_crtc_state(state, crtc);
struct intel_plane *plane;
- if (new_crtc_state->gamma_enable == old_crtc_state->gamma_enable)
+ if (new_crtc_state->gamma_enable == old_crtc_state->gamma_enable &&
+ new_crtc_state->csc_enable == old_crtc_state->csc_enable)
return 0;
for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
@@ -704,6 +705,7 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
const struct drm_property_blob *degamma_lut = crtc_state->base.degamma_lut;
+ bool limited_color_range = false;
int gamma_length, degamma_length;
u32 gamma_tests, degamma_tests;
int ret;
@@ -717,7 +719,11 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
if (INTEL_GEN(dev_priv) >= 9 ||
IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
- crtc_state->csc_enable = true;
+ limited_color_range = crtc_state->limited_color_range;
+
+ crtc_state->csc_enable =
+ crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
+ crtc_state->base.ctm || limited_color_range;
ret = intel_color_add_affected_planes(crtc_state);
if (ret)
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 12/13] drm/i915: Disable pipe gamma when C8 pixel format is used
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (10 preceding siblings ...)
2019-02-05 16:08 ` [PATCH v2 11/13] drm/i915: Turn off pipe CSC " Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 13/13] drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable() Ville Syrjala
` (4 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Planes scanning out C8 will want to use the legacy lut as
their palette. That means the LUT content are unlikely to
be useful for gamma correction on other planes. Thus we
should disable pipe gamma for all the other planes. And
we should reject any non legacy LUT configurations when
C8 planes are present.
Fixes the appearance of the hw cursor when running
X -depth 8.
Note that CHV with it's independent CGM degamma/gamma LUTs
could probably use the CGM for gamma correction even when
the legacy LUT is used for C8. But that would require a
new uapi for configuring the legacy LUT and CGM LUTs at
the same time. Totally not worth it.
v2: Fix typo (Uma)
Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_atomic_plane.c | 5 +++++
drivers/gpu/drm/i915/intel_color.c | 8 +++++++-
drivers/gpu/drm/i915/intel_drv.h | 1 +
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index a1a263026574..1c3c1eeafd1a 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -119,6 +119,7 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
new_crtc_state->active_planes &= ~BIT(plane->id);
new_crtc_state->nv12_planes &= ~BIT(plane->id);
+ new_crtc_state->c8_planes &= ~BIT(plane->id);
new_plane_state->base.visible = false;
if (!new_plane_state->base.crtc && !old_plane_state->base.crtc)
@@ -136,6 +137,10 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
new_plane_state->base.fb->format->format == DRM_FORMAT_NV12)
new_crtc_state->nv12_planes |= BIT(plane->id);
+ if (new_plane_state->base.visible &&
+ new_plane_state->base.fb->format->format == DRM_FORMAT_C8)
+ new_crtc_state->c8_planes |= BIT(plane->id);
+
if (new_plane_state->base.visible || old_plane_state->base.visible)
new_crtc_state->update_planes |= BIT(plane->id);
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index df7c212bf68d..5c7789e9fed5 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -715,7 +715,13 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
- crtc_state->gamma_enable = gamma_lut || degamma_lut;
+ /* C8 needs the legacy LUT all to itself */
+ if (crtc_state->c8_planes &&
+ !crtc_state_is_legacy_gamma(crtc_state))
+ return -EINVAL;
+
+ crtc_state->gamma_enable = (gamma_lut || degamma_lut) &&
+ !crtc_state->c8_planes;
if (INTEL_GEN(dev_priv) >= 9 ||
IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b3e3fa289aed..593a7a49070c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -935,6 +935,7 @@ struct intel_crtc_state {
/* bitmask of visible planes (enum plane_id) */
u8 active_planes;
u8 nv12_planes;
+ u8 c8_planes;
/* bitmask of planes that will be updated during the commit */
u8 update_planes;
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 13/13] drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable()
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (11 preceding siblings ...)
2019-02-05 16:08 ` [PATCH v2 12/13] drm/i915: Disable pipe gamma when C8 pixel format is used Ville Syrjala
@ 2019-02-05 16:08 ` Ville Syrjala
2019-02-07 15:58 ` Maarten Lankhorst
2019-02-05 16:57 ` ✗ Fi.CI.CHECKPATCH: warning for Enable/disable gamma/csc dynamically and fix C8 (rev2) Patchwork
` (3 subsequent siblings)
16 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2019-02-05 16:08 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
On g4x+ we depend on the primary plane DSPCNTR gamma/csc enable
bits for the pipe bottom color. To guarantee that those are
correct already when enabling the crtc let's do an explicit
->disable_plane() call before enabling the pipe.
On skl+ this will be handled by the explicit PIPE_BOTTOM_COLOR
register which is already part of the normal color commit we
do durign crtc enable.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_color.c | 4 ++++
drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 5c7789e9fed5..79c3adaefadc 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -663,6 +663,10 @@ intel_color_add_affected_planes(struct intel_crtc_state *new_crtc_state)
intel_atomic_get_old_crtc_state(state, crtc);
struct intel_plane *plane;
+ if (!new_crtc_state->base.active ||
+ drm_atomic_crtc_needs_modeset(&new_crtc_state->base))
+ return 0;
+
if (new_crtc_state->gamma_enable == old_crtc_state->gamma_enable &&
new_crtc_state->csc_enable == old_crtc_state->csc_enable)
return 0;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c802055d23a6..f36ac23f7195 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5715,6 +5715,14 @@ static void intel_encoders_update_pipe(struct drm_crtc *crtc,
}
}
+static void intel_disable_primary_plane(const struct intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+
+ plane->disable_plane(plane, crtc_state);
+}
+
static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
struct drm_atomic_state *old_state)
{
@@ -5780,6 +5788,8 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
*/
intel_color_load_luts(pipe_config);
intel_color_commit(pipe_config);
+ /* update DSPCNTR to configure gamma for pipe bottom color */
+ intel_disable_primary_plane(pipe_config);
if (dev_priv->display.initial_watermarks != NULL)
dev_priv->display.initial_watermarks(old_intel_state, pipe_config);
@@ -5909,6 +5919,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
*/
intel_color_load_luts(pipe_config);
intel_color_commit(pipe_config);
+ /* update DSPCNTR to configure gamma/csc for pipe bottom color */
+ if (INTEL_GEN(dev_priv) < 9)
+ intel_disable_primary_plane(pipe_config);
/*
* Display WA #1153: enable hardware to bypass the alpha math
@@ -6274,6 +6287,8 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
intel_color_load_luts(pipe_config);
intel_color_commit(pipe_config);
+ /* update DSPCNTR to configure gamma for pipe bottom color */
+ intel_disable_primary_plane(pipe_config);
dev_priv->display.initial_watermarks(old_intel_state,
pipe_config);
@@ -6331,6 +6346,8 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
intel_color_load_luts(pipe_config);
intel_color_commit(pipe_config);
+ /* update DSPCNTR to configure gamma for pipe bottom color */
+ intel_disable_primary_plane(pipe_config);
if (dev_priv->display.initial_watermarks != NULL)
dev_priv->display.initial_watermarks(old_intel_state,
--
2.19.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for Enable/disable gamma/csc dynamically and fix C8 (rev2)
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (12 preceding siblings ...)
2019-02-05 16:08 ` [PATCH v2 13/13] drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable() Ville Syrjala
@ 2019-02-05 16:57 ` Patchwork
2019-02-05 17:02 ` ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2019-02-05 16:57 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: Enable/disable gamma/csc dynamically and fix C8 (rev2)
URL : https://patchwork.freedesktop.org/series/55081/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
9cdee3f9e7a4 drm/i915: Split the gamma/csc enable bits from the plane_ctl() function
0fd667c43e7a drm/i915: Precompute gamma_mode
c8911d530e36 drm/i915: Constify the state arguments to the color management stuff
d6d5f9a92dd0 drm/i915: Pull GAMMA_MODE write out from haswell_load_luts()
19b423bb1346 drm/i915: Split color mgmt based on single vs. double buffered registers
a03988b0bb93 drm/i915: Move LUT programming to happen after vblank waits
b6745ce0a618 drm/i915: Populate gamma_mode for all platforms
-:33: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#33: FILE: drivers/gpu/drm/i915/i915_reg.h:5600:
+#define PIPECONF_GAMMA_MODE(x) ((x)<<24) /* pass in GAMMA_MODE_MODE_* */
^
total: 0 errors, 0 warnings, 1 checks, 146 lines checked
48b22ec5be11 drm/i915: Track pipe gamma enable/disable in crtc state
05d03c26af9b drm/i915: Track pipe csc enable in crtc state
ce89cf314d8c drm/i915: Turn off pipe gamma when it's not needed
e803ec9dc66b drm/i915: Turn off pipe CSC when it's not needed
29f28a57e210 drm/i915: Disable pipe gamma when C8 pixel format is used
9e2c3db18ed4 drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable()
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✗ Fi.CI.SPARSE: warning for Enable/disable gamma/csc dynamically and fix C8 (rev2)
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (13 preceding siblings ...)
2019-02-05 16:57 ` ✗ Fi.CI.CHECKPATCH: warning for Enable/disable gamma/csc dynamically and fix C8 (rev2) Patchwork
@ 2019-02-05 17:02 ` Patchwork
2019-02-05 17:24 ` ✓ Fi.CI.BAT: success " Patchwork
2019-02-05 21:19 ` ✓ Fi.CI.IGT: " Patchwork
16 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2019-02-05 17:02 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: Enable/disable gamma/csc dynamically and fix C8 (rev2)
URL : https://patchwork.freedesktop.org/series/55081/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Split the gamma/csc enable bits from the plane_ctl() function
Okay!
Commit: drm/i915: Precompute gamma_mode
Okay!
Commit: drm/i915: Constify the state arguments to the color management stuff
Okay!
Commit: drm/i915: Pull GAMMA_MODE write out from haswell_load_luts()
Okay!
Commit: drm/i915: Split color mgmt based on single vs. double buffered registers
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3565:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3577:16: warning: expression using sizeof(void)
Commit: drm/i915: Move LUT programming to happen after vblank waits
Okay!
Commit: drm/i915: Populate gamma_mode for all platforms
Okay!
Commit: drm/i915: Track pipe gamma enable/disable in crtc state
Okay!
Commit: drm/i915: Track pipe csc enable in crtc state
Okay!
Commit: drm/i915: Turn off pipe gamma when it's not needed
Okay!
Commit: drm/i915: Turn off pipe CSC when it's not needed
Okay!
Commit: drm/i915: Disable pipe gamma when C8 pixel format is used
Okay!
Commit: drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable()
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✓ Fi.CI.BAT: success for Enable/disable gamma/csc dynamically and fix C8 (rev2)
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (14 preceding siblings ...)
2019-02-05 17:02 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-02-05 17:24 ` Patchwork
2019-02-05 21:19 ` ✓ Fi.CI.IGT: " Patchwork
16 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2019-02-05 17:24 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: Enable/disable gamma/csc dynamically and fix C8 (rev2)
URL : https://patchwork.freedesktop.org/series/55081/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5542 -> Patchwork_12144
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/55081/revisions/2/mbox/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_12144:
### IGT changes ###
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@pm_rpm@module-reload:
- {fi-whl-u}: PASS -> DMESG-WARN
* {igt@runner@aborted}:
- {fi-whl-u}: NOTRUN -> FAIL
Known issues
------------
Here are the changes found in Patchwork_12144 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_busy@basic-flip-b:
- fi-gdg-551: PASS -> FAIL [fdo#103182]
* igt@kms_flip@basic-flip-vs-modeset:
- fi-skl-6700hq: PASS -> DMESG-WARN [fdo#105998]
* igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
- fi-byt-clapper: PASS -> FAIL [fdo#107362]
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362] +1
#### Possible fixes ####
* igt@i915_selftest@live_execlists:
- {fi-icl-y}: INCOMPLETE -> PASS
* igt@kms_busy@basic-flip-a:
- fi-gdg-551: FAIL [fdo#103182] -> PASS
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-byt-clapper: FAIL [fdo#103191] / [fdo#107362] -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
Participating hosts (51 -> 42)
------------------------------
Missing (9): fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-apl-guc fi-bdw-samus fi-skl-6600u
Build changes
-------------
* Linux: CI_DRM_5542 -> Patchwork_12144
CI_DRM_5542: 5e5b9350e18d90a1c3ac9b29b2c6fae794323d23 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4810: 1f89f1a04016cef20aa278ad05508cafdb9976f5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12144: 9e2c3db18ed4b847ec67995ca39d3f3ef3e99472 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
9e2c3db18ed4 drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable()
29f28a57e210 drm/i915: Disable pipe gamma when C8 pixel format is used
e803ec9dc66b drm/i915: Turn off pipe CSC when it's not needed
ce89cf314d8c drm/i915: Turn off pipe gamma when it's not needed
05d03c26af9b drm/i915: Track pipe csc enable in crtc state
48b22ec5be11 drm/i915: Track pipe gamma enable/disable in crtc state
b6745ce0a618 drm/i915: Populate gamma_mode for all platforms
a03988b0bb93 drm/i915: Move LUT programming to happen after vblank waits
19b423bb1346 drm/i915: Split color mgmt based on single vs. double buffered registers
d6d5f9a92dd0 drm/i915: Pull GAMMA_MODE write out from haswell_load_luts()
c8911d530e36 drm/i915: Constify the state arguments to the color management stuff
0fd667c43e7a drm/i915: Precompute gamma_mode
9cdee3f9e7a4 drm/i915: Split the gamma/csc enable bits from the plane_ctl() function
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12144/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✓ Fi.CI.IGT: success for Enable/disable gamma/csc dynamically and fix C8 (rev2)
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
` (15 preceding siblings ...)
2019-02-05 17:24 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-02-05 21:19 ` Patchwork
16 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2019-02-05 21:19 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: Enable/disable gamma/csc dynamically and fix C8 (rev2)
URL : https://patchwork.freedesktop.org/series/55081/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5542_full -> Patchwork_12144_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Known issues
------------
Here are the changes found in Patchwork_12144_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_schedule@pi-ringfull-bsd2:
- shard-kbl: NOTRUN -> FAIL [fdo#103158]
* igt@gem_mmap_gtt@hang:
- shard-glk: PASS -> FAIL [fdo#109469]
* igt@kms_available_modes_crc@available_mode_test_crc:
- shard-kbl: PASS -> FAIL [fdo#106641]
* igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
- shard-hsw: PASS -> DMESG-WARN [fdo#107956]
* igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
- shard-kbl: NOTRUN -> DMESG-WARN [fdo#107956] +1
* igt@kms_color@pipe-c-legacy-gamma:
- shard-apl: PASS -> FAIL [fdo#104782]
* igt@kms_cursor_crc@cursor-128x128-onscreen:
- shard-apl: PASS -> FAIL [fdo#103232] +2
* igt@kms_cursor_crc@cursor-256x256-random:
- shard-glk: PASS -> FAIL [fdo#103232] +18
* igt@kms_cursor_crc@cursor-64x64-suspend:
- shard-apl: PASS -> FAIL [fdo#103191] / [fdo#103232]
* igt@kms_cursor_legacy@flip-vs-cursor-atomic:
- shard-glk: PASS -> FAIL [fdo#102670]
* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
- shard-glk: PASS -> FAIL [fdo#103167] +3
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
- shard-glk: PASS -> FAIL [fdo#108145]
* igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
- shard-kbl: NOTRUN -> FAIL [fdo#108145]
* igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
- shard-glk: PASS -> FAIL [fdo#103166] +4
* igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
- shard-apl: PASS -> FAIL [fdo#103166] +2
* igt@kms_rotation_crc@multiplane-rotation-cropping-top:
- shard-kbl: PASS -> FAIL [fdo#109016]
#### Possible fixes ####
* igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
- shard-glk: FAIL [fdo#108145] -> PASS
* igt@kms_cursor_crc@cursor-256x85-random:
- shard-apl: FAIL [fdo#103232] -> PASS +3
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-apl: FAIL [fdo#102887] / [fdo#105363] -> PASS
* {igt@kms_flip@flip-vs-suspend-interruptible}:
- shard-kbl: INCOMPLETE [fdo#103665] -> PASS
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
- shard-glk: FAIL [fdo#103167] -> PASS
* igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
- shard-apl: FAIL [fdo#108948] -> PASS
* igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
- shard-apl: FAIL [fdo#103166] -> PASS +2
* igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
- shard-glk: FAIL [fdo#103166] -> PASS +1
* igt@pm_rc6_residency@rc6-accuracy:
- shard-kbl: {SKIP} [fdo#109271] -> PASS
#### Warnings ####
* igt@i915_suspend@shrink:
- shard-glk: INCOMPLETE [fdo#103359] / [fdo#106886] / [k.org#198133] -> DMESG-WARN [fdo#109244]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102670]: https://bugs.freedesktop.org/show_bug.cgi?id=102670
[fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
[fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
[fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
[fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
[fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
[fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109469]: https://bugs.freedesktop.org/show_bug.cgi?id=109469
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
Participating hosts (7 -> 5)
------------------------------
Missing (2): shard-skl shard-iclb
Build changes
-------------
* Linux: CI_DRM_5542 -> Patchwork_12144
CI_DRM_5542: 5e5b9350e18d90a1c3ac9b29b2c6fae794323d23 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4810: 1f89f1a04016cef20aa278ad05508cafdb9976f5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_12144: 9e2c3db18ed4b847ec67995ca39d3f3ef3e99472 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12144/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 06/13] drm/i915: Move LUT programming to happen after vblank waits
2019-02-05 16:08 ` [PATCH v2 06/13] drm/i915: Move LUT programming to happen after vblank waits Ville Syrjala
@ 2019-02-07 15:46 ` Maarten Lankhorst
2019-02-07 19:24 ` Ville Syrjälä
0 siblings, 1 reply; 26+ messages in thread
From: Maarten Lankhorst @ 2019-02-07 15:46 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
Hey,
Op 05-02-2019 om 17:08 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The LUTs are single buffered so we should program them after
> the double buffered pipe updates have been latched by the
> hardware.
>
> We'll also fix up the IPS vs. split gamma w/a to do the IPS
> disable like everyone else. Note that this is currently dead
> code as we don't use the split gamma mode on HSW, but that
> will be fixed up shortly.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Hmm I guess there is no good way to do this, either before or after. :/
The idea was that we do it before was to make sure that the frame after the flip would be perfect. I guess
we can't be sure, and after is as good as before..
For patch 1-6:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms
2019-02-05 16:08 ` [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms Ville Syrjala
@ 2019-02-07 15:49 ` Maarten Lankhorst
2019-02-07 16:27 ` Ville Syrjälä
0 siblings, 1 reply; 26+ messages in thread
From: Maarten Lankhorst @ 2019-02-07 15:49 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
Op 05-02-2019 om 17:08 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On pre-HSW gamma mode is configured via PIPECONF. The bits are
> the same except shifted up, so we can reuse just store them in
> crtc_state->gamma_mode in the HSW+ way, allowing us to share
> some code later.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 10 ++++-
> drivers/gpu/drm/i915/intel_color.c | 60 +++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_display.c | 14 ++++++-
> 3 files changed, 66 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ede54fdc1676..41111a17e519 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5590,9 +5590,15 @@ enum {
> #define PIPECONF_SINGLE_WIDE 0
> #define PIPECONF_PIPE_UNLOCKED 0
> #define PIPECONF_PIPE_LOCKED (1 << 25)
> -#define PIPECONF_PALETTE 0
> -#define PIPECONF_GAMMA (1 << 24)
> #define PIPECONF_FORCE_BORDER (1 << 25)
> +#define PIPECONF_GAMMA_MODE_MASK_I9XX (1 << 24) /* gmch */
> +#define PIPECONF_GAMMA_MODE_MASK_ILK (3 << 24) /* ilk-ivb */
> +#define PIPECONF_GAMMA_MODE_8BIT (0 << 24) /* gmch,ilk-ivb */
> +#define PIPECONF_GAMMA_MODE_10BIT (1 << 24) /* gmch,ilk-ivb */
> +#define PIPECONF_GAMMA_MODE_12BIT (2 << 24) /* ilk-ivb */
> +#define PIPECONF_GAMMA_MODE_SPLIT (3 << 24) /* ivb */
> +#define PIPECONF_GAMMA_MODE(x) ((x)<<24) /* pass in GAMMA_MODE_MODE_* */
> +#define PIPECONF_GAMMA_MODE_SHIFT 24
> #define PIPECONF_INTERLACE_MASK (7 << 21)
> #define PIPECONF_INTERLACE_MASK_HSW (3 << 21)
> /* Note that pre-gen3 does not support interlaced display directly. Panel
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 2a371eed8061..e7dd07490c68 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -351,6 +351,32 @@ static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
> i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
> }
>
> +static void i9xx_color_commit(const struct intel_crtc_state *crtc_state)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum pipe pipe = crtc->pipe;
> + u32 val;
> +
> + val = I915_READ(PIPECONF(pipe));
> + val &= ~PIPECONF_GAMMA_MODE_MASK_I9XX;
> + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> + I915_WRITE(PIPECONF(pipe), val);
> +}
> +
> +static void ilk_color_commit(const struct intel_crtc_state *crtc_state)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum pipe pipe = crtc->pipe;
> + u32 val;
> +
> + val = I915_READ(PIPECONF(pipe));
> + val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
> + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> + I915_WRITE(PIPECONF(pipe), val);
> +}
> +
> static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> @@ -585,8 +611,7 @@ void intel_color_commit(const struct intel_crtc_state *crtc_state)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>
> - if (dev_priv->display.color_commit)
> - dev_priv->display.color_commit(crtc_state);
> + dev_priv->display.color_commit(crtc_state);
> }
>
> static int check_lut_size(const struct drm_property_blob *lut, int expected)
> @@ -649,20 +674,25 @@ void intel_color_init(struct intel_crtc *crtc)
>
> drm_mode_crtc_set_gamma_size(&crtc->base, 256);
>
> - if (IS_CHERRYVIEW(dev_priv)) {
> - dev_priv->display.load_luts = cherryview_load_luts;
> - } else if (IS_HASWELL(dev_priv)) {
> - dev_priv->display.load_luts = i9xx_load_luts;
> - dev_priv->display.color_commit = hsw_color_commit;
> - } else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
> - IS_BROXTON(dev_priv)) {
> - dev_priv->display.load_luts = broadwell_load_luts;
> - dev_priv->display.color_commit = hsw_color_commit;
> - } else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> - dev_priv->display.load_luts = glk_load_luts;
> - dev_priv->display.color_commit = hsw_color_commit;
> + if (HAS_GMCH_DISPLAY(dev_priv)) {
> + if (IS_CHERRYVIEW(dev_priv))
> + dev_priv->display.load_luts = cherryview_load_luts;
> + else
> + dev_priv->display.load_luts = i9xx_load_luts;
> +
> + dev_priv->display.color_commit = i9xx_color_commit;
> } else {
> - dev_priv->display.load_luts = i9xx_load_luts;
> + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> + dev_priv->display.load_luts = glk_load_luts;
> + else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> + dev_priv->display.load_luts = broadwell_load_luts;
> + else
> + dev_priv->display.load_luts = i9xx_load_luts;
> +
> + if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
> + dev_priv->display.color_commit = hsw_color_commit;
> + else
> + dev_priv->display.color_commit = ilk_color_commit;
> }
>
> /* Enable color management support when we have degamma & gamma LUTs. */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ad5d39d81d6e..8e93459f312f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3450,7 +3450,7 @@ static void i9xx_disable_plane(struct intel_plane *plane,
> *
> * On pre-g4x there is no way to gamma correct the
> * pipe bottom color but we'll keep on doing this
> - * anyway.
> + * anyway so that the crtc state readout works correctly.
> */
> dspcntr = i9xx_plane_ctl_crtc(crtc_state);
>
> @@ -7674,6 +7674,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
> crtc_state->limited_color_range)
> pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
>
> + pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> +
> I915_WRITE(PIPECONF(crtc->pipe), pipeconf);
> POSTING_READ(PIPECONF(crtc->pipe));
> }
> @@ -8126,6 +8128,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> (tmp & PIPECONF_COLOR_RANGE_SELECT))
> pipe_config->limited_color_range = true;
>
> + pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_I9XX) >>
> + PIPECONF_GAMMA_MODE_SHIFT;
> +
> if (INTEL_GEN(dev_priv) < 4)
> pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
>
> @@ -8665,6 +8670,8 @@ static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state)
> if (crtc_state->limited_color_range)
> val |= PIPECONF_COLOR_RANGE_SELECT;
>
> + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> +
> I915_WRITE(PIPECONF(pipe), val);
> POSTING_READ(PIPECONF(pipe));
> }
> @@ -9199,6 +9206,9 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> if (tmp & PIPECONF_COLOR_RANGE_SELECT)
> pipe_config->limited_color_range = true;
>
> + pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >>
> + PIPECONF_GAMMA_MODE_SHIFT;
> +
> if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
> struct intel_shared_dpll *pll;
> enum intel_dpll_id pll_id;
> @@ -12069,6 +12079,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>
> PIPE_CONF_CHECK_BOOL(double_wide);
>
> + PIPE_CONF_CHECK_X(gamma_mode);
Hmm... We should only check this if adjust is unset, or we will start skipping fastset on a lot of platforms.
With that fixed:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 13/13] drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable()
2019-02-05 16:08 ` [PATCH v2 13/13] drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable() Ville Syrjala
@ 2019-02-07 15:58 ` Maarten Lankhorst
2019-02-07 16:29 ` Ville Syrjälä
0 siblings, 1 reply; 26+ messages in thread
From: Maarten Lankhorst @ 2019-02-07 15:58 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
Op 05-02-2019 om 17:08 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On g4x+ we depend on the primary plane DSPCNTR gamma/csc enable
> bits for the pipe bottom color. To guarantee that those are
> correct already when enabling the crtc let's do an explicit
> ->disable_plane() call before enabling the pipe.
>
> On skl+ this will be handled by the explicit PIPE_BOTTOM_COLOR
> register which is already part of the normal color commit we
> do durign crtc enable.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_color.c | 4 ++++
> drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 5c7789e9fed5..79c3adaefadc 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -663,6 +663,10 @@ intel_color_add_affected_planes(struct intel_crtc_state *new_crtc_state)
> intel_atomic_get_old_crtc_state(state, crtc);
> struct intel_plane *plane;
>
> + if (!new_crtc_state->base.active ||
> + drm_atomic_crtc_needs_modeset(&new_crtc_state->base))
> + return 0;
Why the needs_modeset skip? We will have already added all planes anyway, but still.
Oh well, assuming it's for a good reason
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> if (new_crtc_state->gamma_enable == old_crtc_state->gamma_enable &&
> new_crtc_state->csc_enable == old_crtc_state->csc_enable)
> return 0;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c802055d23a6..f36ac23f7195 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5715,6 +5715,14 @@ static void intel_encoders_update_pipe(struct drm_crtc *crtc,
> }
> }
>
> +static void intel_disable_primary_plane(const struct intel_crtc_state *crtc_state)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> +
> + plane->disable_plane(plane, crtc_state);
> +}
> +
> static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> struct drm_atomic_state *old_state)
> {
> @@ -5780,6 +5788,8 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> */
> intel_color_load_luts(pipe_config);
> intel_color_commit(pipe_config);
> + /* update DSPCNTR to configure gamma for pipe bottom color */
> + intel_disable_primary_plane(pipe_config);
>
> if (dev_priv->display.initial_watermarks != NULL)
> dev_priv->display.initial_watermarks(old_intel_state, pipe_config);
> @@ -5909,6 +5919,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> */
> intel_color_load_luts(pipe_config);
> intel_color_commit(pipe_config);
> + /* update DSPCNTR to configure gamma/csc for pipe bottom color */
> + if (INTEL_GEN(dev_priv) < 9)
> + intel_disable_primary_plane(pipe_config);
>
> /*
> * Display WA #1153: enable hardware to bypass the alpha math
> @@ -6274,6 +6287,8 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>
> intel_color_load_luts(pipe_config);
> intel_color_commit(pipe_config);
> + /* update DSPCNTR to configure gamma for pipe bottom color */
> + intel_disable_primary_plane(pipe_config);
>
> dev_priv->display.initial_watermarks(old_intel_state,
> pipe_config);
> @@ -6331,6 +6346,8 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>
> intel_color_load_luts(pipe_config);
> intel_color_commit(pipe_config);
> + /* update DSPCNTR to configure gamma for pipe bottom color */
> + intel_disable_primary_plane(pipe_config);
>
> if (dev_priv->display.initial_watermarks != NULL)
> dev_priv->display.initial_watermarks(old_intel_state,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms
2019-02-07 15:49 ` Maarten Lankhorst
@ 2019-02-07 16:27 ` Ville Syrjälä
2019-02-07 17:27 ` Ville Syrjälä
2019-02-08 8:51 ` Maarten Lankhorst
0 siblings, 2 replies; 26+ messages in thread
From: Ville Syrjälä @ 2019-02-07 16:27 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Thu, Feb 07, 2019 at 04:49:47PM +0100, Maarten Lankhorst wrote:
> Op 05-02-2019 om 17:08 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > On pre-HSW gamma mode is configured via PIPECONF. The bits are
> > the same except shifted up, so we can reuse just store them in
> > crtc_state->gamma_mode in the HSW+ way, allowing us to share
> > some code later.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 10 ++++-
> > drivers/gpu/drm/i915/intel_color.c | 60 +++++++++++++++++++++-------
> > drivers/gpu/drm/i915/intel_display.c | 14 ++++++-
> > 3 files changed, 66 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index ede54fdc1676..41111a17e519 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5590,9 +5590,15 @@ enum {
> > #define PIPECONF_SINGLE_WIDE 0
> > #define PIPECONF_PIPE_UNLOCKED 0
> > #define PIPECONF_PIPE_LOCKED (1 << 25)
> > -#define PIPECONF_PALETTE 0
> > -#define PIPECONF_GAMMA (1 << 24)
> > #define PIPECONF_FORCE_BORDER (1 << 25)
> > +#define PIPECONF_GAMMA_MODE_MASK_I9XX (1 << 24) /* gmch */
> > +#define PIPECONF_GAMMA_MODE_MASK_ILK (3 << 24) /* ilk-ivb */
> > +#define PIPECONF_GAMMA_MODE_8BIT (0 << 24) /* gmch,ilk-ivb */
> > +#define PIPECONF_GAMMA_MODE_10BIT (1 << 24) /* gmch,ilk-ivb */
> > +#define PIPECONF_GAMMA_MODE_12BIT (2 << 24) /* ilk-ivb */
> > +#define PIPECONF_GAMMA_MODE_SPLIT (3 << 24) /* ivb */
> > +#define PIPECONF_GAMMA_MODE(x) ((x)<<24) /* pass in GAMMA_MODE_MODE_* */
> > +#define PIPECONF_GAMMA_MODE_SHIFT 24
> > #define PIPECONF_INTERLACE_MASK (7 << 21)
> > #define PIPECONF_INTERLACE_MASK_HSW (3 << 21)
> > /* Note that pre-gen3 does not support interlaced display directly. Panel
> > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > index 2a371eed8061..e7dd07490c68 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -351,6 +351,32 @@ static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
> > i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
> > }
> >
> > +static void i9xx_color_commit(const struct intel_crtc_state *crtc_state)
> > +{
> > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > + enum pipe pipe = crtc->pipe;
> > + u32 val;
> > +
> > + val = I915_READ(PIPECONF(pipe));
> > + val &= ~PIPECONF_GAMMA_MODE_MASK_I9XX;
> > + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > + I915_WRITE(PIPECONF(pipe), val);
> > +}
> > +
> > +static void ilk_color_commit(const struct intel_crtc_state *crtc_state)
> > +{
> > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > + enum pipe pipe = crtc->pipe;
> > + u32 val;
> > +
> > + val = I915_READ(PIPECONF(pipe));
> > + val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
> > + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > + I915_WRITE(PIPECONF(pipe), val);
> > +}
> > +
> > static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
> > {
> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > @@ -585,8 +611,7 @@ void intel_color_commit(const struct intel_crtc_state *crtc_state)
> > {
> > struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> >
> > - if (dev_priv->display.color_commit)
> > - dev_priv->display.color_commit(crtc_state);
> > + dev_priv->display.color_commit(crtc_state);
> > }
> >
> > static int check_lut_size(const struct drm_property_blob *lut, int expected)
> > @@ -649,20 +674,25 @@ void intel_color_init(struct intel_crtc *crtc)
> >
> > drm_mode_crtc_set_gamma_size(&crtc->base, 256);
> >
> > - if (IS_CHERRYVIEW(dev_priv)) {
> > - dev_priv->display.load_luts = cherryview_load_luts;
> > - } else if (IS_HASWELL(dev_priv)) {
> > - dev_priv->display.load_luts = i9xx_load_luts;
> > - dev_priv->display.color_commit = hsw_color_commit;
> > - } else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
> > - IS_BROXTON(dev_priv)) {
> > - dev_priv->display.load_luts = broadwell_load_luts;
> > - dev_priv->display.color_commit = hsw_color_commit;
> > - } else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > - dev_priv->display.load_luts = glk_load_luts;
> > - dev_priv->display.color_commit = hsw_color_commit;
> > + if (HAS_GMCH_DISPLAY(dev_priv)) {
> > + if (IS_CHERRYVIEW(dev_priv))
> > + dev_priv->display.load_luts = cherryview_load_luts;
> > + else
> > + dev_priv->display.load_luts = i9xx_load_luts;
> > +
> > + dev_priv->display.color_commit = i9xx_color_commit;
> > } else {
> > - dev_priv->display.load_luts = i9xx_load_luts;
> > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > + dev_priv->display.load_luts = glk_load_luts;
> > + else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> > + dev_priv->display.load_luts = broadwell_load_luts;
> > + else
> > + dev_priv->display.load_luts = i9xx_load_luts;
> > +
> > + if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
> > + dev_priv->display.color_commit = hsw_color_commit;
> > + else
> > + dev_priv->display.color_commit = ilk_color_commit;
> > }
> >
> > /* Enable color management support when we have degamma & gamma LUTs. */
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ad5d39d81d6e..8e93459f312f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3450,7 +3450,7 @@ static void i9xx_disable_plane(struct intel_plane *plane,
> > *
> > * On pre-g4x there is no way to gamma correct the
> > * pipe bottom color but we'll keep on doing this
> > - * anyway.
> > + * anyway so that the crtc state readout works correctly.
> > */
> > dspcntr = i9xx_plane_ctl_crtc(crtc_state);
> >
> > @@ -7674,6 +7674,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
> > crtc_state->limited_color_range)
> > pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
> >
> > + pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > +
> > I915_WRITE(PIPECONF(crtc->pipe), pipeconf);
> > POSTING_READ(PIPECONF(crtc->pipe));
> > }
> > @@ -8126,6 +8128,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> > (tmp & PIPECONF_COLOR_RANGE_SELECT))
> > pipe_config->limited_color_range = true;
> >
> > + pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_I9XX) >>
> > + PIPECONF_GAMMA_MODE_SHIFT;
> > +
> > if (INTEL_GEN(dev_priv) < 4)
> > pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
> >
> > @@ -8665,6 +8670,8 @@ static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state)
> > if (crtc_state->limited_color_range)
> > val |= PIPECONF_COLOR_RANGE_SELECT;
> >
> > + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > +
> > I915_WRITE(PIPECONF(pipe), val);
> > POSTING_READ(PIPECONF(pipe));
> > }
> > @@ -9199,6 +9206,9 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> > if (tmp & PIPECONF_COLOR_RANGE_SELECT)
> > pipe_config->limited_color_range = true;
> >
> > + pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >>
> > + PIPECONF_GAMMA_MODE_SHIFT;
> > +
> > if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
> > struct intel_shared_dpll *pll;
> > enum intel_dpll_id pll_id;
> > @@ -12069,6 +12079,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
> >
> > PIPE_CONF_CHECK_BOOL(double_wide);
> >
> > + PIPE_CONF_CHECK_X(gamma_mode);
>
> Hmm... We should only check this if adjust is unset, or we will start skipping fastset on a lot of platforms.
There's no sane macros for this sort of stuff AFAICS. Maybe someone
should add those first? Heck, I don't even know what 'adjust'
really means in this context.
Also on a related note, this fastboot thing has made the logs rather
confusing. It's not at all obvious that all the state mismatch prints
are harmless (especially when the function is called something_err()).
Some people (myself included) were already lead astray by these
apparent "errors".
>
> With that fixed:
>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> ~Maarten
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 13/13] drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable()
2019-02-07 15:58 ` Maarten Lankhorst
@ 2019-02-07 16:29 ` Ville Syrjälä
0 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjälä @ 2019-02-07 16:29 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Thu, Feb 07, 2019 at 04:58:22PM +0100, Maarten Lankhorst wrote:
> Op 05-02-2019 om 17:08 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > On g4x+ we depend on the primary plane DSPCNTR gamma/csc enable
> > bits for the pipe bottom color. To guarantee that those are
> > correct already when enabling the crtc let's do an explicit
> > ->disable_plane() call before enabling the pipe.
> >
> > On skl+ this will be handled by the explicit PIPE_BOTTOM_COLOR
> > register which is already part of the normal color commit we
> > do durign crtc enable.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_color.c | 4 ++++
> > drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
> > 2 files changed, 21 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > index 5c7789e9fed5..79c3adaefadc 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -663,6 +663,10 @@ intel_color_add_affected_planes(struct intel_crtc_state *new_crtc_state)
> > intel_atomic_get_old_crtc_state(state, crtc);
> > struct intel_plane *plane;
> >
> > + if (!new_crtc_state->base.active ||
> > + drm_atomic_crtc_needs_modeset(&new_crtc_state->base))
> > + return 0;
>
> Why the needs_modeset skip? We will have already added all planes anyway, but still.
No point in doing pointless work.
>
>
> Oh well, assuming it's for a good reason
>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> > if (new_crtc_state->gamma_enable == old_crtc_state->gamma_enable &&
> > new_crtc_state->csc_enable == old_crtc_state->csc_enable)
> > return 0;
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index c802055d23a6..f36ac23f7195 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5715,6 +5715,14 @@ static void intel_encoders_update_pipe(struct drm_crtc *crtc,
> > }
> > }
> >
> > +static void intel_disable_primary_plane(const struct intel_crtc_state *crtc_state)
> > +{
> > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > + struct intel_plane *plane = to_intel_plane(crtc->base.primary);
> > +
> > + plane->disable_plane(plane, crtc_state);
> > +}
> > +
> > static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> > struct drm_atomic_state *old_state)
> > {
> > @@ -5780,6 +5788,8 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> > */
> > intel_color_load_luts(pipe_config);
> > intel_color_commit(pipe_config);
> > + /* update DSPCNTR to configure gamma for pipe bottom color */
> > + intel_disable_primary_plane(pipe_config);
> >
> > if (dev_priv->display.initial_watermarks != NULL)
> > dev_priv->display.initial_watermarks(old_intel_state, pipe_config);
> > @@ -5909,6 +5919,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> > */
> > intel_color_load_luts(pipe_config);
> > intel_color_commit(pipe_config);
> > + /* update DSPCNTR to configure gamma/csc for pipe bottom color */
> > + if (INTEL_GEN(dev_priv) < 9)
> > + intel_disable_primary_plane(pipe_config);
> >
> > /*
> > * Display WA #1153: enable hardware to bypass the alpha math
> > @@ -6274,6 +6287,8 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
> >
> > intel_color_load_luts(pipe_config);
> > intel_color_commit(pipe_config);
> > + /* update DSPCNTR to configure gamma for pipe bottom color */
> > + intel_disable_primary_plane(pipe_config);
> >
> > dev_priv->display.initial_watermarks(old_intel_state,
> > pipe_config);
> > @@ -6331,6 +6346,8 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
> >
> > intel_color_load_luts(pipe_config);
> > intel_color_commit(pipe_config);
> > + /* update DSPCNTR to configure gamma for pipe bottom color */
> > + intel_disable_primary_plane(pipe_config);
> >
> > if (dev_priv->display.initial_watermarks != NULL)
> > dev_priv->display.initial_watermarks(old_intel_state,
>
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms
2019-02-07 16:27 ` Ville Syrjälä
@ 2019-02-07 17:27 ` Ville Syrjälä
2019-02-08 8:51 ` Maarten Lankhorst
1 sibling, 0 replies; 26+ messages in thread
From: Ville Syrjälä @ 2019-02-07 17:27 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Thu, Feb 07, 2019 at 06:27:19PM +0200, Ville Syrjälä wrote:
> On Thu, Feb 07, 2019 at 04:49:47PM +0100, Maarten Lankhorst wrote:
> > Op 05-02-2019 om 17:08 schreef Ville Syrjala:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > On pre-HSW gamma mode is configured via PIPECONF. The bits are
> > > the same except shifted up, so we can reuse just store them in
> > > crtc_state->gamma_mode in the HSW+ way, allowing us to share
> > > some code later.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 10 ++++-
> > > drivers/gpu/drm/i915/intel_color.c | 60 +++++++++++++++++++++-------
> > > drivers/gpu/drm/i915/intel_display.c | 14 ++++++-
> > > 3 files changed, 66 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index ede54fdc1676..41111a17e519 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -5590,9 +5590,15 @@ enum {
> > > #define PIPECONF_SINGLE_WIDE 0
> > > #define PIPECONF_PIPE_UNLOCKED 0
> > > #define PIPECONF_PIPE_LOCKED (1 << 25)
> > > -#define PIPECONF_PALETTE 0
> > > -#define PIPECONF_GAMMA (1 << 24)
> > > #define PIPECONF_FORCE_BORDER (1 << 25)
> > > +#define PIPECONF_GAMMA_MODE_MASK_I9XX (1 << 24) /* gmch */
> > > +#define PIPECONF_GAMMA_MODE_MASK_ILK (3 << 24) /* ilk-ivb */
> > > +#define PIPECONF_GAMMA_MODE_8BIT (0 << 24) /* gmch,ilk-ivb */
> > > +#define PIPECONF_GAMMA_MODE_10BIT (1 << 24) /* gmch,ilk-ivb */
> > > +#define PIPECONF_GAMMA_MODE_12BIT (2 << 24) /* ilk-ivb */
> > > +#define PIPECONF_GAMMA_MODE_SPLIT (3 << 24) /* ivb */
> > > +#define PIPECONF_GAMMA_MODE(x) ((x)<<24) /* pass in GAMMA_MODE_MODE_* */
> > > +#define PIPECONF_GAMMA_MODE_SHIFT 24
> > > #define PIPECONF_INTERLACE_MASK (7 << 21)
> > > #define PIPECONF_INTERLACE_MASK_HSW (3 << 21)
> > > /* Note that pre-gen3 does not support interlaced display directly. Panel
> > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > > index 2a371eed8061..e7dd07490c68 100644
> > > --- a/drivers/gpu/drm/i915/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/intel_color.c
> > > @@ -351,6 +351,32 @@ static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
> > > i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
> > > }
> > >
> > > +static void i9xx_color_commit(const struct intel_crtc_state *crtc_state)
> > > +{
> > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > + enum pipe pipe = crtc->pipe;
> > > + u32 val;
> > > +
> > > + val = I915_READ(PIPECONF(pipe));
> > > + val &= ~PIPECONF_GAMMA_MODE_MASK_I9XX;
> > > + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > > + I915_WRITE(PIPECONF(pipe), val);
> > > +}
> > > +
> > > +static void ilk_color_commit(const struct intel_crtc_state *crtc_state)
> > > +{
> > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > + enum pipe pipe = crtc->pipe;
> > > + u32 val;
> > > +
> > > + val = I915_READ(PIPECONF(pipe));
> > > + val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
> > > + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > > + I915_WRITE(PIPECONF(pipe), val);
> > > +}
> > > +
> > > static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
> > > {
> > > struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > > @@ -585,8 +611,7 @@ void intel_color_commit(const struct intel_crtc_state *crtc_state)
> > > {
> > > struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > >
> > > - if (dev_priv->display.color_commit)
> > > - dev_priv->display.color_commit(crtc_state);
> > > + dev_priv->display.color_commit(crtc_state);
> > > }
> > >
> > > static int check_lut_size(const struct drm_property_blob *lut, int expected)
> > > @@ -649,20 +674,25 @@ void intel_color_init(struct intel_crtc *crtc)
> > >
> > > drm_mode_crtc_set_gamma_size(&crtc->base, 256);
> > >
> > > - if (IS_CHERRYVIEW(dev_priv)) {
> > > - dev_priv->display.load_luts = cherryview_load_luts;
> > > - } else if (IS_HASWELL(dev_priv)) {
> > > - dev_priv->display.load_luts = i9xx_load_luts;
> > > - dev_priv->display.color_commit = hsw_color_commit;
> > > - } else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
> > > - IS_BROXTON(dev_priv)) {
> > > - dev_priv->display.load_luts = broadwell_load_luts;
> > > - dev_priv->display.color_commit = hsw_color_commit;
> > > - } else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > > - dev_priv->display.load_luts = glk_load_luts;
> > > - dev_priv->display.color_commit = hsw_color_commit;
> > > + if (HAS_GMCH_DISPLAY(dev_priv)) {
> > > + if (IS_CHERRYVIEW(dev_priv))
> > > + dev_priv->display.load_luts = cherryview_load_luts;
> > > + else
> > > + dev_priv->display.load_luts = i9xx_load_luts;
> > > +
> > > + dev_priv->display.color_commit = i9xx_color_commit;
> > > } else {
> > > - dev_priv->display.load_luts = i9xx_load_luts;
> > > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > > + dev_priv->display.load_luts = glk_load_luts;
> > > + else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> > > + dev_priv->display.load_luts = broadwell_load_luts;
> > > + else
> > > + dev_priv->display.load_luts = i9xx_load_luts;
> > > +
> > > + if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
> > > + dev_priv->display.color_commit = hsw_color_commit;
> > > + else
> > > + dev_priv->display.color_commit = ilk_color_commit;
> > > }
> > >
> > > /* Enable color management support when we have degamma & gamma LUTs. */
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index ad5d39d81d6e..8e93459f312f 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3450,7 +3450,7 @@ static void i9xx_disable_plane(struct intel_plane *plane,
> > > *
> > > * On pre-g4x there is no way to gamma correct the
> > > * pipe bottom color but we'll keep on doing this
> > > - * anyway.
> > > + * anyway so that the crtc state readout works correctly.
> > > */
> > > dspcntr = i9xx_plane_ctl_crtc(crtc_state);
> > >
> > > @@ -7674,6 +7674,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
> > > crtc_state->limited_color_range)
> > > pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
> > >
> > > + pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > > +
> > > I915_WRITE(PIPECONF(crtc->pipe), pipeconf);
> > > POSTING_READ(PIPECONF(crtc->pipe));
> > > }
> > > @@ -8126,6 +8128,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> > > (tmp & PIPECONF_COLOR_RANGE_SELECT))
> > > pipe_config->limited_color_range = true;
> > >
> > > + pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_I9XX) >>
> > > + PIPECONF_GAMMA_MODE_SHIFT;
> > > +
> > > if (INTEL_GEN(dev_priv) < 4)
> > > pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
> > >
> > > @@ -8665,6 +8670,8 @@ static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state)
> > > if (crtc_state->limited_color_range)
> > > val |= PIPECONF_COLOR_RANGE_SELECT;
> > >
> > > + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
> > > +
> > > I915_WRITE(PIPECONF(pipe), val);
> > > POSTING_READ(PIPECONF(pipe));
> > > }
> > > @@ -9199,6 +9206,9 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> > > if (tmp & PIPECONF_COLOR_RANGE_SELECT)
> > > pipe_config->limited_color_range = true;
> > >
> > > + pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >>
> > > + PIPECONF_GAMMA_MODE_SHIFT;
> > > +
> > > if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
> > > struct intel_shared_dpll *pll;
> > > enum intel_dpll_id pll_id;
> > > @@ -12069,6 +12079,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
> > >
> > > PIPE_CONF_CHECK_BOOL(double_wide);
> > >
> > > + PIPE_CONF_CHECK_X(gamma_mode);
> >
> > Hmm... We should only check this if adjust is unset, or we will start skipping fastset on a lot of platforms.
>
> There's no sane macros for this sort of stuff AFAICS. Maybe someone
> should add those first? Heck, I don't even know what 'adjust'
> really means in this context.
I guess it means "ignore the mismatch for select pieces of state".
Hmm. Looks like we've just added a bunch of things to the
'if (!adjust)' block. I guess I can move these things there.
>
> Also on a related note, this fastboot thing has made the logs rather
> confusing. It's not at all obvious that all the state mismatch prints
> are harmless (especially when the function is called something_err()).
> Some people (myself included) were already lead astray by these
> apparent "errors".
>
> >
> > With that fixed:
> >
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >
> > ~Maarten
>
> --
> Ville Syrjälä
> Intel
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 06/13] drm/i915: Move LUT programming to happen after vblank waits
2019-02-07 15:46 ` Maarten Lankhorst
@ 2019-02-07 19:24 ` Ville Syrjälä
0 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjälä @ 2019-02-07 19:24 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Thu, Feb 07, 2019 at 04:46:54PM +0100, Maarten Lankhorst wrote:
> Hey,
>
> Op 05-02-2019 om 17:08 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The LUTs are single buffered so we should program them after
> > the double buffered pipe updates have been latched by the
> > hardware.
> >
> > We'll also fix up the IPS vs. split gamma w/a to do the IPS
> > disable like everyone else. Note that this is currently dead
> > code as we don't use the split gamma mode on HSW, but that
> > will be fixed up shortly.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Hmm I guess there is no good way to do this, either before or after. :/
>
> The idea was that we do it before was to make sure that the frame after the flip would be perfect. I guess
>
> we can't be sure, and after is as good as before..
After is much better than before since we have actually have a
chance that we're inside the correct vblank interval. Doing it before
guarantees that we're either in the middle of the previous frame or in
the vblank before the previous frame (which would make the entire
previous frame use the wrong LUT values).
Anyways, it should be mostly OK for single pipe updates since
we'll just wait for the vblank of that one pipe, and thus should still
be in the vblank for the LUT reprogramming. For multi-pipe we'd need to
get significantly more lucky.
>
> For patch 1-6:
>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms
2019-02-07 16:27 ` Ville Syrjälä
2019-02-07 17:27 ` Ville Syrjälä
@ 2019-02-08 8:51 ` Maarten Lankhorst
1 sibling, 0 replies; 26+ messages in thread
From: Maarten Lankhorst @ 2019-02-08 8:51 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Op 07-02-2019 om 17:27 schreef Ville Syrjälä:
> On Thu, Feb 07, 2019 at 04:49:47PM +0100, Maarten Lankhorst wrote:
>> Op 05-02-2019 om 17:08 schreef Ville Syrjala:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> On pre-HSW gamma mode is configured via PIPECONF. The bits are
>>> the same except shifted up, so we can reuse just store them in
>>> crtc_state->gamma_mode in the HSW+ way, allowing us to share
>>> some code later.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_reg.h | 10 ++++-
>>> drivers/gpu/drm/i915/intel_color.c | 60 +++++++++++++++++++++-------
>>> drivers/gpu/drm/i915/intel_display.c | 14 ++++++-
>>> 3 files changed, 66 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index ede54fdc1676..41111a17e519 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -5590,9 +5590,15 @@ enum {
>>> #define PIPECONF_SINGLE_WIDE 0
>>> #define PIPECONF_PIPE_UNLOCKED 0
>>> #define PIPECONF_PIPE_LOCKED (1 << 25)
>>> -#define PIPECONF_PALETTE 0
>>> -#define PIPECONF_GAMMA (1 << 24)
>>> #define PIPECONF_FORCE_BORDER (1 << 25)
>>> +#define PIPECONF_GAMMA_MODE_MASK_I9XX (1 << 24) /* gmch */
>>> +#define PIPECONF_GAMMA_MODE_MASK_ILK (3 << 24) /* ilk-ivb */
>>> +#define PIPECONF_GAMMA_MODE_8BIT (0 << 24) /* gmch,ilk-ivb */
>>> +#define PIPECONF_GAMMA_MODE_10BIT (1 << 24) /* gmch,ilk-ivb */
>>> +#define PIPECONF_GAMMA_MODE_12BIT (2 << 24) /* ilk-ivb */
>>> +#define PIPECONF_GAMMA_MODE_SPLIT (3 << 24) /* ivb */
>>> +#define PIPECONF_GAMMA_MODE(x) ((x)<<24) /* pass in GAMMA_MODE_MODE_* */
>>> +#define PIPECONF_GAMMA_MODE_SHIFT 24
>>> #define PIPECONF_INTERLACE_MASK (7 << 21)
>>> #define PIPECONF_INTERLACE_MASK_HSW (3 << 21)
>>> /* Note that pre-gen3 does not support interlaced display directly. Panel
>>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>>> index 2a371eed8061..e7dd07490c68 100644
>>> --- a/drivers/gpu/drm/i915/intel_color.c
>>> +++ b/drivers/gpu/drm/i915/intel_color.c
>>> @@ -351,6 +351,32 @@ static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
>>> i9xx_load_luts_internal(crtc_state, crtc_state->base.gamma_lut);
>>> }
>>>
>>> +static void i9xx_color_commit(const struct intel_crtc_state *crtc_state)
>>> +{
>>> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>>> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>> + enum pipe pipe = crtc->pipe;
>>> + u32 val;
>>> +
>>> + val = I915_READ(PIPECONF(pipe));
>>> + val &= ~PIPECONF_GAMMA_MODE_MASK_I9XX;
>>> + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
>>> + I915_WRITE(PIPECONF(pipe), val);
>>> +}
>>> +
>>> +static void ilk_color_commit(const struct intel_crtc_state *crtc_state)
>>> +{
>>> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>>> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>> + enum pipe pipe = crtc->pipe;
>>> + u32 val;
>>> +
>>> + val = I915_READ(PIPECONF(pipe));
>>> + val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
>>> + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
>>> + I915_WRITE(PIPECONF(pipe), val);
>>> +}
>>> +
>>> static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
>>> {
>>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>>> @@ -585,8 +611,7 @@ void intel_color_commit(const struct intel_crtc_state *crtc_state)
>>> {
>>> struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>>>
>>> - if (dev_priv->display.color_commit)
>>> - dev_priv->display.color_commit(crtc_state);
>>> + dev_priv->display.color_commit(crtc_state);
>>> }
>>>
>>> static int check_lut_size(const struct drm_property_blob *lut, int expected)
>>> @@ -649,20 +674,25 @@ void intel_color_init(struct intel_crtc *crtc)
>>>
>>> drm_mode_crtc_set_gamma_size(&crtc->base, 256);
>>>
>>> - if (IS_CHERRYVIEW(dev_priv)) {
>>> - dev_priv->display.load_luts = cherryview_load_luts;
>>> - } else if (IS_HASWELL(dev_priv)) {
>>> - dev_priv->display.load_luts = i9xx_load_luts;
>>> - dev_priv->display.color_commit = hsw_color_commit;
>>> - } else if (IS_BROADWELL(dev_priv) || IS_GEN9_BC(dev_priv) ||
>>> - IS_BROXTON(dev_priv)) {
>>> - dev_priv->display.load_luts = broadwell_load_luts;
>>> - dev_priv->display.color_commit = hsw_color_commit;
>>> - } else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
>>> - dev_priv->display.load_luts = glk_load_luts;
>>> - dev_priv->display.color_commit = hsw_color_commit;
>>> + if (HAS_GMCH_DISPLAY(dev_priv)) {
>>> + if (IS_CHERRYVIEW(dev_priv))
>>> + dev_priv->display.load_luts = cherryview_load_luts;
>>> + else
>>> + dev_priv->display.load_luts = i9xx_load_luts;
>>> +
>>> + dev_priv->display.color_commit = i9xx_color_commit;
>>> } else {
>>> - dev_priv->display.load_luts = i9xx_load_luts;
>>> + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>>> + dev_priv->display.load_luts = glk_load_luts;
>>> + else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
>>> + dev_priv->display.load_luts = broadwell_load_luts;
>>> + else
>>> + dev_priv->display.load_luts = i9xx_load_luts;
>>> +
>>> + if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
>>> + dev_priv->display.color_commit = hsw_color_commit;
>>> + else
>>> + dev_priv->display.color_commit = ilk_color_commit;
>>> }
>>>
>>> /* Enable color management support when we have degamma & gamma LUTs. */
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index ad5d39d81d6e..8e93459f312f 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -3450,7 +3450,7 @@ static void i9xx_disable_plane(struct intel_plane *plane,
>>> *
>>> * On pre-g4x there is no way to gamma correct the
>>> * pipe bottom color but we'll keep on doing this
>>> - * anyway.
>>> + * anyway so that the crtc state readout works correctly.
>>> */
>>> dspcntr = i9xx_plane_ctl_crtc(crtc_state);
>>>
>>> @@ -7674,6 +7674,8 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
>>> crtc_state->limited_color_range)
>>> pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
>>>
>>> + pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
>>> +
>>> I915_WRITE(PIPECONF(crtc->pipe), pipeconf);
>>> POSTING_READ(PIPECONF(crtc->pipe));
>>> }
>>> @@ -8126,6 +8128,9 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>>> (tmp & PIPECONF_COLOR_RANGE_SELECT))
>>> pipe_config->limited_color_range = true;
>>>
>>> + pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_I9XX) >>
>>> + PIPECONF_GAMMA_MODE_SHIFT;
>>> +
>>> if (INTEL_GEN(dev_priv) < 4)
>>> pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
>>>
>>> @@ -8665,6 +8670,8 @@ static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state)
>>> if (crtc_state->limited_color_range)
>>> val |= PIPECONF_COLOR_RANGE_SELECT;
>>>
>>> + val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
>>> +
>>> I915_WRITE(PIPECONF(pipe), val);
>>> POSTING_READ(PIPECONF(pipe));
>>> }
>>> @@ -9199,6 +9206,9 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>>> if (tmp & PIPECONF_COLOR_RANGE_SELECT)
>>> pipe_config->limited_color_range = true;
>>>
>>> + pipe_config->gamma_mode = (tmp & PIPECONF_GAMMA_MODE_MASK_ILK) >>
>>> + PIPECONF_GAMMA_MODE_SHIFT;
>>> +
>>> if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
>>> struct intel_shared_dpll *pll;
>>> enum intel_dpll_id pll_id;
>>> @@ -12069,6 +12079,8 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>>>
>>> PIPE_CONF_CHECK_BOOL(double_wide);
>>>
>>> + PIPE_CONF_CHECK_X(gamma_mode);
>> Hmm... We should only check this if adjust is unset, or we will start skipping fastset on a lot of platforms.
> There's no sane macros for this sort of stuff AFAICS. Maybe someone
> should add those first? Heck, I don't even know what 'adjust'
> really means in this context.
>
> Also on a related note, this fastboot thing has made the logs rather
> confusing. It's not at all obvious that all the state mismatch prints
> are harmless (especially when the function is called something_err()).
> Some people (myself included) were already lead astray by these
> apparent "errors".
Could we change the messages then for fastset?
We already pass adjust to pipe_config_err, so we could add something like fastset cannot be done to the message.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2019-02-08 8:51 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-05 16:08 [PATCH v2 00/13] Enable/disable gamma/csc dynamically and fix C8 Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 01/13] drm/i915: Split the gamma/csc enable bits from the plane_ctl() function Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 02/13] drm/i915: Precompute gamma_mode Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 03/13] drm/i915: Constify the state arguments to the color management stuff Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 04/13] drm/i915: Pull GAMMA_MODE write out from haswell_load_luts() Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 05/13] drm/i915: Split color mgmt based on single vs. double buffered registers Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 06/13] drm/i915: Move LUT programming to happen after vblank waits Ville Syrjala
2019-02-07 15:46 ` Maarten Lankhorst
2019-02-07 19:24 ` Ville Syrjälä
2019-02-05 16:08 ` [PATCH v2 07/13] drm/i915: Populate gamma_mode for all platforms Ville Syrjala
2019-02-07 15:49 ` Maarten Lankhorst
2019-02-07 16:27 ` Ville Syrjälä
2019-02-07 17:27 ` Ville Syrjälä
2019-02-08 8:51 ` Maarten Lankhorst
2019-02-05 16:08 ` [PATCH v2 08/13] drm/i915: Track pipe gamma enable/disable in crtc state Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 09/13] drm/i915: Track pipe csc enable " Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 10/13] drm/i915: Turn off pipe gamma when it's not needed Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 11/13] drm/i915: Turn off pipe CSC " Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 12/13] drm/i915: Disable pipe gamma when C8 pixel format is used Ville Syrjala
2019-02-05 16:08 ` [PATCH v2 13/13] drm/i915: Update DSPCNTR gamma/csc bits during crtc_enable() Ville Syrjala
2019-02-07 15:58 ` Maarten Lankhorst
2019-02-07 16:29 ` Ville Syrjälä
2019-02-05 16:57 ` ✗ Fi.CI.CHECKPATCH: warning for Enable/disable gamma/csc dynamically and fix C8 (rev2) Patchwork
2019-02-05 17:02 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-02-05 17:24 ` ✓ Fi.CI.BAT: success " Patchwork
2019-02-05 21:19 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.