[Intel-gfx] [Nouveau] [PATCH 1/6] drm/dp_mst: Add drm_dp_get_payload_info()

[Lyude Paul]

On Tue, 2018-11-27 at 22:23 +0100, Daniel Vetter wrote:
> On Fri, Nov 16, 2018 at 07:21:15PM -0500, Lyude Paul wrote:
> > Some hardware (nvidia hardware in particular) needs to be notified of
> > the exact VCPI and payload settings that the topology manager decided on
> > for each mstb port. Since there isn't currently any way to get this
> > information without going through port (which drivers are very much not
> > supposed to do by themselves, ever), let's add one.
> > 
> > Signed-off-by: Lyude Paul <lyude at redhat.com>
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 56 +++++++++++++++++++++++++++
> >  include/drm/drm_dp_mst_helper.h       |  5 ++-
> >  2 files changed, 60 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 529414556962..4336d17ce904 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -1982,6 +1982,62 @@ int drm_dp_update_payload_part2(struct
> > drm_dp_mst_topology_mgr *mgr)
> >  }
> >  EXPORT_SYMBOL(drm_dp_update_payload_part2);
> >  
> > +/**
> > + * drm_dp_get_payload_info() - Retrieve payload/vcpi information for the
> > given
> > + *                             @port
> > + * @mgr: manager to use
> > + * @port: the port to get the relevant payload information for
> > + * @vcpi_out: where to copy the port's VCPI information to
> > + * @payload_out: where to copy the port's payload information to
> > + *
> > + * Searches the current payloads for @mgr and finds the relevant payload
> > and
> > + * VCPI information that was programmed by the topology mgr, then copies
> > it
> > + * into @vcpi_out and @payload_out. Drivers which need to know this
> > + * information must use this helper as opposed to checking @port
> > themselves,
> > + * as this helper will ensure the port reference is still valid and grab
> > the
> > + * appropriate locks in @mgr.
> > + *
> > + * Returns:
> > + * 0 on success, negative error code if the port is no longer valid or a
> > + * programmed payload could not be found for @port.
> > + */
> > +int drm_dp_get_payload_info(struct drm_dp_mst_topology_mgr *mgr,
> > +			    struct drm_dp_mst_port *port,
> > +			    struct drm_dp_vcpi *vcpi_out,
> > +			    struct drm_dp_payload *payload_out)
> > +{
> > +	struct drm_dp_payload *payload = NULL;
> > +	int i;
> > +	int ret = 0;
> > +
> > +	port = drm_dp_get_validated_port_ref(mgr, port);
> > +	if (!port)
> > +		return -EINVAL;
> 
> This is the part that I mean in our other/irc discussions. The
> dp_get_validated_port here could fail when it's going to surprise the
> driver. With the dp_port_malloc_get stuff we could instead just grab a
> port_malloc_kref when storing the port in mgr->payload, which would
> guarantee that the port based lookup below still works.
Yeah, that makes more sense now that there's context :P, sgtm.

> 
> > +
> > +	mutex_lock(&mgr->payload_lock);
> > +	/* Figure out which of the payloads belongs to this port */
> > +	for (i = 0; i < mgr->max_payloads; i++) {
> > +		if (mgr->payloads[i].vcpi == port->vcpi.vcpi) {
> 
> Or maybe even rework the lookup here to use the port pointer (as an
> abstract key) instead of port->vcpi.vcpi. With port_malloc_kref we could
> guarantee that it keeps working, even after the port has been destroyed.
> 
> And (without checking) I think that's needed anyway to clean up the
> payload update hacks in the connector destroy work ...
> -Daniel
> 
> > +			payload = &mgr->payloads[i];
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (!payload) {
> > +		DRM_DEBUG_KMS("Failed to find payload for port %p\n", port);
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	*payload_out = *payload;
> > +	*vcpi_out = port->vcpi;
> > +out:
> > +	mutex_unlock(&mgr->payload_lock);
> > +	drm_dp_put_port(port);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(drm_dp_get_payload_info);
> > +
> >  #if 0 /* unused as of yet */
> >  static int drm_dp_send_dpcd_read(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 59f005b419cf..9cc93ea60e7e 100644
> > --- a/include/drm/drm_dp_mst_helper.h
> > +++ b/include/drm/drm_dp_mst_helper.h
> > @@ -592,7 +592,10 @@ bool drm_dp_mst_allocate_vcpi(struct
> > drm_dp_mst_topology_mgr *mgr,
> >  			      struct drm_dp_mst_port *port, int pbn, int
> > slots);
> >  
> >  int drm_dp_mst_get_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, struct
> > drm_dp_mst_port *port);
> > -
> > +int drm_dp_get_payload_info(struct drm_dp_mst_topology_mgr *mgr,
> > +			    struct drm_dp_mst_port *port,
> > +			    struct drm_dp_vcpi *vcpi_out,
> > +			    struct drm_dp_payload *payload_out);
> >  
> >  void drm_dp_mst_reset_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> > struct drm_dp_mst_port *port);
> >  
> > -- 
> > 2.19.1
> > 
> > _______________________________________________
> > Nouveau mailing list
> > Nouveau at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
-- 
Cheers,
	Lyude Paul