Re: [PATCH 0/3] RFC: Add a drm_aux-dev module.

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

On Tue, Sep 15, 2015 at 07:46:55PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 15, 2015 at 07:41:06PM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 15, 2015 at 09:34:15AM -0700, Rafael Antognolli wrote:
> > > On Tue, Sep 15, 2015 at 10:35:19AM +0300, Ville Syrjälä wrote:
> > > > On Mon, Sep 14, 2015 at 04:12:29PM -0700, Rafael Antognolli wrote:
> > > > > This is a tentative implementation of a module that allows reading/writing
> > > > > arbitrary dpcd registers, following the suggestion from Daniel Vetter. It
> > > > > assumes the drm aux helpers were used by the driver.
> > > > > 
> > > > > I tried to follow the i2c-dev implementation as close as possible, but the only
> > > > > operations that are provided on the dev node are two different ioctl's, one for
> > > > > reading a register and another one for writing it.
> > > > 
> > > > Why would you use ioctls instead of normal read/write syscalls?
> > > > 
> > > 
> > > For writing, that should work fine, I can easily change that if you
> > > prefer.
> > > 
> > > For reading, one has to first tell which register address is going to be
> > > read.
> > 
> > seek()
> 
> Sorry typo. Make that lseek(). 
> 
Oh, nice, I'll update the patch with this and remove the notifier stuff,
thanks!

> > 
> > > So it would require to first write the address on the file, to
> > > then read. It was suggested that an ioctl to be used, and I understood
> > > it was to solve this, but maybe I'm wrong.
> > > 
> > > I don't have any particular preference for the API, could easily change
> > > this code to use just read/writes.
> > > 
> > > Thanks,
> > > Rafael
> > > 
> > > > > 
> > > > > I have at least 2 open questions:
> > > > > 
> > > > >  * Right now the AUX channels are stored in a global list inside the
> > > > >    drm_dp_helper implementation, and I assume that's not ideal. A different
> > > > >    approach would be to iterate over the list of connectors, instead of the
> > > > >    list of AUX channels, but that would require the struct drm_connector or
> > > > >    similar to know about the respective aux helper. It could be added as a
> > > > >    function to register that, or as a new method on the drm_connector_funcs to
> > > > >    retrieve the aux channel helper.
> > > > > 
> > > > >  * From the created sysfs drm_aux-dev device it's possible to know what is the
> > > > >    respective connector, but not the other way around. Is this good enough?
> > > > > 
> > > > > Please provide any feedback you have and tell me if this is the idea you had
> > > > > initially for this kind of implementation. Or, if it's nothing like this, let
> > > > > me know what else you had in mind.
> > > > > 
> > > > > If I'm going in the right direction, I'll refine the patch to provide full
> > > > > documentation and tests if needed.
> > > > > 
> > > > > Rafael Antognolli (3):
> > > > >   drm/dp: Keep a list of drm_dp_aux helper.
> > > > >   drm/dp: Store the drm_connector device pointer on the helper.
> > > > >   drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
> > > > > 
> > > > >  Documentation/ioctl/ioctl-number.txt |   1 +
> > > > >  drivers/gpu/drm/Kconfig              |   4 +
> > > > >  drivers/gpu/drm/Makefile             |   1 +
> > > > >  drivers/gpu/drm/drm_aux-dev.c        | 390 +++++++++++++++++++++++++++++++++++
> > > > >  drivers/gpu/drm/drm_dp_helper.c      |  71 +++++++
> > > > >  drivers/gpu/drm/i915/intel_dp.c      |   1 +
> > > > >  include/drm/drm_dp_helper.h          |   7 +
> > > > >  include/uapi/linux/Kbuild            |   1 +
> > > > >  include/uapi/linux/drm_aux-dev.h     |  45 ++++
> > > > >  9 files changed, 521 insertions(+)
> > > > >  create mode 100644 drivers/gpu/drm/drm_aux-dev.c
> > > > >  create mode 100644 include/uapi/linux/drm_aux-dev.h
> > > > > 
> > > > > -- 
> > > > > 2.4.0
> > > > > 
> > > > > _______________________________________________
> > > > > dri-devel mailing list
> > > > > dri-devel@lists.freedesktop.org
> > > > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > > 
> > > > -- 
> > > > Ville Syrjälä
> > > > Intel OTC
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel