| Message ID | 1384171235-2498-2-git-send-email-treding@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Thierry, I have already sent patch with DSI bus implementation [1]. It was posted as the first step of CDF implementation attempt, but in fact it do not depend on CDF. [1] http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg45252.html One comment below. On 11/11/2013 01:00 PM, Thierry Reding wrote: > In order to support DSI peripherals, add a DSI bus type that devices and > drivers can be registered with. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- [snip] > + > +/* > + * DSI packet data types > + */ > + > +/* processor-sourced packets */ > +#define DSI_CMD_VSYNC_START 0x01 > +#define DSI_CMD_VSYNC_END 0x11 > +#define DSI_CMD_HSYNC_START 0x21 > +#define DSI_CMD_HSYNC_END 0x31 > +#define DSI_CMD_EOT 0x08 > +#define DSI_CMD_COLOR_MODE_OFF 0x02 > +#define DSI_CMD_COLOR_MODE_ON 0x12 > +#define DSI_CMD_SHUT_DOWN 0x22 > +#define DSI_CMD_TURN_ON 0x32 > +#define DSI_CMD_GEN_SHORT_WRITE_0 0x03 > +#define DSI_CMD_GEN_SHORT_WRITE_1 0x13 > +#define DSI_CMD_GEN_SHORT_WRITE_2 0x23 > +#define DSI_CMD_GEN_SHORT_READ_0 0x04 > +#define DSI_CMD_GEN_SHORT_READ_1 0x14 > +#define DSI_CMD_GEN_SHORT_READ_2 0x24 > +#define DSI_CMD_DCS_SHORT_WRITE_0 0x05 > +#define DSI_CMD_DCS_SHORT_WRITE_1 0x15 > +#define DSI_CMD_DCS_SHORT_READ 0x06 > +#define DSI_CMD_SET_MAX_RETURN_PACKET_SIZE 0x37 > +#define DSI_CMD_NULL 0x09 > +#define DSI_CMD_BLANK 0x19 > +#define DSI_CMD_GEN_LONG_WRITE 0x29 > +#define DSI_CMD_DCS_LONG_WRITE 0x39 > +#define DSI_CMD_YCbCr422_20 0x0c > +#define DSI_CMD_YCbCr422_24 0x1c > +#define DSI_CMD_YCbCr422_16 0x2c > +#define DSI_CMD_RGB30 0x0d > +#define DSI_CMD_RGB36 0x1d > +#define DSI_CMD_YCbCr420 0x3d > +#define DSI_CMD_RGB16 0x0e > +#define DSI_CMD_RGB18 0x1e > +#define DSI_CMD_RGB18NP 0x2e > +#define DSI_CMD_RGB24 0x3e > + > +/* peripheral-sourced */ > +#define DSI_RSP_ACK_ERR 0x02 > +#define DSI_RSP_EOT 0x08 > +#define DSI_RSP_GEN_SHORT_READ_1 0x11 > +#define DSI_RSP_GEN_SHORT_READ_2 0x12 > +#define DSI_RSP_GEN_LONG_READ 0x1a > +#define DSI_RSP_DCS_LONG_READ 0x1c > +#define DSI_RSP_DCS_SHORT_READ_1 0x21 > +#define DSI_RSP_DCS_SHORT_READ_2 0x22 > + Those macros are already defined in include/video/mipi_display.h Regards Andrzej
On Tue, Nov 12, 2013 at 03:14:22PM +0100, Andrzej Hajda wrote: > Hi Thierry, > > I have already sent patch with DSI bus implementation [1]. > It was posted as the first step of CDF implementation attempt, > but in fact it do not depend on CDF. > > [1] > http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg45252.html Seems like that patchset was never merged. I guess probably because it was posted as part of CDF work. Do you have any plans on continuing work on that? If not I could extract the DSI bus patch from the series, it's largely similar to the patch I proposed here, and rework it somewhat. I'd very much like to avoid putting the code in drivers/video, though, since that's considered obsolete. Furthermore I think if we kept the transfer function proposed in my patch should make it easier to address Bert's comments from your posting. > One comment below. > > On 11/11/2013 01:00 PM, Thierry Reding wrote: > > In order to support DSI peripherals, add a DSI bus type that devices and > > drivers can be registered with. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > [snip] > > + > > +/* > > + * DSI packet data types > > + */ > > + > > +/* processor-sourced packets */ > > +#define DSI_CMD_VSYNC_START 0x01 > > +#define DSI_CMD_VSYNC_END 0x11 > > +#define DSI_CMD_HSYNC_START 0x21 > > +#define DSI_CMD_HSYNC_END 0x31 > > +#define DSI_CMD_EOT 0x08 > > +#define DSI_CMD_COLOR_MODE_OFF 0x02 > > +#define DSI_CMD_COLOR_MODE_ON 0x12 > > +#define DSI_CMD_SHUT_DOWN 0x22 > > +#define DSI_CMD_TURN_ON 0x32 > > +#define DSI_CMD_GEN_SHORT_WRITE_0 0x03 > > +#define DSI_CMD_GEN_SHORT_WRITE_1 0x13 > > +#define DSI_CMD_GEN_SHORT_WRITE_2 0x23 > > +#define DSI_CMD_GEN_SHORT_READ_0 0x04 > > +#define DSI_CMD_GEN_SHORT_READ_1 0x14 > > +#define DSI_CMD_GEN_SHORT_READ_2 0x24 > > +#define DSI_CMD_DCS_SHORT_WRITE_0 0x05 > > +#define DSI_CMD_DCS_SHORT_WRITE_1 0x15 > > +#define DSI_CMD_DCS_SHORT_READ 0x06 > > +#define DSI_CMD_SET_MAX_RETURN_PACKET_SIZE 0x37 > > +#define DSI_CMD_NULL 0x09 > > +#define DSI_CMD_BLANK 0x19 > > +#define DSI_CMD_GEN_LONG_WRITE 0x29 > > +#define DSI_CMD_DCS_LONG_WRITE 0x39 > > +#define DSI_CMD_YCbCr422_20 0x0c > > +#define DSI_CMD_YCbCr422_24 0x1c > > +#define DSI_CMD_YCbCr422_16 0x2c > > +#define DSI_CMD_RGB30 0x0d > > +#define DSI_CMD_RGB36 0x1d > > +#define DSI_CMD_YCbCr420 0x3d > > +#define DSI_CMD_RGB16 0x0e > > +#define DSI_CMD_RGB18 0x1e > > +#define DSI_CMD_RGB18NP 0x2e > > +#define DSI_CMD_RGB24 0x3e > > + > > +/* peripheral-sourced */ > > +#define DSI_RSP_ACK_ERR 0x02 > > +#define DSI_RSP_EOT 0x08 > > +#define DSI_RSP_GEN_SHORT_READ_1 0x11 > > +#define DSI_RSP_GEN_SHORT_READ_2 0x12 > > +#define DSI_RSP_GEN_LONG_READ 0x1a > > +#define DSI_RSP_DCS_LONG_READ 0x1c > > +#define DSI_RSP_DCS_SHORT_READ_1 0x21 > > +#define DSI_RSP_DCS_SHORT_READ_2 0x22 > > + > > Those macros are already defined in include/video/mipi_display.h I wasn't aware, thanks for the pointer. It's somewhat unfortunate that that's in include/video, but judging by the git log, it has been there for quite a while. Thierry
Hi Thierry, On 11/13/2013 10:38 PM, Thierry Reding wrote: > On Tue, Nov 12, 2013 at 03:14:22PM +0100, Andrzej Hajda wrote: >> Hi Thierry, >> >> I have already sent patch with DSI bus implementation [1]. >> It was posted as the first step of CDF implementation attempt, >> but in fact it do not depend on CDF. >> >> [1] >> http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg45252.html > Seems like that patchset was never merged. I guess probably because it > was posted as part of CDF work. > > Do you have any plans on continuing work on that?If not I could extract > the DSI bus patch from the series, it's largely similar to the patch I > proposed here, and rework it somewhat. I will soon sent new patch with the current version of the bus. It could be a better base to your rework. > I'd very much like to avoid > putting the code in drivers/video, though, since that's considered > obsolete. I have followed convention proposed by Laurent in his DBI bus. It seems to me OK - DSI bus is more related to video than to drm. I know that drivers/video is mostly occupied by FB drivers, but according to Kconfig it is not only for FB. Of course this is only my suggestion. > Furthermore I think if we kept the transfer function proposed > in my patch should make it easier to address Bert's comments from your > posting. I am not sure which part of Barts comment you are addressing. Anyway I also prefer passing struct and returning ssize_t. I am not sure about splitting type and channel but this seems to be a minor detail. Regards Andrzej
On 11/14/2013 14:16, Andrzej Hajda wrote: > On 11/13/2013 10:38 PM, Thierry Reding wrote: > > Furthermore I think if we kept the transfer function proposed > > in my patch should make it easier to address Bert's comments from your > > posting. > I am not sure which part of Barts comment you are addressing. > Anyway I also prefer passing struct and returning ssize_t. > I am not sure about splitting type and channel but this seems to be a > minor detail. Most of the comments I made about Andrzej's patch from last month apply here, and would result in extensions to struct dsi_msg. Some means of choosing whether the transfer should be in HS or LP mode is necessary. For video mode panels some means of specifying a period (VFP, VBP, etc) for sending the transfer is needed. Perhaps struct dsi_msg should include: #define DSI_WINDOW_VFP (1 << 0) #define DSI_WINDOW_ACT (1 << 1) #define DSI_WINDOW_VBP (1 << 2) #define DSI_WINDOW_VSY (1 << 3) /** * struct dsi_msg - DSI command message * @channel: virtual channel to send the message to * @type: data ID of the message * @lp_mode: send in LP mode if non-zero * @window: video period when transfer is allowed - bitmask of DSI_WINDOW_* * @tx_len: length of transmission buffer * @tx: transmission buffer * @rx_len: length of reception buffer * @rx: reception buffer */ struct dsi_msg { u8 channel; u8 type; u8 lp_mode; u8 window; size_t tx_len; void *tx; size_t rx_len; void *rx; }; The ability to specify synchronisation with a tearing effect control signal for a command mode panel is obviously important. It's somewhat analogous to the "window" option for video mode. They're little used, but a mechanism for sending triggers should be included. They're probably sufficiently different that it should be a different op. All the best, Bert.
On Thu, Nov 14, 2013 at 03:04:19PM +0000, Bert Kenward wrote: > On 11/14/2013 14:16, Andrzej Hajda wrote: > > On 11/13/2013 10:38 PM, Thierry Reding wrote: > > > Furthermore I think if we kept the transfer function proposed > > > in my patch should make it easier to address Bert's comments from your > > > posting. > > I am not sure which part of Barts comment you are addressing. > > Anyway I also prefer passing struct and returning ssize_t. > > I am not sure about splitting type and channel but this seems to be a > > minor detail. > > Most of the comments I made about Andrzej's patch from last month > apply here, and would result in extensions to struct dsi_msg. Some > means of choosing whether the transfer should be in HS or LP mode is > necessary. For video mode panels some means of specifying a period > (VFP, VBP, etc) for sending the transfer is needed. Perhaps struct > dsi_msg should include: > > #define DSI_WINDOW_VFP (1 << 0) > #define DSI_WINDOW_ACT (1 << 1) > #define DSI_WINDOW_VBP (1 << 2) > #define DSI_WINDOW_VSY (1 << 3) > > /** > * struct dsi_msg - DSI command message > * @channel: virtual channel to send the message to > * @type: data ID of the message > * @lp_mode: send in LP mode if non-zero Perhaps a flags field would be more flexible here. I can easily imagine other I can imagine that other flags may be needed eventually. > * @window: video period when transfer is allowed - bitmask of DSI_WINDOW_* I'm not sure if this is the right interface. What will happen for instance if the hardware doesn't support any of the bits in that mask? Perhaps a slightly better approach might be to expose the capabilities of the DSI host, so that the DSI core knows up front which windows can be used. > * @tx_len: length of transmission buffer > * @tx: transmission buffer > * @rx_len: length of reception buffer > * @rx: reception buffer > */ > struct dsi_msg { > u8 channel; > u8 type; > u8 lp_mode; > u8 window; > > size_t tx_len; > void *tx; > > size_t rx_len; > void *rx; > }; > > The ability to specify synchronisation with a tearing effect control > signal for a command mode panel is obviously important. It's somewhat > analogous to the "window" option for video mode. > > They're little used, but a mechanism for sending triggers should be > included. They're probably sufficiently different that it should be a > different op. I agree. While I currently have no use-case where this is required, I think having a struct dsi_msg makes it easier to extend the featureset on an as-needed basis. Thierry
On 11/18/2013 11:22, Thierry Reding wrote: > On Thu, Nov 14, 2013 at 03:04:19PM +0000, Bert Kenward wrote: > > #define DSI_WINDOW_VFP (1 << 0) > > #define DSI_WINDOW_ACT (1 << 1) > > #define DSI_WINDOW_VBP (1 << 2) > > #define DSI_WINDOW_VSY (1 << 3) > > > > /** > > * struct dsi_msg - DSI command message > > * @channel: virtual channel to send the message to > > * @type: data ID of the message > > * @lp_mode: send in LP mode if non-zero > > Perhaps a flags field would be more flexible here. I can easily imagine > other I can imagine that other flags may be needed eventually. Agreed. "TE synchronised" would be one such extra flag, for supporting command mode updates. > > * @window: video period when transfer is allowed - bitmask of > DSI_WINDOW_* > > I'm not sure if this is the right interface. What will happen for > instance if the hardware doesn't support any of the bits in that mask? > Perhaps a slightly better approach might be to expose the capabilities > of the DSI host, so that the DSI core knows up front which windows can > be used. Exposing the capabilities seems like the smart thing to do, certainly - but you'd still need a way to specify which of those capabilities you want to use for each transfer. I'd suggest that hardware would ignore bits that it couldn't support - in the limit, hardware that has no way to choose when to send a command during video would ignore this completely. I realise that could well cause confusion when trying to work out why a particularly display is misbehaving when you think you're sending commands at the right time. Bert.
On Mon, Nov 18, 2013 at 11:59:23AM +0000, Bert Kenward wrote: > On 11/18/2013 11:22, Thierry Reding wrote: > > On Thu, Nov 14, 2013 at 03:04:19PM +0000, Bert Kenward wrote: > > > #define DSI_WINDOW_VFP (1 << 0) > > > #define DSI_WINDOW_ACT (1 << 1) > > > #define DSI_WINDOW_VBP (1 << 2) > > > #define DSI_WINDOW_VSY (1 << 3) > > > > > > /** > > > * struct dsi_msg - DSI command message > > > * @channel: virtual channel to send the message to > > > * @type: data ID of the message > > > * @lp_mode: send in LP mode if non-zero > > > > Perhaps a flags field would be more flexible here. I can easily imagine > > other I can imagine that other flags may be needed eventually. > > Agreed. "TE synchronised" would be one such extra flag, for supporting command mode updates. > > > > * @window: video period when transfer is allowed - bitmask of > > DSI_WINDOW_* > > > > I'm not sure if this is the right interface. What will happen for > > instance if the hardware doesn't support any of the bits in that mask? > > Perhaps a slightly better approach might be to expose the capabilities > > of the DSI host, so that the DSI core knows up front which windows can > > be used. > > Exposing the capabilities seems like the smart thing to do, certainly > - but you'd still need a way to specify which of those capabilities > you want to use for each transfer. Yes. I think we'll still need to have that. It's just that for some transfers it doesn't matter during which window they are executed. Although I guess in those cases the caller could just specify all bits to signal that it doesn't care. > I'd suggest that hardware would ignore bits that it couldn't support - > in the limit, hardware that has no way to choose when to send a > command during video would ignore this completely. I realise that > could well cause confusion when trying to work out why a particularly > display is misbehaving when you think you're sending commands at the > right time. I think at the very least if there's no match between the requested set of windows and the ones that a particular DSI host supports, then the driver should report an error. The reason why I thought that exposing the capabilities might be useful is that the caller could be smart about how to transfer a message, or perhaps use different messages to the same effect. But perhaps that's not even possible and maybe not worth considering. A smart thing to do in my opinion will be to not try to overengineer this from the beginning. That's why I'm thinking of splitting out the whose dsi_msg support in a first step, so that the bus infrastructure can be merged without having discussed all possible cases. And even so dsi_msg doesn't have to be perfect from the start. It's an in-kernel API, therefore can change easily if needed. The less we require of it now the easier it will be to extend when the need arises. Thierry
On Thu, Nov 14, 2013 at 03:15:57PM +0100, Andrzej Hajda wrote: > Hi Thierry, > > On 11/13/2013 10:38 PM, Thierry Reding wrote: > > On Tue, Nov 12, 2013 at 03:14:22PM +0100, Andrzej Hajda wrote: > >> Hi Thierry, > >> > >> I have already sent patch with DSI bus implementation [1]. > >> It was posted as the first step of CDF implementation attempt, > >> but in fact it do not depend on CDF. > >> > >> [1] > >> http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg45252.html > > Seems like that patchset was never merged. I guess probably because it > > was posted as part of CDF work. > > > > Do you have any plans on continuing work on that?If not I could extract > > the DSI bus patch from the series, it's largely similar to the patch I > > proposed here, and rework it somewhat. > I will soon sent new patch with the current version of the bus. > It could be a better base to your rework. Any estimate on when this will be? I want to get started on this as soon as possible so we can get this in good shape for 3.14. I suspect that if I start based on the current patch, I won't have to redo everything when updating for the next version. Quite a bit should remain similar, right? > > I'd very much like to avoid > > putting the code in drivers/video, though, since that's considered > > obsolete. > I have followed convention proposed by Laurent in his DBI bus. > It seems to me OK - DSI bus is more related to video than to drm. > I know that drivers/video is mostly occupied by FB drivers, > but according to Kconfig it is not only for FB. > Of course this is only my suggestion. I've had an IRC discussion with Dave and Rob (added on Cc), and they both agreed that implementing it as part of DRM would be preferred. The reason, as I've said before, is primarily that drivers/video is considered obsolete and new drivers and features should be implemented within the context of DRM/KMS. Additionally this has the benefit of being able to reuse the native DRM data structures and types, as well as helper functions, without having to go through an extra layer of compatibility. > > Furthermore I think if we kept the transfer function proposed > > in my patch should make it easier to address Bert's comments from your > > posting. > I am not sure which part of Barts comment you are addressing. > Anyway I also prefer passing struct and returning ssize_t. > I am not sure about splitting type and channel but this seems to be a > minor detail. I find that to be a perfectly natural split. A DSI peripheral will have an associated virtual channel number anyway, and having a separate field will allow drivers to just copy that field into the dsi_msg's equivalent. The alternative would be to require each driver to properly encode the channel and data type. Thierry
On 11/18/2013 01:57 PM, Thierry Reding wrote: > On Thu, Nov 14, 2013 at 03:15:57PM +0100, Andrzej Hajda wrote: >> Hi Thierry, >> >> On 11/13/2013 10:38 PM, Thierry Reding wrote: >>> On Tue, Nov 12, 2013 at 03:14:22PM +0100, Andrzej Hajda wrote: >>>> Hi Thierry, >>>> >>>> I have already sent patch with DSI bus implementation [1]. >>>> It was posted as the first step of CDF implementation attempt, >>>> but in fact it do not depend on CDF. >>>> >>>> [1] >>>> http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg45252.html >>> Seems like that patchset was never merged. I guess probably because it >>> was posted as part of CDF work. >>> >>> Do you have any plans on continuing work on that?If not I could extract >>> the DSI bus patch from the series, it's largely similar to the patch I >>> proposed here, and rework it somewhat. >> I will soon sent new patch with the current version of the bus. >> It could be a better base to your rework. > Any estimate on when this will be? I want to get started on this as soon > as possible so we can get this in good shape for 3.14. I suspect that if > I start based on the current patch, I won't have to redo everything when > updating for the next version. Quite a bit should remain similar, right? Patch sent. > >>> I'd very much like to avoid >>> putting the code in drivers/video, though, since that's considered >>> obsolete. >> I have followed convention proposed by Laurent in his DBI bus. >> It seems to me OK - DSI bus is more related to video than to drm. >> I know that drivers/video is mostly occupied by FB drivers, >> but according to Kconfig it is not only for FB. >> Of course this is only my suggestion. > I've had an IRC discussion with Dave and Rob (added on Cc), and they > both agreed that implementing it as part of DRM would be preferred. > > The reason, as I've said before, is primarily that drivers/video is > considered obsolete and new drivers and features should be implemented > within the context of DRM/KMS. Additionally this has the benefit of > being able to reuse the native DRM data structures and types, as well as > helper functions, without having to go through an extra layer of > compatibility. > >>> Furthermore I think if we kept the transfer function proposed >>> in my patch should make it easier to address Bert's comments from your >>> posting. >> I am not sure which part of Barts comment you are addressing. >> Anyway I also prefer passing struct and returning ssize_t. >> I am not sure about splitting type and channel but this seems to be a >> minor detail. > I find that to be a perfectly natural split. A DSI peripheral will have > an associated virtual channel number anyway, and having a separate field > will allow drivers to just copy that field into the dsi_msg's > equivalent. > > The alternative would be to require each driver to properly encode the > channel and data type. Other alternatives: - use helpers functions to call transfer op, encoding could be done by them, - fill channel by DSI master, based on DSI slave(seems to be little over-engineering) But I see no big difference between them. Regards Andrzej > > Thierry
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index f86427591167..7faefcdd6854 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -20,6 +20,10 @@ menuconfig DRM details. You should also select and configure AGP (/dev/agpgart) support if it is available for your platform. +config DRM_DSI + bool + depends on DRM + config DRM_USB tristate depends on DRM diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index cc08b845f965..eef34abc1e45 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -19,6 +19,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o drm-$(CONFIG_PCI) += ati_pcigart.o +drm-dsi-y := drm_dsi.o drm-usb-y := drm_usb.o drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o @@ -31,6 +32,7 @@ obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o CFLAGS_drm_trace_points.o := -I$(src) obj-$(CONFIG_DRM) += drm.o +obj-$(CONFIG_DRM_DSI) += drm_dsi.o obj-$(CONFIG_DRM_USB) += drm_usb.o obj-$(CONFIG_DRM_TTM) += ttm/ obj-$(CONFIG_DRM_TDFX) += tdfx/ diff --git a/drivers/gpu/drm/drm_dsi.c b/drivers/gpu/drm/drm_dsi.c new file mode 100644 index 000000000000..bead3cc0e9e3 --- /dev/null +++ b/drivers/gpu/drm/drm_dsi.c @@ -0,0 +1,306 @@ +/* + * Copyright (C) 2013 NVIDIA Corporation + * + * Permission to use, copy, modify, distribute, and sell this software and its + * documentation for any purpose is hereby granted without fee, provided that + * the above copyright notice appear in all copies and that both that copyright + * notice and this permission notice appear in supporting documentation, and + * that the name of the copyright holders not be used in advertising or + * publicity pertaining to distribution of the software without specific, + * written prior permission. The copyright holders make no representations + * about the suitability of this software for any purpose. It is provided "as + * is" without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE, + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE + * OF THIS SOFTWARE. + */ + +#include <linux/device.h> +#include <linux/err.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/slab.h> + +#include <drm/drm_dsi.h> + +static int dsi_device_match(struct device *dev, struct device_driver *drv) +{ + if (of_driver_match_device(dev, drv)) + return 1; + + return 0; +} + +static struct bus_type dsi_bus_type = { + .name = "dsi", + .match = dsi_device_match, +}; + +static void dsi_device_release(struct device *dev) +{ + struct dsi_device *dsi = to_dsi_device(dev); + + of_node_put(dsi->dev.of_node); + dsi_host_put(dsi->host); + kfree(dsi); +} + +static struct dsi_device *dsi_device_alloc(struct dsi_host *host) +{ + struct dsi_device *dsi; + + if (!dsi_host_get(host)) + return ERR_PTR(-EINVAL); + + dsi = kzalloc(sizeof(*dsi), GFP_KERNEL); + if (!dsi) { + dsi_host_put(host); + return ERR_PTR(-ENOMEM); + } + + dsi->host = dsi_host_get(host); + + dsi->dev.parent = host->dev; + dsi->dev.bus = &dsi_bus_type; + dsi->dev.release = dsi_device_release; + + device_initialize(&dsi->dev); + + return dsi; +} + +static int dsi_device_add(struct dsi_device *dsi) +{ + struct dsi_host *host = dsi->host; + int err; + + dev_set_name(&dsi->dev, "%s.%u", dev_name(host->dev), dsi->channel); + + err = device_add(&dsi->dev); + if (err < 0) { + dsi_device_put(dsi); + return err; + } + + return 0; +} + +static int of_dsi_host_register(struct dsi_host *host) +{ + if (!host->dev->of_node) + return -ENODEV; + + return 0; +} + +static int of_dsi_register_devices(struct dsi_host *host) +{ + struct device_node *np; + + if (!host->dev->of_node) + return -ENODEV; + + for_each_available_child_of_node(host->dev->of_node, np) { + struct dsi_device *dsi; + u32 value; + int err; + + dsi = dsi_device_alloc(host); + if (IS_ERR(dsi)) { + dev_err(host->dev, + "failed to allocate DSI device for %s: %ld\n", + np->full_name, PTR_ERR(dsi)); + continue; + } + + dsi->dev.of_node = of_node_get(np); + + err = of_property_read_u32(np, "reg", &value); + if (err) { + dev_err(host->dev, + "device %s has no valid 'reg' property: %d\n", + np->full_name, err); + dsi_device_put(dsi); + continue; + } + + if (value > 3) { + dev_err(host->dev, + "device %s has invalid virtual channel %u\n", + np->full_name, value); + dsi_device_put(dsi); + continue; + } + + dsi->channel = value; + + err = dsi_device_add(dsi); + if (err < 0) { + dev_err(host->dev, + "failed to add DSI device for %s: %d\n", + np->full_name, err); + dsi_device_put(dsi); + continue; + } + } + + return 0; +} + +int dsi_host_register(struct dsi_host *host) +{ + int err; + + if (IS_ENABLED(CONFIG_OF)) { + err = of_dsi_host_register(host); + if (err < 0) + return err; + } + + if (IS_ENABLED(CONFIG_OF)) { + err = of_dsi_register_devices(host); + if (err < 0) + return err; + } + + return 0; +} +EXPORT_SYMBOL(dsi_host_register); + +static int __dsi_device_unregister(struct device *dev, void *data) +{ + device_unregister(dev); + return 0; +} + +/** + * dsi_host_unregister() - unregister a DSI host controller + * @host: a DSI host controller + * + * Returns 0 on success or a negative error-code on failure. + */ +int dsi_host_unregister(struct dsi_host *host) +{ + device_for_each_child(host->dev, NULL, __dsi_device_unregister); + + return 0; +} +EXPORT_SYMBOL(dsi_host_unregister); + +/** + * dsi_host_transfer() - transfer a DSI message between host and peripheral + * @host: DSI host + * @msg: DSI message to transfer + * + * Returns 0 on success or a negative error-code on failure. + */ +ssize_t dsi_host_transfer(struct dsi_host *host, struct dsi_msg *msg) +{ + if (host->ops && host->ops->transfer) + return host->ops->transfer(host, msg); + + return -ENOSYS; +} +EXPORT_SYMBOL(dsi_host_transfer); + +/** + * dsi_device_attach() - attach a DSI peripheral to its DSI host + * @device: DSI peripheral + * + * Returns 0 on success or a negative error-code on failure. + */ +int dsi_device_attach(struct dsi_device *device) +{ + if (device->host->ops && device->host->ops->attach) + return device->host->ops->attach(device->host, device); + + return -ENOSYS; +} +EXPORT_SYMBOL(dsi_device_attach); + +/** + * dsi_device_detach() - detach a DSI peripheral from its DSI host + * @device: DSI peripheral + * + * Returns 0 on success or a negative error-code on failure. + */ +int dsi_device_detach(struct dsi_device *device) +{ + if (device->host->ops && device->host->ops->detach) + return device->host->ops->detach(device->host, device); + + return -ENOSYS; +} +EXPORT_SYMBOL(dsi_device_detach); + +static int dsi_driver_probe(struct device *dev) +{ + const struct dsi_driver *drv = to_dsi_driver(dev->driver); + struct dsi_device *dsi = to_dsi_device(dev); + + return drv->probe(dsi); +} + +static int dsi_driver_remove(struct device *dev) +{ + const struct dsi_driver *drv = to_dsi_driver(dev->driver); + struct dsi_device *dsi = to_dsi_device(dev); + + return drv->remove(dsi); +} + +static void dsi_driver_shutdown(struct device *dev) +{ + const struct dsi_driver *drv = to_dsi_driver(dev->driver); + struct dsi_device *dsi = to_dsi_device(dev); + + drv->shutdown(dsi); +} + +/** + * dsi_register_driver() - register a DSI driver + * @drv: DSI driver + * + * Returns 0 on success or a negative error-code on failure. + */ +int dsi_register_driver(struct dsi_driver *drv) +{ + drv->driver.bus = &dsi_bus_type; + + if (drv->probe) + drv->driver.probe = dsi_driver_probe; + + if (drv->remove) + drv->driver.remove = dsi_driver_remove; + + if (drv->shutdown) + drv->driver.shutdown = dsi_driver_shutdown; + + return driver_register(&drv->driver); +} +EXPORT_SYMBOL(dsi_register_driver); + +/** + * dsi_unregister_driver() - unregister a DSI driver + * @drv: DSI driver + */ +void dsi_unregister_driver(struct dsi_driver *drv) +{ + driver_unregister(&drv->driver); +} +EXPORT_SYMBOL(dsi_unregister_driver); + +static int __init dsi_init(void) +{ + return bus_register(&dsi_bus_type); +} +postcore_initcall(dsi_init); + +MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>"); +MODULE_DESCRIPTION("DRM DSI infrastructure"); +MODULE_LICENSE("GPL and additional rights"); diff --git a/include/drm/drm_dsi.h b/include/drm/drm_dsi.h new file mode 100644 index 000000000000..0886160b9aa2 --- /dev/null +++ b/include/drm/drm_dsi.h @@ -0,0 +1,206 @@ +/* + * Copyright (C) 2013 NVIDIA Corporation + * + * Permission to use, copy, modify, distribute, and sell this software and its + * documentation for any purpose is hereby granted without fee, provided that + * the above copyright notice appear in all copies and that both that copyright + * notice and this permission notice appear in supporting documentation, and + * that the name of the copyright holders not be used in advertising or + * publicity pertaining to distribution of the software without specific, + * written prior permission. The copyright holders make no representations + * about the suitability of this software for any purpose. It is provided "as + * is" without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE, + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE + * OF THIS SOFTWARE. + */ + +#ifndef _DRM_DSI_H_ +#define _DRM_DSI_H_ + +#include <linux/types.h> + +struct dsi_device; +struct dsi_host; + +/* + * DSI packet data types + */ + +/* processor-sourced packets */ +#define DSI_CMD_VSYNC_START 0x01 +#define DSI_CMD_VSYNC_END 0x11 +#define DSI_CMD_HSYNC_START 0x21 +#define DSI_CMD_HSYNC_END 0x31 +#define DSI_CMD_EOT 0x08 +#define DSI_CMD_COLOR_MODE_OFF 0x02 +#define DSI_CMD_COLOR_MODE_ON 0x12 +#define DSI_CMD_SHUT_DOWN 0x22 +#define DSI_CMD_TURN_ON 0x32 +#define DSI_CMD_GEN_SHORT_WRITE_0 0x03 +#define DSI_CMD_GEN_SHORT_WRITE_1 0x13 +#define DSI_CMD_GEN_SHORT_WRITE_2 0x23 +#define DSI_CMD_GEN_SHORT_READ_0 0x04 +#define DSI_CMD_GEN_SHORT_READ_1 0x14 +#define DSI_CMD_GEN_SHORT_READ_2 0x24 +#define DSI_CMD_DCS_SHORT_WRITE_0 0x05 +#define DSI_CMD_DCS_SHORT_WRITE_1 0x15 +#define DSI_CMD_DCS_SHORT_READ 0x06 +#define DSI_CMD_SET_MAX_RETURN_PACKET_SIZE 0x37 +#define DSI_CMD_NULL 0x09 +#define DSI_CMD_BLANK 0x19 +#define DSI_CMD_GEN_LONG_WRITE 0x29 +#define DSI_CMD_DCS_LONG_WRITE 0x39 +#define DSI_CMD_YCbCr422_20 0x0c +#define DSI_CMD_YCbCr422_24 0x1c +#define DSI_CMD_YCbCr422_16 0x2c +#define DSI_CMD_RGB30 0x0d +#define DSI_CMD_RGB36 0x1d +#define DSI_CMD_YCbCr420 0x3d +#define DSI_CMD_RGB16 0x0e +#define DSI_CMD_RGB18 0x1e +#define DSI_CMD_RGB18NP 0x2e +#define DSI_CMD_RGB24 0x3e + +/* peripheral-sourced */ +#define DSI_RSP_ACK_ERR 0x02 +#define DSI_RSP_EOT 0x08 +#define DSI_RSP_GEN_SHORT_READ_1 0x11 +#define DSI_RSP_GEN_SHORT_READ_2 0x12 +#define DSI_RSP_GEN_LONG_READ 0x1a +#define DSI_RSP_DCS_LONG_READ 0x1c +#define DSI_RSP_DCS_SHORT_READ_1 0x21 +#define DSI_RSP_DCS_SHORT_READ_2 0x22 + +#define DSI_ACK 0x84 +#define DSI_ESC 0x87 + +/** + * struct dsi_msg - DSI command message + * @channel: virtual channel to send the message to + * @type: data ID of the message + * @tx_len: length of transmission buffer + * @tx: transmission buffer + * @rx_len: length of reception buffer + * @rx: reception buffer + */ +struct dsi_msg { + u8 channel; + u8 type; + + size_t tx_len; + void *tx; + + size_t rx_len; + void *rx; +}; + +/** + * struct dsi_host_ops - DSI host operations + * @attach: called when a peripheral is attached to the host + * @detach: called when a peripheral is detached from the host + * @transfer: transfer a DSI command message to a peripheral + */ +struct dsi_host_ops { + int (*attach)(struct dsi_host *host, struct dsi_device *device); + int (*detach)(struct dsi_host *host, struct dsi_device *device); + ssize_t (*transfer)(struct dsi_host *host, struct dsi_msg *msg); +}; + +/** + * struct dsi_host - DSI host + * @dev: device providing the DSI host functionality + * @ops: pointer to DSI host operations + */ +struct dsi_host { + struct device *dev; + + const struct dsi_host_ops *ops; +}; + +static inline struct dsi_host *dsi_host_get(struct dsi_host *host) +{ + if (!host || !get_device(host->dev)) + return NULL; + + return host; +} + +static inline void dsi_host_put(struct dsi_host *host) +{ + if (host) + put_device(host->dev); +} + +int dsi_host_register(struct dsi_host *host); +int dsi_host_unregister(struct dsi_host *host); + +ssize_t dsi_host_transfer(struct dsi_host *host, struct dsi_msg *msg); + +/** + * struct dsi_device - DSI peripheral + * @host: DSI host that this peripheral is attached to + * @dev: device to tie the peripheral into the device tree + * @channel: virtual channel of the peripheral + */ +struct dsi_device { + struct dsi_host *host; + struct device dev; + + unsigned int channel; +}; + +static inline struct dsi_device *to_dsi_device(struct device *dev) +{ + return dev ? container_of(dev, struct dsi_device, dev) : NULL; +} + +static inline struct dsi_device *dsi_device_get(struct dsi_device *dsi) +{ + if (!dsi || !get_device(&dsi->dev)) + return NULL; + + return dsi; +} + +static inline void dsi_device_put(struct dsi_device *dsi) +{ + if (dsi) + put_device(&dsi->dev); +} + +int dsi_device_attach(struct dsi_device *device); +int dsi_device_detach(struct dsi_device *device); + +/** + * struct dsi_driver - DSI driver + * @driver: device driver model driver + * @probe: callback for device binding + * @remove: callback for device unbinding + * @shutdown: callback for device shutdown + */ +struct dsi_driver { + struct device_driver driver; + int (*probe)(struct dsi_device *dsi); + int (*remove)(struct dsi_device *dsi); + void (*shutdown)(struct dsi_device *dsi); +}; + +static inline struct dsi_driver *to_dsi_driver(struct device_driver *drv) +{ + return drv ? container_of(drv, struct dsi_driver, driver) : NULL; +} + +int dsi_register_driver(struct dsi_driver *drv); +void dsi_unregister_driver(struct dsi_driver *drv); + +#define module_dsi_driver(__dsi_driver) \ + module_driver(__dsi_driver, dsi_register_driver, \ + dsi_unregister_driver) + +#endif
In order to support DSI peripherals, add a DSI bus type that devices and drivers can be registered with. Signed-off-by: Thierry Reding <treding@nvidia.com> --- drivers/gpu/drm/Kconfig | 4 + drivers/gpu/drm/Makefile | 2 + drivers/gpu/drm/drm_dsi.c | 306 ++++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_dsi.h | 206 +++++++++++++++++++++++++++++++ 4 files changed, 518 insertions(+) create mode 100644 drivers/gpu/drm/drm_dsi.c create mode 100644 include/drm/drm_dsi.h