[Intel-gfx] [PATCH v3] drm/i915/glk: Refactor handling of PLANE_COLOR_CTL for GLK+
Ville Syrjälä
ville.syrjala at linux.intel.com
Fri Nov 10 19:20:56 UTC 2017
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 at intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at 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 at 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
More information about the Intel-gfx
mailing list