From 7eb31a0bb2c1df7328692c875a6423b406b09ad1 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 5 Feb 2019 18:08:36 +0200 Subject: [PATCH] drm/i915: Split the gamma/csc enable bits from the plane_ctl() function MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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ä Link: https://patchwork.freedesktop.org/patch/msgid/20190205160848.24662-2-ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst --- 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 bf564a638dca..be5a73b6805a 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); @@ -9945,11 +9989,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); } @@ -10019,7 +10067,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); @@ -10086,27 +10136,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; @@ -10231,7 +10290,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 79f102dfe37c..9717b1250e6f 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1765,9 +1765,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.30.2