[PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[José Roberto de Souza]

This 3 non-atomic drivers all have the same function getting the
only encoder available in the connector, also atomic drivers have
this fallback. So moving it a common place and sharing between atomic
and non-atomic drivers.

While at it I also removed the mention of
drm_atomic_helper_best_encoder() that was renamed in
commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
drm_atomic_helper_best_encoder").

v3: moving drm_connector_get_single_encoder to drm_kms_helper module

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/ast/ast_mode.c             | 12 ------------
 drivers/gpu/drm/drm_atomic_helper.c        | 15 ++-------------
 drivers/gpu/drm/drm_crtc_helper.c          | 17 ++++++++++++++++-
 drivers/gpu/drm/drm_crtc_helper_internal.h |  3 +++
 drivers/gpu/drm/mgag200/mgag200_mode.c     | 11 -----------
 drivers/gpu/drm/udl/udl_connector.c        |  8 --------
 include/drm/drm_modeset_helper_vtables.h   |  6 +++---
 7 files changed, 24 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index d349c721501c..eef95e1af06b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct drm_encoder *encoder)
 	kfree(encoder);
 }
 
-
-static struct drm_encoder *ast_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);
-	return NULL;
-}
-
-
 static const struct drm_encoder_funcs ast_enc_funcs = {
 	.destroy = ast_encoder_destroy,
 };
@@ -847,7 +836,6 @@ static void ast_connector_destroy(struct drm_connector *connector)
 static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
 	.mode_valid = ast_mode_valid,
 	.get_modes = ast_get_modes,
-	.best_encoder = ast_best_single_encoder,
 };
 
 static const struct drm_connector_funcs ast_connector_funcs = {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 4706439fb490..9d7e4da6c292 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
 	}
 }
 
-/*
- * For connectors that support multiple encoders, either the
- * .atomic_best_encoder() or .best_encoder() operation must be implemented.
- */
-static struct drm_encoder *
-pick_single_encoder_for_connector(struct drm_connector *connector)
-{
-	WARN_ON(connector->encoder_ids[1]);
-	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
-}
-
 static int handle_conflicting_encoders(struct drm_atomic_state *state,
 				       bool disable_conflicting_encoders)
 {
@@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 		else if (funcs->best_encoder)
 			new_encoder = funcs->best_encoder(connector);
 		else
-			new_encoder = pick_single_encoder_for_connector(connector);
+			new_encoder = drm_connector_get_single_encoder(connector);
 
 		if (new_encoder) {
 			if (encoder_mask & drm_encoder_mask(new_encoder)) {
@@ -359,7 +348,7 @@ update_connector_routing(struct drm_atomic_state *state,
 	else if (funcs->best_encoder)
 		new_encoder = funcs->best_encoder(connector);
 	else
-		new_encoder = pick_single_encoder_for_connector(connector);
+		new_encoder = drm_connector_get_single_encoder(connector);
 
 	if (!new_encoder) {
 		DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index a51824a7e7c1..4a7447a53cea 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -460,6 +460,17 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
 	__drm_helper_disable_unused_functions(dev);
 }
 
+/*
+ * For connectors that support multiple encoders, either the
+ * .atomic_best_encoder() or .best_encoder() operation must be implemented.
+ */
+struct drm_encoder *
+drm_connector_get_single_encoder(struct drm_connector *connector)
+{
+	WARN_ON(connector->encoder_ids[1]);
+	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
+}
+
 /**
  * drm_crtc_helper_set_config - set a new config from userspace
  * @set: mode set configuration
@@ -625,7 +636,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 		new_encoder = connector->encoder;
 		for (ro = 0; ro < set->num_connectors; ro++) {
 			if (set->connectors[ro] == connector) {
-				new_encoder = connector_funcs->best_encoder(connector);
+				if (connector_funcs->best_encoder)
+					new_encoder = connector_funcs->best_encoder(connector);
+				else
+					new_encoder = drm_connector_get_single_encoder(connector);
+
 				/* if we can't get an encoder for a connector
 				   we are setting now - then fail */
 				if (new_encoder == NULL)
diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h b/drivers/gpu/drm/drm_crtc_helper_internal.h
index b5ac1581e623..f0a66ef47e5a 100644
--- a/drivers/gpu/drm/drm_crtc_helper_internal.h
+++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
@@ -75,3 +75,6 @@ enum drm_mode_status drm_encoder_mode_valid(struct drm_encoder *encoder,
 					    const struct drm_display_mode *mode);
 enum drm_mode_status drm_connector_mode_valid(struct drm_connector *connector,
 					      struct drm_display_mode *mode);
+
+struct drm_encoder *
+drm_connector_get_single_encoder(struct drm_connector *connector);
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 5e778b5f1a10..68226556044b 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1638,16 +1638,6 @@ static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector,
 	return MODE_OK;
 }
 
-static struct drm_encoder *mga_connector_best_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);
-	return NULL;
-}
-
 static void mga_connector_destroy(struct drm_connector *connector)
 {
 	struct mga_connector *mga_connector = to_mga_connector(connector);
@@ -1659,7 +1649,6 @@ static void mga_connector_destroy(struct drm_connector *connector)
 static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs = {
 	.get_modes = mga_vga_get_modes,
 	.mode_valid = mga_vga_mode_valid,
-	.best_encoder = mga_connector_best_encoder,
 };
 
 static const struct drm_connector_funcs mga_vga_connector_funcs = {
diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
index ddb61a60c610..b4ae3e89a7b4 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -90,13 +90,6 @@ udl_detect(struct drm_connector *connector, bool force)
 	return connector_status_connected;
 }
 
-static struct drm_encoder*
-udl_best_single_encoder(struct drm_connector *connector)
-{
-	int enc_id = connector->encoder_ids[0];
-	return drm_encoder_find(connector->dev, NULL, enc_id);
-}
-
 static int udl_connector_set_property(struct drm_connector *connector,
 				      struct drm_property *property,
 				      uint64_t val)
@@ -120,7 +113,6 @@ static void udl_connector_destroy(struct drm_connector *connector)
 static const struct drm_connector_helper_funcs udl_connector_helper_funcs = {
 	.get_modes = udl_get_modes,
 	.mode_valid = udl_mode_valid,
-	.best_encoder = udl_best_single_encoder,
 };
 
 static const struct drm_connector_funcs udl_connector_funcs = {
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 6b18c8adfe9d..b55412c6ce3a 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -955,8 +955,8 @@ struct drm_connector_helper_funcs {
 	 * @atomic_best_encoder.
 	 *
 	 * You can leave this function to NULL if the connector is only
-	 * attached to a single encoder and you are using the atomic helpers.
-	 * In this case, the core will call drm_atomic_helper_best_encoder()
+	 * attached to a single encoder.
+	 * In this case, the core will call drm_connector_get_single_encoder()
 	 * for you.
 	 *
 	 * RETURNS:
@@ -977,7 +977,7 @@ struct drm_connector_helper_funcs {
 	 *
 	 * This function is used by drm_atomic_helper_check_modeset().
 	 * If it is not implemented, the core will fallback to @best_encoder
-	 * (or drm_atomic_helper_best_encoder() if @best_encoder is NULL).
+	 * (or drm_connector_get_single_encoder() if @best_encoder is NULL).
 	 *
 	 * NOTE:
 	 *
-- 
2.23.0

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

[PATCH 2/2] drm/connector: Allow max possible encoders to attach to a connector

[José Roberto de Souza]

Currently we restrict the number of encoders that can be linked to
a connector to 3, increase it to match the maximum number of encoders
that can be initialized(32).

To more effiently do that lets switch from an array of encoder ids to
bitmask.

v2: Fixing missed return on amdgpu_dm_connector_to_encoder()

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 23 +++++---------
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c      |  5 ++-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  8 ++++-
 drivers/gpu/drm/drm_client_modeset.c          |  3 +-
 drivers/gpu/drm/drm_connector.c               | 31 +++++--------------
 drivers/gpu/drm/drm_crtc_helper.c             |  9 ++++--
 drivers/gpu/drm/drm_probe_helper.c            |  3 +-
 drivers/gpu/drm/nouveau/dispnv04/disp.c       |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c       |  2 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c   |  7 ++---
 drivers/gpu/drm/radeon/radeon_connectors.c    | 27 ++++++----------
 include/drm/drm_connector.h                   | 18 +++++------
 12 files changed, 55 insertions(+), 83 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index ece55c8fa673..d8729285f731 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -217,11 +217,10 @@ amdgpu_connector_update_scratch_regs(struct drm_connector *connector,
 	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);
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		if ((encoder == best_encoder) && (status == connector_status_connected))
 			connected = true;
 		else
@@ -236,9 +235,8 @@ amdgpu_connector_find_encoder(struct drm_connector *connector,
 			       int encoder_type)
 {
 	struct drm_encoder *encoder;
-	int i;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		if (encoder->encoder_type == encoder_type)
 			return encoder;
 	}
@@ -347,10 +345,9 @@ static struct drm_encoder *
 amdgpu_connector_best_single_encoder(struct drm_connector *connector)
 {
 	struct drm_encoder *encoder;
-	int i;
 
 	/* pick the first one */
-	drm_connector_for_each_possible_encoder(connector, encoder, i)
+	drm_connector_for_each_possible_encoder(connector, encoder)
 		return encoder;
 
 	return NULL;
@@ -1065,9 +1062,8 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
 	/* find analog encoder */
 	if (amdgpu_connector->dac_load_detect) {
 		struct drm_encoder *encoder;
-		int i;
 
-		drm_connector_for_each_possible_encoder(connector, encoder, i) {
+		drm_connector_for_each_possible_encoder(connector, encoder) {
 			if (encoder->encoder_type != DRM_MODE_ENCODER_DAC &&
 			    encoder->encoder_type != DRM_MODE_ENCODER_TVDAC)
 				continue;
@@ -1117,9 +1113,8 @@ amdgpu_connector_dvi_encoder(struct drm_connector *connector)
 {
 	struct amdgpu_connector *amdgpu_connector = to_amdgpu_connector(connector);
 	struct drm_encoder *encoder;
-	int i;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		if (amdgpu_connector->use_digital == true) {
 			if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)
 				return encoder;
@@ -1134,7 +1129,7 @@ amdgpu_connector_dvi_encoder(struct drm_connector *connector)
 
 	/* then check use digitial */
 	/* pick the first one */
-	drm_connector_for_each_possible_encoder(connector, encoder, i)
+	drm_connector_for_each_possible_encoder(connector, encoder)
 		return encoder;
 
 	return NULL;
@@ -1271,9 +1266,8 @@ u16 amdgpu_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn
 {
 	struct drm_encoder *encoder;
 	struct amdgpu_encoder *amdgpu_encoder;
-	int i;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		amdgpu_encoder = to_amdgpu_encoder(encoder);
 
 		switch (amdgpu_encoder->encoder_id) {
@@ -1292,10 +1286,9 @@ static bool amdgpu_connector_encoder_is_hbr2(struct drm_connector *connector)
 {
 	struct drm_encoder *encoder;
 	struct amdgpu_encoder *amdgpu_encoder;
-	int i;
 	bool found = false;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		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 c9608ae8643b..e4f94863332c 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -260,15 +260,14 @@ static struct drm_encoder *
 dce_virtual_encoder(struct drm_connector *connector)
 {
 	struct drm_encoder *encoder;
-	int i;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL)
 			return encoder;
 	}
 
 	/* pick the first one */
-	drm_connector_for_each_possible_encoder(connector, encoder, i)
+	drm_connector_for_each_possible_encoder(connector, encoder)
 		return encoder;
 
 	return NULL;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index e1b09bb432bd..860c29c489c2 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4836,7 +4836,13 @@ static int to_drm_connector_type(enum signal_type st)
 
 static struct drm_encoder *amdgpu_dm_connector_to_encoder(struct drm_connector *connector)
 {
-	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
+	struct drm_encoder *encoder;
+
+	/* There is only one encoder per connector */
+	drm_connector_for_each_possible_encoder(connector, encoder)
+		return encoder;
+
+	return NULL;
 }
 
 static void amdgpu_dm_get_native_mode(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index c8922b7cac09..895b73f23079 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -415,9 +415,8 @@ static bool connector_has_possible_crtc(struct drm_connector *connector,
 					struct drm_crtc *crtc)
 {
 	struct drm_encoder *encoder;
-	int i;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		if (encoder->possible_crtcs & drm_crtc_mask(crtc))
 			return true;
 	}
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4c766624b20d..43896c711b50 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -365,8 +365,6 @@ EXPORT_SYMBOL(drm_connector_attach_edid_property);
 int drm_connector_attach_encoder(struct drm_connector *connector,
 				 struct drm_encoder *encoder)
 {
-	int i;
-
 	/*
 	 * In the past, drivers have attempted to model the static association
 	 * of connector to encoder in simple connector/encoder devices using a
@@ -381,18 +379,15 @@ int drm_connector_attach_encoder(struct drm_connector *connector,
 	if (WARN_ON(connector->encoder))
 		return -EINVAL;
 
-	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;
-		}
-	}
-	return -ENOMEM;
+	connector->possible_encoders |= drm_encoder_mask(encoder);
+
+	return 0;
 }
 EXPORT_SYMBOL(drm_connector_attach_encoder);
 
 /**
- * drm_connector_has_possible_encoder - check if the connector and encoder are assosicated with each other
+ * drm_connector_has_possible_encoder - check if the connector and encoder are
+ * associated with each other
  * @connector: the connector
  * @encoder: the encoder
  *
@@ -402,15 +397,7 @@ EXPORT_SYMBOL(drm_connector_attach_encoder);
 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;
+	return connector->possible_encoders & drm_encoder_mask(encoder);
 }
 EXPORT_SYMBOL(drm_connector_has_possible_encoder);
 
@@ -2121,7 +2108,6 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	int encoders_count = 0;
 	int ret = 0;
 	int copied = 0;
-	int i;
 	struct drm_mode_modeinfo u_mode;
 	struct drm_mode_modeinfo __user *mode_ptr;
 	uint32_t __user *encoder_ptr;
@@ -2136,14 +2122,13 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	if (!connector)
 		return -ENOENT;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i)
-		encoders_count++;
+	encoders_count = hweight32(connector->possible_encoders);
 
 	if ((out_resp->count_encoders >= encoders_count) && encoders_count) {
 		copied = 0;
 		encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);
 
-		drm_connector_for_each_possible_encoder(connector, encoder, i) {
+		drm_connector_for_each_possible_encoder(connector, encoder) {
 			if (put_user(encoder->base.id, encoder_ptr + copied)) {
 				ret = -EFAULT;
 				goto out;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 4a7447a53cea..499b05aaccfc 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -467,8 +467,13 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
 struct drm_encoder *
 drm_connector_get_single_encoder(struct drm_connector *connector)
 {
-	WARN_ON(connector->encoder_ids[1]);
-	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
+	struct drm_encoder *encoder;
+
+	WARN_ON(hweight32(connector->possible_encoders) > 1);
+	drm_connector_for_each_possible_encoder(connector, encoder)
+		return encoder;
+
+	return NULL;
 }
 
 /**
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 351cbc40f0f8..a7c87abe88d0 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -93,7 +93,6 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
 	struct drm_device *dev = connector->dev;
 	enum drm_mode_status ret = MODE_OK;
 	struct drm_encoder *encoder;
-	int i;
 
 	/* Step 1: Validate against connector */
 	ret = drm_connector_mode_valid(connector, mode);
@@ -101,7 +100,7 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode,
 		return ret;
 
 	/* Step 2: Validate against encoders and crtcs */
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		struct drm_crtc *crtc;
 
 		ret = drm_encoder_mode_valid(encoder, mode);
diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c b/drivers/gpu/drm/nouveau/dispnv04/disp.c
index dc64863b5fd8..44ee82d0c9b6 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
@@ -256,7 +256,7 @@ nv04_display_create(struct drm_device *dev)
 
 	list_for_each_entry_safe(connector, ct,
 				 &dev->mode_config.connector_list, head) {
-		if (!connector->encoder_ids[0]) {
+		if (!connector->possible_encoders) {
 			NV_WARN(drm, "%s has no encoders, removing\n",
 				connector->name);
 			connector->funcs->destroy(connector);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 307584107d77..98c87195d711 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2391,7 +2391,7 @@ nv50_display_create(struct drm_device *dev)
 
 	/* cull any connectors we created that don't have an encoder */
 	list_for_each_entry_safe(connector, tmp, &dev->mode_config.connector_list, head) {
-		if (connector->encoder_ids[0])
+		if (connector->possible_encoders)
 			continue;
 
 		NV_WARN(drm, "%s has no encoders, removing\n",
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 56871d34e3fb..3a5db17bc5c7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -365,9 +365,8 @@ find_encoder(struct drm_connector *connector, int type)
 {
 	struct nouveau_encoder *nv_encoder;
 	struct drm_encoder *enc;
-	int i;
 
-	drm_connector_for_each_possible_encoder(connector, enc, i) {
+	drm_connector_for_each_possible_encoder(connector, enc) {
 		nv_encoder = nouveau_encoder(enc);
 
 		if (type == DCB_OUTPUT_ANY ||
@@ -414,10 +413,10 @@ nouveau_connector_ddc_detect(struct drm_connector *connector)
 	struct drm_device *dev = connector->dev;
 	struct nouveau_encoder *nv_encoder = NULL, *found = NULL;
 	struct drm_encoder *encoder;
-	int i, ret;
+	int ret;
 	bool switcheroo_ddc = false;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		nv_encoder = nouveau_encoder(encoder);
 
 		switch (nv_encoder->dcb->type) {
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index b684cd719612..c07427d3c199 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -249,11 +249,10 @@ radeon_connector_update_scratch_regs(struct drm_connector *connector, enum drm_c
 	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);
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		if ((encoder == best_encoder) && (status == connector_status_connected))
 			connected = true;
 		else
@@ -269,9 +268,8 @@ radeon_connector_update_scratch_regs(struct drm_connector *connector, enum drm_c
 static struct drm_encoder *radeon_find_encoder(struct drm_connector *connector, int encoder_type)
 {
 	struct drm_encoder *encoder;
-	int i;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		if (encoder->encoder_type == encoder_type)
 			return encoder;
 	}
@@ -380,10 +378,9 @@ static int radeon_ddc_get_modes(struct drm_connector *connector)
 static struct drm_encoder *radeon_best_single_encoder(struct drm_connector *connector)
 {
 	struct drm_encoder *encoder;
-	int i;
 
 	/* pick the first one */
-	drm_connector_for_each_possible_encoder(connector, encoder, i)
+	drm_connector_for_each_possible_encoder(connector, encoder)
 		return encoder;
 
 	return NULL;
@@ -428,14 +425,13 @@ radeon_connector_analog_encoder_conflict_solve(struct drm_connector *connector,
 
 	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);
 
-		drm_connector_for_each_possible_encoder(conflict, enc, i) {
+		drm_connector_for_each_possible_encoder(conflict, enc) {
 			/* if the IDs match */
 			if (enc == encoder) {
 				if (conflict->status != connector_status_connected)
@@ -1363,9 +1359,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
 
 	/* find analog encoder */
 	if (radeon_connector->dac_load_detect) {
-		int i;
-
-		drm_connector_for_each_possible_encoder(connector, encoder, i) {
+		drm_connector_for_each_possible_encoder(connector, encoder) {
 			if (encoder->encoder_type != DRM_MODE_ENCODER_DAC &&
 			    encoder->encoder_type != DRM_MODE_ENCODER_TVDAC)
 				continue;
@@ -1443,9 +1437,8 @@ static struct drm_encoder *radeon_dvi_encoder(struct drm_connector *connector)
 {
 	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
 	struct drm_encoder *encoder;
-	int i;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		if (radeon_connector->use_digital == true) {
 			if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)
 				return encoder;
@@ -1460,7 +1453,7 @@ static struct drm_encoder *radeon_dvi_encoder(struct drm_connector *connector)
 
 	/* then check use digitial */
 	/* pick the first one */
-	drm_connector_for_each_possible_encoder(connector, encoder, i)
+	drm_connector_for_each_possible_encoder(connector, encoder)
 		return encoder;
 
 	return NULL;
@@ -1603,9 +1596,8 @@ u16 radeon_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn
 {
 	struct drm_encoder *encoder;
 	struct radeon_encoder *radeon_encoder;
-	int i;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		radeon_encoder = to_radeon_encoder(encoder);
 
 		switch (radeon_encoder->encoder_id) {
@@ -1624,10 +1616,9 @@ static bool radeon_connector_encoder_is_hbr2(struct drm_connector *connector)
 {
 	struct drm_encoder *encoder;
 	struct radeon_encoder *radeon_encoder;
-	int i;
 	bool found = false;
 
-	drm_connector_for_each_possible_encoder(connector, encoder, i) {
+	drm_connector_for_each_possible_encoder(connector, encoder) {
 		radeon_encoder = to_radeon_encoder(encoder);
 		if (radeon_encoder->caps & ATOM_ENCODER_CAP_RECORD_HBR2)
 			found = true;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 681cb590f952..c6e993e78dbd 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1288,12 +1288,12 @@ struct drm_connector {
 	/** @override_edid: has the EDID been overwritten through debugfs for testing? */
 	bool override_edid;
 
-#define DRM_CONNECTOR_MAX_ENCODER 3
 	/**
-	 * @encoder_ids: Valid encoders for this connector. Please only use
-	 * drm_connector_for_each_possible_encoder() to enumerate these.
+	 * @possible_encoders: Bit mask of encoders that can drive this
+	 * connector, drm_encoder_index() determines the index into the bitfield
+	 * and the bits are set with drm_connector_attach_encoder().
 	 */
-	uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER];
+	u32 possible_encoders;
 
 	/**
 	 * @encoder: Currently bound encoder driving this connector, if any.
@@ -1608,13 +1608,9 @@ bool drm_connector_has_possible_encoder(struct drm_connector *connector,
  * drm_connector_for_each_possible_encoder - iterate connector's possible encoders
  * @connector: &struct drm_connector pointer
  * @encoder: &struct drm_encoder pointer used as cursor
- * @__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)])) \
+#define drm_connector_for_each_possible_encoder(connector, encoder) \
+	drm_for_each_encoder_mask(encoder, (connector)->dev, \
+				  (connector)->possible_encoders)
 
 #endif
-- 
2.23.0

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder
URL   : https://patchwork.freedesktop.org/series/66619/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
87c2a5a45bc0 drm/connector: Share with non-atomic drivers the function to get the single encoder
fbd5f383c05e drm/connector: Allow max possible encoders to attach to a connector
-:497: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'connector' - possible side-effects?
#497: FILE: include/drm/drm_connector.h:1612:
+#define drm_connector_for_each_possible_encoder(connector, encoder) \
+	drm_for_each_encoder_mask(encoder, (connector)->dev, \
+				  (connector)->possible_encoders)

total: 0 errors, 0 warnings, 1 checks, 390 lines checked

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

✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder
URL   : https://patchwork.freedesktop.org/series/66619/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/connector: Share with non-atomic drivers the function to get the single encoder
+drivers/gpu/drm/drm_crtc_helper.c:467:20: warning: symbol 'drm_connector_get_single_encoder' was not declared. Should it be static?

Commit: drm/connector: Allow max possible encoders to attach to a connector
+drivers/gpu/drm/drm_crtc_helper.c:467:20: warning: symbol 'drm_connector_get_single_encoder' was not declared. Should it be static?
-O:drivers/gpu/drm/drm_crtc_helper.c:467:20: warning: symbol 'drm_connector_get_single_encoder' was not declared. Should it be static?

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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder
URL   : https://patchwork.freedesktop.org/series/66619/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6885 -> Patchwork_14387
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/

Possible new issues
-------------------

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_close_race@basic-threads:
    - {fi-tgl-u2}:        [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/fi-tgl-u2/igt@gem_close_race@basic-threads.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-tgl-u2/igt@gem_close_race@basic-threads.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - {fi-tgl-u}:         NOTRUN -> [SKIP][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-tgl-u/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_pm_rpm@basic-rte:
    - {fi-tgl-u}:         NOTRUN -> [FAIL][4]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-tgl-u/igt@i915_pm_rpm@basic-rte.html

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_create@basic-files:
    - fi-bxt-dsi:         [PASS][5] -> [INCOMPLETE][6] ([fdo#103927])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/fi-bxt-dsi/igt@gem_ctx_create@basic-files.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-bxt-dsi/igt@gem_ctx_create@basic-files.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [PASS][7] -> [INCOMPLETE][8] ([fdo#107718])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@prime_vgem@basic-read:
    - fi-icl-u3:          [PASS][9] -> [DMESG-WARN][10] ([fdo#107724])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/fi-icl-u3/igt@prime_vgem@basic-read.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-icl-u3/igt@prime_vgem@basic-read.html

  
#### Possible fixes ####

  * igt@debugfs_test@read_all_entries:
    - {fi-tgl-u2}:        [DMESG-WARN][11] ([fdo#111600]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/fi-tgl-u2/igt@debugfs_test@read_all_entries.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-tgl-u2/igt@debugfs_test@read_all_entries.html

  * igt@gem_exec_suspend@basic-s3:
    - {fi-tgl-u}:         [INCOMPLETE][13] -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/fi-tgl-u/igt@gem_exec_suspend@basic-s3.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-tgl-u/igt@gem_exec_suspend@basic-s3.html

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u4}:        [FAIL][15] ([fdo#103167]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-icl-u4/igt@kms_frontbuffer_tracking@basic.html
    - fi-icl-u2:          [FAIL][17] ([fdo#103167]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  * igt@prime_vgem@basic-busy-default:
    - fi-icl-u3:          [DMESG-WARN][19] ([fdo#107724]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/fi-icl-u3/igt@prime_vgem@basic-busy-default.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-icl-u3/igt@prime_vgem@basic-busy-default.html

  
#### Warnings ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][21] ([fdo#111096]) -> [FAIL][22] ([fdo#111407])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

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

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [fdo#111600]: https://bugs.freedesktop.org/show_bug.cgi?id=111600


Participating hosts (55 -> 48)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6885 -> Patchwork_14387

  CI-20190529: 20190529
  CI_DRM_6885: 11786d27cb029a083556ac9b82e33d74e250ce26 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5178: efb4539494d94f03374874d3b61bd04ef3802aaa @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14387: fbd5f383c05e7c0e137fa0a1d4110a676cbbad6f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

fbd5f383c05e drm/connector: Allow max possible encoders to attach to a connector
87c2a5a45bc0 drm/connector: Share with non-atomic drivers the function to get the single encoder

== Logs ==

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

Re: [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[Ville Syrjälä]

On Thu, Sep 12, 2019 at 12:51:31PM -0700, José Roberto de Souza wrote:
> This 3 non-atomic drivers all have the same function getting the
> only encoder available in the connector, also atomic drivers have
> this fallback. So moving it a common place and sharing between atomic
> and non-atomic drivers.
> 
> While at it I also removed the mention of
> drm_atomic_helper_best_encoder() that was renamed in
> commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
> drm_atomic_helper_best_encoder").
> 
> v3: moving drm_connector_get_single_encoder to drm_kms_helper module
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/ast/ast_mode.c             | 12 ------------
>  drivers/gpu/drm/drm_atomic_helper.c        | 15 ++-------------
>  drivers/gpu/drm/drm_crtc_helper.c          | 17 ++++++++++++++++-
>  drivers/gpu/drm/drm_crtc_helper_internal.h |  3 +++
>  drivers/gpu/drm/mgag200/mgag200_mode.c     | 11 -----------
>  drivers/gpu/drm/udl/udl_connector.c        |  8 --------
>  include/drm/drm_modeset_helper_vtables.h   |  6 +++---
>  7 files changed, 24 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index d349c721501c..eef95e1af06b 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct drm_encoder *encoder)
>  	kfree(encoder);
>  }
>  
> -
> -static struct drm_encoder *ast_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);
> -	return NULL;
> -}
> -
> -
>  static const struct drm_encoder_funcs ast_enc_funcs = {
>  	.destroy = ast_encoder_destroy,
>  };
> @@ -847,7 +836,6 @@ static void ast_connector_destroy(struct drm_connector *connector)
>  static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
>  	.mode_valid = ast_mode_valid,
>  	.get_modes = ast_get_modes,
> -	.best_encoder = ast_best_single_encoder,
>  };
>  
>  static const struct drm_connector_funcs ast_connector_funcs = {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 4706439fb490..9d7e4da6c292 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
>  	}
>  }
>  
> -/*
> - * For connectors that support multiple encoders, either the
> - * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> - */
> -static struct drm_encoder *
> -pick_single_encoder_for_connector(struct drm_connector *connector)
> -{
> -	WARN_ON(connector->encoder_ids[1]);
> -	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
> -}
> -
>  static int handle_conflicting_encoders(struct drm_atomic_state *state,
>  				       bool disable_conflicting_encoders)
>  {
> @@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
>  		else if (funcs->best_encoder)
>  			new_encoder = funcs->best_encoder(connector);
>  		else
> -			new_encoder = pick_single_encoder_for_connector(connector);
> +			new_encoder = drm_connector_get_single_encoder(connector);
>  
>  		if (new_encoder) {
>  			if (encoder_mask & drm_encoder_mask(new_encoder)) {
> @@ -359,7 +348,7 @@ update_connector_routing(struct drm_atomic_state *state,
>  	else if (funcs->best_encoder)
>  		new_encoder = funcs->best_encoder(connector);
>  	else
> -		new_encoder = pick_single_encoder_for_connector(connector);
> +		new_encoder = drm_connector_get_single_encoder(connector);
>  
>  	if (!new_encoder) {
>  		DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index a51824a7e7c1..4a7447a53cea 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -460,6 +460,17 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
>  	__drm_helper_disable_unused_functions(dev);
>  }
>  
> +/*
> + * For connectors that support multiple encoders, either the
> + * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> + */
> +struct drm_encoder *
> +drm_connector_get_single_encoder(struct drm_connector *connector)
> +{
> +	WARN_ON(connector->encoder_ids[1]);
> +	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
> +}
> +
>  /**
>   * drm_crtc_helper_set_config - set a new config from userspace
>   * @set: mode set configuration
> @@ -625,7 +636,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
>  		new_encoder = connector->encoder;
>  		for (ro = 0; ro < set->num_connectors; ro++) {
>  			if (set->connectors[ro] == connector) {
> -				new_encoder = connector_funcs->best_encoder(connector);
> +				if (connector_funcs->best_encoder)
> +					new_encoder = connector_funcs->best_encoder(connector);
> +				else
> +					new_encoder = drm_connector_get_single_encoder(connector);
> +
>  				/* if we can't get an encoder for a connector
>  				   we are setting now - then fail */
>  				if (new_encoder == NULL)
> diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h b/drivers/gpu/drm/drm_crtc_helper_internal.h
> index b5ac1581e623..f0a66ef47e5a 100644
> --- a/drivers/gpu/drm/drm_crtc_helper_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
> @@ -75,3 +75,6 @@ enum drm_mode_status drm_encoder_mode_valid(struct drm_encoder *encoder,
>  					    const struct drm_display_mode *mode);
>  enum drm_mode_status drm_connector_mode_valid(struct drm_connector *connector,
>  					      struct drm_display_mode *mode);
> +
> +struct drm_encoder *
> +drm_connector_get_single_encoder(struct drm_connector *connector);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 5e778b5f1a10..68226556044b 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1638,16 +1638,6 @@ static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector,
>  	return MODE_OK;
>  }
>  
> -static struct drm_encoder *mga_connector_best_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);
> -	return NULL;
> -}
> -
>  static void mga_connector_destroy(struct drm_connector *connector)
>  {
>  	struct mga_connector *mga_connector = to_mga_connector(connector);
> @@ -1659,7 +1649,6 @@ static void mga_connector_destroy(struct drm_connector *connector)
>  static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs = {
>  	.get_modes = mga_vga_get_modes,
>  	.mode_valid = mga_vga_mode_valid,
> -	.best_encoder = mga_connector_best_encoder,
>  };
>  
>  static const struct drm_connector_funcs mga_vga_connector_funcs = {
> diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
> index ddb61a60c610..b4ae3e89a7b4 100644
> --- a/drivers/gpu/drm/udl/udl_connector.c
> +++ b/drivers/gpu/drm/udl/udl_connector.c
> @@ -90,13 +90,6 @@ udl_detect(struct drm_connector *connector, bool force)
>  	return connector_status_connected;
>  }
>  
> -static struct drm_encoder*
> -udl_best_single_encoder(struct drm_connector *connector)
> -{
> -	int enc_id = connector->encoder_ids[0];
> -	return drm_encoder_find(connector->dev, NULL, enc_id);
> -}
> -
>  static int udl_connector_set_property(struct drm_connector *connector,
>  				      struct drm_property *property,
>  				      uint64_t val)
> @@ -120,7 +113,6 @@ static void udl_connector_destroy(struct drm_connector *connector)
>  static const struct drm_connector_helper_funcs udl_connector_helper_funcs = {
>  	.get_modes = udl_get_modes,
>  	.mode_valid = udl_mode_valid,
> -	.best_encoder = udl_best_single_encoder,
>  };
>  
>  static const struct drm_connector_funcs udl_connector_funcs = {
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 6b18c8adfe9d..b55412c6ce3a 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -955,8 +955,8 @@ struct drm_connector_helper_funcs {
>  	 * @atomic_best_encoder.
>  	 *
>  	 * You can leave this function to NULL if the connector is only
> -	 * attached to a single encoder and you are using the atomic helpers.
> -	 * In this case, the core will call drm_atomic_helper_best_encoder()
> +	 * attached to a single encoder.
> +	 * In this case, the core will call drm_connector_get_single_encoder()
>  	 * for you.
>  	 *
>  	 * RETURNS:
> @@ -977,7 +977,7 @@ struct drm_connector_helper_funcs {
>  	 *
>  	 * This function is used by drm_atomic_helper_check_modeset().
>  	 * If it is not implemented, the core will fallback to @best_encoder
> -	 * (or drm_atomic_helper_best_encoder() if @best_encoder is NULL).
> +	 * (or drm_connector_get_single_encoder() if @best_encoder is NULL).
>  	 *
>  	 * NOTE:
>  	 *
> -- 
> 2.23.0

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

Re: [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[Laurent Pinchart]

Hi José,

Thank you for the patch.

On Thu, Sep 12, 2019 at 12:51:31PM -0700, José Roberto de Souza wrote:
> This 3 non-atomic drivers all have the same function getting the
> only encoder available in the connector, also atomic drivers have
> this fallback. So moving it a common place and sharing between atomic
> and non-atomic drivers.
> 
> While at it I also removed the mention of
> drm_atomic_helper_best_encoder() that was renamed in
> commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
> drm_atomic_helper_best_encoder").
> 
> v3: moving drm_connector_get_single_encoder to drm_kms_helper module
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/ast/ast_mode.c             | 12 ------------
>  drivers/gpu/drm/drm_atomic_helper.c        | 15 ++-------------
>  drivers/gpu/drm/drm_crtc_helper.c          | 17 ++++++++++++++++-
>  drivers/gpu/drm/drm_crtc_helper_internal.h |  3 +++
>  drivers/gpu/drm/mgag200/mgag200_mode.c     | 11 -----------
>  drivers/gpu/drm/udl/udl_connector.c        |  8 --------
>  include/drm/drm_modeset_helper_vtables.h   |  6 +++---
>  7 files changed, 24 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index d349c721501c..eef95e1af06b 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct drm_encoder *encoder)
>  	kfree(encoder);
>  }
>  
> -
> -static struct drm_encoder *ast_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);
> -	return NULL;
> -}
> -
> -
>  static const struct drm_encoder_funcs ast_enc_funcs = {
>  	.destroy = ast_encoder_destroy,
>  };
> @@ -847,7 +836,6 @@ static void ast_connector_destroy(struct drm_connector *connector)
>  static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
>  	.mode_valid = ast_mode_valid,
>  	.get_modes = ast_get_modes,
> -	.best_encoder = ast_best_single_encoder,
>  };
>  
>  static const struct drm_connector_funcs ast_connector_funcs = {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 4706439fb490..9d7e4da6c292 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
>  	}
>  }
>  
> -/*
> - * For connectors that support multiple encoders, either the
> - * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> - */
> -static struct drm_encoder *
> -pick_single_encoder_for_connector(struct drm_connector *connector)
> -{
> -	WARN_ON(connector->encoder_ids[1]);
> -	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
> -}
> -
>  static int handle_conflicting_encoders(struct drm_atomic_state *state,
>  				       bool disable_conflicting_encoders)
>  {
> @@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
>  		else if (funcs->best_encoder)
>  			new_encoder = funcs->best_encoder(connector);
>  		else
> -			new_encoder = pick_single_encoder_for_connector(connector);
> +			new_encoder = drm_connector_get_single_encoder(connector);
>  
>  		if (new_encoder) {
>  			if (encoder_mask & drm_encoder_mask(new_encoder)) {
> @@ -359,7 +348,7 @@ update_connector_routing(struct drm_atomic_state *state,
>  	else if (funcs->best_encoder)
>  		new_encoder = funcs->best_encoder(connector);
>  	else
> -		new_encoder = pick_single_encoder_for_connector(connector);
> +		new_encoder = drm_connector_get_single_encoder(connector);
>  
>  	if (!new_encoder) {
>  		DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index a51824a7e7c1..4a7447a53cea 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -460,6 +460,17 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
>  	__drm_helper_disable_unused_functions(dev);
>  }
>  
> +/*
> + * For connectors that support multiple encoders, either the
> + * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> + */
> +struct drm_encoder *
> +drm_connector_get_single_encoder(struct drm_connector *connector)
> +{
> +	WARN_ON(connector->encoder_ids[1]);
> +	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
> +}
> +
>  /**
>   * drm_crtc_helper_set_config - set a new config from userspace
>   * @set: mode set configuration
> @@ -625,7 +636,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
>  		new_encoder = connector->encoder;
>  		for (ro = 0; ro < set->num_connectors; ro++) {
>  			if (set->connectors[ro] == connector) {
> -				new_encoder = connector_funcs->best_encoder(connector);
> +				if (connector_funcs->best_encoder)
> +					new_encoder = connector_funcs->best_encoder(connector);
> +				else
> +					new_encoder = drm_connector_get_single_encoder(connector);
> +
>  				/* if we can't get an encoder for a connector
>  				   we are setting now - then fail */
>  				if (new_encoder == NULL)
> diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h b/drivers/gpu/drm/drm_crtc_helper_internal.h
> index b5ac1581e623..f0a66ef47e5a 100644
> --- a/drivers/gpu/drm/drm_crtc_helper_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
> @@ -75,3 +75,6 @@ enum drm_mode_status drm_encoder_mode_valid(struct drm_encoder *encoder,
>  					    const struct drm_display_mode *mode);
>  enum drm_mode_status drm_connector_mode_valid(struct drm_connector *connector,
>  					      struct drm_display_mode *mode);
> +
> +struct drm_encoder *
> +drm_connector_get_single_encoder(struct drm_connector *connector);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 5e778b5f1a10..68226556044b 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1638,16 +1638,6 @@ static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector,
>  	return MODE_OK;
>  }
>  
> -static struct drm_encoder *mga_connector_best_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);
> -	return NULL;
> -}
> -
>  static void mga_connector_destroy(struct drm_connector *connector)
>  {
>  	struct mga_connector *mga_connector = to_mga_connector(connector);
> @@ -1659,7 +1649,6 @@ static void mga_connector_destroy(struct drm_connector *connector)
>  static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs = {
>  	.get_modes = mga_vga_get_modes,
>  	.mode_valid = mga_vga_mode_valid,
> -	.best_encoder = mga_connector_best_encoder,
>  };
>  
>  static const struct drm_connector_funcs mga_vga_connector_funcs = {
> diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
> index ddb61a60c610..b4ae3e89a7b4 100644
> --- a/drivers/gpu/drm/udl/udl_connector.c
> +++ b/drivers/gpu/drm/udl/udl_connector.c
> @@ -90,13 +90,6 @@ udl_detect(struct drm_connector *connector, bool force)
>  	return connector_status_connected;
>  }
>  
> -static struct drm_encoder*
> -udl_best_single_encoder(struct drm_connector *connector)
> -{
> -	int enc_id = connector->encoder_ids[0];
> -	return drm_encoder_find(connector->dev, NULL, enc_id);
> -}
> -
>  static int udl_connector_set_property(struct drm_connector *connector,
>  				      struct drm_property *property,
>  				      uint64_t val)
> @@ -120,7 +113,6 @@ static void udl_connector_destroy(struct drm_connector *connector)
>  static const struct drm_connector_helper_funcs udl_connector_helper_funcs = {
>  	.get_modes = udl_get_modes,
>  	.mode_valid = udl_mode_valid,
> -	.best_encoder = udl_best_single_encoder,
>  };
>  
>  static const struct drm_connector_funcs udl_connector_funcs = {
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 6b18c8adfe9d..b55412c6ce3a 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -955,8 +955,8 @@ struct drm_connector_helper_funcs {
>  	 * @atomic_best_encoder.
>  	 *
>  	 * You can leave this function to NULL if the connector is only
> -	 * attached to a single encoder and you are using the atomic helpers.
> -	 * In this case, the core will call drm_atomic_helper_best_encoder()
> +	 * attached to a single encoder.
> +	 * In this case, the core will call drm_connector_get_single_encoder()
>  	 * for you.

+	 * attached to a single encoder. In this case, the core will call
+	 * drm_connector_get_single_encoder() for you.

With this small issue fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	 *
>  	 * RETURNS:
> @@ -977,7 +977,7 @@ struct drm_connector_helper_funcs {
>  	 *
>  	 * This function is used by drm_atomic_helper_check_modeset().
>  	 * If it is not implemented, the core will fallback to @best_encoder
> -	 * (or drm_atomic_helper_best_encoder() if @best_encoder is NULL).
> +	 * (or drm_connector_get_single_encoder() if @best_encoder is NULL).
>  	 *
>  	 * NOTE:
>  	 *

-- 
Regards,

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

✓ Fi.CI.IGT: success for series starting with [1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder
URL   : https://patchwork.freedesktop.org/series/66619/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6885_full -> Patchwork_14387_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

New tests
---------

  New tests have been introduced between CI_DRM_6885_full and Patchwork_14387_full:

### New Piglit tests (7) ###

  * spec@arb_gpu_shader5@texturegather@vs-rgba-2-float-2darray:
    - Statuses : 1 incomplete(s)
    - Exec time: [0.0] s

  * spec@arb_gpu_shader5@texturegather@vs-rgba-3-float-2darray:
    - Statuses : 1 incomplete(s)
    - Exec time: [0.0] s

  * spec@arb_gpu_shader5@texturegatheroffset@vs-rgba-0-float-2darray:
    - Statuses : 1 incomplete(s)
    - Exec time: [0.0] s

  * spec@arb_gpu_shader5@texturegatheroffset@vs-rgba-0-float-2darray-const:
    - Statuses : 1 incomplete(s)
    - Exec time: [0.0] s

  * spec@arb_gpu_shader5@texturegatheroffsets@vs-rgba-1-float-2d:
    - Statuses : 1 incomplete(s)
    - Exec time: [0.0] s

  * spec@arb_gpu_shader5@texturegatheroffsets@vs-rgba-2-float-2d:
    - Statuses : 1 incomplete(s)
    - Exec time: [0.0] s

  * spec@arb_gpu_shader5@texturegatheroffsets@vs-rgba-3-float-2d:
    - Statuses : 1 incomplete(s)
    - Exec time: [0.0] s

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +6 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-apl8/igt@gem_ctx_isolation@rcs0-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-apl3/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_exec_schedule@fifo-bsd1:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#109276]) +19 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb4/igt@gem_exec_schedule@fifo-bsd1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb8/igt@gem_exec_schedule@fifo-bsd1.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#111325]) +8 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb7/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb1/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_softpin@noreloc-s3:
    - shard-iclb:         [PASS][7] -> [INCOMPLETE][8] ([fdo#107713] / [fdo#109100])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb1/igt@gem_softpin@noreloc-s3.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb3/igt@gem_softpin@noreloc-s3.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([fdo#105363])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-glk3/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - shard-apl:          [PASS][11] -> [INCOMPLETE][12] ([fdo#103927])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-apl6/igt@kms_flip@basic-flip-vs-dpms.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-apl4/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         [PASS][13] -> [FAIL][14] ([fdo#103167]) +6 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][15] -> [FAIL][16] ([fdo#108145] / [fdo#110403])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-skl7/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][17] -> [SKIP][18] ([fdo#109642] / [fdo#111068])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb5/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109441]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb7/igt@kms_psr@psr2_primary_page_flip.html

  
#### Possible fixes ####

  * igt@gem_exec_reloc@basic-write-wc-active:
    - shard-apl:          [INCOMPLETE][21] ([fdo#103927]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-apl6/igt@gem_exec_reloc@basic-write-wc-active.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-apl5/igt@gem_exec_reloc@basic-write-wc-active.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [SKIP][23] ([fdo#111325]) -> [PASS][24] +3 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb1/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb8/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [INCOMPLETE][25] ([fdo#104108]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-skl9/igt@gem_softpin@noreloc-s3.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-skl9/igt@gem_softpin@noreloc-s3.html

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-skl:          [INCOMPLETE][27] ([fdo#104108] / [fdo#107807]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-skl7/igt@i915_pm_rpm@system-suspend-execbuf.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-skl7/igt@i915_pm_rpm@system-suspend-execbuf.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [DMESG-WARN][29] ([fdo#108566]) -> [PASS][30] +3 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-apl3/igt@i915_suspend@sysfs-reader.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-apl2/igt@i915_suspend@sysfs-reader.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [FAIL][31] ([fdo#104873]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-glk6/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-glk8/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [FAIL][33] ([fdo#105363]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-panning-interruptible:
    - shard-skl:          [DMESG-WARN][35] ([fdo#106107]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-skl1/igt@kms_flip@flip-vs-panning-interruptible.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-skl4/igt@kms_flip@flip-vs-panning-interruptible.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          [INCOMPLETE][37] ([fdo#109507]) -> [PASS][38] +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-skl1/igt@kms_flip@flip-vs-suspend.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-skl1/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         [FAIL][39] ([fdo#103167]) -> [PASS][40] +9 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][41] ([fdo#108145] / [fdo#110403]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [FAIL][43] ([fdo#103166]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb4/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [SKIP][45] ([fdo#109441]) -> [PASS][46] +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb5/igt@kms_psr@psr2_primary_mmap_cpu.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][47] ([fdo#99912]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-apl8/igt@kms_setmode@basic.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-apl4/igt@kms_setmode@basic.html

  * igt@perf@polling:
    - shard-skl:          [FAIL][49] ([fdo#110728]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-skl1/igt@perf@polling.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-skl4/igt@perf@polling.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [SKIP][51] ([fdo#109276]) -> [PASS][52] +23 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb8/igt@prime_vgem@fence-wait-bsd2.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb4/igt@prime_vgem@fence-wait-bsd2.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [SKIP][53] ([fdo#109276]) -> [FAIL][54] ([fdo#111329])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb3/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb2/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_mocs_settings@mocs-settings-bsd2:
    - shard-iclb:         [SKIP][55] ([fdo#109276]) -> [FAIL][56] ([fdo#111330]) +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb3/igt@gem_mocs_settings@mocs-settings-bsd2.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb2/igt@gem_mocs_settings@mocs-settings-bsd2.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [SKIP][57] ([fdo#109349]) -> [DMESG-WARN][58] ([fdo#107724])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-iclb5/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][59] ([fdo#102887]) -> [FAIL][60] ([fdo#105363])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-skl7/igt@kms_flip@flip-vs-expired-vblank.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-skl6/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-apl:          [DMESG-WARN][61] ([fdo#108566]) -> [INCOMPLETE][62] ([fdo#103927])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6885/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14387/shard-apl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6885 -> Patchwork_14387

  CI-20190529: 20190529
  CI_DRM_6885: 11786d27cb029a083556ac9b82e33d74e250ce26 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5178: efb4539494d94f03374874d3b61bd04ef3802aaa @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14387: fbd5f383c05e7c0e137fa0a1d4110a676cbbad6f @ 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_14387/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[Ville Syrjälä]

On Wed, Sep 11, 2019 at 08:01:55PM +0000, Souza, Jose wrote:
> On Wed, 2019-09-11 at 21:10 +0300, Ville Syrjälä wrote:
> > On Wed, Sep 11, 2019 at 10:56:02AM -0700, José Roberto de Souza
> > wrote:
> > > This 3 non-atomic drivers all have the same function getting the
> > > only encoder available in the connector, also atomic drivers have
> > > this fallback. So moving it a common place and sharing between
> > > atomic
> > > and non-atomic drivers.
> > > 
> > > While at it I also removed the mention of
> > > drm_atomic_helper_best_encoder() that was renamed in
> > > commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
> > > drm_atomic_helper_best_encoder").
> > > 
> > > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: intel-gfx@lists.freedesktop.org
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/ast/ast_mode.c           | 12 ------------
> > >  drivers/gpu/drm/drm_atomic_helper.c      | 15 ++-------------
> > >  drivers/gpu/drm/drm_connector.c          | 11 +++++++++++
> > >  drivers/gpu/drm/drm_crtc_helper.c        |  8 +++++++-
> > >  drivers/gpu/drm/drm_crtc_internal.h      |  2 ++
> > >  drivers/gpu/drm/mgag200/mgag200_mode.c   | 11 -----------
> > >  drivers/gpu/drm/udl/udl_connector.c      |  8 --------
> > >  include/drm/drm_modeset_helper_vtables.h |  6 +++---
> > >  8 files changed, 25 insertions(+), 48 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/ast/ast_mode.c
> > > b/drivers/gpu/drm/ast/ast_mode.c
> > > index d349c721501c..eef95e1af06b 100644
> > > --- a/drivers/gpu/drm/ast/ast_mode.c
> > > +++ b/drivers/gpu/drm/ast/ast_mode.c
> > > @@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct
> > > drm_encoder *encoder)
> > >  	kfree(encoder);
> > >  }
> > >  
> > > -
> > > -static struct drm_encoder *ast_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);
> > > -	return NULL;
> > > -}
> > > -
> > > -
> > >  static const struct drm_encoder_funcs ast_enc_funcs = {
> > >  	.destroy = ast_encoder_destroy,
> > >  };
> > > @@ -847,7 +836,6 @@ static void ast_connector_destroy(struct
> > > drm_connector *connector)
> > >  static const struct drm_connector_helper_funcs
> > > ast_connector_helper_funcs = {
> > >  	.mode_valid = ast_mode_valid,
> > >  	.get_modes = ast_get_modes,
> > > -	.best_encoder = ast_best_single_encoder,
> > >  };
> > >  
> > >  static const struct drm_connector_funcs ast_connector_funcs = {
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > index 4706439fb490..9d7e4da6c292 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct
> > > drm_atomic_state *state,
> > >  	}
> > >  }
> > >  
> > > -/*
> > > - * For connectors that support multiple encoders, either the
> > > - * .atomic_best_encoder() or .best_encoder() operation must be
> > > implemented.
> > > - */
> > > -static struct drm_encoder *
> > > -pick_single_encoder_for_connector(struct drm_connector *connector)
> > > -{
> > > -	WARN_ON(connector->encoder_ids[1]);
> > > -	return drm_encoder_find(connector->dev, NULL, connector-
> > > >encoder_ids[0]);
> > > -}
> > > -
> > >  static int handle_conflicting_encoders(struct drm_atomic_state
> > > *state,
> > >  				       bool
> > > disable_conflicting_encoders)
> > >  {
> > > @@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct
> > > drm_atomic_state *state,
> > >  		else if (funcs->best_encoder)
> > >  			new_encoder = funcs->best_encoder(connector);
> > >  		else
> > > -			new_encoder =
> > > pick_single_encoder_for_connector(connector);
> > > +			new_encoder =
> > > drm_connector_get_single_encoder(connector);
> > >  
> > >  		if (new_encoder) {
> > >  			if (encoder_mask &
> > > drm_encoder_mask(new_encoder)) {
> > > @@ -359,7 +348,7 @@ update_connector_routing(struct
> > > drm_atomic_state *state,
> > >  	else if (funcs->best_encoder)
> > >  		new_encoder = funcs->best_encoder(connector);
> > >  	else
> > > -		new_encoder =
> > > pick_single_encoder_for_connector(connector);
> > > +		new_encoder =
> > > drm_connector_get_single_encoder(connector);
> > >  
> > >  	if (!new_encoder) {
> > >  		DRM_DEBUG_ATOMIC("No suitable encoder found for
> > > [CONNECTOR:%d:%s]\n",
> > > diff --git a/drivers/gpu/drm/drm_connector.c
> > > b/drivers/gpu/drm/drm_connector.c
> > > index 4c766624b20d..3e2a632cf861 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -2334,3 +2334,14 @@ struct drm_tile_group
> > > *drm_mode_create_tile_group(struct drm_device *dev,
> > >  	return tg;
> > >  }
> > >  EXPORT_SYMBOL(drm_mode_create_tile_group);
> > > +
> > > +/*
> > > + * For connectors that support multiple encoders, either the
> > > + * .atomic_best_encoder() or .best_encoder() operation must be
> > > implemented.
> > > + */
> > > +struct drm_encoder *
> > > +drm_connector_get_single_encoder(struct drm_connector *connector)
> > > +{
> > > +	WARN_ON(connector->encoder_ids[1]);
> > > +	return drm_encoder_find(connector->dev, NULL, connector-
> > > >encoder_ids[0]);
> > > +}
> > 
> > I believe we need EXPORT_SYMBOL.
> 
> I don't think we should allow drivers to call this function.

Not drivers. But you already call it from the other module.
In fact all your calls seem to be from the other module, so
perhaps you should move the function into that module.

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

Re: [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[Souza, Jose]

On Wed, 2019-09-11 at 21:10 +0300, Ville Syrjälä wrote:
> On Wed, Sep 11, 2019 at 10:56:02AM -0700, José Roberto de Souza
> wrote:
> > This 3 non-atomic drivers all have the same function getting the
> > only encoder available in the connector, also atomic drivers have
> > this fallback. So moving it a common place and sharing between
> > atomic
> > and non-atomic drivers.
> > 
> > While at it I also removed the mention of
> > drm_atomic_helper_best_encoder() that was renamed in
> > commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
> > drm_atomic_helper_best_encoder").
> > 
> > Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/ast/ast_mode.c           | 12 ------------
> >  drivers/gpu/drm/drm_atomic_helper.c      | 15 ++-------------
> >  drivers/gpu/drm/drm_connector.c          | 11 +++++++++++
> >  drivers/gpu/drm/drm_crtc_helper.c        |  8 +++++++-
> >  drivers/gpu/drm/drm_crtc_internal.h      |  2 ++
> >  drivers/gpu/drm/mgag200/mgag200_mode.c   | 11 -----------
> >  drivers/gpu/drm/udl/udl_connector.c      |  8 --------
> >  include/drm/drm_modeset_helper_vtables.h |  6 +++---
> >  8 files changed, 25 insertions(+), 48 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ast/ast_mode.c
> > b/drivers/gpu/drm/ast/ast_mode.c
> > index d349c721501c..eef95e1af06b 100644
> > --- a/drivers/gpu/drm/ast/ast_mode.c
> > +++ b/drivers/gpu/drm/ast/ast_mode.c
> > @@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct
> > drm_encoder *encoder)
> >  	kfree(encoder);
> >  }
> >  
> > -
> > -static struct drm_encoder *ast_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);
> > -	return NULL;
> > -}
> > -
> > -
> >  static const struct drm_encoder_funcs ast_enc_funcs = {
> >  	.destroy = ast_encoder_destroy,
> >  };
> > @@ -847,7 +836,6 @@ static void ast_connector_destroy(struct
> > drm_connector *connector)
> >  static const struct drm_connector_helper_funcs
> > ast_connector_helper_funcs = {
> >  	.mode_valid = ast_mode_valid,
> >  	.get_modes = ast_get_modes,
> > -	.best_encoder = ast_best_single_encoder,
> >  };
> >  
> >  static const struct drm_connector_funcs ast_connector_funcs = {
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 4706439fb490..9d7e4da6c292 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct
> > drm_atomic_state *state,
> >  	}
> >  }
> >  
> > -/*
> > - * For connectors that support multiple encoders, either the
> > - * .atomic_best_encoder() or .best_encoder() operation must be
> > implemented.
> > - */
> > -static struct drm_encoder *
> > -pick_single_encoder_for_connector(struct drm_connector *connector)
> > -{
> > -	WARN_ON(connector->encoder_ids[1]);
> > -	return drm_encoder_find(connector->dev, NULL, connector-
> > >encoder_ids[0]);
> > -}
> > -
> >  static int handle_conflicting_encoders(struct drm_atomic_state
> > *state,
> >  				       bool
> > disable_conflicting_encoders)
> >  {
> > @@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct
> > drm_atomic_state *state,
> >  		else if (funcs->best_encoder)
> >  			new_encoder = funcs->best_encoder(connector);
> >  		else
> > -			new_encoder =
> > pick_single_encoder_for_connector(connector);
> > +			new_encoder =
> > drm_connector_get_single_encoder(connector);
> >  
> >  		if (new_encoder) {
> >  			if (encoder_mask &
> > drm_encoder_mask(new_encoder)) {
> > @@ -359,7 +348,7 @@ update_connector_routing(struct
> > drm_atomic_state *state,
> >  	else if (funcs->best_encoder)
> >  		new_encoder = funcs->best_encoder(connector);
> >  	else
> > -		new_encoder =
> > pick_single_encoder_for_connector(connector);
> > +		new_encoder =
> > drm_connector_get_single_encoder(connector);
> >  
> >  	if (!new_encoder) {
> >  		DRM_DEBUG_ATOMIC("No suitable encoder found for
> > [CONNECTOR:%d:%s]\n",
> > diff --git a/drivers/gpu/drm/drm_connector.c
> > b/drivers/gpu/drm/drm_connector.c
> > index 4c766624b20d..3e2a632cf861 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -2334,3 +2334,14 @@ struct drm_tile_group
> > *drm_mode_create_tile_group(struct drm_device *dev,
> >  	return tg;
> >  }
> >  EXPORT_SYMBOL(drm_mode_create_tile_group);
> > +
> > +/*
> > + * For connectors that support multiple encoders, either the
> > + * .atomic_best_encoder() or .best_encoder() operation must be
> > implemented.
> > + */
> > +struct drm_encoder *
> > +drm_connector_get_single_encoder(struct drm_connector *connector)
> > +{
> > +	WARN_ON(connector->encoder_ids[1]);
> > +	return drm_encoder_find(connector->dev, NULL, connector-
> > >encoder_ids[0]);
> > +}
> 
> I believe we need EXPORT_SYMBOL.

I don't think we should allow drivers to call this function.

> 
> > diff --git a/drivers/gpu/drm/drm_crtc_helper.c
> > b/drivers/gpu/drm/drm_crtc_helper.c
> > index a51824a7e7c1..a1f3c388e398 100644
> > --- a/drivers/gpu/drm/drm_crtc_helper.c
> > +++ b/drivers/gpu/drm/drm_crtc_helper.c
> > @@ -48,6 +48,8 @@
> >  #include <drm/drm_print.h>
> >  #include <drm/drm_vblank.h>
> >  
> > +#include "drm_crtc_internal.h"
> > +
> >  /**
> >   * DOC: overview
> >   *
> > @@ -625,7 +627,11 @@ int drm_crtc_helper_set_config(struct
> > drm_mode_set *set,
> >  		new_encoder = connector->encoder;
> >  		for (ro = 0; ro < set->num_connectors; ro++) {
> >  			if (set->connectors[ro] == connector) {
> > -				new_encoder = connector_funcs-
> > >best_encoder(connector);
> > +				if (connector_funcs->best_encoder)
> > +					new_encoder = connector_funcs-
> > >best_encoder(connector);
> > +				else
> > +					new_encoder =
> > drm_connector_get_single_encoder(connector);
> > +
> >  				/* if we can't get an encoder for a
> > connector
> >  				   we are setting now - then fail */
> >  				if (new_encoder == NULL)
> > diff --git a/drivers/gpu/drm/drm_crtc_internal.h
> > b/drivers/gpu/drm/drm_crtc_internal.h
> > index c7d5e4c21423..80ade1fa66e5 100644
> > --- a/drivers/gpu/drm/drm_crtc_internal.h
> > +++ b/drivers/gpu/drm/drm_crtc_internal.h
> > @@ -181,6 +181,8 @@ int drm_connector_set_obj_prop(struct
> > drm_mode_object *obj,
> >  int drm_connector_create_standard_properties(struct drm_device
> > *dev);
> >  const char *drm_get_connector_force_name(enum drm_connector_force
> > force);
> >  void drm_connector_free_work_fn(struct work_struct *work);
> > +struct drm_encoder *
> > +drm_connector_get_single_encoder(struct drm_connector *connector);
> >  
> >  /* IOCTL */
> >  int drm_connector_property_set_ioctl(struct drm_device *dev,
> > diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
> > b/drivers/gpu/drm/mgag200/mgag200_mode.c
> > index 5e778b5f1a10..68226556044b 100644
> > --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> > +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> > @@ -1638,16 +1638,6 @@ static enum drm_mode_status
> > mga_vga_mode_valid(struct drm_connector *connector,
> >  	return MODE_OK;
> >  }
> >  
> > -static struct drm_encoder *mga_connector_best_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);
> > -	return NULL;
> > -}
> > -
> >  static void mga_connector_destroy(struct drm_connector *connector)
> >  {
> >  	struct mga_connector *mga_connector =
> > to_mga_connector(connector);
> > @@ -1659,7 +1649,6 @@ static void mga_connector_destroy(struct
> > drm_connector *connector)
> >  static const struct drm_connector_helper_funcs
> > mga_vga_connector_helper_funcs = {
> >  	.get_modes = mga_vga_get_modes,
> >  	.mode_valid = mga_vga_mode_valid,
> > -	.best_encoder = mga_connector_best_encoder,
> >  };
> >  
> >  static const struct drm_connector_funcs mga_vga_connector_funcs =
> > {
> > diff --git a/drivers/gpu/drm/udl/udl_connector.c
> > b/drivers/gpu/drm/udl/udl_connector.c
> > index ddb61a60c610..b4ae3e89a7b4 100644
> > --- a/drivers/gpu/drm/udl/udl_connector.c
> > +++ b/drivers/gpu/drm/udl/udl_connector.c
> > @@ -90,13 +90,6 @@ udl_detect(struct drm_connector *connector, bool
> > force)
> >  	return connector_status_connected;
> >  }
> >  
> > -static struct drm_encoder*
> > -udl_best_single_encoder(struct drm_connector *connector)
> > -{
> > -	int enc_id = connector->encoder_ids[0];
> > -	return drm_encoder_find(connector->dev, NULL, enc_id);
> > -}
> > -
> >  static int udl_connector_set_property(struct drm_connector
> > *connector,
> >  				      struct drm_property *property,
> >  				      uint64_t val)
> > @@ -120,7 +113,6 @@ static void udl_connector_destroy(struct
> > drm_connector *connector)
> >  static const struct drm_connector_helper_funcs
> > udl_connector_helper_funcs = {
> >  	.get_modes = udl_get_modes,
> >  	.mode_valid = udl_mode_valid,
> > -	.best_encoder = udl_best_single_encoder,
> >  };
> >  
> >  static const struct drm_connector_funcs udl_connector_funcs = {
> > diff --git a/include/drm/drm_modeset_helper_vtables.h
> > b/include/drm/drm_modeset_helper_vtables.h
> > index 6b18c8adfe9d..b55412c6ce3a 100644
> > --- a/include/drm/drm_modeset_helper_vtables.h
> > +++ b/include/drm/drm_modeset_helper_vtables.h
> > @@ -955,8 +955,8 @@ struct drm_connector_helper_funcs {
> >  	 * @atomic_best_encoder.
> >  	 *
> >  	 * You can leave this function to NULL if the connector is only
> > -	 * attached to a single encoder and you are using the atomic
> > helpers.
> > -	 * In this case, the core will call
> > drm_atomic_helper_best_encoder()
> > +	 * attached to a single encoder.
> > +	 * In this case, the core will call
> > drm_connector_get_single_encoder()
> >  	 * for you.
> >  	 *
> >  	 * RETURNS:
> > @@ -977,7 +977,7 @@ struct drm_connector_helper_funcs {
> >  	 *
> >  	 * This function is used by drm_atomic_helper_check_modeset().
> >  	 * If it is not implemented, the core will fallback to
> > @best_encoder
> > -	 * (or drm_atomic_helper_best_encoder() if @best_encoder is
> > NULL).
> > +	 * (or drm_connector_get_single_encoder() if @best_encoder is
> > NULL).
> >  	 *
> >  	 * NOTE:
> >  	 *
> > -- 
> > 2.23.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[Ville Syrjälä]

On Wed, Sep 11, 2019 at 10:56:02AM -0700, José Roberto de Souza wrote:
> This 3 non-atomic drivers all have the same function getting the
> only encoder available in the connector, also atomic drivers have
> this fallback. So moving it a common place and sharing between atomic
> and non-atomic drivers.
> 
> While at it I also removed the mention of
> drm_atomic_helper_best_encoder() that was renamed in
> commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
> drm_atomic_helper_best_encoder").
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/ast/ast_mode.c           | 12 ------------
>  drivers/gpu/drm/drm_atomic_helper.c      | 15 ++-------------
>  drivers/gpu/drm/drm_connector.c          | 11 +++++++++++
>  drivers/gpu/drm/drm_crtc_helper.c        |  8 +++++++-
>  drivers/gpu/drm/drm_crtc_internal.h      |  2 ++
>  drivers/gpu/drm/mgag200/mgag200_mode.c   | 11 -----------
>  drivers/gpu/drm/udl/udl_connector.c      |  8 --------
>  include/drm/drm_modeset_helper_vtables.h |  6 +++---
>  8 files changed, 25 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index d349c721501c..eef95e1af06b 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct drm_encoder *encoder)
>  	kfree(encoder);
>  }
>  
> -
> -static struct drm_encoder *ast_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);
> -	return NULL;
> -}
> -
> -
>  static const struct drm_encoder_funcs ast_enc_funcs = {
>  	.destroy = ast_encoder_destroy,
>  };
> @@ -847,7 +836,6 @@ static void ast_connector_destroy(struct drm_connector *connector)
>  static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
>  	.mode_valid = ast_mode_valid,
>  	.get_modes = ast_get_modes,
> -	.best_encoder = ast_best_single_encoder,
>  };
>  
>  static const struct drm_connector_funcs ast_connector_funcs = {
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 4706439fb490..9d7e4da6c292 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
>  	}
>  }
>  
> -/*
> - * For connectors that support multiple encoders, either the
> - * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> - */
> -static struct drm_encoder *
> -pick_single_encoder_for_connector(struct drm_connector *connector)
> -{
> -	WARN_ON(connector->encoder_ids[1]);
> -	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
> -}
> -
>  static int handle_conflicting_encoders(struct drm_atomic_state *state,
>  				       bool disable_conflicting_encoders)
>  {
> @@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
>  		else if (funcs->best_encoder)
>  			new_encoder = funcs->best_encoder(connector);
>  		else
> -			new_encoder = pick_single_encoder_for_connector(connector);
> +			new_encoder = drm_connector_get_single_encoder(connector);
>  
>  		if (new_encoder) {
>  			if (encoder_mask & drm_encoder_mask(new_encoder)) {
> @@ -359,7 +348,7 @@ update_connector_routing(struct drm_atomic_state *state,
>  	else if (funcs->best_encoder)
>  		new_encoder = funcs->best_encoder(connector);
>  	else
> -		new_encoder = pick_single_encoder_for_connector(connector);
> +		new_encoder = drm_connector_get_single_encoder(connector);
>  
>  	if (!new_encoder) {
>  		DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..3e2a632cf861 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2334,3 +2334,14 @@ struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
>  	return tg;
>  }
>  EXPORT_SYMBOL(drm_mode_create_tile_group);
> +
> +/*
> + * For connectors that support multiple encoders, either the
> + * .atomic_best_encoder() or .best_encoder() operation must be implemented.
> + */
> +struct drm_encoder *
> +drm_connector_get_single_encoder(struct drm_connector *connector)
> +{
> +	WARN_ON(connector->encoder_ids[1]);
> +	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
> +}

I believe we need EXPORT_SYMBOL.

> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index a51824a7e7c1..a1f3c388e398 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -48,6 +48,8 @@
>  #include <drm/drm_print.h>
>  #include <drm/drm_vblank.h>
>  
> +#include "drm_crtc_internal.h"
> +
>  /**
>   * DOC: overview
>   *
> @@ -625,7 +627,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
>  		new_encoder = connector->encoder;
>  		for (ro = 0; ro < set->num_connectors; ro++) {
>  			if (set->connectors[ro] == connector) {
> -				new_encoder = connector_funcs->best_encoder(connector);
> +				if (connector_funcs->best_encoder)
> +					new_encoder = connector_funcs->best_encoder(connector);
> +				else
> +					new_encoder = drm_connector_get_single_encoder(connector);
> +
>  				/* if we can't get an encoder for a connector
>  				   we are setting now - then fail */
>  				if (new_encoder == NULL)
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index c7d5e4c21423..80ade1fa66e5 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -181,6 +181,8 @@ int drm_connector_set_obj_prop(struct drm_mode_object *obj,
>  int drm_connector_create_standard_properties(struct drm_device *dev);
>  const char *drm_get_connector_force_name(enum drm_connector_force force);
>  void drm_connector_free_work_fn(struct work_struct *work);
> +struct drm_encoder *
> +drm_connector_get_single_encoder(struct drm_connector *connector);
>  
>  /* IOCTL */
>  int drm_connector_property_set_ioctl(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 5e778b5f1a10..68226556044b 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1638,16 +1638,6 @@ static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector,
>  	return MODE_OK;
>  }
>  
> -static struct drm_encoder *mga_connector_best_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);
> -	return NULL;
> -}
> -
>  static void mga_connector_destroy(struct drm_connector *connector)
>  {
>  	struct mga_connector *mga_connector = to_mga_connector(connector);
> @@ -1659,7 +1649,6 @@ static void mga_connector_destroy(struct drm_connector *connector)
>  static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs = {
>  	.get_modes = mga_vga_get_modes,
>  	.mode_valid = mga_vga_mode_valid,
> -	.best_encoder = mga_connector_best_encoder,
>  };
>  
>  static const struct drm_connector_funcs mga_vga_connector_funcs = {
> diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
> index ddb61a60c610..b4ae3e89a7b4 100644
> --- a/drivers/gpu/drm/udl/udl_connector.c
> +++ b/drivers/gpu/drm/udl/udl_connector.c
> @@ -90,13 +90,6 @@ udl_detect(struct drm_connector *connector, bool force)
>  	return connector_status_connected;
>  }
>  
> -static struct drm_encoder*
> -udl_best_single_encoder(struct drm_connector *connector)
> -{
> -	int enc_id = connector->encoder_ids[0];
> -	return drm_encoder_find(connector->dev, NULL, enc_id);
> -}
> -
>  static int udl_connector_set_property(struct drm_connector *connector,
>  				      struct drm_property *property,
>  				      uint64_t val)
> @@ -120,7 +113,6 @@ static void udl_connector_destroy(struct drm_connector *connector)
>  static const struct drm_connector_helper_funcs udl_connector_helper_funcs = {
>  	.get_modes = udl_get_modes,
>  	.mode_valid = udl_mode_valid,
> -	.best_encoder = udl_best_single_encoder,
>  };
>  
>  static const struct drm_connector_funcs udl_connector_funcs = {
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 6b18c8adfe9d..b55412c6ce3a 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -955,8 +955,8 @@ struct drm_connector_helper_funcs {
>  	 * @atomic_best_encoder.
>  	 *
>  	 * You can leave this function to NULL if the connector is only
> -	 * attached to a single encoder and you are using the atomic helpers.
> -	 * In this case, the core will call drm_atomic_helper_best_encoder()
> +	 * attached to a single encoder.
> +	 * In this case, the core will call drm_connector_get_single_encoder()
>  	 * for you.
>  	 *
>  	 * RETURNS:
> @@ -977,7 +977,7 @@ struct drm_connector_helper_funcs {
>  	 *
>  	 * This function is used by drm_atomic_helper_check_modeset().
>  	 * If it is not implemented, the core will fallback to @best_encoder
> -	 * (or drm_atomic_helper_best_encoder() if @best_encoder is NULL).
> +	 * (or drm_connector_get_single_encoder() if @best_encoder is NULL).
>  	 *
>  	 * NOTE:
>  	 *
> -- 
> 2.23.0

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

[PATCH 1/2] drm/connector: Share with non-atomic drivers the function to get the single encoder

[José Roberto de Souza]

This 3 non-atomic drivers all have the same function getting the
only encoder available in the connector, also atomic drivers have
this fallback. So moving it a common place and sharing between atomic
and non-atomic drivers.

While at it I also removed the mention of
drm_atomic_helper_best_encoder() that was renamed in
commit 297e30b5d9b6 ("drm/atomic-helper: Unexport
drm_atomic_helper_best_encoder").

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/ast/ast_mode.c           | 12 ------------
 drivers/gpu/drm/drm_atomic_helper.c      | 15 ++-------------
 drivers/gpu/drm/drm_connector.c          | 11 +++++++++++
 drivers/gpu/drm/drm_crtc_helper.c        |  8 +++++++-
 drivers/gpu/drm/drm_crtc_internal.h      |  2 ++
 drivers/gpu/drm/mgag200/mgag200_mode.c   | 11 -----------
 drivers/gpu/drm/udl/udl_connector.c      |  8 --------
 include/drm/drm_modeset_helper_vtables.h |  6 +++---
 8 files changed, 25 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index d349c721501c..eef95e1af06b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -687,17 +687,6 @@ static void ast_encoder_destroy(struct drm_encoder *encoder)
 	kfree(encoder);
 }
 
-
-static struct drm_encoder *ast_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);
-	return NULL;
-}
-
-
 static const struct drm_encoder_funcs ast_enc_funcs = {
 	.destroy = ast_encoder_destroy,
 };
@@ -847,7 +836,6 @@ static void ast_connector_destroy(struct drm_connector *connector)
 static const struct drm_connector_helper_funcs ast_connector_helper_funcs = {
 	.mode_valid = ast_mode_valid,
 	.get_modes = ast_get_modes,
-	.best_encoder = ast_best_single_encoder,
 };
 
 static const struct drm_connector_funcs ast_connector_funcs = {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 4706439fb490..9d7e4da6c292 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -97,17 +97,6 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
 	}
 }
 
-/*
- * For connectors that support multiple encoders, either the
- * .atomic_best_encoder() or .best_encoder() operation must be implemented.
- */
-static struct drm_encoder *
-pick_single_encoder_for_connector(struct drm_connector *connector)
-{
-	WARN_ON(connector->encoder_ids[1]);
-	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
-}
-
 static int handle_conflicting_encoders(struct drm_atomic_state *state,
 				       bool disable_conflicting_encoders)
 {
@@ -135,7 +124,7 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
 		else if (funcs->best_encoder)
 			new_encoder = funcs->best_encoder(connector);
 		else
-			new_encoder = pick_single_encoder_for_connector(connector);
+			new_encoder = drm_connector_get_single_encoder(connector);
 
 		if (new_encoder) {
 			if (encoder_mask & drm_encoder_mask(new_encoder)) {
@@ -359,7 +348,7 @@ update_connector_routing(struct drm_atomic_state *state,
 	else if (funcs->best_encoder)
 		new_encoder = funcs->best_encoder(connector);
 	else
-		new_encoder = pick_single_encoder_for_connector(connector);
+		new_encoder = drm_connector_get_single_encoder(connector);
 
 	if (!new_encoder) {
 		DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4c766624b20d..3e2a632cf861 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2334,3 +2334,14 @@ struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
 	return tg;
 }
 EXPORT_SYMBOL(drm_mode_create_tile_group);
+
+/*
+ * For connectors that support multiple encoders, either the
+ * .atomic_best_encoder() or .best_encoder() operation must be implemented.
+ */
+struct drm_encoder *
+drm_connector_get_single_encoder(struct drm_connector *connector)
+{
+	WARN_ON(connector->encoder_ids[1]);
+	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
+}
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index a51824a7e7c1..a1f3c388e398 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -48,6 +48,8 @@
 #include <drm/drm_print.h>
 #include <drm/drm_vblank.h>
 
+#include "drm_crtc_internal.h"
+
 /**
  * DOC: overview
  *
@@ -625,7 +627,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set,
 		new_encoder = connector->encoder;
 		for (ro = 0; ro < set->num_connectors; ro++) {
 			if (set->connectors[ro] == connector) {
-				new_encoder = connector_funcs->best_encoder(connector);
+				if (connector_funcs->best_encoder)
+					new_encoder = connector_funcs->best_encoder(connector);
+				else
+					new_encoder = drm_connector_get_single_encoder(connector);
+
 				/* if we can't get an encoder for a connector
 				   we are setting now - then fail */
 				if (new_encoder == NULL)
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index c7d5e4c21423..80ade1fa66e5 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -181,6 +181,8 @@ int drm_connector_set_obj_prop(struct drm_mode_object *obj,
 int drm_connector_create_standard_properties(struct drm_device *dev);
 const char *drm_get_connector_force_name(enum drm_connector_force force);
 void drm_connector_free_work_fn(struct work_struct *work);
+struct drm_encoder *
+drm_connector_get_single_encoder(struct drm_connector *connector);
 
 /* IOCTL */
 int drm_connector_property_set_ioctl(struct drm_device *dev,
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 5e778b5f1a10..68226556044b 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1638,16 +1638,6 @@ static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector,
 	return MODE_OK;
 }
 
-static struct drm_encoder *mga_connector_best_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);
-	return NULL;
-}
-
 static void mga_connector_destroy(struct drm_connector *connector)
 {
 	struct mga_connector *mga_connector = to_mga_connector(connector);
@@ -1659,7 +1649,6 @@ static void mga_connector_destroy(struct drm_connector *connector)
 static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs = {
 	.get_modes = mga_vga_get_modes,
 	.mode_valid = mga_vga_mode_valid,
-	.best_encoder = mga_connector_best_encoder,
 };
 
 static const struct drm_connector_funcs mga_vga_connector_funcs = {
diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
index ddb61a60c610..b4ae3e89a7b4 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -90,13 +90,6 @@ udl_detect(struct drm_connector *connector, bool force)
 	return connector_status_connected;
 }
 
-static struct drm_encoder*
-udl_best_single_encoder(struct drm_connector *connector)
-{
-	int enc_id = connector->encoder_ids[0];
-	return drm_encoder_find(connector->dev, NULL, enc_id);
-}
-
 static int udl_connector_set_property(struct drm_connector *connector,
 				      struct drm_property *property,
 				      uint64_t val)
@@ -120,7 +113,6 @@ static void udl_connector_destroy(struct drm_connector *connector)
 static const struct drm_connector_helper_funcs udl_connector_helper_funcs = {
 	.get_modes = udl_get_modes,
 	.mode_valid = udl_mode_valid,
-	.best_encoder = udl_best_single_encoder,
 };
 
 static const struct drm_connector_funcs udl_connector_funcs = {
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 6b18c8adfe9d..b55412c6ce3a 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -955,8 +955,8 @@ struct drm_connector_helper_funcs {
 	 * @atomic_best_encoder.
 	 *
 	 * You can leave this function to NULL if the connector is only
-	 * attached to a single encoder and you are using the atomic helpers.
-	 * In this case, the core will call drm_atomic_helper_best_encoder()
+	 * attached to a single encoder.
+	 * In this case, the core will call drm_connector_get_single_encoder()
 	 * for you.
 	 *
 	 * RETURNS:
@@ -977,7 +977,7 @@ struct drm_connector_helper_funcs {
 	 *
 	 * This function is used by drm_atomic_helper_check_modeset().
 	 * If it is not implemented, the core will fallback to @best_encoder
-	 * (or drm_atomic_helper_best_encoder() if @best_encoder is NULL).
+	 * (or drm_connector_get_single_encoder() if @best_encoder is NULL).
 	 *
 	 * NOTE:
 	 *
-- 
2.23.0

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