[PATCH xf86-video-intel 1/2] sna: Refactor property parsing

[PATCH xf86-video-intel 1/2] sna: Refactor property parsing

[Ville Syrjala]

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

Generalize the code that parses the plane properties to be useable
for crtc (or any kms object) properties as well.

Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 src/sna/sna_display.c | 67 +++++++++++++++++++++++++++++++------------
 1 file changed, 48 insertions(+), 19 deletions(-)

diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index fe67f85b62f1..916cc3c04e07 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -215,6 +215,7 @@ struct sna_crtc {
 	uint32_t rotation;
 	struct plane {
 		uint32_t id;
+		uint32_t type;
 		struct {
 			uint32_t prop;
 			uint32_t supported;
@@ -3391,33 +3392,40 @@ void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc,
 				 p->color_encoding.values[colorspace]);
 }
 
-static int plane_details(struct sna *sna, struct plane *p)
+typedef void (*parse_prop_func)(struct sna *sna,
+				struct drm_mode_get_property *prop,
+				uint64_t value,
+				void *data);
+static void parse_props(struct sna *sna,
+		       uint32_t obj_type, uint32_t obj_id,
+		       parse_prop_func parse_prop,
+		       void *data)
 {
 #define N_STACK_PROPS 32 /* must be a multiple of 2 */
 	struct local_mode_obj_get_properties arg;
 	uint64_t stack[N_STACK_PROPS + N_STACK_PROPS/2];
 	uint64_t *values = stack;
 	uint32_t *props = (uint32_t *)(values + N_STACK_PROPS);
-	int i, type = DRM_PLANE_TYPE_OVERLAY;
+	int i;
 
 	memset(&arg, 0, sizeof(struct local_mode_obj_get_properties));
-	arg.obj_id = p->id;
-	arg.obj_type = LOCAL_MODE_OBJECT_PLANE;
+	arg.obj_id = obj_id;
+	arg.obj_type = obj_type;
 
 	arg.props_ptr = (uintptr_t)props;
 	arg.prop_values_ptr = (uintptr_t)values;
 	arg.count_props = N_STACK_PROPS;
 
 	if (drmIoctl(sna->kgem.fd, LOCAL_IOCTL_MODE_OBJ_GETPROPERTIES, &arg))
-		return -1;
+		return;
 
 	DBG(("%s: object %d (type %x) has %d props\n", __FUNCTION__,
-	     p->id, LOCAL_MODE_OBJECT_PLANE, arg.count_props));
+	     obj_id, obj_type, arg.count_props));
 
 	if (arg.count_props > N_STACK_PROPS) {
 		values = malloc(2*sizeof(uint64_t)*arg.count_props);
 		if (values == NULL)
-			return -1;
+			return;
 
 		props = (uint32_t *)(values + arg.count_props);
 
@@ -3444,25 +3452,46 @@ static int plane_details(struct sna *sna, struct plane *p)
 		DBG(("%s: prop[%d] .id=%ld, .name=%s, .flags=%x, .value=%ld\n", __FUNCTION__, i,
 		     (long)props[i], prop.name, (unsigned)prop.flags, (long)values[i]));
 
-		if (strcmp(prop.name, "type") == 0) {
-			type = values[i];
-		} else if (prop_is_rotation(&prop)) {
-			parse_rotation_prop(sna, p, &prop, values[i]);
-		} else if (prop_is_color_encoding(&prop)) {
-			parse_color_encoding_prop(sna, p, &prop, values[i]);
-		}
+		parse_prop(sna, &prop, values[i], data);
 	}
 
+	if (values != stack)
+		free(values);
+
+#undef N_STACK_PROPS
+}
+
+static bool prop_is_type(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 1, "type");
+}
+
+static void plane_parse_prop(struct sna *sna,
+			     struct drm_mode_get_property *prop,
+			     uint64_t value, void *data)
+{
+	struct plane *p = data;
+
+	if (prop_is_type(prop))
+		p->type = value;
+	else if (prop_is_rotation(prop))
+		parse_rotation_prop(sna, p, prop, value);
+	else if (prop_is_color_encoding(prop))
+		parse_color_encoding_prop(sna, p, prop, value);
+}
+
+static int plane_details(struct sna *sna, struct plane *p)
+{
+	parse_props(sna, LOCAL_MODE_OBJECT_PLANE, p->id,
+		    plane_parse_prop, p);
+
 	p->rotation.supported &= DBG_NATIVE_ROTATION;
 	if (!xf86ReturnOptValBool(sna->Options, OPTION_ROTATION, TRUE))
 		p->rotation.supported = RR_Rotate_0;
 
-	if (values != stack)
-		free(values);
+	DBG(("%s: plane=%d type=%d\n", __FUNCTION__, p->id, p->type));
 
-	DBG(("%s: plane=%d type=%d\n", __FUNCTION__, p->id, type));
-	return type;
-#undef N_STACK_PROPS
+	return p->type;
 }
 
 static void add_sprite_plane(struct sna_crtc *crtc,
-- 
2.19.2

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

[PATCH xf86-video-intel 2/2] sna: Support 10bpc gamma via the GAMMA_LUT crtc property

[Ville Syrjala]

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

Probe the GAMMA_LUT/GAMMA_LUT_SIZE props and utilize them when
the running with > 8bpc.

Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 src/sna/sna_display.c | 247 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 208 insertions(+), 39 deletions(-)

diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index 916cc3c04e07..2f82e1fa3d86 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -127,6 +127,7 @@ struct local_mode_obj_get_properties {
 	uint32_t obj_type;
 	uint32_t pad;
 };
+#define LOCAL_MODE_OBJECT_CRTC 0xcccccccc
 #define LOCAL_MODE_OBJECT_PLANE 0xeeeeeeee
 
 struct local_mode_set_plane {
@@ -229,6 +230,11 @@ struct sna_crtc {
 	} primary;
 	struct list sprites;
 
+	struct drm_color_lut *gamma_lut;
+	uint64_t gamma_lut_prop;
+	uint64_t gamma_lut_blob;
+	uint32_t gamma_lut_size;
+
 	uint32_t mode_serial, flip_serial;
 
 	uint32_t last_seq, wrap_seq;
@@ -317,6 +323,9 @@ static void __sna_output_dpms(xf86OutputPtr output, int dpms, int fixup);
 static void sna_crtc_disable_cursor(struct sna *sna, struct sna_crtc *crtc);
 static bool sna_crtc_flip(struct sna *sna, struct sna_crtc *crtc,
 			  struct kgem_bo *bo, int x, int y);
+static void sna_crtc_gamma_set(xf86CrtcPtr crtc,
+			       CARD16 *red, CARD16 *green,
+			       CARD16 *blue, int size);
 
 static bool is_zaphod(ScrnInfoPtr scrn)
 {
@@ -3150,11 +3159,9 @@ sna_crtc_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	       mode->VDisplay <= sna->mode.max_crtc_height);
 
 #if HAS_GAMMA
-	drmModeCrtcSetGamma(sna->kgem.fd, __sna_crtc_id(sna_crtc),
-			    crtc->gamma_size,
-			    crtc->gamma_red,
-			    crtc->gamma_green,
-			    crtc->gamma_blue);
+	sna_crtc_gamma_set(crtc,
+			   crtc->gamma_red, crtc->gamma_green,
+			   crtc->gamma_blue, crtc->gamma_size);
 #endif
 
 	saved_kmode = sna_crtc->kmode;
@@ -3212,12 +3219,44 @@ void sna_mode_adjust_frame(struct sna *sna, int x, int y)
 
 static void
 sna_crtc_gamma_set(xf86CrtcPtr crtc,
-		       CARD16 *red, CARD16 *green, CARD16 *blue, int size)
+		   CARD16 *red, CARD16 *green, CARD16 *blue, int size)
 {
-	assert(to_sna_crtc(crtc));
-	drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
-			    sna_crtc_id(crtc),
-			    size, red, green, blue);
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
+	struct drm_color_lut *lut = sna_crtc->gamma_lut;
+	uint32_t blob_size = size * sizeof(lut[0]);
+	uint32_t blob_id;
+	int ret, i;
+
+	DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
+
+	if (!lut) {
+		assert(size == 256);
+
+		drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
+				    sna_crtc_id(crtc),
+				    size, red, green, blue);
+		return;
+	}
+
+	assert(size == sna_crtc->gamma_lut_size);
+
+	for (i = 0; i < size; i++) {
+		lut[i].red = red[i];
+		lut[i].green = green[i];
+		lut[i].blue = blue[i];
+	}
+
+	ret = drmModeCreatePropertyBlob(sna->kgem.fd, lut, blob_size, &blob_id);
+	if (ret)
+		return;
+
+	ret = drmModeObjectSetProperty(sna->kgem.fd,
+				       sna_crtc->id, DRM_MODE_OBJECT_CRTC,
+				       sna_crtc->gamma_lut_prop,
+				       blob_id);
+
+	drmModeDestroyPropertyBlob(sna->kgem.fd, blob_id);
 }
 
 static void
@@ -3229,6 +3268,8 @@ sna_crtc_destroy(xf86CrtcPtr crtc)
 	if (sna_crtc == NULL)
 		return;
 
+	free(sna_crtc->gamma_lut);
+
 	list_for_each_entry_safe(sprite, sn, &sna_crtc->sprites, link)
 		free(sprite);
 
@@ -3663,6 +3704,55 @@ bool sna_has_sprite_format(struct sna *sna, uint32_t format)
 	return false;
 }
 
+inline static bool prop_is_gamma_lut(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 4, "GAMMA_LUT");
+}
+
+inline static bool prop_is_gamma_lut_size(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 1, "GAMMA_LUT_SIZE");
+}
+
+static void sna_crtc_parse_prop(struct sna *sna,
+				struct drm_mode_get_property *prop,
+				uint64_t value, void *data)
+{
+	struct sna_crtc *crtc = data;
+
+	if (prop_is_gamma_lut(prop)) {
+		crtc->gamma_lut_prop = prop->prop_id;
+		crtc->gamma_lut_blob = value;
+	} else if (prop_is_gamma_lut_size(prop)) {
+		crtc->gamma_lut_size = value;
+	}
+}
+
+static void
+sna_crtc_init__props(struct sna *sna, struct sna_crtc *crtc)
+{
+	ScrnInfoPtr scrn = sna->scrn;
+
+	parse_props(sna, LOCAL_MODE_OBJECT_CRTC, crtc->id,
+		    sna_crtc_parse_prop, crtc);
+
+	/* use high precision gamma LUT for > 8bpc */
+	/* X-Server < 1.20 mishandles > 256 slots / > 8 bpc color maps. */
+	if (scrn->rgbBits <= 8 ||
+	    XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(1,20,0,0,0))
+		crtc->gamma_lut_size = 0;
+
+	if (crtc->gamma_lut_size) {
+		crtc->gamma_lut = calloc(max(crtc->gamma_lut_size, 256),
+					 sizeof(crtc->gamma_lut[0]));
+		if (!crtc->gamma_lut)
+			crtc->gamma_lut_size = 0;
+	}
+
+	DBG(("%s: CRTC:%d, gamma_lut_size=%d\n", __FUNCTION__,
+	     sna_crtc_id(crtc), crtc->gamma_lut_size));
+}
+
 static void
 sna_crtc_init__rotation(struct sna *sna, struct sna_crtc *crtc)
 {
@@ -3723,6 +3813,9 @@ sna_crtc_add(ScrnInfoPtr scrn, unsigned id)
 
 	list_init(&sna_crtc->sprites);
 	sna_crtc_init__rotation(sna, sna_crtc);
+
+	sna_crtc_init__props(sna, sna_crtc);
+
 	sna_crtc_find_planes(sna, sna_crtc);
 
 	DBG(("%s: CRTC:%d [pipe=%d], primary id=%x: supported-rotations=%x, current-rotation=%x\n",
@@ -7282,36 +7375,110 @@ static void output_set_gamma(xf86OutputPtr output, xf86CrtcPtr crtc)
 			  mon->mon_gamma_blue);
 }
 
+static bool
+crtc_get_gamma_lut(xf86CrtcPtr crtc,
+		   CARD16 *red, CARD16 *green, CARD16 *blue, int size)
+{
+
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
+	struct drm_color_lut *lut = sna_crtc->gamma_lut;
+	struct drm_mode_get_blob blob;
+	int lut_size, i;
+
+	DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
+
+	memset(&blob, 0, sizeof(blob));
+	blob.blob_id = sna_crtc->gamma_lut_blob;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
+		return false;
+
+	lut_size = blob.length / sizeof(lut[0]);
+
+	if (lut_size == 0 ||
+	    lut_size > max(sna_crtc->gamma_lut_size, 256))
+		return false;
+
+	blob.data = (uintptr_t)lut;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
+		return false;
+
+	for (i = 0; i < size; i++) {
+		struct drm_color_lut *entry =
+			&lut[i * (lut_size - 1) / (size - 1)];
+
+		red[i] = entry->red;
+		green[i] = entry->green;
+		blue[i] = entry->blue;
+	}
+
+	return red[size - 1] &&
+		green[size - 1] &&
+		blue[size - 1];
+}
+
+static bool crtc_get_gamma_legacy(xf86CrtcPtr crtc,
+				  CARD16 *red,
+				  CARD16 *green,
+				  CARD16 *blue,
+				  int size)
+{
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
+	struct drm_mode_crtc_lut lut = {
+		.crtc_id = __sna_crtc_id(sna_crtc),
+		.gamma_size = size,
+		.red = (uintptr_t)red,
+		.green = (uintptr_t)green,
+		.blue = (uintptr_t)blue,
+	};
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) != 0)
+		return false;
+
+	VG(VALGRIND_MAKE_MEM_DEFINED(red, size*sizeof(red[0])));
+	VG(VALGRIND_MAKE_MEM_DEFINED(green, size*sizeof(green[0])));
+	VG(VALGRIND_MAKE_MEM_DEFINED(bluered, size*sizeof(blue[0])));
+
+	return red[size - 1] &&
+		green[size - 1] &&
+		blue[size - 1];
+}
+
 static void crtc_init_gamma(xf86CrtcPtr crtc)
 {
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
 	uint16_t *gamma;
+	int size;
+
+	assert(sna_crtc);
+
+	size = sna_crtc->gamma_lut_size;
+	if (!size)
+		size = 256;
 
 	/* Initialize the gamma ramps */
 	gamma = NULL;
-	if (crtc->gamma_size == 256)
+	if (crtc->gamma_size == size)
 		gamma = crtc->gamma_red;
 	if (gamma == NULL)
-		gamma = malloc(3 * 256 * sizeof(uint16_t));
+		gamma = malloc(3 * size * sizeof(uint16_t));
 	if (gamma) {
-		struct sna *sna = to_sna(crtc->scrn);
-		struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
 		struct drm_mode_crtc_lut lut;
-		bool gamma_set = false;
-
-		assert(sna_crtc);
-
-		lut.crtc_id = __sna_crtc_id(sna_crtc);
-		lut.gamma_size = 256;
-		lut.red = (uintptr_t)(gamma);
-		lut.green = (uintptr_t)(gamma + 256);
-		lut.blue = (uintptr_t)(gamma + 2 * 256);
-		if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) == 0) {
-			VG(VALGRIND_MAKE_MEM_DEFINED(gamma, 3*256*sizeof(gamma[0])));
-			gamma_set =
-				gamma[256 - 1] &&
-				gamma[2*256 - 1] &&
-				gamma[3*256 - 1];
-		}
+		bool gamma_set;
+		uint16_t *red = gamma;
+		uint16_t *green = gamma + size;
+		uint16_t *blue = gamma + 2 * size;
+
+		if (sna_crtc->gamma_lut_size)
+			gamma_set = crtc_get_gamma_lut(crtc, red,
+						       green, blue, size);
+		else
+			gamma_set = crtc_get_gamma_legacy(crtc, red,
+							  green, blue, size);
 
 		DBG(("%s: CRTC:%d, pipe=%d: gamma set?=%d\n",
 		     __FUNCTION__, __sna_crtc_id(sna_crtc), __sna_crtc_pipe(sna_crtc),
@@ -7319,19 +7486,21 @@ static void crtc_init_gamma(xf86CrtcPtr crtc)
 		if (!gamma_set) {
 			int i;
 
-			for (i = 0; i < 256; i++) {
-				gamma[i] = i << 8;
-				gamma[256 + i] = i << 8;
-				gamma[2*256 + i] = i << 8;
+			for (i = 0; i < size; i++) {
+				uint16_t val = i * 0xffff / (size - 1);
+
+				red[i] = val;
+				green[i] = val;
+				blue[i] = val;
 			}
 		}
 
-		if (gamma != crtc->gamma_red) {
+		if (red != crtc->gamma_red) {
 			free(crtc->gamma_red);
-			crtc->gamma_red = gamma;
-			crtc->gamma_green = gamma + 256;
-			crtc->gamma_blue = gamma + 2*256;
-			crtc->gamma_size = 256;
+			crtc->gamma_red = red;
+			crtc->gamma_green = green;
+			crtc->gamma_blue = blue;
+			crtc->gamma_size = size;
 		}
 	}
 }
-- 
2.19.2

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

Re: [PATCH xf86-video-intel 2/2] sna: Support 10bpc gamma via the GAMMA_LUT crtc property

[Chris Wilson]

Quoting Ville Syrjala (2019-02-18 19:50:52)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Probe the GAMMA_LUT/GAMMA_LUT_SIZE props and utilize them when
> the running with > 8bpc.
> 
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Looks fine to me, so I'll push unless any one objects.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH xf86-video-intel v2 2/2] sna: Support 10bpc gamma via the GAMMA_LUT crtc property

[Ville Syrjala]

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

Probe the GAMMA_LUT/GAMMA_LUT_SIZE props and utilize them when
the running with > 8bpc.

v2: s/sna_crtc_id/__sna_crtc_id/ in DBG since we have a sna_crtc

Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 src/sna/sna_display.c | 245 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 207 insertions(+), 38 deletions(-)

diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index 41edfec12839..6d671dce8c14 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -127,6 +127,7 @@ struct local_mode_obj_get_properties {
 	uint32_t obj_type;
 	uint32_t pad;
 };
+#define LOCAL_MODE_OBJECT_CRTC 0xcccccccc
 #define LOCAL_MODE_OBJECT_PLANE 0xeeeeeeee
 
 struct local_mode_set_plane {
@@ -229,6 +230,11 @@ struct sna_crtc {
 	} primary;
 	struct list sprites;
 
+	struct drm_color_lut *gamma_lut;
+	uint64_t gamma_lut_prop;
+	uint64_t gamma_lut_blob;
+	uint32_t gamma_lut_size;
+
 	uint32_t mode_serial, flip_serial;
 
 	uint32_t last_seq, wrap_seq;
@@ -317,6 +323,9 @@ static void __sna_output_dpms(xf86OutputPtr output, int dpms, int fixup);
 static void sna_crtc_disable_cursor(struct sna *sna, struct sna_crtc *crtc);
 static bool sna_crtc_flip(struct sna *sna, struct sna_crtc *crtc,
 			  struct kgem_bo *bo, int x, int y);
+static void sna_crtc_gamma_set(xf86CrtcPtr crtc,
+			       CARD16 *red, CARD16 *green,
+			       CARD16 *blue, int size);
 
 static bool is_zaphod(ScrnInfoPtr scrn)
 {
@@ -3150,11 +3159,9 @@ sna_crtc_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	       mode->VDisplay <= sna->mode.max_crtc_height);
 
 #if HAS_GAMMA
-	drmModeCrtcSetGamma(sna->kgem.fd, __sna_crtc_id(sna_crtc),
-			    crtc->gamma_size,
-			    crtc->gamma_red,
-			    crtc->gamma_green,
-			    crtc->gamma_blue);
+	sna_crtc_gamma_set(crtc,
+			   crtc->gamma_red, crtc->gamma_green,
+			   crtc->gamma_blue, crtc->gamma_size);
 #endif
 
 	saved_kmode = sna_crtc->kmode;
@@ -3212,12 +3219,44 @@ void sna_mode_adjust_frame(struct sna *sna, int x, int y)
 
 static void
 sna_crtc_gamma_set(xf86CrtcPtr crtc,
-		       CARD16 *red, CARD16 *green, CARD16 *blue, int size)
+		   CARD16 *red, CARD16 *green, CARD16 *blue, int size)
 {
-	assert(to_sna_crtc(crtc));
-	drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
-			    sna_crtc_id(crtc),
-			    size, red, green, blue);
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
+	struct drm_color_lut *lut = sna_crtc->gamma_lut;
+	uint32_t blob_size = size * sizeof(lut[0]);
+	uint32_t blob_id;
+	int ret, i;
+
+	DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
+
+	if (!lut) {
+		assert(size == 256);
+
+		drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
+				    sna_crtc_id(crtc),
+				    size, red, green, blue);
+		return;
+	}
+
+	assert(size == sna_crtc->gamma_lut_size);
+
+	for (i = 0; i < size; i++) {
+		lut[i].red = red[i];
+		lut[i].green = green[i];
+		lut[i].blue = blue[i];
+	}
+
+	ret = drmModeCreatePropertyBlob(sna->kgem.fd, lut, blob_size, &blob_id);
+	if (ret)
+		return;
+
+	ret = drmModeObjectSetProperty(sna->kgem.fd,
+				       sna_crtc->id, DRM_MODE_OBJECT_CRTC,
+				       sna_crtc->gamma_lut_prop,
+				       blob_id);
+
+	drmModeDestroyPropertyBlob(sna->kgem.fd, blob_id);
 }
 
 static void
@@ -3229,6 +3268,8 @@ sna_crtc_destroy(xf86CrtcPtr crtc)
 	if (sna_crtc == NULL)
 		return;
 
+	free(sna_crtc->gamma_lut);
+
 	list_for_each_entry_safe(sprite, sn, &sna_crtc->sprites, link)
 		free(sprite);
 
@@ -3663,6 +3704,55 @@ bool sna_has_sprite_format(struct sna *sna, uint32_t format)
 	return false;
 }
 
+inline static bool prop_is_gamma_lut(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 4, "GAMMA_LUT");
+}
+
+inline static bool prop_is_gamma_lut_size(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 1, "GAMMA_LUT_SIZE");
+}
+
+static void sna_crtc_parse_prop(struct sna *sna,
+				struct drm_mode_get_property *prop,
+				uint64_t value, void *data)
+{
+	struct sna_crtc *crtc = data;
+
+	if (prop_is_gamma_lut(prop)) {
+		crtc->gamma_lut_prop = prop->prop_id;
+		crtc->gamma_lut_blob = value;
+	} else if (prop_is_gamma_lut_size(prop)) {
+		crtc->gamma_lut_size = value;
+	}
+}
+
+static void
+sna_crtc_init__props(struct sna *sna, struct sna_crtc *crtc)
+{
+	ScrnInfoPtr scrn = sna->scrn;
+
+	parse_props(sna, LOCAL_MODE_OBJECT_CRTC, crtc->id,
+		    sna_crtc_parse_prop, crtc);
+
+	/* use high precision gamma LUT for > 8bpc */
+	/* X-Server < 1.20 mishandles > 256 slots / > 8 bpc color maps. */
+	if (scrn->rgbBits <= 8 ||
+	    XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(1,20,0,0,0))
+		crtc->gamma_lut_size = 0;
+
+	if (crtc->gamma_lut_size) {
+		crtc->gamma_lut = calloc(max(crtc->gamma_lut_size, 256),
+					 sizeof(crtc->gamma_lut[0]));
+		if (!crtc->gamma_lut)
+			crtc->gamma_lut_size = 0;
+	}
+
+	DBG(("%s: CRTC:%d, gamma_lut_size=%d\n", __FUNCTION__,
+	     __sna_crtc_id(crtc), crtc->gamma_lut_size));
+}
+
 static void
 sna_crtc_init__rotation(struct sna *sna, struct sna_crtc *crtc)
 {
@@ -3723,6 +3813,9 @@ sna_crtc_add(ScrnInfoPtr scrn, unsigned id)
 
 	list_init(&sna_crtc->sprites);
 	sna_crtc_init__rotation(sna, sna_crtc);
+
+	sna_crtc_init__props(sna, sna_crtc);
+
 	sna_crtc_find_planes(sna, sna_crtc);
 
 	DBG(("%s: CRTC:%d [pipe=%d], primary id=%x: supported-rotations=%x, current-rotation=%x\n",
@@ -7274,36 +7367,110 @@ static void output_set_gamma(xf86OutputPtr output, xf86CrtcPtr crtc)
 			  mon->mon_gamma_blue);
 }
 
+static bool
+crtc_get_gamma_lut(xf86CrtcPtr crtc,
+		   CARD16 *red, CARD16 *green, CARD16 *blue, int size)
+{
+
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
+	struct drm_color_lut *lut = sna_crtc->gamma_lut;
+	struct drm_mode_get_blob blob;
+	int lut_size, i;
+
+	DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
+
+	memset(&blob, 0, sizeof(blob));
+	blob.blob_id = sna_crtc->gamma_lut_blob;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
+		return false;
+
+	lut_size = blob.length / sizeof(lut[0]);
+
+	if (lut_size == 0 ||
+	    lut_size > max(sna_crtc->gamma_lut_size, 256))
+		return false;
+
+	blob.data = (uintptr_t)lut;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
+		return false;
+
+	for (i = 0; i < size; i++) {
+		struct drm_color_lut *entry =
+			&lut[i * (lut_size - 1) / (size - 1)];
+
+		red[i] = entry->red;
+		green[i] = entry->green;
+		blue[i] = entry->blue;
+	}
+
+	return red[size - 1] &&
+		green[size - 1] &&
+		blue[size - 1];
+}
+
+static bool crtc_get_gamma_legacy(xf86CrtcPtr crtc,
+				  CARD16 *red,
+				  CARD16 *green,
+				  CARD16 *blue,
+				  int size)
+{
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
+	struct drm_mode_crtc_lut lut = {
+		.crtc_id = __sna_crtc_id(sna_crtc),
+		.gamma_size = size,
+		.red = (uintptr_t)red,
+		.green = (uintptr_t)green,
+		.blue = (uintptr_t)blue,
+	};
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) != 0)
+		return false;
+
+	VG(VALGRIND_MAKE_MEM_DEFINED(red, size*sizeof(red[0])));
+	VG(VALGRIND_MAKE_MEM_DEFINED(green, size*sizeof(green[0])));
+	VG(VALGRIND_MAKE_MEM_DEFINED(bluered, size*sizeof(blue[0])));
+
+	return red[size - 1] &&
+		green[size - 1] &&
+		blue[size - 1];
+}
+
 static void crtc_init_gamma(xf86CrtcPtr crtc)
 {
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
 	uint16_t *gamma;
+	int size;
+
+	assert(sna_crtc);
+
+	size = sna_crtc->gamma_lut_size;
+	if (!size)
+		size = 256;
 
 	/* Initialize the gamma ramps */
 	gamma = NULL;
-	if (crtc->gamma_size == 256)
+	if (crtc->gamma_size == size)
 		gamma = crtc->gamma_red;
 	if (gamma == NULL)
-		gamma = malloc(3 * 256 * sizeof(uint16_t));
+		gamma = malloc(3 * size * sizeof(uint16_t));
 	if (gamma) {
-		struct sna *sna = to_sna(crtc->scrn);
-		struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
 		struct drm_mode_crtc_lut lut;
-		bool gamma_set = false;
+		bool gamma_set;
+		uint16_t *red = gamma;
+		uint16_t *green = gamma + size;
+		uint16_t *blue = gamma + 2 * size;
 
-		assert(sna_crtc);
-
-		lut.crtc_id = __sna_crtc_id(sna_crtc);
-		lut.gamma_size = 256;
-		lut.red = (uintptr_t)(gamma);
-		lut.green = (uintptr_t)(gamma + 256);
-		lut.blue = (uintptr_t)(gamma + 2 * 256);
-		if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) == 0) {
-			VG(VALGRIND_MAKE_MEM_DEFINED(gamma, 3*256*sizeof(gamma[0])));
-			gamma_set =
-				gamma[256 - 1] &&
-				gamma[2*256 - 1] &&
-				gamma[3*256 - 1];
-		}
+		if (sna_crtc->gamma_lut_size)
+			gamma_set = crtc_get_gamma_lut(crtc, red,
+						       green, blue, size);
+		else
+			gamma_set = crtc_get_gamma_legacy(crtc, red,
+							  green, blue, size);
 
 		DBG(("%s: CRTC:%d, pipe=%d: gamma set?=%d\n",
 		     __FUNCTION__, __sna_crtc_id(sna_crtc), __sna_crtc_pipe(sna_crtc),
@@ -7311,19 +7478,21 @@ static void crtc_init_gamma(xf86CrtcPtr crtc)
 		if (!gamma_set) {
 			int i;
 
-			for (i = 0; i < 256; i++) {
-				gamma[i] = i << 8;
-				gamma[256 + i] = i << 8;
-				gamma[2*256 + i] = i << 8;
+			for (i = 0; i < size; i++) {
+				uint16_t val = i * 0xffff / (size - 1);
+
+				red[i] = val;
+				green[i] = val;
+				blue[i] = val;
 			}
 		}
 
-		if (gamma != crtc->gamma_red) {
+		if (red != crtc->gamma_red) {
 			free(crtc->gamma_red);
-			crtc->gamma_red = gamma;
-			crtc->gamma_green = gamma + 256;
-			crtc->gamma_blue = gamma + 2*256;
-			crtc->gamma_size = 256;
+			crtc->gamma_red = red;
+			crtc->gamma_green = green;
+			crtc->gamma_blue = blue;
+			crtc->gamma_size = size;
 		}
 	}
 }
-- 
2.21.0

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

Re: [PATCH xf86-video-intel v2 2/2] sna: Support 10bpc gamma via the GAMMA_LUT crtc property

[Mario Kleiner]

On Fri, Apr 26, 2019 at 6:32 PM Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Probe the GAMMA_LUT/GAMMA_LUT_SIZE props and utilize them when
> the running with > 8bpc.
>
> v2: s/sna_crtc_id/__sna_crtc_id/ in DBG since we have a sna_crtc
>
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  src/sna/sna_display.c | 245 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 207 insertions(+), 38 deletions(-)
>
> diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
> index 41edfec12839..6d671dce8c14 100644
> --- a/src/sna/sna_display.c
> +++ b/src/sna/sna_display.c
> @@ -127,6 +127,7 @@ struct local_mode_obj_get_properties {
>         uint32_t obj_type;
>         uint32_t pad;
>  };
> +#define LOCAL_MODE_OBJECT_CRTC 0xcccccccc
>  #define LOCAL_MODE_OBJECT_PLANE 0xeeeeeeee
>
>  struct local_mode_set_plane {
> @@ -229,6 +230,11 @@ struct sna_crtc {
>         } primary;
>         struct list sprites;
>
> +       struct drm_color_lut *gamma_lut;
> +       uint64_t gamma_lut_prop;
> +       uint64_t gamma_lut_blob;
> +       uint32_t gamma_lut_size;
> +
>         uint32_t mode_serial, flip_serial;
>
>         uint32_t last_seq, wrap_seq;
> @@ -317,6 +323,9 @@ static void __sna_output_dpms(xf86OutputPtr output, int dpms, int fixup);
>  static void sna_crtc_disable_cursor(struct sna *sna, struct sna_crtc *crtc);
>  static bool sna_crtc_flip(struct sna *sna, struct sna_crtc *crtc,
>                           struct kgem_bo *bo, int x, int y);
> +static void sna_crtc_gamma_set(xf86CrtcPtr crtc,
> +                              CARD16 *red, CARD16 *green,
> +                              CARD16 *blue, int size);
>
>  static bool is_zaphod(ScrnInfoPtr scrn)
>  {
> @@ -3150,11 +3159,9 @@ sna_crtc_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
>                mode->VDisplay <= sna->mode.max_crtc_height);
>
>  #if HAS_GAMMA
> -       drmModeCrtcSetGamma(sna->kgem.fd, __sna_crtc_id(sna_crtc),
> -                           crtc->gamma_size,
> -                           crtc->gamma_red,
> -                           crtc->gamma_green,
> -                           crtc->gamma_blue);
> +       sna_crtc_gamma_set(crtc,
> +                          crtc->gamma_red, crtc->gamma_green,
> +                          crtc->gamma_blue, crtc->gamma_size);
>  #endif
>
>         saved_kmode = sna_crtc->kmode;
> @@ -3212,12 +3219,44 @@ void sna_mode_adjust_frame(struct sna *sna, int x, int y)
>
>  static void
>  sna_crtc_gamma_set(xf86CrtcPtr crtc,
> -                      CARD16 *red, CARD16 *green, CARD16 *blue, int size)
> +                  CARD16 *red, CARD16 *green, CARD16 *blue, int size)
>  {
> -       assert(to_sna_crtc(crtc));
> -       drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
> -                           sna_crtc_id(crtc),
> -                           size, red, green, blue);
> +       struct sna *sna = to_sna(crtc->scrn);
> +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
> +       struct drm_color_lut *lut = sna_crtc->gamma_lut;
> +       uint32_t blob_size = size * sizeof(lut[0]);
> +       uint32_t blob_id;
> +       int ret, i;
> +
> +       DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
> +
> +       if (!lut) {
> +               assert(size == 256);
> +
> +               drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
> +                                   sna_crtc_id(crtc),
> +                                   size, red, green, blue);
> +               return;
> +       }
> +
> +       assert(size == sna_crtc->gamma_lut_size);
> +
> +       for (i = 0; i < size; i++) {
> +               lut[i].red = red[i];
> +               lut[i].green = green[i];
> +               lut[i].blue = blue[i];
> +       }
> +
> +       ret = drmModeCreatePropertyBlob(sna->kgem.fd, lut, blob_size, &blob_id);
> +       if (ret)
> +               return;
> +
> +       ret = drmModeObjectSetProperty(sna->kgem.fd,
> +                                      sna_crtc->id, DRM_MODE_OBJECT_CRTC,
> +                                      sna_crtc->gamma_lut_prop,
> +                                      blob_id);
> +
> +       drmModeDestroyPropertyBlob(sna->kgem.fd, blob_id);
>  }
>
>  static void
> @@ -3229,6 +3268,8 @@ sna_crtc_destroy(xf86CrtcPtr crtc)
>         if (sna_crtc == NULL)
>                 return;
>
> +       free(sna_crtc->gamma_lut);
> +
>         list_for_each_entry_safe(sprite, sn, &sna_crtc->sprites, link)
>                 free(sprite);
>
> @@ -3663,6 +3704,55 @@ bool sna_has_sprite_format(struct sna *sna, uint32_t format)
>         return false;
>  }
>
> +inline static bool prop_is_gamma_lut(const struct drm_mode_get_property *prop)
> +{
> +       return prop_has_type_and_name(prop, 4, "GAMMA_LUT");
> +}
> +
> +inline static bool prop_is_gamma_lut_size(const struct drm_mode_get_property *prop)
> +{
> +       return prop_has_type_and_name(prop, 1, "GAMMA_LUT_SIZE");
> +}
> +
> +static void sna_crtc_parse_prop(struct sna *sna,
> +                               struct drm_mode_get_property *prop,
> +                               uint64_t value, void *data)
> +{
> +       struct sna_crtc *crtc = data;
> +
> +       if (prop_is_gamma_lut(prop)) {
> +               crtc->gamma_lut_prop = prop->prop_id;
> +               crtc->gamma_lut_blob = value;
> +       } else if (prop_is_gamma_lut_size(prop)) {
> +               crtc->gamma_lut_size = value;
> +       }
> +}
> +
> +static void
> +sna_crtc_init__props(struct sna *sna, struct sna_crtc *crtc)
> +{
> +       ScrnInfoPtr scrn = sna->scrn;
> +
> +       parse_props(sna, LOCAL_MODE_OBJECT_CRTC, crtc->id,
> +                   sna_crtc_parse_prop, crtc);
> +
> +       /* use high precision gamma LUT for > 8bpc */
> +       /* X-Server < 1.20 mishandles > 256 slots / > 8 bpc color maps. */
> +       if (scrn->rgbBits <= 8 ||
> +           XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(1,20,0,0,0))
> +               crtc->gamma_lut_size = 0;
> +

One suggestion: Might make sense to print some xf86DrivMsg() info or
warning message
to the startup output of the driver [sna_driver.c - sna_pre_init()]
iff one tries to select a
X-Screen depth >= 30 aka scrn->rgbBits > 8 on a pre-1.20.0 server, so
users get some
hint that they need a 1.20+ server to actually get any extra precision
out of a depth 30
screen beyond the 8 bpc of the legacy gamma lut's?

Of course this could also be done in a followup up patch, not to stop
this from getting merged.

> +       if (crtc->gamma_lut_size) {
> +               crtc->gamma_lut = calloc(max(crtc->gamma_lut_size, 256),
> +                                        sizeof(crtc->gamma_lut[0]));
> +               if (!crtc->gamma_lut)
> +                       crtc->gamma_lut_size = 0;
> +       }
> +
> +       DBG(("%s: CRTC:%d, gamma_lut_size=%d\n", __FUNCTION__,
> +            __sna_crtc_id(crtc), crtc->gamma_lut_size));
> +}
> +
>  static void
>  sna_crtc_init__rotation(struct sna *sna, struct sna_crtc *crtc)
>  {
> @@ -3723,6 +3813,9 @@ sna_crtc_add(ScrnInfoPtr scrn, unsigned id)
>
>         list_init(&sna_crtc->sprites);
>         sna_crtc_init__rotation(sna, sna_crtc);
> +
> +       sna_crtc_init__props(sna, sna_crtc);
> +
>         sna_crtc_find_planes(sna, sna_crtc);
>
>         DBG(("%s: CRTC:%d [pipe=%d], primary id=%x: supported-rotations=%x, current-rotation=%x\n",
> @@ -7274,36 +7367,110 @@ static void output_set_gamma(xf86OutputPtr output, xf86CrtcPtr crtc)
>                           mon->mon_gamma_blue);
>  }
>
> +static bool
> +crtc_get_gamma_lut(xf86CrtcPtr crtc,
> +                  CARD16 *red, CARD16 *green, CARD16 *blue, int size)
> +{
> +
> +       struct sna *sna = to_sna(crtc->scrn);
> +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
> +       struct drm_color_lut *lut = sna_crtc->gamma_lut;
> +       struct drm_mode_get_blob blob;
> +       int lut_size, i;
> +
> +       DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
> +
> +       memset(&blob, 0, sizeof(blob));
> +       blob.blob_id = sna_crtc->gamma_lut_blob;
> +
> +       if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
> +               return false;
> +
> +       lut_size = blob.length / sizeof(lut[0]);
> +
> +       if (lut_size == 0 ||
> +           lut_size > max(sna_crtc->gamma_lut_size, 256))
> +               return false;
> +
> +       blob.data = (uintptr_t)lut;
> +
> +       if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
> +               return false;
> +
> +       for (i = 0; i < size; i++) {
> +               struct drm_color_lut *entry =
> +                       &lut[i * (lut_size - 1) / (size - 1)];
> +
> +               red[i] = entry->red;
> +               green[i] = entry->green;
> +               blue[i] = entry->blue;
> +       }
> +
> +       return red[size - 1] &&
> +               green[size - 1] &&
> +               blue[size - 1];
> +}
> +
> +static bool crtc_get_gamma_legacy(xf86CrtcPtr crtc,
> +                                 CARD16 *red,
> +                                 CARD16 *green,
> +                                 CARD16 *blue,
> +                                 int size)
> +{
> +       struct sna *sna = to_sna(crtc->scrn);
> +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
> +       struct drm_mode_crtc_lut lut = {
> +               .crtc_id = __sna_crtc_id(sna_crtc),
> +               .gamma_size = size,
> +               .red = (uintptr_t)red,
> +               .green = (uintptr_t)green,
> +               .blue = (uintptr_t)blue,
> +       };
> +
> +       if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) != 0)
> +               return false;
> +
> +       VG(VALGRIND_MAKE_MEM_DEFINED(red, size*sizeof(red[0])));
> +       VG(VALGRIND_MAKE_MEM_DEFINED(green, size*sizeof(green[0])));
> +       VG(VALGRIND_MAKE_MEM_DEFINED(bluered, size*sizeof(blue[0])));

Typo in last VALGRIND_MAKE... macro: bluered -> blue

Other than that this looks fine and testing on an Ironlake and
Baytrail gpu under X-Server 1.20 with depth 24 and depth 30 confirms
it working.

With that fix, this patch is

Reviewed-and-tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>

Thanks a lot for adding the 10 bpc gamma lut support to the driver and
also i915-kms for Linux 5.2!
-mario

> +
> +       return red[size - 1] &&
> +               green[size - 1] &&
> +               blue[size - 1];
> +}
> +
>  static void crtc_init_gamma(xf86CrtcPtr crtc)
>  {
> +       struct sna *sna = to_sna(crtc->scrn);
> +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
>         uint16_t *gamma;
> +       int size;
> +
> +       assert(sna_crtc);
> +
> +       size = sna_crtc->gamma_lut_size;
> +       if (!size)
> +               size = 256;
>
>         /* Initialize the gamma ramps */
>         gamma = NULL;
> -       if (crtc->gamma_size == 256)
> +       if (crtc->gamma_size == size)
>                 gamma = crtc->gamma_red;
>         if (gamma == NULL)
> -               gamma = malloc(3 * 256 * sizeof(uint16_t));
> +               gamma = malloc(3 * size * sizeof(uint16_t));
>         if (gamma) {
> -               struct sna *sna = to_sna(crtc->scrn);
> -               struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
>                 struct drm_mode_crtc_lut lut;
> -               bool gamma_set = false;
> +               bool gamma_set;
> +               uint16_t *red = gamma;
> +               uint16_t *green = gamma + size;
> +               uint16_t *blue = gamma + 2 * size;
>
> -               assert(sna_crtc);
> -
> -               lut.crtc_id = __sna_crtc_id(sna_crtc);
> -               lut.gamma_size = 256;
> -               lut.red = (uintptr_t)(gamma);
> -               lut.green = (uintptr_t)(gamma + 256);
> -               lut.blue = (uintptr_t)(gamma + 2 * 256);
> -               if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) == 0) {
> -                       VG(VALGRIND_MAKE_MEM_DEFINED(gamma, 3*256*sizeof(gamma[0])));
> -                       gamma_set =
> -                               gamma[256 - 1] &&
> -                               gamma[2*256 - 1] &&
> -                               gamma[3*256 - 1];
> -               }
> +               if (sna_crtc->gamma_lut_size)
> +                       gamma_set = crtc_get_gamma_lut(crtc, red,
> +                                                      green, blue, size);
> +               else
> +                       gamma_set = crtc_get_gamma_legacy(crtc, red,
> +                                                         green, blue, size);
>
>                 DBG(("%s: CRTC:%d, pipe=%d: gamma set?=%d\n",
>                      __FUNCTION__, __sna_crtc_id(sna_crtc), __sna_crtc_pipe(sna_crtc),
> @@ -7311,19 +7478,21 @@ static void crtc_init_gamma(xf86CrtcPtr crtc)
>                 if (!gamma_set) {
>                         int i;
>
> -                       for (i = 0; i < 256; i++) {
> -                               gamma[i] = i << 8;
> -                               gamma[256 + i] = i << 8;
> -                               gamma[2*256 + i] = i << 8;
> +                       for (i = 0; i < size; i++) {
> +                               uint16_t val = i * 0xffff / (size - 1);
> +
> +                               red[i] = val;
> +                               green[i] = val;
> +                               blue[i] = val;
>                         }
>                 }
>
> -               if (gamma != crtc->gamma_red) {
> +               if (red != crtc->gamma_red) {
>                         free(crtc->gamma_red);
> -                       crtc->gamma_red = gamma;
> -                       crtc->gamma_green = gamma + 256;
> -                       crtc->gamma_blue = gamma + 2*256;
> -                       crtc->gamma_size = 256;
> +                       crtc->gamma_red = red;
> +                       crtc->gamma_green = green;
> +                       crtc->gamma_blue = blue;
> +                       crtc->gamma_size = size;
>                 }
>         }
>  }
> --
> 2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH xf86-video-intel v2 2/2] sna: Support 10bpc gamma via the GAMMA_LUT crtc property

[Ville Syrjälä]

On Thu, May 16, 2019 at 09:54:55PM +0200, Mario Kleiner wrote:
> On Fri, Apr 26, 2019 at 6:32 PM Ville Syrjala
> <ville.syrjala@linux.intel.com> wrote:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Probe the GAMMA_LUT/GAMMA_LUT_SIZE props and utilize them when
> > the running with > 8bpc.
> >
> > v2: s/sna_crtc_id/__sna_crtc_id/ in DBG since we have a sna_crtc
> >
> > Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  src/sna/sna_display.c | 245 +++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 207 insertions(+), 38 deletions(-)
> >
> > diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
> > index 41edfec12839..6d671dce8c14 100644
> > --- a/src/sna/sna_display.c
> > +++ b/src/sna/sna_display.c
> > @@ -127,6 +127,7 @@ struct local_mode_obj_get_properties {
> >         uint32_t obj_type;
> >         uint32_t pad;
> >  };
> > +#define LOCAL_MODE_OBJECT_CRTC 0xcccccccc
> >  #define LOCAL_MODE_OBJECT_PLANE 0xeeeeeeee
> >
> >  struct local_mode_set_plane {
> > @@ -229,6 +230,11 @@ struct sna_crtc {
> >         } primary;
> >         struct list sprites;
> >
> > +       struct drm_color_lut *gamma_lut;
> > +       uint64_t gamma_lut_prop;
> > +       uint64_t gamma_lut_blob;
> > +       uint32_t gamma_lut_size;
> > +
> >         uint32_t mode_serial, flip_serial;
> >
> >         uint32_t last_seq, wrap_seq;
> > @@ -317,6 +323,9 @@ static void __sna_output_dpms(xf86OutputPtr output, int dpms, int fixup);
> >  static void sna_crtc_disable_cursor(struct sna *sna, struct sna_crtc *crtc);
> >  static bool sna_crtc_flip(struct sna *sna, struct sna_crtc *crtc,
> >                           struct kgem_bo *bo, int x, int y);
> > +static void sna_crtc_gamma_set(xf86CrtcPtr crtc,
> > +                              CARD16 *red, CARD16 *green,
> > +                              CARD16 *blue, int size);
> >
> >  static bool is_zaphod(ScrnInfoPtr scrn)
> >  {
> > @@ -3150,11 +3159,9 @@ sna_crtc_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
> >                mode->VDisplay <= sna->mode.max_crtc_height);
> >
> >  #if HAS_GAMMA
> > -       drmModeCrtcSetGamma(sna->kgem.fd, __sna_crtc_id(sna_crtc),
> > -                           crtc->gamma_size,
> > -                           crtc->gamma_red,
> > -                           crtc->gamma_green,
> > -                           crtc->gamma_blue);
> > +       sna_crtc_gamma_set(crtc,
> > +                          crtc->gamma_red, crtc->gamma_green,
> > +                          crtc->gamma_blue, crtc->gamma_size);
> >  #endif
> >
> >         saved_kmode = sna_crtc->kmode;
> > @@ -3212,12 +3219,44 @@ void sna_mode_adjust_frame(struct sna *sna, int x, int y)
> >
> >  static void
> >  sna_crtc_gamma_set(xf86CrtcPtr crtc,
> > -                      CARD16 *red, CARD16 *green, CARD16 *blue, int size)
> > +                  CARD16 *red, CARD16 *green, CARD16 *blue, int size)
> >  {
> > -       assert(to_sna_crtc(crtc));
> > -       drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
> > -                           sna_crtc_id(crtc),
> > -                           size, red, green, blue);
> > +       struct sna *sna = to_sna(crtc->scrn);
> > +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
> > +       struct drm_color_lut *lut = sna_crtc->gamma_lut;
> > +       uint32_t blob_size = size * sizeof(lut[0]);
> > +       uint32_t blob_id;
> > +       int ret, i;
> > +
> > +       DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
> > +
> > +       if (!lut) {
> > +               assert(size == 256);
> > +
> > +               drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
> > +                                   sna_crtc_id(crtc),
> > +                                   size, red, green, blue);
> > +               return;
> > +       }
> > +
> > +       assert(size == sna_crtc->gamma_lut_size);
> > +
> > +       for (i = 0; i < size; i++) {
> > +               lut[i].red = red[i];
> > +               lut[i].green = green[i];
> > +               lut[i].blue = blue[i];
> > +       }
> > +
> > +       ret = drmModeCreatePropertyBlob(sna->kgem.fd, lut, blob_size, &blob_id);
> > +       if (ret)
> > +               return;
> > +
> > +       ret = drmModeObjectSetProperty(sna->kgem.fd,
> > +                                      sna_crtc->id, DRM_MODE_OBJECT_CRTC,
> > +                                      sna_crtc->gamma_lut_prop,
> > +                                      blob_id);
> > +
> > +       drmModeDestroyPropertyBlob(sna->kgem.fd, blob_id);
> >  }
> >
> >  static void
> > @@ -3229,6 +3268,8 @@ sna_crtc_destroy(xf86CrtcPtr crtc)
> >         if (sna_crtc == NULL)
> >                 return;
> >
> > +       free(sna_crtc->gamma_lut);
> > +
> >         list_for_each_entry_safe(sprite, sn, &sna_crtc->sprites, link)
> >                 free(sprite);
> >
> > @@ -3663,6 +3704,55 @@ bool sna_has_sprite_format(struct sna *sna, uint32_t format)
> >         return false;
> >  }
> >
> > +inline static bool prop_is_gamma_lut(const struct drm_mode_get_property *prop)
> > +{
> > +       return prop_has_type_and_name(prop, 4, "GAMMA_LUT");
> > +}
> > +
> > +inline static bool prop_is_gamma_lut_size(const struct drm_mode_get_property *prop)
> > +{
> > +       return prop_has_type_and_name(prop, 1, "GAMMA_LUT_SIZE");
> > +}
> > +
> > +static void sna_crtc_parse_prop(struct sna *sna,
> > +                               struct drm_mode_get_property *prop,
> > +                               uint64_t value, void *data)
> > +{
> > +       struct sna_crtc *crtc = data;
> > +
> > +       if (prop_is_gamma_lut(prop)) {
> > +               crtc->gamma_lut_prop = prop->prop_id;
> > +               crtc->gamma_lut_blob = value;
> > +       } else if (prop_is_gamma_lut_size(prop)) {
> > +               crtc->gamma_lut_size = value;
> > +       }
> > +}
> > +
> > +static void
> > +sna_crtc_init__props(struct sna *sna, struct sna_crtc *crtc)
> > +{
> > +       ScrnInfoPtr scrn = sna->scrn;
> > +
> > +       parse_props(sna, LOCAL_MODE_OBJECT_CRTC, crtc->id,
> > +                   sna_crtc_parse_prop, crtc);
> > +
> > +       /* use high precision gamma LUT for > 8bpc */
> > +       /* X-Server < 1.20 mishandles > 256 slots / > 8 bpc color maps. */
> > +       if (scrn->rgbBits <= 8 ||
> > +           XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(1,20,0,0,0))
> > +               crtc->gamma_lut_size = 0;
> > +
> 
> One suggestion: Might make sense to print some xf86DrivMsg() info or
> warning message
> to the startup output of the driver [sna_driver.c - sna_pre_init()]
> iff one tries to select a
> X-Screen depth >= 30 aka scrn->rgbBits > 8 on a pre-1.20.0 server, so
> users get some
> hint that they need a 1.20+ server to actually get any extra precision
> out of a depth 30
> screen beyond the 8 bpc of the legacy gamma lut's?

Perhaps. Not sure how many people still on older xservers
would update their ddx though.

> 
> Of course this could also be done in a followup up patch, not to stop
> this from getting merged.

I supoose I can post a followup.

> 
> > +       if (crtc->gamma_lut_size) {
> > +               crtc->gamma_lut = calloc(max(crtc->gamma_lut_size, 256),
> > +                                        sizeof(crtc->gamma_lut[0]));
> > +               if (!crtc->gamma_lut)
> > +                       crtc->gamma_lut_size = 0;
> > +       }
> > +
> > +       DBG(("%s: CRTC:%d, gamma_lut_size=%d\n", __FUNCTION__,
> > +            __sna_crtc_id(crtc), crtc->gamma_lut_size));
> > +}
> > +
> >  static void
> >  sna_crtc_init__rotation(struct sna *sna, struct sna_crtc *crtc)
> >  {
> > @@ -3723,6 +3813,9 @@ sna_crtc_add(ScrnInfoPtr scrn, unsigned id)
> >
> >         list_init(&sna_crtc->sprites);
> >         sna_crtc_init__rotation(sna, sna_crtc);
> > +
> > +       sna_crtc_init__props(sna, sna_crtc);
> > +
> >         sna_crtc_find_planes(sna, sna_crtc);
> >
> >         DBG(("%s: CRTC:%d [pipe=%d], primary id=%x: supported-rotations=%x, current-rotation=%x\n",
> > @@ -7274,36 +7367,110 @@ static void output_set_gamma(xf86OutputPtr output, xf86CrtcPtr crtc)
> >                           mon->mon_gamma_blue);
> >  }
> >
> > +static bool
> > +crtc_get_gamma_lut(xf86CrtcPtr crtc,
> > +                  CARD16 *red, CARD16 *green, CARD16 *blue, int size)
> > +{
> > +
> > +       struct sna *sna = to_sna(crtc->scrn);
> > +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
> > +       struct drm_color_lut *lut = sna_crtc->gamma_lut;
> > +       struct drm_mode_get_blob blob;
> > +       int lut_size, i;
> > +
> > +       DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
> > +
> > +       memset(&blob, 0, sizeof(blob));
> > +       blob.blob_id = sna_crtc->gamma_lut_blob;
> > +
> > +       if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
> > +               return false;
> > +
> > +       lut_size = blob.length / sizeof(lut[0]);
> > +
> > +       if (lut_size == 0 ||
> > +           lut_size > max(sna_crtc->gamma_lut_size, 256))
> > +               return false;
> > +
> > +       blob.data = (uintptr_t)lut;
> > +
> > +       if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
> > +               return false;
> > +
> > +       for (i = 0; i < size; i++) {
> > +               struct drm_color_lut *entry =
> > +                       &lut[i * (lut_size - 1) / (size - 1)];
> > +
> > +               red[i] = entry->red;
> > +               green[i] = entry->green;
> > +               blue[i] = entry->blue;
> > +       }
> > +
> > +       return red[size - 1] &&
> > +               green[size - 1] &&
> > +               blue[size - 1];
> > +}
> > +
> > +static bool crtc_get_gamma_legacy(xf86CrtcPtr crtc,
> > +                                 CARD16 *red,
> > +                                 CARD16 *green,
> > +                                 CARD16 *blue,
> > +                                 int size)
> > +{
> > +       struct sna *sna = to_sna(crtc->scrn);
> > +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
> > +       struct drm_mode_crtc_lut lut = {
> > +               .crtc_id = __sna_crtc_id(sna_crtc),
> > +               .gamma_size = size,
> > +               .red = (uintptr_t)red,
> > +               .green = (uintptr_t)green,
> > +               .blue = (uintptr_t)blue,
> > +       };
> > +
> > +       if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) != 0)
> > +               return false;
> > +
> > +       VG(VALGRIND_MAKE_MEM_DEFINED(red, size*sizeof(red[0])));
> > +       VG(VALGRIND_MAKE_MEM_DEFINED(green, size*sizeof(green[0])));
> > +       VG(VALGRIND_MAKE_MEM_DEFINED(bluered, size*sizeof(blue[0])));
> 
> Typo in last VALGRIND_MAKE... macro: bluered -> blue

Doh. I guess I didn't actually build this with valgrind enabled.

> 
> Other than that this looks fine and testing on an Ironlake and
> Baytrail gpu under X-Server 1.20 with depth 24 and depth 30 confirms
> it working.

Cool. Thanks for the review and testing.

> 
> With that fix, this patch is
> 
> Reviewed-and-tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> 
> Thanks a lot for adding the 10 bpc gamma lut support to the driver and
> also i915-kms for Linux 5.2!
> -mario
> 
> > +
> > +       return red[size - 1] &&
> > +               green[size - 1] &&
> > +               blue[size - 1];
> > +}
> > +
> >  static void crtc_init_gamma(xf86CrtcPtr crtc)
> >  {
> > +       struct sna *sna = to_sna(crtc->scrn);
> > +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
> >         uint16_t *gamma;
> > +       int size;
> > +
> > +       assert(sna_crtc);
> > +
> > +       size = sna_crtc->gamma_lut_size;
> > +       if (!size)
> > +               size = 256;
> >
> >         /* Initialize the gamma ramps */
> >         gamma = NULL;
> > -       if (crtc->gamma_size == 256)
> > +       if (crtc->gamma_size == size)
> >                 gamma = crtc->gamma_red;
> >         if (gamma == NULL)
> > -               gamma = malloc(3 * 256 * sizeof(uint16_t));
> > +               gamma = malloc(3 * size * sizeof(uint16_t));
> >         if (gamma) {
> > -               struct sna *sna = to_sna(crtc->scrn);
> > -               struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
> >                 struct drm_mode_crtc_lut lut;
> > -               bool gamma_set = false;
> > +               bool gamma_set;
> > +               uint16_t *red = gamma;
> > +               uint16_t *green = gamma + size;
> > +               uint16_t *blue = gamma + 2 * size;
> >
> > -               assert(sna_crtc);
> > -
> > -               lut.crtc_id = __sna_crtc_id(sna_crtc);
> > -               lut.gamma_size = 256;
> > -               lut.red = (uintptr_t)(gamma);
> > -               lut.green = (uintptr_t)(gamma + 256);
> > -               lut.blue = (uintptr_t)(gamma + 2 * 256);
> > -               if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) == 0) {
> > -                       VG(VALGRIND_MAKE_MEM_DEFINED(gamma, 3*256*sizeof(gamma[0])));
> > -                       gamma_set =
> > -                               gamma[256 - 1] &&
> > -                               gamma[2*256 - 1] &&
> > -                               gamma[3*256 - 1];
> > -               }
> > +               if (sna_crtc->gamma_lut_size)
> > +                       gamma_set = crtc_get_gamma_lut(crtc, red,
> > +                                                      green, blue, size);
> > +               else
> > +                       gamma_set = crtc_get_gamma_legacy(crtc, red,
> > +                                                         green, blue, size);
> >
> >                 DBG(("%s: CRTC:%d, pipe=%d: gamma set?=%d\n",
> >                      __FUNCTION__, __sna_crtc_id(sna_crtc), __sna_crtc_pipe(sna_crtc),
> > @@ -7311,19 +7478,21 @@ static void crtc_init_gamma(xf86CrtcPtr crtc)
> >                 if (!gamma_set) {
> >                         int i;
> >
> > -                       for (i = 0; i < 256; i++) {
> > -                               gamma[i] = i << 8;
> > -                               gamma[256 + i] = i << 8;
> > -                               gamma[2*256 + i] = i << 8;
> > +                       for (i = 0; i < size; i++) {
> > +                               uint16_t val = i * 0xffff / (size - 1);
> > +
> > +                               red[i] = val;
> > +                               green[i] = val;
> > +                               blue[i] = val;
> >                         }
> >                 }
> >
> > -               if (gamma != crtc->gamma_red) {
> > +               if (red != crtc->gamma_red) {
> >                         free(crtc->gamma_red);
> > -                       crtc->gamma_red = gamma;
> > -                       crtc->gamma_green = gamma + 256;
> > -                       crtc->gamma_blue = gamma + 2*256;
> > -                       crtc->gamma_size = 256;
> > +                       crtc->gamma_red = red;
> > +                       crtc->gamma_green = green;
> > +                       crtc->gamma_blue = blue;
> > +                       crtc->gamma_size = size;
> >                 }
> >         }
> >  }
> > --
> > 2.21.0
> >

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

[PATCH xf86-video-intel v3 2/2] sna: Support 10bpc gamma via the GAMMA_LUT crtc property

[Ville Syrjala]

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

Probe the GAMMA_LUT/GAMMA_LUT_SIZE props and utilize them when
the running with > 8bpc.

v2: s/sna_crtc_id/__sna_crtc_id/ in DBG since we have a sna_crtc
v3: Fix the vg "bluered" typo (Mario)
    This time I even build tested with vg support

Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-and-tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 src/sna/sna_display.c | 247 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 208 insertions(+), 39 deletions(-)

diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index 41edfec12839..d6210cc7bbc8 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -127,6 +127,7 @@ struct local_mode_obj_get_properties {
 	uint32_t obj_type;
 	uint32_t pad;
 };
+#define LOCAL_MODE_OBJECT_CRTC 0xcccccccc
 #define LOCAL_MODE_OBJECT_PLANE 0xeeeeeeee
 
 struct local_mode_set_plane {
@@ -229,6 +230,11 @@ struct sna_crtc {
 	} primary;
 	struct list sprites;
 
+	struct drm_color_lut *gamma_lut;
+	uint64_t gamma_lut_prop;
+	uint64_t gamma_lut_blob;
+	uint32_t gamma_lut_size;
+
 	uint32_t mode_serial, flip_serial;
 
 	uint32_t last_seq, wrap_seq;
@@ -317,6 +323,9 @@ static void __sna_output_dpms(xf86OutputPtr output, int dpms, int fixup);
 static void sna_crtc_disable_cursor(struct sna *sna, struct sna_crtc *crtc);
 static bool sna_crtc_flip(struct sna *sna, struct sna_crtc *crtc,
 			  struct kgem_bo *bo, int x, int y);
+static void sna_crtc_gamma_set(xf86CrtcPtr crtc,
+			       CARD16 *red, CARD16 *green,
+			       CARD16 *blue, int size);
 
 static bool is_zaphod(ScrnInfoPtr scrn)
 {
@@ -3150,11 +3159,9 @@ sna_crtc_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 	       mode->VDisplay <= sna->mode.max_crtc_height);
 
 #if HAS_GAMMA
-	drmModeCrtcSetGamma(sna->kgem.fd, __sna_crtc_id(sna_crtc),
-			    crtc->gamma_size,
-			    crtc->gamma_red,
-			    crtc->gamma_green,
-			    crtc->gamma_blue);
+	sna_crtc_gamma_set(crtc,
+			   crtc->gamma_red, crtc->gamma_green,
+			   crtc->gamma_blue, crtc->gamma_size);
 #endif
 
 	saved_kmode = sna_crtc->kmode;
@@ -3212,12 +3219,44 @@ void sna_mode_adjust_frame(struct sna *sna, int x, int y)
 
 static void
 sna_crtc_gamma_set(xf86CrtcPtr crtc,
-		       CARD16 *red, CARD16 *green, CARD16 *blue, int size)
+		   CARD16 *red, CARD16 *green, CARD16 *blue, int size)
 {
-	assert(to_sna_crtc(crtc));
-	drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
-			    sna_crtc_id(crtc),
-			    size, red, green, blue);
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
+	struct drm_color_lut *lut = sna_crtc->gamma_lut;
+	uint32_t blob_size = size * sizeof(lut[0]);
+	uint32_t blob_id;
+	int ret, i;
+
+	DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
+
+	if (!lut) {
+		assert(size == 256);
+
+		drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
+				    sna_crtc_id(crtc),
+				    size, red, green, blue);
+		return;
+	}
+
+	assert(size == sna_crtc->gamma_lut_size);
+
+	for (i = 0; i < size; i++) {
+		lut[i].red = red[i];
+		lut[i].green = green[i];
+		lut[i].blue = blue[i];
+	}
+
+	ret = drmModeCreatePropertyBlob(sna->kgem.fd, lut, blob_size, &blob_id);
+	if (ret)
+		return;
+
+	ret = drmModeObjectSetProperty(sna->kgem.fd,
+				       sna_crtc->id, DRM_MODE_OBJECT_CRTC,
+				       sna_crtc->gamma_lut_prop,
+				       blob_id);
+
+	drmModeDestroyPropertyBlob(sna->kgem.fd, blob_id);
 }
 
 static void
@@ -3229,6 +3268,8 @@ sna_crtc_destroy(xf86CrtcPtr crtc)
 	if (sna_crtc == NULL)
 		return;
 
+	free(sna_crtc->gamma_lut);
+
 	list_for_each_entry_safe(sprite, sn, &sna_crtc->sprites, link)
 		free(sprite);
 
@@ -3663,6 +3704,55 @@ bool sna_has_sprite_format(struct sna *sna, uint32_t format)
 	return false;
 }
 
+inline static bool prop_is_gamma_lut(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 4, "GAMMA_LUT");
+}
+
+inline static bool prop_is_gamma_lut_size(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 1, "GAMMA_LUT_SIZE");
+}
+
+static void sna_crtc_parse_prop(struct sna *sna,
+				struct drm_mode_get_property *prop,
+				uint64_t value, void *data)
+{
+	struct sna_crtc *crtc = data;
+
+	if (prop_is_gamma_lut(prop)) {
+		crtc->gamma_lut_prop = prop->prop_id;
+		crtc->gamma_lut_blob = value;
+	} else if (prop_is_gamma_lut_size(prop)) {
+		crtc->gamma_lut_size = value;
+	}
+}
+
+static void
+sna_crtc_init__props(struct sna *sna, struct sna_crtc *crtc)
+{
+	ScrnInfoPtr scrn = sna->scrn;
+
+	parse_props(sna, LOCAL_MODE_OBJECT_CRTC, crtc->id,
+		    sna_crtc_parse_prop, crtc);
+
+	/* use high precision gamma LUT for > 8bpc */
+	/* X-Server < 1.20 mishandles > 256 slots / > 8 bpc color maps. */
+	if (scrn->rgbBits <= 8 ||
+	    XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(1,20,0,0,0))
+		crtc->gamma_lut_size = 0;
+
+	if (crtc->gamma_lut_size) {
+		crtc->gamma_lut = calloc(max(crtc->gamma_lut_size, 256),
+					 sizeof(crtc->gamma_lut[0]));
+		if (!crtc->gamma_lut)
+			crtc->gamma_lut_size = 0;
+	}
+
+	DBG(("%s: CRTC:%d, gamma_lut_size=%d\n", __FUNCTION__,
+	     __sna_crtc_id(crtc), crtc->gamma_lut_size));
+}
+
 static void
 sna_crtc_init__rotation(struct sna *sna, struct sna_crtc *crtc)
 {
@@ -3723,6 +3813,9 @@ sna_crtc_add(ScrnInfoPtr scrn, unsigned id)
 
 	list_init(&sna_crtc->sprites);
 	sna_crtc_init__rotation(sna, sna_crtc);
+
+	sna_crtc_init__props(sna, sna_crtc);
+
 	sna_crtc_find_planes(sna, sna_crtc);
 
 	DBG(("%s: CRTC:%d [pipe=%d], primary id=%x: supported-rotations=%x, current-rotation=%x\n",
@@ -7274,36 +7367,110 @@ static void output_set_gamma(xf86OutputPtr output, xf86CrtcPtr crtc)
 			  mon->mon_gamma_blue);
 }
 
+static bool
+crtc_get_gamma_lut(xf86CrtcPtr crtc,
+		   CARD16 *red, CARD16 *green, CARD16 *blue, int size)
+{
+
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
+	struct drm_color_lut *lut = sna_crtc->gamma_lut;
+	struct drm_mode_get_blob blob;
+	int lut_size, i;
+
+	DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
+
+	memset(&blob, 0, sizeof(blob));
+	blob.blob_id = sna_crtc->gamma_lut_blob;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
+		return false;
+
+	lut_size = blob.length / sizeof(lut[0]);
+
+	if (lut_size == 0 ||
+	    lut_size > max(sna_crtc->gamma_lut_size, 256))
+		return false;
+
+	blob.data = (uintptr_t)lut;
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
+		return false;
+
+	for (i = 0; i < size; i++) {
+		struct drm_color_lut *entry =
+			&lut[i * (lut_size - 1) / (size - 1)];
+
+		red[i] = entry->red;
+		green[i] = entry->green;
+		blue[i] = entry->blue;
+	}
+
+	return red[size - 1] &&
+		green[size - 1] &&
+		blue[size - 1];
+}
+
+static bool crtc_get_gamma_legacy(xf86CrtcPtr crtc,
+				  CARD16 *red,
+				  CARD16 *green,
+				  CARD16 *blue,
+				  int size)
+{
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
+	struct drm_mode_crtc_lut lut = {
+		.crtc_id = __sna_crtc_id(sna_crtc),
+		.gamma_size = size,
+		.red = (uintptr_t)red,
+		.green = (uintptr_t)green,
+		.blue = (uintptr_t)blue,
+	};
+
+	if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) != 0)
+		return false;
+
+	VG(VALGRIND_MAKE_MEM_DEFINED(red, size*sizeof(red[0])));
+	VG(VALGRIND_MAKE_MEM_DEFINED(green, size*sizeof(green[0])));
+	VG(VALGRIND_MAKE_MEM_DEFINED(blue, size*sizeof(blue[0])));
+
+	return red[size - 1] &&
+		green[size - 1] &&
+		blue[size - 1];
+}
+
 static void crtc_init_gamma(xf86CrtcPtr crtc)
 {
+	struct sna *sna = to_sna(crtc->scrn);
+	struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
 	uint16_t *gamma;
+	int size;
+
+	assert(sna_crtc);
+
+	size = sna_crtc->gamma_lut_size;
+	if (!size)
+		size = 256;
 
 	/* Initialize the gamma ramps */
 	gamma = NULL;
-	if (crtc->gamma_size == 256)
+	if (crtc->gamma_size == size)
 		gamma = crtc->gamma_red;
 	if (gamma == NULL)
-		gamma = malloc(3 * 256 * sizeof(uint16_t));
+		gamma = malloc(3 * size * sizeof(uint16_t));
 	if (gamma) {
-		struct sna *sna = to_sna(crtc->scrn);
-		struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
 		struct drm_mode_crtc_lut lut;
-		bool gamma_set = false;
-
-		assert(sna_crtc);
-
-		lut.crtc_id = __sna_crtc_id(sna_crtc);
-		lut.gamma_size = 256;
-		lut.red = (uintptr_t)(gamma);
-		lut.green = (uintptr_t)(gamma + 256);
-		lut.blue = (uintptr_t)(gamma + 2 * 256);
-		if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) == 0) {
-			VG(VALGRIND_MAKE_MEM_DEFINED(gamma, 3*256*sizeof(gamma[0])));
-			gamma_set =
-				gamma[256 - 1] &&
-				gamma[2*256 - 1] &&
-				gamma[3*256 - 1];
-		}
+		bool gamma_set;
+		uint16_t *red = gamma;
+		uint16_t *green = gamma + size;
+		uint16_t *blue = gamma + 2 * size;
+
+		if (sna_crtc->gamma_lut_size)
+			gamma_set = crtc_get_gamma_lut(crtc, red,
+						       green, blue, size);
+		else
+			gamma_set = crtc_get_gamma_legacy(crtc, red,
+							  green, blue, size);
 
 		DBG(("%s: CRTC:%d, pipe=%d: gamma set?=%d\n",
 		     __FUNCTION__, __sna_crtc_id(sna_crtc), __sna_crtc_pipe(sna_crtc),
@@ -7311,19 +7478,21 @@ static void crtc_init_gamma(xf86CrtcPtr crtc)
 		if (!gamma_set) {
 			int i;
 
-			for (i = 0; i < 256; i++) {
-				gamma[i] = i << 8;
-				gamma[256 + i] = i << 8;
-				gamma[2*256 + i] = i << 8;
+			for (i = 0; i < size; i++) {
+				uint16_t val = i * 0xffff / (size - 1);
+
+				red[i] = val;
+				green[i] = val;
+				blue[i] = val;
 			}
 		}
 
-		if (gamma != crtc->gamma_red) {
+		if (red != crtc->gamma_red) {
 			free(crtc->gamma_red);
-			crtc->gamma_red = gamma;
-			crtc->gamma_green = gamma + 256;
-			crtc->gamma_blue = gamma + 2*256;
-			crtc->gamma_size = 256;
+			crtc->gamma_red = red;
+			crtc->gamma_green = green;
+			crtc->gamma_blue = blue;
+			crtc->gamma_size = size;
 		}
 	}
 }
-- 
2.21.0

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

Re: [PATCH xf86-video-intel v3 2/2] sna: Support 10bpc gamma via the GAMMA_LUT crtc property

[Mario Kleiner]

Hi Ville,

now somebody just needs to merge these two 10 bit gamma lut patches
into intel-ddx?

thanks,
-mario

On Fri, May 17, 2019 at 3:51 PM Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Probe the GAMMA_LUT/GAMMA_LUT_SIZE props and utilize them when
> the running with > 8bpc.
>
> v2: s/sna_crtc_id/__sna_crtc_id/ in DBG since we have a sna_crtc
> v3: Fix the vg "bluered" typo (Mario)
>     This time I even build tested with vg support
>
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-and-tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> ---
>  src/sna/sna_display.c | 247 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 208 insertions(+), 39 deletions(-)
>
> diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
> index 41edfec12839..d6210cc7bbc8 100644
> --- a/src/sna/sna_display.c
> +++ b/src/sna/sna_display.c
> @@ -127,6 +127,7 @@ struct local_mode_obj_get_properties {
>         uint32_t obj_type;
>         uint32_t pad;
>  };
> +#define LOCAL_MODE_OBJECT_CRTC 0xcccccccc
>  #define LOCAL_MODE_OBJECT_PLANE 0xeeeeeeee
>
>  struct local_mode_set_plane {
> @@ -229,6 +230,11 @@ struct sna_crtc {
>         } primary;
>         struct list sprites;
>
> +       struct drm_color_lut *gamma_lut;
> +       uint64_t gamma_lut_prop;
> +       uint64_t gamma_lut_blob;
> +       uint32_t gamma_lut_size;
> +
>         uint32_t mode_serial, flip_serial;
>
>         uint32_t last_seq, wrap_seq;
> @@ -317,6 +323,9 @@ static void __sna_output_dpms(xf86OutputPtr output, int dpms, int fixup);
>  static void sna_crtc_disable_cursor(struct sna *sna, struct sna_crtc *crtc);
>  static bool sna_crtc_flip(struct sna *sna, struct sna_crtc *crtc,
>                           struct kgem_bo *bo, int x, int y);
> +static void sna_crtc_gamma_set(xf86CrtcPtr crtc,
> +                              CARD16 *red, CARD16 *green,
> +                              CARD16 *blue, int size);
>
>  static bool is_zaphod(ScrnInfoPtr scrn)
>  {
> @@ -3150,11 +3159,9 @@ sna_crtc_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
>                mode->VDisplay <= sna->mode.max_crtc_height);
>
>  #if HAS_GAMMA
> -       drmModeCrtcSetGamma(sna->kgem.fd, __sna_crtc_id(sna_crtc),
> -                           crtc->gamma_size,
> -                           crtc->gamma_red,
> -                           crtc->gamma_green,
> -                           crtc->gamma_blue);
> +       sna_crtc_gamma_set(crtc,
> +                          crtc->gamma_red, crtc->gamma_green,
> +                          crtc->gamma_blue, crtc->gamma_size);
>  #endif
>
>         saved_kmode = sna_crtc->kmode;
> @@ -3212,12 +3219,44 @@ void sna_mode_adjust_frame(struct sna *sna, int x, int y)
>
>  static void
>  sna_crtc_gamma_set(xf86CrtcPtr crtc,
> -                      CARD16 *red, CARD16 *green, CARD16 *blue, int size)
> +                  CARD16 *red, CARD16 *green, CARD16 *blue, int size)
>  {
> -       assert(to_sna_crtc(crtc));
> -       drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
> -                           sna_crtc_id(crtc),
> -                           size, red, green, blue);
> +       struct sna *sna = to_sna(crtc->scrn);
> +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
> +       struct drm_color_lut *lut = sna_crtc->gamma_lut;
> +       uint32_t blob_size = size * sizeof(lut[0]);
> +       uint32_t blob_id;
> +       int ret, i;
> +
> +       DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
> +
> +       if (!lut) {
> +               assert(size == 256);
> +
> +               drmModeCrtcSetGamma(to_sna(crtc->scrn)->kgem.fd,
> +                                   sna_crtc_id(crtc),
> +                                   size, red, green, blue);
> +               return;
> +       }
> +
> +       assert(size == sna_crtc->gamma_lut_size);
> +
> +       for (i = 0; i < size; i++) {
> +               lut[i].red = red[i];
> +               lut[i].green = green[i];
> +               lut[i].blue = blue[i];
> +       }
> +
> +       ret = drmModeCreatePropertyBlob(sna->kgem.fd, lut, blob_size, &blob_id);
> +       if (ret)
> +               return;
> +
> +       ret = drmModeObjectSetProperty(sna->kgem.fd,
> +                                      sna_crtc->id, DRM_MODE_OBJECT_CRTC,
> +                                      sna_crtc->gamma_lut_prop,
> +                                      blob_id);
> +
> +       drmModeDestroyPropertyBlob(sna->kgem.fd, blob_id);
>  }
>
>  static void
> @@ -3229,6 +3268,8 @@ sna_crtc_destroy(xf86CrtcPtr crtc)
>         if (sna_crtc == NULL)
>                 return;
>
> +       free(sna_crtc->gamma_lut);
> +
>         list_for_each_entry_safe(sprite, sn, &sna_crtc->sprites, link)
>                 free(sprite);
>
> @@ -3663,6 +3704,55 @@ bool sna_has_sprite_format(struct sna *sna, uint32_t format)
>         return false;
>  }
>
> +inline static bool prop_is_gamma_lut(const struct drm_mode_get_property *prop)
> +{
> +       return prop_has_type_and_name(prop, 4, "GAMMA_LUT");
> +}
> +
> +inline static bool prop_is_gamma_lut_size(const struct drm_mode_get_property *prop)
> +{
> +       return prop_has_type_and_name(prop, 1, "GAMMA_LUT_SIZE");
> +}
> +
> +static void sna_crtc_parse_prop(struct sna *sna,
> +                               struct drm_mode_get_property *prop,
> +                               uint64_t value, void *data)
> +{
> +       struct sna_crtc *crtc = data;
> +
> +       if (prop_is_gamma_lut(prop)) {
> +               crtc->gamma_lut_prop = prop->prop_id;
> +               crtc->gamma_lut_blob = value;
> +       } else if (prop_is_gamma_lut_size(prop)) {
> +               crtc->gamma_lut_size = value;
> +       }
> +}
> +
> +static void
> +sna_crtc_init__props(struct sna *sna, struct sna_crtc *crtc)
> +{
> +       ScrnInfoPtr scrn = sna->scrn;
> +
> +       parse_props(sna, LOCAL_MODE_OBJECT_CRTC, crtc->id,
> +                   sna_crtc_parse_prop, crtc);
> +
> +       /* use high precision gamma LUT for > 8bpc */
> +       /* X-Server < 1.20 mishandles > 256 slots / > 8 bpc color maps. */
> +       if (scrn->rgbBits <= 8 ||
> +           XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(1,20,0,0,0))
> +               crtc->gamma_lut_size = 0;
> +
> +       if (crtc->gamma_lut_size) {
> +               crtc->gamma_lut = calloc(max(crtc->gamma_lut_size, 256),
> +                                        sizeof(crtc->gamma_lut[0]));
> +               if (!crtc->gamma_lut)
> +                       crtc->gamma_lut_size = 0;
> +       }
> +
> +       DBG(("%s: CRTC:%d, gamma_lut_size=%d\n", __FUNCTION__,
> +            __sna_crtc_id(crtc), crtc->gamma_lut_size));
> +}
> +
>  static void
>  sna_crtc_init__rotation(struct sna *sna, struct sna_crtc *crtc)
>  {
> @@ -3723,6 +3813,9 @@ sna_crtc_add(ScrnInfoPtr scrn, unsigned id)
>
>         list_init(&sna_crtc->sprites);
>         sna_crtc_init__rotation(sna, sna_crtc);
> +
> +       sna_crtc_init__props(sna, sna_crtc);
> +
>         sna_crtc_find_planes(sna, sna_crtc);
>
>         DBG(("%s: CRTC:%d [pipe=%d], primary id=%x: supported-rotations=%x, current-rotation=%x\n",
> @@ -7274,36 +7367,110 @@ static void output_set_gamma(xf86OutputPtr output, xf86CrtcPtr crtc)
>                           mon->mon_gamma_blue);
>  }
>
> +static bool
> +crtc_get_gamma_lut(xf86CrtcPtr crtc,
> +                  CARD16 *red, CARD16 *green, CARD16 *blue, int size)
> +{
> +
> +       struct sna *sna = to_sna(crtc->scrn);
> +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
> +       struct drm_color_lut *lut = sna_crtc->gamma_lut;
> +       struct drm_mode_get_blob blob;
> +       int lut_size, i;
> +
> +       DBG(("%s: gamma_size %d\n", __FUNCTION__, size));
> +
> +       memset(&blob, 0, sizeof(blob));
> +       blob.blob_id = sna_crtc->gamma_lut_blob;
> +
> +       if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
> +               return false;
> +
> +       lut_size = blob.length / sizeof(lut[0]);
> +
> +       if (lut_size == 0 ||
> +           lut_size > max(sna_crtc->gamma_lut_size, 256))
> +               return false;
> +
> +       blob.data = (uintptr_t)lut;
> +
> +       if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETPROPBLOB, &blob))
> +               return false;
> +
> +       for (i = 0; i < size; i++) {
> +               struct drm_color_lut *entry =
> +                       &lut[i * (lut_size - 1) / (size - 1)];
> +
> +               red[i] = entry->red;
> +               green[i] = entry->green;
> +               blue[i] = entry->blue;
> +       }
> +
> +       return red[size - 1] &&
> +               green[size - 1] &&
> +               blue[size - 1];
> +}
> +
> +static bool crtc_get_gamma_legacy(xf86CrtcPtr crtc,
> +                                 CARD16 *red,
> +                                 CARD16 *green,
> +                                 CARD16 *blue,
> +                                 int size)
> +{
> +       struct sna *sna = to_sna(crtc->scrn);
> +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
> +       struct drm_mode_crtc_lut lut = {
> +               .crtc_id = __sna_crtc_id(sna_crtc),
> +               .gamma_size = size,
> +               .red = (uintptr_t)red,
> +               .green = (uintptr_t)green,
> +               .blue = (uintptr_t)blue,
> +       };
> +
> +       if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) != 0)
> +               return false;
> +
> +       VG(VALGRIND_MAKE_MEM_DEFINED(red, size*sizeof(red[0])));
> +       VG(VALGRIND_MAKE_MEM_DEFINED(green, size*sizeof(green[0])));
> +       VG(VALGRIND_MAKE_MEM_DEFINED(blue, size*sizeof(blue[0])));
> +
> +       return red[size - 1] &&
> +               green[size - 1] &&
> +               blue[size - 1];
> +}
> +
>  static void crtc_init_gamma(xf86CrtcPtr crtc)
>  {
> +       struct sna *sna = to_sna(crtc->scrn);
> +       struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
>         uint16_t *gamma;
> +       int size;
> +
> +       assert(sna_crtc);
> +
> +       size = sna_crtc->gamma_lut_size;
> +       if (!size)
> +               size = 256;
>
>         /* Initialize the gamma ramps */
>         gamma = NULL;
> -       if (crtc->gamma_size == 256)
> +       if (crtc->gamma_size == size)
>                 gamma = crtc->gamma_red;
>         if (gamma == NULL)
> -               gamma = malloc(3 * 256 * sizeof(uint16_t));
> +               gamma = malloc(3 * size * sizeof(uint16_t));
>         if (gamma) {
> -               struct sna *sna = to_sna(crtc->scrn);
> -               struct sna_crtc *sna_crtc = to_sna_crtc(crtc);
>                 struct drm_mode_crtc_lut lut;
> -               bool gamma_set = false;
> -
> -               assert(sna_crtc);
> -
> -               lut.crtc_id = __sna_crtc_id(sna_crtc);
> -               lut.gamma_size = 256;
> -               lut.red = (uintptr_t)(gamma);
> -               lut.green = (uintptr_t)(gamma + 256);
> -               lut.blue = (uintptr_t)(gamma + 2 * 256);
> -               if (drmIoctl(sna->kgem.fd, DRM_IOCTL_MODE_GETGAMMA, &lut) == 0) {
> -                       VG(VALGRIND_MAKE_MEM_DEFINED(gamma, 3*256*sizeof(gamma[0])));
> -                       gamma_set =
> -                               gamma[256 - 1] &&
> -                               gamma[2*256 - 1] &&
> -                               gamma[3*256 - 1];
> -               }
> +               bool gamma_set;
> +               uint16_t *red = gamma;
> +               uint16_t *green = gamma + size;
> +               uint16_t *blue = gamma + 2 * size;
> +
> +               if (sna_crtc->gamma_lut_size)
> +                       gamma_set = crtc_get_gamma_lut(crtc, red,
> +                                                      green, blue, size);
> +               else
> +                       gamma_set = crtc_get_gamma_legacy(crtc, red,
> +                                                         green, blue, size);
>
>                 DBG(("%s: CRTC:%d, pipe=%d: gamma set?=%d\n",
>                      __FUNCTION__, __sna_crtc_id(sna_crtc), __sna_crtc_pipe(sna_crtc),
> @@ -7311,19 +7478,21 @@ static void crtc_init_gamma(xf86CrtcPtr crtc)
>                 if (!gamma_set) {
>                         int i;
>
> -                       for (i = 0; i < 256; i++) {
> -                               gamma[i] = i << 8;
> -                               gamma[256 + i] = i << 8;
> -                               gamma[2*256 + i] = i << 8;
> +                       for (i = 0; i < size; i++) {
> +                               uint16_t val = i * 0xffff / (size - 1);
> +
> +                               red[i] = val;
> +                               green[i] = val;
> +                               blue[i] = val;
>                         }
>                 }
>
> -               if (gamma != crtc->gamma_red) {
> +               if (red != crtc->gamma_red) {
>                         free(crtc->gamma_red);
> -                       crtc->gamma_red = gamma;
> -                       crtc->gamma_green = gamma + 256;
> -                       crtc->gamma_blue = gamma + 2*256;
> -                       crtc->gamma_size = 256;
> +                       crtc->gamma_red = red;
> +                       crtc->gamma_green = green;
> +                       crtc->gamma_blue = blue;
> +                       crtc->gamma_size = size;
>                 }
>         }
>  }
> --
> 2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH xf86-video-intel v3 2/2] sna: Support 10bpc gamma via the GAMMA_LUT crtc property

[Chris Wilson]

Quoting Mario Kleiner (2019-07-09 19:34:54)
> Hi Ville,
> 
> now somebody just needs to merge these two 10 bit gamma lut patches
> into intel-ddx?

And so it is done. Thank you,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH xf86-video-intel v2 1/2] sna: Refactor property parsing

[Ville Syrjala]

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

Generalize the code that parses the plane properties to be useable
for crtc (or any kms object) properties as well.

v2: plane 'type' prop is enum not range!

Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 src/sna/sna_display.c | 69 ++++++++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 20 deletions(-)

diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index 119ea981d243..41edfec12839 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -215,6 +215,7 @@ struct sna_crtc {
 	uint32_t rotation;
 	struct plane {
 		uint32_t id;
+		uint32_t type;
 		struct {
 			uint32_t prop;
 			uint32_t supported;
@@ -3391,33 +3392,40 @@ void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc,
 				 p->color_encoding.values[colorspace]);
 }
 
-static int plane_details(struct sna *sna, struct plane *p)
+typedef void (*parse_prop_func)(struct sna *sna,
+				struct drm_mode_get_property *prop,
+				uint64_t value,
+				void *data);
+static void parse_props(struct sna *sna,
+		       uint32_t obj_type, uint32_t obj_id,
+		       parse_prop_func parse_prop,
+		       void *data)
 {
 #define N_STACK_PROPS 32 /* must be a multiple of 2 */
 	struct local_mode_obj_get_properties arg;
 	uint64_t stack[N_STACK_PROPS + N_STACK_PROPS/2];
 	uint64_t *values = stack;
 	uint32_t *props = (uint32_t *)(values + N_STACK_PROPS);
-	int i, type = DRM_PLANE_TYPE_OVERLAY;
+	int i;
 
 	memset(&arg, 0, sizeof(struct local_mode_obj_get_properties));
-	arg.obj_id = p->id;
-	arg.obj_type = LOCAL_MODE_OBJECT_PLANE;
+	arg.obj_id = obj_id;
+	arg.obj_type = obj_type;
 
 	arg.props_ptr = (uintptr_t)props;
 	arg.prop_values_ptr = (uintptr_t)values;
 	arg.count_props = N_STACK_PROPS;
 
 	if (drmIoctl(sna->kgem.fd, LOCAL_IOCTL_MODE_OBJ_GETPROPERTIES, &arg))
-		return -1;
+		return;
 
 	DBG(("%s: object %d (type %x) has %d props\n", __FUNCTION__,
-	     p->id, LOCAL_MODE_OBJECT_PLANE, arg.count_props));
+	     obj_id, obj_type, arg.count_props));
 
 	if (arg.count_props > N_STACK_PROPS) {
 		values = malloc(2*sizeof(uint64_t)*arg.count_props);
 		if (values == NULL)
-			return -1;
+			return;
 
 		props = (uint32_t *)(values + arg.count_props);
 
@@ -3444,27 +3452,48 @@ static int plane_details(struct sna *sna, struct plane *p)
 		DBG(("%s: prop[%d] .id=%ld, .name=%s, .flags=%x, .value=%ld\n", __FUNCTION__, i,
 		     (long)props[i], prop.name, (unsigned)prop.flags, (long)values[i]));
 
-		if (strcmp(prop.name, "type") == 0) {
-			type = values[i];
-		} else if (prop_is_rotation(&prop)) {
-			parse_rotation_prop(sna, p, &prop, values[i]);
-		} else if (prop_is_color_encoding(&prop)) {
-			parse_color_encoding_prop(sna, p, &prop, values[i]);
-		}
+		parse_prop(sna, &prop, values[i], data);
 	}
 
-	p->rotation.supported &= DBG_NATIVE_ROTATION;
-	if (!xf86ReturnOptValBool(sna->Options, OPTION_ROTATION, TRUE))
-		p->rotation.supported = RR_Rotate_0;
-
 	if (values != stack)
 		free(values);
 
-	DBG(("%s: plane=%d type=%d\n", __FUNCTION__, p->id, type));
-	return type;
 #undef N_STACK_PROPS
 }
 
+static bool prop_is_type(const struct drm_mode_get_property *prop)
+{
+	return prop_has_type_and_name(prop, 3, "type");
+}
+
+static void plane_parse_prop(struct sna *sna,
+			     struct drm_mode_get_property *prop,
+			     uint64_t value, void *data)
+{
+	struct plane *p = data;
+
+	if (prop_is_type(prop))
+		p->type = value;
+	else if (prop_is_rotation(prop))
+		parse_rotation_prop(sna, p, prop, value);
+	else if (prop_is_color_encoding(prop))
+		parse_color_encoding_prop(sna, p, prop, value);
+}
+
+static int plane_details(struct sna *sna, struct plane *p)
+{
+	parse_props(sna, LOCAL_MODE_OBJECT_PLANE, p->id,
+		    plane_parse_prop, p);
+
+	p->rotation.supported &= DBG_NATIVE_ROTATION;
+	if (!xf86ReturnOptValBool(sna->Options, OPTION_ROTATION, TRUE))
+		p->rotation.supported = RR_Rotate_0;
+
+	DBG(("%s: plane=%d type=%d\n", __FUNCTION__, p->id, p->type));
+
+	return p->type;
+}
+
 static void add_sprite_plane(struct sna_crtc *crtc,
 			     struct plane *details)
 {
-- 
2.21.0

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

Re: [PATCH xf86-video-intel v2 1/2] sna: Refactor property parsing

[Mario Kleiner]

On Fri, Apr 26, 2019 at 6:32 PM Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Generalize the code that parses the plane properties to be useable
> for crtc (or any kms object) properties as well.
>
> v2: plane 'type' prop is enum not range!
>
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---

This patch is

Reviewed-and-tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>

-mario

>  src/sna/sna_display.c | 69 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 49 insertions(+), 20 deletions(-)
>
> diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
> index 119ea981d243..41edfec12839 100644
> --- a/src/sna/sna_display.c
> +++ b/src/sna/sna_display.c
> @@ -215,6 +215,7 @@ struct sna_crtc {
>         uint32_t rotation;
>         struct plane {
>                 uint32_t id;
> +               uint32_t type;
>                 struct {
>                         uint32_t prop;
>                         uint32_t supported;
> @@ -3391,33 +3392,40 @@ void sna_crtc_set_sprite_colorspace(xf86CrtcPtr crtc,
>                                  p->color_encoding.values[colorspace]);
>  }
>
> -static int plane_details(struct sna *sna, struct plane *p)
> +typedef void (*parse_prop_func)(struct sna *sna,
> +                               struct drm_mode_get_property *prop,
> +                               uint64_t value,
> +                               void *data);
> +static void parse_props(struct sna *sna,
> +                      uint32_t obj_type, uint32_t obj_id,
> +                      parse_prop_func parse_prop,
> +                      void *data)
>  {
>  #define N_STACK_PROPS 32 /* must be a multiple of 2 */
>         struct local_mode_obj_get_properties arg;
>         uint64_t stack[N_STACK_PROPS + N_STACK_PROPS/2];
>         uint64_t *values = stack;
>         uint32_t *props = (uint32_t *)(values + N_STACK_PROPS);
> -       int i, type = DRM_PLANE_TYPE_OVERLAY;
> +       int i;
>
>         memset(&arg, 0, sizeof(struct local_mode_obj_get_properties));
> -       arg.obj_id = p->id;
> -       arg.obj_type = LOCAL_MODE_OBJECT_PLANE;
> +       arg.obj_id = obj_id;
> +       arg.obj_type = obj_type;
>
>         arg.props_ptr = (uintptr_t)props;
>         arg.prop_values_ptr = (uintptr_t)values;
>         arg.count_props = N_STACK_PROPS;
>
>         if (drmIoctl(sna->kgem.fd, LOCAL_IOCTL_MODE_OBJ_GETPROPERTIES, &arg))
> -               return -1;
> +               return;
>
>         DBG(("%s: object %d (type %x) has %d props\n", __FUNCTION__,
> -            p->id, LOCAL_MODE_OBJECT_PLANE, arg.count_props));
> +            obj_id, obj_type, arg.count_props));
>
>         if (arg.count_props > N_STACK_PROPS) {
>                 values = malloc(2*sizeof(uint64_t)*arg.count_props);
>                 if (values == NULL)
> -                       return -1;
> +                       return;
>
>                 props = (uint32_t *)(values + arg.count_props);
>
> @@ -3444,27 +3452,48 @@ static int plane_details(struct sna *sna, struct plane *p)
>                 DBG(("%s: prop[%d] .id=%ld, .name=%s, .flags=%x, .value=%ld\n", __FUNCTION__, i,
>                      (long)props[i], prop.name, (unsigned)prop.flags, (long)values[i]));
>
> -               if (strcmp(prop.name, "type") == 0) {
> -                       type = values[i];
> -               } else if (prop_is_rotation(&prop)) {
> -                       parse_rotation_prop(sna, p, &prop, values[i]);
> -               } else if (prop_is_color_encoding(&prop)) {
> -                       parse_color_encoding_prop(sna, p, &prop, values[i]);
> -               }
> +               parse_prop(sna, &prop, values[i], data);
>         }
>
> -       p->rotation.supported &= DBG_NATIVE_ROTATION;
> -       if (!xf86ReturnOptValBool(sna->Options, OPTION_ROTATION, TRUE))
> -               p->rotation.supported = RR_Rotate_0;
> -
>         if (values != stack)
>                 free(values);
>
> -       DBG(("%s: plane=%d type=%d\n", __FUNCTION__, p->id, type));
> -       return type;
>  #undef N_STACK_PROPS
>  }
>
> +static bool prop_is_type(const struct drm_mode_get_property *prop)
> +{
> +       return prop_has_type_and_name(prop, 3, "type");
> +}
> +
> +static void plane_parse_prop(struct sna *sna,
> +                            struct drm_mode_get_property *prop,
> +                            uint64_t value, void *data)
> +{
> +       struct plane *p = data;
> +
> +       if (prop_is_type(prop))
> +               p->type = value;
> +       else if (prop_is_rotation(prop))
> +               parse_rotation_prop(sna, p, prop, value);
> +       else if (prop_is_color_encoding(prop))
> +               parse_color_encoding_prop(sna, p, prop, value);
> +}
> +
> +static int plane_details(struct sna *sna, struct plane *p)
> +{
> +       parse_props(sna, LOCAL_MODE_OBJECT_PLANE, p->id,
> +                   plane_parse_prop, p);
> +
> +       p->rotation.supported &= DBG_NATIVE_ROTATION;
> +       if (!xf86ReturnOptValBool(sna->Options, OPTION_ROTATION, TRUE))
> +               p->rotation.supported = RR_Rotate_0;
> +
> +       DBG(("%s: plane=%d type=%d\n", __FUNCTION__, p->id, p->type));
> +
> +       return p->type;
> +}
> +
>  static void add_sprite_plane(struct sna_crtc *crtc,
>                              struct plane *details)
>  {
> --
> 2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx