[v3,weston,1/5] compositor-drm: Implement support for GAMMA_LUT drm property

Submitted by Harsha Manjula Mallikarjun (RBEI/ECF3) on June 28, 2018, 1:27 p.m.

Details

Message ID 1530192480-23334-2-git-send-email-harsha.manjulamallikarjun@in.bosch.com
State Changes Requested
Headers show
Series "Implement support for drm properties "GAMMA_LUT" and "CTM"" ( rev: 3 ) in Wayland (DEPRECATED)

Commit Message

Harsha Manjula Mallikarjun (RBEI/ECF3) June 28, 2018, 1:27 p.m.
From: Harsha M M <harsha.manjulamallikarjun@in.bosch.com>

drmModeCrtcSetGamma is a legacy now. Use the generic drm property
to set Gamma. If GAMMA_LUT is not supported then legacy api will
be used.

Signed-off-by: Harsha M M <harsha.manjulamallikarjun@in.bosch.com>
---
 libweston/compositor-drm.c | 83 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 77 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 8b1ea66..27c95da 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -196,12 +196,16 @@  static const struct drm_property_info connector_props[] = {
 enum wdrm_crtc_property {
 	WDRM_CRTC_MODE_ID = 0,
 	WDRM_CRTC_ACTIVE,
+	WDRM_CRTC_GAMMA_LUT,
+	WDRM_CRTC_GAMMA_LUT_SIZE,
 	WDRM_CRTC__COUNT
 };
 
 static const struct drm_property_info crtc_props[] = {
 	[WDRM_CRTC_MODE_ID] = { .name = "MODE_ID", },
 	[WDRM_CRTC_ACTIVE] = { .name = "ACTIVE", },
+	[WDRM_CRTC_GAMMA_LUT] = { .name = "GAMMA_LUT", },
+	[WDRM_CRTC_GAMMA_LUT_SIZE] = { .name = "GAMMA_LUT_SIZE", },
 };
 
 /**
@@ -1752,16 +1756,63 @@  drm_output_set_gamma(struct weston_output *output_base,
 	struct drm_output *output = to_drm_output(output_base);
 	struct drm_backend *backend =
 		to_drm_backend(output->base.compositor);
+	uint32_t gamma_prop_id;
+	uint32_t gamma_size_prop_id;
+	struct drm_color_lut *lut = NULL;
+	uint32_t gamma_blobid = 0;
+	uint32_t loop;
+
 
 	/* check */
 	if (output_base->gamma_size != size)
 		return;
 
-	rc = drmModeCrtcSetGamma(backend->drm.fd,
-				 output->crtc_id,
-				 size, r, g, b);
-	if (rc)
-		weston_log("set gamma failed: %m\n");
+	gamma_prop_id = output->props_crtc[WDRM_CRTC_GAMMA_LUT].prop_id;
+	gamma_size_prop_id = output->props_crtc[WDRM_CRTC_GAMMA_LUT_SIZE].
+								prop_id;
+
+	if (gamma_prop_id && gamma_size_prop_id) {
+		lut = malloc(sizeof(struct drm_color_lut) * size);
+		if (!lut) {
+			weston_log("failed to allocate memory for gamma lut\n");
+			goto out;
+		}
+
+		for (loop = 0; loop < size; loop++) {
+			lut[loop].red = r[loop];
+			lut[loop].green = g[loop];
+			lut[loop].blue = b[loop];
+		}
+
+		rc = drmModeCreatePropertyBlob(backend->drm.fd, lut,
+		      sizeof(struct drm_color_lut) * size, &gamma_blobid);
+		if (rc) {
+			weston_log("failed to create drm gamma blob: %m\n");
+			goto out;
+		}
+
+		rc = drmModeObjectSetProperty(backend->drm.fd, output->crtc_id,
+			DRM_MODE_OBJECT_CRTC, gamma_prop_id, gamma_blobid);
+
+		if (rc)
+			weston_log("set of gamma lut property failed for crtc %d: %m\n",
+			     output->crtc_id);
+	} else {
+		rc = drmModeCrtcSetGamma(backend->drm.fd,
+					 output->crtc_id,
+					 size, r, g, b);
+		if (rc)
+			weston_log("set gamma failed for crtc %d: %m\n",
+				   output->crtc_id);
+
+		return;
+	}
+out:
+	if (lut)
+		free(lut);
+	if (gamma_blobid)
+		drmModeDestroyPropertyBlob(backend->drm.fd, gamma_blobid);
+
 }
 
 /* Determine the type of vblank synchronization to use for the output.
@@ -4758,15 +4809,35 @@  drm_output_init_gamma_size(struct drm_output *output)
 {
 	struct drm_backend *backend = to_drm_backend(output->base.compositor);
 	drmModeCrtc *crtc;
+	uint32_t gamma_prop_id;
+	uint32_t gamma_size_prop_id;
+	struct drm_property_info *props_crtc;
+	drmModeObjectProperties *props;
 
 	assert(output->base.compositor);
 	assert(output->crtc_id != 0);
+
 	crtc = drmModeGetCrtc(backend->drm.fd, output->crtc_id);
 	if (!crtc)
 		return -1;
 
 	output->base.gamma_size = crtc->gamma_size;
-
+	props_crtc = &output->props_crtc[0];
+	gamma_prop_id = props_crtc[WDRM_CRTC_GAMMA_LUT].prop_id;
+	gamma_size_prop_id = props_crtc[WDRM_CRTC_GAMMA_LUT_SIZE].prop_id;
+
+	if (gamma_prop_id && gamma_size_prop_id) {
+		props  = drmModeObjectGetProperties(backend->drm.fd,
+						   output->crtc_id,
+						   DRM_MODE_OBJECT_CRTC);
+		if (props) {
+			output->base.gamma_size = drm_property_get_value(
+				       &props_crtc[WDRM_CRTC_GAMMA_LUT_SIZE],
+				       props,
+				       WDRM_CRTC__COUNT);
+		}
+	}
+
 	drmModeFreeCrtc(crtc);
 
 	return 0;

Comments

Hi Harsha,
Some small comments here, but more substantial comments in another patch.

On Thu, 28 Jun 2018 at 14:28, <harsha.manjulamallikarjun@in.bosch.com> wrote:
> +       gamma_prop_id = output->props_crtc[WDRM_CRTC_GAMMA_LUT].prop_id;
> +       gamma_size_prop_id = output->props_crtc[WDRM_CRTC_GAMMA_LUT_SIZE].
> +                                                               prop_id;
> +
> +       if (gamma_prop_id && gamma_size_prop_id) {
> +               lut = malloc(sizeof(struct drm_color_lut) * size);
> +               if (!lut) {
> +                       weston_log("failed to allocate memory for gamma lut\n");
> +                       goto out;
> +               }
> +
> +               for (loop = 0; loop < size; loop++) {
> +                       lut[loop].red = r[loop];
> +                       lut[loop].green = g[loop];
> +                       lut[loop].blue = b[loop];
> +               }

Since this is the new interface, it would be nice to switch Weston's
internals to use the same structure internally; this would mean a copy
when using the old drmModeSetGamma interface, and no copies when using
the property interface.

> @@ -4758,15 +4809,35 @@ drm_output_init_gamma_size(struct drm_output *output)
> +       props_crtc = &output->props_crtc[0];

This should just be output->props_crtc, but per below, can be
completely eliminated.

> +       gamma_prop_id = props_crtc[WDRM_CRTC_GAMMA_LUT].prop_id;
> +       gamma_size_prop_id = props_crtc[WDRM_CRTC_GAMMA_LUT_SIZE].prop_id;
> +
> +       if (gamma_prop_id && gamma_size_prop_id) {
> +               props  = drmModeObjectGetProperties(backend->drm.fd,
> +                                                  output->crtc_id,
> +                                                  DRM_MODE_OBJECT_CRTC);
> +               if (props) {
> +
> +                       output->base.gamma_size = drm_property_get_value(
> +                                      &props_crtc[WDRM_CRTC_GAMMA_LUT_SIZE],
> +                                      props,
> +                                      WDRM_CRTC__COUNT);

The last argument for this function is a default value. You could
simply rewrite this to unconditionally get the CRTC properties and
pass the gamma size from drmModeCrtc as the default, which avoids both
branches and all the temporary variables.

Cheers,
Daniel