[PATCH v5 0/3] CRTC background color

[PATCH v5 0/3] CRTC background color

[Matt Roper]

Previous patch series/discussion was here:
  https://lists.freedesktop.org/archives/dri-devel/2019-January/205504.html

Reviewed userspace (chromeos) is here:
  https://chromium-review.googlesource.com/c/chromium/src/+/1278858
  https://chromium-review.googlesource.com/c/chromiumos/platform/drm-tests/+/1241436

i-g-t test is here:
  https://lists.freedesktop.org/archives/igt-dev/2019-January/008751.html


Just minor changes in this revision:
 - Patch 1:
    * drm_crtc_add_bgcolor_property() also explicitly sets the crtc
      state field to solid black in case the driver doesn't already do
      this.
    * Added kerneldoc for drm_crtc_add_bgcolor_property()

 - Patch 2:
    * s/uint64_t/u64/

 - Patch 3:
    * New i915 patch to add bgcolor to hardware readout & state
      verification as suggested by Ville


I think patches #1 and 2 have the necessary reviews and ABI prereqs to
merge at this point.  Dave/Daniel, any worries if we merge this through
the intel tree rather than drm-misc?  I think merging through the Intel
tree may help avoid some minor conflicts with an upcoming series from
Ville.


Matt Roper (3):
  drm: Add CRTC background color property (v5)
  drm/i915/gen9+: Add support for pipe background color (v5)
  drm/i915: Add background color hardware readout and state check

 drivers/gpu/drm/drm_atomic_uapi.c    |  4 ++
 drivers/gpu/drm/drm_blend.c          | 41 +++++++++++++++++--
 drivers/gpu/drm/drm_mode_config.c    |  6 +++
 drivers/gpu/drm/i915/i915_debugfs.c  |  9 +++++
 drivers/gpu/drm/i915/intel_display.c | 78 +++++++++++++++++++++++++++++++-----
 include/drm/drm_blend.h              |  1 +
 include/drm/drm_crtc.h               | 12 ++++++
 include/drm/drm_mode_config.h        |  5 +++
 include/uapi/drm/drm_mode.h          | 28 +++++++++++++
 9 files changed, 170 insertions(+), 14 deletions(-)

-- 
2.14.5

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

[PATCH v5 1/3] drm: Add CRTC background color property (v5)

[Matt Roper]

Some display controllers can be programmed to present non-black colors
for pixels not covered by any plane (or pixels covered by the
transparent regions of higher planes).  Compositors that want a UI with
a solid color background can potentially save memory bandwidth by
setting the CRTC background property and using smaller planes to display
the rest of the content.

To avoid confusion between different ways of encoding RGB data, we
define a standard 64-bit format that should be used for this property's
value.  Helper functions and macros are provided to generate and dissect
values in this standard format with varying component precision values.

v2:
 - Swap internal representation's blue and red bits to make it easier
   to read if printed out.  (Ville)
 - Document bgcolor property in drm_blend.c.  (Sean Paul)
 - s/background_color/bgcolor/ for consistency between property name and
   value storage field.  (Sean Paul)
 - Add a convenience function to attach property to a given crtc.

v3:
 - Restructure ARGB component extraction macros to be easier to
   understand and enclose the parameters in () to avoid calculations
   if expressions are passed.  (Sean Paul)
 - s/rgba/argb/ in helper function/macro names.  Even though the idea is
   to not worry about the internal representation of the u64, it can
   still be confusing to look at code that uses 'rgba' terminology, but
   stores values with argb ordering.  (Ville)

v4:
 - Drop the bgcolor_changed flag.  (Ville, Brian Starkey)
 - Clarify in kerneldoc that background color is expected to undergo the
   same pipe-level degamma/csc/gamma transformations that planes do.
   (Brian Starkey)
 - Update kerneldoc to indicate non-opaque colors are allowed, but are
   generally only useful in special cases such as when writeback
   connectors are used (Brian Starkey / Eric Anholt)

v5:
 - Set crtc->state->bgcolor to solid black inside
   drm_crtc_add_bgcolor_property() in case drivers don't do this
   themselves.  (Ville)
 - Add kerneldoc to drm_crtc_add_bgcolor_property() function.

Cc: dri-devel@lists.freedesktop.org
Cc: wei.c.li@intel.com
Cc: harish.krupo.kps@intel.com
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Brian Starkey <Brian.Starkey@arm.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Stéphane Marchesin <marcheu@chromium.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by(v2): Sean Paul <sean@poorly.run>
Reviewed-by: Brian Starkey <brian.starkey@arm.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  4 ++++
 drivers/gpu/drm/drm_blend.c       | 41 ++++++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/drm_mode_config.c |  6 ++++++
 include/drm/drm_blend.h           |  1 +
 include/drm/drm_crtc.h            | 12 ++++++++++++
 include/drm/drm_mode_config.h     |  5 +++++
 include/uapi/drm/drm_mode.h       | 28 ++++++++++++++++++++++++++
 7 files changed, 94 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 9a1f41adfc67..d569e20e34e3 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -469,6 +469,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 			return -EFAULT;
 
 		set_out_fence_for_crtc(state->state, crtc, fence_ptr);
+	} else if (property == config->bgcolor_property) {
+		state->bgcolor = val;
 	} else if (crtc->funcs->atomic_set_property) {
 		return crtc->funcs->atomic_set_property(crtc, state, property, val);
 	} else {
@@ -503,6 +505,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
 	else if (property == config->prop_out_fence_ptr)
 		*val = 0;
+	else if (property == config->bgcolor_property)
+		*val = state->bgcolor;
 	else if (crtc->funcs->atomic_get_property)
 		return crtc->funcs->atomic_get_property(crtc, state, property, val);
 	else
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 0c78ca386cbe..2ff69fae385c 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -175,9 +175,22 @@
  *		 plane does not expose the "alpha" property, then this is
  *		 assumed to be 1.0
  *
- * Note that all the property extensions described here apply either to the
- * plane or the CRTC (e.g. for the background color, which currently is not
- * exposed and assumed to be black).
+ * The property extensions described above all apply to the plane.  Drivers
+ * may also expose the following crtc property extension:
+ *
+ * BACKGROUND_COLOR:
+ *	Background color is setup with drm_crtc_add_bgcolor_property().  It
+ *	controls the ARGB color of a full-screen layer that exists below all
+ *	planes.  This color will be used for pixels not covered by any plane
+ *	and may also be blended with plane contents as allowed by a plane's
+ *	alpha values.  The background color defaults to black, and is assumed
+ *	to be black for drivers that do not expose this property.  Although
+ *	background color isn't a plane, it is assumed that the color provided
+ *	here undergoes the same pipe-level degamma/CSC/gamma transformations
+ *	that planes undergo.  Note that the color value provided here includes
+ *	an alpha channel...non-opaque background color values are allowed,
+ *	but are generally only honored in special cases (e.g., when a memory
+ *	writeback connector is in use).
  */
 
 /**
@@ -593,3 +606,25 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane,
 	return 0;
 }
 EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+
+/**
+ * drm_crtc_add_bgcolor_property - add background color property
+ * @crtc: drm crtc
+ *
+ * Adds the background color property to @crtc.  The property defaults to
+ * solid black and will accept 64-bit ARGB values in the format generated by
+ * drm_argb().  @crtc's state will also be updated to hold a solid black
+ * background color when this function is called.
+ */
+void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc)
+{
+	u64 initval = drm_argb(16, 0xffff, 0, 0, 0);
+
+	drm_object_attach_property(&crtc->base,
+				   crtc->dev->mode_config.bgcolor_property,
+				   initval);
+
+	if (crtc->state)
+		crtc->state->bgcolor = initval;
+}
+EXPORT_SYMBOL(drm_crtc_add_bgcolor_property);
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 4a1c2023ccf0..8a7c346b3191 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -364,6 +364,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.modifiers_property = prop;
 
+	prop = drm_property_create_range(dev, 0, "BACKGROUND_COLOR",
+					 0, GENMASK_ULL(63, 0));
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.bgcolor_property = prop;
+
 	return 0;
 }
 
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 88bdfec3bd88..9e2538dd7b9a 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -58,4 +58,5 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
 			      struct drm_atomic_state *state);
 int drm_plane_create_blend_mode_property(struct drm_plane *plane,
 					 unsigned int supported_modes);
+void drm_crtc_add_bgcolor_property(struct drm_crtc *crtc);
 #endif
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 85abd3fe9e83..dbe0b45d5da6 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -274,6 +274,18 @@ struct drm_crtc_state {
 	 */
 	struct drm_property_blob *gamma_lut;
 
+	/**
+	 * @bgcolor:
+	 *
+	 * RGB value representing the pipe's background color.  The background
+	 * color (aka "canvas color") of a pipe is the color that will be used
+	 * for pixels not covered by a plane, or covered by transparent pixels
+	 * of a plane.  The value here should be built via drm_argb();
+	 * individual color components can be extracted with desired precision
+	 * via the DRM_ARGB_*() macros.
+	 */
+	u64 bgcolor;
+
 	/**
 	 * @target_vblank:
 	 *
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 1e6cb885994d..0463d3f4ae59 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -836,6 +836,11 @@ struct drm_mode_config {
 	 */
 	struct drm_property *writeback_out_fence_ptr_property;
 
+	/**
+	 * @bgcolor_property: RGB background color for CRTC.
+	 */
+	struct drm_property *bgcolor_property;
+
 	/* dumb ioctl parameters */
 	uint32_t preferred_depth, prefer_shadow;
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index a439c2e67896..5f31e6a05bd9 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -907,6 +907,34 @@ struct drm_mode_rect {
 	__s32 y2;
 };
 
+/*
+ * Put ARGB values into a standard 64-bit representation that can be used
+ * for ioctl parameters, inter-driver commmunication, etc.  If the component
+ * values being provided contain less than 16 bits of precision, they'll
+ * be shifted into the most significant bits.
+ */
+static inline __u64
+drm_argb(__u8 bpc, __u16 alpha, __u16 red, __u16 green, __u16 blue)
+{
+	int msb_shift = 16 - bpc;
+
+	return (__u64)alpha << msb_shift << 48 |
+	       (__u64)red   << msb_shift << 32 |
+	       (__u64)green << msb_shift << 16 |
+	       (__u64)blue  << msb_shift;
+}
+
+/*
+ * Extract the specified number of bits of a specific color component from a
+ * standard 64-bit ARGB value.
+ */
+#define DRM_ARGB_COMP(c, shift, numbits) \
+	(__u16)(((c) & 0xFFFFull << (shift)) >> ((shift) + 16 - (numbits)))
+#define DRM_ARGB_BLUE(c, numbits)  DRM_ARGB_COMP(c, 0, numbits)
+#define DRM_ARGB_GREEN(c, numbits) DRM_ARGB_COMP(c, 16, numbits)
+#define DRM_ARGB_RED(c, numbits)   DRM_ARGB_COMP(c, 32, numbits)
+#define DRM_ARGB_ALPHA(c, numbits) DRM_ARGB_COMP(c, 48, numbits)
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.14.5

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

[PATCH v5 2/3] drm/i915/gen9+: Add support for pipe background color (v5)

[Matt Roper]

Gen9+ platforms allow CRTC's to be programmed with a background/canvas
color below the programmable planes.  Let's expose this for use by
compositors.

v2:
 - Split out bgcolor sanitization and programming of csc/gamma bits to a
   separate patch that we can land before the ABI changes are ready to
   go in.  (Ville)
 - Change a temporary variable name to be more consistent with
   other similar functions.  (Ville)
 - Change register name to SKL_CANVAS for consistency with the
   CHV_CANVAS register.

v3:
 - Switch register name back to SKL_BOTTOM_COLOR.  (Ville)
 - Use non-_FW register write.  (Ville)
 - Minor parameter rename for consistency.  (Ville)

v4:
 - Removed use of bgcolor_changed flag.

v5:
 - s/uint64_t/u64/

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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  9 +++++++
 drivers/gpu/drm/i915/intel_display.c | 46 +++++++++++++++++++++++++++---------
 2 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index fa2c226fc779..e96fef9fa97c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3092,6 +3092,15 @@ static int i915_display_info(struct seq_file *m, void *unused)
 			intel_plane_info(m, crtc);
 		}
 
+		if (INTEL_GEN(dev_priv) >= 9 && pipe_config->base.active) {
+			u64 background = pipe_config->base.bgcolor;
+
+			seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
+				   DRM_ARGB_RED(background, 10),
+				   DRM_ARGB_GREEN(background, 10),
+				   DRM_ARGB_BLUE(background, 10));
+		}
+
 		seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",
 			   yesno(!crtc->cpu_fifo_underrun_disabled),
 			   yesno(!crtc->pch_fifo_underrun_disabled));
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a6d8985fe2e0..469480fe01a4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3896,6 +3896,28 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 	clear_bit(I915_RESET_MODESET, &dev_priv->gpu_error.flags);
 }
 
+static void
+skl_update_background_color(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u64 propval = crtc_state->base.bgcolor;
+	u32 tmp;
+
+	/* Hardware is programmed with 10 bits of precision */
+	tmp = DRM_ARGB_RED(propval, 10) << 20
+	    | DRM_ARGB_GREEN(propval, 10) << 10
+	    | DRM_ARGB_BLUE(propval, 10);
+
+	/*
+	 * Set CSC and gamma for bottom color to ensure background pixels
+	 * receive the same color transformations as plane content.
+	 */
+	tmp |= SKL_BOTTOM_COLOR_CSC_ENABLE | SKL_BOTTOM_COLOR_GAMMA_ENABLE;
+
+	I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe), tmp);
+}
+
 static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state,
 				     const struct intel_crtc_state *new_crtc_state)
 {
@@ -3931,15 +3953,8 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
 			ironlake_pfit_disable(old_crtc_state);
 	}
 
-	/*
-	 * We don't (yet) allow userspace to control the pipe background color,
-	 * so force it to black, but apply pipe gamma and CSC so that its
-	 * handling will match how we program our planes.
-	 */
 	if (INTEL_GEN(dev_priv) >= 9)
-		I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe),
-			   SKL_BOTTOM_COLOR_GAMMA_ENABLE |
-			   SKL_BOTTOM_COLOR_CSC_ENABLE);
+		skl_update_background_color(new_crtc_state);
 }
 
 static void intel_fdi_normal_train(struct intel_crtc *crtc)
@@ -11042,6 +11057,8 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_crtc_state *pipe_config =
 		to_intel_crtc_state(crtc_state);
+	struct drm_crtc_state *old_crtc_state =
+		drm_atomic_get_old_crtc_state(crtc_state->state, crtc);
 	int ret;
 	bool mode_changed = needs_modeset(crtc_state);
 
@@ -11069,6 +11086,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 		crtc_state->planes_changed = true;
 	}
 
+	if (crtc_state->bgcolor != old_crtc_state->bgcolor)
+		pipe_config->update_pipe = true;
+
 	ret = 0;
 	if (dev_priv->display.compute_pipe_wm) {
 		ret = dev_priv->display.compute_pipe_wm(pipe_config);
@@ -14238,6 +14258,9 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 	WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
 
+	if (INTEL_GEN(dev_priv) >= 9)
+		drm_crtc_add_bgcolor_property(&intel_crtc->base);
+
 	return 0;
 
 fail:
@@ -15480,6 +15503,9 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 
+	/* Always force bgcolor to solid black */
+	crtc_state->base.bgcolor = drm_argb(16, 0xFFFF, 0, 0, 0);
+
 	/* Clear any frame start delays used for debugging left by the BIOS */
 	if (crtc->active && !transcoder_is_dsi(cpu_transcoder)) {
 		i915_reg_t reg = PIPECONF(cpu_transcoder);
@@ -15506,9 +15532,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 		 * gamma and CSC to match how we program our planes.
 		 */
 		if (INTEL_GEN(dev_priv) >= 9)
-			I915_WRITE(SKL_BOTTOM_COLOR(crtc->pipe),
-				   SKL_BOTTOM_COLOR_GAMMA_ENABLE |
-				   SKL_BOTTOM_COLOR_CSC_ENABLE);
+			skl_update_background_color(crtc_state);
 	}
 
 	/* Adjust the state of the output pipe according to whether we
-- 
2.14.5

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

[PATCH v5 3/3] drm/i915: Add background color hardware readout and state check

[Matt Roper]

We should support readout and verification of crtc background color as
we do with other pipe state.  Note that our hardware holds less bits of
precision than the CRTC state allows, so we need to take care to only
verify the most significant bits of the color after performing readout.

At boot time the pipe color is already sanitized to full black as
required by ABI, so the new readout here won't break that requirement.

Suggested-by: Ville Syrjälä <ville.syrjala@linux.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/intel_display.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 469480fe01a4..ecbfc8ce7b54 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9740,6 +9740,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum intel_display_power_domain power_domain;
 	u64 power_domain_mask;
+	u32 bgcolor;
 	bool active;
 
 	intel_crtc_init_scalers(crtc, pipe_config);
@@ -9806,6 +9807,15 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		pipe_config->pixel_multiplier = 1;
 	}
 
+	if (INTEL_GEN(dev_priv) >= 9) {
+		bgcolor = I915_READ(SKL_BOTTOM_COLOR(crtc->pipe));
+		pipe_config->base.bgcolor =
+			drm_argb(10, 0xFFFF,
+				 bgcolor >> 20 & 0x3FF,
+				 bgcolor >> 10 & 0x3FF,
+				 bgcolor       & 0x3FF);
+	}
+
 out:
 	for_each_power_domain(power_domain, power_domain_mask)
 		intel_display_power_put_unchecked(dev_priv, power_domain);
@@ -11422,6 +11432,10 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 				      drm_rect_width(&state->base.dst),
 				      drm_rect_height(&state->base.dst));
 	}
+
+	if (INTEL_GEN(dev_priv) >= 9)
+		DRM_DEBUG_KMS("background color: %llx\n",
+			      pipe_config->base.bgcolor);
 }
 
 static bool check_digital_port_conflicts(struct drm_atomic_state *state)
@@ -11784,6 +11798,16 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
 	} \
 } while (0)
 
+#define PIPE_CONF_CHECK_LLX_MASKED(name, mask) do { \
+	if ((current_config->name & mask) != (pipe_config->name & mask)) { \
+		pipe_config_err(adjust, __stringify(name), \
+			  "(expected 0x%016llx, found 0x%016llx)\n", \
+			  current_config->name & mask, \
+			  pipe_config->name & mask); \
+		ret = false; \
+	} \
+} while (0)
+
 #define PIPE_CONF_CHECK_I(name) do { \
 	if (current_config->name != pipe_config->name) { \
 		pipe_config_err(adjust, __stringify(name), \
@@ -12035,6 +12059,14 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
 
 	PIPE_CONF_CHECK_I(min_voltage_level);
 
+	/*
+	 * Hardware only holds top 10 bits of each color component; ignore
+	 * bottom six bits (and all of alpha) when comparing against readout.
+	 */
+	if (INTEL_GEN(dev_priv) >= 9)
+		PIPE_CONF_CHECK_LLX_MASKED(base.bgcolor, 0x0000FFC0FFC0FFC0);
+
+#undef PIPE_CONF_CHECK_LLX_MASKED
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
 #undef PIPE_CONF_CHECK_BOOL
-- 
2.14.5

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

✗ Fi.CI.CHECKPATCH: warning for CRTC background color (rev6)

[Patchwork]

== Series Details ==

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

== Summary ==

$ dim checkpatch origin/drm-tip
8c549af67706 drm: Add CRTC background color property (v5)
-:239: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'shift' - possible side-effects?
#239: FILE: include/uapi/drm/drm_mode.h:931:
+#define DRM_ARGB_COMP(c, shift, numbits) \
+	(__u16)(((c) & 0xFFFFull << (shift)) >> ((shift) + 16 - (numbits)))

total: 0 errors, 0 warnings, 1 checks, 146 lines checked
e8c3b924963a drm/i915/gen9+: Add support for pipe background color (v5)
7d3a7c26e2ef drm/i915: Add background color hardware readout and state check
-:65: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#65: FILE: drivers/gpu/drm/i915/intel_display.c:11801:
+#define PIPE_CONF_CHECK_LLX_MASKED(name, mask) do { \
+	if ((current_config->name & mask) != (pipe_config->name & mask)) { \
+		pipe_config_err(adjust, __stringify(name), \
+			  "(expected 0x%016llx, found 0x%016llx)\n", \
+			  current_config->name & mask, \
+			  pipe_config->name & mask); \
+		ret = false; \
+	} \
+} while (0)

-:65: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#65: FILE: drivers/gpu/drm/i915/intel_display.c:11801:
+#define PIPE_CONF_CHECK_LLX_MASKED(name, mask) do { \
+	if ((current_config->name & mask) != (pipe_config->name & mask)) { \
+		pipe_config_err(adjust, __stringify(name), \
+			  "(expected 0x%016llx, found 0x%016llx)\n", \
+			  current_config->name & mask, \
+			  pipe_config->name & mask); \
+		ret = false; \
+	} \
+} while (0)

-:65: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'mask' - possible side-effects?
#65: FILE: drivers/gpu/drm/i915/intel_display.c:11801:
+#define PIPE_CONF_CHECK_LLX_MASKED(name, mask) do { \
+	if ((current_config->name & mask) != (pipe_config->name & mask)) { \
+		pipe_config_err(adjust, __stringify(name), \
+			  "(expected 0x%016llx, found 0x%016llx)\n", \
+			  current_config->name & mask, \
+			  pipe_config->name & mask); \
+		ret = false; \
+	} \
+} while (0)

-:65: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'mask' may be better as '(mask)' to avoid precedence issues
#65: FILE: drivers/gpu/drm/i915/intel_display.c:11801:
+#define PIPE_CONF_CHECK_LLX_MASKED(name, mask) do { \
+	if ((current_config->name & mask) != (pipe_config->name & mask)) { \
+		pipe_config_err(adjust, __stringify(name), \
+			  "(expected 0x%016llx, found 0x%016llx)\n", \
+			  current_config->name & mask, \
+			  pipe_config->name & mask); \
+		ret = false; \
+	} \
+} while (0)

total: 0 errors, 0 warnings, 4 checks, 62 lines checked

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

✗ Fi.CI.SPARSE: warning for CRTC background color (rev6)

[Patchwork]

== Series Details ==

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

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm: Add CRTC background color property (v5)
Okay!

Commit: drm/i915/gen9+: Add support for pipe background color (v5)
Okay!

Commit: drm/i915: Add background color hardware readout and state check
+drivers/gpu/drm/i915/intel_display.c:12067:17: warning: constant 0x0000FFC0FFC0FFC0 is so big it is long
+drivers/gpu/drm/i915/intel_display.c:12067:17: warning: constant 0x0000FFC0FFC0FFC0 is so big it is long
+drivers/gpu/drm/i915/intel_display.c:12067:17: warning: constant 0x0000FFC0FFC0FFC0 is so big it is long
+drivers/gpu/drm/i915/intel_display.c:12067:17: warning: constant 0x0000FFC0FFC0FFC0 is so big it is long

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

✓ Fi.CI.BAT: success for CRTC background color (rev6)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5521 -> Patchwork_12112
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103167]

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  
#### Possible fixes ####

  * igt@i915_selftest@live_hangcheck:
    - {fi-icl-y}:         INCOMPLETE -> PASS

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS +1

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

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (49 -> 45)
------------------------------

  Additional (1): fi-hsw-4770r 
  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5521 -> Patchwork_12112

  CI_DRM_5521: dbd2e19079beb3b7f4077706179fba66d321e49f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4802: 4049adf01014af077df2174def4fadf7cecb066e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12112: 7d3a7c26e2ef212cb7e71c1ec19308d6f27b6506 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

7d3a7c26e2ef drm/i915: Add background color hardware readout and state check
e8c3b924963a drm/i915/gen9+: Add support for pipe background color (v5)
8c549af67706 drm: Add CRTC background color property (v5)

== Logs ==

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

✓ Fi.CI.IGT: success for CRTC background color (rev6)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5521_full -> Patchwork_12112_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@reset-stress:
    - shard-snb:          PASS -> INCOMPLETE [fdo#105411]

  * igt@kms_atomic_transition@plane-use-after-nonblocking-unbind:
    - shard-hsw:          PASS -> DMESG-WARN [fdo#102614] +1

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          PASS -> FAIL [fdo#105454] / [fdo#106509]

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-apl:          PASS -> FAIL [fdo#103166]

  * igt@kms_setmode@basic:
    - shard-hsw:          PASS -> FAIL [fdo#99912]

  
#### Possible fixes ####

  * igt@kms_color@pipe-c-ctm-max:
    - shard-apl:          FAIL [fdo#108147] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-dpms:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          FAIL [fdo#105363] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-apl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-glk:          FAIL [fdo#103166] -> PASS +2

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

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105454]: https://bugs.freedesktop.org/show_bug.cgi?id=105454
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (6 -> 4)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * Linux: CI_DRM_5521 -> Patchwork_12112

  CI_DRM_5521: dbd2e19079beb3b7f4077706179fba66d321e49f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4802: 4049adf01014af077df2174def4fadf7cecb066e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12112: 7d3a7c26e2ef212cb7e71c1ec19308d6f27b6506 @ 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_12112/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 3/3] drm/i915: Add background color hardware readout and state check

[Ville Syrjälä]

On Thu, Jan 31, 2019 at 03:55:08PM -0800, Matt Roper wrote:
> We should support readout and verification of crtc background color as
> we do with other pipe state.  Note that our hardware holds less bits of
> precision than the CRTC state allows, so we need to take care to only
> verify the most significant bits of the color after performing readout.
> 
> At boot time the pipe color is already sanitized to full black as
> required by ABI, so the new readout here won't break that requirement.
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 469480fe01a4..ecbfc8ce7b54 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9740,6 +9740,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum intel_display_power_domain power_domain;
>  	u64 power_domain_mask;
> +	u32 bgcolor;
>  	bool active;
>  
>  	intel_crtc_init_scalers(crtc, pipe_config);
> @@ -9806,6 +9807,15 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		pipe_config->pixel_multiplier = 1;
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 9) {
> +		bgcolor = I915_READ(SKL_BOTTOM_COLOR(crtc->pipe));
> +		pipe_config->base.bgcolor =
> +			drm_argb(10, 0xFFFF,
> +				 bgcolor >> 20 & 0x3FF,
> +				 bgcolor >> 10 & 0x3FF,
> +				 bgcolor       & 0x3FF);
> +	}
> +
>  out:
>  	for_each_power_domain(power_domain, power_domain_mask)
>  		intel_display_power_put_unchecked(dev_priv, power_domain);
> @@ -11422,6 +11432,10 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  				      drm_rect_width(&state->base.dst),
>  				      drm_rect_height(&state->base.dst));
>  	}
> +
> +	if (INTEL_GEN(dev_priv) >= 9)
> +		DRM_DEBUG_KMS("background color: %llx\n",
> +			      pipe_config->base.bgcolor);
>  }
>  
>  static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> @@ -11784,6 +11798,16 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>  	} \
>  } while (0)
>  
> +#define PIPE_CONF_CHECK_LLX_MASKED(name, mask) do { \
> +	if ((current_config->name & mask) != (pipe_config->name & mask)) { \
> +		pipe_config_err(adjust, __stringify(name), \
> +			  "(expected 0x%016llx, found 0x%016llx)\n", \
> +			  current_config->name & mask, \
> +			  pipe_config->name & mask); \
> +		ret = false; \
> +	} \
> +} while (0)
> +
>  #define PIPE_CONF_CHECK_I(name) do { \
>  	if (current_config->name != pipe_config->name) { \
>  		pipe_config_err(adjust, __stringify(name), \
> @@ -12035,6 +12059,14 @@ intel_pipe_config_compare(struct drm_i915_private *dev_priv,
>  
>  	PIPE_CONF_CHECK_I(min_voltage_level);
>  
> +	/*
> +	 * Hardware only holds top 10 bits of each color component; ignore
> +	 * bottom six bits (and all of alpha) when comparing against readout.
> +	 */
> +	if (INTEL_GEN(dev_priv) >= 9)
> +		PIPE_CONF_CHECK_LLX_MASKED(base.bgcolor, 0x0000FFC0FFC0FFC0);
> +
> +#undef PIPE_CONF_CHECK_LLX_MASKED
>  #undef PIPE_CONF_CHECK_X
>  #undef PIPE_CONF_CHECK_I
>  #undef PIPE_CONF_CHECK_BOOL
> -- 
> 2.14.5

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