[[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver
spanda at codeaurora.org
spanda at codeaurora.org
Wed Apr 18 18:31:27 UTC 2018
On 2018-04-18 19:41, Sean Paul wrote:
> On Wed, Apr 18, 2018 at 05:49:59PM +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.
>>
>> Signed-off-by: Sandeep Panda <spanda at codeaurora.org>
>> ---
>> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 742
>> ++++++++++++++++++++++++++++++++++
>> 1 file changed, 742 insertions(+)
>> create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> new file mode 100644
>> index 0000000..c798f2f
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> @@ -0,0 +1,742 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#include <linux/types.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/delay.h>
>> +#include <linux/i2c.h>
>> +#include <linux/gpio.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>> +#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>
>
> I have a hard time believing that you need all of these includes. Off
> the top of
> my head, you could probably remove types, kernel, init,
> platform_device, delay,
> drmP, drm_atomic, drm_crtc_helper. You could probably trim it even
> further with
> the help of your compiler. These should also be in alphabetical order.
>
I will take care of alphabetical order and generic kernel header file
inclusion. But the header files of
drmP, drm_atomic, drm_crtc_helper are required here.
drmP, drm_atomic, drm_crtc_helper all these are needed because of the
drm api that we using in this driver
for modes, connectors and bridges. the drm header files i have already
checked with compiler.
>> +
>> +#define SN_BRIDGE_REVISION_ID 0x2
>> +
>> +/* Link Training specific registers */
>> +#define SN_DEVICE_REV_REG 0x08
>> +#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
>> +#define SN_PAGE_SELECT_REG 0xFF
>> +#define SN_AUX_RDATA0_REG 0x79
>> +/* 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 SN_COLOR_BAR_CONFIG_REG 0x3C
>> +
>> +#define DPPLL_CLK_SRC_REFCLK 0
>> +#define DPPLL_CLK_SRC_DSICLK 1
>> +#define MIN_DSI_CLK_FREQ_MHZ 40
>> +
>> +enum ti_sn_bridge_ref_clks {
>> + SN_REF_CLK_12_MHZ = 0,
>> + SN_REF_CLK_19_2_MHZ,
>> + SN_REF_CLK_26_MHZ,
>> + SN_REF_CLK_27_MHZ,
>> + SN_REF_CLK_38_4_MHZ,
>> + SN_REF_CLK_MAX,
>> +};
>> +
>> +struct ti_sn_bridge {
>> + struct device *dev;
>> + struct drm_bridge bridge;
>> + struct drm_connector connector;
>> + struct device_node *host_node;
>> + struct mipi_dsi_device *dsi;
>> + /* handle to the connected eDP panel */
>> + struct drm_panel *panel;
>> + int irq;
>> + struct gpio_desc *irq_gpio;
>> + /* list of gpios needed to enable the bridge functionality */
>> + struct gpio_descs *enable_gpios;
>
> Why are the enable gpios a list?
>
>> + unsigned int num_supplies;
>> + struct regulator_bulk_data *supplies;
>> + struct i2c_client *i2c_client;
>> + struct i2c_adapter *ddc;
>> + bool power_on;
>> + u32 num_modes;
>> + struct drm_display_mode curr_mode;
>> +};
>> +
>> +static int ti_sn_bridge_write(struct ti_sn_bridge *pdata, u8 reg, u8
>> val)
>> +{
>> + struct i2c_client *client = pdata->i2c_client;
>> + u8 buf[2] = {reg, val};
>> + struct i2c_msg msg = {
>> + .addr = client->addr,
>> + .flags = 0,
>> + .len = 2,
>> + .buf = buf,
>> + };
>> +
>> + if (i2c_transfer(client->adapter, &msg, 1) < 1) {
>> + DRM_ERROR("i2c write failed\n");
>> + return -EIO;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int ti_sn_bridge_read(struct ti_sn_bridge *pdata,
>> + u8 reg, char *buf, u32 size)
>> +{
>> + struct i2c_client *client = pdata->i2c_client;
>> + struct i2c_msg msg[2] = {
>> + {
>> + .addr = client->addr,
>> + .flags = 0,
>> + .len = 1,
>> + .buf = ®,
>> + },
>> + {
>> + .addr = client->addr,
>> + .flags = I2C_M_RD,
>> + .len = size,
>> + .buf = buf,
>> + }
>> + };
>> +
>> + if (i2c_transfer(client->adapter, msg, 2) != 2) {
>> + DRM_ERROR("i2c read failed\n");
>> + return -EIO;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void ti_sn_bridge_gpio_configure(struct ti_sn_bridge *pdata,
>> bool on)
>> +{
>> + int value, i = 0, num = 0;
>> + int *value_array = NULL;
>> +
>> + if (!pdata)
>> + return;
>> +
>> + value = on ? 1 : 0;
>> + num = pdata->enable_gpios->ndescs;
>> + value_array = kmalloc(num * sizeof(int), GFP_KERNEL);
>> + if (value_array) {
>> + for (i = 0; i < num; i++)
>> + value_array[i] = value;
>> + gpiod_set_array_value(num, pdata->enable_gpios->desc,
>> + value_array);
>> + kfree(value_array);
>> + }
>> +
>> + if (pdata->irq_gpio)
>> + gpiod_set_value(pdata->irq_gpio, value);
>> +
>> + DRM_DEBUG("config to: %d\n", value);
>> +}
>> +
>> +static void ti_sn_bridge_power_ctrl(struct ti_sn_bridge *pdata, bool
>> enable)
>> +{
>> + static int ref_count;
>> +
>> + if (!pdata)
>> + return;
>> +
>> + if (enable)
>> + ref_count++;
>> + else
>> + ref_count--;
>> +
>> + if (enable && (ref_count == 1)) {
>> + ti_sn_bridge_gpio_configure(pdata, true);
>> +
>> + if (regulator_bulk_enable(pdata->num_supplies,
>> + pdata->supplies)) {
>> + DRM_ERROR("bridge regulator enable failed\n");
>> + return;
>> + }
>> + pdata->power_on = true;
>> + } else if (!enable && !ref_count) {
>> + regulator_bulk_disable(pdata->num_supplies, pdata->supplies);
>> +
>> + ti_sn_bridge_gpio_configure(pdata, false);
>> + pdata->power_on = false;
>> + } else {
>> + DRM_DEBUG("ti_sn_bridge power ctrl: %d refcount: %d\n",
>> + enable, ref_count);
>> + }
>> +}
>> +
>> +/* 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);
>> + } else {
>> + /* get from EDID */
>> + if (!pdata->ddc)
>> + return 0;
>> + ti_sn_bridge_power_ctrl(pdata, true);
>> + edid = drm_get_edid(connector, pdata->ddc);
>> + if (edid) {
>> + drm_mode_connector_update_edid_property(connector,
>> + edid);
>> + pdata->num_modes = drm_add_edid_modes(connector, edid);
>> + drm_edid_to_eld(connector, edid);
>> + kfree(edid);
>> + } else {
>> + DRM_DEBUG("failed to get edid\n");
>> + }
>> + ti_sn_bridge_power_ctrl(pdata, false);
>> + }
>> +
>> + 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_disconnected;
>> +}
>> +
>> +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)
>> +{
>> + u8 rev = 0;
>> + int ret = 0;
>> +
>> + ret = ti_sn_bridge_read(pdata, SN_DEVICE_REV_REG, &rev, 1);
>> + if (ret)
>> + goto exit;
>> +
>> + if (rev == SN_BRIDGE_REVISION_ID) {
>> + DRM_DEBUG("ti_sn_bridge revision id: 0x%x\n", rev);
>> + } else {
>> + DRM_ERROR("ti_sn_bridge revision id mismatch\n");
>> + ret = -EINVAL;
>> + }
>> +
>> +exit:
>> + return ret;
>> +}
>> +
>> +static const char * const ti_sn_bridge_supply_names[] = {
>> + "vccio",
>> + "vcca",
>> +};
>> +
>> +static int ti_sn_bridge_parse_regulators(struct ti_sn_bridge *pdata)
>> +{
>> + unsigned int i;
>> + int ret;
>> +
>> + 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];
>> +
>> + ret = devm_regulator_bulk_get(pdata->dev,
>> + pdata->num_supplies, pdata->supplies);
>> +
>> + return ret;
>> +}
>> +
>> +static void ti_sn_bridge_attach_panel(struct ti_sn_bridge *pdata)
>> +{
>> + struct device_node *panel_node, *port, *endpoint;
>> + struct drm_panel *panel = NULL;
>> +
>> + port = of_graph_get_port_by_id(pdata->dev->of_node, 1);
>> + if (port) {
>> + endpoint = of_get_child_by_name(port, "endpoint");
>> + of_node_put(port);
>> + if (!endpoint) {
>> + dev_err(pdata->dev, "no output endpoint found\n");
>> + goto error;
>> + }
>> +
>> + panel_node = of_graph_get_remote_port_parent(endpoint);
>> + of_node_put(endpoint);
>> + if (!panel_node) {
>> + dev_err(pdata->dev, "no output node found\n");
>> + goto error;
>> + }
>> +
>> + panel = of_drm_find_panel(panel_node);
>> + of_node_put(panel_node);
>> + if (!panel) {
>> + dev_err(pdata->dev, "no panel node found\n");
>> + goto error;
>> + }
>> + }
>> +
>> + pdata->panel = panel;
>> + drm_panel_attach(panel, &pdata->connector);
>> +
>> + return;
>> +
>> +error:
>> + pdata->panel = NULL;
>> +}
>> +
>> +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");
>> + 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);
>> + goto err_dsi_device;
>> + }
>> +
>> + /* 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");
>> + goto err_dsi_attach;
>> + }
>> +
>> + pdata->dsi = dsi;
>> +
>> + DRM_DEBUG("bridge attached\n");
>> + /* attach panel to bridge */
>> + ti_sn_bridge_attach_panel(pdata);
>> +
>> + return 0;
>> +
>> +err_dsi_attach:
>> + mipi_dsi_device_unregister(dsi);
>> +err_dsi_device:
>> + return ret;
>> +}
>> +
>> +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);
>> + }
>> +
>> + ti_sn_bridge_power_ctrl(pdata, false);
>> +}
>> +
>> +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;
>> + struct drm_display_mode *mode = &pdata->curr_mode;
>> + u32 bit_rate_mhz, clk_freq_mhz;
>> + u8 val = 0;
>> +
>> + if (!pdata->num_modes)
>> + return;
>> +
>> + ti_sn_bridge_power_ctrl(pdata, true);
>> +
>> + if (panel)
>> + drm_panel_prepare(panel);
>> +
>> + /* CLK_SRC config */
>> + ti_sn_bridge_write(pdata, SN_REFCLK_FREQ_REG,
>> + (DPPLL_CLK_SRC_REFCLK | (SN_REF_CLK_19_2_MHZ << 0x1)));
>> +
>> + /* DSI_A lane config */
>> + ti_sn_bridge_read(pdata, SN_DSI_LANES_REG, &val, 0x1);
>> + val |= ((0x4 - pdata->dsi->lanes) & 0x03) << 0x3;
>> + ti_sn_bridge_write(pdata, SN_DSI_LANES_REG, val);
>> +
>> + /* DP lane config */
>> + ti_sn_bridge_read(pdata, SN_SSC_CONFIG_REG, &val, 0x1);
>> + val |= ((pdata->dsi->lanes - 0x1) & 0x03) << 0x4;
>> + ti_sn_bridge_write(pdata, SN_SSC_CONFIG_REG, val);
>> +
>> + /* set dsi clk frequency value */
>> + 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 / 0x5) +
>> + (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 0x5) & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_DSIA_CLK_FREQ_REG, val);
>> +
>> + ti_sn_bridge_write(pdata, SN_DATARATE_CONFIG_REG, 0x80); /* set HBR
>> */
>> +
>> + ti_sn_bridge_write(pdata, SN_PLL_ENABLE_REG, 0x1); /* Enable PLL */
>> + usleep_range(10000, 10001);
>> +
>> + /* DPCD Register 0x0010A in Sink */
>> + ti_sn_bridge_write(pdata, SN_AUX_WDATA0_REG, 0x01);
>> + ti_sn_bridge_write(pdata, SN_AUX_ADDR_19_16_REG, 0x00);
>> + ti_sn_bridge_write(pdata, SN_AUX_ADDR_15_8_REG, 0x01);
>> + ti_sn_bridge_write(pdata, SN_AUX_ADDR_7_0_REG, 0x0A);
>> + ti_sn_bridge_write(pdata, SN_AUX_LENGTH_REG, 0x01);
>> + ti_sn_bridge_write(pdata, SN_AUX_CMD_REG, 0x81);
>> + usleep_range(10000, 10001);
>> +
>> + /* Semi auto link training mode */
>> + ti_sn_bridge_write(pdata, SN_ML_TX_MODE_REG, 0x0A);
>> + usleep_range(20000, 20001);
>> +
>> + /* config video parameters */
>> + ti_sn_bridge_write(pdata, SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG,
>> + mode->hdisplay & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG,
>> + (mode->hdisplay >> 0x8) & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG,
>> + mode->vdisplay & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG,
>> + (mode->hdisplay >> 0x8) & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG,
>> + (mode->htotal - mode->hsync_end) & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG,
>> + ((mode->htotal - mode->hsync_end) >> 0x8) & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG,
>> + (mode->vtotal - mode->vsync_end) & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG,
>> + ((mode->vtotal - mode->vsync_end) >> 0x8) & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_CHA_HORIZONTAL_BACK_PORCH_REG,
>> + (mode->hsync_start - mode->hdisplay) & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_CHA_VERTICAL_BACK_PORCH_REG,
>> + (mode->vsync_start - mode->vdisplay) & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_CHA_HORIZONTAL_FRONT_PORCH_REG,
>> + (mode->hsync_end - mode->hsync_start) & 0xFF);
>> + ti_sn_bridge_write(pdata, SN_CHA_VERTICAL_FRONT_PORCH_REG,
>> + (mode->vsync_end - mode->vsync_start) & 0xFF);
>> + if (pdata->dsi->format == MIPI_DSI_FMT_RGB888)
>> + ti_sn_bridge_write(pdata, SN_DATA_FORMAT_REG, 0x00);
>> + else
>> + ti_sn_bridge_write(pdata, SN_DATA_FORMAT_REG, 0x01);
>> + usleep_range(10000, 10001);
>> +
>> + /* enable video stream */
>> + ti_sn_bridge_read(pdata, SN_ENH_FRAME_REG, &val, 0x1);
>> + val |= BIT(3);
>> + ti_sn_bridge_write(pdata, SN_ENH_FRAME_REG, val);
>> +
>> + if (panel)
>> + drm_panel_enable(panel);
>
> As mentioned in the previous review, you should configure the mode in
> mode_set,
> not enable. This eliminates the need for curr_mode.
If we move this to mode_set then it does not match with the datasheet
init sequence
provided in page 71. First mode_set is called in commit which will set
the current mode.
after that when bridge enable gets called gpio and regulator enable and
then the bridge registers
needs to be programmed based on current mode.
>
>> +}
>> +
>> +static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
>> + .attach = ti_sn_bridge_attach,
>> + .enable = ti_sn_bridge_enable,
>> + .disable = ti_sn_bridge_disable,
>> + .mode_set = ti_sn_bridge_mode_set,
>> +};
>> +
>> +static int ti_sn_bridge_parse_gpios(struct ti_sn_bridge *pdata)
>> +{
>> + int ret = 0;
>> +
>> + if (!pdata || !pdata->dev)
>> + return -EINVAL;
>> +
>> + pdata->enable_gpios = devm_gpiod_get_array(pdata->dev,
>> + "enable", GPIOD_OUT_LOW);
>> + if (IS_ERR(pdata->enable_gpios)) {
>> + DRM_ERROR("failed to get enable gpio from DT\n");
>> + ret = PTR_ERR(pdata->enable_gpios);
>> + goto exit;
>> + }
>> +
>> + pdata->irq_gpio = devm_gpiod_get_optional(pdata->dev, "irq",
>> + GPIOD_OUT_LOW);
>> + if (IS_ERR(pdata->irq_gpio))
>> + DRM_ERROR("failed to get irq gpio from DT\n");
>> +
>> +exit:
>> + return ret;
>> +}
>> +
>> +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 (!client || !client->dev.of_node) {
>> + DRM_ERROR("invalid input\n");
>> + return -EINVAL;
>> + }
>> +
>> + 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->power_on = false;
>> + pdata->dev = &client->dev;
>> + pdata->i2c_client = client;
>> + DRM_DEBUG("I2C address is %x\n", client->addr);
>> +
>> + ret = ti_sn_bridge_parse_dsi_host(pdata);
>> + if (ret) {
>> + DRM_ERROR("failed to parse device tree\n");
>> + goto err_dt_parse;
>> + }
>> +
>> + ret = ti_sn_bridge_parse_gpios(pdata);
>> + if (ret) {
>> + DRM_ERROR("failed to parse gpios from DT\n");
>> + goto err_dt_parse;
>> + }
>> +
>> + ret = ti_sn_bridge_parse_regulators(pdata);
>> + if (ret) {
>> + DRM_ERROR("failed to parse regulators\n");
>> + goto err_dt_parse;
>> + }
>> +
>> + 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");
>> + }
>> +
>> + ti_sn_bridge_power_ctrl(pdata, true);
>> + ret = ti_sn_bridge_read_device_rev(pdata);
>> + if (ret) {
>> + DRM_ERROR("failed to read chip rev\n");
>> + goto err_config;
>> + } else {
>> + DRM_DEBUG("bridge chip enabled successfully\n");
>> + }
>> + ti_sn_bridge_power_ctrl(pdata, false);
>> +
>> + i2c_set_clientdata(client, pdata);
>> + dev_set_drvdata(&client->dev, pdata);
>> +
>> + pdata->bridge.funcs = &ti_sn_bridge_funcs;
>> + pdata->bridge.of_node = client->dev.of_node;
>> +
>> + drm_bridge_add(&pdata->bridge);
>> +
>> + return ret;
>> +
>> +err_config:
>> + ti_sn_bridge_power_ctrl(pdata, false);
>> +err_dt_parse:
>> + devm_kfree(&client->dev, pdata);
>> + return ret;
>> +}
>> +
>> +static int ti_sn_bridge_remove(struct i2c_client *client)
>> +{
>> + int ret = -EINVAL;
>> + struct ti_sn_bridge *pdata = i2c_get_clientdata(client);
>> +
>> + if (!pdata)
>> + goto end;
>> +
>> + mipi_dsi_detach(pdata->dsi);
>> + mipi_dsi_device_unregister(pdata->dsi);
>> +
>> + drm_bridge_remove(&pdata->bridge);
>> +
>> + disable_irq(pdata->irq);
>> + free_irq(pdata->irq, pdata);
>
> This is uninitialized.
since irq is anyways not used in this driver yet so i will remove this
part.
>
>> +
>> + ti_sn_bridge_gpio_configure(pdata, false);
>> + i2c_put_adapter(pdata->ddc);
>> +
>> + devm_kfree(&client->dev, pdata);
>> +
>> +end:
>> + return ret;
>> +}
>> +
>> +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",
>> + .owner = THIS_MODULE,
>> + .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");
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project
>>
More information about the dri-devel
mailing list