[PATCH v11 2/5] drm/bridge: add support for sn65dsi86 bridge driver
spanda at codeaurora.org
spanda at codeaurora.org
Mon Jun 18 05:51:46 UTC 2018
On 2018-06-15 16:59, Andrzej Hajda wrote:
> Hi Sandeep,
>
> Thanks for the patch, I hope it will be merged soon.
>
> On 15.06.2018 08:43, Sandeep Panda wrote:
>> 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).
>>
>> Changes in v7:
>> - Use static supply entries instead of dynamic allocation (Andrzej
>> Hajda).
>> - Defer bridge driver probe if panel is not probed (Andrzej Hajda).
>> - Update of_graph APIs for correct node reference management.
>> (Andrzej
>> Hajda).
>> - Remove local display_mode object (Andrzej Hajda).
>> - Remove version id check function from driver.
>>
>> Changes in v8:
>> - Move dsi register/attach function to bridge driver probe (Andrzej
>> Hajda).
>> - Introduce a new helper function to write 16bit words into
>> consecutive
>> registers (Andrzej Hajda).
>> - Remove unnecessary macros (Andrzej Hajda).
>>
>> Changes in v9:
>> - Remove dsi register/attach from bridge probe, since dsi dev
>> register
>> completion also waits for any panel or bridge to get added. This
>> creates
>> deadlock situation when bridge driver calls dsi dev register and
>> attach before bridge add, in its probe function.
>> - Fix issues faced during testing of bridge driver on actual HW.
>> - Remove unnecessary initializations (Stephen Boyd).
>> - Use local refclk lut size instead of global macro (Sean Paul).
>>
>> Changes in v10:
>> - Use refclk to determine if continuous dsi clock is needed or not.
>>
>> Changes in v11:
>> - Read DPPLL_SRC register to determine continuous clock instead of
>> using refclk handle (Stephen Boyd).
>>
>> Signed-off-by: Sandeep Panda <spanda at codeaurora.org>
>> ---
>> drivers/gpu/drm/bridge/Kconfig | 9 +
>> drivers/gpu/drm/bridge/Makefile | 1 +
>> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 696
>> ++++++++++++++++++++++++++++++++++
>> 3 files changed, 706 insertions(+)
>> create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>
>> diff --git a/drivers/gpu/drm/bridge/Kconfig
>> b/drivers/gpu/drm/bridge/Kconfig
>> index 3b99d5a..8153150 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -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"
>> diff --git a/drivers/gpu/drm/bridge/Makefile
>> b/drivers/gpu/drm/bridge/Makefile
>> index 373eb28..a7ca000 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -11,5 +11,6 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o
>> obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>> obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>> obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>> +obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
>> obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
>> obj-y += synopsys/
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> new file mode 100644
>> index 0000000..5473456
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> @@ -0,0 +1,696 @@
>> +// 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_of.h>
>> +#include <drm/drm_panel.h>
>> +#include <linux/clk.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>> +
>> +/* Link Training specific registers */
>> +#define SN_DEVICE_REV_REG 0x08
>> +#define SN_HPD_DISABLE_REG 0x5C
>> +#define SN_DPPLL_SRC_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_VERTICAL_DISPLAY_SIZE_LOW_REG 0x24
>> +#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_REFCLK_FREQ_OFFSET 1
>> +#define SN_DSIA_LANE_OFFSET 3
>> +#define SN_DP_LANE_OFFSET 4
>> +#define SN_DP_DATA_RATE_OFFSET 5
>> +#define SN_SYNC_POLARITY_OFFSET 7
>> +
>> +#define SN_ENABLE_VID_STREAM_BIT BIT(3)
>> +#define SN_REFCLK_FREQ_BITS (BIT(3) | BIT(2) | BIT(1))
>> +#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))
>
> For contiguous bitmasks you could use GENMASK(h, l), but it is up to
> you.
Ok will update in next patch.
>
>> +#define SN_HPD_DISABLE_BIT BIT(0)
>> +
>> +#define SN_REGULATOR_SUPPLY_NUM 4
>> +
>> +struct ti_sn_bridge {
>> + struct device *dev;
>> + struct regmap *regmap;
>> + 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;
>> + struct regulator_bulk_data supplies[SN_REGULATOR_SUPPLY_NUM];
>> + struct i2c_adapter *ddc;
>> +};
>> +
>> +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),
>> +};
>> +
>> +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,
>> +};
>> +
>> +static void ti_sn_bridge_write_u16(struct ti_sn_bridge *pdata,
>> + unsigned int reg, u16 val)
>> +{
>> + regmap_write(pdata->regmap, reg, val & 0xFF);
>> + regmap_write(pdata->regmap, reg + 1, val >> 8);
>> +}
>> +
>> +static int __maybe_unused ti_sn_bridge_resume(struct device *dev)
>> +{
>> + struct ti_sn_bridge *pdata = dev_get_drvdata(dev);
>> + int ret;
>> +
>> + ret = regulator_bulk_enable(SN_REGULATOR_SUPPLY_NUM,
>> 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 __maybe_unused ti_sn_bridge_suspend(struct device *dev)
>> +{
>> + struct ti_sn_bridge *pdata = dev_get_drvdata(dev);
>> + int ret;
>> +
>> + gpiod_set_value(pdata->enable_gpio, 0);
>> +
>> + ret = regulator_bulk_disable(SN_REGULATOR_SUPPLY_NUM,
>> pdata->supplies);
>> + if (ret)
>> + DRM_ERROR("failed to disable supplies %d\n", ret);
>> +
>> + return ret;
>> +}
>> +
>> +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 edid *edid;
>> + u32 num_modes;
>> +
>> + if (pdata->panel) {
>> + DRM_DEBUG_KMS("get mode from connected drm_panel\n");
>> + return drm_panel_get_modes(pdata->panel);
>> + }
>> +
>> + if (!pdata->ddc)
>> + return 0;
> I think it should not happen, ie probe should fail if both panel and
> ddc
> are absent.
Ok i will remove this check in next patchset.
>> +
>> + pm_runtime_get_sync(pdata->dev);
>> + edid = drm_get_edid(connector, pdata->ddc);
>> + pm_runtime_put_sync(pdata->dev);
>
> Minor nit, you can use here pm_runtime_put, I guess you do not care
> when
> it will be turned off.
Ok.
>
>> + 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_parse_regulators(struct ti_sn_bridge *pdata)
>> +{
>> + unsigned int i;
>> + const char * const ti_sn_bridge_supply_names[] = {
>> + "vcca", "vcc", "vccio", "vpll",
>> + };
>> +
>> + for (i = 0; i < SN_REGULATOR_SUPPLY_NUM; i++)
>> + pdata->supplies[i].supply = ti_sn_bridge_supply_names[i];
>> +
>> + return devm_regulator_bulk_get(pdata->dev, SN_REGULATOR_SUPPLY_NUM,
>> + pdata->supplies);
>> +}
>> +
>> +static int ti_sn_bridge_attach(struct drm_bridge *bridge)
>> +{
>> + int ret, val;
>> + struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>> + struct mipi_dsi_host *host;
>> + struct mipi_dsi_device *dsi;
>> + const struct mipi_dsi_device_info info = { .type = "ti_sn_bridge",
>> + .channel = 0,
>> + .node = NULL,
>> + };
>> +
>> + 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);
>> + 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");
>> + ret = -ENODEV;
>> + goto err_dsi_host;
>> + }
>
> In case of late host probing it will fail, instead of probe deferring.
> If I remember I have asked you to move this code to probe, as deferring
> is only possible there.
>
I moved the this code to probe, and returned probe defer if host not
found in patchset v9.
This caused a dead lock when i was testing on actual HW. The reason
being,
DSI host probe completion was waiting for any drm_panel/drm_bridge to
get
added to the global panel/bridge list, and here in bridge driver we are
waiting for
DSI host probe completion to happen so that host node gets added to the
global host list.
Also i found other up-streamed bridge drivers are also doing the same in
bridge_attach.
>> +
>> + dsi = mipi_dsi_device_register_full(host, &info);
>> + if (IS_ERR(dsi)) {
>> + DRM_ERROR("failed to create dsi device\n");
>> + ret = PTR_ERR(dsi);
>> + goto err_dsi_host;
>> + }
>> +
>> + /* 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;
>> +
>> + /* check if continuous dsi clock is required or not */
>> + pm_runtime_get_sync(pdata->dev);
>> + regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val);
>> + pm_runtime_put_sync(pdata->dev);
>> + if (!(val & DPPLL_CLK_SRC_DSICLK))
>> + dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
>> +
>> + ret = mipi_dsi_attach(dsi);
>> + if (ret < 0) {
>> + DRM_ERROR("failed to attach dsi to host\n");
>> + goto err_dsi_attach;
>> + }
>> + pdata->dsi = dsi;
>> +
>> + /* attach panel to bridge */
>> + if (pdata->panel)
>> + drm_panel_attach(pdata->panel, &pdata->connector);
>> +
>> + return 0;
>> +
>> +err_dsi_attach:
>> + mipi_dsi_device_unregister(dsi);
>> +err_dsi_host:
>> + drm_connector_cleanup(&pdata->connector);
>> + return ret;
>> +}
>> +
>> +static void ti_sn_bridge_disable(struct drm_bridge *bridge)
>> +{
>> + struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>> +
>> + if (pdata->panel) {
>> + drm_panel_disable(pdata->panel);
>> + drm_panel_unprepare(pdata->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);
>
> Maybe in your particular case it does not matter but generally
> drm_panel_disable should be called 1st, but drm_panel_unprepare should
> be called somewhere after disabling video stream.
Ok i wil move panel unprepare to end in next patchset.
>
>> +}
>> +
>> +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->bridge.encoder->crtc->state->adjusted_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;
>> +}
>> +
>> +/* 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_freq(struct ti_sn_bridge *pdata)
>> +{
>> + int i;
>> + u32 refclk_rate;
>> + const u32 *refclk_lut;
>> + size_t refclk_lut_size;
>> +
>> + if (pdata->refclk) {
>> + refclk_rate = clk_get_rate(pdata->refclk);
>> + refclk_lut = ti_sn_bridge_refclk_lut;
>> + refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_refclk_lut);
>> + clk_prepare_enable(pdata->refclk);
>> + } else {
>> + refclk_rate = ti_sn_bridge_get_dsi_freq(pdata) * 1000;
>> + refclk_lut = ti_sn_bridge_dsiclk_lut;
>> + refclk_lut_size = ARRAY_SIZE(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_update_bits(pdata->regmap, SN_DPPLL_SRC_REG,
>> + SN_REFCLK_FREQ_BITS, i << SN_REFCLK_FREQ_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, i;
>> + struct drm_display_mode *mode =
>> + &pdata->bridge.encoder->crtc->state->adjusted_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) /
>> + DP_CLK_FUDGE_DEN;
>> + for (i = 0; i < ARRAY_SIZE(ti_sn_bridge_dp_rate_lut) - 1; i++)
>> + if (ti_sn_bridge_dp_rate_lut[i] > dp_rate_mhz)
>> + break;
>> +
>> + 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->bridge.encoder->crtc->state->adjusted_mode;
>> + u8 hsync_polarity = 0, vsync_polarity = 0;
>> +
>> + if (mode->flags & DRM_MODE_FLAG_PHSYNC)
>> + hsync_polarity = 0x1;
>
> You can assign:
>
> hsync_polarity = BIT(SN_SYNC_POLARITY_OFFSET);
>
> and then use it directly in regmap_write.
>
>> + if (mode->flags & DRM_MODE_FLAG_PVSYNC)
>> + vsync_polarity = 0x1;
>
> The same here.
>
Ok.
>> +
>> + ti_sn_bridge_write_u16(pdata, SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG,
>> + mode->hdisplay);
>> + ti_sn_bridge_write_u16(pdata, SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG,
>> + mode->vdisplay);
>> + 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) >> 8) & 0x7F)
>> + | (hsync_polarity << SN_SYNC_POLARITY_OFFSET));
>> + 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) >> 8) & 0x7F)
>> + | (vsync_polarity << SN_SYNC_POLARITY_OFFSET));
>> +
>> + 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);
>> + unsigned int val;
>> +
>> + if (pdata->panel) {
>> + drm_panel_prepare(pdata->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);
>
> I think HPD should be disabled much earlier, I guess somewhere in
> initialization/probe.
>
As per the driver code, we are enabling the bridge power supplies and
gpio
only in pre-enable function, i can move this HDP disable to pre-enable.
>> + }
>> +
>> + /* 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 (pdata->panel)
>> + drm_panel_enable(pdata->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 ref_clk */
>> + ti_sn_bridge_set_refclk_freq(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,
>> +};
>> +
>> +static int ti_sn_bridge_parse_dsi_host(struct ti_sn_bridge *pdata)
>> +{
>> + struct device_node *np = pdata->dev->of_node;
>> +
>> + pdata->host_node = of_graph_get_remote_node(np, 0, 0);
>> +
>> + if (!pdata->host_node) {
>> + DRM_ERROR("remote dsi host node not found\n");
>> + return -ENODEV;
>> + }
>> +
>> + 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;
>> +
>> + 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->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->dev = &client->dev;
>> + dev_set_drvdata(&client->dev, pdata);
>> +
>> + 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 = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, 0,
>> + &pdata->panel, NULL);
>> + if (ret) {
>> + DRM_ERROR("could not find any drm panel node\n");
>> + return ret;
>> + }
>> +
>> + pdata->refclk = devm_clk_get(pdata->dev, "refclk");
>> + if (IS_ERR(pdata->refclk)) {
>> + ret = PTR_ERR(pdata->refclk);
>> + if (ret == -EPROBE_DEFER)
>> + return ret;
>> + DRM_DEBUG_KMS("refclk not found\n");
>> + pdata->refclk = NULL;
>> + }
>> +
>> + 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");
>> + return -EPROBE_DEFER;
>> + }
>> + } else {
>> + DRM_DEBUG_KMS("no ddc property found\n");
>> + }
>
> I guess there should be exclusively panel or ddc, please fail if
> both/none are present.
>
Just curious, can't an edp panel will have EDID flashed?
>
> Regards
> Andrzej
>
>> +
>> + ret = ti_sn_bridge_parse_dsi_host(pdata);
>> + if (ret)
>> + return ret;
>> +
>> + pm_runtime_enable(pdata->dev);
>> +
>> + 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;
>> +}
>> +
>> +static int ti_sn_bridge_remove(struct i2c_client *client)
>> +{
>> + struct ti_sn_bridge *pdata = i2c_get_clientdata(client);
>> +
>> + if (!pdata)
>> + return -EINVAL;
>> +
>> + if (pdata->host_node)
>> + of_node_put(pdata->host_node);
>> +
>> + pm_runtime_disable(pdata->dev);
>> +
>> + if (pdata->dsi) {
>> + mipi_dsi_detach(pdata->dsi);
>> + mipi_dsi_device_unregister(pdata->dsi);
>> + }
>> +
>> + drm_bridge_remove(&pdata->bridge);
>> + 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");
More information about the dri-devel
mailing list