Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lilygo T5-4.7 - Current leaking to 3v3 rail after epd_poweroff #136

Closed
kaweksl opened this issue Jan 9, 2022 · 33 comments
Closed

Lilygo T5-4.7 - Current leaking to 3v3 rail after epd_poweroff #136

kaweksl opened this issue Jan 9, 2022 · 33 comments
Labels
bug Something isn't working

Comments

@kaweksl
Copy link
Contributor

kaweksl commented Jan 9, 2022

Hi,

On Lilygo T5-4.7 powered with battery after screen init and epd_poweron() board takes ~115mA@3.6V , and after epd_poweroff() but with ESP32 fully running ( NOT in deep sleep ) power consumption increases to ~235mA .

Looks like some control pins on ESP32 are staying high after epd_poweroff(); and some current passes from esp32 via epd chip clamping diodes to switched off 3v3 rail . This is not happening before epd initialization .

I have noticed after epd_poweroff() esp32 pins IO26 (STH) , IO5(CKH) and IO2(EP_D4) have ~2.2V what may indicate high current draw and drive overload .

Current draw measured with PPK2

EDIT: I need to clarify Lilygo T5-4.7 have rail called VDD3V3 that is powering ESP32 and shift register , it also have 3V3 rail that provide power to epd and LT1945, that rail is controlled by shift register and epd_poweron/poweroff . My theory is that when epd is off, current from esp32 pins go through clamping diodes of epd chip to not powered 3v3 rail and from there wants to power up LT1945 .
This doesn't happen if powered by USB because then EPD and LT1945 is always powered on .

@mcer12
Copy link
Collaborator

mcer12 commented Jan 19, 2022

First off, this is not the place to debug lilygo board. Even though it's supported by the library, you should open an issue on lilygo page, not here.

It doesn't make any sense for the board to draw double the current when the power to the display is off compared to when display is on. The pins can't even draw such current because to my knowledge ESP pins are limited to around 20mA max. Also, these are signal pins and there's no way the display would draw such excessive current through data lines.

The Lilygo board is not particularly great, it's based off of old outdated epdiy design and it has issues with the display fading out when sleeping.

@kaweksl
Copy link
Contributor Author

kaweksl commented Jan 19, 2022

As far i know ESP32 has Cumulative IO output current = 1200mA and max ~40mA per IO, that also depends on pin power domain. I have tracked down 3 pins, 3x40mA = 120mA..
What if VDD = 0V, screen (driver IC ) data lines have clamping diodes to VDD (common protection method) , voltage on data lines is higher than on VDD, current is passed trough diode to VDD, something else on VDD is trying to draw that current as not only screen is connected to VDD .

(red line is current path)
Screenshot from 2022-01-19 22-54-56

I know lilygo is not great design but hardware have been made, software can be changed.

#90
This might be related as there is lower than expected battery life and screen damage as passing 100mA trough driver ic is most likely out off spec

@mcer12
Copy link
Collaborator

mcer12 commented Jan 20, 2022

The easiest way to test this would be just to pull all the suspected pins low or set them as input after shutting down the display

@kaweksl
Copy link
Contributor Author

kaweksl commented Jan 20, 2022

Here is current draw graph with state annotations
ppk-20220120T201346
From what i observed current draw after poweroff may depend on last "byte" send to screen

@kaweksl
Copy link
Contributor Author

kaweksl commented Jan 20, 2022

I made some changes in display_ops.c

static void gpio_setup_out(int gpio, int sig, bool invert) {
  if (gpio == -1)
    return;
  PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[gpio], PIN_FUNC_GPIO);
  gpio_set_direction(gpio, GPIO_MODE_DEF_OUTPUT);
  gpio_matrix_out(gpio, sig, invert, false);
}

void epd_poweron() { 
  gpio_num_t I2S_GPIO_BUS[] = {D6, D7, D4, D5, D2, D3, D0, D1};
  
  gpio_set_direction(STH, GPIO_MODE_OUTPUT);
  gpio_set_level(STH, 1);

  int signal_base = I2S1O_DATA_OUT0_IDX;

  for (int x = 0; x < 8; x++) {
    gpio_setup_out(I2S_GPIO_BUS[x], signal_base + x, false);
  }

  gpio_setup_out(CKH, I2S1O_WS_OUT_IDX, true);

  cfg_poweron(&config_reg);
}

void epd_poweroff() { 
  cfg_poweroff(&config_reg);
  gpio_set_direction(D0, GPIO_MODE_INPUT);
  gpio_set_direction(D1, GPIO_MODE_INPUT);
  gpio_set_direction(D2, GPIO_MODE_INPUT);
  gpio_set_direction(D3, GPIO_MODE_INPUT);
  gpio_set_direction(D4, GPIO_MODE_INPUT);
  gpio_set_direction(D5, GPIO_MODE_INPUT);
  gpio_set_direction(D6, GPIO_MODE_INPUT);
  gpio_set_direction(D7, GPIO_MODE_INPUT);
  gpio_set_direction(STH, GPIO_MODE_INPUT);
  gpio_set_direction(CKH, GPIO_MODE_INPUT);
}

and now graph looks like that
ppk-20220120T220251

Much better. Offcourse there is better way to implement code above.

@ashald
Copy link
Contributor

ashald commented Feb 3, 2022

@kaweksl would you submit a pull request?

P.S.: Without this change I see severe fading if epd_poweroff() is called less than a second after epd_hl_update_screen() completes, but there is no issue with this change.

@kaweksl
Copy link
Contributor Author

kaweksl commented Feb 7, 2022

@ashald
Code above was just to proof my theory, i don't know if it's proper way to do it or if doesn't create another issues. I don't have enough awareness how epdiy code works, and learning make take some time. :P

P.S.: Without this change I see severe fading if epd_poweroff() is called less than a second after epd_hl_update_screen() completes, but there is no issue with this change.

Before noticing increased current consumption i was also delaying for ~700ms before poweroff . Good to know that now i can avoid that delay.

@ashald
Copy link
Contributor

ashald commented Feb 8, 2022

I just received my PPK2 and tested whether I can reproduce the above. I can, and the difference is dramatic.

I've got Lilygo T5-4.7 setup to wake up, received data over BLE advertisement, refresh the screen, and go back to deep sleep. I added 3s delay between epd_pwoeroff() and epd_deinit(), and measure the power consumption with PPK2.

Here is the original profile. Please note gray box in the bottom right corder that sums up the selected area.
after_power_off

And here is the same cycle with the above fix.
after_power_off_fixed

Basically, ~4x difference.

As @kaweksl said above

hardware have been made, software can be changed.

I'd love to see this incorporated into the library.

To the best of my understanding, we're sure there is an issue, and we know that it's solvable. Now the question is if the solution is adequate or it may have unwanted side-effects?

@mcer12 what could help us move this forward? Thanks!

@mcer12
Copy link
Collaborator

mcer12 commented Feb 8, 2022

@ashald modifying the library for broken unofficial hardware isn’t the way. I would merely add a note on wiki about this since it’s such a simple fix. Btw instead of delay, light sleep may be better if you want lowest power consumption possible. Give it a try.

@martinberlin
Copy link
Sponsor Collaborator

In my opinion if it's just setting the data lines to input in epd_poweroff() it could be done with a precompiler define and a proper for...loop to set the lines only when CONFIG_EPD_BOARD_REVISION_LILYGO_T5_47 is defined.
But I completely agree with @mcer12 on the fact that if a library should be modified for each hardware designs that leak current somewhere, it can be messy and hard to maintain.

@errolt
Copy link

errolt commented Feb 8, 2022

@mcer12 The issue isn't caused by "broken hardware". Phantom powering the display WILL affect all designs. Unless "official hardware" completely disconnects the ESP IO from the display when the display is powered off, and, looking at the schematic, it doesn't disconnect the IO.

IO going to any device that can be powered down should be set low, or set to input, whenever the part they connect to is powered down. ALWAYS.

I have seen this issue in other commercial hardware, where a chip is powered off but the IO going to the chip is allowed to power the chip. In that case sleep power was higher than expected and there was reliability issues.

@mcer12
Copy link
Collaborator

mcer12 commented Feb 8, 2022

@errolt Except this doesn't happen on official board.

@errolt
Copy link

errolt commented Feb 8, 2022

Maybe the other displays send a "0" as the last byte in all transactions, and the display on the Lilygo doesn't? Dunno. But there isn't significant hardware differences between official and lilygo hardware, not that I can see.

Please don't get me wrong, I'm not arguing.

I'm just talking from past commercial experience. My previous employer had 1M devices in the field before I was tasked to find the reason for batteries lasting 3 years instead of 8 years, which they promised their users. So I know how this issue can slip in. And changing the last byte sent to the display can cause, or hide, this intermittent issue.
It is just best practice to make sure you power off IO that connect to a powered off device. Else changing one line of code somewhere else can introduce the issue on official hardware.

@mcer12
Copy link
Collaborator

mcer12 commented Feb 8, 2022

@errolt no, it's not about the display or leakage current from GPIOs. I don't know what design you're looking at but lilygo schematic is very different from V5 or V6 and is based off of very early epdiy schematic. Not to mention some mistakes in the design like using 100nF capacitor at the output of -20V rail. And 100nF capacitors on the linear regulators as well, that won't result in good clean output and may be the reason for some glitches reported by users.

The sole fact that adding a delay fixes it leads me to think it has nothing to do with GPIO leakage ;)

Design I am talking about (if it's not the correct one, please share yours):
https://github.com/Xinyuan-LilyGO/LilyGo-EPD47/blob/master/schematic/LilyGo-EPD47.pdf

Also please don't take this as agressive, I'm just trying to explain my point of view.

@errolt
Copy link

errolt commented Feb 8, 2022

The sole fact that adding a delay fixes it leads me to think it has nothing to do with GPIO leakage ;)

Which delay solution are you referring to? The code fix changes the IO lines to INPUT on epd_poweroff(), restores it in epd_poweron(). The only reference I see to a delay was to prevent fading, which was caused by the phantom power issue in the first place. Phantom powering a device can result is many a wonderful, and hard to debug, issue...

@mcer12
Copy link
Collaborator

mcer12 commented Feb 8, 2022

@errolt I am reffering to @ashald post with screenshots, apparently the delay helps with power consumption.

@errolt
Copy link

errolt commented Feb 8, 2022

@mcer12 I think you misunderstood his/her post. Their first line in the the post says that they confirm the issue, and the fix.

They added a 3s delay to make the issue more visible, so they can highlight it in the graph. Then they applied the fix mentioned above, and the issue went away... The delay did not fix the problem...

@mcer12
Copy link
Collaborator

mcer12 commented Feb 8, 2022

@errolt you're right lol, I can't read it looks like.

@errolt
Copy link

errolt commented Feb 8, 2022

@mcer12 Don't worry, has happened to me too... ;-)
Does that mean that you will reconsider adding it to the core library?

@ashald
Copy link
Contributor

ashald commented Feb 8, 2022

@mcer12, yes, as @errolt mentioned - I just added a delay to make the issue more visible - and that's what screenshots demonstrate. Not that I don't trust @kaweksl who found the issue originally, but I wanted to make sure it's reproducible by other people so that we're confident it's not a one off.

I understand your concern with regards to having special tweak for different boards, and their issues, and I would've just stoped there if not one thing that I don't understand. epdiy's library already has a bunch of statements like

if defined(CONFIG_EPD_BOARD_REVISION_LILYGO_T5_47)

including the display_ops.c among other places where I saw it (e.g., not only waverforms). To me, this reads that we already have special behavior for different boards to address their specifics.

Lastly - and here could be just my lack of knowledge to blame - I couldn't find a way to add those custom functions on my own. I'm using this library via ESPHome, which uses PlatformIO to add it as a dependency. It's based on C++ code, and I wasn't able to get access to config_reg defined in display_ops.c.

If you're hesitant to add another ifdef CONFIG_EPD_BOARD_REVISION_LILYGO_T5_47 to address the issue, would you mind adding a function that would provide a reference to config_reg, so that these custom versions of epd_poweron() and epd_poweroff() can be implemented on the library user side.

@martinberlin
Copy link
Sponsor Collaborator

martinberlin commented Feb 8, 2022

I would suggest not to make this a never ending conversation and just make a nice and clean pull request with the updated code. I think the one that decides if something goes or not in this library is the maintainer.
In my opinion there is no need to do a:

 gpio_set_direction(D0, GPIO_MODE_INPUT);

If would be enough to put the pins low, in a for loop, declaring the IOs in an gpio_num_t array.
GPIOs should be ideally set up once and you don't need to care about them, even if you power up the display or not.
Can you make the tests to see if this would work, has the same effect of consuming less, and make a PR?
Then I can make the test with a V5 and V6 board, to check it does not affect any other thing.
As commented before, Lilygo used a old version of this library and never cared about updating it. I've myself redirected people over this library and reject to update their code in their repository, since there where not much interest in updating the EPDiy library.

@ashald
Copy link
Contributor

ashald commented Feb 9, 2022

@kaweksl do you feel like submitting a PR? If no, I can try to do that based on your work.

@vroland
Copy link
Owner

vroland commented Feb 9, 2022

I think isolating or setting the outputs to low definitely seems like the correct solution here. Set the pins to output on poweron() and to input on poweroff(). Though I'm not sure if the I2S peripheral stays initialized when changing the pin direction, or if that would require to re-initialize the complete peripheral.

@vroland vroland added the bug Something isn't working label Feb 9, 2022
@kaweksl
Copy link
Contributor Author

kaweksl commented Feb 9, 2022

I have tested that and gpio's have to be set back to output with proper matrix setting and i2s don't have to be re-initialized.

I may create some PR but i have to find proper place to put that code.

@ashald
Copy link
Contributor

ashald commented Feb 9, 2022

@kaweksl don't we want to put it into display_ops.c wrapped into if defined?
E.g.:

void epd_poweron() { 
#if defined(CONFIG_EPD_BOARD_REVISION_LILYGO_T5_47)
  // extra code here
#endif
  cfg_poweron(&config_reg);
}


void epd_poweroff() { 
  cfg_poweroff(&config_reg);
#if defined(CONFIG_EPD_BOARD_REVISION_LILYGO_T5_47)
  // extra code here
#endif
}

?

@martinberlin
Copy link
Sponsor Collaborator

Without any desire to criticize just giving my honest opinion @ashald : this looks really ugly. If doing this does not affect any other boards performance it should be done for all, just need to be tested properly when the PR is made.

@ashald
Copy link
Contributor

ashald commented Feb 9, 2022

@martinberlin pardon me if I'm missing anything, but to me it looks like a common pattern across most of the codebase, including display_ops.c, e.g. https://github.com/vroland/epdiy/blob/master/src/epd_driver/display_ops.c#L58-L115

Is the intention to move away from that pattern start with this PR?.. Or am I missing something?

And if we want to to test it with other boards - could you suggest anyone who can help with this? Or if this becomes such a big controversy, would you just prefer adding a function that returns &config_reg so that people like me can just implement custom powernon() and poweroff() in their code without need to copy-paster section from the library?

@ashald
Copy link
Contributor

ashald commented Feb 9, 2022

Please disregard my above comment. I realized that one can just manipulate pins in their code and then just call original epd_poweron() and epd_poweroff(). 🤦‍♂️ Given this there's no real need to modify anything in the library.

@vroland
Copy link
Owner

vroland commented Feb 10, 2022

From what I gathered, the issue is twofold for the LILYGO board, but still present for the epdiy boards:
For one, there may be current leaking from the control register pins. This is fixed for epdiy boards, but not for the lilygo board.
Secondly, current may leak from the data + clock lines. This is a general issue and could be added to display_ops.c, IMO.

@martinberlin
Copy link
Sponsor Collaborator

martinberlin commented Feb 10, 2022

Thanks for your followup. Tomorrow I will take some time to measure in my Lilygo board the battery consumption after epd_poweroff() and:

  1. Check if setting to low all data pins makes a difference. If it does then this is the easiest way.
  2. Or if additionally it's needed to set this to input mode, which is IMHO not the nicest way, since they should be configured again like described above. In that case then this should by done only for LILYGO and if V5 is affected also for this board (Need to check this since I gave mine to my father)
gpio_num_t DATA_BUS[] = {D6, D7, D4, D5, D2, D3, D0, D1, STH, CKH};
for (uint8_t pin = 0; pin < 10; pin++) {
   gpio_set_level(DATA_BUS[pin], 0);
   // Or if that keeps consumption up, then direction. But need to configure again in epd_poweron()
   // gpio_set_direction(DATA_BUS[pin], GPIO_MODE_INPUT);
}

Will report my findings here after taking the time to put the miliamperimeter in Serie with the battery and see how much it consumes. But I can confirm that in comparison to V6 this board it consumes much more. Lilygo designs are beautiful but they never cared much about keeping the lowest possible consumption levels on deepsleep.

@ashald
Copy link
Contributor

ashald commented Feb 10, 2022

@martinberlin I have PPK2 wired with a JST connector so I can easily check things on my Likygo in terms of power consumption.

If that is helpful, I can quickly run some tests if you'd share exact code you want to try.

@martinberlin
Copy link
Sponsor Collaborator

martinberlin commented Feb 10, 2022

As is the code now, after power out in the dragon example, without any deepsleep this is what I see in the testing.

As is without modifying anything:
135 mA
Turning the GPIOs to 0 using gpio_set_level does not change almost anything:
134,6 mA
Turning the GPIOs to input again does consume less:
44,5 mA

Using deepsleep after rendering the dragon
As is without modifying anything:
2.67 mA
Turning the GPIOs to input again does not consume less:
2.67 mA (My V6 board consumes 0.4 mA)

Will leave later a proposed update but it needs to be tested in other boards. Important to mention my Lilygo EPD47 has touch connected, no idea if I2C touch leaks somewhere, but it might be lower that consumption in deepsleep without touch.
Is a very thin FPC and not meant to play around (I've had bad luck with FPC's and 2 days ago I trashed a very nice ED060XC5 EPD)

martinberlin added a commit to martinberlin/epdiy-rotation that referenced this issue Feb 10, 2022
@martinberlin
Copy link
Sponsor Collaborator

This fix was merged so we can close here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants