Re: [PATCH v4 06/13] drm/bridge: Add Synopsys DesignWare MIPI DSI host controller driver

Chris Zhong
2015-11-26T07:04:11Z

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@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