On Thu, 7 Sep 2017 15:06:13 +0530
Archit Taneja <architt@xxxxxxxxxxxxxx> wrote:
> Hi,
>
> On 08/31/2017 09:25 PM, Boris Brezillon wrote:
> > Add a driver for Cadence DPI -> DSI bridge.
> >
> > This driver only support a subset of Cadence DSI bridge capabilities.
> >
> > Here is a non-exhaustive list of missing features:
> > * burst mode
> > * dynamic configuration of the DPHY based on the
> > * support for additional input interfaces (SDI input)
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>
> > ---
> > Changes in v3:
> > - replace magic values by real timing calculation. The DPHY PLL clock
> > is still hardcoded since we don't have a working DPHY block yet, and
> > this is the piece of HW we need to dynamically configure the PLL
> > rate based on the display refresh rate and the resolution.
> > - parse DSI devices represented with the OF-graph. This is needed to
> > support DSI devices controlled through an external bus like I2C or
> > SPI.
> > - use the DRM panel-bridge infrastructure to simplify the DRM panel
> > logic
> >
> > Changes in v2:
> > - rebase on v4.12-rc1 and adapt to driver to the drm_bridge API changes
> > - return the correct error when devm_clk_get(sysclk) fails
> > - add missing depends on OF and select DRM_PANEL in the Kconfig entry
> > ---
> > drivers/gpu/drm/bridge/Kconfig | 9 +
> > drivers/gpu/drm/bridge/Makefile | 1 +
> > drivers/gpu/drm/bridge/cdns-dsi.c | 1090 +++++++++++++++++++++++++++++++++++++
> > 3 files changed, 1100 insertions(+)
> > create mode 100644 drivers/gpu/drm/bridge/cdns-dsi.c
> >
> > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > index adf9ae0e0b7c..88c324b12e16 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -25,6 +25,15 @@ config DRM_ANALOGIX_ANX78XX
> > the HDMI output of an application processor to MyDP
> > or DisplayPort.
> >
> > +config DRM_CDNS_DSI
> > + tristate "Cadence DPI/DSI bridge"
> > + select DRM_KMS_HELPER
> > + select DRM_MIPI_DSI
> > + select DRM_PANEL
> > + depends on OF
> > + help
> > + Support Cadence DPI to DSI bridge.
>
> Maybe we can briefly mention here that it's a internal bridge/IP, and not an external chip?
Sure.
>
> > +
> > config DRM_DUMB_VGA_DAC
> > tristate "Dumb VGA DAC Bridge support"
> > depends on OF
> > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> > index defcf1e7ca1c..77b65e8ecf59 100644
> > --- a/drivers/gpu/drm/bridge/Makefile
> > +++ b/drivers/gpu/drm/bridge/Makefile
> > @@ -1,4 +1,5 @@
> > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> > +obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
> > obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
> > obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
> > obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o
> > diff --git a/drivers/gpu/drm/bridge/cdns-dsi.c b/drivers/gpu/drm/bridge/cdns-dsi.c
>
> <snip>
>
> > +
> > +static void cdns_dsi_bridge_disable(struct drm_bridge *bridge)
> > +{
> > + struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge);
> > + struct cdns_dsi *dsi = input_to_dsi(input);
> > + u32 val;
> > +
> > + val = readl(dsi->regs + MCTL_MAIN_DATA_CTL);
> > + val &= ~(IF_VID_SELECT_MASK | IF_VID_MODE | VID_EN | HOST_EOT_GEN |
> > + DISP_EOT_GEN);
>
> I see some truncation related sparse warnings here and a couple of other places
> when building against arm32. Those would be nice to fix.
I'll have a look.
> > +static int cdns_dsi_attach(struct mipi_dsi_host *host,
> > + struct mipi_dsi_device *dev)
> > +{
> > + struct cdns_dsi *dsi = to_cdns_dsi(host);
> > + struct cdns_dsi_output *output = &dsi->output;
> > + struct cdns_dsi_input *input = &dsi->input;
> > + struct drm_bridge *bridge;
> > + struct drm_panel *panel;
> > + struct device_node *np;
> > + int ret;
> > +
> > + /*
> > + * We currently do not support connecting several DSI devices to the
> > + * same host. In order to support that we'd need the DRM bridge
> > + * framework to allow dynamic reconfiguration of the bridge chain.
> > + */
> > + if (output->dev)
> > + return -EBUSY;
> > +
> > + /* We do not support burst mode yet. */
> > + if (dev->mode_flags & MIPI_DSI_MODE_VIDEO_BURST)
> > + return -ENOTSUPP;
> > +
> > + /*
> > + * The host <-> device link might be described using an OF-graph
> > + * representation, in this case we extract the device of_node from
> > + * this representation, otherwise we use dsidev->dev.of_node which
> > + * should have been filled by the core.
> > + */
> > + np = of_graph_get_remote_node(dsi->base.dev->of_node, DSI_OUTPUT_PORT,
> > + dev->channel);
> > + if (!np)
> > + np = of_node_get(dev->dev.of_node); > +
> > + panel = of_drm_find_panel(np);
> > + if (panel) {
> > + bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DSI);
> > + } else {
> > + bridge = of_drm_find_bridge(dev->dev.of_node);
> > + if (!bridge)
> > + bridge = ERR_PTR(-EINVAL);
> > + }
> > +
> > + of_node_put(np);
>
> It would have been nice to use drm_of_find_panel_or_bridge() here, but I guess
> you couldn't use it because you have to handle both OF-graph based links, and
> children directly under the host DSI bus.
Yep.
Thanks for you review, and sorry for taking so long to reply.
Boris
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
