[PATCH v2 0/5] Geminilake pipe CSC

[PATCH v2 0/5] Geminilake pipe CSC

[Ander Conselvan de Oliveira]

Ander Conselvan de Oliveira (5):
  drm/i915: Disable plane gamma in SKL+ sprite planes
  drm/i915/glk: Plane color correction register changes
  drm/i915: Split broadwell_load_luts() into smaller functions
  drm/i915/glk: Program pipe gamma and degamma tables
  drm/i915/glk: Enable pipe CSC

 drivers/gpu/drm/i915/i915_pci.c      |   1 +
 drivers/gpu/drm/i915/i915_reg.h      |  34 +++++++++++-
 drivers/gpu/drm/i915/intel_color.c   | 101 +++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_display.c |  17 ++++--
 drivers/gpu/drm/i915/intel_sprite.c  |  17 ++++--
 5 files changed, 150 insertions(+), 20 deletions(-)

-- 
2.5.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/5] drm/i915: Disable plane gamma in SKL+ sprite planes

[Ander Conselvan de Oliveira]

The plane gamma tables are never programmed, so just disable it, like it
is done for the primary plane.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9ef5468..c05545f 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -226,6 +226,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 	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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ander Conselvan de Oliveira]

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
+#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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ander Conselvan De Oliveira]

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
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/5] drm/i915: Split broadwell_load_luts() into smaller functions

[Ander Conselvan de Oliveira]

Split the logic for progamming each LUT out of broadwell_load_luts(), so
we can reuse part of it for geminilake.

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    |  1 +
 drivers/gpu/drm/i915/intel_color.c | 43 ++++++++++++++++++++++++++++----------
 2 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9947354..06bbe55 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8171,6 +8171,7 @@ enum {
 #define   PAL_PREC_10_12_BIT		(0 << 31)
 #define   PAL_PREC_SPLIT_MODE		(1 << 31)
 #define   PAL_PREC_AUTO_INCREMENT	(1 << 15)
+#define   PAL_PREC_INDEX_VALUE_MASK	(0x3ff << 0)
 #define _PAL_PREC_DATA_A	0x4A404
 #define _PAL_PREC_DATA_B	0x4AC04
 #define _PAL_PREC_DATA_C	0x4B404
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 34952d0..82a3bc9 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -340,20 +340,12 @@ static void haswell_load_luts(struct drm_crtc_state *crtc_state)
 		hsw_enable_ips(intel_crtc);
 }
 
-/* Loads the palette/gamma unit for the CRTC on Broadwell+. */
-static void broadwell_load_luts(struct drm_crtc_state *state)
+static void bdw_load_degamma_lut(struct drm_crtc_state *state)
 {
-	struct drm_crtc *crtc = state->crtc;
-	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
-	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
-	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
+	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
 	uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
 
-	if (crtc_state_is_legacy(state)) {
-		haswell_load_luts(state);
-		return;
-	}
-
 	I915_WRITE(PREC_PAL_INDEX(pipe),
 		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
 
@@ -377,6 +369,18 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
 				   (v << 20) | (v << 10) | v);
 		}
 	}
+}
+
+static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
+	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
+	uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+
+	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
+
+	I915_WRITE(PREC_PAL_INDEX(pipe),
+		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | offset);
 
 	if (state->gamma_lut) {
 		struct drm_color_lut *lut =
@@ -410,6 +414,23 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
 		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1);
 		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1);
 	}
+}
+
+/* Loads the palette/gamma unit for the CRTC on Broadwell+. */
+static void broadwell_load_luts(struct drm_crtc_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
+	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
+	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
+
+	if (crtc_state_is_legacy(state)) {
+		haswell_load_luts(state);
+		return;
+	}
+
+	bdw_load_degamma_lut(state);
+	bdw_load_gamma_lut(state,
+			   INTEL_INFO(dev_priv)->color.degamma_lut_size);
 
 	intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
 	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
-- 
2.5.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/5] drm/i915: Split broadwell_load_luts() into smaller functions

[Ville Syrjälä]

On Thu, Jan 26, 2017 at 01:24:23PM +0200, Ander Conselvan de Oliveira wrote:
> Split the logic for progamming each LUT out of broadwell_load_luts(), so
> we can reuse part of it for geminilake.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  1 +
>  drivers/gpu/drm/i915/intel_color.c | 43 ++++++++++++++++++++++++++++----------
>  2 files changed, 33 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9947354..06bbe55 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8171,6 +8171,7 @@ enum {
>  #define   PAL_PREC_10_12_BIT		(0 << 31)
>  #define   PAL_PREC_SPLIT_MODE		(1 << 31)
>  #define   PAL_PREC_AUTO_INCREMENT	(1 << 15)
> +#define   PAL_PREC_INDEX_VALUE_MASK	(0x3ff << 0)
>  #define _PAL_PREC_DATA_A	0x4A404
>  #define _PAL_PREC_DATA_B	0x4AC04
>  #define _PAL_PREC_DATA_C	0x4B404
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 34952d0..82a3bc9 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -340,20 +340,12 @@ static void haswell_load_luts(struct drm_crtc_state *crtc_state)
>  		hsw_enable_ips(intel_crtc);
>  }
>  
> -/* Loads the palette/gamma unit for the CRTC on Broadwell+. */
> -static void broadwell_load_luts(struct drm_crtc_state *state)
> +static void bdw_load_degamma_lut(struct drm_crtc_state *state)
>  {
> -	struct drm_crtc *crtc = state->crtc;
> -	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> -	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
> -	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
> +	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
>  	uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
>  
> -	if (crtc_state_is_legacy(state)) {
> -		haswell_load_luts(state);
> -		return;
> -	}
> -
>  	I915_WRITE(PREC_PAL_INDEX(pipe),
>  		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
>  
> @@ -377,6 +369,18 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
>  				   (v << 20) | (v << 10) | v);
>  		}
>  	}
> +}
> +
> +static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
> +	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
> +	uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> +
> +	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
> +
> +	I915_WRITE(PREC_PAL_INDEX(pipe),
> +		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | offset);
>  
>  	if (state->gamma_lut) {
>  		struct drm_color_lut *lut =
> @@ -410,6 +414,23 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
>  		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1);
>  		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1);
>  	}
> +}
> +
> +/* Loads the palette/gamma unit for the CRTC on Broadwell+. */
> +static void broadwell_load_luts(struct drm_crtc_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
> +	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
> +	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
> +
> +	if (crtc_state_is_legacy(state)) {
> +		haswell_load_luts(state);
> +		return;
> +	}
> +
> +	bdw_load_degamma_lut(state);
> +	bdw_load_gamma_lut(state,
> +			   INTEL_INFO(dev_priv)->color.degamma_lut_size);
>  
>  	intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
>  	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
> -- 
> 2.5.5

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 4/5] drm/i915/glk: Program pipe gamma and degamma tables

[Ander Conselvan de Oliveira]

The gamma tables in Geminilake were changed. There is no split-gamma
mode. Instead, there is a dedicated degamma table that is enabled
whenever pipe CSC is enabled.

The dedicated gamma table has 16 bit precision but doesn't support
separate channels. Since that doesn't match the per-channel format of
the degamma LUT property, for now only a linear table is loaded and the
property ignored.

v2: Remove empty line. (Ville)
    Reuse broadwell code. (Ville)

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_pci.c    |  1 +
 drivers/gpu/drm/i915/i915_reg.h    | 14 +++++++++
 drivers/gpu/drm/i915/intel_color.c | 60 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index ecb487b..df2051b 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -403,6 +403,7 @@ static const struct intel_device_info intel_geminilake_info = {
 	.platform = INTEL_GEMINILAKE,
 	.is_alpha_support = 1,
 	.ddb_size = 1024,
+	.color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
 };
 
 static const struct intel_device_info intel_kabylake_info = {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 06bbe55..e029691 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8181,12 +8181,26 @@ enum {
 #define _PAL_PREC_EXT_GC_MAX_A	0x4A420
 #define _PAL_PREC_EXT_GC_MAX_B	0x4AC20
 #define _PAL_PREC_EXT_GC_MAX_C	0x4B420
+#define _PAL_PREC_EXT2_GC_MAX_A	0x4A430
+#define _PAL_PREC_EXT2_GC_MAX_B	0x4AC30
+#define _PAL_PREC_EXT2_GC_MAX_C	0x4B430
 
 #define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B)
 #define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B)
 #define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4)
 #define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4)
 
+#define _PRE_CSC_GAMC_INDEX_A	0x4A484
+#define _PRE_CSC_GAMC_INDEX_B	0x4AC84
+#define _PRE_CSC_GAMC_INDEX_C	0x4B484
+#define   PRE_CSC_GAMC_AUTO_INCREMENT	(1 << 10)
+#define _PRE_CSC_GAMC_DATA_A	0x4A488
+#define _PRE_CSC_GAMC_DATA_B	0x4AC88
+#define _PRE_CSC_GAMC_DATA_C	0x4B488
+
+#define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
+#define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
+
 /* pipe CSC & degamma/gamma LUTs on CHV */
 #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
 #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 82a3bc9..2125aa3 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -380,7 +380,9 @@ static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset)
 	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
 
 	I915_WRITE(PREC_PAL_INDEX(pipe),
-		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | offset);
+		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
+		   PAL_PREC_AUTO_INCREMENT |
+		   offset);
 
 	if (state->gamma_lut) {
 		struct drm_color_lut *lut =
@@ -443,6 +445,59 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
 	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
 }
 
+static void glk_load_degamma_lut(struct drm_crtc_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
+	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
+	const uint32_t lut_size = 33;
+	uint32_t i;
+
+	/*
+	 * When setting the auto-increment bit, the hardware seems to
+	 * ignore the index bits, so we need to reset it to index 0
+	 * separately.
+	 */
+	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
+	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
+
+	/*
+	 *  FIXME: The pipe degamma table in geminilake doesn't support
+	 *  different values per channel, so this just loads a linear table.
+	 */
+	for (i = 0; i < lut_size; i++) {
+		uint32_t v = (i * ((1 << 16) - 1)) / (lut_size - 1);
+
+		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
+	}
+
+	/* Clamp values > 1.0. */
+	while (i++ < 35)
+		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16) - 1);
+}
+
+static void glk_load_luts(struct drm_crtc_state *state)
+{
+	struct drm_crtc *crtc = state->crtc;
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+
+	if (crtc_state_is_legacy(state)) {
+		haswell_load_luts(state);
+		return;
+	}
+
+	glk_load_degamma_lut(state);
+	bdw_load_gamma_lut(state, 0);
+
+	intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
+	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
+	POSTING_READ(GAMMA_MODE(pipe));
+
+	I915_WRITE(PIPE_CSC_MODE(pipe), 0);
+}
+
 /* Loads the palette/gamma unit for the CRTC on CherryView. */
 static void cherryview_load_luts(struct drm_crtc_state *state)
 {
@@ -561,6 +616,9 @@ void intel_color_init(struct drm_crtc *crtc)
 		   IS_BROXTON(dev_priv)) {
 		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
 		dev_priv->display.load_luts = broadwell_load_luts;
+	} else if (IS_GEMINILAKE(dev_priv)) {
+		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
+		dev_priv->display.load_luts = glk_load_luts;
 	} else {
 		dev_priv->display.load_luts = i9xx_load_luts;
 	}
-- 
2.5.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 4/5] drm/i915/glk: Program pipe gamma and degamma tables

[Ville Syrjälä]

