All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/mst: Enhance MST topology logging
@ 2021-03-25 18:06 ` Eryk Brol
  0 siblings, 0 replies; 8+ messages in thread
From: Eryk Brol @ 2021-03-25 18:06 UTC (permalink / raw)
  To: lyude, manasi.d.navare, daniel, harry.wentland, rodrigo.siqueira,
	nicholas.kazlauskas, jerry.zuo, wayne.lin, eryk.brol
  Cc: dri-devel, amd-gfx

[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@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];
-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2] drm/mst: Enhance MST topology logging
@ 2021-03-25 18:06 ` Eryk Brol
  0 siblings, 0 replies; 8+ messages in thread
From: Eryk Brol @ 2021-03-25 18:06 UTC (permalink / raw)
  To: lyude, manasi.d.navare, daniel, harry.wentland, rodrigo.siqueira,
	nicholas.kazlauskas, jerry.zuo, wayne.lin, eryk.brol
  Cc: dri-devel, amd-gfx

[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@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];
-- 
2.25.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] drm/mst: Enhance MST topology logging
  2021-03-25 18:06 ` Eryk Brol
@ 2021-03-25 22:30   ` Lyude Paul
  -1 siblings, 0 replies; 8+ messages in thread
From: Lyude Paul @ 2021-03-25 22:30 UTC (permalink / raw)
  To: Eryk Brol, manasi.d.navare, daniel, harry.wentland,
	rodrigo.siqueira, nicholas.kazlauskas, jerry.zuo, wayne.lin
  Cc: dri-devel, amd-gfx

Reviewed-by: Lyude Paul <lyude@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@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!

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] drm/mst: Enhance MST topology logging
@ 2021-03-25 22:30   ` Lyude Paul
  0 siblings, 0 replies; 8+ messages in thread
From: Lyude Paul @ 2021-03-25 22:30 UTC (permalink / raw)
  To: Eryk Brol, manasi.d.navare, daniel, harry.wentland,
	rodrigo.siqueira, nicholas.kazlauskas, jerry.zuo, wayne.lin
  Cc: dri-devel, amd-gfx

Reviewed-by: Lyude Paul <lyude@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@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!

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] drm/mst: Enhance MST topology logging
  2021-03-25 22:30   ` Lyude Paul
@ 2021-03-26 18:12     ` Brol, Eryk
  -1 siblings, 0 replies; 8+ messages in thread
From: Brol, Eryk @ 2021-03-26 18:12 UTC (permalink / raw)
  To: manasi.d.navare, daniel, Wentland, Harry, Siqueira,  Rodrigo,
	Kazlauskas, Nicholas, Zuo, Jerry, Lin, Wayne, lyude
  Cc: dri-devel, amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 7140 bytes --]

[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@redhat.com>
Sent: Thursday, March 25, 2021 6:30 PM
To: Brol, Eryk <Eryk.Brol@amd.com>; manasi.d.navare@intel.com <manasi.d.navare@intel.com>; daniel@ffwll.ch <daniel@ffwll.ch>; Wentland, Harry <Harry.Wentland@amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Zuo, Jerry <Jerry.Zuo@amd.com>; Lin, Wayne <Wayne.Lin@amd.com>
Cc: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH v2] drm/mst: Enhance MST topology logging

Reviewed-by: Lyude Paul <lyude@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@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!


[-- Attachment #1.2: Type: text/html, Size: 16914 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] drm/mst: Enhance MST topology logging
@ 2021-03-26 18:12     ` Brol, Eryk
  0 siblings, 0 replies; 8+ messages in thread
From: Brol, Eryk @ 2021-03-26 18:12 UTC (permalink / raw)
  To: manasi.d.navare, daniel, Wentland, Harry, Siqueira,  Rodrigo,
	Kazlauskas, Nicholas, Zuo, Jerry, Lin, Wayne, lyude
  Cc: dri-devel, amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 7140 bytes --]

[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@redhat.com>
Sent: Thursday, March 25, 2021 6:30 PM
To: Brol, Eryk <Eryk.Brol@amd.com>; manasi.d.navare@intel.com <manasi.d.navare@intel.com>; daniel@ffwll.ch <daniel@ffwll.ch>; Wentland, Harry <Harry.Wentland@amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>; Zuo, Jerry <Jerry.Zuo@amd.com>; Lin, Wayne <Wayne.Lin@amd.com>
Cc: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH v2] drm/mst: Enhance MST topology logging

Reviewed-by: Lyude Paul <lyude@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@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!


[-- Attachment #1.2: Type: text/html, Size: 16914 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] drm/mst: Enhance MST topology logging
  2021-03-25 18:06 ` Eryk Brol
@ 2021-03-26 21:20   ` Lyude Paul
  -1 siblings, 0 replies; 8+ messages in thread
From: Lyude Paul @ 2021-03-26 21:20 UTC (permalink / raw)
  To: Eryk Brol, manasi.d.navare, daniel, harry.wentland,
	rodrigo.siqueira, nicholas.kazlauskas, jerry.zuo, wayne.lin
  Cc: dri-devel, amd-gfx

pushed! thanks for the patch

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@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!

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] drm/mst: Enhance MST topology logging
@ 2021-03-26 21:20   ` Lyude Paul
  0 siblings, 0 replies; 8+ messages in thread
From: Lyude Paul @ 2021-03-26 21:20 UTC (permalink / raw)
  To: Eryk Brol, manasi.d.navare, daniel, harry.wentland,
	rodrigo.siqueira, nicholas.kazlauskas, jerry.zuo, wayne.lin
  Cc: dri-devel, amd-gfx

pushed! thanks for the patch

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@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!

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-03-26 21:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-25 18:06 [PATCH v2] drm/mst: Enhance MST topology logging Eryk Brol
2021-03-25 18:06 ` Eryk Brol
2021-03-25 22:30 ` Lyude Paul
2021-03-25 22:30   ` Lyude Paul
2021-03-26 18:12   ` Brol, Eryk
2021-03-26 18:12     ` Brol, Eryk
2021-03-26 21:20 ` Lyude Paul
2021-03-26 21:20   ` Lyude Paul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.