From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] backlight: Avoid double fbcon backlight handling Date: Thu, 4 Aug 2016 12:28:11 +0200 Message-ID: <20160804102811.GC6232@phenom.ffwll.local> References: <1467286256-8870-1-git-send-email-chris@chris-wilson.co.uk> <20160712122119.GR23520@phenom.ffwll.local> <87a8gsq540.fsf@intel.com> <20160804095027.GB6232@phenom.ffwll.local> <20160804095729.GN12611@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20160804095729.GN12611@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Daniel Vetter , Jani Nikula , Milo Kim , Krzysztof Kozlowski , David Airlie , nouveau@lists.freedesktop.org, Jon Nettleton , Nicolas Ferre , linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, Thierry Reding , Paul Mackerras , Laurent Pinchart , Daniel Vetter , Lee Jones , Daniel Drake , Jens Frederich , linux-acpi@vger.kernel.org, Bruno =?iso-8859-1?Q?Pr=E9mont?= , Tomi Valkeinen , Ben Skeggs , Zhang Rui , Jean-Ch List-Id: linux-acpi@vger.kernel.org T24gVGh1LCBBdWcgMDQsIDIwMTYgYXQgMTA6NTc6MjlBTSArMDEwMCwgQ2hyaXMgV2lsc29uIHdy b3RlOgo+IE9uIFRodSwgQXVnIDA0LCAyMDE2IGF0IDExOjUwOjI3QU0gKzAyMDAsIERhbmllbCBW ZXR0ZXIgd3JvdGU6Cj4gPiBPbiBUaHUsIEF1ZyAwNCwgMjAxNiBhdCAxMjowMjoyM1BNICswMzAw LCBKYW5pIE5pa3VsYSB3cm90ZToKPiA+ID4gT24gVHVlLCAxMiBKdWwgMjAxNiwgRGFuaWVsIFZl dHRlciA8ZGFuaWVsQGZmd2xsLmNoPiB3cm90ZToKPiA+ID4gPiBPbiBUaHUsIEp1biAzMCwgMjAx NiBhdCAxMjozMDo1NlBNICswMTAwLCBDaHJpcyBXaWxzb24gd3JvdGU6Cj4gPiA+ID4+IEJhY2ts aWdodHMgY29udHJvbGxlZCBieSBpOTE1LmtvIGFuZCBvbmx5IGFzc29jaWF0ZWQgd2l0aCBpdHMg Y29ubmVjdG9ycwo+ID4gPiA+PiBhbmQgYWxzbyBvbmx5IGFzc29jaWF0ZWQgd2l0aCB0aGUgaW50 ZWxfZHJtZmIgZmJjb24sIGNvbnRyb2xsZWQgYnkKPiA+ID4gPj4gaTkxNS5rby4gSW4gdGhpcyBz aXR1YXRpb24sIHdlIGFscmVhZHkgaGFuZGxlIGFkanVzdGluZyB0aGUgYmFja2xpZ2h0Cj4gPiA+ ID4+IHdoZW4gdGhlIGZiY29uIGlzIGJsYW5rZWQvdW5ibGFua2VkIGFuZCBkbyBub3QgcmVxdWly ZSBiYWNrbGlnaHQgdHJ5aW5nCj4gPiA+ID4+IHRvIGRvIHRoZSBzYW1lLgo+ID4gPiA+PiAKPiA+ ID4gPj4gQXR0ZW1wdGluZyB0byByZWdpc3RlciB3aXRoIHRoZSBmYmRldiBhcyBhIGNsaWVudCBj YXVzZXMgbG9ja2RlcCB0byB3YXJuCj4gPiA+ID4+IGFib3V0IGEgZGVwZW5kZW5jeSBjeWNsZToK PiA+ID4gPgo+ID4gPiA+IFRoZSBmYmRldiBub3RpZmllciBzdHJpa2VzIGFnYWluIQo+ID4gPiA+ Cj4gPiA+ID4gTGFzdCB0aW1lIEkgbG9va2VkIGludG8gdGhpcyBJIHRoaW5rIHRoZSBwcm9wZXIg c29sdXRpb24gd291bGQgYmUgdG8gc3BsaXQKPiA+ID4gPiB0aGUgYmFja2xpZ2h0IHBhcnQgZnJv bSB0aGUgb3RoZXIgZmJkZXYgbm90aWZpZXIgKHdoaWNoIGlzIHVzZWQgYnkgZmJjb24KPiA+ID4g PiBmb3IgcmVhY3RpbmcgdG8gZmJkZXYgZGV2aWNlIHJlZy91bnJlZyBldmVudHMpLgo+ID4gPiA+ Cj4gPiA+ID4gSSB0aGluayB0aGF0IHdvdWxkIGZpeCB0aGlzIHRvbywgd2l0aCB0aGUgYWRkZWQg Ym9udXMgb2Ygc2xpZ2h0bHkKPiA+ID4gPiB1bnRhbmdsaW5nIHRoZSBmYmNvbiBsb2NraW5nIG1l c3MuIEFuZCBpdCdzIGFsc28gdGhlIG9uZSBwYXJ0IG9mCj4gPiA+ID4gdW50YW5nbGluZyB0aGlz IG1lc3Mgd2hpY2ggc2hvdWxkIGJlIHBvc3NpYmxlIHdpdGhvdXQgYW55IHRyb3VibGUgLSBJJ3Zl Cj4gPiA+ID4gc2ltcGx5IG5ldmVyIGRvbmUgaXQgc2luY2UgZW50aXJlbHkgZ2V0dGluZyByaWQg b2YgdGhlIGZiZGV2IG5vdGlmaWVyIGZvcgo+ID4gPiA+IGZiY29uIGlzIGEgbG90IG1vcmUgd29y ay4KPiA+ID4gCj4gPiA+IFNvIHdoYXQgZG8gd2UgZG8gd2l0aCB0aGlzPyBJdCBmaXhlcyBhIHBy b2JsZW0gdXBzdHJlYW0uIFRoZXJlJ3Mgbm8KPiA+ID4gRml4ZXM6IHRvIGlkZW50aWZ5IHRoZSBi YWQgY29tbWl0LiBBbnkgaWRlYSBvbiB0aGF0PyBJdCdzIGVpdGhlciB0aGlzIG9yCj4gPiA+IHdl IGRpZyBvdXQgdGhlIGJhZCBjb21taXQgKENocmlzIHByb2JhYmx5IGtub3dzIHdoaWNoIG9uZT8p IGFuZCByZXZlcnQuCj4gPiAKPiA+IFRoZSByZWFsIHRyb3VibGUgaXMgdGhlIGRybV9mb3JfZWFj aF9jb25uZWN0b3IgaW4KPiA+IGRybV9jb25uZWN0b3JfcmVnaXN0ZXJfYWxsKCkuIFRoaXMgaW50 cm9kdWNlZCB0aGUgbmV3IGRlcGVuY3kuIFRoZSBwcm9wZXIKPiA+IGZpeCBpbW8gaXMgdG8gZml4 IHVwIHRoZSBjb25uZWN0b3JfbGlzdCBsb2NraW5nLCBidXQgZm9yIDQuOCB3ZSBjb3VsZCBkbwo+ ID4gdGhlIHNhbWUgaGFjaytjb21tZW50IGxpa2Ugd2UgZG8gaW4gdW5yZWdpc3Rlcl9hbGwuIEl0 J3Mgbm90IHRoZSBvbmx5Cj4gPiBwbGFjZSB0aGF0J3MgYnJva2VuIGFueXdheSwgYW5kIG11Y2gg bGVzcyBpbnZhc2l2ZSB0aGFuIHRoaXMgaGVyZS4KPiAKPiBZb3Ugc3RpbGwgaGF2ZSB0aGUgdW5k ZXJseWluZyBpc3N1ZSBvZiBtdWx0aXBsZSBkcml2ZXJzIHRyeWluZyB0bwo+IGNvbnRyb2wgdGhl IHNhbWUgcGllY2Ugb2YgaGFyZHdhcmUsIGNhdXNpbmcgZHVwbGljYXRlIHdvcmsgKGF0IGJlc3Qp LgoKWWVzLCBhbmQgdGhlIHVuZGVybHlpbmcgaXNzdWUgb2YgdGhlIGZiIGJhY2tsaWdodCBub3Rp ZmllciBiZWluZyB0YW5nbGVkCnVwIGluIGV2ZXJ5dGhpbmcgZWxzZSBpcyBhbHNvIHN0aWxsIHRo ZXJlLiBCdXQgSSB0aGluayBib3RoIGFyZSBhIGJpdCB0b28KYmlnIHRvIGJlIHRhY2tsZWQgaW4g YW4gLXJjIChldmVuIGlmIC1yYzEgaXNuJ3QgZXZlbiB0aGVyZSB5ZXQpLgoKSSB0aGluayB3ZSBz aG91bGQgdHJ5IHRvIGdldCB5b3VyIHBhdGNoIGluIHN0aWxsIGZvciA0LjksIGFuZCBtYXliZSB3 ZSBjYW4KdHJpY2sgc29tZW9uZSBpbnRvIGF0IGxlYXN0IHVudGFuZ2xpbmcgdGhlIGJhY2tsaWdo dCBzdHVmZiBmcm9tIHRoZSBmYgpub3RpZmllciBhdCBsYXJnZS4KLURhbmllbAotLSAKRGFuaWVs IFZldHRlcgpTb2Z0d2FyZSBFbmdpbmVlciwgSW50ZWwgQ29ycG9yYXRpb24KaHR0cDovL2Jsb2cu ZmZ3bGwuY2gKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18K SW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0 dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Date: Thu, 04 Aug 2016 10:28:11 +0000 Subject: Re: [Intel-gfx] [PATCH] backlight: Avoid double fbcon backlight handling Message-Id: <20160804102811.GC6232@phenom.ffwll.local> List-Id: References: <1467286256-8870-1-git-send-email-chris@chris-wilson.co.uk> <20160712122119.GR23520@phenom.ffwll.local> <87a8gsq540.fsf@intel.com> <20160804095027.GB6232@phenom.ffwll.local> <20160804095729.GN12611@nuc-i3427.alporthouse.com> In-Reply-To: <20160804095729.GN12611@nuc-i3427.alporthouse.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Chris Wilson , Daniel Vetter , Jani Nikula , Milo Kim , Krzysztof Kozlowski , David Airlie , nouveau@lists.freedesktop.org, Jon Nettleton , Nicolas Ferre , linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, Thierry Reding , Paul Mackerras , Laurent Pinchart , Daniel Vetter , Lee Jones , Daniel Drake , Jens Frederich , linux-acpi@vger.kernel.org, Bruno =?iso-8859-1?Q?Pr=E9mont?= , Tomi Valkeinen , Ben Skeggs , Zhang Rui , Jean-Ch On Thu, Aug 04, 2016 at 10:57:29AM +0100, Chris Wilson wrote: > On Thu, Aug 04, 2016 at 11:50:27AM +0200, Daniel Vetter wrote: > > On Thu, Aug 04, 2016 at 12:02:23PM +0300, Jani Nikula wrote: > > > On Tue, 12 Jul 2016, Daniel Vetter wrote: > > > > On Thu, Jun 30, 2016 at 12:30:56PM +0100, Chris Wilson wrote: > > > >> Backlights controlled by i915.ko and only associated with its connectors > > > >> and also only associated with the intel_drmfb fbcon, controlled by > > > >> i915.ko. In this situation, we already handle adjusting the backlight > > > >> when the fbcon is blanked/unblanked and do not require backlight trying > > > >> to do the same. > > > >> > > > >> Attempting to register with the fbdev as a client causes lockdep to warn > > > >> about a dependency cycle: > > > > > > > > The fbdev notifier strikes again! > > > > > > > > Last time I looked into this I think the proper solution would be to split > > > > the backlight part from the other fbdev notifier (which is used by fbcon > > > > for reacting to fbdev device reg/unreg events). > > > > > > > > I think that would fix this too, with the added bonus of slightly > > > > untangling the fbcon locking mess. And it's also the one part of > > > > untangling this mess which should be possible without any trouble - I've > > > > simply never done it since entirely getting rid of the fbdev notifier for > > > > fbcon is a lot more work. > > > > > > So what do we do with this? It fixes a problem upstream. There's no > > > Fixes: to identify the bad commit. Any idea on that? It's either this or > > > we dig out the bad commit (Chris probably knows which one?) and revert. > > > > The real trouble is the drm_for_each_connector in > > drm_connector_register_all(). This introduced the new depency. The proper > > fix imo is to fix up the connector_list locking, but for 4.8 we could do > > the same hack+comment like we do in unregister_all. It's not the only > > place that's broken anyway, and much less invasive than this here. > > You still have the underlying issue of multiple drivers trying to > control the same piece of hardware, causing duplicate work (at best). Yes, and the underlying issue of the fb backlight notifier being tangled up in everything else is also still there. But I think both are a bit too big to be tackled in an -rc (even if -rc1 isn't even there yet). I think we should try to get your patch in still for 4.9, and maybe we can trick someone into at least untangling the backlight stuff from the fb notifier at large. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f68.google.com ([74.125.82.68]:34409 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752482AbcHDKmH (ORCPT ); Thu, 4 Aug 2016 06:42:07 -0400 Received: by mail-wm0-f68.google.com with SMTP id q128so42817746wma.1 for ; Thu, 04 Aug 2016 03:41:55 -0700 (PDT) Date: Thu, 4 Aug 2016 12:28:11 +0200 From: Daniel Vetter To: Chris Wilson , Daniel Vetter , Jani Nikula , Milo Kim , Krzysztof Kozlowski , David Airlie , nouveau@lists.freedesktop.org, Jon Nettleton , Nicolas Ferre , linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, Thierry Reding , Paul Mackerras , Laurent Pinchart , Daniel Vetter , Lee Jones , Daniel Drake , Jens Frederich , linux-acpi@vger.kernel.org, Bruno =?iso-8859-1?Q?Pr=E9mont?= , Tomi Valkeinen , Ben Skeggs , Zhang Rui , Jean-Christophe Plagniol-Villard , Michael Hennerich , intel-gfx@lists.freedesktop.org, Jiri Kosina , linux-omap@vger.kernel.org, Thomas Petazzoni , Jingoo Han , linux-renesas-soc@vger.kernel.org, Noralf =?iso-8859-1?Q?Tr=F8nnes?= , Kukjin Kim , Alex Deucher , Antonino Daplas , Christian =?iso-8859-1?Q?K=F6nig?= Subject: Re: [Intel-gfx] [PATCH] backlight: Avoid double fbcon backlight handling Message-ID: <20160804102811.GC6232@phenom.ffwll.local> References: <1467286256-8870-1-git-send-email-chris@chris-wilson.co.uk> <20160712122119.GR23520@phenom.ffwll.local> <87a8gsq540.fsf@intel.com> <20160804095027.GB6232@phenom.ffwll.local> <20160804095729.GN12611@nuc-i3427.alporthouse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160804095729.GN12611@nuc-i3427.alporthouse.com> Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: On Thu, Aug 04, 2016 at 10:57:29AM +0100, Chris Wilson wrote: > On Thu, Aug 04, 2016 at 11:50:27AM +0200, Daniel Vetter wrote: > > On Thu, Aug 04, 2016 at 12:02:23PM +0300, Jani Nikula wrote: > > > On Tue, 12 Jul 2016, Daniel Vetter wrote: > > > > On Thu, Jun 30, 2016 at 12:30:56PM +0100, Chris Wilson wrote: > > > >> Backlights controlled by i915.ko and only associated with its connectors > > > >> and also only associated with the intel_drmfb fbcon, controlled by > > > >> i915.ko. In this situation, we already handle adjusting the backlight > > > >> when the fbcon is blanked/unblanked and do not require backlight trying > > > >> to do the same. > > > >> > > > >> Attempting to register with the fbdev as a client causes lockdep to warn > > > >> about a dependency cycle: > > > > > > > > The fbdev notifier strikes again! > > > > > > > > Last time I looked into this I think the proper solution would be to split > > > > the backlight part from the other fbdev notifier (which is used by fbcon > > > > for reacting to fbdev device reg/unreg events). > > > > > > > > I think that would fix this too, with the added bonus of slightly > > > > untangling the fbcon locking mess. And it's also the one part of > > > > untangling this mess which should be possible without any trouble - I've > > > > simply never done it since entirely getting rid of the fbdev notifier for > > > > fbcon is a lot more work. > > > > > > So what do we do with this? It fixes a problem upstream. There's no > > > Fixes: to identify the bad commit. Any idea on that? It's either this or > > > we dig out the bad commit (Chris probably knows which one?) and revert. > > > > The real trouble is the drm_for_each_connector in > > drm_connector_register_all(). This introduced the new depency. The proper > > fix imo is to fix up the connector_list locking, but for 4.8 we could do > > the same hack+comment like we do in unregister_all. It's not the only > > place that's broken anyway, and much less invasive than this here. > > You still have the underlying issue of multiple drivers trying to > control the same piece of hardware, causing duplicate work (at best). Yes, and the underlying issue of the fb backlight notifier being tangled up in everything else is also still there. But I think both are a bit too big to be tackled in an -rc (even if -rc1 isn't even there yet). I think we should try to get your patch in still for 4.9, and maybe we can trick someone into at least untangling the backlight stuff from the fb notifier at large. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch