[PATCH 0/3] RFC: Add a drm_aux-dev module.
On Wed, 16 Sep 2015, Ville Syrjälä wrote: > On Wed, Sep 16, 2015 at 01:09:54PM -0700, Daniel Vetter wrote: >> On Tue, Sep 15, 2015 at 10:03:09AM -0700, Rafael Antognolli wrote: >> > 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! >> >> Well there's also other modes like i2c over aux, and that would be easier >> to support with an ioctl in a uniform way. So not sure how much value >> there is in reusing read/write for i2c ... > > We already have i2c-dev for i2c. So not sure what you're after here. Yeah, definitely don't reinvent the wheel for this. BR, Jani. > > i2c is a bit of a mess on the protocol level. Lots of devices do > interesting things with it, so it can't really make do with just > read/write/lseek. Apart from i2c-over-aux, the rest is all about > DPCD access, and for that read/write/lseek is all you ever need. > > -- > Ville Syrjälä > Intel OTC > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center
[PATCH 0/3] RFC: Add a drm_aux-dev module.
On Thu, Sep 17, 2015 at 12:01 AM, Jani Nikula wrote: > On Wed, 16 Sep 2015, Ville Syrjälä wrote: >> On Wed, Sep 16, 2015 at 01:09:54PM -0700, Daniel Vetter wrote: >>> On Tue, Sep 15, 2015 at 10:03:09AM -0700, Rafael Antognolli wrote: >>> > 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! >>> >>> Well there's also other modes like i2c over aux, and that would be easier >>> to support with an ioctl in a uniform way. So not sure how much value >>> there is in reusing read/write for i2c ... >> >> We already have i2c-dev for i2c. So not sure what you're after here. > > Yeah, definitely don't reinvent the wheel for this. Of course I didn't mean to reinvent the i2c-dev wheel, but just expose the underlying low-level dp aux ops to do i2c transaction - that might be useful for hacking around to figure out some of the recent i2c changes we've done. Otoh we could add such a raw mode later on as an ioctl, for plain dpcd reads/writes read/write/lseek seems indeed a good fit. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 0/3] RFC: Add a drm_aux-dev module.
On Wed, Sep 16, 2015 at 01:09:54PM -0700, Daniel Vetter wrote: > On Tue, Sep 15, 2015 at 10:03:09AM -0700, Rafael Antognolli wrote: > > 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! > > Well there's also other modes like i2c over aux, and that would be easier > to support with an ioctl in a uniform way. So not sure how much value > there is in reusing read/write for i2c ... We already have i2c-dev for i2c. So not sure what you're after here. i2c is a bit of a mess on the protocol level. Lots of devices do interesting things with it, so it can't really make do with just read/write/lseek. Apart from i2c-over-aux, the rest is all about DPCD access, and for that read/write/lseek is all you ever need. -- Ville Syrjälä Intel OTC
[PATCH 0/3] RFC: Add a drm_aux-dev module.
On Wed, Sep 16, 2015 at 11:47:21PM +0300, Ville Syrjälä wrote: > On Wed, Sep 16, 2015 at 01:09:54PM -0700, Daniel Vetter wrote: > > On Tue, Sep 15, 2015 at 10:03:09AM -0700, Rafael Antognolli wrote: > > > 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! > > > > Well there's also other modes like i2c over aux, and that would be easier > > to support with an ioctl in a uniform way. So not sure how much value > > there is in reusing read/write for i2c ... > > We already have i2c-dev for i2c. So not sure what you're after here. > > i2c is a bit of a mess on the protocol level. Lots of devices do > interesting things with it, so it can't really make do with just > read/write/lseek. Apart from i2c-over-aux, the rest is all about > DPCD access, and for that read/write/lseek is all you ever need. I just noticed that I forgot to cc you guys, but yesterday I sent an updated version of the patch set to this list: http://lists.freedesktop.org/archives/dri-devel/2015-September/090366.html I also don't have a strong opinion about ioctl vs read/write/lseek, but at least my second implementation did get a little cleaner. Thanks, Rafael
[PATCH 0/3] RFC: Add a drm_aux-dev module.
On Tue, Sep 15, 2015 at 10:03:09AM -0700, Rafael Antognolli wrote: > 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! Well there's also other modes like i2c over aux, and that would be easier to support with an ioctl in a uniform way. So not sure how much value there is in reusing read/write for i2c ... But I really don't have a strong opinion about this. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH 0/3] RFC: Add a drm_aux-dev module.
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
[PATCH 0/3] RFC: Add a drm_aux-dev module.
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() > 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
[PATCH 0/3] RFC: Add a drm_aux-dev module.
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? > > 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
[PATCH 0/3] RFC: Add a drm_aux-dev module.
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 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
[PATCH 0/3] RFC: Add a drm_aux-dev module.
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 at lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel OTC
[PATCH 0/3] RFC: Add a drm_aux-dev module.
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. 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
