mbox series

[v2,0/2] Add driver for BOE tv110c9m-ll3 panel

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

Message

cong yang Aug. 23, 2021, 11:51 a.m. UTC
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
  dt-bindngs: display: panel: Add BOE tv110c9m-ll3 panel bindings

 .../display/panel/boe,tv110c9m-ll3.yaml       |   83 ++
 drivers/gpu/drm/panel/Kconfig                 |   10 +
 drivers/gpu/drm/panel/Makefile                |    1 +
 drivers/gpu/drm/panel/panel-boe-tv110c9m.c    | 1303 +++++++++++++++++
 4 files changed, 1397 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/boe,tv110c9m-ll3.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-boe-tv110c9m.c

Comments

Sam Ravnborg Aug. 23, 2021, 3:37 p.m. UTC | #1
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
cong yang Aug. 24, 2021, 1:41 a.m. UTC | #2
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.
Doug Anderson Aug. 26, 2021, 5:17 p.m. UTC | #3
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
Doug Anderson Aug. 27, 2021, 1:47 p.m. UTC | #4
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