On Wed, May 02, 2018 at 10:01:59AM +0530, 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.
> - 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.
It seems like you also added 2 new supply names, as well as remove the
configurable gpios?
>
> Changes in v5:
> - Fixed Kbuild test service reported warnings.
>
> Signed-off-by: Sandeep Panda <spanda@xxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/bridge/Kconfig | 9 +
> drivers/gpu/drm/bridge/Makefile | 1 +
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 725 ++++++++++++++++++++++++++++++++++
> 3 files changed, 735 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..3711be8 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -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/
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> new file mode 100644
> index 0000000..019c7cd
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -0,0 +1,725 @@
> +// 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/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_graph.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
> +#define SN_DEFAULT_REF_CLK_KHZ 19200
> +
> +/* 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))
> +#define SN_HPD_DISABLE_BIT BIT(0)
> +
> +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;
> + unsigned int refclk_khz;
> + struct drm_panel *panel;
> + struct gpio_desc *enable_gpio;
> + unsigned int num_supplies;
> + struct regulator_bulk_data *supplies;
> + struct i2c_adapter *ddc;
> + unsigned int num_modes;
You only use this in one function, it should be a local.
> + struct drm_display_mode curr_mode;
> + struct mutex lock;
> + unsigned int ctrl_ref_count;
> +};
> +
> +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 int ti_sn_bridge_power_ctrl(struct ti_sn_bridge *pdata, bool enable)
> +{
> + int ret = 0;
> +
> + mutex_lock(&pdata->lock);
> + if (enable)
> + pdata->ctrl_ref_count++;
> + else
> + pdata->ctrl_ref_count--;
I think you should use a kref instead of rolling your own ref_count. You can
handle release by calling kref_put_mutex(), which will handle the reference and
the lock. On the acquire side, you can use kref_get_unless_zero which will be
fast if the reference is already active.
> +
> + if (enable && (pdata->ctrl_ref_count == 1)) {
> + ret = regulator_bulk_enable(pdata->num_supplies,
> + pdata->supplies);
> + if (ret) {
> + DRM_ERROR("bridge regulator enable failed\n");
print ret on failure
> + goto exit;
> + }
> +
> + gpiod_set_value(pdata->enable_gpio, 1);
> + } else if (!enable && !pdata->ctrl_ref_count) {
> + gpiod_set_value(pdata->enable_gpio, 0);
> +
> + regulator_bulk_disable(pdata->num_supplies, pdata->supplies);
> + } else {
> + DRM_DEBUG("ti_sn_bridge power ctrl: %d refcount: %d\n",
> + enable, pdata->ctrl_ref_count);
No need for this.
> + }
> +
> +exit:
> + mutex_unlock(&pdata->lock);
> + return ret;
> +}
> +
> +/* 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;
> +
> + if (panel) {
> + DRM_DEBUG("get mode from connected drm_panel\n");
> + pdata->num_modes = drm_panel_get_modes(panel);
> + return pdata->num_modes;
> + }
> +
> + /* get from EDID */
This comment isn't useful.
> + if (!pdata->ddc)
> + return 0;
> +
> + ti_sn_bridge_power_ctrl(pdata, true);
> + edid = drm_get_edid(connector, pdata->ddc);
> + ti_sn_bridge_power_ctrl(pdata, false);
> + if (!edid)
> + return 0;
> +
> + drm_mode_connector_update_edid_property(connector, edid);
> + pdata->num_modes = drm_add_edid_modes(connector, edid);
> + kfree(edid);
> +
> + return pdata->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;
> +
> + ret = regmap_read(pdata->regmap, SN_DEVICE_REV_REG, &rev);
> + if (ret)
> + return ret;
> +
> + if (rev != SN_BRIDGE_REVISION_ID) {
> + DRM_ERROR("ti_sn_bridge revision id: 0x%x mismatch\n", rev);
> + ret = -EINVAL;
> + }
> +
> + 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)
> +{
> + 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);
> + 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;
> + port = of_graph_get_port_by_id(pdata->dev->of_node, 1);
> + if (port) {
Flip this to avoid indenting the whole function. ie:
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;
> + }
> +
> + 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;
> + }
> +
> + pdata->panel = of_drm_find_panel(panel_node);
> + of_node_put(panel_node);
> + if (!pdata->panel) {
> + DRM_ERROR("no panel node found\n");
> + return -EINVAL;
> + }
> + drm_panel_attach(pdata->panel, &pdata->connector);
> + DRM_DEBUG("panel attached\n");
This log should be DRM_DEBUG_KMS and you should be more descriptive in your
message (ie: "Attached sn65dsi86 panel").
> + }
> +
> + 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,
> + };
> +
> + if (!bridge->encoder) {
> + DRM_ERROR("Parent encoder object not found\n");
> + return -ENODEV;
> + }
> +
> + /* HPD not supported */
> + pdata->connector.polled = 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);
> + 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;
> + }
> +
> + 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) {
> + DRM_ERROR("failed to attach dsi to host\n");
> + mipi_dsi_device_unregister(dsi);
> + return ret;
> + }
> +
> + pdata->dsi = dsi;
> +
> + DRM_DEBUG("bridge attached\n");
Same comment here regarding being more descriptive.
> + /* attach panel to bridge */
> + ti_sn_bridge_attach_panel(pdata);
> +
> + 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",
> + adj_mode->hdisplay, adj_mode->vdisplay,
> + adj_mode->vrefresh, adj_mode->clock);
> +
> + drm_mode_copy(&pdata->curr_mode, adj_mode);
> +}
> +
> +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);
> +}
> +
> +/* reference clk frequencies supported by the bridge in KHz */
> +static u32 ti_sn_bridge_ref_clk_table[] = {
> + 12000,
> + 19200,
> + 26000,
> + 27000,
> + 38400,
> +};
> +
> +static void ti_sn_bridge_set_refclk(struct ti_sn_bridge *pdata)
> +{
> + int i = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(ti_sn_bridge_ref_clk_table); i++)
> + if (ti_sn_bridge_ref_clk_table[i] == pdata->refclk_khz)
> + break;
> + regmap_write(pdata->regmap, SN_REFCLK_FREQ_REG,
> + (DPPLL_CLK_SRC_REFCLK | (i << SN_DSIA_REFCLK_OFFSET)));
What if the refclk frequency isn't found?
> +}
> +
> +struct dp_data_rate {
> + unsigned int bit_val;
> + unsigned int dp_rate;
> +};
> +
> +/* dp data rate supported by the bridge Mbps */
> +static struct dp_data_rate ti_sn_bridge_dp_rate_table[] = {
> + {1, 1620},
> + {2, 2160},
> + {3, 2430},
> + {4, 2700},
> + {5, 3240},
> + {6, 4320},
> + {7, 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) /
> + DP_CLK_FUDGE_DEN;
> + for (i = 0; i < ARRAY_SIZE(ti_sn_bridge_dp_rate_table); i++)
> + if (ti_sn_bridge_dp_rate_table[i].dp_rate > dp_rate_mhz)
> + break;
> + if (i == ARRAY_SIZE(ti_sn_bridge_dp_rate_table))
> + i--; /* set to maximum possible */
> +
> + val = ti_sn_bridge_dp_rate_table[i].bit_val << SN_DP_DATA_RATE_OFFSET;
> + regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
> + SN_DP_DATA_RATE_BITS, val);
I think this can be simplified.
/* Array index corresponds to register value */
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)
{
...
for (i = ARRAY_SIZE(ti_sn_bridge_dp_rate_lut) - 1; i >= 0; 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->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, 10050); /* 10ms delay recommended by spec */
You would probably be fine to set the upper bound to 10500 so we don't trigger
an unnecessary interrupt.
> +}
> +
> +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, 10050); /* 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, 10050); /* 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);
> +
> + ti_sn_bridge_power_ctrl(pdata, true);
> +
> + /* 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);
> +
> + ti_sn_bridge_power_ctrl(pdata, false);
> +}
> +
> +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;
> + }
> +
> + 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);
> +
> + 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);
pdata = devm_kzalloc(&client->dev, sizeof(struct ti_sn_bridge),
GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> +
> + pdata->dev = &client->dev;
> + DRM_DEBUG("I2C address is %x\n", client->addr);
unnecessary
> +
> + pdata->regmap = devm_regmap_init_i2c(client,
> + &ti_sn_bridge_regmap_config);
indentation is wrong here, should be:
pdata->regmap = devm_regmap_init_i2c(client,
&ti_sn_bridge_regmap_config);
> + if (IS_ERR(pdata->regmap))
Print an error?
> + 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 = of_property_read_u32(pdata->dev->of_node,
> + "refclk-freq-khz", &pdata->refclk_khz);
> + if (ret)
> + pdata->refclk_khz = SN_DEFAULT_REF_CLK_KHZ;
> +
> + ret = ti_sn_bridge_parse_dsi_host(pdata);
> + if (ret)
DRM_ERROR?
> + return ret;
> +
> + 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) {
> + dev_dbg(pdata->dev, "failed to read ddc node\n");
> + return -EPROBE_DEFER;
> + }
> + } else {
> + dev_dbg(pdata->dev, "no ddc property found\n");
You've copied dev_dbg() logs, they should be DRM_DEBUG_KMS().
> + }
> +
> + ti_sn_bridge_power_ctrl(pdata, true);
> + ret = ti_sn_bridge_read_device_rev(pdata);
> + ti_sn_bridge_power_ctrl(pdata, false);
This could be done before handling refclk or ddc.
> + if (ret)
> + return ret;
> +
> + i2c_set_clientdata(client, pdata);
> + mutex_init(&pdata->lock);
> +
> + pdata->bridge.funcs = &ti_sn_bridge_funcs;
> + pdata->bridge.of_node = client->dev.of_node;
> +
> + drm_bridge_add(&pdata->bridge);
> +
> + DRM_DEBUG("bridge device registered successfully\n");
unnecessary
> +
> + 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;
> +
> + 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,
> + },
> + .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");
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
