All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] CRTC background color
@ 2018-10-10 23:50 Matt Roper
  2018-10-10 23:50 ` [PATCH 1/2] drm: Add CRTC background color property Matt Roper
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Matt Roper @ 2018-10-10 23:50 UTC (permalink / raw)
  To: intel-gfx; +Cc: wei.c.li, dri-devel, 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.

Earlier versions of these patches were floated on dri-devel about 2.5
years ago, but at that time the only userspace software that made use of
this was closed-source (product-specific Wayland compositors), so we
never landed the patches upstream.  I'm told that there's now some
renewed interest in this functionality from both the ChromeOS camp and
the Weston camp, so I'm re-posting updated kernel patches here to get
the ball rolling again.  As always, we'll still need the patches for at
least one of those projects to get posted (and reviewed) somewhere
public before we actually merge these kernel patches.

Cc: dri-devel@lists.freedesktop.org
Cc: wei.c.li@intel.com
Cc: harish.krupo.kps@intel.com

Matt Roper (2):
  drm: Add CRTC background color property
  drm/i915/gen9+: Add support for pipe background color

 drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
 drivers/gpu/drm/drm_atomic_uapi.c         |  5 +++++
 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      | 34 +++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h                    | 17 ++++++++++++++++
 include/drm/drm_mode_config.h             |  5 +++++
 include/uapi/drm/drm_mode.h               | 26 +++++++++++++++++++++++
 9 files changed, 109 insertions(+)

-- 
2.14.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] drm: Add CRTC background color property
  2018-10-10 23:50 [PATCH 0/2] CRTC background color Matt Roper
@ 2018-10-10 23:50 ` Matt Roper
  2018-10-11 11:30   ` Ville Syrjälä
  2018-11-07 16:14   ` [Intel-gfx] " Sean Paul
  2018-10-10 23:50 ` [PATCH 2/2] drm/i915/gen9+: Add support for pipe background color Matt Roper
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 12+ messages in thread
From: Matt Roper @ 2018-10-10 23:50 UTC (permalink / raw)
  To: intel-gfx; +Cc: wei.c.li, dri-devel, 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.

Cc: dri-devel@lists.freedesktop.org
Cc: wei.c.li@intel.com
Cc: harish.krupo.kps@intel.com
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_mode_config.c         |  6 ++++++
 include/drm/drm_crtc.h                    | 17 +++++++++++++++++
 include/drm/drm_mode_config.h             |  5 +++++
 include/uapi/drm/drm_mode.h               | 26 ++++++++++++++++++++++++++
 6 files changed, 60 insertions(+)

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 d5b7f315098c..399f0ead5370 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->background_color = 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->background_color;
 	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_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_crtc.h b/include/drm/drm_crtc.h
index b21437bc95bf..ddfdad9ccecb 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;
 
+	/**
+	 * @background_color:
+	 *
+	 * 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 background_color;
+
 	/**
 	 * @target_vblank:
 	 *
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 928e4172a0bb..c3f57a9e5b31 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -807,6 +807,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;
 	bool quirk_addfb_prefer_xbgr_30bpp;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index d3e0fe31efc5..66e2e2c2630e 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)blue  << msb_shift << 32 |
+	       (__u64)green << msb_shift << 16 |
+	       (__u64)red   << msb_shift;
+}
+
+/*
+ * Extract the specified number of bits of a specific color component from a
+ * standard 64-bit RGBA value.
+ */
+#define DRM_RGBA_RED(c, numbits)   (__u16)((c & 0xFFFFull)     >> (16-numbits))
+#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
+#define DRM_RGBA_BLUE(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] 12+ messages in thread

* [PATCH 2/2] drm/i915/gen9+: Add support for pipe background color
  2018-10-10 23:50 [PATCH 0/2] CRTC background color Matt Roper
  2018-10-10 23:50 ` [PATCH 1/2] drm: Add CRTC background color property Matt Roper
@ 2018-10-10 23:50 ` Matt Roper
  2018-10-11 11:33   ` Ville Syrjälä
  2018-10-10 23:55 ` [PATCH i-g-t] tests/kms_crtc_background_color: overhaul for latest ABI proposal Matt Roper
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Matt Roper @ 2018-10-10 23:50 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.

Cc: dri-devel@lists.freedesktop.org
Cc: wei.c.li@intel.com
Cc: harish.krupo.kps@intel.com
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  9 +++++++++
 drivers/gpu/drm/i915/i915_reg.h      |  6 ++++++
 drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 4565eda29c87..cc423f7f3e62 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.background_color;
+
+			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/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 20785417953d..988183870f6e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5661,6 +5661,12 @@ enum {
 #define   PIPEMISC_DITHER_TYPE_SP	(0 << 2)
 #define PIPEMISC(pipe)			_MMIO_PIPE2(pipe, _PIPE_MISC_A)
 
+/* Skylake+ pipe bottom (background) color */
+#define _PIPE_BOTTOM_COLOR_A		0x70034
+#define PIPE_BOTTOM_GAMMA_ENABLE	(1 << 31)
+#define PIPE_BOTTOM_CSC_ENABLE		(1 << 30)
+#define PIPE_BOTTOM_COLOR(pipe)		_MMIO_PIPE2(pipe, _PIPE_BOTTOM_COLOR_A)
+
 #define VLV_DPFLIPSTAT				_MMIO(VLV_DISPLAY_BASE + 0x70028)
 #define   PIPEB_LINE_COMPARE_INT_EN		(1 << 29)
 #define   PIPEB_HLINE_INT_EN			(1 << 28)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a145efba9157..2ee402a98e70 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3853,6 +3853,28 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 	clear_bit(I915_RESET_MODESET, &dev_priv->gpu_error.flags);
 }
 
+static void skl_update_background_color(const struct intel_crtc_state *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.background_color;
+	uint32_t hwval;
+
+	/* Hardware is programmed with 10 bits of precision */
+	hwval = 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.
+	 */
+	hwval |= PIPE_BOTTOM_CSC_ENABLE;
+	hwval |= PIPE_BOTTOM_GAMMA_ENABLE;
+
+	I915_WRITE_FW(PIPE_BOTTOM_COLOR(crtc->pipe), hwval);
+}
+
 static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state,
 				     const struct intel_crtc_state *new_crtc_state)
 {
@@ -3887,6 +3909,9 @@ 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);
 	}
+
+	if (new_crtc_state->base.bgcolor_changed)
+		skl_update_background_color(new_crtc_state);
 }
 
 static void intel_fdi_normal_train(struct intel_crtc *crtc)
@@ -10791,6 +10816,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);
@@ -13831,6 +13859,7 @@ static void intel_crtc_init_scalers(struct intel_crtc *crtc,
 
 static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
+	struct drm_mode_config *mode_config = &dev_priv->drm.mode_config;
 	struct intel_crtc *intel_crtc;
 	struct intel_crtc_state *crtc_state = NULL;
 	struct intel_plane *primary = NULL;
@@ -13905,6 +13934,11 @@ 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_object_attach_property(&intel_crtc->base.base,
+					   mode_config->bgcolor_property,
+					   drm_rgba(16, 0, 0, 0, 0xffff));
+
 	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] 12+ messages in thread

* [PATCH i-g-t] tests/kms_crtc_background_color: overhaul for latest ABI proposal
  2018-10-10 23:50 [PATCH 0/2] CRTC background color Matt Roper
  2018-10-10 23:50 ` [PATCH 1/2] drm: Add CRTC background color property Matt Roper
  2018-10-10 23:50 ` [PATCH 2/2] drm/i915/gen9+: Add support for pipe background color Matt Roper
@ 2018-10-10 23:55 ` Matt Roper
  2018-10-11  0:06 ` ✗ Fi.CI.CHECKPATCH: warning for CRTC background color Patchwork
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Matt Roper @ 2018-10-10 23:55 UTC (permalink / raw)
  To: intel-gfx

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.

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 b2cbaa11..f34817b5 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..9c5a7523 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)blue  << msb_shift << 32 |
+              (__u64)green << msb_shift << 16 |
+              (__u64)red   << msb_shift;
+}
+#define LOCAL_RGBA_RED(c, numbits)   (__u16)((c & 0xFFFFull)     >> (16-numbits))
+#define LOCAL_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
+#define LOCAL_RGBA_BLUE(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] 12+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for CRTC background color
  2018-10-10 23:50 [PATCH 0/2] CRTC background color Matt Roper
                   ` (2 preceding siblings ...)
  2018-10-10 23:55 ` [PATCH i-g-t] tests/kms_crtc_background_color: overhaul for latest ABI proposal Matt Roper
@ 2018-10-11  0:06 ` Patchwork
  2018-10-11  0:28 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-10-11  0:06 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

