On Wed, 01 Apr 2015, "R, Durgadoss" <durgadoss.r@xxxxxxxxx> wrote: > Hi Jani, > >>-----Original Message----- >>From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Jani Nikula >>Sent: Wednesday, April 1, 2015 1:45 PM >>To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx >>Cc: Nikula, Jani >>Subject: [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@xxxxxxxxx> >>--- >> 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@xxxxxxxxxxxxxxxxxxxxx >>http://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
