[PATCH 1/3] drm/edid: Add drm_edid_get_monitor_name()

[PATCH 1/3] drm/edid: Add drm_edid_get_monitor_name()

[Jim Bride]

In order to include monitor name information in debugfs
output we needed to add a function that would extract the
monitor name from the EDID, and that function needed to
reside in the file  where the rest of the EDID helper
functions are implemented.

cc: dri-devel@lists.freedesktop.org
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 28 ++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  1 +
 2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 558ef9f..fc69a46 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3569,6 +3569,34 @@ struct drm_connector *drm_select_eld(struct drm_encoder *encoder)
 EXPORT_SYMBOL(drm_select_eld);
 
 /**
+ * drm_edid_get_monitor_name - fetch the monitor name from the edid
+ * @edid: monitor EDID information
+ * @name: pointer to a character array of at least 13 chars to hold the name
+ *
+ * Return: True if the name could be extracted, false otherwise
+ */
+bool drm_edid_get_monitor_name(struct edid *edid, char *name)
+{
+	char *edid_name = NULL;
+	int mnl;
+
+	if (!edid || !name)
+		return false;
+
+	drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name);
+	for (mnl = 0; edid_name && mnl < 13; mnl++) {
+		if (edid_name[mnl] == 0x0a) {
+			name[mnl] = '\0';
+			break;
+		}
+		name[mnl] = edid_name[mnl];
+	}
+
+	return mnl ? true : false;
+}
+EXPORT_SYMBOL(drm_edid_get_monitor_name);
+
+/**
  * drm_detect_hdmi_monitor - detect whether monitor is HDMI
  * @edid: monitor EDID information
  *
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8cb377c..2590168 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -2500,6 +2500,7 @@ extern int drm_edid_header_is_valid(const u8 *raw_edid);
 extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
 				 bool *edid_corrupt);
 extern bool drm_edid_is_valid(struct edid *edid);
+extern bool drm_edid_get_monitor_name(struct edid *edid, char *name);
 
 extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
 							 char topology[8]);
-- 
2.5.0

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

Re: [PATCH 1/3] drm/edid: Add drm_edid_get_monitor_name()

[Jani Nikula]

On Tue, 05 Apr 2016, Jim Bride <jim.bride@linux.intel.com> wrote:
> In order to include monitor name information in debugfs
> output we needed to add a function that would extract the
> monitor name from the EDID, and that function needed to
> reside in the file  where the rest of the EDID helper
> functions are implemented.
>
> cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 28 ++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h     |  1 +
>  2 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 558ef9f..fc69a46 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3569,6 +3569,34 @@ struct drm_connector *drm_select_eld(struct drm_encoder *encoder)
>  EXPORT_SYMBOL(drm_select_eld);
>  
>  /**
> + * drm_edid_get_monitor_name - fetch the monitor name from the edid
> + * @edid: monitor EDID information
> + * @name: pointer to a character array of at least 13 chars to hold the name

Mixed feelings about "at least 13 chars". It might be simpler for this
one thing, but I hate to see this assumption of "at least 13 chars" get
spread around in patch 2 to where it's no longer obvious where this
requirement comes from.

Seeing that this is mostly copy-paste from drm_edid_to_eld(), I think
this patch should be an abstraction of that code to a separate function.

> + *
> + * Return: True if the name could be extracted, false otherwise
> + */
> +bool drm_edid_get_monitor_name(struct edid *edid, char *name)
> +{
> +	char *edid_name = NULL;
> +	int mnl;
> +
> +	if (!edid || !name)
> +		return false;
> +
> +	drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name);
> +	for (mnl = 0; edid_name && mnl < 13; mnl++) {
> +		if (edid_name[mnl] == 0x0a) {
> +			name[mnl] = '\0';

Depending on edid_name you might not terminate the string. And, in fact,
if you make this an abstraction for drm_edid_to_eld(), you might be
better off *not* terminating, which OTOH is not nice for your other use
in patch 2.

So how about first adding an internal abstraction:

static int get_monitor_name(struct edid *edid, char name[13])

which does *not* terminate the string, and returns length, 0 for
edid_name == NULL. Works nicely for drm_edid_to_eld().

Then you could add the external interface on top of that:

static void drm_edid_get_monitor_name(struct edid *edid, char *buf, int bufsize)

which would always terminate the string (assuming bufsize > 0), even on
errors so you can get rid of the return value. This simplifies your
follow up code, and if we later on need more robust error handling, it's
easy to add the return value.

BR,
Jani.


> +			break;
> +		}
> +		name[mnl] = edid_name[mnl];
> +	}
> +
> +	return mnl ? true : false;
> +}
> +EXPORT_SYMBOL(drm_edid_get_monitor_name);
> +
> +/**
>   * drm_detect_hdmi_monitor - detect whether monitor is HDMI
>   * @edid: monitor EDID information
>   *
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 8cb377c..2590168 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -2500,6 +2500,7 @@ extern int drm_edid_header_is_valid(const u8 *raw_edid);
>  extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>  				 bool *edid_corrupt);
>  extern bool drm_edid_is_valid(struct edid *edid);
> +extern bool drm_edid_get_monitor_name(struct edid *edid, char *name);
>  
>  extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
>  							 char topology[8]);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/edid: Add drm_edid_get_monitor_name()

[Jim Bride]

On Wed, Apr 06, 2016 at 11:35:44AM +0300, Jani Nikula wrote:
> On Tue, 05 Apr 2016, Jim Bride <jim.bride@linux.intel.com> wrote:
> > In order to include monitor name information in debugfs
> > output we needed to add a function that would extract the
> > monitor name from the EDID, and that function needed to
> > reside in the file  where the rest of the EDID helper
> > functions are implemented.
> >
> > cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_edid.c | 28 ++++++++++++++++++++++++++++
> >  include/drm/drm_crtc.h     |  1 +
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 558ef9f..fc69a46 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -3569,6 +3569,34 @@ struct drm_connector *drm_select_eld(struct drm_encoder *encoder)
> >  EXPORT_SYMBOL(drm_select_eld);
> >  
> >  /**
> > + * drm_edid_get_monitor_name - fetch the monitor name from the edid
> > + * @edid: monitor EDID information
> > + * @name: pointer to a character array of at least 13 chars to hold the name
> 
> Mixed feelings about "at least 13 chars". It might be simpler for this
> one thing, but I hate to see this assumption of "at least 13 chars" get
> spread around in patch 2 to where it's no longer obvious where this
> requirement comes from.
> 
> Seeing that this is mostly copy-paste from drm_edid_to_eld(), I think
> this patch should be an abstraction of that code to a separate function.

Yeah, I see what you mean.  Writing something that works with both this
use and drm_edid_to_eld() for the name parsing makes sense.

> > + *
> > + * Return: True if the name could be extracted, false otherwise
> > + */
> > +bool drm_edid_get_monitor_name(struct edid *edid, char *name)
> > +{
> > +	char *edid_name = NULL;
> > +	int mnl;
> > +
> > +	if (!edid || !name)
> > +		return false;
> > +
> > +	drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name);
> > +	for (mnl = 0; edid_name && mnl < 13; mnl++) {
> > +		if (edid_name[mnl] == 0x0a) {
> > +			name[mnl] = '\0';
> 
> Depending on edid_name you might not terminate the string. And, in fact,
> if you make this an abstraction for drm_edid_to_eld(), you might be
> better off *not* terminating, which OTOH is not nice for your other use
> in patch 2.
> 
> So how about first adding an internal abstraction:
> 
> static int get_monitor_name(struct edid *edid, char name[13])
> 
> which does *not* terminate the string, and returns length, 0 for
> edid_name == NULL. Works nicely for drm_edid_to_eld().
> 
> Then you could add the external interface on top of that:
> 
> static void drm_edid_get_monitor_name(struct edid *edid, char *buf, int bufsize)
> 
> which would always terminate the string (assuming bufsize > 0), even on
> errors so you can get rid of the return value. This simplifies your
> follow up code, and if we later on need more robust error handling, it's
> easy to add the return value.

Seems reasonable; I'll update the patch.

Jim

> BR,
> Jani.
> 
> 
> > +			break;
> > +		}
> > +		name[mnl] = edid_name[mnl];
> > +	}
> > +
> > +	return mnl ? true : false;
> > +}
> > +EXPORT_SYMBOL(drm_edid_get_monitor_name);
> > +
> > +/**
> >   * drm_detect_hdmi_monitor - detect whether monitor is HDMI
> >   * @edid: monitor EDID information
> >   *
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 8cb377c..2590168 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -2500,6 +2500,7 @@ extern int drm_edid_header_is_valid(const u8 *raw_edid);
> >  extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
> >  				 bool *edid_corrupt);
> >  extern bool drm_edid_is_valid(struct edid *edid);
> > +extern bool drm_edid_get_monitor_name(struct edid *edid, char *name);
> >  
> >  extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
> >  							 char topology[8]);
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/dp/mst: Enhance DP MST debugfs output

[Jim Bride]

Add some additional information (input vs. output port, sink associated
with VC, peer device type, max number of VCs supported) and ensure that
any embedded '\0' characters in a branch device's devid string are not
written to debugfs.

cc: dri-devel@lists.freedesktop.org
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 27fbd79..1afa36d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2729,7 +2729,7 @@ static void drm_dp_mst_dump_mstb(struct seq_file *m,
 
 	seq_printf(m, "%smst: %p, %d\n", prefix, mstb, mstb->num_ports);
 	list_for_each_entry(port, &mstb->ports, next) {
-		seq_printf(m, "%sport: %d: ddps: %d ldps: %d, sdp: %d/%d, %p, conn: %p\n", prefix, port->port_num, port->ddps, port->ldps, port->num_sdp_streams, port->num_sdp_stream_sinks, port, port->connector);
+		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);
 		if (port->mstb)
 			drm_dp_mst_dump_mstb(m, port->mstb);
 	}
@@ -2750,6 +2750,17 @@ static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr,
 	return false;
 }
 
+static bool fetch_monitor_name(struct drm_dp_mst_topology_mgr *mgr,
+			       struct drm_dp_mst_port *port, char *name)
+{
+	struct edid *mst_edid = NULL;
+
+	mst_edid = drm_dp_mst_get_edid(port->connector, mgr, port);
+	if (mst_edid == NULL)
+		return false;
+	return drm_edid_get_monitor_name(mst_edid, name);
+}
+
 /**
  * drm_dp_mst_dump_topology(): dump topology to seq file.
  * @m: seq_file to dump output to
@@ -2762,6 +2773,8 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
 {
 	int i;
 	struct drm_dp_mst_port *port;
+	bool mname_valid = false;
+
 	mutex_lock(&mgr->lock);
 	if (mgr->mst_primary)
 		drm_dp_mst_dump_mstb(m, mgr->mst_primary);
@@ -2770,14 +2783,22 @@ 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\n", mgr->payload_mask, mgr->vcpi_mask);
+	seq_printf(m, "vcpi: %lx %lx %d\n", mgr->payload_mask, mgr->vcpi_mask,
+		mgr->max_payloads);
 
 	for (i = 0; i < mgr->max_payloads; i++) {
 		if (mgr->proposed_vcpis[i]) {
+			char name[13];
+
+			memset(name, 0, 13 * sizeof(char));
 			port = container_of(mgr->proposed_vcpis[i], struct drm_dp_mst_port, vcpi);
-			seq_printf(m, "vcpi %d: %d %d %d\n", i, port->port_num, port->vcpi.vcpi, port->vcpi.num_slots);
+			mname_valid = fetch_monitor_name(mgr, port, name);
+			seq_printf(m, "vcpi %d: %d %d %d sink name: %s\n", i,
+				   port->port_num, port->vcpi.vcpi,
+				   port->vcpi.num_slots, mname_valid ? name :
+				   "Unknown");
 		} else
-			seq_printf(m, "vcpi %d:unsed\n", i);
+			seq_printf(m, "vcpi %d:unused\n", i);
 	}
 	for (i = 0; i < mgr->max_payloads; i++) {
 		seq_printf(m, "payload %d: %d, %d, %d\n",
@@ -2818,7 +2839,11 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
 			seq_printf(m, "%02x", buf[i]);
 		seq_printf(m, " devid: ");
 		for (i = 0x3; i < 0x8; i++)
-			seq_printf(m, "%c", buf[i]);
+			if (buf[i] != '\0')
+				seq_printf(m, "%c", buf[i]);
+			else
+				break;
+
 		seq_printf(m, " revision: hw: %x.%x sw: %x.%x", buf[0x9] >> 4, buf[0x9] & 0xf, buf[0xa], buf[0xb]);
 		seq_printf(m, "\n");
 		bret = dump_dp_payload_table(mgr, buf);
-- 
2.5.0

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

[PATCH v2 1/3] drm/edid: Add drm_edid_get_monitor_name()

[Jim Bride]

In order to include monitor name information in debugfs
output we needed to add a function that would extract the
monitor name from the EDID, and that function needed to
reside in the file  where the rest of the EDID helper
functions are implemented.

v2: Refactor to have drm_edid_get_monitor_name() and drm_edid_to_eld()
    use a common helper function to extract the monitor name from the
    edid. [Jani] + rebase.

cc: dri-devel@lists.freedesktop.org
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 50 +++++++++++++++++++++++++++++++++++++---------
 include/drm/drm_crtc.h     |  2 ++
 2 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 558ef9f..daf53df 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3293,6 +3293,45 @@ monitor_name(struct detailed_timing *t, void *data)
 		*(u8 **)data = t->data.other_data.data.str.str;
 }
 
+static int get_monitor_name(struct edid *edid, char name[13])
+{
+	char *edid_name = NULL;
+	int mnl;
+
+	if (!edid || !name)
+		return 0;
+
+	drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name);
+	for (mnl = 0; edid_name && mnl < 13; mnl++) {
+		if (edid_name[mnl] == 0x0a)
+			break;
+
+		name[mnl] = edid_name[mnl];
+	}
+
+	return mnl;
+}
+
+/**
+ * drm_edid_get_monitor_name - fetch the monitor name from the edid
+ * @edid: monitor EDID information
+ * @name: pointer to a character array to hold the name of the monitor
+ * @bufsize: The size of the name buffer (should be at least 13 chars.)
+ *
+ */
+void drm_edid_get_monitor_name(struct edid *edid, char *name, int bufsize)
+{
+	int name_length = -1;
+
+	if (bufsize < 13)
+		return;
+
+	name_length = get_monitor_name(edid, name);
+	if (name_length)
+		name[name_length] = '\0';
+}
+EXPORT_SYMBOL(drm_edid_get_monitor_name);
+
 /**
  * drm_edid_to_eld - build ELD from EDID
  * @connector: connector corresponding to the HDMI/DP sink
@@ -3306,7 +3345,6 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 {
 	uint8_t *eld = connector->eld;
 	u8 *cea;
-	u8 *name;
 	u8 *db;
 	int total_sad_count = 0;
 	int mnl;
@@ -3320,14 +3358,8 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 		return;
 	}
 
-	name = NULL;
-	drm_for_each_detailed_block((u8 *)edid, monitor_name, &name);
-	/* max: 13 bytes EDID, 16 bytes ELD */
-	for (mnl = 0; name && mnl < 13; mnl++) {
-		if (name[mnl] == 0x0a)
-			break;
-		eld[20 + mnl] = name[mnl];
-	}
+	mnl = get_monitor_name(edid, eld + 20);
+
 	eld[4] = (cea[1] << 5) | mnl;
 	DRM_DEBUG_KMS("ELD monitor %s\n", eld + 20);
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8cb377c..6d46842 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -2500,6 +2500,8 @@ extern int drm_edid_header_is_valid(const u8 *raw_edid);
 extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
 				 bool *edid_corrupt);
 extern bool drm_edid_is_valid(struct edid *edid);
+extern void drm_edid_get_monitor_name(struct edid *edid, char *name,
+				      int buflen);
 
 extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
 							 char topology[8]);
-- 
2.5.0

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

Re: [Intel-gfx] [PATCH v2 1/3] drm/edid: Add drm_edid_get_monitor_name()

[Jani Nikula]

On Thu, 07 Apr 2016, Jim Bride <jim.bride@linux.intel.com> wrote:
> In order to include monitor name information in debugfs
> output we needed to add a function that would extract the
> monitor name from the EDID, and that function needed to
> reside in the file  where the rest of the EDID helper
> functions are implemented.
>
> v2: Refactor to have drm_edid_get_monitor_name() and drm_edid_to_eld()
>     use a common helper function to extract the monitor name from the
>     edid. [Jani] + rebase.
>
> cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 50 +++++++++++++++++++++++++++++++++++++---------
>  include/drm/drm_crtc.h     |  2 ++
>  2 files changed, 43 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 558ef9f..daf53df 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3293,6 +3293,45 @@ monitor_name(struct detailed_timing *t, void *data)
>  		*(u8 **)data = t->data.other_data.data.str.str;
>  }
>  
> +static int get_monitor_name(struct edid *edid, char name[13])
> +{
> +	char *edid_name = NULL;
> +	int mnl;
> +
> +	if (!edid || !name)
> +		return 0;
> +
> +	drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name);
> +	for (mnl = 0; edid_name && mnl < 13; mnl++) {
> +		if (edid_name[mnl] == 0x0a)
> +			break;
> +
> +		name[mnl] = edid_name[mnl];
> +	}
> +
> +	return mnl;
> +}
> +
> +/**
> + * drm_edid_get_monitor_name - fetch the monitor name from the edid
> + * @edid: monitor EDID information
> + * @name: pointer to a character array to hold the name of the monitor
> + * @bufsize: The size of the name buffer (should be at least 13 chars.)
> + *
> + */
> +void drm_edid_get_monitor_name(struct edid *edid, char *name, int bufsize)
> +{
> +	int name_length = -1;
> +
> +	if (bufsize < 13)
> +		return;
> +
> +	name_length = get_monitor_name(edid, name);
> +	if (name_length)
> +		name[name_length] = '\0';

I was thinking something along the lines of:

	char buf[13];
	int len;

	if (bufsize <= 0)
		return;

	len = min(get_monitor_name(edid, buf), bufsize - 1);
	memcpy(name, buf, len);
	name[len] = '\0';

BR,
Jani.

> +}
> +EXPORT_SYMBOL(drm_edid_get_monitor_name);
> +
>  /**
>   * drm_edid_to_eld - build ELD from EDID
>   * @connector: connector corresponding to the HDMI/DP sink
> @@ -3306,7 +3345,6 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>  {
>  	uint8_t *eld = connector->eld;
>  	u8 *cea;
> -	u8 *name;
>  	u8 *db;
>  	int total_sad_count = 0;
>  	int mnl;
> @@ -3320,14 +3358,8 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>  		return;
>  	}
>  
> -	name = NULL;
> -	drm_for_each_detailed_block((u8 *)edid, monitor_name, &name);
> -	/* max: 13 bytes EDID, 16 bytes ELD */
> -	for (mnl = 0; name && mnl < 13; mnl++) {
> -		if (name[mnl] == 0x0a)
> -			break;
> -		eld[20 + mnl] = name[mnl];
> -	}
> +	mnl = get_monitor_name(edid, eld + 20);
> +
>  	eld[4] = (cea[1] << 5) | mnl;
>  	DRM_DEBUG_KMS("ELD monitor %s\n", eld + 20);
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 8cb377c..6d46842 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -2500,6 +2500,8 @@ extern int drm_edid_header_is_valid(const u8 *raw_edid);
>  extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>  				 bool *edid_corrupt);
>  extern bool drm_edid_is_valid(struct edid *edid);
> +extern void drm_edid_get_monitor_name(struct edid *edid, char *name,
> +				      int buflen);
>  
>  extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
>  							 char topology[8]);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/3] drm/dp/mst: Enhance DP MST debugfs output

[Jim Bride]

Add some additional information (input vs. output port, sink associated
with VC, peer device type, max number of VCs supported) and ensure that
any embedded '\0' characters in a branch device's devid string are not
written to debugfs.

v2: Rebase + change drm_edid_get_monitor_name() call to reflect new
    signature.

cc: dri-devel@lists.freedesktop.org
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 27fbd79..0d3873e 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2729,7 +2729,7 @@ static void drm_dp_mst_dump_mstb(struct seq_file *m,
 
 	seq_printf(m, "%smst: %p, %d\n", prefix, mstb, mstb->num_ports);
 	list_for_each_entry(port, &mstb->ports, next) {
-		seq_printf(m, "%sport: %d: ddps: %d ldps: %d, sdp: %d/%d, %p, conn: %p\n", prefix, port->port_num, port->ddps, port->ldps, port->num_sdp_streams, port->num_sdp_stream_sinks, port, port->connector);
+		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);
 		if (port->mstb)
 			drm_dp_mst_dump_mstb(m, port->mstb);
 	}
@@ -2750,6 +2750,18 @@ static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr,
 	return false;
 }
 
+static void fetch_monitor_name(struct drm_dp_mst_topology_mgr *mgr,
+			       struct drm_dp_mst_port *port, char *name,
+			       int namelen)
+{
+	struct edid *mst_edid = NULL;
+
+	mst_edid = drm_dp_mst_get_edid(port->connector, mgr, port);
+	if (mst_edid == NULL)
+		return;
+	drm_edid_get_monitor_name(mst_edid, name, namelen);
+}
+
 /**
  * drm_dp_mst_dump_topology(): dump topology to seq file.
  * @m: seq_file to dump output to
@@ -2762,6 +2774,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
 {
 	int i;
 	struct drm_dp_mst_port *port;
+
 	mutex_lock(&mgr->lock);
 	if (mgr->mst_primary)
 		drm_dp_mst_dump_mstb(m, mgr->mst_primary);
@@ -2770,14 +2783,22 @@ 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\n", mgr->payload_mask, mgr->vcpi_mask);
+	seq_printf(m, "vcpi: %lx %lx %d\n", mgr->payload_mask, mgr->vcpi_mask,
+		mgr->max_payloads);
 
 	for (i = 0; i < mgr->max_payloads; i++) {
 		if (mgr->proposed_vcpis[i]) {
+			char name[13];
+
+			memset(name, 0, 13 * sizeof(char));
 			port = container_of(mgr->proposed_vcpis[i], struct drm_dp_mst_port, vcpi);
-			seq_printf(m, "vcpi %d: %d %d %d\n", i, port->port_num, port->vcpi.vcpi, port->vcpi.num_slots);
+			fetch_monitor_name(mgr, port, name, 13);
+			seq_printf(m, "vcpi %d: %d %d %d sink name: %s\n", i,
+				   port->port_num, port->vcpi.vcpi,
+				   port->vcpi.num_slots,
+				   (name[0] != 0) ? name :  "Unknown");
 		} else
-			seq_printf(m, "vcpi %d:unsed\n", i);
+			seq_printf(m, "vcpi %d:unused\n", i);
 	}
 	for (i = 0; i < mgr->max_payloads; i++) {
 		seq_printf(m, "payload %d: %d, %d, %d\n",
@@ -2818,7 +2839,11 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
 			seq_printf(m, "%02x", buf[i]);
 		seq_printf(m, " devid: ");
 		for (i = 0x3; i < 0x8; i++)
-			seq_printf(m, "%c", buf[i]);
+			if (buf[i] != '\0')
+				seq_printf(m, "%c", buf[i]);
+			else
+				break;
+
 		seq_printf(m, " revision: hw: %x.%x sw: %x.%x", buf[0x9] >> 4, buf[0x9] & 0xf, buf[0xa], buf[0xb]);
 		seq_printf(m, "\n");
 		bret = dump_dp_payload_table(mgr, buf);
-- 
2.5.0

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

Re: [PATCH v2 2/3] drm/dp/mst: Enhance DP MST debugfs output

[Jani Nikula]

On Thu, 07 Apr 2016, Jim Bride <jim.bride@linux.intel.com> wrote:
> Add some additional information (input vs. output port, sink associated
> with VC, peer device type, max number of VCs supported) and ensure that
> any embedded '\0' characters in a branch device's devid string are not
> written to debugfs.
>
> v2: Rebase + change drm_edid_get_monitor_name() call to reflect new
>     signature.
>
> cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 35 ++++++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 27fbd79..0d3873e 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2729,7 +2729,7 @@ static void drm_dp_mst_dump_mstb(struct seq_file *m,
>  
>  	seq_printf(m, "%smst: %p, %d\n", prefix, mstb, mstb->num_ports);
>  	list_for_each_entry(port, &mstb->ports, next) {
> -		seq_printf(m, "%sport: %d: ddps: %d ldps: %d, sdp: %d/%d, %p, conn: %p\n", prefix, port->port_num, port->ddps, port->ldps, port->num_sdp_streams, port->num_sdp_stream_sinks, port, port->connector);
> +		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);
>  		if (port->mstb)
>  			drm_dp_mst_dump_mstb(m, port->mstb);
>  	}
> @@ -2750,6 +2750,18 @@ static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr,
>  	return false;
>  }
>  
> +static void fetch_monitor_name(struct drm_dp_mst_topology_mgr *mgr,
> +			       struct drm_dp_mst_port *port, char *name,
> +			       int namelen)
> +{
> +	struct edid *mst_edid = NULL;

No need to initialize.

> +
> +	mst_edid = drm_dp_mst_get_edid(port->connector, mgr, port);
> +	if (mst_edid == NULL)
> +		return;

Leave this check out to ensure a terminated name for mst_edid == NULL
from the call below.

> +	drm_edid_get_monitor_name(mst_edid, name, namelen);
> +}
> +
>  /**
>   * drm_dp_mst_dump_topology(): dump topology to seq file.
>   * @m: seq_file to dump output to
> @@ -2762,6 +2774,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>  {
>  	int i;
>  	struct drm_dp_mst_port *port;
> +
>  	mutex_lock(&mgr->lock);
>  	if (mgr->mst_primary)
>  		drm_dp_mst_dump_mstb(m, mgr->mst_primary);
> @@ -2770,14 +2783,22 @@ 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\n", mgr->payload_mask, mgr->vcpi_mask);
> +	seq_printf(m, "vcpi: %lx %lx %d\n", mgr->payload_mask, mgr->vcpi_mask,
> +		mgr->max_payloads);
>  
>  	for (i = 0; i < mgr->max_payloads; i++) {
>  		if (mgr->proposed_vcpis[i]) {
> +			char name[13];

You'll need 14 chars (i.e. +1 for termination).

> +
> +			memset(name, 0, 13 * sizeof(char));

This is no longer needed.

>  			port = container_of(mgr->proposed_vcpis[i], struct drm_dp_mst_port, vcpi);
> -			seq_printf(m, "vcpi %d: %d %d %d\n", i, port->port_num, port->vcpi.vcpi, port->vcpi.num_slots);
> +			fetch_monitor_name(mgr, port, name, 13);

Replace 13 with sizeof(name).

> +			seq_printf(m, "vcpi %d: %d %d %d sink name: %s\n", i,
> +				   port->port_num, port->vcpi.vcpi,
> +				   port->vcpi.num_slots,
> +				   (name[0] != 0) ? name :  "Unknown");

Perhaps *name ? name : "Unknown"?

>  		} else
> -			seq_printf(m, "vcpi %d:unsed\n", i);
> +			seq_printf(m, "vcpi %d:unused\n", i);
>  	}
>  	for (i = 0; i < mgr->max_payloads; i++) {
>  		seq_printf(m, "payload %d: %d, %d, %d\n",
> @@ -2818,7 +2839,11 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>  			seq_printf(m, "%02x", buf[i]);
>  		seq_printf(m, " devid: ");
>  		for (i = 0x3; i < 0x8; i++)

You could just make the loop condition i < 0x8 && buf[i].

> -			seq_printf(m, "%c", buf[i]);
> +			if (buf[i] != '\0')
> +				seq_printf(m, "%c", buf[i]);
> +			else
> +				break;
> +
>  		seq_printf(m, " revision: hw: %x.%x sw: %x.%x", buf[0x9] >> 4, buf[0x9] & 0xf, buf[0xa], buf[0xb]);
>  		seq_printf(m, "\n");
>  		bret = dump_dp_payload_table(mgr, buf);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 1/3] drm/edid: Add drm_edid_get_monitor_name()

[Jim Bride]

In order to include monitor name information in debugfs
output we needed to add a function that would extract the
monitor name from the EDID, and that function needed to
reside in the file  where the rest of the EDID helper
functions are implemented.

v2: Refactor to have drm_edid_get_monitor_name() and drm_edid_to_eld()
    use a common helper function to extract the monitor name from the
    edid. [Jani] + rebase.

v3: Minor changes suggested by Jani + rebase.

cc: dri-devel@lists.freedesktop.org
cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 51 ++++++++++++++++++++++++++++++++++++++--------
 include/drm/drm_crtc.h     |  2 ++
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 558ef9f..da30ce3 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3293,6 +3293,46 @@ monitor_name(struct detailed_timing *t, void *data)
 		*(u8 **)data = t->data.other_data.data.str.str;
 }
 
+static int get_monitor_name(struct edid *edid, char name[13])
+{
+	char *edid_name = NULL;
+	int mnl;
+
+	if (!edid || !name)
+		return 0;
+
+	drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name);
+	for (mnl = 0; edid_name && mnl < 13; mnl++) {
+		if (edid_name[mnl] == 0x0a)
+			break;
+
+		name[mnl] = edid_name[mnl];
+	}
+
+	return mnl;
+}
+
+/**
+ * drm_edid_get_monitor_name - fetch the monitor name from the edid
+ * @edid: monitor EDID information
+ * @name: pointer to a character array to hold the name of the monitor
+ * @bufsize: The size of the name buffer (should be at least 13 chars.)
+ *
+ */
+void drm_edid_get_monitor_name(struct edid *edid, char *name, int bufsize)
+{
+	int name_length = -1;
+	char buf[13];
+	
+	if (bufsize <= 0)
+		return;
+
+	name_length = min(get_monitor_name(edid, buf), bufsize - 1);
+	memcpy(name, buf, name_length);
+	name[name_length] = '\0';
+}
+EXPORT_SYMBOL(drm_edid_get_monitor_name);
+
 /**
  * drm_edid_to_eld - build ELD from EDID
  * @connector: connector corresponding to the HDMI/DP sink
@@ -3306,7 +3346,6 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 {
 	uint8_t *eld = connector->eld;
 	u8 *cea;
-	u8 *name;
 	u8 *db;
 	int total_sad_count = 0;
 	int mnl;
@@ -3320,14 +3359,8 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 		return;
 	}
 
-	name = NULL;
-	drm_for_each_detailed_block((u8 *)edid, monitor_name, &name);
-	/* max: 13 bytes EDID, 16 bytes ELD */
-	for (mnl = 0; name && mnl < 13; mnl++) {
-		if (name[mnl] == 0x0a)
-			break;
-		eld[20 + mnl] = name[mnl];
-	}
+	mnl = get_monitor_name(edid, eld + 20);
+
 	eld[4] = (cea[1] << 5) | mnl;
 	DRM_DEBUG_KMS("ELD monitor %s\n", eld + 20);
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8cb377c..6d46842 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -2500,6 +2500,8 @@ extern int drm_edid_header_is_valid(const u8 *raw_edid);
 extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
 				 bool *edid_corrupt);
 extern bool drm_edid_is_valid(struct edid *edid);
+extern void drm_edid_get_monitor_name(struct edid *edid, char *name,
+				      int buflen);
 
 extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
 							 char topology[8]);
-- 
2.5.0

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

Re: [PATCH v3 1/3] drm/edid: Add drm_edid_get_monitor_name()

[Jani Nikula]

On Mon, 11 Apr 2016, Jim Bride <jim.bride@linux.intel.com> wrote:
> In order to include monitor name information in debugfs
> output we needed to add a function that would extract the
> monitor name from the EDID, and that function needed to
> reside in the file  where the rest of the EDID helper
> functions are implemented.
>
> v2: Refactor to have drm_edid_get_monitor_name() and drm_edid_to_eld()
>     use a common helper function to extract the monitor name from the
>     edid. [Jani] + rebase.
>
> v3: Minor changes suggested by Jani + rebase.
>
> cc: dri-devel@lists.freedesktop.org
> cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 51 ++++++++++++++++++++++++++++++++++++++--------
>  include/drm/drm_crtc.h     |  2 ++
>  2 files changed, 44 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 558ef9f..da30ce3 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3293,6 +3293,46 @@ monitor_name(struct detailed_timing *t, void *data)
>  		*(u8 **)data = t->data.other_data.data.str.str;
>  }
>  
> +static int get_monitor_name(struct edid *edid, char name[13])
> +{
> +	char *edid_name = NULL;
> +	int mnl;
> +
> +	if (!edid || !name)
> +		return 0;
> +
> +	drm_for_each_detailed_block((u8 *)edid, monitor_name, &edid_name);
> +	for (mnl = 0; edid_name && mnl < 13; mnl++) {
> +		if (edid_name[mnl] == 0x0a)
> +			break;
> +
> +		name[mnl] = edid_name[mnl];
> +	}
> +
> +	return mnl;
> +}
> +
> +/**
> + * drm_edid_get_monitor_name - fetch the monitor name from the edid
> + * @edid: monitor EDID information
> + * @name: pointer to a character array to hold the name of the monitor
> + * @bufsize: The size of the name buffer (should be at least 13 chars.)

Nitpick, should be 13 + termination = 14 bytes.

> + *
> + */
> +void drm_edid_get_monitor_name(struct edid *edid, char *name, int bufsize)
> +{
> +	int name_length = -1;

Nitpick, no need to initialize.

Other than those,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +	char buf[13];
> +	
> +	if (bufsize <= 0)
> +		return;
> +
> +	name_length = min(get_monitor_name(edid, buf), bufsize - 1);
> +	memcpy(name, buf, name_length);
> +	name[name_length] = '\0';
> +}
> +EXPORT_SYMBOL(drm_edid_get_monitor_name);
> +
>  /**
>   * drm_edid_to_eld - build ELD from EDID
>   * @connector: connector corresponding to the HDMI/DP sink
> @@ -3306,7 +3346,6 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>  {
>  	uint8_t *eld = connector->eld;
>  	u8 *cea;
> -	u8 *name;
>  	u8 *db;
>  	int total_sad_count = 0;
>  	int mnl;
> @@ -3320,14 +3359,8 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>  		return;
>  	}
>  
> -	name = NULL;
> -	drm_for_each_detailed_block((u8 *)edid, monitor_name, &name);
> -	/* max: 13 bytes EDID, 16 bytes ELD */
> -	for (mnl = 0; name && mnl < 13; mnl++) {
> -		if (name[mnl] == 0x0a)
> -			break;
> -		eld[20 + mnl] = name[mnl];
> -	}
> +	mnl = get_monitor_name(edid, eld + 20);
> +
>  	eld[4] = (cea[1] << 5) | mnl;
>  	DRM_DEBUG_KMS("ELD monitor %s\n", eld + 20);
>  
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 8cb377c..6d46842 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -2500,6 +2500,8 @@ extern int drm_edid_header_is_valid(const u8 *raw_edid);
>  extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
>  				 bool *edid_corrupt);
>  extern bool drm_edid_is_valid(struct edid *edid);
> +extern void drm_edid_get_monitor_name(struct edid *edid, char *name,
> +				      int buflen);
>  
>  extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
>  							 char topology[8]);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 2/3] drm/dp/mst: Enhance DP MST debugfs output

[Jim Bride]

Add some additional information (input vs. output port, sink associated
with VC, peer device type, max number of VCs supported) and ensure that
any embedded '\0' characters in a branch device's devid string are not
written to debugfs.

v2: Rebase + change drm_edid_get_monitor_name() call to reflect new
    signature.

v3: Minor changes suggested by Jani + rebase.

cc: dri-devel@lists.freedesktop.org
cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 27fbd79..06d8b0a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2729,7 +2729,7 @@ static void drm_dp_mst_dump_mstb(struct seq_file *m,
 
 	seq_printf(m, "%smst: %p, %d\n", prefix, mstb, mstb->num_ports);
 	list_for_each_entry(port, &mstb->ports, next) {
-		seq_printf(m, "%sport: %d: ddps: %d ldps: %d, sdp: %d/%d, %p, conn: %p\n", prefix, port->port_num, port->ddps, port->ldps, port->num_sdp_streams, port->num_sdp_stream_sinks, port, port->connector);
+		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);
 		if (port->mstb)
 			drm_dp_mst_dump_mstb(m, port->mstb);
 	}
@@ -2750,6 +2750,16 @@ static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr,
 	return false;
 }
 
+static void fetch_monitor_name(struct drm_dp_mst_topology_mgr *mgr,
+			       struct drm_dp_mst_port *port, char *name,
+			       int namelen)
+{
+	struct edid *mst_edid;
+
+	mst_edid = drm_dp_mst_get_edid(port->connector, mgr, port);
+	drm_edid_get_monitor_name(mst_edid, name, namelen);
+}
+
 /**
  * drm_dp_mst_dump_topology(): dump topology to seq file.
  * @m: seq_file to dump output to
@@ -2762,6 +2772,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
 {
 	int i;
 	struct drm_dp_mst_port *port;
+
 	mutex_lock(&mgr->lock);
 	if (mgr->mst_primary)
 		drm_dp_mst_dump_mstb(m, mgr->mst_primary);
@@ -2770,14 +2781,21 @@ 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\n", mgr->payload_mask, mgr->vcpi_mask);
+	seq_printf(m, "vcpi: %lx %lx %d\n", mgr->payload_mask, mgr->vcpi_mask,
+		mgr->max_payloads);
 
 	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);
-			seq_printf(m, "vcpi %d: %d %d %d\n", i, port->port_num, port->vcpi.vcpi, port->vcpi.num_slots);
+			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,
+				   port->vcpi.num_slots,
+				   (*name != 0) ? name :  "Unknown");
 		} else
-			seq_printf(m, "vcpi %d:unsed\n", i);
+			seq_printf(m, "vcpi %d:unused\n", i);
 	}
 	for (i = 0; i < mgr->max_payloads; i++) {
 		seq_printf(m, "payload %d: %d, %d, %d\n",
@@ -2817,8 +2835,9 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
 		for (i = 0; i < 0x3; i++)
 			seq_printf(m, "%02x", buf[i]);
 		seq_printf(m, " devid: ");
-		for (i = 0x3; i < 0x8; i++)
+		for (i = 0x3; i < 0x8 && buf[i]; i++)
 			seq_printf(m, "%c", buf[i]);
+
 		seq_printf(m, " revision: hw: %x.%x sw: %x.%x", buf[0x9] >> 4, buf[0x9] & 0xf, buf[0xa], buf[0xb]);
 		seq_printf(m, "\n");
 		bret = dump_dp_payload_table(mgr, buf);
-- 
2.5.0

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

Re: [PATCH v3 2/3] drm/dp/mst: Enhance DP MST debugfs output

[Jani Nikula]

On Mon, 11 Apr 2016, Jim Bride <jim.bride@linux.intel.com> wrote:
> Add some additional information (input vs. output port, sink associated
> with VC, peer device type, max number of VCs supported) and ensure that
> any embedded '\0' characters in a branch device's devid string are not
> written to debugfs.
>
> v2: Rebase + change drm_edid_get_monitor_name() call to reflect new
>     signature.
>
> v3: Minor changes suggested by Jani + rebase.
>
> cc: dri-devel@lists.freedesktop.org
> cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 27fbd79..06d8b0a 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2729,7 +2729,7 @@ static void drm_dp_mst_dump_mstb(struct seq_file *m,
>  
>  	seq_printf(m, "%smst: %p, %d\n", prefix, mstb, mstb->num_ports);
>  	list_for_each_entry(port, &mstb->ports, next) {
> -		seq_printf(m, "%sport: %d: ddps: %d ldps: %d, sdp: %d/%d, %p, conn: %p\n", prefix, port->port_num, port->ddps, port->ldps, port->num_sdp_streams, port->num_sdp_stream_sinks, port, port->connector);
> +		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);
>  		if (port->mstb)
>  			drm_dp_mst_dump_mstb(m, port->mstb);
>  	}
> @@ -2750,6 +2750,16 @@ static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr,
>  	return false;
>  }
>  
> +static void fetch_monitor_name(struct drm_dp_mst_topology_mgr *mgr,
> +			       struct drm_dp_mst_port *port, char *name,
> +			       int namelen)
> +{
> +	struct edid *mst_edid;
> +
> +	mst_edid = drm_dp_mst_get_edid(port->connector, mgr, port);
> +	drm_edid_get_monitor_name(mst_edid, name, namelen);
> +}
> +
>  /**
>   * drm_dp_mst_dump_topology(): dump topology to seq file.
>   * @m: seq_file to dump output to
> @@ -2762,6 +2772,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>  {
>  	int i;
>  	struct drm_dp_mst_port *port;
> +
>  	mutex_lock(&mgr->lock);
>  	if (mgr->mst_primary)
>  		drm_dp_mst_dump_mstb(m, mgr->mst_primary);
> @@ -2770,14 +2781,21 @@ 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\n", mgr->payload_mask, mgr->vcpi_mask);
> +	seq_printf(m, "vcpi: %lx %lx %d\n", mgr->payload_mask, mgr->vcpi_mask,
> +		mgr->max_payloads);
>  
>  	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);
> -			seq_printf(m, "vcpi %d: %d %d %d\n", i, port->port_num, port->vcpi.vcpi, port->vcpi.num_slots);
> +			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,
> +				   port->vcpi.num_slots,
> +				   (*name != 0) ? name :  "Unknown");
>  		} else
> -			seq_printf(m, "vcpi %d:unsed\n", i);
> +			seq_printf(m, "vcpi %d:unused\n", i);
>  	}
>  	for (i = 0; i < mgr->max_payloads; i++) {
>  		seq_printf(m, "payload %d: %d, %d, %d\n",
> @@ -2817,8 +2835,9 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>  		for (i = 0; i < 0x3; i++)
>  			seq_printf(m, "%02x", buf[i]);
>  		seq_printf(m, " devid: ");
> -		for (i = 0x3; i < 0x8; i++)
> +		for (i = 0x3; i < 0x8 && buf[i]; i++)
>  			seq_printf(m, "%c", buf[i]);
> +
>  		seq_printf(m, " revision: hw: %x.%x sw: %x.%x", buf[0x9] >> 4, buf[0x9] & 0xf, buf[0xa], buf[0xb]);
>  		seq_printf(m, "\n");
>  		bret = dump_dp_payload_table(mgr, buf);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 2/3] drm/dp/mst: Enhance DP MST debugfs output

[Daniel Vetter]

On Thu, Apr 14, 2016 at 09:47:22AM +0300, Jani Nikula wrote:
> On Mon, 11 Apr 2016, Jim Bride <jim.bride@linux.intel.com> wrote:
> > Add some additional information (input vs. output port, sink associated
> > with VC, peer device type, max number of VCs supported) and ensure that
> > any embedded '\0' characters in a branch device's devid string are not
> > written to debugfs.
> >
> > v2: Rebase + change drm_edid_get_monitor_name() call to reflect new
> >     signature.
> >
> > v3: Minor changes suggested by Jani + rebase.
> >
> > cc: dri-devel@lists.freedesktop.org
> > cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Patch series seems a bit messed up here (I don't have a 3/3 here). Can you
pls resend the entire pile, with nitpicks+reviewed-by and everything
included, in a new thread?

Thanks, Daniel

> 
> 
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 29 ++++++++++++++++++++++++-----
> >  1 file changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 27fbd79..06d8b0a 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -2729,7 +2729,7 @@ static void drm_dp_mst_dump_mstb(struct seq_file *m,
> >  
> >  	seq_printf(m, "%smst: %p, %d\n", prefix, mstb, mstb->num_ports);
> >  	list_for_each_entry(port, &mstb->ports, next) {
> > -		seq_printf(m, "%sport: %d: ddps: %d ldps: %d, sdp: %d/%d, %p, conn: %p\n", prefix, port->port_num, port->ddps, port->ldps, port->num_sdp_streams, port->num_sdp_stream_sinks, port, port->connector);
> > +		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);
> >  		if (port->mstb)
> >  			drm_dp_mst_dump_mstb(m, port->mstb);
> >  	}
> > @@ -2750,6 +2750,16 @@ static bool dump_dp_payload_table(struct drm_dp_mst_topology_mgr *mgr,
> >  	return false;
> >  }
> >  
> > +static void fetch_monitor_name(struct drm_dp_mst_topology_mgr *mgr,
> > +			       struct drm_dp_mst_port *port, char *name,
> > +			       int namelen)
> > +{
> > +	struct edid *mst_edid;
> > +
> > +	mst_edid = drm_dp_mst_get_edid(port->connector, mgr, port);
> > +	drm_edid_get_monitor_name(mst_edid, name, namelen);
> > +}
> > +
> >  /**
> >   * drm_dp_mst_dump_topology(): dump topology to seq file.
> >   * @m: seq_file to dump output to
> > @@ -2762,6 +2772,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
> >  {
> >  	int i;
> >  	struct drm_dp_mst_port *port;
> > +
> >  	mutex_lock(&mgr->lock);
> >  	if (mgr->mst_primary)
> >  		drm_dp_mst_dump_mstb(m, mgr->mst_primary);
> > @@ -2770,14 +2781,21 @@ 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\n", mgr->payload_mask, mgr->vcpi_mask);
> > +	seq_printf(m, "vcpi: %lx %lx %d\n", mgr->payload_mask, mgr->vcpi_mask,
> > +		mgr->max_payloads);
> >  
> >  	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);
> > -			seq_printf(m, "vcpi %d: %d %d %d\n", i, port->port_num, port->vcpi.vcpi, port->vcpi.num_slots);
> > +			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,
> > +				   port->vcpi.num_slots,
> > +				   (*name != 0) ? name :  "Unknown");
> >  		} else
> > -			seq_printf(m, "vcpi %d:unsed\n", i);
> > +			seq_printf(m, "vcpi %d:unused\n", i);
> >  	}
> >  	for (i = 0; i < mgr->max_payloads; i++) {
> >  		seq_printf(m, "payload %d: %d, %d, %d\n",
> > @@ -2817,8 +2835,9 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
> >  		for (i = 0; i < 0x3; i++)
> >  			seq_printf(m, "%02x", buf[i]);
> >  		seq_printf(m, " devid: ");
> > -		for (i = 0x3; i < 0x8; i++)
> > +		for (i = 0x3; i < 0x8 && buf[i]; i++)
> >  			seq_printf(m, "%c", buf[i]);
> > +
> >  		seq_printf(m, " revision: hw: %x.%x sw: %x.%x", buf[0x9] >> 4, buf[0x9] & 0xf, buf[0xa], buf[0xb]);
> >  		seq_printf(m, "\n");
> >  		bret = dump_dp_payload_table(mgr, buf);
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx