From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: Laurent Pinchart To: Magnus Damm Cc: Geert Uytterhoeven , DRI Development , Linux-Renesas , David Airlie , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] drm: rcar-du: Fix R-Car Gen3 crash when VSP is disabled Date: Tue, 06 Dec 2016 16:07:02 +0200 Message-ID: <6891266.RiLSMIFh5n@avalon> In-Reply-To: References: <20161026033143.6617.5834.sendpatchset@little-apple> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: Hello, On Wednesday 26 Oct 2016 18:13:23 Magnus Damm wrote: > On Wed, Oct 26, 2016 at 4:31 PM, Geert Uytterhoeven wrote: > > On Wed, Oct 26, 2016 at 5:31 AM, Magnus Damm wrote: > >> From: Magnus Damm > >> > >> For the DU to operate on R-Car Gen3 hardware a combination of DU > >> and VSP devices are required. Since the DU driver also supports > >> earlier generations hardware the VSP portion is enabled via Kconfig. > >> > >> The arm64 defconfig is as of v4.9-rc1 having the DU driver enabled > >> as a module, however this is not enough to support R-Car Gen3. In > >> the current case of CONFIG_DRM_RCAR_VSP=n then the kernel crashes > >> when loading the module. This patch is fixing that particular case. > >> > >> In more detail, the crash triggers in drm_atomic_get_plane_state() > >> when __drm_atomic_helper_set_config() passes NULL as crtc->primary. > >> > >> This patch corrects this issue by failing to load the DU driver on > >> R-Car Gen3 when VSP is not available. > >> > >> Signed-off-by: Magnus Damm > >> --- > >> > >> drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> --- 0001/drivers/gpu/drm/rcar-du/rcar_du_vsp.h > >> +++ work/drivers/gpu/drm/rcar-du/rcar_du_vsp.h 2016-10-26 > >> 00:01:12.920607110 +0900 @@ -70,7 +70,7 @@ void > >> rcar_du_vsp_disable(struct rcar_du_ > >> > >> void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc); > >> void rcar_du_vsp_atomic_flush(struct rcar_du_crtc *crtc); > >> #else > >> > >> -static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return 0; > >> }; > >> +static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp) { return > >> -ENXIO; }; With this patch applied the DU will fail to probe on Gen2 if DRM_RCAR_VSP is disabled. DRM_RCAR_DU should instead depend on VIDEO_RENESAS_VSP1 (with DRM_RCAR_VSP always set) on Gen3. > > -ENODEV sounds more appropriate > > Ok, however -ENXIO is the same error code as the DU driver currently > returns when dealing with the VSP and not finding DT nodes. > > To avoid dealing with this again I would prefer skipping per-driver > Kconfig options entirely, for instance something like this: > > [PATCH/RFC] Simplify Gen3 DU and VSP Kconfig bits > https://www.spinics.net/lists/linux-sh/msg45978.html > > >> static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { }; > >> static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { }; > >> static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) { > >> }; > > > > Alternatively, DRM_RCAR_DU can force DRM_RCAR_VSP to y if ARCH_R8A7795 > > or ARCH_R8A7796 is enabled. > > The DU driver has symbol dependencies on VSP and FCP driver code > (V4L2), so we need to take the state of driver support for those > modules into consideration as well instead of only depending on > SoC-specific Kconfig symbols. > > On Wed, Oct 26, 2016 at 4:31 PM, Geert Uytterhoeven wrote: > > On Wed, Oct 26, 2016 at 5:31 AM, Magnus Damm wrote: > >> The arm64 defconfig is as of v4.9-rc1 having the DU driver enabled > >> as a module, however this is not enough to support R-Car Gen3. In > > > > Nope, arm64 defconfig on v4.9-rc1: > > # CONFIG_DRM_RCAR_DU is not set > > Right, you are correct. Sorry for the noise. It seems that I was using > renesas-drivers-2016-10-18-v4.9-rc1 - not mainline v4.9-rc1. > > Without this fix the driver still crashes, but not due to defconfig in > v4.9-rc1. -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH] drm: rcar-du: Fix R-Car Gen3 crash when VSP is disabled Date: Tue, 06 Dec 2016 16:07:02 +0200 Message-ID: <6891266.RiLSMIFh5n@avalon> References: <20161026033143.6617.5834.sendpatchset@little-apple> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [185.26.127.97]) by gabe.freedesktop.org (Postfix) with ESMTPS id 53F7689DFF for ; Tue, 6 Dec 2016 14:06:41 +0000 (UTC) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Magnus Damm Cc: Linux-Renesas , Geert Uytterhoeven , "linux-kernel@vger.kernel.org" , DRI Development List-Id: dri-devel@lists.freedesktop.org SGVsbG8sCgpPbiBXZWRuZXNkYXkgMjYgT2N0IDIwMTYgMTg6MTM6MjMgTWFnbnVzIERhbW0gd3Jv dGU6Cj4gT24gV2VkLCBPY3QgMjYsIDIwMTYgYXQgNDozMSBQTSwgR2VlcnQgVXl0dGVyaG9ldmVu IHdyb3RlOgo+ID4gT24gV2VkLCBPY3QgMjYsIDIwMTYgYXQgNTozMSBBTSwgTWFnbnVzIERhbW0g d3JvdGU6Cj4gPj4gRnJvbTogTWFnbnVzIERhbW0gPGRhbW0rcmVuZXNhc0BvcGVuc291cmNlLnNl Pgo+ID4+IAo+ID4+IEZvciB0aGUgRFUgdG8gb3BlcmF0ZSBvbiBSLUNhciBHZW4zIGhhcmR3YXJl IGEgY29tYmluYXRpb24gb2YgRFUKPiA+PiBhbmQgVlNQIGRldmljZXMgYXJlIHJlcXVpcmVkLiBT aW5jZSB0aGUgRFUgZHJpdmVyIGFsc28gc3VwcG9ydHMKPiA+PiBlYXJsaWVyIGdlbmVyYXRpb25z IGhhcmR3YXJlIHRoZSBWU1AgcG9ydGlvbiBpcyBlbmFibGVkIHZpYSBLY29uZmlnLgo+ID4+IAo+ ID4+IFRoZSBhcm02NCBkZWZjb25maWcgaXMgYXMgb2YgdjQuOS1yYzEgaGF2aW5nIHRoZSBEVSBk cml2ZXIgZW5hYmxlZAo+ID4+IGFzIGEgbW9kdWxlLCBob3dldmVyIHRoaXMgaXMgbm90IGVub3Vn aCB0byBzdXBwb3J0IFItQ2FyIEdlbjMuIEluCj4gPj4gdGhlIGN1cnJlbnQgY2FzZSBvZiBDT05G SUdfRFJNX1JDQVJfVlNQPW4gdGhlbiB0aGUga2VybmVsIGNyYXNoZXMKPiA+PiB3aGVuIGxvYWRp bmcgdGhlIG1vZHVsZS4gVGhpcyBwYXRjaCBpcyBmaXhpbmcgdGhhdCBwYXJ0aWN1bGFyIGNhc2Uu Cj4gPj4gCj4gPj4gSW4gbW9yZSBkZXRhaWwsIHRoZSBjcmFzaCB0cmlnZ2VycyBpbiBkcm1fYXRv bWljX2dldF9wbGFuZV9zdGF0ZSgpCj4gPj4gd2hlbiBfX2RybV9hdG9taWNfaGVscGVyX3NldF9j b25maWcoKSBwYXNzZXMgTlVMTCBhcyBjcnRjLT5wcmltYXJ5Lgo+ID4+IAo+ID4+IFRoaXMgcGF0 Y2ggY29ycmVjdHMgdGhpcyBpc3N1ZSBieSBmYWlsaW5nIHRvIGxvYWQgdGhlIERVIGRyaXZlciBv bgo+ID4+IFItQ2FyIEdlbjMgd2hlbiBWU1AgaXMgbm90IGF2YWlsYWJsZS4KPiA+PiAKPiA+PiBT aWduZWQtb2ZmLWJ5OiBNYWdudXMgRGFtbSA8ZGFtbStyZW5lc2FzQG9wZW5zb3VyY2Uuc2U+Cj4g Pj4gLS0tCj4gPj4gCj4gPj4gIGRyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVfdnNwLmgg fCAgICAyICstCj4gPj4gIDEgZmlsZSBjaGFuZ2VkLCAxIGluc2VydGlvbigrKSwgMSBkZWxldGlv bigtKQo+ID4+IAo+ID4+IC0tLSAwMDAxL2RyaXZlcnMvZ3B1L2RybS9yY2FyLWR1L3JjYXJfZHVf dnNwLmgKPiA+PiArKysgd29yay9kcml2ZXJzL2dwdS9kcm0vcmNhci1kdS9yY2FyX2R1X3ZzcC5o ICAyMDE2LTEwLTI2Cj4gPj4gMDA6MDE6MTIuOTIwNjA3MTEwICswOTAwIEBAIC03MCw3ICs3MCw3 IEBAIHZvaWQKPiA+PiByY2FyX2R1X3ZzcF9kaXNhYmxlKHN0cnVjdCByY2FyX2R1Xwo+ID4+IAo+ ID4+ICB2b2lkIHJjYXJfZHVfdnNwX2F0b21pY19iZWdpbihzdHJ1Y3QgcmNhcl9kdV9jcnRjICpj cnRjKTsKPiA+PiAgdm9pZCByY2FyX2R1X3ZzcF9hdG9taWNfZmx1c2goc3RydWN0IHJjYXJfZHVf Y3J0YyAqY3J0Yyk7Cj4gPj4gICNlbHNlCj4gPj4gCj4gPj4gLXN0YXRpYyBpbmxpbmUgaW50IHJj YXJfZHVfdnNwX2luaXQoc3RydWN0IHJjYXJfZHVfdnNwICp2c3ApIHsgcmV0dXJuIDA7Cj4gPj4g fTsKPiA+PiArc3RhdGljIGlubGluZSBpbnQgcmNhcl9kdV92c3BfaW5pdChzdHJ1Y3QgcmNhcl9k dV92c3AgKnZzcCkgeyByZXR1cm4KPiA+PiAtRU5YSU87IH07CgpXaXRoIHRoaXMgcGF0Y2ggYXBw bGllZCB0aGUgRFUgd2lsbCBmYWlsIHRvIHByb2JlIG9uIEdlbjIgaWYgRFJNX1JDQVJfVlNQIGlz IApkaXNhYmxlZC4KCkRSTV9SQ0FSX0RVIHNob3VsZCBpbnN0ZWFkIGRlcGVuZCBvbiBWSURFT19S RU5FU0FTX1ZTUDEgKHdpdGggRFJNX1JDQVJfVlNQIAphbHdheXMgc2V0KSBvbiBHZW4zLgoKPiA+ IC1FTk9ERVYgc291bmRzIG1vcmUgYXBwcm9wcmlhdGUKPiAKPiBPaywgaG93ZXZlciAtRU5YSU8g aXMgdGhlIHNhbWUgZXJyb3IgY29kZSBhcyB0aGUgRFUgZHJpdmVyIGN1cnJlbnRseQo+IHJldHVy bnMgd2hlbiBkZWFsaW5nIHdpdGggdGhlIFZTUCBhbmQgbm90IGZpbmRpbmcgRFQgbm9kZXMuCj4g Cj4gVG8gYXZvaWQgZGVhbGluZyB3aXRoIHRoaXMgYWdhaW4gSSB3b3VsZCBwcmVmZXIgc2tpcHBp bmcgcGVyLWRyaXZlcgo+IEtjb25maWcgb3B0aW9ucyBlbnRpcmVseSwgZm9yIGluc3RhbmNlIHNv bWV0aGluZyBsaWtlIHRoaXM6Cj4gCj4gW1BBVENIL1JGQ10gU2ltcGxpZnkgR2VuMyBEVSBhbmQg VlNQIEtjb25maWcgYml0cwo+IGh0dHBzOi8vd3d3LnNwaW5pY3MubmV0L2xpc3RzL2xpbnV4LXNo L21zZzQ1OTc4Lmh0bWwKPiAKPiA+PiAgc3RhdGljIGlubGluZSB2b2lkIHJjYXJfZHVfdnNwX2Vu YWJsZShzdHJ1Y3QgcmNhcl9kdV9jcnRjICpjcnRjKSB7IH07Cj4gPj4gIHN0YXRpYyBpbmxpbmUg dm9pZCByY2FyX2R1X3ZzcF9kaXNhYmxlKHN0cnVjdCByY2FyX2R1X2NydGMgKmNydGMpIHsgfTsK PiA+PiAgc3RhdGljIGlubGluZSB2b2lkIHJjYXJfZHVfdnNwX2F0b21pY19iZWdpbihzdHJ1Y3Qg cmNhcl9kdV9jcnRjICpjcnRjKSB7Cj4gPj4gIH07Cj4gPiAKPiA+IEFsdGVybmF0aXZlbHksICBE Uk1fUkNBUl9EVSBjYW4gZm9yY2UgRFJNX1JDQVJfVlNQIHRvIHkgaWYgQVJDSF9SOEE3Nzk1Cj4g PiBvciBBUkNIX1I4QTc3OTYgaXMgZW5hYmxlZC4KPiAKPiBUaGUgRFUgZHJpdmVyIGhhcyBzeW1i b2wgZGVwZW5kZW5jaWVzIG9uIFZTUCBhbmQgRkNQIGRyaXZlciBjb2RlCj4gKFY0TDIpLCBzbyB3 ZSBuZWVkIHRvIHRha2UgdGhlIHN0YXRlIG9mIGRyaXZlciBzdXBwb3J0IGZvciB0aG9zZQo+IG1v ZHVsZXMgaW50byBjb25zaWRlcmF0aW9uIGFzIHdlbGwgaW5zdGVhZCBvZiBvbmx5IGRlcGVuZGlu ZyBvbgo+IFNvQy1zcGVjaWZpYyBLY29uZmlnIHN5bWJvbHMuCj4gCj4gT24gV2VkLCBPY3QgMjYs IDIwMTYgYXQgNDozMSBQTSwgR2VlcnQgVXl0dGVyaG9ldmVuIHdyb3RlOgo+ID4gT24gV2VkLCBP Y3QgMjYsIDIwMTYgYXQgNTozMSBBTSwgTWFnbnVzIERhbW0gd3JvdGU6Cj4gPj4gVGhlIGFybTY0 IGRlZmNvbmZpZyBpcyBhcyBvZiB2NC45LXJjMSBoYXZpbmcgdGhlIERVIGRyaXZlciBlbmFibGVk Cj4gPj4gYXMgYSBtb2R1bGUsIGhvd2V2ZXIgdGhpcyBpcyBub3QgZW5vdWdoIHRvIHN1cHBvcnQg Ui1DYXIgR2VuMy4gSW4KPiA+IAo+ID4gTm9wZSwgYXJtNjQgZGVmY29uZmlnIG9uIHY0LjktcmMx Ogo+ID4gIyBDT05GSUdfRFJNX1JDQVJfRFUgaXMgbm90IHNldAo+IAo+IFJpZ2h0LCB5b3UgYXJl IGNvcnJlY3QuIFNvcnJ5IGZvciB0aGUgbm9pc2UuIEl0IHNlZW1zIHRoYXQgSSB3YXMgdXNpbmcK PiByZW5lc2FzLWRyaXZlcnMtMjAxNi0xMC0xOC12NC45LXJjMSAtIG5vdCBtYWlubGluZSB2NC45 LXJjMS4KPiAKPiBXaXRob3V0IHRoaXMgZml4IHRoZSBkcml2ZXIgc3RpbGwgY3Jhc2hlcywgYnV0 IG5vdCBkdWUgdG8gZGVmY29uZmlnIGluCj4gdjQuOS1yYzEuCgotLSAKUmVnYXJkcywKCkxhdXJl bnQgUGluY2hhcnQKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9y ZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZl bAo=