diff mbox series

[v4.1,1/3] drm/i915: Force background color to black for gen9+ (v2)

Message ID 20190130185122.10322-2-matthew.d.roper@intel.com (mailing list archive)
State New, archived
Headers show
Series CRTC background color | expand

Commit Message

Matt Roper Jan. 30, 2019, 6:51 p.m. UTC
We don't yet allow userspace to control the CRTC background color, but
we should manually program the color to black to ensure the BIOS didn't
leave us with some other color.  We should also set the pipe gamma and
pipe CSC bits so that the background color goes through the same color
management transformations that a plane with black pixels would.

v2: Rename register to SKL_BOTTOM_COLOR to more closely follow
    bspec naming.  (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  6 ++++++
 drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
 2 files changed, 25 insertions(+)

Comments

Ville Syrjälä Jan. 30, 2019, 9:03 p.m. UTC | #1
On Wed, Jan 30, 2019 at 10:51:20AM -0800, Matt Roper wrote:
> We don't yet allow userspace to control the CRTC background color, but
> we should manually program the color to black to ensure the BIOS didn't
> leave us with some other color.  We should also set the pipe gamma and
> pipe CSC bits so that the background color goes through the same color
> management transformations that a plane with black pixels would.
> 
> v2: Rename register to SKL_BOTTOM_COLOR to more closely follow
>     bspec naming.  (Ville)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  6 ++++++
>  drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 03adcf3838de..a64deeb4e517 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5710,6 +5710,12 @@ enum {
>  #define   PIPEMISC_DITHER_TYPE_SP	(0 << 2)
>  #define PIPEMISC(pipe)			_MMIO_PIPE2(pipe, _PIPE_MISC_A)
>  
> +/* Skylake+ pipe bottom (background) color */
> +#define _SKL_BOTTOM_COLOR_A		0x70034
> +#define   SKL_BOTTOM_COLOR_GAMMA_ENABLE	(1 << 31)
> +#define   SKL_BOTTOM_COLOR_CSC_ENABLE	(1 << 30)
> +#define SKL_BOTTOM_COLOR(pipe)		_MMIO_PIPE2(pipe, _SKL_BOTTOM_COLOR_A)
> +
>  #define VLV_DPFLIPSTAT				_MMIO(VLV_DISPLAY_BASE + 0x70028)
>  #define   PIPEB_LINE_COMPARE_INT_EN		(1 << 29)
>  #define   PIPEB_HLINE_INT_EN			(1 << 28)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 539d8915b55f..a025efb1d7c6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3930,6 +3930,16 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
>  		else if (old_crtc_state->pch_pfit.enabled)
>  			ironlake_pfit_disable(old_crtc_state);
>  	}
> +
> +	/*
> +	 * We don't (yet) allow userspace to control the pipe background color,
> +	 * so force it to black, but apply pipe gamma and CSC so that its
> +	 * handling will match how we program our planes.
> +	 */
> +	if (INTEL_GEN(dev_priv) >= 9)
> +		I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe),
> +			   SKL_BOTTOM_COLOR_GAMMA_ENABLE |
> +			   SKL_BOTTOM_COLOR_CSC_ENABLE);
>  }
>  
>  static void intel_fdi_normal_train(struct intel_crtc *crtc)
> @@ -15488,6 +15498,15 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  			    plane->base.type != DRM_PLANE_TYPE_PRIMARY)
>  				intel_plane_disable_noatomic(crtc, plane);
>  		}
> +
> +		/*
> +		 * Disable any background color set by the BIOS, but enable the
> +		 * gamma and CSC to match how we program our planes.
> +		 */
> +		if (INTEL_GEN(dev_priv) >= 9)
> +			I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe),
> +				   SKL_BOTTOM_COLOR_GAMMA_ENABLE |
> +				   SKL_BOTTOM_COLOR_CSC_ENABLE);
>  	}
>  
>  	/* Adjust the state of the output pipe according to whether we
> -- 
> 2.14.5
Matt Roper Jan. 31, 2019, 12:29 a.m. UTC | #2
On Wed, Jan 30, 2019 at 11:03:33PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 30, 2019 at 10:51:20AM -0800, Matt Roper wrote:
> > We don't yet allow userspace to control the CRTC background color, but
> > we should manually program the color to black to ensure the BIOS didn't
> > leave us with some other color.  We should also set the pipe gamma and
> > pipe CSC bits so that the background color goes through the same color
> > management transformations that a plane with black pixels would.
> > 
> > v2: Rename register to SKL_BOTTOM_COLOR to more closely follow
> >     bspec naming.  (Ville)
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Applied just this first patch to dinq since it's a bugfix and doesn't
impact the ABI that comes in the later patches.  Thanks for the review.


Matt

> 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |  6 ++++++
> >  drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++
> >  2 files changed, 25 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 03adcf3838de..a64deeb4e517 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5710,6 +5710,12 @@ enum {
> >  #define   PIPEMISC_DITHER_TYPE_SP	(0 << 2)
> >  #define PIPEMISC(pipe)			_MMIO_PIPE2(pipe, _PIPE_MISC_A)
> >  
> > +/* Skylake+ pipe bottom (background) color */
> > +#define _SKL_BOTTOM_COLOR_A		0x70034
> > +#define   SKL_BOTTOM_COLOR_GAMMA_ENABLE	(1 << 31)
> > +#define   SKL_BOTTOM_COLOR_CSC_ENABLE	(1 << 30)
> > +#define SKL_BOTTOM_COLOR(pipe)		_MMIO_PIPE2(pipe, _SKL_BOTTOM_COLOR_A)
> > +
> >  #define VLV_DPFLIPSTAT				_MMIO(VLV_DISPLAY_BASE + 0x70028)
> >  #define   PIPEB_LINE_COMPARE_INT_EN		(1 << 29)
> >  #define   PIPEB_HLINE_INT_EN			(1 << 28)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 539d8915b55f..a025efb1d7c6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3930,6 +3930,16 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
> >  		else if (old_crtc_state->pch_pfit.enabled)
> >  			ironlake_pfit_disable(old_crtc_state);
> >  	}
> > +
> > +	/*
> > +	 * We don't (yet) allow userspace to control the pipe background color,
> > +	 * so force it to black, but apply pipe gamma and CSC so that its
> > +	 * handling will match how we program our planes.
> > +	 */
> > +	if (INTEL_GEN(dev_priv) >= 9)
> > +		I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe),
> > +			   SKL_BOTTOM_COLOR_GAMMA_ENABLE |
> > +			   SKL_BOTTOM_COLOR_CSC_ENABLE);
> >  }
> >  
> >  static void intel_fdi_normal_train(struct intel_crtc *crtc)
> > @@ -15488,6 +15498,15 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
> >  			    plane->base.type != DRM_PLANE_TYPE_PRIMARY)
> >  				intel_plane_disable_noatomic(crtc, plane);
> >  		}
> > +
> > +		/*
> > +		 * Disable any background color set by the BIOS, but enable the
> > +		 * gamma and CSC to match how we program our planes.
> > +		 */
> > +		if (INTEL_GEN(dev_priv) >= 9)
> > +			I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe),
> > +				   SKL_BOTTOM_COLOR_GAMMA_ENABLE |
> > +				   SKL_BOTTOM_COLOR_CSC_ENABLE);
> >  	}
> >  
> >  	/* Adjust the state of the output pipe according to whether we
> > -- 
> > 2.14.5
> 
> -- 
> Ville Syrjälä
> Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 03adcf3838de..a64deeb4e517 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5710,6 +5710,12 @@  enum {
 #define   PIPEMISC_DITHER_TYPE_SP	(0 << 2)
 #define PIPEMISC(pipe)			_MMIO_PIPE2(pipe, _PIPE_MISC_A)
 
+/* Skylake+ pipe bottom (background) color */
+#define _SKL_BOTTOM_COLOR_A		0x70034
+#define   SKL_BOTTOM_COLOR_GAMMA_ENABLE	(1 << 31)
+#define   SKL_BOTTOM_COLOR_CSC_ENABLE	(1 << 30)
+#define SKL_BOTTOM_COLOR(pipe)		_MMIO_PIPE2(pipe, _SKL_BOTTOM_COLOR_A)
+
 #define VLV_DPFLIPSTAT				_MMIO(VLV_DISPLAY_BASE + 0x70028)
 #define   PIPEB_LINE_COMPARE_INT_EN		(1 << 29)
 #define   PIPEB_HLINE_INT_EN			(1 << 28)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 539d8915b55f..a025efb1d7c6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3930,6 +3930,16 @@  static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
 		else if (old_crtc_state->pch_pfit.enabled)
 			ironlake_pfit_disable(old_crtc_state);
 	}
+
+	/*
+	 * We don't (yet) allow userspace to control the pipe background color,
+	 * so force it to black, but apply pipe gamma and CSC so that its
+	 * handling will match how we program our planes.
+	 */
+	if (INTEL_GEN(dev_priv) >= 9)
+		I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe),
+			   SKL_BOTTOM_COLOR_GAMMA_ENABLE |
+			   SKL_BOTTOM_COLOR_CSC_ENABLE);
 }
 
 static void intel_fdi_normal_train(struct intel_crtc *crtc)
@@ -15488,6 +15498,15 @@  static void intel_sanitize_crtc(struct intel_crtc *crtc,
 			    plane->base.type != DRM_PLANE_TYPE_PRIMARY)
 				intel_plane_disable_noatomic(crtc, plane);
 		}
+
+		/*
+		 * Disable any background color set by the BIOS, but enable the
+		 * gamma and CSC to match how we program our planes.
+		 */
+		if (INTEL_GEN(dev_priv) >= 9)
+			I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe),
+				   SKL_BOTTOM_COLOR_GAMMA_ENABLE |
+				   SKL_BOTTOM_COLOR_CSC_ENABLE);
 	}
 
 	/* Adjust the state of the output pipe according to whether we