| Message ID | 20190903204645.25487-18-lyude@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DP MST Refactors + debugging tools + suspend/resume reprobing | expand |
On Tue, Sep 03, 2019 at 04:45:55PM -0400, Lyude Paul wrote: > The names for these functions are rather confusing. drm_dp_add_port() > sounds like a function that would simply create a port and add it to a > topology, and do nothing more. Similarly, drm_dp_update_port() would be > assumed to be the function that should be used to update port > information after initial creation. > > While those assumptions are currently correct in how these functions are > used, a quick glance at drm_dp_add_port() reveals that drm_dp_add_port() > can also update the information on a port, and seems explicitly designed > to do so. This can be explained pretty simply by the fact that there's > more situations that would involve updating the port information based > on a link address response as opposed to a connection status > notification than the driver's initial topology probe. Case in point: > reprobing link addresses after suspend/resume. > > Since we're about to start using drm_dp_add_port() differently for > suspend/resume reprobing, let's rename both functions to clarify what > they actually do. > > Cc: Juston Li <juston.li@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Harry Wentland <hwentlan@amd.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Lyude Paul <lyude@redhat.com> Reviewed-by: Sean Paul <sean@poorly.run> > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c > index 9944ef2ce885..cfaf9eb7ace9 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -1900,9 +1900,10 @@ void drm_dp_mst_connector_early_unregister(struct drm_connector *connector, > } > EXPORT_SYMBOL(drm_dp_mst_connector_early_unregister); > > -static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, > - struct drm_device *dev, > - struct drm_dp_link_addr_reply_port *port_msg) > +static void > +drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb, > + struct drm_device *dev, > + struct drm_dp_link_addr_reply_port *port_msg) > { > struct drm_dp_mst_topology_mgr *mgr = mstb->mgr; > struct drm_dp_mst_port *port; > @@ -2011,8 +2012,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, > drm_dp_mst_topology_put_port(port); > } > > -static void drm_dp_update_port(struct drm_dp_mst_branch *mstb, > - struct drm_dp_connection_status_notify *conn_stat) > +static void > +drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb, > + struct drm_dp_connection_status_notify *conn_stat) > { > struct drm_dp_mst_port *port; > int old_ddps; > @@ -2464,7 +2466,8 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr, > drm_dp_check_mstb_guid(mstb, reply->guid); > > for (i = 0; i < reply->nports; i++) > - drm_dp_add_port(mstb, mgr->dev, &reply->ports[i]); > + drm_dp_mst_handle_link_address_port(mstb, mgr->dev, > + &reply->ports[i]); > > drm_kms_helper_hotplug_event(mgr->dev); > > @@ -3324,7 +3327,7 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr) > } > > if (msg.req_type == DP_CONNECTION_STATUS_NOTIFY) { > - drm_dp_update_port(mstb, &msg.u.conn_stat); > + drm_dp_mst_handle_conn_stat(mstb, &msg.u.conn_stat); > > DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d pdt: %d\n", > msg.u.conn_stat.port_number, > -- > 2.21.0 >
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 9944ef2ce885..cfaf9eb7ace9 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1900,9 +1900,10 @@ void drm_dp_mst_connector_early_unregister(struct drm_connector *connector, } EXPORT_SYMBOL(drm_dp_mst_connector_early_unregister); -static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, - struct drm_device *dev, - struct drm_dp_link_addr_reply_port *port_msg) +static void +drm_dp_mst_handle_link_address_port(struct drm_dp_mst_branch *mstb, + struct drm_device *dev, + struct drm_dp_link_addr_reply_port *port_msg) { struct drm_dp_mst_topology_mgr *mgr = mstb->mgr; struct drm_dp_mst_port *port; @@ -2011,8 +2012,9 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, drm_dp_mst_topology_put_port(port); } -static void drm_dp_update_port(struct drm_dp_mst_branch *mstb, - struct drm_dp_connection_status_notify *conn_stat) +static void +drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb, + struct drm_dp_connection_status_notify *conn_stat) { struct drm_dp_mst_port *port; int old_ddps; @@ -2464,7 +2466,8 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr, drm_dp_check_mstb_guid(mstb, reply->guid); for (i = 0; i < reply->nports; i++) - drm_dp_add_port(mstb, mgr->dev, &reply->ports[i]); + drm_dp_mst_handle_link_address_port(mstb, mgr->dev, + &reply->ports[i]); drm_kms_helper_hotplug_event(mgr->dev); @@ -3324,7 +3327,7 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr) } if (msg.req_type == DP_CONNECTION_STATUS_NOTIFY) { - drm_dp_update_port(mstb, &msg.u.conn_stat); + drm_dp_mst_handle_conn_stat(mstb, &msg.u.conn_stat); DRM_DEBUG_KMS("Got CSN: pn: %d ldps:%d ddps: %d mcs: %d ip: %d pdt: %d\n", msg.u.conn_stat.port_number,
The names for these functions are rather confusing. drm_dp_add_port() sounds like a function that would simply create a port and add it to a topology, and do nothing more. Similarly, drm_dp_update_port() would be assumed to be the function that should be used to update port information after initial creation. While those assumptions are currently correct in how these functions are used, a quick glance at drm_dp_add_port() reveals that drm_dp_add_port() can also update the information on a port, and seems explicitly designed to do so. This can be explained pretty simply by the fact that there's more situations that would involve updating the port information based on a link address response as opposed to a connection status notification than the driver's initial topology probe. Case in point: reprobing link addresses after suspend/resume. Since we're about to start using drm_dp_add_port() differently for suspend/resume reprobing, let's rename both functions to clarify what they actually do. Cc: Juston Li <juston.li@intel.com> Cc: Imre Deak <imre.deak@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Harry Wentland <hwentlan@amd.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Lyude Paul <lyude@redhat.com> --- drivers/gpu/drm/drm_dp_mst_topology.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)