From mboxrd@z Thu Jan 1 00:00:00 1970 From: heiko@sntech.de (Heiko =?ISO-8859-1?Q?St=FCbner?=) Date: Wed, 22 Apr 2015 14:13:53 +0200 Subject: [PATCH RFC v9 11/20] drm/bridge: Add Synopsys DesignWare MIPI DSI host controller driver In-Reply-To: <552F4B2E.2000209@codeaurora.org> References: <1423720903-24806-1-git-send-email-Ying.Liu@freescale.com> <20150409084353.GD12103@ulmo> <552F4B2E.2000209@codeaurora.org> Message-ID: <54991164.nrScLQEecU@diego> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Am Donnerstag, 16. April 2015, 11:09:58 schrieb Archit Taneja: > On 04/09/2015 02:13 PM, Thierry Reding wrote: > > On Thu, Feb 12, 2015 at 02:01:34PM +0800, Liu Ying wrote: > > [...] > > > >> diff --git a/drivers/gpu/drm/bridge/dw_mipi_dsi.c > >> b/drivers/gpu/drm/bridge/dw_mipi_dsi.c> > > [...] > > > >> +struct dw_mipi_dsi { > >> + struct mipi_dsi_host dsi_host; > >> + struct drm_connector connector; > >> + struct drm_encoder *encoder; > >> + struct drm_bridge *bridge; > >> + 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; > >> + struct drm_display_mode *mode; > >> + > >> + const struct dw_mipi_dsi_plat_data *pdata; > >> + > >> + bool enabled; > >> +}; > > > > While reviewing this I kept thinking whether this is really the right > > architectural design. This driver is a MIPI DSI host, a connector and > > a bridge, all in one. But it seems to me like it should really be an > > encoder/connector and a MIPI DSI host. Why the need for a bridge? The > > bridge abstraction targets blocks outside of the SoC, but it is my > > understanding that these DesignWare IP blocks are designed into SoCs. > > The msm driver uses bridges for blocks within the SoC too. We have too > many sub blocks in the display controller that use up crtcs and encoder > entities. A bridge is the only option one has if an encoder in the > display chain is already taken. > > In the above designware configuration, if some one created a board that > used an external chip to further convert DSI to some other output > format, then we would be completely exhausted of all entities. > > I posted a patch that allows us to create a chain of bridges for such > cases. It seems to work well as an interim solution. Ideally, it would > be better if we could make bridge a special case of an encoder, and let > one encoder connect to another encoder. > > Such a thing would also help us unify i2c slave encoders and bridge > drivers too. A chip designed as an i2c slave encoder would work well > with a drm driver that doesn't have an encoder, but won't work for SoCs > SoCs that already have an encoder and were expecting a bridge entity > instead. Yeah, having encoder-after-encoder chains would be great. I have the same issue on Rockchip where on the rk3288 the lvds-IP hogs the lcd-pins of the soc which are used both for panels as well as external encoders, while on the older Rockchip SoCs these pins are used by external encoders directly. So in my current (pending review) patchset the lvds acts as encoder for panels and as bridge if external encoders are in use. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko =?ISO-8859-1?Q?St=FCbner?= Subject: Re: [PATCH RFC v9 11/20] drm/bridge: Add Synopsys DesignWare MIPI DSI host controller driver Date: Wed, 22 Apr 2015 14:13:53 +0200 Message-ID: <54991164.nrScLQEecU@diego> References: <1423720903-24806-1-git-send-email-Ying.Liu@freescale.com> <20150409084353.GD12103@ulmo> <552F4B2E.2000209@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <552F4B2E.2000209@codeaurora.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: dri-devel@lists.freedesktop.org Cc: stefan.wahren@i2se.com, devicetree@vger.kernel.org, linux@arm.linux.org.uk, sboyd@codeaurora.org, linux-kernel@vger.kernel.org, a.hajda@samsung.com, kernel@pengutronix.de, andy.yan@rock-chips.com, mturquette@linaro.org, linux-arm-kernel@lists.infradead.org List-Id: devicetree@vger.kernel.org QW0gRG9ubmVyc3RhZywgMTYuIEFwcmlsIDIwMTUsIDExOjA5OjU4IHNjaHJpZWIgQXJjaGl0IFRh bmVqYToKPiBPbiAwNC8wOS8yMDE1IDAyOjEzIFBNLCBUaGllcnJ5IFJlZGluZyB3cm90ZToKPiA+ IE9uIFRodSwgRmViIDEyLCAyMDE1IGF0IDAyOjAxOjM0UE0gKzA4MDAsIExpdSBZaW5nIHdyb3Rl Ogo+ID4gWy4uLl0KPiA+IAo+ID4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vYnJpZGdl L2R3X21pcGlfZHNpLmMKPiA+PiBiL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvZHdfbWlwaV9kc2ku Yz4gCj4gPiBbLi4uXQo+ID4gCj4gPj4gK3N0cnVjdCBkd19taXBpX2RzaSB7Cj4gPj4gKwlzdHJ1 Y3QgbWlwaV9kc2lfaG9zdCBkc2lfaG9zdDsKPiA+PiArCXN0cnVjdCBkcm1fY29ubmVjdG9yIGNv bm5lY3RvcjsKPiA+PiArCXN0cnVjdCBkcm1fZW5jb2RlciAqZW5jb2RlcjsKPiA+PiArCXN0cnVj dCBkcm1fYnJpZGdlICpicmlkZ2U7Cj4gPj4gKwlzdHJ1Y3QgZHJtX3BhbmVsICpwYW5lbDsKPiA+ PiArCXN0cnVjdCBkZXZpY2UgKmRldjsKPiA+PiArCj4gPj4gKwl2b2lkIF9faW9tZW0gKmJhc2U7 Cj4gPj4gKwo+ID4+ICsJc3RydWN0IGNsayAqcGxscmVmX2NsazsKPiA+PiArCXN0cnVjdCBjbGsg KmNmZ19jbGs7Cj4gPj4gKwlzdHJ1Y3QgY2xrICpwY2xrOwo+ID4+ICsKPiA+PiArCXVuc2lnbmVk IGludCBsYW5lX21icHM7IC8qIHBlciBsYW5lICovCj4gPj4gKwl1MzIgY2hhbm5lbDsKPiA+PiAr CXUzMiBsYW5lczsKPiA+PiArCXUzMiBmb3JtYXQ7Cj4gPj4gKwlzdHJ1Y3QgZHJtX2Rpc3BsYXlf bW9kZSAqbW9kZTsKPiA+PiArCj4gPj4gKwljb25zdCBzdHJ1Y3QgZHdfbWlwaV9kc2lfcGxhdF9k YXRhICpwZGF0YTsKPiA+PiArCj4gPj4gKwlib29sIGVuYWJsZWQ7Cj4gPj4gK307Cj4gPiAKPiA+ IFdoaWxlIHJldmlld2luZyB0aGlzIEkga2VwdCB0aGlua2luZyB3aGV0aGVyIHRoaXMgaXMgcmVh bGx5IHRoZSByaWdodAo+ID4gYXJjaGl0ZWN0dXJhbCBkZXNpZ24uIFRoaXMgZHJpdmVyIGlzIGEg TUlQSSBEU0kgaG9zdCwgYSBjb25uZWN0b3IgYW5kCj4gPiBhIGJyaWRnZSwgYWxsIGluIG9uZS4g QnV0IGl0IHNlZW1zIHRvIG1lIGxpa2UgaXQgc2hvdWxkIHJlYWxseSBiZSBhbgo+ID4gZW5jb2Rl ci9jb25uZWN0b3IgYW5kIGEgTUlQSSBEU0kgaG9zdC4gV2h5IHRoZSBuZWVkIGZvciBhIGJyaWRn ZT8gVGhlCj4gPiBicmlkZ2UgYWJzdHJhY3Rpb24gdGFyZ2V0cyBibG9ja3Mgb3V0c2lkZSBvZiB0 aGUgU29DLCBidXQgaXQgaXMgbXkKPiA+IHVuZGVyc3RhbmRpbmcgdGhhdCB0aGVzZSBEZXNpZ25X YXJlIElQIGJsb2NrcyBhcmUgZGVzaWduZWQgaW50byBTb0NzLgo+IAo+IFRoZSBtc20gZHJpdmVy IHVzZXMgYnJpZGdlcyBmb3IgYmxvY2tzIHdpdGhpbiB0aGUgU29DIHRvby4gV2UgaGF2ZSB0b28K PiBtYW55IHN1YiBibG9ja3MgaW4gdGhlIGRpc3BsYXkgY29udHJvbGxlciB0aGF0IHVzZSB1cCBj cnRjcyBhbmQgZW5jb2Rlcgo+IGVudGl0aWVzLiBBIGJyaWRnZSBpcyB0aGUgb25seSBvcHRpb24g b25lIGhhcyBpZiBhbiBlbmNvZGVyIGluIHRoZQo+IGRpc3BsYXkgY2hhaW4gaXMgYWxyZWFkeSB0 YWtlbi4KPiAKPiBJbiB0aGUgYWJvdmUgZGVzaWdud2FyZSBjb25maWd1cmF0aW9uLCBpZiBzb21l IG9uZSBjcmVhdGVkIGEgYm9hcmQgdGhhdAo+IHVzZWQgYW4gZXh0ZXJuYWwgY2hpcCB0byBmdXJ0 aGVyIGNvbnZlcnQgRFNJIHRvIHNvbWUgb3RoZXIgb3V0cHV0Cj4gZm9ybWF0LCB0aGVuIHdlIHdv dWxkIGJlIGNvbXBsZXRlbHkgZXhoYXVzdGVkIG9mIGFsbCBlbnRpdGllcy4KPiAKPiBJIHBvc3Rl ZCBhIHBhdGNoIHRoYXQgYWxsb3dzIHVzIHRvIGNyZWF0ZSBhIGNoYWluIG9mIGJyaWRnZXMgZm9y IHN1Y2gKPiBjYXNlcy4gSXQgc2VlbXMgdG8gd29yayB3ZWxsIGFzIGFuIGludGVyaW0gc29sdXRp b24uIElkZWFsbHksIGl0IHdvdWxkCj4gYmUgYmV0dGVyIGlmIHdlIGNvdWxkIG1ha2UgYnJpZGdl IGEgc3BlY2lhbCBjYXNlIG9mIGFuIGVuY29kZXIsIGFuZCBsZXQKPiBvbmUgZW5jb2RlciBjb25u ZWN0IHRvIGFub3RoZXIgZW5jb2Rlci4KPiAKPiBTdWNoIGEgdGhpbmcgd291bGQgYWxzbyBoZWxw IHVzIHVuaWZ5IGkyYyBzbGF2ZSBlbmNvZGVycyBhbmQgYnJpZGdlCj4gZHJpdmVycyB0b28uIEEg Y2hpcCBkZXNpZ25lZCBhcyBhbiBpMmMgc2xhdmUgZW5jb2RlciB3b3VsZCB3b3JrIHdlbGwKPiB3 aXRoIGEgZHJtIGRyaXZlciB0aGF0IGRvZXNuJ3QgaGF2ZSBhbiBlbmNvZGVyLCBidXQgd29uJ3Qg d29yayBmb3IgU29Dcwo+IFNvQ3MgdGhhdCBhbHJlYWR5IGhhdmUgYW4gZW5jb2RlciBhbmQgd2Vy ZSBleHBlY3RpbmcgYSBicmlkZ2UgZW50aXR5Cj4gaW5zdGVhZC4KClllYWgsIGhhdmluZyBlbmNv ZGVyLWFmdGVyLWVuY29kZXIgY2hhaW5zIHdvdWxkIGJlIGdyZWF0LiBJIGhhdmUgdGhlIHNhbWUg Cmlzc3VlIG9uIFJvY2tjaGlwIHdoZXJlIG9uIHRoZSByazMyODggdGhlIGx2ZHMtSVAgaG9ncyB0 aGUgbGNkLXBpbnMgb2YgdGhlIHNvYyAKd2hpY2ggYXJlIHVzZWQgYm90aCBmb3IgcGFuZWxzIGFz IHdlbGwgYXMgZXh0ZXJuYWwgZW5jb2RlcnMsIHdoaWxlIG9uIHRoZSAKb2xkZXIgUm9ja2NoaXAg U29DcyB0aGVzZSBwaW5zIGFyZSB1c2VkIGJ5IGV4dGVybmFsIGVuY29kZXJzIGRpcmVjdGx5LgoK U28gaW4gbXkgY3VycmVudCAocGVuZGluZyByZXZpZXcpIHBhdGNoc2V0IHRoZSBsdmRzIGFjdHMg YXMgZW5jb2RlciBmb3IgcGFuZWxzIAphbmQgYXMgYnJpZGdlIGlmIGV4dGVybmFsIGVuY29kZXJz IGFyZSBpbiB1c2UuCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9y ZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933725AbbDVMOg (ORCPT ); Wed, 22 Apr 2015 08:14:36 -0400 Received: from gloria.sntech.de ([95.129.55.99]:49624 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751405AbbDVMOd (ORCPT ); Wed, 22 Apr 2015 08:14:33 -0400 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: dri-devel@lists.freedesktop.org Cc: Archit Taneja , Thierry Reding , Liu Ying , stefan.wahren@i2se.com, devicetree@vger.kernel.org, linux@arm.linux.org.uk, mturquette@linaro.org, sboyd@codeaurora.org, linux-kernel@vger.kernel.org, a.hajda@samsung.com, kernel@pengutronix.de, andy.yan@rock-chips.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH RFC v9 11/20] drm/bridge: Add Synopsys DesignWare MIPI DSI host controller driver Date: Wed, 22 Apr 2015 14:13:53 +0200 Message-ID: <54991164.nrScLQEecU@diego> User-Agent: KMail/4.14.1 (Linux/3.16.0-4-amd64; KDE/4.14.2; x86_64; ; ) In-Reply-To: <552F4B2E.2000209@codeaurora.org> References: <1423720903-24806-1-git-send-email-Ying.Liu@freescale.com> <20150409084353.GD12103@ulmo> <552F4B2E.2000209@codeaurora.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am Donnerstag, 16. April 2015, 11:09:58 schrieb Archit Taneja: > On 04/09/2015 02:13 PM, Thierry Reding wrote: > > On Thu, Feb 12, 2015 at 02:01:34PM +0800, Liu Ying wrote: > > [...] > > > >> diff --git a/drivers/gpu/drm/bridge/dw_mipi_dsi.c > >> b/drivers/gpu/drm/bridge/dw_mipi_dsi.c> > > [...] > > > >> +struct dw_mipi_dsi { > >> + struct mipi_dsi_host dsi_host; > >> + struct drm_connector connector; > >> + struct drm_encoder *encoder; > >> + struct drm_bridge *bridge; > >> + 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; > >> + struct drm_display_mode *mode; > >> + > >> + const struct dw_mipi_dsi_plat_data *pdata; > >> + > >> + bool enabled; > >> +}; > > > > While reviewing this I kept thinking whether this is really the right > > architectural design. This driver is a MIPI DSI host, a connector and > > a bridge, all in one. But it seems to me like it should really be an > > encoder/connector and a MIPI DSI host. Why the need for a bridge? The > > bridge abstraction targets blocks outside of the SoC, but it is my > > understanding that these DesignWare IP blocks are designed into SoCs. > > The msm driver uses bridges for blocks within the SoC too. We have too > many sub blocks in the display controller that use up crtcs and encoder > entities. A bridge is the only option one has if an encoder in the > display chain is already taken. > > In the above designware configuration, if some one created a board that > used an external chip to further convert DSI to some other output > format, then we would be completely exhausted of all entities. > > I posted a patch that allows us to create a chain of bridges for such > cases. It seems to work well as an interim solution. Ideally, it would > be better if we could make bridge a special case of an encoder, and let > one encoder connect to another encoder. > > Such a thing would also help us unify i2c slave encoders and bridge > drivers too. A chip designed as an i2c slave encoder would work well > with a drm driver that doesn't have an encoder, but won't work for SoCs > SoCs that already have an encoder and were expecting a bridge entity > instead. Yeah, having encoder-after-encoder chains would be great. I have the same issue on Rockchip where on the rk3288 the lvds-IP hogs the lcd-pins of the soc which are used both for panels as well as external encoders, while on the older Rockchip SoCs these pins are used by external encoders directly. So in my current (pending review) patchset the lvds acts as encoder for panels and as bridge if external encoders are in use.