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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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. 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@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux