[PATCH v2 0/2] CRTC background color support for i915

[PATCH v2 0/2] CRTC background color support for i915

[Matt Roper]

Some platforms (e.g., Intel SKL+) support a programmable background canvas
color below the hardware planes.  For more details, see the cover letter for
the previous version of the series posted here:

    https://lists.freedesktop.org/archives/intel-gfx/2015-October/078687.html

I haven't made any updates to libdrm or IGT since the last update, so I'm only
reposting updates of the kernel patches; use the link above to get to the
corresponding libdrm/igt changes.

Note that this series isn't mergeable yet since we don't (yet) have an open
source userspace that can make use of it.  Eventual userspace candidates for
this functionality might be Weston, ChromeOS, or Android hwcomposer.


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

 Documentation/DocBook/gpu.tmpl       | 10 +++-
 drivers/gpu/drm/drm_atomic.c         |  4 ++
 drivers/gpu/drm/drm_crtc.c           | 39 +++++++++++++++
 drivers/gpu/drm/i915/i915_debugfs.c  |  8 ++++
 drivers/gpu/drm/i915/i915_reg.h      |  9 ++++
 drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++
 include/drm/drm_crtc.h               | 92 ++++++++++++++++++++++++++++++++++++
 7 files changed, 207 insertions(+), 1 deletion(-)

-- 
2.1.4

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

[PATCH v2 1/2] drm: Add infrastructure for CRTC background color property (v2)

[Matt Roper]

To support CRTC background color, we need a way of communicating RGB
color values to the DRM.  However there is often a mismatch between how
userspace wants to represent the color value vs how it must be
programmed into the hardware; this mismatch can easily lead to
non-obvious bugs.  Let's create a kernel-side property type that
standardizes the user<->kernel format and add some macros that allow
drivers to extract the bits they care about without having to worry
about the internal representation.  This RGBA property type may also be
useful for future properties like color keys.

These properties are still exposed to userspace as range properties, so
the only userspace change we need are some helpers to build RGBA values
appropriately.

v2:
 - Just use 'struct rgba' rather than a typedef as our opaque RGBA
   datatype. (Emil)
 - Actually use drm_property_create_rgba() to create the background
   color property. (Bob)
 - Add helper to build 64-bit RGBA internal value in appropriate format
   (e.g., for the initial value when attaching a property).  (Bob)

Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/drm_atomic.c |  4 ++
 drivers/gpu/drm/drm_crtc.c   | 39 +++++++++++++++++++
 include/drm/drm_crtc.h       | 92 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8fb469c..e47c250 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -413,6 +413,8 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 
 	if (property == config->prop_active)
 		state->active = val;
+	else if (property == config->prop_background_color)
+		state->background_color.v = val;
 	else if (property == config->prop_mode_id) {
 		struct drm_property_blob *mode =
 			drm_property_lookup_blob(dev, val);
@@ -456,6 +458,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val = state->active;
 	else if (property == config->prop_mode_id)
 		*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
+	else if (property == config->prop_background_color)
+		*val = state->background_color.v;
 	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_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 65258ac..f86fd2d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3933,6 +3933,30 @@ struct drm_property *drm_property_create_bool(struct drm_device *dev, int flags,
 EXPORT_SYMBOL(drm_property_create_bool);
 
 /**
+ * drm_property_create_rgba - create a new RGBA property type
+ * @dev: drm device
+ * @flags: flags specifying the property type
+ * @name: name of the property
+ *
+ * This creates a new generic drm property which can then be attached to a drm
+ * object with drm_object_attach_property. The returned property object must be
+ * freed with drm_property_destroy.
+ *
+ * Userspace should use the DRM_RGBA() macro to build values with the proper
+ * bit layout.
+ *
+ * Returns:
+ * A pointer to the newly created property on success, NULL on failure.
+ */
+struct drm_property *drm_property_create_rgba(struct drm_device *dev, int flags,
+					      const char *name)
+{
+	return drm_property_create_range(dev, flags, name,
+					 0, GENMASK_ULL(63, 0));
+}
+EXPORT_SYMBOL(drm_property_create_rgba);
+
+/**
  * drm_property_add_enum - add a possible value to an enumeration property
  * @property: enumeration property to change
  * @index: index of the new enumeration
@@ -5943,6 +5967,21 @@ struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
 EXPORT_SYMBOL(drm_mode_create_rotation_property);
 
 /**
+ * drm_mode_create_background_color_property - create CRTC color property
+ * @dev: DRM device
+ *
+ * Creates a property to represent CRTC background/canvas color.  Called by a
+ * driver the first time it's needed, must be attached to desired CRTC's.
+ */
+struct drm_property *
+drm_mode_create_background_color_property(struct drm_device *dev)
+{
+	return drm_property_create_rgba(dev, DRM_MODE_PROP_ATOMIC,
+					 "background_color");
+}
+EXPORT_SYMBOL(drm_mode_create_background_color_property);
+
+/**
  * DOC: Tile group
  *
  * Tile groups are used to represent tiled monitors with a unique
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8c7fb3d..e59dace 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -297,6 +297,90 @@ struct drm_connector_helper_funcs;
 struct drm_plane_helper_funcs;
 
 /**
+ * struct drm_rgba - RGBA property value type
+ * @v: Internal representation of RGBA, stored in 16bpc format
+ *
+ * Structure to abstract away the representation of RGBA values with precision
+ * up to 16 bits per color component.  This is primarily intended for use with
+ * DRM properties that need to take a color value since we don't want userspace
+ * to have to worry about the bit layout expected by the underlying hardware.
+ *
+ * We wrap the value in a structure here so that the compiler will flag any
+ * accidental attempts by driver code to directly attempt bitwise operations
+ * that could potentially misinterpret the value.  Drivers should instead use
+ * the DRM_RGBA_{RED,GREEN,BLUE,ALPHA}BITS() macros to obtain the component
+ * bits and then build values in the format their hardware expects.
+ */
+struct drm_rgba {
+	uint64_t v;
+};
+
+/**
+ * drm_rgba - Build RGBA property values
+ * @bpc: Bits per component value for @red, @green, @blue, and @alpha parameters
+ * @red: Red component value
+ * @green: Green component value
+ * @blue: Blue component value
+ * @alpha: Alpha component value
+ *
+ * Helper to build RGBA 16bpc color values with the bits laid out in the format
+ * expected by DRM RGBA properties.
+ *
+ * Returns:
+ * An RGBA structure encapsulating the requested RGBA value.
+ */
+static inline struct drm_rgba
+drm_rgba(unsigned bpc,
+	 uint16_t red,
+	 uint16_t green,
+	 uint16_t blue,
+	 uint16_t alpha)
+{
+	int shift;
+	struct drm_rgba val;
+
+	if (WARN_ON(bpc > 16))
+		bpc = 16;
+
+	/*
+	 * If we were provided with fewer than 16 bpc, shift the value we
+	 * received into the most significant bits.
+	 */
+	shift = 16 - bpc;
+
+	val.v = red << shift;
+	val.v <<= 16;
+	val.v |= green << shift;
+	val.v <<= 16;
+	val.v |= blue << shift;
+	val.v <<= 16;
+	val.v |= alpha << shift;
+
+	return val;
+}
+
+static inline uint16_t
+drm_rgba_bits(struct drm_rgba c, unsigned compshift, unsigned bits) {
+	uint64_t val;
+
+	if (WARN_ON(bits > 16))
+		bits = 16;
+
+	val = c.v & GENMASK_ULL(compshift + 15, compshift);
+	return val >> (compshift + 16 - bits);
+}
+
+/*
+ * Macros to access the individual color components of an RGBA property value.
+ * If the requested number of bits is less than 16, only the most significant
+ * bits of the color component will be returned.
+ */
+#define DRM_RGBA_REDBITS(c, bits)   drm_rgba_bits(c, 48, bits)
+#define DRM_RGBA_GREENBITS(c, bits) drm_rgba_bits(c, 32, bits)
+#define DRM_RGBA_BLUEBITS(c, bits)  drm_rgba_bits(c, 16, bits)
+#define DRM_RGBA_ALPHABITS(c, bits) drm_rgba_bits(c, 0, bits)
+
+/**
  * struct drm_crtc_state - mutable CRTC state
  * @crtc: backpointer to the CRTC
  * @enable: whether the CRTC should be enabled, gates all other state
@@ -312,6 +396,7 @@ struct drm_plane_helper_funcs;
  * 	update to ensure framebuffer cleanup isn't done too early
  * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
  * @mode: current mode timings
+ * @background_color: background/canvas color of regions not covered by planes
  * @event: optional pointer to a DRM event to signal upon completion of the
  * 	state update
  * @state: backpointer to global drm_atomic_state
@@ -355,6 +440,9 @@ struct drm_crtc_state {
 	/* blob property to expose current mode to atomic userspace */
 	struct drm_property_blob *mode_blob;
 
+	/* CRTC background color */
+	struct drm_rgba background_color;
+
 	struct drm_pending_vblank_event *event;
 
 	struct drm_atomic_state *state;
@@ -2101,6 +2189,7 @@ struct drm_mode_config {
 	struct drm_property *prop_crtc_id;
 	struct drm_property *prop_active;
 	struct drm_property *prop_mode_id;
+	struct drm_property *prop_background_color;
 
 	/* DVI-I properties */
 	struct drm_property *dvi_i_subconnector_property;
@@ -2371,6 +2460,8 @@ struct drm_property *drm_property_create_object(struct drm_device *dev,
 					 int flags, const char *name, uint32_t type);
 struct drm_property *drm_property_create_bool(struct drm_device *dev, int flags,
 					 const char *name);
+struct drm_property *drm_property_create_rgba(struct drm_device *dev,
+					      int flags, const char *name);
 struct drm_property_blob *drm_property_create_blob(struct drm_device *dev,
                                                    size_t length,
                                                    const void *data);
@@ -2503,6 +2594,7 @@ extern int drm_format_plane_height(int height, uint32_t format, int plane);
 extern const char *drm_get_format_name(uint32_t format);
 extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
 							      unsigned int supported_rotations);
+extern struct drm_property *drm_mode_create_background_color_property(struct drm_device *dev);
 extern unsigned int drm_rotation_simplify(unsigned int rotation,
 					  unsigned int supported_rotations);
 
-- 
2.1.4

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

Re: [Intel-gfx] [PATCH v2 1/2] drm: Add infrastructure for CRTC background color property (v2)

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 14766 bytes --]

Hi Matt,

[auto build test WARNING on drm/drm-next]
[also build test WARNING on v4.5-rc3 next-20160210]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Roper/CRTC-background-color-support-for-i915/20160211-103451
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'fmt'
   include/drm/drm_crtc.h:447: warning: No description found for parameter 'mode_blob'
   include/drm/drm_crtc.h:862: warning: No description found for parameter 'name'
   include/drm/drm_crtc.h:1320: warning: No description found for parameter 'tile_blob_ptr'
   include/drm/drm_crtc.h:1359: warning: No description found for parameter 'rotation'
   include/drm/drm_crtc.h:1621: warning: No description found for parameter 'name'
   include/drm/drm_crtc.h:1621: warning: No description found for parameter 'mutex'
   include/drm/drm_crtc.h:1621: warning: No description found for parameter 'helper_private'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tile_idr'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'delayed_event'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'edid_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'dpms_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'path_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tile_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'plane_type_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'rotation_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_src_x'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_src_y'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_src_w'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_src_h'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_crtc_x'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_crtc_y'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_crtc_w'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_crtc_h'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_fb_id'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_crtc_id'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_active'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_mode_id'
>> include/drm/drm_crtc.h:2231: warning: No description found for parameter 'prop_background_color'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'dvi_i_subconnector_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'dvi_i_select_subconnector_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_subconnector_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_select_subconnector_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_mode_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_left_margin_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_right_margin_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_top_margin_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_bottom_margin_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_brightness_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_contrast_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_flicker_reduction_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_overscan_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_saturation_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'tv_hue_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'scaling_mode_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'aspect_ratio_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'dirty_info_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'suggested_x_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'suggested_y_property'
   include/drm/drm_crtc.h:2231: warning: No description found for parameter 'allow_fb_modifiers'
   include/drm/drm_dp_helper.h:749: warning: No description found for parameter 'i2c_nack_count'
   include/drm/drm_dp_helper.h:749: warning: No description found for parameter 'i2c_defer_count'
   drivers/gpu/drm/drm_dp_mst_topology.c:2364: warning: No description found for parameter 'connector'
   include/drm/drm_dp_mst_helper.h:92: warning: No description found for parameter 'cached_edid'
   include/drm/drm_dp_mst_helper.h:92: warning: No description found for parameter 'has_audio'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'max_dpcd_transaction_bytes'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'sink_count'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'total_slots'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'avail_slots'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'total_pbn'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'qlock'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_msg_downq'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_down_in_progress'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'payload_lock'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'proposed_vcpis'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'payloads'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'payload_mask'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'vcpi_mask'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_waitq'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'work'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'tx_work'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'destroy_connector_list'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'destroy_connector_lock'
   include/drm/drm_dp_mst_helper.h:466: warning: No description found for parameter 'destroy_connector_work'
   drivers/gpu/drm/drm_dp_mst_topology.c:2364: warning: No description found for parameter 'connector'
   drivers/gpu/drm/drm_irq.c:176: warning: No description found for parameter 'flags'
   include/drm/drmP.h:168: warning: No description found for parameter 'fmt'
   include/drm/drmP.h:184: warning: No description found for parameter 'fmt'
   include/drm/drmP.h:202: warning: No description found for parameter 'fmt'
   include/drm/drmP.h:247: warning: No description found for parameter 'dev'
   include/drm/drmP.h:247: warning: No description found for parameter 'data'
   include/drm/drmP.h:247: warning: No description found for parameter 'file_priv'
   include/drm/drmP.h:280: warning: No description found for parameter 'ioctl'
   include/drm/drmP.h:280: warning: No description found for parameter '_func'
   include/drm/drmP.h:280: warning: No description found for parameter '_flags'
   include/drm/drmP.h:362: warning: cannot understand function prototype: 'struct drm_lock_data '
   include/drm/drmP.h:415: warning: cannot understand function prototype: 'struct drm_driver '
   include/drm/drmP.h:672: warning: cannot understand function prototype: 'struct drm_info_list '
   include/drm/drmP.h:682: warning: cannot understand function prototype: 'struct drm_info_node '
   include/drm/drmP.h:692: warning: cannot understand function prototype: 'struct drm_minor '
   include/drm/drmP.h:740: warning: cannot understand function prototype: 'struct drm_device '
   drivers/gpu/drm/i915/intel_runtime_pm.c:2173: warning: No description found for parameter 'resume'
   drivers/gpu/drm/i915/intel_runtime_pm.c:2173: warning: No description found for parameter 'resume'
   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2659: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'args'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1029: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1029: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1029: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1245: warning: No description found for parameter 'rps'
   drivers/gpu/drm/i915/i915_gem.c:1461: warning: No description found for parameter 'req'
   drivers/gpu/drm/i915/i915_gem.c:1496: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:1496: warning: No description found for parameter 'readonly'
   drivers/gpu/drm/i915/i915_gem.c:1619: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1619: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1619: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1682: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1682: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1682: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1727: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1727: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1727: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:2015: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:2015: warning: No description found for parameter 'size'

vim +/prop_background_color +2231 include/drm/drm_crtc.h

019d96cb Dave Airlie   2011-09-29  2215  
5bb2bbf5 Dave Airlie   2014-11-10  2216  	/* properties for virtual machine layout */
5bb2bbf5 Dave Airlie   2014-11-10  2217  	struct drm_property *suggested_x_property;
5bb2bbf5 Dave Airlie   2014-11-10  2218  	struct drm_property *suggested_y_property;
5bb2bbf5 Dave Airlie   2014-11-10  2219  
019d96cb Dave Airlie   2011-09-29  2220  	/* dumb ioctl parameters */
019d96cb Dave Airlie   2011-09-29  2221  	uint32_t preferred_depth, prefer_shadow;
62f2104f Keith Packard 2013-07-22  2222  
62f2104f Keith Packard 2013-07-22  2223  	/* whether async page flip is supported or not */
62f2104f Keith Packard 2013-07-22  2224  	bool async_page_flip;
8716ed4e Alex Deucher  2014-02-12  2225  
e3eb3250 Rob Clark     2015-02-05  2226  	/* whether the driver supports fb modifiers */
e3eb3250 Rob Clark     2015-02-05  2227  	bool allow_fb_modifiers;
e3eb3250 Rob Clark     2015-02-05  2228  
8716ed4e Alex Deucher  2014-02-12  2229  	/* cursor size */
8716ed4e Alex Deucher  2014-02-12  2230  	uint32_t cursor_width, cursor_height;
f453ba04 Dave Airlie   2008-11-07 @2231  };
f453ba04 Dave Airlie   2008-11-07  2232  
dd275956 Rob Clark     2014-11-25  2233  /**
dd275956 Rob Clark     2014-11-25  2234   * drm_for_each_plane_mask - iterate over planes specified by bitmask
dd275956 Rob Clark     2014-11-25  2235   * @plane: the loop cursor
dd275956 Rob Clark     2014-11-25  2236   * @dev: the DRM device
dd275956 Rob Clark     2014-11-25  2237   * @plane_mask: bitmask of plane indices
dd275956 Rob Clark     2014-11-25  2238   *
dd275956 Rob Clark     2014-11-25  2239   * Iterate over all planes specified by bitmask.

:::::: The code at line 2231 was first introduced by commit
:::::: f453ba0460742ad027ae0c4c7d61e62817b3e7ef DRM: add mode setting support

:::::: TO: Dave Airlie <airlied@redhat.com>
:::::: CC: Dave Airlie <airlied@linux.ie>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6229 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

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

[Matt Roper]

Gen9 platforms allow CRTC's to be programmed with a background/canvas
color below the programmable planes.  Let's expose this as a property to
allow userspace to program a desired value.

This patch is based on earlier work by Chandra Konduru; unfortunately
the driver has evolved so much since his patches were written (in the
pre-atomic era) that the functionality had to be pretty much completely
rewritten for the new i915 atomic internals.

v2:
 - Set initial background color (black) via proper helper function (Bob)
 - Fix debugfs output
 - General rebasing

Cc: Chandra Konduru <chandra.konduru@intel.com>
Cc: Bob Paauwe <bob.j.paauwe@intel.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 Documentation/DocBook/gpu.tmpl       | 10 +++++++-
 drivers/gpu/drm/i915/i915_debugfs.c  |  8 +++++++
 drivers/gpu/drm/i915/i915_reg.h      |  9 +++++++
 drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index fe6b36a..9e003cd 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
-	<td rowspan="20" valign="top" >i915</td>
+	<td rowspan="21" valign="top" >i915</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
 	<td valign="top" >ENUM</td>
@@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
+	<td rowspan="1" valign="top" >CRTC</td>
+	<td valign="top" >“background_color”</td>
+	<td valign="top" >RGBA</td>
+	<td valign="top" >&nbsp;</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >Background color of regions not covered by a plane</td>
+	</tr>
+	<tr>
 	<td rowspan="17" valign="top" >SDVO-TV</td>
 	<td valign="top" >“mode”</td>
 	<td valign="top" >ENUM</td>
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ec0c2a05e..e7352fc 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused)
 			intel_scaler_info(m, crtc);
 			intel_plane_info(m, crtc);
 		}
+		if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) {
+			struct drm_rgba background = pipe_config->base.background_color;
+
+			seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
+				   DRM_RGBA_REDBITS(background, 10),
+				   DRM_RGBA_GREENBITS(background, 10),
+				   DRM_RGBA_BLUEBITS(background, 10));
+		}
 
 		seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",
 			   yesno(!crtc->cpu_fifo_underrun_disabled),
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 144586e..b0b014d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7649,6 +7649,15 @@ enum skl_disp_power_wells {
 #define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
 #define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
 
+/* Skylake pipe bottom color */
+#define _PIPE_BOTTOM_COLOR_A        0x70034
+#define _PIPE_BOTTOM_COLOR_B        0x71034
+#define _PIPE_BOTTOM_COLOR_C        0x72034
+#define PIPE_BOTTOM_GAMMA_ENABLE   (1 << 31)
+#define PIPE_BOTTOM_CSC_ENABLE     (1 << 30)
+#define PIPE_BOTTOM_COLOR_MASK     0x3FFFFFFF
+#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B)
+
 /* MIPI DSI registers */
 
 #define _MIPI_PORT(port, a, c)	_PORT3(port, a, 0, c)	/* ports A and C only */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 836bbdc..a616ac42 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc_state *pipe_config =
 		to_intel_crtc_state(crtc->base.state);
