Previous patch series was here: https://lists.freedesktop.org/archives/dri-devel/2018-December/201949.html
I'm told the ChromeOS userspace code to make use of the background color has been reviewed and is ready for use: * https://chromium-review.googlesource.com/c/chromium/src/+/1278858 * https://chromium-review.googlesource.com/c/chromium/src/+/1278858
So I think ABI-wise we've met the userspace consumer requirements to upstream this; we just need to get reviews on the two i915-specific patches and a clean CI report.
v4.1 is identical to v4 aside from a rebase onto the latest drm-tip.
Matt Roper (3): drm/i915: Force background color to black for gen9+ (v2) drm: Add CRTC background color property (v4) drm/i915/gen9+: Add support for pipe background color (v4)
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_blend.c | 27 +++++++++++++++++++--- drivers/gpu/drm/drm_mode_config.c | 6 +++++ drivers/gpu/drm/i915/i915_debugfs.c | 9 ++++++++ drivers/gpu/drm/i915/i915_reg.h | 6 +++++ drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++++++++++++++++++++ include/drm/drm_blend.h | 1 + include/drm/drm_crtc.h | 12 ++++++++++ include/drm/drm_mode_config.h | 5 +++++ include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++++++ 10 files changed, 138 insertions(+), 3 deletions(-)
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(+)
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
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
Some display controllers can be programmed to present non-black colors for pixels not covered by any plane (or pixels covered by the transparent regions of higher planes). Compositors that want a UI with a solid color background can potentially save memory bandwidth by setting the CRTC background property and using smaller planes to display the rest of the content.
To avoid confusion between different ways of encoding RGB data, we define a standard 64-bit format that should be used for this property's value. Helper functions and macros are provided to generate and dissect values in this standard format with varying component precision values.
v2: - Swap internal representation's blue and red bits to make it easier to read if printed out. (Ville) - Document bgcolor property in drm_blend.c. (Sean Paul) - s/background_color/bgcolor/ for consistency between property name and value storage field. (Sean Paul) - Add a convenience function to attach property to a given crtc.
v3: - Restructure ARGB component extraction macros to be easier to understand and enclose the parameters in () to avoid calculations if expressions are passed. (Sean Paul) - s/rgba/argb/ in helper function/macro names. Even though the idea is to not worry about the internal representation of the u64, it can still be confusing to look at code that uses 'rgba' terminology, but stores values with argb ordering. (Ville)
v4: - Drop the bgcolor_changed flag. (Ville, Brian Starkey) - Clarify in kerneldoc that background color is expected to undergo the same pipe-level degamma/csc/gamma transformations that planes do. (Brian Starkey) - Update kerneldoc to indicate non-opaque colors are allowed, but are generally only useful in special cases such as when writeback connectors are used (Brian Starkey / Eric Anholt)
Cc: dri-devel@lists.freedesktop.org Cc: wei.c.li@intel.com Cc: harish.krupo.kps@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Sean Paul sean@poorly.run Cc: Brian Starkey Brian.Starkey@arm.com Cc: Eric Anholt eric@anholt.net Cc: Stéphane Marchesin marcheu@chromium.org Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Matt Roper matthew.d.roper@intel.com Reviewed-by(v2): Sean Paul sean@poorly.run Reviewed-by: Brian Starkey brian.starkey@arm.com --- drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_blend.c | 27 ++++++++++++++++++++++++--- drivers/gpu/drm/drm_mode_config.c | 6 ++++++ include/drm/drm_blend.h | 1 + include/drm/drm_crtc.h | 12 ++++++++++++ include/drm/drm_mode_config.h | 5 +++++ include/uapi/drm/drm_mode.h | 28 ++++++++++++++++++++++++++++ 7 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 9a1f41adfc67..d569e20e34e3 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -469,6 +469,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc, return -EFAULT;
set_out_fence_for_crtc(state->state, crtc, fence_ptr); + } else if (property == config->bgcolor_property) { + state->bgcolor = val; } else if (crtc->funcs->atomic_set_property) { return crtc->funcs->atomic_set_property(crtc, state, property, val); } else { @@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0; else if (property == config->prop_out_fence_ptr) *val = 0; + else if (property == config->bgcolor_property) + *val = state->bgcolor; else if (crtc->funcs->atomic_get_property) return crtc->funcs->atomic_get_property(crtc, state, property, val); else diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index 0c78ca386cbe..d451ac9e1d6d 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -175,9 +175,22 @@ * plane does not expose the "alpha" property, then this is * assumed to be 1.0 * - * Note that all the property extensions described here apply either to the - * plane or the CRTC (e.g. for the background color, which currently is not - * exposed and assumed to be black). + * The property extensions described above all apply to the plane. Drivers + * may also expose the following crtc property extension: + * + * BACKGROUND_COLOR: + * Background color is setup with drm_crtc_add_bgcolor_property(). It + * controls the ARGB color of a full-screen layer that exists below all + * planes. This color will be used for pixels not covered by any plane + * and may also be blended with plane contents as allowed by a plane's + * alpha values. The background color defaults to black, and is assumed + * to be black for drivers that do not expose this property. Although + * background color isn't a plane, it is assumed that the color provided + * here undergoes the same pipe-level degamma/CSC/gamma transformations + * that planes undergo. Note that the color value provided here includes + * an alpha channel...non-opaque background color values are allowed, + * but are generally only honored in special cases (e.g., when a memory + * writeback connector is in use). */
/** @@ -593,3 +606,11 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane, return 0; } EXPORT_SYMBOL(drm_plane_create_blend_mode_property); + +void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc) +{ + drm_object_attach_property(&crtc->base, + crtc->dev->mode_config.bgcolor_property, + drm_argb(16, 0xffff, 0, 0, 0)); +} +EXPORT_SYMBOL(drm_crtc_add_bgcolor_property); diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 4a1c2023ccf0..8a7c346b3191 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -364,6 +364,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.modifiers_property = prop;
+ prop = drm_property_create_range(dev, 0, "BACKGROUND_COLOR", + 0, GENMASK_ULL(63, 0)); + if (!prop) + return -ENOMEM; + dev->mode_config.bgcolor_property = prop; + return 0; }
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h index 88bdfec3bd88..9e2538dd7b9a 100644 --- a/include/drm/drm_blend.h +++ b/include/drm/drm_blend.h @@ -58,4 +58,5 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, struct drm_atomic_state *state); int drm_plane_create_blend_mode_property(struct drm_plane *plane, unsigned int supported_modes); +void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc); #endif diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 85abd3fe9e83..dbe0b45d5da6 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -274,6 +274,18 @@ struct drm_crtc_state { */ struct drm_property_blob *gamma_lut;
+ /** + * @bgcolor: + * + * RGB value representing the pipe's background color. The background + * color (aka "canvas color") of a pipe is the color that will be used + * for pixels not covered by a plane, or covered by transparent pixels + * of a plane. The value here should be built via drm_argb(); + * individual color components can be extracted with desired precision + * via the DRM_ARGB_*() macros. + */ + u64 bgcolor; + /** * @target_vblank: * diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 1e6cb885994d..0463d3f4ae59 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -836,6 +836,11 @@ struct drm_mode_config { */ struct drm_property *writeback_out_fence_ptr_property;
+ /** + * @bgcolor_property: RGB background color for CRTC. + */ + struct drm_property *bgcolor_property; + /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index a439c2e67896..5f31e6a05bd9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -907,6 +907,34 @@ struct drm_mode_rect { __s32 y2; };
+/* + * Put ARGB values into a standard 64-bit representation that can be used + * for ioctl parameters, inter-driver commmunication, etc. If the component + * values being provided contain less than 16 bits of precision, they'll + * be shifted into the most significant bits. + */ +static inline __u64 +drm_argb(__u8 bpc, __u16 alpha, __u16 red, __u16 green, __u16 blue) +{ + int msb_shift = 16 - bpc; + + return (__u64)alpha << msb_shift << 48 | + (__u64)red << msb_shift << 32 | + (__u64)green << msb_shift << 16 | + (__u64)blue << msb_shift; +} + +/* + * Extract the specified number of bits of a specific color component from a + * standard 64-bit ARGB value. + */ +#define DRM_ARGB_COMP(c, shift, numbits) \ + (__u16)(((c) & 0xFFFFull << (shift)) >> ((shift) + 16 - (numbits))) +#define DRM_ARGB_BLUE(c, numbits) DRM_ARGB_COMP(c, 0, numbits) +#define DRM_ARGB_GREEN(c, numbits) DRM_ARGB_COMP(c, 16, numbits) +#define DRM_ARGB_RED(c, numbits) DRM_ARGB_COMP(c, 32, numbits) +#define DRM_ARGB_ALPHA(c, numbits) DRM_ARGB_COMP(c, 48, numbits) + #if defined(__cplusplus) } #endif
On Wed, Jan 30, 2019 at 10:51:21AM -0800, Matt Roper wrote:
Some display controllers can be programmed to present non-black colors for pixels not covered by any plane (or pixels covered by the transparent regions of higher planes). Compositors that want a UI with a solid color background can potentially save memory bandwidth by setting the CRTC background property and using smaller planes to display the rest of the content.
To avoid confusion between different ways of encoding RGB data, we define a standard 64-bit format that should be used for this property's value. Helper functions and macros are provided to generate and dissect values in this standard format with varying component precision values.
v2:
- Swap internal representation's blue and red bits to make it easier to read if printed out. (Ville)
- Document bgcolor property in drm_blend.c. (Sean Paul)
- s/background_color/bgcolor/ for consistency between property name and value storage field. (Sean Paul)
- Add a convenience function to attach property to a given crtc.
v3:
- Restructure ARGB component extraction macros to be easier to understand and enclose the parameters in () to avoid calculations if expressions are passed. (Sean Paul)
- s/rgba/argb/ in helper function/macro names. Even though the idea is to not worry about the internal representation of the u64, it can still be confusing to look at code that uses 'rgba' terminology, but stores values with argb ordering. (Ville)
v4:
- Drop the bgcolor_changed flag. (Ville, Brian Starkey)
- Clarify in kerneldoc that background color is expected to undergo the same pipe-level degamma/csc/gamma transformations that planes do. (Brian Starkey)
- Update kerneldoc to indicate non-opaque colors are allowed, but are generally only useful in special cases such as when writeback connectors are used (Brian Starkey / Eric Anholt)
Cc: dri-devel@lists.freedesktop.org Cc: wei.c.li@intel.com Cc: harish.krupo.kps@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Sean Paul sean@poorly.run Cc: Brian Starkey Brian.Starkey@arm.com Cc: Eric Anholt eric@anholt.net Cc: Stéphane Marchesin marcheu@chromium.org Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Matt Roper matthew.d.roper@intel.com Reviewed-by(v2): Sean Paul sean@poorly.run Reviewed-by: Brian Starkey brian.starkey@arm.com
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_blend.c | 27 ++++++++++++++++++++++++--- drivers/gpu/drm/drm_mode_config.c | 6 ++++++ include/drm/drm_blend.h | 1 + include/drm/drm_crtc.h | 12 ++++++++++++ include/drm/drm_mode_config.h | 5 +++++ include/uapi/drm/drm_mode.h | 28 ++++++++++++++++++++++++++++ 7 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 9a1f41adfc67..d569e20e34e3 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -469,6 +469,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc, return -EFAULT;
set_out_fence_for_crtc(state->state, crtc, fence_ptr);
- } else if (property == config->bgcolor_property) {
state->bgcolor = val;@@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0; else if (property == config->prop_out_fence_ptr) *val = 0;
- else if (property == config->bgcolor_property)
*val = state->bgcolor;diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index 0c78ca386cbe..d451ac9e1d6d 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -175,9 +175,22 @@
plane does not expose the "alpha" property, then this isassumed to be 1.0
- Note that all the property extensions described here apply either to the
- plane or the CRTC (e.g. for the background color, which currently is not
- exposed and assumed to be black).
- The property extensions described above all apply to the plane. Drivers
- may also expose the following crtc property extension:
- BACKGROUND_COLOR:
- Background color is setup with drm_crtc_add_bgcolor_property(). It
- controls the ARGB color of a full-screen layer that exists below all
- planes. This color will be used for pixels not covered by any plane
- and may also be blended with plane contents as allowed by a plane's
- alpha values. The background color defaults to black, and is assumed
- to be black for drivers that do not expose this property. Although
- background color isn't a plane, it is assumed that the color provided
- here undergoes the same pipe-level degamma/CSC/gamma transformations
- that planes undergo. Note that the color value provided here includes
- an alpha channel...non-opaque background color values are allowed,
- but are generally only honored in special cases (e.g., when a memory
- writeback connector is in use).
What would the alpha<1.0 do in that case? Blend the writeback output with what's already there?
*/
/** @@ -593,3 +606,11 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane, return 0; } EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc) +{
- drm_object_attach_property(&crtc->base,
crtc->dev->mode_config.bgcolor_property,drm_argb(16, 0xffff, 0, 0, 0));
if (crtc->state) crtc->state->bg = default value; ?
+} +EXPORT_SYMBOL(drm_crtc_add_bgcolor_property); diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 4a1c2023ccf0..8a7c346b3191 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -364,6 +364,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.modifiers_property = prop;
- prop = drm_property_create_range(dev, 0, "BACKGROUND_COLOR",
0, GENMASK_ULL(63, 0));- if (!prop)
return -ENOMEM;- dev->mode_config.bgcolor_property = prop;
- return 0;
}
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h index 88bdfec3bd88..9e2538dd7b9a 100644 --- a/include/drm/drm_blend.h +++ b/include/drm/drm_blend.h @@ -58,4 +58,5 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, struct drm_atomic_state *state); int drm_plane_create_blend_mode_property(struct drm_plane *plane, unsigned int supported_modes); +void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc); #endif diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 85abd3fe9e83..dbe0b45d5da6 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -274,6 +274,18 @@ struct drm_crtc_state { */ struct drm_property_blob *gamma_lut;
- /**
* @bgcolor:** RGB value representing the pipe's background color. The background* color (aka "canvas color") of a pipe is the color that will be used* for pixels not covered by a plane, or covered by transparent pixels* of a plane. The value here should be built via drm_argb();* individual color components can be extracted with desired precision* via the DRM_ARGB_*() macros.*/- u64 bgcolor;
- /**
- @target_vblank:
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 1e6cb885994d..0463d3f4ae59 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -836,6 +836,11 @@ struct drm_mode_config { */ struct drm_property *writeback_out_fence_ptr_property;
- /**
* @bgcolor_property: RGB background color for CRTC.*/- struct drm_property *bgcolor_property;
- /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index a439c2e67896..5f31e6a05bd9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -907,6 +907,34 @@ struct drm_mode_rect { __s32 y2; };
+/*
- Put ARGB values into a standard 64-bit representation that can be used
- for ioctl parameters, inter-driver commmunication, etc. If the component
- values being provided contain less than 16 bits of precision, they'll
- be shifted into the most significant bits.
- */
+static inline __u64 +drm_argb(__u8 bpc, __u16 alpha, __u16 red, __u16 green, __u16 blue) +{
- int msb_shift = 16 - bpc;
- return (__u64)alpha << msb_shift << 48 |
(__u64)red << msb_shift << 32 |(__u64)green << msb_shift << 16 |(__u64)blue << msb_shift;+}
+/*
- Extract the specified number of bits of a specific color component from a
- standard 64-bit ARGB value.
- */
+#define DRM_ARGB_COMP(c, shift, numbits) \
- (__u16)(((c) & 0xFFFFull << (shift)) >> ((shift) + 16 - (numbits)))
+#define DRM_ARGB_BLUE(c, numbits) DRM_ARGB_COMP(c, 0, numbits) +#define DRM_ARGB_GREEN(c, numbits) DRM_ARGB_COMP(c, 16, numbits) +#define DRM_ARGB_RED(c, numbits) DRM_ARGB_COMP(c, 32, numbits) +#define DRM_ARGB_ALPHA(c, numbits) DRM_ARGB_COMP(c, 48, numbits)
#if defined(__cplusplus) }
#endif
2.14.5
On Wed, Jan 30, 2019 at 11:01:25PM +0200, Ville Syrjälä wrote:
On Wed, Jan 30, 2019 at 10:51:21AM -0800, Matt Roper wrote:
Some display controllers can be programmed to present non-black colors for pixels not covered by any plane (or pixels covered by the transparent regions of higher planes). Compositors that want a UI with a solid color background can potentially save memory bandwidth by setting the CRTC background property and using smaller planes to display the rest of the content.
To avoid confusion between different ways of encoding RGB data, we define a standard 64-bit format that should be used for this property's value. Helper functions and macros are provided to generate and dissect values in this standard format with varying component precision values.
v2:
- Swap internal representation's blue and red bits to make it easier to read if printed out. (Ville)
- Document bgcolor property in drm_blend.c. (Sean Paul)
- s/background_color/bgcolor/ for consistency between property name and value storage field. (Sean Paul)
- Add a convenience function to attach property to a given crtc.
v3:
- Restructure ARGB component extraction macros to be easier to understand and enclose the parameters in () to avoid calculations if expressions are passed. (Sean Paul)
- s/rgba/argb/ in helper function/macro names. Even though the idea is to not worry about the internal representation of the u64, it can still be confusing to look at code that uses 'rgba' terminology, but stores values with argb ordering. (Ville)
v4:
- Drop the bgcolor_changed flag. (Ville, Brian Starkey)
- Clarify in kerneldoc that background color is expected to undergo the same pipe-level degamma/csc/gamma transformations that planes do. (Brian Starkey)
- Update kerneldoc to indicate non-opaque colors are allowed, but are generally only useful in special cases such as when writeback connectors are used (Brian Starkey / Eric Anholt)
Cc: dri-devel@lists.freedesktop.org Cc: wei.c.li@intel.com Cc: harish.krupo.kps@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Sean Paul sean@poorly.run Cc: Brian Starkey Brian.Starkey@arm.com Cc: Eric Anholt eric@anholt.net Cc: Stéphane Marchesin marcheu@chromium.org Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Matt Roper matthew.d.roper@intel.com Reviewed-by(v2): Sean Paul sean@poorly.run Reviewed-by: Brian Starkey brian.starkey@arm.com
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_blend.c | 27 ++++++++++++++++++++++++--- drivers/gpu/drm/drm_mode_config.c | 6 ++++++ include/drm/drm_blend.h | 1 + include/drm/drm_crtc.h | 12 ++++++++++++ include/drm/drm_mode_config.h | 5 +++++ include/uapi/drm/drm_mode.h | 28 ++++++++++++++++++++++++++++ 7 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 9a1f41adfc67..d569e20e34e3 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -469,6 +469,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc, return -EFAULT;
set_out_fence_for_crtc(state->state, crtc, fence_ptr);
- } else if (property == config->bgcolor_property) {
state->bgcolor = val;@@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0; else if (property == config->prop_out_fence_ptr) *val = 0;
- else if (property == config->bgcolor_property)
*val = state->bgcolor;diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index 0c78ca386cbe..d451ac9e1d6d 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -175,9 +175,22 @@
plane does not expose the "alpha" property, then this isassumed to be 1.0
- Note that all the property extensions described here apply either to the
- plane or the CRTC (e.g. for the background color, which currently is not
- exposed and assumed to be black).
- The property extensions described above all apply to the plane. Drivers
- may also expose the following crtc property extension:
- BACKGROUND_COLOR:
- Background color is setup with drm_crtc_add_bgcolor_property(). It
- controls the ARGB color of a full-screen layer that exists below all
- planes. This color will be used for pixels not covered by any plane
- and may also be blended with plane contents as allowed by a plane's
- alpha values. The background color defaults to black, and is assumed
- to be black for drivers that do not expose this property. Although
- background color isn't a plane, it is assumed that the color provided
- here undergoes the same pipe-level degamma/CSC/gamma transformations
- that planes undergo. Note that the color value provided here includes
- an alpha channel...non-opaque background color values are allowed,
- but are generally only honored in special cases (e.g., when a memory
- writeback connector is in use).
What would the alpha<1.0 do in that case? Blend the writeback output with what's already there?
I think it's more for cases where your hardware's pipe writes back actual ARGB data to memory, including an alpha component, rather than just RGB or XRGB. You could then take that ARGB writeback buffer and pass it through some other form of useful compositing if the bgcolor pixels have a non-opaque alpha.
Matt
*/
/** @@ -593,3 +606,11 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane, return 0; } EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc) +{
- drm_object_attach_property(&crtc->base,
crtc->dev->mode_config.bgcolor_property,drm_argb(16, 0xffff, 0, 0, 0));if (crtc->state) crtc->state->bg = default value; ?
+} +EXPORT_SYMBOL(drm_crtc_add_bgcolor_property); diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 4a1c2023ccf0..8a7c346b3191 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -364,6 +364,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.modifiers_property = prop;
- prop = drm_property_create_range(dev, 0, "BACKGROUND_COLOR",
0, GENMASK_ULL(63, 0));- if (!prop)
return -ENOMEM;- dev->mode_config.bgcolor_property = prop;
- return 0;
}
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h index 88bdfec3bd88..9e2538dd7b9a 100644 --- a/include/drm/drm_blend.h +++ b/include/drm/drm_blend.h @@ -58,4 +58,5 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, struct drm_atomic_state *state); int drm_plane_create_blend_mode_property(struct drm_plane *plane, unsigned int supported_modes); +void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc); #endif diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 85abd3fe9e83..dbe0b45d5da6 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -274,6 +274,18 @@ struct drm_crtc_state { */ struct drm_property_blob *gamma_lut;
- /**
* @bgcolor:** RGB value representing the pipe's background color. The background* color (aka "canvas color") of a pipe is the color that will be used* for pixels not covered by a plane, or covered by transparent pixels* of a plane. The value here should be built via drm_argb();* individual color components can be extracted with desired precision* via the DRM_ARGB_*() macros.*/- u64 bgcolor;
- /**
- @target_vblank:
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 1e6cb885994d..0463d3f4ae59 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -836,6 +836,11 @@ struct drm_mode_config { */ struct drm_property *writeback_out_fence_ptr_property;
- /**
* @bgcolor_property: RGB background color for CRTC.*/- struct drm_property *bgcolor_property;
- /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index a439c2e67896..5f31e6a05bd9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -907,6 +907,34 @@ struct drm_mode_rect { __s32 y2; };
+/*
- Put ARGB values into a standard 64-bit representation that can be used
- for ioctl parameters, inter-driver commmunication, etc. If the component
- values being provided contain less than 16 bits of precision, they'll
- be shifted into the most significant bits.
- */
+static inline __u64 +drm_argb(__u8 bpc, __u16 alpha, __u16 red, __u16 green, __u16 blue) +{
- int msb_shift = 16 - bpc;
- return (__u64)alpha << msb_shift << 48 |
(__u64)red << msb_shift << 32 |(__u64)green << msb_shift << 16 |(__u64)blue << msb_shift;+}
+/*
- Extract the specified number of bits of a specific color component from a
- standard 64-bit ARGB value.
- */
+#define DRM_ARGB_COMP(c, shift, numbits) \
- (__u16)(((c) & 0xFFFFull << (shift)) >> ((shift) + 16 - (numbits)))
+#define DRM_ARGB_BLUE(c, numbits) DRM_ARGB_COMP(c, 0, numbits) +#define DRM_ARGB_GREEN(c, numbits) DRM_ARGB_COMP(c, 16, numbits) +#define DRM_ARGB_RED(c, numbits) DRM_ARGB_COMP(c, 32, numbits) +#define DRM_ARGB_ALPHA(c, numbits) DRM_ARGB_COMP(c, 48, numbits)
#if defined(__cplusplus) }
#endif
2.14.5
-- Ville Syrjälä Intel
On Wed, Jan 30, 2019 at 06:11:16PM -0800, Matt Roper wrote:
On Wed, Jan 30, 2019 at 11:01:25PM +0200, Ville Syrjälä wrote:
On Wed, Jan 30, 2019 at 10:51:21AM -0800, Matt Roper wrote:
Some display controllers can be programmed to present non-black colors for pixels not covered by any plane (or pixels covered by the transparent regions of higher planes). Compositors that want a UI with a solid color background can potentially save memory bandwidth by setting the CRTC background property and using smaller planes to display the rest of the content.
To avoid confusion between different ways of encoding RGB data, we define a standard 64-bit format that should be used for this property's value. Helper functions and macros are provided to generate and dissect values in this standard format with varying component precision values.
v2:
- Swap internal representation's blue and red bits to make it easier to read if printed out. (Ville)
- Document bgcolor property in drm_blend.c. (Sean Paul)
- s/background_color/bgcolor/ for consistency between property name and value storage field. (Sean Paul)
- Add a convenience function to attach property to a given crtc.
v3:
- Restructure ARGB component extraction macros to be easier to understand and enclose the parameters in () to avoid calculations if expressions are passed. (Sean Paul)
- s/rgba/argb/ in helper function/macro names. Even though the idea is to not worry about the internal representation of the u64, it can still be confusing to look at code that uses 'rgba' terminology, but stores values with argb ordering. (Ville)
v4:
- Drop the bgcolor_changed flag. (Ville, Brian Starkey)
- Clarify in kerneldoc that background color is expected to undergo the same pipe-level degamma/csc/gamma transformations that planes do. (Brian Starkey)
- Update kerneldoc to indicate non-opaque colors are allowed, but are generally only useful in special cases such as when writeback connectors are used (Brian Starkey / Eric Anholt)
Cc: dri-devel@lists.freedesktop.org Cc: wei.c.li@intel.com Cc: harish.krupo.kps@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Sean Paul sean@poorly.run Cc: Brian Starkey Brian.Starkey@arm.com Cc: Eric Anholt eric@anholt.net Cc: Stéphane Marchesin marcheu@chromium.org Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Matt Roper matthew.d.roper@intel.com Reviewed-by(v2): Sean Paul sean@poorly.run Reviewed-by: Brian Starkey brian.starkey@arm.com
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_blend.c | 27 ++++++++++++++++++++++++--- drivers/gpu/drm/drm_mode_config.c | 6 ++++++ include/drm/drm_blend.h | 1 + include/drm/drm_crtc.h | 12 ++++++++++++ include/drm/drm_mode_config.h | 5 +++++ include/uapi/drm/drm_mode.h | 28 ++++++++++++++++++++++++++++ 7 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 9a1f41adfc67..d569e20e34e3 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -469,6 +469,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc, return -EFAULT;
set_out_fence_for_crtc(state->state, crtc, fence_ptr);
- } else if (property == config->bgcolor_property) {
state->bgcolor = val;@@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0; else if (property == config->prop_out_fence_ptr) *val = 0;
- else if (property == config->bgcolor_property)
*val = state->bgcolor;diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c index 0c78ca386cbe..d451ac9e1d6d 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -175,9 +175,22 @@
plane does not expose the "alpha" property, then this isassumed to be 1.0
- Note that all the property extensions described here apply either to the
- plane or the CRTC (e.g. for the background color, which currently is not
- exposed and assumed to be black).
- The property extensions described above all apply to the plane. Drivers
- may also expose the following crtc property extension:
- BACKGROUND_COLOR:
- Background color is setup with drm_crtc_add_bgcolor_property(). It
- controls the ARGB color of a full-screen layer that exists below all
- planes. This color will be used for pixels not covered by any plane
- and may also be blended with plane contents as allowed by a plane's
- alpha values. The background color defaults to black, and is assumed
- to be black for drivers that do not expose this property. Although
- background color isn't a plane, it is assumed that the color provided
- here undergoes the same pipe-level degamma/CSC/gamma transformations
- that planes undergo. Note that the color value provided here includes
- an alpha channel...non-opaque background color values are allowed,
- but are generally only honored in special cases (e.g., when a memory
- writeback connector is in use).
What would the alpha<1.0 do in that case? Blend the writeback output with what's already there?
I think it's more for cases where your hardware's pipe writes back actual ARGB data to memory, including an alpha component, rather than just RGB or XRGB. You could then take that ARGB writeback buffer and pass it through some other form of useful compositing if the bgcolor pixels have a non-opaque alpha.
I guess someone should specify the blend equation for the alpha channel as well. I presume it should just be 'da = sa + (1-sa)*da' regardless of the plane pre-multiply setting?
Matt
*/
/** @@ -593,3 +606,11 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane, return 0; } EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc) +{
- drm_object_attach_property(&crtc->base,
crtc->dev->mode_config.bgcolor_property,drm_argb(16, 0xffff, 0, 0, 0));if (crtc->state) crtc->state->bg = default value; ?
+} +EXPORT_SYMBOL(drm_crtc_add_bgcolor_property); diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 4a1c2023ccf0..8a7c346b3191 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -364,6 +364,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.modifiers_property = prop;
- prop = drm_property_create_range(dev, 0, "BACKGROUND_COLOR",
0, GENMASK_ULL(63, 0));- if (!prop)
return -ENOMEM;- dev->mode_config.bgcolor_property = prop;
- return 0;
}
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h index 88bdfec3bd88..9e2538dd7b9a 100644 --- a/include/drm/drm_blend.h +++ b/include/drm/drm_blend.h @@ -58,4 +58,5 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, struct drm_atomic_state *state); int drm_plane_create_blend_mode_property(struct drm_plane *plane, unsigned int supported_modes); +void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc); #endif diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 85abd3fe9e83..dbe0b45d5da6 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -274,6 +274,18 @@ struct drm_crtc_state { */ struct drm_property_blob *gamma_lut;
- /**
* @bgcolor:** RGB value representing the pipe's background color. The background* color (aka "canvas color") of a pipe is the color that will be used* for pixels not covered by a plane, or covered by transparent pixels* of a plane. The value here should be built via drm_argb();* individual color components can be extracted with desired precision* via the DRM_ARGB_*() macros.*/- u64 bgcolor;
- /**
- @target_vblank:
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 1e6cb885994d..0463d3f4ae59 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -836,6 +836,11 @@ struct drm_mode_config { */ struct drm_property *writeback_out_fence_ptr_property;
- /**
* @bgcolor_property: RGB background color for CRTC.*/- struct drm_property *bgcolor_property;
- /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index a439c2e67896..5f31e6a05bd9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -907,6 +907,34 @@ struct drm_mode_rect { __s32 y2; };
+/*
- Put ARGB values into a standard 64-bit representation that can be used
- for ioctl parameters, inter-driver commmunication, etc. If the component
- values being provided contain less than 16 bits of precision, they'll
- be shifted into the most significant bits.
- */
+static inline __u64 +drm_argb(__u8 bpc, __u16 alpha, __u16 red, __u16 green, __u16 blue) +{
- int msb_shift = 16 - bpc;
- return (__u64)alpha << msb_shift << 48 |
(__u64)red << msb_shift << 32 |(__u64)green << msb_shift << 16 |(__u64)blue << msb_shift;+}
+/*
- Extract the specified number of bits of a specific color component from a
- standard 64-bit ARGB value.
- */
+#define DRM_ARGB_COMP(c, shift, numbits) \
- (__u16)(((c) & 0xFFFFull << (shift)) >> ((shift) + 16 - (numbits)))
+#define DRM_ARGB_BLUE(c, numbits) DRM_ARGB_COMP(c, 0, numbits) +#define DRM_ARGB_GREEN(c, numbits) DRM_ARGB_COMP(c, 16, numbits) +#define DRM_ARGB_RED(c, numbits) DRM_ARGB_COMP(c, 32, numbits) +#define DRM_ARGB_ALPHA(c, numbits) DRM_ARGB_COMP(c, 48, numbits)
#if defined(__cplusplus) }
#endif
2.14.5
-- Ville Syrjälä Intel
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795
Gen9+ platforms allow CRTC's to be programmed with a background/canvas color below the programmable planes. Let's expose this for use by compositors.
v2: - Split out bgcolor sanitization and programming of csc/gamma bits to a separate patch that we can land before the ABI changes are ready to go in. (Ville) - Change a temporary variable name to be more consistent with other similar functions. (Ville) - Change register name to SKL_CANVAS for consistency with the CHV_CANVAS register.
v3: - Switch register name back to SKL_BOTTOM_COLOR. (Ville) - Use non-_FW register write. (Ville) - Minor parameter rename for consistency. (Ville)
v4: - Removed use of bgcolor_changed flag.
Cc: dri-devel@lists.freedesktop.org Cc: wei.c.li@intel.com Cc: harish.krupo.kps@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 9 +++++++ drivers/gpu/drm/i915/intel_display.c | 46 +++++++++++++++++++++++++++--------- 2 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index fa2c226fc779..8b07dd05c54e 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3092,6 +3092,15 @@ static int i915_display_info(struct seq_file *m, void *unused) intel_plane_info(m, crtc); }
+ if (INTEL_GEN(dev_priv) >= 9 && pipe_config->base.active) { + uint64_t background = pipe_config->base.bgcolor; + + seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n", + DRM_ARGB_RED(background, 10), + DRM_ARGB_GREEN(background, 10), + DRM_ARGB_BLUE(background, 10)); + } + seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", yesno(!crtc->cpu_fifo_underrun_disabled), yesno(!crtc->pch_fifo_underrun_disabled)); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a025efb1d7c6..bc78743e1411 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3896,6 +3896,28 @@ void intel_finish_reset(struct drm_i915_private *dev_priv) clear_bit(I915_RESET_MODESET, &dev_priv->gpu_error.flags); }
+static void +skl_update_background_color(const struct intel_crtc_state *crtc_state) +{ + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + uint64_t propval = crtc_state->base.bgcolor; + uint32_t tmp; + + /* Hardware is programmed with 10 bits of precision */ + tmp = DRM_ARGB_RED(propval, 10) << 20 + | DRM_ARGB_GREEN(propval, 10) << 10 + | DRM_ARGB_BLUE(propval, 10); + + /* + * Set CSC and gamma for bottom color to ensure background pixels + * receive the same color transformations as plane content. + */ + tmp |= SKL_BOTTOM_COLOR_CSC_ENABLE | SKL_BOTTOM_COLOR_GAMMA_ENABLE; + + I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe), tmp); +} + static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state, const struct intel_crtc_state *new_crtc_state) { @@ -3931,15 +3953,8 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta 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); + skl_update_background_color(new_crtc_state); }
static void intel_fdi_normal_train(struct intel_crtc *crtc) @@ -11042,6 +11057,8 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state); + struct drm_crtc_state *old_crtc_state = + drm_atomic_get_old_crtc_state(crtc_state->state, crtc); int ret; bool mode_changed = needs_modeset(crtc_state);
@@ -11069,6 +11086,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, crtc_state->planes_changed = true; }
+ if (crtc_state->bgcolor != old_crtc_state->bgcolor) + pipe_config->update_pipe = true; + ret = 0; if (dev_priv->display.compute_pipe_wm) { ret = dev_priv->display.compute_pipe_wm(pipe_config); @@ -14238,6 +14258,9 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
+ if (INTEL_GEN(dev_priv) >= 9) + drm_crtc_add_bgcolor_property(&intel_crtc->base); + return 0;
fail: @@ -15478,6 +15501,9 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+ /* Always force bgcolor to solid black */ + crtc_state->base.bgcolor = drm_argb(16, 0xFFFF, 0, 0, 0); + /* Clear any frame start delays used for debugging left by the BIOS */ if (crtc->active && !transcoder_is_dsi(cpu_transcoder)) { i915_reg_t reg = PIPECONF(cpu_transcoder); @@ -15504,9 +15530,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, * 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); + skl_update_background_color(crtc_state); }
/* Adjust the state of the output pipe according to whether we
On Wed, Jan 30, 2019 at 10:51:22AM -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 for use by compositors.
v2:
- Split out bgcolor sanitization and programming of csc/gamma bits to a separate patch that we can land before the ABI changes are ready to go in. (Ville)
- Change a temporary variable name to be more consistent with other similar functions. (Ville)
- Change register name to SKL_CANVAS for consistency with the CHV_CANVAS register.
v3:
- Switch register name back to SKL_BOTTOM_COLOR. (Ville)
- Use non-_FW register write. (Ville)
- Minor parameter rename for consistency. (Ville)
v4:
- Removed use of bgcolor_changed flag.
Cc: dri-devel@lists.freedesktop.org Cc: wei.c.li@intel.com Cc: harish.krupo.kps@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
drivers/gpu/drm/i915/i915_debugfs.c | 9 +++++++ drivers/gpu/drm/i915/intel_display.c | 46 +++++++++++++++++++++++++++--------- 2 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index fa2c226fc779..8b07dd05c54e 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3092,6 +3092,15 @@ static int i915_display_info(struct seq_file *m, void *unused) intel_plane_info(m, crtc); }
if (INTEL_GEN(dev_priv) >= 9 && pipe_config->base.active) {uint64_t background = pipe_config->base.bgcolor;seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",DRM_ARGB_RED(background, 10),DRM_ARGB_GREEN(background, 10),DRM_ARGB_BLUE(background, 10));}- seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", yesno(!crtc->cpu_fifo_underrun_disabled), yesno(!crtc->pch_fifo_underrun_disabled));
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a025efb1d7c6..bc78743e1411 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3896,6 +3896,28 @@ void intel_finish_reset(struct drm_i915_private *dev_priv) clear_bit(I915_RESET_MODESET, &dev_priv->gpu_error.flags); }
+static void +skl_update_background_color(const struct intel_crtc_state *crtc_state) +{
- struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
- struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
- uint64_t propval = crtc_state->base.bgcolor;
- uint32_t tmp;
- /* Hardware is programmed with 10 bits of precision */
- tmp = DRM_ARGB_RED(propval, 10) << 20
| DRM_ARGB_GREEN(propval, 10) << 10| DRM_ARGB_BLUE(propval, 10);- /*
* Set CSC and gamma for bottom color to ensure background pixels* receive the same color transformations as plane content.*/- tmp |= SKL_BOTTOM_COLOR_CSC_ENABLE | SKL_BOTTOM_COLOR_GAMMA_ENABLE;
- I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe), tmp);
+}
Readout + state check would be nice too.
Anyways Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state, const struct intel_crtc_state *new_crtc_state) { @@ -3931,15 +3953,8 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta 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.*/I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe),SKL_BOTTOM_COLOR_GAMMA_ENABLE |SKL_BOTTOM_COLOR_CSC_ENABLE);
skl_update_background_color(new_crtc_state);}
static void intel_fdi_normal_train(struct intel_crtc *crtc) @@ -11042,6 +11057,8 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state);
- struct drm_crtc_state *old_crtc_state =
drm_atomic_get_old_crtc_state(crtc_state->state, crtc);@@ -11069,6 +11086,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, crtc_state->planes_changed = true; }
- if (crtc_state->bgcolor != old_crtc_state->bgcolor)
pipe_config->update_pipe = true;- ret = 0; if (dev_priv->display.compute_pipe_wm) { ret = dev_priv->display.compute_pipe_wm(pipe_config);
@@ -14238,6 +14258,9 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
- if (INTEL_GEN(dev_priv) >= 9)
drm_crtc_add_bgcolor_property(&intel_crtc->base);- return 0;
fail: @@ -15478,6 +15501,9 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
- /* Always force bgcolor to solid black */
- crtc_state->base.bgcolor = drm_argb(16, 0xFFFF, 0, 0, 0);
- /* Clear any frame start delays used for debugging left by the BIOS */ if (crtc->active && !transcoder_is_dsi(cpu_transcoder)) { i915_reg_t reg = PIPECONF(cpu_transcoder);
@@ -15504,9 +15530,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, * 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);
skl_update_background_color(crtc_state);}
/* Adjust the state of the output pipe according to whether we
-- 2.14.5
On Wed, Jan 30, 2019 at 10:51:19AM -0800, Matt Roper wrote:
Previous patch series was here: https://lists.freedesktop.org/archives/dri-devel/2018-December/201949.html
I'm told the ChromeOS userspace code to make use of the background color has been reviewed and is ready for use:
Woops, the second link here should have been to
https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1...
which I believe is some unit tests to go along with the main userspace code.
Matt
So I think ABI-wise we've met the userspace consumer requirements to upstream this; we just need to get reviews on the two i915-specific patches and a clean CI report.
v4.1 is identical to v4 aside from a rebase onto the latest drm-tip.
Matt Roper (3): drm/i915: Force background color to black for gen9+ (v2) drm: Add CRTC background color property (v4) drm/i915/gen9+: Add support for pipe background color (v4)
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_blend.c | 27 +++++++++++++++++++--- drivers/gpu/drm/drm_mode_config.c | 6 +++++ drivers/gpu/drm/i915/i915_debugfs.c | 9 ++++++++ drivers/gpu/drm/i915/i915_reg.h | 6 +++++ drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++++++++++++++++++++ include/drm/drm_blend.h | 1 + include/drm/drm_crtc.h | 12 ++++++++++ include/drm/drm_mode_config.h | 5 +++++ include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++++++ 10 files changed, 138 insertions(+), 3 deletions(-)
-- 2.14.5
On Wed, Jan 30, 2019 at 10:56:26AM -0800, Matt Roper wrote:
On Wed, Jan 30, 2019 at 10:51:19AM -0800, Matt Roper wrote:
Previous patch series was here: https://lists.freedesktop.org/archives/dri-devel/2018-December/201949.html
I'm told the ChromeOS userspace code to make use of the background color has been reviewed and is ready for use:
Woops, the second link here should have been to
https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1...
which I believe is some unit tests to go along with the main userspace code.
Do we have this as igts too? -Daniel
Matt
So I think ABI-wise we've met the userspace consumer requirements to upstream this; we just need to get reviews on the two i915-specific patches and a clean CI report.
v4.1 is identical to v4 aside from a rebase onto the latest drm-tip.
Matt Roper (3): drm/i915: Force background color to black for gen9+ (v2) drm: Add CRTC background color property (v4) drm/i915/gen9+: Add support for pipe background color (v4)
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_blend.c | 27 +++++++++++++++++++--- drivers/gpu/drm/drm_mode_config.c | 6 +++++ drivers/gpu/drm/i915/i915_debugfs.c | 9 ++++++++ drivers/gpu/drm/i915/i915_reg.h | 6 +++++ drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++++++++++++++++++++ include/drm/drm_blend.h | 1 + include/drm/drm_crtc.h | 12 ++++++++++ include/drm/drm_mode_config.h | 5 +++++ include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++++++ 10 files changed, 138 insertions(+), 3 deletions(-)
-- 2.14.5
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Jan 30, 2019 at 09:57:10PM +0100, Daniel Vetter wrote:
On Wed, Jan 30, 2019 at 10:56:26AM -0800, Matt Roper wrote:
On Wed, Jan 30, 2019 at 10:51:19AM -0800, Matt Roper wrote:
Previous patch series was here: https://lists.freedesktop.org/archives/dri-devel/2018-December/201949.html
I'm told the ChromeOS userspace code to make use of the background color has been reviewed and is ready for use:
Woops, the second link here should have been to
https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1...
which I believe is some unit tests to go along with the main userspace code.
Do we have this as igts too? -Daniel
Yeah, I posted it along with some of the earlier revisions of the series, but haven't reposted the latest copy lately. I'll check and see if it needs a rebase and then post it shortly.
Unfortunately the IGT isn't as useful as I'd like it to be since the CRC's for a plane filled with a solid color never come up the same as a pure background color (at least on the APL platform I'm using). I can run the IGT in interactive mode and the colors seem identical to visual inspection, but the CRC values never agree, so I've disabled the CRC comparison in the test for now. I'm not sure if this is related to the other blending quirks of gen9; it will be interesting to see if the CRC's match on an Icelake or something.
FWIW, I've mmap'd the Cairo-generated plane framebuffer and verified that it contains exactly the pixel values we'd expect (so it's not a matter of bad roundoff in Cairo), and I've tried flipping both 8bpc and 10bpc pixel formats (to match the background color's 10 bits per component), but nothing seems to make the CRC's match. :-(
Matt
Matt
So I think ABI-wise we've met the userspace consumer requirements to upstream this; we just need to get reviews on the two i915-specific patches and a clean CI report.
v4.1 is identical to v4 aside from a rebase onto the latest drm-tip.
Matt Roper (3): drm/i915: Force background color to black for gen9+ (v2) drm: Add CRTC background color property (v4) drm/i915/gen9+: Add support for pipe background color (v4)
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_blend.c | 27 +++++++++++++++++++--- drivers/gpu/drm/drm_mode_config.c | 6 +++++ drivers/gpu/drm/i915/i915_debugfs.c | 9 ++++++++ drivers/gpu/drm/i915/i915_reg.h | 6 +++++ drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++++++++++++++++++++ include/drm/drm_blend.h | 1 + include/drm/drm_crtc.h | 12 ++++++++++ include/drm/drm_mode_config.h | 5 +++++ include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++++++ 10 files changed, 138 insertions(+), 3 deletions(-)
-- 2.14.5
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Wed, Jan 30, 2019 at 03:48:50PM -0800, Matt Roper wrote:
On Wed, Jan 30, 2019 at 09:57:10PM +0100, Daniel Vetter wrote:
On Wed, Jan 30, 2019 at 10:56:26AM -0800, Matt Roper wrote:
On Wed, Jan 30, 2019 at 10:51:19AM -0800, Matt Roper wrote:
Previous patch series was here: https://lists.freedesktop.org/archives/dri-devel/2018-December/201949.html
I'm told the ChromeOS userspace code to make use of the background color has been reviewed and is ready for use:
Woops, the second link here should have been to
https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1...
which I believe is some unit tests to go along with the main userspace code.
Do we have this as igts too? -Daniel
Yeah, I posted it along with some of the earlier revisions of the series, but haven't reposted the latest copy lately. I'll check and see if it needs a rebase and then post it shortly.
Unfortunately the IGT isn't as useful as I'd like it to be since the CRC's for a plane filled with a solid color never come up the same as a pure background color (at least on the APL platform I'm using). I can run the IGT in interactive mode and the colors seem identical to visual inspection, but the CRC values never agree, so I've disabled the CRC comparison in the test for now. I'm not sure if this is related to the other blending quirks of gen9; it will be interesting to see if the CRC's match on an Icelake or something.
Please don't do that, we need to know when stuff doesn't work. Also, igt (at least for more generic stuff like this) shouldn't be bent to exactly match intel hw bugs.
And yes if the blending is generally broken on gen9 then I'd be surprised if they managed to not screw it up for the background color. Usually backgroun color works as if it's a separate additional plane that you blend the others with. -Daniel
FWIW, I've mmap'd the Cairo-generated plane framebuffer and verified that it contains exactly the pixel values we'd expect (so it's not a matter of bad roundoff in Cairo), and I've tried flipping both 8bpc and 10bpc pixel formats (to match the background color's 10 bits per component), but nothing seems to make the CRC's match. :-(
Matt
Matt
So I think ABI-wise we've met the userspace consumer requirements to upstream this; we just need to get reviews on the two i915-specific patches and a clean CI report.
v4.1 is identical to v4 aside from a rebase onto the latest drm-tip.
Matt Roper (3): drm/i915: Force background color to black for gen9+ (v2) drm: Add CRTC background color property (v4) drm/i915/gen9+: Add support for pipe background color (v4)
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_blend.c | 27 +++++++++++++++++++--- drivers/gpu/drm/drm_mode_config.c | 6 +++++ drivers/gpu/drm/i915/i915_debugfs.c | 9 ++++++++ drivers/gpu/drm/i915/i915_reg.h | 6 +++++ drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++++++++++++++++++++ include/drm/drm_blend.h | 1 + include/drm/drm_crtc.h | 12 ++++++++++ include/drm/drm_mode_config.h | 5 +++++ include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++++++ 10 files changed, 138 insertions(+), 3 deletions(-)
-- 2.14.5
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795
On Fri, Feb 01, 2019 at 06:13:48PM +0100, Daniel Vetter wrote:
On Wed, Jan 30, 2019 at 03:48:50PM -0800, Matt Roper wrote:
On Wed, Jan 30, 2019 at 09:57:10PM +0100, Daniel Vetter wrote:
On Wed, Jan 30, 2019 at 10:56:26AM -0800, Matt Roper wrote:
On Wed, Jan 30, 2019 at 10:51:19AM -0800, Matt Roper wrote:
Previous patch series was here: https://lists.freedesktop.org/archives/dri-devel/2018-December/201949.html
I'm told the ChromeOS userspace code to make use of the background color has been reviewed and is ready for use:
Woops, the second link here should have been to
https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1...
which I believe is some unit tests to go along with the main userspace code.
Do we have this as igts too? -Daniel
Yeah, I posted it along with some of the earlier revisions of the series, but haven't reposted the latest copy lately. I'll check and see if it needs a rebase and then post it shortly.
Unfortunately the IGT isn't as useful as I'd like it to be since the CRC's for a plane filled with a solid color never come up the same as a pure background color (at least on the APL platform I'm using). I can run the IGT in interactive mode and the colors seem identical to visual inspection, but the CRC values never agree, so I've disabled the CRC comparison in the test for now. I'm not sure if this is related to the other blending quirks of gen9; it will be interesting to see if the CRC's match on an Icelake or something.
Please don't do that, we need to know when stuff doesn't work. Also, igt (at least for more generic stuff like this) shouldn't be bent to exactly match intel hw bugs.
And yes if the blending is generally broken on gen9 then I'd be surprised if they managed to not screw it up for the background color. Usually backgroun color works as if it's a separate additional plane that you blend the others with. -Daniel
+igt-dev list
So looking at the bspec closer, it sounds like it might not actually be valid for us to take a DMUX (pipe) CRC of just the background color. The bspec for gen9+ states
"The DMUX CRC should only be enabled when the pipe, and one or more planes in the pipe are enabled."
which implies that just using the background color (which is always on) isn't sufficient.
I'll uncomment the CRC check when we actually push the test so that it can still be used on non-Intel platforms that don't have this restriction. Should we just make the test skip on the relevant i915 platforms, or should we do something else to mark the test as an expected failure due to hardware?
Matt
FWIW, I've mmap'd the Cairo-generated plane framebuffer and verified that it contains exactly the pixel values we'd expect (so it's not a matter of bad roundoff in Cairo), and I've tried flipping both 8bpc and 10bpc pixel formats (to match the background color's 10 bits per component), but nothing seems to make the CRC's match. :-(
Matt
Matt
So I think ABI-wise we've met the userspace consumer requirements to upstream this; we just need to get reviews on the two i915-specific patches and a clean CI report.
v4.1 is identical to v4 aside from a rebase onto the latest drm-tip.
Matt Roper (3): drm/i915: Force background color to black for gen9+ (v2) drm: Add CRTC background color property (v4) drm/i915/gen9+: Add support for pipe background color (v4)
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++++ drivers/gpu/drm/drm_blend.c | 27 +++++++++++++++++++--- drivers/gpu/drm/drm_mode_config.c | 6 +++++ drivers/gpu/drm/i915/i915_debugfs.c | 9 ++++++++ drivers/gpu/drm/i915/i915_reg.h | 6 +++++ drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++++++++++++++++++++ include/drm/drm_blend.h | 1 + include/drm/drm_crtc.h | 12 ++++++++++ include/drm/drm_mode_config.h | 5 +++++ include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++++++ 10 files changed, 138 insertions(+), 3 deletions(-)
-- 2.14.5
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
-- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Matt Roper -
Ville Syrjälä