Message-Id: <20180420122545.40014-1-stschake@gmail.com>
Date:   Fri, 20 Apr 2018 05:25:44 -0700
From:   Stefan Schake <stschake@...il.com>
To:     eric@...olt.net
Cc:     airlied@...ux.ie, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, Stefan Schake <stschake@...il.com>
Subject: [PATCH v4 1/2] drm/vc4: Add CTM support

The hardware has a single block for applying a CTM prior to gamma lut.
It can be fed with pixels from one of our CRTC at a time and uses a
matrix with S0.9 scalars. Use private atomic state to reject attempts
from userland to apply CTM for more than one CRTC at a time and reject
matrices with scalars that we can't approximate without integer bits.

Signed-off-by: Stefan Schake <stschake@...il.com>
Signed-off-by: Eric Anholt <eric@...olt.net>
---
Eric, I removed your r-b since there were a bunch more changes.

v4: Handle CTM disabling in check
	Don't use drm_atomic_get_private_obj_state in commit
	Fix S31.32 -> S0.9 conversion
	Squashed in Erics fixup series:
	Don't take the CTM lock unless a CTM change is happening (Eric)
	Lock across changes to the CTM (Eric)
	Handle allocation failure in out CTM priv state (Eric)
	Move CTM object init before FBDEV setup (Eric)
	Clean up the CTM private object manager on unload (Eric)

v3: New in the series.

 drivers/gpu/drm/vc4/vc4_crtc.c |   5 +
 drivers/gpu/drm/vc4/vc4_drv.c  |   3 +
 drivers/gpu/drm/vc4/vc4_drv.h  |   4 +
 drivers/gpu/drm/vc4/vc4_kms.c  | 204 ++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 215 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 08fe8dd7d8df..83d3b7912fc2 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1018,6 +1018,11 @@ static int vc4_crtc_bind(struct device *dev, struct device *master, void *data)
 	drm_mode_crtc_set_gamma_size(crtc, ARRAY_SIZE(vc4_crtc->lut_r));
 	drm_crtc_enable_color_mgmt(crtc, 0, false, crtc->gamma_size);
 
