From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH v3 1/2] drm/bridge: Add Cadence DSI driver Date: Wed, 20 Sep 2017 14:55:02 +0300 Message-ID: <27834b62-664f-d403-6396-2339cb4fda1c@ti.com> References: <20170831155519.3704-1-boris.brezillon@free-electrons.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20170831155519.3704-1-boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> Content-Language: en-US Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Boris Brezillon , David Airlie , Daniel Vetter , dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, Archit Taneja Cc: Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Neil Webb , Richard Sproul , Simon Hatliff , Maxime Ripard , Thomas Petazzoni , Cyprian Wronka , Alan Douglas , Jyri Sarha List-Id: devicetree@vger.kernel.org =EF=BB=BFHi Boris, Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Bu= siness ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 31/08/17 18:55, Boris Brezillon wrote: > Add a driver for Cadence DPI -> DSI bridge. >=20 > This driver only support a subset of Cadence DSI bridge capabilities. >=20 > 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) >=20 > Signed-off-by: Boris Brezillon > --- > 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 >=20 > 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 We need some power management. At the moment the clocks are kept always enabled. Those need to be turned off when the IP is not used. > +static irqreturn_t cdns_dsi_interrupt(int irq, void *data) > +{ > + struct cdns_dsi *dsi =3D data; > + irqreturn_t ret =3D IRQ_NONE; > + u32 flag, ctl; > + > + flag =3D readl(dsi->regs + DIRECT_CMD_STS_FLAG); > + if (flag) { > + ctl =3D readl(dsi->regs + DIRECT_CMD_STS_CTL); > + ctl &=3D ~flag; > + writel(ctl, dsi->regs + DIRECT_CMD_STS_CTL); I presume it's the enable/disable bit in STS_CTL that prevents the interrupt from triggering again, instead of the status flag? Just making sure, because I think on some IPs the status flag has been the one that triggers the interrupt. > + complete(&dsi->direct_cmd_comp); > + ret =3D IRQ_HANDLED; > + } > + > + return ret; > +} > + > +static ssize_t cdns_dsi_transfer(struct mipi_dsi_host *host, > + const struct mipi_dsi_msg *msg) > +{ > + struct cdns_dsi *dsi =3D to_cdns_dsi(host); > + u32 cmd, sts, val, wait =3D WRITE_COMPLETED, ctl =3D 0; > + struct mipi_dsi_packet packet; > + int ret, i, tx_len, rx_len; > + > + ret =3D mipi_dsi_create_packet(&packet, msg); > + if (ret) > + return ret; > + > + tx_len =3D msg->tx_buf ? msg->tx_len : 0; > + rx_len =3D msg->rx_buf ? msg->rx_len : 0; > + > + /* For read operations, the maximum TX len is 2. */ Hmm, why is that? > + if (rx_len && tx_len > 2) > + return -ENOTSUPP; > + > + /* TX len is limited by the CMD FIFO depth. */ > + if (tx_len > dsi->direct_cmd_fifo_depth) > + return -ENOTSUPP; > + > + /* RX len is limited by the RX FIFO depth. */ > + if (rx_len > dsi->rx_fifo_depth) > + return -ENOTSUPP; > + > + cmd =3D CMD_SIZE(tx_len) | CMD_VCHAN_ID(msg->channel) | > + CMD_DATATYPE(msg->type); > + > + if (msg->flags & MIPI_DSI_MSG_USE_LPM) > + cmd |=3D CMD_LP_EN; > + > + if (mipi_dsi_packet_format_is_long(msg->type)) > + cmd |=3D CMD_LONG; > + > + if (rx_len) { > + cmd |=3D READ_CMD; > + wait =3D READ_COMPLETED_WITH_ERR | READ_COMPLETED; > + ctl =3D READ_EN | BTA_EN; > + } else if (msg->flags & MIPI_DSI_MSG_REQ_ACK) { > + cmd |=3D BTA_REQ; > + wait =3D ACK_WITH_ERR_RCVD | ACK_RCVD; > + ctl =3D BTA_EN; > + } It's been a while since I worked with DSI, but... Shouldn't there be a check somewhere that the packet(s) can fit into the blanking intervals? Tomi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html