Remove all bad dp_mst_port uses and hide struct def
Subsystems (5)
Lyude Paul
maintainercover Remove all bad dp_mst_port uses and hide struct def
So we don't ever have to worry about drivers touching drm_dp_mst_port structs without verifying them and crashing again.
Lyude Paul (6):
drm/dp_mst: Add drm_dp_get_payload_info()
drm/nouveau: Use drm_dp_get_payload_info() for getting payload/vcpi drm/nouveau: Stop reading port->mgr in nv50_mstc_get_modes() drm/nouveau: Stop reading port->mgr in nv50_mstc_detect() drm/dp_mst: Hide drm_dp_mst_port contents from drivers drm/i915: Start using struct drm_dp_mst_port again
drivers/gpu/drm/drm_dp_mst_topology.c | 115 ++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_dp_mst.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 60 +++++-------- include/drm/drm_dp_mst_helper.h | 65 ++------------ 5 files changed, 146 insertions(+), 98 deletions(-)
Signature · 6 lines
2.19.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lyude Paul
maintainerPATCH 1/6 drm/dp_mst: Add drm_dp_get_payload_info()
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.
--- drivers/gpu/drm/drm_dp_mst_topology.c | 56 +++++++++++++++++++++++++++ include/drm/drm_dp_mst_helper.h | 5 ++- 2 files changed, 60 insertions(+), 1 deletion(-)
drivers/gpu/drm/drm_dp_mst_topology.c +56 -0
@@ -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 ; + +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) { + 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,
include/drm/drm_dp_mst_helper.h +4 -1
@@ -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);
Signature · 6 lines
2.19.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
A message that is not in the archive (sent off-list or not yet received): 20181117002120.28703-2-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Re: [PATCH 1/6] drm/dp_mst: Add drm_dp_get_payload_info()
Quoted 41 lines · Lyude Paul
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 <[email protected]>--- drivers/gpu/drm/drm_dp_mst_topology.c | 56 +++++++++++++++++++++++++++ include/drm/drm_dp_mst_helper.h | 5 ++- 2 files changed, 60 insertions(+), 1 deletion(-)drivers/gpu/drm/drm_dp_mst_topology.c +21
-0@@ -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,
drivers/gpu/drm/drm_dp_mst_topology.c int drm_dp_update_payload_part2(struct drm_dp_mst_topology_mgr *mgr) · line 2015
Quoted 10 lines
drivers/gpu/drm/drm_dp_mst_topology.c +10
-0+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.
drivers/gpu/drm/drm_dp_mst_topology.c int drm_dp_update_payload_part2(struct drm_dp_mst_topology_mgr *mgr) · line 2020
Quoted 5 lines
drivers/gpu/drm/drm_dp_mst_topology.c +5
-0+ +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
Quoted 46 lines
drivers/gpu/drm/drm_dp_mst_topology.c +20
-0+ 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,include/drm/drm_dp_mst_helper.h +4
-1@@ -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);Signature · 6 lines
2.19.1 _______________________________________________ Nouveau mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/nouveau
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Nouveau mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/nouveau
Lyude Paul
maintainerRe: [Nouveau] [PATCH 1/6] drm/dp_mst: Add drm_dp_get_payload_info()
Quoted 57 lines · Daniel Vetter
Quoted 56 lines · Lyude Paul
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 <[email protected]>--- drivers/gpu/drm/drm_dp_mst_topology.c | 56 +++++++++++++++++++++++++++ include/drm/drm_dp_mst_helper.h | 5 ++- 2 files changed, 60 insertions(+), 1 deletion(-)drivers/gpu/drm/drm_dp_mst_topology.c +2
-0@@ -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.
+ */drivers/gpu/drm/drm_dp_mst_topology.c +10
-0+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);
Quoted 8 lines
drivers/gpu/drm/drm_dp_mst_topology.c int drm_dp_update_payload_part2(struct drm_dp_mst_topology_mgr *mgr) · line 2015
Quoted 2 lines
drivers/gpu/drm/drm_dp_mst_topology.c +2
-0+if (!port) +return -EINVAL ;This is the part that I mean in our other/irc discussions. The
dp_get_validated_porthere could fail when it's going to surprise the driver. With thedp_port_malloc_getstuff we could instead just grab aport_malloc_krefwhen storing the port inmgr->payload, which would guarantee that the port based lookup below still works.
Yeah, that makes more sense now that there's context :P, sgtm.
Quoted 66 lines
drivers/gpu/drm/drm_dp_mst_topology.c int drm_dp_update_payload_part2(struct drm_dp_mst_topology_mgr *mgr) · line 2020
Quoted 6 lines
drivers/gpu/drm/drm_dp_mst_topology.c +5
-0+ +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. Withport_malloc_krefwe 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
Quoted 51 lines
drivers/gpu/drm/drm_dp_mst_topology.c +20
-0+ 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,include/drm/drm_dp_mst_helper.h +4
-1@@ -592,7 +592,10 @@ bool drm_dp_mst_allocate_vcpi(structdrm_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,
structdrm_dp_mst_port*port);Signature · 6 lines
2.19.1 _______________________________________________ Nouveau mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/nouveau
Signature · 7 lines
Cheers, Lyude Paul _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
A message that is not in the archive (sent off-list or not yet received): 20181117002120.28703-1-lyude-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
PATCH 2/6 drm/nouveau: Use drm_dp_get_payload_info() for getting payload/vcpi
Currently, nouveau tries to go through the drm_dp_mst_port structures itself in order to retrieve the relevant payload and VCPI information that it needs to report to the GPU. This is wrong: mstc->port could be destroyed at any point, and additionally the payload could be changed at any point because it doesn't bother trying to grab the payload lock. So; remove nv50_msto_payload entirely and use the new drm_dp_get_payload_info() helper.
--- drivers/gpu/drm/nouveau/dispnv50/disp.c | 55 ++++++++++--------------- 1 file changed, 21 insertions(+), 34 deletions(-)
drivers/gpu/drm/nouveau/dispnv50/disp.c +21 -34
@@ -665,41 +665,24 @@ struct nv50_msto {bool disabled; }; -static struct drm_dp_payload * -nv50_msto_payload (struct nv50_msto *msto) -{ -struct nouveau_drm *drm =nouveau_drm (msto->encoder.dev); -struct nv50_mstc *mstc = msto->mstc; -struct nv50_mstm *mstm = mstc->mstm; -int vcpi = mstc->port->vcpi.vcpi, i; - -NV_ATOMIC (drm,"%s: vcpi %d\n" , msto->encoder.name, vcpi); -for (i =0 ; i < mstm->mgr.max_payloads; i++) { -struct drm_dp_payload *payload = &mstm->mgr.payloads[i]; -NV_ATOMIC (drm,"%s: %d: vcpi %d start 0x%02x slots 0x%02x\n" , - mstm->outp->base.base.name, i, payload->vcpi, - payload->start_slot, payload->num_slots); - } - -for (i =0 ; i < mstm->mgr.max_payloads; i++) { -struct drm_dp_payload *payload = &mstm->mgr.payloads[i]; -if (payload->vcpi == vcpi) -return payload; - } - -return NULL ; -} -static void nv50_msto_cleanup (struct nv50_msto *msto) {struct nouveau_drm *drm =nouveau_drm (msto->encoder.dev);struct nv50_mstc *mstc = msto->mstc;struct nv50_mstm *mstm = mstc->mstm; +struct drm_dp_payload payload; +struct drm_dp_vcpi vcpi; +int ret;NV_ATOMIC (drm,"%s: msto cleanup\n" , msto->encoder.name); -if (mstc->port && mstc->port->vcpi.vcpi >0 && !nv50_msto_payload (msto)) -drm_dp_mst_deallocate_vcpi (&mstm->mgr, mstc->port); +if (mstc->port) { + ret =drm_dp_get_payload_info (&mstm->mgr, mstc->port, + &vcpi, &payload); +if (!ret) +drm_dp_mst_deallocate_vcpi (&mstm->mgr, mstc->port); + } +if (msto->disabled) { msto->mstc =NULL ; msto->head =NULL ;
@@ -713,6 +696,9 @@ nv50_msto_prepare(struct nv50_msto *msto)struct nouveau_drm *drm =nouveau_drm (msto->encoder.dev);struct nv50_mstc *mstc = msto->mstc;struct nv50_mstm *mstm = mstc->mstm; +struct drm_dp_payload payload; +struct drm_dp_vcpi vcpi; +int ret;struct {struct nv50_disp_mthd_v1 base;struct nv50_disp_sor_dp_mst_vcpi_v0 vcpi;
@@ -725,13 +711,14 @@ nv50_msto_prepare(struct nv50_msto *msto) };NV_ATOMIC (drm,"%s: msto prepare\n" , msto->encoder.name); -if (mstc->port && mstc->port->vcpi.vcpi >0 ) { -struct drm_dp_payload *payload =nv50_msto_payload (msto); -if (payload) { - args.vcpi.start_slot = payload->start_slot; - args.vcpi.num_slots = payload->num_slots; - args.vcpi.pbn = mstc->port->vcpi.pbn; - args.vcpi.aligned_pbn = mstc->port->vcpi.aligned_pbn; +if (mstc->port) { + ret =drm_dp_get_payload_info (&mstm->mgr, mstc->port, + &vcpi, &payload); +if (!ret) { + args.vcpi.start_slot = payload.start_slot; + args.vcpi.num_slots = payload.num_slots; + args.vcpi.pbn = vcpi.pbn; + args.vcpi.aligned_pbn = vcpi.aligned_pbn; } }
Signature · 6 lines
2.19.1 _______________________________________________ Nouveau mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/nouveau
PATCH 5/6 drm/dp_mst: Hide drm_dp_mst_port contents from drivers
In reply to a message not in the archiveIt hasn't been OK to access any of the contents of struct drm_dp_mst_port without validating the port first for quite a long time now, since a drm_dp_mst_port structure can be freed at any given moment in time outside of the driver's contorl. Any kind of information a driver needs from drm_dp_mst_port should be exposed through a helper function instead that handles validating the port pointer, along with anything else that's needed.
Since we've removed the last dangerous remanents of ->port accesses in the DRM tree, let's finish this off and move the struct drm_dp_mst_port definition out of drm_dp_mst_helper.h, into drm_dp_mst_topology.c, and then replace it's header definition with an incomplete struct type. This way drivers can still use the struct type, and no one else will make the mistake of trying to access the contents of port.
--- drivers/gpu/drm/drm_dp_mst_topology.c | 59 ++++++++++++++++++++++++++ include/drm/drm_dp_mst_helper.h | 60 +-------------------------- 2 files changed, 60 insertions(+), 59 deletions(-)
drivers/gpu/drm/drm_dp_mst_topology.c +59 -0
@@ -41,6 +41,65 @@* protocol. The helpers contain a topology manager and bandwidth manager. * The helpers encapsulate the sending and received of sideband msgs. */ + +/** +* struct drm_dp_mst_port - MST port +* @kref: reference count for this port. +* @port_num: port number +* @input: if this port is an input port. +* @mcs: message capability status - DP 1.2 spec. +* @ddps: DisplayPort Device Plug Status - DP 1.2 +* @pdt: Peer Device Type +* @ldps: Legacy Device Plug Status +* @dpcd_rev: DPCD revision of device on this port +* @num_sdp_streams: Number of simultaneous streams +* @num_sdp_stream_sinks: Number of stream sinks +* @available_pbn: Available bandwidth for this port. +* @next: link to next port on this branch device +* @mstb: branch device attach below this port +* @aux: i2c aux transport to talk to device connected to this port. +* @parent: branch device parent of this port +* @vcpi: Virtual Channel Payload info for this port. +* @connector: DRM connector this port is connected to. +* @mgr: topology manager this port lives under. +* +* This structure represents an MST port endpoint on a device somewhere +* in the MST topology. +*/ +struct drm_dp_mst_port { +struct kref kref; + +u8 port_num; +bool input; +bool mcs; +bool ddps; +u8 pdt; +bool ldps; +u8 dpcd_rev; +u8 num_sdp_streams; +u8 num_sdp_stream_sinks; +uint16_t available_pbn; +struct list_head next; +struct drm_dp_mst_branch *mstb;/* pointer to an mstb if this port has one */ +struct drm_dp_aux aux;/* i2c bus for this port? */ +struct drm_dp_mst_branch *parent; + +struct drm_dp_vcpi vcpi; +struct drm_connector *connector; +struct drm_dp_mst_topology_mgr *mgr; + +/** +* @cached_edid: for DP logical ports - make tiling work by ensuring +* that the EDID for all connectors is read immediately. +*/ +struct edid *cached_edid; +/** +* @has_audio: Tracks whether the sink connector to this port is +* audio-capable. +*/ +bool has_audio; +}; +static bool dump_dp_payload_table (struct drm_dp_mst_topology_mgr *mgr,char *buf);static int test_calc_pbn_mode (void );
include/drm/drm_dp_mst_helper.h +1 -59
72 lines not on this page · raw message
Signature · 6 lines
2.19.1 _______________________________________________ Nouveau mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/nouveau
Patches 1 and 5 are Acked-by: Karol Herbst <[email protected]> Patches 2-4 are Reviewed-by: Karol Herbst <[email protected]>
Quoted 19 lines · Lyude Paul
So we don't ever have to worry about drivers touching
drm_dp_mst_portstructs without verifying them and crashing again.Lyude Paul (6):
drm/dp_mst: Adddrm_dp_get_payload_info()
drm/nouveau: Usedrm_dp_get_payload_info()for getting payload/vcpi drm/nouveau: Stop readingport->mgrinnv50_mstc_get_modes()drm/nouveau: Stop readingport->mgrinnv50_mstc_detect()drm/dp_mst: Hidedrm_dp_mst_portcontents from drivers drm/i915: Start using structdrm_dp_mst_portagaindrivers/gpu/drm/drm_dp_mst_topology.c | 115 ++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_dp_mst.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 60 +++++-------- include/drm/drm_dp_mst_helper.h | 65 ++------------ 5 files changed, 146 insertions(+), 98 deletions(-)
Quoted 7 lines
--
2.19.1_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
Lyude Paul
maintainerPATCH 3/6 drm/nouveau: Stop reading port->mgr in nv50_mstc_get_modes()
In reply to Lyude Paulmstc->port isn't validated here so it could be null or worse when we access it. And drivers aren't ever supposed to be looking at it's contents anyway. Plus, we can already get the MST manager from &mstc->mstm->mgr.
--- drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/gpu/drm/nouveau/dispnv50/disp.c +2 -1
@@ -893,7 +893,8 @@ nv50_mstc_get_modes(struct drm_connector *connector)struct nv50_mstc *mstc =nv50_mstc (connector);int ret =0 ; - mstc->edid =drm_dp_mst_get_edid (&mstc->connector, mstc->port->mgr, mstc->port); + mstc->edid =drm_dp_mst_get_edid (&mstc->connector, + &mstc->mstm->mgr, mstc->port);drm_connector_update_edid_property (&mstc->connector, mstc->edid);if (mstc->edid) ret =drm_add_edid_modes (&mstc->connector, mstc->edid);
Signature · 1 line
2.19.1
Sasha Levin
maintainerRe: [PATCH 3/6] drm/nouveau: Stop reading port->mgr in nv50_mstc_get_modes()
Hi,
[This is an automated email]
This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v4.19.2, v4.18.19, v4.14.81, v4.9.137, v4.4.163, v3.18.125,
v4.19.2: Build OK!
v4.18.19: Failed to apply! Possible dependencies:
Unable to calculate
v4.14.81: Failed to apply! Possible dependencies:
Unable to calculate
v4.9.137: Failed to apply! Possible dependencies:
f479c0ba4a17 ("drm/nouveau/kms/nv50: initial support for DP 1.2 multi-stream")v4.4.163: Failed to apply! Possible dependencies:
f479c0ba4a17 ("drm/nouveau/kms/nv50: initial support for DP 1.2 multi-stream")v3.18.125: Failed to apply! Possible dependencies:
f479c0ba4a17 ("drm/nouveau/kms/nv50: initial support for DP 1.2 multi-stream")How should we proceed with this patch?
--
Thanks,
Sasha
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Lyude Paul
maintainerPATCH 4/6 drm/nouveau: Stop reading port->mgr in nv50_mstc_detect()
In reply to Lyude PaulSame as the previous commit, but for nv50_mstc_detect() this time.
--- drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/nouveau/dispnv50/disp.c +1 -1
@@ -930,7 +930,7 @@ nv50_mstc_detect(struct drm_connector *connector, bool force)if (ret <0 && ret != -EACCES )return connector_status_disconnected; - conn_status =drm_dp_mst_detect_port (connector, mstc->port->mgr, + conn_status =drm_dp_mst_detect_port (connector, &mstc->mstm->mgr, mstc->port);pm_runtime_mark_last_busy (connector->dev->dev);
Signature · 1 line
2.19.1
Sasha Levin
maintainerRe: [PATCH 4/6] drm/nouveau: Stop reading port->mgr in nv50_mstc_detect()
Hi,
[This is an automated email]
This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v4.19.2, v4.18.19, v4.14.81, v4.9.137, v4.4.163, v3.18.125,
v4.19.2: Build OK!
v4.18.19: Build OK!
v4.14.81: Failed to apply! Possible dependencies:
e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")v4.9.137: Failed to apply! Possible dependencies:
e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")v4.4.163: Failed to apply! Possible dependencies:
e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")v3.18.125: Failed to apply! Possible dependencies:
e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")How should we proceed with this patch?
--
Thanks,
Sasha
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Lyude Paul
maintainerRe: [PATCH 4/6] drm/nouveau: Stop reading port->mgr in nv50_mstc_detect()
Quoted 24 lines · Sasha Levin
Hi,
[This is an automated email]
This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v4.19.2, v4.18.19, v4.14.81, v4.9.137, v4.4.163, v3.18.125,
v4.19.2: Build OK!
v4.18.19: Build OK!
v4.14.81: Failed to apply! Possible dependencies:
e46368cf77f2("drm/nouveau/drm/nouveau: Grab runtime PM ref innv50_mstc_detect()")v4.9.137: Failed to apply! Possible dependencies:
e46368cf77f2("drm/nouveau/drm/nouveau: Grab runtime PM ref innv50_mstc_detect()")v4.4.163: Failed to apply! Possible dependencies:
e46368cf77f2("drm/nouveau/drm/nouveau: Grab runtime PM ref innv50_mstc_detect()")
Quoted 6 lines
v3.18.125: Failed to apply! Possible dependencies:
e46368cf77f2("drm/nouveau/drm/nouveau: Grab runtime PM ref innv50_mstc_detect()")How should we proceed with this patch?
e46368cf77f2 ("drm/nouveau/drm/nouveau: Grab runtime PM ref in nv50_mstc_detect()")
Should also be backported for v4.9+, and then this patch should be applied on top of that.
Quoted 4 lines
--
Thanks,
Sasha
Signature · 2 lines
Cheers, Lyude Paul
Lyude Paul
maintainerPATCH 6/6 drm/i915: Start using struct drm_dp_mst_port again
In reply to Lyude PaulOriginally we started storing pointers to the drm_dp_mst_port struct for each intel_connector as void* to stop people from trying to dereference them. Now that we've removed the public struct definition for drm_dp_mst_port however, it's no longer possible to dereference the port structure even when using the proper type. So, move back to struct drm_dp_mst_port from void* for clarity.
--- drivers/gpu/drm/i915/intel_dp_mst.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/intel_dp_mst.c +1 -1
@@ -39,7 +39,7 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,struct intel_digital_port *intel_dig_port = intel_mst->primary;struct intel_dp *intel_dp = &intel_dig_port->dp;struct drm_connector *connector = conn_state->connector; -void *port =to_intel_connector (connector)->port; +struct drm_dp_mst_port *port =to_intel_connector (connector)->port;struct drm_atomic_state *state = pipe_config->base.state;int bpp;int lane_count, slots =0 ;
drivers/gpu/drm/i915/intel_drv.h +1 -1
@@ -415,7 +415,7 @@ struct intel_connector {state of connector->polled in case hotplug storm detection changes it */ u8 polled; -void *port;/* store this opaque as its illegal to dereference it */ +struct drm_dp_mst_port *port;struct intel_dp *mst_port;
Signature · 6 lines
2.19.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✓ Fi.CI.BAT: success for Remove all bad dp_mst_port uses and hide struct def
In reply to Lyude Paul== Series Details ==
== Series Details ==
Series: Remove all bad dp_mst_port uses and hide struct def
URL : https://patchwork.freedesktop.org/series/52636/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5154 -> Patchwork_10844 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/52636/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_10844 that come from known issues:
=== IGT changes ===
==== Possible fixes ====
igt@gem_ctx_create@basic-files:
fi-bsw-kefka: FAIL (fdo#108656) -> PASS
igt@i915_module_load@reload:
fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
fi-byt-clapper: FAIL (fdo#103191, fdo#107362) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-b:
fi-byt-clapper: FAIL (fdo#107362) -> PASS fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656
== Participating hosts (50 -> 46) ==
Missing (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_5154 -> Patchwork_10844
CI_DRM_5154: 2ba924da1cb3a684f538e0c25e2409b942a5a35a @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4718: 8ac5cfb4db9c7bc593beec18a6be1e2ff163106c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10844: af900c99ec9037164b46a79abadd6d22d02af78e @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
af900c99ec90 drm/i915: Start using struct drm_dp_mst_port again f7e93f99d599 drm/dp_mst: Hide drm_dp_mst_port contents from drivers 1affa2235cf2 drm/nouveau: Stop reading port->mgr in nv50_mstc_detect() 010aca1e996b drm/nouveau: Stop reading port->mgr in nv50_mstc_get_modes() 81eb3aab81da drm/nouveau: Use drm_dp_get_payload_info() for getting payload/vcpi 268300d13d06 drm/dp_mst: Add drm_dp_get_payload_info()
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10844/issues.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✓ Fi.CI.IGT: success for Remove all bad dp_mst_port uses and hide struct def
In reply to Lyude Paul== Series Details ==
== Series Details ==
Series: Remove all bad dp_mst_port uses and hide struct def
URL : https://patchwork.freedesktop.org/series/52636/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_5154_full -> Patchwork_10844_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10844_full need to be verified manually.
If you think the reported changes have nothing to do with the changes introduced in Patchwork_10844_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10844_full:
=== IGT changes ===
==== Warnings ====
igt@i915_suspend@shrink:
shard-skl: INCOMPLETE (fdo#106886) -> DMESG-WARN
igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-mmap-wc:
shard-hsw: SKIP -> PASS== Known issues ==
Here are the changes found in Patchwork_10844_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_schedule@pi-ringfull-bsd:
shard-skl: NOTRUN -> FAIL (fdo#103158)
igt@gem_ppgtt@blt-vs-render-ctx0:
shard-skl: NOTRUN -> TIMEOUT (fdo#108039)
igt@kms_chv_cursor_fail@pipe-a-128x128-left-edge:
shard-skl: NOTRUN -> FAIL (fdo#104671)
igt@kms_cursor_crc@cursor-128x128-random:
shard-glk: PASS -> FAIL (fdo#103232)
igt@kms_cursor_crc@cursor-256x256-offscreen:
shard-skl: NOTRUN -> FAIL (fdo#103232) +1
igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
shard-glk: PASS -> DMESG-WARN (fdo#105763, fdo#106538)
igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-untiled:
shard-glk: PASS -> DMESG-FAIL (fdo#103184, fdo#105763, fdo#106538)
igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled:
shard-skl: PASS -> FAIL (fdo#103184)
igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled:
shard-glk: PASS -> FAIL (fdo#103184, fdo#108472)
igt@kms_flip@2x-flip-vs-expired-vblank:
shard-glk: PASS -> FAIL (fdo#105363)
igt@kms_flip@flip-vs-modeset-vs-hang-interruptible:
shard-apl: PASS -> INCOMPLETE (fdo#103927)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
shard-skl: NOTRUN -> FAIL (fdo#103167) +4
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
shard-glk: PASS -> FAIL (fdo#103167) +3
igt@kms_frontbuffer_tracking@fbcpsr-modesetfrombusy:
shard-skl: NOTRUN -> FAIL (fdo#105682)
igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-cpu:
shard-skl: PASS -> FAIL (fdo#103167)
igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
shard-skl: NOTRUN -> FAIL (fdo#108145) +2
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-glk: PASS -> FAIL (fdo#103166) +2
igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
shard-apl: PASS -> FAIL (fdo#103166)
igt@kms_properties@connector-properties-atomic:
shard-skl: NOTRUN -> FAIL (fdo#108642)
igt@kms_setmode@basic:
shard-apl: PASS -> FAIL (fdo#99912)
igt@kms_sysfs_edid_timing:
shard-skl: NOTRUN -> FAIL (fdo#100047)
igt@kms_universal_plane@universal-plane-gen9-features-pipe-a:
shard-kbl: PASS -> DMESG-WARN (fdo#105602, fdo#103558) +13
igt@kms_universal_plane@universal-plane-pipe-a-sanity:
shard-glk: PASS -> INCOMPLETE (k.org#198133, fdo#103359)
igt@perf@polling:
shard-hsw: PASS -> FAIL (fdo#102252)
igt@pm_rpm@system-suspend-devices:
shard-skl: PASS -> INCOMPLETE (fdo#107807)
==== Possible fixes ====
igt@kms_chv_cursor_fail@pipe-c-128x128-top-edge:
shard-skl: FAIL (fdo#104671) -> PASS
igt@kms_cursor_crc@cursor-64x21-random:
shard-apl: FAIL (fdo#103232) -> PASS +3
igt@kms_cursor_crc@cursor-64x64-offscreen:
shard-skl: FAIL (fdo#103232) -> PASS
igt@kms_flip_tiling@flip-y-tiled:
shard-skl: FAIL (fdo#108303) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
shard-glk: FAIL (fdo#103167) -> PASS +1
igt@kms_plane@plane-position-covered-pipe-c-planes:
shard-apl: FAIL (fdo#103166) -> PASS +1
igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
shard-skl: FAIL (fdo#107815) -> PASS
igt@kms_universal_plane@universal-plane-pipe-a-functional:
shard-glk: FAIL (fdo#103166) -> PASS +1
igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
shard-skl: INCOMPLETE (fdo#104108, fdo#107773) -> PASS
igt@pm_rpm@gem-execbuf-stress-pc8:
shard-skl: INCOMPLETE (fdo#107807) -> SKIP
==== Warnings ====
igt@kms_setmode@basic:
shard-kbl: FAIL (fdo#99912) -> DMESG-WARN (fdo#105602, fdo#103558) fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
fdo#108303 https://bugs.freedesktop.org/show_bug.cgi?id=108303
fdo#108472 https://bugs.freedesktop.org/show_bug.cgi?id=108472
fdo#108642 https://bugs.freedesktop.org/show_bug.cgi?id=108642
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (7 -> 6) ==
Missing (1): shard-iclb
== Build changes ==
* Linux: CI_DRM_5154 -> Patchwork_10844
CI_DRM_5154: 2ba924da1cb3a684f538e0c25e2409b942a5a35a @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4718: 8ac5cfb4db9c7bc593beec18a6be1e2ff163106c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10844: af900c99ec9037164b46a79abadd6d22d02af78e @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10844/shards.html
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
For the series:
Quoted 19 lines · Lyude Paul
So we don't ever have to worry about drivers touching
drm_dp_mst_portstructs without verifying them and crashing again.Lyude Paul (6):
drm/dp_mst: Adddrm_dp_get_payload_info()
drm/nouveau: Usedrm_dp_get_payload_info()for getting payload/vcpi drm/nouveau: Stop readingport->mgrinnv50_mstc_get_modes()drm/nouveau: Stop readingport->mgrinnv50_mstc_detect()drm/dp_mst: Hidedrm_dp_mst_portcontents from drivers drm/i915: Start using structdrm_dp_mst_portagaindrivers/gpu/drm/drm_dp_mst_topology.c | 115 ++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_dp_mst.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 60 +++++-------- include/drm/drm_dp_mst_helper.h | 65 ++------------ 5 files changed, 146 insertions(+), 98 deletions(-)
Quoted 7 lines
--
2.19.1_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel