| Message ID | 20200519201110.286501-1-paul@crapouillou.net (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | clocksource: Ingenic: Add high resolution timer support for SMP. | expand |
On Tuesday 19. May 2020 22.11.10 Paul Cercueil wrote: > > I took the liberty to clean your patch so that it doesn't create a > struct ingenic_tcu per CPU timer. > > Tested, and fully working on the JZ4770 with CONFIG_SMP disabled, and > also with CONFIG_SMP enabled (even though JZ4770 has only one CPU) with > a fixed smp.c and USB disabled (otherwise it crashes at boot). Thanks for looking at this and also your continuing work with the DRM driver. I'll try and look a bit more at the DRM driver for the JZ4780 and see what I am doing wrong with regard to JZ4780-specific configuration. In principle, there should not be any, but I don't really have the full picture (literally and also in terms of documentation or understanding). One thing that seems to be done with the Imagination/Ingenic drivers is dual DMA channel initialisation. Usually, this seems to be necessary only for dual- panel configurations, and I wonder whether there is some kind of requirement for the same arrangement when using the HDMI output. Another thing that is done involves setting a foreground, which might be relevant to your more recent contributions adding IPU support. Again, I don't really understand why this seems necessary, but the documentation hardly provides any details that might explain it. Paul
Hi Paul, I think the ingenic-drm driver is fine, even though the old 3.8 kernel worked differently, the IP is backwards-compatible so it should work no problem. I think the problem is somewhere in the Synopsis HDMI code or the glue code. Because the LCDC does seem to send data, which is not encoded properly by the HDMI chip. -Paul Le jeu. 21 mai 2020 à 0:14, Paul Boddie <paul@boddie.org.uk> a écrit : > On Tuesday 19. May 2020 22.11.10 Paul Cercueil wrote: >> >> I took the liberty to clean your patch so that it doesn't create a >> struct ingenic_tcu per CPU timer. >> >> Tested, and fully working on the JZ4770 with CONFIG_SMP disabled, >> and >> also with CONFIG_SMP enabled (even though JZ4770 has only one CPU) >> with >> a fixed smp.c and USB disabled (otherwise it crashes at boot). > > Thanks for looking at this and also your continuing work with the DRM > driver. > I'll try and look a bit more at the DRM driver for the JZ4780 and see > what I > am doing wrong with regard to JZ4780-specific configuration. In > principle, > there should not be any, but I don't really have the full picture > (literally > and also in terms of documentation or understanding). > > One thing that seems to be done with the Imagination/Ingenic drivers > is dual > DMA channel initialisation. Usually, this seems to be necessary only > for dual- > panel configurations, and I wonder whether there is some kind of > requirement > for the same arrangement when using the HDMI output. > > Another thing that is done involves setting a foreground, which might > be > relevant to your more recent contributions adding IPU support. Again, > I don't > really understand why this seems necessary, but the documentation > hardly > provides any details that might explain it. > > Paul
On Friday 22. May 2020 14.26.15 Paul Cercueil wrote: > Hi Paul, > > I think the ingenic-drm driver is fine, even though the old 3.8 kernel > worked differently, the IP is backwards-compatible so it should work no > problem. I think the problem is somewhere in the Synopsis HDMI code or > the glue code. Because the LCDC does seem to send data, which is not > encoded properly by the HDMI chip. There was one interesting insight related to vertical blank interrupts, where it would appear that the end-of-frame condition does not occur, with this failure then obstructing driver initialisation. I aim to look into that further. Indeed, to approach this from another angle, I started to experiment with the HDMI peripheral in the L4 Runtime Environment, which has been helpful when investigating things like the RTC chip on the CI20 and its interaction with the power management system. I have previously demonstrated the LCD controller initialisation in this environment for other Ingenic devices, so I should have a chance of building on that success without too many added complications. So far, I have managed to reproduce EDID retrieval using the HDMI peripheral's own I2C support, and I plan to reproduce the HDMI peripheral initialisation itself. However, it is perhaps more interesting to get the LCD controller working first and potentially delivering end-of-frame interrupts: this might help me understand whether this problem is a serious obstacle or not. Paul
On Friday 22. May 2020 21.16.10 Paul Boddie wrote: > On Friday 22. May 2020 14.26.15 Paul Cercueil wrote: > > Hi Paul, > > > > I think the ingenic-drm driver is fine, even though the old 3.8 kernel > > worked differently, the IP is backwards-compatible so it should work no > > problem. I think the problem is somewhere in the Synopsis HDMI code or > > the glue code. Because the LCDC does seem to send data, which is not > > encoded properly by the HDMI chip. > > There was one interesting insight related to vertical blank interrupts, > where it would appear that the end-of-frame condition does not occur, with > this failure then obstructing driver initialisation. I aim to look into > that further. Some further experiments indicate that interrupt generation is indeed a problem... [L4Re experimentation] > So far, I have managed to reproduce EDID retrieval using the HDMI > peripheral's own I2C support, and I plan to reproduce the HDMI peripheral > initialisation itself. However, it is perhaps more interesting to get the > LCD controller working first and potentially delivering end-of-frame > interrupts: this might help me understand whether this problem is a serious > obstacle or not. First of all, I managed to get the HDMI connector hotplug detection working. This was a relatively simple matter of setting the appropriate flags, binding to an interrupt and then waiting for one to arrive. Consequently, booting without the connector inserted means that my program is halted until the interrupt arrives upon insertion; then, the EDID is read, which seems to work reliably. However, I then found that the LCD controller could not be activated. The solution to this involves the TVE clock on the JZ4780 which appears to be necessary in addition to the LCD clock. Ingenic documentation being what it is, I suspect that the LCD clock in the block diagram is really the pixel clock(s), with the TVE clock not even appearing in the diagram. The 3.18 kernel's device tree for the JZ4780 plus the CGU code provide the necessary hints, without any explanation, of course. With the LCD controller now willing to retain values stored in its registers, I have been attempting to set up descriptors and do the usual general configuration exercise that works on the JZ4720 and JZ4730. However, I have never enabled interrupts on those devices, so I don't know what I need to do other than to set the appropriate control and command (descriptor) flags. Doing so doesn't manage to generate any interrupts, though. The 3.18 kernel driver sets up the "new" 8-word descriptor and other new things. Initially, I ignored these things, but then I thought that they might actually be mandatory. Still, introducing practically the same details as seen in the 3.18 driver seems to have no effect. So, I imagine I am missing something pretty obvious: it took me a while to realise that I wasn't even enabling the LCD controller, after all. One thing I would point out is that the operation of the JZ4780 is not exactly the same as earlier products. For example, various configuration register bits related to pixel depths are now read-only. Presumably, the idea is to set the pixel depth in the "new" descriptor fields instead, enabling some kind of mixing of different kinds of pixel data. I also wonder how well supported some of the older functions are in the newer hardware. Anyway, I'm rather stuck with this at the moment. I don't know whether I should be reproducing the HDMI initialisation in my test environment under the assumption that the LCD controller isn't sending data without some kind of output path, or whether I should fake up some other output path (maybe a serial LCD) by setting GPIO alternate pin functions. But it turns out not to be entirely trivial to just have the LCD controller do its own thing and generate these interrupts all by itself. But again, I may well be overlooking an obvious mistake of my own. Paul
Hi Paul, > Am 26.05.2020 um 01:03 schrieb Paul Boddie <paul@boddie.org.uk>: > > On Friday 22. May 2020 21.16.10 Paul Boddie wrote: >> On Friday 22. May 2020 14.26.15 Paul Cercueil wrote: >>> Hi Paul, >>> >>> I think the ingenic-drm driver is fine, even though the old 3.8 kernel >>> worked differently, the IP is backwards-compatible so it should work no >>> problem. I think the problem is somewhere in the Synopsis HDMI code or >>> the glue code. Because the LCDC does seem to send data, which is not >>> encoded properly by the HDMI chip. >> >> There was one interesting insight related to vertical blank interrupts, >> where it would appear that the end-of-frame condition does not occur, with >> this failure then obstructing driver initialisation. I aim to look into >> that further. > > Some further experiments indicate that interrupt generation is indeed a > problem... > > [L4Re experimentation] > >> So far, I have managed to reproduce EDID retrieval using the HDMI >> peripheral's own I2C support, and I plan to reproduce the HDMI peripheral >> initialisation itself. However, it is perhaps more interesting to get the >> LCD controller working first and potentially delivering end-of-frame >> interrupts: this might help me understand whether this problem is a serious >> obstacle or not. > > First of all, I managed to get the HDMI connector hotplug detection working. > This was a relatively simple matter of setting the appropriate flags, binding > to an interrupt and then waiting for one to arrive. Consequently, booting > without the connector inserted means that my program is halted until the > interrupt arrives upon insertion; then, the EDID is read, which seems to work > reliably. Well, with Linux my hack to set dev->mode_config.poll_enabled = true; in drm_probe_helper.c seems to make it works in all cases without halting. And what I observed is that interrupts are generated and arriving. They are just not properly delivered because dev->mode_config.poll_enabled = false has some wrong side-effect. So I think there must be a simpler solution (for Linux). > > However, I then found that the LCD controller could not be activated. In my tests the code to setup the LCDC was never called so I could never debug anything there. I still have to digest the mail "DRM interaction problems on Ingenic CI20 / jz4780 with dw-hdmi and ingenic-drm" from Paul. > The > solution to this involves the TVE clock on the JZ4780 which appears to be > necessary in addition to the LCD clock. Ingenic documentation being what it > is, I suspect that the LCD clock in the block diagram is really the pixel > clock(s), with the TVE clock not even appearing in the diagram. The 3.18 > kernel's device tree for the JZ4780 plus the CGU code provide the necessary > hints, without any explanation, of course. > > With the LCD controller now willing to retain values stored in its registers, > I have been attempting to set up descriptors and do the usual general > configuration exercise that works on the JZ4720 and JZ4730. However, I have > never enabled interrupts on those devices, so I don't know what I need to do > other than to set the appropriate control and command (descriptor) flags. > Doing so doesn't manage to generate any interrupts, though. > > The 3.18 kernel driver sets up the "new" 8-word descriptor and other new > things. Initially, I ignored these things, but then I thought that they might > actually be mandatory. Still, introducing practically the same details as seen > in the 3.18 driver seems to have no effect. So, I imagine I am missing > something pretty obvious: it took me a while to realise that I wasn't even > enabling the LCD controller, after all. > > One thing I would point out is that the operation of the JZ4780 is not exactly > the same as earlier products. For example, various configuration register bits > related to pixel depths are now read-only. And what I observed is that there are additional registers compared to a JZ4740 LCDC. My assumption would be that leaving them 0x00 would be ok. > Presumably, the idea is to set the > pixel depth in the "new" descriptor fields instead, enabling some kind of > mixing of different kinds of pixel data. I also wonder how well supported some > of the older functions are in the newer hardware. > > Anyway, I'm rather stuck with this at the moment. I don't know whether I > should be reproducing the HDMI initialisation in my test environment under the > assumption that the LCD controller isn't sending data without some kind of > output path, or whether I should fake up some other output path (maybe a > serial LCD) by setting GPIO alternate pin functions. But it turns out not to > be entirely trivial to just have the LCD controller do its own thing and > generate these interrupts all by itself. > > But again, I may well be overlooking an obvious mistake of my own. > > Paul BR, Nikolaus
Hi Paul, Le mar. 26 mai 2020 à 1:03, Paul Boddie <paul@boddie.org.uk> a écrit : > On Friday 22. May 2020 21.16.10 Paul Boddie wrote: >> On Friday 22. May 2020 14.26.15 Paul Cercueil wrote: >> > Hi Paul, >> > >> > I think the ingenic-drm driver is fine, even though the old 3.8 >> kernel >> > worked differently, the IP is backwards-compatible so it should >> work no >> > problem. I think the problem is somewhere in the Synopsis HDMI >> code or >> > the glue code. Because the LCDC does seem to send data, which is >> not >> > encoded properly by the HDMI chip. >> >> There was one interesting insight related to vertical blank >> interrupts, >> where it would appear that the end-of-frame condition does not >> occur, with >> this failure then obstructing driver initialisation. I aim to look >> into >> that further. > > Some further experiments indicate that interrupt generation is indeed > a > problem... > > [L4Re experimentation] > >> So far, I have managed to reproduce EDID retrieval using the HDMI >> peripheral's own I2C support, and I plan to reproduce the HDMI >> peripheral >> initialisation itself. However, it is perhaps more interesting to >> get the >> LCD controller working first and potentially delivering end-of-frame >> interrupts: this might help me understand whether this problem is a >> serious >> obstacle or not. > > First of all, I managed to get the HDMI connector hotplug detection > working. > This was a relatively simple matter of setting the appropriate flags, > binding > to an interrupt and then waiting for one to arrive. Consequently, > booting > without the connector inserted means that my program is halted until > the > interrupt arrives upon insertion; then, the EDID is read, which seems > to work > reliably. > > However, I then found that the LCD controller could not be activated. > The > solution to this involves the TVE clock on the JZ4780 which appears > to be > necessary in addition to the LCD clock. Ingenic documentation being > what it > is, I suspect that the LCD clock in the block diagram is really the > pixel > clock(s), with the TVE clock not even appearing in the diagram. The > 3.18 > kernel's device tree for the JZ4780 plus the CGU code provide the > necessary > hints, without any explanation, of course. "lcd0pixclk" and "tve" are for LCD0, "lcd1pixclk" and "lcd" are for LCD1. > With the LCD controller now willing to retain values stored in its > registers, > I have been attempting to set up descriptors and do the usual general > configuration exercise that works on the JZ4720 and JZ4730. However, > I have > never enabled interrupts on those devices, so I don't know what I > need to do > other than to set the appropriate control and command (descriptor) > flags. > Doing so doesn't manage to generate any interrupts, though. > > The 3.18 kernel driver sets up the "new" 8-word descriptor and other > new > things. Initially, I ignored these things, but then I thought that > they might > actually be mandatory. Still, introducing practically the same > details as seen > in the 3.18 driver seems to have no effect. So, I imagine I am missing > something pretty obvious: it took me a while to realise that I wasn't > even > enabling the LCD controller, after all. > > One thing I would point out is that the operation of the JZ4780 is > not exactly > the same as earlier products. For example, various configuration > register bits > related to pixel depths are now read-only. Presumably, the idea is to > set the > pixel depth in the "new" descriptor fields instead, enabling some > kind of > mixing of different kinds of pixel data. I also wonder how well > supported some > of the older functions are in the newer hardware. OK, indeed the BPP and OSD config is read-only, and it's not a doc typo. How annoying. I tried to configure the LCD controller for a 8-byte descriptor without much success. No IRQs here either. -Paul > Anyway, I'm rather stuck with this at the moment. I don't know > whether I > should be reproducing the HDMI initialisation in my test environment > under the > assumption that the LCD controller isn't sending data without some > kind of > output path, or whether I should fake up some other output path > (maybe a > serial LCD) by setting GPIO alternate pin functions. But it turns out > not to > be entirely trivial to just have the LCD controller do its own thing > and > generate these interrupts all by itself. > > But again, I may well be overlooking an obvious mistake of my own. > > Paul
Paul, Thanks for the reply! On Tuesday 26. May 2020 17.03.04 Paul Cercueil wrote: > > "lcd0pixclk" and "tve" are for LCD0, "lcd1pixclk" and "lcd" are for > LCD1. The 3.0.8 kernel actually uses LCD0 for what the documentation and 3.18 kernel call TVE, and it uses LCD1 for what the others call LCD. That earlier kernel indicates that LCD1 is the parent clock of LCD0. I actually found that you can enable LCD0 and not LCD1 and the LCD controller (LCDC0) still operates to an extent, but without LCD1 enabled I didn't see a DMA command value in the appropriate register, discussed below. [...] > OK, indeed the BPP and OSD config is read-only, and it's not a doc > typo. How annoying. > > I tried to configure the LCD controller for a 8-byte descriptor without > much success. No IRQs here either. I had a look at the interrupt controller registers to see whether I was missing anything obvious, but the mask was correctly configured to unmask LCD interrupts (bit 31 of ICMR0). I did wonder whether the PDMA interrupts might need unmasking, just in case there is some interaction between the peripherals and that part of the hardware, but unmasking LCD interrupts there (bit 31 of DMR0) didn't make any difference. One observation I can make is that the length or size field of the LCD command register (LCDCMD0) does get initialised to the appropriate value as set in a descriptor. Since I don't set this register explicitly myself (unlike, I think, the current Ingenic DRM driver in the Linux kernel), the value must have been set up appropriately by a DMA transfer, as configured using the descriptor address register (LCDDA0). However, the command flags I also set in the descriptor are not reflected in the register. So, 0x44140000 becomes 0x00140000. I thought I should check the interrupt ID register (LCDIID) to see what it reveals. Despite setting a value in the appropriate descriptor field, the register contains only zero. I think I must probably tackle the job of initialising the HDMI controller to see if that makes a difference. If the interrupts are not working but are also not necessary, then maybe I get a visual indication of success. Thanks for the feedback! Paul
Hi, Le mer. 27 mai 2020 à 0:44, Paul Boddie <paul@boddie.org.uk> a écrit : > Paul, > > Thanks for the reply! > > On Tuesday 26. May 2020 17.03.04 Paul Cercueil wrote: >> >> "lcd0pixclk" and "tve" are for LCD0, "lcd1pixclk" and "lcd" are for >> LCD1. > > The 3.0.8 kernel actually uses LCD0 for what the documentation and > 3.18 kernel > call TVE, and it uses LCD1 for what the others call LCD. That earlier > kernel > indicates that LCD1 is the parent clock of LCD0. > > I actually found that you can enable LCD0 and not LCD1 and the LCD > controller > (LCDC0) still operates to an extent, but without LCD1 enabled I > didn't see a > DMA command value in the appropriate register, discussed below. Yes, I was preparing a patch for the clock driver, then I noticed that too. > [...] > >> OK, indeed the BPP and OSD config is read-only, and it's not a doc >> typo. How annoying. >> >> I tried to configure the LCD controller for a 8-byte descriptor >> without >> much success. No IRQs here either. > > I had a look at the interrupt controller registers to see whether I > was > missing anything obvious, but the mask was correctly configured to > unmask LCD > interrupts (bit 31 of ICMR0). I did wonder whether the PDMA > interrupts might > need unmasking, just in case there is some interaction between the > peripherals > and that part of the hardware, but unmasking LCD interrupts there > (bit 31 of > DMR0) didn't make any difference. > > One observation I can make is that the length or size field of the > LCD command > register (LCDCMD0) does get initialised to the appropriate value as > set in a > descriptor. Since I don't set this register explicitly myself > (unlike, I > think, the current Ingenic DRM driver in the Linux kernel), the value > must > have been set up appropriately by a DMA transfer, as configured using > the > descriptor address register (LCDDA0). However, the command flags I > also set in > the descriptor are not reflected in the register. So, 0x44140000 > becomes > 0x00140000. > > I thought I should check the interrupt ID register (LCDIID) to see > what it > reveals. Despite setting a value in the appropriate descriptor field, > the > register contains only zero. > > I think I must probably tackle the job of initialising the HDMI > controller to > see if that makes a difference. If the interrupts are not working but > are also > not necessary, then maybe I get a visual indication of success. Don't focus too much on interrupts right now. You don't get interrupts because the data is not flowing. Which in turns is caused either by the LCDC not being correctly configured, or the HDMI not sending hsync/vsync signals. For now, what seems to be the problem is that the DMA descriptors aren't loaded properly. Whatever I do, the debug registers (LCDSAx/LCDIDx/etc) are still zero here. -Paul
On Wednesday 27. May 2020 01.07.41 Paul Cercueil wrote: > > Don't focus too much on interrupts right now. You don't get interrupts > because the data is not flowing. Which in turns is caused either by the > LCDC not being correctly configured, or the HDMI not sending > hsync/vsync signals. > > For now, what seems to be the problem is that the DMA descriptors > aren't loaded properly. Whatever I do, the debug registers > (LCDSAx/LCDIDx/etc) are still zero here. I checked the LCDSA0 (source address) and LCDFID0 (frame identifier) registers, and they get populated with what I put in the descriptor, so I am inclined to think that some kind of initialisation does happen. However, as previously noted, the LCDIID (interrupt identifier) remains zero. What I have subsequently done is to introduce the HDMI driver functionality from Linux into my test environment (L4Re) to investigate the configuration process. Eventually, I managed to get the HDMI signal enabled, meaning that I can switch my monitor to the DVI input and be told what kind of picture it thinks it is getting (1280x1024 @ 62Hz, and although I asked for 60Hz, the 111MHz pixel clock will probably generate something closer to 62Hz). Where this gets interesting/infuriating is that the signal persists but there is no actual picture (just a black screen, but illuminated), but accompanying this is a "FIFO 0 under run" condition (IFU0 in LCDSTATE). Looking at the manual, there is no other mention of this "FIFO 0", but I did wonder whether it had anything to do with the OSD functionality because it seems to be the case that the OSD is always enabled (OSDEN is set in LCDOSDC) and cannot be unset just by clearing the OSDEN bit. I thought that the "new" 8-word descriptor arrangement might be important in this regard. Although the manual doesn't make this explicit, it seems to be the case that the extra 4 words are configuring the OSD foreground planes. So, I enabled the populated extra words in a similar way to what the 3.18 driver does. But even with various other JZ4780-specific registers set up (RGB control, priority threshold), what actually happens is that the monitor fails to acknowledge a signal and switches back to its default VGA input. I have tried a few things, such as enabling the IPU clock in the CPM unit (which is probably unnecessary since it is enabled by default) and enabling the IPU clock in the LCD controller (IPU_CLKEN in LCDOSDCTRL), but neither changes the situation. I also noted a recent patch on the dri-devel list adding OSD support... "[PATCH 09/12] gpu/drm: Ingenic: Add support for OSD mode" What is interesting about that patch is that it attempts to enable the OSD foreground planes by setting F0EN and F1EN on LCDOSDC. However, these are marked as read-only in the JZ4780 and cannot actually be set. So, I am currently looking for some more ideas about how to get this working. It is entirely possible that I have taken one too many shortcuts with the HDMI initialisation: I have only implemented "DVI mode" and have sought only to implement what is necessary for that and for RGB data. It is also likely that I have missed something from the LCD initialisation, but whatever it is escapes me at present. Any suggestions would be appreciated at this point! Thanks in advance, Paul P.S. I can easily imagine that the obstacle to getting things working in a modern Linux kernel is just the data format/encoding. The 3.18 driver supports only RGB data, which is presumably converted by the HDMI peripheral to whatever the preferred output actually is.
On Wednesday, 27 May 2020 01:07:41 CEST Paul Cercueil wrote: > > Don't focus too much on interrupts right now. You don't get interrupts > because the data is not flowing. Which in turns is caused either by the > LCDC not being correctly configured, or the HDMI not sending > hsync/vsync signals. > > For now, what seems to be the problem is that the DMA descriptors > aren't loaded properly. Whatever I do, the debug registers > (LCDSAx/LCDIDx/etc) are still zero here. So, as I may have mentioned already, I got the LCD controller and HDMI peripheral working in a simple L4Re-based configuration, verifying a working configuration of the LCD controller. Since then, I have been trying to build working Linux drivers with this knowledge. At the moment, I can indeed get the LCDC to start and to produce end-of-frame interrupts which are counted by the DRM framework. However, my current problem to solve is how to get anything other than "Input not supported" from my monitor. I have pursuing the laborious process of verifying the HDMI register settings, which is rather perverse given that my L4Re code effectively takes the simplified logic of the HDMI driver (plus my JZ4780 specialisations) and works fine. So, I am trying to reproduce the configuration of something that should already work from my clone of that same thing. One thing that was in dispute before was the matter of the input bus formats. It is definitely the case that the Ingenic driver will never see any format information based on what the Synopsys driver does. I don't know whether the DRM framework is supposed to magically propagate the information, but as of right now it just doesn't. Currently, I hack the display information's input bus format in the bridge driver's atomic_check function along with bus flags that I also think are needed. Setting the format enables the Ingenic DRM driver to start up. For some reason, the signal over the cable cannot be handled by the monitor. Anyway, I have included the Ingenic DRM driver patch along with the Synopsys driver hack and a patch adding the specialised driver, all of which apply against v5.8-rc1 of torvalds/linux, so that they might be inspected. I hope that some additional insight can move things forward. Debugging this in the Linux environment is particularly tiresome. Regards, Paul
diff --git a/drivers/clocksource/ingenic-timer.c b/drivers/clocksource/ingenic-timer.c index 496333650de2..a068e4620c44 100644 --- a/drivers/clocksource/ingenic-timer.c +++ b/drivers/clocksource/ingenic-timer.c @@ -2,6 +2,7 @@ /* * JZ47xx SoCs TCU IRQ driver * Copyright (C) 2019 Paul Cercueil <paul@crapouillou.net> + * Copyright (C) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> */ #include <linux/bitops.h> @@ -15,24 +16,35 @@ #include <linux/of_address.h> #include <linux/of_irq.h> #include <linux/of_platform.h> +#include <linux/overflow.h> #include <linux/platform_device.h> #include <linux/regmap.h> #include <linux/sched_clock.h> #include <dt-bindings/clock/ingenic,tcu.h> +static DEFINE_PER_CPU(call_single_data_t, ingenic_cevt_csd); + struct ingenic_soc_info { unsigned int num_channels; }; +struct ingenic_tcu_timer { + unsigned int cpu; + unsigned int channel; + struct clock_event_device cevt; + struct clk *clk; + char name[8]; +}; + struct ingenic_tcu { struct regmap *map; - struct clk *timer_clk, *cs_clk; - unsigned int timer_channel, cs_channel; - struct clock_event_device cevt; + struct device_node *np; + struct clk *cs_clk; + unsigned int cs_channel; struct clocksource cs; - char name[4]; unsigned long pwm_channels_mask; + struct ingenic_tcu_timer timers[]; }; static struct ingenic_tcu *ingenic_tcu; @@ -52,16 +64,24 @@ static u64 notrace ingenic_tcu_timer_cs_read(struct clocksource *cs) return ingenic_tcu_timer_read(); } -static inline struct ingenic_tcu *to_ingenic_tcu(struct clock_event_device *evt) +static inline struct ingenic_tcu * +to_ingenic_tcu(struct ingenic_tcu_timer *timer) +{ + return container_of(timer, struct ingenic_tcu, timers[timer->cpu]); +} + +static inline struct ingenic_tcu_timer * +to_ingenic_tcu_timer(struct clock_event_device *evt) { - return container_of(evt, struct ingenic_tcu, cevt); + return container_of(evt, struct ingenic_tcu_timer, cevt); } static int ingenic_tcu_cevt_set_state_shutdown(struct clock_event_device *evt) { - struct ingenic_tcu *tcu = to_ingenic_tcu(evt); + struct ingenic_tcu_timer *timer = to_ingenic_tcu_timer(evt); + struct ingenic_tcu *tcu = to_ingenic_tcu(timer); - regmap_write(tcu->map, TCU_REG_TECR, BIT(tcu->timer_channel)); + regmap_write(tcu->map, TCU_REG_TECR, BIT(timer->channel)); return 0; } @@ -69,27 +89,40 @@ static int ingenic_tcu_cevt_set_state_shutdown(struct clock_event_device *evt) static int ingenic_tcu_cevt_set_next(unsigned long next, struct clock_event_device *evt) { - struct ingenic_tcu *tcu = to_ingenic_tcu(evt); + struct ingenic_tcu_timer *timer = to_ingenic_tcu_timer(evt); + struct ingenic_tcu *tcu = to_ingenic_tcu(timer); if (next > 0xffff) return -EINVAL; - regmap_write(tcu->map, TCU_REG_TDFRc(tcu->timer_channel), next); - regmap_write(tcu->map, TCU_REG_TCNTc(tcu->timer_channel), 0); - regmap_write(tcu->map, TCU_REG_TESR, BIT(tcu->timer_channel)); + regmap_write(tcu->map, TCU_REG_TDFRc(timer->channel), next); + regmap_write(tcu->map, TCU_REG_TCNTc(timer->channel), 0); + regmap_write(tcu->map, TCU_REG_TESR, BIT(timer->channel)); return 0; } +static void ingenic_per_cpu_event_handler(void *info) +{ + struct clock_event_device *cevt = (struct clock_event_device *) info; + + cevt->event_handler(cevt); +} + static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id) { - struct clock_event_device *evt = dev_id; - struct ingenic_tcu *tcu = to_ingenic_tcu(evt); + struct ingenic_tcu_timer *timer = dev_id; + struct ingenic_tcu *tcu = to_ingenic_tcu(timer); + call_single_data_t *csd; - regmap_write(tcu->map, TCU_REG_TECR, BIT(tcu->timer_channel)); + regmap_write(tcu->map, TCU_REG_TECR, BIT(timer->channel)); - if (evt->event_handler) - evt->event_handler(evt); + if (timer->cevt.event_handler) { + csd = &per_cpu(ingenic_cevt_csd, timer->cpu); + csd->info = (void *) &timer->cevt; + csd->func = ingenic_per_cpu_event_handler; + smp_call_function_single_async(timer->cpu, csd); + } return IRQ_HANDLED; } @@ -105,64 +138,66 @@ static struct clk * __init ingenic_tcu_get_clock(struct device_node *np, int id) return of_clk_get_from_provider(&args); } -static int __init ingenic_tcu_timer_init(struct device_node *np, - struct ingenic_tcu *tcu) +static int ingenic_tcu_setup_cevt(unsigned int cpu) { - unsigned int timer_virq, channel = tcu->timer_channel; + struct ingenic_tcu *tcu = ingenic_tcu; + struct ingenic_tcu_timer *timer = &tcu->timers[cpu]; + unsigned int timer_virq; struct irq_domain *domain; unsigned long rate; int err; - tcu->timer_clk = ingenic_tcu_get_clock(np, channel); - if (IS_ERR(tcu->timer_clk)) - return PTR_ERR(tcu->timer_clk); + timer->clk = ingenic_tcu_get_clock(tcu->np, timer->channel); + if (IS_ERR(timer->clk)) + return PTR_ERR(timer->clk); - err = clk_prepare_enable(tcu->timer_clk); + err = clk_prepare_enable(timer->clk); if (err) goto err_clk_put; - rate = clk_get_rate(tcu->timer_clk); + rate = clk_get_rate(timer->clk); if (!rate) { err = -EINVAL; goto err_clk_disable; } - domain = irq_find_host(np); + domain = irq_find_host(tcu->np); if (!domain) { err = -ENODEV; goto err_clk_disable; } - timer_virq = irq_create_mapping(domain, channel); + timer_virq = irq_create_mapping(domain, timer->channel); if (!timer_virq) { err = -EINVAL; goto err_clk_disable; } - snprintf(tcu->name, sizeof(tcu->name), "TCU"); + snprintf(timer->name, sizeof(timer->name), "TCU%u", timer->channel); err = request_irq(timer_virq, ingenic_tcu_cevt_cb, IRQF_TIMER, - tcu->name, &tcu->cevt); + timer->name, timer); if (err) goto err_irq_dispose_mapping; - tcu->cevt.cpumask = cpumask_of(smp_processor_id()); - tcu->cevt.features = CLOCK_EVT_FEAT_ONESHOT; - tcu->cevt.name = tcu->name; - tcu->cevt.rating = 200; - tcu->cevt.set_state_shutdown = ingenic_tcu_cevt_set_state_shutdown; - tcu->cevt.set_next_event = ingenic_tcu_cevt_set_next; + timer->cpu = smp_processor_id(); + timer->cevt.cpumask = cpumask_of(smp_processor_id()); + timer->cevt.features = CLOCK_EVT_FEAT_ONESHOT; + timer->cevt.name = timer->name; + timer->cevt.rating = 200; + timer->cevt.set_state_shutdown = ingenic_tcu_cevt_set_state_shutdown; + timer->cevt.set_next_event = ingenic_tcu_cevt_set_next; - clockevents_config_and_register(&tcu->cevt, rate, 10, 0xffff); + clockevents_config_and_register(&timer->cevt, rate, 10, 0xffff); return 0; err_irq_dispose_mapping: irq_dispose_mapping(timer_virq); err_clk_disable: - clk_disable_unprepare(tcu->timer_clk); + clk_disable_unprepare(timer->clk); err_clk_put: - clk_put(tcu->timer_clk); + clk_put(timer->clk); return err; } @@ -238,10 +273,12 @@ static int __init ingenic_tcu_init(struct device_node *np) { const struct of_device_id *id = of_match_node(ingenic_tcu_of_match, np); const struct ingenic_soc_info *soc_info = id->data; + struct ingenic_tcu_timer *timer; struct ingenic_tcu *tcu; struct regmap *map; + unsigned int cpu; + int ret, last_bit = -1; long rate; - int ret; of_node_clear_flag(np, OF_POPULATED); @@ -249,17 +286,23 @@ static int __init ingenic_tcu_init(struct device_node *np) if (IS_ERR(map)) return PTR_ERR(map); - tcu = kzalloc(sizeof(*tcu), GFP_KERNEL); + tcu = kzalloc(struct_size(tcu, timers, num_possible_cpus()), + GFP_KERNEL); if (!tcu) return -ENOMEM; - /* Enable all TCU channels for PWM use by default except channels 0/1 */ - tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1, 2); + /* + * Enable all TCU channels for PWM use by default except channels 0/1, + * and channel 2 if target CPU is JZ4780 and SMP is selected. + */ + tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1, + num_possible_cpus() + 1); of_property_read_u32(np, "ingenic,pwm-channels-mask", (u32 *)&tcu->pwm_channels_mask); - /* Verify that we have at least two free channels */ - if (hweight8(tcu->pwm_channels_mask) > soc_info->num_channels - 2) { + /* Verify that we have at least num_possible_cpus() + 1 free channels */ + if (hweight8(tcu->pwm_channels_mask) > + soc_info->num_channels - num_possible_cpus() + 1) { pr_crit("%s: Invalid PWM channel mask: 0x%02lx\n", __func__, tcu->pwm_channels_mask); ret = -EINVAL; @@ -267,13 +310,22 @@ static int __init ingenic_tcu_init(struct device_node *np) } tcu->map = map; + tcu->np = np; ingenic_tcu = tcu; - tcu->timer_channel = find_first_zero_bit(&tcu->pwm_channels_mask, - soc_info->num_channels); + for (cpu = 0; cpu < num_possible_cpus(); cpu++) { + timer = &tcu->timers[cpu]; + + timer->cpu = cpu; + timer->channel = find_next_zero_bit(&tcu->pwm_channels_mask, + soc_info->num_channels, + last_bit + 1); + last_bit = timer->channel; + } + tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask, soc_info->num_channels, - tcu->timer_channel + 1); + last_bit + 1); ret = ingenic_tcu_clocksource_init(np, tcu); if (ret) { @@ -281,9 +333,13 @@ static int __init ingenic_tcu_init(struct device_node *np) goto err_free_ingenic_tcu; } - ret = ingenic_tcu_timer_init(np, tcu); - if (ret) + /* Setup clock events on each CPU core */ + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic XBurst: online", + ingenic_tcu_setup_cevt, NULL); + if (ret < 0) { + pr_crit("%s: Unable to start CPU timers: %d\n", __func__, ret); goto err_tcu_clocksource_cleanup; + } /* Register the sched_clock at the end as there's no way to undo it */ rate = clk_get_rate(tcu->cs_clk); @@ -315,28 +371,38 @@ static int __init ingenic_tcu_probe(struct platform_device *pdev) static int __maybe_unused ingenic_tcu_suspend(struct device *dev) { struct ingenic_tcu *tcu = dev_get_drvdata(dev); + unsigned int cpu; clk_disable(tcu->cs_clk); - clk_disable(tcu->timer_clk); + + for (cpu = 0; cpu < num_online_cpus(); cpu++) + clk_disable(tcu->timers[cpu].clk); + return 0; } static int __maybe_unused ingenic_tcu_resume(struct device *dev) { struct ingenic_tcu *tcu = dev_get_drvdata(dev); + unsigned int cpu; int ret; - ret = clk_enable(tcu->timer_clk); - if (ret) - return ret; + for (cpu = 0; cpu < num_online_cpus(); cpu++) { + ret = clk_enable(tcu->timers[cpu].clk); + if (ret) + goto err_timer_clk_disable; + } ret = clk_enable(tcu->cs_clk); - if (ret) { - clk_disable(tcu->timer_clk); - return ret; - } + if (ret) + goto err_timer_clk_disable; return 0; + +err_timer_clk_disable: + for (; cpu > 0; cpu--) + clk_disable(tcu->timers[cpu - 1].clk); + return ret; } static const struct dev_pm_ops __maybe_unused ingenic_tcu_pm_ops = {