On Tue, Apr 22, 2014 at 08:06:19PM +0530, Ajay kumar wrote: > Hi Thierry, > > > On Tue, Apr 22, 2014 at 1:49 PM, Thierry Reding <thierry.reding at gmail.com> > wrote: > > On Tue, Apr 22, 2014 at 04:09:11AM +0530, Ajay Kumar wrote: > >> Most of the panels need an init sequence as mentioned below: > >> -- poweron LCD unit/LCD_EN > >> -- start video data > >> -- poweron LED unit/BL_EN > >> And, a de-init sequence as mentioned below: > >> -- poweroff LED unit/BL_EN > >> -- stop video data > >> -- poweroff LCD unit/LCD_EN > >> With existing callbacks for drm panel, we cannot accomodate such panels, > >> since only two callbacks, i.e "panel_enable" and panel_disable are > supported. > >> > >> This patch adds: > >> -- "pre_enable" callback which can be called before > >> the actual video data is on, and then call the "enable" > >> callback after the video data is available. > >> > >> -- "post_disable" callback which can be called after > >> the video data is off, and use "disable" callback > >> to do something before switching off the video data. > >> > >> Now, we can easily map the above scenario as shown below: > >> poweron LCD unit/LCD_EN = "pre_enable" callback > >> poweron LED unit/BL_EN = "enable" callback > >> poweroff LED unit/BL_EN = "disable" callback > >> poweroff LCD unit/LCD_EN = "post_disable" callback > > > > I don't like this. What happens when the next panel comes around that > > has a yet slightly different requirement? Will we introduce a new > > pre_pre_enable() and post_post_disable() function then? > > > As I have already explained, these 2 callbacks are sufficient enough to > take care > the power up/down sequence for LVDS and eDP panels. And, definitely having > just 2 callbacks "enable" and "disable" is not at all sufficient. > > I initially thought of using panel_simple_enable from panel-simple driver. > But it doesn't go well with case wherein there are 2 regulators(one for LCD > and one for LED) > a BL_EN signal etc. And, often(Believe me, I have referred to both eDP > panel datasheets > and LVDS panel datasheets) proper powerup sequence for such panels is as > mentioned below: > > powerup: > -- power up the supply (LCD_VCC). > -- start video data. > -- enable backlight. > > powerdown > -- disable backlight. > -- stop video data. > -- power off the supply (LCD VCC) > > For the above cases, if I have to somehow fit in all the required settings, > it breaks the sequence and I end up getting glitches during bootup/DPMS. > > Also, when the drm_bridge can support pre_enable and post_disable, why not > drm_panel provide that both should work in tandem?
Imo this makes sense, especially if we go through with the idea talked about yesterday of creating a drm_bridge to wrap-up a drm_panel with sufficient isolation between all components. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
