On Wed, Nov 14, 2018 at 09:36:39AM -0800, Matt Roper wrote:
> On Wed, Nov 14, 2018 at 07:28:12PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 13, 2018 at 03:21:47PM -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.
> > > 
> > > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > > Signed-off-by: Matt Roper <matthew.d.ro...@intel.com>
> > > ---
> > > We may want to land this patch before the rest of the series since it's
> > > still valuable even without the new ABI the rest of the series adds.
> > > 
> > >  drivers/gpu/drm/i915/i915_reg.h      |  6 ++++++
> > >  drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++++++++
> > >  2 files changed, 24 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index fe4b913e46ac..b92a721c9bcb 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -5662,6 +5662,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_CANVAS_A                    0x70034
> > > +#define   SKL_CANVAS_GAMMA_ENABLE        (1 << 31)
> > > +#define   SKL_CANVAS_CSC_ENABLE          (1 << 30)
> > > +#define SKL_CANVAS(pipe)         _MMIO_PIPE2(pipe, _SKL_CANVAS_A)
> > 
> > Didn't the spec call this BOTTOM_COLOR or something like that?
> 
> The register definition is called BOTTOM_COLOR, but other areas of the
> spec refer to this as either "background color" or "canvas color."  We
> already have a CHV_CANVAS register that I believe provides the same
> functionality for CHV (although I can't find details for the bit layout
> in the spec), so I decided to just keep this name consistent with that
> one.

While CHV_CANVAS does provide simiar functionality (actually
just the color, not the gamma/csc control) it is still a
different register. IMO better to stick to the naming used
in the spec.

> 
> 
> Matt
> 
> > 
> > > +
> > >  #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 132e978227fb..1d089d93d88b 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3868,6 +3868,15 @@ 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_CANVAS(crtc->pipe),
> > > +                    SKL_CANVAS_GAMMA_ENABLE | SKL_CANVAS_CSC_ENABLE);
> > >  }
> > >  
> > >  static void intel_fdi_normal_train(struct intel_crtc *crtc)
> > > @@ -15356,6 +15365,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_CANVAS(crtc->pipe),
> > > +                            SKL_CANVAS_GAMMA_ENABLE |
> > > +                            SKL_CANVAS_CSC_ENABLE);
> > >   }
> > >  
> > >   /* Adjust the state of the output pipe according to whether we
> > > -- 
> > > 2.14.4
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

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

Reply via email to