On Thu, Jan 26, 2017 at 01:24:24PM +0200, Ander Conselvan de Oliveira wrote:
> The gamma tables in Geminilake were changed. There is no split-gamma
> mode. Instead, there is a dedicated degamma table that is enabled
> whenever pipe CSC is enabled.
> 
> The dedicated gamma table has 16 bit precision but doesn't support
> separate channels. Since that doesn't match the per-channel format of
> the degamma LUT property, for now only a linear table is loaded and the
> property ignored.
> 
> v2: Remove empty line. (Ville)
>     Reuse broadwell code. (Ville)
> 
> 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_pci.c    |  1 +
>  drivers/gpu/drm/i915/i915_reg.h    | 14 +++++++++
>  drivers/gpu/drm/i915/intel_color.c | 60 +++++++++++++++++++++++++++++++++++++-
>  3 files changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index ecb487b..df2051b 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -403,6 +403,7 @@ static const struct intel_device_info intel_geminilake_info = {
>  	.platform = INTEL_GEMINILAKE,
>  	.is_alpha_support = 1,
>  	.ddb_size = 1024,
> +	.color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
>  };
>  
>  static const struct intel_device_info intel_kabylake_info = {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 06bbe55..e029691 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8181,12 +8181,26 @@ enum {
>  #define _PAL_PREC_EXT_GC_MAX_A	0x4A420
>  #define _PAL_PREC_EXT_GC_MAX_B	0x4AC20
>  #define _PAL_PREC_EXT_GC_MAX_C	0x4B420
> +#define _PAL_PREC_EXT2_GC_MAX_A	0x4A430
> +#define _PAL_PREC_EXT2_GC_MAX_B	0x4AC30
> +#define _PAL_PREC_EXT2_GC_MAX_C	0x4B430
>  
>  #define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B)
>  #define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B)
>  #define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4)
>  #define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4)
>  
> +#define _PRE_CSC_GAMC_INDEX_A	0x4A484
> +#define _PRE_CSC_GAMC_INDEX_B	0x4AC84
> +#define _PRE_CSC_GAMC_INDEX_C	0x4B484
> +#define   PRE_CSC_GAMC_AUTO_INCREMENT	(1 << 10)
> +#define _PRE_CSC_GAMC_DATA_A	0x4A488
> +#define _PRE_CSC_GAMC_DATA_B	0x4AC88
> +#define _PRE_CSC_GAMC_DATA_C	0x4B488
> +
> +#define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
> +#define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
> +
>  /* pipe CSC & degamma/gamma LUTs on CHV */
>  #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
>  #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 82a3bc9..2125aa3 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -380,7 +380,9 @@ static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset)
>  	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
>  
>  	I915_WRITE(PREC_PAL_INDEX(pipe),
> -		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | offset);
> +		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
> +		   PAL_PREC_AUTO_INCREMENT |
> +		   offset);

This confused me for a bit. I was thinking we're using this to write the
deamma part for the split gamma case as well, which would end up
disabling the split gamma mode when doing that. But that's not actually
what's happening since you had another function to write the degamma
half.

>  
>  	if (state->gamma_lut) {
>  		struct drm_color_lut *lut =
> @@ -443,6 +445,59 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
>  	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>  }
>  
> +static void glk_load_degamma_lut(struct drm_crtc_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
> +	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
> +	const uint32_t lut_size = 33;
> +	uint32_t i;
> +
> +	/*
> +	 * When setting the auto-increment bit, the hardware seems to
> +	 * ignore the index bits, so we need to reset it to index 0
> +	 * separately.
> +	 */

Interesting. Do we know if the same problem might be present in other
gamma tables?

> +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
> +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
> +
> +	/*
> +	 *  FIXME: The pipe degamma table in geminilake doesn't support
> +	 *  different values per channel, so this just loads a linear table.
> +	 */
> +	for (i = 0; i < lut_size; i++) {
> +		uint32_t v = (i * ((1 << 16) - 1)) / (lut_size - 1);
> +
> +		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
> +	}
> +
> +	/* Clamp values > 1.0. */
> +	while (i++ < 35)
> +		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16) - 1);
> +}
> +
> +static void glk_load_luts(struct drm_crtc_state *state)
> +{
> +	struct drm_crtc *crtc = state->crtc;
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +
> +	if (crtc_state_is_legacy(state)) {
> +		haswell_load_luts(state);
> +		return;
> +	}
> +
> +	glk_load_degamma_lut(state);
> +	bdw_load_gamma_lut(state, 0);
> +
> +	intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
> +	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
> +	POSTING_READ(GAMMA_MODE(pipe));
> +
> +	I915_WRITE(PIPE_CSC_MODE(pipe), 0);

Why are we writing the CSC_MODE register here?

> +}
> +
>  /* Loads the palette/gamma unit for the CRTC on CherryView. */
>  static void cherryview_load_luts(struct drm_crtc_state *state)
>  {
> @@ -561,6 +616,9 @@ void intel_color_init(struct drm_crtc *crtc)
>  		   IS_BROXTON(dev_priv)) {
>  		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
>  		dev_priv->display.load_luts = broadwell_load_luts;
> +	} else if (IS_GEMINILAKE(dev_priv)) {
> +		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
> +		dev_priv->display.load_luts = glk_load_luts;
>  	} else {
>  		dev_priv->display.load_luts = i9xx_load_luts;
>  	}
> -- 
> 2.5.5

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 4/5] drm/i915/glk: Program pipe gamma and degamma tables

[Conselvan De Oliveira, Ander]

On Thu, 2017-01-26 at 16:21 +0200, Ville Syrjälä wrote:
> On Thu, Jan 26, 2017 at 01:24:24PM +0200, Ander Conselvan de Oliveira wrote:
> > The gamma tables in Geminilake were changed. There is no split-gamma
> > mode. Instead, there is a dedicated degamma table that is enabled
> > whenever pipe CSC is enabled.
> > 
> > The dedicated gamma table has 16 bit precision but doesn't support
> > separate channels. Since that doesn't match the per-channel format of
> > the degamma LUT property, for now only a linear table is loaded and the
> > property ignored.
> > 
> > v2: Remove empty line. (Ville)
> >     Reuse broadwell code. (Ville)
> > 
> > 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_pci.c    |  1 +
> >  drivers/gpu/drm/i915/i915_reg.h    | 14 +++++++++
> >  drivers/gpu/drm/i915/intel_color.c | 60
> > +++++++++++++++++++++++++++++++++++++-
> >  3 files changed, 74 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > b/drivers/gpu/drm/i915/i915_pci.c
> > index ecb487b..df2051b 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -403,6 +403,7 @@ static const struct intel_device_info
> > intel_geminilake_info = {
> >  	.platform = INTEL_GEMINILAKE,
> >  	.is_alpha_support = 1,
> >  	.ddb_size = 1024,
> > +	.color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
> >  };
> >  
> >  static const struct intel_device_info intel_kabylake_info = {
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 06bbe55..e029691 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -8181,12 +8181,26 @@ enum {
> >  #define _PAL_PREC_EXT_GC_MAX_A	0x4A420
> >  #define _PAL_PREC_EXT_GC_MAX_B	0x4AC20
> >  #define _PAL_PREC_EXT_GC_MAX_C	0x4B420
> > +#define _PAL_PREC_EXT2_GC_MAX_A	0x4A430
> > +#define _PAL_PREC_EXT2_GC_MAX_B	0x4AC30
> > +#define _PAL_PREC_EXT2_GC_MAX_C	0x4B430
> >  
> >  #define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe,
> > _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B)
> >  #define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe,
> > _PAL_PREC_DATA_A, _PAL_PREC_DATA_B)
> >  #define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe,
> > _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4)
> >  #define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe,
> > _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4)
> >  
> > +#define _PRE_CSC_GAMC_INDEX_A	0x4A484
> > +#define _PRE_CSC_GAMC_INDEX_B	0x4AC84
> > +#define _PRE_CSC_GAMC_INDEX_C	0x4B484
> > +#define   PRE_CSC_GAMC_AUTO_INCREMENT	(1 << 10)
> > +#define _PRE_CSC_GAMC_DATA_A	0x4A488
> > +#define _PRE_CSC_GAMC_DATA_B	0x4AC88
> > +#define _PRE_CSC_GAMC_DATA_C	0x4B488
> > +
> > +#define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe,
> > _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
> > +#define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe,
> > _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
> > +
> >  /* pipe CSC & degamma/gamma LUTs on CHV */
> >  #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
> >  #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
> > diff --git a/drivers/gpu/drm/i915/intel_color.c
> > b/drivers/gpu/drm/i915/intel_color.c
> > index 82a3bc9..2125aa3 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -380,7 +380,9 @@ static void bdw_load_gamma_lut(struct drm_crtc_state
> > *state, u32 offset)
> >  	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
> >  
> >  	I915_WRITE(PREC_PAL_INDEX(pipe),
> > -		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | offset);
> > +		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
> > +		   PAL_PREC_AUTO_INCREMENT |
> > +		   offset);
> 
> This confused me for a bit. I was thinking we're using this to write the
> deamma part for the split gamma case as well, which would end up
> disabling the split gamma mode when doing that. But that's not actually
> what's happening since you had another function to write the degamma
> half.
> 
> >  
> >  	if (state->gamma_lut) {
> >  		struct drm_color_lut *lut =
> > @@ -443,6 +445,59 @@ static void broadwell_load_luts(struct drm_crtc_state
> > *state)
> >  	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> >  }
> >  
> > +static void glk_load_degamma_lut(struct drm_crtc_state *state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
> > +	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
> > +	const uint32_t lut_size = 33;
> > +	uint32_t i;
> > +
> > +	/*
> > +	 * When setting the auto-increment bit, the hardware seems to
> > +	 * ignore the index bits, so we need to reset it to index 0
> > +	 * separately.
> > +	 */
> 
> Interesting. Do we know if the same problem might be present in other
> gamma tables?

I haven't tested, but might be the case for the plane gamma tables, since they
are very similar.

> > +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
> > +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
> > +
> > +	/*
> > +	 *  FIXME: The pipe degamma table in geminilake doesn't support
> > +	 *  different values per channel, so this just loads a linear
> > table.
> > +	 */
> > +	for (i = 0; i < lut_size; i++) {
> > +		uint32_t v = (i * ((1 << 16) - 1)) / (lut_size - 1);
> > +
> > +		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
> > +	}
> > +
> > +	/* Clamp values > 1.0. */
> > +	while (i++ < 35)
> > +		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16) - 1);
> > +}
> > +
> > +static void glk_load_luts(struct drm_crtc_state *state)
> > +{
> > +	struct drm_crtc *crtc = state->crtc;
> > +	struct drm_device *dev = crtc->dev;
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > +	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
> > +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> > +
> > +	if (crtc_state_is_legacy(state)) {
> > +		haswell_load_luts(state);
> > +		return;
> > +	}
> > +
> > +	glk_load_degamma_lut(state);
> > +	bdw_load_gamma_lut(state, 0);
> > +
> > +	intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
> > +	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
> > +	POSTING_READ(GAMMA_MODE(pipe));
> > +
> > +	I915_WRITE(PIPE_CSC_MODE(pipe), 0);
> 
> Why are we writing the CSC_MODE register here?

Because I can't read the spec. Somehow I got confused about the CSC position
relative to gamma in previous platforms and got to the conclusion it was
necessary to arm CSC after setting the gamma tables. Turns out that's not the
case, as the spec doesn't mention any such thing and as shown by testing.

I'll send a v3.

Ander

> 
> > +}
> > +
> >  /* Loads the palette/gamma unit for the CRTC on CherryView. */
> >  static void cherryview_load_luts(struct drm_crtc_state *state)
> >  {
> > @@ -561,6 +616,9 @@ void intel_color_init(struct drm_crtc *crtc)
> >  		   IS_BROXTON(dev_priv)) {
> >  		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
> >  		dev_priv->display.load_luts = broadwell_load_luts;
> > +	} else if (IS_GEMINILAKE(dev_priv)) {
> > +		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
> > +		dev_priv->display.load_luts = glk_load_luts;
> >  	} else {
> >  		dev_priv->display.load_luts = i9xx_load_luts;
> >  	}
> > -- 
> > 2.5.5
> 
> 
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3] drm/i915/glk: Program pipe gamma and degamma tables

[Ander Conselvan de Oliveira]

The gamma tables in Geminilake were changed. There is no split-gamma
mode. Instead, there is a dedicated degamma table that is enabled
whenever pipe CSC is enabled.

The dedicated gamma table has 16 bit precision but doesn't support
separate channels. Since that doesn't match the per-channel format of
the degamma LUT property, for now only a linear table is loaded and the
property ignored.

v2: Remove empty line. (Ville)
    Reuse broadwell code. (Ville)

v3: Don't write PIPE_CSC_MODE. (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Don't write CSC_MODE
---
 drivers/gpu/drm/i915/i915_pci.c    |  1 +
 drivers/gpu/drm/i915/i915_reg.h    | 14 +++++++++
 drivers/gpu/drm/i915/intel_color.c | 58 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index ecb487b..df2051b 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -403,6 +403,7 @@ static const struct intel_device_info intel_geminilake_info = {
 	.platform = INTEL_GEMINILAKE,
 	.is_alpha_support = 1,
 	.ddb_size = 1024,
+	.color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
 };
 
 static const struct intel_device_info intel_kabylake_info = {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 06bbe55..e029691 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8181,12 +8181,26 @@ enum {
 #define _PAL_PREC_EXT_GC_MAX_A	0x4A420
 #define _PAL_PREC_EXT_GC_MAX_B	0x4AC20
 #define _PAL_PREC_EXT_GC_MAX_C	0x4B420
+#define _PAL_PREC_EXT2_GC_MAX_A	0x4A430
+#define _PAL_PREC_EXT2_GC_MAX_B	0x4AC30
+#define _PAL_PREC_EXT2_GC_MAX_C	0x4B430
 
 #define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B)
 #define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B)
 #define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4)
 #define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4)
 
+#define _PRE_CSC_GAMC_INDEX_A	0x4A484
+#define _PRE_CSC_GAMC_INDEX_B	0x4AC84
+#define _PRE_CSC_GAMC_INDEX_C	0x4B484
+#define   PRE_CSC_GAMC_AUTO_INCREMENT	(1 << 10)
+#define _PRE_CSC_GAMC_DATA_A	0x4A488
+#define _PRE_CSC_GAMC_DATA_B	0x4AC88
+#define _PRE_CSC_GAMC_DATA_C	0x4B488
+
+#define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
+#define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
+
 /* pipe CSC & degamma/gamma LUTs on CHV */
 #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
 #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 82a3bc9..0627eee 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -380,7 +380,9 @@ static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset)
 	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
 
 	I915_WRITE(PREC_PAL_INDEX(pipe),
-		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | offset);
+		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
+		   PAL_PREC_AUTO_INCREMENT |
+		   offset);
 
 	if (state->gamma_lut) {
 		struct drm_color_lut *lut =
@@ -443,6 +445,57 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
 	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
 }
 
+static void glk_load_degamma_lut(struct drm_crtc_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
+	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
+	const uint32_t lut_size = 33;
+	uint32_t i;
+
+	/*
+	 * When setting the auto-increment bit, the hardware seems to
+	 * ignore the index bits, so we need to reset it to index 0
+	 * separately.
+	 */
+	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
+	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
+
+	/*
+	 *  FIXME: The pipe degamma table in geminilake doesn't support
+	 *  different values per channel, so this just loads a linear table.
+	 */
+	for (i = 0; i < lut_size; i++) {
+		uint32_t v = (i * ((1 << 16) - 1)) / (lut_size - 1);
+
+		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
+	}
+
+	/* Clamp values > 1.0. */
+	while (i++ < 35)
+		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16) - 1);
+}
+
+static void glk_load_luts(struct drm_crtc_state *state)
+{
+	struct drm_crtc *crtc = state->crtc;
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+
+	if (crtc_state_is_legacy(state)) {
+		haswell_load_luts(state);
+		return;
+	}
+
+	glk_load_degamma_lut(state);
+	bdw_load_gamma_lut(state, 0);
+
+	intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
+	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
+	POSTING_READ(GAMMA_MODE(pipe));
+}
+
 /* Loads the palette/gamma unit for the CRTC on CherryView. */
 static void cherryview_load_luts(struct drm_crtc_state *state)
 {
@@ -561,6 +614,9 @@ void intel_color_init(struct drm_crtc *crtc)
 		   IS_BROXTON(dev_priv)) {
 		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
 		dev_priv->display.load_luts = broadwell_load_luts;
+	} else if (IS_GEMINILAKE(dev_priv)) {
+		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
+		dev_priv->display.load_luts = glk_load_luts;
 	} else {
 		dev_priv->display.load_luts = i9xx_load_luts;
 	}
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915/glk: Program pipe gamma and degamma tables

[Ville Syrjälä]

On Fri, Jan 27, 2017 at 11:02:30AM +0200, Ander Conselvan de Oliveira wrote:
> The gamma tables in Geminilake were changed. There is no split-gamma
> mode. Instead, there is a dedicated degamma table that is enabled
> whenever pipe CSC is enabled.
> 
> The dedicated gamma table has 16 bit precision but doesn't support
> separate channels. Since that doesn't match the per-channel format of
> the degamma LUT property, for now only a linear table is loaded and the
> property ignored.
> 
> v2: Remove empty line. (Ville)
>     Reuse broadwell code. (Ville)
> 
> v3: Don't write PIPE_CSC_MODE. (Ville)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Don't write CSC_MODE
> ---
>  drivers/gpu/drm/i915/i915_pci.c    |  1 +
>  drivers/gpu/drm/i915/i915_reg.h    | 14 +++++++++
>  drivers/gpu/drm/i915/intel_color.c | 58 +++++++++++++++++++++++++++++++++++++-
>  3 files changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index ecb487b..df2051b 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -403,6 +403,7 @@ static const struct intel_device_info intel_geminilake_info = {
>  	.platform = INTEL_GEMINILAKE,
>  	.is_alpha_support = 1,
>  	.ddb_size = 1024,
> +	.color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
>  };
>  
>  static const struct intel_device_info intel_kabylake_info = {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 06bbe55..e029691 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8181,12 +8181,26 @@ enum {
>  #define _PAL_PREC_EXT_GC_MAX_A	0x4A420
>  #define _PAL_PREC_EXT_GC_MAX_B	0x4AC20
>  #define _PAL_PREC_EXT_GC_MAX_C	0x4B420
> +#define _PAL_PREC_EXT2_GC_MAX_A	0x4A430
> +#define _PAL_PREC_EXT2_GC_MAX_B	0x4AC30
> +#define _PAL_PREC_EXT2_GC_MAX_C	0x4B430
>  
>  #define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B)
>  #define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B)
>  #define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4)
>  #define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4)
>  
> +#define _PRE_CSC_GAMC_INDEX_A	0x4A484
> +#define _PRE_CSC_GAMC_INDEX_B	0x4AC84
> +#define _PRE_CSC_GAMC_INDEX_C	0x4B484
> +#define   PRE_CSC_GAMC_AUTO_INCREMENT	(1 << 10)
> +#define _PRE_CSC_GAMC_DATA_A	0x4A488
> +#define _PRE_CSC_GAMC_DATA_B	0x4AC88
> +#define _PRE_CSC_GAMC_DATA_C	0x4B488
> +
> +#define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
> +#define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
> +
>  /* pipe CSC & degamma/gamma LUTs on CHV */
>  #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
>  #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 82a3bc9..0627eee 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -380,7 +380,9 @@ static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset)
>  	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
>  
>  	I915_WRITE(PREC_PAL_INDEX(pipe),
> -		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | offset);
> +		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
> +		   PAL_PREC_AUTO_INCREMENT |
> +		   offset);
>  
>  	if (state->gamma_lut) {
>  		struct drm_color_lut *lut =
> @@ -443,6 +445,57 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
>  	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>  }
>  
> +static void glk_load_degamma_lut(struct drm_crtc_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
> +	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
> +	const uint32_t lut_size = 33;
> +	uint32_t i;
> +
> +	/*
> +	 * When setting the auto-increment bit, the hardware seems to
> +	 * ignore the index bits, so we need to reset it to index 0
> +	 * separately.
> +	 */
> +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
> +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
> +
> +	/*
> +	 *  FIXME: The pipe degamma table in geminilake doesn't support
> +	 *  different values per channel, so this just loads a linear table.
> +	 */
> +	for (i = 0; i < lut_size; i++) {
> +		uint32_t v = (i * ((1 << 16) - 1)) / (lut_size - 1);
> +
> +		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
> +	}
> +
> +	/* Clamp values > 1.0. */
> +	while (i++ < 35)
> +		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16) - 1);
> +}
> +
> +static void glk_load_luts(struct drm_crtc_state *state)
> +{
> +	struct drm_crtc *crtc = state->crtc;
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +
> +	if (crtc_state_is_legacy(state)) {
> +		haswell_load_luts(state);
> +		return;
> +	}
> +
> +	glk_load_degamma_lut(state);
> +	bdw_load_gamma_lut(state, 0);
> +
> +	intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
> +	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
> +	POSTING_READ(GAMMA_MODE(pipe));
> +}
> +
>  /* Loads the palette/gamma unit for the CRTC on CherryView. */
>  static void cherryview_load_luts(struct drm_crtc_state *state)
>  {
> @@ -561,6 +614,9 @@ void intel_color_init(struct drm_crtc *crtc)
>  		   IS_BROXTON(dev_priv)) {
>  		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
>  		dev_priv->display.load_luts = broadwell_load_luts;
> +	} else if (IS_GEMINILAKE(dev_priv)) {
> +		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
> +		dev_priv->display.load_luts = glk_load_luts;
>  	} else {
>  		dev_priv->display.load_luts = i9xx_load_luts;
>  	}
> -- 
> 2.9.3

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915/glk: Program pipe gamma and degamma tables

[Ander Conselvan De Oliveira]

On Fri, 2017-01-27 at 16:04 +0200, Ville Syrjälä wrote:
> On Fri, Jan 27, 2017 at 11:02:30AM +0200, Ander Conselvan de Oliveira wrote:
> > The gamma tables in Geminilake were changed. There is no split-gamma
> > mode. Instead, there is a dedicated degamma table that is enabled
> > whenever pipe CSC is enabled.
> > 
> > The dedicated gamma table has 16 bit precision but doesn't support
> > separate channels. Since that doesn't match the per-channel format of
> > the degamma LUT property, for now only a linear table is loaded and the
> > property ignored.
> > 
> > v2: Remove empty line. (Ville)
> >     Reuse broadwell code. (Ville)
> > 
> > v3: Don't write PIPE_CSC_MODE. (Ville)
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@inte
> > l.com>
> 
> lgtm
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks for the reviews. Pushed up to here.

Ander

> 
> > 
> > Don't write CSC_MODE
> > ---
> >  drivers/gpu/drm/i915/i915_pci.c    |  1 +
> >  drivers/gpu/drm/i915/i915_reg.h    | 14 +++++++++
> >  drivers/gpu/drm/i915/intel_color.c | 58
> > +++++++++++++++++++++++++++++++++++++-
> >  3 files changed, 72 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > b/drivers/gpu/drm/i915/i915_pci.c
> > index ecb487b..df2051b 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -403,6 +403,7 @@ static const struct intel_device_info
> > intel_geminilake_info = {
> >  	.platform = INTEL_GEMINILAKE,
> >  	.is_alpha_support = 1,
> >  	.ddb_size = 1024,
> > +	.color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
> >  };
> >  
> >  static const struct intel_device_info intel_kabylake_info = {
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 06bbe55..e029691 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -8181,12 +8181,26 @@ enum {
> >  #define _PAL_PREC_EXT_GC_MAX_A	0x4A420
> >  #define _PAL_PREC_EXT_GC_MAX_B	0x4AC20
> >  #define _PAL_PREC_EXT_GC_MAX_C	0x4B420
> > +#define _PAL_PREC_EXT2_GC_MAX_A	0x4A430
> > +#define _PAL_PREC_EXT2_GC_MAX_B	0x4AC30
> > +#define _PAL_PREC_EXT2_GC_MAX_C	0x4B430
> >  
> >  #define PREC_PAL_INDEX(pipe)		_MMIO_PIPE(pipe,
> > _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B)
> >  #define PREC_PAL_DATA(pipe)		_MMIO_PIPE(pipe,
> > _PAL_PREC_DATA_A, _PAL_PREC_DATA_B)
> >  #define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe,
> > _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4)
> >  #define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE(pipe,
> > _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4)
> >  
> > +#define _PRE_CSC_GAMC_INDEX_A	0x4A484
> > +#define _PRE_CSC_GAMC_INDEX_B	0x4AC84
> > +#define _PRE_CSC_GAMC_INDEX_C	0x4B484
> > +#define   PRE_CSC_GAMC_AUTO_INCREMENT	(1 << 10)
> > +#define _PRE_CSC_GAMC_DATA_A	0x4A488
> > +#define _PRE_CSC_GAMC_DATA_B	0x4AC88
> > +#define _PRE_CSC_GAMC_DATA_C	0x4B488
> > +
> > +#define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe,
> > _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
> > +#define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe,
> > _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
> > +
> >  /* pipe CSC & degamma/gamma LUTs on CHV */
> >  #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
> >  #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
> > diff --git a/drivers/gpu/drm/i915/intel_color.c
> > b/drivers/gpu/drm/i915/intel_color.c
> > index 82a3bc9..0627eee 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -380,7 +380,9 @@ static void bdw_load_gamma_lut(struct drm_crtc_state
> > *state, u32 offset)
> >  	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
> >  
> >  	I915_WRITE(PREC_PAL_INDEX(pipe),
> > -		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | offset);
> > +		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
> > +		   PAL_PREC_AUTO_INCREMENT |
> > +		   offset);
> >  
> >  	if (state->gamma_lut) {
> >  		struct drm_color_lut *lut =
> > @@ -443,6 +445,57 @@ static void broadwell_load_luts(struct drm_crtc_state
> > *state)
> >  	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> >  }
> >  
> > +static void glk_load_degamma_lut(struct drm_crtc_state *state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
> > +	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
> > +	const uint32_t lut_size = 33;
> > +	uint32_t i;
> > +
> > +	/*
> > +	 * When setting the auto-increment bit, the hardware seems to
> > +	 * ignore the index bits, so we need to reset it to index 0
> > +	 * separately.
> > +	 */
> > +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
> > +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
> > +
> > +	/*
> > +	 *  FIXME: The pipe degamma table in geminilake doesn't support
> > +	 *  different values per channel, so this just loads a linear
> > table.
> > +	 */
> > +	for (i = 0; i < lut_size; i++) {
> > +		uint32_t v = (i * ((1 << 16) - 1)) / (lut_size - 1);
> > +
> > +		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
> > +	}
> > +
> > +	/* Clamp values > 1.0. */
> > +	while (i++ < 35)
> > +		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16) - 1);
> > +}
> > +
> > +static void glk_load_luts(struct drm_crtc_state *state)
> > +{
> > +	struct drm_crtc *crtc = state->crtc;
> > +	struct drm_device *dev = crtc->dev;
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > +	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
> > +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> > +
> > +	if (crtc_state_is_legacy(state)) {
> > +		haswell_load_luts(state);
> > +		return;
> > +	}
> > +
> > +	glk_load_degamma_lut(state);
> > +	bdw_load_gamma_lut(state, 0);
> > +
> > +	intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
> > +	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
> > +	POSTING_READ(GAMMA_MODE(pipe));
> > +}
> > +
> >  /* Loads the palette/gamma unit for the CRTC on CherryView. */
> >  static void cherryview_load_luts(struct drm_crtc_state *state)
> >  {
> > @@ -561,6 +614,9 @@ void intel_color_init(struct drm_crtc *crtc)
> >  		   IS_BROXTON(dev_priv)) {
> >  		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
> >  		dev_priv->display.load_luts = broadwell_load_luts;
> > +	} else if (IS_GEMINILAKE(dev_priv)) {
> > +		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
> > +		dev_priv->display.load_luts = glk_load_luts;
> >  	} else {
> >  		dev_priv->display.load_luts = i9xx_load_luts;
> >  	}
> > -- 
> > 2.9.3
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 5/5] drm/i915/glk: Enable pipe CSC

