[Intel-gfx] [PATCH 05/10] drm/i915: Add debugfs interface for Displayport debug and compliance testing

[Jesse Barnes]

On Thu, 23 Oct 2014 17:43:01 +0200
Daniel Vetter <daniel at ffwll.ch> wrote:

> On Thu, Oct 23, 2014 at 10:58:59AM -0200, Paulo Zanoni wrote:
> > 2014-10-23 10:50 GMT-02:00 Daniel Vetter <daniel at ffwll.ch>:
> > > I think it's better to expose this as drm properties on the DP
> > > connector. This has a pile of upsides:
> > > - We don't need to invent a new parser.
> > > - Users can play with them to test out different theories.
> > > - We could even use this to fix broken panels/boards without vbt
> > > or anything using our plan to set up the initial config with a dt
> > > firmware blob.
> > >
> > > So would be a lot more useful than this private interface.
> > >
> > > For the properties themselves I think we should go with explicit
> > > enumerations with default value "automatic" and then an
> > > enumeration of all values allowed by DP. For the naming of the
> > > properties I guess something like DP_link_lanes,
> > > DP_link_clock, ... would look pretty. The properties should be in
> > > dev->mode_config (since they're reallly generic) and I think we
> > > want one function to register them all in one go in the
> > > drm_dp_helper.c library. Maybe we could even put the values into
> > > the existing dp source struct so that we don't have to reinvent
> > > the decode logic every single time.
> > 
> > I disagree. I do prefer the debugfs thing. Think about all the
> > examples of people that break their systems by passing
> > i915.enable_foo=1 or i915.enable_rc6=7 and then open bug reports...
> > With debug stuff exposed as DP properties, we're going to increase
> > that problem, and in a way that will make it even harder to detect.
> > I really really think these things should be exposed only to the
> > debugfs users, because then if the debugfs file is closed, we can
> > just ignore all the arguments and keep doing "normal" unaffected
> > modesets.
> > 
> > If we don't want the parser, maybe we can make it a binary protocol:
> > we'lll still have to parse it, but it can get much easier.
> 
> We already expose piles of funky stuff to users in these properties
> (e.g. audio on hdmi). And contrary to the module options most of
> these will just result in black screens if you don't understand what
> you're doing, so I think the risk is low. There's also a bit of an
> issue with the current interface as it's not clear which line
> corresponds to which DP interface. Using properties would solve this.
> Also these options have a much more direct impact - if you set them
> and the screen goes dark then the user hopefully realizes that this
> is something they shouldn't touch.
> 
> For fbc and rc6 and all those the problem is that nothing really
> happens, the system just becomes a bit more unstable and randomly
> crashes. Which users then report.
> 
> But If you're really concerned about users doing crappy stuff we
> could add a module option to drm_dp_helper.c which hides these, and
> then taint the kernel if any user sets them. But I really think this
> is overkill. -Daniel

Just chatted with Todd about the state of this patchset.  He's going to
post an update today or tomorrow and summarize the feedback from July
ad what he's done to address it, add changelogs, and address outstanding
review feedback from this posting.

I like the idea of exposing some DP stuff like lane count, preemph,
etc, as new properties, but I don't think it's reasonable to block the
testing stuff on it, for a few reasons:
  1) we should think pretty hard about how we want new ABI like
     this exposed
  2) the compliance spec changes pretty regularly, so keeping an
     unstable interface for it in debugfs may make sense anyway
  3) even if we decide to move the userland test code over to
     properties in the future, the fact that debugfs is unstable means
     we can drop it at that time

So I asked Todd to file a JIRA for the properties feature request,
since it's definitely worth looking at.

Which isn't to say the current interface doesn't have issues, just that
Todd shouldn't rewrite things again to include a new, stable ABI,
probably keeping compliance testing and additional DP test coverage out
of the tree for even longer.

Thoughts, objections?

Thanks,
Jesse