Re: [RFC 11/13] drm/dp: Add helper to dump DPCD

[Rafael Antognolli <rafael.antognolli@intel.com>]

On Mon, Aug 17, 2015 at 10:02:04AM +0300, Jani Nikula wrote:
> On Fri, 14 Aug 2015, Rafael Antognolli <rafael.antognolli@intel.com> wrote:
> > On Fri, Aug 14, 2015 at 02:56:55PM +0300, Jani Nikula wrote:
> >> On Wed, 12 Aug 2015, Thierry Reding <thierry.reding@gmail.com> wrote:
> >> > From: Thierry Reding <treding@nvidia.com>
> >> >
> >> > The new drm_dp_dpcd_dump() helper dumps the contents of a DPCD to a
> >> > seq_file and can be used to make the DPCD available via debugfs for
> >> > example.
> >> 
> >> See i915/i915_debugfs.c for one DPCD dump implementation.
> >> 
> >> Around the time that was added, there was also some discussion (and
> >> patches [1]) to expose a read/write debugfs interface to DPCD, letting
> >> userspace access arbitrary DPCD registers.
> >> 
> >> Just this week there was some discussion about revisiting that. It was
> >> about accessing some proprietary panel features, but there's also the
> >> ease of debugging without having to keep updating the kernel to dump
> >> more.
> >> 
> >> I think it would be great to agree on a common debugfs interface to
> >> access DPCD arbitrarily. Last time I checked, the blocker to that was
> >> access to the aux channel from generic code; it's always driver
> >> specific. SMOP. ;)
> >
> > Do you mean it would require the generic code/interface to somehow route
> > this to the driver specific code? I am not sure yet how this works (if
> > there's something like it around), but I'll take a look.
> 
> Drivers can choose to support DP AUX any way they like, and they don't
> have to use the common helpers to do so. It's pretty much an
> implementation detail. I think we could require the use of the common
> helpers to support generic DPCD access from debugfs, but we'd still have
> to come up with a way to get hold of struct drm_dp_aux (again, driver
> internals) given a drm_connector in generic debugfs code.

I was under the assumption they always used the helpers, but I
understand that's not always the case.

Anyway, I was going to propose a new helper to "store" the drm_dp_aux
inside the drm_connector. And just expose something on debugfs if it was
used. Something like:

int drm_dp_aux_store(struct drm_dp_aux *aux,
                     struct drm_connector *connector);

The helpers don't seem to know about drm_connector's though, so I'm not sure
this is a good approach.

> Thierry, do you see any problems with having a connector function to get
> hold of its drm_dp_aux?

Would this be a new function pointer inside struct drm_connector_funcs?
If so, I'm fine with this approach too.

Regarding the interface, you mentioned that you didn't like it because
it had a state. What about just dumping the content of the register into
dmesg when one tries to read it, and use a different sintaxe for
writing, passing both the register addr and the value?

Daniel had another suggestion, regarding an ioctl in debugfs, but I'm
not sure I clearly understand that yet.

Thanks,
Rafael
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel