Re: [PATCH v2] drm/i915: Add debugfs to read any DPCD register

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx]
>Sent: Thursday, April 16, 2015 8:03 PM
>To: Jani Nikula
>Cc: R, Durgadoss; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Syrjala, Ville; Zanoni, Paulo R
>Subject: Re:  [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@xxxxxxxxx> 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@xxxxxxxxx>
>> > ---
>> >  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, &reg_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@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux