Improvments for tc358775 with support for tc358765
Subsystems (4)
v5 cover Improvments for tc358775 with support for tc358765
Hi all,
Here are v5 patches to improve tc358775 driver and add support for tc358765.
Regards,
Tony
Changes since v4:
- Update devicetree binding to move allOf after required block as noted by Krzysztof
Changes since v3:
- Fixed dts binding warnings
Changes since v2:
- Only make stby-gpios optional for tc358775, and disallow them for tc358765 as noted by Krzysztof
- Added additionalProperties: false for port-base as noted by Krzysztof
- Updated patch description for why there can be a data-lanes property for both the DSI host and the bridge as noted by Dmitry
- Improved the old dtb data-lanes warning as suggested by Michael
- Fix warning on casting of_device_get_match_data() as noted by the kernel test robot
Changes since v1:
- After a brief offline discussion with Michael, merge series with Michael's patch series to make stby gpio and supplies optional as they may be hardwired
- Use Michael's better patch for the jeida timings change
- Parse lanes on the bridge side like other bridge devices do, and if not found, also parse on the DSI host side and warn
Michael Walle (3):
dt-bindings: display: bridge: tc358775: make stby gpio optional drm/bridge: tc358775: fix support for jeida-18 and jeida-24 drm/bridge: tc358775: make standby GPIO optional
Tony Lindgren (7):
dt-bindings: display: bridge: tc358775: Add data-lanes
dt-bindings: display: bridge: tc358775: Add support for tc358765 drm/bridge: tc358775: Get bridge data lanes instead of the DSI host
lanes
drm/bridge: tc358775: Add burst and low-power modes drm/bridge: tc358775: Enable pre_enable_prev_first flag drm/bridge: tc358775: Add support for tc358765 drm/bridge: tc358775: Configure hs_rate and lp_rate
.../display/bridge/toshiba,tc358775.yaml | 39 ++++++-- drivers/gpu/drm/bridge/tc358775.c | 98 ++++++++++++------- 2 files changed, 94 insertions(+), 43 deletions(-)
Signature · 1 line
2.43.1
PATCH v5 1/10 dt-bindings: display: bridge: tc358775: make stby gpio optional
From: Michael Walle <[email protected]>
For a normal operation, the stby GPIO is not needed.
The reset pin is required because once the PPI (PHY protocol interface) is started, it can only be stopped by asserting the reset pin.
--- .../devicetree/bindings/display/bridge/toshiba,tc358775.yaml | 1 - 1 file changed, 1 deletion(-)
Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml +0 -1
@@ -70,7 +70,6 @@ required: - reg - vdd-supply - vddio-supply - - stby-gpios - reset-gpios - ports
Signature · 1 line
2.43.1
PATCH v5 2/10 dt-bindings: display: bridge: tc358775: Add data-lanes
In reply to Tony LindgrenThe device uses a clock lane, and 1 to 4 DSI data lanes. Let's add the data-lanes property starting at 1 similar to what the other bridge bindings are doing.
Let's also drop the data-lanes properties in the example for the DSI host controller to avoid confusion. The configuration of the DSI host depends on the controller used and is unrelated to the bridge binding.
--- .../display/bridge/toshiba,tc358775.yaml | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml +19 -3
@@ -46,11 +46,27 @@ properties: properties: port@0: - $ref: /schemas/graph.yaml#/properties/port + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false description: | DSI Input. The remote endpoint phandle should be a reference to a valid mipi_dsi_host device node. + properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + properties: + data-lanes: + description: array of physical DSI data lane indexes. + minItems: 1 + items: + - const: 1 + - const: 2 + - const: 3 + - const: 4 + port@1: $ref: /schemas/graph.yaml#/properties/port description: |
@@ -107,6 +123,7 @@ examples: reg = <0>; d2l_in_test: endpoint { remote-endpoint = <&dsi0_out>; + data-lanes = <1 2 3 4>; }; };
@@ -131,7 +148,6 @@ examples: reg = <1>; dsi0_out: endpoint { remote-endpoint = <&d2l_in_test>; - data-lanes = <0 1 2 3>; }; }; };
@@ -166,6 +182,7 @@ examples: reg = <0>; d2l_in_dual: endpoint { remote-endpoint = <&dsi0_out_dual>; + data-lanes = <1 2 3 4>; }; };
@@ -197,7 +214,6 @@ examples: reg = <1>; dsi0_out_dual: endpoint { remote-endpoint = <&d2l_in_dual>; - data-lanes = <0 1 2 3>; }; }; };
Signature · 1 line
2.43.1
PATCH v5 3/10 dt-bindings: display: bridge: tc358775: Add support for tc358765
In reply to Tony LindgrenThe tc358765 is similar to tc358775. The tc358765 just an earlier version of the hardware, and it's pin and register compatible with tc358775 for most part.
From the binding point of view the only difference is that the tc358765 does not have stdby-gpios.
--- .../display/bridge/toshiba,tc358775.yaml | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/display/bridge/toshiba,tc358775.yaml +14 -2
@@ -10,7 +10,7 @@ maintainers: - Vinay Simha BN <[email protected]> description: | - This binding supports DSI to LVDS bridge TC358775 + This binding supports DSI to LVDS bridges TC358765 and TC358775 MIPI DSI-RX Data 4-lane, CLK 1-lane with data rates up to 800 Mbps/lane. Video frame size:
@@ -21,7 +21,9 @@ description: | properties: compatible: - const: toshiba,tc358775 + enum: + - toshiba,tc358765 + - toshiba,tc358775 reg: maxItems: 1
@@ -89,6 +91,16 @@ required: - reset-gpios - ports +allOf: + - if: + properties: + compatible: + contains: + const: toshiba,tc358765 + then: + properties: + stby-gpios: false + additionalProperties: false examples:
Signature · 1 line
2.43.1
PATCH v5 4/10 drm/bridge: tc358775: fix support for jeida-18 and jeida-24
In reply to Tony LindgrenFrom: Michael Walle <[email protected]>
The bridge always uses 24bpp internally. Therefore, for jeida-18 mapping we need to discard the lowest two bits for each channel and thus starting with LV_[RGB]2. jeida-24 has the same mapping but uses four lanes instead of three, with the forth pair transmitting the lowest two bits of each channel. Thus, the mapping between jeida-18 and jeida-24 is actually the same, except that one channel is turned off (by selecting the RGB666 format in VPCTRL).
While at it, remove the bogus comment about the hardware default because the default is overwritten in any case.
Tested with a jeida-18 display (Evervision VGG644804).
--- drivers/gpu/drm/bridge/tc358775.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-)
drivers/gpu/drm/bridge/tc358775.c +9 -12
@@ -454,10 +454,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)dev_dbg (tc->dev,"bus_formats %04x bpc %d\n" , connector->display_info.bus_formats[0 ], tc->bpc); -/* -* Default hardware register settings of tc358775 configured -* with MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA jeida-24 format -*/ if (connector->display_info.bus_formats[0 ] ==MEDIA_BUS_FMT_RGB888_1X7X4_SPWG ) {/* VESA-24 */
@@ -468,14 +464,15 @@ static void tc_bridge_enable(struct drm_bridge *bridge)d2l_write (tc->i2c,LV_MX1619 ,LV_MX (LVI_B6 ,LVI_B7 ,LVI_B1 ,LVI_B2 ));d2l_write (tc->i2c,LV_MX2023 ,LV_MX (LVI_B3 ,LVI_B4 ,LVI_B5 ,LVI_L0 ));d2l_write (tc->i2c,LV_MX2427 ,LV_MX (LVI_HS ,LVI_VS ,LVI_DE ,LVI_R6 )); - }else {/* MEDIA_BUS_FMT_RGB666_1X7X3_SPWG - JEIDA-18 */ -d2l_write (tc->i2c,LV_MX0003 ,LV_MX (LVI_R0 ,LVI_R1 ,LVI_R2 ,LVI_R3 )); -d2l_write (tc->i2c,LV_MX0407 ,LV_MX (LVI_R4 ,LVI_L0 ,LVI_R5 ,LVI_G0 )); -d2l_write (tc->i2c,LV_MX0811 ,LV_MX (LVI_G1 ,LVI_G2 ,LVI_L0 ,LVI_L0 )); -d2l_write (tc->i2c,LV_MX1215 ,LV_MX (LVI_G3 ,LVI_G4 ,LVI_G5 ,LVI_B0 )); -d2l_write (tc->i2c,LV_MX1619 ,LV_MX (LVI_L0 ,LVI_L0 ,LVI_B1 ,LVI_B2 )); -d2l_write (tc->i2c,LV_MX2023 ,LV_MX (LVI_B3 ,LVI_B4 ,LVI_B5 ,LVI_L0 )); -d2l_write (tc->i2c,LV_MX2427 ,LV_MX (LVI_HS ,LVI_VS ,LVI_DE ,LVI_L0 )); + }else { +/* JEIDA-18 and JEIDA-24 */ +d2l_write (tc->i2c,LV_MX0003 ,LV_MX (LVI_R2 ,LVI_R3 ,LVI_R4 ,LVI_R5 )); +d2l_write (tc->i2c,LV_MX0407 ,LV_MX (LVI_R6 ,LVI_R1 ,LVI_R7 ,LVI_G2 )); +d2l_write (tc->i2c,LV_MX0811 ,LV_MX (LVI_G3 ,LVI_G4 ,LVI_G0 ,LVI_G1 )); +d2l_write (tc->i2c,LV_MX1215 ,LV_MX (LVI_G5 ,LVI_G6 ,LVI_G7 ,LVI_B2 )); +d2l_write (tc->i2c,LV_MX1619 ,LV_MX (LVI_B0 ,LVI_B1 ,LVI_B3 ,LVI_B4 )); +d2l_write (tc->i2c,LV_MX2023 ,LV_MX (LVI_B5 ,LVI_B6 ,LVI_B7 ,LVI_L0 )); +d2l_write (tc->i2c,LV_MX2427 ,LV_MX (LVI_HS ,LVI_VS ,LVI_DE ,LVI_R0 )); }d2l_write (tc->i2c,VFUEN ,VFUEN_EN );
Signature · 1 line
2.43.1
PATCH v5 5/10 drm/bridge: tc358775: make standby GPIO optional
In reply to Tony LindgrenFrom: Michael Walle <[email protected]>
The stby pin is optional. It is only needed for power-up and down sequencing. It is not needed, if the power rails cannot by dynamically enabled.
Because the GPIO is now optional, remove the error message.
--- drivers/gpu/drm/bridge/tc358775.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/gpu/drm/bridge/tc358775.c +3 -6
@@ -669,12 +669,9 @@ static int tc_probe(struct i2c_client *client)return ret; } - tc->stby_gpio =devm_gpiod_get (dev,"stby" ,GPIOD_OUT_HIGH ); -if (IS_ERR (tc->stby_gpio)) { - ret =PTR_ERR (tc->stby_gpio); -dev_err (dev,"cannot get stby-gpio %d\n" , ret); -return ret; - } + tc->stby_gpio =devm_gpiod_get_optional (dev,"stby" ,GPIOD_OUT_HIGH ); +if (IS_ERR (tc->stby_gpio)) +return PTR_ERR (tc->stby_gpio); tc->reset_gpio =devm_gpiod_get (dev,"reset" ,GPIOD_OUT_HIGH );if (IS_ERR (tc->reset_gpio)) {
Signature · 1 line
2.43.1
PATCH v5 6/10 drm/bridge: tc358775: Get bridge data lanes instead of the DSI host lanes
In reply to Tony LindgrenThe current code assumes the data-lanes property is configured on the DSI host side instead of the bridge side, and assumes DSI host endpoint 1.
Let's standardize on what the other bridge drivers are doing and parse the data-lanes property for the bridge. Only if data-lanes property is not found, let's be nice and also check the DSI host for old dtb in use and warn.
And as Dmitry pointed out, the lanes for the host and the bridge may be different because the lanes may be swapped on the host side.
--- drivers/gpu/drm/bridge/tc358775.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-)
drivers/gpu/drm/bridge/tc358775.c +11 -14
@@ -525,27 +525,24 @@ tc_mode_valid(struct drm_bridge *bridge,static int tc358775_parse_dt (struct device_node *np,struct tc_data *tc) {struct device_node *endpoint; -struct device_node *parent;struct device_node *remote;int dsi_lanes =-1 ; -/* -* To get the data-lanes of dsi, we need to access the dsi0_out of port1 -* of dsi0 endpoint from bridge port0 of d2l_in -*/ endpoint =of_graph_get_endpoint_by_regs (tc->dev->of_node,TC358775_DSI_IN ,-1 ); -if (endpoint) { -/* dsi0_out node */ - parent =of_graph_get_remote_port_parent (endpoint); -of_node_put (endpoint); -if (parent) { -/* dsi0 port 1 */ - dsi_lanes =drm_of_get_data_lanes_count_ep (parent,1 ,-1 ,1 ,4 ); -of_node_put (parent); - } + dsi_lanes =drm_of_get_data_lanes_count (endpoint,1 ,4 ); + +/* Quirk old dtb: Use data lanes from the DSI host side instead of bridge */ +if (dsi_lanes == -EINVAL || dsi_lanes == -ENODEV ) { + remote =of_graph_get_remote_endpoint (endpoint); + dsi_lanes =drm_of_get_data_lanes_count (remote,1 ,4 ); +of_node_put (remote); +if (dsi_lanes >=1 ) +dev_warn (tc->dev,"no dsi-lanes for the bridge, using host lanes\n" ); } +of_node_put (endpoint); +if (dsi_lanes <0 )return dsi_lanes;
Signature · 1 line
2.43.1
PATCH v5 7/10 drm/bridge: tc358775: Add burst and low-power modes
In reply to Tony LindgrenBurst and low-power modes are supported both for tc358765 and tc358775.
--- drivers/gpu/drm/bridge/tc358775.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/gpu/drm/bridge/tc358775.c +2 -1
@@ -619,7 +619,8 @@ static int tc_attach_host(struct tc_data *tc) dsi->lanes = tc->num_dsi_lanes; dsi->format =MIPI_DSI_FMT_RGB888 ; - dsi->mode_flags =MIPI_DSI_MODE_VIDEO ; + dsi->mode_flags =MIPI_DSI_MODE_VIDEO |MIPI_DSI_MODE_VIDEO_BURST | +MIPI_DSI_MODE_LPM ; ret =devm_mipi_dsi_attach (dev, dsi);if (ret <0 ) {
Signature · 1 line
2.43.1
PATCH v5 8/10 drm/bridge: tc358775: Enable pre_enable_prev_first flag
In reply to Tony LindgrenSet pre_enable_prev_first to ensure the previous bridge is enabled first.
--- drivers/gpu/drm/bridge/tc358775.c | 1 + 1 file changed, 1 insertion(+)
drivers/gpu/drm/bridge/tc358775.c +1 -0
@@ -680,6 +680,7 @@ static int tc_probe(struct i2c_client *client) tc->bridge.funcs = &tc_bridge_funcs; tc->bridge.of_node = dev->of_node; + tc->bridge.pre_enable_prev_first = true;drm_bridge_add (&tc->bridge);i2c_set_clientdata (client, tc);
Signature · 1 line
2.43.1
PATCH v5 9/10 drm/bridge: tc358775: Add support for tc358765
In reply to Tony LindgrenThe tc358775 bridge is pin compatible with earlier tc358765 according to the tc358774xbg_datasheet_en_20190118.pdf documentation. Compared to the tc358765, the tc358775 supports a STBY GPIO and higher data rates.
The tc358765 has a register bit for video event mode vs video pulse mode. We must set it to video event mode for the LCD output to work, and on the tc358775, this bit no longer exists.
Looks like the registers seem to match otherwise based on a quick glance comparing the defines to the earlier Android kernel tc358765 driver.
--- drivers/gpu/drm/bridge/tc358775.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-)
drivers/gpu/drm/bridge/tc358775.c +22 -4
@@ -15,6 +15,7 @@#include <linux/kernel.h> #include <linux/media-bus-format.h> #include <linux/module.h> +#include <linux/of_device.h> #include <linux/regulator/consumer.h> #include <linux/slab.h>
@@ -107,6 +108,7 @@#define RDPKTLN 0x0404/* Command Read Packet Length */ #define VPCTRL 0x0450/* Video Path Control */ +#define EVTMODE BIT(5)/* Video event mode enable, tc35876x only */ #define HTIM1 0x0454/* Horizontal Timing Control 1 */ #define HTIM2 0x0458/* Horizontal Timing Control 2 */ #define VTIM1 0x045C/* Vertical Timing Control 1 */
@@ -254,6 +256,11 @@ enum tc358775_ports { TC358775_LVDS_OUT1, }; +enum tc3587x5_type { +TC358765 =0x65 , +TC358775 =0x75 , +}; +struct tc_data {struct i2c_client *i2c;struct device *dev;
@@ -271,6 +278,8 @@ struct tc_data {struct gpio_desc *stby_gpio;u8 lvds_link;/* single-link or dual-link */ u8 bpc; + +enum tc3587x5_type type; };static inline struct tc_data *bridge_to_tc (struct drm_bridge *b)
@@ -424,10 +433,16 @@ static void tc_bridge_enable(struct drm_bridge *bridge)d2l_write (tc->i2c,PPI_STARTPPI ,PPI_START_FUNCTION );d2l_write (tc->i2c,DSI_STARTDSI ,DSI_RX_START ); +/* Video event mode vs pulse mode bit, does not exist for tc358775 */ +if (tc->type ==TC358765 ) + val =EVTMODE ; +else + val =0 ; +if (tc->bpc ==8 ) - val =TC358775_VPCTRL_OPXLFMT (1 ); + val |=TC358775_VPCTRL_OPXLFMT (1 );else /* bpc = 6; */ - val =TC358775_VPCTRL_MSF (1 ); + val |=TC358775_VPCTRL_MSF (1 ); dsiclk = mode->crtc_clock *3 * tc->bpc / tc->num_dsi_lanes /1000 ; clkdiv = dsiclk / (tc->lvds_link ==DUAL_LINK ?DIVIDE_BY_6 :DIVIDE_BY_3 );
@@ -643,6 +658,7 @@ static int tc_probe(struct i2c_client *client) tc->dev = dev; tc->i2c = client; + tc->type = (enum tc3587x5_type )(unsigned long )of_device_get_match_data (dev); tc->panel_bridge =devm_drm_of_get_bridge (dev, dev->of_node,TC358775_LVDS_OUT0 ,0 );
@@ -704,13 +720,15 @@ static void tc_remove(struct i2c_client *client) }static const struct i2c_device_id tc358775_i2c_ids[] = { - {"tc358775" ,0 }, + {"tc358765" ,TC358765 , }, + {"tc358775" ,TC358775 , }, { } };MODULE_DEVICE_TABLE (i2c, tc358775_i2c_ids);static const struct of_device_id tc358775_of_ids[] = { - { .compatible ="toshiba,tc358775" , }, + { .compatible ="toshiba,tc358765" , .data = (void *)TC358765 , }, + { .compatible ="toshiba,tc358775" , .data = (void *)TC358775 , }, { } };MODULE_DEVICE_TABLE (of, tc358775_of_ids);
Signature · 1 line
2.43.1
PATCH v5 10/10 drm/bridge: tc358775: Configure hs_rate and lp_rate
In reply to Tony LindgrenThe hs_rate and lp_rate may be used by the dsi host for timing calculations. The tc358775 has a maximum bit rate of 1 Gbps/lane, tc358765 has maximurate of 800 Mbps per lane.
--- drivers/gpu/drm/bridge/tc358775.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
drivers/gpu/drm/bridge/tc358775.c +13 -0
@@ -637,6 +637,19 @@ static int tc_attach_host(struct tc_data *tc) dsi->mode_flags =MIPI_DSI_MODE_VIDEO |MIPI_DSI_MODE_VIDEO_BURST |MIPI_DSI_MODE_LPM ; +/* +* The hs_rate and lp_rate are data rate values. The HS mode is +* differential, while the LP mode is single ended. As the HS mode +* uses DDR, the DSI clock frequency is half the hs_rate. The 10 Mbs +* data rate for LP mode is not specified in the bridge data sheet, +* but seems to be part of the MIPI DSI spec. +*/ +if (tc->type ==TC358765 ) + dsi->hs_rate =800000000 ; +else + dsi->hs_rate =1000000000 ; + dsi->lp_rate =10000000 ; + ret =devm_mipi_dsi_attach (dev, dsi);if (ret <0 ) {dev_err (dev,"failed to attach dsi to host\n" );
Signature · 1 line
2.43.1
Hi DRM maintainers,
Quoted 2 lines · Tony Lindgren
Here are v5 patches to improve tc358775 driver and add support for tc358765.
Any news on this series? Is there anything open or can it be merged?
FWIW, I have another tc358775 improvement series based on this.
-michael
Quoted 8 lines · Tony Lindgren
Here are v5 patches to improve tc358775 driver and add support for tc358765.
Regards,
Tony
[...]
Thanks for the really nice series, sorry about the delay in applying it.
Applied, thanks!
[01/10] dt-bindings: display: bridge: tc358775: make stby gpio optional
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=51debb6d4a21 [02/10] dt-bindings: display: bridge: tc358775: Add data-lanes https://cgit.freedesktop.org/drm/drm-misc/commit/?id=497f0a1bdc06 [03/10] dt-bindings: display: bridge: tc358775: Add support for tc358765 https://cgit.freedesktop.org/drm/drm-misc/commit/?id=efcfac3e8e37 [04/10] drm/bridge: tc358775: fix support for jeida-18 and jeida-24 https://cgit.freedesktop.org/drm/drm-misc/commit/?id=30ea09a182cb [05/10] drm/bridge: tc358775: make standby GPIO optional https://cgit.freedesktop.org/drm/drm-misc/commit/?id=733daaebe250 [06/10] drm/bridge: tc358775: Get bridge data lanes instead of the DSI host lanes https://cgit.freedesktop.org/drm/drm-misc/commit/?id=005102727d9e [07/10] drm/bridge: tc358775: Add burst and low-power modes https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a4ed72e85c46 [08/10] drm/bridge: tc358775: Enable pre_enable_prev_first flag https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e2ee8e82cf42 [09/10] drm/bridge: tc358775: Add support for tc358765 https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ce2196dbba66 [10/10] drm/bridge: tc358775: Configure hs_rate and lp_rate https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ec710af54a1c
Rob