== Series Details ==

Series: CRTC background color
URL   : https://patchwork.freedesktop.org/series/50834/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
472f325f4ac4 drm: Add CRTC background color property
-:87: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield.  Prefer bool bitfields as unsigned int or u<8|16|32>
#87: FILE: include/drm/drm_crtc.h:175:
+	bool bgcolor_changed : 1;

-:155: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#155: FILE: include/uapi/drm/drm_mode.h:912:
+#define DRM_RGBA_RED(c, numbits)   (__u16)((c & 0xFFFFull)     >> (16-numbits))
                                                                      ^

-:155: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'c' may be better as '(c)' to avoid precedence issues
#155: FILE: include/uapi/drm/drm_mode.h:912:
+#define DRM_RGBA_RED(c, numbits)   (__u16)((c & 0xFFFFull)     >> (16-numbits))

-:155: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'numbits' may be better as '(numbits)' to avoid precedence issues
#155: FILE: include/uapi/drm/drm_mode.h:912:
+#define DRM_RGBA_RED(c, numbits)   (__u16)((c & 0xFFFFull)     >> (16-numbits))

-:156: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#156: FILE: include/uapi/drm/drm_mode.h:913:
+#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
                                                          ^

-:156: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#156: FILE: include/uapi/drm/drm_mode.h:913:
+#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
                                                                      ^

-:156: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'c' may be better as '(c)' to avoid precedence issues
#156: FILE: include/uapi/drm/drm_mode.h:913:
+#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))

-:156: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'numbits' may be better as '(numbits)' to avoid precedence issues
#156: FILE: include/uapi/drm/drm_mode.h:913:
+#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))

-:157: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#157: FILE: include/uapi/drm/drm_mode.h:914:
+#define DRM_RGBA_BLUE(c, numbits)  (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
                                                          ^

-:157: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#157: FILE: include/uapi/drm/drm_mode.h:914:
+#define DRM_RGBA_BLUE(c, numbits)  (__u16)((c & 0xFFFFull<<32) >> (48-numbits))
                                                                      ^

-:157: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'c' may be better as '(c)' to avoid precedence issues
#157: FILE: include/uapi/drm/drm_mode.h:914:
+#define DRM_RGBA_BLUE(c, numbits)  (__u16)((c & 0xFFFFull<<32) >> (48-numbits))

-:157: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'numbits' may be better as '(numbits)' to avoid precedence issues
#157: FILE: include/uapi/drm/drm_mode.h:914:
+#define DRM_RGBA_BLUE(c, numbits)  (__u16)((c & 0xFFFFull<<32) >> (48-numbits))

-:158: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#158: FILE: include/uapi/drm/drm_mode.h:915:
+#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
                                                          ^

-:158: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#158: FILE: include/uapi/drm/drm_mode.h:915:
+#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))
                                                                      ^

-:158: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'c' may be better as '(c)' to avoid precedence issues
#158: FILE: include/uapi/drm/drm_mode.h:915:
+#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xFFFFull<<48) >> (64-numbits))

-:158: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'numbits' may be better as '(numbits)' to avoid precedence issues
#158: 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, 108 lines checked
a0be1f8ad993 drm/i915/gen9+: Add support for pipe background color

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.BAT: success for CRTC background color
  2018-10-10 23:50 [PATCH 0/2] CRTC background color Matt Roper
                   ` (3 preceding siblings ...)
  2018-10-11  0:06 ` ✗ Fi.CI.CHECKPATCH: warning for CRTC background color Patchwork
@ 2018-10-11  0:28 ` Patchwork
  2018-10-11  0:50 ` ✓ Fi.CI.BAT: success for tests/kms_crtc_background_color: overhaul for latest ABI proposal Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-10-11  0:28 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

== Series Details ==

Series: CRTC background color
URL   : https://patchwork.freedesktop.org/series/50834/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4966 -> Patchwork_10418 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50834/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10418 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       PASS -> DMESG-WARN (fdo#105128, fdo#107139)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s3:
      fi-cfl-8109u:       INCOMPLETE (fdo#107187, fdo#108126) -> PASS

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362, fdo#103191) -> PASS +1

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107187 https://bugs.freedesktop.org/show_bug.cgi?id=107187
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#108126 https://bugs.freedesktop.org/show_bug.cgi?id=108126


== Participating hosts (46 -> 41) ==

  Missing    (5): fi-ctg-p8600 fi-bsw-cyan fi-ilk-m540 fi-byt-squawks fi-icl-u2 


== Build changes ==

    * Linux: CI_DRM_4966 -> Patchwork_10418

  CI_DRM_4966: 0ac03a984dd32a77b02eabb6f0ef110ec5b5bdf2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4673: 54cb1aeb4e50dea9f3abae632e317875d147c4ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10418: a0be1f8ad993cd192497be261ebfcc25525018b9 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a0be1f8ad993 drm/i915/gen9+: Add support for pipe background color
472f325f4ac4 drm: Add CRTC background color property

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10418/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.BAT: success for tests/kms_crtc_background_color: overhaul for latest ABI proposal
  2018-10-10 23:50 [PATCH 0/2] CRTC background color Matt Roper
                   ` (4 preceding siblings ...)
  2018-10-11  0:28 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-10-11  0:50 ` Patchwork
  2018-10-11  8:23 ` ✓ Fi.CI.IGT: success for CRTC background color Patchwork
  2018-10-11  9:00 ` ✗ Fi.CI.IGT: failure for tests/kms_crtc_background_color: overhaul for latest ABI proposal Patchwork
  7 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-10-11  0:50 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

== Series Details ==

Series: tests/kms_crtc_background_color: overhaul for latest ABI proposal
URL   : https://patchwork.freedesktop.org/series/50835/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4958 -> IGTPW_1935 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50835/revisions/1/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1935 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@amdgpu/amd_cs_nop@sync-fork-gfx0:
      fi-kbl-8809g:       PASS -> DMESG-WARN (fdo#107762)

    igt@kms_flip@basic-flip-vs-dpms:
      fi-hsw-4770r:       PASS -> DMESG-WARN (fdo#105602)

    
    ==== Possible fixes ====

    igt@pm_rpm@basic-pci-d3-state:
      fi-skl-6600u:       FAIL (fdo#107707) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#107707 https://bugs.freedesktop.org/show_bug.cgi?id=107707
  fdo#107762 https://bugs.freedesktop.org/show_bug.cgi?id=107762


== Participating hosts (48 -> 41) ==

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bwr-2160 fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * IGT: IGT_4672 -> IGTPW_1935

  CI_DRM_4958: 9990e1665029dc2ef4a9c0632b8a2f516263e595 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1935: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1935/
  IGT_4672: 4497591d2572831a9f07fd9e48a2571bfcffe354 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1935/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.IGT: success for CRTC background color
  2018-10-10 23:50 [PATCH 0/2] CRTC background color Matt Roper
                   ` (5 preceding siblings ...)
  2018-10-11  0:50 ` ✓ Fi.CI.BAT: success for tests/kms_crtc_background_color: overhaul for latest ABI proposal Patchwork
@ 2018-10-11  8:23 ` Patchwork
  2018-10-11  9:00 ` ✗ Fi.CI.IGT: failure for tests/kms_crtc_background_color: overhaul for latest ABI proposal Patchwork
  7 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-10-11  8:23 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

== Series Details ==

Series: CRTC background color
URL   : https://patchwork.freedesktop.org/series/50834/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4966_full -> Patchwork_10418_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10418_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10418_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10418_full:

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10418_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_cpu_reloc@full:
      shard-skl:          NOTRUN -> INCOMPLETE (fdo#108073)

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108039)

    igt@kms_atomic_transition@2x-modeset-transitions-nonblocking-fencing:
      shard-glk:          PASS -> INCOMPLETE (k.org#198133, fdo#103359)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956)
      shard-kbl:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_concurrent@pipe-b:
      shard-hsw:          PASS -> DMESG-WARN (fdo#102614)

    igt@kms_fbcon_fbt@psr:
      shard-skl:          NOTRUN -> FAIL (fdo#107882)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363, fdo#102887)

    igt@kms_flip@plain-flip-ts-check:
      shard-skl:          PASS -> FAIL (fdo#100368)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-skl:          NOTRUN -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
      shard-apl:          PASS -> FAIL (fdo#103167) +3

    igt@kms_frontbuffer_tracking@fbc-1p-rte:
      shard-glk:          PASS -> FAIL (fdo#103167, fdo#105682)

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
      shard-glk:          PASS -> FAIL (fdo#103167) +4

    igt@kms_frontbuffer_tracking@fbc-stridechange:
      shard-skl:          NOTRUN -> FAIL (fdo#105683)

    igt@kms_plane@pixel-format-pipe-c-planes:
      shard-skl:          NOTRUN -> DMESG-FAIL (fdo#103166, fdo#106885)

    {igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb}:
      shard-skl:          NOTRUN -> FAIL (fdo#108145)

    igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
      shard-apl:          PASS -> FAIL (fdo#103166) +1

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-glk:          PASS -> FAIL (fdo#103166)

    igt@pm_rpm@sysfs-read:
      shard-skl:          NOTRUN -> INCOMPLETE (fdo#107807)

    igt@pm_rpm@system-suspend-devices:
      shard-skl:          PASS -> INCOMPLETE (fdo#107807)

    
    ==== Possible fixes ====

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-apl:          FAIL (fdo#106641) -> PASS

    igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
      shard-glk:          FAIL (fdo#108145) -> PASS

    igt@kms_cursor_crc@cursor-128x128-random:
      shard-apl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_cursor_crc@cursor-64x64-sliding:
      shard-glk:          FAIL (fdo#103232) -> PASS

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          DMESG-WARN (fdo#105763, fdo#106538) -> PASS +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-apl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-glk:          FAIL (fdo#103167) -> PASS +3

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-mmap-cpu:
      shard-glk:          DMESG-FAIL (fdo#106538) -> PASS

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-apl:          FAIL (fdo#103166) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
      shard-glk:          FAIL (fdo#103166) -> PASS +3

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#105683 https://bugs.freedesktop.org/show_bug.cgi?id=105683
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106885 https://bugs.freedesktop.org/show_bug.cgi?id=106885
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107882 https://bugs.freedesktop.org/show_bug.cgi?id=107882
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
  fdo#108073 https://bugs.freedesktop.org/show_bug.cgi?id=108073
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4966 -> Patchwork_10418

  CI_DRM_4966: 0ac03a984dd32a77b02eabb6f0ef110ec5b5bdf2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4673: 54cb1aeb4e50dea9f3abae632e317875d147c4ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10418: a0be1f8ad993cd192497be261ebfcc25525018b9 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10418/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✗ Fi.CI.IGT: failure for tests/kms_crtc_background_color: overhaul for latest ABI proposal
  2018-10-10 23:50 [PATCH 0/2] CRTC background color Matt Roper
                   ` (6 preceding siblings ...)
  2018-10-11  8:23 ` ✓ Fi.CI.IGT: success for CRTC background color Patchwork
@ 2018-10-11  9:00 ` Patchwork
  7 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-10-11  9:00 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

== Series Details ==

Series: tests/kms_crtc_background_color: overhaul for latest ABI proposal
URL   : https://patchwork.freedesktop.org/series/50835/
State : failure

== Summary ==

= CI Bug Log - changes from IGT_4672_full -> IGTPW_1935_full =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1935_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1935_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/50835/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1935_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_eio@in-flight-suspend:
      shard-glk:          PASS -> FAIL

    
    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-kbl:          PASS -> SKIP +1

    
== Known issues ==

  Here are the changes found in IGTPW_1935_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_cursor_crc@cursor-128x42-random:
      shard-apl:          PASS -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-64x21-onscreen:
      shard-glk:          PASS -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-kbl:          PASS -> FAIL (fdo#103191, fdo#103232)

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#106538, fdo#105763) +2

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-apl:          PASS -> FAIL (fdo#103167) +4

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-blt:
      shard-glk:          PASS -> DMESG-FAIL (fdo#106538)

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-glk:          PASS -> FAIL (fdo#103167) +2

    {igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max}:
      shard-glk:          PASS -> FAIL (fdo#108145)
      shard-kbl:          PASS -> FAIL (fdo#108145)
      shard-apl:          PASS -> FAIL (fdo#108145)

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-apl:          PASS -> FAIL (fdo#103166) +1

    igt@perf@blocking:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    igt@prime_mmap@test_refcounting:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-apl:          FAIL (fdo#106641) -> PASS

    igt@kms_cursor_crc@cursor-128x42-onscreen:
      shard-apl:          FAIL (fdo#103232) -> PASS +2

    igt@kms_cursor_crc@cursor-256x256-sliding:
      shard-glk:          FAIL (fdo#103232) -> PASS +3

    igt@kms_cursor_crc@cursor-64x64-sliding:
      shard-kbl:          FAIL (fdo#103232) -> PASS

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
      shard-hsw:          FAIL (fdo#105767) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
      shard-apl:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
      shard-glk:          FAIL (fdo#103167) -> PASS +1

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-glk:          FAIL (fdo#103166) -> PASS +3

    {igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max}:
      shard-glk:          FAIL (fdo#108145) -> PASS +1
      shard-apl:          FAIL (fdo#108145) -> PASS
      shard-kbl:          FAIL (fdo#108145) -> PASS

    igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
      shard-apl:          FAIL (fdo#103166) -> PASS +2

    
    ==== Warnings ====

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-glk:          INCOMPLETE (k.org#198133, fdo#103359) -> FAIL (fdo#103232)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  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#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-skl 


== Build changes ==

    * IGT: IGT_4672 -> IGTPW_1935
    * Linux: CI_DRM_4952 -> CI_DRM_4958

  CI_DRM_4952: a62e43ba13605a478b22307ea1790d48aea029a6 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4958: 9990e1665029dc2ef4a9c0632b8a2f516263e595 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1935: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1935/
  IGT_4672: 4497591d2572831a9f07fd9e48a2571bfcffe354 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1935/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] drm: Add CRTC background color property
  2018-10-10 23:50 ` [PATCH 1/2] drm: Add CRTC background color property Matt Roper
@ 2018-10-11 11:30   ` Ville Syrjälä
  2018-11-07 16:14   ` [Intel-gfx] " Sean Paul
  1 sibling, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2018-10-11 11:30 UTC (permalink / raw)
  To: Matt Roper; +Cc: wei.c.li, intel-gfx, dri-devel, harish.krupo.kps

On Wed, Oct 10, 2018 at 04:50:50PM -0700, 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.
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: wei.c.li@intel.com
> Cc: harish.krupo.kps@intel.com
> 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_mode_config.c         |  6 ++++++
>  include/drm/drm_crtc.h                    | 17 +++++++++++++++++
>  include/drm/drm_mode_config.h             |  5 +++++
>  include/uapi/drm/drm_mode.h               | 26 ++++++++++++++++++++++++++
>  6 files changed, 60 insertions(+)
> 
> 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 d5b7f315098c..399f0ead5370 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->background_color = 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->background_color;
>  	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_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_crtc.h b/include/drm/drm_crtc.h
> index b21437bc95bf..ddfdad9ccecb 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;
>  
> +	/**
> +	 * @background_color:
> +	 *
> +	 * 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 background_color;
> +
>  	/**
>  	 * @target_vblank:
>  	 *
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 928e4172a0bb..c3f57a9e5b31 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -807,6 +807,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;
>  	bool quirk_addfb_prefer_xbgr_30bpp;
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index d3e0fe31efc5..66e2e2c2630e 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)blue  << msb_shift << 32 |
> +	       (__u64)green << msb_shift << 16 |
> +	       (__u64)red   << msb_shift;

I would still call that abgr in drm terminology (eg. when compared
to the fourccs). And I'd still prefer the argb order as it's what
our hardware expects anyway :) Would make me less confused when
looking at a 64bit hex color value at least.


> +}
> +
> +/*
> + * Extract the specified number of bits of a specific color component from a
> + * standard 64-bit RGBA value.
> + */
> +#define DRM_RGBA_RED(c, numbits)   (__u16)((c & 0xFFFFull)     >> (16-numbits))
> +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
> +#define DRM_RGBA_BLUE(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
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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] 12+ messages in thread

* Re: [PATCH 2/2] drm/i915/gen9+: Add support for pipe background color
  2018-10-10 23:50 ` [PATCH 2/2] drm/i915/gen9+: Add support for pipe background color Matt Roper
@ 2018-10-11 11:33   ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2018-10-11 11:33 UTC (permalink / raw)
  To: Matt Roper; +Cc: wei.c.li, intel-gfx, dri-devel, harish.krupo.kps

On Wed, Oct 10, 2018 at 04:50:51PM -0700, 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.
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: wei.c.li@intel.com
> Cc: harish.krupo.kps@intel.com
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  9 +++++++++
>  drivers/gpu/drm/i915/i915_reg.h      |  6 ++++++
>  drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++++++++++++++++
>  3 files changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 4565eda29c87..cc423f7f3e62 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.background_color;
> +
> +			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/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 20785417953d..988183870f6e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5661,6 +5661,12 @@ enum {
>  #define   PIPEMISC_DITHER_TYPE_SP	(0 << 2)
>  #define PIPEMISC(pipe)			_MMIO_PIPE2(pipe, _PIPE_MISC_A)
>  
> +/* Skylake+ pipe bottom (background) color */
> +#define _PIPE_BOTTOM_COLOR_A		0x70034
> +#define PIPE_BOTTOM_GAMMA_ENABLE	(1 << 31)
> +#define PIPE_BOTTOM_CSC_ENABLE		(1 << 30)
> +#define PIPE_BOTTOM_COLOR(pipe)		_MMIO_PIPE2(pipe, _PIPE_BOTTOM_COLOR_A)
> +
>  #define VLV_DPFLIPSTAT				_MMIO(VLV_DISPLAY_BASE + 0x70028)
>  #define   PIPEB_LINE_COMPARE_INT_EN		(1 << 29)
>  #define   PIPEB_HLINE_INT_EN			(1 << 28)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a145efba9157..2ee402a98e70 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3853,6 +3853,28 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
>  	clear_bit(I915_RESET_MODESET, &dev_priv->gpu_error.flags);
>  }
>  
> +static void skl_update_background_color(const struct intel_crtc_state *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.background_color;
> +	uint32_t hwval;

Just 'val' or 'tmp' would be more consistent with existing code.

> +
> +	/* Hardware is programmed with 10 bits of precision */
> +	hwval = 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.
> +	 */
> +	hwval |= PIPE_BOTTOM_CSC_ENABLE;
> +	hwval |= PIPE_BOTTOM_GAMMA_ENABLE;

Maybe we want these as a separate bugfix up front?

> +
> +	I915_WRITE_FW(PIPE_BOTTOM_COLOR(crtc->pipe), hwval);
> +}
> +
>  static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state,
>  				     const struct intel_crtc_state *new_crtc_state)
>  {
> @@ -3887,6 +3909,9 @@ 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);
>  	}
> +
> +	if (new_crtc_state->base.bgcolor_changed)
> +		skl_update_background_color(new_crtc_state);
>  }
>  
>  static void intel_fdi_normal_train(struct intel_crtc *crtc)
> @@ -10791,6 +10816,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);
> @@ -13831,6 +13859,7 @@ static void intel_crtc_init_scalers(struct intel_crtc *crtc,
>  
>  static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
> +	struct drm_mode_config *mode_config = &dev_priv->drm.mode_config;
>  	struct intel_crtc *intel_crtc;
>  	struct intel_crtc_state *crtc_state = NULL;
>  	struct intel_plane *primary = NULL;
> @@ -13905,6 +13934,11 @@ 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_object_attach_property(&intel_crtc->base.base,
> +					   mode_config->bgcolor_property,
> +					   drm_rgba(16, 0, 0, 0, 0xffff));
> +
>  	return 0;
>  
>  fail:
> -- 
> 2.14.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm: Add CRTC background color property
  2018-10-10 23:50 ` [PATCH 1/2] drm: Add CRTC background color property Matt Roper
  2018-10-11 11:30   ` Ville Syrjälä
@ 2018-11-07 16:14   ` Sean Paul
  1 sibling, 0 replies; 12+ messages in thread
From: Sean Paul @ 2018-11-07 16:14 UTC (permalink / raw)
  To: Matt Roper; +Cc: wei.c.li, intel-gfx, dri-devel, harish.krupo.kps

On Wed, Oct 10, 2018 at 04:50:50PM -0700, 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.
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: wei.c.li@intel.com
> Cc: harish.krupo.kps@intel.com
> 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_mode_config.c         |  6 ++++++
>  include/drm/drm_crtc.h                    | 17 +++++++++++++++++
>  include/drm/drm_mode_config.h             |  5 +++++
>  include/uapi/drm/drm_mode.h               | 26 ++++++++++++++++++++++++++
>  6 files changed, 60 insertions(+)
> 
> 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 d5b7f315098c..399f0ead5370 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->background_color = 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->background_color;
>  	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_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",

This property should be documented somewhere? Maybe under "Color Management
Properties", since they apply to crtc?


> +					 0, GENMASK_ULL(63, 0));
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.bgcolor_property = prop;
> +
>  	return 0;
>  }
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index b21437bc95bf..ddfdad9ccecb 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;
>  
> +	/**
> +	 * @background_color:
> +	 *
> +	 * 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 background_color;

nit: fwiw, I find it's more useful keep the property name consistent throughout
the codebase for easier grepping. So either expand the others from bgcolor to
background_color, or vice versa.

> +
>  	/**
>  	 * @target_vblank:
>  	 *
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 928e4172a0bb..c3f57a9e5b31 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -807,6 +807,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;
>  	bool quirk_addfb_prefer_xbgr_30bpp;
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index d3e0fe31efc5..66e2e2c2630e 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)blue  << msb_shift << 32 |
> +	       (__u64)green << msb_shift << 16 |
> +	       (__u64)red   << msb_shift;
> +}
> +
> +/*
> + * Extract the specified number of bits of a specific color component from a
> + * standard 64-bit RGBA value.
> + */
> +#define DRM_RGBA_RED(c, numbits)   (__u16)((c & 0xFFFFull)     >> (16-numbits))
> +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xFFFFull<<16) >> (32-numbits))
> +#define DRM_RGBA_BLUE(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

-- 
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] 12+ messages in thread

end of thread, other threads:[~2018-11-07 16:14 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-10 23:50 [PATCH 0/2] CRTC background color Matt Roper
2018-10-10 23:50 ` [PATCH 1/2] drm: Add CRTC background color property Matt Roper
2018-10-11 11:30   ` Ville Syrjälä
2018-11-07 16:14   ` [Intel-gfx] " Sean Paul
2018-10-10 23:50 ` [PATCH 2/2] drm/i915/gen9+: Add support for pipe background color Matt Roper
2018-10-11 11:33   ` Ville Syrjälä
2018-10-10 23:55 ` [PATCH i-g-t] tests/kms_crtc_background_color: overhaul for latest ABI proposal Matt Roper
2018-10-11  0:06 ` ✗ Fi.CI.CHECKPATCH: warning for CRTC background color Patchwork
2018-10-11  0:28 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-11  0:50 ` ✓ Fi.CI.BAT: success for tests/kms_crtc_background_color: overhaul for latest ABI proposal Patchwork
2018-10-11  8:23 ` ✓ Fi.CI.IGT: success for CRTC background color Patchwork
2018-10-11  9:00 ` ✗ Fi.CI.IGT: failure for tests/kms_crtc_background_color: overhaul for latest ABI proposal 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.