RE: [PATCH v6 2/2] drm: rcar-du: Add RZ/G2L DSI driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

> Subject: Re: [PATCH v6 2/2] drm: rcar-du: Add RZ/G2L DSI driver
> 
> Hi Biju,
> 
> On Tue, Aug 30, 2022 at 08:22:08AM +0000, Biju Das wrote:
> > Subject: Re: [PATCH v6 2/2] drm: rcar-du: Add RZ/G2L DSI driver
> > > On Mon, Aug 29, 2022 at 10:19:01AM +0100, Biju Das wrote:
> > > > This driver supports the MIPI DSI encoder found in the RZ/G2L SoC.
> > > > It currently supports DSI video mode only.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > Acked-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> > > > ---
> > > > v5->v6:
> > > >  * Updated commit description
> > > >  * Moved handling of arst and prst from rzg2l_mipi_dsi_startup-
> > > >runtime
> > > >    PM suspend/resume handlers.
> > > >  * Max lane capability read at probe(), and enforced in
> > > >    rzg2l_mipi_dsi_host_attach()
> > > >  * Simplified vich1ppsetr setting.
> > > >  * Renamed hsclk_running_mode,hsclk_mode->is_clk_cont.
> > > >  * Fixed typo in probe error message(arst->rst).
> > > >  * Reordered DRM bridge initaization in probe()
> > > >  * Updated typo in e-mail address.
> > > > v4->v5:
> > > >  * Added Ack from Sam.
> > > >  * Added a trivial change, replaced rzg2l_mipi_dsi_parse_dt()
> > > >    with drm_of_get_data_lanes_count_ep() in probe.
> > > > v3->v4:
> > > >  * Updated error handling in rzg2l_mipi_dsi_startup() and
> > > > rzg2l_mipi_dsi_atomic_enable()
> > > > v2->v3:
> > > >  * pass rzg2l_mipi_dsi pointer to {Link,Phy} register rd/wr
> > > > function
> > > instead
> > > >    of the memory pointer
> > > >  * Fixed the comment in rzg2l_mipi_dsi_startup()
> > > >  * Removed unnecessary dbg message from
> > > > rzg2l_mipi_dsi_start_video()
> > > >  * DRM bridge parameter initialization moved to probe
> > > >  * Replaced dev_dbg->dev_err in rzg2l_mipi_dsi_parse_dt()
> > > >  * Inserted the missing blank lane after return in probe()
> > > >  * Added missing MODULE_DEVICE_TABLE
> > > >  * Added include linux/bits.h in header file
> > > >  * Fixed various macros in header file.
> > > >  * Reorder the make file for DSI, so that it is no more dependent
> > > >    on RZ/G2L DU patch series.
> > > > v1->v2:
> > > >  * Rework based on dt-binding change (DSI + D-PHY) as single block
> > > >  * Replaced link_mmio and phy_mmio with mmio in struct
> > > > rzg2l_mipi_dsi
> > > >  * Replaced rzg2l_mipi_phy_write with rzg2l_mipi_dsi_phy_write
> > > >    and rzg2l_mipi_dsi_link_write
> > > >  * Replaced rzg2l_mipi_phy_read->rzg2l_mipi_dsi_link_read
> > > > RFC->v1:
> > > >  * Added "depends on ARCH_RENESAS || COMPILE_TEST" on KCONFIG
> > > >    and dropped DRM as it is implied by DRM_BRIDGE
> > > >  * Used devm_reset_control_get_exclusive() for reset handle
> > > >  * Removed bool hsclkmode from struct rzg2l_mipi_dsi
> > > >  * Added error check for pm, using pm_runtime_resume_and_get()
> > > > instead
> > > of
> > > >    pm_runtime_get_sync()
> > > >  * Added check for unsupported formats in
> > > > rzg2l_mipi_dsi_host_attach()
> > > >  * Avoided read-modify-write stopping hsclock
> > > >  * Used devm_platform_ioremap_resource for resource allocation
> > > >  * Removed unnecessary assert call from probe and remove.
> > > >  * wrap the line after the PTR_ERR() in probe()
> > > >  * Updated reset failure messages in probe
> > > >  * Fixed the typo arstc->prstc
> > > >  * Made hex constants to lower case.
> > > > RFC:
> > > >  *
> > > > ---
> > > >  drivers/gpu/drm/rcar-du/Kconfig               |   8 +
> > > >  drivers/gpu/drm/rcar-du/Makefile              |   2 +
> > > >  drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c      | 703
> > > ++++++++++++++++++
> > > >  drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h | 151 ++++
> > > >  4 files changed, 864 insertions(+)  create mode 100644
> > > > drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> > > >  create mode 100644 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h
> > > >
> > > > diff --git a/drivers/gpu/drm/rcar-du/Kconfig
> > > > b/drivers/gpu/drm/rcar-du/Kconfig index c959e8c6be7d..58ffb8c2443b
> > > > 100644
> > > > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > > > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > > > @@ -51,6 +51,14 @@ config DRM_RCAR_MIPI_DSI
> > > >  	help
> > > >  	  Enable support for the R-Car Display Unit embedded MIPI
> DSI
> > > encoders.
> > > >
> > > > +config DRM_RZG2L_MIPI_DSI
> > > > +	tristate "RZ/G2L MIPI DSI Encoder Support"
> > > > +	depends on DRM_BRIDGE && OF
> > > > +	depends on ARCH_RENESAS || COMPILE_TEST
> > > > +	select DRM_MIPI_DSI
> > > > +	help
> > > > +	  Enable support for the RZ/G2L Display Unit embedded MIPI
> DSI
> > > encoders.
> > > > +
> > > >  config DRM_RCAR_VSP
> > > >  	bool "R-Car DU VSP Compositor Support" if ARM
> > > >  	default y if ARM64
> > > > diff --git a/drivers/gpu/drm/rcar-du/Makefile
> > > > b/drivers/gpu/drm/rcar-du/Makefile
> > > > index 6f132325c8b7..b8f2c82651d9 100644
> > > > --- a/drivers/gpu/drm/rcar-du/Makefile
> > > > +++ b/drivers/gpu/drm/rcar-du/Makefile
> > > > @@ -14,3 +14,5 @@ obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-
> drm.o
> > > >  obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
> > > >  obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
> > > >  obj-$(CONFIG_DRM_RCAR_MIPI_DSI)		+= rcar_mipi_dsi.o
> > > > +
> > > > +obj-$(CONFIG_DRM_RZG2L_MIPI_DSI)	+= rzg2l_mipi_dsi.o
> > > > diff --git a/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> > > > b/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> > > > new file mode 100644
> > > > index 000000000000..46a71e39e336
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> > > > @@ -0,0 +1,703 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * RZ/G2L MIPI DSI Encoder Driver
> > > > + *
> > > > + * Copyright (C) 2022 Renesas Electronics Corporation  */
> > > > +#include <linux/clk.h> #include <linux/delay.h> #include
> > > > +<linux/io.h> #include <linux/iopoll.h> #include <linux/module.h>
> > > > +#include <linux/of.h> #include <linux/of_device.h> #include
> > > > +<linux/of_graph.h> #include <linux/platform_device.h> #include
> > > > +<linux/pm_runtime.h> #include <linux/reset.h> #include
> > > > +<linux/slab.h>
> > > > +
> > > > +#include <drm/drm_atomic.h>
> > > > +#include <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h>
> > > > +#include <drm/drm_mipi_dsi.h> #include <drm/drm_of.h> #include
> > > > +<drm/drm_panel.h> #include <drm/drm_probe_helper.h>
> > > > +
> > > > +#include "rzg2l_mipi_dsi_regs.h"
> > > > +
> > > > +struct rzg2l_mipi_dsi {
> > > > +	struct device *dev;
> > > > +	void __iomem *mmio;
> > > > +
> > > > +	struct reset_control *rstc;
> > > > +	struct reset_control *arstc;
> > > > +	struct reset_control *prstc;
> > > > +
> > > > +	struct mipi_dsi_host host;
> > > > +	struct drm_bridge bridge;
> > > > +	struct drm_bridge *next_bridge;
> > > > +
> > > > +	struct clk *vclk;
> > > > +
> > > > +	enum mipi_dsi_pixel_format format;
> > > > +	unsigned int num_data_lanes;
> > > > +	unsigned int max_lanes;
> > > > +	unsigned int lanes;
> > > > +	unsigned long mode_flags;
> > > > +};
> > > > +
> > > > +static inline struct rzg2l_mipi_dsi *
> > > > +bridge_to_rzg2l_mipi_dsi(struct drm_bridge *bridge) {
> > > > +	return container_of(bridge, struct rzg2l_mipi_dsi, bridge);
> }
> > > > +
> > > > +static inline struct rzg2l_mipi_dsi *
> > > > +host_to_rzg2l_mipi_dsi(struct mipi_dsi_host *host) {
> > > > +	return container_of(host, struct rzg2l_mipi_dsi, host); }
> > > > +
> > > > +static void rzg2l_mipi_dsi_phy_write(struct rzg2l_mipi_dsi *dsi,
> > > > +u32 reg, u32 data) {
> > > > +	iowrite32(data, dsi->mmio + reg); }
> > > > +
> > > > +static void rzg2l_mipi_dsi_link_write(struct rzg2l_mipi_dsi *dsi,
> > > > +u32 reg, u32 data) {
> > > > +	iowrite32(data, dsi->mmio + LINK_REG_OFFSET + reg); }
> > > > +
> > > > +static u32 rzg2l_mipi_dsi_link_read(struct rzg2l_mipi_dsi *dsi,
> > > > +u32
> > > > +reg) {
> > > > +	return ioread32(dsi->mmio + LINK_REG_OFFSET + reg); }
> > > > +
> > > > +/*
> > > > +-----------------------------------------------------------------
> > > > +----
> > > > +--------
> > > > + * Hardware Setup
> > > > + */
> > > > +
> > > > +static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi *dsi,
> > > > +				  const struct drm_display_mode *mode) {
> > > > +	unsigned long hsfreq;
> > > > +	u32 txsetr;
> > > > +	u32 clstptsetr;
> > > > +	u32 lptrnstsetr;
> > > > +	u32 clkkpt;
> > > > +	u32 clkbfht;
> > > > +	u32 clkstpt;
> > > > +	u32 golpbkt;
> > > > +	unsigned int bpp;
> > > > +	struct {
> > > > +		u32 tclk_miss;
> > > > +		u32 t_init;
> > > > +		u32 tclk_prepare;
> > > > +		u32 tclk_settle;
> > > > +		u32 tclk_trail;
> > > > +		u32 tclk_post;
> > > > +		u32 tclk_pre;
> > > > +		u32 tclk_zero;
> > > > +		u32 tlpx;
> > > > +		u32 ths_prepare;
> > > > +		u32 ths_settle;
> > > > +		u32 ths_exit;
> > > > +		u32 ths_trail;
> > > > +		u32 ths_zero;
> > > > +	} timings;
> > > > +	u32 dphyctrl0;
> > > > +	u32 dphytim0;
> > > > +	u32 dphytim1;
> > > > +	u32 dphytim2;
> > > > +	u32 dphytim3;
> > > > +	int ret;
> > > > +
> > > > +	/*
> > > > +	 * Relationship between hsclk and vclk must follow
> > > > +	 * vclk * bpp = hsclk * 8 * lanes
> > > > +	 * where vclk: video clock (Hz)
> > > > +	 *       bpp: video pixel bit depth
> > > > +	 *       hsclk: DSI HS Byte clock frequency (Hz)
> > > > +	 *       lanes: number of data lanes
> > > > +	 *
> > > > +	 * hsclk(bit) = hsclk(byte) * 8
> > > > +	 */
> > > > +	bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> > > > +	hsfreq = (mode->clock * bpp * 8) / (8 * dsi->lanes);
> > > > +
> > > > +	/* All DSI global operation timings are set with recommended
> > > setting */
> > > > +	if (hsfreq > 250000) {
> > > > +		timings.tclk_miss = 1;
> > > > +		timings.t_init = 79801;
> > > > +		timings.tclk_prepare = 8;
> > > > +		timings.tclk_settle = 9;
> > > > +		timings.tclk_trail = 7;
> > > > +		timings.tclk_post = 35;
> > > > +		timings.tclk_pre = 4;
> > > > +		timings.tclk_zero = 33;
> > > > +		timings.tlpx = 6;
> > > > +		timings.ths_prepare = 9;
> > > > +		timings.ths_settle = 9;
> > > > +		timings.ths_exit = 13;
> > > > +		timings.ths_trail = 9;
> > > > +		timings.ths_zero = 16;
> > > > +	} else {
> > > > +		timings.tclk_miss = 1;
> > > > +		timings.t_init = 79801;
> > > > +		timings.tclk_prepare = 8;
> > > > +		timings.tclk_settle = 9;
> > > > +		timings.tclk_trail = 10;
> > > > +		timings.tclk_post = 94;
> > > > +		timings.tclk_pre = 13;
> > > > +		timings.tclk_zero = 33;
> > > > +		timings.tlpx = 6;
> > > > +		timings.ths_prepare = 12;
> > > > +		timings.ths_settle = 9;
> > > > +		timings.ths_exit = 13;
> > > > +		timings.ths_trail = 17;
> > > > +		timings.ths_zero = 23;
> > > > +	}
> > > > +
> > > > +	ret = pm_runtime_resume_and_get(dsi->dev);
> > > > +	if (ret < 0)
> > > > +		return ret;
> > > > +
> > > > +	clk_set_rate(dsi->vclk, mode->clock * 1000);
> > > > +
> > > > +	/* Initializing DPHY before accessing LINK */
> > > > +	dphyctrl0 = DSIDPHYCTRL0_CAL_EN_HSRX_OFS |
> > > DSIDPHYCTRL0_CMN_MASTER_EN |
> > > > +		    DSIDPHYCTRL0_RE_VDD_DETVCCQLV18 |
> DSIDPHYCTRL0_EN_BGR;
> > > > +
> > > > +	rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYCTRL0, dphyctrl0);
> > > > +	usleep_range(20, 30);
> > > > +
> > > > +	dphyctrl0 |= DSIDPHYCTRL0_EN_LDO1200;
> > > > +	rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYCTRL0, dphyctrl0);
> > > > +	usleep_range(10, 20);
> > > > +
> > > > +	dphytim0 = DSIDPHYTIM0_TCLK_MISS(timings.tclk_miss) |
> > > > +		   DSIDPHYTIM0_T_INIT(timings.t_init);
> > > > +	dphytim1 = DSIDPHYTIM1_THS_PREPARE(timings.ths_prepare) |
> > > > +		   DSIDPHYTIM1_TCLK_PREPARE(timings.tclk_prepare) |
> > > > +		   DSIDPHYTIM1_THS_SETTLE(timings.ths_settle) |
> > > > +		   DSIDPHYTIM1_TCLK_SETTLE(timings.tclk_settle);
> > > > +	dphytim2 = DSIDPHYTIM2_TCLK_TRAIL(timings.tclk_trail) |
> > > > +		   DSIDPHYTIM2_TCLK_POST(timings.tclk_post) |
> > > > +		   DSIDPHYTIM2_TCLK_PRE(timings.tclk_pre) |
> > > > +		   DSIDPHYTIM2_TCLK_ZERO(timings.tclk_zero);
> > > > +	dphytim3 = DSIDPHYTIM3_TLPX(timings.tlpx) |
> > > > +		   DSIDPHYTIM3_THS_EXIT(timings.ths_exit) |
> > > > +		   DSIDPHYTIM3_THS_TRAIL(timings.ths_trail) |
> > > > +		   DSIDPHYTIM3_THS_ZERO(timings.ths_zero);
> > > > +
> > > > +	rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYTIM0, dphytim0);
> > > > +	rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYTIM1, dphytim1);
> > > > +	rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYTIM2, dphytim2);
> > > > +	rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYTIM3, dphytim3);
> > > > +
> > > > +	ret = reset_control_deassert(dsi->rstc);
> > > > +	if (ret < 0)
> > > > +		goto err_rstc;
> > > > +
> > > > +	udelay(1);
> > > > +
> > > > +	/* Enable Data lanes and Clock lanes */
> > > > +	txsetr = TXSETR_DLEN | TXSETR_NUMLANEUSE(dsi->lanes - 1) |
> > > TXSETR_CLEN;
> > > > +	rzg2l_mipi_dsi_link_write(dsi, TXSETR, txsetr);
> > > > +
> > > > +	/*
> > > > +	 * Global timings characteristic depends on high speed Clock
> > > Frequency
> > > > +	 * Currently MIPI DSI-IF just supports maximum FHD@60 with:
> > > > +	 * - videoclock = 148.5 (MHz)
> > > > +	 * - bpp: maximum 24bpp
> > > > +	 * - data lanes: maximum 4 lanes
> > > > +	 * Therefore maximum hsclk will be 891 Mbps.
> > > > +	 */
> > > > +	if (hsfreq > 445500) {
> > > > +		clkkpt = 12;
> > > > +		clkbfht = 15;
> > > > +		clkstpt = 48;
> > > > +		golpbkt = 75;
> > > > +	} else if (hsfreq > 250000) {
> > > > +		clkkpt = 7;
> > > > +		clkbfht = 8;
> > > > +		clkstpt = 27;
> > > > +		golpbkt = 40;
> > > > +	} else {
> > > > +		clkkpt = 8;
> > > > +		clkbfht = 6;
> > > > +		clkstpt = 24;
> > > > +		golpbkt = 29;
> > > > +	}
> > > > +
> > > > +	clstptsetr = CLSTPTSETR_CLKKPT(clkkpt) |
> > > CLSTPTSETR_CLKBFHT(clkbfht) |
> > > > +		     CLSTPTSETR_CLKSTPT(clkstpt);
> > > > +	rzg2l_mipi_dsi_link_write(dsi, CLSTPTSETR, clstptsetr);
> > > > +
> > > > +	lptrnstsetr = LPTRNSTSETR_GOLPBKT(golpbkt);
> > > > +	rzg2l_mipi_dsi_link_write(dsi, LPTRNSTSETR, lptrnstsetr);
> > > > +
> > > > +	return 0;
> > > > +
> > > > +err_rstc:
> > > > +	pm_runtime_put(dsi->dev);
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static void rzg2l_mipi_dsi_set_display_timing(struct
> > > > +rzg2l_mipi_dsi
> > > *dsi,
> > > > +					      const struct drm_display_mode
> *mode)
> > > {
> > > > +	u32 vich1ppsetr;
> > > > +	u32 vich1vssetr;
> > > > +	u32 vich1vpsetr;
> > > > +	u32 vich1hssetr;
> > > > +	u32 vich1hpsetr;
> > > > +	int dsi_format;
> > > > +	u32 delay[2];
> > > > +	u8 index;
> > > > +
> > > > +	/* Configuration for Pixel Packet */
> > > > +	dsi_format = mipi_dsi_pixel_format_to_bpp(dsi->format);
> > > > +	switch (dsi_format) {
> > > > +	case 24:
> > > > +		vich1ppsetr = VICH1PPSETR_DT_RGB24;
> > > > +		break;
> > > > +	case 18:
> > > > +		vich1ppsetr = VICH1PPSETR_DT_RGB18;
> > > > +		break;
> > > > +	}
> > > > +
> > > > +	if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) &&
> > > > +	    !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST))
> > > > +		vich1ppsetr |= VICH1PPSETR_TXESYNC_PULSE;
> > > > +
> > > > +	rzg2l_mipi_dsi_link_write(dsi, VICH1PPSETR, vich1ppsetr);
> > > > +
> > > > +	/* Configuration for Video Parameters */
> > > > +	vich1vssetr = VICH1VSSETR_VACTIVE(mode->vdisplay) |
> > > > +		      VICH1VSSETR_VSA(mode->vsync_end - mode-
> >vsync_start);
> > > > +	vich1vssetr |= (mode->flags & DRM_MODE_FLAG_PVSYNC) ?
> > > > +			VICH1VSSETR_VSPOL_HIGH : VICH1VSSETR_VSPOL_LOW;
> > > > +
> > > > +	vich1vpsetr = VICH1VPSETR_VFP(mode->vsync_start -
> > > > +mode->vdisplay)
> > > |
> > > > +		      VICH1VPSETR_VBP(mode->vtotal - mode->vsync_end);
> > > > +
> > > > +	vich1hssetr = VICH1HSSETR_HACTIVE(mode->hdisplay) |
> > > > +		      VICH1HSSETR_HSA(mode->hsync_end - mode-
> >hsync_start);
> > > > +	vich1hssetr |= (mode->flags & DRM_MODE_FLAG_PHSYNC) ?
> > > > +			VICH1HSSETR_HSPOL_HIGH : VICH1HSSETR_HSPOL_LOW;
> > > > +
> > > > +	vich1hpsetr = VICH1HPSETR_HFP(mode->hsync_start -
> > > > +mode->hdisplay)
> > > |
> > > > +		      VICH1HPSETR_HBP(mode->htotal - mode->hsync_end);
> > > > +
> > > > +	rzg2l_mipi_dsi_link_write(dsi, VICH1VSSETR, vich1vssetr);
> > > > +	rzg2l_mipi_dsi_link_write(dsi, VICH1VPSETR, vich1vpsetr);
> > > > +	rzg2l_mipi_dsi_link_write(dsi, VICH1HSSETR, vich1hssetr);
> > > > +	rzg2l_mipi_dsi_link_write(dsi, VICH1HPSETR, vich1hpsetr);
> > > > +
> > > > +	/*
> > > > +	 * Configuration for Delay Value
> > > > +	 * Delay value based on 2 ranges of video clock.
> > > > +	 * 74.25MHz is videoclock of HD@60p or FHD@30p
> > > > +	 */
> > > > +	if (mode->clock > 74250) {
> > > > +		delay[0] = 231;
> > > > +		delay[1] = 216;
> > > > +	} else {
> > > > +		delay[0] = 220;
> > > > +		delay[1] = 212;
> > > > +	}
> > > > +
> > > > +	if (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS)
> > > > +		index = 0;
> > > > +	else
> > > > +		index = 1;
> > > > +
> > > > +	rzg2l_mipi_dsi_link_write(dsi, VICH1SET1R,
> > > > +				  VICH1SET1R_DLY(delay[index])); }
> > > > +
> > > > +static int rzg2l_mipi_dsi_start_hs_clock(struct rzg2l_mipi_dsi
> > > > +*dsi) {
> > > > +	bool is_clk_cont;
> > > > +	u32 hsclksetr;
> > > > +	u32 status;
> > > > +	int ret;
> > > > +
> > > > +	is_clk_cont = !(dsi->mode_flags &
> > > > +MIPI_DSI_CLOCK_NON_CONTINUOUS);
> > > > +
> > > > +	/* Start HS clock */
> > > > +	hsclksetr = HSCLKSETR_HSCLKRUN_HS | (is_clk_cont ?
> > > > +					     HSCLKSETR_HSCLKMODE_CONT :
> > > > +					     HSCLKSETR_HSCLKMODE_NON_CONT);
> > > > +	rzg2l_mipi_dsi_link_write(dsi, HSCLKSETR, hsclksetr);
> > > > +
> > > > +	if (is_clk_cont) {
> > > > +		ret = read_poll_timeout(rzg2l_mipi_dsi_link_read,
> status,
> > > > +					status & PLSR_CLLP2HS,
> > > > +					2000, 20000, false, dsi, PLSR);
> > > > +		if (ret < 0) {
> > > > +			dev_err(dsi->dev, "failed to start HS clock\n");
> > > > +			return ret;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	dev_dbg(dsi->dev, "Start High Speed Clock with %s clock
> mode",
> > > > +		is_clk_cont ? "continuous" : "non-continuous");
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int rzg2l_mipi_dsi_stop_hs_clock(struct rzg2l_mipi_dsi
> *dsi) {
> > > > +	bool is_clk_cont;
> > > > +	u32 status;
> > > > +	int ret;
> > > > +
> > > > +	is_clk_cont = !(dsi->mode_flags &
> > > > +MIPI_DSI_CLOCK_NON_CONTINUOUS);
> > > > +
> > > > +	/* Stop HS clock */
> > > > +	rzg2l_mipi_dsi_link_write(dsi, HSCLKSETR,
> > > > +				  is_clk_cont ? HSCLKSETR_HSCLKMODE_CONT :
> > > > +				  HSCLKSETR_HSCLKMODE_NON_CONT);
> > > > +
> > > > +	if (is_clk_cont) {
> > > > +		ret = read_poll_timeout(rzg2l_mipi_dsi_link_read,
> status,
> > > > +					status & PLSR_CLHS2LP,
> > > > +					2000, 20000, false, dsi, PLSR);
> > > > +		if (ret < 0) {
> > > > +			dev_err(dsi->dev, "failed to stop HS clock\n");
> > > > +			return ret;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int rzg2l_mipi_dsi_start_video(struct rzg2l_mipi_dsi *dsi)
> {
> > > > +	u32 vich1set0r;
> > > > +	u32 status;
> > > > +	int ret;
> > > > +
> > > > +	/* Configuration for Blanking sequence and start video
> input*/
> > > > +	vich1set0r = VICH1SET0R_HFPNOLP | VICH1SET0R_HBPNOLP |
> > > > +		     VICH1SET0R_HSANOLP | VICH1SET0R_VSTART;
> > > > +	rzg2l_mipi_dsi_link_write(dsi, VICH1SET0R, vich1set0r);
> > > > +
> > > > +	ret = read_poll_timeout(rzg2l_mipi_dsi_link_read, status,
> > > > +				status & VICH1SR_VIRDY,
> > > > +				2000, 20000, false, dsi, VICH1SR);
> > > > +	if (ret < 0)
> > > > +		dev_err(dsi->dev, "Failed to start video signal
> input\n");
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static int rzg2l_mipi_dsi_stop_video(struct rzg2l_mipi_dsi *dsi)
> {
> > > > +	u32 status;
> > > > +	int ret;
> > > > +
> > > > +	rzg2l_mipi_dsi_link_write(dsi, VICH1SET0R,
> VICH1SET0R_VSTPAFT);
> > > > +	ret = read_poll_timeout(rzg2l_mipi_dsi_link_read, status,
> > > > +				(status & VICH1SR_STOP) && (!(status &
> > > VICH1SR_RUNNING)),
> > > > +				2000, 20000, false, dsi, VICH1SR);
> > > > +	if (ret < 0)
> > > > +		goto err;
> > > > +
> > > > +	ret = read_poll_timeout(rzg2l_mipi_dsi_link_read, status,
> > > > +				!(status & LINKSR_HSBUSY),
> > > > +				2000, 20000, false, dsi, LINKSR);
> > > > +	if (ret < 0)
> > > > +		goto err;
> > > > +
> > > > +	return 0;
> > > > +
> > > > +err:
> > > > +	dev_err(dsi->dev, "Failed to stop video signal input\n");
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +/*
> > > > +-----------------------------------------------------------------
> > > > +----
> > > > +--------
> > > > + * Bridge
> > > > + */
> > > > +
> > > > +static int rzg2l_mipi_dsi_attach(struct drm_bridge *bridge,
> > > > +				 enum drm_bridge_attach_flags flags) {
> > > > +	struct rzg2l_mipi_dsi *dsi =
> bridge_to_rzg2l_mipi_dsi(bridge);
> > > > +
> > > > +	return drm_bridge_attach(bridge->encoder, dsi->next_bridge,
> bridge,
> > > > +				 flags);
> > > > +}
> > > > +
> > > > +static void rzg2l_mipi_dsi_atomic_enable(struct drm_bridge
> *bridge,
> > > > +					 struct drm_bridge_state
> *old_bridge_state) {
> > > > +	struct drm_atomic_state *state = old_bridge_state-
> >base.state;
> > > > +	struct rzg2l_mipi_dsi *dsi =
> bridge_to_rzg2l_mipi_dsi(bridge);
> > > > +	const struct drm_display_mode *mode;
> > > > +	struct drm_connector *connector;
> > > > +	struct drm_crtc *crtc;
> > > > +	int ret;
> > > > +
> > > > +	connector = drm_atomic_get_new_connector_for_encoder(state,
> bridge->encoder);
> > > > +	crtc = drm_atomic_get_new_connector_state(state, connector)-
> >crtc;
> > > > +	mode = &drm_atomic_get_new_crtc_state(state,
> > > > +crtc)->adjusted_mode;
> > > > +
> > > > +	ret = rzg2l_mipi_dsi_startup(dsi, mode);
> > > > +	if (ret < 0)
> > > > +		return;
> > > > +
> > > > +	rzg2l_mipi_dsi_set_display_timing(dsi, mode);
> > > > +
> > > > +	ret = rzg2l_mipi_dsi_start_hs_clock(dsi);
> > > > +	if (ret < 0)
> > > > +		goto err_hs_clock;
> > > > +
> > > > +	ret = rzg2l_mipi_dsi_start_video(dsi);
> > > > +	if (ret < 0)
> > > > +		goto err_video;
> > > > +
> > > > +	return;
> > > > +
> > > > +err_video:
> > > > +	rzg2l_mipi_dsi_stop_hs_clock(dsi);
> > > > +err_hs_clock:
> > > > +	reset_control_assert(dsi->rstc);
> > > > +	pm_runtime_put(dsi->dev);
> > >
> > > A function that wraps those two calls would be nice, you could call
> > > it here and in rzg2l_mipi_dsi_atomic_disable(). It would be the
> > > counterpart of rzg2l_mipi_dsi_startup(), the same way you have
> > > rzg2l_mipi_dsi_start_hs_clock() vs. rzg2l_mipi_dsi_stop_hs_clock()
> > > and
> > > rzg2l_mipi_dsi_stop_video() vs. rzg2l_mipi_dsi_start_video().
> >
> > OK, will wrap these calls into a function
> > rzg2l_mipi_dsi_assert_rst_and_stop_clocks() and
> > is called from rzg2l_mipi_dsi_atomic_{enable,disable} to
> > assert cmn_rst followed by stopping dsi clocks.
> 
> Could you maybe call it rzg2l_mipi_dsi_stop() to make it clear it's the
> counterpart of rzg2l_mipi_dsi_startup() ?

Sure. Now I got it.

Cheers,
Biju




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux