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

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 2/5] drm/i915/glk: Plane color correction register changes
Date: Thu, 26 Jan 2017 13:32:11 +0200	[thread overview]
Message-ID: <20170126113211.GA31595@intel.com> (raw)
In-Reply-To: <1485429865-10687-3-git-send-email-ander.conselvan.de.oliveira@intel.com>

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