Hi, On Thu, Oct 26, 2023 at 7:37 AM Jonas Mark (BT-FS/ENG1-GRB) <Mark.Jonas@xxxxxxxxxxxx> wrote: > > Hi, > > We have a parallel LCD panel which is driven by panel/panel-simple. The power-off sequence specified in the datasheet requires that the enable-gpio must be deasserted for a number of VSYNC cycles before shutting down all other control signals. See the diagram below: > __ __ __ __ __ > CLK, VSYNC, DE, HSYNC: __><__><__><__><__\_____________________ > ______ > enable-gpio : \_________________________________ > > So far, in kernel 5.4 we relied on the unprepare delay time for making sure that the enable-gpio timing requirements are fulfilled. That is, the > panel_simple_unprepare() would: > > 1. Deassert the enable-gpio > 2. Switch off the voltage regulator > 3. Wait display_desc.delay.unprepare milliseconds > > Afterwards the IPU was shutdown, and all the control signals stopped. > > But with the below commits: > > - 3235b0f20a0a4135e9053f1174d096eff166d0fb > "drm/panel: panel-simple: Use runtime pm to avoid excessive unprepare / prepare" > - e5e30dfcf3db1534019c40d94ed58fd2869f9359 > "drm: panel: simple: Defer unprepare delay till next prepare to shorten it" > > The enable-gpio is now deasserted in panel_simple_suspend(), which is called some time after the disablement of control signals are stopped: > __ __ __ __ __ > CLK, VSYNC, DE, HSYNC: __><__><__><__><__\_____________________ > __________________________ > enable-gpio : \_____________ > > With the latest panel-simple, is there a way which allows us to deassert enable-gpio before the control signals stop? As I understood it, the "unprepare" time was originally intended to meet minimum power off timings and that's how I always saw it used, but it doesn't totally surprise me that there was someone relying on the old behavior. I personally wouldn't object to adding another field to panel-simple that allowed you to get the delay you needed and then change your panel details to use that new field instead of the "unprepare" milliseconds. ...or you could rename the current "unprepare" delay to something like "min_poweroff" and then re-introduce an "unprepare" delay that does what you want. Oh! ...but even this won't _really_ do what you want, right? The bigger issue here is that panel-simple is using auto-suspend now and thus the enable line can go off much, much later. What it kind-of sounds like is that you want the "enable" GPIO to be controlled by the "enable" and "disable" functions of panel-simple. Then you could use the "disable" delay, right? I think I've looked at this exact case before and then realized that there's a better solution. At least in all cases I looked at the "enable-gpio" you're talking about was actually better modeled as a _backlight_ enable GPIO. The "backlight" is turned off before panel-simple's disable() function is called (see drm_panel_disable(). So if you move the GPIO to the backlight and add a "disable" delay then you're all set. Does that work for you? Does it make sense for this GPIO to be modeled as a backlight GPIO? -Doug
