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

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

[Matt Roper]

Recent Intel platforms (SKL+) support a background canvas color below the
hardware planes.  This background color can be seen on areas not covered by a
plane, or if we program the hardware with various plane blending modes.

Chandra Konduru previously took a stab at writing i915 patches to support this
feature, but we were in the middle of the great i915 atomic transition at the
time, so there was too much driver churn to ever get them in.  Now that i915
atomic work has settled down a bit, let's make another attempt to get this
feature support in. 

One of the items that came up in review of Chandra's earlier patches was that
although userspace needs to pass a color value to the kernel, it would be best
if userspace didn't have to care about the formatting of color values used by
the underyling hardware (i.e., number of bits, ordering of R,G,B components,
etc.).  The first patch here aims to address that by setting up an 'RGBA'
property type, specifying a fixed bit layout (with 16bpc), and providing helper
macros to fetch specific color bits with the desired precision.  This should
prevent drivers from trying to shove property values directly into the
hardware, which could lead to incompatibility between platforms if we aren't
careful.

A libdrm patch that provides helper macros to build color values of the
appropriate format will be provided as a followup, as will an update to
the i-g-t test for this feature (which is surprisingly already merged, even
though the kernel support isn't there yet).

Cc: dri-devel@lists.freedesktop.org
Cc: Chandra Konduru <chandra.konduru@intel.com>

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

 Documentation/DocBook/gpu.tmpl       | 10 +++++++-
 drivers/gpu/drm/drm_atomic.c         |  4 +++
 drivers/gpu/drm/drm_crtc.c           | 33 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_debugfs.c  |  9 +++++++
 drivers/gpu/drm/i915/i915_reg.h      | 10 ++++++++
 drivers/gpu/drm/i915/intel_display.c | 43 +++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h               | 49 ++++++++++++++++++++++++++++++++++++
 7 files changed, 157 insertions(+), 1 deletion(-)

-- 
2.1.4

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

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

[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 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.

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.

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

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7bb3845..688ca75 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -415,6 +415,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);
@@ -450,6 +452,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 e54660a..1e0dd09 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3807,6 +3807,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
@@ -5778,6 +5802,15 @@ struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_mode_create_rotation_property);
 
+struct drm_property
+*drm_mode_create_background_color_property(struct drm_device *dev)
+{
+	return drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+					 "background_color",
+					 0, GENMASK_ULL(63, 0));
+}
+EXPORT_SYMBOL(drm_mode_create_background_color_property);
+
 /**
  * DOC: Tile group
  *
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3f0c690..64f3e62 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -251,6 +251,45 @@ struct drm_bridge;
 struct drm_atomic_state;
 
 /**
+ * drm_rgba_t - RGBA property value type
+ *
+ * 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.
+ */
+typedef struct {
+	uint64_t v;
+} drm_rgba_t;
+
+static inline uint16_t
+drm_rgba_bits(drm_rgba_t 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
@@ -264,6 +303,7 @@ struct drm_atomic_state;
  * 	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 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
@@ -304,6 +344,9 @@ struct drm_crtc_state {
 	/* blob property to expose current mode to atomic userspace */
 	struct drm_property_blob *mode_blob;
 
+	/* CRTC background color */
+	drm_rgba_t background_color;
+
 	struct drm_pending_vblank_event *event;
 
 	struct drm_atomic_state *state;
@@ -1115,6 +1158,9 @@ struct drm_mode_config {
 	struct drm_property *prop_active;
 	struct drm_property *prop_mode_id;
 
+	/* crtc properties */
+	struct drm_property *prop_background_color;
+
 	/* DVI-I properties */
 	struct drm_property *dvi_i_subconnector_property;
 	struct drm_property *dvi_i_select_subconnector_property;
@@ -1365,6 +1411,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);
@@ -1495,6 +1543,7 @@ extern int drm_format_vert_chroma_subsampling(uint32_t format);
 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

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

[PATCH 2/2] drm/i915/skl: Add support for pipe background color

[Matt Roper]

SKL and BXT 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
(pre-atomic) that the functionality had to be pretty much completely
rewritten for the new i915 atomic internals.

Cc: Chandra Konduru <chandra.konduru@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  |  9 ++++++++
 drivers/gpu/drm/i915/i915_reg.h      | 10 +++++++++
 drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index c05d7df..40c0d49 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2819,7 +2819,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>
@@ -2835,6 +2835,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 a3b22bd..acb05e5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2977,6 +2977,15 @@ static int i915_display_info(struct seq_file *m, void *unused)
 				   crtc->base.cursor->state->crtc_h,
 				   crtc->cursor_addr, yesno(active));
 		}
+		if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) {
+			drm_rgba_t background = pipe_config->base.background_color;
+
+			seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
+				   background.v,
+				   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 9ebf032..95ea394 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7562,6 +7562,16 @@ enum skl_disp_power_wells {
 #define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
 #define PIPE_CSC_POSTOFF_LO(pipe) _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) _PIPE3(pipe, _PIPE_BOTTOM_COLOR_A, \
+	_PIPE_BOTTOM_COLOR_B, _PIPE_BOTTOM_COLOR_C)
+
 /* 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 1fc1d24..b00fca9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3332,6 +3332,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);
+	drm_rgba_t 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;
@@ -3368,6 +3370,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)
@@ -11788,6 +11810,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;
 }
 
@@ -13230,6 +13255,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,
 };
@@ -13780,6 +13806,19 @@ 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, 0);
+}
+
 static void intel_crtc_init(struct drm_device *dev, int pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -13855,6 +13894,10 @@ 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)
+		intel_create_background_color_property(dev, intel_crtc);
+
 	return;
 
 fail:
-- 
2.1.4

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

[PATCH libdrm] xf86drmMode: Add RGBA property helpers

[Matt Roper]

Now that we've added an RGBA property type to the kernel that handles
RGBA values with a specific bit layout, let's add a userspace helper to
allow userspace to easily build values in the proper format.

Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 xf86drmMode.c | 36 ++++++++++++++++++++++++++++++++++++
 xf86drmMode.h |  7 +++++++
 2 files changed, 43 insertions(+)

diff --git a/xf86drmMode.c b/xf86drmMode.c
index ab6b519..4bfd419 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -1445,3 +1445,39 @@ drmModeDestroyPropertyBlob(int fd, uint32_t id)
 	destroy.blob_id = id;
 	return DRM_IOCTL(fd, DRM_IOCTL_MODE_DESTROYPROPBLOB, &destroy);
 }
+
+/*
+ * Builds an RGBA 16bpc color value with bits laid out in the format expected
+ * by DRM RGBA properties.  @bpc is the number of bits per component value
+ * being provided as parameters.
+ */
+uint64_t
+drmModeRGBA(unsigned bpc,
+	    uint16_t red,
+	    uint16_t green,
+	    uint16_t blue,
+	    uint16_t alpha)
+{
+	int shift;
+	uint64_t val;
+
+	if (bpc > 16)
+		return -ERANGE;
+
+	/*
+	 * If we were provided with fewer than 16 bpc, shift the value we
+	 * received into the most significant bits.
+	 */
+	shift = 16 - bpc;
+
+	val = red << shift;
+	val <<= 16;
+	val |= green << shift;
+	val <<= 16;
+	val |= blue << shift;
+	val <<= 16;
+	val |= alpha << shift;
+
+	return val;
+}
+
diff --git a/xf86drmMode.h b/xf86drmMode.h
index 4de7bbb..4aa4917 100644
--- a/xf86drmMode.h
+++ b/xf86drmMode.h
@@ -507,6 +507,13 @@ extern int drmModeCreatePropertyBlob(int fd, const void *data, size_t size,
 				     uint32_t *id);
 extern int drmModeDestroyPropertyBlob(int fd, uint32_t id);
 
+extern uint64_t drmModeRGBA(unsigned bpc,
+			    uint16_t red,
+			    uint16_t green,
+			    uint16_t blue,
+			    uint16_t alpha);
+#define DRM_RGBA8888(r, g, b, a)     drmModeRGBA(8, r, g, b, a)
+#define DRM_RGBA16161616(r, g, b, a) drmModeRGBA(16, r, g, b, a)
 
 #if defined(__cplusplus)
 }
-- 
2.1.4

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Add support for pipe background color

[kbuild test robot]

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

Hi Matt,

[auto build test WARNING on drm/drm-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Matt-Roper/CRTC-background-color-support-for-i915/20151023-082852
config: x86_64-randconfig-x005-10211812 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/i915_debugfs.c: In function 'i915_display_info':
>> drivers/gpu/drm/i915/i915_debugfs.c:2983:18: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'uint64_t {aka long long unsigned int}' [-Wformat=]
       seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
                     ^
>> drivers/gpu/drm/i915/i915_debugfs.c:2983:18: warning: too many arguments for format [-Wformat-extra-args]

vim +2983 drivers/gpu/drm/i915/i915_debugfs.c

  2967				   crtc->base.base.id, pipe_name(crtc->pipe),
  2968				   yesno(pipe_config->base.active),
  2969				   pipe_config->pipe_src_w, pipe_config->pipe_src_h);
  2970			if (pipe_config->base.active) {
  2971				intel_crtc_info(m, crtc);
  2972	
  2973				active = cursor_position(dev, crtc->pipe, &x, &y);
  2974				seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x, active? %s\n",
  2975					   yesno(crtc->cursor_base),
  2976					   x, y, crtc->base.cursor->state->crtc_w,
  2977					   crtc->base.cursor->state->crtc_h,
  2978					   crtc->cursor_addr, yesno(active));
  2979			}
  2980			if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) {
  2981				drm_rgba_t background = pipe_config->base.background_color;
  2982	
> 2983				seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
  2984					   background.v,
  2985					   DRM_RGBA_REDBITS(background, 10),
  2986					   DRM_RGBA_GREENBITS(background, 10),
  2987					   DRM_RGBA_BLUEBITS(background, 10));
  2988			}
  2989	
  2990			seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",
  2991				   yesno(!crtc->cpu_fifo_underrun_disabled),

---
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: 24098 bytes --]

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

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Add support for pipe background color

[kbuild test robot]

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

Hi Matt,

[auto build test WARNING on drm/drm-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Matt-Roper/CRTC-background-color-support-for-i915/20151023-082852
config: x86_64-rhel (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/i915_debugfs.c: In function 'i915_display_info':
>> drivers/gpu/drm/i915/i915_debugfs.c:2987:8: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'uint64_t' [-Wformat=]
           DRM_RGBA_BLUEBITS(background, 10));
           ^
   drivers/gpu/drm/i915/i915_debugfs.c:2987:8: warning: too many arguments for format [-Wformat-extra-args]

vim +2987 drivers/gpu/drm/i915/i915_debugfs.c

  2971				intel_crtc_info(m, crtc);
  2972	
  2973				active = cursor_position(dev, crtc->pipe, &x, &y);
  2974				seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x, active? %s\n",
  2975					   yesno(crtc->cursor_base),
  2976					   x, y, crtc->base.cursor->state->crtc_w,
  2977					   crtc->base.cursor->state->crtc_h,
  2978					   crtc->cursor_addr, yesno(active));
  2979			}
  2980			if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) {
  2981				drm_rgba_t background = pipe_config->base.background_color;
  2982	
  2983				seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
  2984					   background.v,
  2985					   DRM_RGBA_REDBITS(background, 10),
  2986					   DRM_RGBA_GREENBITS(background, 10),
> 2987					   DRM_RGBA_BLUEBITS(background, 10));
  2988			}
  2989	
  2990			seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",
  2991				   yesno(!crtc->cpu_fifo_underrun_disabled),
  2992				   yesno(!crtc->pch_fifo_underrun_disabled));
  2993		}
  2994	
  2995		seq_printf(m, "\n");