+	struct drm_rgba background = pipe_config->base.background_color;
+	uint32_t val;
 
 	/* drm_atomic_helper_update_legacy_modeset_state might not be called. */
 	crtc->base.mode = crtc->base.state->mode;
@@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
 		else if (old_crtc_state->pch_pfit.enabled)
 			ironlake_pfit_disable(crtc, true);
 	}
+
+	if (INTEL_INFO(dev)->gen >= 9) {
+		/* BGR 16bpc ==> RGB 10bpc */
+		val = DRM_RGBA_REDBITS(background, 10) << 20
+		    | DRM_RGBA_GREENBITS(background, 10) << 10
+		    | DRM_RGBA_BLUEBITS(background, 10);
+
+		/*
+		 * Set CSC and gamma for bottom color.
+		 *
+		 * FIXME:  We turn these on unconditionally for now to match
+		 * how we've setup the various planes.  Once the color
+		 * management framework lands, it may or may not choose to
+		 * set these bits.
+		 */
+		val |= PIPE_BOTTOM_CSC_ENABLE;
+		val |= PIPE_BOTTOM_GAMMA_ENABLE;
+
+		I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val);
+	}
 }
 
 static void intel_fdi_normal_train(struct drm_crtc *crtc)
@@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 							 pipe_config);
 	}
 
+	if (crtc->state->background_color.v != crtc_state->background_color.v)
+		pipe_config->update_pipe = true;
+
 	return ret;
 }
 
@@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = intel_crtc_destroy,
 	.page_flip = intel_crtc_page_flip,
+	.set_property = drm_atomic_helper_crtc_set_property,
 	.atomic_duplicate_state = intel_crtc_duplicate_state,
 	.atomic_destroy_state = intel_crtc_destroy_state,
 };
@@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
 	scaler_state->scaler_id = -1;
 }
 
+static void intel_create_background_color_property(struct drm_device *dev,
+						   struct intel_crtc *crtc)
+{
+	if (!dev->mode_config.prop_background_color)
+		dev->mode_config.prop_background_color =
+			drm_mode_create_background_color_property(dev);
+	if (!dev->mode_config.prop_background_color)
+		return;
+
+	drm_object_attach_property(&crtc->base.base,
+				   dev->mode_config.prop_background_color,
+				   crtc->base.state->background_color.v);
+}
+
 static void intel_crtc_init(struct drm_device *dev, int pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
 
 	WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
+
+	if (INTEL_INFO(dev)->gen >= 9) {
+		crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0);
+		intel_create_background_color_property(dev, intel_crtc);
+	}
+
 	return;
 
 fail:
-- 
2.1.4

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

Re: [PATCH v2 2/2] drm/i915/gen9: Add support for pipe background color (v2)

[Ville Syrjälä]

On Wed, Feb 10, 2016 at 06:32:59PM -0800, Matt Roper wrote:
> Gen9 platforms allow CRTC's to be programmed with a background/canvas
> color below the programmable planes.  Let's expose this as a property to
> allow userspace to program a desired value.
> 
> This patch is based on earlier work by Chandra Konduru; unfortunately
> the driver has evolved so much since his patches were written (in the
> pre-atomic era) that the functionality had to be pretty much completely
> rewritten for the new i915 atomic internals.
> 
> v2:
>  - Set initial background color (black) via proper helper function (Bob)
>  - Fix debugfs output
>  - General rebasing
> 
> Cc: Chandra Konduru <chandra.konduru@intel.com>
> Cc: Bob Paauwe <bob.j.paauwe@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  Documentation/DocBook/gpu.tmpl       | 10 +++++++-
>  drivers/gpu/drm/i915/i915_debugfs.c  |  8 +++++++
>  drivers/gpu/drm/i915/i915_reg.h      |  9 +++++++
>  drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index fe6b36a..9e003cd 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="20" valign="top" >i915</td>
> +	<td rowspan="21" valign="top" >i915</td>
>  	<td rowspan="2" valign="top" >Generic</td>
>  	<td valign="top" >"Broadcast RGB"</td>
>  	<td valign="top" >ENUM</td>
> @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> +	<td rowspan="1" valign="top" >CRTC</td>
> +	<td valign="top" >“background_color”</td>
> +	<td valign="top" >RGBA</td>
> +	<td valign="top" >&nbsp;</td>
> +	<td valign="top" >CRTC</td>
> +	<td valign="top" >Background color of regions not covered by a plane</td>
> +	</tr>
> +	<tr>
>  	<td rowspan="17" valign="top" >SDVO-TV</td>
>  	<td valign="top" >“mode”</td>
>  	<td valign="top" >ENUM</td>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index ec0c2a05e..e7352fc 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused)
>  			intel_scaler_info(m, crtc);
>  			intel_plane_info(m, crtc);
>  		}
> +		if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) {
> +			struct drm_rgba background = pipe_config->base.background_color;
> +
> +			seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
> +				   DRM_RGBA_REDBITS(background, 10),
> +				   DRM_RGBA_GREENBITS(background, 10),
> +				   DRM_RGBA_BLUEBITS(background, 10));
> +		}
>  
>  		seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",
>  			   yesno(!crtc->cpu_fifo_underrun_disabled),
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 144586e..b0b014d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells {
>  #define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
>  #define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
>  
> +/* Skylake pipe bottom color */
> +#define _PIPE_BOTTOM_COLOR_A        0x70034
> +#define _PIPE_BOTTOM_COLOR_B        0x71034
> +#define _PIPE_BOTTOM_COLOR_C        0x72034
> +#define PIPE_BOTTOM_GAMMA_ENABLE   (1 << 31)
> +#define PIPE_BOTTOM_CSC_ENABLE     (1 << 30)
> +#define PIPE_BOTTOM_COLOR_MASK     0x3FFFFFFF
> +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B)
> +
>  /* MIPI DSI registers */
>  
>  #define _MIPI_PORT(port, a, c)	_PORT3(port, a, 0, c)	/* ports A and C only */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 836bbdc..a616ac42 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc_state *pipe_config =
>  		to_intel_crtc_state(crtc->base.state);
> +	struct drm_rgba background = pipe_config->base.background_color;
> +	uint32_t val;
>  
>  	/* drm_atomic_helper_update_legacy_modeset_state might not be called. */
>  	crtc->base.mode = crtc->base.state->mode;
> @@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
>  		else if (old_crtc_state->pch_pfit.enabled)
>  			ironlake_pfit_disable(crtc, true);
>  	}
> +
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		/* BGR 16bpc ==> RGB 10bpc */
> +		val = DRM_RGBA_REDBITS(background, 10) << 20
> +		    | DRM_RGBA_GREENBITS(background, 10) << 10
> +		    | DRM_RGBA_BLUEBITS(background, 10);

I'm not a fan of the drm_rgba thing having alpha in the low bits. It
goes against the existing precedent set by eg. the colorkey ioctls
where alpha (if it would be used) would be in the high bits. I think I
complained about this before already, or maybe it was also about
some BGR vs. RGB ordering thing?

Looks like drm_rgba isn't actually part of this series, and I can't see
it in the tree either, so I guess it comes in via some other series?

> +
> +		/*
> +		 * Set CSC and gamma for bottom color.
> +		 *
> +		 * FIXME:  We turn these on unconditionally for now to match
> +		 * how we've setup the various planes.  Once the color
> +		 * management framework lands, it may or may not choose to
> +		 * set these bits.
> +		 */
> +		val |= PIPE_BOTTOM_CSC_ENABLE;
> +		val |= PIPE_BOTTOM_GAMMA_ENABLE;
> +
> +		I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val);
> +	}
>  }
>  
>  static void intel_fdi_normal_train(struct drm_crtc *crtc)
> @@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
>  							 pipe_config);
>  	}
>  
> +	if (crtc->state->background_color.v != crtc_state->background_color.v)
> +		pipe_config->update_pipe = true;
> +
>  	return ret;
>  }
>  
> @@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
>  	.set_config = drm_atomic_helper_set_config,
>  	.destroy = intel_crtc_destroy,
>  	.page_flip = intel_crtc_page_flip,
> +	.set_property = drm_atomic_helper_crtc_set_property,
>  	.atomic_duplicate_state = intel_crtc_duplicate_state,
>  	.atomic_destroy_state = intel_crtc_destroy_state,
>  };
> @@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
>  	scaler_state->scaler_id = -1;
>  }
>  
> +static void intel_create_background_color_property(struct drm_device *dev,
> +						   struct intel_crtc *crtc)
> +{
> +	if (!dev->mode_config.prop_background_color)
> +		dev->mode_config.prop_background_color =
> +			drm_mode_create_background_color_property(dev);
> +	if (!dev->mode_config.prop_background_color)
> +		return;
> +
> +	drm_object_attach_property(&crtc->base.base,
> +				   dev->mode_config.prop_background_color,
> +				   crtc->base.state->background_color.v);
> +}
> +
>  static void intel_crtc_init(struct drm_device *dev, int pipe)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
>  
>  	WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
> +
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0);
> +		intel_create_background_color_property(dev, intel_crtc);
> +	}
> +
>  	return;
>  
>  fail:
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] drm/i915/gen9: Add support for pipe background color (v2)

[Matt Roper]

