[PATCH v2 2/9] drm/dp: Kill unused MST vcpi slot availability tracking

[Daniel Vetter]

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 at intel.com>
> ---
>  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