>-----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, ®_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
