dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/dp/mst: don't pass port into the path builder function
@ 2015-09-16  1:22 Dave Airlie
  2015-09-16  1:22 ` [PATCH 2/2] drm/dp/mst: fixup handling hotplug on port removal Dave Airlie
  2015-09-30  7:22 ` [PATCH 1/2] drm/dp/mst: don't pass port into the path builder function Daniel Vetter
  0 siblings, 2 replies; 4+ messages in thread
From: Dave Airlie @ 2015-09-16  1:22 UTC (permalink / raw)
  To: dri-devel

From: Dave Airlie <airlied@redhat.com>

This is unnecessary and it makes it easier to see what is needed
from port.

also add blank line to make things nicer.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index e23df5f..87112d3 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1027,8 +1027,8 @@ static void drm_dp_check_port_guid(struct drm_dp_mst_branch *mstb,
 	}
 }
 
-static void build_mst_prop_path(struct drm_dp_mst_port *port,
-				struct drm_dp_mst_branch *mstb,
+static void build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
+				int pnum,
 				char *proppath,
 				size_t proppath_size)
 {
@@ -1041,7 +1041,7 @@ static void build_mst_prop_path(struct drm_dp_mst_port *port,
 		snprintf(temp, sizeof(temp), "-%d", port_num);
 		strlcat(proppath, temp, proppath_size);
 	}
-	snprintf(temp, sizeof(temp), "-%d", port->port_num);
+	snprintf(temp, sizeof(temp), "-%d", pnum);
 	strlcat(proppath, temp, proppath_size);
 }
 
@@ -1113,7 +1113,8 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
 
 	if (created && !port->input) {
 		char proppath[255];
-		build_mst_prop_path(port, mstb, proppath, sizeof(proppath));
+
+		build_mst_prop_path(mstb, port->port_num, proppath, sizeof(proppath));
 		port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr, port, proppath);
 
 		if (port->port_num >= 8) {
-- 
2.4.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] drm/dp/mst: fixup handling hotplug on port removal.
  2015-09-16  1:22 [PATCH 1/2] drm/dp/mst: don't pass port into the path builder function Dave Airlie
@ 2015-09-16  1:22 ` Dave Airlie
  2015-09-30  7:30   ` Daniel Vetter
  2015-09-30  7:22 ` [PATCH 1/2] drm/dp/mst: don't pass port into the path builder function Daniel Vetter
  1 sibling, 1 reply; 4+ messages in thread
From: Dave Airlie @ 2015-09-16  1:22 UTC (permalink / raw)
  To: dri-devel

From: Dave Airlie <airlied@redhat.com>

output ports should always have a connector, unless
in the rare case connector allocation fails in the
driver.

In this case we only need to teardown the pdt,
and free the struct, and there is no need to
send a hotplug msg.

In the case were we add the port to the destroy
list we need to send a hotplug if we destroy
any connectors, so userspace knows to reprobe
stuff.

this patch also handles port->connector allocation
failing which should be a rare event, but makes
the code consistent.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 36 +++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 87112d3..d11052c 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -863,29 +863,33 @@ static void drm_dp_destroy_port(struct kref *kref)
 {
 	struct drm_dp_mst_port *port = container_of(kref, struct drm_dp_mst_port, kref);
 	struct drm_dp_mst_topology_mgr *mgr = port->mgr;
+
 	if (!port->input) {
 		port->vcpi.num_slots = 0;
 
 		kfree(port->cached_edid);
 
-		/* we can't destroy the connector here, as
-		   we might be holding the mode_config.mutex
-		   from an EDID retrieval */
+		/*
+		 * 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);
-
-		if (!port->input && port->vcpi.vcpi > 0)
-			drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
 	}
 	kfree(port);
-
-	(*mgr->cbs->hotplug)(mgr);
 }
 
 static void drm_dp_put_port(struct drm_dp_mst_port *port)
@@ -1116,12 +1120,21 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
 
 		build_mst_prop_path(mstb, port->port_num, proppath, sizeof(proppath));
 		port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr, port, proppath);
-
+		if (!port->connector) {
+			/* remove it from the port list */
+			mutex_lock(&mstb->mgr->lock);
+			list_del(&port->next);
+			mutex_unlock(&mstb->mgr->lock);
+			/* drop port list reference */
+			drm_dp_put_port(port);
+			goto out;
+		}
 		if (port->port_num >= 8) {
 			port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
 		}
 	}
 
+out:
 	/* put reference to this port */
 	drm_dp_put_port(port);
 }
@@ -2672,7 +2685,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work)
 {
 	struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct drm_dp_mst_topology_mgr, destroy_connector_work);
 	struct drm_dp_mst_port *port;
-
+	bool send_hotplug = false;
 	/*
 	 * Not a regular list traverse as we have to drop the destroy
 	 * connector lock before destroying the connector, to avoid AB->BA
@@ -2695,7 +2708,10 @@ static void drm_dp_destroy_connector_work(struct work_struct *work)
 		if (!port->input && port->vcpi.vcpi > 0)
 			drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
 		kfree(port);
+		send_hotplug = true;
 	}
+	if (send_hotplug)
+		(*mgr->cbs->hotplug)(mgr);
 }
 
 /**
-- 
2.4.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] drm/dp/mst: don't pass port into the path builder function
  2015-09-16  1:22 [PATCH 1/2] drm/dp/mst: don't pass port into the path builder function Dave Airlie
  2015-09-16  1:22 ` [PATCH 2/2] drm/dp/mst: fixup handling hotplug on port removal Dave Airlie
@ 2015-09-30  7:22 ` Daniel Vetter
  1 sibling, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2015-09-30  7:22 UTC (permalink / raw)
  To: Dave Airlie; +Cc: dri-devel

On Wed, Sep 16, 2015 at 11:22:11AM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> This is unnecessary and it makes it easier to see what is needed
> from port.
> 
> also add blank line to make things nicer.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index e23df5f..87112d3 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1027,8 +1027,8 @@ static void drm_dp_check_port_guid(struct drm_dp_mst_branch *mstb,
>  	}
>  }
>  
> -static void build_mst_prop_path(struct drm_dp_mst_port *port,
> -				struct drm_dp_mst_branch *mstb,
> +static void build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
> +				int pnum,
>  				char *proppath,
>  				size_t proppath_size)
>  {
> @@ -1041,7 +1041,7 @@ static void build_mst_prop_path(struct drm_dp_mst_port *port,
>  		snprintf(temp, sizeof(temp), "-%d", port_num);
>  		strlcat(proppath, temp, proppath_size);
>  	}
> -	snprintf(temp, sizeof(temp), "-%d", port->port_num);
> +	snprintf(temp, sizeof(temp), "-%d", pnum);
>  	strlcat(proppath, temp, proppath_size);
>  }
>  
> @@ -1113,7 +1113,8 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
>  
>  	if (created && !port->input) {
>  		char proppath[255];
> -		build_mst_prop_path(port, mstb, proppath, sizeof(proppath));
> +
> +		build_mst_prop_path(mstb, port->port_num, proppath, sizeof(proppath));
>  		port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr, port, proppath);
>  
>  		if (port->port_num >= 8) {
> -- 
> 2.4.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] drm/dp/mst: fixup handling hotplug on port removal.
  2015-09-16  1:22 ` [PATCH 2/2] drm/dp/mst: fixup handling hotplug on port removal Dave Airlie
@ 2015-09-30  7:30   ` Daniel Vetter
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2015-09-30  7:30 UTC (permalink / raw)
  To: Dave Airlie; +Cc: dri-devel

On Wed, Sep 16, 2015 at 11:22:12AM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> output ports should always have a connector, unless
> in the rare case connector allocation fails in the
> driver.
> 
> In this case we only need to teardown the pdt,
> and free the struct, and there is no need to
> send a hotplug msg.
> 
> In the case were we add the port to the destroy
> list we need to send a hotplug if we destroy
> any connectors, so userspace knows to reprobe
> stuff.
> 
> this patch also handles port->connector allocation
> failing which should be a rare event, but makes
> the code consistent.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 36 +++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 87112d3..d11052c 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -863,29 +863,33 @@ static void drm_dp_destroy_port(struct kref *kref)
>  {
>  	struct drm_dp_mst_port *port = container_of(kref, struct drm_dp_mst_port, kref);
>  	struct drm_dp_mst_topology_mgr *mgr = port->mgr;
> +
>  	if (!port->input) {
>  		port->vcpi.num_slots = 0;
>  
>  		kfree(port->cached_edid);
>  
> -		/* we can't destroy the connector here, as
> -		   we might be holding the mode_config.mutex
> -		   from an EDID retrieval */
> +		/*
> +		 * 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);
> -
> -		if (!port->input && port->vcpi.vcpi > 0)
> -			drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
>  	}
>  	kfree(port);
> -
> -	(*mgr->cbs->hotplug)(mgr);
>  }
>  
>  static void drm_dp_put_port(struct drm_dp_mst_port *port)
> @@ -1116,12 +1120,21 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,
>  
>  		build_mst_prop_path(mstb, port->port_num, proppath, sizeof(proppath));
>  		port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr, port, proppath);
> -
> +		if (!port->connector) {
> +			/* remove it from the port list */
> +			mutex_lock(&mstb->mgr->lock);
> +			list_del(&port->next);
> +			mutex_unlock(&mstb->mgr->lock);
> +			/* drop port list reference */
> +			drm_dp_put_port(port);
> +			goto out;
> +		}
>  		if (port->port_num >= 8) {
>  			port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc);
>  		}
>  	}
>  
> +out:
>  	/* put reference to this port */
>  	drm_dp_put_port(port);
>  }
> @@ -2672,7 +2685,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work)
>  {
>  	struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct drm_dp_mst_topology_mgr, destroy_connector_work);
>  	struct drm_dp_mst_port *port;
> -
> +	bool send_hotplug = false;
>  	/*
>  	 * Not a regular list traverse as we have to drop the destroy
>  	 * connector lock before destroying the connector, to avoid AB->BA
> @@ -2695,7 +2708,10 @@ static void drm_dp_destroy_connector_work(struct work_struct *work)
>  		if (!port->input && port->vcpi.vcpi > 0)
>  			drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
>  		kfree(port);
> +		send_hotplug = true;
>  	}
> +	if (send_hotplug)
> +		(*mgr->cbs->hotplug)(mgr);
>  }
>  
>  /**
> -- 
> 2.4.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-09-30  7:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-16  1:22 [PATCH 1/2] drm/dp/mst: don't pass port into the path builder function Dave Airlie
2015-09-16  1:22 ` [PATCH 2/2] drm/dp/mst: fixup handling hotplug on port removal Dave Airlie
2015-09-30  7:30   ` Daniel Vetter
2015-09-30  7:22 ` [PATCH 1/2] drm/dp/mst: don't pass port into the path builder function Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).