+	/* We support CTM, but only for one CRTC at a time. It's therefore
+	 * implemented as private driver state in vc4_kms, not here.
+	 */
+	drm_crtc_enable_color_mgmt(crtc, 0, true, crtc->gamma_size);
+
 	/* Set up some arbitrary number of planes.  We're not limited
 	 * by a set number of physical registers, just the space in
 	 * the HVS (16k) and how small an plane can be (28 bytes).
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 94b99c90425a..52bfe0e9ddda 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -320,6 +320,7 @@ static void vc4_drm_unbind(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct drm_device *drm = platform_get_drvdata(pdev);
+	struct vc4_dev *vc4 = to_vc4_dev(drm);
 
 	drm_dev_unregister(drm);
 
@@ -327,6 +328,8 @@ static void vc4_drm_unbind(struct device *dev)
 
 	drm_mode_config_cleanup(drm);
 
+	drm_atomic_private_obj_fini(&vc4->ctm_manager);
+
 	drm_dev_unref(drm);
 }
 
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 4288615b66a2..22589d39083c 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -10,6 +10,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_encoder.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_atomic.h>
 
 #include "uapi/drm/vc4_drm.h"
 
@@ -193,6 +194,9 @@ struct vc4_dev {
 	} hangcheck;
 
 	struct semaphore async_modeset;
+
+	struct drm_modeset_lock ctm_state_lock;
+	struct drm_private_obj ctm_manager;
 };
 
 static inline struct vc4_dev *
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index e791e498a3dd..8a411e5f8776 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -23,6 +23,117 @@
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include "vc4_drv.h"
+#include "vc4_regs.h"
+
+struct vc4_ctm_state {
+	struct drm_private_state base;
+	struct drm_color_ctm *ctm;
+	int fifo;
+};
+
+static struct vc4_ctm_state *to_vc4_ctm_state(struct drm_private_state *priv)
+{
+	return container_of(priv, struct vc4_ctm_state, base);
+}
+
+static struct vc4_ctm_state *vc4_get_ctm_state(struct drm_atomic_state *state,
+					       struct drm_private_obj *manager)
+{
+	struct drm_device *dev = state->dev;
+	struct vc4_dev *vc4 = dev->dev_private;
+	struct drm_private_state *priv_state;
+	int ret;
+
+	ret = drm_modeset_lock(&vc4->ctm_state_lock, state->acquire_ctx);
+	if (ret)
+		return ERR_PTR(ret);
+
+	priv_state = drm_atomic_get_private_obj_state(state, manager);
+	if (IS_ERR(priv_state))
+		return ERR_CAST(priv_state);
+
+	return to_vc4_ctm_state(priv_state);
+}
+
+static struct drm_private_state *
+vc4_ctm_duplicate_state(struct drm_private_obj *obj)
+{
+	struct vc4_ctm_state *state;
+
+	state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return NULL;
+
+	__drm_atomic_helper_private_obj_duplicate_state(obj, &state->base);
+
+	return &state->base;
+}
+
+static void vc4_ctm_destroy_state(struct drm_private_obj *obj,
+				  struct drm_private_state *state)
+{
+	struct vc4_ctm_state *ctm_state = to_vc4_ctm_state(state);
+
+	kfree(ctm_state);
+}
+
+static const struct drm_private_state_funcs vc4_ctm_state_funcs = {
+	.atomic_duplicate_state = vc4_ctm_duplicate_state,
+	.atomic_destroy_state = vc4_ctm_destroy_state,
+};
+
+/* Converts a DRM S31.32 value to the HW S0.9 format. */
+static u16 vc4_ctm_s31_32_to_s0_9(u64 in)
+{
+	u16 r;
+
+	/* Sign bit. */
+	r = in & BIT_ULL(63) ? BIT(9) : 0;
+
+	if ((in & GENMASK_ULL(62, 32)) > 0) {
+		/* We have zero integer bits so we can only saturate here. */
+		r |= GENMASK(8, 0);
+	} else {
+		/* Otherwise take the 9 most important fractional bits. */
+		r |= (in >> 23) & GENMASK(8, 0);
+	}
+
+	return r;
+}
+
+static void
+vc4_ctm_commit(struct vc4_dev *vc4, struct drm_atomic_state *state)
+{
+	struct vc4_ctm_state *ctm_state = to_vc4_ctm_state(vc4->ctm_manager.state);
+	struct drm_color_ctm *ctm = ctm_state->ctm;
+
+	if (ctm_state->fifo) {
+		HVS_WRITE(SCALER_OLEDCOEF2,
+			  VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[0]),
+					SCALER_OLEDCOEF2_R_TO_R) |
+			  VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[3]),
+					SCALER_OLEDCOEF2_R_TO_G) |
+			  VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[6]),
+					SCALER_OLEDCOEF2_R_TO_B));
+		HVS_WRITE(SCALER_OLEDCOEF1,
+			  VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[1]),
+					SCALER_OLEDCOEF1_G_TO_R) |
+			  VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[4]),
+					SCALER_OLEDCOEF1_G_TO_G) |
+			  VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[7]),
+					SCALER_OLEDCOEF1_G_TO_B));
+		HVS_WRITE(SCALER_OLEDCOEF0,
+			  VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[2]),
+					SCALER_OLEDCOEF0_B_TO_R) |
+			  VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[5]),
+					SCALER_OLEDCOEF0_B_TO_G) |
+			  VC4_SET_FIELD(vc4_ctm_s31_32_to_s0_9(ctm->matrix[8]),
+					SCALER_OLEDCOEF0_B_TO_B));
+	}
+
+	HVS_WRITE(SCALER_OLEDOFFS,
+		  VC4_SET_FIELD(ctm_state->fifo, SCALER_OLEDOFFS_DISPFIFO));
+}
 
 static void
 vc4_atomic_complete_commit(struct drm_atomic_state *state)
@@ -36,6 +147,8 @@ vc4_atomic_complete_commit(struct drm_atomic_state *state)
 
 	drm_atomic_helper_commit_modeset_disables(dev, state);
 
+	vc4_ctm_commit(vc4, state);
+
 	drm_atomic_helper_commit_planes(dev, state, 0);
 
 	drm_atomic_helper_commit_modeset_enables(dev, state);
@@ -207,9 +320,89 @@ static struct drm_framebuffer *vc4_fb_create(struct drm_device *dev,
 	return drm_gem_fb_create(dev, file_priv, mode_cmd);
 }
 
+/* Our CTM has some peculiar limitations: we can only enable it for one CRTC
+ * at a time and the HW only supports S0.9 scalars. To account for the latter,
+ * we don't allow userland to set a CTM that we have no hope of approximating.
+ */
+static int
+vc4_ctm_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
+{
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+	struct vc4_ctm_state *ctm_state = NULL;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+	struct drm_color_ctm *ctm;
+	int i;
+
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		/* CTM is being disabled. */
+		if (!new_crtc_state->ctm && old_crtc_state->ctm) {
+			ctm_state = vc4_get_ctm_state(state, &vc4->ctm_manager);
+			if (IS_ERR(ctm_state))
+				return PTR_ERR(ctm_state);
+			ctm_state->fifo = 0;
+		}
+	}
+
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		if (new_crtc_state->ctm == old_crtc_state->ctm)
+			continue;
+
+		if (!ctm_state) {
+			ctm_state = vc4_get_ctm_state(state, &vc4->ctm_manager);
+			if (IS_ERR(ctm_state))
+				return PTR_ERR(ctm_state);
+		}
+
+		/* CTM is being enabled or the matrix changed. */
+		if (new_crtc_state->ctm) {
+			/* fifo is 1-based since 0 disables CTM. */
+			int fifo = to_vc4_crtc(crtc)->channel + 1;
+
+			/* Check userland isn't trying to turn on CTM for more
+			 * than one CRTC at a time.
+			 */
+			if (ctm_state->fifo && ctm_state->fifo != fifo) {
+				DRM_DEBUG_DRIVER("Too many CTM configured\n");
+				return -EINVAL;
+			}
+
+			/* Check we can approximate the specified CTM.
+			 * We disallow scalars |c| > 1.0 since the HW has
+			 * no integer bits.
+			 */
+			ctm = new_crtc_state->ctm->data;
+			for (i = 0; i < ARRAY_SIZE(ctm->matrix); i++) {
+				u64 val = ctm->matrix[i];
+
+				val &= ~BIT_ULL(63);
+				if (val > BIT_ULL(32))
+					return -EINVAL;
+			}
+
+			ctm_state->fifo = fifo;
+			ctm_state->ctm = ctm;
+		}
+	}
+
+	return 0;
+}
+
+static int
+vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
+{
+	int ret;
+
+	ret = vc4_ctm_atomic_check(dev, state);
+	if (ret < 0)
+		return ret;
+
+	return drm_atomic_helper_check(dev, state);
+}
+
 static const struct drm_mode_config_funcs vc4_mode_funcs = {
 	.output_poll_changed = drm_fb_helper_output_poll_changed,
-	.atomic_check = drm_atomic_helper_check,
+	.atomic_check = vc4_atomic_check,
 	.atomic_commit = vc4_atomic_commit,
 	.fb_create = vc4_fb_create,
 };
@@ -217,6 +410,7 @@ static const struct drm_mode_config_funcs vc4_mode_funcs = {
 int vc4_kms_load(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
+	struct vc4_ctm_state *ctm_state;
 	int ret;
 
 	sema_init(&vc4->async_modeset, 1);
@@ -237,6 +431,14 @@ int vc4_kms_load(struct drm_device *dev)
 	dev->mode_config.async_page_flip = true;
 	dev->mode_config.allow_fb_modifiers = true;
 
+	drm_modeset_lock_init(&vc4->ctm_state_lock);
+
+	ctm_state = kzalloc(sizeof(*ctm_state), GFP_KERNEL);
+	if (!ctm_state)
+		return -ENOMEM;
+	drm_atomic_private_obj_init(&vc4->ctm_manager, &ctm_state->base,
+				    &vc4_ctm_state_funcs);
+
 	drm_mode_config_reset(dev);
 
 	if (dev->mode_config.num_connector)
-- 
2.14.1

Powered by blists - more mailing lists