On Thu, Feb 11, 2016 at 12:00:50PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 10, 2016 at 06:32:59PM -0800, Matt Roper wrote:
> > Gen9 platforms allow CRTC's to be programmed with a background/canvas
> > color below the programmable planes.  Let's expose this as a property to
> > allow userspace to program a desired value.
> > 
> > This patch is based on earlier work by Chandra Konduru; unfortunately
> > the driver has evolved so much since his patches were written (in the
> > pre-atomic era) that the functionality had to be pretty much completely
> > rewritten for the new i915 atomic internals.
> > 
> > v2:
> >  - Set initial background color (black) via proper helper function (Bob)
> >  - Fix debugfs output
> >  - General rebasing
> > 
> > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > Cc: Bob Paauwe <bob.j.paauwe@intel.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  Documentation/DocBook/gpu.tmpl       | 10 +++++++-
> >  drivers/gpu/drm/i915/i915_debugfs.c  |  8 +++++++
> >  drivers/gpu/drm/i915/i915_reg.h      |  9 +++++++
> >  drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++
> >  4 files changed, 72 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> > index fe6b36a..9e003cd 100644
> > --- a/Documentation/DocBook/gpu.tmpl
> > +++ b/Documentation/DocBook/gpu.tmpl
> > @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev)
> >  	<td valign="top" >TBD</td>
> >  	</tr>
> >  	<tr>
> > -	<td rowspan="20" valign="top" >i915</td>
> > +	<td rowspan="21" valign="top" >i915</td>
> >  	<td rowspan="2" valign="top" >Generic</td>
> >  	<td valign="top" >"Broadcast RGB"</td>
> >  	<td valign="top" >ENUM</td>
> > @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev)
> >  	<td valign="top" >TBD</td>
> >  	</tr>
> >  	<tr>
> > +	<td rowspan="1" valign="top" >CRTC</td>
> > +	<td valign="top" >“background_color”</td>
> > +	<td valign="top" >RGBA</td>
> > +	<td valign="top" >&nbsp;</td>
> > +	<td valign="top" >CRTC</td>
> > +	<td valign="top" >Background color of regions not covered by a plane</td>
> > +	</tr>
> > +	<tr>
> >  	<td rowspan="17" valign="top" >SDVO-TV</td>
> >  	<td valign="top" >“mode”</td>
> >  	<td valign="top" >ENUM</td>
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index ec0c2a05e..e7352fc 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused)
> >  			intel_scaler_info(m, crtc);
> >  			intel_plane_info(m, crtc);
> >  		}
> > +		if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) {
> > +			struct drm_rgba background = pipe_config->base.background_color;
> > +
> > +			seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
> > +				   DRM_RGBA_REDBITS(background, 10),
> > +				   DRM_RGBA_GREENBITS(background, 10),
> > +				   DRM_RGBA_BLUEBITS(background, 10));
> > +		}
> >  
> >  		seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",
> >  			   yesno(!crtc->cpu_fifo_underrun_disabled),
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 144586e..b0b014d 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells {
> >  #define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
> >  #define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
> >  
> > +/* Skylake pipe bottom color */
> > +#define _PIPE_BOTTOM_COLOR_A        0x70034
> > +#define _PIPE_BOTTOM_COLOR_B        0x71034
> > +#define _PIPE_BOTTOM_COLOR_C        0x72034
> > +#define PIPE_BOTTOM_GAMMA_ENABLE   (1 << 31)
> > +#define PIPE_BOTTOM_CSC_ENABLE     (1 << 30)
> > +#define PIPE_BOTTOM_COLOR_MASK     0x3FFFFFFF
> > +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B)
> > +
> >  /* MIPI DSI registers */
> >  
> >  #define _MIPI_PORT(port, a, c)	_PORT3(port, a, 0, c)	/* ports A and C only */
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 836bbdc..a616ac42 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	struct intel_crtc_state *pipe_config =
> >  		to_intel_crtc_state(crtc->base.state);
> > +	struct drm_rgba background = pipe_config->base.background_color;
> > +	uint32_t val;
> >  
> >  	/* drm_atomic_helper_update_legacy_modeset_state might not be called. */
> >  	crtc->base.mode = crtc->base.state->mode;
> > @@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
> >  		else if (old_crtc_state->pch_pfit.enabled)
> >  			ironlake_pfit_disable(crtc, true);
> >  	}
> > +
> > +	if (INTEL_INFO(dev)->gen >= 9) {
> > +		/* BGR 16bpc ==> RGB 10bpc */
> > +		val = DRM_RGBA_REDBITS(background, 10) << 20
> > +		    | DRM_RGBA_GREENBITS(background, 10) << 10
> > +		    | DRM_RGBA_BLUEBITS(background, 10);
> 
> I'm not a fan of the drm_rgba thing having alpha in the low bits. It
> goes against the existing precedent set by eg. the colorkey ioctls
> where alpha (if it would be used) would be in the high bits. I think I
> complained about this before already, or maybe it was also about
> some BGR vs. RGB ordering thing?
> 
> Looks like drm_rgba isn't actually part of this series, and I can't see
> it in the tree either, so I guess it comes in via some other series?

drm_rgba was in patch #1 of this series; if that didn't come through for
you, it's in patchwork here:
  https://patchwork.freedesktop.org/patch/73224/

We could certainly flip around the internal ordering of bits, but I'm
not sure it really matters.  The whole point of drm_rgba is to be
somewhat opaque so that drivers don't try to work directly on the
internal representation (and potentially misinterpret the format).


Matt

> 
> > +
> > +		/*
> > +		 * Set CSC and gamma for bottom color.
> > +		 *
> > +		 * FIXME:  We turn these on unconditionally for now to match
> > +		 * how we've setup the various planes.  Once the color
> > +		 * management framework lands, it may or may not choose to
> > +		 * set these bits.
> > +		 */
> > +		val |= PIPE_BOTTOM_CSC_ENABLE;
> > +		val |= PIPE_BOTTOM_GAMMA_ENABLE;
> > +
> > +		I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val);
> > +	}
> >  }
> >  
> >  static void intel_fdi_normal_train(struct drm_crtc *crtc)
> > @@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> >  							 pipe_config);
> >  	}
> >  
> > +	if (crtc->state->background_color.v != crtc_state->background_color.v)
> > +		pipe_config->update_pipe = true;
> > +
> >  	return ret;
> >  }
> >  
> > @@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
> >  	.set_config = drm_atomic_helper_set_config,
> >  	.destroy = intel_crtc_destroy,
> >  	.page_flip = intel_crtc_page_flip,
> > +	.set_property = drm_atomic_helper_crtc_set_property,
> >  	.atomic_duplicate_state = intel_crtc_duplicate_state,
> >  	.atomic_destroy_state = intel_crtc_destroy_state,
> >  };
> > @@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
> >  	scaler_state->scaler_id = -1;
> >  }
> >  
> > +static void intel_create_background_color_property(struct drm_device *dev,
> > +						   struct intel_crtc *crtc)
> > +{
> > +	if (!dev->mode_config.prop_background_color)
> > +		dev->mode_config.prop_background_color =
> > +			drm_mode_create_background_color_property(dev);
> > +	if (!dev->mode_config.prop_background_color)
> > +		return;
> > +
> > +	drm_object_attach_property(&crtc->base.base,
> > +				   dev->mode_config.prop_background_color,
> > +				   crtc->base.state->background_color.v);
> > +}
> > +
> >  static void intel_crtc_init(struct drm_device *dev, int pipe)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> >  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
> >  
> >  	WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
> > +
> > +	if (INTEL_INFO(dev)->gen >= 9) {
> > +		crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0);
> > +		intel_create_background_color_property(dev, intel_crtc);
> > +	}
> > +
> >  	return;
> >  
> >  fail:
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915/gen9: Add support for pipe background color (v2)

[Ville Syrjälä]

On Thu, Feb 11, 2016 at 08:05:26AM -0800, Matt Roper wrote:
> On Thu, Feb 11, 2016 at 12:00:50PM +0200, Ville Syrjälä wrote:
> > On Wed, Feb 10, 2016 at 06:32:59PM -0800, Matt Roper wrote:
> > > Gen9 platforms allow CRTC's to be programmed with a background/canvas
> > > color below the programmable planes.  Let's expose this as a property to
> > > allow userspace to program a desired value.
> > > 
> > > This patch is based on earlier work by Chandra Konduru; unfortunately
> > > the driver has evolved so much since his patches were written (in the
> > > pre-atomic era) that the functionality had to be pretty much completely
> > > rewritten for the new i915 atomic internals.
> > > 
> > > v2:
> > >  - Set initial background color (black) via proper helper function (Bob)
> > >  - Fix debugfs output
> > >  - General rebasing
> > > 
> > > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > > Cc: Bob Paauwe <bob.j.paauwe@intel.com>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > ---
> > >  Documentation/DocBook/gpu.tmpl       | 10 +++++++-
> > >  drivers/gpu/drm/i915/i915_debugfs.c  |  8 +++++++
> > >  drivers/gpu/drm/i915/i915_reg.h      |  9 +++++++
> > >  drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 72 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> > > index fe6b36a..9e003cd 100644
> > > --- a/Documentation/DocBook/gpu.tmpl
> > > +++ b/Documentation/DocBook/gpu.tmpl
> > > @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev)
> > >  	<td valign="top" >TBD</td>
> > >  	</tr>
> > >  	<tr>
> > > -	<td rowspan="20" valign="top" >i915</td>
> > > +	<td rowspan="21" valign="top" >i915</td>
> > >  	<td rowspan="2" valign="top" >Generic</td>
> > >  	<td valign="top" >"Broadcast RGB"</td>
> > >  	<td valign="top" >ENUM</td>
> > > @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev)
> > >  	<td valign="top" >TBD</td>
> > >  	</tr>
> > >  	<tr>
> > > +	<td rowspan="1" valign="top" >CRTC</td>
> > > +	<td valign="top" >“background_color”</td>
> > > +	<td valign="top" >RGBA</td>
> > > +	<td valign="top" >&nbsp;</td>
> > > +	<td valign="top" >CRTC</td>
> > > +	<td valign="top" >Background color of regions not covered by a plane</td>
> > > +	</tr>
> > > +	<tr>
> > >  	<td rowspan="17" valign="top" >SDVO-TV</td>
> > >  	<td valign="top" >“mode”</td>
> > >  	<td valign="top" >ENUM</td>
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index ec0c2a05e..e7352fc 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused)
> > >  			intel_scaler_info(m, crtc);
> > >  			intel_plane_info(m, crtc);
> > >  		}
> > > +		if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) {
> > > +			struct drm_rgba background = pipe_config->base.background_color;
> > > +
> > > +			seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
> > > +				   DRM_RGBA_REDBITS(background, 10),
> > > +				   DRM_RGBA_GREENBITS(background, 10),
> > > +				   DRM_RGBA_BLUEBITS(background, 10));
> > > +		}
> > >  
> > >  		seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",
> > >  			   yesno(!crtc->cpu_fifo_underrun_disabled),
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 144586e..b0b014d 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells {
> > >  #define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
> > >  #define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
> > >  
> > > +/* Skylake pipe bottom color */
> > > +#define _PIPE_BOTTOM_COLOR_A        0x70034
> > > +#define _PIPE_BOTTOM_COLOR_B        0x71034
> > > +#define _PIPE_BOTTOM_COLOR_C        0x72034
> > > +#define PIPE_BOTTOM_GAMMA_ENABLE   (1 << 31)
> > > +#define PIPE_BOTTOM_CSC_ENABLE     (1 << 30)
> > > +#define PIPE_BOTTOM_COLOR_MASK     0x3FFFFFFF
> > > +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B)
> > > +
> > >  /* MIPI DSI registers */
> > >  
> > >  #define _MIPI_PORT(port, a, c)	_PORT3(port, a, 0, c)	/* ports A and C only */
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 836bbdc..a616ac42 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
> > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > >  	struct intel_crtc_state *pipe_config =
> > >  		to_intel_crtc_state(crtc->base.state);
> > > +	struct drm_rgba background = pipe_config->base.background_color;
> > > +	uint32_t val;
> > >  
> > >  	/* drm_atomic_helper_update_legacy_modeset_state might not be called. */
> > >  	crtc->base.mode = crtc->base.state->mode;
> > > @@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
> > >  		else if (old_crtc_state->pch_pfit.enabled)
> > >  			ironlake_pfit_disable(crtc, true);
> > >  	}
> > > +
> > > +	if (INTEL_INFO(dev)->gen >= 9) {
> > > +		/* BGR 16bpc ==> RGB 10bpc */
> > > +		val = DRM_RGBA_REDBITS(background, 10) << 20
> > > +		    | DRM_RGBA_GREENBITS(background, 10) << 10
> > > +		    | DRM_RGBA_BLUEBITS(background, 10);
> > 
> > I'm not a fan of the drm_rgba thing having alpha in the low bits. It
> > goes against the existing precedent set by eg. the colorkey ioctls
> > where alpha (if it would be used) would be in the high bits. I think I
> > complained about this before already, or maybe it was also about
> > some BGR vs. RGB ordering thing?
> > 
> > Looks like drm_rgba isn't actually part of this series, and I can't see
> > it in the tree either, so I guess it comes in via some other series?
> 
> drm_rgba was in patch #1 of this series; if that didn't come through for
> you, it's in patchwork here:
>   https://patchwork.freedesktop.org/patch/73224/

It did come through, but somehow I didn't see it.

> 
> We could certainly flip around the internal ordering of bits, but I'm
> not sure it really matters.  The whole point of drm_rgba is to be
> somewhat opaque so that drivers don't try to work directly on the
> internal representation (and potentially misinterpret the format).

Sure. Just goes against existing practice a bit. And feels a bit
strange to have the component that's most often the one that's not
present occupy the low bits.

OTOH the fact that RGBA is a fairly obscure format hardware wise,
might catch people trying using the API the wrong way.

> 
> 
> Matt
> 
> > 
> > > +
> > > +		/*
> > > +		 * Set CSC and gamma for bottom color.
> > > +		 *
> > > +		 * FIXME:  We turn these on unconditionally for now to match
> > > +		 * how we've setup the various planes.  Once the color
> > > +		 * management framework lands, it may or may not choose to
> > > +		 * set these bits.
> > > +		 */
> > > +		val |= PIPE_BOTTOM_CSC_ENABLE;
> > > +		val |= PIPE_BOTTOM_GAMMA_ENABLE;
> > > +
> > > +		I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val);
> > > +	}
> > >  }
> > >  
> > >  static void intel_fdi_normal_train(struct drm_crtc *crtc)
> > > @@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> > >  							 pipe_config);
> > >  	}
> > >  
> > > +	if (crtc->state->background_color.v != crtc_state->background_color.v)
> > > +		pipe_config->update_pipe = true;
> > > +
> > >  	return ret;
> > >  }
> > >  
> > > @@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
> > >  	.set_config = drm_atomic_helper_set_config,
> > >  	.destroy = intel_crtc_destroy,
> > >  	.page_flip = intel_crtc_page_flip,
> > > +	.set_property = drm_atomic_helper_crtc_set_property,
> > >  	.atomic_duplicate_state = intel_crtc_duplicate_state,
> > >  	.atomic_destroy_state = intel_crtc_destroy_state,
> > >  };
> > > @@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
> > >  	scaler_state->scaler_id = -1;
> > >  }
> > >  
> > > +static void intel_create_background_color_property(struct drm_device *dev,
> > > +						   struct intel_crtc *crtc)
> > > +{
> > > +	if (!dev->mode_config.prop_background_color)
> > > +		dev->mode_config.prop_background_color =
> > > +			drm_mode_create_background_color_property(dev);
> > > +	if (!dev->mode_config.prop_background_color)
> > > +		return;
> > > +
> > > +	drm_object_attach_property(&crtc->base.base,
> > > +				   dev->mode_config.prop_background_color,
> > > +				   crtc->base.state->background_color.v);
> > > +}
> > > +
> > >  static void intel_crtc_init(struct drm_device *dev, int pipe)
> > >  {
> > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > > @@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> > >  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
> > >  
> > >  	WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
> > > +
> > > +	if (INTEL_INFO(dev)->gen >= 9) {
> > > +		crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0);
> > > +		intel_create_background_color_property(dev, intel_crtc);
> > > +	}
> > > +
> > >  	return;
> > >  
> > >  fail:
> > > -- 
> > > 2.1.4
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

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

Re: [PATCH v2 2/2] drm/i915/gen9: Add support for pipe background color (v2)

[Lionel Landwerlin]

Hi Matt,

On 11/02/16 02:32, Matt Roper wrote:
> Gen9 platforms allow CRTC's to be programmed with a background/canvas
> color below the programmable planes.  Let's expose this as a property to
> allow userspace to program a desired value.
>
> This patch is based on earlier work by Chandra Konduru; unfortunately
> the driver has evolved so much since his patches were written (in the
> pre-atomic era) that the functionality had to be pretty much completely
> rewritten for the new i915 atomic internals.
>
> v2:
>   - Set initial background color (black) via proper helper function (Bob)
>   - Fix debugfs output
>   - General rebasing
>
> Cc: Chandra Konduru <chandra.konduru@intel.com>
> Cc: Bob Paauwe <bob.j.paauwe@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>   Documentation/DocBook/gpu.tmpl       | 10 +++++++-
>   drivers/gpu/drm/i915/i915_debugfs.c  |  8 +++++++
>   drivers/gpu/drm/i915/i915_reg.h      |  9 +++++++
>   drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++
>   4 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index fe6b36a..9e003cd 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev)
>   	<td valign="top" >TBD</td>
>   	</tr>
>   	<tr>
> -	<td rowspan="20" valign="top" >i915</td>
> +	<td rowspan="21" valign="top" >i915</td>
>   	<td rowspan="2" valign="top" >Generic</td>
>   	<td valign="top" >"Broadcast RGB"</td>
>   	<td valign="top" >ENUM</td>
> @@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev)
>   	<td valign="top" >TBD</td>
>   	</tr>
>   	<tr>
> +	<td rowspan="1" valign="top" >CRTC</td>
> +	<td valign="top" >“background_color”</td>
> +	<td valign="top" >RGBA</td>
> +	<td valign="top" >&nbsp;</td>
> +	<td valign="top" >CRTC</td>
> +	<td valign="top" >Background color of regions not covered by a plane</td>
> +	</tr>
> +	<tr>
>   	<td rowspan="17" valign="top" >SDVO-TV</td>
>   	<td valign="top" >“mode”</td>
>   	<td valign="top" >ENUM</td>

Why make this property i915 specific?
Have you considered make this a generic optional property?

Just asking because your first patch seems to imply this could be generic.

> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index ec0c2a05e..e7352fc 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused)
>   			intel_scaler_info(m, crtc);
>   			intel_plane_info(m, crtc);
>   		}
> +		if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) {
> +			struct drm_rgba background = pipe_config->base.background_color;
> +
> +			seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
> +				   DRM_RGBA_REDBITS(background, 10),
> +				   DRM_RGBA_GREENBITS(background, 10),
> +				   DRM_RGBA_BLUEBITS(background, 10));
> +		}
>   
>   		seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",
>   			   yesno(!crtc->cpu_fifo_underrun_disabled),
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 144586e..b0b014d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7649,6 +7649,15 @@ enum skl_disp_power_wells {
>   #define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
>   #define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
>   
> +/* Skylake pipe bottom color */
> +#define _PIPE_BOTTOM_COLOR_A        0x70034
> +#define _PIPE_BOTTOM_COLOR_B        0x71034
> +#define _PIPE_BOTTOM_COLOR_C        0x72034
> +#define PIPE_BOTTOM_GAMMA_ENABLE   (1 << 31)
> +#define PIPE_BOTTOM_CSC_ENABLE     (1 << 30)
> +#define PIPE_BOTTOM_COLOR_MASK     0x3FFFFFFF
> +#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B)
> +
>   /* MIPI DSI registers */
>   
>   #define _MIPI_PORT(port, a, c)	_PORT3(port, a, 0, c)	/* ports A and C only */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 836bbdc..a616ac42 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_crtc_state *pipe_config =
>   		to_intel_crtc_state(crtc->base.state);
> +	struct drm_rgba background = pipe_config->base.background_color;
> +	uint32_t val;
>   
>   	/* drm_atomic_helper_update_legacy_modeset_state might not be called. */
>   	crtc->base.mode = crtc->base.state->mode;
> @@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
>   		else if (old_crtc_state->pch_pfit.enabled)
>   			ironlake_pfit_disable(crtc, true);
>   	}
> +
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		/* BGR 16bpc ==> RGB 10bpc */
> +		val = DRM_RGBA_REDBITS(background, 10) << 20
> +		    | DRM_RGBA_GREENBITS(background, 10) << 10
> +		    | DRM_RGBA_BLUEBITS(background, 10);
> +
> +		/*
> +		 * Set CSC and gamma for bottom color.
> +		 *
> +		 * FIXME:  We turn these on unconditionally for now to match
> +		 * how we've setup the various planes.  Once the color
> +		 * management framework lands, it may or may not choose to
> +		 * set these bits.
> +		 */
> +		val |= PIPE_BOTTOM_CSC_ENABLE;
> +		val |= PIPE_BOTTOM_GAMMA_ENABLE;
> +
> +		I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val);
> +	}
>   }
>   
>   static void intel_fdi_normal_train(struct drm_crtc *crtc)
> @@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
>   							 pipe_config);
>   	}
>   
> +	if (crtc->state->background_color.v != crtc_state->background_color.v)
> +		pipe_config->update_pipe = true;
> +
>   	return ret;
>   }
>   
> @@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
>   	.set_config = drm_atomic_helper_set_config,
>   	.destroy = intel_crtc_destroy,
>   	.page_flip = intel_crtc_page_flip,
> +	.set_property = drm_atomic_helper_crtc_set_property,
>   	.atomic_duplicate_state = intel_crtc_duplicate_state,
>   	.atomic_destroy_state = intel_crtc_destroy_state,
>   };
> @@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
>   	scaler_state->scaler_id = -1;
>   }
>   
> +static void intel_create_background_color_property(struct drm_device *dev,
> +						   struct intel_crtc *crtc)
> +{
> +	if (!dev->mode_config.prop_background_color)
> +		dev->mode_config.prop_background_color =
> +			drm_mode_create_background_color_property(dev);
> +	if (!dev->mode_config.prop_background_color)
> +		return;
> +
> +	drm_object_attach_property(&crtc->base.base,
> +				   dev->mode_config.prop_background_color,
> +				   crtc->base.state->background_color.v);
> +}
> +
>   static void intel_crtc_init(struct drm_device *dev, int pipe)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>   	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
>   
>   	WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
> +
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0);
> +		intel_create_background_color_property(dev, intel_crtc);
> +	}
> +
>   	return;
>   
>   fail:

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

Re: [PATCH v2 2/2] drm/i915/gen9: Add support for pipe background color (v2)

[Matt Roper]

On Thu, Mar 03, 2016 at 03:50:23PM +0000, Lionel Landwerlin wrote:
> Hi Matt,
> 
> On 11/02/16 02:32, Matt Roper wrote:
> >Gen9 platforms allow CRTC's to be programmed with a background/canvas
> >color below the programmable planes.  Let's expose this as a property to
> >allow userspace to program a desired value.
> >
> >This patch is based on earlier work by Chandra Konduru; unfortunately
> >the driver has evolved so much since his patches were written (in the
> >pre-atomic era) that the functionality had to be pretty much completely
> >rewritten for the new i915 atomic internals.
> >
> >v2:
> >  - Set initial background color (black) via proper helper function (Bob)
> >  - Fix debugfs output
> >  - General rebasing
> >
> >Cc: Chandra Konduru <chandra.konduru@intel.com>
> >Cc: Bob Paauwe <bob.j.paauwe@intel.com>
> >Cc: dri-devel@lists.freedesktop.org
> >Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> >---
> >  Documentation/DocBook/gpu.tmpl       | 10 +++++++-
> >  drivers/gpu/drm/i915/i915_debugfs.c  |  8 +++++++
> >  drivers/gpu/drm/i915/i915_reg.h      |  9 +++++++
> >  drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++++++
> >  4 files changed, 72 insertions(+), 1 deletion(-)
> >
> >diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> >index fe6b36a..9e003cd 100644
> >--- a/Documentation/DocBook/gpu.tmpl
> >+++ b/Documentation/DocBook/gpu.tmpl
> >@@ -2092,7 +2092,7 @@ void intel_crt_init(struct drm_device *dev)
> >  	<td valign="top" >TBD</td>
> >  	</tr>
> >  	<tr>
> >-	<td rowspan="20" valign="top" >i915</td>
> >+	<td rowspan="21" valign="top" >i915</td>
> >  	<td rowspan="2" valign="top" >Generic</td>
> >  	<td valign="top" >"Broadcast RGB"</td>
> >  	<td valign="top" >ENUM</td>
> >@@ -2108,6 +2108,14 @@ void intel_crt_init(struct drm_device *dev)
> >  	<td valign="top" >TBD</td>
> >  	</tr>
> >  	<tr>
> >+	<td rowspan="1" valign="top" >CRTC</td>
> >+	<td valign="top" >“background_color”</td>
> >+	<td valign="top" >RGBA</td>
> >+	<td valign="top" >&nbsp;</td>
> >+	<td valign="top" >CRTC</td>
> >+	<td valign="top" >Background color of regions not covered by a plane</td>
> >+	</tr>
> >+	<tr>
> >  	<td rowspan="17" valign="top" >SDVO-TV</td>
> >  	<td valign="top" >“mode”</td>
> >  	<td valign="top" >ENUM</td>
> 
> Why make this property i915 specific?
> Have you considered make this a generic optional property?
> 
> Just asking because your first patch seems to imply this could be generic.

Yeah, the intention is for it to be generic.  I guess I should have
stuck this under the 'DRM/Optional' section of DocBook rather than the
i915-specific section.  Thanks for pointing that out.


Matt

> 
> >diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >index ec0c2a05e..e7352fc 100644
> >--- a/drivers/gpu/drm/i915/i915_debugfs.c
> >+++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >@@ -3104,6 +3104,14 @@ static int i915_display_info(struct seq_file *m, void *unused)
> >  			intel_scaler_info(m, crtc);
> >  			intel_plane_info(m, crtc);
> >  		}
> >+		if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) {
> >+			struct drm_rgba background = pipe_config->base.background_color;
> >+
> >+			seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
> >+				   DRM_RGBA_REDBITS(background, 10),
> >+				   DRM_RGBA_GREENBITS(background, 10),
> >+				   DRM_RGBA_BLUEBITS(background, 10));
> >+		}
> >  		seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",
> >  			   yesno(!crtc->cpu_fifo_underrun_disabled),
> >diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >index 144586e..b0b014d 100644
> >--- a/drivers/gpu/drm/i915/i915_reg.h
> >+++ b/drivers/gpu/drm/i915/i915_reg.h
> >@@ -7649,6 +7649,15 @@ enum skl_disp_power_wells {
> >  #define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
> >  #define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
> >+/* Skylake pipe bottom color */
> >+#define _PIPE_BOTTOM_COLOR_A        0x70034
> >+#define _PIPE_BOTTOM_COLOR_B        0x71034
> >+#define _PIPE_BOTTOM_COLOR_C        0x72034
> >+#define PIPE_BOTTOM_GAMMA_ENABLE   (1 << 31)
> >+#define PIPE_BOTTOM_CSC_ENABLE     (1 << 30)
> >+#define PIPE_BOTTOM_COLOR_MASK     0x3FFFFFFF
> >+#define PIPE_BOTTOM_COLOR(pipe) _MMIO_PIPE(pipe, _PIPE_BOTTOM_COLOR_A, _PIPE_BOTTOM_COLOR_B)
> >+
> >  /* MIPI DSI registers */
> >  #define _MIPI_PORT(port, a, c)	_PORT3(port, a, 0, c)	/* ports A and C only */
> >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >index 836bbdc..a616ac42 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -3299,6 +3299,8 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	struct intel_crtc_state *pipe_config =
> >  		to_intel_crtc_state(crtc->base.state);
> >+	struct drm_rgba background = pipe_config->base.background_color;
> >+	uint32_t val;
> >  	/* drm_atomic_helper_update_legacy_modeset_state might not be called. */
> >  	crtc->base.mode = crtc->base.state->mode;
> >@@ -3335,6 +3337,26 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
> >  		else if (old_crtc_state->pch_pfit.enabled)
> >  			ironlake_pfit_disable(crtc, true);
> >  	}
> >+
> >+	if (INTEL_INFO(dev)->gen >= 9) {
> >+		/* BGR 16bpc ==> RGB 10bpc */
> >+		val = DRM_RGBA_REDBITS(background, 10) << 20
> >+		    | DRM_RGBA_GREENBITS(background, 10) << 10
> >+		    | DRM_RGBA_BLUEBITS(background, 10);
> >+
> >+		/*
> >+		 * Set CSC and gamma for bottom color.
> >+		 *
> >+		 * FIXME:  We turn these on unconditionally for now to match
> >+		 * how we've setup the various planes.  Once the color
> >+		 * management framework lands, it may or may not choose to
> >+		 * set these bits.
> >+		 */
> >+		val |= PIPE_BOTTOM_CSC_ENABLE;
> >+		val |= PIPE_BOTTOM_GAMMA_ENABLE;
> >+
> >+		I915_WRITE(PIPE_BOTTOM_COLOR(crtc->pipe), val);
> >+	}
> >  }
> >  static void intel_fdi_normal_train(struct drm_crtc *crtc)
> >@@ -12032,6 +12054,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> >  							 pipe_config);
> >  	}
> >+	if (crtc->state->background_color.v != crtc_state->background_color.v)
> >+		pipe_config->update_pipe = true;
> >+
> >  	return ret;
> >  }
> >@@ -13660,6 +13685,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
> >  	.set_config = drm_atomic_helper_set_config,
> >  	.destroy = intel_crtc_destroy,
> >  	.page_flip = intel_crtc_page_flip,
> >+	.set_property = drm_atomic_helper_crtc_set_property,
> >  	.atomic_duplicate_state = intel_crtc_duplicate_state,
> >  	.atomic_destroy_state = intel_crtc_destroy_state,
> >  };
> >@@ -14254,6 +14280,20 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
> >  	scaler_state->scaler_id = -1;
> >  }
> >+static void intel_create_background_color_property(struct drm_device *dev,
> >+						   struct intel_crtc *crtc)
> >+{
> >+	if (!dev->mode_config.prop_background_color)
> >+		dev->mode_config.prop_background_color =
> >+			drm_mode_create_background_color_property(dev);
> >+	if (!dev->mode_config.prop_background_color)
> >+		return;
> >+
> >+	drm_object_attach_property(&crtc->base.base,
> >+				   dev->mode_config.prop_background_color,
> >+				   crtc->base.state->background_color.v);
> >+}
> >+
> >  static void intel_crtc_init(struct drm_device *dev, int pipe)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >@@ -14329,6 +14369,12 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> >  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
> >  	WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
> >+
> >+	if (INTEL_INFO(dev)->gen >= 9) {
> >+		crtc_state->base.background_color = drm_rgba(16, 0, 0, 0, 0);
> >+		intel_create_background_color_property(dev, intel_crtc);
> >+	}
> >+
> >  	return;
> >  fail:
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.BAT: failure for CRTC background color support for i915

[Patchwork]

== Summary ==

Series 3265v1 CRTC background color support for i915
http://patchwork.freedesktop.org/api/1.0/series/3265/revisions/1/mbox/

Test gem_sync:
        Subgroup basic-default:
                pass       -> DMESG-FAIL (hsw-brixbox)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (bsw-nuc-2)
                pass       -> INCOMPLETE (hsw-gt2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                fail       -> PASS       (bdw-nuci7)
        Subgroup basic-rte:
                dmesg-warn -> PASS       (bsw-nuc-2)
                pass       -> DMESG-WARN (byt-nuc) UNSTABLE

bdw-nuci7        total:162  pass:152  dwarn:0   dfail:0   fail:0   skip:10 
bdw-ultra        total:165  pass:152  dwarn:0   dfail:0   fail:0   skip:13 
bsw-nuc-2        total:165  pass:136  dwarn:0   dfail:0   fail:0   skip:29 
byt-nuc          total:165  pass:140  dwarn:1   dfail:0   fail:0   skip:24 
hsw-brixbox      total:165  pass:150  dwarn:0   dfail:1   fail:0   skip:14 
hsw-gt2          total:86   pass:80   dwarn:0   dfail:0   fail:1   skip:4  
ilk-hp8440p      total:165  pass:116  dwarn:0   dfail:0   fail:1   skip:48 
ivb-t430s        total:165  pass:150  dwarn:0   dfail:0   fail:1   skip:14 
snb-dellxps      total:165  pass:142  dwarn:0   dfail:0   fail:1   skip:22 
snb-x220t        total:165  pass:142  dwarn:0   dfail:0   fail:2   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1392/

f2110d8eac120416f8f5669f2aa561d9ab330a77 drm-intel-nightly: 2016y-02m-15d-09h-53m-04s UTC integration manifest
02ba7fa3e68b07163a900e9fd2609874a6ac425b drm/i915/gen9: Add support for pipe background color (v2)
1db340510effedb03d3174211f288ba0f77a78c5 drm: Add infrastructure for CRTC background color property (v2)

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