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(). > > > 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 at 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