Message ID | 20210823115125.1070257-1-yangcong5@huaqin.corp-partner.google.com (mailing list archive) |
---|---|
Headers | show |
Series | Add driver for BOE tv110c9m-ll3 panel | expand |
Hi yangcong, On Mon, Aug 23, 2021 at 07:51:23PM +0800, yangcong wrote: > Documentation/devicetree/bindings/display/panel/boe,tv110c9m-ll3.yaml: > > Compared with v1, add a space in the required list. > > yangcong (2): > drm/panel: support for BOE tv1110c9m-ll3 wuxga dsi video mode panel Can you help me understand why this needs a new driver and is not just added to the already existing panel-boe-tv101wum-nl6.c? Thanks in advance, Sam
Hi Sam, This driver support boe tv1110c9m-ll3 and inx hj110iz-01a panel.The IC chip is used NT36523, which is a new IC. 1: panel-boe-tv101wum-nl6.c driver cannot meet the timing requirements of the current two panel. 2: The screen cannot be work in HS mode, panel-boe-tv101wum-nl6.c will switch to HS mode when boe_panel_enter_sleep_mode. static int boe_panel_enter_sleep_mode(struct boe_panel *boe) { struct mipi_dsi_device *dsi = boe->dsi; int ret; dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; ... ... } thanks.
Hi, On Mon, Aug 23, 2021 at 6:41 PM yangcong <yangcong5@huaqin.corp-partner.google.com> wrote: > > Hi Sam, > This driver support boe tv1110c9m-ll3 and inx hj110iz-01a panel.The IC chip is used NT36523, which is a new IC. > > 1: panel-boe-tv101wum-nl6.c driver cannot meet the timing requirements of the current two panel. > > 2: The screen cannot be work in HS mode, panel-boe-tv101wum-nl6.c will switch to HS mode when boe_panel_enter_sleep_mode. > > static int boe_panel_enter_sleep_mode(struct boe_panel *boe) > { > struct mipi_dsi_device *dsi = boe->dsi; > int ret; > > dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; > ... > ... > } It's really up to Sam how he wants to proceed here, but certainly you could support things with the existing driver even if there are differences. In general you can add more things to the `struct panel_desc` in the driver and then make them different for your panel. Look, for instance, at `discharge_on_disable`. Not all panels supported by this driver do that, so you could support your "cannot work in HS mode" in a similar way. For the timings, you could also add another bit to the `struct panel_desc` to select a different delay for your panel or (if it's just a small delay) you could just increase it across the board. I guess you need a 10 ms delay instead of a 5 ms delay in probe? I'd just make it 10 ms across the board and call it done. Similarly looks like something needs .5 ms => 1 ms. Again, this is likely fine across the board for all panels. Unless Sam comes back and says "no, wait, keep it two drivers!" then I'd suggest that you post a new version that works as Sam suggests. In the worst case if having it combined into one driver looks too ugly then we can always go back to a split driver. -Doug
Hi, On Thu, Aug 26, 2021 at 10:17 AM Doug Anderson <dianders@google.com> wrote: > > Hi, > > On Mon, Aug 23, 2021 at 6:41 PM yangcong > <yangcong5@huaqin.corp-partner.google.com> wrote: > > > > Hi Sam, > > This driver support boe tv1110c9m-ll3 and inx hj110iz-01a panel.The IC chip is used NT36523, which is a new IC. > > > > 1: panel-boe-tv101wum-nl6.c driver cannot meet the timing requirements of the current two panel. > > > > 2: The screen cannot be work in HS mode, panel-boe-tv101wum-nl6.c will switch to HS mode when boe_panel_enter_sleep_mode. > > > > static int boe_panel_enter_sleep_mode(struct boe_panel *boe) > > { > > struct mipi_dsi_device *dsi = boe->dsi; > > int ret; > > > > dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; > > ... > > ... > > } > > It's really up to Sam how he wants to proceed here, but certainly you > could support things with the existing driver even if there are > differences. In general you can add more things to the `struct > panel_desc` in the driver and then make them different for your panel. > Look, for instance, at `discharge_on_disable`. Not all panels > supported by this driver do that, so you could support your "cannot > work in HS mode" in a similar way. > > For the timings, you could also add another bit to the `struct > panel_desc` to select a different delay for your panel or (if it's > just a small delay) you could just increase it across the board. I > guess you need a 10 ms delay instead of a 5 ms delay in probe? I'd > just make it 10 ms across the board and call it done. Similarly looks > like something needs .5 ms => 1 ms. Again, this is likely fine across > the board for all panels. > > Unless Sam comes back and says "no, wait, keep it two drivers!" then > I'd suggest that you post a new version that works as Sam suggests. In > the worst case if having it combined into one driver looks too ugly > then we can always go back to a split driver. Breadcrumbs: it looks like yangcong has attempted this in the confusingly numbered v3 of his other patch series. See: https://lore.kernel.org/r/20210827082407.101053-4-yangcong5@huaqin.corp-partner.google.com -Doug