On Wed, Jul 17, 2019 at 09:42:29PM -0400, Lyude Paul wrote:
> This will allow us to add some locking for port PDTs, which can't be
> done from drm_dp_destroy_port() since we don't know what locks the
> caller might be holding. Also, this gets rid of a good bit of unneeded
> code.
>
> Cc: Juston Li <juston.li@xxxxxxxxx>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Harry Wentland <hwentlan@xxxxxxx>
> Signed-off-by: Lyude Paul <lyude@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 42 +++++++++++----------------
> 1 file changed, 17 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index defc5e09fb9a..0295e007c836 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1509,31 +1509,22 @@ static void drm_dp_destroy_port(struct kref *kref)
> container_of(kref, struct drm_dp_mst_port, topology_kref);
> struct drm_dp_mst_topology_mgr *mgr = port->mgr;
>
> - if (!port->input) {
> - kfree(port->cached_edid);
> -
> - /*
> - * The only time we don't have a connector
> - * on an output port is if the connector init
> - * fails.
> - */
> - if (port->connector) {
> - /* we can't destroy the connector here, as
> - * we might be holding the mode_config.mutex
> - * from an EDID retrieval */
> -
> - mutex_lock(&mgr->destroy_connector_lock);
> - list_add(&port->next, &mgr->destroy_connector_list);
> - mutex_unlock(&mgr->destroy_connector_lock);
> - schedule_work(&mgr->destroy_connector_work);
> - return;
> - }
> - /* no need to clean up vcpi
> - * as if we have no connector we never setup a vcpi */
> - drm_dp_port_teardown_pdt(port, port->pdt);
> - port->pdt = DP_PEER_DEVICE_NONE;
> + /* There's nothing that needs locking to destroy an input port yet */
> + if (port->input) {
> + drm_dp_mst_put_port_malloc(port);
> + return;
> }
> - drm_dp_mst_put_port_malloc(port);
> +
> + kfree(port->cached_edid);
> +
> + /*
> + * we can't destroy the connector here, as we might be holding the
> + * mode_config.mutex from an EDID retrieval
> + */
> + mutex_lock(&mgr->destroy_connector_lock);
> + list_add(&port->next, &mgr->destroy_connector_list);
> + mutex_unlock(&mgr->destroy_connector_lock);
> + schedule_work(&mgr->destroy_connector_work);
So if I'm not completely blind this just flattens the above code flow (by
inverting the if (port->input)).
> }
>
> /**
> @@ -3881,7 +3872,8 @@ drm_dp_finish_destroy_port(struct drm_dp_mst_port *port)
> {
> INIT_LIST_HEAD(&port->next);
>
> - port->mgr->cbs->destroy_connector(port->mgr, port->connector);
> + if (port->connector)
And this here I can't connect with the commit message. I'm confused, did
something go wrong with some rebase here, and this patch should have a
different title/summary?
-Daniel
> + port->mgr->cbs->destroy_connector(port->mgr, port->connector);
>
> drm_dp_port_teardown_pdt(port, port->pdt);
> port->pdt = DP_PEER_DEVICE_NONE;
> --
> 2.21.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
