[PATCH xf86-video-intel] sna: Add XV_COLORSPACE attribute support for sprite Xv adaptors

[PATCH xf86-video-intel] sna: Add XV_COLORSPACE attribute support for sprite Xv adaptors

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use the new "COLOR_ENCODING" plane property to implement the
XV_COLORSPACE port attribute for sprite Xv adaptors.

Cc: Jyri Sarha <jsarha@ti.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 src/sna/sna.h              |   1 +
 src/sna/sna_display.c      | 148 +++++++++++++++++++++++++++++++++++----------
 src/sna/sna_video.h        |   3 +
 src/sna/sna_video_sprite.c |  22 ++++++-
 4 files changed, 142 insertions(+), 32 deletions(-)

diff --git a/src/sna/sna.h b/src/sna/sna.h
index 5283ce436a3b..c9097d3db158 100644
--- a/src/sna/sna.h
+++ b/src/sna/sna.h
@@ -634,6 +634,7 @@ static inline void sna_present_cancel_flip(struct sna *sna) { }
 
 extern unsigned sna_crtc_count_sprites(xf86CrtcPtr crtc);
 extern bool sna_crtc_set_sprite_rotation(xf86CrtcPtr crtc, unsigned idx, uint32_t rotation);
+extern void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc, unsigned idx, int colorspace);
 extern uint32_t sna_crtc_to_sprite(xf86CrtcPtr crtc, unsigned idx);
 extern bool sna_crtc_is_transformed(xf86CrtcPtr crtc);
 
diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index ea2f148d213c..86aa711f886d 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -222,6 +222,10 @@ struct sna_crtc {
 			uint32_t supported;
 			uint32_t current;
 		} rotation;
+		struct {
+			uint32_t prop;
+			uint64_t values[2];
+		} color_encoding;
 		struct list link;
 	} primary;
 	struct list sprites;
@@ -3293,17 +3297,124 @@ static const xf86CrtcFuncsRec sna_crtc_funcs = {
 #endif
 };
 
-inline static bool prop_is_rotation(struct drm_mode_get_property *prop)
+inline static bool prop_has_type_and_name(const struct drm_mode_get_property *prop,
+					  unsigned int type, const char *name)
 {
-	if ((prop->flags & (1 << 5)) == 0)
+	if ((prop->flags & (1 << type)) == 0)
 		return false;
 
-	if (strcmp(prop->name, "rotation"))
+	if (strcmp(prop->name, name))
 		return false;
 
 	return true;
 }
 
+inline static bool prop_is_rotation(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 5, "rotation");
+}
+
+static void parse_rotation_prop(struct sna *sna, struct plane *p,
+				struct drm_mode_get_property *prop,
+				uint64_t value)
+{
+	struct drm_mode_property_enum *enums;
+	int j;
+
+	p->rotation.prop = prop->prop_id;
+	p->rotation.current = value;
+
+	DBG(("%s: found rotation property .id=%d, value=%ld, num_enums=%d\n",
+	     __FUNCTION__, prop->prop_id, value, prop->count_enum_blobs));
+
+	enums = malloc(prop->count_enum_blobs * sizeof(struct drm_mode_property_enum));
+	if (!enums)
+		return;
+
+	prop->count_values = 0;
+	prop->enum_blob_ptr = (uintptr_t)enums;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPERTY, prop)) {
+		free(enums);
+		return;
+	}
+
+	/* XXX we assume that the mapping between kernel enum and
+	 * RandR remains fixed for our lifetimes.
+	 */
+	VG(VALGRIND_MAKE_MEM_DEFINED(enums, sizeof(*enums)*prop->count_enum_blobs));
+	for (j = 0; j < prop->count_enum_blobs; j++) {
+		DBG(("%s: rotation[%d] = %s [%lx]\n", __FUNCTION__,
+		     j, enums[j].name, (long)enums[j].value));
+		p->rotation.supported |= 1 << enums[j].value;
+	}
+
+	free(enums);
+}
+
+inline static bool prop_is_color_encoding(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 3, "COLOR_ENCODING");
+}
+
+static void parse_color_encoding_prop(struct sna *sna, struct plane *p,
+				      struct drm_mode_get_property *prop,
+				      uint64_t value)
+{
+	struct drm_mode_property_enum *enums;
+	unsigned int supported = 0;
+	int j;
+
+	DBG(("%s: found color encoding property .id=%d, value=%ld, num_enums=%d\n",
+	     __FUNCTION__, prop->prop_id, (long)value, prop->count_enum_blobs));
+
+	enums = malloc(prop->count_enum_blobs * sizeof(struct drm_mode_property_enum));
+	if (!enums)
+		return;
+
+	prop->count_values = 0;
+	prop->enum_blob_ptr = (uintptr_t)enums;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPERTY, prop)) {
+		free(enums);
+		return;
+	}
+
+	VG(VALGRIND_MAKE_MEM_DEFINED(enums, sizeof(*enums)*prop->count_enum_blobs));
+	for (j = 0; j < prop->count_enum_blobs; j++) {
+		if (!strcmp(enums[j].name, "ITU-R BT.601 YCbCr")) {
+			p->color_encoding.values[0] = enums[j].value;
+			supported |= 1 << 0;
+		} else if (!strcmp(enums[j].name, "ITU-R BT.709 YCbCr")) {
+			p->color_encoding.values[1] = enums[j].value;
+			supported |= 1 << 1;
+		}
+	}
+
+	free(enums);
+
+	if (supported == 3)
+		p->color_encoding.prop = prop->prop_id;
+}
+
+void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc,
+				    unsigned idx, int colorspace)
+{
+	struct plane *p;
+
+	assert(to_sna_crtc(crtc));
+
+	p = lookup_sprite(to_sna_crtc(crtc), idx);
+
+	if (!p->color_encoding.prop)
+		return;
+
+	drmModeObjectSetProperty(to_sna(crtc->scrn)->kgem.fd,
+				 p->id, DRM_MODE_OBJECT_PLANE,
+				 p->color_encoding.prop,
+				 p->color_encoding.values[colorspace]);
+}
+
 static int plane_details(struct sna *sna, struct plane *p)
 {
 #define N_STACK_PROPS 32 /* must be a multiple of 2 */
@@ -3360,34 +3471,9 @@ static int plane_details(struct sna *sna, struct plane *p)
 		if (strcmp(prop.name, "type") == 0) {
 			type = values[i];
 		} else if (prop_is_rotation(&prop)) {
-			struct drm_mode_property_enum *enums;
-
-			p->rotation.prop = props[i];
-			p->rotation.current = values[i];
-
-			DBG(("%s: found rotation property .id=%d, value=%ld, num_enums=%d\n",
-			     __FUNCTION__, prop.prop_id, (long)values[i], prop.count_enum_blobs));
-			enums = malloc(prop.count_enum_blobs * sizeof(struct drm_mode_property_enum));
-			if (enums != NULL) {
-				prop.count_values = 0;
-				prop.enum_blob_ptr = (uintptr_t)enums;
-
-				if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPERTY, &prop) == 0) {
-					int j;
-
-					/* XXX we assume that the mapping between kernel enum and
-					 * RandR remains fixed for our lifetimes.
-					 */
-					VG(VALGRIND_MAKE_MEM_DEFINED(enums, sizeof(*enums)*prop.count_enum_blobs));
-					for (j = 0; j < prop.count_enum_blobs; j++) {
-						DBG(("%s: rotation[%d] = %s [%lx]\n", __FUNCTION__,
-						     j, enums[j].name, (long)enums[j].value));
-						p->rotation.supported |= 1 << enums[j].value;
-					}
-				}
-
-				free(enums);
-			}
+			parse_rotation_prop(sna, p, &prop, values[i]);
+		} else if (prop_is_color_encoding(&prop)) {
+			parse_color_encoding_prop(sna, p, &prop, values[i]);
 		}
 	}
 
diff --git a/src/sna/sna_video.h b/src/sna/sna_video.h
index 6b1c864ce9d8..6abf6d540b18 100644
--- a/src/sna/sna_video.h
+++ b/src/sna/sna_video.h
@@ -100,6 +100,9 @@ struct sna_video {
 	unsigned color_key_changed;
 	bool has_color_key;
 
+	unsigned colorspace;
+	unsigned colorspace_changed;
+
 	/** YUV data buffers */
 	struct kgem_bo *old_buf[2];
 	struct kgem_bo *buf;
diff --git a/src/sna/sna_video_sprite.c b/src/sna/sna_video_sprite.c
index 44898a748e1c..f713ba356b41 100644
--- a/src/sna/sna_video_sprite.c
+++ b/src/sna/sna_video_sprite.c
@@ -69,11 +69,12 @@ struct local_mode_set_plane {
 
 #define MAKE_ATOM(a) MakeAtom(a, sizeof(a) - 1, true)
 
-static Atom xvColorKey, xvAlwaysOnTop, xvSyncToVblank;
+static Atom xvColorKey, xvAlwaysOnTop, xvSyncToVblank, xvColorspace;
 
 static XvFormatRec formats[] = { {15}, {16}, {24} };
 static const XvImageRec images[] = { XVIMAGE_YUY2, XVIMAGE_UYVY, XVMC_RGB888, XVMC_RGB565 };
 static const XvAttributeRec attribs[] = {
+	{ XvSettable | XvGettable, 0, 1, (char *)"XV_COLORSPACE" }, /* BT.601, BT.709 */
 	{ XvSettable | XvGettable, 0, 0xffffff, (char *)"XV_COLORKEY" },
 	{ XvSettable | XvGettable, 0, 1, (char *)"XV_ALWAYS_ON_TOP" },
 };
@@ -119,6 +120,10 @@ static int sna_video_sprite_set_attr(ddSetPortAttribute_ARGS)
 		video->color_key = value;
 		RegionEmpty(&video->clip);
 		DBG(("COLORKEY = %ld\n", (long)value));
+	} else if (attribute == xvColorspace) {
+		video->colorspace_changed = ~0;
+		video->colorspace = value;
+		DBG(("COLORSPACE = %ld\n", (long)value));
 	} else if (attribute == xvSyncToVblank) {
 		DBG(("%s: SYNC_TO_VBLANK: %d -> %d\n", __FUNCTION__,
 		     video->SyncToVblank, !!value));
@@ -140,6 +145,8 @@ static int sna_video_sprite_get_attr(ddGetPortAttribute_ARGS)
 
 	if (attribute == xvColorKey)
 		*value = video->color_key;
+	else if (attribute == xvColorspace)
+		*value = video->colorspace;
 	else if (attribute == xvAlwaysOnTop)
 		*value = video->AlwaysOnTop;
 	else if (attribute == xvSyncToVblank)
@@ -265,6 +272,16 @@ sna_video_sprite_show(struct sna *sna,
 		video->color_key_changed &= ~(1 << pipe);
 	}
 
+	if (video->colorspace_changed & (1 << pipe)) {
+		DBG(("%s: updating colorspace: %x\n",
+		     __FUNCTION__, video->colorspace));
+
+		sna_crtc_set_sprite_colorspace(crtc, video->idx,
+					       video->colorspace);
+
+		video->colorspace_changed &= ~(1 << pipe);
+	}
+
 	update_dst_box_to_crtc_coords(sna, crtc, dstBox);
 	if (frame->rotation & (RR_Rotate_90 | RR_Rotate_270)) {
 		int tmp = frame->width;
@@ -769,6 +786,8 @@ void sna_video_sprite_setup(struct sna *sna, ScreenPtr screen)
 		video->alignment = 64;
 		video->color_key = sna_video_sprite_color_key(sna);
 		video->color_key_changed = ~0;
+		video->colorspace = 1; /* BT.709 */
+		video->colorspace_changed = ~0;
 		video->has_color_key = true;
 		video->brightness = -19;	/* (255/219) * -16 */
 		video->contrast = 75;	/* 255/219 * 64 */
@@ -789,6 +808,7 @@ void sna_video_sprite_setup(struct sna *sna, ScreenPtr screen)
 	adaptor->base_id = adaptor->pPorts[0].id;
 
 	xvColorKey = MAKE_ATOM("XV_COLORKEY");
+	xvColorspace = MAKE_ATOM("XV_COLORSPACE");
 	xvAlwaysOnTop = MAKE_ATOM("XV_ALWAYS_ON_TOP");
 	xvSyncToVblank = MAKE_ATOM("XV_SYNC_TO_VBLANK");
 
-- 
2.13.6

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

Re: [PATCH xf86-video-intel] sna: Add XV_COLORSPACE attribute support for sprite Xv adaptors

[Chris Wilson]

Quoting Ville Syrjala (2018-02-14 19:21:08)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use the new "COLOR_ENCODING" plane property to implement the
> XV_COLORSPACE port attribute for sprite Xv adaptors.
> 
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> +void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc,
> +                                   unsigned idx, int colorspace)
> +{
> +       struct plane *p;
> +
> +       assert(to_sna_crtc(crtc));

assert(colorspace < ARRAY_SIZE(p->color_encoding.values));

> +
> +       p = lookup_sprite(to_sna_crtc(crtc), idx);
> +
> +       if (!p->color_encoding.prop)
> +               return;
> +
> +       drmModeObjectSetProperty(to_sna(crtc->scrn)->kgem.fd,
> +                                p->id, DRM_MODE_OBJECT_PLANE,
> +                                p->color_encoding.prop,
> +                                p->color_encoding.values[colorspace]);

Does changing the property trigger an immediate update, or is that
deferred until the next SetPlane (or atomic update)?

It makes no difference to this patch, except we might redisplay the
image immediately rather than wait for the client?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH xf86-video-intel] sna: Add XV_COLORSPACE attribute support for sprite Xv adaptors

[Ville Syrjälä]

On Wed, Feb 14, 2018 at 07:42:59PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-02-14 19:21:08)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Use the new "COLOR_ENCODING" plane property to implement the
> > XV_COLORSPACE port attribute for sprite Xv adaptors.
> > 
> > Cc: Jyri Sarha <jsarha@ti.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > +void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc,
> > +                                   unsigned idx, int colorspace)
> > +{
> > +       struct plane *p;
> > +
> > +       assert(to_sna_crtc(crtc));
> 
> assert(colorspace < ARRAY_SIZE(p->color_encoding.values));
> 
> > +
> > +       p = lookup_sprite(to_sna_crtc(crtc), idx);
> > +
> > +       if (!p->color_encoding.prop)
> > +               return;
> > +
> > +       drmModeObjectSetProperty(to_sna(crtc->scrn)->kgem.fd,
> > +                                p->id, DRM_MODE_OBJECT_PLANE,
> > +                                p->color_encoding.prop,
> > +                                p->color_encoding.values[colorspace]);
> 
> Does changing the property trigger an immediate update, or is that
> deferred until the next SetPlane (or atomic update)?

Legacy setprop so it'll trigger an update. So if the plane is
already enabled I suppose we'll get an extra frame of lag here.
Same deal as for the colorkey ioctl actually.

I think generally one can assume the the attribute won't be changed
very often with the plane already enabled. Generally I'd expect the
client to set this up before the first putimage.

Converting this to atomic would avoid that extra lag. But we'd need
to convert the colorkey ioctl to properties as well. And the next step
after that would tearfree+atomic for making it actually nice :)

> 
> It makes no difference to this patch, except we might redisplay the
> image immediately rather than wait for the client?

We call this from the putimage hook, so we need the client to
actually repaint before anything happens. And since we don't have
reputimage manipulating the window/clipping won't update the
sprite unless the client repaints (which eg. mplayer won't do
when paused unless it gets expose events).

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

Re: [PATCH xf86-video-intel] sna: Add XV_COLORSPACE attribute support for sprite Xv adaptors

[Chris Wilson]

Quoting Ville Syrjälä (2018-02-14 20:06:17)
> On Wed, Feb 14, 2018 at 07:42:59PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjala (2018-02-14 19:21:08)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Use the new "COLOR_ENCODING" plane property to implement the
> > > XV_COLORSPACE port attribute for sprite Xv adaptors.
> > > 
> > > Cc: Jyri Sarha <jsarha@ti.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > +void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc,
> > > +                                   unsigned idx, int colorspace)
> > > +{
> > > +       struct plane *p;
> > > +
> > > +       assert(to_sna_crtc(crtc));
> > 
> > assert(colorspace < ARRAY_SIZE(p->color_encoding.values));
> > 
> > > +
> > > +       p = lookup_sprite(to_sna_crtc(crtc), idx);
> > > +
> > > +       if (!p->color_encoding.prop)
> > > +               return;
> > > +
> > > +       drmModeObjectSetProperty(to_sna(crtc->scrn)->kgem.fd,
> > > +                                p->id, DRM_MODE_OBJECT_PLANE,
> > > +                                p->color_encoding.prop,
> > > +                                p->color_encoding.values[colorspace]);
> > 
> > Does changing the property trigger an immediate update, or is that
> > deferred until the next SetPlane (or atomic update)?
> 
> Legacy setprop so it'll trigger an update. So if the plane is
> already enabled I suppose we'll get an extra frame of lag here.
> Same deal as for the colorkey ioctl actually.
> 
> I think generally one can assume the the attribute won't be changed
> very often with the plane already enabled. Generally I'd expect the
> client to set this up before the first putimage.

Agreed.

> Converting this to atomic would avoid that extra lag. But we'd need
> to convert the colorkey ioctl to properties as well. And the next step
> after that would tearfree+atomic for making it actually nice :)

If we wanted nice, we wouldn't start with the X11 protocol! Oh well, if
all the shiny new features are going to be hidden being atomic, probably
going to have to bite the bullet sooner or later.

> > It makes no difference to this patch, except we might redisplay the
> > image immediately rather than wait for the client?
> 
> We call this from the putimage hook, so we need the client to
> actually repaint before anything happens. And since we don't have
> reputimage manipulating the window/clipping won't update the
> sprite unless the client repaints (which eg. mplayer won't do
> when paused unless it gets expose events).

I knew you were going to mention the lack of ReputImage! :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH xf86-video-intel v2] sna: Add XV_COLORSPACE attribute support for sprite Xv adaptors

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use the new "COLOR_ENCODING" plane property to implement the
XV_COLORSPACE port attribute for sprite Xv adaptors.

v2: assert(colorspace < ARRAY_SIZE) (Chris)

Cc: Jyri Sarha <jsarha@ti.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 src/sna/sna.h              |   1 +
 src/sna/sna_display.c      | 149 +++++++++++++++++++++++++++++++++++----------
 src/sna/sna_video.h        |   3 +
 src/sna/sna_video_sprite.c |  22 ++++++-
 4 files changed, 143 insertions(+), 32 deletions(-)

diff --git a/src/sna/sna.h b/src/sna/sna.h
index 5283ce436a3b..c9097d3db158 100644
--- a/src/sna/sna.h
+++ b/src/sna/sna.h
@@ -634,6 +634,7 @@ static inline void sna_present_cancel_flip(struct sna *sna) { }
 
 extern unsigned sna_crtc_count_sprites(xf86CrtcPtr crtc);
 extern bool sna_crtc_set_sprite_rotation(xf86CrtcPtr crtc, unsigned idx, uint32_t rotation);
+extern void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc, unsigned idx, int colorspace);
 extern uint32_t sna_crtc_to_sprite(xf86CrtcPtr crtc, unsigned idx);
 extern bool sna_crtc_is_transformed(xf86CrtcPtr crtc);
 
diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index ea2f148d213c..091932f73971 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -222,6 +222,10 @@ struct sna_crtc {
 			uint32_t supported;
 			uint32_t current;
 		} rotation;
+		struct {
+			uint32_t prop;
+			uint64_t values[2];
+		} color_encoding;
 		struct list link;
 	} primary;
 	struct list sprites;
@@ -3293,17 +3297,125 @@ static const xf86CrtcFuncsRec sna_crtc_funcs = {
 #endif
 };
 
-inline static bool prop_is_rotation(struct drm_mode_get_property *prop)
+inline static bool prop_has_type_and_name(const struct drm_mode_get_property *prop,
+					  unsigned int type, const char *name)
 {
-	if ((prop->flags & (1 << 5)) == 0)
+	if ((prop->flags & (1 << type)) == 0)
 		return false;
 
-	if (strcmp(prop->name, "rotation"))
+	if (strcmp(prop->name, name))
 		return false;
 
 	return true;
 }
 
+inline static bool prop_is_rotation(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 5, "rotation");
+}
+
+static void parse_rotation_prop(struct sna *sna, struct plane *p,
+				struct drm_mode_get_property *prop,
+				uint64_t value)
+{
+	struct drm_mode_property_enum *enums;
+	int j;
+
+	p->rotation.prop = prop->prop_id;
+	p->rotation.current = value;
+
+	DBG(("%s: found rotation property .id=%d, value=%ld, num_enums=%d\n",
+	     __FUNCTION__, prop->prop_id, value, prop->count_enum_blobs));
+
+	enums = malloc(prop->count_enum_blobs * sizeof(struct drm_mode_property_enum));
+	if (!enums)
+		return;
+
+	prop->count_values = 0;
+	prop->enum_blob_ptr = (uintptr_t)enums;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPERTY, prop)) {
+		free(enums);
+		return;
+	}
+
+	/* XXX we assume that the mapping between kernel enum and
+	 * RandR remains fixed for our lifetimes.
+	 */
+	VG(VALGRIND_MAKE_MEM_DEFINED(enums, sizeof(*enums)*prop->count_enum_blobs));
+	for (j = 0; j < prop->count_enum_blobs; j++) {
+		DBG(("%s: rotation[%d] = %s [%lx]\n", __FUNCTION__,
+		     j, enums[j].name, (long)enums[j].value));
+		p->rotation.supported |= 1 << enums[j].value;
+	}
+
+	free(enums);
+}
+
+inline static bool prop_is_color_encoding(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 3, "COLOR_ENCODING");
+}
+
+static void parse_color_encoding_prop(struct sna *sna, struct plane *p,
+				      struct drm_mode_get_property *prop,
+				      uint64_t value)
+{
+	struct drm_mode_property_enum *enums;
+	unsigned int supported = 0;
+	int j;
+
+	DBG(("%s: found color encoding property .id=%d, value=%ld, num_enums=%d\n",
+	     __FUNCTION__, prop->prop_id, (long)value, prop->count_enum_blobs));
+
+	enums = malloc(prop->count_enum_blobs * sizeof(struct drm_mode_property_enum));
+	if (!enums)
+		return;
+
+	prop->count_values = 0;
+	prop->enum_blob_ptr = (uintptr_t)enums;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPERTY, prop)) {
+		free(enums);
+		return;
+	}
+
+	VG(VALGRIND_MAKE_MEM_DEFINED(enums, sizeof(*enums)*prop->count_enum_blobs));
+	for (j = 0; j < prop->count_enum_blobs; j++) {
+		if (!strcmp(enums[j].name, "ITU-R BT.601 YCbCr")) {
+			p->color_encoding.values[0] = enums[j].value;
+			supported |= 1 << 0;
+		} else if (!strcmp(enums[j].name, "ITU-R BT.709 YCbCr")) {
+			p->color_encoding.values[1] = enums[j].value;
+			supported |= 1 << 1;
+		}
+	}
+
+	free(enums);
+
+	if (supported == 3)
+		p->color_encoding.prop = prop->prop_id;
+}
+
+void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc,
+				    unsigned idx, int colorspace)
+{
+	struct plane *p;
+
+	assert(to_sna_crtc(crtc));
+	assert(colorspace < ARRAY_SIZE(p->color_encoding.values));
+
+	p = lookup_sprite(to_sna_crtc(crtc), idx);
+
+	if (!p->color_encoding.prop)
+		return;
+
+	drmModeObjectSetProperty(to_sna(crtc->scrn)->kgem.fd,
+				 p->id, DRM_MODE_OBJECT_PLANE,
+				 p->color_encoding.prop,
+				 p->color_encoding.values[colorspace]);
+}
+
 static int plane_details(struct sna *sna, struct plane *p)
 {
 #define N_STACK_PROPS 32 /* must be a multiple of 2 */
@@ -3360,34 +3472,9 @@ static int plane_details(struct sna *sna, struct plane *p)
 		if (strcmp(prop.name, "type") == 0) {
 			type = values[i];
 		} else if (prop_is_rotation(&prop)) {
-			struct drm_mode_property_enum *enums;
-
-			p->rotation.prop = props[i];
-			p->rotation.current = values[i];
-
-			DBG(("%s: found rotation property .id=%d, value=%ld, num_enums=%d\n",
-			     __FUNCTION__, prop.prop_id, (long)values[i], prop.count_enum_blobs));
-			enums = malloc(prop.count_enum_blobs * sizeof(struct drm_mode_property_enum));
-			if (enums != NULL) {
-				prop.count_values = 0;
-				prop.enum_blob_ptr = (uintptr_t)enums;
-
-				if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPERTY, &prop) == 0) {
-					int j;
-
-					/* XXX we assume that the mapping between kernel enum and
-					 * RandR remains fixed for our lifetimes.
-					 */
-					VG(VALGRIND_MAKE_MEM_DEFINED(enums, sizeof(*enums)*prop.count_enum_blobs));
-					for (j = 0; j < prop.count_enum_blobs; j++) {
-						DBG(("%s: rotation[%d] = %s [%lx]\n", __FUNCTION__,
-						     j, enums[j].name, (long)enums[j].value));
-						p->rotation.supported |= 1 << enums[j].value;
-					}
-				}
-
-				free(enums);
-			}
+			parse_rotation_prop(sna, p, &prop, values[i]);
+		} else if (prop_is_color_encoding(&prop)) {
+			parse_color_encoding_prop(sna, p, &prop, values[i]);
 		}
 	}
 
diff --git a/src/sna/sna_video.h b/src/sna/sna_video.h
index 6b1c864ce9d8..6abf6d540b18 100644
--- a/src/sna/sna_video.h
+++ b/src/sna/sna_video.h
@@ -100,6 +100,9 @@ struct sna_video {
 	unsigned color_key_changed;
 	bool has_color_key;
 
+	unsigned colorspace;
+	unsigned colorspace_changed;
+
 	/** YUV data buffers */
 	struct kgem_bo *old_buf[2];
 	struct kgem_bo *buf;
diff --git a/src/sna/sna_video_sprite.c b/src/sna/sna_video_sprite.c
index 44898a748e1c..f713ba356b41 100644
--- a/src/sna/sna_video_sprite.c
+++ b/src/sna/sna_video_sprite.c
@@ -69,11 +69,12 @@ struct local_mode_set_plane {
 
 #define MAKE_ATOM(a) MakeAtom(a, sizeof(a) - 1, true)
 
-static Atom xvColorKey, xvAlwaysOnTop, xvSyncToVblank;
+static Atom xvColorKey, xvAlwaysOnTop, xvSyncToVblank, xvColorspace;
 
 static XvFormatRec formats[] = { {15}, {16}, {24} };
 static const XvImageRec images[] = { XVIMAGE_YUY2, XVIMAGE_UYVY, XVMC_RGB888, XVMC_RGB565 };
 static const XvAttributeRec attribs[] = {
+	{ XvSettable | XvGettable, 0, 1, (char *)"XV_COLORSPACE" }, /* BT.601, BT.709 */
 	{ XvSettable | XvGettable, 0, 0xffffff, (char *)"XV_COLORKEY" },
 	{ XvSettable | XvGettable, 0, 1, (char *)"XV_ALWAYS_ON_TOP" },
 };
@@ -119,6 +120,10 @@ static int sna_video_sprite_set_attr(ddSetPortAttribute_ARGS)
 		video->color_key = value;
 		RegionEmpty(&video->clip);
 		DBG(("COLORKEY = %ld\n", (long)value));
+	} else if (attribute == xvColorspace) {
+		video->colorspace_changed = ~0;
+		video->colorspace = value;
+		DBG(("COLORSPACE = %ld\n", (long)value));
 	} else if (attribute == xvSyncToVblank) {
 		DBG(("%s: SYNC_TO_VBLANK: %d -> %d\n", __FUNCTION__,
 		     video->SyncToVblank, !!value));
@@ -140,6 +145,8 @@ static int sna_video_sprite_get_attr(ddGetPortAttribute_ARGS)
 
 	if (attribute == xvColorKey)
 		*value = video->color_key;
+	else if (attribute == xvColorspace)
+		*value = video->colorspace;
 	else if (attribute == xvAlwaysOnTop)
 		*value = video->AlwaysOnTop;
 	else if (attribute == xvSyncToVblank)
@@ -265,6 +272,16 @@ sna_video_sprite_show(struct sna *sna,
 		video->color_key_changed &= ~(1 << pipe);
 	}
 
+	if (video->colorspace_changed & (1 << pipe)) {
+		DBG(("%s: updating colorspace: %x\n",
+		     __FUNCTION__, video->colorspace));
+
+		sna_crtc_set_sprite_colorspace(crtc, video->idx,
+					       video->colorspace);
+
+		video->colorspace_changed &= ~(1 << pipe);
+	}
+
 	update_dst_box_to_crtc_coords(sna, crtc, dstBox);
 	if (frame->rotation & (RR_Rotate_90 | RR_Rotate_270)) {
 		int tmp = frame->width;
@@ -769,6 +786,8 @@ void sna_video_sprite_setup(struct sna *sna, ScreenPtr screen)
 		video->alignment = 64;
 		video->color_key = sna_video_sprite_color_key(sna);
 		video->color_key_changed = ~0;
+		video->colorspace = 1; /* BT.709 */
+		video->colorspace_changed = ~0;
 		video->has_color_key = true;
 		video->brightness = -19;	/* (255/219) * -16 */
 		video->contrast = 75;	/* 255/219 * 64 */
@@ -789,6 +808,7 @@ void sna_video_sprite_setup(struct sna *sna, ScreenPtr screen)
 	adaptor->base_id = adaptor->pPorts[0].id;
 
 	xvColorKey = MAKE_ATOM("XV_COLORKEY");
+	xvColorspace = MAKE_ATOM("XV_COLORSPACE");
 	xvAlwaysOnTop = MAKE_ATOM("XV_ALWAYS_ON_TOP");
 	xvSyncToVblank = MAKE_ATOM("XV_SYNC_TO_VBLANK");
 
-- 
2.13.6

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

Re: [PATCH xf86-video-intel v2] sna: Add XV_COLORSPACE attribute support for sprite Xv adaptors

[Chris Wilson]

Quoting Ville Syrjala (2018-03-02 11:59:18)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use the new "COLOR_ENCODING" plane property to implement the
> XV_COLORSPACE port attribute for sprite Xv adaptors.
> 
> v2: assert(colorspace < ARRAY_SIZE) (Chris)
> 
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Is this (i.e. the kernel implementation is done) ready to land?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH xf86-video-intel v2] sna: Add XV_COLORSPACE attribute support for sprite Xv adaptors

[Ville Syrjälä]

On Fri, Mar 02, 2018 at 12:01:42PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-03-02 11:59:18)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Use the new "COLOR_ENCODING" plane property to implement the
> > XV_COLORSPACE port attribute for sprite Xv adaptors.
> > 
> > v2: assert(colorspace < ARRAY_SIZE) (Chris)
> > 
> > Cc: Jyri Sarha <jsarha@ti.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Is this (i.e. the kernel implementation is done) ready to land?

No one has complained about the proposed uapi and I have sufficient
r-bs, so I'm ready to push the kernel stuff as soon as there's a green
light from the user space side.

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

Re: [PATCH xf86-video-intel v2] sna: Add XV_COLORSPACE attribute support for sprite Xv adaptors

[Chris Wilson]

Quoting Ville Syrjälä (2018-03-02 12:12:26)
> On Fri, Mar 02, 2018 at 12:01:42PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjala (2018-03-02 11:59:18)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Use the new "COLOR_ENCODING" plane property to implement the
> > > XV_COLORSPACE port attribute for sprite Xv adaptors.
> > > 
> > > v2: assert(colorspace < ARRAY_SIZE) (Chris)
> > > 
> > > Cc: Jyri Sarha <jsarha@ti.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Is this (i.e. the kernel implementation is done) ready to land?
> 
> No one has complained about the proposed uapi and I have sufficient
> r-bs, so I'm ready to push the kernel stuff as soon as there's a green
> light from the user space side.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH xf86-video-intel v2] sna: Add XV_COLORSPACE attribute support for sprite Xv adaptors

[Chris Wilson]

Quoting Chris Wilson (2018-03-02 12:14:07)
> Quoting Ville Syrjälä (2018-03-02 12:12:26)
> > On Fri, Mar 02, 2018 at 12:01:42PM +0000, Chris Wilson wrote:
> > > Quoting Ville Syrjala (2018-03-02 11:59:18)
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Use the new "COLOR_ENCODING" plane property to implement the
> > > > XV_COLORSPACE port attribute for sprite Xv adaptors.
> > > > 
> > > > v2: assert(colorspace < ARRAY_SIZE) (Chris)
> > > > 
> > > > Cc: Jyri Sarha <jsarha@ti.com>
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Is this (i.e. the kernel implementation is done) ready to land?
> > 
> > No one has complained about the proposed uapi and I have sufficient
> > r-bs, so I'm ready to push the kernel stuff as soon as there's a green
> > light from the user space side.
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

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

[PATCH 0/5] drm/i915; Support for COLOR_ENCODING and COLOR_RANGE props

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

While reviewing the COLOR_ENCODING/COLOR_RANGE props I decided that I
might as well implement them in i915. And here is the result.

With this the user can now select between BT.601 vs. BT.709, and
limited vs. full range on every platform where we currently have
YCbCr framebuffer support. I had to fix up GLK a bit more since it
wasn't actually doing the YCbCr->RGB conversion at all.

In order to unblock this further I also hacked together some userspace
support in the form of the XV_COLORSPACE port attribute in the intel
ddx. For that I needed to hack off the atomic requirement for these
properties. I don't think there's much point in restricting these 
to atomic anyway.

I've pushed the entire series (including Jyri's stuff with my hack) here:
git://github.com/vsyrjala/linux.git color_encoding_prop

And the intel ddx xv stuff is available here:
git://github.com/vsyrjala/xf86-video-intel.git xv_colorspace

Cc: Jyri Sarha <jsarha@ti.com>

Ville Syrjälä (5):
  drm/i915: Correctly handle limited range YCbCr data on VLV/CHV
  drm/i915: Fix plane YCbCr->RGB conversion for GLK
  drm/i915: Add support for the YCbCr COLOR_ENCODING property
  drm/i915: Change the COLOR_ENCODING prop default value to BT.709
  drm/i915: Add support for the YCbCr COLOR_RANGE property

 drivers/gpu/drm/i915/i915_reg.h      |  22 ++++++
 drivers/gpu/drm/i915/intel_display.c |  41 ++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 drivers/gpu/drm/i915/intel_sprite.c  | 130 +++++++++++++++++++++++++++--------
 4 files changed, 164 insertions(+), 31 deletions(-)

-- 
2.13.0

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

[PATCH xf86-video-intel] sna: Add XV_COLORSPACE attribute support for sprite Xv adaptors

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use the new "COLOR_ENCODING" plane property to implement the
XV_COLORSPACE port attribute for sprite Xv adaptors.

Cc: Jyri Sarha <jsarha@ti.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 src/sna/sna.h              |   1 +
 src/sna/sna_display.c      | 148 +++++++++++++++++++++++++++++++++++----------
 src/sna/sna_video.h        |   3 +
 src/sna/sna_video_sprite.c |  22 ++++++-
 4 files changed, 142 insertions(+), 32 deletions(-)

diff --git a/src/sna/sna.h b/src/sna/sna.h
index 7861110a99a9..189407c7cd9d 100644
--- a/src/sna/sna.h
+++ b/src/sna/sna.h
@@ -630,6 +630,7 @@ static inline void sna_present_cancel_flip(struct sna *sna) { }
 
 extern unsigned sna_crtc_count_sprites(xf86CrtcPtr crtc);
 extern bool sna_crtc_set_sprite_rotation(xf86CrtcPtr crtc, unsigned idx, uint32_t rotation);
+extern void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc, unsigned idx, int colorspace);
 extern uint32_t sna_crtc_to_sprite(xf86CrtcPtr crtc, unsigned idx);
 extern bool sna_crtc_is_transformed(xf86CrtcPtr crtc);
 
diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index d1f01218a233..78fdca3a1c3a 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -222,6 +222,10 @@ struct sna_crtc {
 			uint32_t supported;
 			uint32_t current;
 		} rotation;
+		struct {
+			uint32_t prop;
+			uint64_t values[2];
+		} color_encoding;
 		struct list link;
 	} primary;
 	struct list sprites;
@@ -3282,17 +3286,124 @@ static const xf86CrtcFuncsRec sna_crtc_funcs = {
 #endif
 };
 
-inline static bool prop_is_rotation(struct drm_mode_get_property *prop)
+inline static bool prop_has_type_and_name(const struct drm_mode_get_property *prop,
+					  unsigned int type, const char *name)
 {
-	if ((prop->flags & (1 << 5)) == 0)
+	if ((prop->flags & (1 << type)) == 0)
 		return false;
 
-	if (strcmp(prop->name, "rotation"))
+	if (strcmp(prop->name, name))
 		return false;
 
 	return true;
 }
 
+inline static bool prop_is_rotation(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 5, "rotation");
+}
+
+static void parse_rotation_prop(struct sna *sna, struct plane *p,
+				struct drm_mode_get_property *prop,
+				uint64_t value)
+{
+	struct drm_mode_property_enum *enums;
+	int j;
+
+	p->rotation.prop = prop->prop_id;
+	p->rotation.current = value;
+
+	DBG(("%s: found rotation property .id=%d, value=%ld, num_enums=%d\n",
+	     __FUNCTION__, prop->prop_id, value, prop->count_enum_blobs));
+
+	enums = malloc(prop->count_enum_blobs * sizeof(struct drm_mode_property_enum));
+	if (!enums)
+		return;
+
+	prop->count_values = 0;
+	prop->enum_blob_ptr = (uintptr_t)enums;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPERTY, prop)) {
+		free(enums);
+		return;
+	}
+
+	/* XXX we assume that the mapping between kernel enum and
+	 * RandR remains fixed for our lifetimes.
+	 */
+	VG(VALGRIND_MAKE_MEM_DEFINED(enums, sizeof(*enums)*prop->count_enum_blobs));
+	for (j = 0; j < prop->count_enum_blobs; j++) {
+		DBG(("%s: rotation[%d] = %s [%lx]\n", __FUNCTION__,
+		     j, enums[j].name, (long)enums[j].value));
+		p->rotation.supported |= 1 << enums[j].value;
+	}
+
+	free(enums);
+}
+
+inline static bool prop_is_color_encoding(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 3, "COLOR_ENCODING");
+}
+
+static void parse_color_encoding_prop(struct sna *sna, struct plane *p,
+				      struct drm_mode_get_property *prop,
+				      uint64_t value)
+{
+	struct drm_mode_property_enum *enums;
+	unsigned int supported = 0;
+	int j;
+
+	DBG(("%s: found color encoding property .id=%d, value=%ld, num_enums=%d\n",
+	     __FUNCTION__, prop->prop_id, (long)value, prop->count_enum_blobs));
+
+	enums = malloc(prop->count_enum_blobs * sizeof(struct drm_mode_property_enum));
+	if (!enums)
+		return;
+
+	prop->count_values = 0;
+	prop->enum_blob_ptr = (uintptr_t)enums;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPERTY, prop)) {
+		free(enums);
+		return;
+	}
+
+	VG(VALGRIND_MAKE_MEM_DEFINED(enums, sizeof(*enums)*prop->count_enum_blobs));
+	for (j = 0; j < prop->count_enum_blobs; j++) {
+		if (!strcmp(enums[j].name, "ITU-R BT.601 YCbCr")) {
+			p->color_encoding.values[0] = enums[j].value;
+			supported |= 1 << 0;
+		} else if (!strcmp(enums[j].name, "ITU-R BT.709 YCbCr")) {
+			p->color_encoding.values[1] = enums[j].value;
+			supported |= 1 << 1;
+		}
+	}
+
+	free(enums);
+
+	if (supported == 3)
+		p->color_encoding.prop = prop->prop_id;
+}
+
+void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc,
+				    unsigned idx, int colorspace)
+{
+	struct plane *p;
+
+	assert(to_sna_crtc(crtc));
+
+	p = lookup_sprite(to_sna_crtc(crtc), idx);
+
+	if (!p->color_encoding.prop)
+		return;
+
+	drmModeObjectSetProperty(to_sna(crtc->scrn)->kgem.fd,
+				 p->id, DRM_MODE_OBJECT_PLANE,
+				 p->color_encoding.prop,
+				 p->color_encoding.values[colorspace]);
+}
+
 static int plane_details(struct sna *sna, struct plane *p)
 {
 #define N_STACK_PROPS 32 /* must be a multiple of 2 */
@@ -3349,34 +3460,9 @@ static int plane_details(struct sna *sna, struct plane *p)
 		if (strcmp(prop.name, "type") == 0) {
 			type = values[i];
 		} else if (prop_is_rotation(&prop)) {
-			struct drm_mode_property_enum *enums;
-
-			p->rotation.prop = props[i];
-			p->rotation.current = values[i];
-
-			DBG(("%s: found rotation property .id=%d, value=%ld, num_enums=%d\n",
-			     __FUNCTION__, prop.prop_id, (long)values[i], prop.count_enum_blobs));
-			enums = malloc(prop.count_enum_blobs * sizeof(struct drm_mode_property_enum));
-			if (enums != NULL) {
-				prop.count_values = 0;
-				prop.enum_blob_ptr = (uintptr_t)enums;
-
-				if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPERTY, &prop) == 0) {
-					int j;
-
-					/* XXX we assume that the mapping between kernel enum and
-					 * RandR remains fixed for our lifetimes.
-					 */
-					VG(VALGRIND_MAKE_MEM_DEFINED(enums, sizeof(*enums)*prop.count_enum_blobs));
-					for (j = 0; j < prop.count_enum_blobs; j++) {
-						DBG(("%s: rotation[%d] = %s [%lx]\n", __FUNCTION__,
-						     j, enums[j].name, (long)enums[j].value));
-						p->rotation.supported |= 1 << enums[j].value;
-					}
-				}
-
-				free(enums);
-			}
+			parse_rotation_prop(sna, p, &prop, values[i]);
+		} else if (prop_is_color_encoding(&prop)) {
+			parse_color_encoding_prop(sna, p, &prop, values[i]);
 		}
 	}
 
diff --git a/src/sna/sna_video.h b/src/sna/sna_video.h
index 39cb725f0050..53ea540a62d3 100644
--- a/src/sna/sna_video.h
+++ b/src/sna/sna_video.h
@@ -90,6 +90,9 @@ struct sna_video {
 	unsigned color_key_changed;
 	bool has_color_key;
 
+	unsigned colorspace;
+	unsigned colorspace_changed;
+
 	/** YUV data buffers */
 	struct kgem_bo *old_buf[2];
 	struct kgem_bo *buf;
diff --git a/src/sna/sna_video_sprite.c b/src/sna/sna_video_sprite.c
index 69bfdfd21b29..86b787f76867 100644
--- a/src/sna/sna_video_sprite.c
+++ b/src/sna/sna_video_sprite.c
@@ -67,11 +67,12 @@ struct local_mode_set_plane {
 
 #define MAKE_ATOM(a) MakeAtom(a, sizeof(a) - 1, true)
 
-static Atom xvColorKey, xvAlwaysOnTop, xvSyncToVblank;
+static Atom xvColorKey, xvAlwaysOnTop, xvSyncToVblank, xvColorspace;
 
 static XvFormatRec formats[] = { {15}, {16}, {24} };
 static const XvImageRec images[] = { XVIMAGE_YUY2, XVIMAGE_UYVY, XVMC_RGB888, XVMC_RGB565 };
 static const XvAttributeRec attribs[] = {
+	{ XvSettable | XvGettable, 0, 1, (char *)"XV_COLORSPACE" }, /* BT.601, BT.709 */
 	{ XvSettable | XvGettable, 0, 0xffffff, (char *)"XV_COLORKEY" },
 	{ XvSettable | XvGettable, 0, 1, (char *)"XV_ALWAYS_ON_TOP" },
 };
@@ -117,6 +118,10 @@ static int sna_video_sprite_set_attr(ddSetPortAttribute_ARGS)
 		video->color_key = value;
 		RegionEmpty(&video->clip);
 		DBG(("COLORKEY = %ld\n", (long)value));
+	} else if (attribute == xvColorspace) {
+		video->colorspace_changed = ~0;
+		video->colorspace = value;
+		DBG(("COLORSPACE = %ld\n", (long)value));
 	} else if (attribute == xvSyncToVblank) {
 		DBG(("%s: SYNC_TO_VBLANK: %d -> %d\n", __FUNCTION__,
 		     video->SyncToVblank, !!value));
@@ -138,6 +143,8 @@ static int sna_video_sprite_get_attr(ddGetPortAttribute_ARGS)
 
 	if (attribute == xvColorKey)
 		*value = video->color_key;
+	else if (attribute == xvColorspace)
+		*value = video->colorspace;
 	else if (attribute == xvAlwaysOnTop)
 		*value = video->AlwaysOnTop;
 	else if (attribute == xvSyncToVblank)
@@ -263,6 +270,16 @@ sna_video_sprite_show(struct sna *sna,
 		video->color_key_changed &= ~(1 << pipe);
 	}
 
+	if (video->colorspace_changed & (1 << pipe)) {
+		DBG(("%s: updating colorspace: %x\n",
+		     __FUNCTION__, video->colorspace));
+
+		sna_crtc_set_sprite_colorspace(crtc, video->idx,
+					       video->colorspace);
+
+		video->colorspace_changed &= ~(1 << pipe);
+	}
+
 	update_dst_box_to_crtc_coords(sna, crtc, dstBox);
 	if (frame->rotation & (RR_Rotate_90 | RR_Rotate_270)) {
 		int tmp = frame->width;
@@ -767,6 +784,8 @@ void sna_video_sprite_setup(struct sna *sna, ScreenPtr screen)
 		video->alignment = 64;
 		video->color_key = sna_video_sprite_color_key(sna);
 		video->color_key_changed = ~0;
+		video->colorspace = 1; /* BT.709 */
+		video->colorspace_changed = ~0;
 		video->has_color_key = true;
 		video->brightness = -19;	/* (255/219) * -16 */
 		video->contrast = 75;	/* 255/219 * 64 */
@@ -787,6 +806,7 @@ void sna_video_sprite_setup(struct sna *sna, ScreenPtr screen)
 	adaptor->base_id = adaptor->pPorts[0].id;
 
 	xvColorKey = MAKE_ATOM("XV_COLORKEY");
+	xvColorspace = MAKE_ATOM("XV_COLORSPACE");
 	xvAlwaysOnTop = MAKE_ATOM("XV_ALWAYS_ON_TOP");
 	xvSyncToVblank = MAKE_ATOM("XV_SYNC_TO_VBLANK");
 
-- 
2.13.0

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