Re: [PATCH v8 1/4] drm/bridge: add support for sn65dsi86 bridge driver

Sean Paul
2018-06-05T22:59:52Z

On Tue, Jun 05, 2018 at 11:10:15AM +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.
> - 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).
>
> Signed-off-by: Sandeep Panda <spanda@codeaurora.org>
> ---
> drivers/gpu/drm/bridge/Kconfig | 9 +
> drivers/gpu/drm/bridge/Makefile | 1 +
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 666 ++++++++++++++++++++++++++++++++++
> 3 files changed, 676 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

SN65DSI86 should be in front of TFP410 here and above.

> 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..add6e0f
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

/snip

> +
> +#define REFCLK_LUT_SIZE 5
> +
> +/* clk frequencies supported by bridge in Hz in case derived from REFCLK pin */
> +static const u32 ti_sn_bridge_refclk_lut[] = {
> + 12000000,
> + 19200000,
> + 26000000,
> + 27000000,
> + 38400000,
> +};
> +
> +/* clk frequencies supported by bridge in Hz in case derived from DACP/N pin */
> +static const u32 ti_sn_bridge_dsiclk_lut[] = {
> + 468000000,
> + 384000000,
> + 416000000,
> + 486000000,
> + 460800000,
> +};
> +
> +static void ti_sn_bridge_set_refclk(struct ti_sn_bridge *pdata)
> +{
> + int i = 0;
> + u8 refclk_src;
> + u32 refclk_rate;
> + const u32 *refclk_lut;
> +
> + if (pdata->refclk) {
> + refclk_src = DPPLL_CLK_SRC_REFCLK;
> + refclk_rate = clk_get_rate(pdata->refclk);
> + refclk_lut = ti_sn_bridge_refclk_lut;
> + clk_prepare_enable(pdata->refclk);
> + } else {
> + refclk_src = DPPLL_CLK_SRC_DSICLK;
> + refclk_rate = ti_sn_bridge_get_dsi_freq(pdata) * 1000;
> + refclk_lut = ti_sn_bridge_dsiclk_lut;
> + }
> +
> + /* for i equals to REFCLK_LUT_SIZE means default frequency */
> + for (i = 0; i < REFCLK_LUT_SIZE; i++)

Instead of REFCLK_LUT_SIZE, use a local variable, ie:

size_t refclk_lut_size;

...

if(pdata->refclk) {
...
refclk_lut = ti_sn_bridge_refclk_lut;
refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_refclk_lut);
...
} else {
...
refclk_lut = ti_sn_bridge_dsiclk_lut;
refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_dsiclk_lut);
}

And remove REFCLK_LUT_SIZE.

> + if (refclk_lut[i] == refclk_rate)
> + break;
> +
> + regmap_write(pdata->regmap, SN_REFCLK_FREQ_REG,
> + (refclk_src | (i << SN_DSIA_REFCLK_OFFSET)));
> +}
> +
> +/**
> + * LUT index corresponds to register value and
> + * LUT values corresponds to dp data rate supported
> + * by the bridge in Mbps unit.
> + */
> +static const unsigned int ti_sn_bridge_dp_rate_lut[] = {
> + 0, 1620, 2160, 2430, 2700, 3240, 4320, 5400
> +};
> +
> +static void ti_sn_bridge_set_dsi_dp_rate(struct ti_sn_bridge *pdata)
> +{
> + unsigned int bit_rate_mhz, clk_freq_mhz, dp_rate_mhz;
> + unsigned int val = 0, i = 0;
> + struct drm_display_mode *mode =
> + &pdata->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;
> +
> + 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);
> + ti_sn_bridge_write_u16(pdata, SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG,
> + mode->hsync_end - mode->hsync_start);
> + ti_sn_bridge_write_u16(pdata, SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG,
> + mode->vsync_end - mode->vsync_start);
> +
> + 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 = 0;
> +
> + 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, !0);

What is !0?

Sean

/snip

--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org