Re: [PATCH v8 12/17] drm/dp_mst: Add branch bandwidth validation to MST atomic check

2019-12-06 Thread Lyude Paul 
On Tue, 2019-12-03 at 09:35 -0500, mikita.lip...@amd.com wrote:
> From: Mikita Lipski 
> 
> 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 
> Cc: Harry Wentland 
> Cc: Lyude Paul 
> Signed-off-by: Mikita Lipski 
> ---
>  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(>next, _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, >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, _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",
> +

Re: [PATCH v8 12/17] drm/dp_mst: Add branch bandwidth validation to MST atomic check

2019-12-05 Thread Harry Wentland 
On 2019-12-03 9:35 a.m., mikita.lip...@amd.com wrote:
> From: Mikita Lipski 
> 
> 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 
> Cc: Harry Wentland 
> Cc: Lyude Paul 
> Signed-off-by: Mikita Lipski 
> ---
>  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(>next, _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, >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, _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;
> + }
> +

[PATCH v8 12/17] drm/dp_mst: Add branch bandwidth validation to MST atomic check

2019-12-03 Thread mikita.lipski 
From: Mikita Lipski 

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 
Cc: Harry Wentland 
Cc: Lyude Paul 
Signed-off-by: Mikita Lipski 
---
 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(>next, _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, >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, _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