| Message ID | 1455157979-7341-3-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 10, 2016 at 06:32:59PM -0800, Matt Roper wrote: > Gen9 platforms allow CRTC's to be programmed with a background/canvas > color below the programmable planes. Let's expose this as a property to > allow userspace to program a desired value. > > This patch is based on earlier work by Chandra Konduru; unfortunately > the driver has evolved so much since his patches were written (in the > pre-atomic era) that the functionality had to be pretty much completely > rewritten for the new i915 atomic internals. > > v2: > - Set initial background color (black) via proper helper function (Bob) > - Fix debugfs output > - General rebasing > > Cc: Chandra Konduru <chandra.konduru@intel.com> > Cc: Bob Paauwe <bob.j.paauwe@intel.com> > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > Documentation/DocBook/gpu.tmpl | 10 +++++++- > drivers/gpu/drm/i915/i915_debugfs.c | 8 +++++++ > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++ > drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++ > 4 files changed, 72 insertions(+), 1 deletion(-) > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > index fe6b36a..9e003cd 100644 > --- a/Documentation/DocBook/gpu.tmpl > +++ b/Documentation/DocBook/gpu.tmpl > @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev) > <td valign="top" >TBD</td> > </tr> > <tr> > - <td rowspan="20" valign="top" >i915</td> > + <td rowspan="21" valign="top" >i915</td> > <td rowspan="2" valign="top" >Generic</td> > <td valign="top" >"Broadcast RGB"</td> > <td valign="top" >ENUM</td> > @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev) > <td valign="top" >TBD</td> > </tr> > <tr> > + <td rowspan="1" valign="top" >CRTC</td> > + <td valign="top" >“background_color”</td> > + <td valign="top" >RGBA</td> > + <td valign="top" > </td> > + <td valign="top" >CRTC</td> > + <td valign="top" >Background color of regions not covered by a plane</td> > + </tr> > + <tr> > <td rowspan="17" valign="top" >SDVO-TV</td> > <td valign="top" >“mode”</td> > <td valign="top" >ENUM</td> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index ec0c2a05e..e7352fc 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused) > intel_scaler_info(m, crtc); > intel_plane_info(m, crtc); > } > + if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) { > + struct drm_rgba background = pipe_config->base.background_color; > + > + seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n", > + DRM_RGBA_REDBITS(background, 10), > + DRM_RGBA_GREENBITS(background, 10), > + DRM_RGBA_BLUEBITS(background, 10)); > + } > > seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", > yesno(!crtc->cpu_fifo_underrun_disabled), > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 144586e..b0b014d 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells { > #define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) > #define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO) > > +/* Skylake pipe bottom color */ > +#define _PIPE_BOTTOM_COLOR_A 0x70034 > +#define _PIPE_BOTTOM_COLOR_B 0x71034 > +#define _PIPE_BOTTOM_COLOR_C 0x72034 > +#define PIPE_BOTTOM_GAMMA_ENABLE (1 << 31) > +#define PIPE_BOTTOM_CSC_ENABLE (1 << 30) > +#define PIPE_BOTTOM_COLOR_MASK 0x3FFFFFFF > +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B) > + > /* MIPI DSI registers */ > > #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */ > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 836bbdc..a616ac42 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc_state *pipe_config = > to_intel_crtc_state(crtc->base.state); > + struct drm_rgba background = pipe_config->base.background_color; > + uint32_t val; > > /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ > crtc->base.mode = crtc->base.state->mode; > @@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, > else if (old_crtc_state->pch_pfit.enabled) > ironlake_pfit_disable(crtc, true); > } > + > + if (INTEL_INFO(dev)->gen >= 9) { > + /* BGR 16bpc ==> RGB 10bpc */ > + val = DRM_RGBA_REDBITS(background, 10) << 20 > + | DRM_RGBA_GREENBITS(background, 10) << 10 > + | DRM_RGBA_BLUEBITS(background, 10); I'm not a fan of the drm_rgba thing having alpha in the low bits. It goes against the existing precedent set by eg. the colorkey ioctls where alpha (if it would be used) would be in the high bits. I think I complained about this before already, or maybe it was also about some BGR vs. RGB ordering thing? Looks like drm_rgba isn't actually part of this series, and I can't see it in the tree either, so I guess it comes in via some other series? > + > + /* > + * Set CSC and gamma for bottom color. > + * > + * FIXME: We turn these on unconditionally for now to match > + * how we've setup the various planes. Once the color > + * management framework lands, it may or may not choose to > + * set these bits. > + */ > + val |= PIPE_BOTTOM_CSC_ENABLE; > + val |= PIPE_BOTTOM_GAMMA_ENABLE; > + > + I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val); > + } > } > > static void intel_fdi_normal_train(struct drm_crtc *crtc) > @@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > pipe_config); > } > > + if (crtc->state->background_color.v != crtc_state->background_color.v) > + pipe_config->update_pipe = true; > + > return ret; > } > > @@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { > .set_config = drm_atomic_helper_set_config, > .destroy = intel_crtc_destroy, > .page_flip = intel_crtc_page_flip, > + .set_property = drm_atomic_helper_crtc_set_property, > .atomic_duplicate_state = intel_crtc_duplicate_state, > .atomic_destroy_state = intel_crtc_destroy_state, > }; > @@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr > scaler_state->scaler_id = -1; > } > > +static void intel_create_background_color_property(struct drm_device *dev, > + struct intel_crtc *crtc) > +{ > + if (!dev->mode_config.prop_background_color) > + dev->mode_config.prop_background_color = > + drm_mode_create_background_color_property(dev); > + if (!dev->mode_config.prop_background_color) > + return; > + > + drm_object_attach_property(&crtc->base.base, > + dev->mode_config.prop_background_color, > + crtc->base.state->background_color.v); > +} > + > static void intel_crtc_init(struct drm_device *dev, int pipe) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs); > > WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe); > + > + if (INTEL_INFO(dev)->gen >= 9) { > + crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0); > + intel_create_background_color_property(dev, intel_crtc); > + } > + > return; > > fail: > -- > 2.1.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Feb 11, 2016 at 12:00:50PM +0200, Ville Syrjälä wrote: > On Wed, Feb 10, 2016 at 06:32:59PM -0800, Matt Roper wrote: > > Gen9 platforms allow CRTC's to be programmed with a background/canvas > > color below the programmable planes. Let's expose this as a property to > > allow userspace to program a desired value. > > > > This patch is based on earlier work by Chandra Konduru; unfortunately > > the driver has evolved so much since his patches were written (in the > > pre-atomic era) that the functionality had to be pretty much completely > > rewritten for the new i915 atomic internals. > > > > v2: > > - Set initial background color (black) via proper helper function (Bob) > > - Fix debugfs output > > - General rebasing > > > > Cc: Chandra Konduru <chandra.konduru@intel.com> > > Cc: Bob Paauwe <bob.j.paauwe@intel.com> > > Cc: dri-devel@lists.freedesktop.org > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > Documentation/DocBook/gpu.tmpl | 10 +++++++- > > drivers/gpu/drm/i915/i915_debugfs.c | 8 +++++++ > > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++ > > drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++ > > 4 files changed, 72 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > > index fe6b36a..9e003cd 100644 > > --- a/Documentation/DocBook/gpu.tmpl > > +++ b/Documentation/DocBook/gpu.tmpl > > @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev) > > <td valign="top" >TBD</td> > > </tr> > > <tr> > > - <td rowspan="20" valign="top" >i915</td> > > + <td rowspan="21" valign="top" >i915</td> > > <td rowspan="2" valign="top" >Generic</td> > > <td valign="top" >"Broadcast RGB"</td> > > <td valign="top" >ENUM</td> > > @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev) > > <td valign="top" >TBD</td> > > </tr> > > <tr> > > + <td rowspan="1" valign="top" >CRTC</td> > > + <td valign="top" >“background_color”</td> > > + <td valign="top" >RGBA</td> > > + <td valign="top" > </td> > > + <td valign="top" >CRTC</td> > > + <td valign="top" >Background color of regions not covered by a plane</td> > > + </tr> > > + <tr> > > <td rowspan="17" valign="top" >SDVO-TV</td> > > <td valign="top" >“mode”</td> > > <td valign="top" >ENUM</td> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > > index ec0c2a05e..e7352fc 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused) > > intel_scaler_info(m, crtc); > > intel_plane_info(m, crtc); > > } > > + if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) { > > + struct drm_rgba background = pipe_config->base.background_color; > > + > > + seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n", > > + DRM_RGBA_REDBITS(background, 10), > > + DRM_RGBA_GREENBITS(background, 10), > > + DRM_RGBA_BLUEBITS(background, 10)); > > + } > > > > seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", > > yesno(!crtc->cpu_fifo_underrun_disabled), > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 144586e..b0b014d 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells { > > #define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) > > #define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO) > > > > +/* Skylake pipe bottom color */ > > +#define _PIPE_BOTTOM_COLOR_A 0x70034 > > +#define _PIPE_BOTTOM_COLOR_B 0x71034 > > +#define _PIPE_BOTTOM_COLOR_C 0x72034 > > +#define PIPE_BOTTOM_GAMMA_ENABLE (1 << 31) > > +#define PIPE_BOTTOM_CSC_ENABLE (1 << 30) > > +#define PIPE_BOTTOM_COLOR_MASK 0x3FFFFFFF > > +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B) > > + > > /* MIPI DSI registers */ > > > > #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */ > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 836bbdc..a616ac42 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, > > struct drm_i915_private *dev_priv = dev->dev_private; > > struct intel_crtc_state *pipe_config = > > to_intel_crtc_state(crtc->base.state); > > + struct drm_rgba background = pipe_config->base.background_color; > > + uint32_t val; > > > > /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ > > crtc->base.mode = crtc->base.state->mode; > > @@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, > > else if (old_crtc_state->pch_pfit.enabled) > > ironlake_pfit_disable(crtc, true); > > } > > + > > + if (INTEL_INFO(dev)->gen >= 9) { > > + /* BGR 16bpc ==> RGB 10bpc */ > > + val = DRM_RGBA_REDBITS(background, 10) << 20 > > + | DRM_RGBA_GREENBITS(background, 10) << 10 > > + | DRM_RGBA_BLUEBITS(background, 10); > > I'm not a fan of the drm_rgba thing having alpha in the low bits. It > goes against the existing precedent set by eg. the colorkey ioctls > where alpha (if it would be used) would be in the high bits. I think I > complained about this before already, or maybe it was also about > some BGR vs. RGB ordering thing? > > Looks like drm_rgba isn't actually part of this series, and I can't see > it in the tree either, so I guess it comes in via some other series? drm_rgba was in patch #1 of this series; if that didn't come through for you, it's in patchwork here: https://patchwork.freedesktop.org/patch/73224/ We could certainly flip around the internal ordering of bits, but I'm not sure it really matters. The whole point of drm_rgba is to be somewhat opaque so that drivers don't try to work directly on the internal representation (and potentially misinterpret the format). Matt > > > + > > + /* > > + * Set CSC and gamma for bottom color. > > + * > > + * FIXME: We turn these on unconditionally for now to match > > + * how we've setup the various planes. Once the color > > + * management framework lands, it may or may not choose to > > + * set these bits. > > + */ > > + val |= PIPE_BOTTOM_CSC_ENABLE; > > + val |= PIPE_BOTTOM_GAMMA_ENABLE; > > + > > + I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val); > > + } > > } > > > > static void intel_fdi_normal_train(struct drm_crtc *crtc) > > @@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > > pipe_config); > > } > > > > + if (crtc->state->background_color.v != crtc_state->background_color.v) > > + pipe_config->update_pipe = true; > > + > > return ret; > > } > > > > @@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { > > .set_config = drm_atomic_helper_set_config, > > .destroy = intel_crtc_destroy, > > .page_flip = intel_crtc_page_flip, > > + .set_property = drm_atomic_helper_crtc_set_property, > > .atomic_duplicate_state = intel_crtc_duplicate_state, > > .atomic_destroy_state = intel_crtc_destroy_state, > > }; > > @@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr > > scaler_state->scaler_id = -1; > > } > > > > +static void intel_create_background_color_property(struct drm_device *dev, > > + struct intel_crtc *crtc) > > +{ > > + if (!dev->mode_config.prop_background_color) > > + dev->mode_config.prop_background_color = > > + drm_mode_create_background_color_property(dev); > > + if (!dev->mode_config.prop_background_color) > > + return; > > + > > + drm_object_attach_property(&crtc->base.base, > > + dev->mode_config.prop_background_color, > > + crtc->base.state->background_color.v); > > +} > > + > > static void intel_crtc_init(struct drm_device *dev, int pipe) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > @@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > > drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs); > > > > WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe); > > + > > + if (INTEL_INFO(dev)->gen >= 9) { > > + crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0); > > + intel_create_background_color_property(dev, intel_crtc); > > + } > > + > > return; > > > > fail: > > -- > > 2.1.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel OTC
On Thu, Feb 11, 2016 at 08:05:26AM -0800, Matt Roper wrote: > On Thu, Feb 11, 2016 at 12:00:50PM +0200, Ville Syrjälä wrote: > > On Wed, Feb 10, 2016 at 06:32:59PM -0800, Matt Roper wrote: > > > Gen9 platforms allow CRTC's to be programmed with a background/canvas > > > color below the programmable planes. Let's expose this as a property to > > > allow userspace to program a desired value. > > > > > > This patch is based on earlier work by Chandra Konduru; unfortunately > > > the driver has evolved so much since his patches were written (in the > > > pre-atomic era) that the functionality had to be pretty much completely > > > rewritten for the new i915 atomic internals. > > > > > > v2: > > > - Set initial background color (black) via proper helper function (Bob) > > > - Fix debugfs output > > > - General rebasing > > > > > > Cc: Chandra Konduru <chandra.konduru@intel.com> > > > Cc: Bob Paauwe <bob.j.paauwe@intel.com> > > > Cc: dri-devel@lists.freedesktop.org > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > > --- > > > Documentation/DocBook/gpu.tmpl | 10 +++++++- > > > drivers/gpu/drm/i915/i915_debugfs.c | 8 +++++++ > > > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++ > > > drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 72 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > > > index fe6b36a..9e003cd 100644 > > > --- a/Documentation/DocBook/gpu.tmpl > > > +++ b/Documentation/DocBook/gpu.tmpl > > > @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev) > > > <td valign="top" >TBD</td> > > > </tr> > > > <tr> > > > - <td rowspan="20" valign="top" >i915</td> > > > + <td rowspan="21" valign="top" >i915</td> > > > <td rowspan="2" valign="top" >Generic</td> > > > <td valign="top" >"Broadcast RGB"</td> > > > <td valign="top" >ENUM</td> > > > @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev) > > > <td valign="top" >TBD</td> > > > </tr> > > > <tr> > > > + <td rowspan="1" valign="top" >CRTC</td> > > > + <td valign="top" >“background_color”</td> > > > + <td valign="top" >RGBA</td> > > > + <td valign="top" > </td> > > > + <td valign="top" >CRTC</td> > > > + <td valign="top" >Background color of regions not covered by a plane</td> > > > + </tr> > > > + <tr> > > > <td rowspan="17" valign="top" >SDVO-TV</td> > > > <td valign="top" >“mode”</td> > > > <td valign="top" >ENUM</td> > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > > > index ec0c2a05e..e7352fc 100644 > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused) > > > intel_scaler_info(m, crtc); > > > intel_plane_info(m, crtc); > > > } > > > + if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) { > > > + struct drm_rgba background = pipe_config->base.background_color; > > > + > > > + seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n", > > > + DRM_RGBA_REDBITS(background, 10), > > > + DRM_RGBA_GREENBITS(background, 10), > > > + DRM_RGBA_BLUEBITS(background, 10)); > > > + } > > > > > > seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", > > > yesno(!crtc->cpu_fifo_underrun_disabled), > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > index 144586e..b0b014d 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells { > > > #define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) > > > #define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO) > > > > > > +/* Skylake pipe bottom color */ > > > +#define _PIPE_BOTTOM_COLOR_A 0x70034 > > > +#define _PIPE_BOTTOM_COLOR_B 0x71034 > > > +#define _PIPE_BOTTOM_COLOR_C 0x72034 > > > +#define PIPE_BOTTOM_GAMMA_ENABLE (1 << 31) > > > +#define PIPE_BOTTOM_CSC_ENABLE (1 << 30) > > > +#define PIPE_BOTTOM_COLOR_MASK 0x3FFFFFFF > > > +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B) > > > + > > > /* MIPI DSI registers */ > > > > > > #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */ > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index 836bbdc..a616ac42 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > struct intel_crtc_state *pipe_config = > > > to_intel_crtc_state(crtc->base.state); > > > + struct drm_rgba background = pipe_config->base.background_color; > > > + uint32_t val; > > > > > > /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ > > > crtc->base.mode = crtc->base.state->mode; > > > @@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, > > > else if (old_crtc_state->pch_pfit.enabled) > > > ironlake_pfit_disable(crtc, true); > > > } > > > + > > > + if (INTEL_INFO(dev)->gen >= 9) { > > > + /* BGR 16bpc ==> RGB 10bpc */ > > > + val = DRM_RGBA_REDBITS(background, 10) << 20 > > > + | DRM_RGBA_GREENBITS(background, 10) << 10 > > > + | DRM_RGBA_BLUEBITS(background, 10); > > > > I'm not a fan of the drm_rgba thing having alpha in the low bits. It > > goes against the existing precedent set by eg. the colorkey ioctls > > where alpha (if it would be used) would be in the high bits. I think I > > complained about this before already, or maybe it was also about > > some BGR vs. RGB ordering thing? > > > > Looks like drm_rgba isn't actually part of this series, and I can't see > > it in the tree either, so I guess it comes in via some other series? > > drm_rgba was in patch #1 of this series; if that didn't come through for > you, it's in patchwork here: > https://patchwork.freedesktop.org/patch/73224/ It did come through, but somehow I didn't see it. > > We could certainly flip around the internal ordering of bits, but I'm > not sure it really matters. The whole point of drm_rgba is to be > somewhat opaque so that drivers don't try to work directly on the > internal representation (and potentially misinterpret the format). Sure. Just goes against existing practice a bit. And feels a bit strange to have the component that's most often the one that's not present occupy the low bits. OTOH the fact that RGBA is a fairly obscure format hardware wise, might catch people trying using the API the wrong way. > > > Matt > > > > > > + > > > + /* > > > + * Set CSC and gamma for bottom color. > > > + * > > > + * FIXME: We turn these on unconditionally for now to match > > > + * how we've setup the various planes. Once the color > > > + * management framework lands, it may or may not choose to > > > + * set these bits. > > > + */ > > > + val |= PIPE_BOTTOM_CSC_ENABLE; > > > + val |= PIPE_BOTTOM_GAMMA_ENABLE; > > > + > > > + I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val); > > > + } > > > } > > > > > > static void intel_fdi_normal_train(struct drm_crtc *crtc) > > > @@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > > > pipe_config); > > > } > > > > > > + if (crtc->state->background_color.v != crtc_state->background_color.v) > > > + pipe_config->update_pipe = true; > > > + > > > return ret; > > > } > > > > > > @@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { > > > .set_config = drm_atomic_helper_set_config, > > > .destroy = intel_crtc_destroy, > > > .page_flip = intel_crtc_page_flip, > > > + .set_property = drm_atomic_helper_crtc_set_property, > > > .atomic_duplicate_state = intel_crtc_duplicate_state, > > > .atomic_destroy_state = intel_crtc_destroy_state, > > > }; > > > @@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr > > > scaler_state->scaler_id = -1; > > > } > > > > > > +static void intel_create_background_color_property(struct drm_device *dev, > > > + struct intel_crtc *crtc) > > > +{ > > > + if (!dev->mode_config.prop_background_color) > > > + dev->mode_config.prop_background_color = > > > + drm_mode_create_background_color_property(dev); > > > + if (!dev->mode_config.prop_background_color) > > > + return; > > > + > > > + drm_object_attach_property(&crtc->base.base, > > > + dev->mode_config.prop_background_color, > > > + crtc->base.state->background_color.v); > > > +} > > > + > > > static void intel_crtc_init(struct drm_device *dev, int pipe) > > > { > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > @@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > > > drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs); > > > > > > WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe); > > > + > > > + if (INTEL_INFO(dev)->gen >= 9) { > > > + crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0); > > > + intel_create_background_color_property(dev, intel_crtc); > > > + } > > > + > > > return; > > > > > > fail: > > > -- > > > 2.1.4 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Ville Syrjälä > > Intel OTC > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795
Hi Matt, On 11/02/16 02:32, Matt Roper wrote: > Gen9 platforms allow CRTC's to be programmed with a background/canvas > color below the programmable planes. Let's expose this as a property to > allow userspace to program a desired value. > > This patch is based on earlier work by Chandra Konduru; unfortunately > the driver has evolved so much since his patches were written (in the > pre-atomic era) that the functionality had to be pretty much completely > rewritten for the new i915 atomic internals. > > v2: > - Set initial background color (black) via proper helper function (Bob) > - Fix debugfs output > - General rebasing > > Cc: Chandra Konduru <chandra.konduru@intel.com> > Cc: Bob Paauwe <bob.j.paauwe@intel.com> > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > Documentation/DocBook/gpu.tmpl | 10 +++++++- > drivers/gpu/drm/i915/i915_debugfs.c | 8 +++++++ > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++ > drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++ > 4 files changed, 72 insertions(+), 1 deletion(-) > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > index fe6b36a..9e003cd 100644 > --- a/Documentation/DocBook/gpu.tmpl > +++ b/Documentation/DocBook/gpu.tmpl > @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev) > <td valign="top" >TBD</td> > </tr> > <tr> > - <td rowspan="20" valign="top" >i915</td> > + <td rowspan="21" valign="top" >i915</td> > <td rowspan="2" valign="top" >Generic</td> > <td valign="top" >"Broadcast RGB"</td> > <td valign="top" >ENUM</td> > @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev) > <td valign="top" >TBD</td> > </tr> > <tr> > + <td rowspan="1" valign="top" >CRTC</td> > + <td valign="top" >“background_color”</td> > + <td valign="top" >RGBA</td> > + <td valign="top" > </td> > + <td valign="top" >CRTC</td> > + <td valign="top" >Background color of regions not covered by a plane</td> > + </tr> > + <tr> > <td rowspan="17" valign="top" >SDVO-TV</td> > <td valign="top" >“mode”</td> > <td valign="top" >ENUM</td> Why make this property i915 specific? Have you considered make this a generic optional property? Just asking because your first patch seems to imply this could be generic. > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index ec0c2a05e..e7352fc 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused) > intel_scaler_info(m, crtc); > intel_plane_info(m, crtc); > } > + if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) { > + struct drm_rgba background = pipe_config->base.background_color; > + > + seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n", > + DRM_RGBA_REDBITS(background, 10), > + DRM_RGBA_GREENBITS(background, 10), > + DRM_RGBA_BLUEBITS(background, 10)); > + } > > seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", > yesno(!crtc->cpu_fifo_underrun_disabled), > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 144586e..b0b014d 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells { > #define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) > #define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO) > > +/* Skylake pipe bottom color */ > +#define _PIPE_BOTTOM_COLOR_A 0x70034 > +#define _PIPE_BOTTOM_COLOR_B 0x71034 > +#define _PIPE_BOTTOM_COLOR_C 0x72034 > +#define PIPE_BOTTOM_GAMMA_ENABLE (1 << 31) > +#define PIPE_BOTTOM_CSC_ENABLE (1 << 30) > +#define PIPE_BOTTOM_COLOR_MASK 0x3FFFFFFF > +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B) > + > /* MIPI DSI registers */ > > #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */ > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 836bbdc..a616ac42 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc_state *pipe_config = > to_intel_crtc_state(crtc->base.state); > + struct drm_rgba background = pipe_config->base.background_color; > + uint32_t val; > > /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ > crtc->base.mode = crtc->base.state->mode; > @@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, > else if (old_crtc_state->pch_pfit.enabled) > ironlake_pfit_disable(crtc, true); > } > + > + if (INTEL_INFO(dev)->gen >= 9) { > + /* BGR 16bpc ==> RGB 10bpc */ > + val = DRM_RGBA_REDBITS(background, 10) << 20 > + | DRM_RGBA_GREENBITS(background, 10) << 10 > + | DRM_RGBA_BLUEBITS(background, 10); > + > + /* > + * Set CSC and gamma for bottom color. > + * > + * FIXME: We turn these on unconditionally for now to match > + * how we've setup the various planes. Once the color > + * management framework lands, it may or may not choose to > + * set these bits. > + */ > + val |= PIPE_BOTTOM_CSC_ENABLE; > + val |= PIPE_BOTTOM_GAMMA_ENABLE; > + > + I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val); > + } > } > > static void intel_fdi_normal_train(struct drm_crtc *crtc) > @@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > pipe_config); > } > > + if (crtc->state->background_color.v != crtc_state->background_color.v) > + pipe_config->update_pipe = true; > + > return ret; > } > > @@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { > .set_config = drm_atomic_helper_set_config, > .destroy = intel_crtc_destroy, > .page_flip = intel_crtc_page_flip, > + .set_property = drm_atomic_helper_crtc_set_property, > .atomic_duplicate_state = intel_crtc_duplicate_state, > .atomic_destroy_state = intel_crtc_destroy_state, > }; > @@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr > scaler_state->scaler_id = -1; > } > > +static void intel_create_background_color_property(struct drm_device *dev, > + struct intel_crtc *crtc) > +{ > + if (!dev->mode_config.prop_background_color) > + dev->mode_config.prop_background_color = > + drm_mode_create_background_color_property(dev); > + if (!dev->mode_config.prop_background_color) > + return; > + > + drm_object_attach_property(&crtc->base.base, > + dev->mode_config.prop_background_color, > + crtc->base.state->background_color.v); > +} > + > static void intel_crtc_init(struct drm_device *dev, int pipe) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs); > > WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe); > + > + if (INTEL_INFO(dev)->gen >= 9) { > + crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0); > + intel_create_background_color_property(dev, intel_crtc); > + } > + > return; > > fail:
On Thu, Mar 03, 2016 at 03:50:23PM +0000, Lionel Landwerlin wrote: > Hi Matt, > > On 11/02/16 02:32, Matt Roper wrote: > >Gen9 platforms allow CRTC's to be programmed with a background/canvas > >color below the programmable planes. Let's expose this as a property to > >allow userspace to program a desired value. > > > >This patch is based on earlier work by Chandra Konduru; unfortunately > >the driver has evolved so much since his patches were written (in the > >pre-atomic era) that the functionality had to be pretty much completely > >rewritten for the new i915 atomic internals. > > > >v2: > > - Set initial background color (black) via proper helper function (Bob) > > - Fix debugfs output > > - General rebasing > > > >Cc: Chandra Konduru <chandra.konduru@intel.com> > >Cc: Bob Paauwe <bob.j.paauwe@intel.com> > >Cc: dri-devel@lists.freedesktop.org > >Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > >--- > > Documentation/DocBook/gpu.tmpl | 10 +++++++- > > drivers/gpu/drm/i915/i915_debugfs.c | 8 +++++++ > > drivers/gpu/drm/i915/i915_reg.h | 9 +++++++ > > drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++ > > 4 files changed, 72 insertions(+), 1 deletion(-) > > > >diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > >index fe6b36a..9e003cd 100644 > >--- a/Documentation/DocBook/gpu.tmpl > >+++ b/Documentation/DocBook/gpu.tmpl > >@@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev) > > <td valign="top" >TBD</td> > > </tr> > > <tr> > >- <td rowspan="20" valign="top" >i915</td> > >+ <td rowspan="21" valign="top" >i915</td> > > <td rowspan="2" valign="top" >Generic</td> > > <td valign="top" >"Broadcast RGB"</td> > > <td valign="top" >ENUM</td> > >@@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev) > > <td valign="top" >TBD</td> > > </tr> > > <tr> > >+ <td rowspan="1" valign="top" >CRTC</td> > >+ <td valign="top" >“background_color”</td> > >+ <td valign="top" >RGBA</td> > >+ <td valign="top" > </td> > >+ <td valign="top" >CRTC</td> > >+ <td valign="top" >Background color of regions not covered by a plane</td> > >+ </tr> > >+ <tr> > > <td rowspan="17" valign="top" >SDVO-TV</td> > > <td valign="top" >“mode”</td> > > <td valign="top" >ENUM</td> > > Why make this property i915 specific? > Have you considered make this a generic optional property? > > Just asking because your first patch seems to imply this could be generic. Yeah, the intention is for it to be generic. I guess I should have stuck this under the 'DRM/Optional' section of DocBook rather than the i915-specific section. Thanks for pointing that out. Matt > > >diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > >index ec0c2a05e..e7352fc 100644 > >--- a/drivers/gpu/drm/i915/i915_debugfs.c > >+++ b/drivers/gpu/drm/i915/i915_debugfs.c > >@@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused) > > intel_scaler_info(m, crtc); > > intel_plane_info(m, crtc); > > } > >+ if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) { > >+ struct drm_rgba background = pipe_config->base.background_color; > >+ > >+ seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n", > >+ DRM_RGBA_REDBITS(background, 10), > >+ DRM_RGBA_GREENBITS(background, 10), > >+ DRM_RGBA_BLUEBITS(background, 10)); > >+ } > > seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", > > yesno(!crtc->cpu_fifo_underrun_disabled), > >diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > >index 144586e..b0b014d 100644 > >--- a/drivers/gpu/drm/i915/i915_reg.h > >+++ b/drivers/gpu/drm/i915/i915_reg.h > >@@ -7649,6 +7649,15 @@ enum skl_disp_power_wells { > > #define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) > > #define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO) > >+/* Skylake pipe bottom color */ > >+#define _PIPE_BOTTOM_COLOR_A 0x70034 > >+#define _PIPE_BOTTOM_COLOR_B 0x71034 > >+#define _PIPE_BOTTOM_COLOR_C 0x72034 > >+#define PIPE_BOTTOM_GAMMA_ENABLE (1 << 31) > >+#define PIPE_BOTTOM_CSC_ENABLE (1 << 30) > >+#define PIPE_BOTTOM_COLOR_MASK 0x3FFFFFFF > >+#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B) > >+ > > /* MIPI DSI registers */ > > #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */ > >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >index 836bbdc..a616ac42 100644 > >--- a/drivers/gpu/drm/i915/intel_display.c > >+++ b/drivers/gpu/drm/i915/intel_display.c > >@@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, > > struct drm_i915_private *dev_priv = dev->dev_private; > > struct intel_crtc_state *pipe_config = > > to_intel_crtc_state(crtc->base.state); > >+ struct drm_rgba background = pipe_config->base.background_color; > >+ uint32_t val; > > /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ > > crtc->base.mode = crtc->base.state->mode; > >@@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, > > else if (old_crtc_state->pch_pfit.enabled) > > ironlake_pfit_disable(crtc, true); > > } > >+ > >+ if (INTEL_INFO(dev)->gen >= 9) { > >+ /* BGR 16bpc ==> RGB 10bpc */ > >+ val = DRM_RGBA_REDBITS(background, 10) << 20 > >+ | DRM_RGBA_GREENBITS(background, 10) << 10 > >+ | DRM_RGBA_BLUEBITS(background, 10); > >+ > >+ /* > >+ * Set CSC and gamma for bottom color. > >+ * > >+ * FIXME: We turn these on unconditionally for now to match > >+ * how we've setup the various planes. Once the color > >+ * management framework lands, it may or may not choose to > >+ * set these bits. > >+ */ > >+ val |= PIPE_BOTTOM_CSC_ENABLE; > >+ val |= PIPE_BOTTOM_GAMMA_ENABLE; > >+ > >+ I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val); > >+ } > > } > > static void intel_fdi_normal_train(struct drm_crtc *crtc) > >@@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > > pipe_config); > > } > >+ if (crtc->state->background_color.v != crtc_state->background_color.v) > >+ pipe_config->update_pipe = true; > >+ > > return ret; > > } > >@@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { > > .set_config = drm_atomic_helper_set_config, > > .destroy = intel_crtc_destroy, > > .page_flip = intel_crtc_page_flip, > >+ .set_property = drm_atomic_helper_crtc_set_property, > > .atomic_duplicate_state = intel_crtc_duplicate_state, > > .atomic_destroy_state = intel_crtc_destroy_state, > > }; > >@@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr > > scaler_state->scaler_id = -1; > > } > >+static void intel_create_background_color_property(struct drm_device *dev, > >+ struct intel_crtc *crtc) > >+{ > >+ if (!dev->mode_config.prop_background_color) > >+ dev->mode_config.prop_background_color = > >+ drm_mode_create_background_color_property(dev); > >+ if (!dev->mode_config.prop_background_color) > >+ return; > >+ > >+ drm_object_attach_property(&crtc->base.base, > >+ dev->mode_config.prop_background_color, > >+ crtc->base.state->background_color.v); > >+} > >+ > > static void intel_crtc_init(struct drm_device *dev, int pipe) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > >@@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) > > drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs); > > WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe); > >+ > >+ if (INTEL_INFO(dev)->gen >= 9) { > >+ crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0); > >+ intel_create_background_color_property(dev, intel_crtc); > >+ } > >+ > > return; > > fail: >
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index fe6b36a..9e003cd 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >TBD</td> </tr> <tr> - <td rowspan="20" valign="top" >i915</td> + <td rowspan="21" valign="top" >i915</td> <td rowspan="2" valign="top" >Generic</td> <td valign="top" >"Broadcast RGB"</td> <td valign="top" >ENUM</td> @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >TBD</td> </tr> <tr> + <td rowspan="1" valign="top" >CRTC</td> + <td valign="top" >“background_color”</td> + <td valign="top" >RGBA</td> + <td valign="top" > </td> + <td valign="top" >CRTC</td> + <td valign="top" >Background color of regions not covered by a plane</td> + </tr> + <tr> <td rowspan="17" valign="top" >SDVO-TV</td> <td valign="top" >“mode”</td> <td valign="top" >ENUM</td> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index ec0c2a05e..e7352fc 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused) intel_scaler_info(m, crtc); intel_plane_info(m, crtc); } + if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) { + struct drm_rgba background = pipe_config->base.background_color; + + seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n", + DRM_RGBA_REDBITS(background, 10), + DRM_RGBA_GREENBITS(background, 10), + DRM_RGBA_BLUEBITS(background, 10)); + } seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", yesno(!crtc->cpu_fifo_underrun_disabled), diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 144586e..b0b014d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells { #define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME) #define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO) +/* Skylake pipe bottom color */ +#define _PIPE_BOTTOM_COLOR_A 0x70034 +#define _PIPE_BOTTOM_COLOR_B 0x71034 +#define _PIPE_BOTTOM_COLOR_C 0x72034 +#define PIPE_BOTTOM_GAMMA_ENABLE (1 << 31) +#define PIPE_BOTTOM_CSC_ENABLE (1 << 30) +#define PIPE_BOTTOM_COLOR_MASK 0x3FFFFFFF +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B) + /* MIPI DSI registers */ #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */ diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 836bbdc..a616ac42 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc->base.state); + struct drm_rgba background = pipe_config->base.background_color; + uint32_t val; /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ crtc->base.mode = crtc->base.state->mode; @@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, else if (old_crtc_state->pch_pfit.enabled) ironlake_pfit_disable(crtc, true); } + + if (INTEL_INFO(dev)->gen >= 9) { + /* BGR 16bpc ==> RGB 10bpc */ + val = DRM_RGBA_REDBITS(background, 10) << 20 + | DRM_RGBA_GREENBITS(background, 10) << 10 + | DRM_RGBA_BLUEBITS(background, 10); + + /* + * Set CSC and gamma for bottom color. + * + * FIXME: We turn these on unconditionally for now to match + * how we've setup the various planes. Once the color + * management framework lands, it may or may not choose to + * set these bits. + */ + val |= PIPE_BOTTOM_CSC_ENABLE; + val |= PIPE_BOTTOM_GAMMA_ENABLE; + + I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val); + } } static void intel_fdi_normal_train(struct drm_crtc *crtc) @@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, pipe_config); } + if (crtc->state->background_color.v != crtc_state->background_color.v) + pipe_config->update_pipe = true; + return ret; } @@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .destroy = intel_crtc_destroy, .page_flip = intel_crtc_page_flip, + .set_property = drm_atomic_helper_crtc_set_property, .atomic_duplicate_state = intel_crtc_duplicate_state, .atomic_destroy_state = intel_crtc_destroy_state, }; @@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr scaler_state->scaler_id = -1; } +static void intel_create_background_color_property(struct drm_device *dev, + struct intel_crtc *crtc) +{ + if (!dev->mode_config.prop_background_color) + dev->mode_config.prop_background_color = + drm_mode_create_background_color_property(dev); + if (!dev->mode_config.prop_background_color) + return; + + drm_object_attach_property(&crtc->base.base, + dev->mode_config.prop_background_color, + crtc->base.state->background_color.v); +} + static void intel_crtc_init(struct drm_device *dev, int pipe) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs); WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe); + + if (INTEL_INFO(dev)->gen >= 9) { + crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0); + intel_create_background_color_property(dev, intel_crtc); + } + return; fail:
Gen9 platforms allow CRTC's to be programmed with a background/canvas color below the programmable planes. Let's expose this as a property to allow userspace to program a desired value. This patch is based on earlier work by Chandra Konduru; unfortunately the driver has evolved so much since his patches were written (in the pre-atomic era) that the functionality had to be pretty much completely rewritten for the new i915 atomic internals. v2: - Set initial background color (black) via proper helper function (Bob) - Fix debugfs output - General rebasing Cc: Chandra Konduru <chandra.konduru@intel.com> Cc: Bob Paauwe <bob.j.paauwe@intel.com> Cc: dri-devel@lists.freedesktop.org Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- Documentation/DocBook/gpu.tmpl | 10 +++++++- drivers/gpu/drm/i915/i915_debugfs.c | 8 +++++++ drivers/gpu/drm/i915/i915_reg.h | 9 +++++++ drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-)