| Message ID | 20191203143530.27262-13-mikita.lipski@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DSC MST support for DRM and AMDGPU | expand |
On 2019-12-03 9:35 a.m., mikita.lipski@amd.com wrote: > From: Mikita Lipski <mikita.lipski@amd.com> > > Adding PBN attribute to drm_dp_vcpi_allocation structure to > keep track of how much bandwidth each Port requires. > Adding drm_dp_mst_atomic_check_bw_limit to verify that > state's bandwidth needs doesn't exceed available bandwidth. > The funtion is called in drm_dp_mst_atomic_check after > drm_dp_mst_atomic_check_topology_state to fully verify that > the proposed topology is supported. > > Cc: Jerry Zuo <Jerry.Zuo@amd.com> > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Lyude Paul <lyude@redhat.com> > Signed-off-by: Mikita Lipski <mikita.lipski@amd.com> > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 67 ++++++++++++++++++++++++++- > include/drm/drm_dp_mst_helper.h | 1 + > 2 files changed, 66 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 5e549f48ffb8..76bcbb4cd8b4 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -4052,7 +4052,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, > { > struct drm_dp_mst_topology_state *topology_state; > struct drm_dp_vcpi_allocation *pos, *vcpi = NULL; > - int prev_slots, req_slots; > + int prev_slots, prev_bw, req_slots, ret; > 'ret' is unused here. Harry > topology_state = drm_atomic_get_mst_topology_state(state, mgr); > if (IS_ERR(topology_state)) > @@ -4063,6 +4063,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, > if (pos->port == port) { > vcpi = pos; > prev_slots = vcpi->vcpi; > + prev_bw = vcpi->pbn; > > /* > * This should never happen, unless the driver tries > @@ -4078,8 +4079,10 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, > break; > } > } > - if (!vcpi) > + if (!vcpi) { > prev_slots = 0; > + prev_bw = 0; > + } > > if (pbn_div <= 0) > pbn_div = mgr->pbn_div; > @@ -4089,6 +4092,9 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] [MST PORT:%p] VCPI %d -> %d\n", > port->connector->base.id, port->connector->name, > port, prev_slots, req_slots); > + DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] [MST PORT:%p] PBN %d -> %d\n", > + port->connector->base.id, port->connector->name, > + port, prev_bw, pbn); > > /* Add the new allocation to the state */ > if (!vcpi) { > @@ -4101,6 +4107,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, > list_add(&vcpi->next, &topology_state->vcpis); > } > vcpi->vcpi = req_slots; > + vcpi->pbn = pbn; > > return req_slots; > } > @@ -4703,6 +4710,59 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj, > kfree(mst_state); > } > > +static bool drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port *port, > + struct drm_dp_mst_branch *branch) > +{ > + while (port->parent) { > + if (port->parent == branch) > + return true; > + > + if (port->parent->port_parent) > + port = port->parent->port_parent; > + else > + break; > + } > + return false; > +} > + > +static inline > +int drm_dp_mst_atomic_check_bw_limit(struct drm_dp_mst_branch *branch, > + struct drm_dp_mst_topology_state *mst_state) > +{ > + struct drm_dp_mst_port *port; > + struct drm_dp_vcpi_allocation *vcpi; > + int pbn_limit = 0, pbn_used = 0; > + > + list_for_each_entry(port, &branch->ports, next) { > + if (port->mstb) { > + if (drm_dp_mst_atomic_check_bw_limit(port->mstb, mst_state)) > + return -EINVAL; > + } > + if (port->available_pbn > 0) > + pbn_limit = port->available_pbn; > + } > + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] branch has %d PBN available\n", > + branch, > + pbn_limit); > + > + list_for_each_entry(vcpi, &mst_state->vcpis, next) { > + if (!vcpi->pbn) > + continue; > + > + if (drm_dp_mst_port_downstream_of_branch(vcpi->port, branch)) > + pbn_used += vcpi->pbn; > + } > + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] branch used %d PBN\n", > + branch, > + pbn_used); > + if (pbn_used > pbn_limit) { > + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] No available bandwidth\n", > + branch); > + return -EINVAL; > + } > + return 0; > +} > + > static inline int > drm_dp_mst_atomic_check_topology_state(struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_topology_state *mst_state) > @@ -4834,6 +4894,9 @@ int drm_dp_mst_atomic_check(struct drm_atomic_state *state) > ret = drm_dp_mst_atomic_check_topology_state(mgr, mst_state); > if (ret) > break; > + ret = drm_dp_mst_atomic_check_bw_limit(mgr->mst_primary, mst_state); > + if (ret) > + break; > } > > return ret; > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h > index 830c94b7f45d..2919d9776af3 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -502,6 +502,7 @@ struct drm_dp_payload { > struct drm_dp_vcpi_allocation { > struct drm_dp_mst_port *port; > int vcpi; > + int pbn; > bool dsc_enabled; > struct list_head next; > }; >
On Tue, 2019-12-03 at 09:35 -0500, mikita.lipski@amd.com wrote: > From: Mikita Lipski <mikita.lipski@amd.com> > > Adding PBN attribute to drm_dp_vcpi_allocation structure to > keep track of how much bandwidth each Port requires. > Adding drm_dp_mst_atomic_check_bw_limit to verify that > state's bandwidth needs doesn't exceed available bandwidth. > The funtion is called in drm_dp_mst_atomic_check after > drm_dp_mst_atomic_check_topology_state to fully verify that > the proposed topology is supported. > > Cc: Jerry Zuo <Jerry.Zuo@amd.com> > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Lyude Paul <lyude@redhat.com> > Signed-off-by: Mikita Lipski <mikita.lipski@amd.com> > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 67 ++++++++++++++++++++++++++- > include/drm/drm_dp_mst_helper.h | 1 + > 2 files changed, 66 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 5e549f48ffb8..76bcbb4cd8b4 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -4052,7 +4052,7 @@ int drm_dp_atomic_find_vcpi_slots(struct > drm_atomic_state *state, > { > struct drm_dp_mst_topology_state *topology_state; > struct drm_dp_vcpi_allocation *pos, *vcpi = NULL; > - int prev_slots, req_slots; > + int prev_slots, prev_bw, req_slots, ret; > > topology_state = drm_atomic_get_mst_topology_state(state, mgr); > if (IS_ERR(topology_state)) > @@ -4063,6 +4063,7 @@ int drm_dp_atomic_find_vcpi_slots(struct > drm_atomic_state *state, > if (pos->port == port) { > vcpi = pos; > prev_slots = vcpi->vcpi; > + prev_bw = vcpi->pbn; > > /* > * This should never happen, unless the driver tries > @@ -4078,8 +4079,10 @@ int drm_dp_atomic_find_vcpi_slots(struct > drm_atomic_state *state, > break; > } > } > - if (!vcpi) > + if (!vcpi) { > prev_slots = 0; > + prev_bw = 0; > + } > > if (pbn_div <= 0) > pbn_div = mgr->pbn_div; > @@ -4089,6 +4092,9 @@ int drm_dp_atomic_find_vcpi_slots(struct > drm_atomic_state *state, > DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] [MST PORT:%p] VCPI %d -> %d\n", > port->connector->base.id, port->connector->name, > port, prev_slots, req_slots); > + DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] [MST PORT:%p] PBN %d -> %d\n", > + port->connector->base.id, port->connector->name, > + port, prev_bw, pbn); > > /* Add the new allocation to the state */ > if (!vcpi) { > @@ -4101,6 +4107,7 @@ int drm_dp_atomic_find_vcpi_slots(struct > drm_atomic_state *state, > list_add(&vcpi->next, &topology_state->vcpis); > } > vcpi->vcpi = req_slots; > + vcpi->pbn = pbn; > > return req_slots; > } > @@ -4703,6 +4710,59 @@ static void drm_dp_mst_destroy_state(struct > drm_private_obj *obj, > kfree(mst_state); > } > > +static bool drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port > *port, > + struct drm_dp_mst_branch > *branch) > +{ > + while (port->parent) { > + if (port->parent == branch) > + return true; > + > + if (port->parent->port_parent) > + port = port->parent->port_parent; > + else > + break; > + } > + return false; > +} > + > +static inline > +int drm_dp_mst_atomic_check_bw_limit(struct drm_dp_mst_branch *branch, > + struct drm_dp_mst_topology_state > *mst_state) > +{ > + struct drm_dp_mst_port *port; > + struct drm_dp_vcpi_allocation *vcpi; > + int pbn_limit = 0, pbn_used = 0; > + > + list_for_each_entry(port, &branch->ports, next) { > + if (port->mstb) { > + if (drm_dp_mst_atomic_check_bw_limit(port->mstb, > mst_state)) > + return -EINVAL; > + } > + if (port->available_pbn > 0) > + pbn_limit = port->available_pbn; > + } > + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] branch has %d PBN available\n", > + branch, > + pbn_limit); Should fix the indenting here > + > + list_for_each_entry(vcpi, &mst_state->vcpis, next) { > + if (!vcpi->pbn) > + continue; > + > + if (drm_dp_mst_port_downstream_of_branch(vcpi->port, branch)) > + pbn_used += vcpi->pbn; > + } > + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] branch used %d PBN\n", > + branch, > + pbn_used); optional bikeshed: put branch and pbn_used on the same line > + if (pbn_used > pbn_limit) { > + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] No available bandwidth\n", > + branch); > + return -EINVAL; > + } > + return 0; > +} > + > static inline int > drm_dp_mst_atomic_check_topology_state(struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_topology_state > *mst_state) > @@ -4834,6 +4894,9 @@ int drm_dp_mst_atomic_check(struct drm_atomic_state > *state) > ret = drm_dp_mst_atomic_check_topology_state(mgr, mst_state); > if (ret) > break; We should probably rename drm_dp_mst_atomic_check_topology_state() to check_payloads or something along those lines And that's it! This looks awesome, with those small changes consider this: Reviewed-by: Lyude Paul <lyude@redhat.com> > + ret = drm_dp_mst_atomic_check_bw_limit(mgr->mst_primary, > mst_state); > + if (ret) > + break; > } > > return ret; > diff --git a/include/drm/drm_dp_mst_helper.h > b/include/drm/drm_dp_mst_helper.h > index 830c94b7f45d..2919d9776af3 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -502,6 +502,7 @@ struct drm_dp_payload { > struct drm_dp_vcpi_allocation { > struct drm_dp_mst_port *port; > int vcpi; > + int pbn; > bool dsc_enabled; > struct list_head next; > };
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 5e549f48ffb8..76bcbb4cd8b4 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4052,7 +4052,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, { struct drm_dp_mst_topology_state *topology_state; struct drm_dp_vcpi_allocation *pos, *vcpi = NULL; - int prev_slots, req_slots; + int prev_slots, prev_bw, req_slots, ret; topology_state = drm_atomic_get_mst_topology_state(state, mgr); if (IS_ERR(topology_state)) @@ -4063,6 +4063,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, if (pos->port == port) { vcpi = pos; prev_slots = vcpi->vcpi; + prev_bw = vcpi->pbn; /* * This should never happen, unless the driver tries @@ -4078,8 +4079,10 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, break; } } - if (!vcpi) + if (!vcpi) { prev_slots = 0; + prev_bw = 0; + } if (pbn_div <= 0) pbn_div = mgr->pbn_div; @@ -4089,6 +4092,9 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] [MST PORT:%p] VCPI %d -> %d\n", port->connector->base.id, port->connector->name, port, prev_slots, req_slots); + DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] [MST PORT:%p] PBN %d -> %d\n", + port->connector->base.id, port->connector->name, + port, prev_bw, pbn); /* Add the new allocation to the state */ if (!vcpi) { @@ -4101,6 +4107,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, list_add(&vcpi->next, &topology_state->vcpis); } vcpi->vcpi = req_slots; + vcpi->pbn = pbn; return req_slots; } @@ -4703,6 +4710,59 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj, kfree(mst_state); } +static bool drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port *port, + struct drm_dp_mst_branch *branch) +{ + while (port->parent) { + if (port->parent == branch) + return true; + + if (port->parent->port_parent) + port = port->parent->port_parent; + else + break; + } + return false; +} + +static inline +int drm_dp_mst_atomic_check_bw_limit(struct drm_dp_mst_branch *branch, + struct drm_dp_mst_topology_state *mst_state) +{ + struct drm_dp_mst_port *port; + struct drm_dp_vcpi_allocation *vcpi; + int pbn_limit = 0, pbn_used = 0; + + list_for_each_entry(port, &branch->ports, next) { + if (port->mstb) { + if (drm_dp_mst_atomic_check_bw_limit(port->mstb, mst_state)) + return -EINVAL; + } + if (port->available_pbn > 0) + pbn_limit = port->available_pbn; + } + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] branch has %d PBN available\n", + branch, + pbn_limit); + + list_for_each_entry(vcpi, &mst_state->vcpis, next) { + if (!vcpi->pbn) + continue; + + if (drm_dp_mst_port_downstream_of_branch(vcpi->port, branch)) + pbn_used += vcpi->pbn; + } + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] branch used %d PBN\n", + branch, + pbn_used); + if (pbn_used > pbn_limit) { + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] No available bandwidth\n", + branch); + return -EINVAL; + } + return 0; +} + static inline int drm_dp_mst_atomic_check_topology_state(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state) @@ -4834,6 +4894,9 @@ int drm_dp_mst_atomic_check(struct drm_atomic_state *state) ret = drm_dp_mst_atomic_check_topology_state(mgr, mst_state); if (ret) break; + ret = drm_dp_mst_atomic_check_bw_limit(mgr->mst_primary, mst_state); + if (ret) + break; } return ret; diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 830c94b7f45d..2919d9776af3 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -502,6 +502,7 @@ struct drm_dp_payload { struct drm_dp_vcpi_allocation { struct drm_dp_mst_port *port; int vcpi; + int pbn; bool dsc_enabled; struct list_head next; };