| Message ID | 20210326031416.2166481-1-Jerry.Zuo@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Update MST First Link Slot Information Based on Encoding Format | expand |
On Thu, 2021-03-25 at 23:14 -0400, Fangzhi Zuo wrote: > 8b/10b encoding format requires to reserve the first slot for > recording metadata. Real data transmission starts from the second slot, > with a total of available 63 slots available. > > In 128b/132b encoding format, metadata is transmitted separately > in LLCP packet before MTP. Real data transmission starts from > the first slot, with a total of 64 slots available. > > Update the slot information after link detect. > > Signed-off-by: Fangzhi Zuo <Jerry.Zuo@amd.com> > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 48 ++++++++++++++++++++++----- > include/drm/drm_dp_mst_helper.h | 8 +++++ > 2 files changed, 47 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 42a0c6888c33..577ed4224778 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -3382,7 +3382,7 @@ int drm_dp_update_payload_part1(struct > drm_dp_mst_topology_mgr *mgr) > struct drm_dp_payload req_payload; > struct drm_dp_mst_port *port; > int i, j; > - int cur_slots = 1; > + int cur_slots = mgr->first_link_start_slot; > > mutex_lock(&mgr->payload_lock); > for (i = 0; i < mgr->max_payloads; i++) { > @@ -4302,8 +4302,13 @@ int drm_dp_find_vcpi_slots(struct > drm_dp_mst_topology_mgr *mgr, > > num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div); > > - /* max. time slots - one slot for MTP header */ > - if (num_slots > 63) > + /** > + * first_link_total_avail_slots: max. time slots > + * first slot reserved for MTP header in 8b/10b, > + * but not required for 128b/132b > + */ > + > + if (num_slots > mgr->first_link_total_avail_slots) This is deprecated code, as indicated in the comments for this function. We really shouldn't be updating this imho. > return -ENOSPC; > return num_slots; > } > @@ -4314,8 +4319,12 @@ static int drm_dp_init_vcpi(struct > drm_dp_mst_topology_mgr *mgr, > { > int ret; > > - /* max. time slots - one slot for MTP header */ > - if (slots > 63) > + /** > + * first_link_total_avail_slots: max. time slots > + * first slot reserved for MTP header in 8b/10b, > + * but not required for 128b/132b > + */ > + if (slots > mgr->first_link_total_avail_slots) why are these kernel docs? putting kdocs in the middle of a function doesn't do anything > return -ENOSPC; > > vcpi->pbn = pbn; > @@ -4488,6 +4497,25 @@ int drm_dp_atomic_release_vcpi_slots(struct > drm_atomic_state *state, > } > EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); > > +/* > + * drm_dp_mst_update_first_link_slot_info() > + * update the first link's total available slots and starting slot > + * @mgr: manager to store the slot info. > + * @encoding_format: detected link encoding format These kernel docs aren't properly formatted, follow the examples in the rest of the file: /** * function() - summary * @mgr: something something * … */ > + */ > +void drm_dp_mst_update_first_link_slot_info( > + struct drm_dp_mst_topology_mgr *mgr, uint8_t encoding_format) Use u8, the kernel discourages using the longer typenames outside of uapi headers. Also this isn't indented correctly - the kernel doesn't break at the starting parenthesis for function definitions. This should be something like drm_dp_mst_update_first_link_slot_info(struct drm_dp_mst_topology_mgr *mgr, u8 encoding_format) Also, where is this function actually used? If amdgpu is intending to use this, it really should be in the same series that this is introduced in so that we can tell if these helpers are what we really want here or not > +{ > + if (encoding_format == DP_CAP_ANSI_128B132B) { > + mgr->first_link_total_avail_slots = 64; > + mgr->first_link_start_slot = 0; > + } > + DRM_DEBUG_KMS("%s encoding format on 0x%p -> total %d slots, start at > slot %d\n", > + (encoding_format == DP_CAP_ANSI_128B132B) ? "128b/132b":"8b/10b", > + mgr, mgr->first_link_total_avail_slots, mgr- > >first_link_start_slot); the indenting here is entirely broken - please follow the same style as the rest of the file. Line continuations need to start at the beginning parenthesis, but the lines after DRM_DEBUG_KMS() don't have any additional indenting here. > +} > +EXPORT_SYMBOL(drm_dp_mst_update_first_link_slot_info); > + > /** > * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel > * @mgr: manager for this port > @@ -4518,8 +4546,8 @@ bool drm_dp_mst_allocate_vcpi(struct > drm_dp_mst_topology_mgr *mgr, > > ret = drm_dp_init_vcpi(mgr, &port->vcpi, pbn, slots); > if (ret) { > - DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n", > - DIV_ROUND_UP(pbn, mgr->pbn_div), ret); > + DRM_DEBUG_KMS("failed to init vcpi slots=%d max=%d ret=%d\n", > + DIV_ROUND_UP(pbn, mgr->pbn_div), mgr- > >first_link_total_avail_slots, ret); > drm_dp_mst_topology_put_port(port); > goto out; > } > @@ -5162,7 +5190,7 @@ 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; > + int avail_slots = mgr->first_link_total_avail_slots, payload_count = > 0; > > list_for_each_entry(vcpi, &mst_state->vcpis, next) { > /* Releasing VCPI is always OK-even if the port is gone */ > @@ -5191,7 +5219,7 @@ drm_dp_mst_atomic_check_vcpi_alloc_limit(struct > drm_dp_mst_topology_mgr *mgr, > } > DRM_DEBUG_ATOMIC("[MST MGR:%p] mst state %p VCPI avail=%d used=%d\n", > mgr, mst_state, avail_slots, > - 63 - avail_slots); > + mgr->first_link_total_avail_slots - avail_slots); > > return 0; > } > @@ -5455,6 +5483,8 @@ int drm_dp_mst_topology_mgr_init(struct > drm_dp_mst_topology_mgr *mgr, > if (!mgr->proposed_vcpis) > return -ENOMEM; > set_bit(0, &mgr->payload_mask); > + mgr->first_link_total_avail_slots = 63; > + mgr->first_link_start_slot = 1; > > mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL); > if (mst_state == NULL) > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h > index bd1c39907b92..f4310b3705e7 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -653,6 +653,13 @@ struct drm_dp_mst_topology_mgr { > */ > int pbn_div; > > + /** > + * @first_link_total_avail_slots: frist link total slots available. > + * @first_link_start_slot: start slot index for real data > transmission. > + */ These kernel docs are also broken, you can't define two variables in the same kernel doc comment if it's embedded in the struct, there has to be one kernel doc comment per-member. Attempting to generate documentation with make htmldocs even mentions this: ./include/drm/drm_dp_mst_helper.h:770: warning: Function parameter or member 'first_link_start_slot' not described in 'drm_dp_mst_topology_mgr' > + u8 first_link_total_avail_slots; > + u8 first_link_start_slot; > + > /** > * @funcs: Atomic helper callbacks > */ > @@ -795,6 +802,7 @@ int drm_dp_mst_get_vcpi_slots(struct > drm_dp_mst_topology_mgr *mgr, struct drm_dp > > void drm_dp_mst_reset_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct > drm_dp_mst_port *port); > > +void drm_dp_mst_update_first_link_slot_info(struct drm_dp_mst_topology_mgr > *mgr, uint8_t encoding_format); > > void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_port *port);
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 42a0c6888c33..577ed4224778 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3382,7 +3382,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr) struct drm_dp_payload req_payload; struct drm_dp_mst_port *port; int i, j; - int cur_slots = 1; + int cur_slots = mgr->first_link_start_slot; mutex_lock(&mgr->payload_lock); for (i = 0; i < mgr->max_payloads; i++) { @@ -4302,8 +4302,13 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div); - /* max. time slots - one slot for MTP header */ - if (num_slots > 63) + /** + * first_link_total_avail_slots: max. time slots + * first slot reserved for MTP header in 8b/10b, + * but not required for 128b/132b + */ + + if (num_slots > mgr->first_link_total_avail_slots) return -ENOSPC; return num_slots; } @@ -4314,8 +4319,12 @@ static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr, { int ret; - /* max. time slots - one slot for MTP header */ - if (slots > 63) + /** + * first_link_total_avail_slots: max. time slots + * first slot reserved for MTP header in 8b/10b, + * but not required for 128b/132b + */ + if (slots > mgr->first_link_total_avail_slots) return -ENOSPC; vcpi->pbn = pbn; @@ -4488,6 +4497,25 @@ int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, } EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); +/* + * drm_dp_mst_update_first_link_slot_info() + * update the first link's total available slots and starting slot + * @mgr: manager to store the slot info. + * @encoding_format: detected link encoding format + */ +void drm_dp_mst_update_first_link_slot_info( + struct drm_dp_mst_topology_mgr *mgr, uint8_t encoding_format) +{ + if (encoding_format == DP_CAP_ANSI_128B132B) { + mgr->first_link_total_avail_slots = 64; + mgr->first_link_start_slot = 0; + } + DRM_DEBUG_KMS("%s encoding format on 0x%p -> total %d slots, start at slot %d\n", + (encoding_format == DP_CAP_ANSI_128B132B) ? "128b/132b":"8b/10b", + mgr, mgr->first_link_total_avail_slots, mgr->first_link_start_slot); +} +EXPORT_SYMBOL(drm_dp_mst_update_first_link_slot_info); + /** * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel * @mgr: manager for this port @@ -4518,8 +4546,8 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, ret = drm_dp_init_vcpi(mgr, &port->vcpi, pbn, slots); if (ret) { - DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n", - DIV_ROUND_UP(pbn, mgr->pbn_div), ret); + DRM_DEBUG_KMS("failed to init vcpi slots=%d max=%d ret=%d\n", + DIV_ROUND_UP(pbn, mgr->pbn_div), mgr->first_link_total_avail_slots, ret); drm_dp_mst_topology_put_port(port); goto out; } @@ -5162,7 +5190,7 @@ 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; + int avail_slots = mgr->first_link_total_avail_slots, payload_count = 0; list_for_each_entry(vcpi, &mst_state->vcpis, next) { /* Releasing VCPI is always OK-even if the port is gone */ @@ -5191,7 +5219,7 @@ drm_dp_mst_atomic_check_vcpi_alloc_limit(struct drm_dp_mst_topology_mgr *mgr, } DRM_DEBUG_ATOMIC("[MST MGR:%p] mst state %p VCPI avail=%d used=%d\n", mgr, mst_state, avail_slots, - 63 - avail_slots); + mgr->first_link_total_avail_slots - avail_slots); return 0; } @@ -5455,6 +5483,8 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, if (!mgr->proposed_vcpis) return -ENOMEM; set_bit(0, &mgr->payload_mask); + mgr->first_link_total_avail_slots = 63; + mgr->first_link_start_slot = 1; mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL); if (mst_state == NULL) diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index bd1c39907b92..f4310b3705e7 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -653,6 +653,13 @@ struct drm_dp_mst_topology_mgr { */ int pbn_div; + /** + * @first_link_total_avail_slots: frist link total slots available. + * @first_link_start_slot: start slot index for real data transmission. + */ + u8 first_link_total_avail_slots; + u8 first_link_start_slot; + /** * @funcs: Atomic helper callbacks */ @@ -795,6 +802,7 @@ int drm_dp_mst_get_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp void drm_dp_mst_reset_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port); +void drm_dp_mst_update_first_link_slot_info(struct drm_dp_mst_topology_mgr *mgr, uint8_t encoding_format); void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
8b/10b encoding format requires to reserve the first slot for recording metadata. Real data transmission starts from the second slot, with a total of available 63 slots available. In 128b/132b encoding format, metadata is transmitted separately in LLCP packet before MTP. Real data transmission starts from the first slot, with a total of 64 slots available. Update the slot information after link detect. Signed-off-by: Fangzhi Zuo <Jerry.Zuo@amd.com> --- drivers/gpu/drm/drm_dp_mst_topology.c | 48 ++++++++++++++++++++++----- include/drm/drm_dp_mst_helper.h | 8 +++++ 2 files changed, 47 insertions(+), 9 deletions(-)