Re: [PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

2019-01-12 Thread Sam Ravnborg
Hi Jagan. > When did .unprepare and .disable are actually called? I turn-off the > backlight by echo 1 > /sys/class/backlight/backlight/bl_power and even > power of the board I assume the video transmission stop so it would > ended-up calling these, but I couldn't see prints. does it the same >

Re: [PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

2019-01-12 Thread Jagan Teki
Hi Sam, On Sat, Jan 12, 2019 at 9:03 PM Sam Ravnborg wrote: > > Hi Jagan. > > > > But as we just assert reset (set it to 0), this timing constraint can be > > > ignored. > > > > But we unaware of reset pulse duration right? it's the hardware that > > bring the reset assert if we set the line 0.

Re: [PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

2019-01-12 Thread Sam Ravnborg
Hi Jagan. > > But as we just assert reset (set it to 0), this timing constraint can be > > ignored. > > But we unaware of reset pulse duration right? it's the hardware that > bring the reset assert if we set the line 0. am I correct or do we > need to explicitly wait 10us after reset initiated?

Re: [PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

2019-01-12 Thread Jagan Teki
On Sat, Jan 12, 2019 at 5:31 PM Sam Ravnborg wrote: > > > > > > > > > > + > > > > > > + msleep(st7701->sleep_delay); > > > > > > + > > > > > > + gpiod_set_value(st7701->reset, 0); > > > > > > + > > > > > > + gpiod_set_value(st7701->reset, 1); > > > > > > + > > > > > > +

Re: [PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

2019-01-12 Thread Sam Ravnborg
> > > > > > > + > > > > > + msleep(st7701->sleep_delay); > > > > > + > > > > > + gpiod_set_value(st7701->reset, 0); > > > > > + > > > > > + gpiod_set_value(st7701->reset, 1); > > > > > + > > > > > + gpiod_set_value(st7701->reset, 0); > > > > No timing constrains here? In prepare

Re: [PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

2019-01-12 Thread Jagan Teki
On Sat, Jan 12, 2019 at 3:44 PM Sam Ravnborg wrote: > > Hi Jagan. > > > On Sat, Jan 12, 2019 at 2:49 AM Sam Ravnborg wrote: > > > > > > Hi Jagan. > > > > > > Gave this another more detailed read - triggered some additional comments. > > > Depite the comments it looks good, and this is all more

Re: [PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

2019-01-12 Thread Sam Ravnborg
Hi Jagan. > On Sat, Jan 12, 2019 at 2:49 AM Sam Ravnborg wrote: > > > > Hi Jagan. > > > > Gave this another more detailed read - triggered some additional comments. > > Depite the comments it looks good, and this is all more or > > less cosmetic improvements. > > Thanks for the review. > > > >

Re: [PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

2019-01-11 Thread Jagan Teki
On Sat, Jan 12, 2019 at 2:49 AM Sam Ravnborg wrote: > > Hi Jagan. > > Gave this another more detailed read - triggered some additional comments. > Depite the comments it looks good, and this is all more or > less cosmetic improvements. Thanks for the review. > > Sam > > > +struct

Re: [PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

2019-01-11 Thread Noralf Trønnes
Den 11.01.2019 22.19, skrev Sam Ravnborg: > Hi Jagan. > > Gave this another more detailed read - triggered some additional comments. > Depite the comments it looks good, and this is all more or > less cosmetic improvements. > > Sam > >> +struct st7701_panel_desc { >> +const struct

Re: [PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

2019-01-11 Thread Sam Ravnborg
Hi Jagan. Gave this another more detailed read - triggered some additional comments. Depite the comments it looks good, and this is all more or less cosmetic improvements. Sam > +struct st7701_panel_desc { > + const struct drm_display_mode *mode; > + unsigned int lanes; > +

[PATCH V7 2/2] drm/panel: Add Sitronix ST7701 panel driver

2019-01-11 Thread Jagan Teki
ST7701 designed for small and medium sizes of TFT LCD display, is capable of supporting up to 480RGBX864 in resolution. It provides several system interfaces like MIPI/RGB/SPI. Currently added support for Techstar TS8550B which is ST7701 based 480x854, 2-lane MIPI DSI LCD panel. Driver now