Add suppport for sn65dsi86 bridge chip and Innolux 2k edp panel
Subsystems (7)
v6 cover Add suppport for sn65dsi86 bridge chip and Innolux 2k edp panel
Changes in current patchset:
- Use PM runtime based ref-counting instead of local ref_count mechanism.
- Simplify dp rate calculation.
- Add support to configure refclk based on input REFCLK pin or DACP/N pin.
- Add compatibility string and display_mode structure to simple panel driver in alphabetical order.
Sandeep Panda (4):
drm/bridge: add support for sn65dsi86 bridge driver
dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings drm/panel: add Innolux TV123WAM panel driver support
dt-bindings: drm/panel: Document Innolux TV123WAM panel bindings
.../bindings/display/bridge/ti,sn65dsi86.txt | 81 +++ .../bindings/display/panel/innolux,tv123wam.txt | 20 + drivers/gpu/drm/bridge/Kconfig | 9 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ti-sn65dsi86.c | 766 +++++++++++++++++++++ drivers/gpu/drm/panel/panel-simple.c | 27 + 6 files changed, 904 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt create mode 100644 Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
A message that is not in the archive (sent off-list or not yet received): 1526363564-13823-1-git-send-email-spanda-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
PATCH v6 1/4 drm/bridge: add support for sn65dsi86 bridge driver
Add support for TI's sn65dsi86 dsi2edp bridge chip. The chip converts DSI transmitted signal to eDP signal, which is fed to the connected eDP panel.
This chip can be controlled via either i2c interface or dsi interface. Currently in driver all the control registers are being accessed through i2c interface only. Also as of now HPD support has not been added to bridge chip driver.
Changes in v1:
- Split the dt-bindings and the driver support into separate patches (Andrzej Hajda).
- Use of gpiod APIs to parse and configure gpios instead of obsolete ones (Andrzej Hajda).
- Use macros to define the register offsets (Andrzej Hajda).
Changes in v2:
- Separate out edp panel specific HW resource handling from bridge driver and create a separate edp panel drivers to handle panel specific mode information and HW resources (Sean Paul).
- Replace pr_* APIs to DRM_* APIs to log error or debug information (Sean Paul).
- Remove some of the unnecessary structure/variable from driver (Sean Paul).
- Rename the function and structure prefix "sn65dsi86" to "ti_sn_bridge" (Sean Paul / Rob Herring).
- Remove most of the hard-coding and modified the bridge init sequence based on current mode (Sean Paul).
- Remove the existing function to retrieve the EDID data and implemented this as an i2c_adapter and use drm_get_edid() (Sean Paul).
- Remove the dummy irq handler implementation, will add back the proper irq handling later (Sean Paul).
- Capture the required enable gpios in a single array based on dt entry instead of having individual descriptor for each gpio (Sean Paul).
Changes in v3:
- Remove usage of irq_gpio and replace it as "interrupts" property (Rob Herring).
- Remove the unnecessary header file inclusions (Sean Paul).
- Rearrange the header files in alphabetical order (Sean Paul).
- Use regmap interface to perform i2c transactions.
- Update Copyright/License field and address other review comments (Jordan Crouse).
Changes in v4:
- Update License/Copyright (Sean Paul).
- Add Kconfig and Makefile changes (Sean Paul).
- Drop i2c gpio handling from this bridge driver, since i2c sda/scl gpios will be handled by i2c master.
- Update required supplies names.
- Remove unnecessary goto statements (Sean Paul).
- Add mutex lock to power_ctrl API to avoid race conditions (Sean Paul).
- Add support to parse reference clk frequency from dt(optional).
- Update the bridge chip enable/disable sequence.
Changes in v5:
- Fixed Kbuild test service reported warnings.
Changes in v6:
- Use PM runtime based ref-counting instead of local ref_count mechanism (Stephen Boyd).
- Clean up some debug logs and indentations (Sean Paul).
- Simplify dp rate calculation (Sean Paul).
- Add support to configure refclk based on input REFCLK pin or DACP/N pin (Stephen Boyd).
--- drivers/gpu/drm/bridge/Kconfig | 9 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ti-sn65dsi86.c | 766 ++++++++++++++++++++++++++++++++++ 3 files changed, 776 insertions(+) create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
drivers/gpu/drm/bridge/Kconfig +9 -0
@@ -108,6 +108,15 @@ config DRM_TI_TFP410 ---help--- Texas Instruments TFP410 DVI/HDMI Transmitter driver +config DRM_TI_SN65DSI86 + tristate "TI SN65DSI86 DSI to eDP bridge" + depends on OF + select DRM_KMS_HELPER + select REGMAP_I2C + select DRM_PANEL + ---help--- + Texas Instruments SN65DSI86 DSI to eDP Bridge driver + source "drivers/gpu/drm/bridge/analogix/Kconfig" source "drivers/gpu/drm/bridge/adv7511/Kconfig"
drivers/gpu/drm/bridge/Makefile +1 -0
@@ -12,4 +12,5 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o +obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o obj-y += synopsys/
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +766 -0
766 lines not on this page · raw message
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [PATCH v6 1/4] drm/bridge: add support for sn65dsi86 bridge driver
Quoted 181 lines · Sandeep Panda
Add support for TI's sn65dsi86 dsi2edp bridge chip. The chip converts DSI transmitted signal to eDP signal, which is fed to the connected eDP panel.
This chip can be controlled via either i2c interface or dsi interface. Currently in driver all the control registers are being accessed through i2c interface only. Also as of now HPD support has not been added to bridge chip driver.
Changes in v1:
- Split the dt-bindings and the driver support into separate patches (Andrzej Hajda).
- Use of gpiod APIs to parse and configure gpios instead of obsolete ones (Andrzej Hajda).
- Use macros to define the register offsets (Andrzej Hajda).Changes in v2:
- Separate out edp panel specific HW resource handling from bridge driver and create a separate edp panel drivers to handle panel specific mode information and HW resources (Sean Paul).
- Replace pr_* APIs to DRM_* APIs to log error or debug information (Sean Paul).
- Remove some of the unnecessary structure/variable from driver (Sean Paul).
- Rename the function and structure prefix "sn65dsi86" to "ti_sn_bridge" (Sean Paul / Rob Herring).
- Remove most of the hard-coding and modified the bridge init sequence based on current mode (Sean Paul).
- Remove the existing function to retrieve the EDID data and implemented this as ani2c_adapterand usedrm_get_edid()(Sean Paul).
- Remove the dummy irq handler implementation, will add back the proper irq handling later (Sean Paul).
- Capture the required enable gpios in a single array based on dt entry instead of having individual descriptor for each gpio (Sean Paul).Changes in v3:
- Remove usage ofirq_gpioand replace it as "interrupts" property (Rob Herring).
- Remove the unnecessary header file inclusions (Sean Paul).
- Rearrange the header files in alphabetical order (Sean Paul).
- Use regmap interface to perform i2c transactions.
- Update Copyright/License field and address other review comments (Jordan Crouse).Changes in v4:
- Update License/Copyright (Sean Paul).
- AddKconfigandMakefilechanges (Sean Paul).
- Drop i2c gpio handling from this bridge driver, since i2c sda/scl gpios will be handled by i2c master.
- Update required supplies names.
- Remove unnecessary goto statements (Sean Paul).
- Add mutex lock topower_ctrlAPI to avoid race conditions (Sean Paul).
- Add support to parse reference clk frequency from dt(optional).
- Update the bridge chip enable/disable sequence.Changes in v5:
- FixedKbuildtest service reported warnings.Changes in v6:
- Use PM runtime based ref-counting instead of localref_countmechanism (Stephen Boyd).
- Clean up some debug logs and indentations (Sean Paul).
- Simplify dp rate calculation (Sean Paul).
- Add support to configure refclk based on input REFCLK pin or DACP/N pin (Stephen Boyd).Signed-off-by Sandeep Panda <[email protected]>--- drivers/gpu/drm/bridge/Kconfig | 9 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ti-sn65dsi86.c | 766 ++++++++++++++++++++++++++++++++++ 3 files changed, 776 insertions(+) create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.cdrivers/gpu/drm/bridge/Kconfig +9
-0@@ -108,6 +108,15 @@ config DRM_TI_TFP410 ---help--- Texas Instruments TFP410 DVI/HDMI Transmitter driver +config DRM_TI_SN65DSI86 + tristate "TI SN65DSI86 DSI to eDP bridge" + depends on OF + select DRM_KMS_HELPER + select REGMAP_I2C + select DRM_PANEL + ---help--- + Texas Instruments SN65DSI86 DSI to eDP Bridge driver + source "drivers/gpu/drm/bridge/analogix/Kconfig" source "drivers/gpu/drm/bridge/adv7511/Kconfig"drivers/gpu/drm/bridge/Makefile +1
-0@@ -12,4 +12,5 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o +obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o obj-y += synopsys/drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +69
-0@@ -0,0 +1,766 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2018, The Linux Foundation. All rights reserved. +*/ + +#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_panel.h> +#include <linux/clk.h> +#include <linux/gpio.h> +#include <linux/i2c.h> +#include <linux/of_gpio.h> +#include <linux/of_graph.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> + +#define SN_BRIDGE_REVISION_ID 0x2 + +/* Link Training specific registers */ +#define SN_DEVICE_REV_REG 0x08 +#define SN_HPD_DISABLE_REG 0x5C +#define SN_REFCLK_FREQ_REG 0x0A +#define SN_DSI_LANES_REG 0x10 +#define SN_DSIA_CLK_FREQ_REG 0x12 +#define SN_ENH_FRAME_REG 0x5A +#define SN_SSC_CONFIG_REG 0x93 +#define SN_DATARATE_CONFIG_REG 0x94 +#define SN_PLL_ENABLE_REG 0x0D +#define SN_SCRAMBLE_CONFIG_REG 0x95 +#define SN_AUX_WDATA0_REG 0x64 +#define SN_AUX_ADDR_19_16_REG 0x74 +#define SN_AUX_ADDR_15_8_REG 0x75 +#define SN_AUX_ADDR_7_0_REG 0x76 +#define SN_AUX_LENGTH_REG 0x77 +#define SN_AUX_CMD_REG 0x78 +#define SN_ML_TX_MODE_REG 0x96 +/* video config specific registers */ +#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG 0x20 +#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG 0x21 +#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG 0x24 +#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG 0x25 +#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG 0x2C +#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG 0x2D +#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG 0x30 +#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG 0x31 +#define SN_CHA_HORIZONTAL_BACK_PORCH_REG 0x34 +#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36 +#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG 0x38 +#define SN_CHA_VERTICAL_FRONT_PORCH_REG 0x3A +#define SN_DATA_FORMAT_REG 0x5B + +#define MIN_DSI_CLK_FREQ_MHZ 40 + +/* fudge factor required to account for 8b/10b encoding */ +#define DP_CLK_FUDGE_NUM 10 +#define DP_CLK_FUDGE_DEN 8 + +#define DPPLL_CLK_SRC_REFCLK 0 +#define DPPLL_CLK_SRC_DSICLK 1 + +#define SN_DSIA_REFCLK_OFFSET 1 +#define SN_DSIA_LANE_OFFSET 3 +#define SN_DP_LANE_OFFSET 4 +#define SN_DP_DATA_RATE_OFFSET 5 +#define SN_TIMING_HIGH_OFFSET 8
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 79
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ +#define SN_ENABLE_VID_STREAM_BIT BIT(3) +#define SN_DSIA_NUM_LANES_BITS (BIT(4) | BIT(3)) +#define SN_DP_NUM_LANES_BITS (BIT(5) | BIT(4)) +#define SN_DP_DATA_RATE_BITS (BIT(7) | BIT(6) | BIT(5)) +#define SN_HPD_DISABLE_BIT BIT(0) + +struct ti_sn_bridge { +struct device *dev; +struct regmap *regmap;
I have not spotted who advised you to use regmap, I think unless you share hardware between multiple drivers it is an overkill. Are there any features of this interface to use it over i2c?
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 90
Quoted 11 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +11
-0+struct drm_bridge bridge; +struct drm_connector connector; +struct device_node *host_node; +struct mipi_dsi_device *dsi; +struct clk *refclk; +struct drm_panel *panel; +struct gpio_desc *enable_gpio; +unsigned int num_supplies; +struct regulator_bulk_data *supplies; +struct i2c_adapter *ddc; +struct drm_display_mode curr_mode;
I think you can drop this field, current mode is always available via:
pdata->bridge.encoder.crtc->state->adjusted_mode;
Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +9
-0+}; + +static const struct regmap_range ti_sn_bridge_volatile_ranges[] = { + { .range_min =0 , .range_max =0xff }, +}; + +static const struct regmap_access_table ti_sn_bridge_volatile_table = { + .yes_ranges = ti_sn_bridge_volatile_ranges, + .n_yes_ranges =ARRAY_SIZE (ti_sn_bridge_volatile_ranges),
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 109
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+}; + +static const struct regmap_config ti_sn_bridge_regmap_config = { + .reg_bits =8 , + .val_bits =8 , + .volatile_table = &ti_sn_bridge_volatile_table, + .cache_type =REGCACHE_NONE , +}; + +#ifdef CONFIG_PM
Please mark PM callbacks with __maybe_unused instead if conditional macros.
Quoted 121 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +121
-0+static int ti_sn_bridge_resume (struct device *dev) +{ +struct ti_sn_bridge *pdata =dev_get_drvdata (dev); +int ret =0 ; + + ret =regulator_bulk_enable (pdata->num_supplies, pdata->supplies); +if (ret) { +DRM_ERROR ("failed to enable supplies %d\n" , ret); +return ret; + } + +gpiod_set_value (pdata->enable_gpio,1 ); + +return ret; +} + +static int ti_sn_bridge_suspend (struct device *dev) +{ +struct ti_sn_bridge *pdata =dev_get_drvdata (dev); +int ret =0 ; + +gpiod_set_value (pdata->enable_gpio,0 ); + + ret =regulator_bulk_disable (pdata->num_supplies, pdata->supplies); +if (ret) +DRM_ERROR ("failed to disable supplies %d\n" , ret); + +return ret; +} +#endif + +static const struct dev_pm_ops ti_sn_bridge_pm_ops = { +SET_RUNTIME_PM_OPS (ti_sn_bridge_suspend, ti_sn_bridge_resume,NULL ) +}; + +/* Connector funcs */ +static struct ti_sn_bridge * +connector_to_ti_sn_bridge (struct drm_connector *connector) +{ + return container_of(connector, struct ti_sn_bridge, connector); +} + +static int ti_sn_bridge_connector_get_modes (struct drm_connector *connector) +{ +struct ti_sn_bridge *pdata =connector_to_ti_sn_bridge (connector); +struct drm_panel *panel = pdata->panel; +struct edid *edid; +u32 num_modes; + +if (panel) { +DRM_DEBUG_KMS ("get mode from connected drm_panel\n" ); +return drm_panel_get_modes (panel); + } + +if (!pdata->ddc) +return 0 ; + +pm_runtime_get_sync (pdata->dev); + edid =drm_get_edid (connector, pdata->ddc); +pm_runtime_put_sync (pdata->dev); +if (!edid) +return 0 ; + +drm_mode_connector_update_edid_property (connector, edid); + num_modes =drm_add_edid_modes (connector, edid); +kfree (edid); + +return num_modes; +} + +static enum drm_mode_status +ti_sn_bridge_connector_mode_valid (struct drm_connector *connector, +struct drm_display_mode *mode) +{ +/* maximum supported resolution is 4K at 60 fps */ +if (mode->clock >594000 ) +return MODE_CLOCK_HIGH ; + +return MODE_OK ; +} + +static struct drm_connector_helper_funcs ti_sn_bridge_connector_helper_funcs = { + .get_modes = ti_sn_bridge_connector_get_modes, + .mode_valid = ti_sn_bridge_connector_mode_valid, +}; + +static enum drm_connector_status +ti_sn_bridge_connector_detect (struct drm_connector *connector,bool force) +{ +struct ti_sn_bridge *pdata =connector_to_ti_sn_bridge (connector); + +/** +* TODO: Currently if drm_panel is present, then always +* return the status as connected. Need to add support to detect +* device state for no panel(hot pluggable) scenarios. +*/ +if (pdata->panel) +return connector_status_connected; +else +return connector_status_unknown; +} + +static const struct drm_connector_funcs ti_sn_bridge_connector_funcs = { + .fill_modes = drm_helper_probe_single_connector_modes, + .detect = ti_sn_bridge_connector_detect, + .destroy = drm_connector_cleanup, + .reset = drm_atomic_helper_connector_reset, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +static struct ti_sn_bridge *bridge_to_ti_sn_bridge (struct drm_bridge *bridge) +{ + return container_of(bridge, struct ti_sn_bridge, bridge); +} + +static int ti_sn_bridge_read_device_rev (struct ti_sn_bridge *pdata) +{ +unsigned int rev =0 ; +int ret =0 ; +
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 240
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ ret =regmap_read (pdata->regmap,SN_DEVICE_REV_REG , &rev); +if (ret) { +DRM_ERROR ("Revision read failed %d\n" , ret); +return ret; + } + +if (rev !=SN_BRIDGE_REVISION_ID ) { +DRM_ERROR ("ti_sn_bridge revision id: 0x%x mismatch\n" , rev); + ret = -EINVAL ; + }
Are you sure it won't work with other revisions?
Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +9
-0+ +return ret; +} + +static const char *const ti_sn_bridge_supply_names[] = { +"vcca" , +"vcc" , +"vccio" , +"vpll" ,
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 259
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+}; + +static int ti_sn_bridge_parse_regulators (struct ti_sn_bridge *pdata) +{ +unsigned int i; + + pdata->num_supplies =ARRAY_SIZE (ti_sn_bridge_supply_names); + + pdata->supplies =devm_kcalloc (pdata->dev, pdata->num_supplies, +sizeof (*pdata->supplies),GFP_KERNEL );
It seems there is constant number of supplies. You can convert supplies to array and avoid dynamic allocation.
Quoted 15 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +15
-0+if (!pdata->supplies) +return -ENOMEM ; + +for (i =0 ; i < pdata->num_supplies; i++) + pdata->supplies[i].supply = ti_sn_bridge_supply_names[i]; + +return devm_regulator_bulk_get (pdata->dev, + pdata->num_supplies, pdata->supplies); +} + +static int ti_sn_bridge_attach_panel (struct ti_sn_bridge *pdata) +{ +struct device_node *panel_node, *port, *endpoint; + + pdata->panel =NULL ;
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 284
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ port =of_graph_get_port_by_id (pdata->dev->of_node,1 ); +if (!port) +return 0 ; + + endpoint =of_get_child_by_name (port,"endpoint" ); +of_node_put (port); +if (!endpoint) { +DRM_ERROR ("no output endpoint found\n" ); +return -EINVAL ; + }
Why not of_graph_get_endpoint_by_regs ?
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 291
Quoted 7 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +7
-0+ + panel_node =of_graph_get_remote_port_parent (endpoint); +of_node_put (endpoint); +if (!panel_node) { +DRM_ERROR ("no output node found\n" ); +return -EINVAL ; + }
Or even of_graph_get_remote_node?
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 294
Quoted 3 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +3
-0+ + pdata->panel =of_drm_find_panel (panel_node); +of_node_put (panel_node);
Or even, even drm_of_find_panel_or_bridge ? :)
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 297
Quoted 3 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +3
-0+if (!pdata->panel) { +DRM_ERROR ("no panel node found\n" ); +return -EINVAL ;
You should probably return -EPROBE_DEFER here.
Quoted 15 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +15
-0+ } +drm_panel_attach (pdata->panel, &pdata->connector); +DRM_DEBUG_KMS ("drm panel attached to ti_sn_bridge\n" ); + +return 0 ; +} + +static int ti_sn_bridge_attach (struct drm_bridge *bridge) +{ +struct mipi_dsi_host *host; +struct mipi_dsi_device *dsi; +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); +int ret; +const struct mipi_dsi_device_info info = { .type ="ti_sn_bridge" , + .channel =0 ,
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 322
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ .node = NULL, + }; + +if (!bridge->encoder) { +DRM_ERROR ("Parent encoder object not found\n" ); +return -ENODEV ; + } + +/* HPD not supported */ + pdata->connector.polled =0 ;
You can skip this line.
Quoted 7 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +7
-0+ + ret =drm_connector_init (bridge->dev, &pdata->connector, + &ti_sn_bridge_connector_funcs, + DRM_MODE_CONNECTOR_eDP); +if (ret) { +DRM_ERROR ("Failed to initialize connector with drm\n" ); +return ret;
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 339
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ } + +drm_connector_helper_add (&pdata->connector, + &ti_sn_bridge_connector_helper_funcs); +drm_mode_connector_attach_encoder (&pdata->connector, bridge->encoder); + + host =of_find_mipi_dsi_host_by_node (pdata->host_node); +if (!host) { +DRM_ERROR ("failed to find dsi host\n" ); +return -ENODEV ;
Again -EPROBE_DEFER.
Quoted 17 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +17
-0+ } + + dsi =mipi_dsi_device_register_full (host, &info); +if (IS_ERR (dsi)) { +DRM_ERROR ("failed to create dsi device\n" ); + ret =PTR_ERR (dsi); +return ret; + } + +/* TODO: setting to 4 lanes always for now */ + dsi->lanes =4 ; + dsi->format =MIPI_DSI_FMT_RGB888 ; + dsi->mode_flags =MIPI_DSI_MODE_VIDEO |MIPI_DSI_MODE_VIDEO_SYNC_PULSE | +MIPI_DSI_MODE_EOT_PACKET |MIPI_DSI_MODE_VIDEO_HSE ; + + ret =mipi_dsi_attach (dsi); +if (ret <0 ) {
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 366
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+DRM_ERROR ("failed to attach dsi to host\n" ); +mipi_dsi_device_unregister (dsi); +return ret; + } + + pdata->dsi = dsi; + +DRM_DEBUG_KMS ("ti_sn_bridge attached to dsi\n" ); +/* attach panel to bridge */ +ti_sn_bridge_attach_panel (pdata);
Function can fail, you should handle it.
Quoted 6 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+ +return 0 ; +} + +static void ti_sn_bridge_mode_set(struct drm_bridge *bridge, + struct drm_display_mode *mode,
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 382
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ struct drm_display_mode *adj_mode) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); + +DRM_DEBUG ("mode_set: hdisplay=%d, vdisplay=%d, vrefresh=%d, clock=%d\n" , + adj_mode->hdisplay, adj_mode->vdisplay, + adj_mode->vrefresh, adj_mode->clock); + +drm_mode_copy (&pdata->curr_mode, adj_mode); +}
This callback can be dropped, see comment for curr_mode.
Quoted 102 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +102
-0+ +static void ti_sn_bridge_disable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); +struct drm_panel *panel = pdata->panel; + +if (panel) { +drm_panel_disable (panel); +drm_panel_unprepare (panel); + } + +/* disable video stream */ +regmap_update_bits (pdata->regmap,SN_ENH_FRAME_REG , +SN_ENABLE_VID_STREAM_BIT ,0 ); +/* semi auto link training mode OFF */ +regmap_write (pdata->regmap,SN_ML_TX_MODE_REG ,0 ); +/* disable DP PLL */ +regmap_write (pdata->regmap,SN_PLL_ENABLE_REG ,0 ); +} + +static u32 ti_sn_bridge_get_dsi_freq (struct ti_sn_bridge *pdata) +{ +u32 bit_rate_khz, clk_freq_khz; +struct drm_display_mode *mode = &pdata->curr_mode; + + bit_rate_khz = mode->clock * +mipi_dsi_pixel_format_to_bpp (pdata->dsi->format); + clk_freq_khz = bit_rate_khz / (pdata->dsi->lanes *2 ); + +return clk_freq_khz; +} + +#define REFCLK_LUT_SIZE 5 + +/* clk frequencies supported by bridge in Hz in case derived from REFCLK pin */ +static const u32 ti_sn_bridge_refclk_lut[] = { +12000000 , +19200000 , +26000000 , +27000000 , +38400000 , +}; + +/* clk frequencies supported by bridge in Hz in case derived from DACP/N pin */ +static const u32 ti_sn_bridge_dsiclk_lut[] = { +468000000 , +384000000 , +416000000 , +486000000 , +460800000 , +}; + +static void ti_sn_bridge_set_refclk (struct ti_sn_bridge *pdata) +{ +int i =0 ; +u8 refclk_src; +u32 refclk_rate; +const u32 *refclk_lut; + +if (pdata->refclk) { + refclk_src =DPPLL_CLK_SRC_REFCLK ; + refclk_rate =clk_get_rate (pdata->refclk); + refclk_lut = ti_sn_bridge_refclk_lut; +clk_prepare_enable (pdata->refclk); + }else { + refclk_src =DPPLL_CLK_SRC_DSICLK ; + refclk_rate =ti_sn_bridge_get_dsi_freq (pdata) *1000 ; + refclk_lut = ti_sn_bridge_dsiclk_lut; + } + +/* for i equals to REFCLK_LUT_SIZE means default frequency */ +for (i =0 ; i <REFCLK_LUT_SIZE ; i++) +if (refclk_lut[i] == refclk_rate) +break ; + +regmap_write (pdata->regmap,SN_REFCLK_FREQ_REG , + (refclk_src | (i <<SN_DSIA_REFCLK_OFFSET ))); +} + +/** +* LUT index corresponds to register value and +* LUT values corresponds to dp data rate supported +* by the bridge in Mbps unit. +*/ +static const unsigned int ti_sn_bridge_dp_rate_lut[] = { +0 ,1620 ,2160 ,2430 ,2700 ,3240 ,4320 ,5400 +}; + +static void ti_sn_bridge_set_dsi_dp_rate (struct ti_sn_bridge *pdata) +{ +unsigned int bit_rate_mhz, clk_freq_mhz, dp_rate_mhz; +unsigned int val =0 , i =0 ; +struct drm_display_mode *mode = &pdata->curr_mode; + +/* set DSIA clk frequency */ + bit_rate_mhz = (mode->clock /1000 ) * +mipi_dsi_pixel_format_to_bpp (pdata->dsi->format); + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes *2 ); + +/* for each increment in val, frequency increases by 5MHz */ + val = (MIN_DSI_CLK_FREQ_MHZ /5 ) + + (((clk_freq_mhz -MIN_DSI_CLK_FREQ_MHZ ) /5 ) &0xFF );
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 494
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+regmap_write (pdata->regmap,SN_DSIA_CLK_FREQ_REG , val); + +/* set DP data rate */ + dp_rate_mhz = ((bit_rate_mhz / pdata->dsi->lanes) *DP_CLK_FUDGE_NUM ) / +DP_CLK_FUDGE_DEN ; +for (i =0 ; i <ARRAY_SIZE (ti_sn_bridge_dp_rate_lut); i++) +if (ti_sn_bridge_dp_rate_lut[i] > dp_rate_mhz) +break ; +if (i ==ARRAY_SIZE (ti_sn_bridge_dp_rate_lut)) + i--;/* set to maximum possible */
Just use: for (i = 0; i < ARRAY_SIZE(ti_sn_bridge_dp_rate_lut) - 1; i++)
Quoted 134 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +134
-0+ +regmap_update_bits (pdata->regmap,SN_DATARATE_CONFIG_REG , +SN_DP_DATA_RATE_BITS , i <<SN_DP_DATA_RATE_OFFSET ); +} + +static void ti_sn_bridge_set_video_timings (struct ti_sn_bridge *pdata) +{ +struct drm_display_mode *mode = &pdata->curr_mode; + +regmap_write (pdata->regmap,SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG , + mode->hdisplay &0xFF ); +regmap_write (pdata->regmap,SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG , + (mode->hdisplay >>SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG , + mode->vdisplay &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG , + (mode->vdisplay >>SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG , + (mode->hsync_end - mode->hsync_start) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG , + ((mode->hsync_end - mode->hsync_start) >> +SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG , + (mode->vsync_end - mode->vsync_start) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG , + ((mode->vsync_end - mode->vsync_start) >> +SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HORIZONTAL_BACK_PORCH_REG , + (mode->htotal - mode->hsync_end) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_BACK_PORCH_REG , + (mode->vtotal - mode->vsync_end) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HORIZONTAL_FRONT_PORCH_REG , + (mode->hsync_start - mode->hdisplay) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_FRONT_PORCH_REG , + (mode->vsync_start - mode->vdisplay) &0xFF ); +usleep_range (10000 ,10500 );/* 10ms delay recommended by spec */ +} + +static void ti_sn_bridge_enable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); +struct drm_panel *panel = pdata->panel; +unsigned int val =0 ; + +if (panel) { +drm_panel_prepare (panel); +/* in case drm_panel is connected then HPD is not supported */ +regmap_update_bits (pdata->regmap,SN_HPD_DISABLE_REG , +SN_HPD_DISABLE_BIT ,SN_HPD_DISABLE_BIT ); + } + +/* DSI_A lane config */ + val = (4 - pdata->dsi->lanes) <<SN_DSIA_LANE_OFFSET ; +regmap_update_bits (pdata->regmap,SN_DSI_LANES_REG , +SN_DSIA_NUM_LANES_BITS , val); + +/* DP lane config */ + val = (pdata->dsi->lanes -1 ) <<SN_DP_LANE_OFFSET ; +regmap_update_bits (pdata->regmap,SN_SSC_CONFIG_REG , +SN_DP_NUM_LANES_BITS , val); + +/* set dsi/dp clk frequency value */ +ti_sn_bridge_set_dsi_dp_rate (pdata); + +/* enable DP PLL */ +regmap_write (pdata->regmap,SN_PLL_ENABLE_REG ,1 ); +usleep_range (10000 ,10500 );/* 10ms delay recommended by spec */ + +/** +* The SN65DSI86 only supports ASSR Display Authentication method and +* this method is enabled by default. An eDP panel must support this +* authentication method. We need to enable this method in the eDP panel +* at DisplayPort address 0x0010A prior to link training. +*/ +regmap_write (pdata->regmap,SN_AUX_WDATA0_REG ,0x01 ); +regmap_write (pdata->regmap,SN_AUX_ADDR_19_16_REG ,0x00 ); +regmap_write (pdata->regmap,SN_AUX_ADDR_15_8_REG ,0x01 ); +regmap_write (pdata->regmap,SN_AUX_ADDR_7_0_REG ,0x0A ); +regmap_write (pdata->regmap,SN_AUX_LENGTH_REG ,0x01 ); +regmap_write (pdata->regmap,SN_AUX_CMD_REG ,0x81 ); +usleep_range (10000 ,10500 );/* 10ms delay recommended by spec */ + +/* Semi auto link training mode */ +regmap_write (pdata->regmap,SN_ML_TX_MODE_REG ,0x0A ); +msleep (20 );/* 20ms delay recommended by spec */ + +/* config video parameters */ +ti_sn_bridge_set_video_timings (pdata); + +/* enable video stream */ +regmap_update_bits (pdata->regmap,SN_ENH_FRAME_REG , +SN_ENABLE_VID_STREAM_BIT ,SN_ENABLE_VID_STREAM_BIT ); + +if (panel) +drm_panel_enable (panel); +} + +static void ti_sn_bridge_pre_enable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); + +pm_runtime_get_sync (pdata->dev); + +/* configure bridge CLK_SRC and ref_clk */ +ti_sn_bridge_set_refclk (pdata); +} + +static void ti_sn_bridge_post_disable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); + +if (pdata->refclk) +clk_disable_unprepare (pdata->refclk); + +pm_runtime_put_sync (pdata->dev); +} + +static const struct drm_bridge_funcs ti_sn_bridge_funcs = { + .attach = ti_sn_bridge_attach, + .pre_enable = ti_sn_bridge_pre_enable, + .enable = ti_sn_bridge_enable, + .disable = ti_sn_bridge_disable, + .post_disable = ti_sn_bridge_post_disable, + .mode_set = ti_sn_bridge_mode_set, +}; + +static int ti_sn_bridge_parse_dsi_host (struct ti_sn_bridge *pdata) +{ +struct device_node *np = pdata->dev->of_node; +struct device_node *end_node; + + end_node =of_graph_get_endpoint_by_regs (np,0 ,0 ); +if (!end_node) { +DRM_ERROR ("remote endpoint not found\n" );
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 638
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+return -ENODEV ; + } + + pdata->host_node =of_graph_get_remote_port_parent (end_node); +of_node_put (end_node); +if (!pdata->host_node) { +DRM_ERROR ("remote node not found\n" ); +return -ENODEV ; + } +of_node_put (pdata->host_node);
Why not of_graph_get_remote_node ?
Quoted 43 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +43
-0+ +return 0 ; +} + +static int ti_sn_bridge_probe (struct i2c_client *client, +const struct i2c_device_id *id) +{ +struct ti_sn_bridge *pdata; +struct device_node *ddc_node; +int ret =0 ; + +if (!i2c_check_functionality (client->adapter,I2C_FUNC_I2C )) { +DRM_ERROR ("device doesn't support I2C\n" ); +return -ENODEV ; + } + + pdata =devm_kzalloc (&client->dev,sizeof (struct ti_sn_bridge ), +GFP_KERNEL ); +if (!pdata) +return -ENOMEM ; + + pdata->dev = &client->dev; +dev_set_drvdata (&client->dev, pdata); + + pdata->regmap =devm_regmap_init_i2c (client, + &ti_sn_bridge_regmap_config); +if (IS_ERR (pdata->regmap)) { +DRM_ERROR ("regmap i2c init failed\n" ); +return PTR_ERR (pdata->regmap); + } + + pdata->enable_gpio =devm_gpiod_get (pdata->dev, +"enable" ,GPIOD_OUT_LOW ); +if (IS_ERR (pdata->enable_gpio)) { +DRM_ERROR ("failed to get enable gpio from DT\n" ); + ret =PTR_ERR (pdata->enable_gpio); +return ret; + } + + ret =ti_sn_bridge_parse_regulators (pdata); +if (ret) { +DRM_ERROR ("failed to parse regulators\n" ); +return ret;
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 691
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ } + + ret =ti_sn_bridge_parse_dsi_host (pdata); +if (ret) +return ret; + +pm_runtime_enable (pdata->dev); +pm_runtime_get_sync (pdata->dev); + ret =ti_sn_bridge_read_device_rev (pdata); +pm_runtime_put_sync (pdata->dev);
I would put it into (pre) enable callbacks, maybe with marking it to run only once.
Regards
Andrzej
Quoted 65 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +65
-0+if (ret) + goto err_rev_read; + + pdata->refclk =devm_clk_get (pdata->dev,"refclk" ); + + ddc_node =of_parse_phandle (pdata->dev->of_node,"ddc-i2c-bus" ,0 ); +if (ddc_node) { + pdata->ddc =of_find_i2c_adapter_by_node (ddc_node); +of_node_put (ddc_node); +if (!pdata->ddc) { +DRM_DEBUG_KMS ("failed to read ddc node\n" ); + ret = -EPROBE_DEFER ; + goto err_rev_read; + } + }else { +DRM_DEBUG_KMS ("no ddc property found\n" ); + } + +i2c_set_clientdata (client, pdata); + + pdata->bridge.funcs = &ti_sn_bridge_funcs; + pdata->bridge.of_node = client->dev.of_node; + +drm_bridge_add (&pdata->bridge); + +return 0 ; + +err_rev_read: +pm_runtime_disable (pdata->dev); +return ret; +} + +static int ti_sn_bridge_remove (struct i2c_client *client) +{ +struct ti_sn_bridge *pdata =i2c_get_clientdata (client); + +if (!pdata) +return -EINVAL ; + +mipi_dsi_detach (pdata->dsi); +mipi_dsi_device_unregister (pdata->dsi); + +drm_bridge_remove (&pdata->bridge); +pm_runtime_disable (pdata->dev); +i2c_put_adapter (pdata->ddc); + +return 0 ; +} + +static struct i2c_device_id ti_sn_bridge_id[] = { + {"ti,sn65dsi86" ,0 }, + {}, +}; +MODULE_DEVICE_TABLE (i2c, ti_sn_bridge_id); + +static const struct of_device_id ti_sn_bridge_match_table[] = { + {.compatible ="ti,sn65dsi86" }, + {}, +}; +MODULE_DEVICE_TABLE (of, ti_sn_bridge_match_table); + +static struct i2c_driver ti_sn_bridge_driver = { + .driver = { + .name ="ti_sn65dsi86" , + .of_match_table = ti_sn_bridge_match_table,
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 766
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ .pm = &ti_sn_bridge_pm_ops, + }, + .probe = ti_sn_bridge_probe, + .remove = ti_sn_bridge_remove, + .id_table = ti_sn_bridge_id, +}; + +module_i2c_driver (ti_sn_bridge_driver); +MODULE_DESCRIPTION ("sn65dsi86 DSI to eDP bridge driver" ); +MODULE_LICENSE ("GPL v2" );
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH v6 1/4] drm/bridge: add support for sn65dsi86 bridge driver
I suppose you wanted to respond on the list, so I have added back all recipients.
Quoted 209 lines · On 16.05.2018 06:39, [email protected] wrote:
Quoted 199 lines · Andrzej Hajda
Quoted 198 lines · Sandeep Panda
Add support for TI's sn65dsi86 dsi2edp bridge chip. The chip converts DSI transmitted signal to eDP signal, which is fed to the connected eDP panel.
This chip can be controlled via either i2c interface or dsi interface. Currently in driver all the control registers are being accessed through i2c interface only. Also as of now HPD support has not been added to bridge chip driver.
Changes in v1:
- Split the dt-bindings and the driver support into separate patches (Andrzej Hajda).
- Use of gpiod APIs to parse and configure gpios instead of obsolete
ones
(Andrzej Hajda).
- Use macros to define the register offsets (Andrzej Hajda).Changes in v2:
- Separate out edp panel specific HW resource handling from bridge driver and create a separate edp panel drivers to handle panel specific mode information and HW resources (Sean Paul).
- Replace pr_* APIs to DRM_* APIs to log error or debug information (Sean Paul).
- Remove some of the unnecessary structure/variable from driver (Sean Paul).
- Rename the function and structure prefix "sn65dsi86" to
"ti_sn_bridge"
(Sean Paul / Rob Herring).
- Remove most of the hard-coding and modified the bridge init
sequence
based on current mode (Sean Paul).
- Remove the existing function to retrieve the EDID data and implemented this as ani2c_adapterand usedrm_get_edid()(Sean
Paul).
- Remove the dummy irq handler implementation, will add back the proper irq handling later (Sean Paul).
- Capture the required enable gpios in a single array based on dt
entry
instead of having individual descriptor for each gpio (Sean Paul).Changes in v3:
- Remove usage ofirq_gpioand replace it as "interrupts" property
(Rob
Herring).
- Remove the unnecessary header file inclusions (Sean Paul).
- Rearrange the header files in alphabetical order (Sean Paul).
- Use regmap interface to perform i2c transactions.
- Update Copyright/License field and address other review comments (Jordan Crouse).Changes in v4:
- Update License/Copyright (Sean Paul).
- AddKconfigandMakefilechanges (Sean Paul).
- Drop i2c gpio handling from this bridge driver, since i2c sda/scl
gpios
will be handled by i2c master.
- Update required supplies names.
- Remove unnecessary goto statements (Sean Paul).
- Add mutex lock topower_ctrlAPI to avoid race conditions (Sean Paul).
- Add support to parse reference clk frequency from dt(optional).
- Update the bridge chip enable/disable sequence.Changes in v5:
- FixedKbuildtest service reported warnings.Changes in v6:
- Use PM runtime based ref-counting instead of localref_count
mechanism
(Stephen Boyd).
- Clean up some debug logs and indentations (Sean Paul).
- Simplify dp rate calculation (Sean Paul).
- Add support to configure refclk based on input REFCLK pin or DACP/N pin (Stephen Boyd).Signed-off-by Sandeep Panda <[email protected]>--- drivers/gpu/drm/bridge/Kconfig | 9 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ti-sn65dsi86.c | 766 ++++++++++++++++++++++++++++++++++ 3 files changed, 776 insertions(+) create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.cdrivers/gpu/drm/bridge/Kconfig +9
-0@@ -108,6 +108,15 @@ config DRM_TI_TFP410 ---help--- Texas Instruments TFP410 DVI/HDMI Transmitter driver +config DRM_TI_SN65DSI86 + tristate "TI SN65DSI86 DSI to eDP bridge" + depends on OF + select DRM_KMS_HELPER + select REGMAP_I2C + select DRM_PANEL + ---help--- + Texas Instruments SN65DSI86 DSI to eDP Bridge driver + source "drivers/gpu/drm/bridge/analogix/Kconfig" source "drivers/gpu/drm/bridge/adv7511/Kconfig"drivers/gpu/drm/bridge/Makefile +1
-0@@ -12,4 +12,5 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o +obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o obj-y += synopsys/drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +74
-0@@ -0,0 +1,766 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2018, The Linux Foundation. All rights reserved. +*/ + +#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_panel.h> +#include <linux/clk.h> +#include <linux/gpio.h> +#include <linux/i2c.h> +#include <linux/of_gpio.h> +#include <linux/of_graph.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> + +#define SN_BRIDGE_REVISION_ID 0x2 + +/* Link Training specific registers */ +#define SN_DEVICE_REV_REG 0x08 +#define SN_HPD_DISABLE_REG 0x5C +#define SN_REFCLK_FREQ_REG 0x0A +#define SN_DSI_LANES_REG 0x10 +#define SN_DSIA_CLK_FREQ_REG 0x12 +#define SN_ENH_FRAME_REG 0x5A +#define SN_SSC_CONFIG_REG 0x93 +#define SN_DATARATE_CONFIG_REG 0x94 +#define SN_PLL_ENABLE_REG 0x0D +#define SN_SCRAMBLE_CONFIG_REG 0x95 +#define SN_AUX_WDATA0_REG 0x64 +#define SN_AUX_ADDR_19_16_REG 0x74 +#define SN_AUX_ADDR_15_8_REG 0x75 +#define SN_AUX_ADDR_7_0_REG 0x76 +#define SN_AUX_LENGTH_REG 0x77 +#define SN_AUX_CMD_REG 0x78 +#define SN_ML_TX_MODE_REG 0x96 +/* video config specific registers */ +#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG 0x20 +#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG 0x21 +#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG 0x24 +#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG 0x25 +#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG 0x2C +#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG 0x2D +#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG 0x30 +#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG 0x31 +#define SN_CHA_HORIZONTAL_BACK_PORCH_REG 0x34 +#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36 +#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG 0x38 +#define SN_CHA_VERTICAL_FRONT_PORCH_REG 0x3A +#define SN_DATA_FORMAT_REG 0x5B + +#define MIN_DSI_CLK_FREQ_MHZ 40 + +/* fudge factor required to account for 8b/10b encoding */ +#define DP_CLK_FUDGE_NUM 10 +#define DP_CLK_FUDGE_DEN 8 + +#define DPPLL_CLK_SRC_REFCLK 0 +#define DPPLL_CLK_SRC_DSICLK 1 + +#define SN_DSIA_REFCLK_OFFSET 1 +#define SN_DSIA_LANE_OFFSET 3 +#define SN_DP_LANE_OFFSET 4 +#define SN_DP_DATA_RATE_OFFSET 5 +#define SN_TIMING_HIGH_OFFSET 8 + +#define SN_ENABLE_VID_STREAM_BIT BIT(3) +#define SN_DSIA_NUM_LANES_BITS (BIT(4) | BIT(3)) +#define SN_DP_NUM_LANES_BITS (BIT(5) | BIT(4)) +#define SN_DP_DATA_RATE_BITS (BIT(7) | BIT(6) | BIT(5))Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 79
Quoted 5 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +5
-0+#define SN_HPD_DISABLE_BIT BIT(0) + +struct ti_sn_bridge { +struct device *dev; +struct regmap *regmap;I have not spotted who advised you to use regmap, I think unless you share hardware between multiple drivers it is an overkill. Are there any features of this interface to use it over i2c?
Quoted 4 lines
Since we are using the i2c mechanism to update the control registers of the bridge, thought that regmap gives a proper interface for handling all the register control operations like read/write/update_bit/mask etc. Also most of the bridge driver present in drm are using this mechanism.
Yes, I know many ppl advertises it but also many are against using it in such simple cases. Anyway I do not want to force you to remove it, do what you prefer.
Quoted 183 lines
Quoted 14 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 90
Quoted 12 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +11
-0+struct drm_bridge bridge; +struct drm_connector connector; +struct device_node *host_node; +struct mipi_dsi_device *dsi; +struct clk *refclk; +struct drm_panel *panel; +struct gpio_desc *enable_gpio; +unsigned int num_supplies; +struct regulator_bulk_data *supplies; +struct i2c_adapter *ddc; +struct drm_display_mode curr_mode;I think you can drop this field, current mode is always available via:
Quoted 3 lines
pdata->bridge.encoder.crtc->state->adjusted_mode;Ok. i will remove this
curr_mode.Quoted 14 lines
Quoted 14 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +7
-0+}; + +static const struct regmap_range ti_sn_bridge_volatile_ranges[] = { + { .range_min =0 , .range_max =0xff }, +}; + +static const struct regmap_access_table ti_sn_bridge_volatile_table ={
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+ .yes_ranges = ti_sn_bridge_volatile_ranges, + .n_yes_ranges = ARRAY_SIZE(ti_sn_bridge_volatile_ranges), +}; + +static const struct regmap_config ti_sn_bridge_regmap_config = { + .reg_bits =8 ,Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 109
Quoted 6 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+ .val_bits = 8, + .volatile_table = &ti_sn_bridge_volatile_table, + .cache_type = REGCACHE_NONE, +}; + +#ifdef CONFIG_PM Please mark PM callbacks with
__maybe_unusedinstead if conditional macros.Ok.
Quoted 130 lines
Quoted 130 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +43
-0+static int ti_sn_bridge_resume (struct device *dev) +{ +struct ti_sn_bridge *pdata =dev_get_drvdata (dev); +int ret =0 ; + + ret =regulator_bulk_enable (pdata->num_supplies, pdata->supplies); +if (ret) { +DRM_ERROR ("failed to enable supplies %d\n" , ret); +return ret; + } + +gpiod_set_value (pdata->enable_gpio,1 ); + +return ret; +} + +static int ti_sn_bridge_suspend (struct device *dev) +{ +struct ti_sn_bridge *pdata =dev_get_drvdata (dev); +int ret =0 ; + +gpiod_set_value (pdata->enable_gpio,0 ); + + ret =regulator_bulk_disable (pdata->num_supplies, pdata->supplies); +if (ret) +DRM_ERROR ("failed to disable supplies %d\n" , ret); + +return ret; +} +#endif + +static const struct dev_pm_ops ti_sn_bridge_pm_ops = { +SET_RUNTIME_PM_OPS (ti_sn_bridge_suspend, ti_sn_bridge_resume,NULL ) +}; + +/* Connector funcs */ +static struct ti_sn_bridge * +connector_to_ti_sn_bridge (struct drm_connector *connector) +{ + return container_of(connector, struct ti_sn_bridge, connector); +} + +static int ti_sn_bridge_connector_get_modes(struct drm_connector*connector)
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +39
-0+{ +struct ti_sn_bridge *pdata =connector_to_ti_sn_bridge (connector); +struct drm_panel *panel = pdata->panel; +struct edid *edid; +u32 num_modes; + +if (panel) { +DRM_DEBUG_KMS ("get mode from connected drm_panel\n" ); +return drm_panel_get_modes (panel); + } + +if (!pdata->ddc) +return 0 ; + +pm_runtime_get_sync (pdata->dev); + edid =drm_get_edid (connector, pdata->ddc); +pm_runtime_put_sync (pdata->dev); +if (!edid) +return 0 ; + +drm_mode_connector_update_edid_property (connector, edid); + num_modes =drm_add_edid_modes (connector, edid); +kfree (edid); + +return num_modes; +} + +static enum drm_mode_status +ti_sn_bridge_connector_mode_valid (struct drm_connector *connector, +struct drm_display_mode *mode) +{ +/* maximum supported resolution is 4K at 60 fps */ +if (mode->clock >594000 ) +return MODE_CLOCK_HIGH ; + +return MODE_OK ; +} + +static struct drm_connector_helper_funcs
ti_sn_bridge_connector_helper_funcs= {drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+ .get_modes = ti_sn_bridge_connector_get_modes, + .mode_valid = ti_sn_bridge_connector_mode_valid, +}; + +static enum drm_connector_status +ti_sn_bridge_connector_detect(struct drm_connector *connector, boolforce)
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +15
-0+{ +struct ti_sn_bridge *pdata =connector_to_ti_sn_bridge (connector); + +/** +* TODO: Currently if drm_panel is present, then always +* return the status as connected. Need to add support to detect +* device state for no panel(hot pluggable) scenarios. +*/ +if (pdata->panel) +return connector_status_connected; +else +return connector_status_unknown; +} + +static const struct drm_connector_funcs ti_sn_bridge_connector_funcs= {
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +5
-0+ .fill_modes = drm_helper_probe_single_connector_modes, + .detect = ti_sn_bridge_connector_detect, + .destroy = drm_connector_cleanup, + .reset = drm_atomic_helper_connector_reset, + .atomic_duplicate_state =
drm_atomic_helper_connector_duplicate_state,drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +4
-0+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +static struct ti_sn_bridge *bridge_to_ti_sn_bridge(struct drm_bridge*bridge)
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +12
-0+{ + return container_of(bridge, struct ti_sn_bridge, bridge); +} + +static int ti_sn_bridge_read_device_rev (struct ti_sn_bridge *pdata) +{ +unsigned int rev =0 ; +int ret =0 ; + + ret =regmap_read (pdata->regmap,SN_DEVICE_REV_REG , &rev); +if (ret) { +DRM_ERROR ("Revision read failed %d\n" , ret);Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 240
Quoted 7 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +7
-0+return ret; + } + +if (rev !=SN_BRIDGE_REVISION_ID ) { +DRM_ERROR ("ti_sn_bridge revision id: 0x%x mismatch\n" , rev); + ret = -EINVAL ; + }Are you sure it won't work with other revisions?
Quoted 5 lines
The datasheet which i have, mentions the revision to be
0x2. I am not sure whether this driver works with other revisions, that's why i have put this check.
Also this check helps to confirm if the gpio and supplies of the bridge chip has been successfully enabled not depending on the value read.
So maybe better convert DRM_ERROR to warning and continue.
Quoted 23 lines
Quoted 14 lines
Quoted 14 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +13
-0+ +return ret; +} + +static const char *const ti_sn_bridge_supply_names[] = { +"vcca" , +"vcc" , +"vccio" , +"vpll" , +}; + +static int ti_sn_bridge_parse_regulators (struct ti_sn_bridge *pdata) +{Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 259
Quoted 6 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+unsigned int i; + + pdata->num_supplies =ARRAY_SIZE (ti_sn_bridge_supply_names); + + pdata->supplies =devm_kcalloc (pdata->dev, pdata->num_supplies, +sizeof (*pdata->supplies),GFP_KERNEL );It seems there is constant number of supplies. You can convert supplies to array and avoid dynamic allocation.
Quoted 5 lines
Just want to understand here what is the issue with dynamic allocation here?
We are using device managed allocation, so the resource management will be properly
done by kernel.
Just less code, no extra allocations, no extra resource management. Dynamic allocations in such case makes sense only if array size is computed in runtime.
Quoted 43 lines
Quoted 37 lines
Quoted 16 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +15
-0+if (!pdata->supplies) +return -ENOMEM ; + +for (i =0 ; i < pdata->num_supplies; i++) + pdata->supplies[i].supply = ti_sn_bridge_supply_names[i]; + +return devm_regulator_bulk_get (pdata->dev, + pdata->num_supplies, pdata->supplies); +} + +static int ti_sn_bridge_attach_panel (struct ti_sn_bridge *pdata) +{ +struct device_node *panel_node, *port, *endpoint; + + pdata->panel =NULL ;drivers/gpu/drm/bridge/ti-sn65dsi86.c line 284
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ port =of_graph_get_port_by_id (pdata->dev->of_node,1 ); +if (!port) +return 0 ; + + endpoint =of_get_child_by_name (port,"endpoint" ); +of_node_put (port); +if (!endpoint) { +DRM_ERROR ("no output endpoint found\n" ); +return -EINVAL ; + }Why not
of_graph_get_endpoint_by_regs?drivers/gpu/drm/bridge/ti-sn65dsi86.c line 291
Quoted 7 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +7
-0+ + panel_node =of_graph_get_remote_port_parent (endpoint); +of_node_put (endpoint); +if (!panel_node) { +DRM_ERROR ("no output node found\n" ); +return -EINVAL ; + }Or even
of_graph_get_remote_node?Quoted 4 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 294
Quoted 3 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +3
-0+ + pdata->panel =of_drm_find_panel (panel_node); +of_node_put (panel_node);Or even, even
drm_of_find_panel_or_bridge? :)Ok. i will use
drm_of_find_panel_or_bridge()here.
Quoted 7 lines
Quoted 5 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 297
Quoted 3 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +3
-0+if (!pdata->panel) { +DRM_ERROR ("no panel node found\n" ); +return -EINVAL ;You should probably return -EPROBE_DEFER here.
This API may not be called from
probe(), is it ok if we return -EPROBE_DEFER here?
Ahh, OK, but in such case this is incorrect, see below.
Quoted 50 lines
Quoted 19 lines
Quoted 19 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +18
-0+ } +drm_panel_attach (pdata->panel, &pdata->connector); +DRM_DEBUG_KMS ("drm panel attached to ti_sn_bridge\n" ); + +return 0 ; +} + +static int ti_sn_bridge_attach (struct drm_bridge *bridge) +{ +struct mipi_dsi_host *host; +struct mipi_dsi_device *dsi; +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); +int ret; +const struct mipi_dsi_device_info info = { .type ="ti_sn_bridge" , + .channel =0 , + .node =NULL , + }; +Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 322
Quoted 7 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +7
-0+if (!bridge->encoder) { +DRM_ERROR ("Parent encoder object not found\n" ); +return -ENODEV ; + } + +/* HPD not supported */ + pdata->connector.polled =0 ;You can skip this line.
OK.
Quoted 11 lines
Quoted 11 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +11
-0+ + ret =drm_connector_init (bridge->dev, &pdata->connector, + &ti_sn_bridge_connector_funcs, + DRM_MODE_CONNECTOR_eDP); +if (ret) { +DRM_ERROR ("Failed to initialize connector with drm\n" ); +return ret; + } + +drm_connector_helper_add (&pdata->connector, + &ti_sn_bridge_connector_helper_funcs);Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 339
Quoted 7 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +1
-0+ drm_mode_connector_attach_encoder(&pdata->connector,
bridge->encoder);drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +5
-0+ + host =of_find_mipi_dsi_host_by_node (pdata->host_node); +if (!host) { +DRM_ERROR ("failed to find dsi host\n" ); +return -ENODEV ;Again -EPROBE_DEFER.
Quoted 2 lines
This API may not be called from
probe(), is it ok if we return -EPROBE_DEFER here?
ditto
Quoted 32 lines
Quoted 23 lines
Quoted 23 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +13
-0+ } + + dsi =mipi_dsi_device_register_full (host, &info); +if (IS_ERR (dsi)) { +DRM_ERROR ("failed to create dsi device\n" ); + ret =PTR_ERR (dsi); +return ret; + } + +/* TODO: setting to 4 lanes always for now */ + dsi->lanes =4 ; + dsi->format =MIPI_DSI_FMT_RGB888 ; + dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
MIPI_DSI_MODE_VIDEO_SYNC_PULSE|drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +8
-0+MIPI_DSI_MODE_EOT_PACKET |MIPI_DSI_MODE_VIDEO_HSE ; + + ret =mipi_dsi_attach (dsi); +if (ret <0 ) { +DRM_ERROR ("failed to attach dsi to host\n" ); +mipi_dsi_device_unregister (dsi); +return ret; + }Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 366
Quoted 6 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+ + pdata->dsi = dsi; + +DRM_DEBUG_KMS ("ti_sn_bridge attached to dsi\n" ); +/* attach panel to bridge */ +ti_sn_bridge_attach_panel (pdata);
Function can fail, you should handle it.
Quoted 2 lines
This is not fatal, if this function fails then we assume there is no edp panel attached to this bridge.
So it seems wrong to me. You do not have guarantee that panel driver is bound at this moment. So in case of different driver binding order you will end up with incorrect setup: bridge will work with assumption there is no panel behind it, but the panel is there, it will be just probed later.
I think the proper solution here is to do not advertise bridge (ie call drm_bridge_add), until all required resources are present, including DSI host and panel. And to make it clear, if the panel is defined in bindings it means it is required.
Quoted 289 lines
Quoted 129 lines
Quoted 13 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +11
-0+ +return 0 ; +} + +static void ti_sn_bridge_mode_set (struct drm_bridge *bridge, +struct drm_display_mode *mode, +struct drm_display_mode *adj_mode) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); + + DRM_DEBUG("mode_set: hdisplay=%d, vdisplay=%d, vrefresh=%d,clock=%d\n",
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 382
Quoted 5 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +5
-0+ adj_mode->hdisplay, adj_mode->vdisplay, + adj_mode->vrefresh, adj_mode->clock); + +drm_mode_copy (&pdata->curr_mode, adj_mode); +}This callback can be dropped, see comment for
curr_mode.Quoted 109 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +35
-0+ +static void ti_sn_bridge_disable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); +struct drm_panel *panel = pdata->panel; + +if (panel) { +drm_panel_disable (panel); +drm_panel_unprepare (panel); + } + +/* disable video stream */ +regmap_update_bits (pdata->regmap,SN_ENH_FRAME_REG , +SN_ENABLE_VID_STREAM_BIT ,0 ); +/* semi auto link training mode OFF */ +regmap_write (pdata->regmap,SN_ML_TX_MODE_REG ,0 ); +/* disable DP PLL */ +regmap_write (pdata->regmap,SN_PLL_ENABLE_REG ,0 ); +} + +static u32 ti_sn_bridge_get_dsi_freq (struct ti_sn_bridge *pdata) +{ +u32 bit_rate_khz, clk_freq_khz; +struct drm_display_mode *mode = &pdata->curr_mode; + + bit_rate_khz = mode->clock * +mipi_dsi_pixel_format_to_bpp (pdata->dsi->format); + clk_freq_khz = bit_rate_khz / (pdata->dsi->lanes *2 ); + +return clk_freq_khz; +} + +#define REFCLK_LUT_SIZE 5 + +/* clk frequencies supported by bridge in Hz in case derived from REFCLK pin */
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +9
-0+static const u32 ti_sn_bridge_refclk_lut[] = { +12000000 , +19200000 , +26000000 , +27000000 , +38400000 , +}; + +/* clk frequencies supported by bridge in Hz in case derived from DACP/N pin */
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +62
-0+static const u32 ti_sn_bridge_dsiclk_lut[] = { +468000000 , +384000000 , +416000000 , +486000000 , +460800000 , +}; + +static void ti_sn_bridge_set_refclk (struct ti_sn_bridge *pdata) +{ +int i =0 ; +u8 refclk_src; +u32 refclk_rate; +const u32 *refclk_lut; + +if (pdata->refclk) { + refclk_src =DPPLL_CLK_SRC_REFCLK ; + refclk_rate =clk_get_rate (pdata->refclk); + refclk_lut = ti_sn_bridge_refclk_lut; +clk_prepare_enable (pdata->refclk); + }else { + refclk_src =DPPLL_CLK_SRC_DSICLK ; + refclk_rate =ti_sn_bridge_get_dsi_freq (pdata) *1000 ; + refclk_lut = ti_sn_bridge_dsiclk_lut; + } + +/* for i equals to REFCLK_LUT_SIZE means default frequency */ +for (i =0 ; i <REFCLK_LUT_SIZE ; i++) +if (refclk_lut[i] == refclk_rate) +break ; + +regmap_write (pdata->regmap,SN_REFCLK_FREQ_REG , + (refclk_src | (i <<SN_DSIA_REFCLK_OFFSET ))); +} + +/** +* LUT index corresponds to register value and +* LUT values corresponds to dp data rate supported +* by the bridge in Mbps unit. +*/ +static const unsigned int ti_sn_bridge_dp_rate_lut[] = { +0 ,1620 ,2160 ,2430 ,2700 ,3240 ,4320 ,5400 +}; + +static void ti_sn_bridge_set_dsi_dp_rate (struct ti_sn_bridge *pdata) +{ +unsigned int bit_rate_mhz, clk_freq_mhz, dp_rate_mhz; +unsigned int val =0 , i =0 ; +struct drm_display_mode *mode = &pdata->curr_mode; + +/* set DSIA clk frequency */ + bit_rate_mhz = (mode->clock /1000 ) * +mipi_dsi_pixel_format_to_bpp (pdata->dsi->format); + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes *2 ); + +/* for each increment in val, frequency increases by 5MHz */ + val = (MIN_DSI_CLK_FREQ_MHZ /5 ) + + (((clk_freq_mhz -MIN_DSI_CLK_FREQ_MHZ ) /5 ) &0xFF ); +regmap_write (pdata->regmap,SN_DSIA_CLK_FREQ_REG , val); + +/* set DP data rate */ + dp_rate_mhz = ((bit_rate_mhz / pdata->dsi->lanes) *
DP_CLK_FUDGE_NUM) /Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 494
Quoted 6 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+DP_CLK_FUDGE_DEN ; +for (i =0 ; i <ARRAY_SIZE (ti_sn_bridge_dp_rate_lut); i++) +if (ti_sn_bridge_dp_rate_lut[i] > dp_rate_mhz) +break ; +if (i ==ARRAY_SIZE (ti_sn_bridge_dp_rate_lut)) + i--;/* set to maximum possible */ Just use: for (i = 0; i <
ARRAY_SIZE(ti_sn_bridge_dp_rate_lut)- 1; i++)Ok.
Quoted 140 lines
Quoted 140 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+ +regmap_update_bits (pdata->regmap,SN_DATARATE_CONFIG_REG , +SN_DP_DATA_RATE_BITS , i <<SN_DP_DATA_RATE_OFFSET ); +} + +static void ti_sn_bridge_set_video_timings(struct ti_sn_bridge*pdata)
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +64
-0+{ +struct drm_display_mode *mode = &pdata->curr_mode; + +regmap_write (pdata->regmap,SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG , + mode->hdisplay &0xFF ); +regmap_write (pdata->regmap,SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG , + (mode->hdisplay >>SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG , + mode->vdisplay &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG , + (mode->vdisplay >>SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG , + (mode->hsync_end - mode->hsync_start) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG , + ((mode->hsync_end - mode->hsync_start) >> +SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG , + (mode->vsync_end - mode->vsync_start) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG , + ((mode->vsync_end - mode->vsync_start) >> +SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HORIZONTAL_BACK_PORCH_REG , + (mode->htotal - mode->hsync_end) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_BACK_PORCH_REG , + (mode->vtotal - mode->vsync_end) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HORIZONTAL_FRONT_PORCH_REG , + (mode->hsync_start - mode->hdisplay) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_FRONT_PORCH_REG , + (mode->vsync_start - mode->vdisplay) &0xFF ); +usleep_range (10000 ,10500 );/* 10ms delay recommended by spec */ +} + +static void ti_sn_bridge_enable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); +struct drm_panel *panel = pdata->panel; +unsigned int val =0 ; + +if (panel) { +drm_panel_prepare (panel); +/* in case drm_panel is connected then HPD is not supported */ +regmap_update_bits (pdata->regmap,SN_HPD_DISABLE_REG , +SN_HPD_DISABLE_BIT ,SN_HPD_DISABLE_BIT ); + } + +/* DSI_A lane config */ + val = (4 - pdata->dsi->lanes) <<SN_DSIA_LANE_OFFSET ; +regmap_update_bits (pdata->regmap,SN_DSI_LANES_REG , +SN_DSIA_NUM_LANES_BITS , val); + +/* DP lane config */ + val = (pdata->dsi->lanes -1 ) <<SN_DP_LANE_OFFSET ; +regmap_update_bits (pdata->regmap,SN_SSC_CONFIG_REG , +SN_DP_NUM_LANES_BITS , val); + +/* set dsi/dp clk frequency value */ +ti_sn_bridge_set_dsi_dp_rate (pdata); + +/* enable DP PLL */ +regmap_write (pdata->regmap,SN_PLL_ENABLE_REG ,1 ); +usleep_range (10000 ,10500 );/* 10ms delay recommended by spec */ + +/** +* The SN65DSI86 only supports ASSR Display Authentication method and
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +2
-0+* this method is enabled by default. An eDP panel must support this +* authentication method. We need to enable this method in the eDP panel
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +65
-0+* at DisplayPort address 0x0010A prior to link training. +*/ +regmap_write (pdata->regmap,SN_AUX_WDATA0_REG ,0x01 ); +regmap_write (pdata->regmap,SN_AUX_ADDR_19_16_REG ,0x00 ); +regmap_write (pdata->regmap,SN_AUX_ADDR_15_8_REG ,0x01 ); +regmap_write (pdata->regmap,SN_AUX_ADDR_7_0_REG ,0x0A ); +regmap_write (pdata->regmap,SN_AUX_LENGTH_REG ,0x01 ); +regmap_write (pdata->regmap,SN_AUX_CMD_REG ,0x81 ); +usleep_range (10000 ,10500 );/* 10ms delay recommended by spec */ + +/* Semi auto link training mode */ +regmap_write (pdata->regmap,SN_ML_TX_MODE_REG ,0x0A ); +msleep (20 );/* 20ms delay recommended by spec */ + +/* config video parameters */ +ti_sn_bridge_set_video_timings (pdata); + +/* enable video stream */ +regmap_update_bits (pdata->regmap,SN_ENH_FRAME_REG , +SN_ENABLE_VID_STREAM_BIT ,SN_ENABLE_VID_STREAM_BIT ); + +if (panel) +drm_panel_enable (panel); +} + +static void ti_sn_bridge_pre_enable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); + +pm_runtime_get_sync (pdata->dev); + +/* configure bridge CLK_SRC and ref_clk */ +ti_sn_bridge_set_refclk (pdata); +} + +static void ti_sn_bridge_post_disable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); + +if (pdata->refclk) +clk_disable_unprepare (pdata->refclk); + +pm_runtime_put_sync (pdata->dev); +} + +static const struct drm_bridge_funcs ti_sn_bridge_funcs = { + .attach = ti_sn_bridge_attach, + .pre_enable = ti_sn_bridge_pre_enable, + .enable = ti_sn_bridge_enable, + .disable = ti_sn_bridge_disable, + .post_disable = ti_sn_bridge_post_disable, + .mode_set = ti_sn_bridge_mode_set, +}; + +static int ti_sn_bridge_parse_dsi_host (struct ti_sn_bridge *pdata) +{ +struct device_node *np = pdata->dev->of_node; +struct device_node *end_node; + + end_node =of_graph_get_endpoint_by_regs (np,0 ,0 ); +if (!end_node) { +DRM_ERROR ("remote endpoint not found\n" ); +return -ENODEV ; + } +Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 638
Quoted 7 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +7
-0+ pdata->host_node =of_graph_get_remote_port_parent (end_node); +of_node_put (end_node); +if (!pdata->host_node) { +DRM_ERROR ("remote node not found\n" ); +return -ENODEV ; + } +of_node_put (pdata->host_node);Why not
of_graph_get_remote_node?
Quoted 2 lines
Just for my understanding, is
of_graph_get_remote_port_parentnot recommended? what is the benefit of usingof_graph_get_remote_node()?
Because it encapsulates both of_graph_get_endpoint_by_regs and of_graph_get_remote_port_parent with correct node reference management.
Quoted 58 lines
Quoted 49 lines
Quoted 49 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +48
-0+ +return 0 ; +} + +static int ti_sn_bridge_probe (struct i2c_client *client, +const struct i2c_device_id *id) +{ +struct ti_sn_bridge *pdata; +struct device_node *ddc_node; +int ret =0 ; + +if (!i2c_check_functionality (client->adapter,I2C_FUNC_I2C )) { +DRM_ERROR ("device doesn't support I2C\n" ); +return -ENODEV ; + } + + pdata =devm_kzalloc (&client->dev,sizeof (struct ti_sn_bridge ), +GFP_KERNEL ); +if (!pdata) +return -ENOMEM ; + + pdata->dev = &client->dev; +dev_set_drvdata (&client->dev, pdata); + + pdata->regmap =devm_regmap_init_i2c (client, + &ti_sn_bridge_regmap_config); +if (IS_ERR (pdata->regmap)) { +DRM_ERROR ("regmap i2c init failed\n" ); +return PTR_ERR (pdata->regmap); + } + + pdata->enable_gpio =devm_gpiod_get (pdata->dev, +"enable" ,GPIOD_OUT_LOW ); +if (IS_ERR (pdata->enable_gpio)) { +DRM_ERROR ("failed to get enable gpio from DT\n" ); + ret =PTR_ERR (pdata->enable_gpio); +return ret; + } + + ret =ti_sn_bridge_parse_regulators (pdata); +if (ret) { +DRM_ERROR ("failed to parse regulators\n" ); +return ret; + } + + ret =ti_sn_bridge_parse_dsi_host (pdata); +if (ret) +return ret;Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 691
Quoted 5 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +5
-0+ +pm_runtime_enable (pdata->dev); +pm_runtime_get_sync (pdata->dev); + ret =ti_sn_bridge_read_device_rev (pdata); +pm_runtime_put_sync (pdata->dev);I would put it into (pre) enable callbacks, maybe with marking it to run
only once.
Quoted 2 lines
do you mean we should put the
ti_sn_bridge_read_device_rev()in pre enable callback?
Yes, to avoid unnecessary switching on/off device.
Regards
Andrzej
Quoted 71 lines
Quoted 71 lines
Regards
AndrzejQuoted 67 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +67
-0+if (ret) + goto err_rev_read; + + pdata->refclk =devm_clk_get (pdata->dev,"refclk" ); + + ddc_node =of_parse_phandle (pdata->dev->of_node,"ddc-i2c-bus" ,0 ); +if (ddc_node) { + pdata->ddc =of_find_i2c_adapter_by_node (ddc_node); +of_node_put (ddc_node); +if (!pdata->ddc) { +DRM_DEBUG_KMS ("failed to read ddc node\n" ); + ret = -EPROBE_DEFER ; + goto err_rev_read; + } + }else { +DRM_DEBUG_KMS ("no ddc property found\n" ); + } + +i2c_set_clientdata (client, pdata); + + pdata->bridge.funcs = &ti_sn_bridge_funcs; + pdata->bridge.of_node = client->dev.of_node; + +drm_bridge_add (&pdata->bridge); + +return 0 ; + +err_rev_read: +pm_runtime_disable (pdata->dev); +return ret; +} + +static int ti_sn_bridge_remove (struct i2c_client *client) +{ +struct ti_sn_bridge *pdata =i2c_get_clientdata (client); + +if (!pdata) +return -EINVAL ; + +mipi_dsi_detach (pdata->dsi); +mipi_dsi_device_unregister (pdata->dsi); + +drm_bridge_remove (&pdata->bridge); +pm_runtime_disable (pdata->dev); +i2c_put_adapter (pdata->ddc); + +return 0 ; +} + +static struct i2c_device_id ti_sn_bridge_id[] = { + {"ti,sn65dsi86" ,0 }, + {}, +}; +MODULE_DEVICE_TABLE (i2c, ti_sn_bridge_id); + +static const struct of_device_id ti_sn_bridge_match_table[] = { + {.compatible ="ti,sn65dsi86" }, + {}, +}; +MODULE_DEVICE_TABLE (of, ti_sn_bridge_match_table); + +static struct i2c_driver ti_sn_bridge_driver = { + .driver = { + .name ="ti_sn65dsi86" , + .of_match_table = ti_sn_bridge_match_table, + .pm = &ti_sn_bridge_pm_ops, + },
Quoted 10 lines
Quoted 8 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 766
Quoted 8 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +8
-0+ .probe = ti_sn_bridge_probe, + .remove = ti_sn_bridge_remove, + .id_table = ti_sn_bridge_id, +}; + +module_i2c_driver (ti_sn_bridge_driver); +MODULE_DESCRIPTION ("sn65dsi86 DSI to eDP bridge driver" ); +MODULE_LICENSE ("GPL v2" );
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
A message that is not in the archive (sent off-list or not yet received): e270a97a-43c9-2725-aace-f3829a916f84-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org
Re: [PATCH v6 1/4] drm/bridge: add support for sn65dsi86 bridge driver
Quoted 415 lines · Andrzej Hajda
I suppose you wanted to respond on the list, so I have added back all recipients.
Quoted 214 lines · On 16.05.2018 06:39, [email protected] wrote:
Quoted 204 lines · Andrzej Hajda
Quoted 203 lines · Sandeep Panda
Add support for TI's sn65dsi86 dsi2edp bridge chip. The chip converts DSI transmitted signal to eDP signal, which is fed to the connected eDP panel.
This chip can be controlled via either i2c interface or dsi interface. Currently in driver all the control registers are being accessed through i2c interface only. Also as of now HPD support has not been added to bridge chip driver.
Changes in v1:
- Split the dt-bindings and the driver support into separate
patches
(Andrzej Hajda).
- Use of gpiod APIs to parse and configure gpios instead of
obsolete
ones
(Andrzej Hajda).
- Use macros to define the register offsets (Andrzej Hajda).Changes in v2:
- Separate out edp panel specific HW resource handling from bridge driver and create a separate edp panel drivers to handle panel specific mode information and HW resources (Sean Paul).
- Replace pr_* APIs to DRM_* APIs to log error or debug information (Sean Paul).
- Remove some of the unnecessary structure/variable from driver
(Sean
Paul).
- Rename the function and structure prefix "sn65dsi86" to
"ti_sn_bridge"
(Sean Paul / Rob Herring).
- Remove most of the hard-coding and modified the bridge init
sequence
based on current mode (Sean Paul).
- Remove the existing function to retrieve the EDID data and implemented this as ani2c_adapterand usedrm_get_edid()(Sean
Paul).
- Remove the dummy irq handler implementation, will add back the proper irq handling later (Sean Paul).
- Capture the required enable gpios in a single array based on dt
entry
instead of having individual descriptor for each gpio (Sean
Paul).Changes in v3:
- Remove usage ofirq_gpioand replace it as "interrupts" property
(Rob
Herring).
- Remove the unnecessary header file inclusions (Sean Paul).
- Rearrange the header files in alphabetical order (Sean Paul).
- Use regmap interface to perform i2c transactions.
- Update Copyright/License field and address other review comments (Jordan Crouse).Changes in v4:
- Update License/Copyright (Sean Paul).
- AddKconfigandMakefilechanges (Sean Paul).
- Drop i2c gpio handling from this bridge driver, since i2c sda/scl
gpios
will be handled by i2c master.
- Update required supplies names.
- Remove unnecessary goto statements (Sean Paul).
- Add mutex lock topower_ctrlAPI to avoid race conditions (Sean Paul).
- Add support to parse reference clk frequency from dt(optional).
- Update the bridge chip enable/disable sequence.Changes in v5:
- FixedKbuildtest service reported warnings.Changes in v6:
- Use PM runtime based ref-counting instead of localref_count
mechanism
(Stephen Boyd).
- Clean up some debug logs and indentations (Sean Paul).
- Simplify dp rate calculation (Sean Paul).
- Add support to configure refclk based on input REFCLK pin or
DACP/N
pin (Stephen Boyd).Signed-off-by Sandeep Panda <[email protected]>--- drivers/gpu/drm/bridge/Kconfig | 9 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ti-sn65dsi86.c | 766 ++++++++++++++++++++++++++++++++++ 3 files changed, 776 insertions(+) create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.cdrivers/gpu/drm/bridge/Kconfig +9
-0@@ -108,6 +108,15 @@ config DRM_TI_TFP410 ---help--- Texas Instruments TFP410 DVI/HDMI Transmitter driver +config DRM_TI_SN65DSI86 + tristate "TI SN65DSI86 DSI to eDP bridge" + depends on OF + select DRM_KMS_HELPER + select REGMAP_I2C + select DRM_PANEL + ---help--- + Texas Instruments SN65DSI86 DSI to eDP Bridge driver + source "drivers/gpu/drm/bridge/analogix/Kconfig" source "drivers/gpu/drm/bridge/adv7511/Kconfig"drivers/gpu/drm/bridge/Makefile +1
-0@@ -12,4 +12,5 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o +obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o obj-y += synopsys/drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +74
-0@@ -0,0 +1,766 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2018, The Linux Foundation. All rights reserved. +*/ + +#include <drm/drmP.h> +#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_panel.h> +#include <linux/clk.h> +#include <linux/gpio.h> +#include <linux/i2c.h> +#include <linux/of_gpio.h> +#include <linux/of_graph.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> + +#define SN_BRIDGE_REVISION_ID 0x2 + +/* Link Training specific registers */ +#define SN_DEVICE_REV_REG 0x08 +#define SN_HPD_DISABLE_REG 0x5C +#define SN_REFCLK_FREQ_REG 0x0A +#define SN_DSI_LANES_REG 0x10 +#define SN_DSIA_CLK_FREQ_REG 0x12 +#define SN_ENH_FRAME_REG 0x5A +#define SN_SSC_CONFIG_REG 0x93 +#define SN_DATARATE_CONFIG_REG 0x94 +#define SN_PLL_ENABLE_REG 0x0D +#define SN_SCRAMBLE_CONFIG_REG 0x95 +#define SN_AUX_WDATA0_REG 0x64 +#define SN_AUX_ADDR_19_16_REG 0x74 +#define SN_AUX_ADDR_15_8_REG 0x75 +#define SN_AUX_ADDR_7_0_REG 0x76 +#define SN_AUX_LENGTH_REG 0x77 +#define SN_AUX_CMD_REG 0x78 +#define SN_ML_TX_MODE_REG 0x96 +/* video config specific registers */ +#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG 0x20 +#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG 0x21 +#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG 0x24 +#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG 0x25 +#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG 0x2C +#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG 0x2D +#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG 0x30 +#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG 0x31 +#define SN_CHA_HORIZONTAL_BACK_PORCH_REG 0x34 +#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36 +#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG 0x38 +#define SN_CHA_VERTICAL_FRONT_PORCH_REG 0x3A +#define SN_DATA_FORMAT_REG 0x5B + +#define MIN_DSI_CLK_FREQ_MHZ 40 + +/* fudge factor required to account for 8b/10b encoding */ +#define DP_CLK_FUDGE_NUM 10 +#define DP_CLK_FUDGE_DEN 8 + +#define DPPLL_CLK_SRC_REFCLK 0 +#define DPPLL_CLK_SRC_DSICLK 1 + +#define SN_DSIA_REFCLK_OFFSET 1 +#define SN_DSIA_LANE_OFFSET 3 +#define SN_DP_LANE_OFFSET 4 +#define SN_DP_DATA_RATE_OFFSET 5 +#define SN_TIMING_HIGH_OFFSET 8 + +#define SN_ENABLE_VID_STREAM_BIT BIT(3) +#define SN_DSIA_NUM_LANES_BITS (BIT(4) | BIT(3)) +#define SN_DP_NUM_LANES_BITS (BIT(5) | BIT(4)) +#define SN_DP_DATA_RATE_BITS (BIT(7) | BIT(6) | BIT(5))Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 79
Quoted 5 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +5
-0+#define SN_HPD_DISABLE_BIT BIT(0) + +struct ti_sn_bridge { +struct device *dev; +struct regmap *regmap;I have not spotted who advised you to use regmap, I think unless you share hardware between multiple drivers it is an overkill. Are there any features of this interface to use it over i2c?
Quoted 4 lines
Since we are using the i2c mechanism to update the control registers of the bridge, thought that regmap gives a proper interface for handling all the register control operations like read/write/update_bit/mask etc. Also most of the bridge driver present in drm are using this mechanism.
Yes, I know many ppl advertises it but also many are against using it in such simple cases. Anyway I do not want to force you to remove it, do what you prefer.
Quoted 188 lines
Quoted 14 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 90
Quoted 12 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +11
-0+struct drm_bridge bridge; +struct drm_connector connector; +struct device_node *host_node; +struct mipi_dsi_device *dsi; +struct clk *refclk; +struct drm_panel *panel; +struct gpio_desc *enable_gpio; +unsigned int num_supplies; +struct regulator_bulk_data *supplies; +struct i2c_adapter *ddc; +struct drm_display_mode curr_mode;I think you can drop this field, current mode is always available via:
Quoted 3 lines
pdata->bridge.encoder.crtc->state->adjusted_mode;Ok. i will remove this
curr_mode.Quoted 15 lines
Quoted 15 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +7
-0+}; + +static const struct regmap_range ti_sn_bridge_volatile_ranges[] = { + { .range_min =0 , .range_max =0xff }, +}; + +static const struct regmap_access_table ti_sn_bridge_volatile_table=
{drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+ .yes_ranges = ti_sn_bridge_volatile_ranges, + .n_yes_ranges = ARRAY_SIZE(ti_sn_bridge_volatile_ranges), +}; + +static const struct regmap_config ti_sn_bridge_regmap_config = { + .reg_bits =8 ,Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 109
Quoted 6 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+ .val_bits = 8, + .volatile_table = &ti_sn_bridge_volatile_table, + .cache_type = REGCACHE_NONE, +}; + +#ifdef CONFIG_PM Please mark PM callbacks with
__maybe_unusedinstead if conditional macros.Ok.
Quoted 143 lines
Quoted 131 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +24
-0+static int ti_sn_bridge_resume (struct device *dev) +{ +struct ti_sn_bridge *pdata =dev_get_drvdata (dev); +int ret =0 ; + + ret =regulator_bulk_enable (pdata->num_supplies, pdata->supplies); +if (ret) { +DRM_ERROR ("failed to enable supplies %d\n" , ret); +return ret; + } + +gpiod_set_value (pdata->enable_gpio,1 ); + +return ret; +} + +static int ti_sn_bridge_suspend (struct device *dev) +{ +struct ti_sn_bridge *pdata =dev_get_drvdata (dev); +int ret =0 ; + +gpiod_set_value (pdata->enable_gpio,0 ); + + ret = regulator_bulk_disable(pdata->num_supplies,
pdata->supplies);drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +9
-0+if (ret) +DRM_ERROR ("failed to disable supplies %d\n" , ret); + +return ret; +} +#endif + +static const struct dev_pm_ops ti_sn_bridge_pm_ops = { + SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume,NULL)
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+}; + +/* Connector funcs */ +static struct ti_sn_bridge * +connector_to_ti_sn_bridge (struct drm_connector *connector) +{ + return container_of(connector, struct ti_sn_bridge, connector); +} + +static int ti_sn_bridge_connector_get_modes(struct drm_connector*connector)
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +39
-0+{ +struct ti_sn_bridge *pdata =connector_to_ti_sn_bridge (connector); +struct drm_panel *panel = pdata->panel; +struct edid *edid; +u32 num_modes; + +if (panel) { +DRM_DEBUG_KMS ("get mode from connected drm_panel\n" ); +return drm_panel_get_modes (panel); + } + +if (!pdata->ddc) +return 0 ; + +pm_runtime_get_sync (pdata->dev); + edid =drm_get_edid (connector, pdata->ddc); +pm_runtime_put_sync (pdata->dev); +if (!edid) +return 0 ; + +drm_mode_connector_update_edid_property (connector, edid); + num_modes =drm_add_edid_modes (connector, edid); +kfree (edid); + +return num_modes; +} + +static enum drm_mode_status +ti_sn_bridge_connector_mode_valid (struct drm_connector *connector, +struct drm_display_mode *mode) +{ +/* maximum supported resolution is 4K at 60 fps */ +if (mode->clock >594000 ) +return MODE_CLOCK_HIGH ; + +return MODE_OK ; +} + +static struct drm_connector_helper_funcs
ti_sn_bridge_connector_helper_funcs= {drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+ .get_modes = ti_sn_bridge_connector_get_modes, + .mode_valid = ti_sn_bridge_connector_mode_valid, +}; + +static enum drm_connector_status +ti_sn_bridge_connector_detect(struct drm_connector *connector, boolforce)
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +15
-0+{ +struct ti_sn_bridge *pdata =connector_to_ti_sn_bridge (connector); + +/** +* TODO: Currently if drm_panel is present, then always +* return the status as connected. Need to add support to detect +* device state for no panel(hot pluggable) scenarios. +*/ +if (pdata->panel) +return connector_status_connected; +else +return connector_status_unknown; +} + +static const struct drm_connector_funcs
ti_sn_bridge_connector_funcs
= {drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +5
-0+ .fill_modes = drm_helper_probe_single_connector_modes, + .detect = ti_sn_bridge_connector_detect, + .destroy = drm_connector_cleanup, + .reset = drm_atomic_helper_connector_reset, + .atomic_duplicate_state =
drm_atomic_helper_connector_duplicate_state,drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +4
-0+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +static struct ti_sn_bridge *bridge_to_ti_sn_bridge(struct
drm_bridge
*bridge)drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +9
-0+{ + return container_of(bridge, struct ti_sn_bridge, bridge); +} + +static int ti_sn_bridge_read_device_rev (struct ti_sn_bridge *pdata) +{ +unsigned int rev =0 ; +int ret =0 ; +drivers/gpu/drm/bridge/ti-sn65dsi86.c line 240
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ ret =regmap_read (pdata->regmap,SN_DEVICE_REV_REG , &rev); +if (ret) { +DRM_ERROR ("Revision read failed %d\n" , ret); +return ret; + } + +if (rev !=SN_BRIDGE_REVISION_ID ) { +DRM_ERROR ("ti_sn_bridge revision id: 0x%x mismatch\n" , rev); + ret = -EINVAL ; + }Are you sure it won't work with other revisions?
Quoted 8 lines
Quoted 5 lines
The datasheet which i have, mentions the revision to be
0x2. I am not sure whether this driver works with other revisions, that's why i have put this check.
Also this check helps to confirm if the gpio and supplies of the bridge chip has been successfully enabled not depending on the value read.So maybe better convert
DRM_ERRORto warning and continue.
Ok. I will remove the failure return in case of id mismatch and just print a warning.
Quoted 30 lines
Quoted 24 lines
Quoted 15 lines
Quoted 15 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +12
-0+ +return ret; +} + +static const char *const ti_sn_bridge_supply_names[] = { +"vcca" , +"vcc" , +"vccio" , +"vpll" , +}; + +static int ti_sn_bridge_parse_regulators(struct ti_sn_bridge*pdata)
+{Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 259
Quoted 6 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+unsigned int i; + + pdata->num_supplies =ARRAY_SIZE (ti_sn_bridge_supply_names); + + pdata->supplies =devm_kcalloc (pdata->dev, pdata->num_supplies, +sizeof (*pdata->supplies),GFP_KERNEL );It seems there is constant number of supplies. You can convert supplies to array and avoid dynamic allocation.
Quoted 5 lines
Just want to understand here what is the issue with dynamic allocation here?
We are using device managed allocation, so the resource management will be properly
done by kernel.
Quoted 4 lines
Just less code, no extra allocations, no extra resource management. Dynamic allocations in such case makes sense only if array size is computed in runtime.
Ok.
Quoted 148 lines
Quoted 43 lines
Quoted 37 lines
Quoted 16 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +15
-0+if (!pdata->supplies) +return -ENOMEM ; + +for (i =0 ; i < pdata->num_supplies; i++) + pdata->supplies[i].supply = ti_sn_bridge_supply_names[i]; + +return devm_regulator_bulk_get (pdata->dev, + pdata->num_supplies, pdata->supplies); +} + +static int ti_sn_bridge_attach_panel (struct ti_sn_bridge *pdata) +{ +struct device_node *panel_node, *port, *endpoint; + + pdata->panel =NULL ;drivers/gpu/drm/bridge/ti-sn65dsi86.c line 284
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ port =of_graph_get_port_by_id (pdata->dev->of_node,1 ); +if (!port) +return 0 ; + + endpoint =of_get_child_by_name (port,"endpoint" ); +of_node_put (port); +if (!endpoint) { +DRM_ERROR ("no output endpoint found\n" ); +return -EINVAL ; + }Why not
of_graph_get_endpoint_by_regs?drivers/gpu/drm/bridge/ti-sn65dsi86.c line 291
Quoted 7 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +7
-0+ + panel_node =of_graph_get_remote_port_parent (endpoint); +of_node_put (endpoint); +if (!panel_node) { +DRM_ERROR ("no output node found\n" ); +return -EINVAL ; + }Or even
of_graph_get_remote_node?Quoted 4 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 294
Quoted 3 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +3
-0+ + pdata->panel =of_drm_find_panel (panel_node); +of_node_put (panel_node);Or even, even
drm_of_find_panel_or_bridge? :)Ok. i will use
drm_of_find_panel_or_bridge()here.Quoted 7 lines
Quoted 5 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 297
Quoted 3 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +3
-0+if (!pdata->panel) { +DRM_ERROR ("no panel node found\n" ); +return -EINVAL ;You should probably return -EPROBE_DEFER here.
This API may not be called from
probe(), is it ok if we return -EPROBE_DEFER here?Ahh, OK, but in such case this is incorrect, see below.
Quoted 50 lines
Quoted 19 lines
Quoted 19 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +18
-0+ } +drm_panel_attach (pdata->panel, &pdata->connector); +DRM_DEBUG_KMS ("drm panel attached to ti_sn_bridge\n" ); + +return 0 ; +} + +static int ti_sn_bridge_attach (struct drm_bridge *bridge) +{ +struct mipi_dsi_host *host; +struct mipi_dsi_device *dsi; +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); +int ret; +const struct mipi_dsi_device_info info = { .type ="ti_sn_bridge" , + .channel =0 , + .node =NULL , + }; +Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 322
Quoted 7 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +7
-0+if (!bridge->encoder) { +DRM_ERROR ("Parent encoder object not found\n" ); +return -ENODEV ; + } + +/* HPD not supported */ + pdata->connector.polled =0 ;You can skip this line.
OK.
Quoted 11 lines
Quoted 11 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +11
-0+ + ret =drm_connector_init (bridge->dev, &pdata->connector, + &ti_sn_bridge_connector_funcs, + DRM_MODE_CONNECTOR_eDP); +if (ret) { +DRM_ERROR ("Failed to initialize connector with drm\n" ); +return ret; + } + +drm_connector_helper_add (&pdata->connector, + &ti_sn_bridge_connector_helper_funcs);Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 339
Quoted 7 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +1
-0+ drm_mode_connector_attach_encoder(&pdata->connector,
bridge->encoder);drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +5
-0+ + host =of_find_mipi_dsi_host_by_node (pdata->host_node); +if (!host) { +DRM_ERROR ("failed to find dsi host\n" ); +return -ENODEV ;Again -EPROBE_DEFER.
Quoted 2 lines
This API may not be called from
probe(), is it ok if we return -EPROBE_DEFER here?ditto
Quoted 32 lines
Quoted 23 lines
Quoted 23 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +13
-0+ } + + dsi =mipi_dsi_device_register_full (host, &info); +if (IS_ERR (dsi)) { +DRM_ERROR ("failed to create dsi device\n" ); + ret =PTR_ERR (dsi); +return ret; + } + +/* TODO: setting to 4 lanes always for now */ + dsi->lanes =4 ; + dsi->format =MIPI_DSI_FMT_RGB888 ; + dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
MIPI_DSI_MODE_VIDEO_SYNC_PULSE|drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +8
-0+MIPI_DSI_MODE_EOT_PACKET |MIPI_DSI_MODE_VIDEO_HSE ; + + ret =mipi_dsi_attach (dsi); +if (ret <0 ) { +DRM_ERROR ("failed to attach dsi to host\n" ); +mipi_dsi_device_unregister (dsi); +return ret; + }Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 366
Quoted 6 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+ + pdata->dsi = dsi; + +DRM_DEBUG_KMS ("ti_sn_bridge attached to dsi\n" ); +/* attach panel to bridge */ +ti_sn_bridge_attach_panel (pdata);
Function can fail, you should handle it.Quoted 2 lines
This is not fatal, if this function fails then we assume there is no edp panel attached to this bridge.
So it seems wrong to me. You do not have guarantee that panel driver is bound at this moment. So in case of different driver binding order you will end up with incorrect setup: bridge will work with assumption there is no panel behind it, but the panel is there, it will be just probed later.
Quoted 6 lines
I think the proper solution here is to do not advertise bridge (ie call
drm_bridge_add), until all required resources are present, including DSI host and panel. And to make it clear, if the panel is defined in bindings it means it is required.
Ok. So i will move this attach code to ti_sn_bridge_probe() and if i dont get the panel there i will return -EPROBE_DEFER. Currently HPD is not supported in the driver, so we will rely on the panel to successfully finish bridge driver's probe.
Quoted 291 lines
Quoted 291 lines
Quoted 130 lines
Quoted 13 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +11
-0+ +return 0 ; +} + +static void ti_sn_bridge_mode_set (struct drm_bridge *bridge, +struct drm_display_mode *mode, +struct drm_display_mode *adj_mode) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); + + DRM_DEBUG("mode_set: hdisplay=%d, vdisplay=%d, vrefresh=%d,clock=%d\n",
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 382
Quoted 5 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +5
-0+ adj_mode->hdisplay, adj_mode->vdisplay, + adj_mode->vrefresh, adj_mode->clock); + +drm_mode_copy (&pdata->curr_mode, adj_mode); +}This callback can be dropped, see comment for
curr_mode.Quoted 110 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +35
-0+ +static void ti_sn_bridge_disable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); +struct drm_panel *panel = pdata->panel; + +if (panel) { +drm_panel_disable (panel); +drm_panel_unprepare (panel); + } + +/* disable video stream */ +regmap_update_bits (pdata->regmap,SN_ENH_FRAME_REG , +SN_ENABLE_VID_STREAM_BIT ,0 ); +/* semi auto link training mode OFF */ +regmap_write (pdata->regmap,SN_ML_TX_MODE_REG ,0 ); +/* disable DP PLL */ +regmap_write (pdata->regmap,SN_PLL_ENABLE_REG ,0 ); +} + +static u32 ti_sn_bridge_get_dsi_freq (struct ti_sn_bridge *pdata) +{ +u32 bit_rate_khz, clk_freq_khz; +struct drm_display_mode *mode = &pdata->curr_mode; + + bit_rate_khz = mode->clock * +mipi_dsi_pixel_format_to_bpp (pdata->dsi->format); + clk_freq_khz = bit_rate_khz / (pdata->dsi->lanes *2 ); + +return clk_freq_khz; +} + +#define REFCLK_LUT_SIZE 5 + +/* clk frequencies supported by bridge in Hz in case derived from REFCLK pin */
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +9
-0+static const u32 ti_sn_bridge_refclk_lut[] = { +12000000 , +19200000 , +26000000 , +27000000 , +38400000 , +}; + +/* clk frequencies supported by bridge in Hz in case derived from DACP/N pin */
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +45
-0+static const u32 ti_sn_bridge_dsiclk_lut[] = { +468000000 , +384000000 , +416000000 , +486000000 , +460800000 , +}; + +static void ti_sn_bridge_set_refclk (struct ti_sn_bridge *pdata) +{ +int i =0 ; +u8 refclk_src; +u32 refclk_rate; +const u32 *refclk_lut; + +if (pdata->refclk) { + refclk_src =DPPLL_CLK_SRC_REFCLK ; + refclk_rate =clk_get_rate (pdata->refclk); + refclk_lut = ti_sn_bridge_refclk_lut; +clk_prepare_enable (pdata->refclk); + }else { + refclk_src =DPPLL_CLK_SRC_DSICLK ; + refclk_rate =ti_sn_bridge_get_dsi_freq (pdata) *1000 ; + refclk_lut = ti_sn_bridge_dsiclk_lut; + } + +/* for i equals to REFCLK_LUT_SIZE means default frequency */ +for (i =0 ; i <REFCLK_LUT_SIZE ; i++) +if (refclk_lut[i] == refclk_rate) +break ; + +regmap_write (pdata->regmap,SN_REFCLK_FREQ_REG , + (refclk_src | (i <<SN_DSIA_REFCLK_OFFSET ))); +} + +/** +* LUT index corresponds to register value and +* LUT values corresponds to dp data rate supported +* by the bridge in Mbps unit. +*/ +static const unsigned int ti_sn_bridge_dp_rate_lut[] = { +0 ,1620 ,2160 ,2430 ,2700 ,3240 ,4320 ,5400 +}; + +static void ti_sn_bridge_set_dsi_dp_rate(struct ti_sn_bridge*pdata)
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +17
-0+{ +unsigned int bit_rate_mhz, clk_freq_mhz, dp_rate_mhz; +unsigned int val =0 , i =0 ; +struct drm_display_mode *mode = &pdata->curr_mode; + +/* set DSIA clk frequency */ + bit_rate_mhz = (mode->clock /1000 ) * +mipi_dsi_pixel_format_to_bpp (pdata->dsi->format); + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes *2 ); + +/* for each increment in val, frequency increases by 5MHz */ + val = (MIN_DSI_CLK_FREQ_MHZ /5 ) + + (((clk_freq_mhz -MIN_DSI_CLK_FREQ_MHZ ) /5 ) &0xFF ); +regmap_write (pdata->regmap,SN_DSIA_CLK_FREQ_REG , val); + +/* set DP data rate */ + dp_rate_mhz = ((bit_rate_mhz / pdata->dsi->lanes) *
DP_CLK_FUDGE_NUM) /Quoted 9 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c line 494
Quoted 6 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+DP_CLK_FUDGE_DEN ; +for (i =0 ; i <ARRAY_SIZE (ti_sn_bridge_dp_rate_lut); i++) +if (ti_sn_bridge_dp_rate_lut[i] > dp_rate_mhz) +break ; +if (i ==ARRAY_SIZE (ti_sn_bridge_dp_rate_lut)) + i--;/* set to maximum possible */ Just use: for (i = 0; i <
ARRAY_SIZE(ti_sn_bridge_dp_rate_lut)- 1; i++)Ok.
Quoted 150 lines
Quoted 138 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+ +regmap_update_bits (pdata->regmap,SN_DATARATE_CONFIG_REG , +SN_DP_DATA_RATE_BITS , i <<SN_DP_DATA_RATE_OFFSET ); +} + +static void ti_sn_bridge_set_video_timings(struct ti_sn_bridge*pdata)
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +64
-0+{ +struct drm_display_mode *mode = &pdata->curr_mode; + +regmap_write (pdata->regmap,SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG , + mode->hdisplay &0xFF ); +regmap_write (pdata->regmap,SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG , + (mode->hdisplay >>SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG , + mode->vdisplay &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG , + (mode->vdisplay >>SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG , + (mode->hsync_end - mode->hsync_start) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG , + ((mode->hsync_end - mode->hsync_start) >> +SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG , + (mode->vsync_end - mode->vsync_start) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG , + ((mode->vsync_end - mode->vsync_start) >> +SN_TIMING_HIGH_OFFSET ) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HORIZONTAL_BACK_PORCH_REG , + (mode->htotal - mode->hsync_end) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_BACK_PORCH_REG , + (mode->vtotal - mode->vsync_end) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_HORIZONTAL_FRONT_PORCH_REG , + (mode->hsync_start - mode->hdisplay) &0xFF ); +regmap_write (pdata->regmap,SN_CHA_VERTICAL_FRONT_PORCH_REG , + (mode->vsync_start - mode->vdisplay) &0xFF ); +usleep_range (10000 ,10500 );/* 10ms delay recommended by spec */ +} + +static void ti_sn_bridge_enable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); +struct drm_panel *panel = pdata->panel; +unsigned int val =0 ; + +if (panel) { +drm_panel_prepare (panel); +/* in case drm_panel is connected then HPD is not supported */ +regmap_update_bits (pdata->regmap,SN_HPD_DISABLE_REG , +SN_HPD_DISABLE_BIT ,SN_HPD_DISABLE_BIT ); + } + +/* DSI_A lane config */ + val = (4 - pdata->dsi->lanes) <<SN_DSIA_LANE_OFFSET ; +regmap_update_bits (pdata->regmap,SN_DSI_LANES_REG , +SN_DSIA_NUM_LANES_BITS , val); + +/* DP lane config */ + val = (pdata->dsi->lanes -1 ) <<SN_DP_LANE_OFFSET ; +regmap_update_bits (pdata->regmap,SN_SSC_CONFIG_REG , +SN_DP_NUM_LANES_BITS , val); + +/* set dsi/dp clk frequency value */ +ti_sn_bridge_set_dsi_dp_rate (pdata); + +/* enable DP PLL */ +regmap_write (pdata->regmap,SN_PLL_ENABLE_REG ,1 ); +usleep_range (10000 ,10500 );/* 10ms delay recommended by spec */ + +/** +* The SN65DSI86 only supports ASSR Display Authentication method and
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +1
-0+* this method is enabled by default. An eDP panel must support this
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +1
-0+* authentication method. We need to enable this method in the eDP panel
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +62
-0+* at DisplayPort address 0x0010A prior to link training. +*/ +regmap_write (pdata->regmap,SN_AUX_WDATA0_REG ,0x01 ); +regmap_write (pdata->regmap,SN_AUX_ADDR_19_16_REG ,0x00 ); +regmap_write (pdata->regmap,SN_AUX_ADDR_15_8_REG ,0x01 ); +regmap_write (pdata->regmap,SN_AUX_ADDR_7_0_REG ,0x0A ); +regmap_write (pdata->regmap,SN_AUX_LENGTH_REG ,0x01 ); +regmap_write (pdata->regmap,SN_AUX_CMD_REG ,0x81 ); +usleep_range (10000 ,10500 );/* 10ms delay recommended by spec */ + +/* Semi auto link training mode */ +regmap_write (pdata->regmap,SN_ML_TX_MODE_REG ,0x0A ); +msleep (20 );/* 20ms delay recommended by spec */ + +/* config video parameters */ +ti_sn_bridge_set_video_timings (pdata); + +/* enable video stream */ +regmap_update_bits (pdata->regmap,SN_ENH_FRAME_REG , +SN_ENABLE_VID_STREAM_BIT ,SN_ENABLE_VID_STREAM_BIT ); + +if (panel) +drm_panel_enable (panel); +} + +static void ti_sn_bridge_pre_enable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); + +pm_runtime_get_sync (pdata->dev); + +/* configure bridge CLK_SRC and ref_clk */ +ti_sn_bridge_set_refclk (pdata); +} + +static void ti_sn_bridge_post_disable (struct drm_bridge *bridge) +{ +struct ti_sn_bridge *pdata =bridge_to_ti_sn_bridge (bridge); + +if (pdata->refclk) +clk_disable_unprepare (pdata->refclk); + +pm_runtime_put_sync (pdata->dev); +} + +static const struct drm_bridge_funcs ti_sn_bridge_funcs = { + .attach = ti_sn_bridge_attach, + .pre_enable = ti_sn_bridge_pre_enable, + .enable = ti_sn_bridge_enable, + .disable = ti_sn_bridge_disable, + .post_disable = ti_sn_bridge_post_disable, + .mode_set = ti_sn_bridge_mode_set, +}; + +static int ti_sn_bridge_parse_dsi_host (struct ti_sn_bridge *pdata) +{ +struct device_node *np = pdata->dev->of_node; +struct device_node *end_node; + + end_node =of_graph_get_endpoint_by_regs (np,0 ,0 ); +if (!end_node) { +DRM_ERROR ("remote endpoint not found\n" );drivers/gpu/drm/bridge/ti-sn65dsi86.c line 638
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+return -ENODEV ; + } + + pdata->host_node =of_graph_get_remote_port_parent (end_node); +of_node_put (end_node); +if (!pdata->host_node) { +DRM_ERROR ("remote node not found\n" ); +return -ENODEV ; + } +of_node_put (pdata->host_node);Why not
of_graph_get_remote_node?
Quoted 7 lines
Quoted 2 lines
Just for my understanding, is
of_graph_get_remote_port_parentnot recommended? what is the benefit of usingof_graph_get_remote_node()?Because it encapsulates both
of_graph_get_endpoint_by_regsandof_graph_get_remote_port_parentwith correct node reference management.
Ok.
Quoted 58 lines
Quoted 58 lines
Quoted 58 lines
Quoted 44 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +43
-0+ +return 0 ; +} + +static int ti_sn_bridge_probe (struct i2c_client *client, +const struct i2c_device_id *id) +{ +struct ti_sn_bridge *pdata; +struct device_node *ddc_node; +int ret =0 ; + +if (!i2c_check_functionality (client->adapter,I2C_FUNC_I2C )) { +DRM_ERROR ("device doesn't support I2C\n" ); +return -ENODEV ; + } + + pdata =devm_kzalloc (&client->dev,sizeof (struct ti_sn_bridge ), +GFP_KERNEL ); +if (!pdata) +return -ENOMEM ; + + pdata->dev = &client->dev; +dev_set_drvdata (&client->dev, pdata); + + pdata->regmap =devm_regmap_init_i2c (client, + &ti_sn_bridge_regmap_config); +if (IS_ERR (pdata->regmap)) { +DRM_ERROR ("regmap i2c init failed\n" ); +return PTR_ERR (pdata->regmap); + } + + pdata->enable_gpio =devm_gpiod_get (pdata->dev, +"enable" ,GPIOD_OUT_LOW ); +if (IS_ERR (pdata->enable_gpio)) { +DRM_ERROR ("failed to get enable gpio from DT\n" ); + ret =PTR_ERR (pdata->enable_gpio); +return ret; + } + + ret =ti_sn_bridge_parse_regulators (pdata); +if (ret) { +DRM_ERROR ("failed to parse regulators\n" ); +return ret;drivers/gpu/drm/bridge/ti-sn65dsi86.c line 691
Quoted 10 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +10
-0+ } + + ret =ti_sn_bridge_parse_dsi_host (pdata); +if (ret) +return ret; + +pm_runtime_enable (pdata->dev); +pm_runtime_get_sync (pdata->dev); + ret =ti_sn_bridge_read_device_rev (pdata); +pm_runtime_put_sync (pdata->dev);I would put it into (pre) enable callbacks, maybe with marking it to run
only once.
Quoted 5 lines
Quoted 2 lines
do you mean we should put the
ti_sn_bridge_read_device_rev()in pre enable callback?Yes, to avoid unnecessary switching on/off device.
Ok. in that case i have add a rev field in the private data and check for this be non-zero every time pre_enable is called else read the rev.
Quoted 85 lines
Regards
AndrzejQuoted 82 lines
Quoted 80 lines
Regards
AndrzejQuoted 76 lines
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +6
-0+if (ret) + goto err_rev_read; + + pdata->refclk =devm_clk_get (pdata->dev,"refclk" ); + + ddc_node = of_parse_phandle(pdata->dev->of_node, "ddc-i2c-bus",0);
drivers/gpu/drm/bridge/ti-sn65dsi86.c new file +69
-0+if (ddc_node) { + pdata->ddc =of_find_i2c_adapter_by_node (ddc_node); +of_node_put (ddc_node); +if (!pdata->ddc) { +DRM_DEBUG_KMS ("failed to read ddc node\n" ); + ret = -EPROBE_DEFER ; + goto err_rev_read; + } + }else { +DRM_DEBUG_KMS ("no ddc property found\n" ); + } + +i2c_set_clientdata (client, pdata); + + pdata->bridge.funcs = &ti_sn_bridge_funcs; + pdata->bridge.of_node = client->dev.of_node; + +drm_bridge_add (&pdata->bridge); + +return 0 ; + +err_rev_read: +pm_runtime_disable (pdata->dev); +return ret; +} + +static int ti_sn_bridge_remove (struct i2c_client *client) +{ +struct ti_sn_bridge *pdata =i2c_get_clientdata (client); + +if (!pdata) +return -EINVAL ; + +mipi_dsi_detach (pdata->dsi); +mipi_dsi_device_unregister (pdata->dsi); + +drm_bridge_remove (&pdata->bridge); +pm_runtime_disable (pdata->dev); +i2c_put_adapter (pdata->ddc); + +return 0 ; +} + +static struct i2c_device_id ti_sn_bridge_id[] = { + {"ti,sn65dsi86" ,0 }, + {}, +}; +MODULE_DEVICE_TABLE (i2c, ti_sn_bridge_id); + +static const struct of_device_id ti_sn_bridge_match_table[] = { + {.compatible ="ti,sn65dsi86" }, + {}, +}; +MODULE_DEVICE_TABLE (of, ti_sn_bridge_match_table); + +static struct i2c_driver ti_sn_bridge_driver = { + .driver = { + .name ="ti_sn65dsi86" , + .of_match_table = ti_sn_bridge_match_table, + .pm = &ti_sn_bridge_pm_ops, + }, + .probe = ti_sn_bridge_probe, + .remove = ti_sn_bridge_remove, + .id_table = ti_sn_bridge_id, +}; + +module_i2c_driver (ti_sn_bridge_driver); +MODULE_DESCRIPTION ("sn65dsi86 DSI to eDP bridge driver" ); +MODULE_LICENSE ("GPL v2" );
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
PATCH v6 2/4 dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings
In reply to a message not in the archiveDocument the bindings used for the sn65dsi86 DSI to eDP bridge.
Changes in v1:
- Rephrase the dt-binding descriptions to be more inline with existing bindings (Andrzej Hajda).
- Add missing dt-binding that are parsed by corresponding driver (Andrzej Hajda).
Changes in v2:
- Remove edp panel specific dt-binding entries. Only keep bridge
specific entries (Sean Paul).
- Remove custom-modes dt entry since its usage is removed from driver also (Sean Paul).
- Remove is-pluggable dt entry since this will not be needed anymore (Sean Paul).
Changes in v3:
- Remove irq-gpio dt entry and instead populate is an interrupt property (Rob Herring).
Changes in v4:
- Add link to bridge chip datasheet (Stephen Boyd)
- Add vpll and vcc regulator supply bindings (Stephen Boyd)
- Add ref clk optional dt binding (Stephen Boyd)
- Add gpio-controller optional dt binding (Stephen Boyd)
Changes in v5:
- Use clock property to specify the input refclk (Stephen Boyd).
- Update gpio cell and pwm cell numbers (Stephen Boyd).
--- .../bindings/display/bridge/ti,sn65dsi86.txt | 81 ++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +81 -0
@@ -0,0 +1,81 @@ +SN65DSI86 DSI to eDP bridge chip +-------------------------------- + +This is the binding for Texas Instruments SN65DSI86 bridge. +http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86&fileType=pdf + +Required properties: +- compatible: Must be "ti,sn65dsi86" +- reg: i2c address of the chip, 0x2d as per datasheet +- enable-gpios: OF device-tree gpio specification for bridge_en pin + +- vccio-supply: A 1.8V supply that powers up the digital IOs. +- vpll-supply: A 1.8V supply that powers up the displayport PLL. +- vcca-supply: A 1.2V supply that powers up the analog circuits. +- vcc-supply: A 1.2V supply that powers up the digital core. + +Optional properties: +- interrupts: Specifier for the SN65DSI86 interrupt line. +- hpd-gpios: OF device-tree gpio specifications for HPD pin. + +- gpio-controller: Marks the device has a GPIO controller. +- #gpio-cells : Should be two. The first cell is the pin number and + the second cell is used to specify flags. + See ../../gpio/gpio.txt for more information. +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description of + the cell formats. + +- clock-names: should be "refclk" +- clocks: OF device-tree clock specification for refclk input. The reference + clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz. + +Required nodes: + +This device has two video ports. Their connections are modelled using the +OF graph bindings specified in Documentation/devicetree/bindings/graph.txt. + +- Video port 0 for DSI input +- Video port 1 for eDP output + +Example +------- + +edp-bridge@2d { + compatible = "ti,sn65dsi86"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x2d>; + + enable-gpios = <&msmgpio 33 GPIO_ACTIVE_HIGH>; + interrupt-parent = <&gpio3>; + interrupts = <4 IRQ_TYPE_EDGE_FALLING>; + + vccio-supply = <&pm8916_l17>; + vcca-supply = <&pm8916_l6>; + vpll-supply = <&pm8916_l17>; + vcc-supply = <&pm8916_l6>; + + clock-names = "refclk"; + clocks = <&input_refclk>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + edp_bridge_in: endpoint { + remote-endpoint = <&dsi_out>; + }; + }; + + port@1 { + reg = <1>; + + edp_bridge_out: endpoint { + remote-endpoint = <&edp_panel_in>; + }; + }; + }; +}
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
A message that is not in the archive (sent off-list or not yet received): 1526363564-13823-3-git-send-email-spanda-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
Re: [PATCH v6 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings
Quoting Sandeep Panda (2018-05-14 22:52:42)
Quoted 9 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +3
-0@@ -0,0 +1,81 @@ +Optional properties: +- interrupts: Specifier for the SN65DSI86 interrupt line. +- hpd-gpios: OF device-tree gpio specifications for HPD pin.
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt line 13
Quoted 10 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +10
-0+ +- gpio-controller: Marks the device has a GPIO controller. +- #gpio-cells : Should be two. The first cell is the pin number and + the second cell is used to specify flags. + See ../../gpio/gpio.txt for more information. +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description of + the cell formats. + +- clock-names: should be "refclk" +- clocks: OF device-tree clock specification for refclk input. The reference
What is "OF device-tree .* specification" providing? This is all an OF device-tree specification.
Quoted 30 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +30
-0+ clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz. + +Required nodes: + +This device has two video ports. Their connections are modelled using the +OF graph bindings specified in Documentation/devicetree/bindings/graph.txt. + +- Video port 0 for DSI input +- Video port 1 for eDP output + +Example +------- + +edp-bridge@2d { + compatible = "ti,sn65dsi86"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x2d>; + + enable-gpios = <&msmgpio 33 GPIO_ACTIVE_HIGH>; + interrupt-parent = <&gpio3>; + interrupts = <4 IRQ_TYPE_EDGE_FALLING>; + + vccio-supply = <&pm8916_l17>; + vcca-supply = <&pm8916_l6>; + vpll-supply = <&pm8916_l17>; + vcc-supply = <&pm8916_l6>; + + clock-names = "refclk"; + clocks = <&input_refclk>;
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt line 69
Quoted 10 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +10
-0+ + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + edp_bridge_in: endpoint { + remote-endpoint = <&dsi_out>;
How do we know the number of lanes that are connected and if there's one channel (A) or two channels (A and B)? Would there be two endpoints in that case?
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt line 77
Quoted 8 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +8
-0+ }; + }; + + port@1 { + reg = <1>; + + edp_bridge_out: endpoint { + remote-endpoint = <&edp_panel_in>;
The hardware looks to support some sort of lane renumbering scheme, where the eDP logical lane 0 can be routed through a different pin than MLP/N0, same for logical lane 1, etc. I don't have a use case for this right now, but I hope that it could be added somewhere in the binding as an optional property to describe this lane remapping feature. It also has some sort of lane polarity inversion feature. Perhaps there needs to be a lane-config property that does this remapping and inversion with two cells.
lane-config = <0 0>, /* Lane 0 logical is lane 0 phys (!inv) */ <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */ <2 0>, /* Lane 2 logical is lane 2 phys (!inv) */ <3 0>; /* Lane 3 logical is lane 3 phys (!inv) */
Or
lane-config = <2 1>, /* Lane 2 logical is lane 0 phys (inv) */ <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */ <3 1>, /* Lane 3 logical is lane 2 phys (inv) */ <0 0>; /* Lane 0 logical is lane 3 phys (!inv) */
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt line 78
Quoted 3 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +3
-0+ }; + }; + };
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
A message that is not in the archive (sent off-list or not yet received): 152648804247.210890.6352930842767133968-n1Xw8LXHxjTHt/[email protected]
Re: [PATCH v6 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings
Quoted 19 lines · Stephen Boyd
Quoting Sandeep Panda (2018-05-14 22:52:42)
Quoted 18 lines
(quoted diff) +0
-0
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txtDocumentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +6
-0@@ -0,0 +1,81 @@ +Optional properties: +- interrupts: Specifier for the SN65DSI86 interrupt line. +- hpd-gpios: OF device-tree gpio specifications for HPD pin. + +- gpio-controller: Marks the device has a GPIO controller. +- #gpio-cells : Should be two. The first cell is the pin numberand
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +3
-0+ the second cell is used to specify flags. + See ../../gpio/gpio.txt for more information. +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for descriptionof
Quoted 8 lines
Quoted 5 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +4
-0+ the cell formats. + +- clock-names: should be "refclk" +- clocks: OF device-tree clock specification for refclk input. Thereference
What is "OF device-tree .* specification" providing? This is all an OF device-tree specification.
Ok. i will remove OF device tree, only keep specification for refclk input.
Quoted 39 lines
Quoted 39 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +5
-0+ clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz. + +Required nodes: + +This device has two video ports. Their connections are modelled usingthe
+OF graph bindings specified inDocumentation/devicetree/bindings/graph.txt.Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +30
-0+ +- Video port 0 for DSI input +- Video port 1 for eDP output + +Example +------- + +edp-bridge@2d { + compatible = "ti,sn65dsi86"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x2d>; + + enable-gpios = <&msmgpio 33 GPIO_ACTIVE_HIGH>; + interrupt-parent = <&gpio3>; + interrupts = <4 IRQ_TYPE_EDGE_FALLING>; + + vccio-supply = <&pm8916_l17>; + vcca-supply = <&pm8916_l6>; + vpll-supply = <&pm8916_l17>; + vcc-supply = <&pm8916_l6>; + + clock-names = "refclk"; + clocks = <&input_refclk>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 {
Quoted 8 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt line 69
Quoted 4 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +4
-0+ reg = <0>; + + edp_bridge_in: endpoint { + remote-endpoint = <&dsi_out>;How do we know the number of lanes that are connected and if there's one channel (A) or two channels (A and B)? Would there be two endpoints in that case?
Currently we have hard coded the number of lanes to 4 in driver. And regarding supporting 2 DSI input channels dt-binding, we are planning to add those entries once we upload a patchset support dual dsi configuration in bridge driver.
Quoted 10 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt line 77
Quoted 9 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +8
-0+ }; + }; + + port@1 { + reg = <1>; + + edp_bridge_out: endpoint { + remote-endpoint = <&edp_panel_in>;
Quoted 8 lines
The hardware looks to support some sort of lane renumbering scheme, where the eDP logical lane 0 can be routed through a different pin than MLP/N0, same for logical lane 1, etc. I don't have a use case for this right now, but I hope that it could be added somewhere in the binding as an optional property to describe this lane remapping feature. It also has some sort of lane polarity inversion feature. Perhaps there needs to be a lane-config property that does this remapping and inversion with two cells.
Ok. I will add this feature to dt binding.
Quoted 6 lines
lane-config = <0 0>, /* Lane 0 logical is lane 0 phys (!inv) */ <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */ <2 0>, /* Lane 2 logical is lane 2 phys (!inv) */ <3 0>; /* Lane 3 logical is lane 3 phys (!inv) */
Quoted 11 lines
Or
lane-config = <2 1>, /* Lane 2 logical is lane 0 phys (inv) */ <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */ <3 1>, /* Lane 3 logical is lane 2 phys (inv) */ <0 0>; /* Lane 0 logical is lane 3 phys (!inv) */Quoted 3 lines
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt new file +3
-0+ }; + }; + };
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
PATCH v6 3/4 drm/panel: add Innolux TV123WAM panel driver support
In reply to a message not in the archiveAdd support for Innolux TV123WAM, which is a 12.3" eDP display panel with 2160x1440 resolution.
Changes in v1:
- Add the compatibility string, display_mode and panel_desc structures in alphabetical order (Sean Paul).
--- drivers/gpu/drm/panel/panel-simple.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
drivers/gpu/drm/panel/panel-simple.c +27 -0
@@ -1190,6 +1190,30 @@ static void panel_simple_shutdown(struct device *dev) }, }; +static const struct drm_display_mode innolux_tv123wam_mode = { + .clock =206016 , + .hdisplay =2160 , + .hsync_start =2160 +48 , + .hsync_end =2160 +48 +32 , + .htotal =2160 +48 +32 +80 , + .vdisplay =1440 , + .vsync_start =1440 +3 , + .vsync_end =1440 +3 +10 , + .vtotal =1440 +3 +10 +27 , + .vrefresh =60 , + .flags =DRM_MODE_FLAG_PHSYNC |DRM_MODE_FLAG_PVSYNC , +}; + +static const struct panel_desc innolux_tv123wam = { + .modes = &innolux_tv123wam_mode, + .num_modes =1 , + .bpc =8 , + .size = { + .width =259 , + .height =173 , + }, +}; +static const struct drm_display_mode innolux_zj070na_01p_mode = { .clock =51501 , .hdisplay =1024 ,
@@ -2037,6 +2061,9 @@ static void panel_simple_shutdown(struct device *dev) .compatible = "innolux,n156bge-l21", .data = &innolux_n156bge_l21, }, { + .compatible = "innolux,tv123wam", + .data = &innolux_tv123wam, + }, { .compatible = "innolux,zj070na-01p", .data = &innolux_zj070na_01p, }, {
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
PATCH v6 4/4 dt-bindings: drm/panel: Document Innolux TV123WAM panel bindings
In reply to a message not in the archiveInnolux TV123WAM is a 12.3" eDP display panel with 2160x1440 resolution, which can be supported by simple panel driver.
Changes in v1:
- Make use of simple panel driver instead of creating a new driver for this panel (Sean Paul).
- Combine dt-binding and driver changes into one patch as done by other existing panel support changes.
Changes in v2:
- Separate driver change from dt-binding documentation (Rob Herring).
- Add the properties from simple-panel binding that are applicable to this panel (Rob Herring).
--- .../bindings/display/panel/innolux,tv123wam.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt new file +20 -0
@@ -0,0 +1,20 @@ +Innolux TV123WAM 12.3 inch eDP 2K display panel + +This binding is compatible with the simple-panel binding, which is specified +in simple-panel.txt in this directory. + +Required properties: +- compatible: should be "innolux,tv123wam" +- power-supply: regulator to provide the supply voltage + +Optional properties: +- enable-gpios: GPIO pin to enable or disable the panel +- backlight: phandle of the backlight device attached to the panel + +Example: + panel_edp: panel-edp { + compatible = "innolux,tv123wam"; + enable-gpios = <&msmgpio 31 GPIO_ACTIVE_LOW>; + power-supply = <&pm8916_l2>; + backlight = <&backlight>; + };
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno