From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ander Conselvan De Oliveira Subject: Re: [PATCH] drm/i915: Allow PCH DPLL sharing regardless of DPLL_SDVO_HIGH_SPEED Date: Wed, 28 Sep 2016 14:32:19 +0300 Message-ID: <1475062339.5813.5.camel@gmail.com> References: <1474878646-17711-1-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-pa0-x243.google.com (mail-pa0-x243.google.com [IPv6:2607:f8b0:400e:c03::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1C53F6E404 for ; Wed, 28 Sep 2016 11:32:26 +0000 (UTC) Received: by mail-pa0-x243.google.com with SMTP id j3so1970474paj.2 for ; Wed, 28 Sep 2016 04:32:26 -0700 (PDT) In-Reply-To: <1474878646-17711-1-git-send-email-ville.syrjala@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org Cc: Nick Yamane , Daniel Vetter , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gTW9uLCAyMDE2LTA5LTI2IGF0IDExOjMwICswMzAwLCB2aWxsZS5zeXJqYWxhQGxpbnV4Lmlu dGVsLmNvbSB3cm90ZToKPiBGcm9tOiBWaWxsZSBTeXJqw6Rsw6QgPHZpbGxlLnN5cmphbGFAbGlu dXguaW50ZWwuY29tPgo+IAo+IERQTExfU0RWT19ISUdIX1NQRUVEIG11c3QgYmUgc2V0IGZvciBT RFZPL0hETUkvRFAsIGJ1dCBub3doZXJlIGlzIGl0Cj4gZm9yYmlkZGVuIHRvIHNldCBpdCBmb3Ig TFZEUy9DUlQgYXMgd2VsbC4gU28gbGV0J3MgYWxzbyBzZXQgaXQgb24KPiBDUlQgdG8gbWFrZSBp dCBwb3NzaWJsZSB0byBzaGFyZSB0aGUgRFBMTCBiZXR3ZWVuIEhETUkgYW5kIENSVC4KPiAKPiBX aGF0IHRoYXQgYml0IGFwcGFyZW50bHkgZG9lcyBpcyBlbmFibGUgdGhlIHg1IGNsb2NrIHRvIHRo ZSBwb3J0LAo+IHdoaWNoIHRoZW4gcHVtcHMgb3V0IHRoZSBiaXRzIG9uIGJvdGggZWRnZXMgb2Yg dGhlIGNsb2NrLiBUaGUgREFDCj4gZG9lc24ndCBuZWVkIHRoYXQgY2xvY2sgc2luY2UgaXQncyBu b3QgcHVtcGluZyBvdXQgYml0cywgYnV0IEkgZG9uJ3QKPiB0aGluayBpdCBodXJ0cyB0byBoYXZl IHRoZSBEUExMIG91dHB1dCB0aGF0IGNsb2NrIGFueXdheS4KPiAKPiBUaGlzIGlzIGZhaXJseSBp bXBvcnRhbnQgb24gSVZCIHNpbmNlIGl0IGhhcyBvbmx5IHR3byBEUExMcyB3aXRoIHRocmVlCj4g cGlwZXMuIFNvIHRyeWluZyB0byBkcml2ZSB0aHJlZSBvciBtb3JlIFBDSCBwb3J0cyB3aXRoIHRo cmVlIHBpcGVzCj4gaXMgb25seSBwb3NzaWJsZSB3aGVuIGF0IGxlYXN0IG9uZSBvZiB0aGUgRFBM THMgZ2V0cyBzaGFyZWQgYmV0d2Vlbgo+IHR3byBvZiB0aGUgcGlwZXMuCj4gCj4gU05CIGRvZXNu J3QgcmVhbGx5IG5lZWQgdG8gZG8gdGhpcyBzaW5jZSBpdCBoYXMgb25seSB0d28gcGlwZXMuIEl0 IGNvdWxkCj4gYmUgZG9uZSB0byBhdm9pZCBlbmFibGluZyB0aGUgc2Vjb25kIERQTEwgYXQgYWxs IGluIGNlcnRhaW4gY2FzZXMsIGJ1dAo+IEknbSBub3Qgc3VyZSB0aGF0J3Mgc3VjaCBhIGh1Z2Ug d2luLiBTbyBsZXQncyBub3QgZG8gaXQgZm9yIFNOQiwgYXQKPiBsZWFzdCBmb3Igbm93LiBPbiBJ TEsgaXQgbmV2ZXIgbWFrZXMgc2Vuc2UgYXMgdGhlIERQTExzIGNhbid0IGJlIHNoYXJlZC4KPiAK PiB2MjogSnVzdCBhbHdheXMgZW5hYmxlIHRoZSBoaWdoIHNwZWVkIGNsb2NrIHRvIGtlZXAgdGhp bmdzIHNpbXBsZSAoRGFuaWVsKQo+IMKgwqDCoMKgQmVlZiB1cCB0aGUgY29tbWl0IG1lc3NhZ2Ug YSBiaXQgKERhbmllbCkKPiAKPiBDYzogTmljayBZYW1hbmUgPG5pY2suZGllZ29AZ21haWwuY29t Pgo+IENjOiBEYW5pZWwgVmV0dGVyIDxkYW5pZWwudmV0dGVyQGZmd2xsLmNoPgo+IENjOiBzdGFi bGVAdmdlci5rZXJuZWwub3JnCj4gVGVzdGVkLWJ5OiBOaWNrIFlhbWFuZSA8bmljay5kaWVnb0Bn bWFpbC5jb20+Cj4gQnVnemlsbGE6IGh0dHBzOi8vYnVncy5mcmVlZGVza3RvcC5vcmcvc2hvd19i dWcuY2dpP2lkPTk3MjA0Cj4gU2lnbmVkLW9mZi1ieTogVmlsbGUgU3lyasOkbMOkIDx2aWxsZS5z eXJqYWxhQGxpbnV4LmludGVsLmNvbT4KPiAtLS0KPiDCoGRyaXZlcnMvZ3B1L2RybS9pOTE1L2lu dGVsX2Rpc3BsYXkuYyB8IDE4ICsrKysrKysrKysrKysrKysrKwo+IMKgMSBmaWxlIGNoYW5nZWQs IDE4IGluc2VydGlvbnMoKykKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUv aW50ZWxfZGlzcGxheS5jCj4gYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kaXNwbGF5LmMK PiBpbmRleCBlNWFkMTAxMGM4YjEuLjQ1ZmY1MDA3NTQ0YyAxMDA2NDQKPiAtLS0gYS9kcml2ZXJz L2dwdS9kcm0vaTkxNS9pbnRlbF9kaXNwbGF5LmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vaTkx NS9pbnRlbF9kaXNwbGF5LmMKPiBAQCAtOTUxMiw2ICs5NTEyLDI0IEBAIHN0YXRpYyB2b2lkIGly b25sYWtlX2NvbXB1dGVfZHBsbChzdHJ1Y3QgaW50ZWxfY3J0Ywo+ICppbnRlbF9jcnRjLAo+IMKg CWlmIChpbnRlbF9jcnRjX2hhc19kcF9lbmNvZGVyKGNydGNfc3RhdGUpKQo+IMKgCQlkcGxsIHw9 IERQTExfU0RWT19ISUdIX1NQRUVEOwo+IMKgCj4gKwkvKgo+ICsJwqAqIFRoZSBoaWdoIHNwZWVk IElPIGNsb2NrIGlzIG9ubHkgcmVhbGx5IHJlcXVpcmVkIGZvcgo+ICsJwqAqIFNEVk8vSERNSS9E UCwgYnV0IHdlIGFsc28gZW5hYmxlIGl0IGZvciBDUlQgdG8gbWFrZSBpdAo+ICsJwqAqIHBvc3Np YmxlIHRvIHNoYXJlIHRoZSBEUExMIGJldHdlZW4gQ1JUIGFuZCBIRE1JLiBFbmFibGluZwo+ICsJ wqAqIHRoZSBjbG9jayBuZWVkbGVzc2x5IGRvZXMgbm8gcmVhbCBoYXJtLCBleGNlcHQgdXNlIHVw IGEKPiArCcKgKiBiaXQgb2YgcG93ZXIgcG90ZW50aWFsbHkuCgpJIGd1ZXNzIHdlIGNvdWxkIGhh dmUgYSBzbWFydGVyIHdheSB0byBjaGVjayBpZiB0d28gY29uZmlndXJhdGlvbnMgYXJlCmNvbXBh dGlibGUgdGhhbiB0aGUgY3VycmVudCBtZW1jbXAoKS4gSS5lLiwgYSBwbGF0Zm9ybSBob29rIHRo YXQgdGFrZXMgdHdvIFBMTApjb25maWdzIGFuZCBlaXRoZXIgcmV0dXJucyBhIG1lcmdlZCBjb25m aWd1cmF0aW9uIHRoYXQgc2F0aXNmeSBib3RoIG9yIHNpZ25hbApmYWlsdXJlLiBUaGF0IHdheSB3 ZSBjb3VsZCBvbmx5IGVuYWJsZSB0aGUgaGlnaCBzcGVlZCBjbG9jayBmb3IgQ1JUIGlmIHJlYWxs eQpuZWNlc3NhcnkuCgpCdXQgbWVoLCBzb3VuZHMgbGlrZSB0b28gbXVjaCB3b3JrIGZvciB2ZXJ5 IGxpdHRsZSBnYWluLgoKVGhlIGRvY3VtZW50YXRpb24gaW5kZWVkIGRvZXNuJ3Qgc2F5IGFueXRo aW5nIGFib3V0IG5vdCBlbmFibGluZyB0aGlzIHdpdGggQ1JULApzbwoKUmV2aWV3ZWQtYnk6IEFu ZGVyIENvbnNlbHZhbiBkZSBPbGl2ZWlyYSA8Y29uc2VsdmFuMkBnbWFpbC5jb20+Cgo+ICsJwqAq Cj4gKwnCoCogV2UnbGwgbGltaXQgdGhpcyB0byBJVkIgd2l0aCAzIHBpcGVzLCBzaW5jZSBpdCBo YXMgb25seSB0d28KPiArCcKgKiBEUExMcyBhbmQgc28gRFBMTCBzaGFyaW5nIGlzIHRoZSBvbmx5 IHdheSB0byBnZXQgdGhyZWUgcGlwZXMKPiArCcKgKiBkcml2aW5nIFBDSCBwb3J0cyBhdCB0aGUg c2FtZSB0aW1lLiBPbiBTTkIgd2UgY291bGQgZG8gdGhpcywKPiArCcKgKiBhbmQgcG90ZW50aWFs bHkgYXZvaWQgZW5hYmxpbmcgdGhlIHNlY29uZCBEUExMLCBidXQgaXQncyBub3QKPiArCcKgKiBj bGVhciBpZiBpdCcncyBhIHdpbiBvciBsb3NzIHBvd2VyIHdpc2UuIE5vIHBvaW50IGluIGRvaW5n Cj4gKwnCoCogdGhpcyBvbiBJTEsgYXQgYWxsIHNpbmNlIGl0IGhhcyBhIGZpeGVkIERQTEw8LT5w aXBlIG1hcHBpbmcuCj4gKwnCoCovCj4gKwlpZiAoSU5URUxfSU5GTyhkZXZfcHJpdiktPm51bV9w aXBlcyA9PSAzICYmCj4gKwnCoMKgwqDCoGludGVsX2NydGNfaGFzX3R5cGUoY3J0Y19zdGF0ZSwg SU5URUxfT1VUUFVUX0FOQUxPRykpCj4gKwkJZHBsbCB8PSBEUExMX1NEVk9fSElHSF9TUEVFRDsK PiArCj4gwqAJLyogY29tcHV0ZSBiaXRtYXNrIGZyb20gcDEgdmFsdWUgKi8KPiDCoAlkcGxsIHw9 ICgxIDw8IChjcnRjX3N0YXRlLT5kcGxsLnAxIC0gMSkpIDw8Cj4gRFBMTF9GUEEwMV9QMV9QT1NU X0RJVl9TSElGVDsKPiDCoAkvKiBhbHNvIEZQQTEgKi8KX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhA bGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxt YW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f65.google.com ([209.85.220.65]:33682 "EHLO mail-pa0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752339AbcI1Lc0 (ORCPT ); Wed, 28 Sep 2016 07:32:26 -0400 Received: by mail-pa0-f65.google.com with SMTP id oz2so1975188pac.0 for ; Wed, 28 Sep 2016 04:32:26 -0700 (PDT) Message-ID: <1475062339.5813.5.camel@gmail.com> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Allow PCH DPLL sharing regardless of DPLL_SDVO_HIGH_SPEED From: Ander Conselvan De Oliveira To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org Cc: Nick Yamane , Daniel Vetter , stable@vger.kernel.org Date: Wed, 28 Sep 2016 14:32:19 +0300 In-Reply-To: <1474878646-17711-1-git-send-email-ville.syrjala@linux.intel.com> References: <1474878646-17711-1-git-send-email-ville.syrjala@linux.intel.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: On Mon, 2016-09-26 at 11:30 +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä > > DPLL_SDVO_HIGH_SPEED must be set for SDVO/HDMI/DP, but nowhere is it > forbidden to set it for LVDS/CRT as well. So let's also set it on > CRT to make it possible to share the DPLL between HDMI and CRT. > > What that bit apparently does is enable the x5 clock to the port, > which then pumps out the bits on both edges of the clock. The DAC > doesn't need that clock since it's not pumping out bits, but I don't > think it hurts to have the DPLL output that clock anyway. > > This is fairly important on IVB since it has only two DPLLs with three > pipes. So trying to drive three or more PCH ports with three pipes > is only possible when at least one of the DPLLs gets shared between > two of the pipes. > > SNB doesn't really need to do this since it has only two pipes. It could > be done to avoid enabling the second DPLL at all in certain cases, but > I'm not sure that's such a huge win. So let's not do it for SNB, at > least for now. On ILK it never makes sense as the DPLLs can't be shared. > > v2: Just always enable the high speed clock to keep things simple (Daniel) >     Beef up the commit message a bit (Daniel) > > Cc: Nick Yamane > Cc: Daniel Vetter > Cc: stable@vger.kernel.org > Tested-by: Nick Yamane > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97204 > Signed-off-by: Ville Syrjälä > --- >  drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++++++++ >  1 file changed, 18 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index e5ad1010c8b1..45ff5007544c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9512,6 +9512,24 @@ static void ironlake_compute_dpll(struct intel_crtc > *intel_crtc, >   if (intel_crtc_has_dp_encoder(crtc_state)) >   dpll |= DPLL_SDVO_HIGH_SPEED; >   > + /* > +  * The high speed IO clock is only really required for > +  * SDVO/HDMI/DP, but we also enable it for CRT to make it > +  * possible to share the DPLL between CRT and HDMI. Enabling > +  * the clock needlessly does no real harm, except use up a > +  * bit of power potentially. I guess we could have a smarter way to check if two configurations are compatible than the current memcmp(). I.e., a platform hook that takes two PLL configs and either returns a merged configuration that satisfy both or signal failure. That way we could only enable the high speed clock for CRT if really necessary. But meh, sounds like too much work for very little gain. The documentation indeed doesn't say anything about not enabling this with CRT, so Reviewed-by: Ander Conselvan de Oliveira > +  * > +  * We'll limit this to IVB with 3 pipes, since it has only two > +  * DPLLs and so DPLL sharing is the only way to get three pipes > +  * driving PCH ports at the same time. On SNB we could do this, > +  * and potentially avoid enabling the second DPLL, but it's not > +  * clear if it''s a win or loss power wise. No point in doing > +  * this on ILK at all since it has a fixed DPLL<->pipe mapping. > +  */ > + if (INTEL_INFO(dev_priv)->num_pipes == 3 && > +     intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) > + dpll |= DPLL_SDVO_HIGH_SPEED; > + >   /* compute bitmask from p1 value */ >   dpll |= (1 << (crtc_state->dpll.p1 - 1)) << > DPLL_FPA01_P1_POST_DIV_SHIFT; >   /* also FPA1 */