From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: Init power domains early in driver load Date: Thu, 7 Jan 2016 16:32:56 +0200 Message-ID: <20160107143256.GE4437@intel.com> References: <1452157856-27360-1-git-send-email-daniel.vetter@ffwll.ch> <1452174665-13025-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <1452174665-13025-1-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: Meelis Roos , Jani Nikula , Intel Graphics Development , DRI Development , stable@vger.kernel.org, Daniel Vetter List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBKYW4gMDcsIDIwMTYgYXQgMDI6NTE6MDVQTSArMDEwMCwgRGFuaWVsIFZldHRlciB3 cm90ZToKPiBTaW5jZQo+IAo+IGNvbW1pdCBhYzliODIzNjU1MWQxMTc3ZmQwN2I1NmFlZjliNTY1 ZDE4NjQ0MjBkCj4gQXV0aG9yOiBWaWxsZSBTeXJqw6Rsw6QgPHZpbGxlLnN5cmphbGFAbGludXgu aW50ZWwuY29tPgo+IERhdGU6ICAgRnJpIE5vdiAyNyAxODo1NToyNiAyMDE1ICswMjAwCj4gCj4g ICAgIGRybS9pOTE1OiBJbnRyb2R1Y2UgYSBnbWJ1cyBwb3dlciBkb21haW4KPiAKPiBnbWJ1cyBh bHNvIG5lZWRzIHRoZSBwb3dlciBkb21haW4gaW5mcmFzdHJ1Y3R1cmUgcmlnaHQgZnJvbSB0aGUg c3RhcnQsCj4gc2luY2UgYXMgc29vbiBhcyB3ZSByZWdpc3RlciB0aGUgaTJjIGNvbnRyb2xsZXJz IHNvbWVvbmUgY2FuIHVzZSB0aGVtLgo+IAo+IHYyOiBBZGp1c3QgY2xlYW51cCBwYXRocyB0b28g KENocmlzKS4KPiAKPiB2MzogUmViYXNlIG9udG8gLW5pZ2h0bHkgKHRvdGFsbHkgYm9ndXMgdHJl ZSBJIGhhZCBseWluZyBhcm91bmQpIGFuZAo+IGFsc28gbW92ZSBkcGlvIGluaXQgaGVhZCAoVmls bGUpLgo+IAo+IHY0OiBWaWxsZSBpbnN0ZWFkIHN1Z2dlc3RlZCB0byBtb3ZlIGdtYnVzIHNldHVw IGxhdGVyIGluIHRoZSBzZXF1ZW5jZSwKPiBzaW5jZSBpdCdzIG9ubHkgbmVlZGVkIGJ5IHRoZSBt b2Rlc2V0IGNvZGUuCj4gCj4gdjU6IE1vdmUgZXZlbiBjbG9zZSB0byB0aGUgYWN0dWFsIHVzZXIs IHJpZ2h0IG5leHQgdG8gdGhlIGNvbW1lbnQgdGhhdAo+IHN0YXRlcyB3aGVyZSB3ZSByZWFsbHkg bmVlZCBnbWJ1cyAoYW5kIGludGVycnVwdHMhKS4KPiAKPiBDYzogVmlsbGUgU3lyasOkbMOkIDx2 aWxsZS5zeXJqYWxhQGxpbnV4LmludGVsLmNvbT4KPiBDYzogUGF0cmlrIEpha29ic3NvbiA8cGF0 cmlrLmpha29ic3NvbkBsaW51eC5pbnRlbC5jb20+Cj4gQ2M6IEltcmUgRGVhayA8aW1yZS5kZWFr QGludGVsLmNvbT4KPiBDYzogSmFuaSBOaWt1bGEgPGphbmkubmlrdWxhQGludGVsLmNvbT4KPiBD YzogTWVlbGlzIFJvb3MgPG1yb29zQGxpbnV4LmVlPgo+IENjOiBDaHJpcyBXaWxzb24gPGNocmlz QGNocmlzLXdpbHNvbi5jby51az4KPiBGaXhlczogYWM5YjgyMzY1NTFkICgiZHJtL2k5MTU6IElu dHJvZHVjZSBhIGdtYnVzIHBvd2VyIGRvbWFpbiIpCj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5v cmcKPiBSZWZlcmVuY2VzOiBodHRwOi8vd3d3LnNwaW5pY3MubmV0L2xpc3RzL2ludGVsLWdmeC9t c2c4MzA3NS5odG1sCj4gU2lnbmVkLW9mZi1ieTogRGFuaWVsIFZldHRlciA8ZGFuaWVsLnZldHRl ckBpbnRlbC5jb20+Cj4gLS0tCj4gCj4gTWVlbGlzLCBjYW4geW91IHBscyByZXRlc3QgdGhpcyBv bmU/Cj4gCj4gVGhhbmtzLCBEYW5pZWwKPiAtLS0KPiAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkx NV9kbWEuYyAgICAgIHwgNiArKystLS0KPiAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZGlz cGxheS5jIHwgMiArKwo+ICAyIGZpbGVzIGNoYW5nZWQsIDUgaW5zZXJ0aW9ucygrKSwgMyBkZWxl dGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kbWEu YyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZG1hLmMKPiBpbmRleCA5ODhhMzgwNjUxMmEu LmQ3MGQ5NmZlNTUzYiAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2Rt YS5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kbWEuYwo+IEBAIC00MDYsNiAr NDA2LDggQEAgc3RhdGljIGludCBpOTE1X2xvYWRfbW9kZXNldF9pbml0KHN0cnVjdCBkcm1fZGV2 aWNlICpkZXYpCj4gIAlpZiAocmV0KQo+ICAJCWdvdG8gY2xlYW51cF9nZW1fc3RvbGVuOwo+ICAK PiArCWludGVsX3NldHVwX2dtYnVzKGRldik7Cj4gKwo+ICAJLyogSW1wb3J0YW50OiBUaGUgb3V0 cHV0IHNldHVwIGZ1bmN0aW9ucyBjYWxsZWQgYnkgbW9kZXNldF9pbml0IG5lZWQKPiAgCSAqIHdv cmtpbmcgaXJxcyBmb3IgZS5nLiBnbWJ1cyBhbmQgZHAgYXV4IHRyYW5zZmVycy4gKi8KPiAgCWlu dGVsX21vZGVzZXRfaW5pdChkZXYpOwo+IEBAIC00NTUsNiArNDU3LDcgQEAgY2xlYW51cF9nZW06 Cj4gIGNsZWFudXBfaXJxOgo+ICAJaW50ZWxfZ3VjX3Vjb2RlX2ZpbmkoZGV2KTsKPiAgCWRybV9p cnFfdW5pbnN0YWxsKGRldik7Cj4gKwlpbnRlbF90ZWFyZG93bl9nbWJ1cyhkZXYpOwo+ICBjbGVh bnVwX2dlbV9zdG9sZW46Cj4gIAlpOTE1X2dlbV9jbGVhbnVwX3N0b2xlbihkZXYpOwo+ICBjbGVh bnVwX3ZnYV9zd2l0Y2hlcm9vOgo+IEBAIC0xMDI4LDcgKzEwMzEsNiBAQCBpbnQgaTkxNV9kcml2 ZXJfbG9hZChzdHJ1Y3QgZHJtX2RldmljZSAqZGV2LCB1bnNpZ25lZCBsb25nIGZsYWdzKQo+ICAK PiAgCS8qIFRyeSB0byBtYWtlIHN1cmUgTUNIQkFSIGlzIGVuYWJsZWQgYmVmb3JlIHBva2luZyBh dCBpdCAqLwo+ICAJaW50ZWxfc2V0dXBfbWNoYmFyKGRldik7Cj4gLQlpbnRlbF9zZXR1cF9nbWJ1 cyhkZXYpOwo+ICAJaW50ZWxfb3ByZWdpb25fc2V0dXAoZGV2KTsKPiAgCj4gIAlpOTE1X2dlbV9s b2FkKGRldik7Cj4gQEAgLTExMDEsNyArMTEwMyw2IEBAIG91dF9nZW1fdW5sb2FkOgo+ICAJaWYg KGRldi0+cGRldi0+bXNpX2VuYWJsZWQpCj4gIAkJcGNpX2Rpc2FibGVfbXNpKGRldi0+cGRldik7 Cj4gIAo+IC0JaW50ZWxfdGVhcmRvd25fZ21idXMoZGV2KTsKPiAgCWludGVsX3RlYXJkb3duX21j aGJhcihkZXYpOwo+ICAJcG1fcW9zX3JlbW92ZV9yZXF1ZXN0KCZkZXZfcHJpdi0+cG1fcW9zKTsK PiAgCWRlc3Ryb3lfd29ya3F1ZXVlKGRldl9wcml2LT5ncHVfZXJyb3IuaGFuZ2NoZWNrX3dxKTsK PiBAQCAtMTIwMyw3ICsxMjA0LDYgQEAgaW50IGk5MTVfZHJpdmVyX3VubG9hZChzdHJ1Y3QgZHJt X2RldmljZSAqZGV2KQo+ICAKPiAgCWludGVsX2Nzcl91Y29kZV9maW5pKGRldl9wcml2KTsKPiAg Cj4gLQlpbnRlbF90ZWFyZG93bl9nbWJ1cyhkZXYpOwo+ICAJaW50ZWxfdGVhcmRvd25fbWNoYmFy KGRldik7Cj4gIAo+ICAJZGVzdHJveV93b3JrcXVldWUoZGV2X3ByaXYtPmhvdHBsdWcuZHBfd3Ep Owo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kaXNwbGF5LmMgYi9k cml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kaXNwbGF5LmMKPiBpbmRleCAzNzk0NWRkYjRkYWQu LmFjMDAzOGJmNGZiZiAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9k aXNwbGF5LmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kaXNwbGF5LmMKPiBA QCAtMTU5NzEsNiArMTU5NzEsOCBAQCB2b2lkIGludGVsX21vZGVzZXRfY2xlYW51cChzdHJ1Y3Qg ZHJtX2RldmljZSAqZGV2KQo+ICAJbXV0ZXhfbG9jaygmZGV2LT5zdHJ1Y3RfbXV0ZXgpOwo+ICAJ aW50ZWxfY2xlYW51cF9ndF9wb3dlcnNhdmUoZGV2KTsKPiAgCW11dGV4X3VubG9jaygmZGV2LT5z dHJ1Y3RfbXV0ZXgpOwo+ICsKPiArCWludGVsX3RlYXJkb3duX2dtYnVzKGRldik7CgpUaGUgY2xl YW51cCBwYXRoIGlzIHdoZXJlIHRoaW5ncyBtaWdodCBzdGlsbCBiZSBhIGJpdCB3b25reS4gU2hv dWxkIHdlIGJlCmRvaW5nIHRoaXMgYmVmb3JlIHR1cm5pbmcgb2ZmIHRoZSBpbnRlcnJ1cHRzPyBC dXQgdGhlbiB0aGF0IG1pZ2h0IG1lYW4KdGhhdCB0aGUgaHBkIGNsZWFudXAgbmVlZHMgdG8gYmUg cmVoYXNoZWQgc29tZXdoYXQgdG8gbWFrZSBzdXJlIHdlIHNodXQKZG93biBocGQgYmVmb3JlIHVu cmVnaXN0ZXJpbmcgZ21idXMuIFRoZSB3aG9sZSBpbml0L2NsZWFudXAgc2VxdWVuY2UgaXMKYSBi aXQgb2YgYSBtZXNzIHRiaCwgc28gYSBtYWpvciBvdmVyaGF1bCBtaWdodCBiZSByZXF1aXJlZCB0 byBtYWtlIGl0CmFjdHVhbGx5IHNhbmUuCgpJbiBhbnkgY2FzZSwgSSdtIHRoaW5raW5nIHRoaXMg aXMgYXQgbGVhc3Qgbm8gd29yc2UgdGhhdCB3aGF0IHdlIGhhZCwgc28gClJldmlld2VkLWJ5OiBW aWxsZSBTeXJqw6Rsw6QgPHZpbGxlLnN5cmphbGFAbGludXguaW50ZWwuY29tPgoKPiAgfQo+ICAK PiAgLyoKPiAtLSAKPiAyLjYuNAoKLS0gClZpbGxlIFN5cmrDpGzDpApJbnRlbCBPVEMKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxp bmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5mcmVl ZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com ([192.55.52.115]:45562 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750722AbcAGOdD (ORCPT ); Thu, 7 Jan 2016 09:33:03 -0500 Date: Thu, 7 Jan 2016 16:32:56 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Daniel Vetter Cc: DRI Development , Intel Graphics Development , Patrik Jakobsson , Imre Deak , Jani Nikula , Meelis Roos , Chris Wilson , stable@vger.kernel.org, Daniel Vetter Subject: Re: [PATCH] drm/i915: Init power domains early in driver load Message-ID: <20160107143256.GE4437@intel.com> References: <1452157856-27360-1-git-send-email-daniel.vetter@ffwll.ch> <1452174665-13025-1-git-send-email-daniel.vetter@ffwll.ch> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1452174665-13025-1-git-send-email-daniel.vetter@ffwll.ch> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Jan 07, 2016 at 02:51:05PM +0100, Daniel Vetter wrote: > Since > > commit ac9b8236551d1177fd07b56aef9b565d1864420d > Author: Ville Syrj�l� > Date: Fri Nov 27 18:55:26 2015 +0200 > > drm/i915: Introduce a gmbus power domain > > gmbus also needs the power domain infrastructure right from the start, > since as soon as we register the i2c controllers someone can use them. > > v2: Adjust cleanup paths too (Chris). > > v3: Rebase onto -nightly (totally bogus tree I had lying around) and > also move dpio init head (Ville). > > v4: Ville instead suggested to move gmbus setup later in the sequence, > since it's only needed by the modeset code. > > v5: Move even close to the actual user, right next to the comment that > states where we really need gmbus (and interrupts!). > > Cc: Ville Syrj�l� > Cc: Patrik Jakobsson > Cc: Imre Deak > Cc: Jani Nikula > Cc: Meelis Roos > Cc: Chris Wilson > Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain") > Cc: stable@vger.kernel.org > References: http://www.spinics.net/lists/intel-gfx/msg83075.html > Signed-off-by: Daniel Vetter > --- > > Meelis, can you pls retest this one? > > Thanks, Daniel > --- > drivers/gpu/drm/i915/i915_dma.c | 6 +++--- > drivers/gpu/drm/i915/intel_display.c | 2 ++ > 2 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 988a3806512a..d70d96fe553b 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -406,6 +406,8 @@ static int i915_load_modeset_init(struct drm_device *dev) > if (ret) > goto cleanup_gem_stolen; > > + intel_setup_gmbus(dev); > + > /* Important: The output setup functions called by modeset_init need > * working irqs for e.g. gmbus and dp aux transfers. */ > intel_modeset_init(dev); > @@ -455,6 +457,7 @@ cleanup_gem: > cleanup_irq: > intel_guc_ucode_fini(dev); > drm_irq_uninstall(dev); > + intel_teardown_gmbus(dev); > cleanup_gem_stolen: > i915_gem_cleanup_stolen(dev); > cleanup_vga_switcheroo: > @@ -1028,7 +1031,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > > /* Try to make sure MCHBAR is enabled before poking at it */ > intel_setup_mchbar(dev); > - intel_setup_gmbus(dev); > intel_opregion_setup(dev); > > i915_gem_load(dev); > @@ -1101,7 +1103,6 @@ out_gem_unload: > if (dev->pdev->msi_enabled) > pci_disable_msi(dev->pdev); > > - intel_teardown_gmbus(dev); > intel_teardown_mchbar(dev); > pm_qos_remove_request(&dev_priv->pm_qos); > destroy_workqueue(dev_priv->gpu_error.hangcheck_wq); > @@ -1203,7 +1204,6 @@ int i915_driver_unload(struct drm_device *dev) > > intel_csr_ucode_fini(dev_priv); > > - intel_teardown_gmbus(dev); > intel_teardown_mchbar(dev); > > destroy_workqueue(dev_priv->hotplug.dp_wq); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 37945ddb4dad..ac0038bf4fbf 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15971,6 +15971,8 @@ void intel_modeset_cleanup(struct drm_device *dev) > mutex_lock(&dev->struct_mutex); > intel_cleanup_gt_powersave(dev); > mutex_unlock(&dev->struct_mutex); > + > + intel_teardown_gmbus(dev); The cleanup path is where things might still be a bit wonky. Should we be doing this before turning off the interrupts? But then that might mean that the hpd cleanup needs to be rehashed somewhat to make sure we shut down hpd before unregistering gmbus. The whole init/cleanup sequence is a bit of a mess tbh, so a major overhaul might be required to make it actually sane. In any case, I'm thinking this is at least no worse that what we had, so Reviewed-by: Ville Syrj�l� > } > > /* > -- > 2.6.4 -- Ville Syrj�l� Intel OTC