* [PATCH v3 1/7] drm/atomic: Clean up update_output_state.
2016-03-03 9:17 [PATCH v3 0/7] drm/atomic: Fix encoder stealing, v3 Maarten Lankhorst
@ 2016-03-03 9:17 ` Maarten Lankhorst
2016-03-03 9:17 ` [PATCH v3 2/7] drm/atomic: Pass connector and state to update_connector_routing Maarten Lankhorst
` (6 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2016-03-03 9:17 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
With the addition of crtc_state->connector_mask other connectors from
different crtc's aren't needed any more, so only call
add_affected_connectors on the target crtc. This allows a cleanup
to first remove all current connectors, then add all set->connectors
to the target crtc.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 41 +++++++++++++++----------------------
1 file changed, 17 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 4da4f2a49078..a0226d4b949a 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1761,28 +1761,18 @@ static int update_output_state(struct drm_atomic_state *state,
struct drm_crtc_state *crtc_state;
struct drm_connector *connector;
struct drm_connector_state *conn_state;
- int ret, i, j;
+ int ret, i;
ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
state->acquire_ctx);
if (ret)
return ret;
- /* First grab all affected connector/crtc states. */
- for (i = 0; i < set->num_connectors; i++) {
- conn_state = drm_atomic_get_connector_state(state,
- set->connectors[i]);
- if (IS_ERR(conn_state))
- return PTR_ERR(conn_state);
- }
-
- for_each_crtc_in_state(state, crtc, crtc_state, i) {
- ret = drm_atomic_add_affected_connectors(state, crtc);
- if (ret)
- return ret;
- }
+ /* First disable all connectors on the target crtc. */
+ ret = drm_atomic_add_affected_connectors(state, set->crtc);
+ if (ret)
+ return ret;
- /* Then recompute connector->crtc links and crtc enabling state. */
for_each_connector_in_state(state, connector, conn_state, i) {
if (conn_state->crtc == set->crtc) {
ret = drm_atomic_set_crtc_for_connector(conn_state,
@@ -1790,16 +1780,19 @@ static int update_output_state(struct drm_atomic_state *state,
if (ret)
return ret;
}
+ }
- for (j = 0; j < set->num_connectors; j++) {
- if (set->connectors[j] == connector) {
- ret = drm_atomic_set_crtc_for_connector(conn_state,
- set->crtc);
- if (ret)
- return ret;
- break;
- }
- }
+ /* Then set all connectors from set->connectors on the target crtc */
+ for (i = 0; i < set->num_connectors; i++) {
+ conn_state = drm_atomic_get_connector_state(state,
+ set->connectors[i]);
+ if (IS_ERR(conn_state))
+ return PTR_ERR(conn_state);
+
+ ret = drm_atomic_set_crtc_for_connector(conn_state,
+ set->crtc);
+ if (ret)
+ return ret;
}
for_each_crtc_in_state(state, crtc, crtc_state, i) {
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 2/7] drm/atomic: Pass connector and state to update_connector_routing.
2016-03-03 9:17 [PATCH v3 0/7] drm/atomic: Fix encoder stealing, v3 Maarten Lankhorst
2016-03-03 9:17 ` [PATCH v3 1/7] drm/atomic: Clean up update_output_state Maarten Lankhorst
@ 2016-03-03 9:17 ` Maarten Lankhorst
2016-03-03 9:17 ` [PATCH v3 3/7] drm/atomic: Always call steal_encoder, v2 Maarten Lankhorst
` (5 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2016-03-03 9:17 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
Minor cleanup, connector and connector_state are always non-NULL here.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index a0226d4b949a..3d1f97a832fc 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -215,22 +215,16 @@ steal_encoder(struct drm_atomic_state *state,
}
static int
-update_connector_routing(struct drm_atomic_state *state, int conn_idx)
+update_connector_routing(struct drm_atomic_state *state,
+ struct drm_connector *connector,
+ struct drm_connector_state *connector_state)
{
const struct drm_connector_helper_funcs *funcs;
struct drm_encoder *new_encoder;
struct drm_crtc *encoder_crtc;
- struct drm_connector *connector;
- struct drm_connector_state *connector_state;
struct drm_crtc_state *crtc_state;
int idx, ret;
- connector = state->connectors[conn_idx];
- connector_state = state->connector_states[conn_idx];
-
- if (!connector)
- return 0;
-
DRM_DEBUG_ATOMIC("Updating routing for [CONNECTOR:%d:%s]\n",
connector->base.id,
connector->name);
@@ -494,7 +488,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
* drivers must set crtc->mode_changed themselves when connector
* properties need to be updated.
*/
- ret = update_connector_routing(state, i);
+ ret = update_connector_routing(state, connector,
+ connector_state);
if (ret)
return ret;
}
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 3/7] drm/atomic: Always call steal_encoder, v2.
2016-03-03 9:17 [PATCH v3 0/7] drm/atomic: Fix encoder stealing, v3 Maarten Lankhorst
2016-03-03 9:17 ` [PATCH v3 1/7] drm/atomic: Clean up update_output_state Maarten Lankhorst
2016-03-03 9:17 ` [PATCH v3 2/7] drm/atomic: Pass connector and state to update_connector_routing Maarten Lankhorst
@ 2016-03-03 9:17 ` Maarten Lankhorst
2016-03-03 9:17 ` [PATCH v3 4/7] drm/atomic: Handle encoder stealing from set_config better Maarten Lankhorst
` (4 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2016-03-03 9:17 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
There's no need to have a separate function to get the crtc
which is stolen, this can already be found when actually
stealing the encoder.
drm_for_each_connector already checks for connection_mutex, so
use that macro now.
Changes since v1:
- Do not check for NULL crtc in connector_state,
this may happen when a crtc is disabled and its encoder stolen.
- Because of this, use connector->state->crtc instead of conn_state->crtc.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 74 ++++++++++---------------------------
1 file changed, 20 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 3d1f97a832fc..72ca85b32260 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -106,25 +106,6 @@ check_pending_encoder_assignment(struct drm_atomic_state *state,
return true;
}
-static struct drm_crtc *
-get_current_crtc_for_encoder(struct drm_device *dev,
- struct drm_encoder *encoder)
-{
- struct drm_mode_config *config = &dev->mode_config;
- struct drm_connector *connector;
-
- WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
-
- drm_for_each_connector(connector, dev) {
- if (connector->state->best_encoder != encoder)
- continue;
-
- return connector->state->crtc;
- }
-
- return NULL;
-}
-
static void
set_best_encoder(struct drm_atomic_state *state,
struct drm_connector_state *conn_state,
@@ -168,38 +149,18 @@ set_best_encoder(struct drm_atomic_state *state,
static int
steal_encoder(struct drm_atomic_state *state,
- struct drm_encoder *encoder,
- struct drm_crtc *encoder_crtc)
+ struct drm_encoder *encoder)
{
- struct drm_mode_config *config = &state->dev->mode_config;
struct drm_crtc_state *crtc_state;
struct drm_connector *connector;
struct drm_connector_state *connector_state;
- /*
- * We can only steal an encoder coming from a connector, which means we
- * must already hold the connection_mutex.
- */
- WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
-
- DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], stealing it\n",
- encoder->base.id, encoder->name,
- encoder_crtc->base.id, encoder_crtc->name);
+ drm_for_each_connector(connector, state->dev) {
+ struct drm_crtc *encoder_crtc;
- crtc_state = drm_atomic_get_crtc_state(state, encoder_crtc);
- if (IS_ERR(crtc_state))
- return PTR_ERR(crtc_state);
-
- crtc_state->connectors_changed = true;
-
- list_for_each_entry(connector, &config->connector_list, head) {
if (connector->state->best_encoder != encoder)
continue;
- DRM_DEBUG_ATOMIC("Stealing encoder from [CONNECTOR:%d:%s]\n",
- connector->base.id,
- connector->name);
-
connector_state = drm_atomic_get_connector_state(state,
connector);
if (IS_ERR(connector_state))
@@ -208,7 +169,18 @@ steal_encoder(struct drm_atomic_state *state,
if (connector_state->best_encoder != encoder)
continue;
+ encoder_crtc = connector->state->crtc;
+
+ DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], stealing it\n",
+ encoder->base.id, encoder->name,
+ encoder_crtc->base.id, encoder_crtc->name);
+
set_best_encoder(state, connector_state, NULL);
+
+ crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
+ crtc_state->connectors_changed = true;
+
+ return 0;
}
return 0;
@@ -221,7 +193,6 @@ update_connector_routing(struct drm_atomic_state *state,
{
const struct drm_connector_helper_funcs *funcs;
struct drm_encoder *new_encoder;
- struct drm_crtc *encoder_crtc;
struct drm_crtc_state *crtc_state;
int idx, ret;
@@ -299,17 +270,12 @@ update_connector_routing(struct drm_atomic_state *state,
return -EINVAL;
}
- encoder_crtc = get_current_crtc_for_encoder(state->dev,
- new_encoder);
-
- if (encoder_crtc) {
- ret = steal_encoder(state, new_encoder, encoder_crtc);
- if (ret) {
- DRM_DEBUG_ATOMIC("Encoder stealing failed for [CONNECTOR:%d:%s]\n",
- connector->base.id,
- connector->name);
- return ret;
- }
+ ret = steal_encoder(state, new_encoder);
+ if (ret) {
+ DRM_DEBUG_ATOMIC("Encoder stealing failed for [CONNECTOR:%d:%s]\n",
+ connector->base.id,
+ connector->name);
+ return ret;
}
if (WARN_ON(!connector_state->crtc))
--
2.1.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 4/7] drm/atomic: Handle encoder stealing from set_config better.
2016-03-03 9:17 [PATCH v3 0/7] drm/atomic: Fix encoder stealing, v3 Maarten Lankhorst
` (2 preceding siblings ...)
2016-03-03 9:17 ` [PATCH v3 3/7] drm/atomic: Always call steal_encoder, v2 Maarten Lankhorst
@ 2016-03-03 9:17 ` Maarten Lankhorst
2016-03-03 9:17 ` [PATCH v3 5/7] drm/atomic: Handle encoder assignment conflicts in a separate check, v3 Maarten Lankhorst
` (3 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2016-03-03 9:17 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
Instead of failing with -EINVAL when conflicting encoders are found,
the legacy set_config will disable other connectors when encoders
conflict.
With the previous commit this becomes a lot easier to implement.
set_config only adds connectors to the state that are modified,
and because of the previous commit that calls add_affected_connectors
only on set->crtc it means any connector not part of the modeset can
be stolen from. We disable the connector in that case, and possibly
the crtc if required.
Atomic modeset itself still doesn't allow encoder stealing, the results
would be too unpredictable.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 69 +++++++++++++++++++++++++++++++++++++
include/drm/drm_crtc.h | 2 ++
2 files changed, 71 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 72ca85b32260..db11c2f9b098 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -86,6 +86,68 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
}
}
+static int disable_conflicting_connectors(struct drm_atomic_state *state)
+{
+ struct drm_connector_state *conn_state;
+ struct drm_connector *connector;
+ struct drm_encoder *encoder;
+ unsigned encoder_mask = 0;
+ int i, ret;
+
+ for_each_connector_in_state(state, connector, conn_state, i) {
+ const struct drm_connector_helper_funcs *funcs = connector->helper_private;
+ struct drm_encoder *new_encoder;
+
+ if (!conn_state->crtc)
+ continue;
+
+ if (funcs->atomic_best_encoder)
+ new_encoder = funcs->atomic_best_encoder(connector, conn_state);
+ else
+ new_encoder = funcs->best_encoder(connector);
+
+ if (new_encoder)
+ encoder_mask |= 1 << drm_encoder_index(new_encoder);
+ }
+
+ drm_for_each_connector(connector, state->dev) {
+ struct drm_crtc_state *crtc_state;
+
+ if (drm_atomic_get_existing_connector_state(state, connector))
+ continue;
+
+ encoder = connector->state->best_encoder;
+ if (!encoder || !(encoder_mask & (1 << drm_encoder_index(encoder))))
+ continue;
+
+ conn_state = drm_atomic_get_connector_state(state, connector);
+ if (IS_ERR(conn_state))
+ return PTR_ERR(conn_state);
+
+ DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], disabling [CONNECTOR:%d:%s]\n",
+ encoder->base.id, encoder->name,
+ conn_state->crtc->base.id, conn_state->crtc->name,
+ connector->base.id, connector->name);
+
+ crtc_state = drm_atomic_get_existing_crtc_state(state, conn_state->crtc);
+
+ ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
+ if (ret)
+ return ret;
+
+ if (!crtc_state->connector_mask) {
+ ret = drm_atomic_set_mode_prop_for_crtc(crtc_state,
+ NULL);
+ if (ret < 0)
+ return ret;
+
+ crtc_state->active = false;
+ }
+ }
+
+ return 0;
+}
+
static bool
check_pending_encoder_assignment(struct drm_atomic_state *state,
struct drm_encoder *new_encoder)
@@ -448,6 +510,12 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
}
}
+ if (state->legacy_set_config) {
+ ret = disable_conflicting_connectors(state);
+ if (ret)
+ return ret;
+ }
+
for_each_connector_in_state(state, connector, connector_state, i) {
/*
* This only sets crtc->mode_changed for routing changes,
@@ -1796,6 +1864,7 @@ int drm_atomic_helper_set_config(struct drm_mode_set *set)
if (!state)
return -ENOMEM;
+ state->legacy_set_config = true;
state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
retry:
ret = __drm_atomic_helper_set_config(set, state);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 7fad193dc645..9a946df27f07 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1677,6 +1677,7 @@ struct drm_bridge {
* @dev: parent DRM device
* @allow_modeset: allow full modeset
* @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics
+ * @legacy_set_config: Disable conflicting encoders instead of failing with -EINVAL.
* @planes: pointer to array of plane pointers
* @plane_states: pointer to array of plane states pointers
* @crtcs: pointer to array of CRTC pointers
@@ -1690,6 +1691,7 @@ struct drm_atomic_state {
struct drm_device *dev;
bool allow_modeset : 1;
bool legacy_cursor_update : 1;
+ bool legacy_set_config : 1;
struct drm_plane **planes;
struct drm_plane_state **plane_states;
struct drm_crtc **crtcs;
--
2.1.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 5/7] drm/atomic: Handle encoder assignment conflicts in a separate check, v3.
2016-03-03 9:17 [PATCH v3 0/7] drm/atomic: Fix encoder stealing, v3 Maarten Lankhorst
` (3 preceding siblings ...)
2016-03-03 9:17 ` [PATCH v3 4/7] drm/atomic: Handle encoder stealing from set_config better Maarten Lankhorst
@ 2016-03-03 9:17 ` Maarten Lankhorst
2016-03-04 13:24 ` Ville Syrjälä
2016-03-03 9:17 ` [PATCH v3 6/7] drm/atomic: Clean up steal_encoder, v2 Maarten Lankhorst
` (2 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Maarten Lankhorst @ 2016-03-03 9:17 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
The current check doesn't handle the case where we don't steal an
encoder, but keep it on the current connector. If we repurpose
disable_conflicting_encoders to do the checking, we just have
to reject the ones that conflict.
Changes since v1:
- Return early with empty encoder_mask, drm_for_each_connector
requires connection_mutex held.
Changes since v2:
- Add comments for the loops.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Testcase: kms_setmode.invalid-clone-single-crtc-stealing
---
drivers/gpu/drm/drm_atomic_helper.c | 77 +++++++++++++++++++++----------------
1 file changed, 43 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index db11c2f9b098..bb60148c5c8d 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -86,7 +86,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
}
}
-static int disable_conflicting_connectors(struct drm_atomic_state *state)
+static int handle_conflicting_encoders(struct drm_atomic_state *state,
+ bool disable_conflicting_encoders)
{
struct drm_connector_state *conn_state;
struct drm_connector *connector;
@@ -94,6 +95,11 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state)
unsigned encoder_mask = 0;
int i, ret;
+ /*
+ * First loop, find all newly assigned encoders from the connectors
+ * part of the state. If the same encoder is assigned to multiple
+ * connectors bail out.
+ */
for_each_connector_in_state(state, connector, conn_state, i) {
const struct drm_connector_helper_funcs *funcs = connector->helper_private;
struct drm_encoder *new_encoder;
@@ -106,10 +112,33 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state)
else
new_encoder = funcs->best_encoder(connector);
- if (new_encoder)
+ if (new_encoder) {
+ if (encoder_mask & (1 << drm_encoder_index(new_encoder))) {
+ DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
+ new_encoder->base.id, new_encoder->name,
+ connector->base.id, connector->name);
+
+ return -EINVAL;
+ }
+
encoder_mask |= 1 << drm_encoder_index(new_encoder);
+ }
}
+ if (!encoder_mask)
+ return 0;
+
+ /*
+ * Second loop, iterate over all connectors not part of the state.
+ *
+ * If a conflicting encoder is found and disable_conflicting_encoders
+ * is not set, an error is returned. Userspace can provide a solution
+ * through the atomic ioctl.
+ *
+ * If the flag is set conflicting connectors are removed from the crtc
+ * and the crtc is disabled if no encoder is left. This preserves
+ * compatibility with the legacy set_config behavior.
+ */
drm_for_each_connector(connector, state->dev) {
struct drm_crtc_state *crtc_state;
@@ -120,6 +149,15 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state)
if (!encoder || !(encoder_mask & (1 << drm_encoder_index(encoder))))
continue;
+ if (!disable_conflicting_encoders) {
+ DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
+ encoder->base.id, encoder->name,
+ connector->state->crtc->base.id,
+ connector->state->crtc->name,
+ connector->base.id, connector->name);
+ return -EINVAL;
+ }
+
conn_state = drm_atomic_get_connector_state(state, connector);
if (IS_ERR(conn_state))
return PTR_ERR(conn_state);
@@ -148,26 +186,6 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state)
return 0;
}
-static bool
-check_pending_encoder_assignment(struct drm_atomic_state *state,
- struct drm_encoder *new_encoder)
-{
- struct drm_connector *connector;
- struct drm_connector_state *conn_state;
- int i;
-
- for_each_connector_in_state(state, connector, conn_state, i) {
- if (conn_state->best_encoder != new_encoder)
- continue;
-
- /* encoder already assigned and we're trying to re-steal it! */
- if (connector->state->best_encoder != conn_state->best_encoder)
- return false;
- }
-
- return true;
-}
-
static void
set_best_encoder(struct drm_atomic_state *state,
struct drm_connector_state *conn_state,
@@ -325,13 +343,6 @@ update_connector_routing(struct drm_atomic_state *state,
return 0;
}
- if (!check_pending_encoder_assignment(state, new_encoder)) {
- DRM_DEBUG_ATOMIC("Encoder for [CONNECTOR:%d:%s] already assigned\n",
- connector->base.id,
- connector->name);
- return -EINVAL;
- }
-
ret = steal_encoder(state, new_encoder);
if (ret) {
DRM_DEBUG_ATOMIC("Encoder stealing failed for [CONNECTOR:%d:%s]\n",
@@ -510,11 +521,9 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
}
}
- if (state->legacy_set_config) {
- ret = disable_conflicting_connectors(state);
- if (ret)
- return ret;
- }
+ ret = handle_conflicting_encoders(state, state->legacy_set_config);
+ if (ret)
+ return ret;
for_each_connector_in_state(state, connector, connector_state, i) {
/*
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 5/7] drm/atomic: Handle encoder assignment conflicts in a separate check, v3.
2016-03-03 9:17 ` [PATCH v3 5/7] drm/atomic: Handle encoder assignment conflicts in a separate check, v3 Maarten Lankhorst
@ 2016-03-04 13:24 ` Ville Syrjälä
0 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2016-03-04 13:24 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel
On Thu, Mar 03, 2016 at 10:17:40AM +0100, Maarten Lankhorst wrote:
> The current check doesn't handle the case where we don't steal an
> encoder, but keep it on the current connector. If we repurpose
> disable_conflicting_encoders to do the checking, we just have
> to reject the ones that conflict.
>
> Changes since v1:
> - Return early with empty encoder_mask, drm_for_each_connector
> requires connection_mutex held.
> Changes since v2:
> - Add comments for the loops.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Testcase: kms_setmode.invalid-clone-single-crtc-stealing
I was a bit lazy on looking at the individual patches 3-5, but the result
made sense to me, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
for all three.
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 77 +++++++++++++++++++++----------------
> 1 file changed, 43 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index db11c2f9b098..bb60148c5c8d 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -86,7 +86,8 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
> }
> }
>
> -static int disable_conflicting_connectors(struct drm_atomic_state *state)
> +static int handle_conflicting_encoders(struct drm_atomic_state *state,
> + bool disable_conflicting_encoders)
> {
> struct drm_connector_state *conn_state;
> struct drm_connector *connector;
> @@ -94,6 +95,11 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state)
> unsigned encoder_mask = 0;
> int i, ret;
>
> + /*
> + * First loop, find all newly assigned encoders from the connectors
> + * part of the state. If the same encoder is assigned to multiple
> + * connectors bail out.
> + */
> for_each_connector_in_state(state, connector, conn_state, i) {
> const struct drm_connector_helper_funcs *funcs = connector->helper_private;
> struct drm_encoder *new_encoder;
> @@ -106,10 +112,33 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state)
> else
> new_encoder = funcs->best_encoder(connector);
>
> - if (new_encoder)
> + if (new_encoder) {
> + if (encoder_mask & (1 << drm_encoder_index(new_encoder))) {
> + DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] on [CONNECTOR:%d:%s] already assigned\n",
> + new_encoder->base.id, new_encoder->name,
> + connector->base.id, connector->name);
> +
> + return -EINVAL;
> + }
> +
> encoder_mask |= 1 << drm_encoder_index(new_encoder);
> + }
> }
>
> + if (!encoder_mask)
> + return 0;
> +
> + /*
> + * Second loop, iterate over all connectors not part of the state.
> + *
> + * If a conflicting encoder is found and disable_conflicting_encoders
> + * is not set, an error is returned. Userspace can provide a solution
> + * through the atomic ioctl.
> + *
> + * If the flag is set conflicting connectors are removed from the crtc
> + * and the crtc is disabled if no encoder is left. This preserves
> + * compatibility with the legacy set_config behavior.
> + */
> drm_for_each_connector(connector, state->dev) {
> struct drm_crtc_state *crtc_state;
>
> @@ -120,6 +149,15 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state)
> if (!encoder || !(encoder_mask & (1 << drm_encoder_index(encoder))))
> continue;
>
> + if (!disable_conflicting_encoders) {
> + DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s] by [CONNECTOR:%d:%s]\n",
> + encoder->base.id, encoder->name,
> + connector->state->crtc->base.id,
> + connector->state->crtc->name,
> + connector->base.id, connector->name);
> + return -EINVAL;
> + }
> +
> conn_state = drm_atomic_get_connector_state(state, connector);
> if (IS_ERR(conn_state))
> return PTR_ERR(conn_state);
> @@ -148,26 +186,6 @@ static int disable_conflicting_connectors(struct drm_atomic_state *state)
> return 0;
> }
>
> -static bool
> -check_pending_encoder_assignment(struct drm_atomic_state *state,
> - struct drm_encoder *new_encoder)
> -{
> - struct drm_connector *connector;
> - struct drm_connector_state *conn_state;
> - int i;
> -
> - for_each_connector_in_state(state, connector, conn_state, i) {
> - if (conn_state->best_encoder != new_encoder)
> - continue;
> -
> - /* encoder already assigned and we're trying to re-steal it! */
> - if (connector->state->best_encoder != conn_state->best_encoder)
> - return false;
> - }
> -
> - return true;
> -}
> -
> static void
> set_best_encoder(struct drm_atomic_state *state,
> struct drm_connector_state *conn_state,
> @@ -325,13 +343,6 @@ update_connector_routing(struct drm_atomic_state *state,
> return 0;
> }
>
> - if (!check_pending_encoder_assignment(state, new_encoder)) {
> - DRM_DEBUG_ATOMIC("Encoder for [CONNECTOR:%d:%s] already assigned\n",
> - connector->base.id,
> - connector->name);
> - return -EINVAL;
> - }
> -
> ret = steal_encoder(state, new_encoder);
> if (ret) {
> DRM_DEBUG_ATOMIC("Encoder stealing failed for [CONNECTOR:%d:%s]\n",
> @@ -510,11 +521,9 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> }
> }
>
> - if (state->legacy_set_config) {
> - ret = disable_conflicting_connectors(state);
> - if (ret)
> - return ret;
> - }
> + ret = handle_conflicting_encoders(state, state->legacy_set_config);
> + if (ret)
> + return ret;
>
> for_each_connector_in_state(state, connector, connector_state, i) {
> /*
> --
> 2.1.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 6/7] drm/atomic: Clean up steal_encoder, v2.
2016-03-03 9:17 [PATCH v3 0/7] drm/atomic: Fix encoder stealing, v3 Maarten Lankhorst
` (4 preceding siblings ...)
2016-03-03 9:17 ` [PATCH v3 5/7] drm/atomic: Handle encoder assignment conflicts in a separate check, v3 Maarten Lankhorst
@ 2016-03-03 9:17 ` Maarten Lankhorst
2016-03-03 9:41 ` [Intel-gfx] " kbuild test robot
2016-03-04 13:27 ` Ville Syrjälä
2016-03-03 9:17 ` [PATCH v3 7/7] drm/atomic: Clean up update_connector_routing Maarten Lankhorst
2016-03-03 9:25 ` ✗ Fi.CI.BAT: failure for drm/atomic: Fix encoder stealing, v3 Patchwork
7 siblings, 2 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2016-03-03 9:17 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
Now that only encoders can be stolen that are part of the state
steal_encoder no longer needs to inspect all connectors,
just those that are part of the atomic state.
Changes since v1:
- Change return value to void, can no longer fail.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 25 +++++--------------------
1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index bb60148c5c8d..2395201eb7ab 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -227,25 +227,18 @@ set_best_encoder(struct drm_atomic_state *state,
conn_state->best_encoder = encoder;
}
-static int
+static void
steal_encoder(struct drm_atomic_state *state,
struct drm_encoder *encoder)
{
struct drm_crtc_state *crtc_state;
struct drm_connector *connector;
struct drm_connector_state *connector_state;
+ int i;
- drm_for_each_connector(connector, state->dev) {
+ for_each_connector_in_state(state, connector, connector_state, i) {
struct drm_crtc *encoder_crtc;
- if (connector->state->best_encoder != encoder)
- continue;
-
- connector_state = drm_atomic_get_connector_state(state,
- connector);
- if (IS_ERR(connector_state))
- return PTR_ERR(connector_state);
-
if (connector_state->best_encoder != encoder)
continue;
@@ -260,10 +253,8 @@ steal_encoder(struct drm_atomic_state *state,
crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
crtc_state->connectors_changed = true;
- return 0;
+ return;
}
-
- return 0;
}
static int
@@ -343,13 +334,7 @@ update_connector_routing(struct drm_atomic_state *state,
return 0;
}
- ret = steal_encoder(state, new_encoder);
- if (ret) {
- DRM_DEBUG_ATOMIC("Encoder stealing failed for [CONNECTOR:%d:%s]\n",
- connector->base.id,
- connector->name);
- return ret;
- }
+ steal_encoder(state, new_encoder);
if (WARN_ON(!connector_state->crtc))
return -EINVAL;
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Intel-gfx] [PATCH v3 6/7] drm/atomic: Clean up steal_encoder, v2.
2016-03-03 9:17 ` [PATCH v3 6/7] drm/atomic: Clean up steal_encoder, v2 Maarten Lankhorst
@ 2016-03-03 9:41 ` kbuild test robot
2016-03-04 13:27 ` Ville Syrjälä
1 sibling, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2016-03-03 9:41 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, kbuild-all, dri-devel
[-- Attachment #1: Type: text/plain, Size: 3137 bytes --]
Hi Maarten,
[auto build test WARNING on drm/drm-next]
[also build test WARNING on next-20160303]
[cannot apply to v4.5-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Maarten-Lankhorst/drm-atomic-Fix-encoder-stealing-v3/20160303-172123
base: git://people.freedesktop.org/~airlied/linux.git drm-next
config: x86_64-randconfig-x014-201609 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/gpu/drm/drm_atomic_helper.c: In function 'update_connector_routing':
>> drivers/gpu/drm/drm_atomic_helper.c:268:11: warning: unused variable 'ret' [-Wunused-variable]
int idx, ret;
^
vim +/ret +268 drivers/gpu/drm/drm_atomic_helper.c
c6c869e2 Maarten Lankhorst 2016-03-03 252
c6c869e2 Maarten Lankhorst 2016-03-03 253 crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
c6c869e2 Maarten Lankhorst 2016-03-03 254 crtc_state->connectors_changed = true;
c6c869e2 Maarten Lankhorst 2016-03-03 255
49bf38f8 Maarten Lankhorst 2016-03-03 256 return;
623369e5 Daniel Vetter 2014-09-16 257 }
623369e5 Daniel Vetter 2014-09-16 258 }
623369e5 Daniel Vetter 2014-09-16 259
623369e5 Daniel Vetter 2014-09-16 260 static int
184a4cc1 Maarten Lankhorst 2016-03-03 261 update_connector_routing(struct drm_atomic_state *state,
184a4cc1 Maarten Lankhorst 2016-03-03 262 struct drm_connector *connector,
184a4cc1 Maarten Lankhorst 2016-03-03 263 struct drm_connector_state *connector_state)
623369e5 Daniel Vetter 2014-09-16 264 {
b5ceff20 Ville Syrjälä 2015-03-10 265 const struct drm_connector_helper_funcs *funcs;
623369e5 Daniel Vetter 2014-09-16 266 struct drm_encoder *new_encoder;
623369e5 Daniel Vetter 2014-09-16 267 struct drm_crtc_state *crtc_state;
623369e5 Daniel Vetter 2014-09-16 @268 int idx, ret;
623369e5 Daniel Vetter 2014-09-16 269
17a38d9c Daniel Vetter 2015-02-22 270 DRM_DEBUG_ATOMIC("Updating routing for [CONNECTOR:%d:%s]\n",
623369e5 Daniel Vetter 2014-09-16 271 connector->base.id,
623369e5 Daniel Vetter 2014-09-16 272 connector->name);
623369e5 Daniel Vetter 2014-09-16 273
623369e5 Daniel Vetter 2014-09-16 274 if (connector->state->crtc != connector_state->crtc) {
623369e5 Daniel Vetter 2014-09-16 275 if (connector->state->crtc) {
623369e5 Daniel Vetter 2014-09-16 276 idx = drm_crtc_index(connector->state->crtc);
:::::: The code at line 268 was first introduced by commit
:::::: 623369e533e8a5f85999d605723efa4523554bae drm: Atomic crtc/connector updates using crtc/plane helper interfaces
:::::: TO: Daniel Vetter <daniel.vetter@ffwll.ch>
:::::: CC: Daniel Vetter <daniel.vetter@ffwll.ch>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 21254 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Intel-gfx] [PATCH v3 6/7] drm/atomic: Clean up steal_encoder, v2.
2016-03-03 9:17 ` [PATCH v3 6/7] drm/atomic: Clean up steal_encoder, v2 Maarten Lankhorst
2016-03-03 9:41 ` [Intel-gfx] " kbuild test robot
@ 2016-03-04 13:27 ` Ville Syrjälä
1 sibling, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2016-03-04 13:27 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel
On Thu, Mar 03, 2016 at 10:17:41AM +0100, Maarten Lankhorst wrote:
> Now that only encoders can be stolen that are part of the state
> steal_encoder no longer needs to inspect all connectors,
> just those that are part of the atomic state.
>
> Changes since v1:
> - Change return value to void, can no longer fail.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 25 +++++--------------------
> 1 file changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index bb60148c5c8d..2395201eb7ab 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -227,25 +227,18 @@ set_best_encoder(struct drm_atomic_state *state,
> conn_state->best_encoder = encoder;
> }
>
> -static int
> +static void
> steal_encoder(struct drm_atomic_state *state,
> struct drm_encoder *encoder)
> {
> struct drm_crtc_state *crtc_state;
> struct drm_connector *connector;
> struct drm_connector_state *connector_state;
> + int i;
>
> - drm_for_each_connector(connector, state->dev) {
> + for_each_connector_in_state(state, connector, connector_state, i) {
> struct drm_crtc *encoder_crtc;
>
> - if (connector->state->best_encoder != encoder)
> - continue;
> -
> - connector_state = drm_atomic_get_connector_state(state,
> - connector);
> - if (IS_ERR(connector_state))
> - return PTR_ERR(connector_state);
> -
> if (connector_state->best_encoder != encoder)
> continue;
>
> @@ -260,10 +253,8 @@ steal_encoder(struct drm_atomic_state *state,
> crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc);
> crtc_state->connectors_changed = true;
>
> - return 0;
> + return;
I was wondering if we should add some kind of sanity check here (or
maybe better do it afterwards across the entire device state?) to
make sure the same encoder didn't end up used multiple times?
Anyway, patch is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> }
> -
> - return 0;
> }
>
> static int
> @@ -343,13 +334,7 @@ update_connector_routing(struct drm_atomic_state *state,
> return 0;
> }
>
> - ret = steal_encoder(state, new_encoder);
> - if (ret) {
> - DRM_DEBUG_ATOMIC("Encoder stealing failed for [CONNECTOR:%d:%s]\n",
> - connector->base.id,
> - connector->name);
> - return ret;
> - }
> + steal_encoder(state, new_encoder);
>
> if (WARN_ON(!connector_state->crtc))
> return -EINVAL;
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 7/7] drm/atomic: Clean up update_connector_routing.
2016-03-03 9:17 [PATCH v3 0/7] drm/atomic: Fix encoder stealing, v3 Maarten Lankhorst
` (5 preceding siblings ...)
2016-03-03 9:17 ` [PATCH v3 6/7] drm/atomic: Clean up steal_encoder, v2 Maarten Lankhorst
@ 2016-03-03 9:17 ` Maarten Lankhorst
2016-03-04 13:29 ` [Intel-gfx] " Ville Syrjälä
2016-03-03 9:25 ` ✗ Fi.CI.BAT: failure for drm/atomic: Fix encoder stealing, v3 Patchwork
7 siblings, 1 reply; 14+ messages in thread
From: Maarten Lankhorst @ 2016-03-03 9:17 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
connector_state->crtc can no longer be unset by accident,
so that check can be removed. The other code open-codes
drm_atomic_get_existing_crtc_state, so use that.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 2395201eb7ab..9f0d16eb04b2 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -265,7 +265,7 @@ update_connector_routing(struct drm_atomic_state *state,
const struct drm_connector_helper_funcs *funcs;
struct drm_encoder *new_encoder;
struct drm_crtc_state *crtc_state;
- int idx, ret;
+ int ret;
DRM_DEBUG_ATOMIC("Updating routing for [CONNECTOR:%d:%s]\n",
connector->base.id,
@@ -273,16 +273,12 @@ update_connector_routing(struct drm_atomic_state *state,
if (connector->state->crtc != connector_state->crtc) {
if (connector->state->crtc) {
- idx = drm_crtc_index(connector->state->crtc);
-
- crtc_state = state->crtc_states[idx];
+ crtc_state = drm_atomic_get_existing_crtc_state(state, connector->state->crtc);
crtc_state->connectors_changed = true;
}
if (connector_state->crtc) {
- idx = drm_crtc_index(connector_state->crtc);
-
- crtc_state = state->crtc_states[idx];
+ crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc);
crtc_state->connectors_changed = true;
}
}
@@ -336,14 +332,9 @@ update_connector_routing(struct drm_atomic_state *state,
steal_encoder(state, new_encoder);
- if (WARN_ON(!connector_state->crtc))
- return -EINVAL;
-
set_best_encoder(state, connector_state, new_encoder);
- idx = drm_crtc_index(connector_state->crtc);
-
- crtc_state = state->crtc_states[idx];
+ crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc);
crtc_state->connectors_changed = true;
DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n",
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Intel-gfx] [PATCH v3 7/7] drm/atomic: Clean up update_connector_routing.
2016-03-03 9:17 ` [PATCH v3 7/7] drm/atomic: Clean up update_connector_routing Maarten Lankhorst
@ 2016-03-04 13:29 ` Ville Syrjälä
2016-03-08 12:20 ` Daniel Vetter
0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2016-03-04 13:29 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, dri-devel
On Thu, Mar 03, 2016 at 10:17:42AM +0100, Maarten Lankhorst wrote:
> connector_state->crtc can no longer be unset by accident,
> so that check can be removed. The other code open-codes
> drm_atomic_get_existing_crtc_state, so use that.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 2395201eb7ab..9f0d16eb04b2 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -265,7 +265,7 @@ update_connector_routing(struct drm_atomic_state *state,
> const struct drm_connector_helper_funcs *funcs;
> struct drm_encoder *new_encoder;
> struct drm_crtc_state *crtc_state;
> - int idx, ret;
> + int ret;
>
> DRM_DEBUG_ATOMIC("Updating routing for [CONNECTOR:%d:%s]\n",
> connector->base.id,
> @@ -273,16 +273,12 @@ update_connector_routing(struct drm_atomic_state *state,
>
> if (connector->state->crtc != connector_state->crtc) {
> if (connector->state->crtc) {
> - idx = drm_crtc_index(connector->state->crtc);
> -
> - crtc_state = state->crtc_states[idx];
> + crtc_state = drm_atomic_get_existing_crtc_state(state, connector->state->crtc);
> crtc_state->connectors_changed = true;
> }
>
> if (connector_state->crtc) {
> - idx = drm_crtc_index(connector_state->crtc);
> -
> - crtc_state = state->crtc_states[idx];
> + crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc);
> crtc_state->connectors_changed = true;
> }
> }
> @@ -336,14 +332,9 @@ update_connector_routing(struct drm_atomic_state *state,
>
> steal_encoder(state, new_encoder);
>
> - if (WARN_ON(!connector_state->crtc))
> - return -EINVAL;
> -
I was going to suggest just this when I read the code previously.
> set_best_encoder(state, connector_state, new_encoder);
>
> - idx = drm_crtc_index(connector_state->crtc);
> -
> - crtc_state = state->crtc_states[idx];
> + crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc);
These could have been a separate patch, but no biggie
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> crtc_state->connectors_changed = true;
>
> DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n",
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 7/7] drm/atomic: Clean up update_connector_routing.
2016-03-04 13:29 ` [Intel-gfx] " Ville Syrjälä
@ 2016-03-08 12:20 ` Daniel Vetter
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2016-03-08 12:20 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, dri-devel
On Fri, Mar 04, 2016 at 03:29:03PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 03, 2016 at 10:17:42AM +0100, Maarten Lankhorst wrote:
> > connector_state->crtc can no longer be unset by accident,
> > so that check can be removed. The other code open-codes
> > drm_atomic_get_existing_crtc_state, so use that.
> >
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 17 ++++-------------
> > 1 file changed, 4 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 2395201eb7ab..9f0d16eb04b2 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -265,7 +265,7 @@ update_connector_routing(struct drm_atomic_state *state,
> > const struct drm_connector_helper_funcs *funcs;
> > struct drm_encoder *new_encoder;
> > struct drm_crtc_state *crtc_state;
> > - int idx, ret;
> > + int ret;
> >
> > DRM_DEBUG_ATOMIC("Updating routing for [CONNECTOR:%d:%s]\n",
> > connector->base.id,
> > @@ -273,16 +273,12 @@ update_connector_routing(struct drm_atomic_state *state,
> >
> > if (connector->state->crtc != connector_state->crtc) {
> > if (connector->state->crtc) {
> > - idx = drm_crtc_index(connector->state->crtc);
> > -
> > - crtc_state = state->crtc_states[idx];
> > + crtc_state = drm_atomic_get_existing_crtc_state(state, connector->state->crtc);
> > crtc_state->connectors_changed = true;
> > }
> >
> > if (connector_state->crtc) {
> > - idx = drm_crtc_index(connector_state->crtc);
> > -
> > - crtc_state = state->crtc_states[idx];
> > + crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc);
> > crtc_state->connectors_changed = true;
> > }
> > }
> > @@ -336,14 +332,9 @@ update_connector_routing(struct drm_atomic_state *state,
> >
> > steal_encoder(state, new_encoder);
> >
> > - if (WARN_ON(!connector_state->crtc))
> > - return -EINVAL;
> > -
>
> I was going to suggest just this when I read the code previously.
>
> > set_best_encoder(state, connector_state, new_encoder);
> >
> > - idx = drm_crtc_index(connector_state->crtc);
> > -
> > - crtc_state = state->crtc_states[idx];
> > + crtc_state = drm_atomic_get_existing_crtc_state(state, connector_state->crtc);
>
> These could have been a separate patch, but no biggie
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Remaining patches applied to drm-misc, thanks.
-Daniel
>
> > crtc_state->connectors_changed = true;
> >
> > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on [CRTC:%d:%s]\n",
> > --
> > 2.1.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/atomic: Fix encoder stealing, v3.
2016-03-03 9:17 [PATCH v3 0/7] drm/atomic: Fix encoder stealing, v3 Maarten Lankhorst
` (6 preceding siblings ...)
2016-03-03 9:17 ` [PATCH v3 7/7] drm/atomic: Clean up update_connector_routing Maarten Lankhorst
@ 2016-03-03 9:25 ` Patchwork
7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2016-03-03 9:25 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/atomic: Fix encoder stealing, v3.
URL : https://patchwork.freedesktop.org/series/4045/
State : failure
== Summary ==
Series 4045v1 drm/atomic: Fix encoder stealing, v3.
http://patchwork.freedesktop.org/api/1.0/series/4045/revisions/1/mbox/
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass -> DMESG-WARN (hsw-brixbox)
pass -> DMESG-WARN (bdw-ultra)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass -> INCOMPLETE (hsw-gt2)
bdw-nuci7 total:169 pass:158 dwarn:0 dfail:0 fail:0 skip:11
bdw-ultra total:169 pass:154 dwarn:1 dfail:0 fail:0 skip:14
bsw-nuc-2 total:169 pass:137 dwarn:1 dfail:0 fail:1 skip:30
byt-nuc total:169 pass:144 dwarn:0 dfail:0 fail:0 skip:25
hsw-brixbox total:169 pass:153 dwarn:1 dfail:0 fail:0 skip:15
hsw-gt2 total:31 pass:30 dwarn:0 dfail:0 fail:0 skip:0
ilk-hp8440p total:169 pass:119 dwarn:0 dfail:0 fail:0 skip:50
ivb-t430s total:169 pass:154 dwarn:0 dfail:0 fail:0 skip:15
skl-i5k-2 total:169 pass:153 dwarn:0 dfail:0 fail:0 skip:16
skl-i7k-2 total:169 pass:153 dwarn:0 dfail:0 fail:0 skip:16
snb-x220t total:169 pass:145 dwarn:1 dfail:0 fail:1 skip:22
Results at /archive/results/CI_IGT_test/Patchwork_1520/
99ba2daf15b76f01916dc645b8d1b03dbbabf13c drm-intel-nightly: 2016y-03m-03d-08h-27m-26s UTC integration manifest
295f18455635ee101a4852f17773809dbce63a00 drm/atomic: Clean up update_connector_routing.
2eb019b3bb0cdaf744306129171cf9a76f2a7228 drm/atomic: Clean up steal_encoder, v2.
08a4219f9e70bc1723915968c5c9c7a4120c4dbf drm/atomic: Handle encoder assignment conflicts in a separate check, v3.
5875bb2f1b4a164f1ea16320faeedafa31a600ee drm/atomic: Handle encoder stealing from set_config better.
44c05a6b31d1602b8858fcd1a1aed40243542b69 drm/atomic: Always call steal_encoder, v2.
ad558b2f2099dfe79fc31a3538aa6ae9f1c1212e drm/atomic: Pass connector and state to update_connector_routing.
c0c9c6cdd42e37e23839bccf01ea6aaa6b388ca2 drm/atomic: Clean up update_output_state.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread