| Message ID | 20200923121320.v3.3.I6c917ff82a1d2ae2253a3bd4d750d58879387cac@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/4] dp/dp_mst: Add support for sink event notify messages | expand |
On Wed, 2020-09-23 at 12:13 +1000, Sam McNally wrote: > From: Hans Verkuil <hans.verkuil@cisco.com> > > These are required for the CEC MST support. > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > Signed-off-by: Sam McNally <sammc@chromium.org> > --- > > (no changes since v1) > > drivers/gpu/drm/drm_dp_mst_topology.c | 6 ++---- > include/drm/drm_dp_mst_helper.h | 4 ++++ > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 0d753201adbd..c783a2a1c114 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -62,8 +62,6 @@ struct drm_dp_pending_up_req { > static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr, > char *buf); > > -static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port); > - > static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr, > int id, > struct drm_dp_payload *payload); > @@ -1864,7 +1862,7 @@ static void drm_dp_mst_topology_get_port(struct > drm_dp_mst_port *port) > * drm_dp_mst_topology_try_get_port() > * drm_dp_mst_topology_get_port() > */ > -static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port) > +void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port) Mhhhhhh-can you think of some way around this? I really don't think it's a good idea for us to be exposing topology references to things as-is, the thing is they're really meant to be used for critical sections in code where it'd become very painful to deal with an mst port disappearing from under us. Outside of MST helpers, everyone else should be dealing with the expectation that these things can disappear as a result of hotplugs at any moment. Note that we do expose malloc refs, but that's intentional as holding a malloc ref to something doesn't cause it to stay around even when it's unplugged - it just stops it from being unallocated. > { > topology_ref_history_lock(port->mgr); > > @@ -1935,7 +1933,7 @@ drm_dp_mst_topology_get_port_validated_locked(struct > drm_dp_mst_branch *mstb, > return NULL; > } > > -static struct drm_dp_mst_port * > +struct drm_dp_mst_port * > drm_dp_mst_topology_get_port_validated(struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_port *port) > { > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h > index c7c79e0ced18..d036222e0d64 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -754,6 +754,10 @@ drm_dp_mst_detect_port(struct drm_connector *connector, > struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_port *port); > > +struct drm_dp_mst_port *drm_dp_mst_topology_get_port_validated > +(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port); > +void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port); > + > struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct > drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port); > >
On Tue, 2 Feb 2021 at 09:03, Lyude Paul <lyude@redhat.com> wrote: > > On Wed, 2020-09-23 at 12:13 +1000, Sam McNally wrote: > > From: Hans Verkuil <hans.verkuil@cisco.com> > > > > These are required for the CEC MST support. > > > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > > Signed-off-by: Sam McNally <sammc@chromium.org> > > --- > > > > (no changes since v1) > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 6 ++---- > > include/drm/drm_dp_mst_helper.h | 4 ++++ > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 0d753201adbd..c783a2a1c114 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -62,8 +62,6 @@ struct drm_dp_pending_up_req { > > static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr, > > char *buf); > > > > -static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port); > > - > > static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr, > > int id, > > struct drm_dp_payload *payload); > > @@ -1864,7 +1862,7 @@ static void drm_dp_mst_topology_get_port(struct > > drm_dp_mst_port *port) > > * drm_dp_mst_topology_try_get_port() > > * drm_dp_mst_topology_get_port() > > */ > > -static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port) > > +void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port) > > Mhhhhhh-can you think of some way around this? I really don't think it's a good > idea for us to be exposing topology references to things as-is, the thing is > they're really meant to be used for critical sections in code where it'd become > very painful to deal with an mst port disappearing from under us. Outside of MST > helpers, everyone else should be dealing with the expectation that these things > can disappear as a result of hotplugs at any moment. > > Note that we do expose malloc refs, but that's intentional as holding a malloc > ref to something doesn't cause it to stay around even when it's unplugged - it > just stops it from being unallocated. > > Yes, it turns out we won't need this after all. > > { > > topology_ref_history_lock(port->mgr); > > > > @@ -1935,7 +1933,7 @@ drm_dp_mst_topology_get_port_validated_locked(struct > > drm_dp_mst_branch *mstb, > > return NULL; > > } > > > > -static struct drm_dp_mst_port * > > +struct drm_dp_mst_port * > > drm_dp_mst_topology_get_port_validated(struct drm_dp_mst_topology_mgr *mgr, > > struct drm_dp_mst_port *port) > > { > > diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h > > index c7c79e0ced18..d036222e0d64 100644 > > --- a/include/drm/drm_dp_mst_helper.h > > +++ b/include/drm/drm_dp_mst_helper.h > > @@ -754,6 +754,10 @@ drm_dp_mst_detect_port(struct drm_connector *connector, > > struct drm_dp_mst_topology_mgr *mgr, > > struct drm_dp_mst_port *port); > > > > +struct drm_dp_mst_port *drm_dp_mst_topology_get_port_validated > > +(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port); > > +void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port); > > + > > struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct > > drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port); > > > > > > -- > Sincerely, > Lyude Paul (she/her) > Software Engineer at Red Hat > > Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've > asked me a question, are waiting for a review/merge on a patch, etc. and I > haven't responded in a while, please feel free to send me another email to check > on my status. I don't bite! >
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 0d753201adbd..c783a2a1c114 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -62,8 +62,6 @@ struct drm_dp_pending_up_req { static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr, char *buf); -static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port); - static int drm_dp_dpcd_write_payload(struct drm_dp_mst_topology_mgr *mgr, int id, struct drm_dp_payload *payload); @@ -1864,7 +1862,7 @@ static void drm_dp_mst_topology_get_port(struct drm_dp_mst_port *port) * drm_dp_mst_topology_try_get_port() * drm_dp_mst_topology_get_port() */ -static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port) +void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port) { topology_ref_history_lock(port->mgr); @@ -1935,7 +1933,7 @@ drm_dp_mst_topology_get_port_validated_locked(struct drm_dp_mst_branch *mstb, return NULL; } -static struct drm_dp_mst_port * +struct drm_dp_mst_port * drm_dp_mst_topology_get_port_validated(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port) { diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index c7c79e0ced18..d036222e0d64 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -754,6 +754,10 @@ drm_dp_mst_detect_port(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port); +struct drm_dp_mst_port *drm_dp_mst_topology_get_port_validated +(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port); +void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port); + struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);