Re: [PATCH 3/3] drm/dp/mst: Track available time slots in DP Multi-Stream Transport Packet

[Manasi Navare <manasi.d.navare@intel.com>]

On Thu, Nov 17, 2016 at 06:03:48PM -0800, Dhinakaran Pandiyan wrote:
> The avail_slots member in struct drm_dp_mst_topology_mgr does not really
> track the available time slots in a MTP(Multi-Stream Transport Packet). It
> is assigned an initial value when the topology manager is setup but not
> updated after that.
> 
> So, let's use avail_slots to store the number of unallocated slots out of
> the total 64. The value will be updated when vcpi allocation or reset
> happens. Since vcpi allocation and deallocation can happen simultaneously,
> the struct drm_dp_mst_topology_mgr.lock mutex is used to protect
> it from concurrent accesses.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 55 ++++++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_dp_mst.c   |  5 +++-
>  include/drm/drm_dp_mst_helper.h       |  2 +-
>  3 files changed, 47 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 26dfd99..19e2250 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2040,7 +2040,9 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
>  		}
>  		mgr->total_pbn = 64 * mgr->pbn_div;
>  		mgr->total_slots = 64;
> -		mgr->avail_slots = mgr->total_slots;
> +
> +		/* 1 slot out of the 64 time slots is used for MTP header */
> +		mgr->avail_slots = mgr->total_slots - 1;
>  
>  		/* add initial branch device at LCT 1 */
>  		mstb = drm_dp_add_mst_branch_device(1, NULL);
> @@ -2465,34 +2467,52 @@ EXPORT_SYMBOL(drm_dp_mst_get_edid);
>   * @pbn: payload bandwidth to convert into slots.
>   */
>  int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> -			   int pbn)
> +			   struct drm_dp_mst_port *port, int pbn)
>  {
> -	int num_slots;
> +	int req_slots, curr_slots, new_slots, ret;
> +
> +	req_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
> +	curr_slots = drm_dp_mst_get_vcpi_slots(mgr, port);
>  
> -	num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
> +	if (req_slots <= curr_slots)
> +		return req_slots;

Are you sure you want to return from the function at this
or should this just be ret = req_slots as you are returning ret at the end of the function.

Manasi
>  
> -	if (num_slots > mgr->avail_slots)
> -		return -ENOSPC;
> -	return num_slots;
> +	new_slots = req_slots - curr_slots;
> +	mutex_lock(&mgr->lock);
> +	if (new_slots <= mgr->avail_slots) {
> +		ret = req_slots;
> +	} else {
> +		DRM_DEBUG_KMS("not enough vcpi slots, req=%d avail=%d\n", req_slots, mgr->avail_slots);
> +		ret =  -ENOSPC;
> +	}
> +	mutex_unlock(&mgr->lock);
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_dp_find_vcpi_slots);
>  
>  static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>  			    struct drm_dp_vcpi *vcpi, int pbn)
>  {
> -	int num_slots;
> +	int req_slots;
>  	int ret;
>  
> -	num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
> +	req_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
>  
> -	if (num_slots > mgr->avail_slots)
> -		return -ENOSPC;
> +	mutex_lock(&mgr->lock);
> +	if (req_slots > mgr->avail_slots) {
> +		ret = -ENOSPC;
> +		goto out;
> +	}
>  
>  	vcpi->pbn = pbn;
> -	vcpi->aligned_pbn = num_slots * mgr->pbn_div;
> -	vcpi->num_slots = num_slots;
> +	vcpi->aligned_pbn = req_slots * mgr->pbn_div;
> +	vcpi->num_slots = req_slots;
>  
>  	ret = drm_dp_mst_assign_payload_id(mgr, vcpi);
> +
> +out:
> +	mutex_unlock(&mgr->lock);
>  	if (ret < 0)
>  		return ret;
>  	return 0;
> @@ -2530,6 +2550,10 @@ bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp
>  	DRM_DEBUG_KMS("initing vcpi for %d %d\n", pbn, port->vcpi.num_slots);
>  	*slots = port->vcpi.num_slots;
>  
> +	mutex_lock(&mgr->lock);
> +	mgr->avail_slots -= port->vcpi.num_slots;
> +	mutex_unlock(&mgr->lock);
> +
>  	drm_dp_put_port(port);
>  	return true;
>  out:
> @@ -2562,6 +2586,11 @@ void drm_dp_mst_reset_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct drm
>  	port = drm_dp_get_validated_port_ref(mgr, port);
>  	if (!port)
>  		return;
> +
> +	mutex_lock(&mgr->lock);
> +	mgr->avail_slots += port->vcpi.num_slots;
> +	mutex_unlock(&mgr->lock);
> +
>  	port->vcpi.num_slots = 0;
>  	drm_dp_put_port(port);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 4280a83..bad9300 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -42,6 +42,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	int lane_count, slots;
>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	int mst_pbn;
> +	struct intel_connector *connector =
> +		to_intel_connector(conn_state->connector);
>  
>  	pipe_config->dp_encoder_is_mst = true;
>  	pipe_config->has_pch_encoder = false;
> @@ -62,7 +64,8 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
>  
>  	pipe_config->pbn = mst_pbn;
> -	slots = drm_dp_find_vcpi_slots(&intel_dp->mst_mgr, mst_pbn);
> +	slots = drm_dp_find_vcpi_slots(&intel_dp->mst_mgr, connector->port,
> +				       mst_pbn);
>  	if (slots < 0) {
>  		DRM_ERROR("not enough available time slots for pbn=%d", mst_pbn);
>  		return false;
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 0032076..5c55528 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -590,7 +590,7 @@ void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>  
>  
>  int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> -			   int pbn);
> +			   struct drm_dp_mst_port *port, int pbn);
>  
>  
>  int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx