Message ID | 20210827234322.2740301-2-Jerry.Zuo@amd.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Update 128b/132b MST Slot Information | expand |
Hi Fangzhi, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.14-rc7 next-20210827] [cannot apply to drm/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Fangzhi-Zuo/Update-128b-132b-MST-Slot-Information/20210828-082044 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: parisc-randconfig-r002-20210827 (attached as .config) compiler: hppa-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/8232240b519fadd6ad9eb814925e513a15407474 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Fangzhi-Zuo/Update-128b-132b-MST-Slot-Information/20210828-082044 git checkout 8232240b519fadd6ad9eb814925e513a15407474 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=parisc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/gpu/drm/drm_dp_mst_topology.c:4512:6: warning: no previous prototype for 'drm_dp_mst_update_encoding_cap' [-Wmissing-prototypes] 4512 | void drm_dp_mst_update_encoding_cap(struct drm_dp_mst_topology_mgr *mgr, uint8_t link_encoding_cap) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/drm_dp_mst_update_encoding_cap +4512 drivers/gpu/drm/drm_dp_mst_topology.c 4511 > 4512 void drm_dp_mst_update_encoding_cap(struct drm_dp_mst_topology_mgr *mgr, uint8_t link_encoding_cap) 4513 { 4514 if (link_encoding_cap == DP_CAP_ANSI_128B132B) { 4515 mgr->total_avail_slots = 64; 4516 mgr->start_slot = 0; 4517 } 4518 DRM_DEBUG_KMS("%s encoding format determined\n", 4519 (link_encoding_cap == DP_CAP_ANSI_128B132B) ? "128b/132b" : "8b/10b"); 4520 } 4521 EXPORT_SYMBOL(drm_dp_mst_update_encoding_cap); 4522 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Fri, 2021-08-27 at 19:43 -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. > > Signed-off-by: Fangzhi Zuo <Jerry.Zuo@amd.com> > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 27 ++++++++++++++++++++------- > include/drm/drm_dp_mst_helper.h | 9 +++++++++ > 2 files changed, 29 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 86d13d6bc463..30544801d2b8 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -3370,7 +3370,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->start_slot; > bool skip; > > mutex_lock(&mgr->payload_lock); > @@ -4323,7 +4323,7 @@ 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) > + if (num_slots > mgr->total_avail_slots) > return -ENOSPC; > return num_slots; > } > @@ -4335,7 +4335,7 @@ 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) > + if (slots > mgr->total_avail_slots) > return -ENOSPC; > > vcpi->pbn = pbn; > @@ -4509,6 +4509,17 @@ int drm_dp_atomic_release_vcpi_slots(struct > drm_atomic_state *state, > } > EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); > > +void drm_dp_mst_update_encoding_cap(struct drm_dp_mst_topology_mgr *mgr, > uint8_t link_encoding_cap) > +{ > + if (link_encoding_cap == DP_CAP_ANSI_128B132B) { > + mgr->total_avail_slots = 64; > + mgr->start_slot = 0; > + } > + DRM_DEBUG_KMS("%s encoding format determined\n", > + (link_encoding_cap == DP_CAP_ANSI_128B132B) ? > "128b/132b" : "8b/10b"); > +} > +EXPORT_SYMBOL(drm_dp_mst_update_encoding_cap); > + This seems to be missing kdocs, can you fix that? Also - I'm not convinced this is all of the work we have to do, as there's no locking taking place here in this function. If we're changing the number of available VCPI slots that we have, we need to be able to factor that into the atomic check logic which means that we can't rely on mgr->* for any kind of data that isn't guaranteed to remain consistent throughout the lifetime of the driver or topology. (Note that some of the old MST code didn't follow this logic, so I wouldn't be surprised if there's still exceptions to this we need to clean up). Note that I still expect we'll have to keep some sort of track of the current total slot count in the topology mgr, but that should be reflecting the currently programmed state and not be relied on from our atomic check. IMHO - the correct way we should go about adding support for this is to add something into drm_dp_mst_topology_state and integrate this into the atomic check helpers. > /** > * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel > * @mgr: manager for this port > @@ -4540,8 +4551,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_dbg_kms(mgr->dev, "failed to init vcpi slots=%d max=63 > ret=%d\n", > - DIV_ROUND_UP(pbn, mgr->pbn_div), ret); > + drm_dbg_kms(mgr->dev, "failed to init vcpi slots=%d max=%d > ret=%d\n", > + DIV_ROUND_UP(pbn, mgr->pbn_div), mgr- > >total_avail_slots, ret); > drm_dp_mst_topology_put_port(port); > goto out; > } > @@ -5228,7 +5239,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->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 */ > @@ -5257,7 +5268,7 @@ drm_dp_mst_atomic_check_vcpi_alloc_limit(struct > drm_dp_mst_topology_mgr *mgr, > } > } > drm_dbg_atomic(mgr->dev, "[MST MGR:%p] mst state %p VCPI avail=%d > used=%d\n", > - mgr, mst_state, avail_slots, 63 - avail_slots); > + mgr, mst_state, avail_slots, mgr->total_avail_slots - > avail_slots); > > return 0; > } > @@ -5529,6 +5540,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->total_avail_slots = 63; > + mgr->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 ddb9231d0309..eac5ce48f214 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -661,6 +661,15 @@ struct drm_dp_mst_topology_mgr { > */ > int pbn_div; > > + /** > + * @total_avail_slots: available slots for data transmission > + */ > + u8 total_avail_slots; > + /** > + * @start_slot: first slot index for data transmission > + */ > + u8 start_slot; > + > /** > * @funcs: Atomic helper callbacks > */
[AMD Official Use Only] > -----Original Message----- > From: Lyude Paul <lyude@redhat.com> > Sent: August 30, 2021 4:00 PM > To: Zuo, Jerry <Jerry.Zuo@amd.com>; dri-devel@lists.freedesktop.org > Cc: Wentland, Harry <Harry.Wentland@amd.com>; Kazlauskas, Nicholas > <Nicholas.Kazlauskas@amd.com>; Lin, Wayne <Wayne.Lin@amd.com> > Subject: Re: [PATCH 1/2] drm: Update MST First Link Slot Information Based > on Encoding Format > > On Fri, 2021-08-27 at 19:43 -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. > > > > Signed-off-by: Fangzhi Zuo <Jerry.Zuo@amd.com> > > --- > > drivers/gpu/drm/drm_dp_mst_topology.c | 27 > > ++++++++++++++++++++------- > > include/drm/drm_dp_mst_helper.h | 9 +++++++++ > > 2 files changed, 29 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 86d13d6bc463..30544801d2b8 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -3370,7 +3370,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->start_slot; > > bool skip; > > > > mutex_lock(&mgr->payload_lock); @@ -4323,7 +4323,7 @@ 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) > > + if (num_slots > mgr->total_avail_slots) > > return -ENOSPC; > > return num_slots; > > } > > @@ -4335,7 +4335,7 @@ 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) > > + if (slots > mgr->total_avail_slots) > > return -ENOSPC; > > > > vcpi->pbn = pbn; > > @@ -4509,6 +4509,17 @@ int drm_dp_atomic_release_vcpi_slots(struct > > drm_atomic_state *state, > > } > > EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); > > > > +void drm_dp_mst_update_encoding_cap(struct > drm_dp_mst_topology_mgr > > +*mgr, > > uint8_t link_encoding_cap) > > +{ > > + if (link_encoding_cap == DP_CAP_ANSI_128B132B) { > > + mgr->total_avail_slots = 64; > > + mgr->start_slot = 0; > > + } > > + DRM_DEBUG_KMS("%s encoding format determined\n", > > + (link_encoding_cap == DP_CAP_ANSI_128B132B) ? > > "128b/132b" : "8b/10b"); > > +} > > +EXPORT_SYMBOL(drm_dp_mst_update_encoding_cap); > > + > > This seems to be missing kdocs, can you fix that? > > Also - I'm not convinced this is all of the work we have to do, as there's no > locking taking place here in this function. If we're changing the number of > available VCPI slots that we have, we need to be able to factor that into the > atomic check logic which means that we can't rely on mgr->* for any kind of > data that isn't guaranteed to remain consistent throughout the lifetime of the > driver or topology. (Note that some of the old MST code didn't follow this > logic, so I wouldn't be surprised if there's still exceptions to this we need to > clean up). > > Note that I still expect we'll have to keep some sort of track of the current > total slot count in the topology mgr, but that should be reflecting the > currently programmed state and not be relied on from our atomic check. > Thanks Lyude for your comments. Seems I should keep existing code to keep track of current slot status in mgr. That information is getting updated each time when topology change detected. That slot information saved in mgr is a sort of static, and could only be used for debug purpose to track what is the current encoding format. > IMHO - the correct way we should go about adding support for this is to add > something into drm_dp_mst_topology_state and integrate this into the > atomic check helpers. The slot information should also be added into drm_dp_mst_topology_state to reflect the real-time slot status. I'd like to confirm the best place to get slot count info. updated. Should the update be done within &drm_mode_config_funcs. atomic_check(), before new stream is created, OR should be updated within drm_dp_mst_atomic_check() ? The updated slot count will be used in drm_dp_mst_atomic_check() to check slot limit, and in drm_dp_update_payload_part1() as initial cur_slots. > > > /** > > * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel > > * @mgr: manager for this port > > @@ -4540,8 +4551,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_dbg_kms(mgr->dev, "failed to init vcpi slots=%d > > max=63 ret=%d\n", > > - DIV_ROUND_UP(pbn, mgr->pbn_div), ret); > > + drm_dbg_kms(mgr->dev, "failed to init vcpi slots=%d > > +max=%d > > ret=%d\n", > > + DIV_ROUND_UP(pbn, mgr->pbn_div), mgr- > > >total_avail_slots, ret); > > drm_dp_mst_topology_put_port(port); > > goto out; > > } > > @@ -5228,7 +5239,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->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 */ @@ -5257,7 +5268,7 @@ > > drm_dp_mst_atomic_check_vcpi_alloc_limit(struct > > drm_dp_mst_topology_mgr *mgr, > > } > > } > > drm_dbg_atomic(mgr->dev, "[MST MGR:%p] mst state %p VCPI > > avail=%d used=%d\n", > > - mgr, mst_state, avail_slots, 63 - avail_slots); > > + mgr, mst_state, avail_slots, > > +mgr->total_avail_slots - > > avail_slots); > > > > return 0; > > } > > @@ -5529,6 +5540,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->total_avail_slots = 63; > > + mgr->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 ddb9231d0309..eac5ce48f214 > > 100644 > > --- a/include/drm/drm_dp_mst_helper.h > > +++ b/include/drm/drm_dp_mst_helper.h > > @@ -661,6 +661,15 @@ struct drm_dp_mst_topology_mgr { > > */ > > int pbn_div; > > > > + /** > > + * @total_avail_slots: available slots for data transmission > > + */ > > + u8 total_avail_slots; > > + /** > > + * @start_slot: first slot index for data transmission > > + */ > > + u8 start_slot; > > + > > /** > > * @funcs: Atomic helper callbacks > > */ > > -- > Cheers, > Lyude Paul (she/her) > Software Engineer at Red Hat
On Tue, 2021-08-31 at 19:44 +0000, Zuo, Jerry wrote: > [AMD Official Use Only] > > > -----Original Message----- > > From: Lyude Paul <lyude@redhat.com> > > Sent: August 30, 2021 4:00 PM > > To: Zuo, Jerry <Jerry.Zuo@amd.com>; dri-devel@lists.freedesktop.org > > Cc: Wentland, Harry <Harry.Wentland@amd.com>; Kazlauskas, Nicholas > > <Nicholas.Kazlauskas@amd.com>; Lin, Wayne <Wayne.Lin@amd.com> > > Subject: Re: [PATCH 1/2] drm: Update MST First Link Slot Information Based > > on Encoding Format > > > > On Fri, 2021-08-27 at 19:43 -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. > > > > > > Signed-off-by: Fangzhi Zuo <Jerry.Zuo@amd.com> > > > --- > > > drivers/gpu/drm/drm_dp_mst_topology.c | 27 > > > ++++++++++++++++++++------- > > > include/drm/drm_dp_mst_helper.h | 9 +++++++++ > > > 2 files changed, 29 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > index 86d13d6bc463..30544801d2b8 100644 > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > @@ -3370,7 +3370,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->start_slot; > > > bool skip; > > > > > > mutex_lock(&mgr->payload_lock); @@ -4323,7 +4323,7 @@ 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) > > > + if (num_slots > mgr->total_avail_slots) > > > return -ENOSPC; > > > return num_slots; > > > } > > > @@ -4335,7 +4335,7 @@ 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) > > > + if (slots > mgr->total_avail_slots) > > > return -ENOSPC; > > > > > > vcpi->pbn = pbn; > > > @@ -4509,6 +4509,17 @@ int drm_dp_atomic_release_vcpi_slots(struct > > > drm_atomic_state *state, > > > } > > > EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); > > > > > > +void drm_dp_mst_update_encoding_cap(struct > > drm_dp_mst_topology_mgr > > > +*mgr, > > > uint8_t link_encoding_cap) > > > +{ > > > + if (link_encoding_cap == DP_CAP_ANSI_128B132B) { > > > + mgr->total_avail_slots = 64; > > > + mgr->start_slot = 0; > > > + } > > > + DRM_DEBUG_KMS("%s encoding format determined\n", > > > + (link_encoding_cap == DP_CAP_ANSI_128B132B) ? > > > "128b/132b" : "8b/10b"); > > > +} > > > +EXPORT_SYMBOL(drm_dp_mst_update_encoding_cap); > > > + > > > > This seems to be missing kdocs, can you fix that? > > > > Also - I'm not convinced this is all of the work we have to do, as there's > > no > > locking taking place here in this function. If we're changing the number > > of > > available VCPI slots that we have, we need to be able to factor that into > > the > > atomic check logic which means that we can't rely on mgr->* for any kind > > of > > data that isn't guaranteed to remain consistent throughout the lifetime of > > the > > driver or topology. (Note that some of the old MST code didn't follow this > > logic, so I wouldn't be surprised if there's still exceptions to this we > > need to > > clean up). > > > > Note that I still expect we'll have to keep some sort of track of the > > current > > total slot count in the topology mgr, but that should be reflecting the > > currently programmed state and not be relied on from our atomic check. > > > > Thanks Lyude for your comments. > > Seems I should keep existing code to keep track of current slot status in > mgr. > That information is getting updated each time when topology change detected. > That slot information saved in mgr is a sort of static, and could only be > used > for debug purpose to track what is the current encoding format. Ahh - in that case maybe we should try getting rid of it (as we could just use the atomic state for debugging output anyway). > > > IMHO - the correct way we should go about adding support for this is to > > add > > something into drm_dp_mst_topology_state and integrate this into the > > atomic check helpers. > > The slot information should also be added into drm_dp_mst_topology_state to > reflect the real-time slot status. > > I'd like to confirm the best place to get slot count info. updated. > Should the update be done within &drm_mode_config_funcs. atomic_check(), > before new stream is created, OR > should be updated within drm_dp_mst_atomic_check() ? drm_dp_mst_atomic_check() would be better, since I imagine that we have to do this for all drivers using MST > > The updated slot count will be used in drm_dp_mst_atomic_check() to check > slot limit, > and in drm_dp_update_payload_part1() as initial cur_slots. > > > > > > /** > > > * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel > > > * @mgr: manager for this port > > > @@ -4540,8 +4551,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_dbg_kms(mgr->dev, "failed to init vcpi slots=%d > > > max=63 ret=%d\n", > > > - DIV_ROUND_UP(pbn, mgr->pbn_div), ret); > > > + drm_dbg_kms(mgr->dev, "failed to init vcpi slots=%d > > > +max=%d > > > ret=%d\n", > > > + DIV_ROUND_UP(pbn, mgr->pbn_div), mgr- > > > > total_avail_slots, ret); > > > drm_dp_mst_topology_put_port(port); > > > goto out; > > > } > > > @@ -5228,7 +5239,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->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 */ @@ -5257,7 +5268,7 @@ > > > drm_dp_mst_atomic_check_vcpi_alloc_limit(struct > > > drm_dp_mst_topology_mgr *mgr, > > > } > > > } > > > drm_dbg_atomic(mgr->dev, "[MST MGR:%p] mst state %p VCPI > > > avail=%d used=%d\n", > > > - mgr, mst_state, avail_slots, 63 - avail_slots); > > > + mgr, mst_state, avail_slots, > > > +mgr->total_avail_slots - > > > avail_slots); > > > > > > return 0; > > > } > > > @@ -5529,6 +5540,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->total_avail_slots = 63; > > > + mgr->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 ddb9231d0309..eac5ce48f214 > > > 100644 > > > --- a/include/drm/drm_dp_mst_helper.h > > > +++ b/include/drm/drm_dp_mst_helper.h > > > @@ -661,6 +661,15 @@ struct drm_dp_mst_topology_mgr { > > > */ > > > int pbn_div; > > > > > > + /** > > > + * @total_avail_slots: available slots for data transmission > > > + */ > > > + u8 total_avail_slots; > > > + /** > > > + * @start_slot: first slot index for data transmission > > > + */ > > > + u8 start_slot; > > > + > > > /** > > > * @funcs: Atomic helper callbacks > > > */ > > > > -- > > Cheers, > > Lyude Paul (she/her) > > Software Engineer at Red Hat >
(I am going to try responding to this tomorrow btw. I haven't been super busy this week, but this has been a surprisingly difficult email to respond to because I need to actually need to do a deep dive some of the MST helpers tomorrow to figure out more of the specifics on why I realized we couldn't just hot add/remove port->connector here). On Tue, 2021-08-31 at 19:44 +0000, Zuo, Jerry wrote: > [AMD Official Use Only] > > > -----Original Message----- > > From: Lyude Paul <lyude@redhat.com> > > Sent: August 30, 2021 4:00 PM > > To: Zuo, Jerry <Jerry.Zuo@amd.com>; dri-devel@lists.freedesktop.org > > Cc: Wentland, Harry <Harry.Wentland@amd.com>; Kazlauskas, Nicholas > > <Nicholas.Kazlauskas@amd.com>; Lin, Wayne <Wayne.Lin@amd.com> > > Subject: Re: [PATCH 1/2] drm: Update MST First Link Slot Information Based > > on Encoding Format > > > > On Fri, 2021-08-27 at 19:43 -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. > > > > > > Signed-off-by: Fangzhi Zuo <Jerry.Zuo@amd.com> > > > --- > > > drivers/gpu/drm/drm_dp_mst_topology.c | 27 > > > ++++++++++++++++++++------- > > > include/drm/drm_dp_mst_helper.h | 9 +++++++++ > > > 2 files changed, 29 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > index 86d13d6bc463..30544801d2b8 100644 > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > @@ -3370,7 +3370,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->start_slot; > > > bool skip; > > > > > > mutex_lock(&mgr->payload_lock); @@ -4323,7 +4323,7 @@ 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) > > > + if (num_slots > mgr->total_avail_slots) > > > return -ENOSPC; > > > return num_slots; > > > } > > > @@ -4335,7 +4335,7 @@ 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) > > > + if (slots > mgr->total_avail_slots) > > > return -ENOSPC; > > > > > > vcpi->pbn = pbn; > > > @@ -4509,6 +4509,17 @@ int drm_dp_atomic_release_vcpi_slots(struct > > > drm_atomic_state *state, > > > } > > > EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); > > > > > > +void drm_dp_mst_update_encoding_cap(struct > > drm_dp_mst_topology_mgr > > > +*mgr, > > > uint8_t link_encoding_cap) > > > +{ > > > + if (link_encoding_cap == DP_CAP_ANSI_128B132B) { > > > + mgr->total_avail_slots = 64; > > > + mgr->start_slot = 0; > > > + } > > > + DRM_DEBUG_KMS("%s encoding format determined\n", > > > + (link_encoding_cap == DP_CAP_ANSI_128B132B) ? > > > "128b/132b" : "8b/10b"); > > > +} > > > +EXPORT_SYMBOL(drm_dp_mst_update_encoding_cap); > > > + > > > > This seems to be missing kdocs, can you fix that? > > > > Also - I'm not convinced this is all of the work we have to do, as there's > > no > > locking taking place here in this function. If we're changing the number > > of > > available VCPI slots that we have, we need to be able to factor that into > > the > > atomic check logic which means that we can't rely on mgr->* for any kind > > of > > data that isn't guaranteed to remain consistent throughout the lifetime of > > the > > driver or topology. (Note that some of the old MST code didn't follow this > > logic, so I wouldn't be surprised if there's still exceptions to this we > > need to > > clean up). > > > > Note that I still expect we'll have to keep some sort of track of the > > current > > total slot count in the topology mgr, but that should be reflecting the > > currently programmed state and not be relied on from our atomic check. > > > > Thanks Lyude for your comments. > > Seems I should keep existing code to keep track of current slot status in > mgr. > That information is getting updated each time when topology change detected. > That slot information saved in mgr is a sort of static, and could only be > used > for debug purpose to track what is the current encoding format. > > > IMHO - the correct way we should go about adding support for this is to > > add > > something into drm_dp_mst_topology_state and integrate this into the > > atomic check helpers. > > The slot information should also be added into drm_dp_mst_topology_state to > reflect the real-time slot status. > > I'd like to confirm the best place to get slot count info. updated. > Should the update be done within &drm_mode_config_funcs. atomic_check(), > before new stream is created, OR > should be updated within drm_dp_mst_atomic_check() ? > > The updated slot count will be used in drm_dp_mst_atomic_check() to check > slot limit, > and in drm_dp_update_payload_part1() as initial cur_slots. > > > > > > /** > > > * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel > > > * @mgr: manager for this port > > > @@ -4540,8 +4551,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_dbg_kms(mgr->dev, "failed to init vcpi slots=%d > > > max=63 ret=%d\n", > > > - DIV_ROUND_UP(pbn, mgr->pbn_div), ret); > > > + drm_dbg_kms(mgr->dev, "failed to init vcpi slots=%d > > > +max=%d > > > ret=%d\n", > > > + DIV_ROUND_UP(pbn, mgr->pbn_div), mgr- > > > > total_avail_slots, ret); > > > drm_dp_mst_topology_put_port(port); > > > goto out; > > > } > > > @@ -5228,7 +5239,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->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 */ @@ -5257,7 +5268,7 @@ > > > drm_dp_mst_atomic_check_vcpi_alloc_limit(struct > > > drm_dp_mst_topology_mgr *mgr, > > > } > > > } > > > drm_dbg_atomic(mgr->dev, "[MST MGR:%p] mst state %p VCPI > > > avail=%d used=%d\n", > > > - mgr, mst_state, avail_slots, 63 - avail_slots); > > > + mgr, mst_state, avail_slots, > > > +mgr->total_avail_slots - > > > avail_slots); > > > > > > return 0; > > > } > > > @@ -5529,6 +5540,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->total_avail_slots = 63; > > > + mgr->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 ddb9231d0309..eac5ce48f214 > > > 100644 > > > --- a/include/drm/drm_dp_mst_helper.h > > > +++ b/include/drm/drm_dp_mst_helper.h > > > @@ -661,6 +661,15 @@ struct drm_dp_mst_topology_mgr { > > > */ > > > int pbn_div; > > > > > > + /** > > > + * @total_avail_slots: available slots for data transmission > > > + */ > > > + u8 total_avail_slots; > > > + /** > > > + * @start_slot: first slot index for data transmission > > > + */ > > > + u8 start_slot; > > > + > > > /** > > > * @funcs: Atomic helper callbacks > > > */ > > > > -- > > Cheers, > > Lyude Paul (she/her) > > Software Engineer at Red Hat >
(sorry - I just wrote a second response to your email I meant to send to your coworker Jerry Zuo! You can ignore it) On Tue, 2021-08-31 at 19:44 +0000, Zuo, Jerry wrote: > [AMD Official Use Only] > > > -----Original Message----- > > From: Lyude Paul <lyude@redhat.com> > > Sent: August 30, 2021 4:00 PM > > To: Zuo, Jerry <Jerry.Zuo@amd.com>; dri-devel@lists.freedesktop.org > > Cc: Wentland, Harry <Harry.Wentland@amd.com>; Kazlauskas, Nicholas > > <Nicholas.Kazlauskas@amd.com>; Lin, Wayne <Wayne.Lin@amd.com> > > Subject: Re: [PATCH 1/2] drm: Update MST First Link Slot Information Based > > on Encoding Format > > > > On Fri, 2021-08-27 at 19:43 -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. > > > > > > Signed-off-by: Fangzhi Zuo <Jerry.Zuo@amd.com> > > > --- > > > drivers/gpu/drm/drm_dp_mst_topology.c | 27 > > > ++++++++++++++++++++------- > > > include/drm/drm_dp_mst_helper.h | 9 +++++++++ > > > 2 files changed, 29 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > index 86d13d6bc463..30544801d2b8 100644 > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > @@ -3370,7 +3370,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->start_slot; > > > bool skip; > > > > > > mutex_lock(&mgr->payload_lock); @@ -4323,7 +4323,7 @@ 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) > > > + if (num_slots > mgr->total_avail_slots) > > > return -ENOSPC; > > > return num_slots; > > > } > > > @@ -4335,7 +4335,7 @@ 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) > > > + if (slots > mgr->total_avail_slots) > > > return -ENOSPC; > > > > > > vcpi->pbn = pbn; > > > @@ -4509,6 +4509,17 @@ int drm_dp_atomic_release_vcpi_slots(struct > > > drm_atomic_state *state, > > > } > > > EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); > > > > > > +void drm_dp_mst_update_encoding_cap(struct > > drm_dp_mst_topology_mgr > > > +*mgr, > > > uint8_t link_encoding_cap) > > > +{ > > > + if (link_encoding_cap == DP_CAP_ANSI_128B132B) { > > > + mgr->total_avail_slots = 64; > > > + mgr->start_slot = 0; > > > + } > > > + DRM_DEBUG_KMS("%s encoding format determined\n", > > > + (link_encoding_cap == DP_CAP_ANSI_128B132B) ? > > > "128b/132b" : "8b/10b"); > > > +} > > > +EXPORT_SYMBOL(drm_dp_mst_update_encoding_cap); > > > + > > > > This seems to be missing kdocs, can you fix that? > > > > Also - I'm not convinced this is all of the work we have to do, as there's > > no > > locking taking place here in this function. If we're changing the number > > of > > available VCPI slots that we have, we need to be able to factor that into > > the > > atomic check logic which means that we can't rely on mgr->* for any kind > > of > > data that isn't guaranteed to remain consistent throughout the lifetime of > > the > > driver or topology. (Note that some of the old MST code didn't follow this > > logic, so I wouldn't be surprised if there's still exceptions to this we > > need to > > clean up). > > > > Note that I still expect we'll have to keep some sort of track of the > > current > > total slot count in the topology mgr, but that should be reflecting the > > currently programmed state and not be relied on from our atomic check. > > > > Thanks Lyude for your comments. > > Seems I should keep existing code to keep track of current slot status in > mgr. > That information is getting updated each time when topology change detected. > That slot information saved in mgr is a sort of static, and could only be > used > for debug purpose to track what is the current encoding format. > > > IMHO - the correct way we should go about adding support for this is to > > add > > something into drm_dp_mst_topology_state and integrate this into the > > atomic check helpers. > > The slot information should also be added into drm_dp_mst_topology_state to > reflect the real-time slot status. > > I'd like to confirm the best place to get slot count info. updated. > Should the update be done within &drm_mode_config_funcs. atomic_check(), > before new stream is created, OR > should be updated within drm_dp_mst_atomic_check() ? > > The updated slot count will be used in drm_dp_mst_atomic_check() to check > slot limit, > and in drm_dp_update_payload_part1() as initial cur_slots. > > > > > > /** > > > * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel > > > * @mgr: manager for this port > > > @@ -4540,8 +4551,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_dbg_kms(mgr->dev, "failed to init vcpi slots=%d > > > max=63 ret=%d\n", > > > - DIV_ROUND_UP(pbn, mgr->pbn_div), ret); > > > + drm_dbg_kms(mgr->dev, "failed to init vcpi slots=%d > > > +max=%d > > > ret=%d\n", > > > + DIV_ROUND_UP(pbn, mgr->pbn_div), mgr- > > > > total_avail_slots, ret); > > > drm_dp_mst_topology_put_port(port); > > > goto out; > > > } > > > @@ -5228,7 +5239,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->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 */ @@ -5257,7 +5268,7 @@ > > > drm_dp_mst_atomic_check_vcpi_alloc_limit(struct > > > drm_dp_mst_topology_mgr *mgr, > > > } > > > } > > > drm_dbg_atomic(mgr->dev, "[MST MGR:%p] mst state %p VCPI > > > avail=%d used=%d\n", > > > - mgr, mst_state, avail_slots, 63 - avail_slots); > > > + mgr, mst_state, avail_slots, > > > +mgr->total_avail_slots - > > > avail_slots); > > > > > > return 0; > > > } > > > @@ -5529,6 +5540,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->total_avail_slots = 63; > > > + mgr->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 ddb9231d0309..eac5ce48f214 > > > 100644 > > > --- a/include/drm/drm_dp_mst_helper.h > > > +++ b/include/drm/drm_dp_mst_helper.h > > > @@ -661,6 +661,15 @@ struct drm_dp_mst_topology_mgr { > > > */ > > > int pbn_div; > > > > > > + /** > > > + * @total_avail_slots: available slots for data transmission > > > + */ > > > + u8 total_avail_slots; > > > + /** > > > + * @start_slot: first slot index for data transmission > > > + */ > > > + u8 start_slot; > > > + > > > /** > > > * @funcs: Atomic helper callbacks > > > */ > > > > -- > > Cheers, > > Lyude Paul (she/her) > > Software Engineer at Red Hat >
...I meant Wayne Lin, whoops. been going through a lot of emails today
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 86d13d6bc463..30544801d2b8 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3370,7 +3370,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->start_slot; bool skip; mutex_lock(&mgr->payload_lock); @@ -4323,7 +4323,7 @@ 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) + if (num_slots > mgr->total_avail_slots) return -ENOSPC; return num_slots; } @@ -4335,7 +4335,7 @@ 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) + if (slots > mgr->total_avail_slots) return -ENOSPC; vcpi->pbn = pbn; @@ -4509,6 +4509,17 @@ int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, } EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); +void drm_dp_mst_update_encoding_cap(struct drm_dp_mst_topology_mgr *mgr, uint8_t link_encoding_cap) +{ + if (link_encoding_cap == DP_CAP_ANSI_128B132B) { + mgr->total_avail_slots = 64; + mgr->start_slot = 0; + } + DRM_DEBUG_KMS("%s encoding format determined\n", + (link_encoding_cap == DP_CAP_ANSI_128B132B) ? "128b/132b" : "8b/10b"); +} +EXPORT_SYMBOL(drm_dp_mst_update_encoding_cap); + /** * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel * @mgr: manager for this port @@ -4540,8 +4551,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_dbg_kms(mgr->dev, "failed to init vcpi slots=%d max=63 ret=%d\n", - DIV_ROUND_UP(pbn, mgr->pbn_div), ret); + drm_dbg_kms(mgr->dev, "failed to init vcpi slots=%d max=%d ret=%d\n", + DIV_ROUND_UP(pbn, mgr->pbn_div), mgr->total_avail_slots, ret); drm_dp_mst_topology_put_port(port); goto out; } @@ -5228,7 +5239,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->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 */ @@ -5257,7 +5268,7 @@ drm_dp_mst_atomic_check_vcpi_alloc_limit(struct drm_dp_mst_topology_mgr *mgr, } } drm_dbg_atomic(mgr->dev, "[MST MGR:%p] mst state %p VCPI avail=%d used=%d\n", - mgr, mst_state, avail_slots, 63 - avail_slots); + mgr, mst_state, avail_slots, mgr->total_avail_slots - avail_slots); return 0; } @@ -5529,6 +5540,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->total_avail_slots = 63; + mgr->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 ddb9231d0309..eac5ce48f214 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -661,6 +661,15 @@ struct drm_dp_mst_topology_mgr { */ int pbn_div; + /** + * @total_avail_slots: available slots for data transmission + */ + u8 total_avail_slots; + /** + * @start_slot: first slot index for data transmission + */ + u8 start_slot; + /** * @funcs: Atomic helper callbacks */
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. Signed-off-by: Fangzhi Zuo <Jerry.Zuo@amd.com> --- drivers/gpu/drm/drm_dp_mst_topology.c | 27 ++++++++++++++++++++------- include/drm/drm_dp_mst_helper.h | 9 +++++++++ 2 files changed, 29 insertions(+), 7 deletions(-)