[Intel-gfx] [PATCH v5 3/5] drm/dp: Add DP MST helpers to atomically find and release vcpi slots

[Pandiyan, Dhinakaran]

On Thu, 2017-03-30 at 01:42 -0700, Dhinakaran Pandiyan wrote:
> From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan at intel.com>
> 
> drm_dp_atomic_find_vcpi_slots() should be called from ->atomic_check() to
> check there are sufficient vcpi slots for a mode and to add that to the
> state. This should be followed by a call to drm_dp_mst_allocate_vcpi()
> in ->atomic_commit() to initialize a struct vcpi for the port.
> 
> drm_dp_atomic_release_vcpi_slots() should be called from
> ->atomic_check() to release a port's vcpi slot allocation from the
> state.
> 
> Drivers that do not make use of this atomic helper are expected to call
> drm_dp_find_vcpi_slots() instead before calling
> drm_dp_mst_allocate_vcpi().
> 
> v2:
> Added checks for verifying the port reference is valid
> Moved get_mst_topology_state() into the helpers (Daniel)
> Changed find_vcpi_slots() to not depend on current allocation
> 
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Archit Taneja <architt at codeaurora.org>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Harry Wentland <Harry.wentland at amd.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 75 +++++++++++++++++++++++++++++++++++
>  include/drm/drm_dp_mst_helper.h       |  6 +++
>  2 files changed, 81 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 0ad0baa..9f3954e 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2498,6 +2498,81 @@ static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>  }
>  
>  /**
> + * drm_dp_atomic_find_vcpi_slots() - Find and add vcpi slots to the state
> + * @state: global atomic state
> + * @mgr: MST topology manager for the port
> + * @port: port to find vcpi slots for
> + * @pbn: bandwidth required for the mode in PBN
> + *
> + * RETURNS:
> + * Total slots in the atomic state assigned for this port or error
> + */
> +int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
> +				  struct drm_dp_mst_topology_mgr *mgr,
> +				  struct drm_dp_mst_port *port, int pbn)
> +{
> +	struct drm_dp_mst_topology_state *topology_state;
> +	int req_slots;
> +
> +	topology_state = drm_atomic_get_mst_topology_state(state, mgr);
> +	if (topology_state == NULL)
> +		return -ENOMEM;
> +
> +	port = drm_dp_get_validated_port_ref(mgr, port);
> +	if (port == NULL)
> +		return -EINVAL;
> +	req_slots = DIV_ROUND_UP(pbn, mgr->pbn_div);
> +	DRM_DEBUG_KMS("vcpi slots req=%d, avail=%d\n",
> +			req_slots, topology_state->avail_slots);
> +
> +	if (req_slots > topology_state->avail_slots) {
> +		drm_dp_put_port(port);
> +		return -ENOSPC;
> +	}
> +
> +	topology_state->avail_slots -= req_slots;
> +	DRM_DEBUG_KMS("vcpi slots avail=%d", topology_state->avail_slots);
> +
> +	drm_dp_put_port(port);
> +	return req_slots;
> +}
> +EXPORT_SYMBOL(drm_dp_atomic_find_vcpi_slots);
> +
> +/**
> + * drm_dp_atomic_release_vcpi_slots() - Release allocated vcpi slots
> + * @state: global atomic state
> + * @mgr: MST topology manager for the port
> + * @port: port to release the vcpi slots for
> + *
> + * RETURNS:
> + * Number of slots released from the atomic state for this port
> + */
> +int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state,
> +				     struct drm_dp_mst_topology_mgr *mgr,
> +				     struct drm_dp_mst_port *port)
> +{
> +	struct drm_dp_mst_topology_state *topology_state;
> +	int curr_slots;
> +
> +	topology_state = drm_atomic_get_mst_topology_state(state, mgr);
> +	if (topology_state == NULL)
> +		return -ENOMEM;
> +
> +	port = drm_dp_get_validated_port_ref(mgr, port);
> +	if (port == NULL)
> +		return -EINVAL;
> +
> +	curr_slots = port->vcpi.num_slots;

This is a problem if the port has been destroyed.

> +	topology_state->avail_slots += curr_slots;
> +	DRM_DEBUG_KMS("vcpi slots released=%d, avail=%d\n",
> +			curr_slots, topology_state->avail_slots);
> +
> +	drm_dp_put_port(port);
> +	return curr_slots;
> +}

Should we do this instead?

int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state,
                                    struct drm_dp_mst_topology_mgr *mgr,
                                    struct drm_dp_mst_port *port, int
slots)
{
       struct drm_dp_mst_topology_state *topology_state;

       topology_state = drm_atomic_get_mst_topology_state(state, mgr);
       if (topology_state == NULL)
               return -ENOMEM;

       /* We cannot rely on port->vcpi.num_slots to update
        * topology_state->avail_slots as the port may not exist if the
parent
        * branch device was unplugged. This should be fixed by tracking
        * per-port slot allocation in drm_dp_mst_topology_state instead
of
        * depending on the caller to tell us how many slots to release.
        */
       topology_state->avail_slots += slots;
       DRM_DEBUG_KMS("vcpi slots released=%d, avail=%d\n",
                       slots, topology_state->avail_slots);

       return 0;
}



> +EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots);
> +
> +/**
>   * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel
>   * @mgr: manager for this port
>   * @port: port to allocate a virtual channel for.
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 0b371df..64e7dac 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -615,5 +615,11 @@ void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr);
>  int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr);
>  struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state,
>  								    struct drm_dp_mst_topology_mgr *mgr);
> +int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
> +				  struct drm_dp_mst_topology_mgr *mgr,
> +				  struct drm_dp_mst_port *port, int pbn);
> +int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state,
> +				     struct drm_dp_mst_topology_mgr *mgr,
> +				     struct drm_dp_mst_port *port);
>  
>  #endif