On Thu, 2017-03-30 at 01:42 -0700, Dhinakaran Pandiyan wrote:

From: "Pandiyan, Dhinakaran" dhinakaran.pandiyan@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@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Archit Taneja architt@codeaurora.org Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Harry Wentland Harry.wentland@amd.com Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Signed-off-by: Dhinakaran Pandiyan dhinakaran.pandiyan@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

Op 30-03-17 om 10:42 schreef Dhinakaran Pandiyan:

From: "Pandiyan, Dhinakaran" dhinakaran.pandiyan@intel.com

Having an ->atomic_release callback is useful to release shared resources that get allocated in compute_config(). This function is expected to be called in the atomic_check() phase before new resources are acquired.

v4: Document that the function is conditionally called and before other atomic_checks() (Daniel) v3: Use the new 'for_each_oldnew_connector_in_state()' macro. v2: Moved the caller hunk to this patch (Daniel)

Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Archit Taneja architt@codeaurora.org Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Harry Wentland Harry.wentland@amd.com Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Suggested-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com

---

drivers/gpu/drm/drm_atomic_helper.c | 19 +++++++++++++++++++ include/drm/drm_modeset_helper_vtables.h | 16 ++++++++++++++++ 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index d14094d..9d07669 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -586,6 +586,25 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, } }

• for_each_oldnew_connector_in_state(state, connector, old_connector_state, new_connector_state, i) {

• const struct drm_connector_helper_funcs *conn_funcs;
  

• struct drm_crtc_state *crtc_state;
  

• conn_funcs = connector->helper_private;
  

• if (!conn_funcs->atomic_release)
  

• 	continue;
  

• if (!old_connector_state->crtc)
  

• 	continue;
  

• crtc_state = drm_atomic_get_existing_crtc_state(state, old_connector_state->crtc);
  

• if (crtc_state->connectors_changed ||
  

•     crtc_state->mode_changed ||
  

•     (crtc_state->active_changed && !crtc_state->active))
  

• 	conn_funcs->atomic_release(connector, new_connector_state);
  

• }
• return mode_fixup(state);

}

Oops, I'm just looking at patch 5 again..

atomic_release should return an int to allow error propogation. There's no good reason why it shouldn't. This would allow handling errors in patch 5 more gracefully.

EXPORT_SYMBOL(drm_atomic_helper_check_modeset); diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 091c422..84e80aa 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -836,6 +836,22 @@ struct drm_connector_helper_funcs { */ struct drm_encoder *(*atomic_best_encoder)(struct drm_connector *connector, struct drm_connector_state *connector_state);

• /**

• * @atomic_release:
  

• *
  

• * This function is conditionally called to release shared resources
  

• * when the attached CRTC's mode changes or it's connectors change or
  

• * becomes inactive. It is called before the corresponding
  

• * &drm_crtc_helper_funcs.atomic_check or
  

• * &drm_crtc_helper_funcs.mode_fixup hooks are called.
  

• *
  

• * NOTE:
  

• *
  

• * This function is called in the check phase of an atomic update.
  

• */
  

• void (*atomic_release)(struct drm_connector *connector,

• 	       struct drm_connector_state *connector_state);
  

};

/**

From: "Pandiyan, Dhinakaran" dhinakaran.pandiyan@intel.com

It is necessary to track states for objects other than connector, crtc and plane for atomic modesets. But adding objects like DP MST link bandwidth to drm_atomic_state would mean that a non-core object will be modified by the core helper functions for swapping and clearing it's state. So, lets add void * objects and helper functions that operate on void * types to keep these objects and states private to the core. Drivers can then implement specific functions to swap and clear states. The other advantage having just void * for these objects in drm_atomic_state is that objects of different types can be managed in the same state array.

v6: More kernel-doc to keep 0-day happy v5: Remove more NULL checks (Maarten) v4: Avoid redundant NULL checks when private_objs array is empty (Maarten) v3: Macro alignment (Chris) v2: Added docs and new iterator to filter private objects (Daniel)

Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Archit Taneja architt@codeaurora.org Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Harry Wentland Harry.wentland@amd.com

Acked-by: Harry Wentland harry.wentland@amd.com Suggested-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com --- drivers/gpu/drm/drm_atomic.c | 65 +++++++++++++++++++++++ drivers/gpu/drm/drm_atomic_helper.c | 5 ++ include/drm/drm_atomic.h | 101 ++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 9b892af..4d37019 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -57,6 +57,7 @@ void drm_atomic_state_default_release(struct drm_atomic_state *state) kfree(state->connectors); kfree(state->crtcs); kfree(state->planes); + kfree(state->private_objs); } EXPORT_SYMBOL(drm_atomic_state_default_release);

@@ -184,6 +185,17 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) state->planes[i].ptr = NULL; state->planes[i].state = NULL; } + + for (i = 0; i < state->num_private_objs; i++) { + void *obj_state = state->private_objs[i].obj_state; + + state->private_objs[i].funcs->destroy_state(obj_state); + state->private_objs[i].obj = NULL; + state->private_objs[i].obj_state = NULL; + state->private_objs[i].funcs = NULL; + } + state->num_private_objs = 0; + } EXPORT_SYMBOL(drm_atomic_state_default_clear);

@@ -978,6 +990,59 @@ static void drm_atomic_plane_print_state(struct drm_printer *p, }

/** + * drm_atomic_get_private_obj_state - get private object state + * @state: global atomic state + * @obj: private object to get the state for + * @funcs: pointer to the struct of function pointers that identify the object + * type + * + * This function returns the private object state for the given private object, + * allocating the state if needed. It does not grab any locks as the caller is + * expected to care of any required locking. + * + * RETURNS: + * + * Either the allocated state or the error code encoded into a pointer. + */ +void * +drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj, + const struct drm_private_state_funcs *funcs) +{ + int index, num_objs, i; + size_t size; + struct __drm_private_objs_state *arr; + + for (i = 0; i < state->num_private_objs; i++) + if (obj == state->private_objs[i].obj && + state->private_objs[i].obj_state) + return state->private_objs[i].obj_state; + + num_objs = state->num_private_objs + 1; + size = sizeof(*state->private_objs) * num_objs; + arr = krealloc(state->private_objs, size, GFP_KERNEL); + if (!arr) + return ERR_PTR(-ENOMEM); + + state->private_objs = arr; + index = state->num_private_objs; + memset(&state->private_objs[index], 0, sizeof(*state->private_objs)); + + state->private_objs[index].obj_state = funcs->duplicate_state(state, obj); + if (!state->private_objs[index].obj_state) + return ERR_PTR(-ENOMEM); + + state->private_objs[index].obj = obj; + state->private_objs[index].funcs = funcs; + state->num_private_objs = num_objs; + + DRM_DEBUG_ATOMIC("Added new private object state %p to %p\n", + state->private_objs[index].obj_state, state); + + return state->private_objs[index].obj_state; +} +EXPORT_SYMBOL(drm_atomic_get_private_obj_state); + +/** * drm_atomic_get_connector_state - get connector state * @state: global atomic state object * @connector: connector to get state object for diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index c4538c5..d14094d 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2001,6 +2001,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, struct drm_plane *plane; struct drm_plane_state *old_plane_state, *new_plane_state; struct drm_crtc_commit *commit; + void *obj, *obj_state; + const struct drm_private_state_funcs *funcs;

if (stall) { for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { @@ -2061,6 +2063,9 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, state->planes[i].state = old_plane_state; plane->state = new_plane_state; } + + __for_each_private_obj(state, obj, obj_state, i, funcs) + funcs->swap_state(obj, &state->private_objs[i].obj_state); } EXPORT_SYMBOL(drm_atomic_helper_swap_state);

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index fd33ed5..80da31c 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -155,6 +155,53 @@ struct __drm_connnectors_state { };

/** + * struct drm_private_state_funcs - atomic state functions for private objects + * + * These hooks are used by atomic helpers to create, swap and destroy states of + * private objects. The structure itself is used as a vtable to identify the + * associated private object type. Each private object type that needs to be + * added to the atomic states is expected to have an implementation of these + * hooks and pass a pointer to it's drm_private_state_funcs struct to + * drm_atomic_get_private_obj_state(). + */ +struct drm_private_state_funcs { + /** + * @duplicate_state: + * + * Duplicate the current state of the private object and return it. It + * is an error to call this before obj->state has been initialized. + * + * RETURNS: + * + * Duplicated atomic state or NULL when obj->state is not + * initialized or allocation failed. + */ + void *(*duplicate_state)(struct drm_atomic_state *state, void *obj); + + /** + * @swap_state: + * + * This function swaps the existing state of a private object @obj with + * it's newly created state, the pointer to which is passed as + * @obj_state_ptr. + */ + void (*swap_state)(void *obj, void **obj_state_ptr); + + /** + * @destroy_state: + * + * Frees the private object state created with @duplicate_state. + */ + void (*destroy_state)(void *obj_state); +}; + +struct __drm_private_objs_state { + void *obj; + void *obj_state; + const struct drm_private_state_funcs *funcs; +}; + +/** * struct drm_atomic_state - the global state object for atomic updates * @ref: count of all references to this state (will not be freed until zero) * @dev: parent DRM device @@ -165,6 +212,8 @@ struct __drm_connnectors_state { * @crtcs: pointer to array of CRTC pointers * @num_connector: size of the @connectors and @connector_states arrays * @connectors: pointer to array of structures with per-connector data + * @num_private_objs: size of the @private_objs array + * @private_objs: pointer to array of private object pointers * @acquire_ctx: acquire context for this atomic modeset state update */ struct drm_atomic_state { @@ -178,6 +227,8 @@ struct drm_atomic_state { struct __drm_crtcs_state *crtcs; int num_connector; struct __drm_connnectors_state *connectors; + int num_private_objs; + struct __drm_private_objs_state *private_objs;

struct drm_modeset_acquire_ctx *acquire_ctx;

@@ -270,6 +321,11 @@ int drm_atomic_connector_set_property(struct drm_connector *connector, struct drm_connector_state *state, struct drm_property *property, uint64_t val);

+void * __must_check +drm_atomic_get_private_obj_state(struct drm_atomic_state *state, + void *obj, + const struct drm_private_state_funcs *funcs); + /** * drm_atomic_get_existing_crtc_state - get crtc state, if it exists * @state: global atomic state object @@ -755,6 +811,51 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); for_each_if (plane)

/** + * __for_each_private_obj - iterate over all private objects + * @__state: &struct drm_atomic_state pointer + * @obj: private object iteration cursor + * @obj_state: private object state iteration cursor + * @__i: int iteration cursor, for macro-internal use + * @__funcs: &struct drm_private_state_funcs iteration cursor + * + * This macro iterates over the array containing private object data in atomic + * state + */ +#define __for_each_private_obj(__state, obj, obj_state, __i, __funcs) \ + for ((__i) = 0; \ + (__i) < (__state)->num_private_objs && \ + ((obj) = (__state)->private_objs[__i].obj, \ + (__funcs) = (__state)->private_objs[__i].funcs, \ + (obj_state) = (__state)->private_objs[__i].obj_state, \ + 1); \ + (__i)++) \ + +/** + * for_each_private_obj - iterate over a specify type of private object + * @__state: &struct drm_atomic_state pointer + * @obj_funcs: &struct drm_private_state_funcs function table to filter + * private objects + * @obj: private object iteration cursor + * @obj_state: private object state iteration cursor + * @__i: int iteration cursor, for macro-internal use + * @__funcs: &struct drm_private_state_funcs iteration cursor + * + * This macro iterates over the private objects state array while filtering the + * objects based on the vfunc table that is passed as @obj_funcs. New macros + * can be created by passing in the vfunc table associated with a specific + * private object. + */ +#define for_each_private_obj(__state, obj_funcs, obj, obj_state, __i, __funcs) \ + for ((__i) = 0; \ + (__i) < (__state)->num_private_objs && \ + ((obj) = (__state)->private_objs[__i].obj, \ + (__funcs) = (__state)->private_objs[__i].funcs, \ + (obj_state) = (__state)->private_objs[__i].obj_state, \ + 1); \ + (__i)++) \ + for_each_if (__funcs == obj_funcs) + +/** * drm_atomic_crtc_needs_modeset - compute combined modeset need * @state: &drm_crtc_state for the CRTC *

From: "Pandiyan, Dhinakaran" dhinakaran.pandiyan@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().

v3: drm_dp_atomic_release_vcpi_slots() now needs to know how many slots to release as we may not have a valid reference to port. 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@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Archit Taneja architt@codeaurora.org Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Harry Wentland Harry.wentland@amd.com Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Signed-off-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 76 +++++++++++++++++++++++++++++++++++ include/drm/drm_dp_mst_helper.h | 6 +++ 2 files changed, 82 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 0ad0baa..cf01ed3 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2498,6 +2498,82 @@ 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 + * @slots: number of vcpi slots to release for @port + * + * RETURNS: + * 0 if @slots were added back to &drm_dp_mst_topology_state->avail_slots or + * negative error code + */ +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..c781ff7 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, int slots);

#endif

From: "Pandiyan, Dhinakaran" dhinakaran.pandiyan@intel.com

Use the added helpers to track MST link bandwidth for atomic modesets. Link bw is acquired in the ->atomic_check() phase when CRTCs are being enabled with drm_atomic_find_vcpi_slots() instead of drm_find_vcpi_slots(). Similarly, link bw is released during ->atomic_check() with the connector helper callback ->atomic_release() when CRTCs are disabled.

v4: Return an int from intel_dp_mst_atomic_release() (Maarten)

v3: Handled the case where ->atomic_release() is called more than once during an atomic_check() for the same state.

v2: Squashed atomic_release() implementation and caller (Daniel) Fixed logic for connector-crtc switching case (Daniel) Fixed logic for suspend-resume case.

Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Archit Taneja architt@codeaurora.org Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Harry Wentland Harry.wentland@amd.com

Signed-off-by: Dhinakaran Pandiyan dhinakaran.pandiyan@intel.com --- drivers/gpu/drm/i915/intel_dp_mst.c | 48 +++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index c1f62eb..1db3845 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -39,9 +39,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct intel_dp *intel_dp = &intel_dig_port->dp; struct intel_connector *connector = to_intel_connector(conn_state->connector); - struct drm_atomic_state *state; + struct drm_atomic_state *state = pipe_config->base.state; int bpp; - int lane_count, slots; + int lane_count, slots = 0; const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; int mst_pbn;

@@ -57,30 +57,63 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, * seem to suggest we should do otherwise. */ lane_count = drm_dp_max_lane_count(intel_dp->dpcd); - pipe_config->lane_count = lane_count;

pipe_config->pipe_bpp = bpp; pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);

- state = pipe_config->base.state; - if (drm_dp_mst_port_has_audio(&intel_dp->mst_mgr, connector->port)) pipe_config->has_audio = true; - mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);

+ 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);

intel_link_compute_m_n(bpp, lane_count, adjusted_mode->crtc_clock, pipe_config->port_clock, &pipe_config->dp_m_n);

+ if (pipe_config->base.active) { + slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr, + connector->port, mst_pbn); + if (slots < 0) { + DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots); + return false; + } + } pipe_config->dp_m_n.tu = slots;

return true; +}

+static int intel_dp_mst_atomic_release(struct drm_connector *connector, + struct drm_connector_state *conn_state) +{ + struct intel_dp_mst_encoder *intel_mst; + struct drm_dp_mst_topology_mgr *mgr; + struct drm_encoder *encoder; + struct intel_connector *intel_connector = to_intel_connector(connector); + struct drm_atomic_state *state = conn_state->state; + struct drm_crtc *old_crtc = connector->state->crtc; + struct intel_crtc_state *old_crtc_state = + to_intel_crtc_state(old_crtc->state); + int slots; + int ret = 0; + + encoder = connector->state->best_encoder; + intel_mst = enc_to_mst(encoder); + mgr = &intel_mst->primary->dp.mst_mgr; + + slots = old_crtc_state->dp_m_n.tu; + if (slots > 0) { + ret = drm_dp_atomic_release_vcpi_slots(state, mgr, + intel_connector->port, slots); + if (ret) + DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", slots, ret); + else + old_crtc_state->dp_m_n.tu = 0; + } + return ret; }

static void intel_mst_disable_dp(struct intel_encoder *encoder, @@ -387,6 +420,7 @@ static const struct drm_connector_helper_funcs intel_dp_mst_connector_helper_fun .mode_valid = intel_dp_mst_mode_valid, .atomic_best_encoder = intel_mst_atomic_best_encoder, .best_encoder = intel_mst_best_encoder, + .atomic_release = intel_dp_mst_atomic_release, };

static void intel_dp_mst_encoder_destroy(struct drm_encoder *encoder)