From: Mikita Lipski mikita.lipski@amd.com
This set of patches is a continuation of DSC enablement patches for AMDGPU. This set enables DSC on MST. It also contains implementation of both encoder and connector atomic check routines.
First 12 patches have been introduced in multiple iterations to the mailing list before. These patches were developed by David Francis as part of his work on DSC.
Other 2 patches add atomic check functionality to encoder and connector to allocate and release VCPI slots on each state atomic check. These changes utilize newly added drm_mst_helper functions for better tracking of VCPI slots.
v2: squashed previously 3 separate atomic check patches, separate atomic check for dsc connectors, track vcpi and pbn on connectors.
David Francis (12): drm/dp_mst: Add PBN calculation for DSC modes drm/dp_mst: Parse FEC capability on MST ports drm/dp_mst: Add MST support to DP DPCD R/W functions drm/dp_mst: Fill branch->num_ports drm/dp_mst: Add helpers for MST DSC and virtual DPCD aux drm/dp_mst: Add new quirk for Synaptics MST hubs drm/amd/display: Use correct helpers to compute timeslots drm/amd/display: Initialize DSC PPS variables to 0 drm/amd/display: Validate DSC caps on MST endpoints drm/amd/display: Write DSC enable to MST DPCD drm/amd/display: MST DSC compute fair share drm/amd/display: Trigger modesets on MST DSC connectors
Mikita Lipski (2): drm/amd/display: Add MST atomic routines drm/amd/display: Recalculate VCPI slots for new DSC connectors
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 179 +++++++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 7 + .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 63 +-- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 449 +++++++++++++++++- .../display/amdgpu_dm/amdgpu_dm_mst_types.h | 4 + .../drm/amd/display/dc/core/dc_link_hwss.c | 3 + .../gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c | 3 + .../drm/amd/display/dc/dcn20/dcn20_resource.c | 7 +- .../drm/amd/display/dc/dcn20/dcn20_resource.h | 1 + drivers/gpu/drm/drm_dp_aux_dev.c | 12 +- drivers/gpu/drm/drm_dp_helper.c | 33 +- drivers/gpu/drm/drm_dp_mst_topology.c | 174 ++++++- drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 +- drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +- include/drm/drm_dp_helper.h | 7 + include/drm/drm_dp_mst_helper.h | 8 +- 17 files changed, 885 insertions(+), 73 deletions(-)
From: Mikita Lipski mikita.lipski@amd.com
- Adding encoder atomic check to find vcpi slots for a connector - Using DRM helper functions to calculate PBN - Adding connector atomic check to release vcpi slots if connector loses CRTC - Calculate PBN and VCPI slots only once during atomic check and store them on amdgpu connector to eliminate redundant calculation - Call drm_dp_mst_atomic_check to verify validity of MST topology during state atomic check
v2: squashed previous 3 separate patches, removed DSC PBN calculation, and added PBN and VCPI slots properties to amdgpu connector
Cc: Jerry Zuo Jerry.Zuo@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: Nicholas Kazlauskas nicholas.kazlauskas@amd.com Cc: Lyude Paul lyude@redhat.com Signed-off-by: Mikita Lipski mikita.lipski@amd.com --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 +++++++++++++++++++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 ++ .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 42 ++----------------- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 32 ++++++++++++++ 4 files changed, 81 insertions(+), 39 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 239b1ae86007..3fc1afccbb33 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4573,6 +4573,41 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) { + struct drm_atomic_state *state = crtc_state->state; + struct drm_connector *connector = conn_state->connector; + struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); + struct dm_crtc_state *dm_new_crtc_state = to_dm_crtc_state(crtc_state); + 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; + int clock, bpp = 0; + + if (!dm_new_crtc_state) + return 0; + + if (!aconnector->port || !aconnector->dc_sink) + return 0; + + mst_port = aconnector->port; + mst_mgr = &aconnector->mst_port->mst_mgr; + + if (!crtc_state->connectors_changed && !crtc_state->mode_changed) + return 0; + + if(!state->duplicated) { + bpp = (uint8_t)connector->display_info.bpc * 3; + clock = adjusted_mode->clock; + aconnector->pbn = drm_dp_calc_pbn_mode(clock, bpp); + } + aconnector->vcpi_slots = drm_dp_atomic_find_vcpi_slots(state, + mst_mgr, + mst_port, + aconnector->pbn); + + if (aconnector->vcpi_slots < 0) { + DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n", aconnector->vcpi_slots); + return aconnector->vcpi_slots; + } return 0; }
@@ -5197,6 +5232,8 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm, aconnector->base.dpms = DRM_MODE_DPMS_OFF; aconnector->hpd.hpd = AMDGPU_HPD_NONE; /* not used */ aconnector->audio_inst = -1; + aconnector->vcpi_slots = 0; + aconnector->pbn = 0; mutex_init(&aconnector->hpd_lock);
/* @@ -7592,6 +7629,11 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, if (ret) goto fail;
+ /* Perform validation of MST topology in the state*/ + ret = drm_dp_mst_atomic_check(state); + if (ret) + goto fail; + if (state->legacy_cursor_update) { /* * This is a fast cursor update coming from the plane update diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index c6fdebee7189..3ce104324096 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -280,6 +280,10 @@ struct amdgpu_dm_connector { struct amdgpu_dm_connector *mst_port; struct amdgpu_encoder *mst_encoder;
+ /* MST specific */ + uint32_t vcpi_slots; + uint32_t pbn; + /* TODO see if we can merge with ddc_bus or make a dm_connector */ struct amdgpu_i2c_adapter *i2c;
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 11e5784aa62a..5256abe32e92 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 @@ -184,11 +184,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( struct amdgpu_dm_connector *aconnector; struct drm_dp_mst_topology_mgr *mst_mgr; struct drm_dp_mst_port *mst_port; - int slots = 0; bool ret; - int clock; - int bpp = 0; - int pbn = 0;
aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
@@ -203,42 +199,10 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( mst_port = aconnector->port;
if (enable) { - clock = stream->timing.pix_clk_100hz / 10; - - switch (stream->timing.display_color_depth) { - - case COLOR_DEPTH_666: - bpp = 6; - break; - case COLOR_DEPTH_888: - bpp = 8; - break; - case COLOR_DEPTH_101010: - bpp = 10; - break; - case COLOR_DEPTH_121212: - bpp = 12; - break; - case COLOR_DEPTH_141414: - bpp = 14; - break; - case COLOR_DEPTH_161616: - bpp = 16; - break; - default: - ASSERT(bpp != 0); - break; - } - - bpp = bpp * 3; - - /* TODO need to know link rate */ - - pbn = drm_dp_calc_pbn_mode(clock, bpp); - - slots = drm_dp_find_vcpi_slots(mst_mgr, pbn); - ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, pbn, slots);
+ ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, + aconnector->pbn, + aconnector->vcpi_slots); if (!ret) return false;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 3af2b429ff1b..7f3ce29bd14c 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -250,10 +250,42 @@ dm_mst_atomic_best_encoder(struct drm_connector *connector, return &to_amdgpu_dm_connector(connector)->mst_encoder->base; }
+static int dm_dp_mst_atomic_check(struct drm_connector *connector, + struct drm_atomic_state *state) +{ + struct drm_connector_state *new_conn_state = + drm_atomic_get_new_connector_state(state, connector); + struct drm_connector_state *old_conn_state = + drm_atomic_get_old_connector_state(state, connector); + struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector); + struct drm_crtc_state *new_crtc_state; + struct drm_dp_mst_topology_mgr *mst_mgr; + struct drm_dp_mst_port *mst_port; + + mst_port = aconnector->port; + mst_mgr = &aconnector->mst_port->mst_mgr; + + if (!old_conn_state->crtc) + return 0; + + if (new_conn_state->crtc) { + new_crtc_state = drm_atomic_get_old_crtc_state(state, new_conn_state->crtc); + if (!new_crtc_state || + !drm_atomic_crtc_needs_modeset(new_crtc_state) || + new_crtc_state->enable) + return 0; + } + + return drm_dp_atomic_release_vcpi_slots(state, + mst_mgr, + mst_port); +} + static const struct drm_connector_helper_funcs dm_dp_mst_connector_helper_funcs = { .get_modes = dm_dp_mst_get_modes, .mode_valid = amdgpu_dm_connector_mode_valid, .atomic_best_encoder = dm_mst_atomic_best_encoder, + .atomic_check = dm_dp_mst_atomic_check, };
static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder)
From: David Francis David.Francis@amd.com
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
Reviewed-by: Manasi Navare manasi.d.navare@intel.com Reviewed-by: Lyude Paul lyude@redhat.com Reviewed-by: Harry Wentland harry.wentland@amd.com Signed-off-by: David Francis David.Francis@amd.com --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- drivers/gpu/drm/drm_dp_mst_topology.c | 16 ++++++++++++---- drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++- drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 ++- drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +- include/drm/drm_dp_mst_helper.h | 3 +-- 6 files changed, 19 insertions(+), 10 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 3fc1afccbb33..59114b52090d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4597,7 +4597,7 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, if(!state->duplicated) { bpp = (uint8_t)connector->display_info.bpc * 3; clock = adjusted_mode->clock; - aconnector->pbn = drm_dp_calc_pbn_mode(clock, bpp); + aconnector->pbn = drm_dp_calc_pbn_mode(clock, bpp, false); } aconnector->vcpi_slots = drm_dp_atomic_find_vcpi_slots(state, mst_mgr, diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 82add736e17d..3e7b7553cf4d 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3534,10 +3534,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) { u64 kbps; s64 peak_kbps; @@ -3555,11 +3556,18 @@ 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 */
numerator = 64 * 1006; denominator = 54 * 8 * 1000 * 1000;
+ if (dsc) + numerator /= 16; + kbps *= numerator; peak_kbps = drm_fixp_from_fraction(kbps, denominator);
@@ -3570,19 +3578,19 @@ EXPORT_SYMBOL(drm_dp_calc_pbn_mode); static int test_calc_pbn_mode(void) { int ret; - ret = drm_dp_calc_pbn_mode(154000, 30); + ret = drm_dp_calc_pbn_mode(154000, 30, false); if (ret != 689) { DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, expected PBN %d, actual PBN %d.\n", 154000, 30, 689, ret); return -EINVAL; } - ret = drm_dp_calc_pbn_mode(234000, 30); + ret = drm_dp_calc_pbn_mode(234000, 30, false); if (ret != 1047) { DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, expected PBN %d, actual PBN %d.\n", 234000, 30, 1047, ret); return -EINVAL; } - ret = drm_dp_calc_pbn_mode(297000, 24); + ret = drm_dp_calc_pbn_mode(297000, 24, false); if (ret != 1063) { DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, expected PBN %d, actual PBN %d.\n", 297000, 24, 1063, ret); diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 2c5ac3dd647f..dfac450841df 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -61,7 +61,8 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder, crtc_state->pipe_bpp = bpp;
crtc_state->pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, - crtc_state->pipe_bpp); + crtc_state->pipe_bpp, + false);
slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, port, crtc_state->pbn); diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index f1dbc7852414..c45832230ccc 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -778,7 +778,8 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, if (!state->duplicated) asyh->dp.pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, - connector->display_info.bpc * 3); + connector->display_info.bpc * 3, + false);
if (crtc_state->mode_changed) { slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr, diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c index 2994f07fbad9..c997f88218f2 100644 --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c @@ -514,7 +514,7 @@ static bool radeon_mst_mode_fixup(struct drm_encoder *encoder,
mst_enc = radeon_encoder->enc_priv;
- mst_enc->pbn = drm_dp_calc_pbn_mode(adjusted_mode->clock, bpp); + mst_enc->pbn = drm_dp_calc_pbn_mode(adjusted_mode->clock, bpp, false);
mst_enc->primary->active_device = mst_enc->primary->devices & mst_enc->connector->devices; DRM_DEBUG_KMS("setting active device to %08x from %08x %08x for encoder %d\n", diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 2ba6253ea6d3..9116b2c95239 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -610,8 +610,7 @@ bool drm_dp_mst_port_has_audio(struct drm_dp_mst_topology_mgr *mgr, struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
-int drm_dp_calc_pbn_mode(int clock, int bpp); - +int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc);
bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, int pbn, int slots);
From: David Francis David.Francis@amd.com
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 lyude@redhat.com Reviewed-by: Harry Wentland harry.wentland@amd.com Signed-off-by: David Francis David.Francis@amd.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++ include/drm/drm_dp_mst_helper.h | 3 +++ 2 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 3e7b7553cf4d..9f3604355705 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -553,6 +553,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; @@ -2183,6 +2184,7 @@ static int drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr, DRM_DEBUG_KMS("enum path resources %d: %d %d\n", txmsg->reply.u.path_resources.port_number, txmsg->reply.u.path_resources.full_payload_bw_number, txmsg->reply.u.path_resources.avail_payload_bw_number); port->available_pbn = txmsg->reply.u.path_resources.avail_payload_bw_number; + port->fec_capable = txmsg->reply.u.path_resources.fec_capable; } }
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 9116b2c95239..f113ae04fa88 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -108,6 +108,8 @@ struct drm_dp_mst_port { * audio-capable. */ bool has_audio; + + bool fec_capable; };
/** @@ -312,6 +314,7 @@ struct drm_dp_port_number_req {
struct drm_dp_enum_path_resources_ack_reply { u8 port_number; + bool fec_capable; u16 full_payload_bw_number; u16 avail_payload_bw_number; };
From: David Francis David.Francis@amd.com
Instead of having drm_dp_dpcd_read/write and drm_dp_mst_dpcd_read/write as entry points into the aux code, have drm_dp_dpcd_read/write handle both.
This means that DRM drivers can make MST DPCD read/writes.
v2: Fix spacing v3: Dump dpcd access on MST read/writes
Reviewed-by: Lyude Paul lyude@redhat.com Reviewed-by: Harry Wentland harry.wentland@amd.com Signed-off-by: David Francis David.Francis@amd.com --- drivers/gpu/drm/drm_dp_aux_dev.c | 12 ++---------- drivers/gpu/drm/drm_dp_helper.c | 31 +++++++++++++++++++++---------- 2 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c index 0cfb386754c3..418cad4f649a 100644 --- a/drivers/gpu/drm/drm_dp_aux_dev.c +++ b/drivers/gpu/drm/drm_dp_aux_dev.c @@ -163,11 +163,7 @@ static ssize_t auxdev_read_iter(struct kiocb *iocb, struct iov_iter *to) break; }
- if (aux_dev->aux->is_remote) - res = drm_dp_mst_dpcd_read(aux_dev->aux, pos, buf, - todo); - else - res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo); + res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
if (res <= 0) break; @@ -215,11 +211,7 @@ static ssize_t auxdev_write_iter(struct kiocb *iocb, struct iov_iter *from) break; }
- if (aux_dev->aux->is_remote) - res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf, - todo); - else - res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo); + res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf, todo);
if (res <= 0) break; diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index ffc68d305afe..af1cd968adfd 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -32,6 +32,8 @@ #include <drm/drm_dp_helper.h> #include <drm/drm_print.h> #include <drm/drm_vblank.h> +#include <drm/drm_dp_mst_helper.h> +#include <drm/drmP.h>
#include "drm_crtc_helper_internal.h"
@@ -251,7 +253,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
/** * drm_dp_dpcd_read() - read a series of bytes from the DPCD - * @aux: DisplayPort AUX channel + * @aux: DisplayPort AUX channel (SST or MST) * @offset: address of the (first) register to read * @buffer: buffer to store the register values * @size: number of bytes in @buffer @@ -280,13 +282,18 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset, * We just have to do it before any DPCD access and hope that the * monitor doesn't power down exactly after the throw away read. */ - ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, DP_DPCD_REV, buffer, - 1); - if (ret != 1) - goto out; + if (!aux->is_remote) { + ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, DP_DPCD_REV, + buffer, 1); + if (ret != 1) + goto out; + }
- ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, buffer, - size); + if (aux->is_remote) + ret = drm_dp_mst_dpcd_read(aux, offset, buffer, size); + else + ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, + buffer, size);
out: drm_dp_dump_access(aux, DP_AUX_NATIVE_READ, offset, buffer, ret); @@ -296,7 +303,7 @@ EXPORT_SYMBOL(drm_dp_dpcd_read);
/** * drm_dp_dpcd_write() - write a series of bytes to the DPCD - * @aux: DisplayPort AUX channel + * @aux: DisplayPort AUX channel (SST or MST) * @offset: address of the (first) register to write * @buffer: buffer containing the values to write * @size: number of bytes in @buffer @@ -313,8 +320,12 @@ ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset, { int ret;
- ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_WRITE, offset, buffer, - size); + if (aux->is_remote) + ret = drm_dp_mst_dpcd_write(aux, offset, buffer, size); + else + ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_WRITE, offset, + buffer, size); + drm_dp_dump_access(aux, DP_AUX_NATIVE_WRITE, offset, buffer, ret); return ret; }
From: David Francis David.Francis@amd.com
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
Reviewed-by: Lyude Paul lyude@redhat.com Reviewed-by: Harry Wentland harry.wentland@amd.com Signed-off-by: David Francis David.Francis@amd.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 9f3604355705..502923c24450 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1669,6 +1669,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, mutex_lock(&mstb->mgr->lock); drm_dp_mst_topology_get_port(port); list_add(&port->next, &mstb->ports); + mstb->num_ports++; mutex_unlock(&mstb->mgr->lock); }
@@ -1703,6 +1704,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, /* remove it from the port list */ mutex_lock(&mstb->mgr->lock); list_del(&port->next); + mstb->num_ports--; mutex_unlock(&mstb->mgr->lock); /* drop port list reference */ drm_dp_mst_topology_put_port(port);
From: David Francis David.Francis@amd.com
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
Reviewed-by: Lyude Paul lyude@redhat.com Reviewed-by: Wenjing Liu Wenjing.Liu@amd.com Signed-off-by: David Francis David.Francis@amd.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 127 ++++++++++++++++++++++++++ include/drm/drm_dp_mst_helper.h | 2 + 2 files changed, 129 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 502923c24450..d8f9ba27b559 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4150,3 +4150,130 @@ 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; + + if (!port) + return NULL; + + if (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; + u8 endpoint_dsc; + u8 endpoint_fec; + + if (drm_dp_dpcd_read(&port->aux, + DP_DSC_SUPPORT, &endpoint_dsc, 1) < 0) + return NULL; + if (drm_dp_dpcd_read(&port->aux, + DP_FEC_CAPABILITY, &endpoint_fec, 1) < 0) + return NULL; + if (drm_dp_dpcd_read(&immediate_upstream_port->aux, + DP_DSC_SUPPORT, &upstream_dsc, 1) < 0) + 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; + + return NULL; +} +EXPORT_SYMBOL(drm_dp_mst_dsc_aux_for_port); diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index f113ae04fa88..4cf738545dfb 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -673,6 +673,8 @@ int __must_check drm_dp_mst_atomic_check(struct drm_atomic_state *state); void drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port); void drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port);
+struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port); + extern const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs;
/**
Sorry this took me a little while to get to, I've been at XDC.
This is closer then, but still a couple more issues below (also-thank you for including the changelog!)
On Tue, 2019-10-01 at 12:17 -0400, mikita.lipski@amd.com wrote:
From: Mikita Lipski mikita.lipski@amd.com
- Adding encoder atomic check to find vcpi slots for a connector
- Using DRM helper functions to calculate PBN
- Adding connector atomic check to release vcpi slots if connector
loses CRTC
- Calculate PBN and VCPI slots only once during atomic
check and store them on amdgpu connector to eliminate redundant calculation
- Call drm_dp_mst_atomic_check to verify validity of MST topology
during state atomic check
v2: squashed previous 3 separate patches, removed DSC PBN calculation, and added PBN and VCPI slots properties to amdgpu connector
Cc: Jerry Zuo Jerry.Zuo@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: Nicholas Kazlauskas nicholas.kazlauskas@amd.com Cc: Lyude Paul lyude@redhat.com Signed-off-by: Mikita Lipski mikita.lipski@amd.com
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 +++++++++++++++++++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 ++ .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 42 ++----------------- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 32 ++++++++++++++ 4 files changed, 81 insertions(+), 39 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 239b1ae86007..3fc1afccbb33 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4573,6 +4573,41 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, struct drm_crtc_state *crtc_state, struct drm_connector_state *conn_state) {
- struct drm_atomic_state *state = crtc_state->state;
- struct drm_connector *connector = conn_state->connector;
- struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
- struct dm_crtc_state *dm_new_crtc_state =
to_dm_crtc_state(crtc_state);
- 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;
- int clock, bpp = 0;
- if (!dm_new_crtc_state)
return 0;
You can remove this, crtc_state will never be NULL
- if (!aconnector->port || !aconnector->dc_sink)
return 0;
This makes me think that this should probably just be moved into amdgpu_dm_mst_types.c since we're not using this encoder check for anything else, but I'll leave that decision up to you.
- mst_port = aconnector->port;
- mst_mgr = &aconnector->mst_port->mst_mgr;
- if (!crtc_state->connectors_changed && !crtc_state->mode_changed)
return 0;- if(!state->duplicated) {
bpp = (uint8_t)connector->display_info.bpc * 3;clock = adjusted_mode->clock;aconnector->pbn = drm_dp_calc_pbn_mode(clock, bpp);
We can't do this...
- }
- aconnector->vcpi_slots = drm_dp_atomic_find_vcpi_slots(state,
mst_mgr,mst_port,aconnector-pbn);
...and we can't do this: you're not allowed to modify anything during the atomic check other than the atomic states that were passed in (e.g. crtc_state along with anything in it's respective struct drm_atomic_state). Remember we're trying to check if a configuration is valid here -before- we've committed anything to hardware. So, the pbn and vcpi values need to be stored in the connector's atomic state.
- if (aconnector->vcpi_slots < 0) {
DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n",aconnector->vcpi_slots);
return aconnector->vcpi_slots;- } return 0;
}
@@ -5197,6 +5232,8 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm, aconnector->base.dpms = DRM_MODE_DPMS_OFF; aconnector->hpd.hpd = AMDGPU_HPD_NONE; /* not used */ aconnector->audio_inst = -1;
aconnector->vcpi_slots = 0;
aconnector->pbn = 0; mutex_init(&aconnector->hpd_lock);
/*
@@ -7592,6 +7629,11 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, if (ret) goto fail;
- /* Perform validation of MST topology in the state*/
- ret = drm_dp_mst_atomic_check(state);
- if (ret)
goto fail;- if (state->legacy_cursor_update) { /*
- This is a fast cursor update coming from the plane update
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index c6fdebee7189..3ce104324096 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -280,6 +280,10 @@ struct amdgpu_dm_connector { struct amdgpu_dm_connector *mst_port; struct amdgpu_encoder *mst_encoder;
- /* MST specific */
- uint32_t vcpi_slots;
- uint32_t pbn;
- /* TODO see if we can merge with ddc_bus or make a dm_connector */ struct amdgpu_i2c_adapter *i2c;
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 11e5784aa62a..5256abe32e92 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 @@ -184,11 +184,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( struct amdgpu_dm_connector *aconnector; struct drm_dp_mst_topology_mgr *mst_mgr; struct drm_dp_mst_port *mst_port;
int slots = 0; bool ret;
int clock;
int bpp = 0;
int pbn = 0;
aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
@@ -203,42 +199,10 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( mst_port = aconnector->port;
if (enable) {
clock = stream->timing.pix_clk_100hz / 10;switch (stream->timing.display_color_depth) {case COLOR_DEPTH_666:bpp = 6;break;case COLOR_DEPTH_888:bpp = 8;break;case COLOR_DEPTH_101010:bpp = 10;break;case COLOR_DEPTH_121212:bpp = 12;break;case COLOR_DEPTH_141414:bpp = 14;break;case COLOR_DEPTH_161616:bpp = 16;break;default:ASSERT(bpp != 0);break;}bpp = bpp * 3;/* TODO need to know link rate */pbn = drm_dp_calc_pbn_mode(clock, bpp);slots = drm_dp_find_vcpi_slots(mst_mgr, pbn);ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, pbn, slots);
ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port,aconnector->pbn,aconnector->vcpi_slots);diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 3af2b429ff1b..7f3ce29bd14c 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -250,10 +250,42 @@ dm_mst_atomic_best_encoder(struct drm_connector *connector, return &to_amdgpu_dm_connector(connector)->mst_encoder->base; }
+static int dm_dp_mst_atomic_check(struct drm_connector *connector,
struct drm_atomic_state *state)+{
- struct drm_connector_state *new_conn_state =
drm_atomic_get_new_connector_state(state, connector);- struct drm_connector_state *old_conn_state =
drm_atomic_get_old_connector_state(state, connector);- struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
- struct drm_crtc_state *new_crtc_state;
- struct drm_dp_mst_topology_mgr *mst_mgr;
- struct drm_dp_mst_port *mst_port;
- mst_port = aconnector->port;
- mst_mgr = &aconnector->mst_port->mst_mgr;
- if (!old_conn_state->crtc)
return 0;- if (new_conn_state->crtc) {
new_crtc_state = drm_atomic_get_old_crtc_state(state,new_conn_state->crtc);
if (!new_crtc_state ||!drm_atomic_crtc_needs_modeset(new_crtc_state) ||new_crtc_state->enable)return 0;}
Need to fix the indenting here. The rest looks good so far though, keep going! :)
- return drm_dp_atomic_release_vcpi_slots(state,
mst_mgr,mst_port);+}
static const struct drm_connector_helper_funcs dm_dp_mst_connector_helper_funcs = { .get_modes = dm_dp_mst_get_modes, .mode_valid = amdgpu_dm_connector_mode_valid, .atomic_best_encoder = dm_mst_atomic_best_encoder,
- .atomic_check = dm_dp_mst_atomic_check,
};
static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder)
On Tue, 01 Oct 2019, mikita.lipski@amd.com wrote:
This set of patches is a continuation of DSC enablement patches for AMDGPU. This set enables DSC on MST. It also contains implementation of both encoder and connector atomic check routines.
Please consider *not* using git send-email --chain-reply-to option (or sendemail.chainReplyTo configuration). Or, if it comes from git format-patch, --thread=deep / format.thread=deep.
BR, Jani.
dri-devel@lists.freedesktop.org
-
Jani Nikula -
Lyude Paul -
mikita.lipski＠amd.com