| Message ID | 20171103165848.23407-1-james.ausmus@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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.
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; }
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(-)