On Tue, 19 Oct 2021, Bhawanpreet Lakha <bhawanpreet.la...@amd.com> 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.

Please send the patches as a fresh series instead of replying to the old
thread, it's getting too confusing. Also, please Cc: intel-gfx mailing
list, so you'll get our CI results on it. (Although I'm not sure how
many machines we have with MST setups, but it's better than nothing.)

BR,
Jani.


>
> v2:
> * Move total/start slots to mst_state, and copy it to mst_mgr in
> atomic_check
>
> v3:
> * Only keep the slot info on the mst_state
> * add a start_slot parameter to the payload function, to facilitate non
>   atomic drivers (this is a temporary workaround and should be removed when
>   we are moving out the non atomic driver helpers)
>
> Signed-off-by: Bhawanpreet Lakha <bhawanpreet.la...@amd.com>
> Signed-off-by: Fangzhi Zuo <jerry....@amd.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  2 +-
>  drivers/gpu/drm/drm_dp_mst_topology.c         | 34 ++++++++++++++++---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  4 +--
>  drivers/gpu/drm/nouveau/dispnv50/disp.c       |  2 +-
>  drivers/gpu/drm/radeon/radeon_dp_mst.c        |  4 +--
>  include/drm/drm_dp_mst_helper.h               |  5 ++-
>  6 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> index ff0f91c93ba4..6169488e2011 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
> @@ -251,7 +251,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
>       }
>  
>       /* It's OK for this to fail */
> -     drm_dp_update_payload_part1(mst_mgr);
> +     drm_dp_update_payload_part1(mst_mgr, 1);
>  
>       /* mst_mgr->->payloads are VC payload notify MST branch using DPCD or
>        * AUX message. The sequence is slot 1-63 allocated sequence for each
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 5ab3b3a46e89..d188a5269070 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3353,6 +3353,9 @@ static int drm_dp_destroy_payload_step2(struct 
> drm_dp_mst_topology_mgr *mgr,
>  /**
>   * drm_dp_update_payload_part1() - Execute payload update part 1
>   * @mgr: manager to use.
> + * @start_slot: this is the cur slot
> + *  NOTE: start_slot is a temporary workaround for non-atomic drivers,
> + *  this will be removed when non-atomic mst helpers are moved out of the 
> helper
>   *
>   * This iterates over all proposed virtual channels, and tries to
>   * allocate space in the link for them. For 0->slots transitions,
> @@ -3363,12 +3366,12 @@ static int drm_dp_destroy_payload_step2(struct 
> drm_dp_mst_topology_mgr *mgr,
>   * after calling this the driver should generate ACT and payload
>   * packets.
>   */
> -int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
> +int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr, int 
> start_slot)
>  {
>       struct drm_dp_payload req_payload;
>       struct drm_dp_mst_port *port;
>       int i, j;
> -     int cur_slots = 1;
> +     int cur_slots = start_slot;
>       bool skip;
>  
>       mutex_lock(&mgr->payload_lock);
> @@ -4503,6 +4506,26 @@ int drm_dp_atomic_release_vcpi_slots(struct 
> drm_atomic_state *state,
>  }
>  EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots);
>  
> +/**
> + * drm_dp_mst_update_slots() - updates the slot info depending on the DP 
> ecoding format
> + * @mst_state: mst_state to update
> + * @link_ecoding_cap: the ecoding format on the link
> + */
> +void drm_dp_mst_update_slots(struct drm_dp_mst_topology_state *mst_state, 
> uint8_t link_ecoding_cap)
> +{
> +     if (link_ecoding_cap == DP_CAP_ANSI_128B132B) {
> +             mst_state->total_avail_slots = 64;
> +             mst_state->start_slot = 0;
> +     } else {
> +             mst_state->total_avail_slots = 63;
> +             mst_state->start_slot = 1;
> +     }
> +
> +     DRM_DEBUG_KMS("%s ecoding format on mst_state 0x%p\n",
> +                     (link_ecoding_cap == DP_CAP_ANSI_128B132B) ? 
> "128b/132b":"8b/10b", mst_state->mgr);
> +}
> +EXPORT_SYMBOL(drm_dp_mst_update_slots);
> +
>  /**
>   * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel
>   * @mgr: manager for this port
> @@ -5222,7 +5245,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 = mst_state->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 */
> @@ -5251,7 +5274,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, 
> mst_state->total_avail_slots - avail_slots);
>  
>       return 0;
>  }
> @@ -5528,6 +5551,9 @@ int drm_dp_mst_topology_mgr_init(struct 
> drm_dp_mst_topology_mgr *mgr,
>       if (mst_state == NULL)
>               return -ENOMEM;
>  
> +     mst_state->total_avail_slots = 63;
> +     mst_state->start_slot = 1;
> +
>       mst_state->mgr = mgr;
>       INIT_LIST_HEAD(&mst_state->vcpis);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index b170e272bdee..d3a24189a12c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -368,7 +368,7 @@ static void intel_mst_disable_dp(struct 
> intel_atomic_state *state,
>  
>       drm_dp_mst_reset_vcpi_slots(&intel_dp->mst_mgr, connector->port);
>  
> -     ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> +     ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr, 1);
>       if (ret) {
>               drm_dbg_kms(&i915->drm, "failed to update payload %d\n", ret);
>       }
> @@ -516,7 +516,7 @@ static void intel_mst_pre_enable_dp(struct 
> intel_atomic_state *state,
>  
>       intel_dp->active_mst_links++;
>  
> -     ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> +     ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr, 1);
>  
>       /*
>        * Before Gen 12 this is not done as part of
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index f949767698fc..6c8c59c26dbf 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -1413,7 +1413,7 @@ nv50_mstm_prepare(struct nv50_mstm *mstm)
>       int ret;
>  
>       NV_ATOMIC(drm, "%s: mstm prepare\n", mstm->outp->base.base.name);
> -     ret = drm_dp_update_payload_part1(&mstm->mgr);
> +     ret = drm_dp_update_payload_part1(&mstm->mgr, 1);
>  
>       drm_for_each_encoder(encoder, mstm->outp->base.base.dev) {
>               if (encoder->encoder_type == DRM_MODE_ENCODER_DPMST) {
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c 
> b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> index ec867fa880a4..751c2c075e09 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> @@ -423,7 +423,7 @@ radeon_mst_encoder_dpms(struct drm_encoder *encoder, int 
> mode)
>               drm_dp_mst_allocate_vcpi(&radeon_connector->mst_port->mst_mgr,
>                                        radeon_connector->port,
>                                        mst_enc->pbn, slots);
> -             
> drm_dp_update_payload_part1(&radeon_connector->mst_port->mst_mgr);
> +             
> drm_dp_update_payload_part1(&radeon_connector->mst_port->mst_mgr, 1);
>  
>               radeon_dp_mst_set_be_cntl(primary, mst_enc,
>                                         radeon_connector->mst_port->hpd.hpd, 
> true);
> @@ -452,7 +452,7 @@ radeon_mst_encoder_dpms(struct drm_encoder *encoder, int 
> mode)
>                       return;
>  
>               
> drm_dp_mst_reset_vcpi_slots(&radeon_connector->mst_port->mst_mgr, 
> mst_enc->port);
> -             
> drm_dp_update_payload_part1(&radeon_connector->mst_port->mst_mgr);
> +             
> drm_dp_update_payload_part1(&radeon_connector->mst_port->mst_mgr, 1);
>  
>               drm_dp_check_act_status(&radeon_connector->mst_port->mst_mgr);
>               /* and this can also fail */
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index ddb9231d0309..3207b49586fc 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -554,6 +554,8 @@ struct drm_dp_mst_topology_state {
>       struct drm_private_state base;
>       struct list_head vcpis;
>       struct drm_dp_mst_topology_mgr *mgr;
> +     u8 total_avail_slots;
> +     u8 start_slot;
>  };
>  
>  #define to_dp_mst_topology_mgr(x) container_of(x, struct 
> drm_dp_mst_topology_mgr, base)
> @@ -806,6 +808,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_slots(struct drm_dp_mst_topology_state *mst_state, 
> uint8_t link_ecoding_cap);
>  
>  void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>                               struct drm_dp_mst_port *port);
> @@ -815,7 +818,7 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr 
> *mgr,
>                          int pbn);
>  
>  
> -int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> +int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr, int 
> start_slot);
>  
>  
>  int drm_dp_update_payload_part2(struct drm_dp_mst_topology_mgr *mgr);

-- 
Jani Nikula, Intel Open Source Graphics Center

Reply via email to