Error trying PWMBacklight with ili9341

mattblovell commented 3 years ago

Type of Raspberry Pi

Raspberry Pi 3 Model B v1.2

Linux Kernel version

Linux raspberrypi 5.4.51-v7+ #1333 SMP Mon Aug 10 16:45:19 BST 2020 armv7l GNU/Linux

Other versions

luma.core 1.17.1 luma.lcd 2.5.0 python 3.7.3

Expected behaviour

I am attempting to enable use of PWM to control backlight brightness for my script (see mattblovell/kodi_panel on github).

The constructor for the ili9341 device is thus called in one of two ways:

serial = spi(port=0, device=0, gpio_DC=24, gpio_RST=25,
             reset_hold_time=0.2, reset_release_time=0.2)

if USE_PWM:
    device = ili9341(serial, active_low=False, width=320, height=240,
                     bus_speed_hz=32000000,
                     gpio_LIGHT=18,
                     pwm_frequency=PWM_FREQ
    )
else:
    device = ili9341(serial, active_low=False, width=320, height=240,
                     bus_speed_hz=32000000,
                     gpio_LIGHT=18
    )

I am getting my "leftover" 2.8-inch panel LCD panel working again on the RPi and thought I would give the PWM functionality a try (even though RPi.GPIO's PWM is still software-based).

The current form of kodi_panel.py, with USE_PWM set to True, works as-is on an Odroid C4. There is some flicker, though, given the pthreads control for the backlight. So, I was expecting somewhat the same behavior on the RPi. Instead an error message results.

Actual behaviour

Instantiating self._pwm
Error:  You must setup() the GPIO channel as an output first
Traceback (most recent call last):
  File "kodi_panel.py", line 286, in <module>
    pwm_frequency=PWM_FREQ
  File "/home/pi/.local/lib/python3.7/site-packages/luma/lcd/device.py", line 548, in __init__
    super(ili9341, self).__init__(luma.lcd.const.ili9341, serial_interface, **kwargs)
  File "/home/pi/.local/lib/python3.7/site-packages/luma/lcd/device.py", line 217, in __init__
    self.backlight(True)
  File "/home/pi/.local/lib/python3.7/site-packages/luma/lcd/device.py", line 177, in __call__
    self._pwm.ChangeDutyCycle(value)
AttributeError: 'PWMBacklight' object has no attribute '_pwm'

I should also note that physical Pin 12 (GPIO18) does work fine for backlight control without PWM on the RPi. Merely setting USE_PWM in my code, such that the ili9341() constructor is invoked with pwm_frequency set, is sufficient to get the above error.

Setting USE_PWM back to False, everything is happy again.

rm-hull commented 3 years ago

What version of Luma.LCD are you using? the line numbers in the stack trace you supplied dont seem to line up with latest on master

thijstriemstra commented 3 years ago

he mentioned 2.5.0 @rm-hull

rm-hull commented 3 years ago

well ... the line numbers dont line up with whats in 2.5.0, so something is off somewhere. That class has full test coverage with these tests: https://github.com/rm-hull/luma.lcd/blob/master/tests/test_backlight.py#L118-L161

The current form of kodi_panel.py, with USE_PWM set to True, works as-is on an Odroid C4. There is some flicker, though, given the pthreads control for the backlight. So, I was expecting somewhat the same behavior on the RPi.

@mattblovell are you saying the PWM backlight appears to work on Odroid, but not on RPi ?

mattblovell commented 3 years ago

@mattblovell are you saying the PWM backlight appears to work on Odroid, but not on RPi ?

Yes. Both SBCs have the same luma.core and luma.led versions installed via pip (1.17.1, 2.5.0 respectively). My same kodi_panel.py script is the same on both as well.

On the Odroid C4, I can switch USE_PWM back and forth between True and False without issue, picking between the two ili9341() constructors I pointed out. I was attempting the same experiment on RPi (primarily to see how "bad" the software-based PWM is there) when I ran across this issue.

The code base is nominally the same, until one gets down to the RPi.GPIO module of course. So, perhaps something in the "real" RPi.GPIO is being more picky (than RPi.GPIO-Odroid)? I opened the issue just to see if it was something trivial. If anything, I was expecting execution on RPi generally to be more robust (since more people are using it). I was therefore surprised to see the error.

If PWMBacklight is working for others using RPi boards, perhaps there is just something odd about my setup.

I'm surprised by the line differences. On the RPi board in particular, I relied solely on pip3 for installation. I don't think I ever poked around in the installed lib files.

mattblovell commented 3 years ago

I can try the RPi experiment again later this afternoon, with a fresh install of luma.* modules.

mattblovell commented 3 years ago

I can try the RPi experiment again later this afternoon, with a fresh install of luma.* modules.

I started with pip3 install --force-reinstall luma.lcd, which concluded with the following results:

...
Installing collected packages: pyserial, pyusb, pyftdi, RPI.GPIO, wrapt, deprecated, smbus2, pillow, cbor2, spidev, luma.core, luma.lcd
  Found existing installation: luma.lcd 2.5.0
    Uninstalling luma.lcd-2.5.0:
      Successfully uninstalled luma.lcd-2.5.0
Successfully installed RPI.GPIO-0.7.0 cbor2-5.2.0 deprecated-1.2.10 luma.core-1.17.2 luma.lcd-2.5.0 pillow-8.0.1 pyftdi-0.52.0 pyserial-3.4 pyusb-1.1.0 smbus2-0.3.0 spidev-3.5 wrapt-1.12.1

I then tried setting USE_PWM to True in my script and invoking it. Line numbers indeed changed compared to the earlier results. No error from GPIO appeared, but the overall results are the same:

Traceback (most recent call last):
  File "kodi_panel.py", line 286, in <module>
    pwm_frequency=PWM_FREQ
  File "/home/pi/.local/lib/python3.7/site-packages/luma/lcd/device.py", line 546, in __init__
    super(ili9341, self).__init__(luma.lcd.const.ili9341, serial_interface, **kwargs)
  File "/home/pi/.local/lib/python3.7/site-packages/luma/lcd/device.py", line 215, in __init__
    self.backlight(True)
  File "/home/pi/.local/lib/python3.7/site-packages/luma/lcd/device.py", line 175, in __call__
    self._pwm.ChangeDutyCycle(value)
AttributeError: 'PWMBacklight' object has no attribute '_pwm'

The backlight is using GPIO18, physical Pin 12.

Repeating the attempt a second time, now the GPIO "channel is already in use" warning appears.

mattblovell commented 3 years ago

Returning to this hiccup...

If I comment out the try...except portion of PWMBacklight.__init__() within luma/lcd/device.py, the error upon startup switches to this:

  File "/home/pi/.local/lib/python3.7/site-packages/luma/lcd/device.py", line 157, in __init__
    self._pwm = self._gpio.PWM(pin, frequency)
RuntimeError: You must setup() the GPIO channel as an output first

I'll keep playing. I've also seen hints googling around that RPi.GPIO may, despite its documentation to the contrary, support hardware PWM. One of the pin functions floating around seems to be HARD_PWM.

That could be useful, as any jitter in PWM brightness for a backlight is readily visible.

mattblovell commented 3 years ago

RuntimeError: You must setup() the GPIO channel as an output first

Adding

self._gpio.setup(pin, self._gpio.OUT)

to that __init__() function gets things working, but some jitter is evident.

mattblovell commented 3 years ago

but some jitter [flickering] is evident.

I observed similar flickering on the Odroid C4. (On the Odroid forums, one of the Hardkernel developers is augmenting RPi.GPIO-Odroid to make use of hardware PWM on their N2 and C4 SBCs. It's still a work in progress, but I'd like to think my gentle poking resulted in something.)

RPi.GPIO uses pthreads for its PWM implementation, which unfortunately isn't particularly rock-solid.

How difficult would it be, rather than treating the backlight signal as a pin handled through RPi.GPIO, to make use of the sysfs interface for hardware PWM control?

The following is an extremely inelegant hack, but it produces a stable brightness. I hacked backlit_device.__init__() to perform these shell commands rather than invoke anything from RPi.GPIO:

        os.system("echo 0 > /sys/class/pwm/pwmchip0/export")
        sleep(0.150)
        os.system("echo 1000000 > /sys/class/pwm/pwmchip0/pwm0/period")
        os.system("echo  100000 > /sys/class/pwm/pwmchip0/pwm0/duty_cycle")
        os.system("echo  1 > /sys/class/pwm/pwmchip0/pwm0/enable")

I have existing functions that were serving as wrappers for turning the backlight on and off; those I modified to just write 1 or 0 to the above enable file.

This approach requires that /sys/class/pwm exist, of course, so a dtoverlay pwm-2chan invocation is needed first.

The sleep is unfortunately necessary to give time for the export to take effect.

Perhaps there is a more elegant mechanism available.

mattblovell commented 3 years ago

Perhaps there is a more elegant mechanism available.

Having a mechanism to tell the backlit_device parent class not to claim the designated gpio_LIGHT pin would do. That would be equivalent to the commenting-out I hacked. With that approach, all the ugliness could just reside in my application code, rather than in luma.lcd.

I do need to google around and see if there is a nicer approach than the shell commands, but they seem to work.

mattblovell commented 3 years ago

I do need to google around and see if there is a nicer approach than the shell commands, but they seem to work.

scottellis/pwmpy (~~license unclear~~ New BSD) and
jdimpson/syspwm (GPL-3.0)

are both variations on a theme. They use os file operations rather than shell commands, so I guess they're a bit more elegant. :)

@rm-hull, any thoughts on being able to (effectively) turn off the backlit_device parent and leave it to the application to handle the backlight and its PWM? (Please see earlier posts regarding RPi-GPIO and flicker.)

I think the root cause for the original error message is also addressed above (just need a GPIO output setup prior to trying to create _pwm).

EDIT: Updated license for pwpmy, per Robert's observation below.

rm-hull commented 3 years ago

Makes sense, but might be worth having a chat with @dhrone and @kevinastone as the best way to proceed here (must admit I am not especially familiar with this part of the codebase), but I would have a slight preference for bringing in some form of /sys/class/pwm/pwmchip/pwm capability into PWMBacklight instead the current gpio implementation, and then we wouldn't have to contemplate turning off the backlight.

Neither project has a release in pypi, so we'd have to manually include i guess, unless we can require directly from github.

oh, and pwmpy looks to be 'New BSD', according to this: https://github.com/scottellis/pwmpy/blob/master/pwm.py#L7

mattblovell commented 3 years ago

... I would have a slight preference for bringing in some form of /sys/class/pwm/pwmchip/pwm capability into PWMBacklight instead the current gpio implementation, and then we wouldn't have to contemplate turning off the backlight.

So far, I've only seen two methods for taking advantage of hardware PWM on GPIO18 / PWM0:

through /sys/class/pwm sysfs (after ensuring it exists, e.g. after loading pwm-2chan or a similar overlay on an RPi) or
through a C library, such as PiGPIO (which does offer python bindings).

Discussions of RPi.GPIO's software-based PWM flickering:

RPi Forum, 2014: Frequency for GPIO.PWM without flickering
RPi StackExchange, 2018: LED pwm fading is flickering
Github issue for gpiozero, 2018: LED flickering while using PWMLED

That gpiozero discussion is interesting. Seems like that library offers different backends for controlling the GPIO pins -- RPi.GPIO, RPIO, and PiGPIO. The documentation for the PiGPIO backend seems to imply one has to also have a daemon (pigpiod) running, with socket communication occurring to control the pins. That seems like a lot of overhead! (I also like that RPi.GPIO has an RPi.GPIO-Odroid variant that supports those boards.)

Perhaps the flicker would be better with some other choice of frequency? In my application, I've been using this when experimenting:

PWM_FREQ      = 362      # frequency, presumably in Hz
PWM_LEVEL     = 75.0     # float value between 0 and 100

In general, if hardware-based PWM is available at relatively low-effort, seems like it would be nice to make use of it.

FWIW, the Waveshare displays all seem to have fairly straightforward rework options that give one PWM control over the backlight via (everyone's favorite!) GPIO18. This is true for the 3.5" SPI-attached IPS panel and 4" HDMI panel. (They have a PDF available online with the rework instructions, but it doesn't have all of their displays. They were quiet speedy in replying when I inquired about the 2 displays I have on-hand.)

kevinastone commented 3 years ago

I mentioned that the lack of hardware PWM in the GPiO library greatly impacts the usefulness of the PWM support in #95. Here's my workaround using gpiozero (which uses pigpio for hardware PWM if installed).

from functools import cached_property

from luma.lcd.device import ili9341 as base_ili9341
from gpiozero import Device, PWMLED

class ili9341(base_ili9341):
    @cached_property
    def backlight_led(self) -> Device:
        return PWMLED(self.backlight._pin, frequency=362)  # type: ignore

    def backlight(self, enabled: bool) -> None:
        self.brightness(1.0 if enabled else 0.0)

    def brightness(self, value: float) -> None:
        self.backlight_led.value = value

mattblovell commented 3 years ago

The display I'm most interested in using right at the moment is an HDMI panel. Since luma's linux_framebuffer device doesn't make use of the backlight parent class, there's no "conflict" to really avoid fortunately.

I think one of the modules listed above will likely fit my needs.

Do we want to go down the path of modifying luma.lcd to make use of something other than RPi.GPIO for the backlight pin?

thijstriemstra commented 3 years ago

Do we want to go down the path of modifying luma.lcd to make use of something other than RPi.GPIO for the backlight pin?

Sounds good to me. But probably means luma.core has to support it as well.

mattblovell commented 3 years ago

Sounds good to me. But probably means luma.core has to support it as well.

Some questions toward that goal...

I take it that luma/lcd/device.py's backlight_device class is typically getting gpio via the @rpi_gpio decorator? I'm new-ish to Python and still getting used to what patterns exist and preferred mechanisms. If multiple classes exist with which to drive gpio_LIGHT, would we leave that decorator as-is or change it to something else?
Currently, the pwm_frequency (after backpack_pin gets checked) is used to distinguish whether the self.backlight member ends up being a PWMBacklight or GPIOBacklight instance. If multiple PWM mechanisms are available, the presence of that argument alone won't be sufficient to pick which one to use. What's the preferred way (for luma) of communicating such a "settings" choice.

Oh, and separately, do we want to include the self._gpio.setup(pin, self._gpio.OUT) call I mentioned above? Has anyone else been able to try the existing RPi.GPIO PWM mechanism to reproduce my original error?

Thanks! Matt

kevinastone commented 3 years ago

Not sure I follow what you mean by "multiple PWM mechanisms". The PWMBacklight class can be improved to leverage different PWM interfaces (e.g. sysfs, drivers like gpiozero or the current software fallback).

mattblovell commented 3 years ago

The PWMBacklight class can be improved to leverage different PWM interfaces (e.g. sysfs, drivers like gpiozero or the current software fallback).

Yes, that's actually what I meant @kevinastone. If multiple such interfaces / mechanisms are available, what's the best way to allow application code to select the mechanism to use?

thijstriemstra commented 3 years ago

python 3.7.3

Are you still using this? I think it's def worth trying later 3.7.x, or of course 3.8 and 3.9, versions of python.

mattblovell commented 3 years ago

Are you still using this?

Yes. I think its the default for python3 on the Buster release of Raspberry Pi OS. At least it's a 3.x. :)

My original motivation for development was for an application running under CoreELEC on an Odroid C4. For lots of reasons, it's still on a 4.9.x Linux kernel; I believe I have Entware to thank for even getting python3 on that particular installation.

kevinastone commented 3 years ago

@mattblovell Do applications need to specify their preferred mechanism? I would have expected there's a natural priority and fallback progression. (e.g. pigpio then sysfs then RPi.GPIO as such)

mattblovell commented 3 years ago

I would have expected there's a natural priority and fallback progression. (e.g. pigpio then sysfs then RPi.GPIO as such)

If the preference order is that clear, and the process of determining what's actually available is also reasonable quick and reliable (e..g., no odd error messages given back to the user, doesn't take a long time to "test" if pigpiod is available), then that would certainly work.

Should it exit testing, the changes that are in-progress for RPi.GPIO-Odroid add a small wrinkle. On the Odroid N2 and C4 boards, that "RPi.GPIO" would itself be capable of hardware PWM. The choice between sysfs and RPi.GPIO doesn't seem as obvious, to me, in that scenario.

rm-hull / luma.lcd