On Mon, Nov 13, 2017 at 10:11:28AM -0800, 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)
> 
> v4: Consolidate skl_plane_ctl_format switch statement on formats that
> return the same settings. (Ville)
> 
> Signed-off-by: James Ausmus <james.aus...@intel.com>
> Cc: Paulo Zanoni <paulo.r.zan...@intel.com>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
> 
> NOTE: Resending without the --in-reply-to, as it seems I managed to
> confuse the CI system, so v4 didn't get properly picked up for testing

Passed now, so pushed to dinq. Thanks for the patch.

> 
>  drivers/gpu/drm/i915/i915_reg.h      | 12 ++++--
>  drivers/gpu/drm/i915/intel_display.c | 73 
> ++++++++++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_drv.h     |  5 +++
>  drivers/gpu/drm/i915/intel_sprite.c  | 11 +++---
>  4 files changed, 76 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 05e33a41fcc7..222aea49c06a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6295,7 +6295,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)
> @@ -6305,7 +6305,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)
> @@ -6318,13 +6318,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)
> @@ -6364,6 +6364,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 0ebf3f283b87..ed6a4a8d9273 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3432,20 +3432,11 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
>       case DRM_FORMAT_RGB565:
>               return PLANE_CTL_FORMAT_RGB_565;
>       case DRM_FORMAT_XBGR8888:
> +     case DRM_FORMAT_ABGR8888:
>               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;
>       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:
> @@ -3465,6 +3456,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) {
> @@ -3521,7 +3539,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 |
> @@ -3540,6 +3559,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;
> @@ -12853,6 +12893,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 10bec8796239..e9b66e0cb647 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
> @@ -1503,6 +1506,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

Reply via email to