We already open-code this quite often, and will be iterating through
payloads even more once we've moved all of the payload tracking into the
atomic state. So, let's add a helper for doing this.
Signed-off-by: Lyude Paul <[email protected]>
Cc: Wayne Lin <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Cc: Fangzhi Zuo <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Imre Deak <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Sean Paul <[email protected]>
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 109 ++++++++----------
1 file changed, 45 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index ec52f91b1f0e..0bc2c7a90c37 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -1737,6 +1737,19 @@ drm_dp_mst_dump_port_topology_history(struct drm_dp_mst_port *port) {}
#define save_port_topology_ref(port, type)
#endif
+static struct drm_dp_mst_atomic_payload *
+drm_atomic_get_mst_payload_state(struct drm_dp_mst_topology_state *state,
+ struct drm_dp_mst_port *port)
+{
+ struct drm_dp_mst_atomic_payload *payload;
+
+ list_for_each_entry(payload, &state->payloads, next)
+ if (payload->port == port)
+ return payload;
+
+ return NULL;
+}
+
static void drm_dp_destroy_mst_branch_device(struct kref *kref)
{
struct drm_dp_mst_branch *mstb =
@@ -4381,39 +4394,31 @@ int drm_dp_atomic_find_time_slots(struct drm_atomic_state *state,
int pbn_div)
{
struct drm_dp_mst_topology_state *topology_state;
- struct drm_dp_mst_atomic_payload *pos, *payload = NULL;
- int prev_slots, prev_bw, req_slots;
+ struct drm_dp_mst_atomic_payload *payload = NULL;
+ int prev_slots = 0, prev_bw = 0, req_slots;
topology_state = drm_atomic_get_mst_topology_state(state, mgr);
if (IS_ERR(topology_state))
return PTR_ERR(topology_state);
/* Find the current allocation for this port, if any */
- list_for_each_entry(pos, &topology_state->payloads, next) {
- if (pos->port == port) {
- payload = pos;
- prev_slots = payload->time_slots;
- prev_bw = payload->pbn;
-
- /*
- * This should never happen, unless the driver tries
- * releasing and allocating the same timeslot allocation,
- * which is an error
- */
- if (WARN_ON(!prev_slots)) {
- drm_err(mgr->dev,
- "cannot allocate and release time slots on [MST PORT:%p] in the same state\n",
- port);
- return -EINVAL;
- }
+ payload = drm_atomic_get_mst_payload_state(topology_state, port);
+ if (payload) {
+ prev_slots = payload->time_slots;
+ prev_bw = payload->pbn;
- break;
+ /*
+ * This should never happen, unless the driver tries
+ * releasing and allocating the same timeslot allocation,
+ * which is an error
+ */
+ if (WARN_ON(!prev_slots)) {
+ drm_err(mgr->dev,
+ "cannot allocate and release time slots on [MST PORT:%p] in the same state\n",
+ port);
+ return -EINVAL;
}
}
- if (!payload) {
- prev_slots = 0;
- prev_bw = 0;
- }
if (pbn_div <= 0)
pbn_div = mgr->pbn_div;
@@ -4474,30 +4479,24 @@ int drm_dp_atomic_release_time_slots(struct drm_atomic_state *state,
struct drm_dp_mst_port *port)
{
struct drm_dp_mst_topology_state *topology_state;
- struct drm_dp_mst_atomic_payload *pos;
- bool found = false;
+ struct drm_dp_mst_atomic_payload *payload;
topology_state = drm_atomic_get_mst_topology_state(state, mgr);
if (IS_ERR(topology_state))
return PTR_ERR(topology_state);
- list_for_each_entry(pos, &topology_state->payloads, next) {
- if (pos->port == port) {
- found = true;
- break;
- }
- }
- if (WARN_ON(!found)) {
+ payload = drm_atomic_get_mst_payload_state(topology_state, port);
+ if (WARN_ON(!payload)) {
drm_err(mgr->dev, "No payload for [MST PORT:%p] found in mst state %p\n",
port, &topology_state->base);
return -EINVAL;
}
- drm_dbg_atomic(mgr->dev, "[MST PORT:%p] TU %d -> 0\n", port, pos->time_slots);
- if (pos->time_slots) {
+ drm_dbg_atomic(mgr->dev, "[MST PORT:%p] TU %d -> 0\n", port, payload->time_slots);
+ if (payload->time_slots) {
drm_dp_mst_put_port_malloc(port);
- pos->time_slots = 0;
- pos->pbn = 0;
+ payload->time_slots = 0;
+ payload->pbn = 0;
}
return 0;
@@ -5194,18 +5193,8 @@ drm_dp_mst_atomic_check_port_bw_limit(struct drm_dp_mst_port *port,
return 0;
if (drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
- bool found = false;
-
- list_for_each_entry(payload, &state->payloads, next) {
- if (payload->port != port)
- continue;
- if (!payload->pbn)
- return 0;
-
- found = true;
- break;
- }
- if (!found)
+ payload = drm_atomic_get_mst_payload_state(state, port);
+ if (!payload)
return 0;
/*
@@ -5360,34 +5349,26 @@ int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state,
bool enable)
{
struct drm_dp_mst_topology_state *mst_state;
- struct drm_dp_mst_atomic_payload *pos;
- bool found = false;
+ struct drm_dp_mst_atomic_payload *payload;
int time_slots = 0;
mst_state = drm_atomic_get_mst_topology_state(state, port->mgr);
-
if (IS_ERR(mst_state))
return PTR_ERR(mst_state);
- list_for_each_entry(pos, &mst_state->payloads, next) {
- if (pos->port == port) {
- found = true;
- break;
- }
- }
-
- if (!found) {
+ payload = drm_atomic_get_mst_payload_state(mst_state, port);
+ if (!payload) {
drm_dbg_atomic(state->dev,
"[MST PORT:%p] Couldn't find payload in mst state %p\n",
port, mst_state);
return -EINVAL;
}
- if (pos->dsc_enabled == enable) {
+ if (payload->dsc_enabled == enable) {
drm_dbg_atomic(state->dev,
"[MST PORT:%p] DSC flag is already set to %d, returning %d time slots\n",
- port, enable, pos->time_slots);
- time_slots = pos->time_slots;
+ port, enable, payload->time_slots);
+ time_slots = payload->time_slots;
}
if (enable) {
@@ -5399,7 +5380,7 @@ int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state,
return -EINVAL;
}
- pos->dsc_enabled = enable;
+ payload->dsc_enabled = enable;
return time_slots;
}
--
2.35.3
On Tue, 07 Jun 2022, Lyude Paul <[email protected]> wrote:
> We already open-code this quite often, and will be iterating through
> payloads even more once we've moved all of the payload tracking into the
> atomic state. So, let's add a helper for doing this.
>
> Signed-off-by: Lyude Paul <[email protected]>
> Cc: Wayne Lin <[email protected]>
> Cc: Ville Syrjälä <[email protected]>
> Cc: Fangzhi Zuo <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Cc: Imre Deak <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Sean Paul <[email protected]>
> ---
> drivers/gpu/drm/display/drm_dp_mst_topology.c | 109 ++++++++----------
> 1 file changed, 45 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index ec52f91b1f0e..0bc2c7a90c37 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -1737,6 +1737,19 @@ drm_dp_mst_dump_port_topology_history(struct drm_dp_mst_port *port) {}
> #define save_port_topology_ref(port, type)
> #endif
>
> +static struct drm_dp_mst_atomic_payload *
> +drm_atomic_get_mst_payload_state(struct drm_dp_mst_topology_state *state,
> + struct drm_dp_mst_port *port)
> +{
> + struct drm_dp_mst_atomic_payload *payload;
> +
> + list_for_each_entry(payload, &state->payloads, next)
> + if (payload->port == port)
> + return payload;
> +
> + return NULL;
> +}
> +
> static void drm_dp_destroy_mst_branch_device(struct kref *kref)
> {
> struct drm_dp_mst_branch *mstb =
> @@ -4381,39 +4394,31 @@ int drm_dp_atomic_find_time_slots(struct drm_atomic_state *state,
> int pbn_div)
> {
> struct drm_dp_mst_topology_state *topology_state;
> - struct drm_dp_mst_atomic_payload *pos, *payload = NULL;
> - int prev_slots, prev_bw, req_slots;
> + struct drm_dp_mst_atomic_payload *payload = NULL;
> + int prev_slots = 0, prev_bw = 0, req_slots;
>
> topology_state = drm_atomic_get_mst_topology_state(state, mgr);
> if (IS_ERR(topology_state))
> return PTR_ERR(topology_state);
>
> /* Find the current allocation for this port, if any */
> - list_for_each_entry(pos, &topology_state->payloads, next) {
> - if (pos->port == port) {
> - payload = pos;
> - prev_slots = payload->time_slots;
> - prev_bw = payload->pbn;
> -
> - /*
> - * This should never happen, unless the driver tries
> - * releasing and allocating the same timeslot allocation,
> - * which is an error
> - */
> - if (WARN_ON(!prev_slots)) {
> - drm_err(mgr->dev,
> - "cannot allocate and release time slots on [MST PORT:%p] in the same state\n",
> - port);
> - return -EINVAL;
> - }
> + payload = drm_atomic_get_mst_payload_state(topology_state, port);
> + if (payload) {
> + prev_slots = payload->time_slots;
> + prev_bw = payload->pbn;
>
> - break;
> + /*
> + * This should never happen, unless the driver tries
> + * releasing and allocating the same timeslot allocation,
> + * which is an error
> + */
> + if (WARN_ON(!prev_slots)) {
> + drm_err(mgr->dev,
> + "cannot allocate and release time slots on [MST PORT:%p] in the same state\n",
> + port);
I guess I'd combine the WARN_ON() and drm_err() to a single
drm_WARN(). Having both is a silly. But can be a follow-up, not part of
this patch really.
> + return -EINVAL;
> }
> }
> - if (!payload) {
> - prev_slots = 0;
> - prev_bw = 0;
> - }
>
> if (pbn_div <= 0)
> pbn_div = mgr->pbn_div;
> @@ -4474,30 +4479,24 @@ int drm_dp_atomic_release_time_slots(struct drm_atomic_state *state,
> struct drm_dp_mst_port *port)
> {
> struct drm_dp_mst_topology_state *topology_state;
> - struct drm_dp_mst_atomic_payload *pos;
> - bool found = false;
> + struct drm_dp_mst_atomic_payload *payload;
>
> topology_state = drm_atomic_get_mst_topology_state(state, mgr);
> if (IS_ERR(topology_state))
> return PTR_ERR(topology_state);
>
> - list_for_each_entry(pos, &topology_state->payloads, next) {
> - if (pos->port == port) {
> - found = true;
> - break;
> - }
> - }
> - if (WARN_ON(!found)) {
> + payload = drm_atomic_get_mst_payload_state(topology_state, port);
> + if (WARN_ON(!payload)) {
> drm_err(mgr->dev, "No payload for [MST PORT:%p] found in mst state %p\n",
> port, &topology_state->base);
Ditto.
BR,
Jani.
> return -EINVAL;
> }
>
> - drm_dbg_atomic(mgr->dev, "[MST PORT:%p] TU %d -> 0\n", port, pos->time_slots);
> - if (pos->time_slots) {
> + drm_dbg_atomic(mgr->dev, "[MST PORT:%p] TU %d -> 0\n", port, payload->time_slots);
> + if (payload->time_slots) {
> drm_dp_mst_put_port_malloc(port);
> - pos->time_slots = 0;
> - pos->pbn = 0;
> + payload->time_slots = 0;
> + payload->pbn = 0;
> }
>
> return 0;
> @@ -5194,18 +5193,8 @@ drm_dp_mst_atomic_check_port_bw_limit(struct drm_dp_mst_port *port,
> return 0;
>
> if (drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
> - bool found = false;
> -
> - list_for_each_entry(payload, &state->payloads, next) {
> - if (payload->port != port)
> - continue;
> - if (!payload->pbn)
> - return 0;
> -
> - found = true;
> - break;
> - }
> - if (!found)
> + payload = drm_atomic_get_mst_payload_state(state, port);
> + if (!payload)
> return 0;
>
> /*
> @@ -5360,34 +5349,26 @@ int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state,
> bool enable)
> {
> struct drm_dp_mst_topology_state *mst_state;
> - struct drm_dp_mst_atomic_payload *pos;
> - bool found = false;
> + struct drm_dp_mst_atomic_payload *payload;
> int time_slots = 0;
>
> mst_state = drm_atomic_get_mst_topology_state(state, port->mgr);
> -
> if (IS_ERR(mst_state))
> return PTR_ERR(mst_state);
>
> - list_for_each_entry(pos, &mst_state->payloads, next) {
> - if (pos->port == port) {
> - found = true;
> - break;
> - }
> - }
> -
> - if (!found) {
> + payload = drm_atomic_get_mst_payload_state(mst_state, port);
> + if (!payload) {
> drm_dbg_atomic(state->dev,
> "[MST PORT:%p] Couldn't find payload in mst state %p\n",
> port, mst_state);
> return -EINVAL;
> }
>
> - if (pos->dsc_enabled == enable) {
> + if (payload->dsc_enabled == enable) {
> drm_dbg_atomic(state->dev,
> "[MST PORT:%p] DSC flag is already set to %d, returning %d time slots\n",
> - port, enable, pos->time_slots);
> - time_slots = pos->time_slots;
> + port, enable, payload->time_slots);
> + time_slots = payload->time_slots;
> }
>
> if (enable) {
> @@ -5399,7 +5380,7 @@ int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state,
> return -EINVAL;
> }
>
> - pos->dsc_enabled = enable;
> + payload->dsc_enabled = enable;
>
> return time_slots;
> }
--
Jani Nikula, Intel Open Source Graphics Center