* [PATCH v2 0/3] CRTC background color
@ 2018-11-13 23:21 Matt Roper
2018-11-13 23:21 ` [PATCH v2 1/3] drm/i915: Force background color to black for gen9+ Matt Roper
` (7 more replies)
0 siblings, 8 replies; 22+ messages in thread
From: Matt Roper @ 2018-11-13 23:21 UTC (permalink / raw)
To: intel-gfx; +Cc: Wei C Li, dri-devel
This is a second revision of the series previously posted here:
https://lists.freedesktop.org/archives/intel-gfx/2018-October/178202.html
As noted before, this functionality adds new ABI so we need a userspace
consumer ready before we merge the kernel work. My understanding is
that some of the folks involved with ChromeOS are looking at this and
that there's a ChromeOS userspace review happening at
https://chromium-review.googlesource.com/c/chromium/src/+/1278858
Since there are a few Intel-specific background color changes that we
want to make independently of the new ABI, I've separated those out into
a new patch #1; we may want to consider landing that patch before the
rest of the series since it fixes an inconsistency in how we currently
program our hardware.
On the i915-side of things, this series only deals with gen9+ at the
moment. It looks like CHV may also have support for background color
functionality, but I couldn't find the register layout details for that
platform, so I haven't added support for it yet.
Cc: dri-devel@lists.freedesktop.org
Cc: Wei C Li <wei.c.li@intel.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Matt Roper (3):
drm/i915: Force background color to black for gen9+
drm: Add CRTC background color property (v2)
drm/i915/gen9+: Add support for pipe background color (v2)
drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
drivers/gpu/drm/drm_atomic_uapi.c | 5 ++++
drivers/gpu/drm/drm_blend.c | 21 ++++++++++++++---
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 | 39 +++++++++++++++++++++++++++++++
include/drm/drm_blend.h | 1 +
include/drm/drm_crtc.h | 17 ++++++++++++++
include/drm/drm_mode_config.h | 5 ++++
include/uapi/drm/drm_mode.h | 26 +++++++++++++++++++++
11 files changed, 133 insertions(+), 3 deletions(-)
--
2.14.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] drm/i915: Force background color to black for gen9+
2018-11-13 23:21 [PATCH v2 0/3] CRTC background color Matt Roper
@ 2018-11-13 23:21 ` Matt Roper
2018-11-14 17:28 ` Ville Syrjälä
2018-11-13 23:21 ` [PATCH v2 2/3] drm: Add CRTC background color property (v2) Matt Roper
` (6 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Matt Roper @ 2018-11-13 23:21 UTC (permalink / raw)
To: intel-gfx
We don't yet allow userspace to control the CRTC background color, but
we should manually program the color to black to ensure the BIOS didn't
leave us with some other color. We should also set the pipe gamma and
pipe CSC bits so that the background color goes through the same color
management transformations that a plane with black pixels would.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
We may want to land this patch before the rest of the series since it's
still valuable even without the new ABI the rest of the series adds.
drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fe4b913e46ac..b92a721c9bcb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5662,6 +5662,12 @@ enum {
#define PIPEMISC_DITHER_TYPE_SP (0 << 2)
#define PIPEMISC(pipe) _MMIO_PIPE2(pipe, _PIPE_MISC_A)
+/* Skylake+ pipe bottom (background) color */
+#define _SKL_CANVAS_A 0x70034
+#define SKL_CANVAS_GAMMA_ENABLE (1 << 31)
+#define SKL_CANVAS_CSC_ENABLE (1 << 30)
+#define SKL_CANVAS(pipe) _MMIO_PIPE2(pipe, _SKL_CANVAS_A)
+
#define VLV_DPFLIPSTAT _MMIO(VLV_DISPLAY_BASE + 0x70028)
#define PIPEB_LINE_COMPARE_INT_EN (1 << 29)
#define PIPEB_HLINE_INT_EN (1 << 28)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 132e978227fb..1d089d93d88b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3868,6 +3868,15 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
else if (old_crtc_state->pch_pfit.enabled)
ironlake_pfit_disable(old_crtc_state);
}
+
+ /*
+ * We don't (yet) allow userspace to control the pipe background color,
+ * so force it to black, but apply pipe gamma and CSC so that its
+ * handling will match how we program our planes.
+ */
+ if (INTEL_GEN(dev_priv) >= 9)
+ I915_WRITE(SKL_CANVAS(crtc->pipe),
+ SKL_CANVAS_GAMMA_ENABLE | SKL_CANVAS_CSC_ENABLE);
}
static void intel_fdi_normal_train(struct intel_crtc *crtc)
@@ -15356,6 +15365,15 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
plane->base.type != DRM_PLANE_TYPE_PRIMARY)
intel_plane_disable_noatomic(crtc, plane);
}
+
+ /*
+ * Disable any background color set by the BIOS, but enable the
+ * gamma and CSC to match how we program our planes.
+ */
+ if (INTEL_GEN(dev_priv) >= 9)
+ I915_WRITE(SKL_CANVAS(crtc->pipe),
+ SKL_CANVAS_GAMMA_ENABLE |
+ SKL_CANVAS_CSC_ENABLE);
}
/* Adjust the state of the output pipe according to whether we
--
2.14.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 2/3] drm: Add CRTC background color property (v2)
2018-11-13 23:21 [PATCH v2 0/3] CRTC background color Matt Roper
2018-11-13 23:21 ` [PATCH v2 1/3] drm/i915: Force background color to black for gen9+ Matt Roper
@ 2018-11-13 23:21 ` Matt Roper
2018-11-14 16:17 ` Sean Paul
2018-11-14 17:20 ` Ville Syrjälä
2018-11-13 23:21 ` [PATCH v2 3/3] drm/i915/gen9+: Add support for pipe background color (v2) Matt Roper
` (5 subsequent siblings)
7 siblings, 2 replies; 22+ messages in thread
From: Matt Roper @ 2018-11-13 23:21 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel, wei.c.li, harish.krupo.kps
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.
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>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
drivers/gpu/drm/drm_atomic_uapi.c | 5 +++++
drivers/gpu/drm/drm_blend.c | 21 ++++++++++++++++++---
drivers/gpu/drm/drm_mode_config.c | 6 ++++++
include/drm/drm_blend.h | 1 +
include/drm/drm_crtc.h | 17 +++++++++++++++++
include/drm/drm_mode_config.h | 5 +++++
include/uapi/drm/drm_mode.h | 26 ++++++++++++++++++++++++++
8 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 3ba996069d69..2f8c55668089 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -101,6 +101,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
state->planes_changed = false;
state->connectors_changed = false;
state->color_mgmt_changed = false;
+ state->bgcolor_changed = false;
state->zpos_changed = false;
state->commit = NULL;
state->event = NULL;
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 86ac33922b09..b95a55a778e2 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -467,6 +467,9 @@ 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;
+ state->bgcolor_changed = true;
} else if (crtc->funcs->atomic_set_property) {
return crtc->funcs->atomic_set_property(crtc, state, property, val);
} else {
@@ -499,6 +502,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..7c73cb83874a 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -175,9 +175,16 @@
* 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:
+ *
+ * bgcolor:
+ * Background color is setup with drm_crtc_add_bgcolor_property(). It
+ * controls the RGB color of a full-screen, fully-opaque 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.
*/
/**
@@ -593,3 +600,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_rgba(16, 0, 0, 0, 0xffff));
+}
+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 ee80788f2c40..75e376755176 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -352,6 +352,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 b21437bc95bf..d78f82f954e4 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -168,6 +168,11 @@ struct drm_crtc_state {
* drivers to steer the atomic commit control flow.
*/
bool color_mgmt_changed : 1;
+ /**
+ * @bgcolor_changed: Background color value has changed. Used by
+ * drivers to steer the atomic commit control flow.
+ */
+ bool bgcolor_changed : 1;
/**
* @no_vblank:
@@ -274,6 +279,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_rgba();
+ * individual color components can be extracted with desired precision
+ * via the DRM_RGBA_*() macros.
+ */
+ u64 bgcolor;
+
/**
* @target_vblank:
*
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 5dbeabdbaf91..8b686d228feb 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -813,6 +813,11 @@ struct drm_mode_config {
*/
struct drm_property *writeback_out_fence_ptr_property;
+ /**
+ * @bgcolor_property: RGBA 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 d3e0fe31efc5..7c4f902aa290 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -888,6 +888,32 @@ struct drm_mode_revoke_lease {
__u32 lessee_id;
};
+/*
+ * Put RGBA 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_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha)
+{
+ 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 RGBA value.
+ */
+#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits))
+#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
+#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
+#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
+
#if defined(__cplusplus)
}
#endif
--
2.14.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 3/3] drm/i915/gen9+: Add support for pipe background color (v2)
2018-11-13 23:21 [PATCH v2 0/3] CRTC background color Matt Roper
2018-11-13 23:21 ` [PATCH v2 1/3] drm/i915: Force background color to black for gen9+ Matt Roper
2018-11-13 23:21 ` [PATCH v2 2/3] drm: Add CRTC background color property (v2) Matt Roper
@ 2018-11-13 23:21 ` Matt Roper
2018-11-14 18:05 ` Ville Syrjälä
2018-11-13 23:22 ` [igt-dev] " Matt Roper
` (4 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Matt Roper @ 2018-11-13 23:21 UTC (permalink / raw)
To: intel-gfx; +Cc: wei.c.li, dri-devel, harish.krupo.kps
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.
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 | 35 ++++++++++++++++++++++++++++-------
2 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 670db5073d70..1f2a19e6ec79 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3254,6 +3254,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_RGBA_RED(background, 10),
+ DRM_RGBA_GREEN(background, 10),
+ DRM_RGBA_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 1d089d93d88b..e7a759e0c021 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3834,6 +3834,27 @@ 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 *cstate)
+{
+ struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ uint64_t propval = cstate->base.bgcolor;
+ uint32_t tmp;
+
+ /* Hardware is programmed with 10 bits of precision */
+ tmp = DRM_RGBA_RED(propval, 10) << 20
+ | DRM_RGBA_GREEN(propval, 10) << 10
+ | DRM_RGBA_BLUE(propval, 10);
+
+ /*
+ * Set CSC and gamma for bottom color to ensure background pixels
+ * receive the same color transformations as plane content.
+ */
+ tmp |= SKL_CANVAS_CSC_ENABLE | SKL_CANVAS_GAMMA_ENABLE;
+
+ I915_WRITE_FW(SKL_CANVAS(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)
{
@@ -3869,14 +3890,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_CANVAS(crtc->pipe),
- SKL_CANVAS_GAMMA_ENABLE | SKL_CANVAS_CSC_ENABLE);
+ skl_update_background_color(new_crtc_state);
}
static void intel_fdi_normal_train(struct intel_crtc *crtc)
@@ -10896,6 +10911,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
crtc_state->planes_changed = true;
}
+ if (crtc_state->bgcolor_changed)
+ pipe_config->update_pipe = true;
+
ret = 0;
if (dev_priv->display.compute_pipe_wm) {
ret = dev_priv->display.compute_pipe_wm(pipe_config);
@@ -14035,6 +14053,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:
--
2.14.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH i-g-t v2] tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2)
2018-11-13 23:21 [PATCH v2 0/3] CRTC background color Matt Roper
@ 2018-11-13 23:22 ` Matt Roper
2018-11-13 23:21 ` [PATCH v2 2/3] drm: Add CRTC background color property (v2) Matt Roper
` (6 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Matt Roper @ 2018-11-13 23:22 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
CRTC background color kernel patches were written about 2.5 years ago
and floated on the upstream mailing list, but since no opensource
userspace materialized, we never actually merged them. However the
corresponding IGT test did get merged and has basically been dead code
ever since.
A couple years later we may finally be getting closer to landing the
kernel patches (there's some interest in this functionality now from
both the ChromeOS and Weston camps), so lets update the IGT test to
match the latest proposed ABI, and to remove some of the cruft from the
original test that wouldn't actually work.
It's worth noting that we don't seem to be able to test this feature
with CRC's. Originally we wanted to draw a color into a plane's FB
(with Cairo) and then compare the CRC to turning off all planes and just
setting the CRTC background to the same color. However the precision
and rounding of the color components causes the CRC's to come out
differently, even though the end result is visually identical. So at
the moment this test is mainly useful for visual inspection in
interactive mode.
v2:
- Swap red and blue ordering in property value to reflect change
in v2 of kernel series.
Cc: igt-dev@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
lib/igt_kms.c | 2 +-
tests/kms_crtc_background_color.c | 221 ++++++++++++++++++++------------------
2 files changed, 120 insertions(+), 103 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index d806ccc1..33d6a6fb 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -180,7 +180,7 @@ const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
};
const char * const igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
- [IGT_CRTC_BACKGROUND] = "background_color",
+ [IGT_CRTC_BACKGROUND] = "BACKGROUND_COLOR",
[IGT_CRTC_CTM] = "CTM",
[IGT_CRTC_GAMMA_LUT] = "GAMMA_LUT",
[IGT_CRTC_GAMMA_LUT_SIZE] = "GAMMA_LUT_SIZE",
diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
index 3df3401f..e990ecfa 100644
--- a/tests/kms_crtc_background_color.c
+++ b/tests/kms_crtc_background_color.c
@@ -25,164 +25,181 @@
#include "igt.h"
#include <math.h>
-
IGT_TEST_DESCRIPTION("Test crtc background color feature");
+/*
+ * The original idea was to paint a desired color into a full-screen primary
+ * plane and then compare that CRC with turning off all planes and setting the
+ * CRTC background to the same color. Unforunately, the rounding and precision
+ * of color values as rendered by cairo vs created by the display controller
+ * are slightly different and give different CRC's, even though they're
+ * visually identical.
+ *
+ * Since we can't really use CRC's for testing, this test is mainly useful for
+ * visual inspection in interactive mode at the moment.
+ */
+
typedef struct {
int gfx_fd;
- igt_display_t display;
- struct igt_fb fb;
- igt_crc_t ref_crc;
- igt_pipe_crc_t *pipe_crc;
+ igt_output_t *output;
+ drmModeModeInfo *mode;
} data_t;
-#define BLACK 0x000000 /* BGR 8bpc */
-#define CYAN 0xFFFF00 /* BGR 8bpc */
-#define PURPLE 0xFF00FF /* BGR 8bpc */
-#define WHITE 0xFFFFFF /* BGR 8bpc */
-
-#define BLACK64 0x000000000000 /* BGR 16bpc */
-#define CYAN64 0xFFFFFFFF0000 /* BGR 16bpc */
-#define PURPLE64 0xFFFF0000FFFF /* BGR 16bpc */
-#define YELLOW64 0x0000FFFFFFFF /* BGR 16bpc */
-#define WHITE64 0xFFFFFFFFFFFF /* BGR 16bpc */
-#define RED64 0x00000000FFFF /* BGR 16bpc */
-#define GREEN64 0x0000FFFF0000 /* BGR 16bpc */
-#define BLUE64 0xFFFF00000000 /* BGR 16bpc */
+/*
+ * Local copy of proposed kernel uapi
+ */
+static inline __u64
+local_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha)
+{
+ 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;
+}
+#define LOCAL_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits))
+#define LOCAL_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
+#define LOCAL_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
+#define LOCAL_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
+
+
+/* 8bpc values */
+#define BLACK local_rgba(8, 0, 0, 0, 0xff)
+#define RED local_rgba(8, 0xff, 0, 0, 0xff)
+#define GREEN local_rgba(8, 0, 0xff, 0, 0xff)
+#define BLUE local_rgba(8, 0, 0, 0xff, 0xff)
+#define YELLOW local_rgba(8, 0xff, 0xff, 0, 0xff)
+#define WHITE local_rgba(8, 0xff, 0xff, 0xff, 0xff)
+
+/* 16bpc values */
+#define BLACK64 local_rgba(16, 0, 0, 0, 0xffff)
+#define RED64 local_rgba(16, 0xffff, 0, 0, 0xffff)
+#define GREEN64 local_rgba(16, 0, 0xffff, 0, 0xffff)
+#define BLUE64 local_rgba(16, 0, 0, 0xffff, 0xffff)
+#define YELLOW64 local_rgba(16, 0xffff, 0xffff, 0, 0xffff)
+#define WHITE64 local_rgba(16, 0xffff, 0xffff, 0xffff, 0xffff)
+
+#if 0
static void
-paint_background(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
- uint32_t background, double alpha)
+paint_fb(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
+ uint64_t color, int prec)
{
cairo_t *cr;
- int w, h;
+ int w = mode->hdisplay;
+ int h = mode->vdisplay;
double r, g, b;
- w = mode->hdisplay;
- h = mode->vdisplay;
+ igt_create_fb(data->gfx_fd, w, h, DRM_FORMAT_XRGB8888,
+ LOCAL_DRM_FORMAT_MOD_NONE, fb);
- cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
+ cr = igt_get_cairo_ctx(data->gfx_fd, fb);
- /* Paint with background color */
- r = (double) (background & 0xFF) / 255.0;
- g = (double) ((background & 0xFF00) >> 8) / 255.0;
- b = (double) ((background & 0xFF0000) >> 16) / 255.0;
- igt_paint_color_alpha(cr, 0, 0, w, h, r, g, b, alpha);
+ /*
+ * Grab color (with appropriate bits of precision) and paint a
+ * framebuffer with it.
+ */
+ r = (double)LOCAL_RGBA_RED(color, prec) / ((1<<prec) - 1);
+ g = (double)LOCAL_RGBA_GREEN(color, prec) / ((1<<prec) - 1);
+ b = (double)LOCAL_RGBA_BLUE(color, prec) / ((1<<prec) - 1);
+ igt_paint_color_alpha(cr, 0, 0, w, h, r, g, b, 1.0);
- igt_put_cairo_ctx(data->gfx_fd, &data->fb, cr);
+ igt_put_cairo_ctx(data->gfx_fd, fb, cr);
}
+#endif
-static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
- igt_plane_t *plane, int opaque_buffer, int plane_color,
- uint64_t pipe_background_color)
+static void prepare_crtc(igt_display_t *display, data_t *data,
+ igt_output_t *output, enum pipe pipe)
{
- drmModeModeInfo *mode;
- igt_display_t *display = &data->display;
- int fb_id;
- double alpha;
-
igt_output_set_pipe(output, pipe);
-
- /* create the pipe_crc object for this pipe */
- igt_pipe_crc_free(data->pipe_crc);
- data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
-
- mode = igt_output_get_mode(output);
-
- fb_id = igt_create_fb(data->gfx_fd,
- mode->hdisplay, mode->vdisplay,
- DRM_FORMAT_XRGB8888,
- LOCAL_DRM_FORMAT_MOD_NONE, /* tiled */
- &data->fb);
- igt_assert(fb_id);
-
- /* To make FB pixel win with background color, set alpha as full opaque */
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, pipe_background_color);
- if (opaque_buffer)
- alpha = 1.0; /* alpha 1 is fully opque */
- else
- alpha = 0.0; /* alpha 0 is fully transparent */
- paint_background(data, &data->fb, mode, plane_color, alpha);
-
- igt_plane_set_fb(plane, &data->fb);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ data->output = output;
+ data->mode = igt_output_get_mode(output);
}
-static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
+static void cleanup_crtc(igt_display_t *display, data_t *data,
+ igt_output_t *output)
{
- igt_display_t *display = &data->display;
-
- igt_pipe_crc_free(data->pipe_crc);
- data->pipe_crc = NULL;
-
- igt_remove_fb(data->gfx_fd, &data->fb);
-
- igt_pipe_obj_set_prop_value(plane->pipe, IGT_CRTC_BACKGROUND, BLACK64);
- igt_plane_set_fb(plane, NULL);
igt_output_set_pipe(output, PIPE_ANY);
-
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
}
-static void test_crtc_background(data_t *data)
+static void test_crtc_background(igt_display_t *display, data_t *data)
{
- igt_display_t *display = &data->display;
igt_output_t *output;
enum pipe pipe;
int valid_tests = 0;
- for_each_pipe_with_valid_output(display, pipe, output) {
+ for_each_pipe_with_single_output(display, pipe, output) {
igt_plane_t *plane;
- igt_output_set_pipe(output, pipe);
-
- plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
igt_require(igt_pipe_has_prop(display, pipe, IGT_CRTC_BACKGROUND));
- prepare_crtc(data, output, pipe, plane, 1, PURPLE, BLACK64);
+ prepare_crtc(display, data, output, pipe);
+ plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
- /* Now set background without using a plane, i.e.,
- * Disable the plane to let hw background color win blend. */
+ /*
+ * Turn off the primary plane (default bgcolor should be black
+ * unless a previous drm master changed it to something else).
+ */
igt_plane_set_fb(plane, NULL);
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, PURPLE64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
-
- /* Try few other background colors */
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, CYAN64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
-
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, YELLOW64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
-
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ /* Explicitly set black as bg color */
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLACK64);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ /*
+ * Test several more colors and precisions. Unfortunately the
+ * CRC's won't match between a cairo-drawn fb and a display
+ * controller bgcolor setting, but these can at least be
+ * visually verified in interactive mode to ensure the colors
+ * look good.
+ */
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, RED64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, RED);
+ igt_display_commit2(display, COMMIT_ATOMIC);
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, GREEN64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, GREEN);
+ igt_display_commit2(display, COMMIT_ATOMIC);
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLUE64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLUE);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, YELLOW64);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, YELLOW);
+ igt_display_commit2(display, COMMIT_ATOMIC);
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, WHITE64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, WHITE);
+ igt_display_commit2(display, COMMIT_ATOMIC);
valid_tests++;
- cleanup_crtc(data, output, plane);
+
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLACK64);
+ cleanup_crtc(display, data, output);
}
igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
}
igt_simple_main
{
+ igt_display_t display;
data_t data = {};
igt_skip_on_simulation();
data.gfx_fd = drm_open_driver(DRIVER_INTEL);
- igt_require_pipe_crc(data.gfx_fd);
- igt_display_require(&data.display, data.gfx_fd);
+ igt_display_require(&display, data.gfx_fd);
- test_crtc_background(&data);
+ test_crtc_background(&display, &data);
- igt_display_fini(&data.display);
+ igt_display_fini(&display);
}
--
2.14.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [igt-dev] [PATCH i-g-t v2] tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2)
@ 2018-11-13 23:22 ` Matt Roper
0 siblings, 0 replies; 22+ messages in thread
From: Matt Roper @ 2018-11-13 23:22 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
CRTC background color kernel patches were written about 2.5 years ago
and floated on the upstream mailing list, but since no opensource
userspace materialized, we never actually merged them. However the
corresponding IGT test did get merged and has basically been dead code
ever since.
A couple years later we may finally be getting closer to landing the
kernel patches (there's some interest in this functionality now from
both the ChromeOS and Weston camps), so lets update the IGT test to
match the latest proposed ABI, and to remove some of the cruft from the
original test that wouldn't actually work.
It's worth noting that we don't seem to be able to test this feature
with CRC's. Originally we wanted to draw a color into a plane's FB
(with Cairo) and then compare the CRC to turning off all planes and just
setting the CRTC background to the same color. However the precision
and rounding of the color components causes the CRC's to come out
differently, even though the end result is visually identical. So at
the moment this test is mainly useful for visual inspection in
interactive mode.
v2:
- Swap red and blue ordering in property value to reflect change
in v2 of kernel series.
Cc: igt-dev@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
lib/igt_kms.c | 2 +-
tests/kms_crtc_background_color.c | 221 ++++++++++++++++++++------------------
2 files changed, 120 insertions(+), 103 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index d806ccc1..33d6a6fb 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -180,7 +180,7 @@ const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
};
const char * const igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
- [IGT_CRTC_BACKGROUND] = "background_color",
+ [IGT_CRTC_BACKGROUND] = "BACKGROUND_COLOR",
[IGT_CRTC_CTM] = "CTM",
[IGT_CRTC_GAMMA_LUT] = "GAMMA_LUT",
[IGT_CRTC_GAMMA_LUT_SIZE] = "GAMMA_LUT_SIZE",
diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
index 3df3401f..e990ecfa 100644
--- a/tests/kms_crtc_background_color.c
+++ b/tests/kms_crtc_background_color.c
@@ -25,164 +25,181 @@
#include "igt.h"
#include <math.h>
-
IGT_TEST_DESCRIPTION("Test crtc background color feature");
+/*
+ * The original idea was to paint a desired color into a full-screen primary
+ * plane and then compare that CRC with turning off all planes and setting the
+ * CRTC background to the same color. Unforunately, the rounding and precision
+ * of color values as rendered by cairo vs created by the display controller
+ * are slightly different and give different CRC's, even though they're
+ * visually identical.
+ *
+ * Since we can't really use CRC's for testing, this test is mainly useful for
+ * visual inspection in interactive mode at the moment.
+ */
+
typedef struct {
int gfx_fd;
- igt_display_t display;
- struct igt_fb fb;
- igt_crc_t ref_crc;
- igt_pipe_crc_t *pipe_crc;
+ igt_output_t *output;
+ drmModeModeInfo *mode;
} data_t;
-#define BLACK 0x000000 /* BGR 8bpc */
-#define CYAN 0xFFFF00 /* BGR 8bpc */
-#define PURPLE 0xFF00FF /* BGR 8bpc */
-#define WHITE 0xFFFFFF /* BGR 8bpc */
-
-#define BLACK64 0x000000000000 /* BGR 16bpc */
-#define CYAN64 0xFFFFFFFF0000 /* BGR 16bpc */
-#define PURPLE64 0xFFFF0000FFFF /* BGR 16bpc */
-#define YELLOW64 0x0000FFFFFFFF /* BGR 16bpc */
-#define WHITE64 0xFFFFFFFFFFFF /* BGR 16bpc */
-#define RED64 0x00000000FFFF /* BGR 16bpc */
-#define GREEN64 0x0000FFFF0000 /* BGR 16bpc */
-#define BLUE64 0xFFFF00000000 /* BGR 16bpc */
+/*
+ * Local copy of proposed kernel uapi
+ */
+static inline __u64
+local_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha)
+{
+ 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;
+}
+#define LOCAL_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits))
+#define LOCAL_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
+#define LOCAL_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
+#define LOCAL_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
+
+
+/* 8bpc values */
+#define BLACK local_rgba(8, 0, 0, 0, 0xff)
+#define RED local_rgba(8, 0xff, 0, 0, 0xff)
+#define GREEN local_rgba(8, 0, 0xff, 0, 0xff)
+#define BLUE local_rgba(8, 0, 0, 0xff, 0xff)
+#define YELLOW local_rgba(8, 0xff, 0xff, 0, 0xff)
+#define WHITE local_rgba(8, 0xff, 0xff, 0xff, 0xff)
+
+/* 16bpc values */
+#define BLACK64 local_rgba(16, 0, 0, 0, 0xffff)
+#define RED64 local_rgba(16, 0xffff, 0, 0, 0xffff)
+#define GREEN64 local_rgba(16, 0, 0xffff, 0, 0xffff)
+#define BLUE64 local_rgba(16, 0, 0, 0xffff, 0xffff)
+#define YELLOW64 local_rgba(16, 0xffff, 0xffff, 0, 0xffff)
+#define WHITE64 local_rgba(16, 0xffff, 0xffff, 0xffff, 0xffff)
+
+#if 0
static void
-paint_background(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
- uint32_t background, double alpha)
+paint_fb(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
+ uint64_t color, int prec)
{
cairo_t *cr;
- int w, h;
+ int w = mode->hdisplay;
+ int h = mode->vdisplay;
double r, g, b;
- w = mode->hdisplay;
- h = mode->vdisplay;
+ igt_create_fb(data->gfx_fd, w, h, DRM_FORMAT_XRGB8888,
+ LOCAL_DRM_FORMAT_MOD_NONE, fb);
- cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
+ cr = igt_get_cairo_ctx(data->gfx_fd, fb);
- /* Paint with background color */
- r = (double) (background & 0xFF) / 255.0;
- g = (double) ((background & 0xFF00) >> 8) / 255.0;
- b = (double) ((background & 0xFF0000) >> 16) / 255.0;
- igt_paint_color_alpha(cr, 0, 0, w, h, r, g, b, alpha);
+ /*
+ * Grab color (with appropriate bits of precision) and paint a
+ * framebuffer with it.
+ */
+ r = (double)LOCAL_RGBA_RED(color, prec) / ((1<<prec) - 1);
+ g = (double)LOCAL_RGBA_GREEN(color, prec) / ((1<<prec) - 1);
+ b = (double)LOCAL_RGBA_BLUE(color, prec) / ((1<<prec) - 1);
+ igt_paint_color_alpha(cr, 0, 0, w, h, r, g, b, 1.0);
- igt_put_cairo_ctx(data->gfx_fd, &data->fb, cr);
+ igt_put_cairo_ctx(data->gfx_fd, fb, cr);
}
+#endif
-static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
- igt_plane_t *plane, int opaque_buffer, int plane_color,
- uint64_t pipe_background_color)
+static void prepare_crtc(igt_display_t *display, data_t *data,
+ igt_output_t *output, enum pipe pipe)
{
- drmModeModeInfo *mode;
- igt_display_t *display = &data->display;
- int fb_id;
- double alpha;
-
igt_output_set_pipe(output, pipe);
-
- /* create the pipe_crc object for this pipe */
- igt_pipe_crc_free(data->pipe_crc);
- data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
-
- mode = igt_output_get_mode(output);
-
- fb_id = igt_create_fb(data->gfx_fd,
- mode->hdisplay, mode->vdisplay,
- DRM_FORMAT_XRGB8888,
- LOCAL_DRM_FORMAT_MOD_NONE, /* tiled */
- &data->fb);
- igt_assert(fb_id);
-
- /* To make FB pixel win with background color, set alpha as full opaque */
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, pipe_background_color);
- if (opaque_buffer)
- alpha = 1.0; /* alpha 1 is fully opque */
- else
- alpha = 0.0; /* alpha 0 is fully transparent */
- paint_background(data, &data->fb, mode, plane_color, alpha);
-
- igt_plane_set_fb(plane, &data->fb);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ data->output = output;
+ data->mode = igt_output_get_mode(output);
}
-static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
+static void cleanup_crtc(igt_display_t *display, data_t *data,
+ igt_output_t *output)
{
- igt_display_t *display = &data->display;
-
- igt_pipe_crc_free(data->pipe_crc);
- data->pipe_crc = NULL;
-
- igt_remove_fb(data->gfx_fd, &data->fb);
-
- igt_pipe_obj_set_prop_value(plane->pipe, IGT_CRTC_BACKGROUND, BLACK64);
- igt_plane_set_fb(plane, NULL);
igt_output_set_pipe(output, PIPE_ANY);
-
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
}
-static void test_crtc_background(data_t *data)
+static void test_crtc_background(igt_display_t *display, data_t *data)
{
- igt_display_t *display = &data->display;
igt_output_t *output;
enum pipe pipe;
int valid_tests = 0;
- for_each_pipe_with_valid_output(display, pipe, output) {
+ for_each_pipe_with_single_output(display, pipe, output) {
igt_plane_t *plane;
- igt_output_set_pipe(output, pipe);
-
- plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
igt_require(igt_pipe_has_prop(display, pipe, IGT_CRTC_BACKGROUND));
- prepare_crtc(data, output, pipe, plane, 1, PURPLE, BLACK64);
+ prepare_crtc(display, data, output, pipe);
+ plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
- /* Now set background without using a plane, i.e.,
- * Disable the plane to let hw background color win blend. */
+ /*
+ * Turn off the primary plane (default bgcolor should be black
+ * unless a previous drm master changed it to something else).
+ */
igt_plane_set_fb(plane, NULL);
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, PURPLE64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
-
- /* Try few other background colors */
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, CYAN64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
-
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, YELLOW64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
-
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ /* Explicitly set black as bg color */
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLACK64);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ /*
+ * Test several more colors and precisions. Unfortunately the
+ * CRC's won't match between a cairo-drawn fb and a display
+ * controller bgcolor setting, but these can at least be
+ * visually verified in interactive mode to ensure the colors
+ * look good.
+ */
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, RED64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, RED);
+ igt_display_commit2(display, COMMIT_ATOMIC);
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, GREEN64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, GREEN);
+ igt_display_commit2(display, COMMIT_ATOMIC);
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLUE64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLUE);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, YELLOW64);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, YELLOW);
+ igt_display_commit2(display, COMMIT_ATOMIC);
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, WHITE64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, WHITE);
+ igt_display_commit2(display, COMMIT_ATOMIC);
valid_tests++;
- cleanup_crtc(data, output, plane);
+
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLACK64);
+ cleanup_crtc(display, data, output);
}
igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
}
igt_simple_main
{
+ igt_display_t display;
data_t data = {};
igt_skip_on_simulation();
data.gfx_fd = drm_open_driver(DRIVER_INTEL);
- igt_require_pipe_crc(data.gfx_fd);
- igt_display_require(&data.display, data.gfx_fd);
+ igt_display_require(&display, data.gfx_fd);
- test_crtc_background(&data);
+ test_crtc_background(&display, &data);
- igt_display_fini(&data.display);
+ igt_display_fini(&display);
}
--
2.14.4
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 22+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for CRTC background color (rev2)
2018-11-13 23:21 [PATCH v2 0/3] CRTC background color Matt Roper
` (3 preceding siblings ...)
2018-11-13 23:22 ` [igt-dev] " Matt Roper
@ 2018-11-13 23:31 ` Patchwork
2018-11-13 23:58 ` ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-11-13 23:31 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== Series Details ==
Series: CRTC background color (rev2)
URL : https://patchwork.freedesktop.org/series/50834/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
99a0ae2cbae8 drm/i915: Force background color to black for gen9+
a25e43dadb92 drm: Add CRTC background color property (v2)
-:146: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32>
#146: FILE: include/drm/drm_crtc.h:175:
+ bool bgcolor_changed : 1;
-:214: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#214: FILE: include/uapi/drm/drm_mode.h:912:
+#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits))
^
-:214: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'c' may be better as '(c)' to avoid precedence issues
#214: FILE: include/uapi/drm/drm_mode.h:912:
+#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits))
-:214: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'numbits' may be better as '(numbits)' to avoid precedence issues
#214: FILE: include/uapi/drm/drm_mode.h:912:
+#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits))
-:215: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#215: FILE: include/uapi/drm/drm_mode.h:913:
+#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
^
-:215: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#215: FILE: include/uapi/drm/drm_mode.h:913:
+#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
^
-:215: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'c' may be better as '(c)' to avoid precedence issues
#215: FILE: include/uapi/drm/drm_mode.h:913:
+#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
-:215: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'numbits' may be better as '(numbits)' to avoid precedence issues
#215: FILE: include/uapi/drm/drm_mode.h:913:
+#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
-:216: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#216: FILE: include/uapi/drm/drm_mode.h:914:
+#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
^
-:216: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#216: FILE: include/uapi/drm/drm_mode.h:914:
+#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
^
-:216: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'c' may be better as '(c)' to avoid precedence issues
#216: FILE: include/uapi/drm/drm_mode.h:914:
+#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
-:216: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'numbits' may be better as '(numbits)' to avoid precedence issues
#216: FILE: include/uapi/drm/drm_mode.h:914:
+#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
-:217: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#217: FILE: include/uapi/drm/drm_mode.h:915:
+#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
^
-:217: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#217: FILE: include/uapi/drm/drm_mode.h:915:
+#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
^
-:217: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'c' may be better as '(c)' to avoid precedence issues
#217: FILE: include/uapi/drm/drm_mode.h:915:
+#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
-:217: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'numbits' may be better as '(numbits)' to avoid precedence issues
#217: FILE: include/uapi/drm/drm_mode.h:915:
+#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
total: 0 errors, 1 warnings, 15 checks, 143 lines checked
470dfe72d6c8 drm/i915/gen9+: Add support for pipe background color (v2)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH i-g-t v2] tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2)
2018-11-13 23:22 ` [igt-dev] " Matt Roper
@ 2018-11-13 23:43 ` Lionel Landwerlin
-1 siblings, 0 replies; 22+ messages in thread
From: Lionel Landwerlin @ 2018-11-13 23:43 UTC (permalink / raw)
To: Matt Roper, intel-gfx; +Cc: igt-dev
On 13/11/2018 23:22, Matt Roper wrote:
> It's worth noting that we don't seem to be able to test this feature
> with CRC's. Originally we wanted to draw a color into a plane's FB
> (with Cairo) and then compare the CRC to turning off all planes and just
> setting the CRTC background to the same color. However the precision
> and rounding of the color components causes the CRC's to come out
> differently, even though the end result is visually identical. So at
> the moment this test is mainly useful for visual inspection in
> interactive mode.
Hey Matt,
Out of curiosity, are the CRCs different only when with 16bpc? Or with
8bpc too?
Thanks,
-
Lionel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t v2] tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2)
@ 2018-11-13 23:43 ` Lionel Landwerlin
0 siblings, 0 replies; 22+ messages in thread
From: Lionel Landwerlin @ 2018-11-13 23:43 UTC (permalink / raw)
To: Matt Roper, intel-gfx; +Cc: igt-dev
On 13/11/2018 23:22, Matt Roper wrote:
> It's worth noting that we don't seem to be able to test this feature
> with CRC's. Originally we wanted to draw a color into a plane's FB
> (with Cairo) and then compare the CRC to turning off all planes and just
> setting the CRTC background to the same color. However the precision
> and rounding of the color components causes the CRC's to come out
> differently, even though the end result is visually identical. So at
> the moment this test is mainly useful for visual inspection in
> interactive mode.
Hey Matt,
Out of curiosity, are the CRCs different only when with 16bpc? Or with
8bpc too?
Thanks,
-
Lionel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH i-g-t v2] tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2)
2018-11-13 23:43 ` [igt-dev] [Intel-gfx] " Lionel Landwerlin
@ 2018-11-13 23:53 ` Matt Roper
-1 siblings, 0 replies; 22+ messages in thread
From: Matt Roper @ 2018-11-13 23:53 UTC (permalink / raw)
To: Lionel Landwerlin; +Cc: igt-dev, intel-gfx
On Tue, Nov 13, 2018 at 11:43:12PM +0000, Lionel Landwerlin wrote:
> On 13/11/2018 23:22, Matt Roper wrote:
> > It's worth noting that we don't seem to be able to test this feature
> > with CRC's. Originally we wanted to draw a color into a plane's FB
> > (with Cairo) and then compare the CRC to turning off all planes and just
> > setting the CRTC background to the same color. However the precision
> > and rounding of the color components causes the CRC's to come out
> > differently, even though the end result is visually identical. So at
> > the moment this test is mainly useful for visual inspection in
> > interactive mode.
> Hey Matt,
> Out of curiosity, are the CRCs different only when with 16bpc? Or with 8bpc
> too?
Both 8 and 16 give different CRC's. I suspect it's because cairo
handles the color values (which get passed as double) slightly
differently internally, although I haven't ruled out a slight inaccuracy
on the hardware side. Given the alpha problems on gen9/gen10, it
wouldn't surprise me if it turns out that our hardware is also treating
a 10bpc component of 0x3ff as 0.999 rather than 1.0.
Matt
>
> Thanks,
>
> -
> Lionel
>
>
>
--
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t v2] tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2)
@ 2018-11-13 23:53 ` Matt Roper
0 siblings, 0 replies; 22+ messages in thread
From: Matt Roper @ 2018-11-13 23:53 UTC (permalink / raw)
To: Lionel Landwerlin; +Cc: igt-dev, intel-gfx
On Tue, Nov 13, 2018 at 11:43:12PM +0000, Lionel Landwerlin wrote:
> On 13/11/2018 23:22, Matt Roper wrote:
> > It's worth noting that we don't seem to be able to test this feature
> > with CRC's. Originally we wanted to draw a color into a plane's FB
> > (with Cairo) and then compare the CRC to turning off all planes and just
> > setting the CRTC background to the same color. However the precision
> > and rounding of the color components causes the CRC's to come out
> > differently, even though the end result is visually identical. So at
> > the moment this test is mainly useful for visual inspection in
> > interactive mode.
> Hey Matt,
> Out of curiosity, are the CRCs different only when with 16bpc? Or with 8bpc
> too?
Both 8 and 16 give different CRC's. I suspect it's because cairo
handles the color values (which get passed as double) slightly
differently internally, although I haven't ruled out a slight inaccuracy
on the hardware side. Given the alpha problems on gen9/gen10, it
wouldn't surprise me if it turns out that our hardware is also treating
a 10bpc component of 0x3ff as 0.999 rather than 1.0.
Matt
>
> Thanks,
>
> -
> Lionel
>
>
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* ✓ Fi.CI.BAT: success for CRTC background color (rev2)
2018-11-13 23:21 [PATCH v2 0/3] CRTC background color Matt Roper
` (4 preceding siblings ...)
2018-11-13 23:31 ` ✗ Fi.CI.CHECKPATCH: warning for CRTC background color (rev2) Patchwork
@ 2018-11-13 23:58 ` Patchwork
2018-11-14 1:34 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2) Patchwork
2018-11-14 4:48 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
7 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-11-13 23:58 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== Series Details ==
Series: CRTC background color (rev2)
URL : https://patchwork.freedesktop.org/series/50834/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5133 -> Patchwork_10819 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/50834/revisions/2/mbox/
== Known issues ==
Here are the changes found in Patchwork_10819 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_ctx_create@basic-files:
fi-bsw-n3050: PASS -> FAIL (fdo#108656)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-byt-clapper: PASS -> FAIL (fdo#107362, fdo#103191)
==== Possible fixes ====
igt@drv_selftest@live_hangcheck:
fi-bwr-2160: DMESG-FAIL (fdo#108735) -> PASS
igt@kms_frontbuffer_tracking@basic:
fi-hsw-peppy: DMESG-WARN (fdo#102614) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
fi-byt-clapper: FAIL (fdo#107362, fdo#103191) -> PASS
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656
fdo#108735 https://bugs.freedesktop.org/show_bug.cgi?id=108735
== Participating hosts (53 -> 44) ==
Missing (9): fi-kbl-soraka fi-ilk-m540 fi-icl-u fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-ctg-p8600 fi-blb-e6850
== Build changes ==
* Linux: CI_DRM_5133 -> Patchwork_10819
CI_DRM_5133: 5c71926a1834348a68951622a950de0355b73450 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10819: 470dfe72d6c81515372d60ce69beb8107a6724c6 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
470dfe72d6c8 drm/i915/gen9+: Add support for pipe background color (v2)
a25e43dadb92 drm: Add CRTC background color property (v2)
99a0ae2cbae8 drm/i915: Force background color to black for gen9+
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10819/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2)
2018-11-13 23:21 [PATCH v2 0/3] CRTC background color Matt Roper
` (5 preceding siblings ...)
2018-11-13 23:58 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-11-14 1:34 ` Patchwork
2018-11-14 4:48 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
7 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-11-14 1:34 UTC (permalink / raw)
To: Matt Roper; +Cc: igt-dev
== Series Details ==
Series: tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2)
URL : https://patchwork.freedesktop.org/series/52456/
State : success
== Summary ==
= CI Bug Log - changes from IGT_4714 -> IGTPW_2058 =
== Summary - WARNING ==
Minor unknown changes coming with IGTPW_2058 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_2058, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/52456/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in IGTPW_2058:
=== IGT changes ===
==== Warnings ====
igt@kms_chamelium@hdmi-hpd-fast:
fi-icl-u2: SKIP -> PASS +3
igt@kms_flip@basic-flip-vs-modeset:
fi-kbl-x1275: SKIP -> PASS +36
igt@prime_vgem@basic-fence-flip:
fi-cfl-s3: SKIP -> PASS
== Known issues ==
Here are the changes found in IGTPW_2058 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@live_contexts:
fi-icl-u: NOTRUN -> DMESG-FAIL (fdo#108569)
igt@kms_chamelium@common-hpd-after-suspend:
fi-icl-u2: SKIP -> DMESG-FAIL (fdo#108070, fdo#103375, fdo#107732)
igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
fi-icl-u2: PASS -> DMESG-WARN (fdo#107732)
==== Possible fixes ====
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-byt-clapper: FAIL (fdo#107362, fdo#103191) -> PASS
==== Warnings ====
igt@drv_selftest@live_contexts:
fi-icl-u2: INCOMPLETE (fdo#108315) -> DMESG-FAIL (fdo#108569)
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107732 https://bugs.freedesktop.org/show_bug.cgi?id=107732
fdo#108070 https://bugs.freedesktop.org/show_bug.cgi?id=108070
fdo#108315 https://bugs.freedesktop.org/show_bug.cgi?id=108315
fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569
== Participating hosts (53 -> 46) ==
Additional (1): fi-icl-u
Missing (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-snb-2520m fi-ctg-p8600 fi-blb-e6850
== Build changes ==
* IGT: IGT_4714 -> IGTPW_2058
CI_DRM_5106: 852b9329fbb6ea8bdbb3dac78328aae73d919305 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2058: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2058/
IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2058/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* [igt-dev] ✗ Fi.CI.IGT: failure for tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2)
2018-11-13 23:21 [PATCH v2 0/3] CRTC background color Matt Roper
` (6 preceding siblings ...)
2018-11-14 1:34 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2) Patchwork
@ 2018-11-14 4:48 ` Patchwork
7 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-11-14 4:48 UTC (permalink / raw)
To: Matt Roper; +Cc: igt-dev
== Series Details ==
Series: tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2)
URL : https://patchwork.freedesktop.org/series/52456/
State : failure
== Summary ==
= CI Bug Log - changes from IGT_4714_full -> IGTPW_2058_full =
== Summary - FAILURE ==
Serious unknown changes coming with IGTPW_2058_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_2058_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/52456/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in IGTPW_2058_full:
=== IGT changes ===
==== Possible regressions ====
igt@gem_exec_reuse@baggage:
shard-apl: PASS -> DMESG-WARN
== Known issues ==
Here are the changes found in IGTPW_2058_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
shard-kbl: NOTRUN -> DMESG-WARN (fdo#107956) +1
igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
shard-apl: PASS -> DMESG-WARN (fdo#107956)
igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
shard-glk: NOTRUN -> DMESG-WARN (fdo#107956) +1
igt@kms_color@pipe-c-legacy-gamma:
shard-apl: PASS -> FAIL (fdo#104782)
igt@kms_cursor_crc@cursor-128x128-random:
shard-apl: PASS -> FAIL (fdo#103232) +5
igt@kms_cursor_crc@cursor-256x256-dpms:
shard-glk: PASS -> FAIL (fdo#103232)
igt@kms_cursor_crc@cursor-256x256-random:
shard-glk: NOTRUN -> FAIL (fdo#103232) +3
igt@kms_cursor_crc@cursor-256x256-suspend:
shard-apl: PASS -> FAIL (fdo#103191, fdo#103232)
igt@kms_cursor_crc@cursor-64x64-onscreen:
shard-kbl: PASS -> FAIL (fdo#103232) +3
igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
shard-glk: PASS -> DMESG-WARN (fdo#105763, fdo#106538)
igt@kms_fbcon_fbt@fbc-suspend:
shard-snb: PASS -> INCOMPLETE (fdo#105411)
igt@kms_flip@2x-flip-vs-expired-vblank:
shard-glk: PASS -> FAIL (fdo#105363)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
shard-glk: NOTRUN -> FAIL (fdo#103167)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
shard-apl: PASS -> FAIL (fdo#103167) +3
shard-kbl: PASS -> FAIL (fdo#103167) +1
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move:
shard-glk: PASS -> FAIL (fdo#103167) +4
igt@kms_plane@plane-position-covered-pipe-a-planes:
shard-glk: NOTRUN -> FAIL (fdo#103166) +1
igt@kms_plane@plane-position-covered-pipe-c-planes:
shard-apl: PASS -> FAIL (fdo#103166) +4
igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
shard-glk: NOTRUN -> FAIL (fdo#108145) +7
igt@kms_properties@connector-properties-atomic:
shard-glk: NOTRUN -> FAIL (fdo#108642)
shard-hsw: NOTRUN -> FAIL (fdo#108642)
igt@kms_setmode@basic:
shard-apl: PASS -> FAIL (fdo#99912)
igt@kms_vblank@invalid:
shard-apl: PASS -> INCOMPLETE (fdo#103927) +1
==== Possible fixes ====
igt@drv_suspend@forcewake:
shard-kbl: INCOMPLETE (fdo#103665) -> PASS
igt@drv_suspend@shrink:
shard-snb: INCOMPLETE (fdo#105411, fdo#106886) -> PASS
igt@gem_ppgtt@blt-vs-render-ctx0:
shard-kbl: INCOMPLETE (fdo#103665, fdo#106023, fdo#106887) -> PASS
igt@kms_busy@extended-modeset-hang-newfb-render-b:
shard-hsw: DMESG-WARN (fdo#107956) -> PASS
igt@kms_cursor_crc@cursor-128x42-sliding:
shard-kbl: FAIL (fdo#103232) -> PASS +3
shard-apl: FAIL (fdo#103232) -> PASS +1
igt@kms_cursor_crc@cursor-256x85-random:
shard-glk: FAIL (fdo#103232) -> PASS +2
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
shard-apl: FAIL (fdo#103167) -> PASS +1
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
shard-glk: FAIL (fdo#103167) -> PASS +3
igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
shard-apl: FAIL (fdo#103166) -> PASS
igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
shard-kbl: FAIL (fdo#103166) -> PASS
igt@kms_universal_plane@universal-plane-pipe-a-functional:
shard-glk: FAIL (fdo#103166) -> PASS +3
igt@kms_vblank@pipe-a-query-busy:
shard-snb: INCOMPLETE (fdo#105411) -> PASS
igt@perf_pmu@busy-start-vcs0:
shard-apl: DMESG-WARN (fdo#105602, fdo#103558) -> PASS
==== Warnings ====
igt@kms_content_protection@atomic:
shard-kbl: DMESG-FAIL (fdo#108550) -> FAIL (fdo#108597)
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
fdo#106887 https://bugs.freedesktop.org/show_bug.cgi?id=106887
fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
fdo#108550 https://bugs.freedesktop.org/show_bug.cgi?id=108550
fdo#108597 https://bugs.freedesktop.org/show_bug.cgi?id=108597
fdo#108642 https://bugs.freedesktop.org/show_bug.cgi?id=108642
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (6 -> 5) ==
Missing (1): shard-skl
== Build changes ==
* IGT: IGT_4714 -> IGTPW_2058
CI_DRM_5106: 852b9329fbb6ea8bdbb3dac78328aae73d919305 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2058: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2058/
IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2058/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/3] drm: Add CRTC background color property (v2)
2018-11-13 23:21 ` [PATCH v2 2/3] drm: Add CRTC background color property (v2) Matt Roper
@ 2018-11-14 16:17 ` Sean Paul
2018-11-15 0:27 ` Matt Roper
2018-11-14 17:20 ` Ville Syrjälä
1 sibling, 1 reply; 22+ messages in thread
From: Sean Paul @ 2018-11-14 16:17 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx, dri-devel, wei.c.li, harish.krupo.kps, Sean Paul
On Tue, Nov 13, 2018 at 03:21:48PM -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.
>
> 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>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
> drivers/gpu/drm/drm_atomic_uapi.c | 5 +++++
> drivers/gpu/drm/drm_blend.c | 21 ++++++++++++++++++---
> drivers/gpu/drm/drm_mode_config.c | 6 ++++++
> include/drm/drm_blend.h | 1 +
> include/drm/drm_crtc.h | 17 +++++++++++++++++
> include/drm/drm_mode_config.h | 5 +++++
> include/uapi/drm/drm_mode.h | 26 ++++++++++++++++++++++++++
> 8 files changed, 79 insertions(+), 3 deletions(-)
/snip
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index d3e0fe31efc5..7c4f902aa290 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -888,6 +888,32 @@ struct drm_mode_revoke_lease {
> __u32 lessee_id;
> };
>
> +/*
> + * Put RGBA 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_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha)
> +{
> + 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 RGBA value.
> + */
> +#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits))
> +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
> +#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
> +#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
#define DRM_RGBA_COMP(c, shift, numbits) \
(__u16)(((c) & 0xFFFFull << (shift)) >> (32 - ((shift) + 16 - (numbits)))
#define DRM_RGBA_BLUE(c, numbits) DRM_RGBA_COMP(c, 0, numbits)
#define DRM_RGBA_GREEN(c, numbits) DRM_RGBA_COMP(c, 16, numbits)
#define DRM_RGBA_RED(c, numbits) DRM_RGBA_COMP(c, 32, numbits)
#define DRM_RGBA_ALPHA(c, numbits) DRM_RGBA_COMP(c, 48, numbits)
With that,
Reviewed-by: Sean Paul <sean@poorly.run>
> +
> #if defined(__cplusplus)
> }
> #endif
> --
> 2.14.4
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/3] drm: Add CRTC background color property (v2)
2018-11-13 23:21 ` [PATCH v2 2/3] drm: Add CRTC background color property (v2) Matt Roper
2018-11-14 16:17 ` Sean Paul
@ 2018-11-14 17:20 ` Ville Syrjälä
2018-11-14 17:29 ` Matt Roper
1 sibling, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2018-11-14 17:20 UTC (permalink / raw)
To: Matt Roper; +Cc: wei.c.li, intel-gfx, Sean Paul, dri-devel, harish.krupo.kps
On Tue, Nov 13, 2018 at 03:21:48PM -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.
>
> 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>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
> drivers/gpu/drm/drm_atomic_uapi.c | 5 +++++
> drivers/gpu/drm/drm_blend.c | 21 ++++++++++++++++++---
> drivers/gpu/drm/drm_mode_config.c | 6 ++++++
> include/drm/drm_blend.h | 1 +
> include/drm/drm_crtc.h | 17 +++++++++++++++++
> include/drm/drm_mode_config.h | 5 +++++
> include/uapi/drm/drm_mode.h | 26 ++++++++++++++++++++++++++
> 8 files changed, 79 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 3ba996069d69..2f8c55668089 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -101,6 +101,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
> state->planes_changed = false;
> state->connectors_changed = false;
> state->color_mgmt_changed = false;
> + state->bgcolor_changed = false;
Wondering a bit about the granulairty of these flags. Is bgcolor
important enough to warrant its own flag, or should it just be part of
color_mgmt_changed for example?
color_mgmt_changed is rather heavy though as it would force LUT
reprogramming, so I've been thinking that it should perhaps be
split into separate
gamma_lut_changed/degamma_lut_changed/other_color_stuff_changed
flags.
Anyways, just thinking out loud here. We can combine some
of these flags later if we start to accumulate too many.
> state->zpos_changed = false;
> state->commit = NULL;
> state->event = NULL;
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 86ac33922b09..b95a55a778e2 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -467,6 +467,9 @@ 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;
> + state->bgcolor_changed = true;
> } else if (crtc->funcs->atomic_set_property) {
> return crtc->funcs->atomic_set_property(crtc, state, property, val);
> } else {
> @@ -499,6 +502,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..7c73cb83874a 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -175,9 +175,16 @@
> * 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:
> + *
> + * bgcolor:
> + * Background color is setup with drm_crtc_add_bgcolor_property(). It
> + * controls the RGB color of a full-screen, fully-opaque 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.
> */
>
> /**
> @@ -593,3 +600,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_rgba(16, 0, 0, 0, 0xffff));
> +}
> +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 ee80788f2c40..75e376755176 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -352,6 +352,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 b21437bc95bf..d78f82f954e4 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -168,6 +168,11 @@ struct drm_crtc_state {
> * drivers to steer the atomic commit control flow.
> */
> bool color_mgmt_changed : 1;
> + /**
> + * @bgcolor_changed: Background color value has changed. Used by
> + * drivers to steer the atomic commit control flow.
> + */
> + bool bgcolor_changed : 1;
>
> /**
> * @no_vblank:
> @@ -274,6 +279,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_rgba();
> + * individual color components can be extracted with desired precision
> + * via the DRM_RGBA_*() macros.
> + */
> + u64 bgcolor;
> +
> /**
> * @target_vblank:
> *
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 5dbeabdbaf91..8b686d228feb 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -813,6 +813,11 @@ struct drm_mode_config {
> */
> struct drm_property *writeback_out_fence_ptr_property;
>
> + /**
> + * @bgcolor_property: RGBA 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 d3e0fe31efc5..7c4f902aa290 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -888,6 +888,32 @@ struct drm_mode_revoke_lease {
> __u32 lessee_id;
> };
>
> +/*
> + * Put RGBA 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_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha)
> +{
> + 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;
So the value is now ARGB but everything still says RGBA?
> +}
> +
> +/*
> + * Extract the specified number of bits of a specific color component from a
> + * standard 64-bit RGBA value.
> + */
> +#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits))
> +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
> +#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
> +#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
> +
> #if defined(__cplusplus)
> }
> #endif
> --
> 2.14.4
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] drm/i915: Force background color to black for gen9+
2018-11-13 23:21 ` [PATCH v2 1/3] drm/i915: Force background color to black for gen9+ Matt Roper
@ 2018-11-14 17:28 ` Ville Syrjälä
2018-11-14 17:36 ` Matt Roper
0 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2018-11-14 17:28 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Tue, Nov 13, 2018 at 03:21:47PM -0800, Matt Roper wrote:
> We don't yet allow userspace to control the CRTC background color, but
> we should manually program the color to black to ensure the BIOS didn't
> leave us with some other color. We should also set the pipe gamma and
> pipe CSC bits so that the background color goes through the same color
> management transformations that a plane with black pixels would.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> We may want to land this patch before the rest of the series since it's
> still valuable even without the new ABI the rest of the series adds.
>
> drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
> drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index fe4b913e46ac..b92a721c9bcb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5662,6 +5662,12 @@ enum {
> #define PIPEMISC_DITHER_TYPE_SP (0 << 2)
> #define PIPEMISC(pipe) _MMIO_PIPE2(pipe, _PIPE_MISC_A)
>
> +/* Skylake+ pipe bottom (background) color */
> +#define _SKL_CANVAS_A 0x70034
> +#define SKL_CANVAS_GAMMA_ENABLE (1 << 31)
> +#define SKL_CANVAS_CSC_ENABLE (1 << 30)
> +#define SKL_CANVAS(pipe) _MMIO_PIPE2(pipe, _SKL_CANVAS_A)
Didn't the spec call this BOTTOM_COLOR or something like that?
> +
> #define VLV_DPFLIPSTAT _MMIO(VLV_DISPLAY_BASE + 0x70028)
> #define PIPEB_LINE_COMPARE_INT_EN (1 << 29)
> #define PIPEB_HLINE_INT_EN (1 << 28)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 132e978227fb..1d089d93d88b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3868,6 +3868,15 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
> else if (old_crtc_state->pch_pfit.enabled)
> ironlake_pfit_disable(old_crtc_state);
> }
> +
> + /*
> + * We don't (yet) allow userspace to control the pipe background color,
> + * so force it to black, but apply pipe gamma and CSC so that its
> + * handling will match how we program our planes.
> + */
> + if (INTEL_GEN(dev_priv) >= 9)
> + I915_WRITE(SKL_CANVAS(crtc->pipe),
> + SKL_CANVAS_GAMMA_ENABLE | SKL_CANVAS_CSC_ENABLE);
> }
>
> static void intel_fdi_normal_train(struct intel_crtc *crtc)
> @@ -15356,6 +15365,15 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
> plane->base.type != DRM_PLANE_TYPE_PRIMARY)
> intel_plane_disable_noatomic(crtc, plane);
> }
> +
> + /*
> + * Disable any background color set by the BIOS, but enable the
> + * gamma and CSC to match how we program our planes.
> + */
> + if (INTEL_GEN(dev_priv) >= 9)
> + I915_WRITE(SKL_CANVAS(crtc->pipe),
> + SKL_CANVAS_GAMMA_ENABLE |
> + SKL_CANVAS_CSC_ENABLE);
> }
>
> /* Adjust the state of the output pipe according to whether we
> --
> 2.14.4
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/3] drm: Add CRTC background color property (v2)
2018-11-14 17:20 ` Ville Syrjälä
@ 2018-11-14 17:29 ` Matt Roper
0 siblings, 0 replies; 22+ messages in thread
From: Matt Roper @ 2018-11-14 17:29 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: wei.c.li, intel-gfx, dri-devel, harish.krupo.kps
On Wed, Nov 14, 2018 at 07:20:36PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 13, 2018 at 03:21:48PM -0800, Matt Roper wrote:
...
> >
> > +/*
> > + * Put RGBA 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_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha)
> > +{
> > + 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;
>
> So the value is now ARGB but everything still says RGBA?
Well, the general idea was that you're providing four color components
and you're not supposed to be looking behind the curtain at the internal
representation (which may not match any given platform's hardware
ordering). If everyone uses the helpers here to pack/unpack the
components, then that avoids accidental differences in interpretation
between different platforms.
But I'm fine with changing the names; I'll do that when I incorporate
Sean's suggestion.
Matt
>
> > +}
> > +
> > +/*
> > + * Extract the specified number of bits of a specific color component from a
> > + * standard 64-bit RGBA value.
> > + */
> > +#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits))
> > +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
> > +#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
> > +#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
> > +
> > #if defined(__cplusplus)
> > }
> > #endif
> > --
> > 2.14.4
>
> --
> Ville Syrjälä
> Intel
--
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] drm/i915: Force background color to black for gen9+
2018-11-14 17:28 ` Ville Syrjälä
@ 2018-11-14 17:36 ` Matt Roper
2018-11-14 17:58 ` Ville Syrjälä
0 siblings, 1 reply; 22+ messages in thread
From: Matt Roper @ 2018-11-14 17:36 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Wed, Nov 14, 2018 at 07:28:12PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 13, 2018 at 03:21:47PM -0800, Matt Roper wrote:
> > We don't yet allow userspace to control the CRTC background color, but
> > we should manually program the color to black to ensure the BIOS didn't
> > leave us with some other color. We should also set the pipe gamma and
> > pipe CSC bits so that the background color goes through the same color
> > management transformations that a plane with black pixels would.
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > We may want to land this patch before the rest of the series since it's
> > still valuable even without the new ABI the rest of the series adds.
> >
> > drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
> > drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++++++++
> > 2 files changed, 24 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index fe4b913e46ac..b92a721c9bcb 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5662,6 +5662,12 @@ enum {
> > #define PIPEMISC_DITHER_TYPE_SP (0 << 2)
> > #define PIPEMISC(pipe) _MMIO_PIPE2(pipe, _PIPE_MISC_A)
> >
> > +/* Skylake+ pipe bottom (background) color */
> > +#define _SKL_CANVAS_A 0x70034
> > +#define SKL_CANVAS_GAMMA_ENABLE (1 << 31)
> > +#define SKL_CANVAS_CSC_ENABLE (1 << 30)
> > +#define SKL_CANVAS(pipe) _MMIO_PIPE2(pipe, _SKL_CANVAS_A)
>
> Didn't the spec call this BOTTOM_COLOR or something like that?
The register definition is called BOTTOM_COLOR, but other areas of the
spec refer to this as either "background color" or "canvas color." We
already have a CHV_CANVAS register that I believe provides the same
functionality for CHV (although I can't find details for the bit layout
in the spec), so I decided to just keep this name consistent with that
one.
Matt
>
> > +
> > #define VLV_DPFLIPSTAT _MMIO(VLV_DISPLAY_BASE + 0x70028)
> > #define PIPEB_LINE_COMPARE_INT_EN (1 << 29)
> > #define PIPEB_HLINE_INT_EN (1 << 28)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 132e978227fb..1d089d93d88b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3868,6 +3868,15 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
> > else if (old_crtc_state->pch_pfit.enabled)
> > ironlake_pfit_disable(old_crtc_state);
> > }
> > +
> > + /*
> > + * We don't (yet) allow userspace to control the pipe background color,
> > + * so force it to black, but apply pipe gamma and CSC so that its
> > + * handling will match how we program our planes.
> > + */
> > + if (INTEL_GEN(dev_priv) >= 9)
> > + I915_WRITE(SKL_CANVAS(crtc->pipe),
> > + SKL_CANVAS_GAMMA_ENABLE | SKL_CANVAS_CSC_ENABLE);
> > }
> >
> > static void intel_fdi_normal_train(struct intel_crtc *crtc)
> > @@ -15356,6 +15365,15 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
> > plane->base.type != DRM_PLANE_TYPE_PRIMARY)
> > intel_plane_disable_noatomic(crtc, plane);
> > }
> > +
> > + /*
> > + * Disable any background color set by the BIOS, but enable the
> > + * gamma and CSC to match how we program our planes.
> > + */
> > + if (INTEL_GEN(dev_priv) >= 9)
> > + I915_WRITE(SKL_CANVAS(crtc->pipe),
> > + SKL_CANVAS_GAMMA_ENABLE |
> > + SKL_CANVAS_CSC_ENABLE);
> > }
> >
> > /* Adjust the state of the output pipe according to whether we
> > --
> > 2.14.4
>
> --
> Ville Syrjälä
> Intel
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] drm/i915: Force background color to black for gen9+
2018-11-14 17:36 ` Matt Roper
@ 2018-11-14 17:58 ` Ville Syrjälä
0 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2018-11-14 17:58 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Wed, Nov 14, 2018 at 09:36:39AM -0800, Matt Roper wrote:
> On Wed, Nov 14, 2018 at 07:28:12PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 13, 2018 at 03:21:47PM -0800, Matt Roper wrote:
> > > We don't yet allow userspace to control the CRTC background color, but
> > > we should manually program the color to black to ensure the BIOS didn't
> > > leave us with some other color. We should also set the pipe gamma and
> > > pipe CSC bits so that the background color goes through the same color
> > > management transformations that a plane with black pixels would.
> > >
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > ---
> > > We may want to land this patch before the rest of the series since it's
> > > still valuable even without the new ABI the rest of the series adds.
> > >
> > > drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
> > > drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++++++++
> > > 2 files changed, 24 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index fe4b913e46ac..b92a721c9bcb 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -5662,6 +5662,12 @@ enum {
> > > #define PIPEMISC_DITHER_TYPE_SP (0 << 2)
> > > #define PIPEMISC(pipe) _MMIO_PIPE2(pipe, _PIPE_MISC_A)
> > >
> > > +/* Skylake+ pipe bottom (background) color */
> > > +#define _SKL_CANVAS_A 0x70034
> > > +#define SKL_CANVAS_GAMMA_ENABLE (1 << 31)
> > > +#define SKL_CANVAS_CSC_ENABLE (1 << 30)
> > > +#define SKL_CANVAS(pipe) _MMIO_PIPE2(pipe, _SKL_CANVAS_A)
> >
> > Didn't the spec call this BOTTOM_COLOR or something like that?
>
> The register definition is called BOTTOM_COLOR, but other areas of the
> spec refer to this as either "background color" or "canvas color." We
> already have a CHV_CANVAS register that I believe provides the same
> functionality for CHV (although I can't find details for the bit layout
> in the spec), so I decided to just keep this name consistent with that
> one.
While CHV_CANVAS does provide simiar functionality (actually
just the color, not the gamma/csc control) it is still a
different register. IMO better to stick to the naming used
in the spec.
>
>
> Matt
>
> >
> > > +
> > > #define VLV_DPFLIPSTAT _MMIO(VLV_DISPLAY_BASE + 0x70028)
> > > #define PIPEB_LINE_COMPARE_INT_EN (1 << 29)
> > > #define PIPEB_HLINE_INT_EN (1 << 28)
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 132e978227fb..1d089d93d88b 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3868,6 +3868,15 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
> > > else if (old_crtc_state->pch_pfit.enabled)
> > > ironlake_pfit_disable(old_crtc_state);
> > > }
> > > +
> > > + /*
> > > + * We don't (yet) allow userspace to control the pipe background color,
> > > + * so force it to black, but apply pipe gamma and CSC so that its
> > > + * handling will match how we program our planes.
> > > + */
> > > + if (INTEL_GEN(dev_priv) >= 9)
> > > + I915_WRITE(SKL_CANVAS(crtc->pipe),
> > > + SKL_CANVAS_GAMMA_ENABLE | SKL_CANVAS_CSC_ENABLE);
> > > }
> > >
> > > static void intel_fdi_normal_train(struct intel_crtc *crtc)
> > > @@ -15356,6 +15365,15 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
> > > plane->base.type != DRM_PLANE_TYPE_PRIMARY)
> > > intel_plane_disable_noatomic(crtc, plane);
> > > }
> > > +
> > > + /*
> > > + * Disable any background color set by the BIOS, but enable the
> > > + * gamma and CSC to match how we program our planes.
> > > + */
> > > + if (INTEL_GEN(dev_priv) >= 9)
> > > + I915_WRITE(SKL_CANVAS(crtc->pipe),
> > > + SKL_CANVAS_GAMMA_ENABLE |
> > > + SKL_CANVAS_CSC_ENABLE);
> > > }
> > >
> > > /* Adjust the state of the output pipe according to whether we
> > > --
> > > 2.14.4
> >
> > --
> > Ville Syrjälä
> > Intel
>
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 3/3] drm/i915/gen9+: Add support for pipe background color (v2)
2018-11-13 23:21 ` [PATCH v2 3/3] drm/i915/gen9+: Add support for pipe background color (v2) Matt Roper
@ 2018-11-14 18:05 ` Ville Syrjälä
0 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2018-11-14 18:05 UTC (permalink / raw)
To: Matt Roper; +Cc: wei.c.li, intel-gfx, dri-devel, harish.krupo.kps
On Tue, Nov 13, 2018 at 03:21:49PM -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.
>
> 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 | 35 ++++++++++++++++++++++++++++-------
> 2 files changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 670db5073d70..1f2a19e6ec79 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3254,6 +3254,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_RGBA_RED(background, 10),
> + DRM_RGBA_GREEN(background, 10),
> + DRM_RGBA_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 1d089d93d88b..e7a759e0c021 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3834,6 +3834,27 @@ 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 *cstate)
s/cstate/crtc_state/ please
> +{
> + struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + uint64_t propval = cstate->base.bgcolor;
> + uint32_t tmp;
> +
> + /* Hardware is programmed with 10 bits of precision */
> + tmp = DRM_RGBA_RED(propval, 10) << 20
> + | DRM_RGBA_GREEN(propval, 10) << 10
> + | DRM_RGBA_BLUE(propval, 10);
> +
> + /*
> + * Set CSC and gamma for bottom color to ensure background pixels
> + * receive the same color transformations as plane content.
> + */
> + tmp |= SKL_CANVAS_CSC_ENABLE | SKL_CANVAS_GAMMA_ENABLE;
> +
> + I915_WRITE_FW(SKL_CANVAS(crtc->pipe), tmp);
Why _FW?
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/3] drm: Add CRTC background color property (v2)
2018-11-14 16:17 ` Sean Paul
@ 2018-11-15 0:27 ` Matt Roper
0 siblings, 0 replies; 22+ messages in thread
From: Matt Roper @ 2018-11-15 0:27 UTC (permalink / raw)
To: Sean Paul; +Cc: wei.c.li, intel-gfx, dri-devel, harish.krupo.kps
On Wed, Nov 14, 2018 at 11:17:25AM -0500, Sean Paul wrote:
> On Tue, Nov 13, 2018 at 03:21:48PM -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.
> >
> > 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>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
> > drivers/gpu/drm/drm_atomic_uapi.c | 5 +++++
> > drivers/gpu/drm/drm_blend.c | 21 ++++++++++++++++++---
> > drivers/gpu/drm/drm_mode_config.c | 6 ++++++
> > include/drm/drm_blend.h | 1 +
> > include/drm/drm_crtc.h | 17 +++++++++++++++++
> > include/drm/drm_mode_config.h | 5 +++++
> > include/uapi/drm/drm_mode.h | 26 ++++++++++++++++++++++++++
> > 8 files changed, 79 insertions(+), 3 deletions(-)
>
> /snip
>
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index d3e0fe31efc5..7c4f902aa290 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -888,6 +888,32 @@ struct drm_mode_revoke_lease {
> > __u32 lessee_id;
> > };
> >
> > +/*
> > + * Put RGBA 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_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha)
> > +{
> > + 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 RGBA value.
> > + */
> > +#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xFFFFull) >> (16-numbits))
> > +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
> > +#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
> > +#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
>
>
> #define DRM_RGBA_COMP(c, shift, numbits) \
> (__u16)(((c) & 0xFFFFull << (shift)) >> (32 - ((shift) + 16 - (numbits)))
I think this should just be
#define DRM_RGBA_COMP(c, shift, numbits) \
(__u16)(((c) & 0xFFFFull << (shift)) >> ((shift) + 16 - (numbits)))
right? I.e., right shift the same amount as the left shift, plus an
additional (16-numbits) to account for the lower precision requested.
Matt
>
> #define DRM_RGBA_BLUE(c, numbits) DRM_RGBA_COMP(c, 0, numbits)
> #define DRM_RGBA_GREEN(c, numbits) DRM_RGBA_COMP(c, 16, numbits)
> #define DRM_RGBA_RED(c, numbits) DRM_RGBA_COMP(c, 32, numbits)
> #define DRM_RGBA_ALPHA(c, numbits) DRM_RGBA_COMP(c, 48, numbits)
>
>
> With that,
>
> Reviewed-by: Sean Paul <sean@poorly.run>
>
>
>
> > +
> > #if defined(__cplusplus)
> > }
> > #endif
> > --
> > 2.14.4
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2018-11-15 0:27 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-13 23:21 [PATCH v2 0/3] CRTC background color Matt Roper
2018-11-13 23:21 ` [PATCH v2 1/3] drm/i915: Force background color to black for gen9+ Matt Roper
2018-11-14 17:28 ` Ville Syrjälä
2018-11-14 17:36 ` Matt Roper
2018-11-14 17:58 ` Ville Syrjälä
2018-11-13 23:21 ` [PATCH v2 2/3] drm: Add CRTC background color property (v2) Matt Roper
2018-11-14 16:17 ` Sean Paul
2018-11-15 0:27 ` Matt Roper
2018-11-14 17:20 ` Ville Syrjälä
2018-11-14 17:29 ` Matt Roper
2018-11-13 23:21 ` [PATCH v2 3/3] drm/i915/gen9+: Add support for pipe background color (v2) Matt Roper
2018-11-14 18:05 ` Ville Syrjälä
2018-11-13 23:22 ` [PATCH i-g-t v2] tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2) Matt Roper
2018-11-13 23:22 ` [igt-dev] " Matt Roper
2018-11-13 23:43 ` Lionel Landwerlin
2018-11-13 23:43 ` [igt-dev] [Intel-gfx] " Lionel Landwerlin
2018-11-13 23:53 ` Matt Roper
2018-11-13 23:53 ` [igt-dev] [Intel-gfx] " Matt Roper
2018-11-13 23:31 ` ✗ Fi.CI.CHECKPATCH: warning for CRTC background color (rev2) Patchwork
2018-11-13 23:58 ` ✓ Fi.CI.BAT: success " Patchwork
2018-11-14 1:34 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_crtc_background_color: overhaul for latest ABI proposal (v2) Patchwork
2018-11-14 4:48 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.