* Re: [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD
2015-04-01 8:15 [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD Jani Nikula
@ 2015-04-01 12:02 ` R, Durgadoss
2015-04-01 12:35 ` Jani Nikula
2015-04-01 12:25 ` shuang.he
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: R, Durgadoss @ 2015-04-01 12:02 UTC (permalink / raw)
To: intel-gfx@lists.freedesktop.org; +Cc: Nikula, Jani
Hi Jani,
>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Jani Nikula
>Sent: Wednesday, April 1, 2015 1:45 PM
>To: intel-gfx@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'.
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@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@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD
2015-04-01 12:02 ` R, Durgadoss
@ 2015-04-01 12:35 ` Jani Nikula
2015-04-01 12:42 ` R, Durgadoss
0 siblings, 1 reply; 7+ messages in thread
From: Jani Nikula @ 2015-04-01 12:35 UTC (permalink / raw)
To: R, Durgadoss, intel-gfx@lists.freedesktop.org
On Wed, 01 Apr 2015, "R, Durgadoss" <durgadoss.r@intel.com> wrote:
> Hi Jani,
>
>>-----Original Message-----
>>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Jani Nikula
>>Sent: Wednesday, April 1, 2015 1:45 PM
>>To: intel-gfx@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@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@lists.freedesktop.org
>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD
2015-04-01 12:35 ` Jani Nikula
@ 2015-04-01 12:42 ` R, Durgadoss
0 siblings, 0 replies; 7+ messages in thread
From: R, Durgadoss @ 2015-04-01 12:42 UTC (permalink / raw)
To: Nikula, Jani, intel-gfx@lists.freedesktop.org
>-----Original Message-----
>From: Nikula, Jani
>Sent: Wednesday, April 1, 2015 6:05 PM
>To: R, Durgadoss; intel-gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD
>
>On Wed, 01 Apr 2015, "R, Durgadoss" <durgadoss.r@intel.com> wrote:
>> Hi Jani,
>>
>>>-----Original Message-----
>>>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Jani Nikula
>>>Sent: Wednesday, April 1, 2015 1:45 PM
>>>To: intel-gfx@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.
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 <jani.nikula@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@lists.freedesktop.org
>>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD
2015-04-01 8:15 [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD Jani Nikula
2015-04-01 12:02 ` R, Durgadoss
@ 2015-04-01 12:25 ` shuang.he
2015-04-01 17:26 ` Bob Paauwe
2015-04-08 6:08 ` R, Durgadoss
3 siblings, 0 replies; 7+ messages in thread
From: shuang.he @ 2015-04-01 12:25 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, jani.nikula
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6111
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV -3 272/272 269/272
ILK -1 302/302 301/302
SNB 303/303 303/303
IVB 338/338 338/338
BYT -1 287/287 286/287
HSW -2 361/361 359/361
BDW 308/308 308/308
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
PNV igt@gem_tiled_pread_pwrite FAIL(3)PASS(3) FAIL(1)PASS(1)
PNV igt@gem_userptr_blits@coherency-sync CRASH(2)PASS(4) CRASH(1)PASS(1)
PNV igt@gen3_render_tiledx_blits FAIL(3)PASS(2) FAIL(1)PASS(1)
*ILK igt@drv_suspend@debugfs-reader PASS(2) DMESG_WARN(2)
(dmesg patch applied)drm:i915_dpcd_show[i915]]*ERROR*dpcd_read(#bytes_at#)failed(-#)@dpcd read .* bytes at .* failed
*BYT igt@gem_exec_bad_domains@conflicting-write-domain PASS(2) FAIL(1)PASS(1)
*HSW igt@gem_pwrite_pread@uncached-copy-performance PASS(2) DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck timer elapsed... blitter ring idle
*HSW igt@pm_rpm@debugfs-read PASS(2) DMESG_WARN(2)
(dmesg patch applied)drm:i915_dpcd_show[i915]]*ERROR*dpcd_read(#bytes_at#)failed(-#)@dpcd read .* bytes at .* failed
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD
2015-04-01 8:15 [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD Jani Nikula
2015-04-01 12:02 ` R, Durgadoss
2015-04-01 12:25 ` shuang.he
@ 2015-04-01 17:26 ` Bob Paauwe
2015-04-08 6:08 ` R, Durgadoss
3 siblings, 0 replies; 7+ messages in thread
From: Bob Paauwe @ 2015-04-01 17:26 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Wed, 1 Apr 2015 11:15:21 +0300
Jani Nikula <jani.nikula@intel.com> wrote:
> 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
> 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@intel.com>
Reviewed-by: Bob Paauwe <bob.j.paauwe@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;
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD
2015-04-01 8:15 [PATCH v2] drm/i915: add i915 specific connector debugfs file for DPCD Jani Nikula
` (2 preceding siblings ...)
2015-04-01 17:26 ` Bob Paauwe
@ 2015-04-08 6:08 ` R, Durgadoss
3 siblings, 0 replies; 7+ messages in thread
From: R, Durgadoss @ 2015-04-08 6:08 UTC (permalink / raw)
To: intel-gfx@lists.freedesktop.org; +Cc: Nikula, Jani
>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Jani Nikula
>Sent: Wednesday, April 1, 2015 1:45 PM
>To: intel-gfx@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
>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.
>
Tested-by: Durgadoss R <durgadoss.r@intel.com>
Thanks,
Durga
>Signed-off-by: Jani Nikula <jani.nikula@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@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread