Re: [Intel-gfx] [PATCH v2 2/5] drm/i915/glk: Plane color correction register changes
On Thu, 2017-01-26 at 13:32 +0200, Ville Syrjälä wrote:
> On Thu, Jan 26, 2017 at 01:24:22PM +0200, Ander Conselvan de Oliveira wrote:
> > In Geminilake, the bits for enabling pipe csc, pipe gamma and plane
> > gamma moved to a new register. So update the plane update functions
> > to set the right bits.
> >
> > Pipe CSC is kept disabled though, since enabling that also enables the
> > dedicated degamma table, and that is not properly programmed yet,
> > leading to a black screen.
> >
> > v2: Use plane_id. (Ville)
> > Remove unnecessary variable. (Ville)
> > Keep registers in offset order. (Ville)
> > Don't set plane gamma disable twice. (Ander)
> >
> > Cc: Ville Syrjälä
> > Signed-off-by: Ander Conselvan De Oliveira > l.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 19 ++-
> > drivers/gpu/drm/i915/intel_display.c | 16
> > drivers/gpu/drm/i915/intel_sprite.c | 17 -
> > 3 files changed, 42 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 8861683..9947354 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5869,11 +5869,18 @@ enum {
> > #define _PLANE_KEYMSK_2_A 0x70298
> > #define _PLANE_KEYMAX_1_A 0x701a0
> > #define _PLANE_KEYMAX_2_A 0x702a0
> > +#define _PLANE_COLOR_CTL_1_A 0x701CC
> > +#define _PLANE_COLOR_CTL_2_A 0x702CC
> > +#define _PLANE_COLOR_CTL_3_A 0x703CC
>
> Maybe add a /* GLK+ */ not to these?
>
> But even without this is
> Reviewed-by: Ville Syrjälä
Thanks. I added the comments while pushing.
Ander
>
> > +#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_BUF_CFG_1_A 0x7027c
> > #define _PLANE_BUF_CFG_2_A 0x7037c
> > #define _PLANE_NV12_BUF_CFG_1_A0x70278
> > #define _PLANE_NV12_BUF_CFG_2_A0x70378
> >
> > +
> > #define _PLANE_CTL_1_B 0x71180
> > #define _PLANE_CTL_2_B 0x71280
> > #define _PLANE_CTL_3_B 0x71380
> > @@ -5968,7 +5975,17 @@ enum {
> > #define PLANE_NV12_BUF_CFG(pipe, plane)\
> > _MMIO_PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe),
> > _PLANE_NV12_BUF_CFG_2(pipe))
> >
> > -/* SKL new cursor registers */
> > +#define _PLANE_COLOR_CTL_1_B 0x711CC
> > +#define _PLANE_COLOR_CTL_2_B 0x712CC
> > +#define _PLANE_COLOR_CTL_3_B 0x713CC
> > +#define _PLANE_COLOR_CTL_1(pipe) \
> > + _PIPE(pipe, _PLANE_COLOR_CTL_1_A, _PLANE_COLOR_CTL_1_B)
> > +#define _PLANE_COLOR_CTL_2(pipe) \
> > + _PIPE(pipe, _PLANE_COLOR_CTL_2_A, _PLANE_COLOR_CTL_2_B)
> > +#define PLANE_COLOR_CTL(pipe, plane) \
> > + _MMIO_PLANE(plane, _PLANE_COLOR_CTL_1(pipe),
> > _PLANE_COLOR_CTL_2(pipe))
> > +
> > +#/* SKL new cursor registers */
> > #define _CUR_BUF_CFG_A 0x7017c
> > #define _CUR_BUF_CFG_B 0x7117c
> > #define CUR_BUF_CFG(pipe) _MMIO_PIPE(pipe, _CUR_BUF_CFG_A,
> > _CUR_BUF_CFG_B)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 5a9da48..13d05bc 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3386,13 +3386,21 @@ static void skylake_update_primary_plane(struct
> > drm_plane *plane,
> > int dst_w = drm_rect_width(&plane_state->base.dst);
> > int dst_h = drm_rect_height(&plane_state->base.dst);
> >
> > - plane_ctl = PLANE_CTL_ENABLE |
> > - PLANE_CTL_PIPE_GAMMA_ENABLE |
> > - PLANE_CTL_PIPE_CSC_ENABLE;
> > + plane_ctl = PLANE_CTL_ENABLE;
> > +
> > + if (IS_GEMINILAKE(dev_priv)) {
> > + I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
> > + PLANE_COLOR_PIPE_GAMMA_ENABLE |
> > + PLANE_COLOR_PLANE_GAMMA_DISABLE);
> > + } 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_tiling(fb->modifier);
> > - plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
> > plane_ctl |= skl_plane_ctl_rotation(rotation);
> >
> > /* Sizes are 0 based */
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index c05545f..b16a295 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -219,14 +219,21 @@ skl_update_pl
Re: [Intel-gfx] [PATCH v2 2/5] drm/i915/glk: Plane color correction register changes
On Thu, Jan 26, 2017 at 01:24:22PM +0200, Ander Conselvan de Oliveira wrote:
> In Geminilake, the bits for enabling pipe csc, pipe gamma and plane
> gamma moved to a new register. So update the plane update functions
> to set the right bits.
>
> Pipe CSC is kept disabled though, since enabling that also enables the
> dedicated degamma table, and that is not properly programmed yet,
> leading to a black screen.
>
> v2: Use plane_id. (Ville)
> Remove unnecessary variable. (Ville)
> Keep registers in offset order. (Ville)
> Don't set plane gamma disable twice. (Ander)
>
> Cc: Ville Syrjälä
> Signed-off-by: Ander Conselvan De Oliveira
>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 19 ++-
> drivers/gpu/drm/i915/intel_display.c | 16
> drivers/gpu/drm/i915/intel_sprite.c | 17 -
> 3 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8861683..9947354 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5869,11 +5869,18 @@ enum {
> #define _PLANE_KEYMSK_2_A0x70298
> #define _PLANE_KEYMAX_1_A0x701a0
> #define _PLANE_KEYMAX_2_A0x702a0
> +#define _PLANE_COLOR_CTL_1_A 0x701CC
> +#define _PLANE_COLOR_CTL_2_A 0x702CC
> +#define _PLANE_COLOR_CTL_3_A 0x703CC
Maybe add a /* GLK+ */ not to these?
But even without this is
Reviewed-by: Ville Syrjälä
> +#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_BUF_CFG_1_A 0x7027c
> #define _PLANE_BUF_CFG_2_A 0x7037c
> #define _PLANE_NV12_BUF_CFG_1_A 0x70278
> #define _PLANE_NV12_BUF_CFG_2_A 0x70378
>
> +
> #define _PLANE_CTL_1_B 0x71180
> #define _PLANE_CTL_2_B 0x71280
> #define _PLANE_CTL_3_B 0x71380
> @@ -5968,7 +5975,17 @@ enum {
> #define PLANE_NV12_BUF_CFG(pipe, plane) \
> _MMIO_PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe),
> _PLANE_NV12_BUF_CFG_2(pipe))
>
> -/* SKL new cursor registers */
> +#define _PLANE_COLOR_CTL_1_B 0x711CC
> +#define _PLANE_COLOR_CTL_2_B 0x712CC
> +#define _PLANE_COLOR_CTL_3_B 0x713CC
> +#define _PLANE_COLOR_CTL_1(pipe) \
> + _PIPE(pipe, _PLANE_COLOR_CTL_1_A, _PLANE_COLOR_CTL_1_B)
> +#define _PLANE_COLOR_CTL_2(pipe) \
> + _PIPE(pipe, _PLANE_COLOR_CTL_2_A, _PLANE_COLOR_CTL_2_B)
> +#define PLANE_COLOR_CTL(pipe, plane) \
> + _MMIO_PLANE(plane, _PLANE_COLOR_CTL_1(pipe), _PLANE_COLOR_CTL_2(pipe))
> +
> +#/* SKL new cursor registers */
> #define _CUR_BUF_CFG_A 0x7017c
> #define _CUR_BUF_CFG_B 0x7117c
> #define CUR_BUF_CFG(pipe)_MMIO_PIPE(pipe, _CUR_BUF_CFG_A, _CUR_BUF_CFG_B)
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 5a9da48..13d05bc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3386,13 +3386,21 @@ static void skylake_update_primary_plane(struct
> drm_plane *plane,
> int dst_w = drm_rect_width(&plane_state->base.dst);
> int dst_h = drm_rect_height(&plane_state->base.dst);
>
> - plane_ctl = PLANE_CTL_ENABLE |
> - PLANE_CTL_PIPE_GAMMA_ENABLE |
> - PLANE_CTL_PIPE_CSC_ENABLE;
> + plane_ctl = PLANE_CTL_ENABLE;
> +
> + if (IS_GEMINILAKE(dev_priv)) {
> + I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
> +PLANE_COLOR_PIPE_GAMMA_ENABLE |
> +PLANE_COLOR_PLANE_GAMMA_DISABLE);
> + } 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_tiling(fb->modifier);
> - plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
> plane_ctl |= skl_plane_ctl_rotation(rotation);
>
> /* Sizes are 0 based */
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index c05545f..b16a295 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -219,14 +219,21 @@ skl_update_plane(struct drm_plane *drm_plane,
> uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
> uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
>
> - plane_ctl = PLANE_CTL_ENABLE |
> - PLANE_CTL_PIPE_GAMMA_ENABLE |
> - PLANE_CTL_PIPE
