[PATCH v2] drm/mst: Enhance MST topology logging
Brol, Eryk
Eryk.Brol at amd.com
Fri Mar 26 18:12:31 UTC 2021
[AMD Official Use Only - Internal Distribution Only]
Hi Lyude,
Yes, I would appreciate it if you could push this to drm-misc-next for me.
Thank you for your comments and review!
Best,
Eryk
________________________________
From: Lyude Paul <lyude at redhat.com>
Sent: Thursday, March 25, 2021 6:30 PM
To: Brol, Eryk <Eryk.Brol at amd.com>; manasi.d.navare at intel.com <manasi.d.navare at intel.com>; daniel at ffwll.ch <daniel at ffwll.ch>; Wentland, Harry <Harry.Wentland at amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira at amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas at amd.com>; Zuo, Jerry <Jerry.Zuo at amd.com>; Lin, Wayne <Wayne.Lin at amd.com>
Cc: amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>; dri-devel at lists.freedesktop.org <dri-devel at lists.freedesktop.org>
Subject: Re: [PATCH v2] drm/mst: Enhance MST topology logging
Reviewed-by: Lyude Paul <lyude at redhat.com>
Let me know if you need me to push this to drm-misc-next for you
On Thu, 2021-03-25 at 14:06 -0400, Eryk Brol wrote:
> [why]
> MST topology print was missing fec logging and pdt printed
> as an int wasn't clear. vcpi and payload info was printed as an
> arbitrary series of ints which requires user to know the ordering
> of the prints, making the logs difficult to use.
>
> [how]
> -add fec logging
> -add pdt parsing into strings
> -format vcpi and payload info into tables with headings
> -clean up topology prints
>
> ---
>
> v2: Addressed Lyude's comments
> -made helper function return const
> -fixed indentation and spacing issues
>
> Signed-off-by: Eryk Brol <eryk.brol at amd.com>
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 59 ++++++++++++++++++++++-----
> 1 file changed, 48 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 932c4641ec3e..de5124ce42cb 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4720,6 +4720,28 @@ static void drm_dp_mst_kick_tx(struct
> drm_dp_mst_topology_mgr *mgr)
> queue_work(system_long_wq, &mgr->tx_work);
> }
>
> +/*
> + * Helper function for parsing DP device types into convenient strings
> + * for use with dp_mst_topology
> + */
> +static const char *pdt_to_string(u8 pdt)
> +{
> + switch (pdt) {
> + case DP_PEER_DEVICE_NONE:
> + return "NONE";
> + case DP_PEER_DEVICE_SOURCE_OR_SST:
> + return "SOURCE OR SST";
> + case DP_PEER_DEVICE_MST_BRANCHING:
> + return "MST BRANCHING";
> + case DP_PEER_DEVICE_SST_SINK:
> + return "SST SINK";
> + case DP_PEER_DEVICE_DP_LEGACY_CONV:
> + return "DP LEGACY CONV";
> + default:
> + return "ERR";
> + }
> +}
> +
> static void drm_dp_mst_dump_mstb(struct seq_file *m,
> struct drm_dp_mst_branch *mstb)
> {
> @@ -4732,9 +4754,20 @@ static void drm_dp_mst_dump_mstb(struct seq_file *m,
> prefix[i] = '\t';
> prefix[i] = '\0';
>
> - seq_printf(m, "%smst: %p, %d\n", prefix, mstb, mstb->num_ports);
> + seq_printf(m, "%smstb - [%p]: num_ports: %d\n", prefix, mstb, mstb-
> >num_ports);
> list_for_each_entry(port, &mstb->ports, next) {
> - seq_printf(m, "%sport: %d: input: %d: pdt: %d, ddps: %d ldps:
> %d, sdp: %d/%d, %p, conn: %p\n", prefix, port->port_num, port->input, port-
> >pdt, port->ddps, port->ldps, port->num_sdp_streams, port-
> >num_sdp_stream_sinks, port, port->connector);
> + seq_printf(m, "%sport %d - [%p] (%s - %s): ddps: %d, ldps: %d,
> sdp: %d/%d, fec: %s, conn: %p\n",
> + prefix,
> + port->port_num,
> + port,
> + port->input ? "input" : "output",
> + pdt_to_string(port->pdt),
> + port->ddps,
> + port->ldps,
> + port->num_sdp_streams,
> + port->num_sdp_stream_sinks,
> + port->fec_capable ? "true" : "false",
> + port->connector);
> if (port->mstb)
> drm_dp_mst_dump_mstb(m, port->mstb);
> }
> @@ -4787,33 +4820,37 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
> mutex_unlock(&mgr->lock);
>
> mutex_lock(&mgr->payload_lock);
> - seq_printf(m, "vcpi: %lx %lx %d\n", mgr->payload_mask, mgr->vcpi_mask,
> - mgr->max_payloads);
> + seq_printf(m, "\n*** VCPI Info ***\n");
> + seq_printf(m, "payload_mask: %lx, vcpi_mask: %lx, max_payloads: %d\n",
> mgr->payload_mask, mgr->vcpi_mask, mgr->max_payloads);
>
> + seq_printf(m, "\n| idx | port # | vcp_id | # slots | sink
> name |\n");
> for (i = 0; i < mgr->max_payloads; i++) {
> if (mgr->proposed_vcpis[i]) {
> char name[14];
>
> port = container_of(mgr->proposed_vcpis[i], struct
> drm_dp_mst_port, vcpi);
> fetch_monitor_name(mgr, port, name, sizeof(name));
> - seq_printf(m, "vcpi %d: %d %d %d sink name: %s\n", i,
> - port->port_num, port->vcpi.vcpi,
> + seq_printf(m, "%10d%10d%10d%10d%20s\n",
> + i,
> + port->port_num,
> + port->vcpi.vcpi,
> port->vcpi.num_slots,
> - (*name != 0) ? name : "Unknown");
> + (*name != 0) ? name : "Unknown");
> } else
> - seq_printf(m, "vcpi %d:unused\n", i);
> + seq_printf(m, "%6d - Unused\n", i);
> }
> + seq_printf(m, "\n*** Payload Info ***\n");
> + seq_printf(m, "| idx | state | start slot | # slots |\n");
> for (i = 0; i < mgr->max_payloads; i++) {
> - seq_printf(m, "payload %d: %d, %d, %d\n",
> + seq_printf(m, "%10d%10d%15d%10d\n",
> i,
> mgr->payloads[i].payload_state,
> mgr->payloads[i].start_slot,
> mgr->payloads[i].num_slots);
> -
> -
> }
> mutex_unlock(&mgr->payload_lock);
>
> + seq_printf(m, "\n*** DPCD Info ***\n");
> mutex_lock(&mgr->lock);
> if (mgr->mst_primary) {
> u8 buf[DP_PAYLOAD_TABLE_SIZE];
--
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!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20210326/a854214b/attachment-0001.htm>
More information about the amd-gfx
mailing list