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

[Sean Paul]

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 
> ---
>  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 000..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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define SN_BRIDGE_REVISION_ID 0x2
> +
> +/* Link Training specific registers */
> +#define SN_DEVICE_REV_REG0x08
> +#define SN_HPD_DISABLE_REG   

Re: [Freedreno] [PATCH v5 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[Stephen Boyd]

Quoting Sandeep Panda (2018-05-01 21:32:00)
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> new file mode 100644
> index 000..0d042ce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> @@ -0,0 +1,76 @@
> +SN65DSI86 DSI to eDP bridge chip
> +
> +
> +This is the binding for Texas Instruments SN65DSI86 bridge.
> +http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86=pdf
> +
> +Required properties:
> +- compatible: Must be "ti,sn65dsi86"
> +- reg: i2c address of the chip, 0x2d as per datasheet
> +- enable-gpios: OF device-tree gpio specification for bridge_en pin
> +
> +- vccio-supply: A 1.8V supply that powers up the digital IOs.
> +- vpll-supply: A 1.8V supply that powers up the displayport PLL.
> +- vcca-supply: A 1.2V supply that powers up the analog circuits.
> +- vcc-supply: A 1.2V supply that powers up the digital core.
> +
> +Optional properties:
> +- interrupts: Specifier for the SN65DSI86 interrupt line.
> +- hpd-gpios: OF device-tree gpio specifications for HPD pin.
> +
> +- refclk-freq-khz: Value specifying the frequency of reference clock in KHz 
> unit.

Please use the clocks property instead. We may need to turn the clk on
first before this can work so the driver would use the clk framework (at
least in linux). clock-names could have 'refclk' because that's the pin
name.

Is there a way in DRM to figure out the frequency of the clock frequency
for DACP/N? It looks like if refclk is grounded, then the DACP/N pins
from the DSI side should be one of a set of frequencies, so I'm just
curious how that will work and if the binding would need to be updated
to indicate what the frequency of the DSI clock lane is, or if DRM can
tell this driver through the port/graph stuff somehow.

> +
> +- gpio-controller: Marks the device has a GPIO controller.
> +- #gpio-cells: Number of GPIO cells. Refer to binding document 
> "gpio/gpio.txt"

What's the number? 2?

> +- #pwm-cells : Number of cells used to specify a PWM. See pwm.txt in this 
> directory
> +  for a description of the cell formats.

What's the number? 1? There's a pwm.txt in this directory?

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno