Subject: Re: [PATCH v4 06/13] drm/bridge: Add Synopsys DesignWare MIPI DSI
 host controller driver
To: Thierry Reding <treding@nvidia.com>
References: <1448007339-10966-1-git-send-email-zyw@rock-chips.com>
 <1448007339-10966-7-git-send-email-zyw@rock-chips.com>
 <20151120160706.GE3300@ulmo.nvidia.com> <5656AEDA.4080607@rock-chips.com>
 <20151126080447.GA28064@ulmo.nvidia.com>
Cc: heiko@sntech.de, linux-rockchip@lists.infradead.org,
        mark.yao@rock-chips.com, emil.l.velikov@gmail.com, airlied@linux.ie,
        ajaykumar.rs@samsung.com, rmk+kernel@arm.linux.org.uk,
        dri-devel@lists.freedesktop.org, Inki Dae <inki.dae@samsung.com>,
        Takashi Iwai <tiwai@suse.de>, Vincent Palatin <vpalatin@chromium.org>,
        Andy Yan <andy.yan@rock-chips.com>,
        Rahul Sharma <rahul.sharma@samsung.com>,
        Sean Paul <seanpaul@chromium.org>,
        Andrew Bresticker <abrestic@chromium.org>,
        linux-kernel@vger.kernel.org, architt@codeaurora.org
From: Chris Zhong <zyw@rock-chips.com>
Message-ID: <5656D983.8090203@rock-chips.com>
Date: Thu, 26 Nov 2015 18:05:55 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.3.0
MIME-Version: 1.0
In-Reply-To: <20151126080447.GA28064@ulmo.nvidia.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2698
Lines: 75


On 11/26/2015 04:04 PM, Thierry Reding wrote:
> On Thu, Nov 26, 2015 at 03:03:54PM +0800, Chris Zhong wrote:
>> 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.
> I don't understand. If you don't make bridge a pointer, but rather embed
> it within the structure, then you can use container_of(), can't you? You
> get it allocated automatically as part of allocating the dw_mipi_dsi
> structure. See for example the ps8622 and ptn3460 bridge drivers.
Yeah, got it, thanks. :)
I am going to modify it in next version patch.
>
> On a slightly unrelated note, I'm not sure if we discussed this already
> or if it was in another thread, but why is this even a bridge driver? A
> bridge is typically some IC outside of the SoC, whereas this clearly is
> IP designed into the SoC. So it's really more of an encoder rather than
> a bridge.
I guess you mean this thread <https://patchwork.kernel.org/patch/5816991/>
I think the bridge is beneficial to share the dw-mipi driver for 
different soc
with same version dw-mipi IP.

>
> Thierry


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/