[Ander Conselvan de Oliveira]

Now that the pre-csc degamma table is set up correctly in Geminilake,
pipe CSC can be enabled without causing a black screen.

v2: Rebase.
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 drivers/gpu/drm/i915/intel_sprite.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 13d05bc..3eee44a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3391,6 +3391,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	if (IS_GEMINILAKE(dev_priv)) {
 		I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
 			   PLANE_COLOR_PIPE_GAMMA_ENABLE |
+			   PLANE_COLOR_PIPE_CSC_ENABLE |
 			   PLANE_COLOR_PLANE_GAMMA_DISABLE);
 	} else {
 		plane_ctl |=
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index b16a295..27e0752 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -224,6 +224,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 	if (IS_GEMINILAKE(dev_priv)) {
 		I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
 			   PLANE_COLOR_PIPE_GAMMA_ENABLE |
+			   PLANE_COLOR_PIPE_CSC_ENABLE |
 			   PLANE_COLOR_PLANE_GAMMA_DISABLE);
 	} else {
 		plane_ctl |=
-- 
2.5.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] drm/i915/glk: Enable pipe CSC

[Ville Syrjälä]

On Thu, Jan 26, 2017 at 01:24:25PM +0200, Ander Conselvan de Oliveira wrote:
> Now that the pre-csc degamma table is set up correctly in Geminilake,
> pipe CSC can be enabled without causing a black screen.
> 
> v2: Rebase.
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 1 +
>  drivers/gpu/drm/i915/intel_sprite.c  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 13d05bc..3eee44a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3391,6 +3391,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
>  	if (IS_GEMINILAKE(dev_priv)) {
>  		I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
>  			   PLANE_COLOR_PIPE_GAMMA_ENABLE |
> +			   PLANE_COLOR_PIPE_CSC_ENABLE |
>  			   PLANE_COLOR_PLANE_GAMMA_DISABLE);
>  	} else {
>  		plane_ctl |=
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index b16a295..27e0752 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -224,6 +224,7 @@ skl_update_plane(struct drm_plane *drm_plane,
>  	if (IS_GEMINILAKE(dev_priv)) {
>  		I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id),
>  			   PLANE_COLOR_PIPE_GAMMA_ENABLE |
> +			   PLANE_COLOR_PIPE_CSC_ENABLE |
>  			   PLANE_COLOR_PLANE_GAMMA_DISABLE);
>  	} else {
>  		plane_ctl |=
> -- 
> 2.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for Geminilake pipe CSC

[Patchwork]

== Series Details ==

Series: Geminilake pipe CSC
URL   : https://patchwork.freedesktop.org/series/18596/
State : success

== Summary ==

Series 18596v1 Geminilake pipe CSC
https://patchwork.freedesktop.org/api/1.0/series/18596/revisions/1/mbox/


fi-bdw-5557u     total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:247  pass:225  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:79   pass:66   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:247  pass:227  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:247  pass:222  dwarn:4   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

3778ca36c6b4e894b581d4f56abe6bbafe927e96 drm-tip: 2017y-01m-26d-10h-11m-00s UTC integration manifest
f61adb1 drm/i915/glk: Enable pipe CSC
017b305 drm/i915/glk: Program pipe gamma and degamma tables
373a782 drm/i915: Split broadwell_load_luts() into smaller functions
956e09a drm/i915/glk: Plane color correction register changes
821b22e drm/i915: Disable plane gamma in SKL+ sprite planes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3611/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH RFC 6/5] drm/i915: Merge BDW pipe gamma and degamma table code

[Ander Conselvan de Oliveira]

The only difference between the code loading the pipe gamma and degamma
tables in BDW is that the gamma code also writes the registers that hold
the maximum values. So we can use the gamma code for the degamma table,
at the expense of writing the maximum value register twice, with
potenttially wrong values in the first time.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---

Ville, does this help with the split gamma enable/disable confusion?

Note that I didn't test this.

Ander

---
 drivers/gpu/drm/i915/intel_color.c | 57 ++++++++++----------------------------
 1 file changed, 15 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 08345e5..c686c37 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -340,54 +340,22 @@ static void haswell_load_luts(struct drm_crtc_state *crtc_state)
 		hsw_enable_ips(intel_crtc);
 }
 
-static void bdw_load_degamma_lut(struct drm_crtc_state *state)
+static void bdw_load_lut(struct drm_crtc_state *state, u32 offset,
+			 struct drm_color_lut *lut, u32 lut_size,
+			 bool split_mode)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
 	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
-	uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
-
-	I915_WRITE(PREC_PAL_INDEX(pipe),
-		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
-
-	if (state->degamma_lut) {
-		struct drm_color_lut *lut =
-			(struct drm_color_lut *) state->degamma_lut->data;
-
-		for (i = 0; i < lut_size; i++) {
-			uint32_t word =
-			drm_color_lut_extract(lut[i].red, 10) << 20 |
-			drm_color_lut_extract(lut[i].green, 10) << 10 |
-			drm_color_lut_extract(lut[i].blue, 10);
-
-			I915_WRITE(PREC_PAL_DATA(pipe), word);
-		}
-	} else {
-		for (i = 0; i < lut_size; i++) {
-			uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1);
-
-			I915_WRITE(PREC_PAL_DATA(pipe),
-				   (v << 20) | (v << 10) | v);
-		}
-	}
-}
-
-static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset)
-{
-	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
-	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
-	uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+	uint32_t i;
 
 	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
 
 	I915_WRITE(PREC_PAL_INDEX(pipe),
-		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
+		   (split_mode ? PAL_PREC_SPLIT_MODE : 0) |
 		   PAL_PREC_AUTO_INCREMENT |
 		   offset);
 
-	if (state->gamma_lut) {
-		struct drm_color_lut *lut =
-			(struct drm_color_lut *) state->gamma_lut->data;
-
+	if (lut) {
 		for (i = 0; i < lut_size; i++) {
 			uint32_t word =
 			(drm_color_lut_extract(lut[i].red, 10) << 20) |
@@ -430,9 +398,12 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
 		return;
 	}
 
-	bdw_load_degamma_lut(state);
-	bdw_load_gamma_lut(state,
-			   INTEL_INFO(dev_priv)->color.degamma_lut_size);
+	bdw_load_lut(state, 0, (struct drm_color_lut *) state->degamma_lut,
+		     INTEL_INFO(dev_priv)->color.degamma_lut_size, true);
+	bdw_load_lut(state, INTEL_INFO(dev_priv)->color.degamma_lut_size,
+		     (struct drm_color_lut *) state->gamma_lut,
+		     INTEL_INFO(dev_priv)->color.gamma_lut_size,
+		     true);
 
 	intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
 	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
@@ -489,7 +460,9 @@ static void glk_load_luts(struct drm_crtc_state *state)
 	}
 
 	glk_load_degamma_lut(state);
-	bdw_load_gamma_lut(state, 0);
+	bdw_load_lut(state, 0, (struct drm_color_lut *) state->gamma_lut,
+		     INTEL_INFO(dev_priv)->color.gamma_lut_size,
+		     false);
 
 	intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
 	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH RFC 6/5] drm/i915: Merge BDW pipe gamma and degamma table code

[Ville Syrjälä]

On Fri, Jan 27, 2017 at 11:19:49AM +0200, Ander Conselvan de Oliveira wrote:
> The only difference between the code loading the pipe gamma and degamma
> tables in BDW is that the gamma code also writes the registers that hold
> the maximum values. So we can use the gamma code for the degamma table,
> at the expense of writing the maximum value register twice, with
> potenttially wrong values in the first time.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
> 
> Ville, does this help with the split gamma enable/disable confusion?

I might. Certainly it avoids a bunch of duplicated code. One comment
below...

> 
> Note that I didn't test this.
> 
> Ander
> 
> ---
>  drivers/gpu/drm/i915/intel_color.c | 57 ++++++++++----------------------------
>  1 file changed, 15 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 08345e5..c686c37 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -340,54 +340,22 @@ static void haswell_load_luts(struct drm_crtc_state *crtc_state)
>  		hsw_enable_ips(intel_crtc);
>  }
>  
> -static void bdw_load_degamma_lut(struct drm_crtc_state *state)
> +static void bdw_load_lut(struct drm_crtc_state *state, u32 offset,
> +			 struct drm_color_lut *lut, u32 lut_size,
> +			 bool split_mode)

Maybe pass PAL_PREC_SPLIT_MODE or 0 here directly? To make the callers
even more clear we could go as far as defining a symbolic name for the 0.

>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
>  	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
> -	uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> -
> -	I915_WRITE(PREC_PAL_INDEX(pipe),
> -		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
> -
> -	if (state->degamma_lut) {
> -		struct drm_color_lut *lut =
> -			(struct drm_color_lut *) state->degamma_lut->data;
> -
> -		for (i = 0; i < lut_size; i++) {
> -			uint32_t word =
> -			drm_color_lut_extract(lut[i].red, 10) << 20 |
> -			drm_color_lut_extract(lut[i].green, 10) << 10 |
> -			drm_color_lut_extract(lut[i].blue, 10);
> -
> -			I915_WRITE(PREC_PAL_DATA(pipe), word);
> -		}
> -	} else {
> -		for (i = 0; i < lut_size; i++) {
> -			uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1);
> -
> -			I915_WRITE(PREC_PAL_DATA(pipe),
> -				   (v << 20) | (v << 10) | v);
> -		}
> -	}
> -}
> -
> -static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
> -	enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
> -	uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> +	uint32_t i;
>  
>  	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
>  
>  	I915_WRITE(PREC_PAL_INDEX(pipe),
> -		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
> +		   (split_mode ? PAL_PREC_SPLIT_MODE : 0) |
>  		   PAL_PREC_AUTO_INCREMENT |
>  		   offset);
>  
> -	if (state->gamma_lut) {
> -		struct drm_color_lut *lut =
> -			(struct drm_color_lut *) state->gamma_lut->data;
> -
> +	if (lut) {
>  		for (i = 0; i < lut_size; i++) {
>  			uint32_t word =
>  			(drm_color_lut_extract(lut[i].red, 10) << 20) |
> @@ -430,9 +398,12 @@ static void broadwell_load_luts(struct drm_crtc_state *state)
>  		return;
>  	}
>  
> -	bdw_load_degamma_lut(state);
> -	bdw_load_gamma_lut(state,
> -			   INTEL_INFO(dev_priv)->color.degamma_lut_size);
> +	bdw_load_lut(state, 0, (struct drm_color_lut *) state->degamma_lut,
> +		     INTEL_INFO(dev_priv)->color.degamma_lut_size, true);
> +	bdw_load_lut(state, INTEL_INFO(dev_priv)->color.degamma_lut_size,
> +		     (struct drm_color_lut *) state->gamma_lut,
> +		     INTEL_INFO(dev_priv)->color.gamma_lut_size,
> +		     true);
>  
>  	intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
>  	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
> @@ -489,7 +460,9 @@ static void glk_load_luts(struct drm_crtc_state *state)
>  	}
>  
>  	glk_load_degamma_lut(state);
> -	bdw_load_gamma_lut(state, 0);
> +	bdw_load_lut(state, 0, (struct drm_color_lut *) state->gamma_lut,
> +		     INTEL_INFO(dev_priv)->color.gamma_lut_size,
> +		     false);
>  
>  	intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
>  	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
> -- 
> 2.9.3

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for Geminilake pipe CSC (rev2)

[Patchwork]

== Series Details ==

Series: Geminilake pipe CSC (rev2)
URL   : https://patchwork.freedesktop.org/series/18596/
State : success

== Summary ==

Series 18596v2 Geminilake pipe CSC
https://patchwork.freedesktop.org/api/1.0/series/18596/revisions/2/mbox/


fi-bdw-5557u     total:247  pass:233  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:247  pass:225  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:79   pass:66   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:247  pass:226  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:247  pass:227  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:247  pass:222  dwarn:4   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:247  pass:234  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:247  pass:215  dwarn:0   dfail:0   fail:0   skip:32 

66c972ab613b1f1d659f413f850047077f3363f0 drm-tip: 2017y-01m-27d-08h-06m-49s UTC integration manifest
40b3da38 drm/i915: Merge BDW pipe gamma and degamma table code
28b9d12 drm/i915/glk: Enable pipe CSC
81389e6 drm/i915/glk: Program pipe gamma and degamma tables
4a0d57f drm/i915: Split broadwell_load_luts() into smaller functions
446ac15 drm/i915/glk: Plane color correction register changes
9a2501c drm/i915: Disable plane gamma in SKL+ sprite planes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3619/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx