diff mbox

[v2,2/5] drm/i915/glk: Plane color correction register changes

Message ID 1485429865-10687-3-git-send-email-ander.conselvan.de.oliveira@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ander Conselvan de Oliveira Jan. 26, 2017, 11:24 a.m. UTC 
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ä <ville.syrjala@linux.intel.com>
Signed-off-by: Ander Conselvan De Oliveira <ander.conselvan.de.oliveira@intel.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(-)

Comments

Ville Syrjälä Jan. 26, 2017, 11:32 a.m. UTC | #1 
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ä <ville.syrjala@linux.intel.com>
> Signed-off-by: Ander Conselvan De Oliveira <ander.conselvan.de.oliveira@intel.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ä <ville.syrjala@linux.intel.com>

> +#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_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);
>  
>  	if (key->flags) {
> -- 
> 2.5.5
Ander Conselvan de Oliveira Jan. 30, 2017, 8:38 a.m. UTC | #2 
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ä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Ander Conselvan De Oliveira <ander.conselvan.de.oliveira@inte
> > 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ä <ville.syrjala@linux.intel.com>

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_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_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);
> >  
> >  	if (key->flags) {
> > -- 
> > 2.5.5
> 
>
diff mbox

Patch

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
+#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_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);
 
 	if (key->flags) {