---
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: 35271 bytes --]

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

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

Re: [PATCH 1/2] drm: Add infrastructure for CRTC background color property

[kbuild test robot]

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

Hi Matt,

[auto build test WARNING on drm/drm-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Matt-Roper/CRTC-background-color-support-for-i915/20151023-082852
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'fmt'
>> include/drm/drm_crtc.h:267: warning: cannot understand function prototype: 'typedef struct '
   include/drm/drm_crtc.h:353: warning: No description found for parameter 'mode_blob'
   include/drm/drm_crtc.h:780: warning: No description found for parameter 'tile_blob_ptr'
   include/drm/drm_crtc.h:819: warning: No description found for parameter 'rotation'
   include/drm/drm_crtc.h:915: warning: No description found for parameter 'mutex'
   include/drm/drm_crtc.h:915: warning: No description found for parameter 'helper_private'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tile_idr'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'delayed_event'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'edid_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'dpms_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'path_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tile_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'plane_type_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'rotation_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_src_x'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_src_y'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_src_w'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_src_h'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_crtc_x'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_crtc_y'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_crtc_w'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_crtc_h'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_fb_id'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_crtc_id'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_active'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_mode_id'
>> include/drm/drm_crtc.h:1203: warning: No description found for parameter 'prop_background_color'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'dvi_i_subconnector_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'dvi_i_select_subconnector_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_subconnector_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_select_subconnector_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_mode_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_left_margin_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_right_margin_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_top_margin_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_bottom_margin_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_brightness_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_contrast_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_flicker_reduction_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_overscan_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_saturation_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'tv_hue_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'scaling_mode_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'aspect_ratio_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'dirty_info_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'suggested_x_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'suggested_y_property'
   include/drm/drm_crtc.h:1203: warning: No description found for parameter 'allow_fb_modifiers'
   include/drm/drm_fb_helper.h:148: warning: No description found for parameter 'connector_info'
   include/drm/drm_dp_helper.h:713: warning: No description found for parameter 'i2c_nack_count'
   include/drm/drm_dp_helper.h:713: warning: No description found for parameter 'i2c_defer_count'
   drivers/gpu/drm/drm_dp_mst_topology.c:2227: warning: No description found for parameter 'connector'
   include/drm/drm_dp_mst_helper.h:97: warning: No description found for parameter 'cached_edid'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'max_dpcd_transaction_bytes'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'sink_count'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'total_slots'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'avail_slots'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'total_pbn'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'qlock'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_msg_downq'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_msg_upq'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_down_in_progress'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_up_in_progress'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'payload_lock'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'proposed_vcpis'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'payloads'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'payload_mask'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'vcpi_mask'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_waitq'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'work'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_work'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'destroy_connector_list'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'destroy_connector_lock'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'destroy_connector_work'
   drivers/gpu/drm/drm_dp_mst_topology.c:2227: warning: No description found for parameter 'connector'
   drivers/gpu/drm/drm_irq.c:173: 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:353: warning: cannot understand function prototype: 'struct drm_lock_data '
   include/drm/drmP.h:406: warning: cannot understand function prototype: 'struct drm_driver '
   include/drm/drmP.h:656: warning: cannot understand function prototype: 'struct drm_info_list '
   include/drm/drmP.h:666: warning: cannot understand function prototype: 'struct drm_info_node '
   include/drm/drmP.h:676: warning: cannot understand function prototype: 'struct drm_minor '
   include/drm/drmP.h:724: warning: cannot understand function prototype: 'struct drm_device '
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2582: 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:1194: warning: No description found for parameter 'rps'
   drivers/gpu/drm/i915/i915_gem.c:1400: warning: No description found for parameter 'req'
   drivers/gpu/drm/i915/i915_gem.c:1435: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:1435: warning: No description found for parameter 'readonly'
   drivers/gpu/drm/i915/i915_gem.c:1558: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1558: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1558: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1621: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1621: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1621: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1666: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1666: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1666: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: No description found for parameter 'size'

vim +267 include/drm/drm_crtc.h

   251	struct drm_atomic_state;
   252	
   253	/**
   254	 * drm_rgba_t - RGBA property value type
   255	 *
   256	 * Structure to abstract away the representation of RGBA values with precision
   257	 * up to 16 bits per color component.  This is primarily intended for use with
   258	 * DRM properties that need to take a color value since we don't want userspace
   259	 * to have to worry about the bit layout expected by the underlying hardware.
   260	 *
   261	 * We wrap the value in a structure here so that the compiler will flag any
   262	 * accidental attempts by driver code to directly attempt bitwise operations
   263	 * that could potentially misinterpret the value.  Drivers should instead use
   264	 * the DRM_RGBA_{RED,GREEN,BLUE,ALPHA}BITS() macros to obtain the component
   265	 * bits and then build values in the format their hardware expects.
   266	 */
 > 267	typedef struct {
   268		uint64_t v;
   269	} drm_rgba_t;
   270	
   271	static inline uint16_t
   272	drm_rgba_bits(drm_rgba_t c, unsigned compshift, unsigned bits) {
   273		uint64_t val;
   274	
   275		if (WARN_ON(bits > 16))

---
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: 6062 bytes --]

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

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

Re: [PATCH 1/2] drm: Add infrastructure for CRTC background color property

[Bob Paauwe]

On Thu, 22 Oct 2015 17:25:34 -0700
Matt Roper <matthew.d.roper@intel.com> wrote:

> 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 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.
> 
> 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.
> 
> Cc: dri-devel@lists.freedesktop.org
> Cc: Chandra Konduru <chandra.konduru@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c |  4 ++++
>  drivers/gpu/drm/drm_crtc.c   | 33 +++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h       | 49 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7bb3845..688ca75 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -415,6 +415,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);
> @@ -450,6 +452,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 e54660a..1e0dd09 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3807,6 +3807,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);

I'm not sure I understand why drm_property_create_rgba was added.  It's
not being used.  Maybe if the
drm_mode_create_background_color_property() below called this instead
of create_range directly, we could then change the underlying property
type used for rgba without having to locate all properties that are
supposed to be of that type.  Was that the intention?

> +
> +/**
>   * drm_property_add_enum - add a possible value to an enumeration property
>   * @property: enumeration property to change
>   * @index: index of the new enumeration
> @@ -5778,6 +5802,15 @@ struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drm_mode_create_rotation_property);
>  
> +struct drm_property
> +*drm_mode_create_background_color_property(struct drm_device *dev)
> +{
> +	return drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
> +					 "background_color",
> +					 0, GENMASK_ULL(63, 0));
> +}
> +EXPORT_SYMBOL(drm_mode_create_background_color_property);
> +
>  /**
>   * DOC: Tile group
>   *
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 3f0c690..64f3e62 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -251,6 +251,45 @@ struct drm_bridge;
>  struct drm_atomic_state;
>  
>  /**
> + * drm_rgba_t - RGBA property value type
> + *
> + * 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.
> + */
> +typedef struct {
> +	uint64_t v;
> +} drm_rgba_t;
> +
> +static inline uint16_t
> +drm_rgba_bits(drm_rgba_t 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)

Do we also need macros that drivers can use to create a RGBA value?
I.E. maybe when doing the attach?

> +
> +/**
>   * struct drm_crtc_state - mutable CRTC state
>   * @crtc: backpointer to the CRTC
>   * @enable: whether the CRTC should be enabled, gates all other state
> @@ -264,6 +303,7 @@ struct drm_atomic_state;
>   * 	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 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
> @@ -304,6 +344,9 @@ struct drm_crtc_state {
>  	/* blob property to expose current mode to atomic userspace */
>  	struct drm_property_blob *mode_blob;
>  
> +	/* CRTC background color */
> +	drm_rgba_t background_color;
> +
>  	struct drm_pending_vblank_event *event;
>  
>  	struct drm_atomic_state *state;
> @@ -1115,6 +1158,9 @@ struct drm_mode_config {
>  	struct drm_property *prop_active;
>  	struct drm_property *prop_mode_id;
>  
> +	/* crtc properties */
> +	struct drm_property *prop_background_color;
> +
>  	/* DVI-I properties */
>  	struct drm_property *dvi_i_subconnector_property;
>  	struct drm_property *dvi_i_select_subconnector_property;
> @@ -1365,6 +1411,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);
> @@ -1495,6 +1543,7 @@ extern int drm_format_vert_chroma_subsampling(uint32_t format);
>  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);
>  



-- 
--
Bob Paauwe                  
Bob.J.Paauwe@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193    

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915/skl: Add support for pipe background color

[Bob Paauwe]

On Thu, 22 Oct 2015 17:25:35 -0700
Matt Roper <matthew.d.roper@intel.com> wrote:

> SKL and BXT 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
> (pre-atomic) that the functionality had to be pretty much completely
> rewritten for the new i915 atomic internals.
> 
> Cc: Chandra Konduru <chandra.konduru@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  |  9 ++++++++
>  drivers/gpu/drm/i915/i915_reg.h      | 10 +++++++++
>  drivers/gpu/drm/i915/intel_display.c | 43 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index c05d7df..40c0d49 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -2819,7 +2819,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>
> @@ -2835,6 +2835,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 a3b22bd..acb05e5 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2977,6 +2977,15 @@ static int i915_display_info(struct seq_file *m, void *unused)
>  				   crtc->base.cursor->state->crtc_h,
>  				   crtc->cursor_addr, yesno(active));
>  		}
> +		if (INTEL_INFO(dev)->gen >= 9 && pipe_config->base.active) {
> +			drm_rgba_t background = pipe_config->base.background_color;
> +
> +			seq_printf(m, "\tbackground color (10bpc): r=%x g=%x b=%x\n",
> +				   background.v,
> +				   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 9ebf032..95ea394 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7562,6 +7562,16 @@ enum skl_disp_power_wells {
>  #define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
>  #define PIPE_CSC_POSTOFF_LO(pipe) _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) _PIPE3(pipe, _PIPE_BOTTOM_COLOR_A, \
> +	_PIPE_BOTTOM_COLOR_B, _PIPE_BOTTOM_COLOR_C)
> +
>  /* 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 1fc1d24..b00fca9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3332,6 +3332,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);
> +	drm_rgba_t 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;
> @@ -3368,6 +3370,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)
> @@ -11788,6 +11810,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;
>  }
>  
> @@ -13230,6 +13255,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,
>  };
> @@ -13780,6 +13806,19 @@ 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, 0);

This only works because right now drm_rgba_t can represent black as 0,
but you're not honoring the abstraction.  What if black really needed
to have alpha set or drm_rgba_t expands to more than 16 bits per color
or we just want to initialize the background color to something other
than black?

> +}
> +
>  static void intel_crtc_init(struct drm_device *dev, int pipe)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -13855,6 +13894,10 @@ 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)
> +		intel_create_background_color_property(dev, intel_crtc);
> +
>  	return;
>  
>  fail:



-- 
--
Bob Paauwe                  
Bob.J.Paauwe@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193    

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

Re: [PATCH 1/2] drm: Add infrastructure for CRTC background color property

[Matt Roper]

On Wed, Nov 18, 2015 at 01:35:54PM -0800, Bob Paauwe wrote:
> On Thu, 22 Oct 2015 17:25:34 -0700
> Matt Roper <matthew.d.roper@intel.com> wrote:
> 
> > 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 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.
> > 
> > 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.
> > 
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/drm_atomic.c |  4 ++++
> >  drivers/gpu/drm/drm_crtc.c   | 33 +++++++++++++++++++++++++++++
> >  include/drm/drm_crtc.h       | 49 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 86 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 7bb3845..688ca75 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -415,6 +415,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);
> > @@ -450,6 +452,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 e54660a..1e0dd09 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -3807,6 +3807,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);
> 
> I'm not sure I understand why drm_property_create_rgba was added.  It's
> not being used.  Maybe if the
> drm_mode_create_background_color_property() below called this instead
> of create_range directly, we could then change the underlying property
> type used for rgba without having to locate all properties that are
> supposed to be of that type.  Was that the intention?

Yeah, I meant to use this for the background_color_property function
below, but I guess I forgot to go back and actually make that change.
This is more just a helper so that drivers that make driver-specific
rgba properties won't have to think about what the internal
representation really is (the same way boolean properties are really
just a helper for range [0,1] properties).

> 
> > +
> > +/**
> >   * drm_property_add_enum - add a possible value to an enumeration property
> >   * @property: enumeration property to change
> >   * @index: index of the new enumeration
> > @@ -5778,6 +5802,15 @@ struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
> >  }
> >  EXPORT_SYMBOL(drm_mode_create_rotation_property);
> >  
> > +struct drm_property
> > +*drm_mode_create_background_color_property(struct drm_device *dev)
> > +{
> > +	return drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
> > +					 "background_color",
> > +					 0, GENMASK_ULL(63, 0));
> > +}
> > +EXPORT_SYMBOL(drm_mode_create_background_color_property);
> > +
> >  /**
> >   * DOC: Tile group
> >   *
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 3f0c690..64f3e62 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -251,6 +251,45 @@ struct drm_bridge;
> >  struct drm_atomic_state;
> >  
> >  /**
> > + * drm_rgba_t - RGBA property value type
> > + *
> > + * 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.
> > + */
> > +typedef struct {
> > +	uint64_t v;
> > +} drm_rgba_t;
> > +
> > +static inline uint16_t
> > +drm_rgba_bits(drm_rgba_t 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)
> 
> Do we also need macros that drivers can use to create a RGBA value?
> I.E. maybe when doing the attach?
> 

Yeah, makes sense; the macro I have in libdrm should also be available
in the kernel.  I'll add that in the next version.


Matt


> > +
> > +/**
> >   * struct drm_crtc_state - mutable CRTC state
> >   * @crtc: backpointer to the CRTC
> >   * @enable: whether the CRTC should be enabled, gates all other state
> > @@ -264,6 +303,7 @@ struct drm_atomic_state;
> >   * 	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 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
> > @@ -304,6 +344,9 @@ struct drm_crtc_state {
> >  	/* blob property to expose current mode to atomic userspace */
> >  	struct drm_property_blob *mode_blob;
> >  
> > +	/* CRTC background color */
> > +	drm_rgba_t background_color;
> > +
> >  	struct drm_pending_vblank_event *event;
> >  
> >  	struct drm_atomic_state *state;
> > @@ -1115,6 +1158,9 @@ struct drm_mode_config {
> >  	struct drm_property *prop_active;
> >  	struct drm_property *prop_mode_id;
> >  
> > +	/* crtc properties */
> > +	struct drm_property *prop_background_color;
> > +
> >  	/* DVI-I properties */
> >  	struct drm_property *dvi_i_subconnector_property;
> >  	struct drm_property *dvi_i_select_subconnector_property;
> > @@ -1365,6 +1411,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);
> > @@ -1495,6 +1543,7 @@ extern int drm_format_vert_chroma_subsampling(uint32_t format);
> >  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);
> >  
> 
> 
> 
> -- 
> --
> Bob Paauwe                  
> Bob.J.Paauwe@intel.com
> IOTG / PED Software Organization
> Intel Corp.  Folsom, CA
> (916) 356-6193    
> 

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

Re: [PATCH 1/2] drm: Add infrastructure for CRTC background color property

[Emil Velikov]

Hi Matt,

On 23 October 2015 at 01:25, Matt Roper <matthew.d.roper@intel.com> wrote:

> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h

> +typedef struct {
> +       uint64_t v;
> +} drm_rgba_t;
> +
Humble request - please don't add typedefs. The drm subsystem (barring
legacy core and certain drivers) is relatively clean of them. The
extra "struct" is not that much to type :-)

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel