From c4e4fccc5d52d881afaac11d3353265ef4eccb8b Mon Sep 17 00:00:00 2001 From: Wayne Lin Date: Fri, 3 Jan 2020 13:50:01 +0800 Subject: 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: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper (v0.6)") Reviewed-by: Harry Wentland Signed-off-by: Wayne Lin Cc: stable@vger.kernel.org Signed-off-by: Lyude Paul Link: https://patchwork.freedesktop.org/patch/msgid/20200103055001.10287-1-Wayne.Lin@amd.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 273dd80fabf3..644c72f9c594 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++; } -- cgit v1.2.3 From dc48529fb14ee8450705c00d91f4dcc155e1c2cb Mon Sep 17 00:00:00 2001 From: David Francis Date: Wed, 21 Aug 2019 10:33:26 -0400 Subject: drm/dp_mst: Add PBN calculation for DSC modes With DSC, bpp can be fractional in multiples of 1/16. Change drm_dp_calc_pbn_mode to reflect this, adding a new parameter bool dsc. When this parameter is true, treat the bpp parameter as having units not of bits per pixel, but 1/16 of a bit per pixel v2: Don't add separate function for this v3: In the equation divide bpp by 16 as it is expected not to leave any remainder v4: Added DSC test parameters for selftest Reviewed-by: Manasi Navare Reviewed-by: Lyude Paul Reviewed-by: Harry Wentland Signed-off-by: David Francis Signed-off-by: Mikita Lipski Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index e68d23043973..2443532341d0 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4415,10 +4415,11 @@ EXPORT_SYMBOL(drm_dp_check_act_status); * drm_dp_calc_pbn_mode() - Calculate the PBN for a mode. * @clock: dot clock for the mode * @bpp: bpp for the mode. + * @dsc: DSC mode. If true, bpp has units of 1/16 of a bit per pixel * * This uses the formula in the spec to calculate the PBN value for a mode. */ -int drm_dp_calc_pbn_mode(int clock, int bpp) +int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc) { /* * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006 @@ -4429,7 +4430,16 @@ int drm_dp_calc_pbn_mode(int clock, int bpp) * peak_kbps *= (1006/1000) * peak_kbps *= (64/54) * peak_kbps *= 8 convert to bytes + * + * If the bpp is in units of 1/16, further divide by 16. Put this + * factor in the numerator rather than the denominator to avoid + * integer overflow */ + + if (dsc) + return DIV_ROUND_UP_ULL(mul_u32_u32(clock * (bpp / 16), 64 * 1006), + 8 * 54 * 1000 * 1000); + return DIV_ROUND_UP_ULL(mul_u32_u32(clock * bpp, 64 * 1006), 8 * 54 * 1000 * 1000); } -- cgit v1.2.3 From a3c2b0ffc007e5d98f5313ed951fff092535fb6d Mon Sep 17 00:00:00 2001 From: David Francis Date: Thu, 6 Jun 2019 11:20:10 -0400 Subject: drm/dp_mst: Parse FEC capability on MST ports As of DP1.4, ENUM_PATH_RESOURCES returns a bit indicating if FEC can be supported up to that point in the MST network. The bit is the first byte of the ENUM_PATH_RESOURCES ack reply, bottom-most bit (refer to section 2.11.9.4 of DP standard, v1.4) That value is needed for FEC and DSC support Store it on drm_dp_mst_port Reviewed-by: Lyude Paul Reviewed-by: Harry Wentland Signed-off-by: David Francis Signed-off-by: Mikita Lipski Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 2443532341d0..77d95e2de897 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -853,6 +853,7 @@ static bool drm_dp_sideband_parse_enum_path_resources_ack(struct drm_dp_sideband { int idx = 1; repmsg->u.path_resources.port_number = (raw->msg[idx] >> 4) & 0xf; + repmsg->u.path_resources.fec_capable = raw->msg[idx] & 0x1; idx++; if (idx > raw->curlen) goto fail_len; @@ -2951,6 +2952,7 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr, path_res->avail_payload_bw_number); port->available_pbn = path_res->avail_payload_bw_number; + port->fec_capable = path_res->fec_capable; } } -- cgit v1.2.3 From b1dee9a716b532566570052686144bfbba62aaf6 Mon Sep 17 00:00:00 2001 From: David Francis Date: Thu, 25 Jul 2019 11:47:46 -0400 Subject: drm/dp_mst: Fill branch->num_ports This field on drm_dp_mst_branch was never filled It is initialized to zero when the port is kzallocced. When a port is added to the list, increment num_ports, and when a port is removed from the list, decrement num_ports. v2: remember to decrement on port removal v3: don't explicitly init to 0 v4: move decrement of num_ports to unlink_port function Reviewed-by: Lyude Paul Reviewed-by: Harry Wentland Signed-off-by: David Francis Signed-off-by: Mikita Lipski Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 77d95e2de897..5b3dfceab365 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2175,6 +2175,7 @@ drm_dp_mst_topology_unlink_port(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port) { mutex_lock(&mgr->lock); + port->parent->num_ports--; list_del(&port->next); mutex_unlock(&mgr->lock); drm_dp_mst_topology_put_port(port); @@ -2274,6 +2275,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb, mutex_lock(&mgr->lock); drm_dp_mst_topology_get_port(port); list_add(&port->next, &mstb->ports); + mstb->num_ports++; mutex_unlock(&mgr->lock); } -- cgit v1.2.3 From c2bc1b6eabe65d6bf26a892d803907dca9097311 Mon Sep 17 00:00:00 2001 From: David Francis Date: Mon, 26 Aug 2019 09:50:28 -0400 Subject: drm/dp_mst: Add helpers for MST DSC and virtual DPCD aux Add drm_dp_mst_dsc_aux_for_port. To enable DSC, the DSC_ENABLED register might have to be written on the leaf port's DPCD, its parent's DPCD, or the MST manager's DPCD. This function finds the correct aux for the job. As part of this, add drm_dp_mst_is_virtual_dpcd. Virtual DPCD is a DP feature new in DP v1.4, which exposes certain DPCD registers on virtual ports. v2: Remember to unlock mutex on all paths v3: Refactor to match coding style and increase brevity v4: - Check DSC capable MST sink connected directly to the device. - Check branch's port_parent to be set Cc: Lyude Paul Reviewed-by: Wenjing Liu Signed-off-by: David Francis Signed-off-by: Mikita Lipski Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 143 ++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 5b3dfceab365..2b20caa4b7e3 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -5076,3 +5076,146 @@ static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux) { i2c_del_adapter(&aux->ddc); } + +/** + * drm_dp_mst_is_virtual_dpcd() - Is the given port a virtual DP Peer Device + * @port: The port to check + * + * A single physical MST hub object can be represented in the topology + * by multiple branches, with virtual ports between those branches. + * + * As of DP1.4, An MST hub with internal (virtual) ports must expose + * certain DPCD registers over those ports. See sections 2.6.1.1.1 + * and 2.6.1.1.2 of Display Port specification v1.4 for details. + * + * May acquire mgr->lock + * + * Returns: + * true if the port is a virtual DP peer device, false otherwise + */ +static bool drm_dp_mst_is_virtual_dpcd(struct drm_dp_mst_port *port) +{ + struct drm_dp_mst_port *downstream_port; + + if (!port || port->dpcd_rev < DP_DPCD_REV_14) + return false; + + /* Virtual DP Sink (Internal Display Panel) */ + if (port->port_num >= 8) + return true; + + /* DP-to-HDMI Protocol Converter */ + if (port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV && + !port->mcs && + port->ldps) + return true; + + /* DP-to-DP */ + mutex_lock(&port->mgr->lock); + if (port->pdt == DP_PEER_DEVICE_MST_BRANCHING && + port->mstb && + port->mstb->num_ports == 2) { + list_for_each_entry(downstream_port, &port->mstb->ports, next) { + if (downstream_port->pdt == DP_PEER_DEVICE_SST_SINK && + !downstream_port->input) { + mutex_unlock(&port->mgr->lock); + return true; + } + } + } + mutex_unlock(&port->mgr->lock); + + return false; +} + +/** + * drm_dp_mst_dsc_aux_for_port() - Find the correct aux for DSC + * @port: The port to check. A leaf of the MST tree with an attached display. + * + * Depending on the situation, DSC may be enabled via the endpoint aux, + * the immediately upstream aux, or the connector's physical aux. + * + * This is both the correct aux to read DSC_CAPABILITY and the + * correct aux to write DSC_ENABLED. + * + * This operation can be expensive (up to four aux reads), so + * the caller should cache the return. + * + * Returns: + * NULL if DSC cannot be enabled on this port, otherwise the aux device + */ +struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port) +{ + struct drm_dp_mst_port *immediate_upstream_port; + struct drm_dp_mst_port *fec_port; + u8 endpoint_fec; + u8 endpoint_dsc; + + if (!port) + return NULL; + + if (port->parent->port_parent) + immediate_upstream_port = port->parent->port_parent; + else + immediate_upstream_port = NULL; + + fec_port = immediate_upstream_port; + while (fec_port) { + /* + * Each physical link (i.e. not a virtual port) between the + * output and the primary device must support FEC + */ + if (!drm_dp_mst_is_virtual_dpcd(fec_port) && + !fec_port->fec_capable) + return NULL; + + fec_port = fec_port->parent->port_parent; + } + + /* DP-to-DP peer device */ + if (drm_dp_mst_is_virtual_dpcd(immediate_upstream_port)) { + u8 upstream_dsc; + + if (drm_dp_dpcd_read(&port->aux, + DP_DSC_SUPPORT, &endpoint_dsc, 1) != 1) + return NULL; + if (drm_dp_dpcd_read(&port->aux, + DP_FEC_CAPABILITY, &endpoint_fec, 1) != 1) + return NULL; + if (drm_dp_dpcd_read(&immediate_upstream_port->aux, + DP_DSC_SUPPORT, &upstream_dsc, 1) != 1) + return NULL; + + /* Enpoint decompression with DP-to-DP peer device */ + if ((endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED) && + (endpoint_fec & DP_FEC_CAPABLE) && + (upstream_dsc & 0x2) /* DSC passthrough */) + return &port->aux; + + /* Virtual DPCD decompression with DP-to-DP peer device */ + return &immediate_upstream_port->aux; + } + + /* Virtual DPCD decompression with DP-to-HDMI or Virtual DP Sink */ + if (drm_dp_mst_is_virtual_dpcd(port)) + return &port->aux; + + /* + * The check below verifies if the MST sink + * connected to the GPU is capable of DSC - + * therefore the endpoint needs to be + * both DSC and FEC capable. + */ + if (drm_dp_dpcd_read(&port->aux, + DP_DSC_SUPPORT, &endpoint_dsc, 1) != 1) + return NULL; + if (drm_dp_dpcd_read(&port->aux, + DP_FEC_CAPABILITY, &endpoint_fec, 1) != 1) + return NULL; + if ((endpoint_dsc & DP_DSC_DECOMPRESSION_IS_SUPPORTED) && + (endpoint_fec & DP_FEC_CAPABLE)) + return &port->aux; + + return NULL; +} +EXPORT_SYMBOL(drm_dp_mst_dsc_aux_for_port); -- cgit v1.2.3 From 5b03f9d86880711441c5681244d352212460a595 Mon Sep 17 00:00:00 2001 From: Mikita Lipski Date: Fri, 20 Sep 2019 15:44:56 -0400 Subject: drm/dp_mst: Add new quirk for Synaptics MST hubs Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not support virtual DPCD registers, but do support DSC. The DSC caps can be read from the physical aux, like in SST DSC. These hubs have many different DEVICE_IDs. Add a new quirk to detect this case. v2: Fix error when checking return of drm_dp_read_desc Reviewed-by: Wenjing Liu Reviewed-by: Lyude Paul Signed-off-by: David Francis Signed-off-by: Mikita Lipski Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 2b20caa4b7e3..480c71a00cf2 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -5148,6 +5148,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port) { struct drm_dp_mst_port *immediate_upstream_port; struct drm_dp_mst_port *fec_port; + struct drm_dp_desc desc = { 0 }; u8 endpoint_fec; u8 endpoint_dsc; @@ -5200,6 +5201,32 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port) if (drm_dp_mst_is_virtual_dpcd(port)) return &port->aux; + /* + * Synaptics quirk + * Applies to ports for which: + * - Physical aux has Synaptics OUI + * - DPv1.4 or higher + * - Port is on primary branch device + * - Not a VGA adapter (DP_DWN_STRM_PORT_TYPE_ANALOG) + */ + if (drm_dp_read_desc(port->mgr->aux, &desc, true)) + return NULL; + + if (drm_dp_has_quirk(&desc, DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) && + port->mgr->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14 && + port->parent == port->mgr->mst_primary) { + u8 downstreamport; + + if (drm_dp_dpcd_read(&port->aux, DP_DOWNSTREAMPORT_PRESENT, + &downstreamport, 1) < 0) + return NULL; + + if ((downstreamport & DP_DWN_STRM_PORT_PRESENT) && + ((downstreamport & DP_DWN_STRM_PORT_TYPE_MASK) + != DP_DWN_STRM_PORT_TYPE_ANALOG)) + return port->mgr->aux; + } + /* * The check below verifies if the MST sink * connected to the GPU is capable of DSC - -- cgit v1.2.3 From 1c6c1cb5afc77cc8afbe563937c3bd1a41172459 Mon Sep 17 00:00:00 2001 From: Mikita Lipski Date: Thu, 14 Nov 2019 16:24:29 -0500 Subject: drm/dp_mst: Manually overwrite PBN divider for calculating timeslots [why] For DSC case we cannot use topology manager's PBN divider variable. The default divider does not take FEC into account. Therefore the driver has to calculate its own divider based on the link rate and lane count its handling, as it is hw specific. [how] Pass pbn_div as an argument, which is used if its more than zero, otherwise default topology manager's pbn_div will be used. Reviewed-by: Lyude Paul Signed-off-by: Mikita Lipski Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 480c71a00cf2..90201827781d 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4093,6 +4093,7 @@ static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr, * @mgr: MST topology manager for the port * @port: port to find vcpi slots for * @pbn: bandwidth required for the mode in PBN + * @pbn_div: divider for DSC mode that takes FEC into account * * Allocates VCPI slots to @port, replacing any previous VCPI allocations it * may have had. Any atomic drivers which support MST must call this function @@ -4119,7 +4120,8 @@ static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr, */ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr, - struct drm_dp_mst_port *port, int pbn) + struct drm_dp_mst_port *port, int pbn, + int pbn_div) { struct drm_dp_mst_topology_state *topology_state; struct drm_dp_vcpi_allocation *pos, *vcpi = NULL; @@ -4152,7 +4154,10 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, if (!vcpi) prev_slots = 0; - req_slots = DIV_ROUND_UP(pbn, mgr->pbn_div); + if (pbn_div <= 0) + pbn_div = mgr->pbn_div; + + req_slots = DIV_ROUND_UP(pbn, pbn_div); DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] [MST PORT:%p] VCPI %d -> %d\n", port->connector->base.id, port->connector->name, -- cgit v1.2.3 From 8afb7e6afadb36b160143794c1d2fdfbde189750 Mon Sep 17 00:00:00 2001 From: Mikita Lipski Date: Mon, 28 Oct 2019 17:33:32 -0400 Subject: drm/dp_mst: Add DSC enablement helpers to DRM Adding a helper function to be called by drivers outside of DRM to enable DSC on the MST ports. Function is called to recalculate VCPI allocation if DSC is enabled and raise the DSC flag to enable. In case of disabling DSC the flag is set to false and recalculation of VCPI slots is expected to be done in encoder's atomic_check. v2: squash separate functions into one and call it per port v3: Fix comment typos Reviewed-by: Lyude Paul Signed-off-by: Mikita Lipski Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 61 +++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 90201827781d..9f7e714c5182 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4789,6 +4789,67 @@ drm_dp_mst_atomic_check_topology_state(struct drm_dp_mst_topology_mgr *mgr, return 0; } +/** + * drm_dp_mst_atomic_enable_dsc - Set DSC Enable Flag to On/Off + * @state: Pointer to the new drm_atomic_state + * @port: Pointer to the affected MST Port + * @pbn: Newly recalculated bw required for link with DSC enabled + * @pbn_div: Divider to calculate correct number of pbn per slot + * @enable: Boolean flag to enable or disable DSC on the port + * + * This function enables DSC on the given Port + * by recalculating its vcpi from pbn provided + * and sets dsc_enable flag to keep track of which + * ports have DSC enabled + * + */ +int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state, + struct drm_dp_mst_port *port, + int pbn, int pbn_div, + bool enable) +{ + struct drm_dp_mst_topology_state *mst_state; + struct drm_dp_vcpi_allocation *pos; + bool found = false; + int vcpi = 0; + + mst_state = drm_atomic_get_mst_topology_state(state, port->mgr); + + if (IS_ERR(mst_state)) + return PTR_ERR(mst_state); + + list_for_each_entry(pos, &mst_state->vcpis, next) { + if (pos->port == port) { + found = true; + break; + } + } + + if (!found) { + DRM_DEBUG_ATOMIC("[MST PORT:%p] Couldn't find VCPI allocation in mst state %p\n", + port, mst_state); + return -EINVAL; + } + + if (pos->dsc_enabled == enable) { + DRM_DEBUG_ATOMIC("[MST PORT:%p] DSC flag is already set to %d, returning %d VCPI slots\n", + port, enable, pos->vcpi); + vcpi = pos->vcpi; + } + + if (enable) { + vcpi = drm_dp_atomic_find_vcpi_slots(state, port->mgr, port, pbn, pbn_div); + DRM_DEBUG_ATOMIC("[MST PORT:%p] Enabling DSC flag, reallocating %d VCPI slots on the port\n", + port, vcpi); + if (vcpi < 0) + return -EINVAL; + } + + pos->dsc_enabled = enable; + + return vcpi; +} +EXPORT_SYMBOL(drm_dp_mst_atomic_enable_dsc); /** * drm_dp_mst_atomic_check - Check that the new state of an MST topology in an * atomic update is valid -- cgit v1.2.3 From cd82d82cbc0484e47918d3166f356c98f0066db8 Mon Sep 17 00:00:00 2001 From: Mikita Lipski Date: Wed, 6 Nov 2019 13:11:23 -0500 Subject: drm/dp_mst: Add branch bandwidth validation to MST atomic check [why] Adding PBN attribute to drm_dp_vcpi_allocation structure to keep track of how much bandwidth each Port requires. Adding drm_dp_mst_atomic_check_bw_limit to verify that state's bandwidth needs doesn't exceed available bandwidth. The funtion is called in drm_dp_mst_atomic_check after drm_dp_mst_atomic_check_topology_state to fully verify that the proposed topology is supported. v2: Fixing some typos and indenting v3: Return correct error enums if no bw space available Reviewed-by: Lyude Paul Signed-off-by: Mikita Lipski Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 66 +++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 9f7e714c5182..c7bea8a95797 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4125,7 +4125,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, { struct drm_dp_mst_topology_state *topology_state; struct drm_dp_vcpi_allocation *pos, *vcpi = NULL; - int prev_slots, req_slots; + int prev_slots, prev_bw, req_slots; topology_state = drm_atomic_get_mst_topology_state(state, mgr); if (IS_ERR(topology_state)) @@ -4136,6 +4136,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, if (pos->port == port) { vcpi = pos; prev_slots = vcpi->vcpi; + prev_bw = vcpi->pbn; /* * This should never happen, unless the driver tries @@ -4151,8 +4152,10 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, break; } } - if (!vcpi) + if (!vcpi) { prev_slots = 0; + prev_bw = 0; + } if (pbn_div <= 0) pbn_div = mgr->pbn_div; @@ -4162,6 +4165,9 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] [MST PORT:%p] VCPI %d -> %d\n", port->connector->base.id, port->connector->name, port, prev_slots, req_slots); + DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] [MST PORT:%p] PBN %d -> %d\n", + port->connector->base.id, port->connector->name, + port, prev_bw, pbn); /* Add the new allocation to the state */ if (!vcpi) { @@ -4174,6 +4180,7 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, list_add(&vcpi->next, &topology_state->vcpis); } vcpi->vcpi = req_slots; + vcpi->pbn = pbn; return req_slots; } @@ -4750,6 +4757,58 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj, kfree(mst_state); } +static bool drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port *port, + struct drm_dp_mst_branch *branch) +{ + while (port->parent) { + if (port->parent == branch) + return true; + + if (port->parent->port_parent) + port = port->parent->port_parent; + else + break; + } + return false; +} + +static inline +int drm_dp_mst_atomic_check_bw_limit(struct drm_dp_mst_branch *branch, + struct drm_dp_mst_topology_state *mst_state) +{ + struct drm_dp_mst_port *port; + struct drm_dp_vcpi_allocation *vcpi; + int pbn_limit = 0, pbn_used = 0; + + list_for_each_entry(port, &branch->ports, next) { + if (port->mstb) + if (drm_dp_mst_atomic_check_bw_limit(port->mstb, mst_state)) + return -ENOSPC; + + if (port->available_pbn > 0) + pbn_limit = port->available_pbn; + } + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] branch has %d PBN available\n", + branch, pbn_limit); + + list_for_each_entry(vcpi, &mst_state->vcpis, next) { + if (!vcpi->pbn) + continue; + + if (drm_dp_mst_port_downstream_of_branch(vcpi->port, branch)) + pbn_used += vcpi->pbn; + } + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] branch used %d PBN\n", + branch, pbn_used); + + if (pbn_used > pbn_limit) { + DRM_DEBUG_ATOMIC("[MST BRANCH:%p] No available bandwidth\n", + branch); + return -ENOSPC; + } + return 0; +} + static inline int drm_dp_mst_atomic_check_topology_state(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_topology_state *mst_state) @@ -4881,6 +4940,9 @@ int drm_dp_mst_atomic_check(struct drm_atomic_state *state) ret = drm_dp_mst_atomic_check_topology_state(mgr, mst_state); if (ret) break; + ret = drm_dp_mst_atomic_check_bw_limit(mgr->mst_primary, mst_state); + if (ret) + break; } return ret; -- cgit v1.2.3 From 9e5b959036ff76dda397998d1185b0ec0ae58568 Mon Sep 17 00:00:00 2001 From: Mikita Lipski Date: Fri, 13 Dec 2019 10:29:48 -0500 Subject: drm/dp_mst: Rename drm_dp_mst_atomic_check_topology_state [why] drm_dp_mst_atomic_check_topology_state() should be renamed to reflect more specific type of check. Since it is verifying payload allocation limit it should be renamed into drm_dp_mst_atomic_check_vcpi_alloc_limit() Reviewed-by: Lyude Paul Signed-off-by: Mikita Lipski Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index c7bea8a95797..6a7078e538e5 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4810,8 +4810,8 @@ int drm_dp_mst_atomic_check_bw_limit(struct drm_dp_mst_branch *branch, } static inline int -drm_dp_mst_atomic_check_topology_state(struct drm_dp_mst_topology_mgr *mgr, - struct drm_dp_mst_topology_state *mst_state) +drm_dp_mst_atomic_check_vcpi_alloc_limit(struct drm_dp_mst_topology_mgr *mgr, + struct drm_dp_mst_topology_state *mst_state) { struct drm_dp_vcpi_allocation *vcpi; int avail_slots = 63, payload_count = 0; @@ -4937,7 +4937,7 @@ int drm_dp_mst_atomic_check(struct drm_atomic_state *state) int i, ret = 0; for_each_new_mst_mgr_in_state(state, mgr, mst_state, i) { - ret = drm_dp_mst_atomic_check_topology_state(mgr, mst_state); + ret = drm_dp_mst_atomic_check_vcpi_alloc_limit(mgr, mst_state); if (ret) break; ret = drm_dp_mst_atomic_check_bw_limit(mgr->mst_primary, mst_state); -- cgit v1.2.3 From 8ec046716ca8ee79a9d2699f78511c08753c2e56 Mon Sep 17 00:00:00 2001 From: Mikita Lipski Date: Sat, 16 Nov 2019 13:32:15 -0500 Subject: drm/dp_mst: Add helper to trigger modeset on affected DSC MST CRTCs [why] Whenever a connector on an MST network is changed or undergoes a modeset, the DSC configs for each stream on that topology will be recalculated. This can change their required bandwidth, requiring a full reprogramming, as though a modeset was performed, even if that stream did not change timing. [how] Adding helper to trigger modesets on MST DSC connectors by setting mode_changed flag on CRTCs in the same topology as affected connector v2: use drm_dp_mst_dsc_aux_for_port function to verify if the port is DSC capable v3: - added _must_check attribute - removed topology manager check - fix typos and indentations Reviewed-by: Lyude Paul Signed-off-by: Mikita Lipski Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 61 +++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 6a7078e538e5..7d174ba78eba 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4848,6 +4848,67 @@ drm_dp_mst_atomic_check_vcpi_alloc_limit(struct drm_dp_mst_topology_mgr *mgr, return 0; } +/** + * drm_dp_mst_add_affected_dsc_crtcs + * @state: Pointer to the new struct drm_dp_mst_topology_state + * @port: Port pointer of connector with new state + * + * Whenever there is a change in mst topology + * DSC configuration would have to be recalculated + * therefore we need to trigger modeset on all affected + * CRTCs in that topology + * + * See also: + * drm_dp_mst_atomic_enable_dsc() + */ +int drm_dp_mst_add_affected_dsc_crtcs(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr) +{ + struct drm_dp_mst_topology_state *mst_state; + struct drm_dp_vcpi_allocation *pos; + struct drm_connector *connector; + struct drm_connector_state *conn_state; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + + mst_state = drm_atomic_get_mst_topology_state(state, mgr); + + if (IS_ERR(mst_state)) + return -EINVAL; + + list_for_each_entry(pos, &mst_state->vcpis, next) { + + connector = pos->port->connector; + + if (!connector) + return -EINVAL; + + conn_state = drm_atomic_get_connector_state(state, connector); + + if (IS_ERR(conn_state)) + return PTR_ERR(conn_state); + + crtc = conn_state->crtc; + + if (WARN_ON(!crtc)) + return -EINVAL; + + if (!drm_dp_mst_dsc_aux_for_port(pos->port)) + continue; + + crtc_state = drm_atomic_get_crtc_state(mst_state->base.state, crtc); + + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); + + DRM_DEBUG_ATOMIC("[MST MGR:%p] Setting mode_changed flag on CRTC %p\n", + mgr, crtc); + + crtc_state->mode_changed = true; + } + return 0; +} +EXPORT_SYMBOL(drm_dp_mst_add_affected_dsc_crtcs); + /** * drm_dp_mst_atomic_enable_dsc - Set DSC Enable Flag to On/Off * @state: Pointer to the new drm_atomic_state -- cgit v1.2.3 From c908b1c4bb56c27fa6fbb48e2d3dbebdad366706 Mon Sep 17 00:00:00 2001 From: "David (Dingchen) Zhang" Date: Fri, 6 Dec 2019 17:56:37 -0500 Subject: drm: add dp helper to initialize remote aux channel. [why] We need to minimally initialize the remote aux channel, e.g. the crc work struct of remote aux to dump the sink's DPRX CRCs in MST setup. [how] Add helper that only initializes the crc work struct of the remote aux, hooke crc work queue to 'drm_dp_aux_crc_work'. Then call this helper in DP MST port initialization. This, plus David Francis' patch [1], fix the issue of MST remote aux DPCD CRCs read. [1] https://patchwork.kernel.org/patch/11217941/ Cc: Leo Li Cc: Harry Wentland Signed-off-by: David (Dingchen) Zhang Reviewed-by: Harry Wentland Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 7d174ba78eba..fa0656697c72 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2200,6 +2200,9 @@ drm_dp_mst_add_port(struct drm_device *dev, port->aux.dev = dev->dev; port->aux.is_remote = true; + /* initialize the MST downstream port's AUX crc work queue */ + drm_dp_remote_aux_init(&port->aux); + /* * Make sure the memory allocation for our parent branch stays * around until our own memory allocation is released -- cgit v1.2.3 From 9edb435aed968a2c792f6d847f524587697edf37 Mon Sep 17 00:00:00 2001 From: Alex Deucher Date: Wed, 8 Jan 2020 22:21:30 -0500 Subject: drm/dp_mst: fix documentation of drm_dp_mst_add_affected_dsc_crtcs the parameter is the mst manager, not the port. Reviewed-by: Mikita Lipski Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index fa0656697c72..5d3c1d379277 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4854,7 +4854,7 @@ drm_dp_mst_atomic_check_vcpi_alloc_limit(struct drm_dp_mst_topology_mgr *mgr, /** * drm_dp_mst_add_affected_dsc_crtcs * @state: Pointer to the new struct drm_dp_mst_topology_state - * @port: Port pointer of connector with new state + * @mgr: MST topology manager * * Whenever there is a change in mst topology * DSC configuration would have to be recalculated -- cgit v1.2.3 From 7617e9621bf2ca3bb58d5f216cc35c86b029105d Mon Sep 17 00:00:00 2001 From: Wayne Lin Date: Mon, 6 Jan 2020 18:21:58 +0800 Subject: 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 Signed-off-by: Lyude Paul [removed cc for stable - there's too many patches this depends on for this to backport cleanly] Link: https://patchwork.freedesktop.org/patch/msgid/20200106102158.28261-1-Wayne.Lin@amd.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 644c72f9c594..03874ee2a033 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,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) -- cgit v1.2.3 From 5a64967a2f3bbc01cc708ee43c7b0893089c61c4 Mon Sep 17 00:00:00 2001 From: Wayne Lin Date: Mon, 13 Jan 2020 17:36:49 +0800 Subject: 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 Signed-off-by: Lyude Paul Link: https://patchwork.freedesktop.org/patch/msgid/20200113093649.11755-1-Wayne.Lin@amd.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 03874ee2a033..5a61a5596912 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1190,6 +1190,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)) { @@ -1199,6 +1201,7 @@ out: } mutex_unlock(&mgr->qlock); + drm_dp_mst_kick_tx(mgr); return ret; } @@ -2741,9 +2744,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; @@ -2783,7 +2788,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); } @@ -3701,6 +3707,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); @@ -3710,6 +3717,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; @@ -4520,7 +4530,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); } -- cgit v1.2.3 From 7b19914383fc008a6b51871f18da72cf9aa43cae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Roberto=20de=20Souza?= Date: Thu, 16 Jan 2020 17:58:34 -0800 Subject: drm/mst: Don't do atomic checks over disabled managers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a main MST port is disconnected drivers should call drm_dp_mst_topology_mgr_set_mst() disabling the MST manager, this function will set manager mst_primary to NULL and it will cause the crash bellow on the next atomic check when trying to access mst_primary->port. As there is no use in running checks over managers that are not active this patch will skip it. [ 305.616450] [drm:drm_dp_mst_atomic_check] [MST PORT:00000000cc2049e9] releases all VCPI slots [ 305.625085] [drm:drm_dp_mst_atomic_check] [MST PORT:00000000020ab43e] releases all VCPI slots [ 305.633729] [drm:drm_dp_mst_atomic_check] [MST MGR:00000000cdd467d4] mst state 00000000b67672eb VCPI avail=63 used=0 [ 305.644405] BUG: kernel NULL pointer dereference, address: 0000000000000030 [ 305.651448] #PF: supervisor read access in kernel mode [ 305.656640] #PF: error_code(0x0000) - not-present page [ 305.661807] PGD 0 P4D 0 [ 305.664396] Oops: 0000 [#1] PREEMPT SMP NOPTI [ 305.668789] CPU: 3 PID: 183 Comm: kworker/3:2 Not tainted 5.5.0-rc6+ #1404 [ 305.675703] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.3201.A00.1905140358 05/14/2019 [ 305.689425] Workqueue: events drm_dp_delayed_destroy_work [ 305.694874] RIP: 0010:drm_dp_mst_atomic_check+0x138/0x2c0 [ 305.700306] Code: 00 00 00 41 29 d9 41 89 d8 4c 89 fa 4c 89 f1 48 c7 c6 b0 b1 34 82 bf 10 00 00 00 45 31 ed e8 3f 99 02 00 4d 8b bf 80 04 00 00 <49> 8b 47 30 49 8d 5f 30 4c 8d 60 e8 48 39 c3 74 35 49 8b 7c 24 28 [ 305.719169] RSP: 0018:ffffc90001687b58 EFLAGS: 00010246 [ 305.724434] RAX: 0000000000000000 RBX: 000000000000003f RCX: 0000000000000000 [ 305.731611] RDX: 0000000000000000 RSI: ffff88849fba8cb8 RDI: 00000000ffffffff [ 305.738785] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000001 [ 305.745962] R10: ffffc900016879a0 R11: ffffc900016879a5 R12: 0000000000000000 [ 305.753139] R13: 0000000000000000 R14: ffff8884905c9bc0 R15: 0000000000000000 [ 305.760315] FS: 0000000000000000(0000) GS:ffff88849fb80000(0000) knlGS:0000000000000000 [ 305.768452] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 305.774263] CR2: 0000000000000030 CR3: 0000000005610006 CR4: 0000000000760ee0 [ 305.781441] PKRU: 55555554 [ 305.784228] Call Trace: [ 305.786739] intel_atomic_check+0xb2e/0x2560 [i915] [ 305.791678] ? printk+0x53/0x6a [ 305.794856] ? drm_atomic_check_only+0x3e/0x810 [ 305.799417] ? __drm_dbg+0x82/0x90 [ 305.802848] drm_atomic_check_only+0x56a/0x810 [ 305.807322] drm_atomic_commit+0xe/0x50 [ 305.811185] drm_client_modeset_commit_atomic+0x1e2/0x250 [ 305.816619] drm_client_modeset_commit_force+0x4d/0x180 [ 305.821921] drm_fb_helper_restore_fbdev_mode_unlocked+0x46/0xa0 [ 305.827963] drm_fb_helper_set_par+0x2b/0x40 [ 305.832265] drm_fb_helper_hotplug_event.part.0+0xb2/0xd0 [ 305.837755] drm_kms_helper_hotplug_event+0x21/0x30 [ 305.842694] process_one_work+0x25b/0x5b0 [ 305.846735] worker_thread+0x4b/0x3b0 [ 305.850439] kthread+0x100/0x140 [ 305.853690] ? process_one_work+0x5b0/0x5b0 [ 305.857901] ? kthread_park+0x80/0x80 [ 305.861588] ret_from_fork+0x24/0x50 [ 305.865202] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 btusb btrtl btbcm btintel bluetooth prime_numbers snd_hda_intel snd_intel_dspcfg snd_hda_codec e1000e snd_hwdep snd_hda_core thunderbolt mei_hdcp mei_me asix cdc_ether x86_pkg_temp_thermal r8152 mei coretemp usbnet snd_pcm mii crct10dif_pclmul ptp crc32_pclmul ecdh_generic ghash_clmulni_intel pps_core ecc i2c_i801 intel_lpss_pci [ 305.903096] CR2: 0000000000000030 [ 305.906431] ---[ end trace 70ee364eed801cb0 ]--- [ 305.940816] RIP: 0010:drm_dp_mst_atomic_check+0x138/0x2c0 [ 305.946261] Code: 00 00 00 41 29 d9 41 89 d8 4c 89 fa 4c 89 f1 48 c7 c6 b0 b1 34 82 bf 10 00 00 00 45 31 ed e8 3f 99 02 00 4d 8b bf 80 04 00 00 <49> 8b 47 30 49 8d 5f 30 4c 8d 60 e8 48 39 c3 74 35 49 8b 7c 24 28 [ 305.965125] RSP: 0018:ffffc90001687b58 EFLAGS: 00010246 [ 305.970382] RAX: 0000000000000000 RBX: 000000000000003f RCX: 0000000000000000 [ 305.977571] RDX: 0000000000000000 RSI: ffff88849fba8cb8 RDI: 00000000ffffffff [ 305.984747] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000001 [ 305.991921] R10: ffffc900016879a0 R11: ffffc900016879a5 R12: 0000000000000000 [ 305.999099] R13: 0000000000000000 R14: ffff8884905c9bc0 R15: 0000000000000000 [ 306.006271] FS: 0000000000000000(0000) GS:ffff88849fb80000(0000) knlGS:0000000000000000 [ 306.014407] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 306.020185] CR2: 0000000000000030 CR3: 000000048b3aa003 CR4: 0000000000760ee0 [ 306.027404] PKRU: 55555554 [ 306.030127] BUG: sleeping function called from invalid context at include/linux/percpu-rwsem.h:38 [ 306.039049] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 183, name: kworker/3:2 [ 306.047272] INFO: lockdep is turned off. [ 306.051217] irq event stamp: 77505 [ 306.054647] hardirqs last enabled at (77505): [] _raw_spin_unlock_irqrestore+0x47/0x60 [ 306.064270] hardirqs last disabled at (77504): [] _raw_spin_lock_irqsave+0xf/0x50 [ 306.073404] softirqs last enabled at (77402): [] __do_softirq+0x389/0x47f [ 306.081885] softirqs last disabled at (77395): [] irq_exit+0xa9/0xc0 [ 306.089859] CPU: 3 PID: 183 Comm: kworker/3:2 Tainted: G D 5.5.0-rc6+ #1404 [ 306.098167] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.3201.A00.1905140358 05/14/2019 [ 306.111882] Workqueue: events drm_dp_delayed_destroy_work [ 306.117314] Call Trace: [ 306.119780] dump_stack+0x71/0xa0 [ 306.123135] ___might_sleep.cold+0xf7/0x10b [ 306.127399] exit_signals+0x2b/0x360 [ 306.131014] do_exit+0xa7/0xc70 [ 306.134189] ? kthread+0x100/0x140 [ 306.137615] rewind_stack_do_exit+0x17/0x20 Fixes: cd82d82cbc04 ("drm/dp_mst: Add branch bandwidth validation to MST atomic check") Cc: Mikita Lipski Cc: Alex Deucher Cc: Lyude Paul Acked-by: Mikita Lipski Reviewed-by: Lyude Paul Signed-off-by: José Roberto de Souza Signed-off-by: Alex Deucher --- drivers/gpu/drm/drm_dp_mst_topology.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 5d3c1d379277..021c5a98db09 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -5001,6 +5001,9 @@ int drm_dp_mst_atomic_check(struct drm_atomic_state *state) int i, ret = 0; for_each_new_mst_mgr_in_state(state, mgr, mst_state, i) { + if (!mgr->mst_state) + continue; + ret = drm_dp_mst_atomic_check_vcpi_alloc_limit(mgr, mst_state); if (ret) break; -- cgit v1.2.3 From db1a079569687eeb7f617a50bbb0474e9e11b64a Mon Sep 17 00:00:00 2001 From: Wayne Lin Date: Fri, 17 Jan 2020 14:03:50 +0800 Subject: drm/dp_mst: Handle SST-only branch device case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [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 Signed-off-by: Lyude Paul Link: https://patchwork.freedesktop.org/patch/msgid/20200117060350.26358-2-Wayne.Lin@amd.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 140 +++++++++++++++++++--------------- 1 file changed, 80 insertions(+), 60 deletions(-) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 5a61a5596912..e6afe4faeca6 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1916,73 +1916,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; + /* 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_dp_add_mst_branch_device(lct, rad); - if (!mstb) { - ret = -ENOMEM; - DRM_ERROR("Failed to create MSTB for port %p", port); - goto out; - } + if (port->pdt != DP_PEER_DEVICE_NONE) { + if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) { + /* add i2c over sideband */ + ret = drm_dp_mst_register_i2c_bus(&port->aux); + } else { + lct = drm_dp_calculate_rad(port, rad); + mstb = drm_dp_add_mst_branch_device(lct, rad); + if (!mstb) { + ret = -ENOMEM; + DRM_ERROR("Failed to create MSTB for port %p", + port); + goto out; + } - mutex_lock(&mgr->lock); - port->mstb = mstb; - mstb->mgr = port->mgr; - mstb->port_parent = port; + mutex_lock(&mgr->lock); + port->mstb = mstb; + mstb->mgr = port->mgr; + mstb->port_parent = port; - /* - * Make sure this port's memory allocation stays - * around until its child MSTB releases it - */ - drm_dp_mst_get_port_malloc(port); - mutex_unlock(&mgr->lock); + /* + * Make sure this port's memory allocation stays + * around until its child MSTB releases it + */ + drm_dp_mst_get_port_malloc(port); + mutex_unlock(&mgr->lock); - /* And make sure we send a link address for this */ - ret = 1; - break; + /* And make sure we send a link address for this */ + ret = 1; + } } out: @@ -2135,9 +2152,8 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb, goto error; } - if ((port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV || - port->pdt == DP_PEER_DEVICE_SST_SINK) && - port->port_num >= DP_MST_LOGICAL_PORT_0) { + if (port->pdt != DP_PEER_DEVICE_NONE && + drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) { port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); drm_connector_set_tile_property(port->connector); @@ -2201,6 +2217,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb, struct drm_dp_mst_port *port; int old_ddps = 0, ret; u8 new_pdt = DP_PEER_DEVICE_NONE; + bool new_mcs = 0; bool created = false, send_link_addr = false, changed = false; port = drm_dp_get_port(mstb, port_msg->port_number); @@ -2245,7 +2262,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb, port->input = port_msg->input_port; if (!port->input) new_pdt = port_msg->peer_device_type; - port->mcs = port_msg->mcs; + new_mcs = port_msg->mcs; port->ddps = port_msg->ddps; port->ldps = port_msg->legacy_device_plug_status; port->dpcd_rev = port_msg->dpcd_revision; @@ -2272,7 +2289,7 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb, } } - ret = drm_dp_port_set_pdt(port, new_pdt); + ret = drm_dp_port_set_pdt(port, new_pdt, new_mcs); if (ret == 1) { send_link_addr = true; } else if (ret < 0) { @@ -2286,7 +2303,8 @@ drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb, * we're coming out of suspend. In this case, always resend the link * address if there's an MSTB on this port */ - if (!created && port->pdt == DP_PEER_DEVICE_MST_BRANCHING) + if (!created && port->pdt == DP_PEER_DEVICE_MST_BRANCHING && + port->mcs) send_link_addr = true; if (port->connector) @@ -2323,6 +2341,7 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb, struct drm_dp_mst_port *port; int old_ddps, old_input, ret, i; u8 new_pdt; + bool new_mcs; bool dowork = false, create_connector = false; port = drm_dp_get_port(mstb, conn_stat->port_number); @@ -2354,7 +2373,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->mcs = conn_stat->message_capability_status; port->ldps = conn_stat->legacy_device_plug_status; port->ddps = conn_stat->displayport_device_plug_status; @@ -2367,8 +2385,8 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb, } new_pdt = port->input ? DP_PEER_DEVICE_NONE : conn_stat->peer_device_type; - - ret = drm_dp_port_set_pdt(port, new_pdt); + new_mcs = conn_stat->message_capability_status; + ret = drm_dp_port_set_pdt(port, new_pdt, new_mcs); if (ret == 1) { dowork = true; } else if (ret < 0) { @@ -3929,6 +3947,8 @@ drm_dp_mst_detect_port(struct drm_connector *connector, switch (port->pdt) { case DP_PEER_DEVICE_NONE: case DP_PEER_DEVICE_MST_BRANCHING: + if (!port->mcs) + ret = connector_status_connected; break; case DP_PEER_DEVICE_SST_SINK: @@ -4541,7 +4561,7 @@ drm_dp_delayed_destroy_port(struct drm_dp_mst_port *port) if (port->connector) port->mgr->cbs->destroy_connector(port->mgr, port->connector); - drm_dp_port_set_pdt(port, DP_PEER_DEVICE_NONE); + drm_dp_port_set_pdt(port, DP_PEER_DEVICE_NONE, port->mcs); drm_dp_mst_put_port_malloc(port); } -- cgit v1.2.3 From 8ccb5bf7619c6523e7a4384a84b72e7be804298c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Roberto=20de=20Souza?= Date: Wed, 29 Jan 2020 15:24:48 -0800 Subject: drm/mst: Fix possible NULL pointer dereference in drm_dp_mst_process_up_req() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to DP specification, DP_SINK_EVENT_NOTIFY is also a broadcast message but as this function only handles DP_CONNECTION_STATUS_NOTIFY I will only make the static analyzer that caught this issue happy by not calling drm_dp_get_mst_branch_device_by_guid() with a NULL guid, causing drm_dp_mst_process_up_req() to return in the "if (!mstb)" right bellow. Fixes: 9408cc94eb04 ("drm/dp_mst: Handle UP requests asynchronously") Cc: Lyude Paul Cc: Sean Paul Cc: # v5.5+ Signed-off-by: José Roberto de Souza [added cc to stable] Signed-off-by: Lyude Paul Link: https://patchwork.freedesktop.org/patch/msgid/20200129232448.84704-1-jose.souza@intel.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_dp_mst_topology.c') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index e6afe4faeca6..105bb5f40166 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3760,7 +3760,8 @@ drm_dp_mst_process_up_req(struct drm_dp_mst_topology_mgr *mgr, else if (msg->req_type == DP_RESOURCE_STATUS_NOTIFY) guid = msg->u.resource_stat.guid; - mstb = drm_dp_get_mst_branch_device_by_guid(mgr, guid); + if (guid) + mstb = drm_dp_get_mst_branch_device_by_guid(mgr, guid); } else { mstb = drm_dp_get_mst_branch_device(mgr, hdr->lct, hdr->rad); } -- cgit v1.2.3