>-----Original Message----- >From: Nikula, Jani >Sent: Wednesday, April 1, 2015 6:05 PM >To: R, Durgadoss; [email protected] >Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: add i915 specific connector >debugfs file for DPCD > >On Wed, 01 Apr 2015, "R, Durgadoss" <[email protected]> wrote: >> Hi Jani, >> >>>-----Original Message----- >>>From: Intel-gfx [mailto:[email protected]] On Behalf >>>Of Jani Nikula >>>Sent: Wednesday, April 1, 2015 1:45 PM >>>To: [email protected] >>>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.
Ok, agreed. > >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! :) Sure... Thanks, Durga > >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 <[email protected]> >>>--- >>> 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 >>>[email protected] >>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> _______________________________________________ >> Intel-gfx mailing list >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
