[Intel-gfx] [PATCH v2] drm/i915: Add debugfs to read any DPCD register
R, Durgadoss
durgadoss.r at intel.com
Thu Apr 16 23:33:57 PDT 2015
>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
>Sent: Thursday, April 16, 2015 8:03 PM
>To: Jani Nikula
>Cc: R, Durgadoss; intel-gfx at lists.freedesktop.org; Syrjala, Ville; Zanoni, Paulo R
>Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Add debugfs to read any DPCD register
>
>On Thu, Apr 16, 2015 at 05:07:00PM +0300, Jani Nikula wrote:
>> On Wed, 08 Apr 2015, Durgadoss R <durgadoss.r at intel.com> wrote:
>> > This patch creates a connector specific debugfs
>> > interface to read any particular DPCD register.
>> > The DPCD register address (hex format) is written
>> > to 'i915_dpcd_addr' interface and the corresponding
>> > value can be read from 'i915_dpcd_val' interface.
>> >
>> > Example usage:
>> > echo 0x70 > i915_dpcd_addr
>> > cat i915_dpcd_val
>> > 0x1
>> >
>> > v2: Address Jani's comments.
>>
>> So, code-wise this looks good to me.
>>
>> But like I said in my review of v1, I am undecided on whether we want to
>> have this interface or not. I could really be persuaded either way.
>>
>> Perhaps Ville or Paulo have an opinion?
>
>I think having the ability to read (and also write actually) DPCD would
>be nice.
>
>So I like the idea, but it does seem a bit limited since it doesn't
>allow writes. So perhaps we should just expose the entire DPCD as a
>binary file and include a tool in i-g-t to read/write individual
>pieces?
>
>We could also have a tool to parse the entire thing like we have
>for the vbt stuff.
Yes, This seems to be a good idea. I will start working on this.
Although I am not sure on the 'write' part.. I guess we can discuss
as we go along..
But I still think, a pointed read/write would also be useful
at times for debug. So, If you agree, I can extend this interface
to take values for write as well and submit a v3.
Thanks,
Durga
>
>>
>> BR,
>> Jani.
>>
>>
>> >
>> > Signed-off-by: Durgadoss R <durgadoss.r at intel.com>
>> > ---
>> > drivers/gpu/drm/i915/i915_debugfs.c | 88 ++++++++++++++++++++++++++++++++++++-
>> > drivers/gpu/drm/i915/intel_drv.h | 1 +
>> > 2 files changed, 88 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> > index 10ca511..f1f365e 100644
>> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> > @@ -4865,6 +4865,87 @@ static const struct file_operations i915_dpcd_fops = {
>> > .release = single_release,
>> > };
>> >
>> > +static ssize_t i915_dpcd_addr_write(struct file *file, const char __user *ubuf,
>> > + size_t len, loff_t *offp)
>> > +{
>> > + struct seq_file *m = file->private_data;
>> > + struct drm_connector *connector = m->private;
>> > + struct intel_dp *intel_dp =
>> > + enc_to_intel_dp(&intel_attached_encoder(connector)->base);
>> > + unsigned int reg_addr;
>> > + char tmp[16];
>> > +
>> > + if (len >= sizeof(tmp))
>> > + return -EINVAL;
>> > +
>> > + if (copy_from_user(tmp, ubuf, len))
>> > + return -EFAULT;
>> > +
>> > + tmp[len] = '\0';
>> > +
>> > + if (kstrtouint(tmp, 16, ®_addr))
>> > + return -EINVAL;
>> > +
>> > + intel_dp->debugfs_dpcd_addr = reg_addr;
>> > +
>> > + return len;
>> > +}
>> > +
>> > +static int i915_dpcd_addr_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);
>> > +
>> > + seq_printf(m, "0x%x\n", intel_dp->debugfs_dpcd_addr);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int i915_dpcd_addr_open(struct inode *inode, struct file *file)
>> > +{
>> > + return single_open(file, i915_dpcd_addr_show, inode->i_private);
>> > +}
>> > +
>> > +static const struct file_operations i915_dpcd_addr_fops = {
>> > + .owner = THIS_MODULE,
>> > + .open = i915_dpcd_addr_open,
>> > + .read = seq_read,
>> > + .write = i915_dpcd_addr_write,
>> > + .llseek = seq_lseek,
>> > + .release = single_release,
>> > +};
>> > +
>> > +static int i915_dpcd_val_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);
>> > + u8 val;
>> > + int ret;
>> > +
>> > + ret = drm_dp_dpcd_readb(&intel_dp->aux,
>> > + intel_dp->debugfs_dpcd_addr, &val);
>> > + if (ret < 0)
>> > + return ret;
>> > +
>> > + seq_printf(m, "0x%x\n", val);
>> > + return 0;
>> > +}
>> > +
>> > +static int i915_dpcd_val_open(struct inode *inode, struct file *file)
>> > +{
>> > + return single_open(file, i915_dpcd_val_show, inode->i_private);
>> > +}
>> > +
>> > +static const struct file_operations i915_dpcd_val_fops = {
>> > + .owner = THIS_MODULE,
>> > + .open = i915_dpcd_val_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
>> > @@ -4883,9 +4964,14 @@ int i915_debugfs_connector_add(struct drm_connector *connector)
>> > return -ENODEV;
>> >
>> > if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
>> > - connector->connector_type == DRM_MODE_CONNECTOR_eDP)
>> > + connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
>> > debugfs_create_file("i915_dpcd", S_IRUGO, root, connector,
>> > &i915_dpcd_fops);
>> > + debugfs_create_file("i915_dpcd_addr", S_IRUGO | S_IWUSR,
>> > + root, connector, &i915_dpcd_addr_fops);
>> > + debugfs_create_file("i915_dpcd_val", S_IRUGO, root,
>> > + connector, &i915_dpcd_val_fops);
>> > + }
>> >
>> > return 0;
>> > }
>> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> > index 4799b11..0594baa 100644
>> > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > @@ -642,6 +642,7 @@ struct intel_dp {
>> > unsigned long last_power_cycle;
>> > unsigned long last_power_on;
>> > unsigned long last_backlight_off;
>> > + unsigned int debugfs_dpcd_addr;
>> >
>> > struct notifier_block edp_notifier;
>> >
>> > --
>> > 1.9.1
>> >
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel OTC
More information about the Intel-gfx
mailing list