Hi Thierry Thanks for your feedback. On 11/21/2015 12:07 AM, Thierry Reding wrote: > On Fri, Nov 20, 2015 at 04:15:32PM +0800, Chris Zhong wrote: >> add Synopsys DesignWare MIPI DSI host controller driver support. >> >> Signed-off-by: Chris Zhong <zyw at rock-chips.com> >> --- >> >> Changes in v4: >> eliminate some warnning >> >> Changes in v3: None >> Changes in v2: None >> >> drivers/gpu/drm/bridge/Kconfig | 11 + >> drivers/gpu/drm/bridge/Makefile | 1 + >> drivers/gpu/drm/bridge/dw_mipi_dsi.c | 1056 ++++++++++++++++++++++++++++++++++ >> include/drm/bridge/dw_mipi_dsi.h | 27 + >> 4 files changed, 1095 insertions(+) >> create mode 100644 drivers/gpu/drm/bridge/dw_mipi_dsi.c >> create mode 100644 include/drm/bridge/dw_mipi_dsi.h >> >> >> +struct dw_mipi_dsi { >> + struct mipi_dsi_host dsi_host; >> + struct drm_connector connector; >> + struct drm_encoder *encoder; > struct drm_bridge already has a pointer to an encoder, can't you reuse > that instead? > >> + struct drm_bridge *bridge; > Typically you'd embed the bridge into the driver structure. I'm almost done with all the modifications according to your comments. Remaining this point I can not understand, the dsi is a pointer of drm_bridge, hence I can not get the bridge by container_of. Although, the "bridge->driver_private = dsi" have done in dw_mipi_dsi_register. > >> + struct drm_panel *panel; >> + struct device *dev; >> + >> + void __iomem *base; >> + >> + struct clk *pllref_clk; >> + struct clk *cfg_clk; >> + struct clk *pclk; >> + >> + unsigned int lane_mbps; /* per lane */ >> + u32 channel; >> + u32 lanes; >> + u32 format; >> + u16 input_div; >> + u16 feedback_div; >> + struct drm_display_mode *mode; >> + >> + const struct dw_mipi_dsi_plat_data *pdata; >> + >> + bool enabled; >> +}; >> + >> >> >> Thierry