[PATCH 3/3] drm/amd/display: Solve mst monitors blank out problem after resume
[Why]
In dm resume, we firstly restore dc state and do the mst resume for
topology probing thereafter. If we change dpcd DP_MSTM_CTRL value
after LT in mst reume, it will cause light up problem on the hub.
[How]
Revert the commit 202dc359adda ("drm/amd/display: Defer handling mst
up request in resume"). And adjust the reason to trigger
dc_link_detect by DETECT_REASON_RESUMEFROMS3S4.
Fixes: 202dc359adda ("drm/amd/display: Defer handling mst up request in resume")
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c64cc2378a94..b01452eb0981 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2569,6 +2569,7 @@ static void resume_mst_branch_status(struct
drm_dp_mst_topology_mgr *mgr)
ret = drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL,
DP_MST_EN |
+DP_UP_REQ_EN |
DP_UPSTREAM_IS_SRC);
if (ret < 0) {
drm_dbg_kms(mgr->dev, "mst write failed - undocked during
suspend?\n");
@@ -3171,7 +3172,7 @@ static int dm_resume(void *handle)
} else {
mutex_lock(&dm->dc_lock);
dc_exit_ips_for_hw_access(dm->dc);
- dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD);
+ dc_link_detect(aconnector->dc_link,
DETECT_REASON_RESUMEFROMS3S4);
mutex_unlock(&dm->dc_lock);
}
--
2.37.3
[PATCH 2/3] drm/dp_mst: Skip CSN if topology probing is not done yet
[Why]
During resume, observe that we receive CSN event before we start topology
probing. Handling CSN at this moment based on uncertain topology is
unnecessary.
[How]
Add checking condition in drm_dp_mst_handle_up_req() to skip handling CSN
if the topology is yet to be probed.
Cc: Lyude Paul
Cc: Harry Wentland
Cc: Jani Nikula
Cc: Imre Deak
Cc: Daniel Vetter
Cc: sta...@vger.kernel.org
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 68831f4e502a..fc2ceae61db2 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -4069,6 +4069,7 @@ static int drm_dp_mst_handle_up_req(struct
drm_dp_mst_topology_mgr *mgr)
if (up_req->msg.req_type == DP_CONNECTION_STATUS_NOTIFY) {
const struct drm_dp_connection_status_notify *conn_stat =
&up_req->msg.u.conn_stat;
+ bool handle_csn;
drm_dbg_kms(mgr->dev, "Got CSN: pn: %d ldps:%d ddps: %d mcs: %d
ip: %d pdt: %d\n",
conn_stat->port_number,
@@ -4077,6 +4078,16 @@ static int drm_dp_mst_handle_up_req(struct
drm_dp_mst_topology_mgr *mgr)
conn_stat->message_capability_status,
conn_stat->input_port,
conn_stat->peer_device_type);
+
+ mutex_lock(&mgr->probe_lock);
+ handle_csn = mgr->mst_primary->link_address_sent;
+ mutex_unlock(&mgr->probe_lock);
+
+ if (!handle_csn) {
+ drm_dbg_kms(mgr->dev, "Got CSN before finish topology
probing. Skip it.");
+ kfree(up_req);
+ goto out;
+ }
} else if (up_req->msg.req_type == DP_RESOURCE_STATUS_NOTIFY) {
const struct drm_dp_resource_status_notify *res_stat =
&up_req->msg.u.resource_stat;
--
2.37.3
[PATCH 1/3] drm/dp_mst: Fix all mstb marked as not probed after suspend/resume
[Why]
After supend/resume, with topology unchanged, observe that
link_address_sent of all mstb are marked as false even the topology probing
is done without any error.
It is caused by wrongly also include "ret == 0" case as a probing failure
case.
[How]
Remove inappropriate checking conditions.
Cc: Lyude Paul
Cc: Harry Wentland
Cc: Jani Nikula
Cc: Imre Deak
Cc: Daniel Vetter
Cc: sta...@vger.kernel.org
Fixes: 37dfdc55ffeb ("drm/dp_mst: Cleanup drm_dp_send_link_address() a bit")
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 7f8e1cfbe19d..68831f4e502a 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -2929,7 +2929,7 @@ static int drm_dp_send_link_address(struct
drm_dp_mst_topology_mgr *mgr,
/* FIXME: Actually do some real error handling here */
ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
- if (ret <= 0) {
+ if (ret < 0) {
drm_err(mgr->dev, "Sending link address failed with %d\n", ret);
goto out;
}
@@ -2981,7 +2981,7 @@ static int drm_dp_send_link_address(struct
drm_dp_mst_topology_mgr *mgr,
mutex_unlock(&mgr->lock);
out:
- if (ret <= 0)
+ if (ret < 0)
mstb->link_address_sent = false;
kfree(txmsg);
return ret < 0 ? ret : changed;
--
2.37.3
[PATCH 0/3] Fix mst daisy chain light up issue after resume
Under DP mst daisy chain configuration, unplug one chained monitor during suspend and then resume, observe left connected monitor not light up. After analyzing, seems it's due to changing dpcd DP_MSTM_CTRL value after LT during reume. We used to defer handling UP request by disabling DP_UP_REQ_EN at the begining of resume process to avoid some problems. However, it turns out that it will cause problem on the hub if we change DP_UP_REQ_EN after LT. This series is trying to solve the problem by another way that we don't explicitly disable DP_UP_REQ_EN at source side. Instead, source should ignore the CSN event before source completeting topology probing during resume. Wayne Lin (3): drm/dp_mst: Fix all mstb marked as not probed after suspend/resume drm/dp_mst: Skip CSN if topology probing is not done yet drm/amd/display: Solve mst monitors blank out problem after resume drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++- drivers/gpu/drm/display/drm_dp_mst_topology.c | 15 +-- 2 files changed, 15 insertions(+), 3 deletions(-) -- 2.37.3
[PATCH] drm/dp_mst: Fix all mstb marked as not probed after suspend/resume
[Why]
After supend/resume, with topology unchanged, observe that
link_address_sent of all mstb are marked as false even the topology probing
is done without any error.
It is caused by wrongly also include "ret == 0" case as a probing failure
case.
[How]
Remove inappropriate checking conditions.
Cc: Lyude Paul
Cc: Harry Wentland
Cc: Jani Nikula
Cc: sta...@vger.kernel.org
Fixes: 37dfdc55ffeb ("drm/dp_mst: Cleanup drm_dp_send_link_address() a bit")
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 7f8e1cfbe19d..68831f4e502a 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -2929,7 +2929,7 @@ static int drm_dp_send_link_address(struct
drm_dp_mst_topology_mgr *mgr,
/* FIXME: Actually do some real error handling here */
ret = drm_dp_mst_wait_tx_reply(mstb, txmsg);
- if (ret <= 0) {
+ if (ret < 0) {
drm_err(mgr->dev, "Sending link address failed with %d\n", ret);
goto out;
}
@@ -2981,7 +2981,7 @@ static int drm_dp_send_link_address(struct
drm_dp_mst_topology_mgr *mgr,
mutex_unlock(&mgr->lock);
out:
- if (ret <= 0)
+ if (ret < 0)
mstb->link_address_sent = false;
kfree(txmsg);
return ret < 0 ? ret : changed;
--
2.37.3
[PATCH] drm/mst: Fix NULL pointer dereference at drm_dp_add_payload_part2
[Why]
Commit:
- commit 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload
allocation/removement")
accidently overwrite the commit
- commit 54d217406afe ("drm: use mgr->dev in drm_dbg_kms in
drm_dp_add_payload_part2")
which cause regression.
[How]
Recover the original NULL fix and remove the unnecessary input parameter
'state' for
drm_dp_add_payload_part2().
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload
allocation/removement")
Reported-by: Leon Weiß
Link:
https://lore.kernel.org/r/38c253ea42072cc825dc969ac4e6b9b600371cc8.ca...@ruhr-uni-bochum.de/
Cc: ly...@redhat.com
Cc: imre.d...@intel.com
Cc: sta...@vger.kernel.org
Cc: regressi...@lists.linux.dev
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +-
drivers/gpu/drm/display/drm_dp_mst_topology.c | 4 +---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
include/drm/display/drm_dp_mst_helper.h | 1 -
5 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index c27063305a13..2c36f3d00ca2 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -363,7 +363,7 @@ void dm_helpers_dp_mst_send_payload_allocation(
mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
new_payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
- ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state,
new_payload);
+ ret = drm_dp_add_payload_part2(mst_mgr, new_payload);
if (ret) {
amdgpu_dm_set_mst_status(&aconnector->mst_status,
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 03d528209426..95fd18f24e94 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3421,7 +3421,6 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2);
/**
* drm_dp_add_payload_part2() - Execute payload update part 2
* @mgr: Manager to use.
- * @state: The global atomic state
* @payload: The payload to update
*
* If @payload was successfully assigned a starting time slot by
drm_dp_add_payload_part1(), this
@@ -3430,14 +3429,13 @@ EXPORT_SYMBOL(drm_dp_remove_payload_part2);
* Returns: 0 on success, negative error code on failure.
*/
int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
-struct drm_atomic_state *state,
struct drm_dp_mst_atomic_payload *payload)
{
int ret = 0;
/* Skip failed payloads */
if (payload->payload_allocation_status !=
DRM_DP_MST_PAYLOAD_ALLOCATION_DFP) {
- drm_dbg_kms(state->dev, "Part 1 of payload creation for %s
failed, skipping part 2\n",
+ drm_dbg_kms(mgr->dev, "Part 1 of payload creation for %s
failed, skipping part 2\n",
payload->port->connector->name);
return -EIO;
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 53aec023ce92..2fba66aec038 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -1160,7 +1160,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state
*state,
if (first_mst_stream)
intel_ddi_wait_for_fec_status(encoder, pipe_config, true);
- drm_dp_add_payload_part2(&intel_dp->mst_mgr, &state->base,
+ drm_dp_add_payload_part2(&intel_dp->mst_mgr,
drm_atomic_get_mst_payload_state(mst_state,
connector->port));
if (DISPLAY_VER(dev_priv) >= 12)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 0c3d88ad0b0e..88728a0b2c25 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -915,7 +915,7 @@ nv50_msto_cleanup(struct drm_atomic_state *state,
msto->disabled = false;
drm_dp_remove_payload_part2(mgr, new_mst_state, old_payload,
new_payload);
} else if (msto->enabled) {
- drm_dp_add_payload_part2(mgr, state, new_payload);
+ drm_dp_add_payload_part2(mgr, new_payload);
msto->enabled = false;
}
}
diff --git a/include/drm/display/drm_dp_mst_helper.h
b/include/drm/display/drm_dp_mst_helper.h
index 9b19d8bd520a..6c9145abc7e2 100644
--- a/include/drm/display/drm_dp_mst_helper.h
+++ b/include/drm/display/drm_dp_mst_helper.h
@@ -851,7 +851,6 @@ int dr
[PATCH] drm/amd/display: Fix mst hub unplug warning
[Why]
Unplug mst hub will cause warning. That's because
dm_helpers_construct_old_payload() is changed to be called after
payload removement from dc link.
In dm_helpers_construct_old_payload(), We refer to the vcpi in
payload allocation table of dc link to construct the old payload
and payload is no longer in the table when we call the function
now.
[How]
Refer to the mst_state to construct the number of time slot for old
payload now. Note that dm_helpers_construct_old_payload() is just
a quick workaround before and we are going to abandon it soon.
Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload
allocation/removement")
Reviewed-by: Jerry Zuo
Signed-off-by: Wayne Lin
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 38 +--
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index baf7e5254fb3..2f94bcf128c0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -204,15 +204,16 @@ void dm_helpers_dp_update_branch_info(
{}
static void dm_helpers_construct_old_payload(
- struct dc_link *link,
- int pbn_per_slot,
+ struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_mst_topology_state *mst_state,
struct drm_dp_mst_atomic_payload *new_payload,
struct drm_dp_mst_atomic_payload *old_payload)
{
- struct link_mst_stream_allocation_table current_link_table =
-
link->mst_stream_alloc_table;
- struct link_mst_stream_allocation *dc_alloc;
- int i;
+ struct drm_dp_mst_atomic_payload *pos;
+ int pbn_per_slot = mst_state->pbn_div;
+ u8 next_payload_vc_start = mgr->next_start_slot;
+ u8 payload_vc_start = new_payload->vc_start_slot;
+ u8 allocated_time_slots;
*old_payload = *new_payload;
@@ -221,20 +222,17 @@ static void dm_helpers_construct_old_payload(
* struct drm_dp_mst_atomic_payload are don't care fields
* while calling drm_dp_remove_payload_part2()
*/
- for (i = 0; i < current_link_table.stream_count; i++) {
- dc_alloc =
- ¤t_link_table.stream_allocations[i];
-
- if (dc_alloc->vcp_id == new_payload->vcpi) {
- old_payload->time_slots = dc_alloc->slot_count;
- old_payload->pbn = dc_alloc->slot_count * pbn_per_slot;
- break;
- }
+ list_for_each_entry(pos, &mst_state->payloads, next) {
+ if (pos != new_payload &&
+ pos->vc_start_slot > payload_vc_start &&
+ pos->vc_start_slot < next_payload_vc_start)
+ next_payload_vc_start = pos->vc_start_slot;
}
- /* make sure there is an old payload*/
- ASSERT(i != current_link_table.stream_count);
+ allocated_time_slots = next_payload_vc_start - payload_vc_start;
+ old_payload->time_slots = allocated_time_slots;
+ old_payload->pbn = allocated_time_slots * pbn_per_slot;
}
/*
@@ -272,8 +270,8 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
drm_dp_add_payload_part1(mst_mgr, mst_state, new_payload);
} else {
/* construct old payload by VCPI*/
- dm_helpers_construct_old_payload(stream->link,
mst_state->pbn_div,
- new_payload, &old_payload);
+ dm_helpers_construct_old_payload(mst_mgr, mst_state,
+new_payload, &old_payload);
target_payload = &old_payload;
drm_dp_remove_payload_part1(mst_mgr, mst_state, new_payload);
@@ -366,7 +364,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
if (enable) {
ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state,
new_payload);
} else {
- dm_helpers_construct_old_payload(stream->link,
mst_state->pbn_div,
+ dm_helpers_construct_old_payload(mst_mgr, mst_state,
new_payload, &old_payload);
drm_dp_remove_payload_part2(mst_mgr, mst_state, &old_payload,
new_payload);
}
--
2.37.3
[PATCH v3 3/3] drm/mst: adjust the function drm_dp_remove_payload_part2()
[Why]
Now in drm_dp_remove_payload_part2(), it utilizes the time slot number
of the payload in old state to represent the one in the payload table
at the moment.
It would be better to clarify the idea by using the latest allocated
time slot number for the port at the moment instead and which info is
already included in new mst_state. By this, we can also remove redundant
workaround for amdgpu driver.
[How]
Remove "old_payload" input of drm_dp_remove_payload_part2() and get the
latest number of allocated time slot for the port from new mst_state
instead.
Signed-off-by: Wayne Lin
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 70 ---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 32 ++---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 +-
include/drm/display/drm_dp_mst_helper.h | 9 ++-
5 files changed, 40 insertions(+), 84 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index cbef4ff28cd8..02cb372260f3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -203,40 +203,6 @@ void dm_helpers_dp_update_branch_info(
const struct dc_link *link)
{}
-static void dm_helpers_construct_old_payload(
- struct dc_link *link,
- int pbn_per_slot,
- struct drm_dp_mst_atomic_payload *new_payload,
- struct drm_dp_mst_atomic_payload *old_payload)
-{
- struct link_mst_stream_allocation_table current_link_table =
-
link->mst_stream_alloc_table;
- struct link_mst_stream_allocation *dc_alloc;
- int i;
-
- *old_payload = *new_payload;
-
- /* Set correct time_slots/PBN of old payload.
-* other fields (delete & dsc_enabled) in
-* struct drm_dp_mst_atomic_payload are don't care fields
-* while calling drm_dp_remove_payload_part2()
-*/
- for (i = 0; i < current_link_table.stream_count; i++) {
- dc_alloc =
- ¤t_link_table.stream_allocations[i];
-
- if (dc_alloc->vcp_id == new_payload->vcpi) {
- old_payload->time_slots = dc_alloc->slot_count;
- old_payload->pbn = dc_alloc->slot_count * pbn_per_slot;
- break;
- }
- }
-
- /* make sure there is an old payload*/
- ASSERT(i != current_link_table.stream_count);
-
-}
-
/*
* Writes payload allocation table in immediate downstream device.
*/
@@ -248,7 +214,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
{
struct amdgpu_dm_connector *aconnector;
struct drm_dp_mst_topology_state *mst_state;
- struct drm_dp_mst_atomic_payload *target_payload, *new_payload,
old_payload;
+ struct drm_dp_mst_atomic_payload *payload;
struct drm_dp_mst_topology_mgr *mst_mgr;
aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
@@ -263,28 +229,21 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
mst_mgr = &aconnector->mst_root->mst_mgr;
mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
- new_payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
-
- if (enable) {
- target_payload = new_payload;
+ payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
+ if (enable)
/* It's OK for this to fail */
- drm_dp_add_payload_part1(mst_mgr, mst_state, new_payload);
- } else {
- /* construct old payload by VCPI*/
- dm_helpers_construct_old_payload(stream->link,
mst_state->pbn_div,
- new_payload, &old_payload);
- target_payload = &old_payload;
+ drm_dp_add_payload_part1(mst_mgr, mst_state, payload);
+ else
- drm_dp_remove_payload_part1(mst_mgr, mst_state, new_payload);
- }
+ drm_dp_remove_payload_part1(mst_mgr, mst_state, payload);
/* mst_mgr->->payloads are VC payload notify MST branch using DPCD or
* AUX message. The sequence is slot 1-63 allocated sequence for each
* stream. AMD ASIC stream slot allocation should follow the same
* sequence. copy DRM MST allocation to dc
*/
- fill_dc_mst_payload_table_from_drm(stream->link, enable,
target_payload, proposed_table);
+ fill_dc_mst_payload_table_from_drm(stream->link, enable, payload,
proposed_table);
return true;
}
@@ -343,7 +302,7 @@ bool dm_helper
[PATCH v3 2/3] drm/mst: Refactor the flow for payload allocation/removement
[Why]
Today, the allocation/deallocation steps and status is a bit unclear.
For instance, payload->vc_start_slot = -1 stands for "the failure of
updating DPCD payload ID table" and can also represent as "payload is not
allocated yet". These two cases should be handled differently and hence
better to distinguish them for better understanding.
[How]
Define enumeration - ALLOCATION_LOCAL, ALLOCATION_DFP and ALLOCATION_REMOTE
to distinguish different allocation status. Adjust the code to handle
different status accordingly for better understanding the sequence of
payload allocation and payload removement.
For payload creation, the procedure should look like this:
DRM part 1:
* step 1 - update sw mst mgr variables to add a new payload
* step 2 - add payload at immediate DFP DPCD payload table
Driver:
* Add new payload in HW and sync up with DFP by sending ACT
DRM Part 2:
* Send ALLOCATE_PAYLOAD sideband message to allocate bandwidth along the
virtual channel.
And as for payload removement, the procedure should look like this:
DRM part 1:
* step 1 - Send ALLOCATE_PAYLOAD sideband message to release bandwidth
along the virtual channel
* step 2 - Clear payload allocation at immediate DFP DPCD payload table
Driver:
* Remove the payload in HW and sync up with DFP by sending ACT
DRM part 2:
* update sw mst mgr variables to remove the payload
Note that it's fine to fail when communicate with the branch device
connected at immediate downstrean-facing port, but updating variables of
SW mst mgr and HW configuration should be conducted anyway. That's because
it's under commit_tail and we need to complete the HW programming.
Changes since v1:
* Remove the set but not use variable 'old_payload' in function
'nv50_msto_prepare'. Catched by kernel test robot
Changes since v2:
* Fix indention
Signed-off-by: Wayne Lin
Reviewed-by: Lyude Paul
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 20 ++-
drivers/gpu/drm/display/drm_dp_mst_topology.c | 159 +++---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 18 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 21 +--
include/drm/display/drm_dp_mst_helper.h | 23 ++-
5 files changed, 153 insertions(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 4b230933b28e..cbef4ff28cd8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -219,7 +219,7 @@ static void dm_helpers_construct_old_payload(
/* Set correct time_slots/PBN of old payload.
* other fields (delete & dsc_enabled) in
* struct drm_dp_mst_atomic_payload are don't care fields
-* while calling drm_dp_remove_payload()
+* while calling drm_dp_remove_payload_part2()
*/
for (i = 0; i < current_link_table.stream_count; i++) {
dc_alloc =
@@ -263,13 +263,12 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
mst_mgr = &aconnector->mst_root->mst_mgr;
mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
-
- /* It's OK for this to fail */
new_payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
if (enable) {
target_payload = new_payload;
+ /* It's OK for this to fail */
drm_dp_add_payload_part1(mst_mgr, mst_state, new_payload);
} else {
/* construct old payload by VCPI*/
@@ -277,7 +276,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
new_payload, &old_payload);
target_payload = &old_payload;
- drm_dp_remove_payload(mst_mgr, mst_state, &old_payload,
new_payload);
+ drm_dp_remove_payload_part1(mst_mgr, mst_state, new_payload);
}
/* mst_mgr->->payloads are VC payload notify MST branch using DPCD or
@@ -344,7 +343,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
struct amdgpu_dm_connector *aconnector;
struct drm_dp_mst_topology_state *mst_state;
struct drm_dp_mst_topology_mgr *mst_mgr;
- struct drm_dp_mst_atomic_payload *payload;
+ struct drm_dp_mst_atomic_payload *new_payload, *old_payload;
enum mst_progress_status set_flag = MST_ALLOCATE_NEW_PAYLOAD;
enum mst_progress_status clr_flag = MST_CLEAR_ALLOCATED_PAYLOAD;
int ret = 0;
@@ -357,15 +356,20 @@ bool dm_helpers_dp_mst_send_payload_allocation(
mst_mgr = &aconnector->mst_root->mst_mgr;
mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
- payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
+ new_payl
[PATCH v3 1/3] drm/mst: delete unnecessary case in drm_dp_add_payload_part2()
[Why]
There is no need to consider payload->delete case since we won't call
drm_dp_add_payload_part2() to create a payload when we're about to
remove it.
[How]
Delete unnecessary case to simplify the code.
Signed-off-by: Wayne Lin
Reviewed-by: Lyude Paul
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index ed96cfcfa304..4d80426757ab 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3411,12 +3411,8 @@ int drm_dp_add_payload_part2(struct
drm_dp_mst_topology_mgr *mgr,
ret = drm_dp_create_payload_step2(mgr, payload);
if (ret < 0) {
- if (!payload->delete)
- drm_err(mgr->dev, "Step 2 of creating MST payload for
%p failed: %d\n",
- payload->port, ret);
- else
- drm_dbg_kms(mgr->dev, "Step 2 of removing MST payload
for %p failed: %d\n",
- payload->port, ret);
+ drm_err(mgr->dev, "Step 2 of creating MST payload for %p
failed: %d\n",
+ payload->port, ret);
}
return ret;
--
2.37.3
[PATCH v3 0/3] Refactor and clean up codes of mst
This patch set is mainly trying to organize the mst code today a bit. Like to clarify and organize the sequence of mst payload allocation and removement.And also clean up some redundant codes today. The main refactor one is the patch "drm/mst: Refactor the flow for payload allocation/removement" which is adding a new enum variable in stuct drm_dp_mst_atomic_payload to represent the status of paylad alloction, and then handle the payload accordingly. Besides, rename some drm mst functions to better express the behind idea. The other two patches are mainly to clean up unnecessary codes. Changes since v1: * Remove the set but not use variable 'old_payload' in function 'nv50_msto_prepare'. Catched by kernel test robot Changes since v2: * Fix indention Wayne Lin (3): drm/mst: delete unnecessary case in drm_dp_add_payload_part2() drm/mst: Refactor the flow for payload allocation/removement drm/mst: adjust the function drm_dp_remove_payload_part2() .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 60 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 189 +++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 13 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 17 +- include/drm/display/drm_dp_mst_helper.h | 22 +- 5 files changed, 159 insertions(+), 142 deletions(-) -- 2.37.3
[Patch v2 2/3] drm/mst: Refactor the flow for payload allocation/removement
[Why]
Today, the allocation/deallocation steps and status is a bit unclear.
For instance, payload->vc_start_slot = -1 stands for "the failure of
updating DPCD payload ID table" and can also represent as "payload is not
allocated yet". These two cases should be handled differently and hence
better to distinguish them for better understanding.
[How]
Define enumeration - ALLOCATION_LOCAL, ALLOCATION_DFP and ALLOCATION_REMOTE
to distinguish different allocation status. Adjust the code to handle
different status accordingly for better understanding the sequence of
payload allocation and payload removement.
For payload creation, the procedure should look like this:
DRM part 1:
* step 1 - update sw mst mgr variables to add a new payload
* step 2 - add payload at immediate DFP DPCD payload table
Driver:
* Add new payload in HW and sync up with DFP by sending ACT
DRM Part 2:
* Send ALLOCATE_PAYLOAD sideband message to allocate bandwidth along the
virtual channel.
And as for payload removement, the procedure should look like this:
DRM part 1:
* step 1 - Send ALLOCATE_PAYLOAD sideband message to release bandwidth
along the virtual channel
* step 2 - Clear payload allocation at immediate DFP DPCD payload table
Driver:
* Remove the payload in HW and sync up with DFP by sending ACT
DRM part 2:
* update sw mst mgr variables to remove the payload
Note that it's fine to fail when communicate with the branch device
connected at immediate downstrean-facing port, but updating variables of
SW mst mgr and HW configuration should be conducted anyway. That's because
it's under commit_tail and we need to complete the HW programming.
Changes since v1:
* Remove the set but not use variable 'old_payload' in function
'nv50_msto_prepare'. Catched by kernel test robot
Signed-off-by: Wayne Lin
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 20 ++-
drivers/gpu/drm/display/drm_dp_mst_topology.c | 159 +++---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 18 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 21 +--
include/drm/display/drm_dp_mst_helper.h | 23 ++-
5 files changed, 153 insertions(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index d9a482908380..9ad509279b0a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -219,7 +219,7 @@ static void dm_helpers_construct_old_payload(
/* Set correct time_slots/PBN of old payload.
* other fields (delete & dsc_enabled) in
* struct drm_dp_mst_atomic_payload are don't care fields
-* while calling drm_dp_remove_payload()
+* while calling drm_dp_remove_payload_part2()
*/
for (i = 0; i < current_link_table.stream_count; i++) {
dc_alloc =
@@ -262,13 +262,12 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
mst_mgr = &aconnector->mst_root->mst_mgr;
mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
-
- /* It's OK for this to fail */
new_payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
if (enable) {
target_payload = new_payload;
+ /* It's OK for this to fail */
drm_dp_add_payload_part1(mst_mgr, mst_state, new_payload);
} else {
/* construct old payload by VCPI*/
@@ -276,7 +275,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
new_payload, &old_payload);
target_payload = &old_payload;
- drm_dp_remove_payload(mst_mgr, mst_state, &old_payload,
new_payload);
+ drm_dp_remove_payload_part1(mst_mgr, mst_state, new_payload);
}
/* mst_mgr->->payloads are VC payload notify MST branch using DPCD or
@@ -342,7 +341,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
struct amdgpu_dm_connector *aconnector;
struct drm_dp_mst_topology_state *mst_state;
struct drm_dp_mst_topology_mgr *mst_mgr;
- struct drm_dp_mst_atomic_payload *payload;
+ struct drm_dp_mst_atomic_payload *new_payload, *old_payload;
enum mst_progress_status set_flag = MST_ALLOCATE_NEW_PAYLOAD;
enum mst_progress_status clr_flag = MST_CLEAR_ALLOCATED_PAYLOAD;
int ret = 0;
@@ -355,15 +354,20 @@ bool dm_helpers_dp_mst_send_payload_allocation(
mst_mgr = &aconnector->mst_root->mst_mgr;
mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
- payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
+ new_payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
[Patch v2 3/3] drm/mst: adjust the function drm_dp_remove_payload_part2()
[Why]
Now in drm_dp_remove_payload_part2(), it utilizes the time slot number
of the payload in old state to represent the one in the payload table
at the moment.
It would be better to clarify the idea by using the latest allocated
time slot number for the port at the moment instead and which info is
already included in new mst_state. By this, we can also remove redundant
workaround for amdgpu driver.
[How]
Remove "old_payload" input of drm_dp_remove_payload_part2() and get the
latest number of allocated time slot for the port from new mst_state
instead.
Signed-off-by: Wayne Lin
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 70 ---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 32 ++---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 +-
include/drm/display/drm_dp_mst_helper.h | 9 ++-
5 files changed, 40 insertions(+), 84 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 9ad509279b0a..e852da686c26 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -203,40 +203,6 @@ void dm_helpers_dp_update_branch_info(
const struct dc_link *link)
{}
-static void dm_helpers_construct_old_payload(
- struct dc_link *link,
- int pbn_per_slot,
- struct drm_dp_mst_atomic_payload *new_payload,
- struct drm_dp_mst_atomic_payload *old_payload)
-{
- struct link_mst_stream_allocation_table current_link_table =
-
link->mst_stream_alloc_table;
- struct link_mst_stream_allocation *dc_alloc;
- int i;
-
- *old_payload = *new_payload;
-
- /* Set correct time_slots/PBN of old payload.
-* other fields (delete & dsc_enabled) in
-* struct drm_dp_mst_atomic_payload are don't care fields
-* while calling drm_dp_remove_payload_part2()
-*/
- for (i = 0; i < current_link_table.stream_count; i++) {
- dc_alloc =
- ¤t_link_table.stream_allocations[i];
-
- if (dc_alloc->vcp_id == new_payload->vcpi) {
- old_payload->time_slots = dc_alloc->slot_count;
- old_payload->pbn = dc_alloc->slot_count * pbn_per_slot;
- break;
- }
- }
-
- /* make sure there is an old payload*/
- ASSERT(i != current_link_table.stream_count);
-
-}
-
/*
* Writes payload allocation table in immediate downstream device.
*/
@@ -248,7 +214,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
{
struct amdgpu_dm_connector *aconnector;
struct drm_dp_mst_topology_state *mst_state;
- struct drm_dp_mst_atomic_payload *target_payload, *new_payload,
old_payload;
+ struct drm_dp_mst_atomic_payload *payload;
struct drm_dp_mst_topology_mgr *mst_mgr;
aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
@@ -262,27 +228,20 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
mst_mgr = &aconnector->mst_root->mst_mgr;
mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
- new_payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
-
- if (enable) {
- target_payload = new_payload;
+ payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
+ if (enable)
/* It's OK for this to fail */
- drm_dp_add_payload_part1(mst_mgr, mst_state, new_payload);
- } else {
- /* construct old payload by VCPI*/
- dm_helpers_construct_old_payload(stream->link,
mst_state->pbn_div,
- new_payload, &old_payload);
- target_payload = &old_payload;
+ drm_dp_add_payload_part1(mst_mgr, mst_state, payload);
+ else
- drm_dp_remove_payload_part1(mst_mgr, mst_state, new_payload);
- }
+ drm_dp_remove_payload_part1(mst_mgr, mst_state, payload);
/* mst_mgr->->payloads are VC payload notify MST branch using DPCD or
* AUX message. The sequence is slot 1-63 allocated sequence for each
* stream. AMD ASIC stream slot allocation should follow the same
* sequence. copy DRM MST allocation to dc */
- fill_dc_mst_payload_table_from_drm(stream->link, enable,
target_payload, proposed_table);
+ fill_dc_mst_payload_table_from_drm(stream->link, enable, payload,
proposed_table);
return true;
}
@@ -341,7 +300,7 @@ bool dm_helpers_dp_mst_send_payload_allo
[Patch v2 1/3] drm/mst: delete unnecessary case in drm_dp_add_payload_part2()
[Why]
There is no need to consider payload->delete case since we won't call
drm_dp_add_payload_part2() to create a payload when we're about to
remove it.
[How]
Delete unnecessary case to simplify the code.
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index ed96cfcfa304..4d80426757ab 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3411,12 +3411,8 @@ int drm_dp_add_payload_part2(struct
drm_dp_mst_topology_mgr *mgr,
ret = drm_dp_create_payload_step2(mgr, payload);
if (ret < 0) {
- if (!payload->delete)
- drm_err(mgr->dev, "Step 2 of creating MST payload for
%p failed: %d\n",
- payload->port, ret);
- else
- drm_dbg_kms(mgr->dev, "Step 2 of removing MST payload
for %p failed: %d\n",
- payload->port, ret);
+ drm_err(mgr->dev, "Step 2 of creating MST payload for %p
failed: %d\n",
+ payload->port, ret);
}
return ret;
--
2.37.3
[Patch v2 0/3] Refactor and clean up codes of mst
This patch set is mainly trying to organize the mst code today a bit. Like to clarify and organize the sequence of mst payload allocation and removement.And also clean up some redundant codes today. The main refactor one is the patch "drm/mst: Refactor the flow for payload allocation/removement" which is adding a new enum variable in stuct drm_dp_mst_atomic_payload to represent the status of paylad alloction, and then handle the payload accordingly. Besides, rename some drm mst functions to better express the behind idea. The other two patches are mainly to clean up unnecessary codes. Changes since v1: * Remove the set but not use variable 'old_payload' in function 'nv50_msto_prepare'. Catched by kernel test robot Wayne Lin (3): drm/mst: delete unnecessary case in drm_dp_add_payload_part2() drm/mst: Refactor the flow for payload allocation/removement drm/mst: adjust the function drm_dp_remove_payload_part2() .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 60 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 189 +++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 13 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 17 +- include/drm/display/drm_dp_mst_helper.h | 22 +- 5 files changed, 159 insertions(+), 142 deletions(-) -- 2.37.3
[PATCH 3/3] drm/mst: adjust the function drm_dp_remove_payload_part2()
[Why]
Now in drm_dp_remove_payload_part2(), it utilizes the time slot number
of the payload in old state to represent the one in the payload table
at the moment.
It would be better to clarify the idea by using the latest allocated
time slot number for the port at the moment instead and which info is
already included in new mst_state. By this, we can also remove redundant
workaround for amdgpu driver.
[How]
Remove "old_payload" input of drm_dp_remove_payload_part2() and get the
latest number of allocated time slot for the port from new mst_state
instead.
Signed-off-by: Wayne Lin
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 70 ---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 32 ++---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 7 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 6 +-
include/drm/display/drm_dp_mst_helper.h | 9 ++-
5 files changed, 40 insertions(+), 84 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 9ad509279b0a..e852da686c26 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -203,40 +203,6 @@ void dm_helpers_dp_update_branch_info(
const struct dc_link *link)
{}
-static void dm_helpers_construct_old_payload(
- struct dc_link *link,
- int pbn_per_slot,
- struct drm_dp_mst_atomic_payload *new_payload,
- struct drm_dp_mst_atomic_payload *old_payload)
-{
- struct link_mst_stream_allocation_table current_link_table =
-
link->mst_stream_alloc_table;
- struct link_mst_stream_allocation *dc_alloc;
- int i;
-
- *old_payload = *new_payload;
-
- /* Set correct time_slots/PBN of old payload.
-* other fields (delete & dsc_enabled) in
-* struct drm_dp_mst_atomic_payload are don't care fields
-* while calling drm_dp_remove_payload_part2()
-*/
- for (i = 0; i < current_link_table.stream_count; i++) {
- dc_alloc =
- ¤t_link_table.stream_allocations[i];
-
- if (dc_alloc->vcp_id == new_payload->vcpi) {
- old_payload->time_slots = dc_alloc->slot_count;
- old_payload->pbn = dc_alloc->slot_count * pbn_per_slot;
- break;
- }
- }
-
- /* make sure there is an old payload*/
- ASSERT(i != current_link_table.stream_count);
-
-}
-
/*
* Writes payload allocation table in immediate downstream device.
*/
@@ -248,7 +214,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
{
struct amdgpu_dm_connector *aconnector;
struct drm_dp_mst_topology_state *mst_state;
- struct drm_dp_mst_atomic_payload *target_payload, *new_payload,
old_payload;
+ struct drm_dp_mst_atomic_payload *payload;
struct drm_dp_mst_topology_mgr *mst_mgr;
aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
@@ -262,27 +228,20 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
mst_mgr = &aconnector->mst_root->mst_mgr;
mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
- new_payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
-
- if (enable) {
- target_payload = new_payload;
+ payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
+ if (enable)
/* It's OK for this to fail */
- drm_dp_add_payload_part1(mst_mgr, mst_state, new_payload);
- } else {
- /* construct old payload by VCPI*/
- dm_helpers_construct_old_payload(stream->link,
mst_state->pbn_div,
- new_payload, &old_payload);
- target_payload = &old_payload;
+ drm_dp_add_payload_part1(mst_mgr, mst_state, payload);
+ else
- drm_dp_remove_payload_part1(mst_mgr, mst_state, new_payload);
- }
+ drm_dp_remove_payload_part1(mst_mgr, mst_state, payload);
/* mst_mgr->->payloads are VC payload notify MST branch using DPCD or
* AUX message. The sequence is slot 1-63 allocated sequence for each
* stream. AMD ASIC stream slot allocation should follow the same
* sequence. copy DRM MST allocation to dc */
- fill_dc_mst_payload_table_from_drm(stream->link, enable,
target_payload, proposed_table);
+ fill_dc_mst_payload_table_from_drm(stream->link, enable, payload,
proposed_table);
return true;
}
@@ -341,7 +300,7 @@ bool dm_helpers_dp_mst_send_payload_allo
[PATCH 1/3] drm/mst: delete unnecessary case in drm_dp_add_payload_part2()
[Why]
There is no need to consider payload->delete case since we won't call
drm_dp_add_payload_part2() to create a payload when we're about to
remove it.
[How]
Delete unnecessary case to simplify the code.
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index ed96cfcfa304..4d80426757ab 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3411,12 +3411,8 @@ int drm_dp_add_payload_part2(struct
drm_dp_mst_topology_mgr *mgr,
ret = drm_dp_create_payload_step2(mgr, payload);
if (ret < 0) {
- if (!payload->delete)
- drm_err(mgr->dev, "Step 2 of creating MST payload for
%p failed: %d\n",
- payload->port, ret);
- else
- drm_dbg_kms(mgr->dev, "Step 2 of removing MST payload
for %p failed: %d\n",
- payload->port, ret);
+ drm_err(mgr->dev, "Step 2 of creating MST payload for %p
failed: %d\n",
+ payload->port, ret);
}
return ret;
--
2.37.3
[PATCH 2/3] drm/mst: Refactor the flow for payload allocation/removement
[Why]
Today, the allocation/deallocation steps and status is a bit unclear.
For instance, payload->vc_start_slot = -1 stands for "the failure of
updating DPCD payload ID table" and can also represent as "payload is not
allocated yet". These two cases should be handled differently and hence
better to distinguish them for better understanding.
[How]
Define enumeration - ALLOCATION_LOCAL, ALLOCATION_DFP and ALLOCATION_REMOTE
to distinguish different allocation status. Adjust the code to handle
different status accordingly for better understanding the sequence of
payload allocation and payload removement.
For payload creation, the procedure should look like this:
DRM part 1:
* step 1 - update sw mst mgr variables to add a new payload
* step 2 - add payload at immediate DFP DPCD payload table
Driver:
* Add new payload in HW and sync up with DFP by sending ACT
DRM Part 2:
* Send ALLOCATE_PAYLOAD sideband message to allocate bandwidth along the
virtual channel.
And as for payload removement, the procedure should look like this:
DRM part 1:
* step 1 - Send ALLOCATE_PAYLOAD sideband message to release bandwidth
along the virtual channel
* step 2 - Clear payload allocation at immediate DFP DPCD payload table
Driver:
* Remove the payload in HW and sync up with DFP by sending ACT
DRM part 2:
* update sw mst mgr variables to remove the payload
Note that it's fine to fail when communicate with the branch device
connected at immediate downstrean-facing port, but updating variables of
SW mst mgr and HW configuration should be conducted anyway. That's because
it's under commit_tail and we need to complete the HW programming.
Signed-off-by: Wayne Lin
---
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 20 ++-
drivers/gpu/drm/display/drm_dp_mst_topology.c | 159 +++---
drivers/gpu/drm/i915/display/intel_dp_mst.c | 18 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 15 +-
include/drm/display/drm_dp_mst_helper.h | 23 ++-
5 files changed, 152 insertions(+), 83 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index d9a482908380..9ad509279b0a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -219,7 +219,7 @@ static void dm_helpers_construct_old_payload(
/* Set correct time_slots/PBN of old payload.
* other fields (delete & dsc_enabled) in
* struct drm_dp_mst_atomic_payload are don't care fields
-* while calling drm_dp_remove_payload()
+* while calling drm_dp_remove_payload_part2()
*/
for (i = 0; i < current_link_table.stream_count; i++) {
dc_alloc =
@@ -262,13 +262,12 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
mst_mgr = &aconnector->mst_root->mst_mgr;
mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
-
- /* It's OK for this to fail */
new_payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
if (enable) {
target_payload = new_payload;
+ /* It's OK for this to fail */
drm_dp_add_payload_part1(mst_mgr, mst_state, new_payload);
} else {
/* construct old payload by VCPI*/
@@ -276,7 +275,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
new_payload, &old_payload);
target_payload = &old_payload;
- drm_dp_remove_payload(mst_mgr, mst_state, &old_payload,
new_payload);
+ drm_dp_remove_payload_part1(mst_mgr, mst_state, new_payload);
}
/* mst_mgr->->payloads are VC payload notify MST branch using DPCD or
@@ -342,7 +341,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
struct amdgpu_dm_connector *aconnector;
struct drm_dp_mst_topology_state *mst_state;
struct drm_dp_mst_topology_mgr *mst_mgr;
- struct drm_dp_mst_atomic_payload *payload;
+ struct drm_dp_mst_atomic_payload *new_payload, *old_payload;
enum mst_progress_status set_flag = MST_ALLOCATE_NEW_PAYLOAD;
enum mst_progress_status clr_flag = MST_CLEAR_ALLOCATED_PAYLOAD;
int ret = 0;
@@ -355,15 +354,20 @@ bool dm_helpers_dp_mst_send_payload_allocation(
mst_mgr = &aconnector->mst_root->mst_mgr;
mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
- payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
+ new_payload = drm_atomic_get_mst_payload_state(mst_state,
aconnector->mst_output_port);
if (!enable) {
set_flag = MST_CLEAR_ALLOCATED_PAYLOAD;
clr_flag = MST_ALLOCATE_NEW_PAYLOAD;
}
- if (ena
[PATCH 0/3] Refactor and clean up codes of mst
This patch set is mainly trying to organize the mst code today a bit. Like to clarify and organize the sequence of mst payload allocation and removement.And also clean up some redundant codes today. The main refactor one is the patch "drm/mst: Refactor the flow for payload allocation/removement" which is adding a new enum variable in stuct drm_dp_mst_atomic_payload to represent the status of paylad alloction, and then handle the payload accordingly. Besides, rename some drm mst fnctions to better express the behind idea. The other two patches are mainly to clean up unnecessary codes. Wayne Lin (3): drm/mst: delete unnecessary case in drm_dp_add_payload_part2() drm/mst: Refactor the flow for payload allocation/removement drm/mst: adjust the function drm_dp_remove_payload_part2() .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 60 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 189 +++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 13 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 11 +- include/drm/display/drm_dp_mst_helper.h | 22 +- 5 files changed, 158 insertions(+), 137 deletions(-) -- 2.37.3
[PATCH v5] drm/dp_mst: Clear MSG_RDY flag before sending new message
[Why]
The sequence for collecting down_reply from source perspective should
be:
Request_n->repeat (get partial reply of Request_n->clear message ready
flag to ack DPRX that the message is received) till all partial
replies for Request_n are received->new Request_n+1.
Now there is chance that drm_dp_mst_hpd_irq() will fire new down
request in the tx queue when the down reply is incomplete. Source is
restricted to generate interveleaved message transactions so we should
avoid it.
Also, while assembling partial reply packets, reading out DPCD DOWN_REP
Sideband MSG buffer + clearing DOWN_REP_MSG_RDY flag should be
wrapped up as a complete operation for reading out a reply packet.
Kicking off a new request before clearing DOWN_REP_MSG_RDY flag might
be risky. e.g. If the reply of the new request has overwritten the
DPRX DOWN_REP Sideband MSG buffer before source writing one to clear
DOWN_REP_MSG_RDY flag, source then unintentionally flushes the reply
for the new request. Should handle the up request in the same way.
[How]
Separete drm_dp_mst_hpd_irq() into 2 steps. After acking the MST IRQ
event, driver calls drm_dp_mst_hpd_irq_send_new_request() and might
trigger drm_dp_mst_kick_tx() only when there is no on going message
transaction.
Changes since v1:
* Reworked on review comments received
-> Adjust the fix to let driver explicitly kick off new down request
when mst irq event is handled and acked
-> Adjust the commit message
Changes since v2:
* Adjust the commit message
* Adjust the naming of the divided 2 functions and add a new input
parameter "ack".
* Adjust code flow as per review comments.
Changes since v3:
* Update the function description of drm_dp_mst_hpd_irq_handle_event
Changes since v4:
* Change ack of drm_dp_mst_hpd_irq_handle_event() to be an array align
the size of esi[]
Signed-off-by: Wayne Lin
Cc: sta...@vger.kernel.org
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 +--
drivers/gpu/drm/display/drm_dp_mst_topology.c | 54 ---
drivers/gpu/drm/i915/display/intel_dp.c | 7 +--
drivers/gpu/drm/nouveau/dispnv50/disp.c | 12 +++--
include/drm/display/drm_dp_mst_helper.h | 7 ++-
5 files changed, 81 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index d5cec03eaa8d..ec629b4037e4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3263,6 +3263,7 @@ static void dm_handle_mst_sideband_msg(struct
amdgpu_dm_connector *aconnector)
while (dret == dpcd_bytes_to_read &&
process_count < max_process_count) {
+ u8 ack[DP_PSR_ERROR_STATUS - DP_SINK_COUNT_ESI] = {};
u8 retry;
dret = 0;
@@ -3271,28 +3272,29 @@ static void dm_handle_mst_sideband_msg(struct
amdgpu_dm_connector *aconnector)
DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
esi[2]);
/* handle HPD short pulse irq */
if (aconnector->mst_mgr.mst_state)
- drm_dp_mst_hpd_irq(
- &aconnector->mst_mgr,
- esi,
- &new_irq_handled);
+ drm_dp_mst_hpd_irq_handle_event(&aconnector->mst_mgr,
+ esi,
+ ack,
+ &new_irq_handled);
if (new_irq_handled) {
/* ACK at DPCD to notify down stream */
- const int ack_dpcd_bytes_to_write =
- dpcd_bytes_to_read - 1;
-
for (retry = 0; retry < 3; retry++) {
- u8 wret;
-
- wret = drm_dp_dpcd_write(
- &aconnector->dm_dp_aux.aux,
- dpcd_addr + 1,
- &esi[1],
- ack_dpcd_bytes_to_write);
- if (wret == ack_dpcd_bytes_to_write)
+ ssize_t wret;
+
+ wret =
drm_dp_dpcd_writeb(&aconnector->dm_dp_aux.aux,
+ dpcd_addr + 1,
+ ack[1]);
+ if (wret == 1)
break;
}
+ if (retry == 3) {
+ DRM_ERROR("Failed to ack MST event.\n");
+ return;
+ }
+
+
drm_dp_mst_h
[PATCH v4] drm/dp_mst: Clear MSG_RDY flag before sending new message
[Why]
The sequence for collecting down_reply from source perspective should
be:
Request_n->repeat (get partial reply of Request_n->clear message ready
flag to ack DPRX that the message is received) till all partial
replies for Request_n are received->new Request_n+1.
Now there is chance that drm_dp_mst_hpd_irq() will fire new down
request in the tx queue when the down reply is incomplete. Source is
restricted to generate interveleaved message transactions so we should
avoid it.
Also, while assembling partial reply packets, reading out DPCD DOWN_REP
Sideband MSG buffer + clearing DOWN_REP_MSG_RDY flag should be
wrapped up as a complete operation for reading out a reply packet.
Kicking off a new request before clearing DOWN_REP_MSG_RDY flag might
be risky. e.g. If the reply of the new request has overwritten the
DPRX DOWN_REP Sideband MSG buffer before source writing one to clear
DOWN_REP_MSG_RDY flag, source then unintentionally flushes the reply
for the new request. Should handle the up request in the same way.
[How]
Separete drm_dp_mst_hpd_irq() into 2 steps. After acking the MST IRQ
event, driver calls drm_dp_mst_hpd_irq_send_new_request() and might
trigger drm_dp_mst_kick_tx() only when there is no on going message
transaction.
Changes since v1:
* Reworked on review comments received
-> Adjust the fix to let driver explicitly kick off new down request
when mst irq event is handled and acked
-> Adjust the commit message
Changes since v2:
* Adjust the commit message
* Adjust the naming of the divided 2 functions and add a new input
parameter "ack".
* Adjust code flow as per review comments.
Changes since v3:
* Update the function description of drm_dp_mst_hpd_irq_handle_event
Signed-off-by: Wayne Lin
Cc: sta...@vger.kernel.org
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 33 ++--
drivers/gpu/drm/display/drm_dp_mst_topology.c | 54 ---
drivers/gpu/drm/i915/display/intel_dp.c | 7 +--
drivers/gpu/drm/nouveau/dispnv50/disp.c | 12 +++--
include/drm/display/drm_dp_mst_helper.h | 7 ++-
5 files changed, 82 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index d5cec03eaa8d..597c3368bcfb 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3236,6 +3236,7 @@ static void dm_handle_mst_sideband_msg(struct
amdgpu_dm_connector *aconnector)
{
u8 esi[DP_PSR_ERROR_STATUS - DP_SINK_COUNT_ESI] = { 0 };
u8 dret;
+ u8 ack;
bool new_irq_handled = false;
int dpcd_addr;
int dpcd_bytes_to_read;
@@ -3265,34 +3266,36 @@ static void dm_handle_mst_sideband_msg(struct
amdgpu_dm_connector *aconnector)
process_count < max_process_count) {
u8 retry;
dret = 0;
+ ack = 0;
process_count++;
DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
esi[2]);
/* handle HPD short pulse irq */
if (aconnector->mst_mgr.mst_state)
- drm_dp_mst_hpd_irq(
- &aconnector->mst_mgr,
- esi,
- &new_irq_handled);
+ drm_dp_mst_hpd_irq_handle_event(&aconnector->mst_mgr,
+ esi,
+ &ack,
+ &new_irq_handled);
if (new_irq_handled) {
/* ACK at DPCD to notify down stream */
- const int ack_dpcd_bytes_to_write =
- dpcd_bytes_to_read - 1;
-
for (retry = 0; retry < 3; retry++) {
- u8 wret;
-
- wret = drm_dp_dpcd_write(
- &aconnector->dm_dp_aux.aux,
- dpcd_addr + 1,
- &esi[1],
- ack_dpcd_bytes_to_write);
- if (wret == ack_dpcd_bytes_to_write)
+ ssize_t wret;
+
+ wret =
drm_dp_dpcd_writeb(&aconnector->dm_dp_aux.aux,
+ dpcd_addr + 1,
+ ack);
+ if (wret == 1)
break;
}
+ if (retry == 3) {
+ DRM_ERROR("Failed to ack MST event.\n");
+ return;
+
[PATCH v2] drm/dp_mst: Clear MSG_RDY flag before sending new message
[Why]
The sequence for collecting down_reply from source perspective should
be:
Request_n->repeat (get partial reply of Request_n->clear message ready
flag to ack DPRX that the message is received) till all partial
replies for Request_n are received->new Request_n+1.
Now there is chance that drm_dp_mst_hpd_irq() will fire new down
request in the tx queue when the down reply is incomplete. Source is
restricted to generate interveleaved message transactions so we should
avoid it.
Also, while assembling partial reply packets, reading out DPCD DOWN_REP
Sideband MSG buffer + clearing DOWN_REP_MSG_RDY flag should be
wrapped up as a complete operation for reading out a reply packet.
Kicking off a new request before clearing DOWN_REP_MSG_RDY flag might
be risky. e.g. If the reply of the new request has overwritten the
DPRX DOWN_REP Sideband MSG buffer before source writing one to clear
DOWN_REP_MSG_RDY flag, source then unintentionally flushes the reply
for the new request. Should handle the up request in the same way.
[How]
Separete drm_dp_mst_hpd_irq() into 2 steps. After acking the MST IRQ
event, driver calls drm_dp_mst_hpd_irq_step2() and might trigger
drm_dp_mst_kick_tx() only when there is no on going message transaction.
Changes since v1:
* Reworked on review comments received
-> Adjust the fix to let driver explicitly kick off new down request
when mst irq event is handled and acked
-> Adjust the commit message
Signed-off-by: Wayne Lin
Cc: sta...@vger.kernel.org
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 35 ---
drivers/gpu/drm/i915/display/intel_dp.c | 5 ++-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 5 ++-
include/drm/display/drm_dp_mst_helper.h | 4 +--
5 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1ad67c2a697e..48bdcb2ee9b1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3259,10 +3259,9 @@ static void dm_handle_mst_sideband_msg(struct
amdgpu_dm_connector *aconnector)
DRM_DEBUG_DRIVER("ESI %02x %02x %02x\n", esi[0], esi[1],
esi[2]);
/* handle HPD short pulse irq */
if (aconnector->mst_mgr.mst_state)
- drm_dp_mst_hpd_irq(
- &aconnector->mst_mgr,
- esi,
- &new_irq_handled);
+ drm_dp_mst_hpd_irq_step1(&aconnector->mst_mgr,
+esi,
+&new_irq_handled);
if (new_irq_handled) {
/* ACK at DPCD to notify down stream */
@@ -3281,6 +3280,7 @@ static void dm_handle_mst_sideband_msg(struct
amdgpu_dm_connector *aconnector)
break;
}
+ drm_dp_mst_hpd_irq_step2(&aconnector->mst_mgr);
/* check if there is new irq to be handled */
dret = drm_dp_dpcd_read(
&aconnector->dm_dp_aux.aux,
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 70df29fe92db..2e0a38a6509c 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -4045,7 +4045,7 @@ static int drm_dp_mst_handle_up_req(struct
drm_dp_mst_topology_mgr *mgr)
}
/**
- * drm_dp_mst_hpd_irq() - MST hotplug IRQ notify
+ * drm_dp_mst_hpd_irq_step1() - MST hotplug IRQ notify
* @mgr: manager to notify irq for.
* @esi: 4 bytes from SINK_COUNT_ESI
* @handled: whether the hpd interrupt was consumed or not
@@ -4055,7 +4055,7 @@ static int drm_dp_mst_handle_up_req(struct
drm_dp_mst_topology_mgr *mgr)
* topology manager will process the sideband messages received as a result
* of this.
*/
-int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool
*handled)
+int drm_dp_mst_hpd_irq_step1(struct drm_dp_mst_topology_mgr *mgr, u8 *esi,
bool *handled)
{
int ret = 0;
int sc;
@@ -4077,11 +4077,38 @@ int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr
*mgr, u8 *esi, bool *handl
*handled = true;
}
- drm_dp_mst_kick_tx(mgr);
return ret;
}
-EXPORT_SYMBOL(drm_dp_mst_hpd_irq);
+EXPORT_SYMBOL(drm_dp_mst_hpd_irq_step1);
+
+/**
+ * drm_dp_mst_hpd_irq_step2() - MST hotplug IRQ 2nd part handling
+ * @mgr: manager to notify irq for.
+ *
+ * This should be called from the driver when mst irq event is handled
+ * and acked. Note that new down request should only be sent when
+ * previous message transaction is done. Source is not suppo
[PATCH] drm/dp_mst: Clear MSG_RDY flag before sending new message
[Why & How]
The sequence for collecting down_reply/up_request from source
perspective should be:
Request_n->repeat (get partial reply of Request_n->clear message ready
flag to ack DPRX that the message is received) till all partial
replies for Request_n are received->new Request_n+1.
While assembling partial reply packets, reading out DPCD DOWN_REP
Sideband MSG buffer + clearing DOWN_REP_MSG_RDY flag should be
wrapped up as a complete operation for reading out a reply packet.
Kicking off a new request before clearing DOWN_REP_MSG_RDY flag might
be risky. e.g. If the reply of the new request has overwritten the
DPRX DOWN_REP Sideband MSG buffer before source writing ack to clear
DOWN_REP_MSG_RDY flag, source then unintentionally flushes the reply
for the new request. Should handle the up request in the same way.
In drm_dp_mst_hpd_irq(), we don't clear MSG_RDY flag before caliing
drm_dp_mst_kick_tx(). Fix that.
Signed-off-by: Wayne Lin
Cc: sta...@vger.kernel.org
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
drivers/gpu/drm/display/drm_dp_mst_topology.c | 22 +++
drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 ++
4 files changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 77277d90b6e2..5313a5656598 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3166,6 +3166,8 @@ static void dm_handle_mst_sideband_msg(struct
amdgpu_dm_connector *aconnector)
for (retry = 0; retry < 3; retry++) {
uint8_t wret;
+ /* MSG_RDY ack is done in drm*/
+ esi[1] &= ~(DP_DOWN_REP_MSG_RDY |
DP_UP_REQ_MSG_RDY);
wret = drm_dp_dpcd_write(
&aconnector->dm_dp_aux.aux,
dpcd_addr + 1,
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 51a46689cda7..02aad713c67c 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -4054,6 +4054,9 @@ int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr
*mgr, u8 *esi, bool *handl
{
int ret = 0;
int sc;
+ const int tosend = 1;
+ int retries = 0;
+ u8 buf = 0;
*handled = false;
sc = DP_GET_SINK_COUNT(esi[0]);
@@ -4072,6 +4075,25 @@ int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr
*mgr, u8 *esi, bool *handl
*handled = true;
}
+ if (*handled) {
+ buf = esi[1] & (DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
+ do {
+ ret = drm_dp_dpcd_write(mgr->aux,
+
DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0,
+ &buf,
+ tosend);
+
+ if (ret == tosend)
+ break;
+
+ retries++;
+ } while (retries < 5);
+
+ if (ret != tosend)
+ drm_dbg_kms(mgr->dev, "failed to write dpcd 0x%x\n",
+ DP_DEVICE_SERVICE_IRQ_VECTOR_ESI0);
+ }
+
drm_dp_mst_kick_tx(mgr);
return ret;
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index bf80f296a8fd..abec3de38b66 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3939,6 +3939,9 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
if (!memchr_inv(ack, 0, sizeof(ack)))
break;
+ /* MSG_RDY ack is done in drm*/
+ ack[1] &= ~(DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
+
if (!intel_dp_ack_sink_irq_esi(intel_dp, ack))
drm_dbg_kms(&i915->drm, "Failed to ack ESI\n");
}
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index edcb2529b402..e905987104ed 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1336,6 +1336,8 @@ nv50_mstm_service(struct nouveau_drm *drm,
if (!handled)
break;
+ /* MSG_RDY ack is done in drm*/
+ esi[1] &= ~(DP_DOWN_REP_MSG_RDY | DP_UP_REQ_MSG_RDY);
rc = drm_dp_dpcd_write(aux, DP_SINK_COUNT_ESI + 1, &esi[1],
3);
if (rc != 3) {
--
2.37.3
[PATCH] Revert "drm/display/dp_mst: Move all payload info into the atomic state"
This reverts commit 4d07b0bc403403438d9cf88450506240c5faf92f.
[Why]
Changes cause regression on amdgpu mst.
E.g.
In fill_dc_mst_payload_table_from_drm(), amdgpu expects to add/remove payload
one by one and call fill_dc_mst_payload_table_from_drm() to update the HW
maintained payload table. But previous change tries to go through all the
payloads in mst_state and update amdpug hw maintained table in once everytime
driver only tries to add/remove a specific payload stream only. The newly
design idea conflicts with the implementation in amdgpu nowadays.
[How]
Revert this patch first. After addressing all regression problems caused by
this previous patch, will add it back and adjust it.
Signed-off-by: Wayne Lin
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2171
Cc: sta...@vger.kernel.org # 6.1
Cc: Lyude Paul
Cc: Harry Wentland
Cc: Mario Limonciello
Cc: Ville Syrjälä
Cc: Ben Skeggs
Cc: Stanislav Lisovskiy
Cc: Fangzhi Zuo
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 53 +-
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 106 ++-
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 87 ++-
.../amd/display/include/link_service_types.h | 3 -
drivers/gpu/drm/display/drm_dp_mst_topology.c | 724 --
drivers/gpu/drm/i915/display/intel_dp_mst.c | 67 +-
drivers/gpu/drm/i915/display/intel_hdcp.c | 24 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 167 ++--
include/drm/display/drm_dp_mst_helper.h | 177 +++--
9 files changed, 878 insertions(+), 530 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 77277d90b6e2..674f5dc1102b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6548,7 +6548,6 @@ static int dm_encoder_helper_atomic_check(struct
drm_encoder *encoder,
const struct drm_display_mode *adjusted_mode =
&crtc_state->adjusted_mode;
struct drm_dp_mst_topology_mgr *mst_mgr;
struct drm_dp_mst_port *mst_port;
- struct drm_dp_mst_topology_state *mst_state;
enum dc_color_depth color_depth;
int clock, bpp = 0;
bool is_y420 = false;
@@ -6562,13 +6561,6 @@ static int dm_encoder_helper_atomic_check(struct
drm_encoder *encoder,
if (!crtc_state->connectors_changed && !crtc_state->mode_changed)
return 0;
- mst_state = drm_atomic_get_mst_topology_state(state, mst_mgr);
- if (IS_ERR(mst_state))
- return PTR_ERR(mst_state);
-
- if (!mst_state->pbn_div)
- mst_state->pbn_div =
dm_mst_get_pbn_divider(aconnector->mst_port->dc_link);
-
if (!state->duplicated) {
int max_bpc = conn_state->max_requested_bpc;
is_y420 = drm_mode_is_420_also(&connector->display_info,
adjusted_mode) &&
@@ -6580,10 +6572,11 @@ static int dm_encoder_helper_atomic_check(struct
drm_encoder *encoder,
clock = adjusted_mode->clock;
dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp,
false);
}
-
- dm_new_connector_state->vcpi_slots =
- drm_dp_atomic_find_time_slots(state, mst_mgr, mst_port,
- dm_new_connector_state->pbn);
+ dm_new_connector_state->vcpi_slots =
drm_dp_atomic_find_time_slots(state,
+
mst_mgr,
+
mst_port,
+
dm_new_connector_state->pbn,
+
dm_mst_get_pbn_divider(aconnector->dc_link));
if (dm_new_connector_state->vcpi_slots < 0) {
DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n",
(int)dm_new_connector_state->vcpi_slots);
return dm_new_connector_state->vcpi_slots;
@@ -6654,14 +6647,17 @@ static int dm_update_mst_vcpi_slots_for_dsc(struct
drm_atomic_state *state,
dm_conn_state->vcpi_slots = slot_num;
ret = drm_dp_mst_atomic_enable_dsc(state,
aconnector->port,
- dm_conn_state->pbn,
false);
+ dm_conn_state->pbn,
0, false);
if (ret < 0)
return ret;
continue;
}
- vcpi = drm_dp_mst_atomic_enable_dsc(state, aconnector->port,
pbn, true);
+ vcpi = drm_dp_mst_atomic_enable_dsc(state,
+ aconnector->port,
+ pbn, pbn_div,
+
[PATCH] drm/dp_mst: Lower down debug info level when receive NAK
[Why]
It's reasonable that we receive NAK while doing DP_REMOTE_DPCD_READ.
Downstream device might reply NAK with the reason and source should
react accordingly.
e.g.
1. When downstream device can't handle corresponding message in time,
it then replies NAK as reason been set as DEFER.
2. When multi-function branch-sink device doesn't enumerate virtual
DP peer devices for those multi-function down facing ports. Without
virtual DPCD, branch device might reply NAK with reason as BAD_PARAM
indicating this port can't do aux DPCD read.
It's expected result. Not an error.
[How]
Use drm_dbg_kms() to replace drm_err() when receive NAK.
Changes since v1:
* drm_dp_mst_topology.c file path changed. Folder was rename from
'dp' to 'display'
Signed-off-by: Wayne Lin
Reviewed-by: Harry Wentland
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 8526aae75c6d..f27aa0b95bea 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3557,9 +3557,8 @@ static int drm_dp_send_dpcd_read(struct
drm_dp_mst_topology_mgr *mgr,
if (ret < 0)
goto fail_free;
- /* DPCD read should never be NACKed */
if (txmsg->reply.reply_type == 1) {
- drm_err(mgr->dev, "mstb %p port %d: DPCD read on addr 0x%x for
%d bytes NAKed\n",
+ drm_dbg_kms(mgr->dev, "mstb %p port %d: DPCD read on addr 0x%x
for %d bytes NAKed\n",
mstb, port->port_num, offset, size);
ret = -EIO;
goto fail_free;
--
2.36.0
[PATCH] drm/dp_mst: Lower down debug info level when receive NAK
[Why]
It's reasonable that we receive NAK while doing DP_REMOTE_DPCD_READ.
Downstream device might reply NAK with the reason and source should
react accordingly.
e.g.
1. When downstream device can't handle corresponding message in time,
it then replies NAK as reason been set as DEFER.
2. When multi-function branch-sink device doesn't enumerate virtual
DP peer devices for those multi-function down facing ports. Without
virtual DPCD, branch device might reply NAK with reason as BAD_PARAM
indicating this port can't do aux DPCD read.
It's expected result. Not an error.
[How]
Use drm_dbg_kms() to replace drm_err() when receive NAK.
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/dp/drm_dp_mst_topology.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/dp/drm_dp_mst_topology.c
b/drivers/gpu/drm/dp/drm_dp_mst_topology.c
index 11300b53d24f..764a6b59bc1e 100644
--- a/drivers/gpu/drm/dp/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/dp/drm_dp_mst_topology.c
@@ -3557,9 +3557,8 @@ static int drm_dp_send_dpcd_read(struct
drm_dp_mst_topology_mgr *mgr,
if (ret < 0)
goto fail_free;
- /* DPCD read should never be NACKed */
if (txmsg->reply.reply_type == 1) {
- drm_err(mgr->dev, "mstb %p port %d: DPCD read on addr 0x%x for
%d bytes NAKed\n",
+ drm_dbg_kms(mgr->dev, "mstb %p port %d: DPCD read on addr 0x%x
for %d bytes NAKed\n",
mstb, port->port_num, offset, size);
ret = -EIO;
goto fail_free;
--
2.35.1
[PATCH 0/4] Unregister mst connectors when hotplug
By patches below, we change to clean up resources of a mst connector and put port's malloc_kref when we start to destroy a connector. * 09b974e8983 drm/amd/amdgpu_dm/mst: Remove ->destroy_connector() callback * 72dc0f51591 drm/dp_mst: Remove drm_dp_mst_topology_cbs.destroy_connector However, find out that mst connectors won't reach connector destroy flow after hot unplug and hence will cause no more resources for new added connectors. Thus, this patch set is trying to solve observed registration/unregistration problem of mst connectors. Wayne Lin (4): drm/dp_mst: Put malloc_kref of vcpi pointing port when disable MST drm/dp_mst: Only create connector for connected end device drm/dp_mst: Put connector of disconnected end device when handling CSN drm/dp_mst: Release disconnected connectors when resume drivers/gpu/drm/drm_dp_mst_topology.c | 39 --- 1 file changed, 36 insertions(+), 3 deletions(-) -- 2.17.1
[PATCH 2/4] drm/dp_mst: Only create connector for connected end device
[Why]
Currently, we will create connectors for all output ports no matter
it's connected or not. However, in MST, we can only determine
whether an output port really stands for a "connector" till it is
connected and check its peer device type as an end device.
In current code, we have chance to create connectors for output ports
connected with branch device and these are redundant connectors. e.g.
StarTech 1-to-4 DP hub is constructed by internal 2 layer 1-to-2 branch
devices. Creating connectors for such internal output ports are
redundant.
[How]
Put constraint on creating connector for connected end device only.
Fixes: 6f85f73821f6 ("drm/dp_mst: Add basic topology reprobing when resuming")
Cc: Juston Li
Cc: Imre Deak
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Daniel Vetter
Cc: Sean Paul
Cc: Lyude Paul
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: David Airlie
Cc: Daniel Vetter
Cc: Alex Deucher
Cc: Nicholas Kazlauskas
Cc: Rodrigo Siqueira
Cc: Aurabindo Pillai
Cc: Eryk Brol
Cc: Bas Nieuwenhuizen
Cc: Nikola Cornij
Cc: Wayne Lin
Cc: "Ville Syrjälä"
Cc: Jani Nikula
Cc: Manasi Navare
Cc: Ankit Nautiyal
Cc: "José Roberto de Souza"
Cc: Sean Paul
Cc: Ben Skeggs
Cc: dri-devel@lists.freedesktop.org
Cc: # v5.5+
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 51cd7f74f026..f13c7187b07f 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2474,7 +2474,8 @@ drm_dp_mst_handle_link_address_port(struct
drm_dp_mst_branch *mstb,
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
- else if (!port->input)
+ else if (!port->input && port->pdt != DP_PEER_DEVICE_NONE &&
+drm_dp_mst_is_end_device(port->pdt, port->mcs))
drm_dp_mst_port_add_connector(mstb, port);
if (send_link_addr && port->mstb) {
@@ -2557,6 +2558,10 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
dowork = false;
}
+ if (!port->input && !port->connector && new_pdt != DP_PEER_DEVICE_NONE
&&
+ drm_dp_mst_is_end_device(new_pdt, new_mcs))
+ create_connector = true;
+
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
else if (create_connector)
--
2.17.1
[PATCH 3/4] drm/dp_mst: Put connector of disconnected end device when handling CSN
[Why]
Now, after receiving CSN notifying that a port is disconnected, we can
unregister child connectors under a branch device via
drm_dp_mst_topology_put_mstb() in drm_dp_port_set_pdt(). However, if
this reported disconnected port is used to connect to an end
device(sst/dp_to_legace converter), we won't unregiser such connector.
[How]
Take sst/dp_to_legacy conveter device into consideration, also
unregister connectors of this case when handling CSN.
In addition, check whether port->connector exist to avoid
null pointer dereference.
Cc: sta...@vger.kernel.org
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index f13c7187b07f..85a959427247 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2567,6 +2567,12 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
else if (create_connector)
drm_dp_mst_port_add_connector(mstb, port);
+ if (port->connector && port->pdt == DP_PEER_DEVICE_NONE) {
+ drm_connector_unregister(port->connector);
+ drm_connector_put(port->connector);
+ port->connector = NULL;
+ }
+
out:
drm_dp_mst_topology_put_port(port);
if (dowork)
@@ -4442,10 +4448,12 @@ int drm_dp_atomic_find_vcpi_slots(struct
drm_atomic_state *state,
req_slots = DIV_ROUND_UP(pbn, pbn_div);
drm_dbg_atomic(mgr->dev, "[CONNECTOR:%d:%s] [MST PORT:%p] VCPI %d ->
%d\n",
- port->connector->base.id, port->connector->name,
+ port->connector ? port->connector->base.id : 0,
+ port->connector ? port->connector->name : "NULL",
port, prev_slots, req_slots);
drm_dbg_atomic(mgr->dev, "[CONNECTOR:%d:%s] [MST PORT:%p] PBN %d ->
%d\n",
- port->connector->base.id, port->connector->name,
+ port->connector ? port->connector->base.id : 0,
+ port->connector ? port->connector->name : "NULL",
port, prev_bw, pbn);
/* Add the new allocation to the state */
--
2.17.1
[PATCH 4/4] drm/dp_mst: Release disconnected connectors when resume
[why]
When resume, we will reprobe the topology to detect any changes during
suspend. If we unplug a branch device during suspend and then resume, we
can eventually unregister child connectors of this branch device because
we call drm_dp_mst_topology_put_mstb() in drm_dp_port_set_pdt().
However, we don't unregister connectors for end devices which is
disconnected during suspend. e.g. Unplug a SST monitor during
suspend then resume. We won't unregister this connector which is no
longer exist in the topology.
[How]
Unregister connectors for disconnected end devices when resume.
Cc: sta...@vger.kernel.org
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 85a959427247..0b04ea65cb8e 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2478,6 +2478,12 @@ drm_dp_mst_handle_link_address_port(struct
drm_dp_mst_branch *mstb,
drm_dp_mst_is_end_device(port->pdt, port->mcs))
drm_dp_mst_port_add_connector(mstb, port);
+ if (port->connector && port->pdt == DP_PEER_DEVICE_NONE) {
+ drm_connector_unregister(port->connector);
+ drm_connector_put(port->connector);
+ port->connector = NULL;
+ }
+
if (send_link_addr && port->mstb) {
ret = drm_dp_send_link_address(mgr, port->mstb);
if (ret == 1) /* MSTB below us changed */
--
2.17.1
[PATCH 1/4] drm/dp_mst: Put malloc_kref of vcpi pointing port when disable MST
[Why]
We directlly clean up proposed_vcpis[] but forget to put malloc_kref of
ports which the proposed_vcpis[] are pointing to.
[How]
Iterate over proposed_vcpis[] and put malloc_kref of ports which used to
have allocated payloads.
Fixes: 8732fe46b20c ("drm/dp_mst: Fix clearing payload state on topology
disable")
Cc: Sean Paul
Cc: Wayne Lin
Cc: Ville Syrjälä
Cc: sta...@vger.kernel.org # v4.4+
Cc: Lyude Paul
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: David Airlie
Cc: Daniel Vetter
Cc: dri-devel@lists.freedesktop.org
Cc: # v5.7+
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index ad0795afc21c..51cd7f74f026 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3765,12 +3765,26 @@ int drm_dp_mst_topology_mgr_set_mst(struct
drm_dp_mst_topology_mgr *mgr, bool ms
ret = 0;
} else {
+ int i;
+ struct drm_dp_vcpi *vcpi;
+ struct drm_dp_mst_port *port;
/* disable MST on the device */
mstb = mgr->mst_primary;
mgr->mst_primary = NULL;
/* this can fail if the device is gone */
drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, 0);
ret = 0;
+
+ for (i = 0; i < mgr->max_payloads; i++) {
+ vcpi = mgr->proposed_vcpis[i];
+ if (vcpi) {
+ port = container_of(vcpi, struct
drm_dp_mst_port,
+ vcpi);
+ if (port)
+ drm_dp_mst_put_port_malloc(port);
+ }
+ }
+
memset(mgr->payloads, 0,
mgr->max_payloads * sizeof(mgr->payloads[0]));
memset(mgr->proposed_vcpis, 0,
--
2.17.1
[PATCH 3/3] drm/dp_mst: Add missing drm parameters to recently added call to drm_dbg_kms()
From: José Roberto de Souza
Commit 3769e4c0af5b ("drm/dp_mst: Avoid to mess up payload table by
ports in stale topology") added to calls to drm_dbg_kms() but it
missed the first parameter, the drm device breaking the build.
Fixes: 3769e4c0af5b ("drm/dp_mst: Avoid to mess up payload table by ports in
stale topology")
Cc: Wayne Lin
Cc: Lyude Paul
Cc: dri-devel@lists.freedesktop.org
Cc: sta...@vger.kernel.org
Signed-off-by: José Roberto de Souza
Reviewed-by: Lyude Paul
Signed-off-by: Lyude Paul
Link:
https://patchwork.freedesktop.org/patch/msgid/20210616194415.36926-1-jose.so...@intel.com
---
drivers/gpu/drm/drm_dp_mst_topology.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index bf6fa98b4ab7..a68dc25a19c6 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3383,7 +3383,9 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
mutex_unlock(&mgr->lock);
if (skip) {
- drm_dbg_kms("Virtual channel %d is not in
current topology\n", i);
+ drm_dbg_kms(mgr->dev,
+ "Virtual channel %d is not in
current topology\n",
+ i);
continue;
}
/* Validated ports don't matter if we're releasing
@@ -3398,7 +3400,8 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
payload->start_slot =
req_payload.start_slot;
continue;
} else {
- drm_dbg_kms("Fail:set payload
to invalid sink");
+ drm_dbg_kms(mgr->dev,
+ "Fail:set payload
to invalid sink");
mutex_unlock(&mgr->payload_lock);
return -EINVAL;
}
--
2.17.1
[PATCH 1/3] drm/dp_mst: Do not set proposed vcpi directly
[Why]
When we receive CSN message to notify one port is disconnected, we will
implicitly set its corresponding num_slots to 0. Later on, we will
eventually call drm_dp_update_payload_part1() to arrange down streams.
In drm_dp_update_payload_part1(), we iterate over all proposed_vcpis[]
to do the update. Not specific to a target sink only. For example, if we
light up 2 monitors, Monitor_A and Monitor_B, and then we unplug
Monitor_B. Later on, when we call drm_dp_update_payload_part1() to try
to update payload for Monitor_A, we'll also implicitly clean payload for
Monitor_B at the same time. And finally, when we try to call
drm_dp_update_payload_part1() to clean payload for Monitor_B, we will do
nothing at this time since payload for Monitor_B has been cleaned up
previously.
For StarTech 1to3 DP hub, it seems like if we didn't update DPCD payload
ID table then polling for "ACT Handled"(BIT_1 of DPCD 002C0h) will fail
and this polling will last for 3 seconds.
Therefore, guess the best way is we don't set the proposed_vcpi[]
diretly. Let user of these herlper functions to set the proposed_vcpi
directly.
[How]
1. Revert commit 7617e9621bf2 ("drm/dp_mst: clear time slots for ports
invalid")
2. Tackle the issue in previous commit by skipping those trasient
proposed VCPIs. These stale VCPIs shoulde be explicitly cleared by
user later on.
Changes since v1:
* Change debug macro to use drm_dbg_kms() instead
* Amend the commit message to add Fixed & Cc tags
Signed-off-by: Wayne Lin
Fixes: 7617e9621bf2 ("drm/dp_mst: clear time slots for ports invalid")
Cc: Lyude Paul
Cc: Wayne Lin
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Cc: # v5.5+
Signed-off-by: Lyude Paul
Link:
https://patchwork.freedesktop.org/patch/msgid/20210616035501.3776-2-wayne@amd.com
Reviewed-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 36 ---
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 159014455fab..2495e2c014ac 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2497,7 +2497,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
- int old_ddps, old_input, ret, i;
+ int old_ddps, ret;
u8 new_pdt;
bool new_mcs;
bool dowork = false, create_connector = false;
@@ -2529,7 +2529,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
}
old_ddps = port->ddps;
- old_input = port->input;
port->input = conn_stat->input_port;
port->ldps = conn_stat->legacy_device_plug_status;
port->ddps = conn_stat->displayport_device_plug_status;
@@ -2552,28 +2551,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
dowork = false;
}
- if (!old_input && old_ddps != port->ddps && !port->ddps) {
- for (i = 0; i < mgr->max_payloads; i++) {
- struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
- struct drm_dp_mst_port *port_validated;
-
- if (!vcpi)
- continue;
-
- port_validated =
- container_of(vcpi, struct drm_dp_mst_port,
vcpi);
- port_validated =
- drm_dp_mst_topology_get_port_validated(mgr,
port_validated);
- if (!port_validated) {
- mutex_lock(&mgr->payload_lock);
- vcpi->num_slots = 0;
- mutex_unlock(&mgr->payload_lock);
- } else {
- drm_dp_mst_topology_put_port(port_validated);
- }
- }
- }
-
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
else if (create_connector)
@@ -3404,8 +3381,15 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
port = drm_dp_mst_topology_get_port_validated(
mgr, port);
if (!port) {
- mutex_unlock(&mgr->payload_lock);
- return -EINVAL;
+ if (vcpi->num_slots ==
payload->num_slots) {
+ cur_slots += vcpi->num_slots;
+ payload->start_slot =
req_payload.start_slot;
+
[PATCH 3/3] drm/dp_mst: Add missing drm parameters to recently added call to drm_dbg_kms()
From: José Roberto de Souza
Commit 3769e4c0af5b ("drm/dp_mst: Avoid to mess up payload table by
ports in stale topology") added to calls to drm_dbg_kms() but it
missed the first parameter, the drm device breaking the build.
Fixes: 3769e4c0af5b ("drm/dp_mst: Avoid to mess up payload table by ports in
stale topology")
Cc: Wayne Lin
Cc: Lyude Paul
Cc: dri-devel@lists.freedesktop.org
Cc: sta...@vger.kernel.org
Signed-off-by: José Roberto de Souza
Reviewed-by: Lyude Paul
Signed-off-by: Lyude Paul
Link:
https://patchwork.freedesktop.org/patch/msgid/20210616194415.36926-1-jose.so...@intel.com
---
drivers/gpu/drm/drm_dp_mst_topology.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index b86a2b7fef39..dfbd90431043 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3385,7 +3385,9 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
mutex_unlock(&mgr->lock);
if (skip) {
- drm_dbg_kms("Virtual channel %d is not in
current topology\n", i);
+ drm_dbg_kms(mgr->dev,
+ "Virtual channel %d is not in
current topology\n",
+ i);
continue;
}
/* Validated ports don't matter if we're releasing
@@ -3400,7 +3402,8 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
payload->start_slot =
req_payload.start_slot;
continue;
} else {
- drm_dbg_kms("Fail:set payload
to invalid sink");
+ drm_dbg_kms(mgr->dev,
+ "Fail:set payload
to invalid sink");
mutex_unlock(&mgr->payload_lock);
return -EINVAL;
}
--
2.17.1
[PATCH 1/3] drm/dp_mst: Do not set proposed vcpi directly
[Why]
When we receive CSN message to notify one port is disconnected, we will
implicitly set its corresponding num_slots to 0. Later on, we will
eventually call drm_dp_update_payload_part1() to arrange down streams.
In drm_dp_update_payload_part1(), we iterate over all proposed_vcpis[]
to do the update. Not specific to a target sink only. For example, if we
light up 2 monitors, Monitor_A and Monitor_B, and then we unplug
Monitor_B. Later on, when we call drm_dp_update_payload_part1() to try
to update payload for Monitor_A, we'll also implicitly clean payload for
Monitor_B at the same time. And finally, when we try to call
drm_dp_update_payload_part1() to clean payload for Monitor_B, we will do
nothing at this time since payload for Monitor_B has been cleaned up
previously.
For StarTech 1to3 DP hub, it seems like if we didn't update DPCD payload
ID table then polling for "ACT Handled"(BIT_1 of DPCD 002C0h) will fail
and this polling will last for 3 seconds.
Therefore, guess the best way is we don't set the proposed_vcpi[]
diretly. Let user of these herlper functions to set the proposed_vcpi
directly.
[How]
1. Revert commit 7617e9621bf2 ("drm/dp_mst: clear time slots for ports
invalid")
2. Tackle the issue in previous commit by skipping those trasient
proposed VCPIs. These stale VCPIs shoulde be explicitly cleared by
user later on.
Changes since v1:
* Change debug macro to use drm_dbg_kms() instead
* Amend the commit message to add Fixed & Cc tags
Signed-off-by: Wayne Lin
Fixes: 7617e9621bf2 ("drm/dp_mst: clear time slots for ports invalid")
Cc: Lyude Paul
Cc: Wayne Lin
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Cc: # v5.5+
Signed-off-by: Lyude Paul
Link:
https://patchwork.freedesktop.org/patch/msgid/20210616035501.3776-2-wayne@amd.com
Reviewed-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 36 ---
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 9c75c8815056..41d790df81bb 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2499,7 +2499,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
- int old_ddps, old_input, ret, i;
+ int old_ddps, ret;
u8 new_pdt;
bool new_mcs;
bool dowork = false, create_connector = false;
@@ -2531,7 +2531,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
}
old_ddps = port->ddps;
- old_input = port->input;
port->input = conn_stat->input_port;
port->ldps = conn_stat->legacy_device_plug_status;
port->ddps = conn_stat->displayport_device_plug_status;
@@ -2554,28 +2553,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
dowork = false;
}
- if (!old_input && old_ddps != port->ddps && !port->ddps) {
- for (i = 0; i < mgr->max_payloads; i++) {
- struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
- struct drm_dp_mst_port *port_validated;
-
- if (!vcpi)
- continue;
-
- port_validated =
- container_of(vcpi, struct drm_dp_mst_port,
vcpi);
- port_validated =
- drm_dp_mst_topology_get_port_validated(mgr,
port_validated);
- if (!port_validated) {
- mutex_lock(&mgr->payload_lock);
- vcpi->num_slots = 0;
- mutex_unlock(&mgr->payload_lock);
- } else {
- drm_dp_mst_topology_put_port(port_validated);
- }
- }
- }
-
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
else if (create_connector)
@@ -3406,8 +3383,15 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
port = drm_dp_mst_topology_get_port_validated(
mgr, port);
if (!port) {
- mutex_unlock(&mgr->payload_lock);
- return -EINVAL;
+ if (vcpi->num_slots ==
payload->num_slots) {
+ cur_slots += vcpi->num_slots;
+ payload->start_slot =
req_payload.start_slot;
+
[PATCH 3/3] drm/dp_mst: Add missing drm parameters to recently added call to drm_dbg_kms()
From: José Roberto de Souza
Commit 3769e4c0af5b ("drm/dp_mst: Avoid to mess up payload table by
ports in stale topology") added to calls to drm_dbg_kms() but it
missed the first parameter, the drm device breaking the build.
Fixes: 3769e4c0af5b ("drm/dp_mst: Avoid to mess up payload table by ports in
stale topology")
Cc: Wayne Lin
Cc: Lyude Paul
Cc: dri-devel@lists.freedesktop.org
Cc: sta...@vger.kernel.org
Signed-off-by: José Roberto de Souza
Reviewed-by: Lyude Paul
Signed-off-by: Lyude Paul
Link:
https://patchwork.freedesktop.org/patch/msgid/20210616194415.36926-1-jose.so...@intel.com
---
drivers/gpu/drm/drm_dp_mst_topology.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 01570f2549e1..861f16dfd1a3 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3385,7 +3385,9 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
mutex_unlock(&mgr->lock);
if (skip) {
- drm_dbg_kms("Virtual channel %d is not in
current topology\n", i);
+ drm_dbg_kms(mgr->dev,
+ "Virtual channel %d is not in
current topology\n",
+ i);
continue;
}
/* Validated ports don't matter if we're releasing
@@ -3400,7 +3402,8 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
payload->start_slot =
req_payload.start_slot;
continue;
} else {
- drm_dbg_kms("Fail:set payload
to invalid sink");
+ drm_dbg_kms(mgr->dev,
+ "Fail:set payload
to invalid sink");
mutex_unlock(&mgr->payload_lock);
return -EINVAL;
}
--
2.17.1
[PATCH 1/3] drm/dp_mst: Do not set proposed vcpi directly
[Why]
When we receive CSN message to notify one port is disconnected, we will
implicitly set its corresponding num_slots to 0. Later on, we will
eventually call drm_dp_update_payload_part1() to arrange down streams.
In drm_dp_update_payload_part1(), we iterate over all proposed_vcpis[]
to do the update. Not specific to a target sink only. For example, if we
light up 2 monitors, Monitor_A and Monitor_B, and then we unplug
Monitor_B. Later on, when we call drm_dp_update_payload_part1() to try
to update payload for Monitor_A, we'll also implicitly clean payload for
Monitor_B at the same time. And finally, when we try to call
drm_dp_update_payload_part1() to clean payload for Monitor_B, we will do
nothing at this time since payload for Monitor_B has been cleaned up
previously.
For StarTech 1to3 DP hub, it seems like if we didn't update DPCD payload
ID table then polling for "ACT Handled"(BIT_1 of DPCD 002C0h) will fail
and this polling will last for 3 seconds.
Therefore, guess the best way is we don't set the proposed_vcpi[]
diretly. Let user of these herlper functions to set the proposed_vcpi
directly.
[How]
1. Revert commit 7617e9621bf2 ("drm/dp_mst: clear time slots for ports
invalid")
2. Tackle the issue in previous commit by skipping those trasient
proposed VCPIs. These stale VCPIs shoulde be explicitly cleared by
user later on.
Changes since v1:
* Change debug macro to use drm_dbg_kms() instead
* Amend the commit message to add Fixed & Cc tags
Signed-off-by: Wayne Lin
Fixes: 7617e9621bf2 ("drm/dp_mst: clear time slots for ports invalid")
Cc: Lyude Paul
Cc: Wayne Lin
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Cc: # v5.5+
Signed-off-by: Lyude Paul
Link:
https://patchwork.freedesktop.org/patch/msgid/20210616035501.3776-2-wayne@amd.com
Reviewed-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 36 ---
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index a08cc6b53bc2..702aa0711f80 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2499,7 +2499,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
- int old_ddps, old_input, ret, i;
+ int old_ddps, ret;
u8 new_pdt;
bool new_mcs;
bool dowork = false, create_connector = false;
@@ -2531,7 +2531,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
}
old_ddps = port->ddps;
- old_input = port->input;
port->input = conn_stat->input_port;
port->ldps = conn_stat->legacy_device_plug_status;
port->ddps = conn_stat->displayport_device_plug_status;
@@ -2554,28 +2553,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
dowork = false;
}
- if (!old_input && old_ddps != port->ddps && !port->ddps) {
- for (i = 0; i < mgr->max_payloads; i++) {
- struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
- struct drm_dp_mst_port *port_validated;
-
- if (!vcpi)
- continue;
-
- port_validated =
- container_of(vcpi, struct drm_dp_mst_port,
vcpi);
- port_validated =
- drm_dp_mst_topology_get_port_validated(mgr,
port_validated);
- if (!port_validated) {
- mutex_lock(&mgr->payload_lock);
- vcpi->num_slots = 0;
- mutex_unlock(&mgr->payload_lock);
- } else {
- drm_dp_mst_topology_put_port(port_validated);
- }
- }
- }
-
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
else if (create_connector)
@@ -3406,8 +3383,15 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
port = drm_dp_mst_topology_get_port_validated(
mgr, port);
if (!port) {
- mutex_unlock(&mgr->payload_lock);
- return -EINVAL;
+ if (vcpi->num_slots ==
payload->num_slots) {
+ cur_slots += vcpi->num_slots;
+ payload->start_slot =
req_payload.start_slot;
+
[PATCH v2 2/2] drm/dp_mst: Avoid to mess up payload table by ports in stale topology
[Why]
After unplug/hotplug hub from the system, userspace might start to
clear stale payloads gradually. If we call drm_dp_mst_deallocate_vcpi()
to release stale VCPI of those ports which are not relating to current
topology, we have chane to wrongly clear active payload table entry for
current topology.
E.g.
We have allocated VCPI 1 in current payload table and we call
drm_dp_mst_deallocate_vcpi() to clean VCPI 1 in stale topology. In
drm_dp_mst_deallocate_vcpi(), it will call drm_dp_mst_put_payload_id()
tp put VCPI 1 and which means ID 1 is available again. Thereafter, if we
want to allocate a new payload stream, it will find ID 1 is available by
drm_dp_mst_assign_payload_id(). However, ID 1 is being used
[How]
Check target sink is relating to current topology or not before doing
any payload table update.
Searching upward to find the target sink's relevant root branch device.
If the found root branch device is not the same root of current
topology, don't update payload table.
Changes since v1:
* Change debug macro to use drm_dbg_kms() instead
* Amend the commit message to add Cc tag.
Signed-off-by: Wayne Lin
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/drm_dp_mst_topology.c | 29 +++
1 file changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index b41b837db66d..9ac148efd9e4 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -94,6 +94,9 @@ static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port
*port);
static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port);
static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr);
+static bool drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port *port,
+struct drm_dp_mst_branch
*branch);
+
#define DBG_PREFIX "[dp_mst]"
#define DP_STR(x) [DP_ ## x] = #x
@@ -3366,6 +3369,7 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
struct drm_dp_mst_port *port;
int i, j;
int cur_slots = 1;
+ bool skip;
mutex_lock(&mgr->payload_lock);
for (i = 0; i < mgr->max_payloads; i++) {
@@ -3380,6 +3384,14 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
port = container_of(vcpi, struct drm_dp_mst_port,
vcpi);
+ mutex_lock(&mgr->lock);
+ skip = !drm_dp_mst_port_downstream_of_branch(port,
mgr->mst_primary);
+ mutex_unlock(&mgr->lock);
+
+ if (skip) {
+ drm_dbg_kms("Virtual channel %d is not in
current topology\n", i);
+ continue;
+ }
/* Validated ports don't matter if we're releasing
* VCPI
*/
@@ -3479,6 +3491,7 @@ int drm_dp_update_payload_part2(struct
drm_dp_mst_topology_mgr *mgr)
struct drm_dp_mst_port *port;
int i;
int ret = 0;
+ bool skip;
mutex_lock(&mgr->payload_lock);
for (i = 0; i < mgr->max_payloads; i++) {
@@ -3488,6 +3501,13 @@ int drm_dp_update_payload_part2(struct
drm_dp_mst_topology_mgr *mgr)
port = container_of(mgr->proposed_vcpis[i], struct
drm_dp_mst_port, vcpi);
+ mutex_lock(&mgr->lock);
+ skip = !drm_dp_mst_port_downstream_of_branch(port,
mgr->mst_primary);
+ mutex_unlock(&mgr->lock);
+
+ if (skip)
+ continue;
+
drm_dbg_kms(mgr->dev, "payload %d %d\n", i,
mgr->payloads[i].payload_state);
if (mgr->payloads[i].payload_state == DP_PAYLOAD_LOCAL) {
ret = drm_dp_create_payload_step2(mgr, port,
mgr->proposed_vcpis[i]->vcpi, &mgr->payloads[i]);
@@ -4574,9 +4594,18 @@ EXPORT_SYMBOL(drm_dp_mst_reset_vcpi_slots);
void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port)
{
+ bool skip;
+
if (!port->vcpi.vcpi)
return;
+ mutex_lock(&mgr->lock);
+ skip = !drm_dp_mst_port_downstream_of_branch(port, mgr->mst_primary);
+ mutex_unlock(&mgr->lock);
+
+ if (skip)
+ return;
+
drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
port->vcpi.num_slots = 0;
port->vcpi.pbn = 0;
--
2.17.1
[PATCH v2 0/2] Fix observed mst problems with StarTech hub
Use Startech 1to3 DP hub to do some mst hotplug tests and find some light up issues. 1. ACT polling timeout: Which is due to we didn't update DPCD payload table but still try to send ACT and polling for "ACT Handled" bit 2. Not all monitors light up: Due to we wrongly set unavailable VCP ID for new streams Changes since v1: * Add appropriate tags: Fixes & Cc * Change debug macro to use drm_dbg_kms() instead Wayne Lin (2): drm/dp_mst: Do not set proposed vcpi directly drm/dp_mst: Avoid to mess up payload table by ports in stale topology drivers/gpu/drm/drm_dp_mst_topology.c | 65 --- 1 file changed, 39 insertions(+), 26 deletions(-) -- 2.17.1
[PATCH v2 1/2] drm/dp_mst: Do not set proposed vcpi directly
[Why]
When we receive CSN message to notify one port is disconnected, we will
implicitly set its corresponding num_slots to 0. Later on, we will
eventually call drm_dp_update_payload_part1() to arrange down streams.
In drm_dp_update_payload_part1(), we iterate over all proposed_vcpis[]
to do the update. Not specific to a target sink only. For example, if we
light up 2 monitors, Monitor_A and Monitor_B, and then we unplug
Monitor_B. Later on, when we call drm_dp_update_payload_part1() to try
to update payload for Monitor_A, we'll also implicitly clean payload for
Monitor_B at the same time. And finally, when we try to call
drm_dp_update_payload_part1() to clean payload for Monitor_B, we will do
nothing at this time since payload for Monitor_B has been cleaned up
previously.
For StarTech 1to3 DP hub, it seems like if we didn't update DPCD payload
ID table then polling for "ACT Handled"(BIT_1 of DPCD 002C0h) will fail
and this polling will last for 3 seconds.
Therefore, guess the best way is we don't set the proposed_vcpi[]
diretly. Let user of these herlper functions to set the proposed_vcpi
directly.
[How]
1. Revert commit 7617e9621bf2 ("drm/dp_mst: clear time slots for ports
invalid")
2. Tackle the issue in previous commit by skipping those trasient
proposed VCPIs. These stale VCPIs shoulde be explicitly cleared by
user later on.
Changes since v1:
* Change debug macro to use drm_dbg_kms() instead
* Amend the commit message to add Fixed & Cc tags
Signed-off-by: Wayne Lin
Fixes: 7617e9621bf2 ("drm/dp_mst: clear time slots for ports invalid")
Cc: Lyude Paul
Cc: Wayne Lin
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Cc: # v5.5+
---
drivers/gpu/drm/drm_dp_mst_topology.c | 36 ---
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 32b7f8983b94..b41b837db66d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2501,7 +2501,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
- int old_ddps, old_input, ret, i;
+ int old_ddps, ret;
u8 new_pdt;
bool new_mcs;
bool dowork = false, create_connector = false;
@@ -2533,7 +2533,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
}
old_ddps = port->ddps;
- old_input = port->input;
port->input = conn_stat->input_port;
port->ldps = conn_stat->legacy_device_plug_status;
port->ddps = conn_stat->displayport_device_plug_status;
@@ -2555,28 +2554,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
dowork = false;
}
- if (!old_input && old_ddps != port->ddps && !port->ddps) {
- for (i = 0; i < mgr->max_payloads; i++) {
- struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
- struct drm_dp_mst_port *port_validated;
-
- if (!vcpi)
- continue;
-
- port_validated =
- container_of(vcpi, struct drm_dp_mst_port,
vcpi);
- port_validated =
- drm_dp_mst_topology_get_port_validated(mgr,
port_validated);
- if (!port_validated) {
- mutex_lock(&mgr->payload_lock);
- vcpi->num_slots = 0;
- mutex_unlock(&mgr->payload_lock);
- } else {
- drm_dp_mst_topology_put_port(port_validated);
- }
- }
- }
-
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
else if (create_connector)
@@ -3410,8 +3387,15 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
port = drm_dp_mst_topology_get_port_validated(
mgr, port);
if (!port) {
- mutex_unlock(&mgr->payload_lock);
- return -EINVAL;
+ if (vcpi->num_slots ==
payload->num_slots) {
+ cur_slots += vcpi->num_slots;
+ payload->start_slot =
req_payload.start_slot;
+ continue;
+ } else {
+ drm_dbg_kms("Fail:set payload
to inv
[PATCH 1/2] drm/dp_mst: Do not set proposed vcpi directly
[Why]
When we receive CSN message to notify one port is disconnected, we will
implicitly set its corresponding num_slots to 0. Later on, we will
eventually call drm_dp_update_payload_part1() to arrange down streams.
In drm_dp_update_payload_part1(), we iterate over all proposed_vcpis[]
to do the update. Not specific to a target sink only. For example, if we
light up 2 monitors, Monitor_A and Monitor_B, and then we unplug
Monitor_B. Later on, when we call drm_dp_update_payload_part1() to try
to update payload for Monitor_A, we'll also implicitly clean payload for
Monitor_B at the same time. And finally, when we try to call
drm_dp_update_payload_part1() to clean payload for Monitor_B, we will do
nothing at this time since payload for Monitor_B has been cleaned up
previously.
For StarTech 1to3 DP hub, it seems like if we didn't update DPCD payload
ID table then polling for "ACT Handled"(BIT_1 of DPCD 002C0h) will fail
and this polling will last for 3 seconds.
Therefore, guess the best way is we don't set the proposed_vcpi[]
diretly. Let user of these herlper functions to set the proposed_vcpi
directly.
[How]
1. Revert commit 7617e9621bf2 ("drm/dp_mst: clear time slots for ports
invalid")
2. Tackle the issue in previous commit by skipping those trasient
proposed VCPIs. These stale VCPIs shoulde be explicitly cleared by
user later on.
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 36 ---
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 8f5a008501d9..5fc261014a20 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2497,7 +2497,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
- int old_ddps, old_input, ret, i;
+ int old_ddps, ret;
u8 new_pdt;
bool new_mcs;
bool dowork = false, create_connector = false;
@@ -2529,7 +2529,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
}
old_ddps = port->ddps;
- old_input = port->input;
port->input = conn_stat->input_port;
port->ldps = conn_stat->legacy_device_plug_status;
port->ddps = conn_stat->displayport_device_plug_status;
@@ -2552,28 +2551,6 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
dowork = false;
}
- if (!old_input && old_ddps != port->ddps && !port->ddps) {
- for (i = 0; i < mgr->max_payloads; i++) {
- struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
- struct drm_dp_mst_port *port_validated;
-
- if (!vcpi)
- continue;
-
- port_validated =
- container_of(vcpi, struct drm_dp_mst_port,
vcpi);
- port_validated =
- drm_dp_mst_topology_get_port_validated(mgr,
port_validated);
- if (!port_validated) {
- mutex_lock(&mgr->payload_lock);
- vcpi->num_slots = 0;
- mutex_unlock(&mgr->payload_lock);
- } else {
- drm_dp_mst_topology_put_port(port_validated);
- }
- }
- }
-
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
else if (create_connector)
@@ -3404,8 +3381,15 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
port = drm_dp_mst_topology_get_port_validated(
mgr, port);
if (!port) {
- mutex_unlock(&mgr->payload_lock);
- return -EINVAL;
+ if (vcpi->num_slots ==
payload->num_slots) {
+ cur_slots += vcpi->num_slots;
+ payload->start_slot =
req_payload.start_slot;
+ continue;
+ } else {
+ DRM_DEBUG_KMS("Fail:set payload
to invalid sink");
+
mutex_unlock(&mgr->payload_lock);
+ return -EINVAL;
+ }
}
put_port = true;
}
--
2.17.1
[PATCH 2/2] drm/dp_mst: Avoid to mess up payload table by ports in stale topology
[Why]
After unplug/hotplug hub from the system, userspace might start to
clear stale payloads gradually. If we call drm_dp_mst_deallocate_vcpi()
to release stale VCPI of those ports which are not relating to current
topology, we have chane to wrongly clear active payload table entry for
current topology.
E.g.
We have allocated VCPI 1 in current payload table and we call
drm_dp_mst_deallocate_vcpi() to clean VCPI 1 in stale topology. In
drm_dp_mst_deallocate_vcpi(), it will call drm_dp_mst_put_payload_id()
tp put VCPI 1 and which means ID 1 is available again. Thereafter, if we
want to allocate a new payload stream, it will find ID 1 is available by
drm_dp_mst_assign_payload_id(). However, ID 1 is being used
[How]
Check target sink is relating to current topology or not before doing
any payload table update.
Searching upward to find the target sink's relevant root branch device.
If the found root branch device is not the same root of current
topology, don't update payload table.
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 29 +++
1 file changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 5fc261014a20..3d988d54df89 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -94,6 +94,9 @@ static int drm_dp_mst_register_i2c_bus(struct drm_dp_mst_port
*port);
static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port);
static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr);
+static bool drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port *port,
+struct drm_dp_mst_branch
*branch);
+
#define DBG_PREFIX "[dp_mst]"
#define DP_STR(x) [DP_ ## x] = #x
@@ -3360,6 +3363,7 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
struct drm_dp_mst_port *port;
int i, j;
int cur_slots = 1;
+ bool skip;
mutex_lock(&mgr->payload_lock);
for (i = 0; i < mgr->max_payloads; i++) {
@@ -3374,6 +3378,14 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
port = container_of(vcpi, struct drm_dp_mst_port,
vcpi);
+ mutex_lock(&mgr->lock);
+ skip = !drm_dp_mst_port_downstream_of_branch(port,
mgr->mst_primary);
+ mutex_unlock(&mgr->lock);
+
+ if (skip) {
+ DRM_DEBUG_KMS("Virtual channel %d is not in
current topology\n", i);
+ continue;
+ }
/* Validated ports don't matter if we're releasing
* VCPI
*/
@@ -3473,6 +3485,7 @@ int drm_dp_update_payload_part2(struct
drm_dp_mst_topology_mgr *mgr)
struct drm_dp_mst_port *port;
int i;
int ret = 0;
+ bool skip;
mutex_lock(&mgr->payload_lock);
for (i = 0; i < mgr->max_payloads; i++) {
@@ -3482,6 +3495,13 @@ int drm_dp_update_payload_part2(struct
drm_dp_mst_topology_mgr *mgr)
port = container_of(mgr->proposed_vcpis[i], struct
drm_dp_mst_port, vcpi);
+ mutex_lock(&mgr->lock);
+ skip = !drm_dp_mst_port_downstream_of_branch(port,
mgr->mst_primary);
+ mutex_unlock(&mgr->lock);
+
+ if (skip)
+ continue;
+
DRM_DEBUG_KMS("payload %d %d\n", i,
mgr->payloads[i].payload_state);
if (mgr->payloads[i].payload_state == DP_PAYLOAD_LOCAL) {
ret = drm_dp_create_payload_step2(mgr, port,
mgr->proposed_vcpis[i]->vcpi, &mgr->payloads[i]);
@@ -4562,9 +4582,18 @@ EXPORT_SYMBOL(drm_dp_mst_reset_vcpi_slots);
void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port)
{
+ bool skip;
+
if (!port->vcpi.vcpi)
return;
+ mutex_lock(&mgr->lock);
+ skip = !drm_dp_mst_port_downstream_of_branch(port, mgr->mst_primary);
+ mutex_unlock(&mgr->lock);
+
+ if (skip)
+ return;
+
drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
port->vcpi.num_slots = 0;
port->vcpi.pbn = 0;
--
2.17.1
[PATCH 0/2] Fix observed mst problems with StarTech hub
Use Startech 1to3 DP hub to do some mst hotplug tests and find some light up issues. 1. ACT polling timeout: Which is due to we didn't update DPCD payload table but still try to send ACT and polling for "ACT Handled" bit 2. Not all monitors light up: Due to we wrongly set unavailable VCP ID for new streams Wayne Lin (2): drm/dp_mst: Do not set proposed vcpi directly drm/dp_mst: Avoid to mess up payload table by ports in stale topology drivers/gpu/drm/drm_dp_mst_topology.c | 65 --- 1 file changed, 39 insertions(+), 26 deletions(-) -- 2.17.1
[PATCH v2 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast
[Why & How] According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast request message and current implementation is incorrect. Fix it. Signed-off-by: Wayne Lin Cc: sta...@vger.kernel.org --- drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index f11b3f718031..c32b98389349 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct drm_dp_sideband_msg_tx *msg) req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE; drm_dp_encode_sideband_req(&req, msg); + msg->path_msg = true; } static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr, req_type = txmsg->msg[0] & 0x7f; if (req_type == DP_CONNECTION_STATUS_NOTIFY || - req_type == DP_RESOURCE_STATUS_NOTIFY) + req_type == DP_RESOURCE_STATUS_NOTIFY || + req_type == DP_CLEAR_PAYLOAD_ID_TABLE) hdr->broadcast = 1; else hdr->broadcast = 0; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/dp_mst: Revise broadcast msg lct & lcr
[Why & How]
According to DP spec, broadcast message LCT equals to 1 and LCR equals
to 6. Current implementation is incorrect. Fix it.
In addition, revise a bit the hdr->rad handling to include broadcast
case.
Signed-off-by: Wayne Lin
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/drm_dp_mst_topology.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 17dbed0a9800..f11b3f718031 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2727,10 +2727,15 @@ static int set_hdr_from_dst_qlock(struct
drm_dp_sideband_msg_hdr *hdr,
else
hdr->broadcast = 0;
hdr->path_msg = txmsg->path_msg;
- hdr->lct = mstb->lct;
- hdr->lcr = mstb->lct - 1;
- if (mstb->lct > 1)
- memcpy(hdr->rad, mstb->rad, mstb->lct / 2);
+ if (hdr->broadcast) {
+ hdr->lct = 1;
+ hdr->lcr = 6;
+ } else {
+ hdr->lct = mstb->lct;
+ hdr->lcr = mstb->lct - 1;
+ }
+
+ memcpy(hdr->rad, mstb->rad, hdr->lct / 2);
return 0;
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request
While testing MST hotplug events on daisy chain monitors, find out that CLEAR_PAYLOAD_ID_TABLE is not broadcasted and payload id table is not reset. Dig in deeper and find out two parts needed to be fixed. 1. Link_Count_Total & Link_Count_Remaining of Broadcast message are incorrect. Should set lct=1 & lcr=6 2. CLEAR_PAYLOAD_ID_TABLE request message is not set as path broadcast request message. Should fix this. Changes since v1: *Refer to the suggestion from Ville Syrjala. While preparing hdr-rad, take broadcast case into consideration. Wayne Lin (2): drm/dp_mst: Revise broadcast msg lct & lcr drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast drivers/gpu/drm/drm_dp_mst_topology.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast
[Why & How] According to DP spec, CLEAR_PAYLOAD_ID_TABLE is a path broadcast request message and current implementation is incorrect. Fix it. Signed-off-by: Wayne Lin Cc: sta...@vger.kernel.org --- drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 713ef3b42054..6d73559046e5 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1072,6 +1072,7 @@ static void build_clear_payload_id_table(struct drm_dp_sideband_msg_tx *msg) req.req_type = DP_CLEAR_PAYLOAD_ID_TABLE; drm_dp_encode_sideband_req(&req, msg); + msg->path_msg = true; } static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, @@ -2722,7 +2723,8 @@ static int set_hdr_from_dst_qlock(struct drm_dp_sideband_msg_hdr *hdr, req_type = txmsg->msg[0] & 0x7f; if (req_type == DP_CONNECTION_STATUS_NOTIFY || - req_type == DP_RESOURCE_STATUS_NOTIFY) + req_type == DP_RESOURCE_STATUS_NOTIFY || + req_type == DP_CLEAR_PAYLOAD_ID_TABLE) hdr->broadcast = 1; else hdr->broadcast = 0; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/dp_mst: Revise broadcast msg lct & lcr
[Why & How]
According to DP spec, broadcast message LCT equals to 1 and LCR equals
to 6. Current implementation is incorrect. Fix it.
Signed-off-by: Wayne Lin
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/drm_dp_mst_topology.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 17dbed0a9800..713ef3b42054 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2727,8 +2727,14 @@ static int set_hdr_from_dst_qlock(struct
drm_dp_sideband_msg_hdr *hdr,
else
hdr->broadcast = 0;
hdr->path_msg = txmsg->path_msg;
- hdr->lct = mstb->lct;
- hdr->lcr = mstb->lct - 1;
+ if (hdr->broadcast) {
+ hdr->lct = 1;
+ hdr->lcr = 6;
+ } else {
+ hdr->lct = mstb->lct;
+ hdr->lcr = mstb->lct - 1;
+ }
+
if (mstb->lct > 1)
memcpy(hdr->rad, mstb->rad, mstb->lct / 2);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] Set CLEAR_PAYLOAD_ID_TABLE as broadcast request
While testing MST hotplug events on daisy chain monitors, find out that CLEAR_PAYLOAD_ID_TABLE is not broadcasted and payload id table is not reset. Dig in deeper and find out two parts needed to be fixed. 1. Link_Count_Total & Link_Count_Remaining of Broadcast message are incorrect. Should set lct=1 & lcr=6 2. CLEAR_PAYLOAD_ID_TABLE request message is not set as path broadcast request message. Should fix this. Wayne Lin (2): drm/dp_mst: Revise broadcast msg lct & lcr drm/dp_mst: Set CLEAR_PAYLOAD_ID_TABLE as broadcast drivers/gpu/drm/drm_dp_mst_topology.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/4] Revert "drm/amd/display: Expose new CRC window property" and changes associated with this commit
Thanks Siqueira. Series is: Reviewed-by: Wayne Lin On 1/12/21 10:55 PM, Rodrigo Siqueira wrote: Hi, In the V1, Wayne pointed out two problems: 1. The revert patch included one extra line that does not belong to it; 2. The original patch also had other fixes in the same commit; I removed the extra line from the reverted patch for tackling this issue, and I added one additional patch to this series that includes the other fix requested by Wayne. Thanks Original cover letter: A couple of weeks ago, Daniel highlighted [1] some issue related to a patch entitle "drm/amd/display: Expose new CRC window property". After discussion, we realize that we can revert that patch because we will need to create a debugfs or full UAPI for CRC soon, which will make this code obsolete. We got two other patches related to this same code; for this reason, this patchset reverts all changes associated with that specific commit. Rodrigo Siqueira (3): Revert "drm/amd/display: Fix unused variable warning" Revert "drm/amdgpu/disply: fix documentation warnings in display manager" Revert "drm/amd/display: Expose new CRC window property" Wayne Lin (1): drm/amd/display: Fix to be able to stop crc calculation .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 142 ++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 38 - .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 54 +-- .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h | 5 +- 4 files changed, 11 insertions(+), 228 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/1] Take SST-only branch device into account
Noticed this while testing 4 ports MST hub from StarTech.com. While plugging in and display a MST monitor(Dell U2417H), change the MST feature to off from OSD. Monitor then can't display anymore. After analyzing, found out that the CSN reports the specific port from Device with MST Branching Unit (Message_Capability_Status=1 && Peer_Device_Type=010) to SST-only Branch Device ((Message_Capability_Status=0 && Peer_Device_Type=010). Current code expects peer device as MST branch device and can handle sideband message once the peer device type is declared as Peer_Device_Type=010. But for SST-only Branch Device, its Peer_Device_Type is 010 and can't handle sideband message. For this case, we should follow the same handling procedure as the case in DP_PEER_DEVICE_DP_LEGACY_CONV & DP_PEER_DEVICE_SST_SINK. This patch is trying to add the SST-only branch device case in and remain the original processing logic in current code. Changes since v1:(https://patchwork.kernel.org/cover/11323075/) * Squash previous two patches into one patch * Combine if statements to have code cleaner Wayne Lin (1): drm/dp_mst: Handle SST-only branch device case drivers/gpu/drm/drm_dp_mst_topology.c | 140 +++--- 1 file changed, 80 insertions(+), 60 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/1] drm/dp_mst: Handle SST-only branch device case
[Why]
While handling LINK_ADDRESS reply, current code expects a peer device
can handle sideband message once the peer device type is reported as
DP_PEER_DEVICE_MST_BRANCHING. However, when the connected device is
a SST branch case, it can't handle the sideband message(MST_CAP=0 in
DPCD 00021h).
Current code will try to send LINK_ADDRESS to SST branch device and end
up with message timeout and monitor can't display normally. As the
result of that, we should take SST branch device into account.
[How]
According to DP 1.4 spec, we can use Peer_Device_Type as
DP_PEER_DEVICE_MST_BRANCHING and Message_Capability_Status as 0 to
indicate peer device as a SST-only branch device.
Fix following:
- Add the function drm_dp_mst_is_dp_mst_end_device() to decide whether a
peer device connected to a DFP is mst end device. Which also indicates
if the peer device is capable of handling message or not.
- Take SST-only branch device case into account in
drm_dp_port_set_pdt() and add a new parameter 'new_mcs'. Take sst branch
device case as the same case as DP_PEER_DEVICE_DP_LEGACY_CONV and
DP_PEER_DEVICE_SST_SINK. All original handling logics remain.
- Take SST-only branch device case into account in
drm_dp_mst_port_add_connector().
- Fix some parts in drm_dp_mst_handle_link_address_port() to have SST
branch device case into consideration.
- Fix the arguments of drm_dp_port_set_pdt() in
drm_dp_mst_handle_conn_stat().
- Have SST branch device also report
connector_status_connected when the ddps is true
in drm_dp_mst_detect_port()
- Fix the arguments of drm_dp_port_set_pdt() in
drm_dp_delayed_destroy_port()
Changes since v1:(https://patchwork.kernel.org/patch/11323079/)
* Squash previous patch into one patch and merge the commit message here.
* Combine the if statements mentioned in comments
Fixes: c485e2c97dae ("drm/dp_mst: Refactor pdt setup/teardown, add more
locking")
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Lyude Paul
Signed-off-by: Wayne Lin
Reviewed-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 140 +++---
1 file changed, 80 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 79691c553182..c40a9bd63cfb 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1914,73 +1914,90 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port
*port,
return parent_lct + 1;
}
-static int drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt)
+static bool drm_dp_mst_is_dp_mst_end_device(u8 pdt, bool mcs)
+{
+ switch (pdt) {
+ case DP_PEER_DEVICE_DP_LEGACY_CONV:
+ case DP_PEER_DEVICE_SST_SINK:
+ return true;
+ case DP_PEER_DEVICE_MST_BRANCHING:
+ /* For sst branch device */
+ if (!mcs)
+ return true;
+
+ return false;
+ }
+ return true;
+}
+
+static int
+drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
+ bool new_mcs)
{
struct drm_dp_mst_topology_mgr *mgr = port->mgr;
struct drm_dp_mst_branch *mstb;
u8 rad[8], lct;
int ret = 0;
- if (port->pdt == new_pdt)
+ if (port->pdt == new_pdt && port->mcs == new_mcs)
return 0;
/* Teardown the old pdt, if there is one */
- switch (port->pdt) {
- case DP_PEER_DEVICE_DP_LEGACY_CONV:
- case DP_PEER_DEVICE_SST_SINK:
- /*
-* If the new PDT would also have an i2c bus, don't bother
-* with reregistering it
-*/
- if (new_pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
- new_pdt == DP_PEER_DEVICE_SST_SINK) {
- port->pdt = new_pdt;
- return 0;
- }
+ if (port->pdt != DP_PEER_DEVICE_NONE) {
+ if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) {
+ /*
+* If the new PDT would also have an i2c bus,
+* don't bother with reregistering it
+*/
+ if (new_pdt != DP_PEER_DEVICE_NONE &&
+ drm_dp_mst_is_dp_mst_end_device(new_pdt, new_mcs)) {
+ port->pdt = new_pdt;
+ port->mcs = new_mcs;
+ return 0;
+ }
- /* remove i2c over sideband */
- drm_dp_mst_unregister_i2c_bus(&port->aux);
- break;
- case DP_PEER_DEVICE_MST_BRANCHING:
- mutex_lock(&mgr->lock);
- drm_dp_mst_topology_put_mstb(port->mstb);
- port->mstb = NULL;
- mutex_unlock(&mgr->lock);
- break;
+
[PATCH] drm/dp_mst: Have DP_Tx send one msg at a time
[Why]
Noticed this while testing MST with the 4 ports MST hub from
StarTech.com. Sometimes can't light up monitors normally and get the
error message as 'sideband msg build failed'.
Look into aux transactions, found out that source sometimes will send
out another down request before receiving the down reply of the
previous down request. On the other hand, in drm_dp_get_one_sb_msg(),
current code doesn't handle the interleaved replies case. Hence, source
can't build up message completely and can't light up monitors.
[How]
For good compatibility, enforce source to send out one down request at a
time. Add a flag, is_waiting_for_dwn_reply, to determine if the source
can send out a down request immediately or not.
- Check the flag before calling process_single_down_tx_qlock to send out
a msg
- Set the flag when successfully send out a down request
- Clear the flag when successfully build up a down reply
- Clear the flag when find erros during sending out a down request
- Clear the flag when find errors during building up a down reply
- Clear the flag when timeout occurs during waiting for a down reply
- Use drm_dp_mst_kick_tx() to try to send another down request in queue
at the end of drm_dp_mst_wait_tx_reply() (attempt to send out messages
in queue when errors occur)
Cc: Lyude Paul
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 14 --
include/drm/drm_dp_mst_helper.h | 6 ++
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 4395d5cc0645..3542af15387a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1195,6 +1195,8 @@ static int drm_dp_mst_wait_tx_reply(struct
drm_dp_mst_branch *mstb,
txmsg->state == DRM_DP_SIDEBAND_TX_SENT) {
mstb->tx_slots[txmsg->seqno] = NULL;
}
+ mgr->is_waiting_for_dwn_reply = false;
+
}
out:
if (unlikely(ret == -EIO) && drm_debug_enabled(DRM_UT_DP)) {
@@ -1204,6 +1206,7 @@ static int drm_dp_mst_wait_tx_reply(struct
drm_dp_mst_branch *mstb,
}
mutex_unlock(&mgr->qlock);
+ drm_dp_mst_kick_tx(mgr);
return ret;
}
@@ -2770,9 +2773,11 @@ static void process_single_down_tx_qlock(struct
drm_dp_mst_topology_mgr *mgr)
ret = process_single_tx_qlock(mgr, txmsg, false);
if (ret == 1) {
/* txmsg is sent it should be in the slots now */
+ mgr->is_waiting_for_dwn_reply = true;
list_del(&txmsg->next);
} else if (ret) {
DRM_DEBUG_KMS("failed to send msg in q %d\n", ret);
+ mgr->is_waiting_for_dwn_reply = false;
list_del(&txmsg->next);
if (txmsg->seqno != -1)
txmsg->dst->tx_slots[txmsg->seqno] = NULL;
@@ -2812,7 +2817,8 @@ static void drm_dp_queue_down_tx(struct
drm_dp_mst_topology_mgr *mgr,
drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
}
- if (list_is_singular(&mgr->tx_msg_downq))
+ if (list_is_singular(&mgr->tx_msg_downq) &&
+ !mgr->is_waiting_for_dwn_reply)
process_single_down_tx_qlock(mgr);
mutex_unlock(&mgr->qlock);
}
@@ -3743,6 +3749,7 @@ static int drm_dp_mst_handle_down_rep(struct
drm_dp_mst_topology_mgr *mgr)
mutex_lock(&mgr->qlock);
txmsg->state = DRM_DP_SIDEBAND_TX_RX;
mstb->tx_slots[slot] = NULL;
+ mgr->is_waiting_for_dwn_reply = false;
mutex_unlock(&mgr->qlock);
wake_up_all(&mgr->tx_waitq);
@@ -3752,6 +3759,9 @@ static int drm_dp_mst_handle_down_rep(struct
drm_dp_mst_topology_mgr *mgr)
no_msg:
drm_dp_mst_topology_put_mstb(mstb);
clear_down_rep_recv:
+ mutex_lock(&mgr->qlock);
+ mgr->is_waiting_for_dwn_reply = false;
+ mutex_unlock(&mgr->qlock);
memset(&mgr->down_rep_recv, 0, sizeof(struct drm_dp_sideband_msg_rx));
return 0;
@@ -4591,7 +4601,7 @@ static void drm_dp_tx_work(struct work_struct *work)
struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct
drm_dp_mst_topology_mgr, tx_work);
mutex_lock(&mgr->qlock);
- if (!list_empty(&mgr->tx_msg_downq))
+ if (!list_empty(&mgr->tx_msg_downq) && !mgr->is_waiting_for_dwn_reply)
process_single_down_tx_qlock(mgr);
mutex_unlock(&mgr->qlock);
}
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 942575de86a0..d0b45468135a 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -610,6 +610,12 @@ struct drm_dp_mst_topology_mgr {
* &dr
[PATCH 1/2] drm/dp_mst: Add a function to determine the mst end device
[Why]
For later usage convenience, add the function
drm_dp_mst_is_dp_mst_end_device() to decide whether a peer device
connected to a DFP is mst end device. Which also indicates if the peer
device is capable of handling message or not.
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index eebf325d7f48..8f54b241db08 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1918,6 +1918,22 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port
*port,
return parent_lct + 1;
}
+static bool drm_dp_mst_is_dp_mst_end_device(u8 pdt, bool mcs)
+{
+ switch (pdt) {
+ case DP_PEER_DEVICE_DP_LEGACY_CONV:
+ case DP_PEER_DEVICE_SST_SINK:
+ return true;
+ case DP_PEER_DEVICE_MST_BRANCHING:
+ /* For sst branch device */
+ if (!mcs)
+ return true;
+
+ return false;
+ }
+ return true;
+}
+
static int drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt)
{
struct drm_dp_mst_topology_mgr *mgr = port->mgr;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/dp_mst: Handle SST-only branch device case
[Why]
While handling LINK_ADDRESS reply, current code expects a peer device
can handle sideband message once the peer device type is reported as
DP_PEER_DEVICE_MST_BRANCHING. However, when the connected device is
a SST branch case, it can't handle the sideband message(MST_CAP=0 in
DPCD 00021h).
Current code will try to send LINK_ADDRESS to SST branch device and end
up with message timeout and monitor can't display normally. As the
result of that, we should take SST branch device into account.
[How]
According to DP 1.4 spec, we can use Peer_Device_Type as
DP_PEER_DEVICE_MST_BRANCHING and Message_Capability_Status as 0 to
indicate peer device as a SST-only branch device.
Fix following:
- Take SST-only branch device case into account in
drm_dp_port_set_pdt() and add a new parameter 'new_mcs'. Take sst branch
device case as the same case as DP_PEER_DEVICE_DP_LEGACY_CONV and
DP_PEER_DEVICE_SST_SINK. All original handling logics remain.
- Take SST-only branch device case into account in
drm_dp_mst_port_add_connector().
- Fix some parts in drm_dp_mst_handle_link_address_port() to have SST
branch device case into consideration.
- Fix the arguments of drm_dp_port_set_pdt() in
drm_dp_mst_handle_conn_stat().
- Have SST branch device also report
connector_status_connected when the ddps is true
in drm_dp_mst_detect_port()
- Fix the arguments of drm_dp_port_set_pdt() in
drm_dp_delayed_destroy_port()
Fixes: c485e2c97dae ("drm/dp_mst: Refactor pdt setup/teardown, add more
locking")
Cc: Ville Syrjälä
Cc: Harry Wentland
Cc: Lyude Paul
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 131 +-
1 file changed, 68 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 8f54b241db08..4395d5cc0645 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1934,73 +1934,74 @@ static bool drm_dp_mst_is_dp_mst_end_device(u8 pdt,
bool mcs)
return true;
}
-static int drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt)
+static int
+drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
+ bool new_mcs)
{
struct drm_dp_mst_topology_mgr *mgr = port->mgr;
struct drm_dp_mst_branch *mstb;
u8 rad[8], lct;
int ret = 0;
- if (port->pdt == new_pdt)
+ if (port->pdt == new_pdt && port->mcs == new_mcs)
return 0;
/* Teardown the old pdt, if there is one */
- switch (port->pdt) {
- case DP_PEER_DEVICE_DP_LEGACY_CONV:
- case DP_PEER_DEVICE_SST_SINK:
- /*
-* If the new PDT would also have an i2c bus, don't bother
-* with reregistering it
-*/
- if (new_pdt == DP_PEER_DEVICE_DP_LEGACY_CONV ||
- new_pdt == DP_PEER_DEVICE_SST_SINK) {
- port->pdt = new_pdt;
- return 0;
- }
+ if (port->pdt != DP_PEER_DEVICE_NONE) {
+ if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) {
+ /*
+* If the new PDT would also have an i2c bus,
+* don't bother with reregistering it
+*/
+ if (new_pdt != DP_PEER_DEVICE_NONE &&
+ drm_dp_mst_is_dp_mst_end_device(new_pdt, new_mcs)) {
+ port->pdt = new_pdt;
+ port->mcs = new_mcs;
+ return 0;
+ }
- /* remove i2c over sideband */
- drm_dp_mst_unregister_i2c_bus(&port->aux);
- break;
- case DP_PEER_DEVICE_MST_BRANCHING:
- mutex_lock(&mgr->lock);
- drm_dp_mst_topology_put_mstb(port->mstb);
- port->mstb = NULL;
- mutex_unlock(&mgr->lock);
- break;
+ /* remove i2c over sideband */
+ drm_dp_mst_unregister_i2c_bus(&port->aux);
+ } else {
+ mutex_lock(&mgr->lock);
+ drm_dp_mst_topology_put_mstb(port->mstb);
+ port->mstb = NULL;
+ mutex_unlock(&mgr->lock);
+ }
}
port->pdt = new_pdt;
- switch (port->pdt) {
- case DP_PEER_DEVICE_DP_LEGACY_CONV:
- case DP_PEER_DEVICE_SST_SINK:
- /* add i2c over sideband */
- ret = drm_dp_mst_register_i2c_bus(&port->aux);
- break;
+ port->mcs = new_mcs;
- case DP_PEER_DEVICE_MST_BRANCHING:
- lct = drm_dp_calculate_rad(port, rad);
- mstb = drm_d
[PATCH 0/2] Take SST-only branch device into account
Noticed this while testing 4 ports MST hub from StarTech.com. While plugging in and display a MST monitor(Dell U2417H), change the MST feature to off from OSD. Monitor then can't display anymore. After analyzing, found out that the CSN reports the specific port from Device with MST Branching Unit (Message_Capability_Status=1 && Peer_Device_Type=010) to SST-only Branch Device ((Message_Capability_Status=0 && Peer_Device_Type=010). Current code expects peer device as MST branch device and can handle sideband message once the peer device type is declared as Peer_Device_Type=010. But for SST-only Branch Device, its Peer_Device_Type is 010 and can't handle sideband message. For this case, we should follow the same handling procedure as the case in DP_PEER_DEVICE_DP_LEGACY_CONV & DP_PEER_DEVICE_SST_SINK. This series is trying to add the SST-only branch device case in and remain the original processing logic in current code. Wayne Lin (2): drm/dp_mst: Add a function to determine the mst end device drm/dp_mst: Handle SST-only branch device case drivers/gpu/drm/drm_dp_mst_topology.c | 147 +++--- 1 file changed, 84 insertions(+), 63 deletions(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/dp_mst: clear time slots for ports invalid
[Why]
When change the connection status in a MST topology, mst device
which detect the event will send out CONNECTION_STATUS_NOTIFY messgae.
e.g. src-mst-mst-sst => src-mst (unplug) mst-sst
Currently, under the above case of unplugging device, ports which have
been allocated payloads and are no longer in the topology still occupy
time slots and recorded in proposed_vcpi[] of topology manager.
If we don't clean up the proposed_vcpi[], when code flow goes to try to
update payload table by calling drm_dp_update_payload_part1(), we will
fail at checking port validation due to there are ports with proposed
time slots but no longer in the mst topology. As the result of that, we
will also stop updating the DPCD payload table of down stream port.
[How]
While handling the CONNECTION_STATUS_NOTIFY message, add a detection to
see if the event indicates that a device is unplugged to an output port.
If the detection is true, then iterrate over all proposed_vcpi[] to
see whether a port of the proposed_vcpi[] is still in the topology or
not. If the port is invalid, set its num_slots to 0.
Thereafter, when try to update payload table by calling
drm_dp_update_payload_part1(), we can successfully update the DPCD
payload table of down stream port and clear the proposed_vcpi[] to NULL.
Changes since v1:(https://patchwork.kernel.org/patch/11275801/)
* Invert the conditional to reduce the indenting
Reviewed-by: Lyude Paul
Signed-off-by: Wayne Lin
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/drm_dp_mst_topology.c | 25 -
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 6e10f6235009..e37cd6ec6e36 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2321,7 +2321,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
- int old_ddps, ret;
+ int old_ddps, old_input, ret, i;
u8 new_pdt;
bool dowork = false, create_connector = false;
@@ -2352,6 +2352,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
}
old_ddps = port->ddps;
+ old_input = port->input;
port->input = conn_stat->input_port;
port->mcs = conn_stat->message_capability_status;
port->ldps = conn_stat->legacy_device_plug_status;
@@ -2376,6 +2377,28 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
dowork = false;
}
+ if (!old_input && old_ddps != port->ddps && !port->ddps) {
+ for (i = 0; i < mgr->max_payloads; i++) {
+ struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
+ struct drm_dp_mst_port *port_validated;
+
+ if (!vcpi)
+ continue;
+
+ port_validated =
+ container_of(vcpi, struct drm_dp_mst_port,
vcpi);
+ port_validated =
+ drm_dp_mst_topology_get_port_validated(mgr,
port_validated);
+ if (!port_validated) {
+ mutex_lock(&mgr->payload_lock);
+ vcpi->num_slots = 0;
+ mutex_unlock(&mgr->payload_lock);
+ } else {
+ drm_dp_mst_topology_put_port(port_validated);
+ }
+ }
+ }
+
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
else if (create_connector)
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/dp_mst: correct the shifting in DP_REMOTE_I2C_READ
[Why] According to DP spec, it should shift left 4 digits for NO_STOP_BIT in REMOTE_I2C_READ message. Not 5 digits. In current code, NO_STOP_BIT is always set to zero which means I2C master is always generating a I2C stop at the end of each I2C write transaction while handling REMOTE_I2C_READ sideband message. This issue might have the generated I2C signal not meeting the requirement. Take random read in I2C for instance, I2C master should generate a repeat start to start to read data after writing the read address. This issue will cause the I2C master to generate a stop-start rather than a re-start which is not expected in I2C random read. [How] Correct the shifting value of NO_STOP_BIT for DP_REMOTE_I2C_READ case in drm_dp_encode_sideband_req(). Changes since v1:(https://patchwork.kernel.org/patch/11312667/) * Add more descriptions in commit and cc to stable Fixes: ad7f8a1f9ce (drm/helper: add Displayport multi-stream helper (v0.6)) Reviewed-by: Harry Wentland Signed-off-by: Wayne Lin Cc: sta...@vger.kernel.org --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 1cf5f8b8bbb8..9d24c98bece1 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -393,7 +393,7 @@ drm_dp_encode_sideband_req(const struct drm_dp_sideband_msg_req_body *req, memcpy(&buf[idx], req->u.i2c_read.transactions[i].bytes, req->u.i2c_read.transactions[i].num_bytes); idx += req->u.i2c_read.transactions[i].num_bytes; - buf[idx] = (req->u.i2c_read.transactions[i].no_stop_bit & 0x1) << 5; + buf[idx] = (req->u.i2c_read.transactions[i].no_stop_bit & 0x1) << 4; buf[idx] |= (req->u.i2c_read.transactions[i].i2c_transaction_delay & 0xf); idx++; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/dp_mst: correct the shifting in DP_REMOTE_I2C_READ
[Why] According to DP spec, it should shift left 4 digits for NO_STOP_BIT in REMOTE_I2C_READ message. Not 5 digits. [How] Correct the shifting value of NO_STOP_BIT for DP_REMOTE_I2C_READ case in drm_dp_encode_sideband_req(). Signed-off-by: Wayne Lin --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 1d1bfa49ca2b..0557e225ff67 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -393,7 +393,7 @@ drm_dp_encode_sideband_req(const struct drm_dp_sideband_msg_req_body *req, memcpy(&buf[idx], req->u.i2c_read.transactions[i].bytes, req->u.i2c_read.transactions[i].num_bytes); idx += req->u.i2c_read.transactions[i].num_bytes; - buf[idx] = (req->u.i2c_read.transactions[i].no_stop_bit & 0x1) << 5; + buf[idx] = (req->u.i2c_read.transactions[i].no_stop_bit & 0x1) << 4; buf[idx] |= (req->u.i2c_read.transactions[i].i2c_transaction_delay & 0xf); idx++; } -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/dp_mst: Avoid NULL pointer dereference
[Why]
Found kernel NULL pointer dereference under the below situation:
src — HDMI_Monitor src — HDMI_Monitor
e.g.: \=>
MSTB — MSTB (unplug) MSTB — MSTB
When display 1 HDMI and 2 DP daisy chain monitors, unplugging the dp
cable connected to source causes kernel NULL pointer dereference at
drm_dp_mst_atomic_check_bw_limit(). When calculating pbn_limit, if
branch is null, accessing "&branch->ports" causes the problem.
[How]
Judge branch is null or not at the beginning. If it is null, return 0.
Signed-off-by: Wayne Lin
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/drm_dp_mst_topology.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 7d2d31eaf003..a6473e3ab448 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4707,6 +4707,9 @@ int drm_dp_mst_atomic_check_bw_limit(struct
drm_dp_mst_branch *branch,
struct drm_dp_vcpi_allocation *vcpi;
int pbn_limit = 0, pbn_used = 0;
+ if (!branch)
+ return 0;
+
list_for_each_entry(port, &branch->ports, next) {
if (port->mstb)
if (drm_dp_mst_atomic_check_bw_limit(port->mstb,
mst_state))
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/dp_mst: clear time slots for ports invalid
[Why]
When change the connection status in a MST topology, mst device
which detect the event will send out CONNECTION_STATUS_NOTIFY messgae.
e.g. src-mst-mst-sst => src-mst (unplug) mst-sst
Currently, under the above case of unplugging device, ports which have
been allocated payloads and are no longer in the topology still occupy
time slots and recorded in proposed_vcpi[] of topology manager.
If we don't clean up the proposed_vcpi[], when code flow goes to try to
update payload table by calling drm_dp_update_payload_part1(), we will
fail at checking port validation due to there are ports with proposed
time slots but no longer in the mst topology. As the result of that, we
will also stop updating the DPCD payload table of down stream port.
[How]
While handling the CONNECTION_STATUS_NOTIFY message, add a detection to
see if the event indicates that a device is unplugged to an output port.
If the detection is true, then iterrate over all proposed_vcpi[] to
see whether a port of the proposed_vcpi[] is still in the topology or
not. If the port is invalid, set its num_slots to 0.
Thereafter, when try to update payload table by calling
drm_dp_update_payload_part1(), we can successfully update the DPCD
payload table of down stream port and clear the proposed_vcpi[] to NULL.
Signed-off-by: Wayne Lin
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/drm_dp_mst_topology.c | 24 +++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 5306c47dc820..2e236b6275c4 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2318,7 +2318,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
{
struct drm_dp_mst_topology_mgr *mgr = mstb->mgr;
struct drm_dp_mst_port *port;
- int old_ddps, ret;
+ int old_ddps, old_input, ret, i;
u8 new_pdt;
bool dowork = false, create_connector = false;
@@ -2349,6 +2349,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
}
old_ddps = port->ddps;
+ old_input = port->input;
port->input = conn_stat->input_port;
port->mcs = conn_stat->message_capability_status;
port->ldps = conn_stat->legacy_device_plug_status;
@@ -2373,6 +2374,27 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch
*mstb,
dowork = false;
}
+ if (!old_input && old_ddps != port->ddps && !port->ddps) {
+ for (i = 0; i < mgr->max_payloads; i++) {
+ struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
+ struct drm_dp_mst_port *port_validated;
+
+ if (vcpi) {
+ port_validated =
+ container_of(vcpi, struct
drm_dp_mst_port, vcpi);
+ port_validated =
+
drm_dp_mst_topology_get_port_validated(mgr, port_validated);
+ if (!port_validated) {
+ mutex_lock(&mgr->payload_lock);
+ vcpi->num_slots = 0;
+ mutex_unlock(&mgr->payload_lock);
+ } else {
+
drm_dp_mst_topology_put_port(port_validated);
+ }
+ }
+ }
+ }
+
if (port->connector)
drm_modeset_unlock(&mgr->base.lock);
else if (create_connector)
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/dp_mst: Remove VCPI while disabling topology mgr
[Why]
This patch is trying to address the issue observed when hotplug DP
daisy chain monitors.
e.g.
src-mstb-mstb-sst -> src (unplug) mstb-mstb-sst -> src-mstb-mstb-sst
(plug in again)
Once unplug a DP MST capable device, driver will call
drm_dp_mst_topology_mgr_set_mst() to disable MST. In this function,
it cleans data of topology manager while disabling mst_state. However,
it doesn't clean up the proposed_vcpis of topology manager.
If proposed_vcpi is not reset, once plug in MST daisy chain monitors
later, code will fail at checking port validation while trying to
allocate payloads.
When MST capable device is plugged in again and try to allocate
payloads by calling drm_dp_update_payload_part1(), this
function will iterate over all proposed virtual channels to see if
any proposed VCPI's num_slots is greater than 0. If any proposed
VCPI's num_slots is greater than 0 and the port which the
specific virtual channel directed to is not in the topology, code then
fails at the port validation. Since there are stale VCPI allocations
from the previous topology enablement in proposed_vcpi[], code will fail
at port validation and reurn EINVAL.
[How]
Clean up the data of stale proposed_vcpi[] and reset mgr->proposed_vcpis
to NULL while disabling mst in drm_dp_mst_topology_mgr_set_mst().
Changes since v1:
*Add on more details in commit message to describe the issue which the
patch is trying to fix
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index ae5809a1f19a..bf4f745a4edb 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3386,6 +3386,7 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw, u8
dp_link_count)
int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool
mst_state)
{
int ret = 0;
+ int i = 0;
struct drm_dp_mst_branch *mstb = NULL;
mutex_lock(&mgr->lock);
@@ -3446,10 +3447,21 @@ int drm_dp_mst_topology_mgr_set_mst(struct
drm_dp_mst_topology_mgr *mgr, bool ms
/* this can fail if the device is gone */
drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, 0);
ret = 0;
+ mutex_lock(&mgr->payload_lock);
memset(mgr->payloads, 0, mgr->max_payloads * sizeof(struct
drm_dp_payload));
mgr->payload_mask = 0;
set_bit(0, &mgr->payload_mask);
+ for (i = 0; i < mgr->max_payloads; i++) {
+ struct drm_dp_vcpi *vcpi = mgr->proposed_vcpis[i];
+
+ if (vcpi) {
+ vcpi->vcpi = 0;
+ vcpi->num_slots = 0;
+ }
+ mgr->proposed_vcpis[i] = NULL;
+ }
mgr->vcpi_mask = 0;
+ mutex_unlock(&mgr->payload_lock);
}
out_unlock:
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/dp_mst: Correct the bug in drm_dp_update_payload_part1()
[Why]
If the payload_state is DP_PAYLOAD_DELETE_LOCAL in series, current
code doesn't delete the payload at current index and just move the
index to next one after shuffling payloads.
[How]
Drop the i++ increasing part in for loop head and decide whether
to increase the index or not according to payload_state of current
payload.
Changes since v1:
* Refine the code to have it easy reading
* Amend the commit message to meet the way code is modified now.
Signed-off-by: Wayne Lin
Reviewed-by: Lyude Paul
---
drivers/gpu/drm/drm_dp_mst_topology.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 81e92b260d7a..4ef6decc0551 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3176,9 +3176,11 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
drm_dp_mst_topology_put_port(port);
}
- for (i = 0; i < mgr->max_payloads; i++) {
- if (mgr->payloads[i].payload_state != DP_PAYLOAD_DELETE_LOCAL)
+ for (i = 0; i < mgr->max_payloads; /* do nothing */) {
+ if (mgr->payloads[i].payload_state != DP_PAYLOAD_DELETE_LOCAL) {
+ i++;
continue;
+ }
DRM_DEBUG_KMS("removing payload %d\n", i);
for (j = i; j < mgr->max_payloads - 1; j++) {
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/dp_mst: Correct the bug in drm_dp_update_payload_part1()
[Why]
If the payload_state is DP_PAYLOAD_DELETE_LOCAL in series, current
code doesn't delete the payload at current index and just move the
index to next one after shuffling payloads.
[How]
After shuffling payloads, decide whether to move on index or not
according to payload_state of current payload.
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 81e92b260d7a..8da5d461ea01 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3176,7 +3176,8 @@ int drm_dp_update_payload_part1(struct
drm_dp_mst_topology_mgr *mgr)
drm_dp_mst_topology_put_port(port);
}
- for (i = 0; i < mgr->max_payloads; i++) {
+ for (i = 0; i < mgr->max_payloads;
+ (mgr->payloads[i].payload_state == DP_PAYLOAD_DELETE_LOCAL) ? i
: i++) {
if (mgr->payloads[i].payload_state != DP_PAYLOAD_DELETE_LOCAL)
continue;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/dp_mst: Remove VCPI while disabling topology mgr
[Why]
While disabling mst topology manager in
drm_dp_mst_topology_mgr_set_mst(), now just reset the mgr->payloads
but doesn't handle the mgr->proposed_vcpis.
[How]
Remove mgr->proposed_vcpis to NULL.
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index ae5809a1f19a..81e92b260d7a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3386,6 +3386,7 @@ static int drm_dp_get_vc_payload_bw(u8 dp_link_bw, u8
dp_link_count)
int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool
mst_state)
{
int ret = 0;
+ int i = 0;
struct drm_dp_mst_branch *mstb = NULL;
mutex_lock(&mgr->lock);
@@ -3446,10 +3447,21 @@ int drm_dp_mst_topology_mgr_set_mst(struct
drm_dp_mst_topology_mgr *mgr, bool ms
/* this can fail if the device is gone */
drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, 0);
ret = 0;
+ mutex_lock(&mgr->payload_lock);
memset(mgr->payloads, 0, mgr->max_payloads * sizeof(struct
drm_dp_payload));
mgr->payload_mask = 0;
set_bit(0, &mgr->payload_mask);
+ for (i = 0; i < mgr->max_payloads; i++) {
+ struct drm_dp_vcpi *tmp_vcpi = mgr->proposed_vcpis[i];
+
+ if (tmp_vcpi) {
+ tmp_vcpi->vcpi = 0;
+ tmp_vcpi->num_slots = 0;
+ }
+ mgr->proposed_vcpis[i] = NULL;
+ }
mgr->vcpi_mask = 0;
+ mutex_unlock(&mgr->payload_lock);
}
out_unlock:
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V2 2/2] drm/edid: Add alternate clock for SMPTE 4K
[Why]
In hdmi_mode_alternate_clock(), it adds an exception for VIC 4
mode (4096x2160@24) due to there is no alternate clock defined for
that mode in HDMI1.4b. But HDMI2.0 adds 23.98Hz for that mode.
[How]
Remove the exception
v2: Adjust the comment description of hdmi_mode_alternate_clock()
due to there is no more exception for VIC 4 mode.
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_edid.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0307cad36f14..e6368c3c4471 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3118,17 +3118,10 @@ static enum hdmi_picture_aspect
drm_get_hdmi_aspect_ratio(const u8 video_code)
/*
* Calculate the alternate clock for HDMI modes (those from the HDMI vendor
* specific block).
- *
- * It's almost like cea_mode_alternate_clock(), we just need to add an
- * exception for the VIC 4 mode (4096x2160@24Hz): no alternate clock for this
- * one.
*/
static unsigned int
hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)
{
- if (hdmi_mode->vdisplay == 4096 && hdmi_mode->hdisplay == 2160)
- return hdmi_mode->clock;
-
return cea_mode_alternate_clock(hdmi_mode);
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V2 1/2] drm/edid: Add aspect ratios to HDMI 4K modes
[Why]
HDMI 2.0 adds aspect ratio attribute to distinguish different
4k modes. According to Appendix E of HDMI 2.0 spec, source should
use VSIF to indicate video mode only when the mode is one defined
in HDMI 1.4b 4K modes. Otherwise, use AVI infoframes to convey VIC.
Current code doesn't take aspect ratio into consideration while
constructing avi infoframe. Should modify that.
[How]
Inherit Ville Syrjälä's work
"drm/edid: Prep for HDMI VIC aspect ratio" at
https://patchwork.kernel.org/patch/11174639/
Add picture_aspect_ratio attributes to edid_4k_modes[] and
construct VIC and HDMI_VIC by taking aspect ratio into
consideration.
v2: Correct missing initializer error at adding aspect ratio of
SMPTE mode.
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_edid.c | 45 +-
1 file changed, 35 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 77a39fc76045..0307cad36f14 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1288,25 +1288,25 @@ static const struct drm_display_mode edid_4k_modes[] = {
3840, 4016, 4104, 4400, 0,
2160, 2168, 2178, 2250, 0,
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
- .vrefresh = 30, },
+ .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
/* 2 - 3840x2160@25Hz */
{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
3840, 4896, 4984, 5280, 0,
2160, 2168, 2178, 2250, 0,
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
- .vrefresh = 25, },
+ .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
/* 3 - 3840x2160@24Hz */
{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
3840, 5116, 5204, 5500, 0,
2160, 2168, 2178, 2250, 0,
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
- .vrefresh = 24, },
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
/* 4 - 4096x2160@24Hz (SMPTE) */
{ DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000,
4096, 5116, 5204, 5500, 0,
2160, 2168, 2178, 2250, 0,
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
- .vrefresh = 24, },
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135,
},
};
/*** DDC fetch and block validation ***/
@@ -3110,6 +3110,11 @@ static enum hdmi_picture_aspect
drm_get_cea_aspect_ratio(const u8 video_code)
return edid_cea_modes[video_code].picture_aspect_ratio;
}
+static enum hdmi_picture_aspect drm_get_hdmi_aspect_ratio(const u8 video_code)
+{
+ return edid_4k_modes[video_code].picture_aspect_ratio;
+}
+
/*
* Calculate the alternate clock for HDMI modes (those from the HDMI vendor
* specific block).
@@ -3136,6 +3141,9 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const
struct drm_display_mode *to_
if (!to_match->clock)
return 0;
+ if (to_match->picture_aspect_ratio)
+ match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];
unsigned int clock1, clock2;
@@ -3171,6 +3179,9 @@ static u8 drm_match_hdmi_mode(const struct
drm_display_mode *to_match)
if (!to_match->clock)
return 0;
+ if (to_match->picture_aspect_ratio)
+ match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];
unsigned int clock1, clock2;
@@ -5118,6 +5129,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
const struct drm_display_mode *mode)
{
enum hdmi_picture_aspect picture_aspect;
+ u8 vic, hdmi_vic;
int err;
if (!frame || !mode)
@@ -5130,7 +5142,8 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
frame->pixel_repeat = 1;
- frame->video_code = drm_mode_cea_vic(connector, mode);
+ vic = drm_mode_cea_vic(connector, mode);
+ hdmi_vic = drm_mode_hdmi_vic(connector, mode);
frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
@@ -5144,11 +5157,15 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
/*
* Populate picture aspect ratio from either
-* user input (if specified) or from the CEA mode list.
+* user input (if specified) or from the CEA/HDMI mode lists.
[PATCH 2/2] drm/edid: Add alternate clock for SMPTE 4K
[Why]
In hdmi_mode_alternate_clock(), it adds an exception for VIC 4
mode (4096x2160@24) due to there is no alternate clock defined for
that mode in HDMI1.4b. But HDMI2.0 adds 23.98Hz for that mode.
[How]
Remove the exception
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_edid.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index fcd7ae29049d..ed2782c53a93 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3126,9 +3126,6 @@ static enum hdmi_picture_aspect
drm_get_hdmi_aspect_ratio(const u8 video_code)
static unsigned int
hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)
{
- if (hdmi_mode->vdisplay == 4096 && hdmi_mode->hdisplay == 2160)
- return hdmi_mode->clock;
-
return cea_mode_alternate_clock(hdmi_mode);
}
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/edid: Add aspect ratios to HDMI 4K modes
[Why]
HDMI 2.0 adds aspect ratio attribute to distinguish different
4k modes. According to Appendix E of HDMI 2.0 spec, source should
use VSIF to indicate video mode only when the mode is one defined
in HDMI 1.4b 4K modes. Otherwise, use AVI infoframes to convey VIC.
Current code doesn't take aspect ratio into consideration while
constructing avi infoframe. Should modify that.
[How]
Inherit Ville Syrjälä's work
"drm/edid: Prep for HDMI VIC aspect ratio" at
https://patchwork.kernel.org/patch/11174639/
Add picture_aspect_ratio attributes to edid_4k_modes[] and
construct VIC and HDMI_VIC by taking aspect ratio into
consideration.
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_edid.c | 45 +-
1 file changed, 35 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 77a39fc76045..fcd7ae29049d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1288,25 +1288,25 @@ static const struct drm_display_mode edid_4k_modes[] = {
3840, 4016, 4104, 4400, 0,
2160, 2168, 2178, 2250, 0,
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
- .vrefresh = 30, },
+ .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
/* 2 - 3840x2160@25Hz */
{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
3840, 4896, 4984, 5280, 0,
2160, 2168, 2178, 2250, 0,
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
- .vrefresh = 25, },
+ .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
/* 3 - 3840x2160@24Hz */
{ DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
3840, 5116, 5204, 5500, 0,
2160, 2168, 2178, 2250, 0,
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
- .vrefresh = 24, },
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
/* 4 - 4096x2160@24Hz (SMPTE) */
{ DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000,
4096, 5116, 5204, 5500, 0,
2160, 2168, 2178, 2250, 0,
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
- .vrefresh = 24, },
+ .vrefresh = 24, HDMI_PICTURE_ASPECT_256_135},
};
/*** DDC fetch and block validation ***/
@@ -3110,6 +3110,11 @@ static enum hdmi_picture_aspect
drm_get_cea_aspect_ratio(const u8 video_code)
return edid_cea_modes[video_code].picture_aspect_ratio;
}
+static enum hdmi_picture_aspect drm_get_hdmi_aspect_ratio(const u8 video_code)
+{
+ return edid_4k_modes[video_code].picture_aspect_ratio;
+}
+
/*
* Calculate the alternate clock for HDMI modes (those from the HDMI vendor
* specific block).
@@ -3136,6 +3141,9 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const
struct drm_display_mode *to_
if (!to_match->clock)
return 0;
+ if (to_match->picture_aspect_ratio)
+ match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];
unsigned int clock1, clock2;
@@ -3171,6 +3179,9 @@ static u8 drm_match_hdmi_mode(const struct
drm_display_mode *to_match)
if (!to_match->clock)
return 0;
+ if (to_match->picture_aspect_ratio)
+ match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];
unsigned int clock1, clock2;
@@ -5118,6 +5129,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
const struct drm_display_mode *mode)
{
enum hdmi_picture_aspect picture_aspect;
+ u8 vic, hdmi_vic;
int err;
if (!frame || !mode)
@@ -5130,7 +5142,8 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
frame->pixel_repeat = 1;
- frame->video_code = drm_mode_cea_vic(connector, mode);
+ vic = drm_mode_cea_vic(connector, mode);
+ hdmi_vic = drm_mode_hdmi_vic(connector, mode);
frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
@@ -5144,11 +5157,15 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
hdmi_avi_infoframe *frame,
/*
* Populate picture aspect ratio from either
-* user input (if specified) or from the CEA mode list.
+* user input (if specified) or from the CEA/HDMI mode lists.
*/
picture_aspect = mode->picture_aspect_ratio;
- if (picture_aspect == HDMI_PICTURE_ASPECT
[PATCH] drm/drm_edid: correct VIC and HDMI_VIC under HDMI 2.0
In HDMI 1.4 defines 4k modes without specific aspect ratio.
However, in HDMI 2.0, adds aspect ratio attribute to distinguish different
4k modes.
According to Appendix E of HDMI 2.0 spec, source should use VSIF to
indicate VIC mode only when the mode is one defined in HDMI 1.4b 4K modes.
Otherwise, use AVI infoframes to convey VIC.
eg: VIC_103 should use AVI infoframes and VIC_93 use VSIF
When the sink is HDMI 2.0, current code in
drm_hdmi_avi_infoframe_from_display_mode will also force mode VIC_103 to
have VIC value 0. This violates the spec and needs to be corrected.
The same situation occurs in drm_hdmi_vendor_infoframe_from_display_mode
and should set HDMI_VIC when the mode is one defined in HDMI 1.4b 4K
modes.
---
drivers/gpu/drm/drm_edid.c | 95 --
1 file changed, 92 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 649cfd8b4200..0fea9bf4ec67 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1306,6 +1306,37 @@ static const struct drm_display_mode edid_4k_modes[] = {
.vrefresh = 24, },
};
+/*
+ * 4k modes of HDMI 1.4 defined in HDMI 2.0. Index using the VIC.
+ */
+static const struct drm_display_mode hdmi_1_4_edid_4k_modes[] = {
+ /* 0 - dummy, VICs start at 1 */
+ { },
+ /* 1 - 3840x2160@30Hz */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
+ 3840, 4016, 4104, 4400, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
+ /* 2 - 3840x2160@25Hz */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
+ 3840, 4896, 4984, 5280, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
+ /* 3 - 3840x2160@24Hz */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
+ 3840, 5116, 5204, 5500, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
+ /* 4 - 4096x2160@24Hz (SMPTE) */
+ { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000,
+ 4096, 5116, 5204, 5500, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135,
},
+};
/*** DDC fetch and block validation ***/
static const u8 edid_header[] = {
@@ -3061,6 +3092,19 @@ hdmi_mode_alternate_clock(const struct drm_display_mode
*hdmi_mode)
return cea_mode_alternate_clock(hdmi_mode);
}
+/*
+ * Calculate the alternate clock for HDMI modes (those from the HDMI vendor
+ * specific block).
+ *
+ * It's almost like hdmi_mode_alternate_clock(), but no exception for VIC 4
mode.
+ * There is an alternate clock (23.98Hz) of VIC 4 mode (4096x2160@24Hz) in
HDMI 2.0
+ */
+static unsigned int
+hdmi_1_4_mode_alternate_clock(const struct drm_display_mode *hdmi_mode)
+{
+ return cea_mode_alternate_clock(hdmi_mode);
+}
+
static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode
*to_match,
unsigned int clock_tolerance)
{
@@ -3121,11 +3165,53 @@ static u8 drm_match_hdmi_mode(const struct
drm_display_mode *to_match)
return 0;
}
+/*
+ * drm_match_hdmi_1_4_mode - look for a HDMI 1.4 mode matching given mode
+ * @to_match: display mode
+ *
+ * An HDMI mode is one defined in the HDMI vendor specific block.
+ * In HDMI 2.0, only few 4k resolutions with specific aspect ratio should
+ * utilize H14b VSIF.
+ *
+ * Returns the HDMI Video ID (VIC) of the mode or 0 if it isn't one.
+ */
+static u8 drm_match_hdmi_1_4_mode(const struct drm_display_mode *to_match)
+{
+ unsigned int match_flags = DRM_MODE_MATCH_TIMINGS |
DRM_MODE_MATCH_FLAGS;
+ u8 vic;
+
+ if (!to_match->clock)
+ return 0;
+
+ if (to_match->picture_aspect_ratio)
+ match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
+ for (vic = 1; vic < ARRAY_SIZE(hdmi_1_4_edid_4k_modes); vic++) {
+ const struct drm_display_mode *hdmi_1_4_mode =
&hdmi_1_4_edid_4k_modes[vic];
+ unsigned int clock1, clock2;
+
+ /* Make sure to also match alternate clocks */
+ clock1 = hdmi_1_4_mode->clock;
+ clock2 = hdmi_1_4_mode_alternate_clock(hdmi_1_4_mode);
+
+ if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) ||
+KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) &&
+ drm_mode_match(to_match, hdmi_1_4_mode, match_flags))
+ return vic;
+ }
+
[PATCH] drm/drm_connector: add additional aspect ratio values
For HDMI2.0 CTS item - HF1-35, it verifies if the source generates
video timing "64:27" video format correctly.
eg: (vic-76) 1920x1080p@60Hz,24bpp
This patch add on "64:27" and "256:135" to drm_aspect_ratio_enum_list.
Thereafter, one can specify the aspect ratio to "64:27" or "256:135"
after creating aspect ratio property.
Change-Id: Ifc9df54e8e8f78e70960fcd737a3a57e49c81152
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_connector.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 3a0cacb71235..c0629a01d08e 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -772,6 +772,8 @@ static const struct drm_prop_enum_list
drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" },
{ DRM_MODE_PICTURE_ASPECT_4_3, "4:3" },
{ DRM_MODE_PICTURE_ASPECT_16_9, "16:9" },
+ { DRM_MODE_PICTURE_ASPECT_64_27, "64:27" },
+ { DRM_MODE_PICTURE_ASPECT_256_135, "256:135" },
};
static const struct drm_prop_enum_list drm_content_type_enum_list[] = {
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
