[[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver
spanda at codeaurora.org
spanda at codeaurora.org
Thu Apr 19 04:37:20 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.
>
>> +
>> +#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?
This as per the review comment in previous patchset
"I see IRQ and EN in the datasheet, but not the others. It seems like
the aux_*
and edp_* gpios are always equal to en. So if you actually need them,
don't
specify a new struct, just add irq_gpio to the main struct and add an
array of
en_gpios to handle the rest."
I see in schematic 2 more gpios are needed to enable aux_channel
communication
through the ddc. So i have put an array of enable gpios. Based on dt it
will dynamically
parse one or more gpios.
>
>> + 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.
>
>> +}
>> +
>> +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.
>
>> +
>> + 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