[Intel-gfx] [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD

Wed Apr 1 05:35:18 PDT 2015

On Wed, 01 Apr 2015, "R, Durgadoss" <durgadoss.r at intel.com> wrote:
> Hi Jani,
>
>>-----Original Message-----
>>From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf Of Jani Nikula
>>Sent: Wednesday, April 1, 2015 1:45 PM
>>To: intel-gfx at lists.freedesktop.org
>>Cc: Nikula, Jani
>>Subject: [Intel-gfx] [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD
>>
>>Occasionally it would be interesting to read some of the DPCD registers
>>for debug purposes, without having to resort to logging. Add an i915
>>specific i915_dpcd debugfs file for DP and eDP connectors to dump parts
>>of the DPCD. Currently the DPCD addresses to be dumped are statically
>
> This is very much needed. I personally use a sysfs variation of something like this.
> But I think we will never have a list that is sufficient for all cases of debug.
>
> So, why not have one interface which takes the 'addr' and another
> one which returns the 'value' of the DPCD register 'addr'.

My main use case is for gathering debugging information from
end-users. I would like to have a simple and fairly fool proof way of
gathering the most relevant information in a dump like this. And we can
extend on it as needed.

I don't think what you propose is user friendly (although it is powerful
for developers), and pretty much requires a script to ensure we do get
the data we want.

However these two are not mutually exclusive... this is the patch to do
what I want, please send us the patch doing what you want! :)

BR,
Jani.

PS. Would we need to whitelist/blacklist the registers for your
approach?

> Something like below:
>
> i915_dpcd_addr
> i915_dpcd_val
>
> And we do:
> echo 0x100 > i915_dpcd_addr
>
> and then:
> cat i915_dpcd_val
> [This fetches and prints the value of DPCD 0x100]
>
> Thanks,
> Durga
>
> ..
>>configured, and more can be added trivially.
>>
>>The implementation also makes it relatively easy to add other i915 and
>>connector specific debugfs files in the future, as necessary.
>>
>>This is currently i915 specific just because there's no generic way to
>>do AUX transactions given just a drm_connector. However it's all pretty
>>straightforward to port to other drivers.
>>
>>v2: Add more DPCD registers to dump.
>>
>>Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>>---
>> drivers/gpu/drm/i915/i915_debugfs.c | 96 +++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_drv.h     |  1 +
>> drivers/gpu/drm/i915/intel_dp.c     |  2 +
>> 3 files changed, 99 insertions(+)
>>
>>diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>index 007c7d7d8295..7164aa7d5fd3 100644
>>--- a/drivers/gpu/drm/i915/i915_debugfs.c
>>+++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>@@ -4780,3 +4780,99 @@ void i915_debugfs_cleanup(struct drm_minor *minor)
>> 		drm_debugfs_remove_files(info_list, 1, minor);
>> 	}
>> }
>>+
>>+struct dpcd_block {
>>+	/* DPCD dump start address. */
>>+	unsigned int offset;
>>+	/* DPCD dump end address, inclusive. If unset, .size will be used. */
>>+	unsigned int end;
>>+	/* DPCD dump size. Used if .end is unset. If unset, defaults to 1. */
>>+	size_t size;
>>+	/* Only valid for eDP. */
>>+	bool edp;
>>+};
>>+
>>+static const struct dpcd_block i915_dpcd_debug[] = {
>>+	{ .offset = DP_DPCD_REV, .size = DP_RECEIVER_CAP_SIZE },
>>+	{ .offset = DP_PSR_SUPPORT, .end = DP_PSR_CAPS },
>>+	{ .offset = DP_DOWNSTREAM_PORT_0, .size = 16 },
>>+	{ .offset = DP_LINK_BW_SET, .end = DP_EDP_CONFIGURATION_SET },
>>+	{ .offset = DP_SINK_COUNT, .end = DP_ADJUST_REQUEST_LANE2_3 },
>>+	{ .offset = DP_SET_POWER },
>>+	{ .offset = DP_EDP_DPCD_REV },
>>+	{ .offset = DP_EDP_GENERAL_CAP_1, .end = DP_EDP_GENERAL_CAP_3 },
>>+	{ .offset = DP_EDP_DISPLAY_CONTROL_REGISTER, .end =
>>DP_EDP_BACKLIGHT_FREQ_CAP_MAX_LSB },
>>+	{ .offset = DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET, .end =
>>DP_EDP_DBC_MAXIMUM_BRIGHTNESS_SET },
>>+};
>>+
>>+static int i915_dpcd_show(struct seq_file *m, void *data)
>>+{
>>+	struct drm_connector *connector = m->private;
>>+	struct intel_dp *intel_dp =
>>+		enc_to_intel_dp(&intel_attached_encoder(connector)->base);
>>+	uint8_t buf[16];
>>+	ssize_t err;
>>+	int i;
>>+
>>+	for (i = 0; i < ARRAY_SIZE(i915_dpcd_debug); i++) {
>>+		const struct dpcd_block *b = &i915_dpcd_debug[i];
>>+		size_t size = b->end ? b->end - b->offset + 1 : (b->size ?: 1);
>>+
>>+		if (b->edp &&
>>+		    connector->connector_type != DRM_MODE_CONNECTOR_eDP)
>>+			continue;
>>+
>>+		/* low tech for now */
>>+		if (WARN_ON(size > sizeof(buf)))
>>+			continue;
>>+
>>+		err = drm_dp_dpcd_read(&intel_dp->aux, b->offset, buf, size);
>>+		if (err <= 0) {
>>+			DRM_ERROR("dpcd read (%zu bytes at %u) failed (%zd)\n",
>>+				  size, b->offset, err);
>>+			continue;
>>+		}
>>+
>>+		seq_printf(m, "%04x: %*ph\n", b->offset, (int) size, buf);
>>+	};
>>+
>>+	return 0;
>>+}
>>+
>>+static int i915_dpcd_open(struct inode *inode, struct file *file)
>>+{
>>+	return single_open(file, i915_dpcd_show, inode->i_private);
>>+}
>>+
>>+static const struct file_operations i915_dpcd_fops = {
>>+	.owner = THIS_MODULE,
>>+	.open = i915_dpcd_open,
>>+	.read = seq_read,
>>+	.llseek = seq_lseek,
>>+	.release = single_release,
>>+};
>>+
>>+/**
>>+ * i915_debugfs_connector_add - add i915 specific connector debugfs files
>>+ * @connector: pointer to a registered drm_connector
>>+ *
>>+ * Cleanup will be done by drm_connector_unregister() through a call to
>>+ * drm_debugfs_connector_remove().
>>+ *
>>+ * Returns 0 on success, negative error codes on error.
>>+ */
>>+int i915_debugfs_connector_add(struct drm_connector *connector)
>>+{
>>+	struct dentry *root = connector->debugfs_entry;
>>+
>>+	/* The connector must have been registered beforehands. */
>>+	if (!root)
>>+		return -ENODEV;
>>+
>>+	if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
>>+	    connector->connector_type == DRM_MODE_CONNECTOR_eDP)
>>+		debugfs_create_file("i915_dpcd", S_IRUGO, root, connector,
>>+				    &i915_dpcd_fops);
>>+
>>+	return 0;
>>+}
>>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>index aadada5a3fe9..868f26780115 100644
>>--- a/drivers/gpu/drm/i915/i915_drv.h
>>+++ b/drivers/gpu/drm/i915/i915_drv.h
>>@@ -3011,6 +3011,7 @@ int i915_verify_lists(struct drm_device *dev);
>> /* i915_debugfs.c */
>> int i915_debugfs_init(struct drm_minor *minor);
>> void i915_debugfs_cleanup(struct drm_minor *minor);
>>+int i915_debugfs_connector_add(struct drm_connector *connector);
>> #ifdef CONFIG_DEBUG_FS
>> void intel_display_crc_init(struct drm_device *dev);
>> #else
>>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>index 7936155acbe8..f91ce9e94aa6 100644
>>--- a/drivers/gpu/drm/i915/intel_dp.c
>>+++ b/drivers/gpu/drm/i915/intel_dp.c
>>@@ -5569,6 +5569,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>> 		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
>> 	}
>>
>>+	i915_debugfs_connector_add(connector);
>>+
>> 	return true;
>> }
>>
>>--
>>2.1.4
>>
>>_______________________________________________
>>Intel-gfx mailing list
>>Intel-gfx at lists.freedesktop.org
>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center