[PATCH 1/1] drm/dp: Add debufs support for DP dpcd log
Sean Paul
seanpaul at chromium.org
Tue Jul 31 13:42:49 UTC 2018
On Tue, Jul 31, 2018 at 05:39:26AM -0700, Tejas Upadhyay wrote:
> Patch adds support for debugging DisplayPort :
>
> * All dpcd read/write transactions are logged
> * Logging done per AUX interface
> * Use following command to to list AUX transactions:
Hi Tejas,
Thanks for your patch. We already have this information going to the logs via
the DRM_DEBUG_KMS() prints in the i2c-over-aux function and via the
drm_dp_dump_access() function in the native read/write functions. So you should
be able to access this information already by writing 0x100 to drm.debug module
param.
Does that satisfy your requirements?
Sean
>
> bash # cat /sys/kernel/debug/drm_dp_aux/$AUXNAME
> @0x0000: 0x12
> @0x0001: 0x14
> @0x0002: 0xc4
> @0x0003: 0x01
> @0x0004: 0x01
> @0x0005: 0x01
> @0x0006: 0x01
> @0x0007: 0x81
> @0x0008: 0x02
> @0x0009: 0x02
> @0x000a: 0x06
> @0x000b: 0x00
> @0x000c: 0x00
> @0x000d: 0x00
> @0x000e: 0x00
> @0x0100: 0x14
> @0x0101: 0x82
> @0x0102: 0x00
> @0x0103: 0x0a
> @0x0104: 0x0a
> @0x0107: 0x10
> @0x0108: 0x01
> @0x0202: 0x77
> @0x0203: 0x00
> @0x0204: 0x01
> @0x0205: 0x02
> @0x0206: 0x66
> @0x0207: 0x00
>
> Above information will help to debug DisplayPort related runtime issues
> knowing what communication was done between DisplayPort sink and source.
>
> Signed-off-by: Tejas Upadhyay <tejas.upadhyay at xilinx.com>
> Signed-off-by: Hyun Kwon <hyun.kwon at xilinx.com>
> ---
> drivers/gpu/drm/Kconfig | 10 +++
> drivers/gpu/drm/drm_dp_helper.c | 174 +++++++++++++++++++++++++++++++++++++++-
> include/drm/drm_dp_helper.h | 7 ++
> 3 files changed, 190 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index f3c71e3..bb16e24 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -34,6 +34,16 @@ config DRM_DP_AUX_CHARDEV
> read and write values to arbitrary DPCD registers on the DP aux
> channel.
>
> +config DRM_DEBUG_DP
> + bool "DRM dp debug helper"
> + default n
> + depends on DRM
> + help
> + Choose this option to enable a debugfs functionality for drm-dp.
> + Will help in debugging display port related issue.
> + Recommended for driver developer as well as tester.
> + Say "Y" to enable debug support.
> +
> config DRM_DEBUG_MM
> bool "Insert extra checks and debug info into the DRM range managers"
> default n
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 08af8d6..e8be361 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -28,6 +28,9 @@
> #include <linux/sched.h>
> #include <linux/i2c.h>
> #include <linux/seq_file.h>
> +#ifdef CONFIG_DRM_DEBUG_DP
> +#include <linux/debugfs.h>
> +#endif
> #include <drm/drm_dp_helper.h>
> #include <drm/drmP.h>
>
> @@ -57,6 +60,171 @@ static u8 dp_get_lane_status(const u8 link_status[DP_LINK_STATUS_SIZE],
> return (l >> s) & 0xf;
> }
>
> +#ifdef CONFIG_DRM_DEBUG_DP
> +
> +static uint drm_dp_debugfs_dpcd_size = 0x5ff;
> +module_param_named(debugfs_dpcd_size, drm_dp_debugfs_dpcd_size, uint, 0444);
> +MODULE_PARM_DESC(debugfs_dpcd_size, "Debugfs DPCD buffer size (default: 1544)");
> +
> +static struct dentry *root;
> +
> +static bool drm_dp_debugfs_bitmap_is_set(struct drm_dp_aux *aux,
> + unsigned int pos)
> +{
> + u8 byte, bit;
> +
> + byte = pos / 8;
> + bit = pos % 8;
> +
> + return !!(aux->dpcd_bitmap[byte] & BIT(bit));
> +}
> +
> +static void drm_dp_debugfs_bitmap_set(struct drm_dp_aux *aux, unsigned int pos)
> +{
> + u8 byte, bit;
> +
> + byte = pos / 8;
> + bit = pos % 8;
> + aux->dpcd_bitmap[byte] |= BIT(bit);
> +}
> +
> +/* DP debugfs read API, to be called by user app */
> +static ssize_t drm_dp_debugfs_read(struct file *f, char __user *buf,
> + size_t size, loff_t *pos)
> +{
> + struct drm_dp_aux *aux = (struct drm_dp_aux *)f->f_inode->i_private;
> + char *kern_buff = NULL;
> + unsigned int i;
> + size_t kern_buff_len;
> + int err;
> +
> + if (size <= 0)
> + return -EINVAL;
> + if (*pos != 0)
> + return 0;
> +
> + kern_buff_len = size;
> + kern_buff = kzalloc(kern_buff_len, GFP_KERNEL);
> + if (!kern_buff)
> + return -ENOMEM;
> +
> + for (i = 0; i < drm_dp_debugfs_dpcd_size; i++) {
> + if (drm_dp_debugfs_bitmap_is_set(aux, i)) {
> + char str[16] = "";
> +
> + sprintf(str, "@0x%04x: 0x%02x\n", i, aux->dpcd_data[i]);
> + if (kern_buff_len < strlen(str))
> + break;
> +
> + strcat(kern_buff, str);
> + kern_buff_len -= strlen(str);
> + }
> + }
> + kern_buff_len = strlen(kern_buff);
> + size = min(size, kern_buff_len);
> + err = copy_to_user(buf, kern_buff, size);
> + kfree(kern_buff);
> + if (err)
> + return err;
> + *pos = size + 1;
> +
> + return size;
> +}
> +
> +static const struct file_operations fops_drm_dp_dbgfs = {
> + .owner = THIS_MODULE,
> + .read = drm_dp_debugfs_read,
> +};
> +
> +static int drm_dp_debugfs_init(struct drm_dp_aux *aux)
> +{
> + struct dentry *entry;
> + u8 *bitmap, *data;
> + int err;
> +
> + /* FIXME: better be under drm debugfs dir, and cleaned up properly */
> + if (!root) {
> + root = debugfs_create_dir("drm_dp_aux", NULL);
> + if (!root) {
> + DRM_ERROR("debugfs_create_dir failed for root\n");
> + return -ENODEV;
> + }
> + }
> +
> + /* FIXME: There may be a better name than the aux name */
> + entry = debugfs_create_file(aux->name, 0444, root, aux,
> + &fops_drm_dp_dbgfs);
> + if (!entry) {
> + DRM_ERROR("debugfs_create_file testcase failed\n");
> + err = -ENODEV;
> + goto err_dbgfs;
> + }
> +
> + bitmap = devm_kzalloc(aux->dev,
> + sizeof(*bitmap) * (drm_dp_debugfs_dpcd_size / 8),
> + GFP_KERNEL);
> + if (unlikely(!bitmap)) {
> + err = -ENOMEM;
> + goto err_dbgfs;
> + }
> +
> + data = devm_kzalloc(aux->dev, sizeof(*data) * drm_dp_debugfs_dpcd_size,
> + GFP_KERNEL);
> + if (unlikely(!data)) {
> + err = -ENOMEM;
> + goto err_dbgfs;
> + }
> +
> + aux->dpcd_data = data;
> + aux->dpcd_bitmap = bitmap;
> +
> + return 0;
> +
> +err_dbgfs:
> + debugfs_remove_recursive(entry);
> + return err;
> +}
> +
> +/* Deinit DP debugfs */
> +static void drm_dp_debugfs_deinit(struct drm_dp_aux *aux)
> +{
> + struct dentry *entry;
> +
> + entry = debugfs_lookup(aux->name, root);
> + debugfs_remove_recursive(entry);
> +}
> +
> +static void drm_dp_debugfs_log(struct drm_dp_aux *aux,
> + struct drm_dp_aux_msg *msg)
> +{
> + u8 *buf;
> + unsigned int i;
> +
> + buf = (u8 *)msg->buffer;
> + for (i = 0; i < msg->size; i++) {
> + aux->dpcd_data[msg->address + i] = buf[i];
> + drm_dp_debugfs_bitmap_set(aux, msg->address + i);
> + }
> +}
> +
> +#else
> +
> +static void drm_dp_debugfs_log(struct drm_dp_aux *aux,
> + struct drm_dp_aux_msg *msg)
> +{
> +}
> +
> +static int drm_dp_debugfs_init(struct drm_dp_aux *aux)
> +{
> + return 0;
> +}
> +
> +static void drm_dp_debugfs_deinit(struct drm_dp_aux *aux)
> +{
> +}
> +
> +#endif
> +
> bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE],
> int lane_count)
> {
> @@ -229,6 +397,8 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
> ret = err;
>
> unlock:
> + /* Log dpcd aux write transactions*/
> + drm_dp_debugfs_log(aux, &msg);
> mutex_unlock(&aux->hw_mutex);
> return ret;
> }
> @@ -1108,7 +1278,8 @@ int drm_dp_aux_register(struct drm_dp_aux *aux)
> drm_dp_aux_unregister_devnode(aux);
> return ret;
> }
> -
> + /* Init DP debufs*/
> + drm_dp_debugfs_init(aux);
> return 0;
> }
> EXPORT_SYMBOL(drm_dp_aux_register);
> @@ -1119,6 +1290,7 @@ EXPORT_SYMBOL(drm_dp_aux_register);
> */
> void drm_dp_aux_unregister(struct drm_dp_aux *aux)
> {
> + drm_dp_debugfs_deinit(aux);
> drm_dp_aux_unregister_devnode(aux);
> i2c_del_adapter(&aux->ddc);
> }
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index b17476a..442dfb8 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -962,6 +962,8 @@ struct drm_dp_aux_msg {
> * @crc_work: worker that captures CRCs for each frame
> * @crc_count: counter of captured frame CRCs
> * @transfer: transfers a message representing a single AUX transaction
> + * @dpcd_data: store aux transactions for dp debugfs
> + * @dpcd_bitmap: store a bitmap for valid dpcd data
> *
> * The .dev field should be set to a pointer to the device that implements
> * the AUX channel.
> @@ -1010,6 +1012,11 @@ struct drm_dp_aux {
> * @i2c_defer_count: Counts I2C DEFERs, used for DP validation.
> */
> unsigned i2c_defer_count;
> +#ifdef CONFIG_DRM_DEBUG_DP
> + /* aux transactions for dp debugfs*/
> + u8 *dpcd_data;
> + u8 *dpcd_bitmap;
> +#endif
> };
>
> ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
> --
> 2.7.4
>
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
--
Sean Paul, Software Engineer, Google / Chromium OS
More information about the dri-devel
mailing list