[PATCH v2 0/9] drm: Third attempt at fixing the fb-helper .best_encoder() mess

[PATCH v2 0/9] drm: Third attempt at fixing the fb-helper .best_encoder() mess

[Ville Syrjala]

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

Changes from the previous version mainly involve Danoie's suggestion
of hiding the drm_encoder_find() in the iterator macro. I also polished
the msm and tilcdc cases a bit more with another small helper.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: freedreno@lists.freedesktop.org
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: nouveau@lists.freedesktop.org
Cc: Rob Clark <robdclark@gmail.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>

Ville Syrjälä (9):
  drm/fb-helper: Eliminate the .best_encoder() usage
  drm/i915: Nuke intel_mst_best_encoder()
  drm: Add drm_connector_for_each_possible_encoder()
  drm/amdgpu: Use drm_connector_for_each_possible_encoder()
  drm/nouveau: Use drm_connector_for_each_possible_encoder()
  drm/radeon: Use drm_connector_for_each_possible_encoder()
  drm: Add drm_connector_has_possible_encoder()
  drm/msm: Use drm_connector_has_possible_encoder()
  drm/tilcdc: Use drm_connector_has_possible_encoder()

 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 81 ++++++-----------------
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c       | 15 ++---
 drivers/gpu/drm/drm_connector.c                | 44 +++++++++----
 drivers/gpu/drm/drm_fb_helper.c                | 34 +++++-----
 drivers/gpu/drm/drm_probe_helper.c             | 10 +--
 drivers/gpu/drm/i915/intel_dp_mst.c            | 10 ---
 drivers/gpu/drm/msm/dsi/dsi_manager.c          |  8 +--
 drivers/gpu/drm/nouveau/nouveau_connector.c    | 21 +-----
 drivers/gpu/drm/radeon/radeon_connectors.c     | 90 ++++++++------------------
 drivers/gpu/drm/tilcdc/tilcdc_external.c       |  9 ++-
 include/drm/drm_connector.h                    | 16 +++++
 11 files changed, 128 insertions(+), 210 deletions(-)

-- 
2.16.4

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH v2 1/9] drm/fb-helper: Eliminate the .best_encoder() usage

[Ville Syrjala]

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

Instead of using the .best_encoder() hook to figure out whether a given
connector+crtc combo will work, let's instead do what userspace does and
just iterate over all the encoders for the connector, and then check
each crtc against each encoder's possible_crtcs bitmask.

v2: Avoid oopsing on NULL encoders (Daniel)
    s/connector_crtc_ok/connector_has_possible_crtc/

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com> #v1
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index cab14f253384..b37f06317d51 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2323,6 +2323,27 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
 	return true;
 }
 
+static bool connector_has_possible_crtc(struct drm_connector *connector,
+					struct drm_crtc *crtc)
+{
+	int i;
+
+	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
+		struct drm_encoder *encoder;
+
+		if (connector->encoder_ids[i] == 0)
+			break;
+
+		encoder = drm_encoder_find(connector->dev, NULL,
+					   connector->encoder_ids[i]);
+
+		if (encoder->possible_crtcs & drm_crtc_mask(crtc))
+			return true;
+	}
+
+	return false;
+}
+
 static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 			  struct drm_fb_helper_crtc **best_crtcs,
 			  struct drm_display_mode **modes,
@@ -2331,7 +2352,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 	int c, o;
 	struct drm_connector *connector;
 	const struct drm_connector_helper_funcs *connector_funcs;
-	struct drm_encoder *encoder;
 	int my_score, best_score, score;
 	struct drm_fb_helper_crtc **crtcs, *crtc;
 	struct drm_fb_helper_connector *fb_helper_conn;
@@ -2362,20 +2382,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 
 	connector_funcs = connector->helper_private;
 
-	/*
-	 * If the DRM device implements atomic hooks and ->best_encoder() is
-	 * NULL we fallback to the default drm_atomic_helper_best_encoder()
-	 * helper.
-	 */
-	if (drm_drv_uses_atomic_modeset(fb_helper->dev) &&
-	    !connector_funcs->best_encoder)
-		encoder = drm_atomic_helper_best_encoder(connector);
-	else
-		encoder = connector_funcs->best_encoder(connector);
-
-	if (!encoder)
-		goto out;
-
 	/*
 	 * select a crtc for this connector and then attempt to configure
 	 * remaining connectors
@@ -2383,7 +2389,8 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 	for (c = 0; c < fb_helper->crtc_count; c++) {
 		crtc = &fb_helper->crtc_info[c];
 
-		if ((encoder->possible_crtcs & (1 << c)) == 0)
+		if (!connector_has_possible_crtc(connector,
+						 crtc->mode_set.crtc))
 			continue;
 
 		for (o = 0; o < n; o++)
@@ -2410,7 +2417,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 			       sizeof(struct drm_fb_helper_crtc *));
 		}
 	}
-out:
+
 	kfree(crtcs);
 	return best_score;
 }
-- 
2.16.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/9] drm/i915: Nuke intel_mst_best_encoder()

[Ville Syrjala]

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

With the fb-helper no longer relying on the non-atomic .best_encoder()
we can eliminate the hook from the MST encoder.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 5890500a3a8b..0f012fbe34eb 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -403,20 +403,10 @@ static struct drm_encoder *intel_mst_atomic_best_encoder(struct drm_connector *c
 	return &intel_dp->mst_encoders[crtc->pipe]->base.base;
 }
 
-static struct drm_encoder *intel_mst_best_encoder(struct drm_connector *connector)
-{
-	struct intel_connector *intel_connector = to_intel_connector(connector);
-	struct intel_dp *intel_dp = intel_connector->mst_port;
-	if (!intel_dp)
-		return NULL;
-	return &intel_dp->mst_encoders[0]->base.base;
-}
-
 static const struct drm_connector_helper_funcs intel_dp_mst_connector_helper_funcs = {
 	.get_modes = intel_dp_mst_get_modes,
 	.mode_valid = intel_dp_mst_mode_valid,
 	.atomic_best_encoder = intel_mst_atomic_best_encoder,
-	.best_encoder = intel_mst_best_encoder,
 	.atomic_check = intel_dp_mst_atomic_check,
 };
 
-- 
2.16.4

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

[PATCH v2 3/9] drm: Add drm_connector_for_each_possible_encoder()

[Ville Syrjala]

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

Add a convenience macro for iterating connector->encoder_ids[].
Isolates the users from the implementation details.

Note that we don't seem to pass the file_priv down to drm_encoder_find()
because encoders apparently don't get leased. No idea why
drm_encoder_finc() even takes the file_priv actually.

Also use ARRAY_SIZE() when populating the array to avoid spreading
knowledge about the array size all over.

v2: Hide the drm_encoder_find() in the macro, and
    rename the macro appropriately (Daniel)

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com> #v1
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_connector.c    | 21 +++++++++------------
 drivers/gpu/drm/drm_fb_helper.c    | 11 ++---------
 drivers/gpu/drm/drm_probe_helper.c | 10 +++-------
 include/drm/drm_connector.h        | 13 +++++++++++++
 4 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2f9ebddd178e..186febcf4e55 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -321,7 +321,7 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector,
 	if (WARN_ON(connector->encoder))
 		return -EINVAL;
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
+	for (i = 0; i < ARRAY_SIZE(connector->encoder_ids); i++) {
 		if (connector->encoder_ids[i] == 0) {
 			connector->encoder_ids[i] = encoder->base.id;
 			return 0;
@@ -1708,22 +1708,19 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	if (!connector)
 		return -ENOENT;
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
-		if (connector->encoder_ids[i] != 0)
-			encoders_count++;
+	drm_connector_for_each_possible_encoder(connector, encoder, i)
+		encoders_count++;
 
 	if ((out_resp->count_encoders >= encoders_count) && encoders_count) {
 		copied = 0;
 		encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);
-		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-			if (connector->encoder_ids[i] != 0) {
-				if (put_user(connector->encoder_ids[i],
-					     encoder_ptr + copied)) {
-					ret = -EFAULT;
-					goto out;
-				}
-				copied++;
+
+		drm_connector_for_each_possible_encoder(connector, encoder, i) {
+			if (put_user(encoder->base.id, encoder_ptr + copied)) {
+				ret = -EFAULT;
+				goto out;
 			}
+			copied++;
 		}
 	}
 	out_resp->count_encoders = encoders_count;
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b37f06317d51..d697c1c4a067 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2326,17 +2326,10 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
 static bool connector_has_possible_crtc(struct drm_connector *connector,
 					struct drm_crtc *crtc)
 {
+	struct drm_encoder *encoder;
 	int i;
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		struct drm_encoder *encoder;
-
-		if (connector->encoder_ids[i] == 0)
-			break;
-
-		encoder = drm_encoder_find(connector->dev, NULL,
-					   connector->encoder_ids[i]);
-
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		if (encoder->possible_crtcs & drm_crtc_mask(crtc))
 			return true;
 	}
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 527743394150..1a901fe9e23e 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -88,9 +88,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
 			    struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
-	uint32_t *ids = connector->encoder_ids;
 	enum drm_mode_status ret = MODE_OK;
-	unsigned int i;
+	struct drm_encoder *encoder;
+	int i;
 
 	/* Step 1: Validate against connector */
 	ret = drm_connector_mode_valid(connector, mode);
@@ -98,13 +98,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
 		return ret;
 
 	/* Step 2: Validate against encoders and crtcs */
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		struct drm_encoder *encoder = drm_encoder_find(dev, NULL, ids[i]);
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		struct drm_crtc *crtc;
 
-		if (!encoder)
-			continue;
-
 		ret = drm_encoder_mode_valid(encoder, mode);
 		if (ret != MODE_OK) {
 			/* No point in continuing for crtc check as this encoder
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 14ab58ade87f..8de3a3aa516a 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1193,4 +1193,17 @@ void drm_connector_list_iter_end(struct drm_connector_list_iter *iter);
 #define drm_for_each_connector_iter(connector, iter) \
 	while ((connector = drm_connector_list_iter_next(iter)))
 
+/**
+ * drm_connector_for_each_possible_encoder - iterate connector's possible encoders
+ * @connector: &struct drm_connector pointer used as cursor
+ * @encoder: the encoder
+ * @__i: int iteration cursor, for macro-internal use
+ */
+#define drm_connector_for_each_possible_encoder(connector, encoder, __i) \
+	for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
+		     (connector)->encoder_ids[(__i)] != 0; (__i)++) \
+		for_each_if((encoder) = \
+			    drm_encoder_find((connector)->dev, NULL, \
+					     (connector)->encoder_ids[(__i)])) \
+
 #endif
-- 
2.16.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 4/9] drm/amdgpu: Use drm_connector_for_each_possible_encoder()

[Ville Syrjala]

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

Use drm_connector_for_each_possible_encoder() for iterating
connector->encoder_ids[]. A bit more convenient not having
to deal with the implementation details.

v2: Replace drm_for_each_connector_encoder_ids() with
    drm_connector_for_each_possible_encoder() (Daniel)

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com> #v1
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 81 +++++++-------------------
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c       | 15 ++---
 2 files changed, 25 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 8e66851eb427..881f7cb7ae6e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -212,30 +212,21 @@ static void
 amdgpu_connector_update_scratch_regs(struct drm_connector *connector,
 				      enum drm_connector_status status)
 {
-	struct drm_encoder *best_encoder = NULL;
-	struct drm_encoder *encoder = NULL;
+	struct drm_encoder *best_encoder;
+	struct drm_encoder *encoder;
 	const struct drm_connector_helper_funcs *connector_funcs = connector->helper_private;
 	bool connected;
 	int i;
 
 	best_encoder = connector_funcs->best_encoder(connector);
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		if (connector->encoder_ids[i] == 0)
-			break;
-
-		encoder = drm_encoder_find(connector->dev, NULL,
-					connector->encoder_ids[i]);
-		if (!encoder)
-			continue;
-
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		if ((encoder == best_encoder) && (status == connector_status_connected))
 			connected = true;
 		else
 			connected = false;
 
 		amdgpu_atombios_encoder_set_bios_scratch_regs(connector, encoder, connected);
-
 	}
 }
 
@@ -246,17 +237,11 @@ amdgpu_connector_find_encoder(struct drm_connector *connector,
 	struct drm_encoder *encoder;
 	int i;
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		if (connector->encoder_ids[i] == 0)
-			break;
-		encoder = drm_encoder_find(connector->dev, NULL,
-					connector->encoder_ids[i]);
-		if (!encoder)
-			continue;
-
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		if (encoder->encoder_type == encoder_type)
 			return encoder;
 	}
+
 	return NULL;
 }
 
@@ -360,11 +345,13 @@ static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector)
 static struct drm_encoder *
 amdgpu_connector_best_single_encoder(struct drm_connector *connector)
 {
-	int enc_id = connector->encoder_ids[0];
+	struct drm_encoder *encoder;
+	int i;
+
+	/* pick the first one */
+	drm_connector_for_each_possible_encoder(connector, encoder, i)
+		return encoder;
 
-	/* pick the encoder ids */
-	if (enc_id)
-		return drm_encoder_find(connector->dev, NULL, enc_id);
 	return NULL;
 }
 
@@ -985,9 +972,8 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
 	struct drm_device *dev = connector->dev;
 	struct amdgpu_device *adev = dev->dev_private;
 	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
-	struct drm_encoder *encoder = NULL;
 	const struct drm_encoder_helper_funcs *encoder_funcs;
-	int i, r;
+	int r;
 	enum drm_connector_status ret = connector_status_disconnected;
 	bool dret = false, broken_edid = false;
 
@@ -1077,14 +1063,10 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
 
 	/* find analog encoder */
 	if (amdgpu_connector->dac_load_detect) {
-		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-			if (connector->encoder_ids[i] == 0)
-				break;
-
-			encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
-			if (!encoder)
-				continue;
+		struct drm_encoder *encoder;
+		int i;
 
+		drm_connector_for_each_possible_encoder(connector, encoder, i) {
 			if (encoder->encoder_type != DRM_MODE_ENCODER_DAC &&
 			    encoder->encoder_type != DRM_MODE_ENCODER_TVDAC)
 				continue;
@@ -1132,18 +1114,11 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
 static struct drm_encoder *
 amdgpu_connector_dvi_encoder(struct drm_connector *connector)
 {
-	int enc_id = connector->encoder_ids[0];
 	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
 	struct drm_encoder *encoder;
 	int i;
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		if (connector->encoder_ids[i] == 0)
-			break;
-
-		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
-		if (!encoder)
-			continue;
 
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		if (amdgpu_connector->use_digital == true) {
 			if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)
 				return encoder;
@@ -1158,8 +1133,9 @@ amdgpu_connector_dvi_encoder(struct drm_connector *connector)
 
 	/* then check use digitial */
 	/* pick the first one */
-	if (enc_id)
-		return drm_encoder_find(connector->dev, NULL, enc_id);
+	drm_connector_for_each_possible_encoder(connector, encoder, i)
+		return encoder;
+
 	return NULL;
 }
 
@@ -1296,15 +1272,7 @@ u16 amdgpu_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn
 	struct amdgpu_encoder *amdgpu_encoder;
 	int i;
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		if (connector->encoder_ids[i] == 0)
-			break;
-
-		encoder = drm_encoder_find(connector->dev, NULL,
-					connector->encoder_ids[i]);
-		if (!encoder)
-			continue;
-
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		amdgpu_encoder = to_amdgpu_encoder(encoder);
 
 		switch (amdgpu_encoder->encoder_id) {
@@ -1326,14 +1294,7 @@ static bool amdgpu_connector_encoder_is_hbr2(struct drm_connector *connector)
 	int i;
 	bool found = false;
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		if (connector->encoder_ids[i] == 0)
-			break;
-		encoder = drm_encoder_find(connector->dev, NULL,
-					connector->encoder_ids[i]);
-		if (!encoder)
-			continue;
-
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		amdgpu_encoder = to_amdgpu_encoder(encoder);
 		if (amdgpu_encoder->caps & ATOM_ENCODER_CAP_RECORD_HBR2)
 			found = true;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index dbf2ccd0c744..016f15093173 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -269,25 +269,18 @@ static int dce_virtual_early_init(void *handle)
 static struct drm_encoder *
 dce_virtual_encoder(struct drm_connector *connector)
 {
-	int enc_id = connector->encoder_ids[0];
 	struct drm_encoder *encoder;
 	int i;
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		if (connector->encoder_ids[i] == 0)
-			break;
-
-		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
-		if (!encoder)
-			continue;
-
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL)
 			return encoder;
 	}
 
 	/* pick the first one */
-	if (enc_id)
-		return drm_encoder_find(connector->dev, NULL, enc_id);
+	drm_connector_for_each_possible_encoder(connector, encoder, i)
+		return encoder;
+
 	return NULL;
 }
 
-- 
2.16.4

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

[PATCH v2 5/9] drm/nouveau: Use drm_connector_for_each_possible_encoder()

[Ville Syrjala]

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

Use drm_connector_for_each_possible_encoder() for iterating
connector->encoder_ids[]. A bit more convenient not having
to deal with the implementation details.

v2: Replace drm_for_each_connector_encoder_ids() with
    drm_connector_for_each_possible_encoder() (Daniel)

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com> #v1
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 7b557c354307..28d7b42cd666 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -363,19 +363,11 @@ module_param_named(hdmimhz, nouveau_hdmimhz, int, 0400);
 struct nouveau_encoder *
 find_encoder(struct drm_connector *connector, int type)
 {
-	struct drm_device *dev = connector->dev;
 	struct nouveau_encoder *nv_encoder;
 	struct drm_encoder *enc;
-	int i, id;
-
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		id = connector->encoder_ids[i];
-		if (!id)
-			break;
+	int i;
 
-		enc = drm_encoder_find(dev, NULL, id);
-		if (!enc)
-			continue;
+	drm_connector_for_each_possible_encoder(connector, enc, i) {
 		nv_encoder = nouveau_encoder(enc);
 
 		if (type == DCB_OUTPUT_ANY ||
@@ -436,14 +428,7 @@ nouveau_connector_ddc_detect(struct drm_connector *connector)
 		}
 	}
 
-	for (i = 0; nv_encoder = NULL, i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		int id = connector->encoder_ids[i];
-		if (id == 0)
-			break;
-
-		encoder = drm_encoder_find(dev, NULL, id);
-		if (!encoder)
-			continue;
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		nv_encoder = nouveau_encoder(encoder);
 
 		if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
-- 
2.16.4

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[PATCH v2 6/9] drm/radeon: Use drm_connector_for_each_possible_encoder()

[Ville Syrjala]

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

Use drm_connector_for_each_possible_encoder() for iterating
connector->encoder_ids[]. A bit more convenient not having
to deal with the implementation details.

v2: Replace drm_for_each_connector_encoder_ids() with
    drm_connector_for_each_possible_encoder() (Daniel)

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com> #v1
---
 drivers/gpu/drm/radeon/radeon_connectors.c | 90 +++++++++---------------------
 1 file changed, 26 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 2aea2bdff99b..0655698f2956 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -244,23 +244,15 @@ radeon_connector_update_scratch_regs(struct drm_connector *connector, enum drm_c
 {
 	struct drm_device *dev = connector->dev;
 	struct radeon_device *rdev = dev->dev_private;
-	struct drm_encoder *best_encoder = NULL;
-	struct drm_encoder *encoder = NULL;
+	struct drm_encoder *best_encoder;
+	struct drm_encoder *encoder;
 	const struct drm_connector_helper_funcs *connector_funcs = connector->helper_private;
 	bool connected;
 	int i;
 
 	best_encoder = connector_funcs->best_encoder(connector);
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		if (connector->encoder_ids[i] == 0)
-			break;
-
-		encoder = drm_encoder_find(connector->dev, NULL,
-					   connector->encoder_ids[i]);
-		if (!encoder)
-			continue;
-
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		if ((encoder == best_encoder) && (status == connector_status_connected))
 			connected = true;
 		else
@@ -270,7 +262,6 @@ radeon_connector_update_scratch_regs(struct drm_connector *connector, enum drm_c
 			radeon_atombios_connected_scratch_regs(connector, encoder, connected);
 		else
 			radeon_combios_connected_scratch_regs(connector, encoder, connected);
-
 	}
 }
 
@@ -279,17 +270,11 @@ static struct drm_encoder *radeon_find_encoder(struct drm_connector *connector,
 	struct drm_encoder *encoder;
 	int i;
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		if (connector->encoder_ids[i] == 0)
-			break;
-
-		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
-		if (!encoder)
-			continue;
-
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		if (encoder->encoder_type == encoder_type)
 			return encoder;
 	}
+
 	return NULL;
 }
 
@@ -393,10 +378,13 @@ static int radeon_ddc_get_modes(struct drm_connector *connector)
 
 static struct drm_encoder *radeon_best_single_encoder(struct drm_connector *connector)
 {
-	int enc_id = connector->encoder_ids[0];
-	/* pick the encoder ids */
-	if (enc_id)
-		return drm_encoder_find(connector->dev, NULL, enc_id);
+	struct drm_encoder *encoder;
+	int i;
+
+	/* pick the first one */
+	drm_connector_for_each_possible_encoder(connector, encoder, i)
+		return encoder;
+
 	return NULL;
 }
 
@@ -436,19 +424,19 @@ radeon_connector_analog_encoder_conflict_solve(struct drm_connector *connector,
 	struct drm_device *dev = connector->dev;
 	struct drm_connector *conflict;
 	struct radeon_connector *radeon_conflict;
-	int i;
 
 	list_for_each_entry(conflict, &dev->mode_config.connector_list, head) {
+		struct drm_encoder *enc;
+		int i;
+
 		if (conflict == connector)
 			continue;
 
 		radeon_conflict = to_radeon_connector(conflict);
-		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-			if (conflict->encoder_ids[i] == 0)
-				break;
 
+		drm_connector_for_each_possible_encoder(conflict, enc, i) {
 			/* if the IDs match */
-			if (conflict->encoder_ids[i] == encoder->base.id) {
+			if (enc == encoder) {
 				if (conflict->status != connector_status_connected)
 					continue;
 
@@ -1256,7 +1244,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
 	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
 	struct drm_encoder *encoder = NULL;
 	const struct drm_encoder_helper_funcs *encoder_funcs;
-	int i, r;
+	int r;
 	enum drm_connector_status ret = connector_status_disconnected;
 	bool dret = false, broken_edid = false;
 
@@ -1374,15 +1362,9 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
 
 	/* find analog encoder */
 	if (radeon_connector->dac_load_detect) {
-		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-			if (connector->encoder_ids[i] == 0)
-				break;
-
-			encoder = drm_encoder_find(connector->dev, NULL,
-						   connector->encoder_ids[i]);
-			if (!encoder)
-				continue;
+		int i;
 
+		drm_connector_for_each_possible_encoder(connector, encoder, i) {
 			if (encoder->encoder_type != DRM_MODE_ENCODER_DAC &&
 			    encoder->encoder_type != DRM_MODE_ENCODER_TVDAC)
 				continue;
@@ -1458,18 +1440,11 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
 /* okay need to be smart in here about which encoder to pick */
 static struct drm_encoder *radeon_dvi_encoder(struct drm_connector *connector)
 {
-	int enc_id = connector->encoder_ids[0];
 	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
 	struct drm_encoder *encoder;
 	int i;
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		if (connector->encoder_ids[i] == 0)
-			break;
-
-		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
-		if (!encoder)
-			continue;
 
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		if (radeon_connector->use_digital == true) {
 			if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)
 				return encoder;
@@ -1484,8 +1459,9 @@ static struct drm_encoder *radeon_dvi_encoder(struct drm_connector *connector)
 
 	/* then check use digitial */
 	/* pick the first one */
-	if (enc_id)
-		return drm_encoder_find(connector->dev, NULL, enc_id);
+	drm_connector_for_each_possible_encoder(connector, encoder, i)
+		return encoder;
+
 	return NULL;
 }
 
@@ -1628,14 +1604,7 @@ u16 radeon_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn
 	struct radeon_encoder *radeon_encoder;
 	int i;
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		if (connector->encoder_ids[i] == 0)
-			break;
-
-		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
-		if (!encoder)
-			continue;
-
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		radeon_encoder = to_radeon_encoder(encoder);
 
 		switch (radeon_encoder->encoder_id) {
@@ -1657,14 +1626,7 @@ static bool radeon_connector_encoder_is_hbr2(struct drm_connector *connector)
 	int i;
 	bool found = false;
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		if (connector->encoder_ids[i] == 0)
-			break;
-
-		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
-		if (!encoder)
-			continue;
-
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		radeon_encoder = to_radeon_encoder(encoder);
 		if (radeon_encoder->caps & ATOM_ENCODER_CAP_RECORD_HBR2)
 			found = true;
-- 
2.16.4

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

[PATCH v2 7/9] drm: Add drm_connector_has_possible_encoder()

[Ville Syrjala]

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

Add a small helper for checking whether a connector and
encoder are associated with each other.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_connector.c | 23 +++++++++++++++++++++++
 include/drm/drm_connector.h     |  3 +++
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 186febcf4e55..84ff2fcdcb55 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -331,6 +331,29 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_mode_connector_attach_encoder);
 
+/**
+ * drm_connector_has_possible_encoder - check if the connector and encoder are assosicated with each other
+ * @connector: the connector
+ * @encoder: the encoder
+ *
+ * Returns:
+ * True if @encoder is one of the possible encoders for @connector.
+ */
+bool drm_connector_has_possible_encoder(struct drm_connector *connector,
+					struct drm_encoder *encoder)
+{
+	struct drm_encoder *enc;
+	int i;
+
+	drm_connector_for_each_possible_encoder(connector, enc, i) {
+		if (enc == encoder)
+			return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(drm_connector_has_possible_encoder);
+
 static void drm_mode_remove(struct drm_connector *connector,
 			    struct drm_display_mode *mode)
 {
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 8de3a3aa516a..9c59485fec36 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1181,6 +1181,9 @@ struct drm_connector *
 drm_connector_list_iter_next(struct drm_connector_list_iter *iter);
 void drm_connector_list_iter_end(struct drm_connector_list_iter *iter);
 
+bool drm_connector_has_possible_encoder(struct drm_connector *connector,
+					struct drm_encoder *encoder);
+
 /**
  * drm_for_each_connector_iter - connector_list iterator macro
  * @connector: &struct drm_connector pointer used as cursor
-- 
2.16.4

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

[PATCH v2 8/9] drm/msm: Use drm_connector_has_possible_encoder()

[Ville Syrjala]

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

Use drm_connector_has_possible_encoder() for checking
whether the encoder has an associated connector.

v2: Replace the drm_for_each_connector_encoder_ids() loop
    with a simple drm_connector_has_possible_encoder() call

Cc: Rob Clark <robdclark@gmail.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 4cb1cb68878b..4beba3f7d067 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -751,12 +751,8 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id)
 	connector_list = &dev->mode_config.connector_list;
 
 	list_for_each_entry(connector, connector_list, head) {
-		int i;
-
-		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-			if (connector->encoder_ids[i] == encoder->base.id)
-				return connector;
-		}
+		if (drm_connector_has_possible_encoder(connector, encoder))
+			return connector;
 	}
 
 	return ERR_PTR(-ENODEV);
-- 
2.16.4

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 9/9] drm/tilcdc: Use drm_connector_has_possible_encoder()

[Ville Syrjala]

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

Use drm_connector_has_possible_encoder() for checking
whether the encoder has an associated connector.

v2: Replace the drm_for_each_connector_encoder_ids() loop
    with a simple drm_connector_has_possible_encoder() call

Cc: Jyri Sarha <jsarha@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_external.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
index d651bdd6597e..b4eaf9bc87f8 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
@@ -103,12 +103,11 @@ struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev,
 						    struct drm_encoder *encoder)
 {
 	struct drm_connector *connector;
-	int i;
 
-	list_for_each_entry(connector, &ddev->mode_config.connector_list, head)
-		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
-			if (connector->encoder_ids[i] == encoder->base.id)
-				return connector;
+	list_for_each_entry(connector, &ddev->mode_config.connector_list, head) {
+		if (drm_connector_has_possible_encoder(connector, encoder))
+			return connector;
+	}
 
 	dev_err(ddev->dev, "No connector found for %s encoder (id %d)\n",
 		encoder->name, encoder->base.id);
-- 
2.16.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.CHECKPATCH: warning for drm: Third attempt at fixing the fb-helper .best_encoder() mess

[Patchwork]

== Series Details ==

Series: drm: Third attempt at fixing the fb-helper .best_encoder() mess
URL   : https://patchwork.freedesktop.org/series/45580/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f29c8d5f4a73 drm/fb-helper: Eliminate the .best_encoder() usage
4433a1a84d27 drm/i915: Nuke intel_mst_best_encoder()
a2b721972a5a drm: Add drm_connector_for_each_possible_encoder()
-:139: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'connector' - possible side-effects?
#139: FILE: include/drm/drm_connector.h:1202:
+#define drm_connector_for_each_possible_encoder(connector, encoder, __i) \
+	for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
+		     (connector)->encoder_ids[(__i)] != 0; (__i)++) \
+		for_each_if((encoder) = \
+			    drm_encoder_find((connector)->dev, NULL, \
+					     (connector)->encoder_ids[(__i)])) \
+

-:139: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__i' - possible side-effects?
#139: FILE: include/drm/drm_connector.h:1202:
+#define drm_connector_for_each_possible_encoder(connector, encoder, __i) \
+	for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
+		     (connector)->encoder_ids[(__i)] != 0; (__i)++) \
+		for_each_if((encoder) = \
+			    drm_encoder_find((connector)->dev, NULL, \
+					     (connector)->encoder_ids[(__i)])) \
+

total: 0 errors, 0 warnings, 2 checks, 99 lines checked
90693dfa299a drm/amdgpu: Use drm_connector_for_each_possible_encoder()
591478288a74 drm/nouveau: Use drm_connector_for_each_possible_encoder()
b47d629c12f3 drm/radeon: Use drm_connector_for_each_possible_encoder()
2e06ee14a7ea drm: Add drm_connector_has_possible_encoder()
-:23: WARNING:LONG_LINE_COMMENT: line over 100 characters
#23: FILE: drivers/gpu/drm/drm_connector.c:335:
+ * drm_connector_has_possible_encoder - check if the connector and encoder are assosicated with each other

total: 0 errors, 1 warnings, 0 checks, 38 lines checked
d081e0a72bd8 drm/msm: Use drm_connector_has_possible_encoder()
2dfef99446dc drm/tilcdc: Use drm_connector_has_possible_encoder()

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

Re: [PATCH v2 9/9] drm/tilcdc: Use drm_connector_has_possible_encoder()

[Jyri Sarha]

On 28/06/18 16:13, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Use drm_connector_has_possible_encoder() for checking
> whether the encoder has an associated connector.
> 
> v2: Replace the drm_for_each_connector_encoder_ids() loop
>     with a simple drm_connector_has_possible_encoder() call
> 
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Acked-by: Jyri Sarha <jsarha@ti.com>

I guess you are going to merge these at one go trough drm-misc.

> ---
>  drivers/gpu/drm/tilcdc/tilcdc_external.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> index d651bdd6597e..b4eaf9bc87f8 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> @@ -103,12 +103,11 @@ struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev,
>  						    struct drm_encoder *encoder)
>  {
>  	struct drm_connector *connector;
> -	int i;
>  
> -	list_for_each_entry(connector, &ddev->mode_config.connector_list, head)
> -		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
> -			if (connector->encoder_ids[i] == encoder->base.id)
> -				return connector;
> +	list_for_each_entry(connector, &ddev->mode_config.connector_list, head) {
> +		if (drm_connector_has_possible_encoder(connector, encoder))
> +			return connector;
> +	}
>  
>  	dev_err(ddev->dev, "No connector found for %s encoder (id %d)\n",
>  		encoder->name, encoder->base.id);
> 


-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/9] drm: Third attempt at fixing the fb-helper .best_encoder() mess

[Ville Syrjälä]

On Thu, Jun 28, 2018 at 04:13:06PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Changes from the previous version mainly involve Danoie's suggestion

Can't type today either: "Daniel's"

> of hiding the drm_encoder_find() in the iterator macro. I also polished
> the msm and tilcdc cases a bit more with another small helper.
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: freedreno@lists.freedesktop.org
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: nouveau@lists.freedesktop.org
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> Ville Syrjälä (9):
>   drm/fb-helper: Eliminate the .best_encoder() usage
>   drm/i915: Nuke intel_mst_best_encoder()
>   drm: Add drm_connector_for_each_possible_encoder()
>   drm/amdgpu: Use drm_connector_for_each_possible_encoder()
>   drm/nouveau: Use drm_connector_for_each_possible_encoder()
>   drm/radeon: Use drm_connector_for_each_possible_encoder()
>   drm: Add drm_connector_has_possible_encoder()
>   drm/msm: Use drm_connector_has_possible_encoder()
>   drm/tilcdc: Use drm_connector_has_possible_encoder()
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 81 ++++++-----------------
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c       | 15 ++---
>  drivers/gpu/drm/drm_connector.c                | 44 +++++++++----
>  drivers/gpu/drm/drm_fb_helper.c                | 34 +++++-----
>  drivers/gpu/drm/drm_probe_helper.c             | 10 +--
>  drivers/gpu/drm/i915/intel_dp_mst.c            | 10 ---
>  drivers/gpu/drm/msm/dsi/dsi_manager.c          |  8 +--
>  drivers/gpu/drm/nouveau/nouveau_connector.c    | 21 +-----
>  drivers/gpu/drm/radeon/radeon_connectors.c     | 90 ++++++++------------------
>  drivers/gpu/drm/tilcdc/tilcdc_external.c       |  9 ++-
>  include/drm/drm_connector.h                    | 16 +++++
>  11 files changed, 128 insertions(+), 210 deletions(-)
> 
> -- 
> 2.16.4

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

✓ Fi.CI.BAT: success for drm: Third attempt at fixing the fb-helper .best_encoder() mess

[Patchwork]

== Series Details ==

Series: drm: Third attempt at fixing the fb-helper .best_encoder() mess
URL   : https://patchwork.freedesktop.org/series/45580/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4397 -> Patchwork_9469 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45580/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9469 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-FAIL (fdo#106103, fdo#102614)

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (44 -> 39) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4397 -> Patchwork_9469

  CI_DRM_4397: 7306233935b0e426454e8adcf09a8022faa03cbc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4530: 0e98bf69f146eb72fe3a7c3b19a049b5786f0ca3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9469: 2dfef99446dcb4c640823c7ac3b5149d12f327e7 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2dfef99446dc drm/tilcdc: Use drm_connector_has_possible_encoder()
d081e0a72bd8 drm/msm: Use drm_connector_has_possible_encoder()
2e06ee14a7ea drm: Add drm_connector_has_possible_encoder()
b47d629c12f3 drm/radeon: Use drm_connector_for_each_possible_encoder()
591478288a74 drm/nouveau: Use drm_connector_for_each_possible_encoder()
90693dfa299a drm/amdgpu: Use drm_connector_for_each_possible_encoder()
a2b721972a5a drm: Add drm_connector_for_each_possible_encoder()
4433a1a84d27 drm/i915: Nuke intel_mst_best_encoder()
f29c8d5f4a73 drm/fb-helper: Eliminate the .best_encoder() usage

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9469/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/9] drm/fb-helper: Eliminate the .best_encoder() usage

[Alex Deucher]

On Thu, Jun 28, 2018 at 9:13 AM, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Instead of using the .best_encoder() hook to figure out whether a given
> connector+crtc combo will work, let's instead do what userspace does and
> just iterate over all the encoders for the connector, and then check
> each crtc against each encoder's possible_crtcs bitmask.
>
> v2: Avoid oopsing on NULL encoders (Daniel)
>     s/connector_crtc_ok/connector_has_possible_crtc/
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com> #v1
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 41 ++++++++++++++++++++++++-----------------
>  1 file changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index cab14f253384..b37f06317d51 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2323,6 +2323,27 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
>         return true;
>  }
>
> +static bool connector_has_possible_crtc(struct drm_connector *connector,
> +                                       struct drm_crtc *crtc)
> +{
> +       int i;
> +
> +       for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> +               struct drm_encoder *encoder;
> +
> +               if (connector->encoder_ids[i] == 0)
> +                       break;
> +
> +               encoder = drm_encoder_find(connector->dev, NULL,
> +                                          connector->encoder_ids[i]);
> +
> +               if (encoder->possible_crtcs & drm_crtc_mask(crtc))
> +                       return true;
> +       }
> +
> +       return false;
> +}
> +
>  static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>                           struct drm_fb_helper_crtc **best_crtcs,
>                           struct drm_display_mode **modes,
> @@ -2331,7 +2352,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>         int c, o;
>         struct drm_connector *connector;
>         const struct drm_connector_helper_funcs *connector_funcs;
> -       struct drm_encoder *encoder;
>         int my_score, best_score, score;
>         struct drm_fb_helper_crtc **crtcs, *crtc;
>         struct drm_fb_helper_connector *fb_helper_conn;
> @@ -2362,20 +2382,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>
>         connector_funcs = connector->helper_private;
>
> -       /*
> -        * If the DRM device implements atomic hooks and ->best_encoder() is
> -        * NULL we fallback to the default drm_atomic_helper_best_encoder()
> -        * helper.
> -        */
> -       if (drm_drv_uses_atomic_modeset(fb_helper->dev) &&
> -           !connector_funcs->best_encoder)
> -               encoder = drm_atomic_helper_best_encoder(connector);
> -       else
> -               encoder = connector_funcs->best_encoder(connector);
> -
> -       if (!encoder)
> -               goto out;
> -
>         /*
>          * select a crtc for this connector and then attempt to configure
>          * remaining connectors
> @@ -2383,7 +2389,8 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>         for (c = 0; c < fb_helper->crtc_count; c++) {
>                 crtc = &fb_helper->crtc_info[c];
>
> -               if ((encoder->possible_crtcs & (1 << c)) == 0)
> +               if (!connector_has_possible_crtc(connector,
> +                                                crtc->mode_set.crtc))
>                         continue;
>
>                 for (o = 0; o < n; o++)
> @@ -2410,7 +2417,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>                                sizeof(struct drm_fb_helper_crtc *));
>                 }
>         }
> -out:
> +
>         kfree(crtcs);
>         return best_score;
>  }
> --
> 2.16.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✓ Fi.CI.IGT: success for drm: Third attempt at fixing the fb-helper .best_encoder() mess

[Patchwork]

== Series Details ==

Series: drm: Third attempt at fixing the fb-helper .best_encoder() mess
URL   : https://patchwork.freedesktop.org/series/45580/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4397_full -> Patchwork_9469_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9469_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9469_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9469_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-blt:
      shard-kbl:          PASS -> SKIP

    igt@gem_linear_blits@interruptible:
      shard-apl:          SKIP -> PASS

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_9469_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_mmap_gtt@coherency:
      shard-glk:          NOTRUN -> FAIL (fdo#100587)

    igt@gem_softpin@noreloc-s3:
      shard-kbl:          NOTRUN -> INCOMPLETE (fdo#103665)

    igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          NOTRUN -> FAIL (fdo#105454, fdo#106509)

    igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
      shard-glk:          NOTRUN -> FAIL (fdo#100368)

    igt@kms_flip@plain-flip-fb-recreate:
      shard-glk:          PASS -> FAIL (fdo#100368) +1

    igt@kms_flip_tiling@flip-x-tiled:
      {shard-glk9}:       NOTRUN -> FAIL (fdo#104724)

    igt@perf_pmu@busy-accuracy-98-vcs1:
      shard-snb:          NOTRUN -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_gtt:
      shard-glk:          INCOMPLETE (k.org#198133, fdo#103359) -> PASS

    igt@gem_exec_schedule@preemptive-hang-render:
      shard-snb:          INCOMPLETE (fdo#105411) -> SKIP

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          FAIL (fdo#105189) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#100587 https://bugs.freedesktop.org/show_bug.cgi?id=100587
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4397 -> Patchwork_9469

  CI_DRM_4397: 7306233935b0e426454e8adcf09a8022faa03cbc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4530: 0e98bf69f146eb72fe3a7c3b19a049b5786f0ca3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9469: 2dfef99446dcb4c640823c7ac3b5149d12f327e7 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9469/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/9] drm: Add drm_connector_for_each_possible_encoder()

[Daniel Vetter]

On Thu, Jun 28, 2018 at 04:13:09PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add a convenience macro for iterating connector->encoder_ids[].
> Isolates the users from the implementation details.
> 
> Note that we don't seem to pass the file_priv down to drm_encoder_find()
> because encoders apparently don't get leased. No idea why
> drm_encoder_finc() even takes the file_priv actually.
> 
> Also use ARRAY_SIZE() when populating the array to avoid spreading
> knowledge about the array size all over.
> 
> v2: Hide the drm_encoder_find() in the macro, and
>     rename the macro appropriately (Daniel)
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com> #v1
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_connector.c    | 21 +++++++++------------
>  drivers/gpu/drm/drm_fb_helper.c    | 11 ++---------
>  drivers/gpu/drm/drm_probe_helper.c | 10 +++-------
>  include/drm/drm_connector.h        | 13 +++++++++++++
>  4 files changed, 27 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2f9ebddd178e..186febcf4e55 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -321,7 +321,7 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector,
>  	if (WARN_ON(connector->encoder))
>  		return -EINVAL;
>  
> -	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> +	for (i = 0; i < ARRAY_SIZE(connector->encoder_ids); i++) {
>  		if (connector->encoder_ids[i] == 0) {
>  			connector->encoder_ids[i] = encoder->base.id;
>  			return 0;
> @@ -1708,22 +1708,19 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	if (!connector)
>  		return -ENOENT;
>  
> -	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
> -		if (connector->encoder_ids[i] != 0)
> -			encoders_count++;
> +	drm_connector_for_each_possible_encoder(connector, encoder, i)
> +		encoders_count++;
>  
>  	if ((out_resp->count_encoders >= encoders_count) && encoders_count) {
>  		copied = 0;
>  		encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);
> -		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> -			if (connector->encoder_ids[i] != 0) {
> -				if (put_user(connector->encoder_ids[i],
> -					     encoder_ptr + copied)) {
> -					ret = -EFAULT;
> -					goto out;
> -				}
> -				copied++;
> +
> +		drm_connector_for_each_possible_encoder(connector, encoder, i) {
> +			if (put_user(encoder->base.id, encoder_ptr + copied)) {
> +				ret = -EFAULT;
> +				goto out;
>  			}
> +			copied++;
>  		}
>  	}
>  	out_resp->count_encoders = encoders_count;
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index b37f06317d51..d697c1c4a067 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2326,17 +2326,10 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
>  static bool connector_has_possible_crtc(struct drm_connector *connector,
>  					struct drm_crtc *crtc)
>  {
> +	struct drm_encoder *encoder;
>  	int i;
>  
> -	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> -		struct drm_encoder *encoder;
> -
> -		if (connector->encoder_ids[i] == 0)
> -			break;
> -
> -		encoder = drm_encoder_find(connector->dev, NULL,
> -					   connector->encoder_ids[i]);
> -
> +	drm_connector_for_each_possible_encoder(connector, encoder, i) {
>  		if (encoder->possible_crtcs & drm_crtc_mask(crtc))
>  			return true;
>  	}
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 527743394150..1a901fe9e23e 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -88,9 +88,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
>  			    struct drm_connector *connector)
>  {
>  	struct drm_device *dev = connector->dev;
> -	uint32_t *ids = connector->encoder_ids;
>  	enum drm_mode_status ret = MODE_OK;
> -	unsigned int i;
> +	struct drm_encoder *encoder;
> +	int i;
>  
>  	/* Step 1: Validate against connector */
>  	ret = drm_connector_mode_valid(connector, mode);
> @@ -98,13 +98,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
>  		return ret;
>  
>  	/* Step 2: Validate against encoders and crtcs */
> -	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> -		struct drm_encoder *encoder = drm_encoder_find(dev, NULL, ids[i]);
> +	drm_connector_for_each_possible_encoder(connector, encoder, i) {
>  		struct drm_crtc *crtc;
>  
> -		if (!encoder)
> -			continue;
> -
>  		ret = drm_encoder_mode_valid(encoder, mode);
>  		if (ret != MODE_OK) {
>  			/* No point in continuing for crtc check as this encoder
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 14ab58ade87f..8de3a3aa516a 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1193,4 +1193,17 @@ void drm_connector_list_iter_end(struct drm_connector_list_iter *iter);
>  #define drm_for_each_connector_iter(connector, iter) \
>  	while ((connector = drm_connector_list_iter_next(iter)))
>  
> +/**
> + * drm_connector_for_each_possible_encoder - iterate connector's possible encoders
> + * @connector: &struct drm_connector pointer used as cursor
> + * @encoder: the encoder

Isn't this the other way round? encoder is the cursor, connector is _the_
connector?

Otherwise Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> and ack from
me on the entire series. Much prettier!
-Daniel

> + * @__i: int iteration cursor, for macro-internal use
> + */
> +#define drm_connector_for_each_possible_encoder(connector, encoder, __i) \
> +	for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
> +		     (connector)->encoder_ids[(__i)] != 0; (__i)++) \
> +		for_each_if((encoder) = \
> +			    drm_encoder_find((connector)->dev, NULL, \
> +					     (connector)->encoder_ids[(__i)])) \
> +
>  #endif
> -- 
> 2.16.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/9] drm: Add drm_connector_for_each_possible_encoder()

[Ville Syrjälä]

On Thu, Jun 28, 2018 at 06:56:40PM +0200, Daniel Vetter wrote:
> On Thu, Jun 28, 2018 at 04:13:09PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Add a convenience macro for iterating connector->encoder_ids[].
> > Isolates the users from the implementation details.
> > 
> > Note that we don't seem to pass the file_priv down to drm_encoder_find()
> > because encoders apparently don't get leased. No idea why
> > drm_encoder_finc() even takes the file_priv actually.
> > 
> > Also use ARRAY_SIZE() when populating the array to avoid spreading
> > knowledge about the array size all over.
> > 
> > v2: Hide the drm_encoder_find() in the macro, and
> >     rename the macro appropriately (Daniel)
> > 
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com> #v1
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_connector.c    | 21 +++++++++------------
> >  drivers/gpu/drm/drm_fb_helper.c    | 11 ++---------
> >  drivers/gpu/drm/drm_probe_helper.c | 10 +++-------
> >  include/drm/drm_connector.h        | 13 +++++++++++++
> >  4 files changed, 27 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 2f9ebddd178e..186febcf4e55 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -321,7 +321,7 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector,
> >  	if (WARN_ON(connector->encoder))
> >  		return -EINVAL;
> >  
> > -	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > +	for (i = 0; i < ARRAY_SIZE(connector->encoder_ids); i++) {
> >  		if (connector->encoder_ids[i] == 0) {
> >  			connector->encoder_ids[i] = encoder->base.id;
> >  			return 0;
> > @@ -1708,22 +1708,19 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
> >  	if (!connector)
> >  		return -ENOENT;
> >  
> > -	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
> > -		if (connector->encoder_ids[i] != 0)
> > -			encoders_count++;
> > +	drm_connector_for_each_possible_encoder(connector, encoder, i)
> > +		encoders_count++;
> >  
> >  	if ((out_resp->count_encoders >= encoders_count) && encoders_count) {
> >  		copied = 0;
> >  		encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);
> > -		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > -			if (connector->encoder_ids[i] != 0) {
> > -				if (put_user(connector->encoder_ids[i],
> > -					     encoder_ptr + copied)) {
> > -					ret = -EFAULT;
> > -					goto out;
> > -				}
> > -				copied++;
> > +
> > +		drm_connector_for_each_possible_encoder(connector, encoder, i) {
> > +			if (put_user(encoder->base.id, encoder_ptr + copied)) {
> > +				ret = -EFAULT;
> > +				goto out;
> >  			}
> > +			copied++;
> >  		}
> >  	}
> >  	out_resp->count_encoders = encoders_count;
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index b37f06317d51..d697c1c4a067 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2326,17 +2326,10 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
> >  static bool connector_has_possible_crtc(struct drm_connector *connector,
> >  					struct drm_crtc *crtc)
> >  {
> > +	struct drm_encoder *encoder;
> >  	int i;
> >  
> > -	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > -		struct drm_encoder *encoder;
> > -
> > -		if (connector->encoder_ids[i] == 0)
> > -			break;
> > -
> > -		encoder = drm_encoder_find(connector->dev, NULL,
> > -					   connector->encoder_ids[i]);
> > -
> > +	drm_connector_for_each_possible_encoder(connector, encoder, i) {
> >  		if (encoder->possible_crtcs & drm_crtc_mask(crtc))
> >  			return true;
> >  	}
> > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> > index 527743394150..1a901fe9e23e 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -88,9 +88,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
> >  			    struct drm_connector *connector)
> >  {
> >  	struct drm_device *dev = connector->dev;
> > -	uint32_t *ids = connector->encoder_ids;
> >  	enum drm_mode_status ret = MODE_OK;
> > -	unsigned int i;
> > +	struct drm_encoder *encoder;
> > +	int i;
> >  
> >  	/* Step 1: Validate against connector */
> >  	ret = drm_connector_mode_valid(connector, mode);
> > @@ -98,13 +98,9 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
> >  		return ret;
> >  
> >  	/* Step 2: Validate against encoders and crtcs */
> > -	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > -		struct drm_encoder *encoder = drm_encoder_find(dev, NULL, ids[i]);
> > +	drm_connector_for_each_possible_encoder(connector, encoder, i) {
> >  		struct drm_crtc *crtc;
> >  
> > -		if (!encoder)
> > -			continue;
> > -
> >  		ret = drm_encoder_mode_valid(encoder, mode);
> >  		if (ret != MODE_OK) {
> >  			/* No point in continuing for crtc check as this encoder
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 14ab58ade87f..8de3a3aa516a 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -1193,4 +1193,17 @@ void drm_connector_list_iter_end(struct drm_connector_list_iter *iter);
> >  #define drm_for_each_connector_iter(connector, iter) \
> >  	while ((connector = drm_connector_list_iter_next(iter)))
> >  
> > +/**
> > + * drm_connector_for_each_possible_encoder - iterate connector's possible encoders
> > + * @connector: &struct drm_connector pointer used as cursor
> > + * @encoder: the encoder
> 
> Isn't this the other way round? encoder is the cursor, connector is _the_
> connector?

Yes indeed. Copy-paste fail on my part. Will fix.

> 
> Otherwise Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> and ack from
> me on the entire series. Much prettier!
> -Daniel
> 
> > + * @__i: int iteration cursor, for macro-internal use
> > + */
> > +#define drm_connector_for_each_possible_encoder(connector, encoder, __i) \
> > +	for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
> > +		     (connector)->encoder_ids[(__i)] != 0; (__i)++) \
> > +		for_each_if((encoder) = \
> > +			    drm_encoder_find((connector)->dev, NULL, \
> > +					     (connector)->encoder_ids[(__i)])) \
> > +
> >  #endif
> > -- 
> > 2.16.4
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

Re: [PATCH v2 9/9] drm/tilcdc: Use drm_connector_has_possible_encoder()

[Ville Syrjälä]

On Thu, Jun 28, 2018 at 04:45:50PM +0300, Jyri Sarha wrote:
> On 28/06/18 16:13, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Use drm_connector_has_possible_encoder() for checking
> > whether the encoder has an associated connector.
> > 
> > v2: Replace the drm_for_each_connector_encoder_ids() loop
> >     with a simple drm_connector_has_possible_encoder() call
> > 
> > Cc: Jyri Sarha <jsarha@ti.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Acked-by: Jyri Sarha <jsarha@ti.com>
> 
> I guess you are going to merge these at one go trough drm-misc.

That's the plan. Thanks for the ack.

> 
> > ---
> >  drivers/gpu/drm/tilcdc/tilcdc_external.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> > index d651bdd6597e..b4eaf9bc87f8 100644
> > --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> > +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> > @@ -103,12 +103,11 @@ struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev,
> >  						    struct drm_encoder *encoder)
> >  {
> >  	struct drm_connector *connector;
> > -	int i;
> >  
> > -	list_for_each_entry(connector, &ddev->mode_config.connector_list, head)
> > -		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
> > -			if (connector->encoder_ids[i] == encoder->base.id)
> > -				return connector;
> > +	list_for_each_entry(connector, &ddev->mode_config.connector_list, head) {
> > +		if (drm_connector_has_possible_encoder(connector, encoder))
> > +			return connector;
> > +	}
> >  
> >  	dev_err(ddev->dev, "No connector found for %s encoder (id %d)\n",
> >  		encoder->name, encoder->base.id);
> > 
> 
> 
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

Re: [PATCH v2 5/9] drm/nouveau: Use drm_connector_for_each_possible_encoder()

[Dan Carpenter]

Hi Ville,

Thank you for the patch! Perhaps something to improve:

url:    https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-Third-attempt-at-fixing-the-fb-helper-best_encoder-mess/20180629-014202
base:   git://people.freedesktop.org/~airlied/linux.git drm-next

smatch warnings:
drivers/gpu/drm/nouveau/nouveau_connector.c:461 nouveau_connector_ddc_detect() error: uninitialized symbol 'nv_encoder'.

# https://github.com/0day-ci/linux/commit/7ec8bb65386edfb0b2bdc8e8391eb5e6eac44c06
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 7ec8bb65386edfb0b2bdc8e8391eb5e6eac44c06
vim +/nv_encoder +461 drivers/gpu/drm/nouveau/nouveau_connector.c

6ee738610 Ben Skeggs      2009-12-11  407  
8777c5c11 Ben Skeggs      2014-06-06  408  static struct nouveau_encoder *
8777c5c11 Ben Skeggs      2014-06-06  409  nouveau_connector_ddc_detect(struct drm_connector *connector)
6ee738610 Ben Skeggs      2009-12-11  410  {
6ee738610 Ben Skeggs      2009-12-11  411  	struct drm_device *dev = connector->dev;
1a1841d30 Ben Skeggs      2012-12-10  412  	struct nouveau_connector *nv_connector = nouveau_connector(connector);
77145f1cb Ben Skeggs      2012-07-31  413  	struct nouveau_drm *drm = nouveau_drm(dev);
1167c6bc5 Ben Skeggs      2016-05-18  414  	struct nvkm_gpio *gpio = nvxx_gpio(&drm->client.device);
8777c5c11 Ben Skeggs      2014-06-06  415  	struct nouveau_encoder *nv_encoder;
6d385c0aa Rob Clark       2014-07-17  416  	struct drm_encoder *encoder;
1a1841d30 Ben Skeggs      2012-12-10  417  	int i, panel = -ENODEV;
1a1841d30 Ben Skeggs      2012-12-10  418  
1a1841d30 Ben Skeggs      2012-12-10  419  	/* eDP panels need powering on by us (if the VBIOS doesn't default it
1a1841d30 Ben Skeggs      2012-12-10  420  	 * to on) before doing any AUX channel transactions.  LVDS panel power
1a1841d30 Ben Skeggs      2012-12-10  421  	 * is handled by the SOR itself, and not required for LVDS DDC.
1a1841d30 Ben Skeggs      2012-12-10  422  	 */
1a1841d30 Ben Skeggs      2012-12-10  423  	if (nv_connector->type == DCB_CONNECTOR_eDP) {
2ea7249fe Ben Skeggs      2015-08-20  424  		panel = nvkm_gpio_get(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff);
1a1841d30 Ben Skeggs      2012-12-10  425  		if (panel == 0) {
2ea7249fe Ben Skeggs      2015-08-20  426  			nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, 1);
1a1841d30 Ben Skeggs      2012-12-10  427  			msleep(300);
1a1841d30 Ben Skeggs      2012-12-10  428  		}
1a1841d30 Ben Skeggs      2012-12-10  429  	}
6ee738610 Ben Skeggs      2009-12-11  430  
7ec8bb653 Ville Syrjälä   2018-06-28  431  	drm_connector_for_each_possible_encoder(connector, encoder, i) {
6d385c0aa Rob Clark       2014-07-17  432  		nv_encoder = nouveau_encoder(encoder);
                                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
If we enter the loop that means nv_encoder is non-NULL.  Smatch can't
prove that we always enter the loop in this case for whatever reason but
my guess is that we always do.

4ca2b7120 Francisco Jerez 2010-08-08  433  
8777c5c11 Ben Skeggs      2014-06-06  434  		if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
8777c5c11 Ben Skeggs      2014-06-06  435  			int ret = nouveau_dp_detect(nv_encoder);
52aa30f25 Ben Skeggs      2016-11-04  436  			if (ret == NOUVEAU_DP_MST)
52aa30f25 Ben Skeggs      2016-11-04  437  				return NULL;
52aa30f25 Ben Skeggs      2016-11-04  438  			if (ret == NOUVEAU_DP_SST)
8777c5c11 Ben Skeggs      2014-06-06  439  				break;
8777c5c11 Ben Skeggs      2014-06-06  440  		} else
39c1c9011 Lukas Wunner    2016-01-11  441  		if ((vga_switcheroo_handler_flags() &
39c1c9011 Lukas Wunner    2016-01-11  442  		     VGA_SWITCHEROO_CAN_SWITCH_DDC) &&
39c1c9011 Lukas Wunner    2016-01-11  443  		    nv_encoder->dcb->type == DCB_OUTPUT_LVDS &&
39c1c9011 Lukas Wunner    2016-01-11  444  		    nv_encoder->i2c) {
39c1c9011 Lukas Wunner    2016-01-11  445  			int ret;
39c1c9011 Lukas Wunner    2016-01-11  446  			vga_switcheroo_lock_ddc(dev->pdev);
39c1c9011 Lukas Wunner    2016-01-11  447  			ret = nvkm_probe_i2c(nv_encoder->i2c, 0x50);
39c1c9011 Lukas Wunner    2016-01-11  448  			vga_switcheroo_unlock_ddc(dev->pdev);
39c1c9011 Lukas Wunner    2016-01-11  449  			if (ret)
39c1c9011 Lukas Wunner    2016-01-11  450  				break;
39c1c9011 Lukas Wunner    2016-01-11  451  		} else
8777c5c11 Ben Skeggs      2014-06-06  452  		if (nv_encoder->i2c) {
2aa5eac51 Ben Skeggs      2015-08-20  453  			if (nvkm_probe_i2c(nv_encoder->i2c, 0x50))
1a1841d30 Ben Skeggs      2012-12-10  454  				break;
6ee738610 Ben Skeggs      2009-12-11  455  		}
6ee738610 Ben Skeggs      2009-12-11  456  	}
6ee738610 Ben Skeggs      2009-12-11  457  
1a1841d30 Ben Skeggs      2012-12-10  458  	/* eDP panel not detected, restore panel power GPIO to previous
1a1841d30 Ben Skeggs      2012-12-10  459  	 * state to avoid confusing the SOR for other output types.
1a1841d30 Ben Skeggs      2012-12-10  460  	 */
8777c5c11 Ben Skeggs      2014-06-06 @461  	if (!nv_encoder && panel == 0)
                                                    ^^^^^^^^^^^
So testing for NULL doesn't make sense.  It's either non-NULL if we
entered the loop at least once or it's uninitialized if we didn't enter
the loop.

2ea7249fe Ben Skeggs      2015-08-20  462  		nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, panel);
1a1841d30 Ben Skeggs      2012-12-10  463  
8777c5c11 Ben Skeggs      2014-06-06  464  	return nv_encoder;
6ee738610 Ben Skeggs      2009-12-11  465  }
6ee738610 Ben Skeggs      2009-12-11  466  


---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/9] drm/nouveau: Use drm_connector_for_each_possible_encoder()

[Ville Syrjälä]

On Sat, Jun 30, 2018 at 10:12:21PM +0300, Dan Carpenter wrote:
> Hi Ville,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> url:    https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-Third-attempt-at-fixing-the-fb-helper-best_encoder-mess/20180629-014202
> base:   git://people.freedesktop.org/~airlied/linux.git drm-next
> 
> smatch warnings:
> drivers/gpu/drm/nouveau/nouveau_connector.c:461 nouveau_connector_ddc_detect() error: uninitialized symbol 'nv_encoder'.
> 
> # https://github.com/0day-ci/linux/commit/7ec8bb65386edfb0b2bdc8e8391eb5e6eac44c06
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 7ec8bb65386edfb0b2bdc8e8391eb5e6eac44c06
> vim +/nv_encoder +461 drivers/gpu/drm/nouveau/nouveau_connector.c
> 
> 6ee738610 Ben Skeggs      2009-12-11  407  
> 8777c5c11 Ben Skeggs      2014-06-06  408  static struct nouveau_encoder *
> 8777c5c11 Ben Skeggs      2014-06-06  409  nouveau_connector_ddc_detect(struct drm_connector *connector)
> 6ee738610 Ben Skeggs      2009-12-11  410  {
> 6ee738610 Ben Skeggs      2009-12-11  411  	struct drm_device *dev = connector->dev;
> 1a1841d30 Ben Skeggs      2012-12-10  412  	struct nouveau_connector *nv_connector = nouveau_connector(connector);
> 77145f1cb Ben Skeggs      2012-07-31  413  	struct nouveau_drm *drm = nouveau_drm(dev);
> 1167c6bc5 Ben Skeggs      2016-05-18  414  	struct nvkm_gpio *gpio = nvxx_gpio(&drm->client.device);
> 8777c5c11 Ben Skeggs      2014-06-06  415  	struct nouveau_encoder *nv_encoder;
> 6d385c0aa Rob Clark       2014-07-17  416  	struct drm_encoder *encoder;
> 1a1841d30 Ben Skeggs      2012-12-10  417  	int i, panel = -ENODEV;
> 1a1841d30 Ben Skeggs      2012-12-10  418  
> 1a1841d30 Ben Skeggs      2012-12-10  419  	/* eDP panels need powering on by us (if the VBIOS doesn't default it
> 1a1841d30 Ben Skeggs      2012-12-10  420  	 * to on) before doing any AUX channel transactions.  LVDS panel power
> 1a1841d30 Ben Skeggs      2012-12-10  421  	 * is handled by the SOR itself, and not required for LVDS DDC.
> 1a1841d30 Ben Skeggs      2012-12-10  422  	 */
> 1a1841d30 Ben Skeggs      2012-12-10  423  	if (nv_connector->type == DCB_CONNECTOR_eDP) {
> 2ea7249fe Ben Skeggs      2015-08-20  424  		panel = nvkm_gpio_get(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff);
> 1a1841d30 Ben Skeggs      2012-12-10  425  		if (panel == 0) {
> 2ea7249fe Ben Skeggs      2015-08-20  426  			nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, 1);
> 1a1841d30 Ben Skeggs      2012-12-10  427  			msleep(300);
> 1a1841d30 Ben Skeggs      2012-12-10  428  		}
> 1a1841d30 Ben Skeggs      2012-12-10  429  	}
> 6ee738610 Ben Skeggs      2009-12-11  430  
> 7ec8bb653 Ville Syrjälä   2018-06-28  431  	drm_connector_for_each_possible_encoder(connector, encoder, i) {
> 6d385c0aa Rob Clark       2014-07-17  432  		nv_encoder = nouveau_encoder(encoder);
>                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> If we enter the loop that means nv_encoder is non-NULL.  Smatch can't
> prove that we always enter the loop in this case for whatever reason but
> my guess is that we always do.
> 
> 4ca2b7120 Francisco Jerez 2010-08-08  433  
> 8777c5c11 Ben Skeggs      2014-06-06  434  		if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
> 8777c5c11 Ben Skeggs      2014-06-06  435  			int ret = nouveau_dp_detect(nv_encoder);
> 52aa30f25 Ben Skeggs      2016-11-04  436  			if (ret == NOUVEAU_DP_MST)
> 52aa30f25 Ben Skeggs      2016-11-04  437  				return NULL;
> 52aa30f25 Ben Skeggs      2016-11-04  438  			if (ret == NOUVEAU_DP_SST)
> 8777c5c11 Ben Skeggs      2014-06-06  439  				break;
> 8777c5c11 Ben Skeggs      2014-06-06  440  		} else
> 39c1c9011 Lukas Wunner    2016-01-11  441  		if ((vga_switcheroo_handler_flags() &
> 39c1c9011 Lukas Wunner    2016-01-11  442  		     VGA_SWITCHEROO_CAN_SWITCH_DDC) &&
> 39c1c9011 Lukas Wunner    2016-01-11  443  		    nv_encoder->dcb->type == DCB_OUTPUT_LVDS &&
> 39c1c9011 Lukas Wunner    2016-01-11  444  		    nv_encoder->i2c) {
> 39c1c9011 Lukas Wunner    2016-01-11  445  			int ret;
> 39c1c9011 Lukas Wunner    2016-01-11  446  			vga_switcheroo_lock_ddc(dev->pdev);
> 39c1c9011 Lukas Wunner    2016-01-11  447  			ret = nvkm_probe_i2c(nv_encoder->i2c, 0x50);
> 39c1c9011 Lukas Wunner    2016-01-11  448  			vga_switcheroo_unlock_ddc(dev->pdev);
> 39c1c9011 Lukas Wunner    2016-01-11  449  			if (ret)
> 39c1c9011 Lukas Wunner    2016-01-11  450  				break;
> 39c1c9011 Lukas Wunner    2016-01-11  451  		} else
> 8777c5c11 Ben Skeggs      2014-06-06  452  		if (nv_encoder->i2c) {
> 2aa5eac51 Ben Skeggs      2015-08-20  453  			if (nvkm_probe_i2c(nv_encoder->i2c, 0x50))
> 1a1841d30 Ben Skeggs      2012-12-10  454  				break;
> 6ee738610 Ben Skeggs      2009-12-11  455  		}
> 6ee738610 Ben Skeggs      2009-12-11  456  	}
> 6ee738610 Ben Skeggs      2009-12-11  457  
> 1a1841d30 Ben Skeggs      2012-12-10  458  	/* eDP panel not detected, restore panel power GPIO to previous
> 1a1841d30 Ben Skeggs      2012-12-10  459  	 * state to avoid confusing the SOR for other output types.
> 1a1841d30 Ben Skeggs      2012-12-10  460  	 */
> 8777c5c11 Ben Skeggs      2014-06-06 @461  	if (!nv_encoder && panel == 0)
>                                                     ^^^^^^^^^^^
> So testing for NULL doesn't make sense.  It's either non-NULL if we
> entered the loop at least once or it's uninitialized if we didn't enter
> the loop.

Yeah, seems like a bogus check. I suppose in theory you could have a
connector with no encoders, although I can't think of any good reason
why anyone would do that.

> 
> 2ea7249fe Ben Skeggs      2015-08-20  462  		nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, panel);
> 1a1841d30 Ben Skeggs      2012-12-10  463  
> 8777c5c11 Ben Skeggs      2014-06-06  464  	return nv_encoder;
> 6ee738610 Ben Skeggs      2009-12-11  465  }
> 6ee738610 Ben Skeggs      2009-12-11  466  
> 
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

Re: [PATCH v2 5/9] drm/nouveau: Use drm_connector_for_each_possible_encoder()

[Ville Syrjälä]

On Mon, Jul 02, 2018 at 04:04:45PM +0300, Ville Syrjälä wrote:
> On Sat, Jun 30, 2018 at 10:12:21PM +0300, Dan Carpenter wrote:
> > Hi Ville,
> > 
> > Thank you for the patch! Perhaps something to improve:
> > 
> > url:    https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-Third-attempt-at-fixing-the-fb-helper-best_encoder-mess/20180629-014202
> > base:   git://people.freedesktop.org/~airlied/linux.git drm-next
> > 
> > smatch warnings:
> > drivers/gpu/drm/nouveau/nouveau_connector.c:461 nouveau_connector_ddc_detect() error: uninitialized symbol 'nv_encoder'.
> > 
> > # https://github.com/0day-ci/linux/commit/7ec8bb65386edfb0b2bdc8e8391eb5e6eac44c06
> > git remote add linux-review https://github.com/0day-ci/linux
> > git remote update linux-review
> > git checkout 7ec8bb65386edfb0b2bdc8e8391eb5e6eac44c06
> > vim +/nv_encoder +461 drivers/gpu/drm/nouveau/nouveau_connector.c
> > 
> > 6ee738610 Ben Skeggs      2009-12-11  407  
> > 8777c5c11 Ben Skeggs      2014-06-06  408  static struct nouveau_encoder *
> > 8777c5c11 Ben Skeggs      2014-06-06  409  nouveau_connector_ddc_detect(struct drm_connector *connector)
> > 6ee738610 Ben Skeggs      2009-12-11  410  {
> > 6ee738610 Ben Skeggs      2009-12-11  411  	struct drm_device *dev = connector->dev;
> > 1a1841d30 Ben Skeggs      2012-12-10  412  	struct nouveau_connector *nv_connector = nouveau_connector(connector);
> > 77145f1cb Ben Skeggs      2012-07-31  413  	struct nouveau_drm *drm = nouveau_drm(dev);
> > 1167c6bc5 Ben Skeggs      2016-05-18  414  	struct nvkm_gpio *gpio = nvxx_gpio(&drm->client.device);
> > 8777c5c11 Ben Skeggs      2014-06-06  415  	struct nouveau_encoder *nv_encoder;
> > 6d385c0aa Rob Clark       2014-07-17  416  	struct drm_encoder *encoder;
> > 1a1841d30 Ben Skeggs      2012-12-10  417  	int i, panel = -ENODEV;
> > 1a1841d30 Ben Skeggs      2012-12-10  418  
> > 1a1841d30 Ben Skeggs      2012-12-10  419  	/* eDP panels need powering on by us (if the VBIOS doesn't default it
> > 1a1841d30 Ben Skeggs      2012-12-10  420  	 * to on) before doing any AUX channel transactions.  LVDS panel power
> > 1a1841d30 Ben Skeggs      2012-12-10  421  	 * is handled by the SOR itself, and not required for LVDS DDC.
> > 1a1841d30 Ben Skeggs      2012-12-10  422  	 */
> > 1a1841d30 Ben Skeggs      2012-12-10  423  	if (nv_connector->type == DCB_CONNECTOR_eDP) {
> > 2ea7249fe Ben Skeggs      2015-08-20  424  		panel = nvkm_gpio_get(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff);
> > 1a1841d30 Ben Skeggs      2012-12-10  425  		if (panel == 0) {
> > 2ea7249fe Ben Skeggs      2015-08-20  426  			nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, 1);
> > 1a1841d30 Ben Skeggs      2012-12-10  427  			msleep(300);
> > 1a1841d30 Ben Skeggs      2012-12-10  428  		}
> > 1a1841d30 Ben Skeggs      2012-12-10  429  	}
> > 6ee738610 Ben Skeggs      2009-12-11  430  
> > 7ec8bb653 Ville Syrjälä   2018-06-28  431  	drm_connector_for_each_possible_encoder(connector, encoder, i) {
> > 6d385c0aa Rob Clark       2014-07-17  432  		nv_encoder = nouveau_encoder(encoder);
> >                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > If we enter the loop that means nv_encoder is non-NULL.  Smatch can't
> > prove that we always enter the loop in this case for whatever reason but
> > my guess is that we always do.
> > 
> > 4ca2b7120 Francisco Jerez 2010-08-08  433  
> > 8777c5c11 Ben Skeggs      2014-06-06  434  		if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
> > 8777c5c11 Ben Skeggs      2014-06-06  435  			int ret = nouveau_dp_detect(nv_encoder);
> > 52aa30f25 Ben Skeggs      2016-11-04  436  			if (ret == NOUVEAU_DP_MST)
> > 52aa30f25 Ben Skeggs      2016-11-04  437  				return NULL;
> > 52aa30f25 Ben Skeggs      2016-11-04  438  			if (ret == NOUVEAU_DP_SST)
> > 8777c5c11 Ben Skeggs      2014-06-06  439  				break;
> > 8777c5c11 Ben Skeggs      2014-06-06  440  		} else
> > 39c1c9011 Lukas Wunner    2016-01-11  441  		if ((vga_switcheroo_handler_flags() &
> > 39c1c9011 Lukas Wunner    2016-01-11  442  		     VGA_SWITCHEROO_CAN_SWITCH_DDC) &&
> > 39c1c9011 Lukas Wunner    2016-01-11  443  		    nv_encoder->dcb->type == DCB_OUTPUT_LVDS &&
> > 39c1c9011 Lukas Wunner    2016-01-11  444  		    nv_encoder->i2c) {
> > 39c1c9011 Lukas Wunner    2016-01-11  445  			int ret;
> > 39c1c9011 Lukas Wunner    2016-01-11  446  			vga_switcheroo_lock_ddc(dev->pdev);
> > 39c1c9011 Lukas Wunner    2016-01-11  447  			ret = nvkm_probe_i2c(nv_encoder->i2c, 0x50);
> > 39c1c9011 Lukas Wunner    2016-01-11  448  			vga_switcheroo_unlock_ddc(dev->pdev);
> > 39c1c9011 Lukas Wunner    2016-01-11  449  			if (ret)
> > 39c1c9011 Lukas Wunner    2016-01-11  450  				break;
> > 39c1c9011 Lukas Wunner    2016-01-11  451  		} else
> > 8777c5c11 Ben Skeggs      2014-06-06  452  		if (nv_encoder->i2c) {
> > 2aa5eac51 Ben Skeggs      2015-08-20  453  			if (nvkm_probe_i2c(nv_encoder->i2c, 0x50))
> > 1a1841d30 Ben Skeggs      2012-12-10  454  				break;
> > 6ee738610 Ben Skeggs      2009-12-11  455  		}
> > 6ee738610 Ben Skeggs      2009-12-11  456  	}
> > 6ee738610 Ben Skeggs      2009-12-11  457  
> > 1a1841d30 Ben Skeggs      2012-12-10  458  	/* eDP panel not detected, restore panel power GPIO to previous
> > 1a1841d30 Ben Skeggs      2012-12-10  459  	 * state to avoid confusing the SOR for other output types.
> > 1a1841d30 Ben Skeggs      2012-12-10  460  	 */
> > 8777c5c11 Ben Skeggs      2014-06-06 @461  	if (!nv_encoder && panel == 0)
> >                                                     ^^^^^^^^^^^
> > So testing for NULL doesn't make sense.  It's either non-NULL if we
> > entered the loop at least once or it's uninitialized if we didn't enter
> > the loop.
> 
> Yeah, seems like a bogus check. I suppose in theory you could have a
> connector with no encoders, although I can't think of any good reason
> why anyone would do that.

In fact this thing seems to upset gcc as well. It's not particularly
helpful in its warning message though:

../drivers/gpu/drm/nouveau/nouveau_connector.c: In function ‘nouveau_connector_detect’:
../drivers/gpu/drm/nouveau/nouveau_connector.c:597:33: warning: ‘nv_encoder’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
   if (nv_partner && ((nv_encoder->dcb->type == DCB_OUTPUT_ANALOG &&
                          ~~~~~~~~~~^~~~~

No indication that it's nouveau_connector_ddc_detect() that supposedly
supplies the uninitialized pointer. I think I'll just toss in a =NULL in
there to silence this noise.

> 
> > 
> > 2ea7249fe Ben Skeggs      2015-08-20  462  		nvkm_gpio_set(gpio, 0, DCB_GPIO_PANEL_POWER, 0xff, panel);
> > 1a1841d30 Ben Skeggs      2012-12-10  463  
> > 8777c5c11 Ben Skeggs      2014-06-06  464  	return nv_encoder;
> > 6ee738610 Ben Skeggs      2009-12-11  465  }
> > 6ee738610 Ben Skeggs      2009-12-11  466  
> > 
> > 
> > ---
> > 0-DAY kernel test infrastructure                Open Source Technology Center
> > https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[PATCH v3 5/9] drm/nouveau: Use drm_connector_for_each_possible_encoder()

[Ville Syrjala]

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

Use drm_connector_for_each_possible_encoder() for iterating
connector->encoder_ids[]. A bit more convenient not having
to deal with the implementation details.

v2: Replace drm_for_each_connector_encoder_ids() with
    drm_connector_for_each_possible_encoder() (Daniel)
v3: Initialize nv_encoder to NULL to shut up gcc/smatch

Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com> #v1
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 7b557c354307..bb46c1d489cf 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -363,19 +363,11 @@ module_param_named(hdmimhz, nouveau_hdmimhz, int, 0400);
 struct nouveau_encoder *
 find_encoder(struct drm_connector *connector, int type)
 {
-	struct drm_device *dev = connector->dev;
 	struct nouveau_encoder *nv_encoder;
 	struct drm_encoder *enc;
-	int i, id;
-
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		id = connector->encoder_ids[i];
-		if (!id)
-			break;
+	int i;
 
-		enc = drm_encoder_find(dev, NULL, id);
-		if (!enc)
-			continue;
+	drm_connector_for_each_possible_encoder(connector, enc, i) {
 		nv_encoder = nouveau_encoder(enc);
 
 		if (type == DCB_OUTPUT_ANY ||
@@ -420,7 +412,7 @@ nouveau_connector_ddc_detect(struct drm_connector *connector)
 	struct nouveau_connector *nv_connector = nouveau_connector(connector);
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct nvkm_gpio *gpio = nvxx_gpio(&drm->client.device);
-	struct nouveau_encoder *nv_encoder;
+	struct nouveau_encoder *nv_encoder = NULL;
 	struct drm_encoder *encoder;
 	int i, panel = -ENODEV;
 
@@ -436,14 +428,7 @@ nouveau_connector_ddc_detect(struct drm_connector *connector)
 		}
 	}
 
-	for (i = 0; nv_encoder = NULL, i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		int id = connector->encoder_ids[i];
-		if (id == 0)
-			break;
-
-		encoder = drm_encoder_find(dev, NULL, id);
-		if (!encoder)
-			continue;
+	drm_connector_for_each_possible_encoder(connector, encoder, i) {
 		nv_encoder = nouveau_encoder(encoder);
 
 		if (nv_encoder->dcb->type == DCB_OUTPUT_DP) {
-- 
2.16.4

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

✗ Fi.CI.CHECKPATCH: warning for drm: Third attempt at fixing the fb-helper .best_encoder() mess (rev2)

[Patchwork]

== Series Details ==

Series: drm: Third attempt at fixing the fb-helper .best_encoder() mess (rev2)
URL   : https://patchwork.freedesktop.org/series/45580/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
27feace2e181 drm/fb-helper: Eliminate the .best_encoder() usage
bd08e651f318 drm/i915: Nuke intel_mst_best_encoder()
c31cba14dcaa drm: Add drm_connector_for_each_possible_encoder()
-:139: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'connector' - possible side-effects?
#139: FILE: include/drm/drm_connector.h:1202:
+#define drm_connector_for_each_possible_encoder(connector, encoder, __i) \
+	for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
+		     (connector)->encoder_ids[(__i)] != 0; (__i)++) \
+		for_each_if((encoder) = \
+			    drm_encoder_find((connector)->dev, NULL, \
+					     (connector)->encoder_ids[(__i)])) \
+

-:139: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__i' - possible side-effects?
#139: FILE: include/drm/drm_connector.h:1202:
+#define drm_connector_for_each_possible_encoder(connector, encoder, __i) \
+	for ((__i) = 0; (__i) < ARRAY_SIZE((connector)->encoder_ids) && \
+		     (connector)->encoder_ids[(__i)] != 0; (__i)++) \
+		for_each_if((encoder) = \
+			    drm_encoder_find((connector)->dev, NULL, \
+					     (connector)->encoder_ids[(__i)])) \
+

total: 0 errors, 0 warnings, 2 checks, 99 lines checked
9ac85a361c63 drm/amdgpu: Use drm_connector_for_each_possible_encoder()
8807427952c1 drm/nouveau: Use drm_connector_for_each_possible_encoder()
a84c10f13c19 drm/radeon: Use drm_connector_for_each_possible_encoder()
f2e15e4ea45b drm: Add drm_connector_has_possible_encoder()
-:23: WARNING:LONG_LINE_COMMENT: line over 100 characters
#23: FILE: drivers/gpu/drm/drm_connector.c:335:
+ * drm_connector_has_possible_encoder - check if the connector and encoder are assosicated with each other

total: 0 errors, 1 warnings, 0 checks, 38 lines checked
51b4ba4680b6 drm/msm: Use drm_connector_has_possible_encoder()
1a05980f45a6 drm/tilcdc: Use drm_connector_has_possible_encoder()

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

✓ Fi.CI.BAT: success for drm: Third attempt at fixing the fb-helper .best_encoder() mess (rev2)

[Patchwork]

== Series Details ==

Series: drm: Third attempt at fixing the fb-helper .best_encoder() mess (rev2)
URL   : https://patchwork.freedesktop.org/series/45580/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4413 -> Patchwork_9500 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45580/revisions/2/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9500 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       PASS -> DMESG-WARN (fdo#105128)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128


== Participating hosts (45 -> 40) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4413 -> Patchwork_9500

  CI_DRM_4413: df42fb566d33f5889c8d61f9929d3e72d4a16813 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4532: 840d12e2f050b784552197403d6575a57b6e896d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9500: 1a05980f45a6d7c09fdfdb2761611bee4803cdad @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1a05980f45a6 drm/tilcdc: Use drm_connector_has_possible_encoder()
51b4ba4680b6 drm/msm: Use drm_connector_has_possible_encoder()
f2e15e4ea45b drm: Add drm_connector_has_possible_encoder()
a84c10f13c19 drm/radeon: Use drm_connector_for_each_possible_encoder()
8807427952c1 drm/nouveau: Use drm_connector_for_each_possible_encoder()
9ac85a361c63 drm/amdgpu: Use drm_connector_for_each_possible_encoder()
c31cba14dcaa drm: Add drm_connector_for_each_possible_encoder()
bd08e651f318 drm/i915: Nuke intel_mst_best_encoder()
27feace2e181 drm/fb-helper: Eliminate the .best_encoder() usage

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9500/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm: Third attempt at fixing the fb-helper .best_encoder() mess (rev2)

[Patchwork]

== Series Details ==

Series: drm: Third attempt at fixing the fb-helper .best_encoder() mess (rev2)
URL   : https://patchwork.freedesktop.org/series/45580/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4413_full -> Patchwork_9500_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9500_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9500_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9500_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-render:
      shard-kbl:          PASS -> SKIP

    igt@gem_mocs_settings@mocs-rc6-vebox:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_9500_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-bsd2:
      shard-kbl:          NOTRUN -> FAIL (fdo#103158)

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#102887) +1

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          PASS -> FAIL (fdo#103822, fdo#104724) +1

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)
      shard-kbl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@gem_workarounds@suspend-resume-context:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_flip@plain-flip-fb-recreate:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_flip_tiling@flip-to-y-tiled:
      shard-glk:          FAIL (fdo#103822, fdo#104724) -> PASS

    
    ==== Warnings ====

    igt@drv_selftest@live_gtt:
      shard-glk:          INCOMPLETE -> FAIL (fdo#105347)

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4413 -> Patchwork_9500

  CI_DRM_4413: df42fb566d33f5889c8d61f9929d3e72d4a16813 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4532: 840d12e2f050b784552197403d6575a57b6e896d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9500: 1a05980f45a6d7c09fdfdb2761611bee4803cdad @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9500/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/9] drm/fb-helper: Eliminate the .best_encoder() usage

[Ville Syrjälä]

On Thu, Jun 28, 2018 at 11:27:07AM -0400, Alex Deucher wrote:
> On Thu, Jun 28, 2018 at 9:13 AM, Ville Syrjala
> <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Instead of using the .best_encoder() hook to figure out whether a given
> > connector+crtc combo will work, let's instead do what userspace does and
> > just iterate over all the encoders for the connector, and then check
> > each crtc against each encoder's possible_crtcs bitmask.
> >
> > v2: Avoid oopsing on NULL encoders (Daniel)
> >     s/connector_crtc_ok/connector_has_possible_crtc/
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com> #v1
> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Series is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Thanks for the review. Series pushed to drm-misc-next.

> 
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c | 41 ++++++++++++++++++++++++-----------------
> >  1 file changed, 24 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index cab14f253384..b37f06317d51 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2323,6 +2323,27 @@ static bool drm_target_preferred(struct drm_fb_helper *fb_helper,
> >         return true;
> >  }
> >
> > +static bool connector_has_possible_crtc(struct drm_connector *connector,
> > +                                       struct drm_crtc *crtc)
> > +{
> > +       int i;
> > +
> > +       for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> > +               struct drm_encoder *encoder;
> > +
> > +               if (connector->encoder_ids[i] == 0)
> > +                       break;
> > +
> > +               encoder = drm_encoder_find(connector->dev, NULL,
> > +                                          connector->encoder_ids[i]);
> > +
> > +               if (encoder->possible_crtcs & drm_crtc_mask(crtc))
> > +                       return true;
> > +       }
> > +
> > +       return false;
> > +}
> > +
> >  static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
> >                           struct drm_fb_helper_crtc **best_crtcs,
> >                           struct drm_display_mode **modes,
> > @@ -2331,7 +2352,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
> >         int c, o;
> >         struct drm_connector *connector;
> >         const struct drm_connector_helper_funcs *connector_funcs;
> > -       struct drm_encoder *encoder;
> >         int my_score, best_score, score;
> >         struct drm_fb_helper_crtc **crtcs, *crtc;
> >         struct drm_fb_helper_connector *fb_helper_conn;
> > @@ -2362,20 +2382,6 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
> >
> >         connector_funcs = connector->helper_private;
> >
> > -       /*
> > -        * If the DRM device implements atomic hooks and ->best_encoder() is
> > -        * NULL we fallback to the default drm_atomic_helper_best_encoder()
> > -        * helper.
> > -        */
> > -       if (drm_drv_uses_atomic_modeset(fb_helper->dev) &&
> > -           !connector_funcs->best_encoder)
> > -               encoder = drm_atomic_helper_best_encoder(connector);
> > -       else
> > -               encoder = connector_funcs->best_encoder(connector);
> > -
> > -       if (!encoder)
> > -               goto out;
> > -
> >         /*
> >          * select a crtc for this connector and then attempt to configure
> >          * remaining connectors
> > @@ -2383,7 +2389,8 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
> >         for (c = 0; c < fb_helper->crtc_count; c++) {
> >                 crtc = &fb_helper->crtc_info[c];
> >
> > -               if ((encoder->possible_crtcs & (1 << c)) == 0)
> > +               if (!connector_has_possible_crtc(connector,
> > +                                                crtc->mode_set.crtc))
> >                         continue;
> >
> >                 for (o = 0; o < n; o++)
> > @@ -2410,7 +2417,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
> >                                sizeof(struct drm_fb_helper_crtc *));
> >                 }
> >         }
> > -out:
> > +
> >         kfree(crtcs);
> >         return best_score;
> >  }
> > --
> > 2.16.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

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