| Message ID | 1399877207-15868-11-git-send-email-airlied@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 12, 2014 at 04:46:47PM +1000, Dave Airlie wrote: [...] > @@ -3813,6 +3838,7 @@ static const struct drm_info_list i915_debugfs_list[] = { > {"i915_pc8_status", i915_pc8_status, 0}, > {"i915_power_domain_info", i915_power_domain_info, 0}, > {"i915_display_info", i915_display_info, 0}, > + {"i915_dp_mst_info", i915_dp_mst_info, 0}, This isn't really specific to this patch, but I've been thinking for a while if perhaps it would be a good idea to try to unify debugfs across drivers to some degree. What I mean by that is try to use common names for files with similar functionality. Off the top of my head I think a couple of drivers expose a list of framebuffers via debugfs, mostly using duplicated code and different file names. Sharing the code would of course be easy, but I think there may be some advantage to keeping the names consistent as well. Given its generic nature, MST sounds like a good candidate as well. Thierry
On Tue, May 13, 2014 at 10:33:27AM +0200, Thierry Reding wrote: > On Mon, May 12, 2014 at 04:46:47PM +1000, Dave Airlie wrote: > [...] > > @@ -3813,6 +3838,7 @@ static const struct drm_info_list i915_debugfs_list[] = { > > {"i915_pc8_status", i915_pc8_status, 0}, > > {"i915_power_domain_info", i915_power_domain_info, 0}, > > {"i915_display_info", i915_display_info, 0}, > > + {"i915_dp_mst_info", i915_dp_mst_info, 0}, > > This isn't really specific to this patch, but I've been thinking for a > while if perhaps it would be a good idea to try to unify debugfs across > drivers to some degree. What I mean by that is try to use common names > for files with similar functionality. Off the top of my head I think a > couple of drivers expose a list of framebuffers via debugfs, mostly > using duplicated code and different file names. Sharing the code would > of course be easy, but I think there may be some advantage to keeping > the names consistent as well. Imo our current approach of having seq_file helpers in libraries that drivers can use works well. At least with i915 we often want to add some more driver-private state to dumps (e.g. for framebuffers), so extracting more than what's already extracted is probably hard to do. But I guess we could add some common files with e.g. just the core framebuffer stuff to all drivers. Not terribly motivated myself though since i915 has it all already ;-) -Daniel
On Tue, May 13, 2014 at 12:18:35PM +0200, Daniel Vetter wrote: > On Tue, May 13, 2014 at 10:33:27AM +0200, Thierry Reding wrote: > > On Mon, May 12, 2014 at 04:46:47PM +1000, Dave Airlie wrote: > > [...] > > > @@ -3813,6 +3838,7 @@ static const struct drm_info_list i915_debugfs_list[] = { > > > {"i915_pc8_status", i915_pc8_status, 0}, > > > {"i915_power_domain_info", i915_power_domain_info, 0}, > > > {"i915_display_info", i915_display_info, 0}, > > > + {"i915_dp_mst_info", i915_dp_mst_info, 0}, > > > > This isn't really specific to this patch, but I've been thinking for a > > while if perhaps it would be a good idea to try to unify debugfs across > > drivers to some degree. What I mean by that is try to use common names > > for files with similar functionality. Off the top of my head I think a > > couple of drivers expose a list of framebuffers via debugfs, mostly > > using duplicated code and different file names. Sharing the code would > > of course be easy, but I think there may be some advantage to keeping > > the names consistent as well. > > Imo our current approach of having seq_file helpers in libraries that > drivers can use works well. At least with i915 we often want to add some > more driver-private state to dumps (e.g. for framebuffers), so extracting > more than what's already extracted is probably hard to do. Good point. > But I guess we could add some common files with e.g. just the core > framebuffer stuff to all drivers. Not terribly motivated myself though > since i915 has it all already ;-) I was thinking it would be convenient if there was some consistency between the debugfs interfaces of the various drivers to make it easier for people to find equivalent information sources. On second thought maybe that's not such a useful idea after all since some of the files may expose similar, but not exactly the same, information, given that some of it may be driver-specific. Thierry
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 88e944f..b8a9a51 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2378,6 +2378,31 @@ struct pipe_crc_info { enum pipe pipe; }; +static int i915_dp_mst_info(struct seq_file *m, void *unused) +{ + struct drm_info_node *node = (struct drm_info_node *) m->private; + struct drm_device *dev = node->minor->dev; + struct drm_encoder *encoder; + struct intel_encoder *intel_encoder; + struct intel_digital_port *intel_dig_port; + drm_modeset_lock_all(dev); + list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { + intel_encoder = to_intel_encoder(encoder); + if (intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT) + continue; + intel_dig_port = enc_to_dig_port(encoder); + if (!intel_dig_port->dp.can_mst) + continue; + + if (!intel_dig_port->dp.is_mst) + continue; + + drm_dp_mst_dump_topology(m, &intel_dig_port->dp.mst_mgr); + } + drm_modeset_unlock_all(dev); + return 0; +} + static int i915_pipe_crc_open(struct inode *inode, struct file *filep) { struct pipe_crc_info *info = inode->i_private; @@ -3813,6 +3838,7 @@ static const struct drm_info_list i915_debugfs_list[] = { {"i915_pc8_status", i915_pc8_status, 0}, {"i915_power_domain_info", i915_power_domain_info, 0}, {"i915_display_info", i915_display_info, 0}, + {"i915_dp_mst_info", i915_dp_mst_info, 0}, }; #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)