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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
