From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ander Conselvan De Oliveira Subject: Re: [Intel-gfx] [PATCH] drm/i915: Clear pipe's pll hw state in hsw_dp_set_ddi_pll_sel() Date: Wed, 01 Jul 2015 17:54:06 +0300 Message-ID: <1435762446.2645.11.camel@gmail.com> References: <878ub1wdur.fsf@intel.com> <1435669838-24747-1-git-send-email-ander.conselvan.de.oliveira@intel.com> <876165wbol.fsf@intel.com> <20150630142256.GZ30960@phenom.ffwll.local> <87pp4d2ogi.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <87pp4d2ogi.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jani Nikula Cc: intel-gfx , Linux Kernel Mailing List , DRI mailing list , Daniel Vetter , Linus Torvalds List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCAyMDE1LTA2LTMwIGF0IDE4OjQxICswMzAwLCBKYW5pIE5pa3VsYSB3cm90ZToKPiBP biBUdWUsIDMwIEp1biAyMDE1LCBEYW5pZWwgVmV0dGVyIDxkYW5pZWxAZmZ3bGwuY2g+IHdyb3Rl Ogo+ID4gT24gVHVlLCBKdW4gMzAsIDIwMTUgYXQgMDQ6NDc6MDZQTSArMDMwMCwgSmFuaSBOaWt1 bGEgd3JvdGU6Cj4gPj4gT24gVHVlLCAzMCBKdW4gMjAxNSwgQW5kZXIgQ29uc2VsdmFuIGRlIE9s aXZlaXJhIDxhbmRlci5jb25zZWx2YW4uZGUub2xpdmVpcmFAaW50ZWwuY29tPiB3cm90ZToKPiA+ PiA+IFNpbWlsYXJseSB0byB3aGF0IGlzIGRvbmUgZm9yIFNLTCwgY2xlYXIgdGhlIGRwbGxfaHdf c3RhdGUgb2YgdGhlIHBpcGUKPiA+PiA+IGNvbmZpZyBpbiBoc3dfZHBfc2V0X2RkaV9wbGxfc2Vs KCksIHNpbmNlIGl0IG1haW4gY29udGFpbiBzdGFsZSB2YWx1ZXMuCj4gPj4gPiBUaGF0IGNhbiBo YXBwZW4gaWYgYSBjcnRjIHRoYXQgd2FzIHByZXZpb3VzbHkgZHJpdmluZyBhbiBIRE1JIGNvbm5l Y3Rvcgo+ID4+ID4gc3dpdGNoZXMgdG8gYSBEUCBjb25uZWN0b3IuIEluIHRoYXQgY2FzZSwgdGhl IHdycGxsIGZpZWxkIHdhcyBsZWZ0IHdpdGgKPiA+PiA+IGl0cyBvbGQgdmFsdWUsIGxlYWRpbmcg dG8gd2FybmluZ3MgbGlrZSB0aGUgb25lIGJlbG93Ogo+ID4+ID4KPiA+PiA+IFtkcm06Y2hlY2tf Y3J0Y19zdGF0ZSBbaTkxNV1dICpFUlJPUiogbWlzbWF0Y2ggaW4gZHBsbF9od19zdGF0ZS53cnBs bCAoZXhwZWN0ZWQgMHhiMDM1MDYxZiwgZm91bmQgMHgwMDAwMDAwMCkKPiA+PiA+IC0tLS0tLS0t LS0tLVsgY3V0IGhlcmUgXS0tLS0tLS0tLS0tLQo+ID4+ID4gV0FSTklORzogQ1BVOiAxIFBJRDog NzY3IGF0IGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rpc3BsYXkuYzoxMjMyNCBjaGVja19j cnRjX3N0YXRlKzB4OTc1LzB4MTBiMCBbaTkxNV0oKQo+ID4+ID4gcGlwZSBzdGF0ZSBkb2Vzbid0 IG1hdGNoIQo+ID4+ID4KPiA+PiA+IFRoaXMgcmVncmVzc2lvbiB3YXMgaW5kcm9kdWNlZCBpbgo+ ID4+ID4KPiA+PiA+IGNvbW1pdCBkZDNjZDc0YWNmMTI3MjMwNDVhNjRmMWYyYzYyOThhYzdiMzRh NWQ1Cj4gPj4gPiBBdXRob3I6IEFuZGVyIENvbnNlbHZhbiBkZSBPbGl2ZWlyYSA8YW5kZXIuY29u c2VsdmFuLmRlLm9saXZlaXJhQGludGVsLmNvbT4KPiA+PiA+IERhdGU6ICAgRnJpIE1heSAxNSAx MzozNDoyOSAyMDE1ICswMzAwCj4gPj4gPgo+ID4+ID4gICAgIGRybS9pOTE1OiBEb24ndCBvdmVy d3JpdGUgKGUpRFAgUExMIHNlbGVjdGlvbiBvbiBTS0wKPiA+PiA+Cj4gPj4gPiBTaWduZWQtb2Zm LWJ5OiBBbmRlciBDb25zZWx2YW4gZGUgT2xpdmVpcmEgPGFuZGVyLmNvbnNlbHZhbi5kZS5vbGl2 ZWlyYUBpbnRlbC5jb20+Cj4gPj4gCj4gPj4gUmVwb3J0ZWQtYnk6IExpbnVzIFRvcnZhbGRzIDx0 b3J2YWxkc0BsaW51eC1mb3VuZGF0aW9uLm9yZz4KPiA+PiBUZXN0ZWQtYnk6IEphbmkgTmlrdWxh IDxqYW5pLm5pa3VsYUBpbnRlbC5jb20+Cj4gPgo+ID4gWWVhaCBtYWtlcyBzZW5zZSBhcyBhIGZp eCBmb3IgNC4yLiBCdXQgZm9yIDQuMyBJIHdvbmRlciB3aGV0aGVyIHRoZQo+ID4gb3JpZ2luYWwg Y29tbWl0IHRoYXQgc3RhcnRlZCB0aGlzIGNoYWluIG5lZWRzIHRvIGJlIGNoYW5nZWQgYSBiaXQ6 Cj4gPgo+ID4gY29tbWl0IDQ5NzhjYzkzZDlhYzI0MGI0MzVjZTYwNDMxYWVmMjQyMzliNGMyNzAK PiA+IEF1dGhvcjogQW5kZXIgQ29uc2VsdmFuIGRlIE9saXZlaXJhIDxhbmRlci5jb25zZWx2YW4u ZGUub2xpdmVpcmFAaW50ZWwuY29tPgo+ID4gRGF0ZTogICBUdWUgQXByIDIxIDE3OjEzOjIxIDIw MTUgKzAzMDAKPiA+Cj4gPiAgICAgZHJtL2k5MTU6IFByZXNlcnZlIHNoYXJlZCBEUExMIGluZm9y bWF0aW9uIGluIG5ldyBwaXBlX2NvbmZpZwo+ID4KPiA+IEFsbCB0aGUgdHJvdWJsZSB0aGlzIGNh dXNlZCBpcyBiZWNhdXNlIGl0IG5vdCBvbmx5IHByZXNlcnZlcyB0aGUgc2hhcmluZwo+ID4gY29u ZmlnIChpbiBjcnRjX3N0YXRlLT5zaGFyZWRfZHBsbCkgYnV0IGFsc28gdGhlIC0+ZHBsbF9od19z dGF0ZS4gQW5kIEkKPiA+IHRoaW5rIHdpdGggTWFhcnRlbidzIGxhdGVzdCBjb2RlIChmb3IgNC4z KSB3ZSdkIGp1c3QgZG8gYW4gdW5jb25kaXRpb25hbAo+ID4gY29tcHV0ZV9jb25maWcgKG5lZWQg aXQgZm9yIGZhc3QgcGZpdCB1cGRhdGVzIGFuZCBmYXN0Ym9vdCksIHdoaWNoIG1lYW5zCj4gPiB0 aGUgYm9ndXMgdmFsdWVzIGluIC0+ZHBsbF9od19zdGF0ZSBhcmVuJ3QgYSBwcm9ibGVtIGFueSBt b3JlIHNpbmNlIHdlJ2xsCj4gPiBvdmVyd3JpdGUgdGhlbSBhZ2Fpbi4gQW5kIHRoZW4gd2UgY291 bGQgcmVtb3ZlIHRoYXQgc3ByaW5rbGUgb2YgbWVtc2V0cyB3ZQo+ID4gaGF2ZSBhbGwgb3Zlciwg d2hpY2ggd291bGQgYmUgZ29vZCAoc2luY2UgdGhlIGN1cnJlbnQgYXBwcm9hY2ggaXMKPiA+IG9i dmlvdXNseSBhIGJpdCBmcmFnaWxlKS4gQW55d2F5Ogo+ID4KPiA+IFJldmlld2VkLWJ5OiBEYW5p ZWwgVmV0dGVyIDxkYW5pZWwudmV0dGVyQGZmd2xsLmNoPgo+IAo+IFB1c2hlZCB0byBkcm0taW50 ZWwtbmV4dC1maXhlcywgdGhhbmtzIGZvciB0aGUgcGF0Y2ggYW5kIHJldmlldy4gT25lCj4gZG93 biwgYW5vdGhlciBvbmUgbGVmdCB0byBmaXguCgpJIG1hZGUgc29tZSBwcm9ncmVzcyBvbiB0aGUg c2Vjb25kIGlzc3VlLCBidXQgSSdtIGFmcmFpZCBKYW5pIG1pZ2h0IGhhdmUKYSBmb3VuZCBhIHRo aXJkIGJ1Zy4gVGhlIHdhcm5pbmcgaGUgZ2V0cyBoYXBwZW5zIGJlY2F1c2Ugd2UgdHJ5IHRvIHdh aXQKZm9yIHZibGFua3Mgd2hpbGUgdXBkYXRpbmcgdGhlIHByaW1hcnkgcGxhbmUgZHVyaW5nIHRo ZSBtb2Rlc2V0LiBBdCB0aGF0CnBvaW50LCB0aGUgY3J0YyBpcyBvZmYuIFRoZSBwcm9ibGVtIGlz IGluIGludGVsX2NoZWNrX3ByaW1hcnlfcGxhbmUoKSwKd2hpY2ggaXMgY2FsbGVkIGZyb20gZHJt X2F0b21pY19oZWxwZXJfY2hlY2tfcGxhbmVzKCkuIFRoYXQgZnVuY3Rpb24KbWFrZXMgZGVjaXNp b25zIGFib3V0IHdhaXRpbmcgZm9yIGEgdmJsYW5rIGJhc2VkIG9uIGludGVsX2NydGMtPmFjdGl2 ZS4KU2luY2UgdGhlIGNoZWNrIGlzIGNhbGxlZCBiZWZvcmUgd2UgZGlzYWJsZSB0aGUgY3J0Y3Ms IGFjdGl2ZSBtaWdodCBiZQp0cnVlLCBldmVuIHRob3VnaCB0aGUgcGxhbmUgdXBkYXRlIGlzIGRv bmUgd2l0aCBjcnRjcyBkaXNhYmxlLgoKVGhlIHBhdGNoIGJlbG93IG1ha2VzIHRoZSB3YXJuaW5n IGdvIGF3YXksIGJ1dCBJIHN0aWxsIG5lZWQgdG8gZmlndXJlCm91dCBob3cgdG8gc2V0IGNydGNf c3RhdGUtPnBsYW5lc19jaGFuZ2VkIHByb3Blcmx5IGlmIHdlIGFyZSBnb2luZyBkb3duCnRoYXQg cm91dGUuCgpkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZGlzcGxheS5j IGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZGlzcGxheS5jCmluZGV4IGRjYjFkMjUuLmYx NDcyN2MgMTAwNjQ0Ci0tLSBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rpc3BsYXkuYwor KysgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kaXNwbGF5LmMKQEAgLTEyNDgwLDEwICsx MjQ4MCw2IEBAIGludGVsX21vZGVzZXRfY29tcHV0ZV9jb25maWcoc3RydWN0IGRybV9jcnRjICpj cnRjLAogCiAgICAgICAgaW50ZWxfZHVtcF9waXBlX2NvbmZpZyh0b19pbnRlbF9jcnRjKGNydGMp LCBwaXBlX2NvbmZpZywiW21vZGVzZXRdIik7CiAKLSAgICAgICByZXQgPSBkcm1fYXRvbWljX2hl bHBlcl9jaGVja19wbGFuZXMoc3RhdGUtPmRldiwgc3RhdGUpOwotICAgICAgIGlmIChyZXQpCi0g ICAgICAgICAgICAgICByZXR1cm4gRVJSX1BUUihyZXQpOwotCiAgICAgICAgcmV0dXJuIHBpcGVf Y29uZmlnOwogfQogCgpUaGUgYmFja3RyYWNlIG9uIExpbnVzJyBtYWNoaW5lIGlzIGRpZmZlcmVu dCwgdGhvdWdoLiBJdCBjb21lcyBmcm9tIHRoZQpjYWxsIHRvIGludGVsX2NydGNfZGlzYWJsZV9w bGFuZXMoKSBpbiBfX2ludGVsX3NldF9tb2RlKCkuIFRoYXQgd291bGQKaW5kaWNhdGUgd2UgaGF2 ZSBhIGNydGMgd2l0aCBjcnRjLT5zdGF0ZS0+ZW5hYmxlID09IHRydWUgYnV0IHRoYXQgaXMKYWN0 dWFsbHkgaW5hY3RpdmUuIEknbSBzdGlsbCBub3Qgc3VyZSBob3cgd2UgY2FuIGdldCBpbiB0aGF0 IHN0YXRlLgoKQW5kZXIKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9w Lm9yZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRl dmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753712AbbGAOyO (ORCPT ); Wed, 1 Jul 2015 10:54:14 -0400 Received: from mail-pa0-f42.google.com ([209.85.220.42]:34851 "EHLO mail-pa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753785AbbGAOyF (ORCPT ); Wed, 1 Jul 2015 10:54:05 -0400 Message-ID: <1435762446.2645.11.camel@gmail.com> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Clear pipe's pll hw state in hsw_dp_set_ddi_pll_sel() From: Ander Conselvan De Oliveira To: Jani Nikula Cc: Daniel Vetter , intel-gfx , Linux Kernel Mailing List , DRI mailing list , Daniel Vetter , Linus Torvalds Date: Wed, 01 Jul 2015 17:54:06 +0300 In-Reply-To: <87pp4d2ogi.fsf@intel.com> References: <878ub1wdur.fsf@intel.com> <1435669838-24747-1-git-send-email-ander.conselvan.de.oliveira@intel.com> <876165wbol.fsf@intel.com> <20150630142256.GZ30960@phenom.ffwll.local> <87pp4d2ogi.fsf@intel.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.10 (3.12.10-1.fc21) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2015-06-30 at 18:41 +0300, Jani Nikula wrote: > On Tue, 30 Jun 2015, Daniel Vetter wrote: > > On Tue, Jun 30, 2015 at 04:47:06PM +0300, Jani Nikula wrote: > >> On Tue, 30 Jun 2015, Ander Conselvan de Oliveira wrote: > >> > Similarly to what is done for SKL, clear the dpll_hw_state of the pipe > >> > config in hsw_dp_set_ddi_pll_sel(), since it main contain stale values. > >> > That can happen if a crtc that was previously driving an HDMI connector > >> > switches to a DP connector. In that case, the wrpll field was left with > >> > its old value, leading to warnings like the one below: > >> > > >> > [drm:check_crtc_state [i915]] *ERROR* mismatch in dpll_hw_state.wrpll (expected 0xb035061f, found 0x00000000) > >> > ------------[ cut here ]------------ > >> > WARNING: CPU: 1 PID: 767 at drivers/gpu/drm/i915/intel_display.c:12324 check_crtc_state+0x975/0x10b0 [i915]() > >> > pipe state doesn't match! > >> > > >> > This regression was indroduced in > >> > > >> > commit dd3cd74acf12723045a64f1f2c6298ac7b34a5d5 > >> > Author: Ander Conselvan de Oliveira > >> > Date: Fri May 15 13:34:29 2015 +0300 > >> > > >> > drm/i915: Don't overwrite (e)DP PLL selection on SKL > >> > > >> > Signed-off-by: Ander Conselvan de Oliveira > >> > >> Reported-by: Linus Torvalds > >> Tested-by: Jani Nikula > > > > Yeah makes sense as a fix for 4.2. But for 4.3 I wonder whether the > > original commit that started this chain needs to be changed a bit: > > > > commit 4978cc93d9ac240b435ce60431aef24239b4c270 > > Author: Ander Conselvan de Oliveira > > Date: Tue Apr 21 17:13:21 2015 +0300 > > > > drm/i915: Preserve shared DPLL information in new pipe_config > > > > All the trouble this caused is because it not only preserves the sharing > > config (in crtc_state->shared_dpll) but also the ->dpll_hw_state. And I > > think with Maarten's latest code (for 4.3) we'd just do an unconditional > > compute_config (need it for fast pfit updates and fastboot), which means > > the bogus values in ->dpll_hw_state aren't a problem any more since we'll > > overwrite them again. And then we could remove that sprinkle of memsets we > > have all over, which would be good (since the current approach is > > obviously a bit fragile). Anyway: > > > > Reviewed-by: Daniel Vetter > > Pushed to drm-intel-next-fixes, thanks for the patch and review. One > down, another one left to fix. I made some progress on the second issue, but I'm afraid Jani might have a found a third bug. The warning he gets happens because we try to wait for vblanks while updating the primary plane during the modeset. At that point, the crtc is off. The problem is in intel_check_primary_plane(), which is called from drm_atomic_helper_check_planes(). That function makes decisions about waiting for a vblank based on intel_crtc->active. Since the check is called before we disable the crtcs, active might be true, even though the plane update is done with crtcs disable. The patch below makes the warning go away, but I still need to figure out how to set crtc_state->planes_changed properly if we are going down that route. diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index dcb1d25..f14727c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12480,10 +12480,6 @@ intel_modeset_compute_config(struct drm_crtc *crtc, intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]"); - ret = drm_atomic_helper_check_planes(state->dev, state); - if (ret) - return ERR_PTR(ret); - return pipe_config; } The backtrace on Linus' machine is different, though. It comes from the call to intel_crtc_disable_planes() in __intel_set_mode(). That would indicate we have a crtc with crtc->state->enable == true but that is actually inactive. I'm still not sure how we can get in that state. Ander