* [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
@ 2017-11-02 23:00 James Ausmus
2017-11-02 23:52 ` ✗ Fi.CI.BAT: warning for " Patchwork
` (7 more replies)
0 siblings, 8 replies; 14+ messages in thread
From: James Ausmus @ 2017-11-02 23:00 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Since GLK, some plane configuration settings have moved to the
PLANE_COLOR_CTL register. Refactor handling of the register to work like
PLANE_CTL. This also allows us to fix the set/read of the plane Alpha
Mode for GLK+.
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 12 +++++---
drivers/gpu/drm/i915/intel_display.c | 60 +++++++++++++++++++++++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 5 +++
drivers/gpu/drm/i915/intel_sprite.c | 14 +++++----
4 files changed, 76 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 68a58cce6ab1..520ff9a15222 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6263,7 +6263,7 @@ enum {
#define _PLANE_CTL_2_A 0x70280
#define _PLANE_CTL_3_A 0x70380
#define PLANE_CTL_ENABLE (1 << 31)
-#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30)
+#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-GLK */
#define PLANE_CTL_FORMAT_MASK (0xf << 24)
#define PLANE_CTL_FORMAT_YUV422 ( 0 << 24)
#define PLANE_CTL_FORMAT_NV12 ( 1 << 24)
@@ -6273,7 +6273,7 @@ enum {
#define PLANE_CTL_FORMAT_AYUV ( 8 << 24)
#define PLANE_CTL_FORMAT_INDEXED ( 12 << 24)
#define PLANE_CTL_FORMAT_RGB_565 ( 14 << 24)
-#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23)
+#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23) /* Pre-GLK */
#define PLANE_CTL_KEY_ENABLE_MASK (0x3 << 21)
#define PLANE_CTL_KEY_ENABLE_SOURCE ( 1 << 21)
#define PLANE_CTL_KEY_ENABLE_DESTINATION ( 2 << 21)
@@ -6286,13 +6286,13 @@ enum {
#define PLANE_CTL_YUV422_VYUY ( 3 << 16)
#define PLANE_CTL_DECOMPRESSION_ENABLE (1 << 15)
#define PLANE_CTL_TRICKLE_FEED_DISABLE (1 << 14)
-#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13)
+#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13) /* Pre-GLK */
#define PLANE_CTL_TILED_MASK (0x7 << 10)
#define PLANE_CTL_TILED_LINEAR ( 0 << 10)
#define PLANE_CTL_TILED_X ( 1 << 10)
#define PLANE_CTL_TILED_Y ( 4 << 10)
#define PLANE_CTL_TILED_YF ( 5 << 10)
-#define PLANE_CTL_ALPHA_MASK (0x3 << 4)
+#define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
#define PLANE_CTL_ALPHA_DISABLE ( 0 << 4)
#define PLANE_CTL_ALPHA_SW_PREMULTIPLY ( 2 << 4)
#define PLANE_CTL_ALPHA_HW_PREMULTIPLY ( 3 << 4)
@@ -6332,6 +6332,10 @@ enum {
#define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30)
#define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23)
#define PLANE_COLOR_PLANE_GAMMA_DISABLE (1 << 13)
+#define PLANE_COLOR_ALPHA_MASK (0x3 << 4)
+#define PLANE_COLOR_ALPHA_DISABLE (0 << 4)
+#define PLANE_COLOR_ALPHA_SW_PREMULTIPLY (2 << 4)
+#define PLANE_COLOR_ALPHA_HW_PREMULTIPLY (3 << 4)
#define _PLANE_BUF_CFG_1_A 0x7027c
#define _PLANE_BUF_CFG_2_A 0x7037c
#define _PLANE_NV12_BUF_CFG_1_A 0x70278
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e2ac976844d8..0883e857dda9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3466,6 +3466,29 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
return 0;
}
+static u32 glk_plane_ctl_format(uint32_t pixel_format)
+{
+ /* GLK+ moves the alpha mask to a different register */
+ return skl_plane_ctl_format(pixel_format) & ~PLANE_CTL_ALPHA_MASK;
+}
+
+static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format)
+{
+ switch (pixel_format) {
+ /*
+ * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
+ * to be already pre-multiplied. We need to add a knob (or a different
+ * DRM_FORMAT) for user-space to configure that.
+ */
+ case DRM_FORMAT_ABGR8888:
+ return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
+ case DRM_FORMAT_ARGB8888:
+ return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
+ default:
+ return PLANE_COLOR_ALPHA_DISABLE;
+ }
+}
+
static u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
{
switch (fb_modifier) {
@@ -3522,14 +3545,16 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
plane_ctl = PLANE_CTL_ENABLE;
- if (!IS_GEMINILAKE(dev_priv) && !IS_CANNONLAKE(dev_priv)) {
+ if (!IS_GEMINILAKE(dev_priv) && !(INTEL_GEN(dev_priv) >= 10)) {
plane_ctl |=
PLANE_CTL_PIPE_GAMMA_ENABLE |
PLANE_CTL_PIPE_CSC_ENABLE |
PLANE_CTL_PLANE_GAMMA_DISABLE;
+ plane_ctl |= skl_plane_ctl_format(fb->format->format);
+ } else {
+ plane_ctl |= glk_plane_ctl_format(fb->format->format);
}
- plane_ctl |= skl_plane_ctl_format(fb->format->format);
plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
plane_ctl |= skl_plane_ctl_rotation(rotation);
@@ -3541,6 +3566,20 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
return plane_ctl;
}
+u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state *plane_state)
+{
+ const struct drm_framebuffer *fb = plane_state->base.fb;
+ u32 plane_color_ctl = 0;
+
+ 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(fb->format->format);
+
+ return plane_color_ctl;
+}
+
static int
__intel_display_resume(struct drm_device *dev,
struct drm_atomic_state *state,
@@ -8425,7 +8464,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- u32 val, base, offset, stride_mult, tiling;
+ u32 val, base, offset, stride_mult, tiling, alpha;
int pipe = crtc->pipe;
int fourcc, pixel_format;
unsigned int aligned_height;
@@ -8447,9 +8486,16 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
goto error;
pixel_format = val & PLANE_CTL_FORMAT_MASK;
+
+ if (!IS_GEMINILAKE(dev_priv) && !(INTEL_GEN(dev_priv) >= 10)) {
+ alpha = val & PLANE_CTL_ALPHA_MASK;
+ } else {
+ alpha = I915_READ(PLANE_COLOR_CTL(pipe, 0));
+ alpha &= PLANE_COLOR_ALPHA_MASK;
+ }
+
fourcc = skl_format_to_fourcc(pixel_format,
- val & PLANE_CTL_ORDER_RGBX,
- val & PLANE_CTL_ALPHA_MASK);
+ val & PLANE_CTL_ORDER_RGBX, alpha);
fb->format = drm_format_info(fourcc);
tiling = val & PLANE_CTL_TILED_MASK;
@@ -12816,6 +12862,10 @@ intel_check_primary_plane(struct intel_plane *plane,
state->ctl = i9xx_plane_ctl(crtc_state, state);
}
+ if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
+ state->color_ctl = glk_plane_color_ctl(crtc_state, state);
+ else
+ state->color_ctl = 0;
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 47d022d48718..7967b3705217 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -420,6 +420,9 @@ struct intel_plane_state {
/* plane control register */
u32 ctl;
+ /* plane color control register */
+ u32 color_ctl;
+
/*
* scaler_id
* = -1 : not using a scaler
@@ -1492,6 +1495,8 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
return i915_ggtt_offset(state->vma);
}
+u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state *plane_state);
u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state);
u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 4fcf80ca91dd..0c09657101b7 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -240,6 +240,7 @@ skl_update_plane(struct intel_plane *plane,
enum plane_id plane_id = plane->id;
enum pipe pipe = plane->pipe;
u32 plane_ctl = plane_state->ctl;
+ u32 plane_color_ctl = plane_state->color_ctl;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 surf_addr = plane_state->main.offset;
unsigned int rotation = plane_state->base.rotation;
@@ -263,13 +264,9 @@ skl_update_plane(struct intel_plane *plane,
spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
- if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+ if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
- PLANE_COLOR_PIPE_GAMMA_ENABLE |
- PLANE_COLOR_PIPE_CSC_ENABLE |
- PLANE_COLOR_PLANE_GAMMA_DISABLE);
- }
-
+ plane_color_ctl);
if (key->flags) {
I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);
@@ -978,6 +975,11 @@ intel_check_sprite_plane(struct intel_plane *plane,
state->ctl = g4x_sprite_ctl(crtc_state, state);
}
+ if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
+ state->color_ctl = glk_plane_color_ctl(crtc_state, state);
+ else
+ state->color_ctl = 0;
+
return 0;
}
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
2017-11-02 23:00 [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
@ 2017-11-02 23:52 ` Patchwork
2017-11-03 10:12 ` [PATCH] " Ville Syrjälä
` (6 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-11-02 23:52 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
URL : https://patchwork.freedesktop.org/series/33087/
State : warning
== Summary ==
Series 33087v1 drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
https://patchwork.freedesktop.org/api/1.0/series/33087/revisions/1/mbox/
Test gem_ctx_switch:
Subgroup basic-default-heavy:
pass -> INCOMPLETE (fi-glk-dsi) fdo#103359
Test kms_force_connector_basic:
Subgroup prune-stale-modes:
pass -> SKIP (fi-snb-2520m)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass -> INCOMPLETE (fi-kbl-7567u) fdo#102846
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#102846 https://bugs.freedesktop.org/show_bug.cgi?id=102846
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:449s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:380s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:526s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:277s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:507s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:500s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:507s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:496s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:432s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:267s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:585s
fi-glk-dsi total:32 pass:22 dwarn:0 dfail:0 fail:0 skip:9
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:433s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:431s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:430s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:461s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:489s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:576s
fi-kbl-7567u total:245 pass:228 dwarn:0 dfail:0 fail:0 skip:16
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:585s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:570s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:454s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:594s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:650s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:525s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:504s
fi-snb-2520m total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:568s
fi-cfl-s failed to connect after reboot
2faf7577f4edf6e233c89b3b217440bcb87b651f drm-tip: 2017y-11m-02d-15h-33m-01s UTC integration manifest
60b48de7835a drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6933/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
2017-11-02 23:00 [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
2017-11-02 23:52 ` ✗ Fi.CI.BAT: warning for " Patchwork
@ 2017-11-03 10:12 ` Ville Syrjälä
2017-11-03 17:01 ` James Ausmus
2017-11-03 16:58 ` [PATCH v2] " James Ausmus
` (5 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2017-11-03 10:12 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx, Paulo Zanoni
On Thu, Nov 02, 2017 at 04:00:49PM -0700, James Ausmus wrote:
> Since GLK, some plane configuration settings have moved to the
> PLANE_COLOR_CTL register. Refactor handling of the register to work like
> PLANE_CTL. This also allows us to fix the set/read of the plane Alpha
> Mode for GLK+.
>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 12 +++++---
> drivers/gpu/drm/i915/intel_display.c | 60 +++++++++++++++++++++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 5 +++
> drivers/gpu/drm/i915/intel_sprite.c | 14 +++++----
> 4 files changed, 76 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 68a58cce6ab1..520ff9a15222 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6263,7 +6263,7 @@ enum {
> #define _PLANE_CTL_2_A 0x70280
> #define _PLANE_CTL_3_A 0x70380
> #define PLANE_CTL_ENABLE (1 << 31)
> -#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30)
> +#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-GLK */
> #define PLANE_CTL_FORMAT_MASK (0xf << 24)
> #define PLANE_CTL_FORMAT_YUV422 ( 0 << 24)
> #define PLANE_CTL_FORMAT_NV12 ( 1 << 24)
> @@ -6273,7 +6273,7 @@ enum {
> #define PLANE_CTL_FORMAT_AYUV ( 8 << 24)
> #define PLANE_CTL_FORMAT_INDEXED ( 12 << 24)
> #define PLANE_CTL_FORMAT_RGB_565 ( 14 << 24)
> -#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23)
> +#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23) /* Pre-GLK */
> #define PLANE_CTL_KEY_ENABLE_MASK (0x3 << 21)
> #define PLANE_CTL_KEY_ENABLE_SOURCE ( 1 << 21)
> #define PLANE_CTL_KEY_ENABLE_DESTINATION ( 2 << 21)
> @@ -6286,13 +6286,13 @@ enum {
> #define PLANE_CTL_YUV422_VYUY ( 3 << 16)
> #define PLANE_CTL_DECOMPRESSION_ENABLE (1 << 15)
> #define PLANE_CTL_TRICKLE_FEED_DISABLE (1 << 14)
> -#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13)
> +#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13) /* Pre-GLK */
> #define PLANE_CTL_TILED_MASK (0x7 << 10)
> #define PLANE_CTL_TILED_LINEAR ( 0 << 10)
> #define PLANE_CTL_TILED_X ( 1 << 10)
> #define PLANE_CTL_TILED_Y ( 4 << 10)
> #define PLANE_CTL_TILED_YF ( 5 << 10)
> -#define PLANE_CTL_ALPHA_MASK (0x3 << 4)
> +#define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
> #define PLANE_CTL_ALPHA_DISABLE ( 0 << 4)
> #define PLANE_CTL_ALPHA_SW_PREMULTIPLY ( 2 << 4)
> #define PLANE_CTL_ALPHA_HW_PREMULTIPLY ( 3 << 4)
> @@ -6332,6 +6332,10 @@ enum {
> #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30)
> #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23)
> #define PLANE_COLOR_PLANE_GAMMA_DISABLE (1 << 13)
> +#define PLANE_COLOR_ALPHA_MASK (0x3 << 4)
> +#define PLANE_COLOR_ALPHA_DISABLE (0 << 4)
> +#define PLANE_COLOR_ALPHA_SW_PREMULTIPLY (2 << 4)
> +#define PLANE_COLOR_ALPHA_HW_PREMULTIPLY (3 << 4)
> #define _PLANE_BUF_CFG_1_A 0x7027c
> #define _PLANE_BUF_CFG_2_A 0x7037c
> #define _PLANE_NV12_BUF_CFG_1_A 0x70278
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e2ac976844d8..0883e857dda9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3466,6 +3466,29 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
> return 0;
> }
>
> +static u32 glk_plane_ctl_format(uint32_t pixel_format)
> +{
> + /* GLK+ moves the alpha mask to a different register */
> + return skl_plane_ctl_format(pixel_format) & ~PLANE_CTL_ALPHA_MASK;
> +}
> +
> +static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format)
> +{
> + switch (pixel_format) {
> + /*
> + * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
> + * to be already pre-multiplied. We need to add a knob (or a different
> + * DRM_FORMAT) for user-space to configure that.
> + */
Why is this comment getting added to the glk function? It's a generic
issue that affects all platforms with alpha blending.
> + case DRM_FORMAT_ABGR8888:
> + return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
> + case DRM_FORMAT_ARGB8888:
> + return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
case A:
case B:
return FOO;
> + default:
> + return PLANE_COLOR_ALPHA_DISABLE;
> + }
> +}
> +
> static u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
> {
> switch (fb_modifier) {
> @@ -3522,14 +3545,16 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
>
> plane_ctl = PLANE_CTL_ENABLE;
>
> - if (!IS_GEMINILAKE(dev_priv) && !IS_CANNONLAKE(dev_priv)) {
> + if (!IS_GEMINILAKE(dev_priv) && !(INTEL_GEN(dev_priv) >= 10)) {
Please flip this entire logic around:
if (GEN >= 10 || IS_GLK) {
...
> plane_ctl |=
> PLANE_CTL_PIPE_GAMMA_ENABLE |
> PLANE_CTL_PIPE_CSC_ENABLE |
> PLANE_CTL_PLANE_GAMMA_DISABLE;
> + plane_ctl |= skl_plane_ctl_format(fb->format->format);
> + } else {
> + plane_ctl |= glk_plane_ctl_format(fb->format->format);
> }
>
> - plane_ctl |= skl_plane_ctl_format(fb->format->format);
> plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
> plane_ctl |= skl_plane_ctl_rotation(rotation);
>
> @@ -3541,6 +3566,20 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> return plane_ctl;
> }
>
> +u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> + const struct intel_plane_state *plane_state)
> +{
> + const struct drm_framebuffer *fb = plane_state->base.fb;
> + u32 plane_color_ctl = 0;
> +
> + 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(fb->format->format);
> +
> + return plane_color_ctl;
> +}
> +
> static int
> __intel_display_resume(struct drm_device *dev,
> struct drm_atomic_state *state,
> @@ -8425,7 +8464,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - u32 val, base, offset, stride_mult, tiling;
> + u32 val, base, offset, stride_mult, tiling, alpha;
> int pipe = crtc->pipe;
> int fourcc, pixel_format;
> unsigned int aligned_height;
> @@ -8447,9 +8486,16 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> goto error;
>
> pixel_format = val & PLANE_CTL_FORMAT_MASK;
> +
> + if (!IS_GEMINILAKE(dev_priv) && !(INTEL_GEN(dev_priv) >= 10)) {
ditto
> + alpha = val & PLANE_CTL_ALPHA_MASK;
> + } else {
> + alpha = I915_READ(PLANE_COLOR_CTL(pipe, 0));
> + alpha &= PLANE_COLOR_ALPHA_MASK;
> + }
> +
> fourcc = skl_format_to_fourcc(pixel_format,
> - val & PLANE_CTL_ORDER_RGBX,
> - val & PLANE_CTL_ALPHA_MASK);
> + val & PLANE_CTL_ORDER_RGBX, alpha);
> fb->format = drm_format_info(fourcc);
>
> tiling = val & PLANE_CTL_TILED_MASK;
> @@ -12816,6 +12862,10 @@ intel_check_primary_plane(struct intel_plane *plane,
>
> state->ctl = i9xx_plane_ctl(crtc_state, state);
> }
> + if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
I prefer the order to go from new to old. Makes for a more logical
read if we add another branch to the if statement with a more recent
platforms eg.
if (LATEST)
...
else if (SLIGHTLY_OLDER || EVEN_OLDER)
...
> + state->color_ctl = glk_plane_color_ctl(crtc_state, state);
> + else
> + state->color_ctl = 0;
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 47d022d48718..7967b3705217 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -420,6 +420,9 @@ struct intel_plane_state {
> /* plane control register */
> u32 ctl;
>
> + /* plane color control register */
> + u32 color_ctl;
> +
> /*
> * scaler_id
> * = -1 : not using a scaler
> @@ -1492,6 +1495,8 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
> return i915_ggtt_offset(state->vma);
> }
>
> +u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> + const struct intel_plane_state *plane_state);
> u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> const struct intel_plane_state *plane_state);
> u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 4fcf80ca91dd..0c09657101b7 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -240,6 +240,7 @@ skl_update_plane(struct intel_plane *plane,
> enum plane_id plane_id = plane->id;
> enum pipe pipe = plane->pipe;
> u32 plane_ctl = plane_state->ctl;
> + u32 plane_color_ctl = plane_state->color_ctl;
> const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> u32 surf_addr = plane_state->main.offset;
> unsigned int rotation = plane_state->base.rotation;
> @@ -263,13 +264,9 @@ skl_update_plane(struct intel_plane *plane,
>
> spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>
> - if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> + if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
ditto
> I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
> - PLANE_COLOR_PIPE_GAMMA_ENABLE |
> - PLANE_COLOR_PIPE_CSC_ENABLE |
> - PLANE_COLOR_PLANE_GAMMA_DISABLE);
> - }
> -
> + plane_color_ctl);
> if (key->flags) {
> I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
> I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);
> @@ -978,6 +975,11 @@ intel_check_sprite_plane(struct intel_plane *plane,
> state->ctl = g4x_sprite_ctl(crtc_state, state);
> }
>
> + if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
ditto
> + state->color_ctl = glk_plane_color_ctl(crtc_state, state);
> + else
> + state->color_ctl = 0;
> +
> return 0;
> }
>
> --
> 2.14.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
2017-11-02 23:00 [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
2017-11-02 23:52 ` ✗ Fi.CI.BAT: warning for " Patchwork
2017-11-03 10:12 ` [PATCH] " Ville Syrjälä
@ 2017-11-03 16:58 ` James Ausmus
2017-11-03 17:31 ` Ville Syrjälä
2017-11-03 17:23 ` ✓ Fi.CI.BAT: success for drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev2) Patchwork
` (4 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: James Ausmus @ 2017-11-03 16:58 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Since GLK, some plane configuration settings have moved to the
PLANE_COLOR_CTL register. Refactor handling of the register to work like
PLANE_CTL. This also allows us to fix the set/read of the plane Alpha
Mode for GLK+.
v2: Adjust ordering of platform checks to be newest->oldest, drop
redundant comment about alpha blending. (Ville)
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 12 +++++---
drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++++++++++++----
drivers/gpu/drm/i915/intel_drv.h | 5 ++++
drivers/gpu/drm/i915/intel_sprite.c | 14 +++++----
4 files changed, 71 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f0f8f6059652..ecd6b236e005 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6263,7 +6263,7 @@ enum {
#define _PLANE_CTL_2_A 0x70280
#define _PLANE_CTL_3_A 0x70380
#define PLANE_CTL_ENABLE (1 << 31)
-#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30)
+#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-GLK */
#define PLANE_CTL_FORMAT_MASK (0xf << 24)
#define PLANE_CTL_FORMAT_YUV422 ( 0 << 24)
#define PLANE_CTL_FORMAT_NV12 ( 1 << 24)
@@ -6273,7 +6273,7 @@ enum {
#define PLANE_CTL_FORMAT_AYUV ( 8 << 24)
#define PLANE_CTL_FORMAT_INDEXED ( 12 << 24)
#define PLANE_CTL_FORMAT_RGB_565 ( 14 << 24)
-#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23)
+#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23) /* Pre-GLK */
#define PLANE_CTL_KEY_ENABLE_MASK (0x3 << 21)
#define PLANE_CTL_KEY_ENABLE_SOURCE ( 1 << 21)
#define PLANE_CTL_KEY_ENABLE_DESTINATION ( 2 << 21)
@@ -6286,13 +6286,13 @@ enum {
#define PLANE_CTL_YUV422_VYUY ( 3 << 16)
#define PLANE_CTL_DECOMPRESSION_ENABLE (1 << 15)
#define PLANE_CTL_TRICKLE_FEED_DISABLE (1 << 14)
-#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13)
+#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13) /* Pre-GLK */
#define PLANE_CTL_TILED_MASK (0x7 << 10)
#define PLANE_CTL_TILED_LINEAR ( 0 << 10)
#define PLANE_CTL_TILED_X ( 1 << 10)
#define PLANE_CTL_TILED_Y ( 4 << 10)
#define PLANE_CTL_TILED_YF ( 5 << 10)
-#define PLANE_CTL_ALPHA_MASK (0x3 << 4)
+#define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
#define PLANE_CTL_ALPHA_DISABLE ( 0 << 4)
#define PLANE_CTL_ALPHA_SW_PREMULTIPLY ( 2 << 4)
#define PLANE_CTL_ALPHA_HW_PREMULTIPLY ( 3 << 4)
@@ -6332,6 +6332,10 @@ enum {
#define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30)
#define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23)
#define PLANE_COLOR_PLANE_GAMMA_DISABLE (1 << 13)
+#define PLANE_COLOR_ALPHA_MASK (0x3 << 4)
+#define PLANE_COLOR_ALPHA_DISABLE (0 << 4)
+#define PLANE_COLOR_ALPHA_SW_PREMULTIPLY (2 << 4)
+#define PLANE_COLOR_ALPHA_HW_PREMULTIPLY (3 << 4)
#define _PLANE_BUF_CFG_1_A 0x7027c
#define _PLANE_BUF_CFG_2_A 0x7037c
#define _PLANE_NV12_BUF_CFG_1_A 0x70278
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..39453276dad1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3466,6 +3466,23 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
return 0;
}
+static u32 glk_plane_ctl_format(uint32_t pixel_format)
+{
+ /* GLK+ moves the alpha mask to a different register */
+ return skl_plane_ctl_format(pixel_format) & ~PLANE_CTL_ALPHA_MASK;
+}
+
+static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format)
+{
+ switch (pixel_format) {
+ case DRM_FORMAT_ABGR8888:
+ case DRM_FORMAT_ARGB8888:
+ return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
+ default:
+ return PLANE_COLOR_ALPHA_DISABLE;
+ }
+}
+
static u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
{
switch (fb_modifier) {
@@ -3522,14 +3539,16 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
plane_ctl = PLANE_CTL_ENABLE;
- if (!IS_GEMINILAKE(dev_priv) && !IS_CANNONLAKE(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+ plane_ctl |= glk_plane_ctl_format(fb->format->format);
+ } else {
plane_ctl |=
PLANE_CTL_PIPE_GAMMA_ENABLE |
PLANE_CTL_PIPE_CSC_ENABLE |
PLANE_CTL_PLANE_GAMMA_DISABLE;
+ plane_ctl |= skl_plane_ctl_format(fb->format->format);
}
- plane_ctl |= skl_plane_ctl_format(fb->format->format);
plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
plane_ctl |= skl_plane_ctl_rotation(rotation);
@@ -3541,6 +3560,20 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
return plane_ctl;
}
+u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state *plane_state)
+{
+ const struct drm_framebuffer *fb = plane_state->base.fb;
+ u32 plane_color_ctl = 0;
+
+ 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(fb->format->format);
+
+ return plane_color_ctl;
+}
+
static int
__intel_display_resume(struct drm_device *dev,
struct drm_atomic_state *state,
@@ -8426,7 +8459,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- u32 val, base, offset, stride_mult, tiling;
+ u32 val, base, offset, stride_mult, tiling, alpha;
int pipe = crtc->pipe;
int fourcc, pixel_format;
unsigned int aligned_height;
@@ -8448,9 +8481,16 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
goto error;
pixel_format = val & PLANE_CTL_FORMAT_MASK;
+
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+ alpha = I915_READ(PLANE_COLOR_CTL(pipe, 0));
+ alpha &= PLANE_COLOR_ALPHA_MASK;
+ } else {
+ alpha = val & PLANE_CTL_ALPHA_MASK;
+ }
+
fourcc = skl_format_to_fourcc(pixel_format,
- val & PLANE_CTL_ORDER_RGBX,
- val & PLANE_CTL_ALPHA_MASK);
+ val & PLANE_CTL_ORDER_RGBX, alpha);
fb->format = drm_format_info(fourcc);
tiling = val & PLANE_CTL_TILED_MASK;
@@ -12831,6 +12871,11 @@ intel_check_primary_plane(struct intel_plane *plane,
state->ctl = i9xx_plane_ctl(crtc_state, state);
}
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+ state->color_ctl = glk_plane_color_ctl(crtc_state, state);
+ else
+ state->color_ctl = 0;
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 00b488688042..92bc1325701b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -425,6 +425,9 @@ struct intel_plane_state {
/* plane control register */
u32 ctl;
+ /* plane color control register */
+ u32 color_ctl;
+
/*
* scaler_id
* = -1 : not using a scaler
@@ -1504,6 +1507,8 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
return i915_ggtt_offset(state->vma);
}
+u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state *plane_state);
u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state);
u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 4fcf80ca91dd..18b9c98bc802 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -240,6 +240,7 @@ skl_update_plane(struct intel_plane *plane,
enum plane_id plane_id = plane->id;
enum pipe pipe = plane->pipe;
u32 plane_ctl = plane_state->ctl;
+ u32 plane_color_ctl = plane_state->color_ctl;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 surf_addr = plane_state->main.offset;
unsigned int rotation = plane_state->base.rotation;
@@ -263,13 +264,9 @@ skl_update_plane(struct intel_plane *plane,
spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
- if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
- PLANE_COLOR_PIPE_GAMMA_ENABLE |
- PLANE_COLOR_PIPE_CSC_ENABLE |
- PLANE_COLOR_PLANE_GAMMA_DISABLE);
- }
-
+ plane_color_ctl);
if (key->flags) {
I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);
@@ -978,6 +975,11 @@ intel_check_sprite_plane(struct intel_plane *plane,
state->ctl = g4x_sprite_ctl(crtc_state, state);
}
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+ state->color_ctl = glk_plane_color_ctl(crtc_state, state);
+ else
+ state->color_ctl = 0;
+
return 0;
}
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
2017-11-03 10:12 ` [PATCH] " Ville Syrjälä
@ 2017-11-03 17:01 ` James Ausmus
0 siblings, 0 replies; 14+ messages in thread
From: James Ausmus @ 2017-11-03 17:01 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Paulo Zanoni
On Fri, Nov 03, 2017 at 12:12:09PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 02, 2017 at 04:00:49PM -0700, James Ausmus wrote:
> > Since GLK, some plane configuration settings have moved to the
> > PLANE_COLOR_CTL register. Refactor handling of the register to work like
> > PLANE_CTL. This also allows us to fix the set/read of the plane Alpha
> > Mode for GLK+.
> >
> > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 12 +++++---
> > drivers/gpu/drm/i915/intel_display.c | 60 +++++++++++++++++++++++++++++++++---
> > drivers/gpu/drm/i915/intel_drv.h | 5 +++
> > drivers/gpu/drm/i915/intel_sprite.c | 14 +++++----
> > 4 files changed, 76 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 68a58cce6ab1..520ff9a15222 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6263,7 +6263,7 @@ enum {
> > #define _PLANE_CTL_2_A 0x70280
> > #define _PLANE_CTL_3_A 0x70380
> > #define PLANE_CTL_ENABLE (1 << 31)
> > -#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30)
> > +#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-GLK */
> > #define PLANE_CTL_FORMAT_MASK (0xf << 24)
> > #define PLANE_CTL_FORMAT_YUV422 ( 0 << 24)
> > #define PLANE_CTL_FORMAT_NV12 ( 1 << 24)
> > @@ -6273,7 +6273,7 @@ enum {
> > #define PLANE_CTL_FORMAT_AYUV ( 8 << 24)
> > #define PLANE_CTL_FORMAT_INDEXED ( 12 << 24)
> > #define PLANE_CTL_FORMAT_RGB_565 ( 14 << 24)
> > -#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23)
> > +#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23) /* Pre-GLK */
> > #define PLANE_CTL_KEY_ENABLE_MASK (0x3 << 21)
> > #define PLANE_CTL_KEY_ENABLE_SOURCE ( 1 << 21)
> > #define PLANE_CTL_KEY_ENABLE_DESTINATION ( 2 << 21)
> > @@ -6286,13 +6286,13 @@ enum {
> > #define PLANE_CTL_YUV422_VYUY ( 3 << 16)
> > #define PLANE_CTL_DECOMPRESSION_ENABLE (1 << 15)
> > #define PLANE_CTL_TRICKLE_FEED_DISABLE (1 << 14)
> > -#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13)
> > +#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13) /* Pre-GLK */
> > #define PLANE_CTL_TILED_MASK (0x7 << 10)
> > #define PLANE_CTL_TILED_LINEAR ( 0 << 10)
> > #define PLANE_CTL_TILED_X ( 1 << 10)
> > #define PLANE_CTL_TILED_Y ( 4 << 10)
> > #define PLANE_CTL_TILED_YF ( 5 << 10)
> > -#define PLANE_CTL_ALPHA_MASK (0x3 << 4)
> > +#define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
> > #define PLANE_CTL_ALPHA_DISABLE ( 0 << 4)
> > #define PLANE_CTL_ALPHA_SW_PREMULTIPLY ( 2 << 4)
> > #define PLANE_CTL_ALPHA_HW_PREMULTIPLY ( 3 << 4)
> > @@ -6332,6 +6332,10 @@ enum {
> > #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30)
> > #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23)
> > #define PLANE_COLOR_PLANE_GAMMA_DISABLE (1 << 13)
> > +#define PLANE_COLOR_ALPHA_MASK (0x3 << 4)
> > +#define PLANE_COLOR_ALPHA_DISABLE (0 << 4)
> > +#define PLANE_COLOR_ALPHA_SW_PREMULTIPLY (2 << 4)
> > +#define PLANE_COLOR_ALPHA_HW_PREMULTIPLY (3 << 4)
> > #define _PLANE_BUF_CFG_1_A 0x7027c
> > #define _PLANE_BUF_CFG_2_A 0x7037c
> > #define _PLANE_NV12_BUF_CFG_1_A 0x70278
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index e2ac976844d8..0883e857dda9 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3466,6 +3466,29 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
> > return 0;
> > }
> >
> > +static u32 glk_plane_ctl_format(uint32_t pixel_format)
> > +{
> > + /* GLK+ moves the alpha mask to a different register */
> > + return skl_plane_ctl_format(pixel_format) & ~PLANE_CTL_ALPHA_MASK;
> > +}
> > +
> > +static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format)
> > +{
> > + switch (pixel_format) {
> > + /*
> > + * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
> > + * to be already pre-multiplied. We need to add a knob (or a different
> > + * DRM_FORMAT) for user-space to configure that.
> > + */
>
> Why is this comment getting added to the glk function? It's a generic
> issue that affects all platforms with alpha blending.
Copy-pasta from the skl_plane_ctl_format function - I'll drop it.
Thanks for the review!
-James
>
> > + case DRM_FORMAT_ABGR8888:
> > + return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
> > + case DRM_FORMAT_ARGB8888:
> > + return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
>
> case A:
> case B:
> return FOO;
>
> > + default:
> > + return PLANE_COLOR_ALPHA_DISABLE;
> > + }
> > +}
> > +
> > static u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
> > {
> > switch (fb_modifier) {
> > @@ -3522,14 +3545,16 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> >
> > plane_ctl = PLANE_CTL_ENABLE;
> >
> > - if (!IS_GEMINILAKE(dev_priv) && !IS_CANNONLAKE(dev_priv)) {
> > + if (!IS_GEMINILAKE(dev_priv) && !(INTEL_GEN(dev_priv) >= 10)) {
>
> Please flip this entire logic around:
> if (GEN >= 10 || IS_GLK) {
> ...
>
> > plane_ctl |=
> > PLANE_CTL_PIPE_GAMMA_ENABLE |
> > PLANE_CTL_PIPE_CSC_ENABLE |
> > PLANE_CTL_PLANE_GAMMA_DISABLE;
> > + plane_ctl |= skl_plane_ctl_format(fb->format->format);
> > + } else {
> > + plane_ctl |= glk_plane_ctl_format(fb->format->format);
> > }
> >
> > - plane_ctl |= skl_plane_ctl_format(fb->format->format);
> > plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
> > plane_ctl |= skl_plane_ctl_rotation(rotation);
> >
> > @@ -3541,6 +3566,20 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> > return plane_ctl;
> > }
> >
> > +u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> > + const struct intel_plane_state *plane_state)
> > +{
> > + const struct drm_framebuffer *fb = plane_state->base.fb;
> > + u32 plane_color_ctl = 0;
> > +
> > + 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(fb->format->format);
> > +
> > + return plane_color_ctl;
> > +}
> > +
> > static int
> > __intel_display_resume(struct drm_device *dev,
> > struct drm_atomic_state *state,
> > @@ -8425,7 +8464,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> > {
> > struct drm_device *dev = crtc->base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > - u32 val, base, offset, stride_mult, tiling;
> > + u32 val, base, offset, stride_mult, tiling, alpha;
> > int pipe = crtc->pipe;
> > int fourcc, pixel_format;
> > unsigned int aligned_height;
> > @@ -8447,9 +8486,16 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> > goto error;
> >
> > pixel_format = val & PLANE_CTL_FORMAT_MASK;
> > +
> > + if (!IS_GEMINILAKE(dev_priv) && !(INTEL_GEN(dev_priv) >= 10)) {
>
> ditto
>
> > + alpha = val & PLANE_CTL_ALPHA_MASK;
> > + } else {
> > + alpha = I915_READ(PLANE_COLOR_CTL(pipe, 0));
> > + alpha &= PLANE_COLOR_ALPHA_MASK;
> > + }
> > +
> > fourcc = skl_format_to_fourcc(pixel_format,
> > - val & PLANE_CTL_ORDER_RGBX,
> > - val & PLANE_CTL_ALPHA_MASK);
> > + val & PLANE_CTL_ORDER_RGBX, alpha);
> > fb->format = drm_format_info(fourcc);
> >
> > tiling = val & PLANE_CTL_TILED_MASK;
> > @@ -12816,6 +12862,10 @@ intel_check_primary_plane(struct intel_plane *plane,
> >
> > state->ctl = i9xx_plane_ctl(crtc_state, state);
> > }
> > + if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
>
> I prefer the order to go from new to old. Makes for a more logical
> read if we add another branch to the if statement with a more recent
> platforms eg.
>
> if (LATEST)
> ...
> else if (SLIGHTLY_OLDER || EVEN_OLDER)
> ...
>
> > + state->color_ctl = glk_plane_color_ctl(crtc_state, state);
> > + else
> > + state->color_ctl = 0;
> >
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 47d022d48718..7967b3705217 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -420,6 +420,9 @@ struct intel_plane_state {
> > /* plane control register */
> > u32 ctl;
> >
> > + /* plane color control register */
> > + u32 color_ctl;
> > +
> > /*
> > * scaler_id
> > * = -1 : not using a scaler
> > @@ -1492,6 +1495,8 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
> > return i915_ggtt_offset(state->vma);
> > }
> >
> > +u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> > + const struct intel_plane_state *plane_state);
> > u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> > const struct intel_plane_state *plane_state);
> > u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 4fcf80ca91dd..0c09657101b7 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -240,6 +240,7 @@ skl_update_plane(struct intel_plane *plane,
> > enum plane_id plane_id = plane->id;
> > enum pipe pipe = plane->pipe;
> > u32 plane_ctl = plane_state->ctl;
> > + u32 plane_color_ctl = plane_state->color_ctl;
> > const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> > u32 surf_addr = plane_state->main.offset;
> > unsigned int rotation = plane_state->base.rotation;
> > @@ -263,13 +264,9 @@ skl_update_plane(struct intel_plane *plane,
> >
> > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> >
> > - if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > + if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
>
> ditto
>
> > I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
> > - PLANE_COLOR_PIPE_GAMMA_ENABLE |
> > - PLANE_COLOR_PIPE_CSC_ENABLE |
> > - PLANE_COLOR_PLANE_GAMMA_DISABLE);
> > - }
> > -
> > + plane_color_ctl);
> > if (key->flags) {
> > I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
> > I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);
> > @@ -978,6 +975,11 @@ intel_check_sprite_plane(struct intel_plane *plane,
> > state->ctl = g4x_sprite_ctl(crtc_state, state);
> > }
> >
> > + if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
>
> ditto
>
> > + state->color_ctl = glk_plane_color_ctl(crtc_state, state);
> > + else
> > + state->color_ctl = 0;
> > +
> > return 0;
> > }
> >
> > --
> > 2.14.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev2)
2017-11-02 23:00 [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
` (2 preceding siblings ...)
2017-11-03 16:58 ` [PATCH v2] " James Ausmus
@ 2017-11-03 17:23 ` Patchwork
2017-11-03 18:25 ` ✗ Fi.CI.IGT: failure " Patchwork
` (3 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-11-03 17:23 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev2)
URL : https://patchwork.freedesktop.org/series/33087/
State : success
== Summary ==
Series 33087v2 drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
https://patchwork.freedesktop.org/api/1.0/series/33087/revisions/2/mbox/
Test gem_exec_reloc:
Subgroup basic-cpu-active:
pass -> FAIL (fi-gdg-551) fdo#102582 +4
Test kms_busy:
Subgroup basic-flip-b:
fail -> PASS (fi-bwr-2160)
Test drv_module_reload:
Subgroup basic-no-display:
fail -> PASS (fi-hsw-4770r) fdo#103534
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fdo#103534 https://bugs.freedesktop.org/show_bug.cgi?id=103534
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:446s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:453s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:380s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:540s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:277s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:506s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:503s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:504s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:494s
fi-cfl-s total:289 pass:254 dwarn:3 dfail:0 fail:0 skip:32 time:565s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:432s
fi-gdg-551 total:289 pass:174 dwarn:1 dfail:0 fail:5 skip:109 time:261s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:584s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:432s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:430s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:426s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:499s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:464s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:490s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:576s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:482s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:588s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:573s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:454s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:595s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:653s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:520s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:502s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:460s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:584s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:424s
de359919ae463cdaef6bc6890156df84e19dee2a drm-tip: 2017y-11m-03d-14h-00m-37s UTC integration manifest
3f6947568e46 drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6945/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
2017-11-03 16:58 ` [PATCH v2] " James Ausmus
@ 2017-11-03 17:31 ` Ville Syrjälä
2017-11-03 17:45 ` James Ausmus
0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2017-11-03 17:31 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx, Paulo Zanoni
On Fri, Nov 03, 2017 at 09:58:48AM -0700, James Ausmus wrote:
> Since GLK, some plane configuration settings have moved to the
> PLANE_COLOR_CTL register. Refactor handling of the register to work like
> PLANE_CTL. This also allows us to fix the set/read of the plane Alpha
> Mode for GLK+.
>
> v2: Adjust ordering of platform checks to be newest->oldest, drop
> redundant comment about alpha blending. (Ville)
>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 12 +++++---
> drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++++++++++++----
> drivers/gpu/drm/i915/intel_drv.h | 5 ++++
> drivers/gpu/drm/i915/intel_sprite.c | 14 +++++----
> 4 files changed, 71 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f0f8f6059652..ecd6b236e005 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6263,7 +6263,7 @@ enum {
> #define _PLANE_CTL_2_A 0x70280
> #define _PLANE_CTL_3_A 0x70380
> #define PLANE_CTL_ENABLE (1 << 31)
> -#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30)
> +#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-GLK */
> #define PLANE_CTL_FORMAT_MASK (0xf << 24)
> #define PLANE_CTL_FORMAT_YUV422 ( 0 << 24)
> #define PLANE_CTL_FORMAT_NV12 ( 1 << 24)
> @@ -6273,7 +6273,7 @@ enum {
> #define PLANE_CTL_FORMAT_AYUV ( 8 << 24)
> #define PLANE_CTL_FORMAT_INDEXED ( 12 << 24)
> #define PLANE_CTL_FORMAT_RGB_565 ( 14 << 24)
> -#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23)
> +#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23) /* Pre-GLK */
> #define PLANE_CTL_KEY_ENABLE_MASK (0x3 << 21)
> #define PLANE_CTL_KEY_ENABLE_SOURCE ( 1 << 21)
> #define PLANE_CTL_KEY_ENABLE_DESTINATION ( 2 << 21)
> @@ -6286,13 +6286,13 @@ enum {
> #define PLANE_CTL_YUV422_VYUY ( 3 << 16)
> #define PLANE_CTL_DECOMPRESSION_ENABLE (1 << 15)
> #define PLANE_CTL_TRICKLE_FEED_DISABLE (1 << 14)
> -#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13)
> +#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13) /* Pre-GLK */
> #define PLANE_CTL_TILED_MASK (0x7 << 10)
> #define PLANE_CTL_TILED_LINEAR ( 0 << 10)
> #define PLANE_CTL_TILED_X ( 1 << 10)
> #define PLANE_CTL_TILED_Y ( 4 << 10)
> #define PLANE_CTL_TILED_YF ( 5 << 10)
> -#define PLANE_CTL_ALPHA_MASK (0x3 << 4)
> +#define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
> #define PLANE_CTL_ALPHA_DISABLE ( 0 << 4)
> #define PLANE_CTL_ALPHA_SW_PREMULTIPLY ( 2 << 4)
> #define PLANE_CTL_ALPHA_HW_PREMULTIPLY ( 3 << 4)
> @@ -6332,6 +6332,10 @@ enum {
> #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30)
> #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23)
> #define PLANE_COLOR_PLANE_GAMMA_DISABLE (1 << 13)
> +#define PLANE_COLOR_ALPHA_MASK (0x3 << 4)
> +#define PLANE_COLOR_ALPHA_DISABLE (0 << 4)
> +#define PLANE_COLOR_ALPHA_SW_PREMULTIPLY (2 << 4)
> +#define PLANE_COLOR_ALPHA_HW_PREMULTIPLY (3 << 4)
> #define _PLANE_BUF_CFG_1_A 0x7027c
> #define _PLANE_BUF_CFG_2_A 0x7037c
> #define _PLANE_NV12_BUF_CFG_1_A 0x70278
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 737de251d0f8..39453276dad1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3466,6 +3466,23 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
> return 0;
> }
>
> +static u32 glk_plane_ctl_format(uint32_t pixel_format)
> +{
> + /* GLK+ moves the alpha mask to a different register */
> + return skl_plane_ctl_format(pixel_format) & ~PLANE_CTL_ALPHA_MASK;
I think it would be less confusing to extract the alpha stuff
from skl_plane_ctl_format() into skl_plane_ctl_alpha().
I guess with that we wouldn't even need glk_plane_ctl_format()?
> +}
> +
> +static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format)
> +{
> + switch (pixel_format) {
> + case DRM_FORMAT_ABGR8888:
> + case DRM_FORMAT_ARGB8888:
> + return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
> + default:
> + return PLANE_COLOR_ALPHA_DISABLE;
> + }
> +}
> +
> static u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
> {
> switch (fb_modifier) {
> @@ -3522,14 +3539,16 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
>
> plane_ctl = PLANE_CTL_ENABLE;
>
> - if (!IS_GEMINILAKE(dev_priv) && !IS_CANNONLAKE(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> + plane_ctl |= glk_plane_ctl_format(fb->format->format);
> + } else {
> plane_ctl |=
> PLANE_CTL_PIPE_GAMMA_ENABLE |
> PLANE_CTL_PIPE_CSC_ENABLE |
> PLANE_CTL_PLANE_GAMMA_DISABLE;
> + plane_ctl |= skl_plane_ctl_format(fb->format->format);
> }
>
> - plane_ctl |= skl_plane_ctl_format(fb->format->format);
> plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
> plane_ctl |= skl_plane_ctl_rotation(rotation);
>
> @@ -3541,6 +3560,20 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> return plane_ctl;
> }
>
> +u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> + const struct intel_plane_state *plane_state)
> +{
> + const struct drm_framebuffer *fb = plane_state->base.fb;
> + u32 plane_color_ctl = 0;
> +
> + 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(fb->format->format);
> +
> + return plane_color_ctl;
> +}
> +
> static int
> __intel_display_resume(struct drm_device *dev,
> struct drm_atomic_state *state,
> @@ -8426,7 +8459,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - u32 val, base, offset, stride_mult, tiling;
> + u32 val, base, offset, stride_mult, tiling, alpha;
> int pipe = crtc->pipe;
> int fourcc, pixel_format;
> unsigned int aligned_height;
> @@ -8448,9 +8481,16 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> goto error;
>
> pixel_format = val & PLANE_CTL_FORMAT_MASK;
> +
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> + alpha = I915_READ(PLANE_COLOR_CTL(pipe, 0));
> + alpha &= PLANE_COLOR_ALPHA_MASK;
> + } else {
> + alpha = val & PLANE_CTL_ALPHA_MASK;
> + }
> +
> fourcc = skl_format_to_fourcc(pixel_format,
> - val & PLANE_CTL_ORDER_RGBX,
> - val & PLANE_CTL_ALPHA_MASK);
> + val & PLANE_CTL_ORDER_RGBX, alpha);
> fb->format = drm_format_info(fourcc);
>
> tiling = val & PLANE_CTL_TILED_MASK;
> @@ -12831,6 +12871,11 @@ intel_check_primary_plane(struct intel_plane *plane,
> state->ctl = i9xx_plane_ctl(crtc_state, state);
> }
>
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> + state->color_ctl = glk_plane_color_ctl(crtc_state, state);
> + else
> + state->color_ctl = 0;
The else branch doesn't seem necessary since we never use color_ctl
on other platforms.
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 00b488688042..92bc1325701b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -425,6 +425,9 @@ struct intel_plane_state {
> /* plane control register */
> u32 ctl;
>
> + /* plane color control register */
> + u32 color_ctl;
> +
> /*
> * scaler_id
> * = -1 : not using a scaler
> @@ -1504,6 +1507,8 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
> return i915_ggtt_offset(state->vma);
> }
>
> +u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> + const struct intel_plane_state *plane_state);
> u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> const struct intel_plane_state *plane_state);
> u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 4fcf80ca91dd..18b9c98bc802 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -240,6 +240,7 @@ skl_update_plane(struct intel_plane *plane,
> enum plane_id plane_id = plane->id;
> enum pipe pipe = plane->pipe;
> u32 plane_ctl = plane_state->ctl;
> + u32 plane_color_ctl = plane_state->color_ctl;
Having a local variable for this seems rather pointless. Probably
you could do a patch to nuke the local variables for the control
register as well, for all platforms.
Another thing I think we should do is move most of the primary plane
stuff from intel_display.c to intel_sprite.c, and rename intel_sprite.c
to intel_plane.c. Would allow us to make more things static, and
it would shrink intel_display.c a little bit.
> const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> u32 surf_addr = plane_state->main.offset;
> unsigned int rotation = plane_state->base.rotation;
> @@ -263,13 +264,9 @@ skl_update_plane(struct intel_plane *plane,
>
> spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>
> - if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
> - PLANE_COLOR_PIPE_GAMMA_ENABLE |
> - PLANE_COLOR_PIPE_CSC_ENABLE |
> - PLANE_COLOR_PLANE_GAMMA_DISABLE);
> - }
> -
> + plane_color_ctl);
> if (key->flags) {
> I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
> I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);
> @@ -978,6 +975,11 @@ intel_check_sprite_plane(struct intel_plane *plane,
> state->ctl = g4x_sprite_ctl(crtc_state, state);
> }
>
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> + state->color_ctl = glk_plane_color_ctl(crtc_state, state);
> + else
> + state->color_ctl = 0;
No need for the else here either.
> +
> return 0;
> }
>
> --
> 2.14.1
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
2017-11-03 17:31 ` Ville Syrjälä
@ 2017-11-03 17:45 ` James Ausmus
2017-11-03 17:59 ` Ville Syrjälä
0 siblings, 1 reply; 14+ messages in thread
From: James Ausmus @ 2017-11-03 17:45 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Paulo Zanoni
On Fri, Nov 03, 2017 at 07:31:44PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 03, 2017 at 09:58:48AM -0700, James Ausmus wrote:
> > Since GLK, some plane configuration settings have moved to the
> > PLANE_COLOR_CTL register. Refactor handling of the register to work like
> > PLANE_CTL. This also allows us to fix the set/read of the plane Alpha
> > Mode for GLK+.
> >
> > v2: Adjust ordering of platform checks to be newest->oldest, drop
> > redundant comment about alpha blending. (Ville)
> >
> > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 12 +++++---
> > drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++++++++++++----
> > drivers/gpu/drm/i915/intel_drv.h | 5 ++++
> > drivers/gpu/drm/i915/intel_sprite.c | 14 +++++----
> > 4 files changed, 71 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index f0f8f6059652..ecd6b236e005 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6263,7 +6263,7 @@ enum {
> > #define _PLANE_CTL_2_A 0x70280
> > #define _PLANE_CTL_3_A 0x70380
> > #define PLANE_CTL_ENABLE (1 << 31)
> > -#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30)
> > +#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-GLK */
> > #define PLANE_CTL_FORMAT_MASK (0xf << 24)
> > #define PLANE_CTL_FORMAT_YUV422 ( 0 << 24)
> > #define PLANE_CTL_FORMAT_NV12 ( 1 << 24)
> > @@ -6273,7 +6273,7 @@ enum {
> > #define PLANE_CTL_FORMAT_AYUV ( 8 << 24)
> > #define PLANE_CTL_FORMAT_INDEXED ( 12 << 24)
> > #define PLANE_CTL_FORMAT_RGB_565 ( 14 << 24)
> > -#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23)
> > +#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23) /* Pre-GLK */
> > #define PLANE_CTL_KEY_ENABLE_MASK (0x3 << 21)
> > #define PLANE_CTL_KEY_ENABLE_SOURCE ( 1 << 21)
> > #define PLANE_CTL_KEY_ENABLE_DESTINATION ( 2 << 21)
> > @@ -6286,13 +6286,13 @@ enum {
> > #define PLANE_CTL_YUV422_VYUY ( 3 << 16)
> > #define PLANE_CTL_DECOMPRESSION_ENABLE (1 << 15)
> > #define PLANE_CTL_TRICKLE_FEED_DISABLE (1 << 14)
> > -#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13)
> > +#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13) /* Pre-GLK */
> > #define PLANE_CTL_TILED_MASK (0x7 << 10)
> > #define PLANE_CTL_TILED_LINEAR ( 0 << 10)
> > #define PLANE_CTL_TILED_X ( 1 << 10)
> > #define PLANE_CTL_TILED_Y ( 4 << 10)
> > #define PLANE_CTL_TILED_YF ( 5 << 10)
> > -#define PLANE_CTL_ALPHA_MASK (0x3 << 4)
> > +#define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
> > #define PLANE_CTL_ALPHA_DISABLE ( 0 << 4)
> > #define PLANE_CTL_ALPHA_SW_PREMULTIPLY ( 2 << 4)
> > #define PLANE_CTL_ALPHA_HW_PREMULTIPLY ( 3 << 4)
> > @@ -6332,6 +6332,10 @@ enum {
> > #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30)
> > #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23)
> > #define PLANE_COLOR_PLANE_GAMMA_DISABLE (1 << 13)
> > +#define PLANE_COLOR_ALPHA_MASK (0x3 << 4)
> > +#define PLANE_COLOR_ALPHA_DISABLE (0 << 4)
> > +#define PLANE_COLOR_ALPHA_SW_PREMULTIPLY (2 << 4)
> > +#define PLANE_COLOR_ALPHA_HW_PREMULTIPLY (3 << 4)
> > #define _PLANE_BUF_CFG_1_A 0x7027c
> > #define _PLANE_BUF_CFG_2_A 0x7037c
> > #define _PLANE_NV12_BUF_CFG_1_A 0x70278
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 737de251d0f8..39453276dad1 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3466,6 +3466,23 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
> > return 0;
> > }
> >
> > +static u32 glk_plane_ctl_format(uint32_t pixel_format)
> > +{
> > + /* GLK+ moves the alpha mask to a different register */
> > + return skl_plane_ctl_format(pixel_format) & ~PLANE_CTL_ALPHA_MASK;
>
> I think it would be less confusing to extract the alpha stuff
> from skl_plane_ctl_format() into skl_plane_ctl_alpha().
>
> I guess with that we wouldn't even need glk_plane_ctl_format()?
Yeah, I like that better. As a matter of fact, the mask and definitions
are the same between PLANE_CTL and PLANE_COLOR_CTL - would it be cleaner
to just rename the defines to PLANE_ALPHA_*, name the function
skl_plane_alpha, and then be able to use the single function for both
PLANE_CTL and PLANE_COLOR_CTL usages?
>
> > +}
> > +
> > +static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format)
> > +{
> > + switch (pixel_format) {
> > + case DRM_FORMAT_ABGR8888:
> > + case DRM_FORMAT_ARGB8888:
> > + return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
> > + default:
> > + return PLANE_COLOR_ALPHA_DISABLE;
> > + }
> > +}
> > +
> > static u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
> > {
> > switch (fb_modifier) {
> > @@ -3522,14 +3539,16 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> >
> > plane_ctl = PLANE_CTL_ENABLE;
> >
> > - if (!IS_GEMINILAKE(dev_priv) && !IS_CANNONLAKE(dev_priv)) {
> > + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> > + plane_ctl |= glk_plane_ctl_format(fb->format->format);
> > + } else {
> > plane_ctl |=
> > PLANE_CTL_PIPE_GAMMA_ENABLE |
> > PLANE_CTL_PIPE_CSC_ENABLE |
> > PLANE_CTL_PLANE_GAMMA_DISABLE;
> > + plane_ctl |= skl_plane_ctl_format(fb->format->format);
> > }
> >
> > - plane_ctl |= skl_plane_ctl_format(fb->format->format);
> > plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
> > plane_ctl |= skl_plane_ctl_rotation(rotation);
> >
> > @@ -3541,6 +3560,20 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> > return plane_ctl;
> > }
> >
> > +u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> > + const struct intel_plane_state *plane_state)
> > +{
> > + const struct drm_framebuffer *fb = plane_state->base.fb;
> > + u32 plane_color_ctl = 0;
> > +
> > + 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(fb->format->format);
> > +
> > + return plane_color_ctl;
> > +}
> > +
> > static int
> > __intel_display_resume(struct drm_device *dev,
> > struct drm_atomic_state *state,
> > @@ -8426,7 +8459,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> > {
> > struct drm_device *dev = crtc->base.dev;
> > struct drm_i915_private *dev_priv = to_i915(dev);
> > - u32 val, base, offset, stride_mult, tiling;
> > + u32 val, base, offset, stride_mult, tiling, alpha;
> > int pipe = crtc->pipe;
> > int fourcc, pixel_format;
> > unsigned int aligned_height;
> > @@ -8448,9 +8481,16 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> > goto error;
> >
> > pixel_format = val & PLANE_CTL_FORMAT_MASK;
> > +
> > + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> > + alpha = I915_READ(PLANE_COLOR_CTL(pipe, 0));
> > + alpha &= PLANE_COLOR_ALPHA_MASK;
> > + } else {
> > + alpha = val & PLANE_CTL_ALPHA_MASK;
> > + }
> > +
> > fourcc = skl_format_to_fourcc(pixel_format,
> > - val & PLANE_CTL_ORDER_RGBX,
> > - val & PLANE_CTL_ALPHA_MASK);
> > + val & PLANE_CTL_ORDER_RGBX, alpha);
> > fb->format = drm_format_info(fourcc);
> >
> > tiling = val & PLANE_CTL_TILED_MASK;
> > @@ -12831,6 +12871,11 @@ intel_check_primary_plane(struct intel_plane *plane,
> > state->ctl = i9xx_plane_ctl(crtc_state, state);
> > }
> >
> > + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> > + state->color_ctl = glk_plane_color_ctl(crtc_state, state);
> > + else
> > + state->color_ctl = 0;
>
> The else branch doesn't seem necessary since we never use color_ctl
> on other platforms.
OK - wasn't sure if it was better to leave it uninitialized since it's
not used, or uselessly initialize it. :)
>
> > +
> > return 0;
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 00b488688042..92bc1325701b 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -425,6 +425,9 @@ struct intel_plane_state {
> > /* plane control register */
> > u32 ctl;
> >
> > + /* plane color control register */
> > + u32 color_ctl;
> > +
> > /*
> > * scaler_id
> > * = -1 : not using a scaler
> > @@ -1504,6 +1507,8 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
> > return i915_ggtt_offset(state->vma);
> > }
> >
> > +u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> > + const struct intel_plane_state *plane_state);
> > u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> > const struct intel_plane_state *plane_state);
> > u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 4fcf80ca91dd..18b9c98bc802 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -240,6 +240,7 @@ skl_update_plane(struct intel_plane *plane,
> > enum plane_id plane_id = plane->id;
> > enum pipe pipe = plane->pipe;
> > u32 plane_ctl = plane_state->ctl;
> > + u32 plane_color_ctl = plane_state->color_ctl;
>
> Having a local variable for this seems rather pointless. Probably
> you could do a patch to nuke the local variables for the control
> register as well, for all platforms.
I'll drop the new local, and follow up with a patch to drop plane_ctl as
well.
>
> Another thing I think we should do is move most of the primary plane
> stuff from intel_display.c to intel_sprite.c, and rename intel_sprite.c
> to intel_plane.c. Would allow us to make more things static, and
> it would shrink intel_display.c a little bit.
Sounds good - I'll throw it on my TODO, unless someone else wants to get
to it before me. :)
>
> > const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> > u32 surf_addr = plane_state->main.offset;
> > unsigned int rotation = plane_state->base.rotation;
> > @@ -263,13 +264,9 @@ skl_update_plane(struct intel_plane *plane,
> >
> > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> >
> > - if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> > + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> > I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
> > - PLANE_COLOR_PIPE_GAMMA_ENABLE |
> > - PLANE_COLOR_PIPE_CSC_ENABLE |
> > - PLANE_COLOR_PLANE_GAMMA_DISABLE);
> > - }
> > -
> > + plane_color_ctl);
> > if (key->flags) {
> > I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
> > I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);
> > @@ -978,6 +975,11 @@ intel_check_sprite_plane(struct intel_plane *plane,
> > state->ctl = g4x_sprite_ctl(crtc_state, state);
> > }
> >
> > + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> > + state->color_ctl = glk_plane_color_ctl(crtc_state, state);
> > + else
> > + state->color_ctl = 0;
>
> No need for the else here either.
ACK
Thanks for the review!
-James
>
> > +
> > return 0;
> > }
> >
> > --
> > 2.14.1
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
2017-11-03 17:45 ` James Ausmus
@ 2017-11-03 17:59 ` Ville Syrjälä
0 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2017-11-03 17:59 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx, Paulo Zanoni
On Fri, Nov 03, 2017 at 10:45:30AM -0700, James Ausmus wrote:
> On Fri, Nov 03, 2017 at 07:31:44PM +0200, Ville Syrjälä wrote:
> > On Fri, Nov 03, 2017 at 09:58:48AM -0700, James Ausmus wrote:
> > > Since GLK, some plane configuration settings have moved to the
> > > PLANE_COLOR_CTL register. Refactor handling of the register to work like
> > > PLANE_CTL. This also allows us to fix the set/read of the plane Alpha
> > > Mode for GLK+.
> > >
> > > v2: Adjust ordering of platform checks to be newest->oldest, drop
> > > redundant comment about alpha blending. (Ville)
> > >
> > > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 12 +++++---
> > > drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++++++++++++----
> > > drivers/gpu/drm/i915/intel_drv.h | 5 ++++
> > > drivers/gpu/drm/i915/intel_sprite.c | 14 +++++----
> > > 4 files changed, 71 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index f0f8f6059652..ecd6b236e005 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6263,7 +6263,7 @@ enum {
> > > #define _PLANE_CTL_2_A 0x70280
> > > #define _PLANE_CTL_3_A 0x70380
> > > #define PLANE_CTL_ENABLE (1 << 31)
> > > -#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30)
> > > +#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-GLK */
> > > #define PLANE_CTL_FORMAT_MASK (0xf << 24)
> > > #define PLANE_CTL_FORMAT_YUV422 ( 0 << 24)
> > > #define PLANE_CTL_FORMAT_NV12 ( 1 << 24)
> > > @@ -6273,7 +6273,7 @@ enum {
> > > #define PLANE_CTL_FORMAT_AYUV ( 8 << 24)
> > > #define PLANE_CTL_FORMAT_INDEXED ( 12 << 24)
> > > #define PLANE_CTL_FORMAT_RGB_565 ( 14 << 24)
> > > -#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23)
> > > +#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23) /* Pre-GLK */
> > > #define PLANE_CTL_KEY_ENABLE_MASK (0x3 << 21)
> > > #define PLANE_CTL_KEY_ENABLE_SOURCE ( 1 << 21)
> > > #define PLANE_CTL_KEY_ENABLE_DESTINATION ( 2 << 21)
> > > @@ -6286,13 +6286,13 @@ enum {
> > > #define PLANE_CTL_YUV422_VYUY ( 3 << 16)
> > > #define PLANE_CTL_DECOMPRESSION_ENABLE (1 << 15)
> > > #define PLANE_CTL_TRICKLE_FEED_DISABLE (1 << 14)
> > > -#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13)
> > > +#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13) /* Pre-GLK */
> > > #define PLANE_CTL_TILED_MASK (0x7 << 10)
> > > #define PLANE_CTL_TILED_LINEAR ( 0 << 10)
> > > #define PLANE_CTL_TILED_X ( 1 << 10)
> > > #define PLANE_CTL_TILED_Y ( 4 << 10)
> > > #define PLANE_CTL_TILED_YF ( 5 << 10)
> > > -#define PLANE_CTL_ALPHA_MASK (0x3 << 4)
> > > +#define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
> > > #define PLANE_CTL_ALPHA_DISABLE ( 0 << 4)
> > > #define PLANE_CTL_ALPHA_SW_PREMULTIPLY ( 2 << 4)
> > > #define PLANE_CTL_ALPHA_HW_PREMULTIPLY ( 3 << 4)
> > > @@ -6332,6 +6332,10 @@ enum {
> > > #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30)
> > > #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23)
> > > #define PLANE_COLOR_PLANE_GAMMA_DISABLE (1 << 13)
> > > +#define PLANE_COLOR_ALPHA_MASK (0x3 << 4)
> > > +#define PLANE_COLOR_ALPHA_DISABLE (0 << 4)
> > > +#define PLANE_COLOR_ALPHA_SW_PREMULTIPLY (2 << 4)
> > > +#define PLANE_COLOR_ALPHA_HW_PREMULTIPLY (3 << 4)
> > > #define _PLANE_BUF_CFG_1_A 0x7027c
> > > #define _PLANE_BUF_CFG_2_A 0x7037c
> > > #define _PLANE_NV12_BUF_CFG_1_A 0x70278
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 737de251d0f8..39453276dad1 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3466,6 +3466,23 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
> > > return 0;
> > > }
> > >
> > > +static u32 glk_plane_ctl_format(uint32_t pixel_format)
> > > +{
> > > + /* GLK+ moves the alpha mask to a different register */
> > > + return skl_plane_ctl_format(pixel_format) & ~PLANE_CTL_ALPHA_MASK;
> >
> > I think it would be less confusing to extract the alpha stuff
> > from skl_plane_ctl_format() into skl_plane_ctl_alpha().
> >
> > I guess with that we wouldn't even need glk_plane_ctl_format()?
>
> Yeah, I like that better. As a matter of fact, the mask and definitions
> are the same between PLANE_CTL and PLANE_COLOR_CTL - would it be cleaner
> to just rename the defines to PLANE_ALPHA_*, name the function
> skl_plane_alpha, and then be able to use the single function for both
> PLANE_CTL and PLANE_COLOR_CTL usages?
Probably not. At least I was scratching my head again recently with
the DDI_PORT_WIDTH() defines which get used with two otherwise totally
different registers. I cooked up a patch to duplicate the defines to
make sure I won't have to be confused again. Didn't send those out yet
though.
>
> >
> > > +}
> > > +
> > > +static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format)
> > > +{
> > > + switch (pixel_format) {
> > > + case DRM_FORMAT_ABGR8888:
> > > + case DRM_FORMAT_ARGB8888:
> > > + return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
> > > + default:
> > > + return PLANE_COLOR_ALPHA_DISABLE;
> > > + }
> > > +}
> > > +
> > > static u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
> > > {
> > > switch (fb_modifier) {
> > > @@ -3522,14 +3539,16 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> > >
> > > plane_ctl = PLANE_CTL_ENABLE;
> > >
> > > - if (!IS_GEMINILAKE(dev_priv) && !IS_CANNONLAKE(dev_priv)) {
> > > + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> > > + plane_ctl |= glk_plane_ctl_format(fb->format->format);
> > > + } else {
> > > plane_ctl |=
> > > PLANE_CTL_PIPE_GAMMA_ENABLE |
> > > PLANE_CTL_PIPE_CSC_ENABLE |
> > > PLANE_CTL_PLANE_GAMMA_DISABLE;
> > > + plane_ctl |= skl_plane_ctl_format(fb->format->format);
> > > }
> > >
> > > - plane_ctl |= skl_plane_ctl_format(fb->format->format);
> > > plane_ctl |= skl_plane_ctl_tiling(fb->modifier);
> > > plane_ctl |= skl_plane_ctl_rotation(rotation);
> > >
> > > @@ -3541,6 +3560,20 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> > > return plane_ctl;
> > > }
> > >
> > > +u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> > > + const struct intel_plane_state *plane_state)
> > > +{
> > > + const struct drm_framebuffer *fb = plane_state->base.fb;
> > > + u32 plane_color_ctl = 0;
> > > +
> > > + 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(fb->format->format);
> > > +
> > > + return plane_color_ctl;
> > > +}
> > > +
> > > static int
> > > __intel_display_resume(struct drm_device *dev,
> > > struct drm_atomic_state *state,
> > > @@ -8426,7 +8459,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> > > {
> > > struct drm_device *dev = crtc->base.dev;
> > > struct drm_i915_private *dev_priv = to_i915(dev);
> > > - u32 val, base, offset, stride_mult, tiling;
> > > + u32 val, base, offset, stride_mult, tiling, alpha;
> > > int pipe = crtc->pipe;
> > > int fourcc, pixel_format;
> > > unsigned int aligned_height;
> > > @@ -8448,9 +8481,16 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> > > goto error;
> > >
> > > pixel_format = val & PLANE_CTL_FORMAT_MASK;
> > > +
> > > + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> > > + alpha = I915_READ(PLANE_COLOR_CTL(pipe, 0));
> > > + alpha &= PLANE_COLOR_ALPHA_MASK;
> > > + } else {
> > > + alpha = val & PLANE_CTL_ALPHA_MASK;
> > > + }
> > > +
> > > fourcc = skl_format_to_fourcc(pixel_format,
> > > - val & PLANE_CTL_ORDER_RGBX,
> > > - val & PLANE_CTL_ALPHA_MASK);
> > > + val & PLANE_CTL_ORDER_RGBX, alpha);
> > > fb->format = drm_format_info(fourcc);
> > >
> > > tiling = val & PLANE_CTL_TILED_MASK;
> > > @@ -12831,6 +12871,11 @@ intel_check_primary_plane(struct intel_plane *plane,
> > > state->ctl = i9xx_plane_ctl(crtc_state, state);
> > > }
> > >
> > > + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> > > + state->color_ctl = glk_plane_color_ctl(crtc_state, state);
> > > + else
> > > + state->color_ctl = 0;
> >
> > The else branch doesn't seem necessary since we never use color_ctl
> > on other platforms.
>
> OK - wasn't sure if it was better to leave it uninitialized since it's
> not used, or uselessly initialize it. :)
We zero initialize most things we allocate, and depend on that fact
elsewhere as well.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✗ Fi.CI.IGT: failure for drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev2)
2017-11-02 23:00 [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
` (3 preceding siblings ...)
2017-11-03 17:23 ` ✓ Fi.CI.BAT: success for drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev2) Patchwork
@ 2017-11-03 18:25 ` Patchwork
2017-11-03 20:43 ` [PATCH v3] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
` (2 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-11-03 18:25 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev2)
URL : https://patchwork.freedesktop.org/series/33087/
State : failure
== Summary ==
Test kms_flip:
Subgroup flip-vs-dpms-interruptible:
pass -> INCOMPLETE (shard-hsw)
shard-hsw total:2526 pass:1425 dwarn:0 dfail:1 fail:8 skip:1091 time:9248s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6945/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
2017-11-02 23:00 [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
` (4 preceding siblings ...)
2017-11-03 18:25 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2017-11-03 20:43 ` James Ausmus
2017-11-10 19:20 ` Ville Syrjälä
2017-11-03 21:25 ` ✗ Fi.CI.BAT: failure for drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev3) Patchwork
2017-11-06 22:15 ` Patchwork
7 siblings, 1 reply; 14+ messages in thread
From: James Ausmus @ 2017-11-03 20:43 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
Since GLK, some plane configuration settings have moved to the
PLANE_COLOR_CTL register. Refactor handling of the register to work like
PLANE_CTL. This also allows us to fix the set/read of the plane Alpha
Mode for GLK+.
v2: Adjust ordering of platform checks to be newest->oldest, drop
redundant comment about alpha blending. (Ville)
v3: Move Alpha Mode bits out of skl_plane_ctl_format into
skl_plane_ctl_alpha, and drop glk_plane_ctl_format, drop initialization
of state->color_ctl on platforms that don't use it, and drop color_ctl
local var. (Ville)
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 12 ++++--
drivers/gpu/drm/i915/intel_display.c | 71 +++++++++++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_drv.h | 5 +++
drivers/gpu/drm/i915/intel_sprite.c | 11 +++---
4 files changed, 76 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f0f8f6059652..ecd6b236e005 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6263,7 +6263,7 @@ enum {
#define _PLANE_CTL_2_A 0x70280
#define _PLANE_CTL_3_A 0x70380
#define PLANE_CTL_ENABLE (1 << 31)
-#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30)
+#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-GLK */
#define PLANE_CTL_FORMAT_MASK (0xf << 24)
#define PLANE_CTL_FORMAT_YUV422 ( 0 << 24)
#define PLANE_CTL_FORMAT_NV12 ( 1 << 24)
@@ -6273,7 +6273,7 @@ enum {
#define PLANE_CTL_FORMAT_AYUV ( 8 << 24)
#define PLANE_CTL_FORMAT_INDEXED ( 12 << 24)
#define PLANE_CTL_FORMAT_RGB_565 ( 14 << 24)
-#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23)
+#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23) /* Pre-GLK */
#define PLANE_CTL_KEY_ENABLE_MASK (0x3 << 21)
#define PLANE_CTL_KEY_ENABLE_SOURCE ( 1 << 21)
#define PLANE_CTL_KEY_ENABLE_DESTINATION ( 2 << 21)
@@ -6286,13 +6286,13 @@ enum {
#define PLANE_CTL_YUV422_VYUY ( 3 << 16)
#define PLANE_CTL_DECOMPRESSION_ENABLE (1 << 15)
#define PLANE_CTL_TRICKLE_FEED_DISABLE (1 << 14)
-#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13)
+#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13) /* Pre-GLK */
#define PLANE_CTL_TILED_MASK (0x7 << 10)
#define PLANE_CTL_TILED_LINEAR ( 0 << 10)
#define PLANE_CTL_TILED_X ( 1 << 10)
#define PLANE_CTL_TILED_Y ( 4 << 10)
#define PLANE_CTL_TILED_YF ( 5 << 10)
-#define PLANE_CTL_ALPHA_MASK (0x3 << 4)
+#define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
#define PLANE_CTL_ALPHA_DISABLE ( 0 << 4)
#define PLANE_CTL_ALPHA_SW_PREMULTIPLY ( 2 << 4)
#define PLANE_CTL_ALPHA_HW_PREMULTIPLY ( 3 << 4)
@@ -6332,6 +6332,10 @@ enum {
#define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30)
#define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23)
#define PLANE_COLOR_PLANE_GAMMA_DISABLE (1 << 13)
+#define PLANE_COLOR_ALPHA_MASK (0x3 << 4)
+#define PLANE_COLOR_ALPHA_DISABLE (0 << 4)
+#define PLANE_COLOR_ALPHA_SW_PREMULTIPLY (2 << 4)
+#define PLANE_COLOR_ALPHA_HW_PREMULTIPLY (3 << 4)
#define _PLANE_BUF_CFG_1_A 0x7027c
#define _PLANE_BUF_CFG_2_A 0x7037c
#define _PLANE_NV12_BUF_CFG_1_A 0x70278
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 737de251d0f8..214c0c119002 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3436,17 +3436,10 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
return PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX;
case DRM_FORMAT_XRGB8888:
return PLANE_CTL_FORMAT_XRGB_8888;
- /*
- * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
- * to be already pre-multiplied. We need to add a knob (or a different
- * DRM_FORMAT) for user-space to configure that.
- */
case DRM_FORMAT_ABGR8888:
- return PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX |
- PLANE_CTL_ALPHA_SW_PREMULTIPLY;
+ return PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX;
case DRM_FORMAT_ARGB8888:
- return PLANE_CTL_FORMAT_XRGB_8888 |
- PLANE_CTL_ALPHA_SW_PREMULTIPLY;
+ return PLANE_CTL_FORMAT_XRGB_8888;
case DRM_FORMAT_XRGB2101010:
return PLANE_CTL_FORMAT_XRGB_2101010;
case DRM_FORMAT_XBGR2101010:
@@ -3466,6 +3459,33 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
return 0;
}
+/*
+ * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
+ * to be already pre-multiplied. We need to add a knob (or a different
+ * DRM_FORMAT) for user-space to configure that.
+ */
+static u32 skl_plane_ctl_alpha(uint32_t pixel_format)
+{
+ switch (pixel_format) {
+ case DRM_FORMAT_ABGR8888:
+ case DRM_FORMAT_ARGB8888:
+ return PLANE_CTL_ALPHA_SW_PREMULTIPLY;
+ default:
+ return PLANE_CTL_ALPHA_DISABLE;
+ }
+}
+
+static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format)
+{
+ switch (pixel_format) {
+ case DRM_FORMAT_ABGR8888:
+ case DRM_FORMAT_ARGB8888:
+ return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
+ default:
+ return PLANE_COLOR_ALPHA_DISABLE;
+ }
+}
+
static u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
{
switch (fb_modifier) {
@@ -3522,7 +3542,8 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
plane_ctl = PLANE_CTL_ENABLE;
- if (!IS_GEMINILAKE(dev_priv) && !IS_CANNONLAKE(dev_priv)) {
+ if (INTEL_GEN(dev_priv) < 10 && !IS_GEMINILAKE(dev_priv)) {
+ plane_ctl |= skl_plane_ctl_alpha(fb->format->format);
plane_ctl |=
PLANE_CTL_PIPE_GAMMA_ENABLE |
PLANE_CTL_PIPE_CSC_ENABLE |
@@ -3541,6 +3562,20 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
return plane_ctl;
}
+u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state *plane_state)
+{
+ const struct drm_framebuffer *fb = plane_state->base.fb;
+ u32 plane_color_ctl = 0;
+
+ 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(fb->format->format);
+
+ return plane_color_ctl;
+}
+
static int
__intel_display_resume(struct drm_device *dev,
struct drm_atomic_state *state,
@@ -8426,7 +8461,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
- u32 val, base, offset, stride_mult, tiling;
+ u32 val, base, offset, stride_mult, tiling, alpha;
int pipe = crtc->pipe;
int fourcc, pixel_format;
unsigned int aligned_height;
@@ -8448,9 +8483,16 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
goto error;
pixel_format = val & PLANE_CTL_FORMAT_MASK;
+
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+ alpha = I915_READ(PLANE_COLOR_CTL(pipe, 0));
+ alpha &= PLANE_COLOR_ALPHA_MASK;
+ } else {
+ alpha = val & PLANE_CTL_ALPHA_MASK;
+ }
+
fourcc = skl_format_to_fourcc(pixel_format,
- val & PLANE_CTL_ORDER_RGBX,
- val & PLANE_CTL_ALPHA_MASK);
+ val & PLANE_CTL_ORDER_RGBX, alpha);
fb->format = drm_format_info(fourcc);
tiling = val & PLANE_CTL_TILED_MASK;
@@ -12831,6 +12873,9 @@ intel_check_primary_plane(struct intel_plane *plane,
state->ctl = i9xx_plane_ctl(crtc_state, state);
}
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+ state->color_ctl = glk_plane_color_ctl(crtc_state, state);
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 00b488688042..92bc1325701b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -425,6 +425,9 @@ struct intel_plane_state {
/* plane control register */
u32 ctl;
+ /* plane color control register */
+ u32 color_ctl;
+
/*
* scaler_id
* = -1 : not using a scaler
@@ -1504,6 +1507,8 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
return i915_ggtt_offset(state->vma);
}
+u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
+ const struct intel_plane_state *plane_state);
u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state);
u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 4fcf80ca91dd..ce615704982a 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -263,13 +263,9 @@ skl_update_plane(struct intel_plane *plane,
spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
- if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
- PLANE_COLOR_PIPE_GAMMA_ENABLE |
- PLANE_COLOR_PIPE_CSC_ENABLE |
- PLANE_COLOR_PLANE_GAMMA_DISABLE);
- }
-
+ plane_state->color_ctl);
if (key->flags) {
I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);
@@ -978,6 +974,9 @@ intel_check_sprite_plane(struct intel_plane *plane,
state->ctl = g4x_sprite_ctl(crtc_state, state);
}
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+ state->color_ctl = glk_plane_color_ctl(crtc_state, state);
+
return 0;
}
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev3)
2017-11-02 23:00 [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
` (5 preceding siblings ...)
2017-11-03 20:43 ` [PATCH v3] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
@ 2017-11-03 21:25 ` Patchwork
2017-11-06 22:15 ` Patchwork
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-11-03 21:25 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev3)
URL : https://patchwork.freedesktop.org/series/33087/
State : failure
== Summary ==
Series 33087v3 drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
https://patchwork.freedesktop.org/api/1.0/series/33087/revisions/3/mbox/
Test gem_basic:
Subgroup create-close:
dmesg-warn -> PASS (fi-cfl-s)
Subgroup create-fd-close:
dmesg-warn -> PASS (fi-cfl-s)
Test gem_exec_reloc:
Subgroup basic-cpu-active:
pass -> FAIL (fi-gdg-551) fdo#102582 +6
Subgroup basic-write-cpu-active:
pass -> FAIL (fi-gdg-551)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-c:
dmesg-warn -> PASS (fi-bsw-n3050)
Subgroup suspend-read-crc-pipe-a:
pass -> FAIL (fi-skl-6700k)
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:442s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:456s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:378s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:534s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:277s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:508s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:505s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:505s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:496s
fi-cfl-s total:289 pass:254 dwarn:3 dfail:0 fail:0 skip:32 time:557s
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:596s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:425s
fi-gdg-551 total:289 pass:170 dwarn:1 dfail:0 fail:9 skip:109 time:260s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:588s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:430s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:435s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:431s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:499s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:463s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:490s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:573s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:479s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:588s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:564s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:451s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:593s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:654s
fi-skl-6700k total:289 pass:264 dwarn:0 dfail:0 fail:1 skip:24 time:522s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:504s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:460s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:564s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:421s
8b0ae6b50a229dc661a02f4034252ee854cc9b83 drm-tip: 2017y-11m-03d-17h-15m-57s UTC integration manifest
d714b22fcb5b drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6953/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev3)
2017-11-02 23:00 [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
` (6 preceding siblings ...)
2017-11-03 21:25 ` ✗ Fi.CI.BAT: failure for drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev3) Patchwork
@ 2017-11-06 22:15 ` Patchwork
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-11-06 22:15 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev3)
URL : https://patchwork.freedesktop.org/series/33087/
State : failure
== Summary ==
Series 33087v3 drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
https://patchwork.freedesktop.org/api/1.0/series/33087/revisions/3/mbox/
Test gem_sync:
Subgroup basic-all:
pass -> SKIP (fi-pnv-d510)
Subgroup basic-each:
pass -> SKIP (fi-pnv-d510)
Subgroup basic-many-each:
pass -> SKIP (fi-pnv-d510)
Subgroup basic-store-all:
pass -> SKIP (fi-pnv-d510)
Subgroup basic-store-each:
pass -> SKIP (fi-pnv-d510)
Test gem_tiled_blits:
Subgroup basic:
pass -> SKIP (fi-pnv-d510)
Test gem_tiled_fence_blits:
Subgroup basic:
pass -> SKIP (fi-pnv-d510)
Test gem_wait:
Subgroup basic-busy-all:
pass -> SKIP (fi-pnv-d510)
pass -> FAIL (fi-glk-dsi)
Subgroup basic-wait-all:
pass -> SKIP (fi-pnv-d510)
Subgroup basic-await-all:
pass -> SKIP (fi-pnv-d510)
Test kms_busy:
Subgroup basic-flip-a:
pass -> SKIP (fi-pnv-d510)
Subgroup basic-flip-b:
pass -> SKIP (fi-pnv-d510)
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
fail -> PASS (fi-gdg-551) fdo#102618
pass -> SKIP (fi-pnv-d510)
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-c:
pass -> INCOMPLETE (fi-glk-1)
fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:440s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:456s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:380s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:543s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:276s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:499s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:505s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:506s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:486s
fi-cfl-s total:289 pass:254 dwarn:3 dfail:0 fail:0 skip:32 time:551s
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:600s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:428s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:263s
fi-glk-1 total:237 pass:210 dwarn:0 dfail:0 fail:0 skip:26
fi-glk-dsi total:289 pass:257 dwarn:0 dfail:0 fail:2 skip:30 time:506s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:433s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:431s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:424s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:492s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:458s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:575s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:479s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:577s
fi-pnv-d510 total:289 pass:209 dwarn:1 dfail:0 fail:0 skip:79 time:551s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:449s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:598s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:651s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:519s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:504s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:459s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:581s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:420s
4295e1469b2127d79d2d02ef34d065509bdded97 drm-tip: 2017y-11m-06d-15h-35m-57s UTC integration manifest
7dce92f9f142 drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6977/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
2017-11-03 20:43 ` [PATCH v3] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
@ 2017-11-10 19:20 ` Ville Syrjälä
0 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2017-11-10 19:20 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx, Paulo Zanoni
On Fri, Nov 03, 2017 at 01:43:54PM -0700, James Ausmus wrote:
> Since GLK, some plane configuration settings have moved to the
> PLANE_COLOR_CTL register. Refactor handling of the register to work like
> PLANE_CTL. This also allows us to fix the set/read of the plane Alpha
> Mode for GLK+.
>
> v2: Adjust ordering of platform checks to be newest->oldest, drop
> redundant comment about alpha blending. (Ville)
>
> v3: Move Alpha Mode bits out of skl_plane_ctl_format into
> skl_plane_ctl_alpha, and drop glk_plane_ctl_format, drop initialization
> of state->color_ctl on platforms that don't use it, and drop color_ctl
> local var. (Ville)
>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 12 ++++--
> drivers/gpu/drm/i915/intel_display.c | 71 +++++++++++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_drv.h | 5 +++
> drivers/gpu/drm/i915/intel_sprite.c | 11 +++---
> 4 files changed, 76 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f0f8f6059652..ecd6b236e005 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6263,7 +6263,7 @@ enum {
> #define _PLANE_CTL_2_A 0x70280
> #define _PLANE_CTL_3_A 0x70380
> #define PLANE_CTL_ENABLE (1 << 31)
> -#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30)
> +#define PLANE_CTL_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-GLK */
> #define PLANE_CTL_FORMAT_MASK (0xf << 24)
> #define PLANE_CTL_FORMAT_YUV422 ( 0 << 24)
> #define PLANE_CTL_FORMAT_NV12 ( 1 << 24)
> @@ -6273,7 +6273,7 @@ enum {
> #define PLANE_CTL_FORMAT_AYUV ( 8 << 24)
> #define PLANE_CTL_FORMAT_INDEXED ( 12 << 24)
> #define PLANE_CTL_FORMAT_RGB_565 ( 14 << 24)
> -#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23)
> +#define PLANE_CTL_PIPE_CSC_ENABLE (1 << 23) /* Pre-GLK */
> #define PLANE_CTL_KEY_ENABLE_MASK (0x3 << 21)
> #define PLANE_CTL_KEY_ENABLE_SOURCE ( 1 << 21)
> #define PLANE_CTL_KEY_ENABLE_DESTINATION ( 2 << 21)
> @@ -6286,13 +6286,13 @@ enum {
> #define PLANE_CTL_YUV422_VYUY ( 3 << 16)
> #define PLANE_CTL_DECOMPRESSION_ENABLE (1 << 15)
> #define PLANE_CTL_TRICKLE_FEED_DISABLE (1 << 14)
> -#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13)
> +#define PLANE_CTL_PLANE_GAMMA_DISABLE (1 << 13) /* Pre-GLK */
> #define PLANE_CTL_TILED_MASK (0x7 << 10)
> #define PLANE_CTL_TILED_LINEAR ( 0 << 10)
> #define PLANE_CTL_TILED_X ( 1 << 10)
> #define PLANE_CTL_TILED_Y ( 4 << 10)
> #define PLANE_CTL_TILED_YF ( 5 << 10)
> -#define PLANE_CTL_ALPHA_MASK (0x3 << 4)
> +#define PLANE_CTL_ALPHA_MASK (0x3 << 4) /* Pre-GLK */
> #define PLANE_CTL_ALPHA_DISABLE ( 0 << 4)
> #define PLANE_CTL_ALPHA_SW_PREMULTIPLY ( 2 << 4)
> #define PLANE_CTL_ALPHA_HW_PREMULTIPLY ( 3 << 4)
> @@ -6332,6 +6332,10 @@ enum {
> #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30)
> #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23)
> #define PLANE_COLOR_PLANE_GAMMA_DISABLE (1 << 13)
> +#define PLANE_COLOR_ALPHA_MASK (0x3 << 4)
> +#define PLANE_COLOR_ALPHA_DISABLE (0 << 4)
> +#define PLANE_COLOR_ALPHA_SW_PREMULTIPLY (2 << 4)
> +#define PLANE_COLOR_ALPHA_HW_PREMULTIPLY (3 << 4)
> #define _PLANE_BUF_CFG_1_A 0x7027c
> #define _PLANE_BUF_CFG_2_A 0x7037c
> #define _PLANE_NV12_BUF_CFG_1_A 0x70278
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 737de251d0f8..214c0c119002 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3436,17 +3436,10 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
> return PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX;
> case DRM_FORMAT_XRGB8888:
> return PLANE_CTL_FORMAT_XRGB_8888;
> - /*
> - * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
> - * to be already pre-multiplied. We need to add a knob (or a different
> - * DRM_FORMAT) for user-space to configure that.
> - */
> case DRM_FORMAT_ABGR8888:
> - return PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX |
> - PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> + return PLANE_CTL_FORMAT_XRGB_8888 | PLANE_CTL_ORDER_RGBX;
> case DRM_FORMAT_ARGB8888:
> - return PLANE_CTL_FORMAT_XRGB_8888 |
> - PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> + return PLANE_CTL_FORMAT_XRGB_8888;
I believe some of the cases are now returning identical values. Could
collapse those together.
Otherwise lgtm so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> case DRM_FORMAT_XRGB2101010:
> return PLANE_CTL_FORMAT_XRGB_2101010;
> case DRM_FORMAT_XBGR2101010:
> @@ -3466,6 +3459,33 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
> return 0;
> }
>
> +/*
> + * XXX: For ARBG/ABGR formats we default to expecting scanout buffers
> + * to be already pre-multiplied. We need to add a knob (or a different
> + * DRM_FORMAT) for user-space to configure that.
> + */
> +static u32 skl_plane_ctl_alpha(uint32_t pixel_format)
> +{
> + switch (pixel_format) {
> + case DRM_FORMAT_ABGR8888:
> + case DRM_FORMAT_ARGB8888:
> + return PLANE_CTL_ALPHA_SW_PREMULTIPLY;
> + default:
> + return PLANE_CTL_ALPHA_DISABLE;
> + }
> +}
> +
> +static u32 glk_plane_color_ctl_alpha(uint32_t pixel_format)
> +{
> + switch (pixel_format) {
> + case DRM_FORMAT_ABGR8888:
> + case DRM_FORMAT_ARGB8888:
> + return PLANE_COLOR_ALPHA_SW_PREMULTIPLY;
> + default:
> + return PLANE_COLOR_ALPHA_DISABLE;
> + }
> +}
> +
> static u32 skl_plane_ctl_tiling(uint64_t fb_modifier)
> {
> switch (fb_modifier) {
> @@ -3522,7 +3542,8 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
>
> plane_ctl = PLANE_CTL_ENABLE;
>
> - if (!IS_GEMINILAKE(dev_priv) && !IS_CANNONLAKE(dev_priv)) {
> + if (INTEL_GEN(dev_priv) < 10 && !IS_GEMINILAKE(dev_priv)) {
> + plane_ctl |= skl_plane_ctl_alpha(fb->format->format);
> plane_ctl |=
> PLANE_CTL_PIPE_GAMMA_ENABLE |
> PLANE_CTL_PIPE_CSC_ENABLE |
> @@ -3541,6 +3562,20 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> return plane_ctl;
> }
>
> +u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> + const struct intel_plane_state *plane_state)
> +{
> + const struct drm_framebuffer *fb = plane_state->base.fb;
> + u32 plane_color_ctl = 0;
> +
> + 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(fb->format->format);
> +
> + return plane_color_ctl;
> +}
> +
> static int
> __intel_display_resume(struct drm_device *dev,
> struct drm_atomic_state *state,
> @@ -8426,7 +8461,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - u32 val, base, offset, stride_mult, tiling;
> + u32 val, base, offset, stride_mult, tiling, alpha;
> int pipe = crtc->pipe;
> int fourcc, pixel_format;
> unsigned int aligned_height;
> @@ -8448,9 +8483,16 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> goto error;
>
> pixel_format = val & PLANE_CTL_FORMAT_MASK;
> +
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> + alpha = I915_READ(PLANE_COLOR_CTL(pipe, 0));
> + alpha &= PLANE_COLOR_ALPHA_MASK;
> + } else {
> + alpha = val & PLANE_CTL_ALPHA_MASK;
> + }
> +
> fourcc = skl_format_to_fourcc(pixel_format,
> - val & PLANE_CTL_ORDER_RGBX,
> - val & PLANE_CTL_ALPHA_MASK);
> + val & PLANE_CTL_ORDER_RGBX, alpha);
> fb->format = drm_format_info(fourcc);
>
> tiling = val & PLANE_CTL_TILED_MASK;
> @@ -12831,6 +12873,9 @@ intel_check_primary_plane(struct intel_plane *plane,
> state->ctl = i9xx_plane_ctl(crtc_state, state);
> }
>
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> + state->color_ctl = glk_plane_color_ctl(crtc_state, state);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 00b488688042..92bc1325701b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -425,6 +425,9 @@ struct intel_plane_state {
> /* plane control register */
> u32 ctl;
>
> + /* plane color control register */
> + u32 color_ctl;
> +
> /*
> * scaler_id
> * = -1 : not using a scaler
> @@ -1504,6 +1507,8 @@ static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
> return i915_ggtt_offset(state->vma);
> }
>
> +u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
> + const struct intel_plane_state *plane_state);
> u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
> const struct intel_plane_state *plane_state);
> u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 4fcf80ca91dd..ce615704982a 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -263,13 +263,9 @@ skl_update_plane(struct intel_plane *plane,
>
> spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>
> - if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id),
> - PLANE_COLOR_PIPE_GAMMA_ENABLE |
> - PLANE_COLOR_PIPE_CSC_ENABLE |
> - PLANE_COLOR_PLANE_GAMMA_DISABLE);
> - }
> -
> + plane_state->color_ctl);
> if (key->flags) {
> I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value);
> I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), key->max_value);
> @@ -978,6 +974,9 @@ intel_check_sprite_plane(struct intel_plane *plane,
> state->ctl = g4x_sprite_ctl(crtc_state, state);
> }
>
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> + state->color_ctl = glk_plane_color_ctl(crtc_state, state);
> +
> return 0;
> }
>
> --
> 2.14.1
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-11-10 19:21 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-02 23:00 [PATCH] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
2017-11-02 23:52 ` ✗ Fi.CI.BAT: warning for " Patchwork
2017-11-03 10:12 ` [PATCH] " Ville Syrjälä
2017-11-03 17:01 ` James Ausmus
2017-11-03 16:58 ` [PATCH v2] " James Ausmus
2017-11-03 17:31 ` Ville Syrjälä
2017-11-03 17:45 ` James Ausmus
2017-11-03 17:59 ` Ville Syrjälä
2017-11-03 17:23 ` ✓ Fi.CI.BAT: success for drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev2) Patchwork
2017-11-03 18:25 ` ✗ Fi.CI.IGT: failure " Patchwork
2017-11-03 20:43 ` [PATCH v3] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ James Ausmus
2017-11-10 19:20 ` Ville Syrjälä
2017-11-03 21:25 ` ✗ Fi.CI.BAT: failure for drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+ (rev3) Patchwork
2017-11-06 22:15 ` 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.