On Tue, Jan 24, 2017 at 03:49:30PM -0800, Dhinakaran Pandiyan wrote:
> The avail_slots member in the MST topology manager is never updated to
> reflect the available vcpi slots. The check is effectively against
> total_slots. So, let's make that check obvious. Secondly, since the total
> vcpi time slots is always 63 and does not depend on the link BW, remove
> total_slots from MST topology manager struct. The third change is to
> remove total_pbn which is hardcoded to 2560. The total PBN that the
> topology manager allocates from depends on the link rate and is not a
> constant. So, fix this by removing the total_pbn member itself. Finally,
> make debug messages more descriptive.
Ok, these are 3 different changes, and they need to be split up. Well, at
least in 2 patches, to get the functional change out of there:
- First hardcode avail_slots to 63, with the commit message explaining in
detail why that's the right thing. You can already remove total_pbn and
total_slots in that patch, since they will be unused. The commit message
should have a reference to the DP spec to support that "it's always 63"
claim.
- Then garbage-collect ->avail_slots in a 2nd patch.
If you smash these together it's a lot less obvious that this is not just
a pure refactoring.
Thanks, Daniel
>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 16 ++++++++--------
> include/drm/drm_dp_mst_helper.h | 12 ------------
> 2 files changed, 8 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 122a1b0..d9edd84 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2042,10 +2042,6 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
> goto out_unlock;
> }
>
> - mgr->total_pbn = 2560;
> - mgr->total_slots = DIV_ROUND_UP(mgr->total_pbn, mgr->pbn_div);
> - mgr->avail_slots = mgr->total_slots;
> -
> /* add initial branch device at LCT 1 */
> mstb = drm_dp_add_mst_branch_device(1, NULL);
> if (mstb == NULL) {
> @@ -2475,7 +2471,8 @@ int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
>
> num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
>
> - if (num_slots > mgr->avail_slots)
> + /* max. time slots - one slot for MTP header */
> + if (num_slots > 63)
> return -ENOSPC;
> return num_slots;
> }
> @@ -2489,7 +2486,8 @@ static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>
> num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
>
> - if (num_slots > mgr->avail_slots)
> + /* max. time slots - one slot for MTP header */
> + if (num_slots > 63)
> return -ENOSPC;
>
> vcpi->pbn = pbn;
> @@ -2528,10 +2526,12 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp
>
> ret = drm_dp_init_vcpi(mgr, &port->vcpi, pbn);
> if (ret) {
> - DRM_DEBUG_KMS("failed to init vcpi %d %d %d\n", DIV_ROUND_UP(pbn, mgr->pbn_div), mgr->avail_slots, ret);
> + DRM_DEBUG_KMS("failed to init vcpi slots=%d max=63 ret=%d\n",
> + DIV_ROUND_UP(pbn, mgr->pbn_div), ret);
> goto out;
> }
> - DRM_DEBUG_KMS("initing vcpi for %d %d\n", pbn, port->vcpi.num_slots);
> + DRM_DEBUG_KMS("initing vcpi for pbn=%d slots=%d\n",
> + pbn, port->vcpi.num_slots);
> *slots = port->vcpi.num_slots;
>
> drm_dp_put_port(port);
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 27f3e99..b0f4a09 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -479,18 +479,6 @@ struct drm_dp_mst_topology_mgr {
> * @pbn_div: PBN to slots divisor.
> */
> int pbn_div;
> - /**
> - * @total_slots: Total slots that can be allocated.
> - */
> - int total_slots;
> - /**
> - * @avail_slots: Still available slots that can be allocated.
> - */
> - int avail_slots;
> - /**
> - * @total_pbn: Total PBN count.
> - */
> - int total_pbn;
>
> /**
> * @qlock: protects @tx_msg_downq, the tx_slots in struct
> --
> 2.7.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
