| Message ID | 20220217002530.396563-5-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: bridge: icn6211: Fix hard-coded panel settings and add I2C support | expand |
On Thu, Feb 17, 2022 at 01:25:22AM +0100, Marek Vasut wrote: > The driver currently hard-codes DSI lane count to two, however the chip > is capable of operating in 1..4 DSI lanes mode. Parse 'data-lanes' DT > property and program the result into DSI_CTRL register. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Jagan Teki <jagan@amarulasolutions.com> > Cc: Maxime Ripard <maxime@cerno.tech> > Cc: Robert Foss <robert.foss@linaro.org> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > To: dri-devel@lists.freedesktop.org > --- > V2: Rebase on next-20220214 > --- > drivers/gpu/drm/bridge/chipone-icn6211.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c > index 2ac8eb7e25f52..7c013a08c7b00 100644 > --- a/drivers/gpu/drm/bridge/chipone-icn6211.c > +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c > @@ -136,10 +136,12 @@ struct chipone { > struct drm_bridge bridge; > struct drm_display_mode mode; > struct drm_bridge *panel_bridge; > + struct device_node *host_node; > struct gpio_desc *enable_gpio; > struct regulator *vdd1; > struct regulator *vdd2; > struct regulator *vdd3; > + int dsi_lanes; > }; > > static inline struct chipone *bridge_to_chipone(struct drm_bridge *bridge) > @@ -212,6 +214,11 @@ static void chipone_atomic_enable(struct drm_bridge *bridge, > /* dsi specific sequence */ > ICN6211_DSI(icn, SYNC_EVENT_DLY, 0x80); > ICN6211_DSI(icn, HFP_MIN, hfp & 0xff); > + > + /* DSI data lane count */ > + ICN6211_DSI(icn, DSI_CTRL, > + DSI_CTRL_UNKNOWN | DSI_CTRL_DSI_LANES(icn->dsi_lanes - 1)); > + > ICN6211_DSI(icn, MIPI_PD_CK_LANE, 0xa0); > ICN6211_DSI(icn, PLL_CTRL(12), 0xff); > > @@ -314,6 +321,7 @@ static const struct drm_bridge_funcs chipone_bridge_funcs = { > static int chipone_parse_dt(struct chipone *icn) > { > struct device *dev = icn->dev; > + struct device_node *endpoint; > struct drm_panel *panel; > int ret; > > @@ -350,6 +358,16 @@ static int chipone_parse_dt(struct chipone *icn) > return PTR_ERR(icn->enable_gpio); > } > > + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0); > + icn->dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); The binding must be amended to allow for the usage of data-lanes, and you need to keep the previous value as default for older device trees Maxime
On 3/2/22 11:01, Maxime Ripard wrote: > On Thu, Feb 17, 2022 at 01:25:22AM +0100, Marek Vasut wrote: >> The driver currently hard-codes DSI lane count to two, however the chip >> is capable of operating in 1..4 DSI lanes mode. Parse 'data-lanes' DT >> property and program the result into DSI_CTRL register. >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> Cc: Jagan Teki <jagan@amarulasolutions.com> >> Cc: Maxime Ripard <maxime@cerno.tech> >> Cc: Robert Foss <robert.foss@linaro.org> >> Cc: Sam Ravnborg <sam@ravnborg.org> >> Cc: Thomas Zimmermann <tzimmermann@suse.de> >> To: dri-devel@lists.freedesktop.org >> --- >> V2: Rebase on next-20220214 >> --- >> drivers/gpu/drm/bridge/chipone-icn6211.c | 21 ++++++++++++++++++++- >> 1 file changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c >> index 2ac8eb7e25f52..7c013a08c7b00 100644 >> --- a/drivers/gpu/drm/bridge/chipone-icn6211.c >> +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c >> @@ -136,10 +136,12 @@ struct chipone { >> struct drm_bridge bridge; >> struct drm_display_mode mode; >> struct drm_bridge *panel_bridge; >> + struct device_node *host_node; >> struct gpio_desc *enable_gpio; >> struct regulator *vdd1; >> struct regulator *vdd2; >> struct regulator *vdd3; >> + int dsi_lanes; >> }; >> >> static inline struct chipone *bridge_to_chipone(struct drm_bridge *bridge) >> @@ -212,6 +214,11 @@ static void chipone_atomic_enable(struct drm_bridge *bridge, >> /* dsi specific sequence */ >> ICN6211_DSI(icn, SYNC_EVENT_DLY, 0x80); >> ICN6211_DSI(icn, HFP_MIN, hfp & 0xff); >> + >> + /* DSI data lane count */ >> + ICN6211_DSI(icn, DSI_CTRL, >> + DSI_CTRL_UNKNOWN | DSI_CTRL_DSI_LANES(icn->dsi_lanes - 1)); >> + >> ICN6211_DSI(icn, MIPI_PD_CK_LANE, 0xa0); >> ICN6211_DSI(icn, PLL_CTRL(12), 0xff); >> >> @@ -314,6 +321,7 @@ static const struct drm_bridge_funcs chipone_bridge_funcs = { >> static int chipone_parse_dt(struct chipone *icn) >> { >> struct device *dev = icn->dev; >> + struct device_node *endpoint; >> struct drm_panel *panel; >> int ret; >> >> @@ -350,6 +358,16 @@ static int chipone_parse_dt(struct chipone *icn) >> return PTR_ERR(icn->enable_gpio); >> } >> >> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0); >> + icn->dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); > > The binding must be amended to allow for the usage of data-lanes, and > you need to keep the previous value as default for older device trees Regarding the default value -- there are no in-tree users of this driver yet (per git grep in current linux-next), do we really care about backward compatibility in this case?
On Wed, Mar 02, 2022 at 04:17:04PM +0100, Marek Vasut wrote: > On 3/2/22 11:01, Maxime Ripard wrote: > > On Thu, Feb 17, 2022 at 01:25:22AM +0100, Marek Vasut wrote: > > > The driver currently hard-codes DSI lane count to two, however the chip > > > is capable of operating in 1..4 DSI lanes mode. Parse 'data-lanes' DT > > > property and program the result into DSI_CTRL register. > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > Cc: Jagan Teki <jagan@amarulasolutions.com> > > > Cc: Maxime Ripard <maxime@cerno.tech> > > > Cc: Robert Foss <robert.foss@linaro.org> > > > Cc: Sam Ravnborg <sam@ravnborg.org> > > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > > To: dri-devel@lists.freedesktop.org > > > --- > > > V2: Rebase on next-20220214 > > > --- > > > drivers/gpu/drm/bridge/chipone-icn6211.c | 21 ++++++++++++++++++++- > > > 1 file changed, 20 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c > > > index 2ac8eb7e25f52..7c013a08c7b00 100644 > > > --- a/drivers/gpu/drm/bridge/chipone-icn6211.c > > > +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c > > > @@ -136,10 +136,12 @@ struct chipone { > > > struct drm_bridge bridge; > > > struct drm_display_mode mode; > > > struct drm_bridge *panel_bridge; > > > + struct device_node *host_node; > > > struct gpio_desc *enable_gpio; > > > struct regulator *vdd1; > > > struct regulator *vdd2; > > > struct regulator *vdd3; > > > + int dsi_lanes; > > > }; > > > static inline struct chipone *bridge_to_chipone(struct drm_bridge *bridge) > > > @@ -212,6 +214,11 @@ static void chipone_atomic_enable(struct drm_bridge *bridge, > > > /* dsi specific sequence */ > > > ICN6211_DSI(icn, SYNC_EVENT_DLY, 0x80); > > > ICN6211_DSI(icn, HFP_MIN, hfp & 0xff); > > > + > > > + /* DSI data lane count */ > > > + ICN6211_DSI(icn, DSI_CTRL, > > > + DSI_CTRL_UNKNOWN | DSI_CTRL_DSI_LANES(icn->dsi_lanes - 1)); > > > + > > > ICN6211_DSI(icn, MIPI_PD_CK_LANE, 0xa0); > > > ICN6211_DSI(icn, PLL_CTRL(12), 0xff); > > > @@ -314,6 +321,7 @@ static const struct drm_bridge_funcs chipone_bridge_funcs = { > > > static int chipone_parse_dt(struct chipone *icn) > > > { > > > struct device *dev = icn->dev; > > > + struct device_node *endpoint; > > > struct drm_panel *panel; > > > int ret; > > > @@ -350,6 +358,16 @@ static int chipone_parse_dt(struct chipone *icn) > > > return PTR_ERR(icn->enable_gpio); > > > } > > > + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0); > > > + icn->dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); > > > > The binding must be amended to allow for the usage of data-lanes, and > > you need to keep the previous value as default for older device trees > > Regarding the default value -- there are no in-tree users of this driver yet > (per git grep in current linux-next), do we really care about backward > compatibility in this case? If it hasn't been in a stable release yet, no. If it did, yes Maxime
On 3/3/22 13:54, Maxime Ripard wrote: [...] >> Regarding the default value -- there are no in-tree users of this driver yet >> (per git grep in current linux-next), do we really care about backward >> compatibility in this case? > > If it hasn't been in a stable release yet, no. If it did, yes It was in a stable release, V3 is out.
diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c index 2ac8eb7e25f52..7c013a08c7b00 100644 --- a/drivers/gpu/drm/bridge/chipone-icn6211.c +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c @@ -136,10 +136,12 @@ struct chipone { struct drm_bridge bridge; struct drm_display_mode mode; struct drm_bridge *panel_bridge; + struct device_node *host_node; struct gpio_desc *enable_gpio; struct regulator *vdd1; struct regulator *vdd2; struct regulator *vdd3; + int dsi_lanes; }; static inline struct chipone *bridge_to_chipone(struct drm_bridge *bridge) @@ -212,6 +214,11 @@ static void chipone_atomic_enable(struct drm_bridge *bridge, /* dsi specific sequence */ ICN6211_DSI(icn, SYNC_EVENT_DLY, 0x80); ICN6211_DSI(icn, HFP_MIN, hfp & 0xff); + + /* DSI data lane count */ + ICN6211_DSI(icn, DSI_CTRL, + DSI_CTRL_UNKNOWN | DSI_CTRL_DSI_LANES(icn->dsi_lanes - 1)); + ICN6211_DSI(icn, MIPI_PD_CK_LANE, 0xa0); ICN6211_DSI(icn, PLL_CTRL(12), 0xff); @@ -314,6 +321,7 @@ static const struct drm_bridge_funcs chipone_bridge_funcs = { static int chipone_parse_dt(struct chipone *icn) { struct device *dev = icn->dev; + struct device_node *endpoint; struct drm_panel *panel; int ret; @@ -350,6 +358,16 @@ static int chipone_parse_dt(struct chipone *icn) return PTR_ERR(icn->enable_gpio); } + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0); + icn->dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes"); + icn->host_node = of_graph_get_remote_port_parent(endpoint); + of_node_put(endpoint); + + if (icn->dsi_lanes < 0 || icn->dsi_lanes > 4) + return -EINVAL; + if (!icn->host_node) + return -ENODEV; + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, NULL); if (ret) return ret; @@ -384,7 +402,7 @@ static int chipone_probe(struct mipi_dsi_device *dsi) drm_bridge_add(&icn->bridge); - dsi->lanes = 4; + dsi->lanes = icn->dsi_lanes; dsi->format = MIPI_DSI_FMT_RGB888; dsi->mode_flags = MIPI_DSI_MODE_VIDEO_SYNC_PULSE; @@ -403,6 +421,7 @@ static int chipone_remove(struct mipi_dsi_device *dsi) mipi_dsi_detach(dsi); drm_bridge_remove(&icn->bridge); + of_node_put(icn->host_node); return 0; }
The driver currently hard-codes DSI lane count to two, however the chip is capable of operating in 1..4 DSI lanes mode. Parse 'data-lanes' DT property and program the result into DSI_CTRL register. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Jagan Teki <jagan@amarulasolutions.com> Cc: Maxime Ripard <maxime@cerno.tech> Cc: Robert Foss <robert.foss@linaro.org> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> To: dri-devel@lists.freedesktop.org --- V2: Rebase on next-20220214 --- drivers/gpu/drm/bridge/chipone-icn6211.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)