Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753380AbcLFOGo (ORCPT ); Tue, 6 Dec 2016 09:06:44 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:35146 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753069AbcLFOGm (ORCPT ); Tue, 6 Dec 2016 09:06:42 -0500 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> User-Agent: KMail/4.14.10 (Linux/4.8.6-gentoo; KDE/4.14.24; x86_64; ; ) 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: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3546 Lines: 90 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