* [PATCH 0/6] drm/dp_mst: Improve VCPI helpers, use in nouveau @ 2018-10-23 23:12 Lyude Paul 2018-10-23 23:12 ` [PATCH 1/6] drm/dp_mst: Deprecate drm_dp_find_vcpi_slots() Lyude Paul ` (4 more replies) 0 siblings, 5 replies; 15+ messages in thread From: Lyude Paul @ 2018-10-23 23:12 UTC (permalink / raw) To: intel-gfx, dri-devel, nouveau This patchset does some cleaning up of the atomic VCPI helpers for MST, and converts nouveau over to using them. I would have included amdgpu in this patch as well, but at the moment moving them over to the atomic helpers is nontrivial. Cc: Daniel Vetter <daniel@ffwll.ch> Lyude Paul (6): drm/dp_mst: Deprecate drm_dp_find_vcpi_slots() drm/dp_mst: Remove all evil duplicate state pointers drm/atomic: Add ->atomic_check() hook for private objects drm/dp_mst: Start tracking per-port VCPI allocations drm/dp_mst: Check payload count in ->atomic_check() drm/nouveau: Use atomic VCPI helpers for MST drivers/gpu/drm/drm_atomic.c | 14 ++ drivers/gpu/drm/drm_dp_mst_topology.c | 187 ++++++++++++++++++++---- drivers/gpu/drm/i915/intel_dp_mst.c | 31 ++-- drivers/gpu/drm/nouveau/dispnv50/disp.c | 46 +++++- include/drm/drm_atomic.h | 16 ++ include/drm/drm_dp_mst_helper.h | 16 +- 6 files changed, 250 insertions(+), 60 deletions(-) -- 2.17.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/6] drm/dp_mst: Deprecate drm_dp_find_vcpi_slots() 2018-10-23 23:12 [PATCH 0/6] drm/dp_mst: Improve VCPI helpers, use in nouveau Lyude Paul @ 2018-10-23 23:12 ` Lyude Paul 2018-10-24 8:27 ` Daniel Vetter 2018-10-23 23:12 ` [PATCH 4/6] drm/dp_mst: Start tracking per-port VCPI allocations Lyude Paul ` (3 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Lyude Paul @ 2018-10-23 23:12 UTC (permalink / raw) To: intel-gfx, dri-devel, nouveau Because we have drm_dp_atomic_find_vcpi_slots(), which actually takes care to update the atomic state of the MST topology, prints valuable debugging output, and actually takes references to the ports it's checking! This explains some incorrect usage I've been seeing across the tree... Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel@ffwll.ch> --- drivers/gpu/drm/drm_dp_mst_topology.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 5ff1d79b86c4..8c3cfac437f4 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2569,9 +2569,16 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_ EXPORT_SYMBOL(drm_dp_mst_get_edid); /** - * drm_dp_find_vcpi_slots() - find slots for this PBN value + * drm_dp_find_vcpi_slots() - Find VCPI slots for this PBN value * @mgr: manager to use * @pbn: payload bandwidth to convert into slots. + * + * Calculate the number of VCPI slots that will be required for the given PBN + * value. This function is deprecated, and should not be used in atomic + * drivers. + * + * RETURNS: + * The total slots required for this port, or error. */ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, int pbn) -- 2.17.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] drm/dp_mst: Deprecate drm_dp_find_vcpi_slots() 2018-10-23 23:12 ` [PATCH 1/6] drm/dp_mst: Deprecate drm_dp_find_vcpi_slots() Lyude Paul @ 2018-10-24 8:27 ` Daniel Vetter 0 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2018-10-24 8:27 UTC (permalink / raw) To: Lyude Paul; +Cc: nouveau, intel-gfx, dri-devel On Tue, Oct 23, 2018 at 07:12:46PM -0400, Lyude Paul wrote: > Because we have drm_dp_atomic_find_vcpi_slots(), which actually takes > care to update the atomic state of the MST topology, prints valuable > debugging output, and actually takes references to the ports it's > checking! This explains some incorrect usage I've been seeing across the > tree... > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 5ff1d79b86c4..8c3cfac437f4 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -2569,9 +2569,16 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_ > EXPORT_SYMBOL(drm_dp_mst_get_edid); > > /** > - * drm_dp_find_vcpi_slots() - find slots for this PBN value > + * drm_dp_find_vcpi_slots() - Find VCPI slots for this PBN value > * @mgr: manager to use > * @pbn: payload bandwidth to convert into slots. > + * > + * Calculate the number of VCPI slots that will be required for the given PBN > + * value. This function is deprecated, and should not be used in atomic > + * drivers. > + * > + * RETURNS: > + * The total slots required for this port, or error. > */ Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, > int pbn) > -- > 2.17.2 > -- 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] 15+ messages in thread
* [PATCH 4/6] drm/dp_mst: Start tracking per-port VCPI allocations 2018-10-23 23:12 [PATCH 0/6] drm/dp_mst: Improve VCPI helpers, use in nouveau Lyude Paul 2018-10-23 23:12 ` [PATCH 1/6] drm/dp_mst: Deprecate drm_dp_find_vcpi_slots() Lyude Paul @ 2018-10-23 23:12 ` Lyude Paul [not found] ` <20181023231251.16883-5-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> [not found] ` <20181023231251.16883-1-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> ` (2 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Lyude Paul @ 2018-10-23 23:12 UTC (permalink / raw) To: intel-gfx, dri-devel, nouveau; +Cc: Daniel Vetter There has been a TODO waiting for quite a long time in drm_dp_mst_topology.c: /* We cannot rely on port->vcpi.num_slots to update * topology_state->avail_slots as the port may not exist if the parent * branch device was unplugged. This should be fixed by tracking * per-port slot allocation in drm_dp_mst_topology_state instead of * depending on the caller to tell us how many slots to release. */ That's not the only reason we should fix this: forcing the driver to track the VCPI allocations throughout a state's atomic check is error prone, because it means that extra care has to be taken with the order that drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() are called in in order to ensure idempotency. Currently the only driver actually using these helpers, i915, doesn't even do this correctly: multiple ->best_encoder() checks with i915's current implementation would not be idempotent and would over-allocate VCPI slots, something I learned trying to implement fallback retraining in MST. So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for each port. This allows us to ensure idempotency without having to rely on the driver as much. Additionally: the driver doesn't need to do any kind of VCPI slot tracking anymore if it doesn't need it for it's own internal state. Additionally; this moves the code for checking whether or not the VCPI allocations in the atomic state are valid into the new ->atomic_check() hook for private objects, so we can ensure that we only check the final VCPI allocation that would be incurred by the new state and not the mid-check VCPI allocations. This prevents us from failing checks which end up allocating VCPI slots to ports before freeing any of the VCPI slots allocated by ports which are about to be disabled. Also: update the documentation and make it more obvious that these /must/ be called by /all/ atomic drivers supporting MST. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_dp_mst_topology.c | 172 +++++++++++++++++++++----- drivers/gpu/drm/i915/intel_dp_mst.c | 31 ++--- include/drm/drm_dp_mst_helper.h | 10 +- 3 files changed, 167 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 8c3cfac437f4..adb4298570cc 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2614,21 +2614,33 @@ static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr, } /** - * drm_dp_atomic_find_vcpi_slots() - Find and add vcpi slots to the state + * drm_dp_atomic_find_vcpi_slots() - Find and add VCPI slots to the state * @state: global atomic state * @mgr: MST topology manager for the port * @port: port to find vcpi slots for * @pbn: bandwidth required for the mode in PBN * + * Allocates VCPI slots to @port, replacing any previous VCPI allocations it + * may have had. Any atomic drivers which support MST must call this function + * in their atomic_check() handlers to change the current VCPI allocation for + * the new state. After the ->atomic_check() hooks of the driver and all other + * mode objects in the state have been called, DRM will check the final VCPI + * allocations to ensure that they will fit into the available bandwidth on + * the topology. + * + * See also: drm_dp_atomic_release_vcpi_slots() + * * RETURNS: - * Total slots in the atomic state assigned for this port or error + * Total slots in the atomic state assigned for this port, or a negative error + * code if the port no longer exists */ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, int pbn) { struct drm_dp_mst_topology_state *topology_state; - int req_slots; + struct drm_dp_vcpi_allocation *pos, *vcpi = NULL; + int prev_slots, req_slots, ret; topology_state = drm_atomic_get_mst_topology_state(state, mgr); if (IS_ERR(topology_state)) @@ -2637,20 +2649,41 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, port = drm_dp_get_validated_port_ref(mgr, port); if (port == NULL) return -EINVAL; - req_slots = DIV_ROUND_UP(pbn, mgr->pbn_div); - DRM_DEBUG_KMS("vcpi slots req=%d, avail=%d\n", - req_slots, topology_state->avail_slots); - if (req_slots > topology_state->avail_slots) { - drm_dp_put_port(port); - return -ENOSPC; + /* Find the current allocation for this port, if any */ + list_for_each_entry(pos, &topology_state->vcpis, next) { + if (pos->port == port) { + vcpi = pos; + prev_slots = vcpi->vcpi; + break; + } } + if (!vcpi) + prev_slots = 0; - topology_state->avail_slots -= req_slots; - DRM_DEBUG_KMS("vcpi slots avail=%d", topology_state->avail_slots); + req_slots = DIV_ROUND_UP(pbn, mgr->pbn_div); + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] [MST PORT:%p] vcpi %d -> %d\n", + port->connector->base.id, port->connector->name, port, + prev_slots, req_slots); + /* Add the new allocation to the state */ + if (!vcpi) { + vcpi = kzalloc(sizeof(*vcpi), GFP_KERNEL); + if (!vcpi) { + ret = -ENOMEM; + goto out; + } + + vcpi->port = port; + list_add(&vcpi->next, &topology_state->vcpis); + } + vcpi->vcpi = req_slots; + + ret = req_slots; +out: drm_dp_put_port(port); - return req_slots; + return ret; } EXPORT_SYMBOL(drm_dp_atomic_find_vcpi_slots); @@ -2658,32 +2691,46 @@ EXPORT_SYMBOL(drm_dp_atomic_find_vcpi_slots); * drm_dp_atomic_release_vcpi_slots() - Release allocated vcpi slots * @state: global atomic state * @mgr: MST topology manager for the port - * @slots: number of vcpi slots to release + * + * Releases any VCPI slots that have been allocated to a port in the atomic + * state. Any atomic drivers which support MST must call this function in + * their connector's atomic_check() handler when the connector will no longer + * have VCPI allocated (e.g. because it's CRTC was removed). + * + * It is OK to call this even if @port has been removed from the system, in + * which case it will just amount to a no-op. + * + * See also: drm_dp_atomic_find_vcpi_slots() * * RETURNS: - * 0 if @slots were added back to &drm_dp_mst_topology_state->avail_slots or - * negative error code + * 0 if all slots for this port were added back to + * &drm_dp_mst_topology_state->avail_slots or negative error code */ int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr, - int slots) + struct drm_dp_mst_port *port) { struct drm_dp_mst_topology_state *topology_state; + struct drm_dp_vcpi_allocation *tmp, *pos; topology_state = drm_atomic_get_mst_topology_state(state, mgr); if (IS_ERR(topology_state)) return PTR_ERR(topology_state); - /* We cannot rely on port->vcpi.num_slots to update - * topology_state->avail_slots as the port may not exist if the parent - * branch device was unplugged. This should be fixed by tracking - * per-port slot allocation in drm_dp_mst_topology_state instead of - * depending on the caller to tell us how many slots to release. - */ - topology_state->avail_slots += slots; - DRM_DEBUG_KMS("vcpi slots released=%d, avail=%d\n", - slots, topology_state->avail_slots); + list_for_each_entry_safe(pos, tmp, &topology_state->vcpis, next) { + if (pos->port == port) { + list_del(&pos->next); + DRM_DEBUG_KMS("[MST PORT:%p] vcpi %d -> 0\n", + port, pos->vcpi); + kfree(pos); + return 0; + } + } + + /* If no allocation was found, all that means is that the port was + * destroyed since the last atomic commit. That's OK! + */ return 0; } EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); @@ -3112,15 +3159,50 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) static struct drm_private_state * drm_dp_mst_duplicate_state(struct drm_private_obj *obj) { - struct drm_dp_mst_topology_state *state; + struct drm_dp_mst_topology_state *state, *old_state = + to_dp_mst_topology_state(obj->state); + struct drm_dp_mst_topology_mgr *mgr = old_state->mgr; + struct drm_dp_mst_port *port; + struct drm_dp_vcpi_allocation *pos, *vcpi; - state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL); + state = kzalloc(sizeof(*state), GFP_KERNEL); if (!state) return NULL; __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base); + state->mgr = mgr; + INIT_LIST_HEAD(&state->vcpis); + + /* Copy over the VCPI allocations for ports that still exist */ + list_for_each_entry(pos, &old_state->vcpis, next) { + port = drm_dp_get_validated_port_ref(mgr, pos->port); + if (!port) { + DRM_DEBUG_ATOMIC("[MST PORT:%p] is gone, vcpi %d -> 0\n", + pos->port, pos->vcpi); + continue; + } + + vcpi = kzalloc(sizeof(*vcpi), GFP_KERNEL); + if (!vcpi) { + drm_dp_put_port(port); + goto fail_alloc; + } + + vcpi->port = port; + vcpi->vcpi = pos->vcpi; + list_add(&vcpi->next, &state->vcpis); + drm_dp_put_port(port); + } + return &state->base; + +fail_alloc: + list_for_each_entry_safe(pos, vcpi, &state->vcpis, next) + kfree(pos); + kfree(state); + + return NULL; } static void drm_dp_mst_destroy_state(struct drm_private_obj *obj, @@ -3128,13 +3210,45 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj, { struct drm_dp_mst_topology_state *mst_state = to_dp_mst_topology_state(state); + struct drm_dp_vcpi_allocation *pos, *tmp; + + list_for_each_entry_safe(pos, tmp, &mst_state->vcpis, next) + kfree(pos); kfree(mst_state); } +static int drm_dp_mst_atomic_check(struct drm_private_obj *obj, + struct drm_private_state *state) +{ + struct drm_dp_mst_topology_state *mst_state = + to_dp_mst_topology_state(state); + struct drm_dp_mst_topology_mgr *mgr = mst_state->mgr; + struct drm_dp_vcpi_allocation *pos; + int avail_slots = 63; + + list_for_each_entry(pos, &mst_state->vcpis, next) { + DRM_DEBUG_ATOMIC("[MST PORT:%p] requires %d vcpi slots\n", + pos->port, pos->vcpi); + + avail_slots -= pos->vcpi; + if (avail_slots < 0) { + DRM_DEBUG_ATOMIC("[MST PORT:%p] not enough vcpi slots in state %p (avail=%d)\n", + pos->port, state, + avail_slots + pos->vcpi); + return -ENOSPC; + } + } + DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p vcpi avail=%d used=%d\n", + mgr, state, avail_slots, 63 - avail_slots); + + return 0; +} + static const struct drm_private_state_funcs mst_state_funcs = { .atomic_duplicate_state = drm_dp_mst_duplicate_state, .atomic_destroy_state = drm_dp_mst_destroy_state, + .atomic_check = drm_dp_mst_atomic_check, }; /** @@ -3213,9 +3327,7 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, return -ENOMEM; mst_state->mgr = mgr; - - /* max. time slots - one slot for MTP header */ - mst_state->avail_slots = 63; + INIT_LIST_HEAD(&mst_state->vcpis); drm_atomic_private_obj_init(&mgr->base, &mst_state->base, diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 8b71d64ebd9d..aaf904738b78 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -114,28 +114,31 @@ static int intel_dp_mst_atomic_check(struct drm_connector *connector, struct drm_connector_state *old_conn_state; struct drm_crtc *old_crtc; struct drm_crtc_state *crtc_state; - int slots, ret = 0; + struct intel_connector *intel_connector = + to_intel_connector(connector); + struct drm_dp_mst_topology_mgr *mgr = + &intel_connector->mst_port->mst_mgr; + struct drm_dp_mst_port *port = intel_connector->port; + int ret = 0; old_conn_state = drm_atomic_get_old_connector_state(state, connector); old_crtc = old_conn_state->crtc; if (!old_crtc) return ret; - crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc); - slots = to_intel_crtc_state(crtc_state)->dp_m_n.tu; - if (drm_atomic_crtc_needs_modeset(crtc_state) && slots > 0) { - struct drm_dp_mst_topology_mgr *mgr; - struct drm_encoder *old_encoder; + /* Free VCPI, since compute_config() won't be run */ + if (!new_conn_state->crtc) { + crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc); - old_encoder = old_conn_state->best_encoder; - mgr = &enc_to_mst(old_encoder)->primary->dp.mst_mgr; - - ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots); - if (ret) - DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", slots, ret); - else - to_intel_crtc_state(crtc_state)->dp_m_n.tu = 0; + ret = drm_dp_atomic_release_vcpi_slots(state, mgr, port); + if (ret) { + DRM_DEBUG_KMS("failed releasing vcpi slots: %d\n", + ret); + return ret; + } + to_intel_crtc_state(crtc_state)->dp_m_n.tu = 0; } + return ret; } diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 59f005b419cf..4c0805e56335 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -406,9 +406,15 @@ struct drm_dp_payload { #define to_dp_mst_topology_state(x) container_of(x, struct drm_dp_mst_topology_state, base) +struct drm_dp_vcpi_allocation { + struct drm_dp_mst_port *port; + int vcpi; + struct list_head next; +}; + struct drm_dp_mst_topology_state { struct drm_private_state base; - int avail_slots; + struct list_head vcpis; struct drm_dp_mst_topology_mgr *mgr; }; @@ -624,7 +630,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_port *port, int pbn); int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr, - int slots); + struct drm_dp_mst_port *port); int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, bool power_up); -- 2.17.2 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
[parent not found: <20181023231251.16883-5-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 4/6] drm/dp_mst: Start tracking per-port VCPI allocations [not found] ` <20181023231251.16883-5-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2018-10-24 8:55 ` Daniel Vetter 0 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2018-10-24 8:55 UTC (permalink / raw) To: Lyude Paul Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On Tue, Oct 23, 2018 at 07:12:49PM -0400, Lyude Paul wrote: > There has been a TODO waiting for quite a long time in > drm_dp_mst_topology.c: > > /* We cannot rely on port->vcpi.num_slots to update > * topology_state->avail_slots as the port may not exist if the parent > * branch device was unplugged. This should be fixed by tracking > * per-port slot allocation in drm_dp_mst_topology_state instead of > * depending on the caller to tell us how many slots to release. > */ > > That's not the only reason we should fix this: forcing the driver to > track the VCPI allocations throughout a state's atomic check is > error prone, because it means that extra care has to be taken with the > order that drm_dp_atomic_find_vcpi_slots() and > drm_dp_atomic_release_vcpi_slots() are called in in order to ensure > idempotency. Currently the only driver actually using these helpers, > i915, doesn't even do this correctly: multiple ->best_encoder() checks > with i915's current implementation would not be idempotent and would > over-allocate VCPI slots, something I learned trying to implement > fallback retraining in MST. > > So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots() > and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for > each port. This allows us to ensure idempotency without having to rely > on the driver as much. Additionally: the driver doesn't need to do any > kind of VCPI slot tracking anymore if it doesn't need it for it's own > internal state. > > Additionally; this moves the code for checking whether or not the > VCPI allocations in the atomic state are valid into the new > ->atomic_check() hook for private objects, so we can ensure that we only > check the final VCPI allocation that would be incurred by the new state > and not the mid-check VCPI allocations. This prevents us from failing > checks which end up allocating VCPI slots to ports before freeing any of > the VCPI slots allocated by ports which are about to be disabled. > > Also: update the documentation and make it more obvious that these > /must/ be called by /all/ atomic drivers supporting MST. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> lgtm, but I'll wait with detailed review until we have the "where should we atomic_check this" question sorted out. -Daniel > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 172 +++++++++++++++++++++----- > drivers/gpu/drm/i915/intel_dp_mst.c | 31 ++--- > include/drm/drm_dp_mst_helper.h | 10 +- > 3 files changed, 167 insertions(+), 46 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 8c3cfac437f4..adb4298570cc 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -2614,21 +2614,33 @@ static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr, > } > > /** > - * drm_dp_atomic_find_vcpi_slots() - Find and add vcpi slots to the state > + * drm_dp_atomic_find_vcpi_slots() - Find and add VCPI slots to the state > * @state: global atomic state > * @mgr: MST topology manager for the port > * @port: port to find vcpi slots for > * @pbn: bandwidth required for the mode in PBN > * > + * Allocates VCPI slots to @port, replacing any previous VCPI allocations it > + * may have had. Any atomic drivers which support MST must call this function > + * in their atomic_check() handlers to change the current VCPI allocation for > + * the new state. After the ->atomic_check() hooks of the driver and all other > + * mode objects in the state have been called, DRM will check the final VCPI > + * allocations to ensure that they will fit into the available bandwidth on > + * the topology. > + * > + * See also: drm_dp_atomic_release_vcpi_slots() > + * > * RETURNS: > - * Total slots in the atomic state assigned for this port or error > + * Total slots in the atomic state assigned for this port, or a negative error > + * code if the port no longer exists > */ > int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, > struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_port *port, int pbn) > { > struct drm_dp_mst_topology_state *topology_state; > - int req_slots; > + struct drm_dp_vcpi_allocation *pos, *vcpi = NULL; > + int prev_slots, req_slots, ret; > > topology_state = drm_atomic_get_mst_topology_state(state, mgr); > if (IS_ERR(topology_state)) > @@ -2637,20 +2649,41 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, > port = drm_dp_get_validated_port_ref(mgr, port); > if (port == NULL) > return -EINVAL; > - req_slots = DIV_ROUND_UP(pbn, mgr->pbn_div); > - DRM_DEBUG_KMS("vcpi slots req=%d, avail=%d\n", > - req_slots, topology_state->avail_slots); > > - if (req_slots > topology_state->avail_slots) { > - drm_dp_put_port(port); > - return -ENOSPC; > + /* Find the current allocation for this port, if any */ > + list_for_each_entry(pos, &topology_state->vcpis, next) { > + if (pos->port == port) { > + vcpi = pos; > + prev_slots = vcpi->vcpi; > + break; > + } > } > + if (!vcpi) > + prev_slots = 0; > > - topology_state->avail_slots -= req_slots; > - DRM_DEBUG_KMS("vcpi slots avail=%d", topology_state->avail_slots); > + req_slots = DIV_ROUND_UP(pbn, mgr->pbn_div); > + > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] [MST PORT:%p] vcpi %d -> %d\n", > + port->connector->base.id, port->connector->name, port, > + prev_slots, req_slots); > > + /* Add the new allocation to the state */ > + if (!vcpi) { > + vcpi = kzalloc(sizeof(*vcpi), GFP_KERNEL); > + if (!vcpi) { > + ret = -ENOMEM; > + goto out; > + } > + > + vcpi->port = port; > + list_add(&vcpi->next, &topology_state->vcpis); > + } > + vcpi->vcpi = req_slots; > + > + ret = req_slots; > +out: > drm_dp_put_port(port); > - return req_slots; > + return ret; > } > EXPORT_SYMBOL(drm_dp_atomic_find_vcpi_slots); > > @@ -2658,32 +2691,46 @@ EXPORT_SYMBOL(drm_dp_atomic_find_vcpi_slots); > * drm_dp_atomic_release_vcpi_slots() - Release allocated vcpi slots > * @state: global atomic state > * @mgr: MST topology manager for the port > - * @slots: number of vcpi slots to release > + * > + * Releases any VCPI slots that have been allocated to a port in the atomic > + * state. Any atomic drivers which support MST must call this function in > + * their connector's atomic_check() handler when the connector will no longer > + * have VCPI allocated (e.g. because it's CRTC was removed). > + * > + * It is OK to call this even if @port has been removed from the system, in > + * which case it will just amount to a no-op. > + * > + * See also: drm_dp_atomic_find_vcpi_slots() > * > * RETURNS: > - * 0 if @slots were added back to &drm_dp_mst_topology_state->avail_slots or > - * negative error code > + * 0 if all slots for this port were added back to > + * &drm_dp_mst_topology_state->avail_slots or negative error code > */ > int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, > struct drm_dp_mst_topology_mgr *mgr, > - int slots) > + struct drm_dp_mst_port *port) > { > struct drm_dp_mst_topology_state *topology_state; > + struct drm_dp_vcpi_allocation *tmp, *pos; > > topology_state = drm_atomic_get_mst_topology_state(state, mgr); > if (IS_ERR(topology_state)) > return PTR_ERR(topology_state); > > - /* We cannot rely on port->vcpi.num_slots to update > - * topology_state->avail_slots as the port may not exist if the parent > - * branch device was unplugged. This should be fixed by tracking > - * per-port slot allocation in drm_dp_mst_topology_state instead of > - * depending on the caller to tell us how many slots to release. > - */ > - topology_state->avail_slots += slots; > - DRM_DEBUG_KMS("vcpi slots released=%d, avail=%d\n", > - slots, topology_state->avail_slots); > + list_for_each_entry_safe(pos, tmp, &topology_state->vcpis, next) { > + if (pos->port == port) { > + list_del(&pos->next); > + DRM_DEBUG_KMS("[MST PORT:%p] vcpi %d -> 0\n", > + port, pos->vcpi); > > + kfree(pos); > + return 0; > + } > + } > + > + /* If no allocation was found, all that means is that the port was > + * destroyed since the last atomic commit. That's OK! > + */ > return 0; > } > EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); > @@ -3112,15 +3159,50 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) > static struct drm_private_state * > drm_dp_mst_duplicate_state(struct drm_private_obj *obj) > { > - struct drm_dp_mst_topology_state *state; > + struct drm_dp_mst_topology_state *state, *old_state = > + to_dp_mst_topology_state(obj->state); > + struct drm_dp_mst_topology_mgr *mgr = old_state->mgr; > + struct drm_dp_mst_port *port; > + struct drm_dp_vcpi_allocation *pos, *vcpi; > > - state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL); > + state = kzalloc(sizeof(*state), GFP_KERNEL); > if (!state) > return NULL; > > __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base); > > + state->mgr = mgr; > + INIT_LIST_HEAD(&state->vcpis); > + > + /* Copy over the VCPI allocations for ports that still exist */ > + list_for_each_entry(pos, &old_state->vcpis, next) { > + port = drm_dp_get_validated_port_ref(mgr, pos->port); > + if (!port) { > + DRM_DEBUG_ATOMIC("[MST PORT:%p] is gone, vcpi %d -> 0\n", > + pos->port, pos->vcpi); > + continue; > + } > + > + vcpi = kzalloc(sizeof(*vcpi), GFP_KERNEL); > + if (!vcpi) { > + drm_dp_put_port(port); > + goto fail_alloc; > + } > + > + vcpi->port = port; > + vcpi->vcpi = pos->vcpi; > + list_add(&vcpi->next, &state->vcpis); > + drm_dp_put_port(port); > + } > + > return &state->base; > + > +fail_alloc: > + list_for_each_entry_safe(pos, vcpi, &state->vcpis, next) > + kfree(pos); > + kfree(state); > + > + return NULL; > } > > static void drm_dp_mst_destroy_state(struct drm_private_obj *obj, > @@ -3128,13 +3210,45 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj, > { > struct drm_dp_mst_topology_state *mst_state = > to_dp_mst_topology_state(state); > + struct drm_dp_vcpi_allocation *pos, *tmp; > + > + list_for_each_entry_safe(pos, tmp, &mst_state->vcpis, next) > + kfree(pos); > > kfree(mst_state); > } > > +static int drm_dp_mst_atomic_check(struct drm_private_obj *obj, > + struct drm_private_state *state) > +{ > + struct drm_dp_mst_topology_state *mst_state = > + to_dp_mst_topology_state(state); > + struct drm_dp_mst_topology_mgr *mgr = mst_state->mgr; > + struct drm_dp_vcpi_allocation *pos; > + int avail_slots = 63; > + > + list_for_each_entry(pos, &mst_state->vcpis, next) { > + DRM_DEBUG_ATOMIC("[MST PORT:%p] requires %d vcpi slots\n", > + pos->port, pos->vcpi); > + > + avail_slots -= pos->vcpi; > + if (avail_slots < 0) { > + DRM_DEBUG_ATOMIC("[MST PORT:%p] not enough vcpi slots in state %p (avail=%d)\n", > + pos->port, state, > + avail_slots + pos->vcpi); > + return -ENOSPC; > + } > + } > + DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p vcpi avail=%d used=%d\n", > + mgr, state, avail_slots, 63 - avail_slots); > + > + return 0; > +} > + > static const struct drm_private_state_funcs mst_state_funcs = { > .atomic_duplicate_state = drm_dp_mst_duplicate_state, > .atomic_destroy_state = drm_dp_mst_destroy_state, > + .atomic_check = drm_dp_mst_atomic_check, > }; > > /** > @@ -3213,9 +3327,7 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, > return -ENOMEM; > > mst_state->mgr = mgr; > - > - /* max. time slots - one slot for MTP header */ > - mst_state->avail_slots = 63; > + INIT_LIST_HEAD(&mst_state->vcpis); > > drm_atomic_private_obj_init(&mgr->base, > &mst_state->base, > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c > index 8b71d64ebd9d..aaf904738b78 100644 > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > @@ -114,28 +114,31 @@ static int intel_dp_mst_atomic_check(struct drm_connector *connector, > struct drm_connector_state *old_conn_state; > struct drm_crtc *old_crtc; > struct drm_crtc_state *crtc_state; > - int slots, ret = 0; > + struct intel_connector *intel_connector = > + to_intel_connector(connector); > + struct drm_dp_mst_topology_mgr *mgr = > + &intel_connector->mst_port->mst_mgr; > + struct drm_dp_mst_port *port = intel_connector->port; > + int ret = 0; > > old_conn_state = drm_atomic_get_old_connector_state(state, connector); > old_crtc = old_conn_state->crtc; > if (!old_crtc) > return ret; > > - crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc); > - slots = to_intel_crtc_state(crtc_state)->dp_m_n.tu; > - if (drm_atomic_crtc_needs_modeset(crtc_state) && slots > 0) { > - struct drm_dp_mst_topology_mgr *mgr; > - struct drm_encoder *old_encoder; > + /* Free VCPI, since compute_config() won't be run */ > + if (!new_conn_state->crtc) { > + crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc); > > - old_encoder = old_conn_state->best_encoder; > - mgr = &enc_to_mst(old_encoder)->primary->dp.mst_mgr; > - > - ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots); > - if (ret) > - DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", slots, ret); > - else > - to_intel_crtc_state(crtc_state)->dp_m_n.tu = 0; > + ret = drm_dp_atomic_release_vcpi_slots(state, mgr, port); > + if (ret) { > + DRM_DEBUG_KMS("failed releasing vcpi slots: %d\n", > + ret); > + return ret; > + } > + to_intel_crtc_state(crtc_state)->dp_m_n.tu = 0; > } > + > return ret; > } > > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h > index 59f005b419cf..4c0805e56335 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -406,9 +406,15 @@ struct drm_dp_payload { > > #define to_dp_mst_topology_state(x) container_of(x, struct drm_dp_mst_topology_state, base) > > +struct drm_dp_vcpi_allocation { > + struct drm_dp_mst_port *port; > + int vcpi; > + struct list_head next; > +}; > + > struct drm_dp_mst_topology_state { > struct drm_private_state base; > - int avail_slots; > + struct list_head vcpis; > struct drm_dp_mst_topology_mgr *mgr; > }; > > @@ -624,7 +630,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, > struct drm_dp_mst_port *port, int pbn); > int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, > struct drm_dp_mst_topology_mgr *mgr, > - int slots); > + struct drm_dp_mst_port *port); > int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_port *port, bool power_up); > > -- > 2.17.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <20181023231251.16883-1-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* [PATCH 2/6] drm/dp_mst: Remove all evil duplicate state pointers [not found] ` <20181023231251.16883-1-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2018-10-23 23:12 ` Lyude Paul 2018-10-24 8:27 ` Daniel Vetter 2018-10-23 23:12 ` [PATCH 3/6] drm/atomic: Add ->atomic_check() hook for private objects Lyude Paul ` (3 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Lyude Paul @ 2018-10-23 23:12 UTC (permalink / raw) To: intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW There's no reason to track the atomic state three times. Unfortunately, this is currently what we're doing, and even worse is that there is only one actually correct state pointer: the one in mst_state->base.state. mgr->state never seems to be used, along with the one in mst_state->state. This confused me for over 4 hours until I realized there was no magic behind these pointers. So, let's save everyone else from the trouble. Signed-off-by: Lyude Paul <lyude@redhat.com>. Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Lyude Paul <lyude@redhat.com> --- include/drm/drm_dp_mst_helper.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 7f78d26a0766..59f005b419cf 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -409,7 +409,6 @@ struct drm_dp_payload { struct drm_dp_mst_topology_state { struct drm_private_state base; int avail_slots; - struct drm_atomic_state *state; struct drm_dp_mst_topology_mgr *mgr; }; @@ -497,11 +496,6 @@ struct drm_dp_mst_topology_mgr { */ int pbn_div; - /** - * @state: State information for topology manager - */ - struct drm_dp_mst_topology_state *state; - /** * @funcs: Atomic helper callbacks */ -- 2.17.2 _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] drm/dp_mst: Remove all evil duplicate state pointers 2018-10-23 23:12 ` [PATCH 2/6] drm/dp_mst: Remove all evil duplicate state pointers Lyude Paul @ 2018-10-24 8:27 ` Daniel Vetter 0 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2018-10-24 8:27 UTC (permalink / raw) To: Lyude Paul; +Cc: nouveau, intel-gfx, dri-devel, Daniel Vetter On Tue, Oct 23, 2018 at 07:12:47PM -0400, Lyude Paul wrote: > There's no reason to track the atomic state three times. Unfortunately, > this is currently what we're doing, and even worse is that there is only > one actually correct state pointer: the one in mst_state->base.state. > mgr->state never seems to be used, along with the one in > mst_state->state. > > This confused me for over 4 hours until I realized there was no magic > behind these pointers. So, let's save everyone else from the trouble. > > Signed-off-by: Lyude Paul <lyude@redhat.com>. > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Lyude Paul <lyude@redhat.com> Uh, that's some fail :-/ Thanks for cleaning up. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > include/drm/drm_dp_mst_helper.h | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h > index 7f78d26a0766..59f005b419cf 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -409,7 +409,6 @@ struct drm_dp_payload { > struct drm_dp_mst_topology_state { > struct drm_private_state base; > int avail_slots; > - struct drm_atomic_state *state; > struct drm_dp_mst_topology_mgr *mgr; > }; > > @@ -497,11 +496,6 @@ struct drm_dp_mst_topology_mgr { > */ > int pbn_div; > > - /** > - * @state: State information for topology manager > - */ > - struct drm_dp_mst_topology_state *state; > - > /** > * @funcs: Atomic helper callbacks > */ > -- > 2.17.2 > -- 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] 15+ messages in thread
* [PATCH 3/6] drm/atomic: Add ->atomic_check() hook for private objects [not found] ` <20181023231251.16883-1-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2018-10-23 23:12 ` [PATCH 2/6] drm/dp_mst: Remove all evil duplicate state pointers Lyude Paul @ 2018-10-23 23:12 ` Lyude Paul 2018-10-24 8:45 ` Daniel Vetter 2018-10-23 23:12 ` [PATCH 5/6] drm/dp_mst: Check payload count in ->atomic_check() Lyude Paul ` (2 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Lyude Paul @ 2018-10-23 23:12 UTC (permalink / raw) To: intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Currently; private objects are mostly used just for driver-specific atomic state, but not entirely. MST also uses private objects for holding it's atomic state, but in order to make our MST helpers safer for atomic we need to be able to check that state after the driver has performed it's own checks on the atomic state. So, add an optional ->atomic_check() callback into drm_private_state_funcs that gets called after the driver's atomic checks. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_atomic.c | 14 ++++++++++++++ include/drm/drm_atomic.h | 16 ++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 3dbfbddae7e6..2db9f219732b 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -966,6 +966,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state) struct drm_crtc_state *crtc_state; struct drm_connector *conn; struct drm_connector_state *conn_state; + struct drm_private_obj *priv_obj; + struct drm_private_state *priv_state; int i, ret = 0; DRM_DEBUG_ATOMIC("checking %p\n", state); @@ -1007,6 +1009,18 @@ int drm_atomic_check_only(struct drm_atomic_state *state) } } + for_each_new_private_obj_in_state(state, priv_obj, priv_state, i) { + if (!priv_obj->funcs->atomic_check) + continue; + + ret = priv_obj->funcs->atomic_check(priv_obj, priv_state); + if (ret) { + DRM_DEBUG_ATOMIC("[PRIVATE:%p] atomic check on state %p failed\n", + priv_obj, priv_state); + return ret; + } + } + if (!state->allow_modeset) { for_each_new_crtc_in_state(state, crtc, crtc_state, i) { if (drm_atomic_crtc_needs_modeset(crtc_state)) { diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index f9b35834c45d..3e504eeb1122 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -216,6 +216,22 @@ struct drm_private_state_funcs { */ void (*atomic_destroy_state)(struct drm_private_obj *obj, struct drm_private_state *state); + + /** + * @atomic_check: + * + * Perform a check of the current state of the private object to + * ensure that it's valid. This is an optional callback. If + * implemented, it will be called after atomic checks have been + * performed on all of the planes, CRTCs, connectors, and the new + * &drm_mode_config in the atomic state. + * + * RETURNS: + * + * 0 on success, negative error code on failure. + */ + int (*atomic_check)(struct drm_private_obj *obj, + struct drm_private_state *state); }; /** -- 2.17.2 _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] drm/atomic: Add ->atomic_check() hook for private objects 2018-10-23 23:12 ` [PATCH 3/6] drm/atomic: Add ->atomic_check() hook for private objects Lyude Paul @ 2018-10-24 8:45 ` Daniel Vetter 0 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2018-10-24 8:45 UTC (permalink / raw) To: Lyude Paul; +Cc: nouveau, intel-gfx, dri-devel, Daniel Vetter On Tue, Oct 23, 2018 at 07:12:48PM -0400, Lyude Paul wrote: > Currently; private objects are mostly used just for driver-specific > atomic state, but not entirely. MST also uses private objects for > holding it's atomic state, but in order to make our MST helpers safer > for atomic we need to be able to check that state after the driver has > performed it's own checks on the atomic state. So, add an optional > ->atomic_check() callback into drm_private_state_funcs that gets called > after the driver's atomic checks. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> I think the overall aim here of putting more of the validation into the dp mst helper makes tons of sense. I'm not sure whether adding an atomic_check callback to private state objects makes sense. These can be used for anything, and drivers do use them for anything really. So it's entirely impossible to have just 1 place where you call the ->atomic_check for all private objects. This doesn't work for all the other more standardized objects either, it's guaranteed to fail for something that's 100% generic, no standard use case. Also, you're putting helper-level logic (the object based ->atomic_check callback) into the core, that's midlayer mixing. Instead I think a better design is to just write something for dp mst. With the private state macros it's easy to iterate just over all the mst states (filter for the mst vtable, you could wrap that into a helper macro with for_each_if even). And then expose one function from mst helpers to drivers that they're supposed to call from their atomic_check, at exactly the right spot. I think the overlap of "drivers that want mst" and "drivers simply enough they don't need their own atomic_check" is zero. This won't impose any trapdoors on people trying to use private objects to e.g. manage plane scaler or things like that. And we could still hook it up by default and e.g. call the drm_dp_mst_topology_atomic_check() from drm_atomic_helper_check(), maybe right after calling drm_atomic_helper_check_modeset(). But not from the same, to give drivers a bit more flexibility in how they handle this (similar to how the zpos thing works). -Daniel > --- > drivers/gpu/drm/drm_atomic.c | 14 ++++++++++++++ > include/drm/drm_atomic.h | 16 ++++++++++++++++ > 2 files changed, 30 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 3dbfbddae7e6..2db9f219732b 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -966,6 +966,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state) > struct drm_crtc_state *crtc_state; > struct drm_connector *conn; > struct drm_connector_state *conn_state; > + struct drm_private_obj *priv_obj; > + struct drm_private_state *priv_state; > int i, ret = 0; > > DRM_DEBUG_ATOMIC("checking %p\n", state); > @@ -1007,6 +1009,18 @@ int drm_atomic_check_only(struct drm_atomic_state *state) > } > } > > + for_each_new_private_obj_in_state(state, priv_obj, priv_state, i) { > + if (!priv_obj->funcs->atomic_check) > + continue; > + > + ret = priv_obj->funcs->atomic_check(priv_obj, priv_state); > + if (ret) { > + DRM_DEBUG_ATOMIC("[PRIVATE:%p] atomic check on state %p failed\n", > + priv_obj, priv_state); > + return ret; > + } > + } > + > if (!state->allow_modeset) { > for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > if (drm_atomic_crtc_needs_modeset(crtc_state)) { > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index f9b35834c45d..3e504eeb1122 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -216,6 +216,22 @@ struct drm_private_state_funcs { > */ > void (*atomic_destroy_state)(struct drm_private_obj *obj, > struct drm_private_state *state); > + > + /** > + * @atomic_check: > + * > + * Perform a check of the current state of the private object to > + * ensure that it's valid. This is an optional callback. If > + * implemented, it will be called after atomic checks have been > + * performed on all of the planes, CRTCs, connectors, and the new > + * &drm_mode_config in the atomic state. > + * > + * RETURNS: > + * > + * 0 on success, negative error code on failure. > + */ > + int (*atomic_check)(struct drm_private_obj *obj, > + struct drm_private_state *state); > }; > > /** > -- > 2.17.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/6] drm/dp_mst: Check payload count in ->atomic_check() [not found] ` <20181023231251.16883-1-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2018-10-23 23:12 ` [PATCH 2/6] drm/dp_mst: Remove all evil duplicate state pointers Lyude Paul 2018-10-23 23:12 ` [PATCH 3/6] drm/atomic: Add ->atomic_check() hook for private objects Lyude Paul @ 2018-10-23 23:12 ` Lyude Paul [not found] ` <20181023231251.16883-6-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2018-10-23 23:12 ` [PATCH 6/6] drm/nouveau: Use atomic VCPI helpers for MST Lyude Paul 2018-10-24 8:50 ` [PATCH 0/6] drm/dp_mst: Improve VCPI helpers, use in nouveau Daniel Vetter 4 siblings, 1 reply; 15+ messages in thread From: Lyude Paul @ 2018-10-23 23:12 UTC (permalink / raw) To: intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW It occurred to me that we never actually check this! So let's start doing that. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_dp_mst_topology.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index adb4298570cc..cafb769a4ec3 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3225,7 +3225,7 @@ static int drm_dp_mst_atomic_check(struct drm_private_obj *obj, to_dp_mst_topology_state(state); struct drm_dp_mst_topology_mgr *mgr = mst_state->mgr; struct drm_dp_vcpi_allocation *pos; - int avail_slots = 63; + int avail_slots = 63, payload_count = 0; list_for_each_entry(pos, &mst_state->vcpis, next) { DRM_DEBUG_ATOMIC("[MST PORT:%p] requires %d vcpi slots\n", @@ -3238,6 +3238,12 @@ static int drm_dp_mst_atomic_check(struct drm_private_obj *obj, avail_slots + pos->vcpi); return -ENOSPC; } + + if (++payload_count > mgr->max_payloads) { + DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p has too many payloads (max=%d)\n", + mgr, state, mgr->max_payloads); + return -EINVAL; + } } DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p vcpi avail=%d used=%d\n", mgr, state, avail_slots, 63 - avail_slots); -- 2.17.2 _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply related [flat|nested] 15+ messages in thread
[parent not found: <20181023231251.16883-6-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 5/6] drm/dp_mst: Check payload count in ->atomic_check() [not found] ` <20181023231251.16883-6-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2018-10-24 8:54 ` Daniel Vetter 0 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2018-10-24 8:54 UTC (permalink / raw) To: Lyude Paul Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On Tue, Oct 23, 2018 at 07:12:50PM -0400, Lyude Paul wrote: > It occurred to me that we never actually check this! So let's start > doing that. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index adb4298570cc..cafb769a4ec3 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -3225,7 +3225,7 @@ static int drm_dp_mst_atomic_check(struct drm_private_obj *obj, > to_dp_mst_topology_state(state); > struct drm_dp_mst_topology_mgr *mgr = mst_state->mgr; > struct drm_dp_vcpi_allocation *pos; > - int avail_slots = 63; > + int avail_slots = 63, payload_count = 0; > > list_for_each_entry(pos, &mst_state->vcpis, next) { > DRM_DEBUG_ATOMIC("[MST PORT:%p] requires %d vcpi slots\n", > @@ -3238,6 +3238,12 @@ static int drm_dp_mst_atomic_check(struct drm_private_obj *obj, > avail_slots + pos->vcpi); > return -ENOSPC; > } > + > + if (++payload_count > mgr->max_payloads) { > + DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p has too many payloads (max=%d)\n", > + mgr, state, mgr->max_payloads); > + return -EINVAL; > + } drm_dp_init_vcpi() shouldn't ever fail on atomic drivers, so maybe worth adding a check to that effect? Probably only after amdgpu is converted over too. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > } > DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p vcpi avail=%d used=%d\n", > mgr, state, avail_slots, 63 - avail_slots); > -- > 2.17.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/6] drm/nouveau: Use atomic VCPI helpers for MST [not found] ` <20181023231251.16883-1-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> ` (2 preceding siblings ...) 2018-10-23 23:12 ` [PATCH 5/6] drm/dp_mst: Check payload count in ->atomic_check() Lyude Paul @ 2018-10-23 23:12 ` Lyude Paul 2018-10-24 8:50 ` [PATCH 0/6] drm/dp_mst: Improve VCPI helpers, use in nouveau Daniel Vetter 4 siblings, 0 replies; 15+ messages in thread From: Lyude Paul @ 2018-10-23 23:12 UTC (permalink / raw) To: intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Currently, nouveau uses the yolo method of setting up MST displays: it uses the old VCPI helpers (drm_dp_find_vcpi_slots()) for computing the display configuration. These helpers don't take care to make sure they take a reference to the mstb port that they're checking, and additionally don't actually check whether or not the topology still has enough bandwidth to provide the VCPI tokens required. So, drop usage of the old helpers and move entirely over to the atomic helpers. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/nouveau/dispnv50/disp.c | 46 +++++++++++++++++++++---- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 6cbbae3f438b..a4466b6c136a 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -747,16 +747,22 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { - struct nv50_mstc *mstc = nv50_mstc(conn_state->connector); + struct drm_atomic_state *state = crtc_state->state; + struct drm_connector *connector = conn_state->connector; + struct nv50_mstc *mstc = nv50_mstc(connector); struct nv50_mstm *mstm = mstc->mstm; - int bpp = conn_state->connector->display_info.bpc * 3; + int bpp = connector->display_info.bpc * 3; int slots; - mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, bpp); - - slots = drm_dp_find_vcpi_slots(&mstm->mgr, mstc->pbn); - if (slots < 0) - return slots; + mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, + bpp); + /* Zombies don't need VCPI */ + if (!drm_connector_is_unregistered(connector)) { + slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr, + mstc->port, mstc->pbn); + if (slots < 0) + return slots; + } return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state, mstc->native); @@ -920,12 +926,38 @@ nv50_mstc_get_modes(struct drm_connector *connector) return ret; } +static int +nv50_mstc_atomic_check(struct drm_connector *connector, + struct drm_connector_state *new_conn_state) +{ + struct drm_atomic_state *state = new_conn_state->state; + struct nv50_mstc *mstc = nv50_mstc(connector); + struct drm_dp_mst_topology_mgr *mgr = &mstc->mstm->mgr; + struct drm_connector_state *old_conn_state; + struct drm_crtc *old_crtc; + + old_conn_state = drm_atomic_get_old_connector_state(state, connector); + old_crtc = old_conn_state->crtc; + + /* We only need to release VCPI here if we're going from having a CRTC + * on this connector, to not having one + */ + if (!old_crtc || new_conn_state->crtc) + return 0; + + /* Release the previous VCPI allocation since the encoder's + * atomic_check() won't be called + */ + return drm_dp_atomic_release_vcpi_slots(state, mgr, mstc->port); +} + static const struct drm_connector_helper_funcs nv50_mstc_help = { .get_modes = nv50_mstc_get_modes, .mode_valid = nv50_mstc_mode_valid, .best_encoder = nv50_mstc_best_encoder, .atomic_best_encoder = nv50_mstc_atomic_best_encoder, + .atomic_check = nv50_mstc_atomic_check, }; static enum drm_connector_status -- 2.17.2 _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 0/6] drm/dp_mst: Improve VCPI helpers, use in nouveau [not found] ` <20181023231251.16883-1-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> ` (3 preceding siblings ...) 2018-10-23 23:12 ` [PATCH 6/6] drm/nouveau: Use atomic VCPI helpers for MST Lyude Paul @ 2018-10-24 8:50 ` Daniel Vetter 4 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2018-10-24 8:50 UTC (permalink / raw) To: Lyude Paul Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Daniel Vetter, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On Tue, Oct 23, 2018 at 07:12:45PM -0400, Lyude Paul wrote: > This patchset does some cleaning up of the atomic VCPI helpers for MST, > and converts nouveau over to using them. I would have included amdgpu in > this patch as well, but at the moment moving them over to the atomic > helpers is nontrivial. > > Cc: Daniel Vetter <daniel@ffwll.ch> I like. Wrt merging, I think this should go in through drm-misc-next :-) You need to ask drm-misc-next maintainers to backmerge drm-next first, so that all the stuff that went in through drm-intel is available. Cheers, Daniel > > Lyude Paul (6): > drm/dp_mst: Deprecate drm_dp_find_vcpi_slots() > drm/dp_mst: Remove all evil duplicate state pointers > drm/atomic: Add ->atomic_check() hook for private objects > drm/dp_mst: Start tracking per-port VCPI allocations > drm/dp_mst: Check payload count in ->atomic_check() > drm/nouveau: Use atomic VCPI helpers for MST > > drivers/gpu/drm/drm_atomic.c | 14 ++ > drivers/gpu/drm/drm_dp_mst_topology.c | 187 ++++++++++++++++++++---- > drivers/gpu/drm/i915/intel_dp_mst.c | 31 ++-- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 46 +++++- > include/drm/drm_atomic.h | 16 ++ > include/drm/drm_dp_mst_helper.h | 16 +- > 6 files changed, 250 insertions(+), 60 deletions(-) > > -- > 2.17.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.BAT: success for drm/dp_mst: Improve VCPI helpers, use in nouveau 2018-10-23 23:12 [PATCH 0/6] drm/dp_mst: Improve VCPI helpers, use in nouveau Lyude Paul ` (2 preceding siblings ...) [not found] ` <20181023231251.16883-1-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2018-10-23 23:46 ` Patchwork 2018-10-24 3:57 ` ✓ Fi.CI.IGT: " Patchwork 4 siblings, 0 replies; 15+ messages in thread From: Patchwork @ 2018-10-23 23:46 UTC (permalink / raw) To: Lyude Paul; +Cc: intel-gfx == Series Details == Series: drm/dp_mst: Improve VCPI helpers, use in nouveau URL : https://patchwork.freedesktop.org/series/51412/ State : success == Summary == = CI Bug Log - changes from CI_DRM_5023 -> Patchwork_10554 = == Summary - WARNING == Minor unknown changes coming with Patchwork_10554 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10554, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/51412/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10554: === IGT changes === ==== Warnings ==== igt@drv_selftest@live_execlists: fi-apl-guc: INCOMPLETE (fdo#106693) -> DMESG-WARN == Known issues == Here are the changes found in Patchwork_10554 that come from known issues: === IGT changes === ==== Issues hit ==== igt@amdgpu/amd_basic@cs-compute: fi-kbl-8809g: NOTRUN -> FAIL (fdo#108094) igt@amdgpu/amd_prime@amd-to-i915: fi-kbl-8809g: NOTRUN -> FAIL (fdo#107341) ==== Possible fixes ==== igt@gem_ctx_switch@basic-default: fi-icl-u: DMESG-FAIL -> PASS igt@gem_exec_suspend@basic-s3: fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS igt@kms_pipe_crc_basic@read-crc-pipe-a: fi-byt-clapper: FAIL (fdo#107362) -> PASS fdo#106693 https://bugs.freedesktop.org/show_bug.cgi?id=106693 fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#108094 https://bugs.freedesktop.org/show_bug.cgi?id=108094 == Participating hosts (49 -> 44) == Additional (1): fi-kbl-7560u Missing (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 == Build changes == * Linux: CI_DRM_5023 -> Patchwork_10554 CI_DRM_5023: 166bc98d7b77005943ab670506f164783cdc3f56 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4688: fa6dbf8c048961356fd642df047cb58ab49309b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10554: ac92e62e238e5c2f53e8bcde1bc618c44a6391f5 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == ac92e62e238e drm/nouveau: Use atomic VCPI helpers for MST a1e58b00701f drm/dp_mst: Check payload count in ->atomic_check() 0b412d2ad156 drm/dp_mst: Start tracking per-port VCPI allocations e03a62f7a57b drm/atomic: Add ->atomic_check() hook for private objects 2999539b9f1b drm/dp_mst: Remove all evil duplicate state pointers 5f55f2c4f6e5 drm/dp_mst: Deprecate drm_dp_find_vcpi_slots() == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10554/issues.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.IGT: success for drm/dp_mst: Improve VCPI helpers, use in nouveau 2018-10-23 23:12 [PATCH 0/6] drm/dp_mst: Improve VCPI helpers, use in nouveau Lyude Paul ` (3 preceding siblings ...) 2018-10-23 23:46 ` ✓ Fi.CI.BAT: success for " Patchwork @ 2018-10-24 3:57 ` Patchwork 4 siblings, 0 replies; 15+ messages in thread From: Patchwork @ 2018-10-24 3:57 UTC (permalink / raw) To: Lyude Paul; +Cc: intel-gfx == Series Details == Series: drm/dp_mst: Improve VCPI helpers, use in nouveau URL : https://patchwork.freedesktop.org/series/51412/ State : success == Summary == = CI Bug Log - changes from CI_DRM_5023_full -> Patchwork_10554_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_10554_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10554_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10554_full: === IGT changes === ==== Warnings ==== igt@pm_rc6_residency@rc6-accuracy: shard-snb: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_10554_full that come from known issues: === IGT changes === ==== Issues hit ==== igt@gem_exec_blt@normal-max: shard-apl: PASS -> INCOMPLETE (fdo#103927) igt@kms_busy@extended-pageflip-hang-newfb-render-b: shard-apl: NOTRUN -> DMESG-WARN (fdo#107956) igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b: shard-snb: NOTRUN -> DMESG-WARN (fdo#107956) igt@kms_color@pipe-a-legacy-gamma: shard-apl: PASS -> FAIL (fdo#104782, fdo#108145) igt@kms_cursor_crc@cursor-128x128-suspend: shard-skl: PASS -> INCOMPLETE (fdo#104108) igt@kms_cursor_crc@cursor-256x85-onscreen: shard-apl: NOTRUN -> FAIL (fdo#103232) shard-glk: PASS -> FAIL (fdo#103232) +2 igt@kms_cursor_crc@cursor-256x85-random: shard-apl: PASS -> FAIL (fdo#103232) igt@kms_cursor_crc@cursor-64x64-suspend: shard-apl: NOTRUN -> FAIL (fdo#103232, fdo#103191) igt@kms_flip@plain-flip-fb-recreate: shard-skl: PASS -> FAIL (fdo#100368) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move: shard-apl: PASS -> FAIL (fdo#103167) +1 igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff: shard-glk: PASS -> FAIL (fdo#103167) +3 igt@kms_plane@plane-position-covered-pipe-a-planes: shard-glk: PASS -> FAIL (fdo#103166) +2 igt@kms_plane@plane-position-covered-pipe-c-planes: shard-apl: PASS -> FAIL (fdo#103166) igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max: shard-glk: PASS -> FAIL (fdo#108145) igt@kms_plane_alpha_blend@pipe-c-alpha-7efc: shard-skl: NOTRUN -> FAIL (fdo#108146) igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max: shard-apl: PASS -> FAIL (fdo#108145) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) shard-kbl: NOTRUN -> FAIL (fdo#99912) shard-snb: NOTRUN -> FAIL (fdo#99912) igt@pm_rpm@gem-execbuf-stress-pc8: shard-skl: SKIP -> INCOMPLETE (fdo#107807) ==== Possible fixes ==== igt@kms_cursor_crc@cursor-256x256-sliding: shard-glk: FAIL (fdo#103232) -> PASS +1 igt@kms_cursor_crc@cursor-size-change: shard-apl: FAIL (fdo#103232) -> PASS igt@kms_cursor_legacy@cursora-vs-flipa-toggle: shard-glk: DMESG-WARN (fdo#105763, fdo#106538) -> PASS +1 igt@kms_draw_crc@draw-method-rgb565-mmap-cpu-xtiled: shard-skl: FAIL (fdo#103184) -> PASS igt@kms_flip@busy-flip-interruptible: shard-skl: FAIL (fdo#103257) -> PASS igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu: shard-skl: FAIL (fdo#103167) -> PASS igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes: shard-skl: INCOMPLETE (fdo#107773, fdo#104108) -> PASS igt@kms_plane_alpha_blend@pipe-a-coverage-7efc: shard-skl: FAIL (fdo#107815, fdo#108145) -> PASS igt@kms_plane_multiple@atomic-pipe-b-tiling-y: shard-glk: FAIL (fdo#103166) -> PASS +1 igt@kms_rotation_crc@sprite-rotation-180: shard-apl: INCOMPLETE (fdo#103927) -> PASS igt@kms_vblank@pipe-a-ts-continuation-suspend: shard-kbl: INCOMPLETE (fdo#103665) -> PASS igt@pm_rpm@legacy-planes-dpms: shard-skl: INCOMPLETE (fdo#105959, fdo#107807) -> PASS ==== Warnings ==== igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-mmap-cpu: shard-snb: INCOMPLETE (fdo#105411) -> DMESG-WARN (fdo#107469) fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232 fdo#103257 https://bugs.freedesktop.org/show_bug.cgi?id=103257 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108 fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763 fdo#105959 https://bugs.freedesktop.org/show_bug.cgi?id=105959 fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538 fdo#107469 https://bugs.freedesktop.org/show_bug.cgi?id=107469 fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773 fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807 fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815 fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956 fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145 fdo#108146 https://bugs.freedesktop.org/show_bug.cgi?id=108146 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (6 -> 6) == No changes in participating hosts == Build changes == * Linux: CI_DRM_5023 -> Patchwork_10554 CI_DRM_5023: 166bc98d7b77005943ab670506f164783cdc3f56 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4688: fa6dbf8c048961356fd642df047cb58ab49309b2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10554: ac92e62e238e5c2f53e8bcde1bc618c44a6391f5 @ 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_10554/shards.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-10-24 8:55 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-23 23:12 [PATCH 0/6] drm/dp_mst: Improve VCPI helpers, use in nouveau Lyude Paul 2018-10-23 23:12 ` [PATCH 1/6] drm/dp_mst: Deprecate drm_dp_find_vcpi_slots() Lyude Paul 2018-10-24 8:27 ` Daniel Vetter 2018-10-23 23:12 ` [PATCH 4/6] drm/dp_mst: Start tracking per-port VCPI allocations Lyude Paul [not found] ` <20181023231251.16883-5-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2018-10-24 8:55 ` Daniel Vetter [not found] ` <20181023231251.16883-1-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2018-10-23 23:12 ` [PATCH 2/6] drm/dp_mst: Remove all evil duplicate state pointers Lyude Paul 2018-10-24 8:27 ` Daniel Vetter 2018-10-23 23:12 ` [PATCH 3/6] drm/atomic: Add ->atomic_check() hook for private objects Lyude Paul 2018-10-24 8:45 ` Daniel Vetter 2018-10-23 23:12 ` [PATCH 5/6] drm/dp_mst: Check payload count in ->atomic_check() Lyude Paul [not found] ` <20181023231251.16883-6-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2018-10-24 8:54 ` Daniel Vetter 2018-10-23 23:12 ` [PATCH 6/6] drm/nouveau: Use atomic VCPI helpers for MST Lyude Paul 2018-10-24 8:50 ` [PATCH 0/6] drm/dp_mst: Improve VCPI helpers, use in nouveau Daniel Vetter 2018-10-23 23:46 ` ✓ Fi.CI.BAT: success for " Patchwork 2018-10-24 3:57 ` ✓ Fi.CI.IGT: " Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.