From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932200AbcFORg0 (ORCPT ); Wed, 15 Jun 2016 13:36:26 -0400 Received: from mail.fireflyinternet.com ([87.106.93.118]:61644 "EHLO fireflyinternet.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752277AbcFORgY (ORCPT ); Wed, 15 Jun 2016 13:36:24 -0400 X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Date: Wed, 15 Jun 2016 18:35:37 +0100 From: Chris Wilson To: Liviu Dudau Cc: Daniel Vetter , devicetree , Emil Velikov , LKML , DRI devel , Rob Herring , LAKML Subject: Re: [PATCH v5 2/3] drm/arm: Add support for Mali Display Processors Message-ID: <20160615173537.GD6754@nuc-i3427.alporthouse.com> Mail-Followup-To: Chris Wilson , Liviu Dudau , Daniel Vetter , devicetree , Emil Velikov , LKML , DRI devel , Rob Herring , LAKML References: <1466002295-24813-1-git-send-email-Liviu.Dudau@arm.com> <1466002295-24813-3-git-send-email-Liviu.Dudau@arm.com> <20160615152309.GI1338@phenom.ffwll.local> <20160615161758.GF9711@e106497-lin.cambridge.arm.com> <20160615172104.GI9711@e106497-lin.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160615172104.GI9711@e106497-lin.cambridge.arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 15, 2016 at 06:21:04PM +0100, Liviu Dudau wrote: > On Wed, Jun 15, 2016 at 07:13:15PM +0200, Daniel Vetter wrote: > > On Wed, Jun 15, 2016 at 6:17 PM, Liviu Dudau wrote: > > > On Wed, Jun 15, 2016 at 05:23:10PM +0200, Daniel Vetter wrote: > > >> On Wed, Jun 15, 2016 at 03:51:34PM +0100, Liviu Dudau wrote: > > >> > Add support for the new family of Display Processors from ARM Ltd. > > >> > This commit adds basic support for Mali DP500, DP550 and DP650 > > >> > parts, with only the display engine being supported at the moment. > > >> > > > >> > Cc: David Brown > > >> > Cc: Brian Starkey > > >> > > > >> > Signed-off-by: Liviu Dudau > > >> > > >> Small thing I noticed: drm_dev_register/connector_register_all should be > > >> the last step in your init code, and unregister the first. Atm it's > > >> somewhere in the middle. But perfectly fine to do that as a follow-up. > > > > > > I've tried that, but the connector and encoder that gets registered as part > > > of the component_bind_all() fails if there is no drm dev registered. You did > > > comment on the v4 version about that and I did test your idea, sorry for > > > forgeting to update you on that. > > > > Why does it fail? That shouldn't happen ... we need to be able to set > > up everything first, before we register. > > Could be the tda998x_drv fault, but I'm getting this splat: > > [ 1.347687] kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: card0) > [ 1.355420] ------------[ cut here ]------------ > [ 1.360015] WARNING: CPU: 3 PID: 1 at /work/repositories/kernel/lib/kobject.c:244 kobject_add_internal+0xd8/0x290 > [ 1.370202] Modules linked in: > [ 1.373238] > [ 1.374724] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc2+ #2 > [ 1.380941] Hardware name: ARM Juno development board (r0) (DT) > [ 1.386816] task: ffffffc976ca0000 ti: ffffffc976ca8000 task.ti: ffffffc976ca8000 > [ 1.394251] PC is at kobject_add_internal+0xd8/0x290 > [ 1.399179] LR is at kobject_add_internal+0xd8/0x290 > [ 1.404107] pc : [] lr : [] pstate: 60000045 > [ 1.411452] sp : ffffffc976cab7f0 > [ 1.414742] x29: ffffffc976cab7f0 x28: ffffff8008c5bbb8 > [ 1.420022] x27: ffffffc0799c3810 x26: ffffff80089f8a10 > [ 1.425302] x25: 0000000000000000 x24: ffffffc0799c2000 > [ 1.430582] x23: ffffff8008bddc78 x22: ffffffc0799c2000 > [ 1.435861] x21: ffffffc0799c2010 x20: 00000000fffffffe > [ 1.441139] x19: ffffffc0799c3810 x18: 0000000000000010 > [ 1.446418] x17: 0000000000000000 x16: 0000000000000000 > [ 1.451697] x15: ffffff8088c35c87 x14: 6163203a746e6572 > [ 1.456976] x13: 617020322d203a72 x12: 6f7272652820312d > [ 1.462255] x11: 412d494d44482d30 x10: 6472616320726f66 > [ 1.467533] x9 : 2064656c69616620 x8 : 00000000000000a1 > [ 1.472812] x7 : 5f6464615f746365 x6 : 000000000000000a > [ 1.478091] x5 : ffffffc976453c18 x4 : 0000000000000000 > [ 1.483370] x3 : 0000000000000000 x2 : ffffff8008baa7b8 > [ 1.488649] x1 : ffffffc976ca8000 x0 : 0000000000000048 > [ 1.493927] > [ 1.495421] ---[ end trace b193c9c9e93296f4 ]--- > [ 1.500002] Call trace: > [ 1.502434] Exception stack(0xffffffc976cab630 to 0xffffffc976cab750) > [ 1.508827] b620: ffffffc0799c3810 00000000fffffffe > [ 1.516608] b640: ffffffc976cab7f0 ffffff8008344730 ffffffc976cab670 ffffff80080f81dc > [ 1.524389] b660: ffffff80089b7570 0000000108c35000 ffffffc976cab710 ffffff80080f8500 > [ 1.532170] b680: ffffffc0799c3810 00000000fffffffe ffffffc0799c2010 ffffffc0799c2000 > [ 1.539951] b6a0: ffffff8008bddc78 ffffffc0799c2000 0000000000000000 ffffff80089f8a10 > [ 1.547731] b6c0: ffffffc0799c3810 ffffff8008c5bbb8 0000000000000048 ffffffc976ca8000 > [ 1.555511] b6e0: ffffff8008baa7b8 0000000000000000 0000000000000000 ffffffc976453c18 > [ 1.563292] b700: 000000000000000a 5f6464615f746365 00000000000000a1 2064656c69616620 > [ 1.571073] b720: 6472616320726f66 412d494d44482d30 6f7272652820312d 617020322d203a72 > [ 1.578851] b740: 6163203a746e6572 ffffff8088c35c87 > [ 1.583695] [] kobject_add_internal+0xd8/0x290 > [ 1.589658] [] kobject_add+0x84/0xd0 > [ 1.594763] [] device_add+0xc4/0x548 > [ 1.599867] [] device_create_groups_vargs+0x108/0x118 > [ 1.606432] [] device_create_with_groups+0x3c/0x48 > [ 1.612742] [] drm_sysfs_connector_add+0x5c/0xd0 > [ 1.618880] [] drm_connector_register+0x18/0xa0 > [ 1.624930] [] tda998x_bind+0x5f8/0x6c0 > [ 1.630292] [] component_bind_all+0xfc/0x258 > [ 1.636083] [] malidp_bind+0x3b4/0x528 > [ 1.641357] [] try_to_bring_up_master+0x140/0x1a0 > [ 1.647579] [] component_add+0x98/0x170 > [ 1.652940] [] tda998x_probe+0x18/0x20 > [ 1.658216] [] i2c_device_probe+0x164/0x228 > [ 1.663921] [] driver_probe_device+0x204/0x2b0 > [ 1.669884] [] __driver_attach+0xac/0xb0 > [ 1.675330] [] bus_for_each_dev+0x60/0xa0 > [ 1.680862] [] driver_attach+0x20/0x28 > [ 1.686135] [] bus_add_driver+0x1d0/0x238 > [ 1.691668] [] driver_register+0x60/0xf8 > [ 1.697116] [] i2c_register_driver+0x38/0x88 > [ 1.702909] [] tda998x_driver_init+0x18/0x20 > [ 1.708701] [] do_one_initcall+0x38/0x128 > [ 1.714234] [] kernel_init_freeable+0x14c/0x1f0 > [ 1.720286] [] kernel_init+0x10/0x100 > [ 1.725475] [] ret_from_fork+0x10/0x40 > [ 1.730771] [drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -2 > [ 1.745136] mali-dp 6f200000.malidp: failed to bind 1-0070 (ops tda998x_ops): -2 > [ 1.752506] [drm:malidp_bind] *ERROR* Failed to bind all components Something like diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index f55bd9602462..0baf5cebb3b5 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -995,6 +995,10 @@ int drm_connector_register(struct drm_connector *connector) if (connector->registered) return 0; + /* Silently fail to register before the device itself is ready. */ + if (!connector->dev->registered) + return 0; + ret = drm_sysfs_connector_add(connector); if (ret) return ret; diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 13b4c9c0fe36..048733006dbb 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -789,6 +789,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) if (ret) goto err_minors; + dev->registered = true; + if (dev->driver->load) { ret = dev->driver->load(dev, flags); if (ret) @@ -840,6 +842,8 @@ void drm_dev_unregister(struct drm_device *dev) list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head) drm_legacy_rmmap(dev, r_list->map); + dev->registered = false; + drm_minor_unregister(dev, DRM_MINOR_LEGACY); drm_minor_unregister(dev, DRM_MINOR_RENDER); drm_minor_unregister(dev, DRM_MINOR_CONTROL); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 057b6ccdbe8e..4a14e5bfcbda 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -867,6 +867,8 @@ struct drm_device { struct drm_vma_offset_manager *vma_offset_manager; /*@} */ int switch_power_state; + + bool registered; }; #define DRM_SWITCH_POWER_ON 0 after "drm: Automatically register/unregister all connectors"? -Chris -- Chris Wilson, Intel Open Source Technology Centre