From 68a6cfd62c6ee354c9da7e3466bdbd746be6ef8d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
Date: Tue, 11 Feb 2020 18:22:06 +0200
Subject: drm: Validate encoder->possible_clones
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Git-commit: 74d2aacbe84042d89f572a3112a146fca05bfcb1
Patch-mainline: v5.8-rc1
References: jsc#SLE-12680, jsc#SLE-12880, jsc#SLE-12882, jsc#SLE-12883, jsc#SLE-13496, jsc#SLE-15322

Many drivers are populating encoder->possible_clones wrong. Let's
persuade them to get it right by adding some loud WARNs.

We'll cross check the bits between any two encoders. So either
both encoders can clone with the other, or neither can.

We'll also complain about effectively empty possible_clones, and
possible_clones containing bits for encoders that don't exist.

v2: encoder->possible_clones now includes the encoder itelf
v3: Move to drm_mode_config_validate() (Daniel)
    Document that you get a WARN when this is wrong (Daniel)
    Extract full_encoder_mask()
v4: !! instead of ! (Daniel)

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200211162208.16224-6-ville.syrjala@linux.intel.com
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Patrik Jakobsson <pjakobsson@suse.de>
---
 drivers/gpu/drm/drm_mode_config.c | 40 +++++++++++++++++++++++++++++++
 include/drm/drm_encoder.h         |  2 ++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 75e357c7e84d..4099a626324a 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -533,6 +533,17 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_config_cleanup);
 
+static u32 full_encoder_mask(struct drm_device *dev)
+{
+	struct drm_encoder *encoder;
+	u32 encoder_mask = 0;
+
+	drm_for_each_encoder(encoder, dev)
+		encoder_mask |= drm_encoder_mask(encoder);
+
+	return encoder_mask;
+}
+
 /*
  * For some reason we want the encoder itself included in
  * possible_clones. Make life easy for drivers by allowing them
@@ -544,10 +555,39 @@ static void fixup_encoder_possible_clones(struct drm_encoder *encoder)
 		encoder->possible_clones = drm_encoder_mask(encoder);
 }
 
+static void validate_encoder_possible_clones(struct drm_encoder *encoder)
+{
+	struct drm_device *dev = encoder->dev;
+	u32 encoder_mask = full_encoder_mask(dev);
+	struct drm_encoder *other;
+
+	drm_for_each_encoder(other, dev) {
+		WARN(!!(encoder->possible_clones & drm_encoder_mask(other)) !=
+		     !!(other->possible_clones & drm_encoder_mask(encoder)),
+		     "possible_clones mismatch: "
+		     "[ENCODER:%d:%s] mask=0x%x possible_clones=0x%x vs. "
+		     "[ENCODER:%d:%s] mask=0x%x possible_clones=0x%x\n",
+		     encoder->base.id, encoder->name,
+		     drm_encoder_mask(encoder), encoder->possible_clones,
+		     other->base.id, other->name,
+		     drm_encoder_mask(other), other->possible_clones);
+	}
+
+	WARN((encoder->possible_clones & drm_encoder_mask(encoder)) == 0 ||
+	     (encoder->possible_clones & ~encoder_mask) != 0,
+	     "Bogus possible_clones: "
+	     "[ENCODER:%d:%s] possible_clones=0x%x (full encoder mask=0x%x)\n",
+	     encoder->base.id, encoder->name,
+	     encoder->possible_clones, encoder_mask);
+}
+
 void drm_mode_config_validate(struct drm_device *dev)
 {
 	struct drm_encoder *encoder;
 
 	drm_for_each_encoder(encoder, dev)
 		fixup_encoder_possible_clones(encoder);
+
+	drm_for_each_encoder(encoder, dev)
+		validate_encoder_possible_clones(encoder);
 }
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index 34b7213bfab6..9d11cbe22f34 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -163,6 +163,8 @@ struct drm_encoder {
 	 * any cloning it can leave @possible_clones set to 0. The core will
 	 * automagically fix this up by setting the bit for the encoder itself.
 	 *
+	 * You will get a WARN if you get this wrong in the driver.
+	 *
 	 * Note that since encoder objects can't be hotplugged the assigned indices
 	 * are stable and hence known before registering all objects.
 	 */
-- 
2.28.0