From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915/vbt: ignore extraneous child devices for a port Date: Wed, 16 Aug 2017 17:33:21 +0300 Message-ID: <20170816143321.GA4914@intel.com> References: <20170811113907.6716-1-jani.nikula@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3BFB4892C1 for ; Wed, 16 Aug 2017 14:33:26 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170811113907.6716-1-jani.nikula@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni , "# v4 . 12+" List-Id: intel-gfx@lists.freedesktop.org T24gRnJpLCBBdWcgMTEsIDIwMTcgYXQgMDI6Mzk6MDdQTSArMDMwMCwgSmFuaSBOaWt1bGEgd3Jv dGU6Cj4gRXZlciBzaW5jZSB3ZSd2ZSBwYXJzZWQgVkJUIGNoaWxkIGRldmljZXMsIHN0YXJ0aW5n IGZyb20gNmFjYWIxNWE3YjBkCj4gKCJkcm0vaTkxNTogdXNlIHRoZSBIRE1JIERESSBidWZmZXIg dHJhbnNsYXRpb25zIGZyb20gVkJUIiksIHdlJ3ZlCj4gaWdub3JlZCB0aGUgY2hpbGQgZGV2aWNl IGluZm9ybWF0aW9uIGlmIG1vcmUgdGhhbiBvbmUgY2hpbGQgZGV2aWNlCj4gcmVmZXJlbmNlcyB0 aGUgc2FtZSBwb3J0LiBUaGUgcmF0aW9uYWxlIGZvciB0aGlzIHNlZW1zIGxvc3QgaW4gdGltZS4K PiAKPiBTaW5jZSBjb21taXQgMzExYTIwOTQ5ZjA0ICgiZHJtL2k5MTU6IGRvbid0IGluaXQgRFAg b3IgSERNSSB3aGVuIG5vdAo+IHN1cHBvcnRlZCBieSBEREkgcG9ydCIpIHdlIHN0YXJ0ZWQgdXNp bmcgdGhpcyBpbmZvcm1hdGlvbiBtb3JlIHRvIHNraXAKPiBIRE1JL0RQIGluaXQgaWYgdGhlIHBv cnQgd2Fzbid0IHRoZXJlIHBlciBWQlQgY2hpbGQgZGV2aWNlcy4gSG93ZXZlciwgYXQKPiB0aGUg c2FtZSB0aW1lIGl0IGFkZGVkIHBvcnQgZGVmYXVsdHMgd2l0aG91dCBmdXJ0aGVyIGV4cGxhbmF0 aW9uLgo+IAo+IFRodXMsIGlmIHRoZSBjaGlsZCBkZXZpY2UgaW5mbyB3YXMgc2tpcHBlZCBkdWUg dG8gbXVsdGlwbGUgY2hpbGQgZGV2aWNlcwo+IHJlZmVyZW5jaW5nIHRoZSBzYW1lIHBvcnQsIHRo ZSBkZXZpY2UgaW5mbyB3b3VsZCBiZSByZXRyaWV2ZWQgZnJvbSB0aGUKPiBzb21ld2hhdCBhcmJp dHJhcnkgZGVmYXVsdHMuCj4gCj4gRmluYWxseSwgd2hlbiBjb21taXQgYmIxZDEzMjkzNWMyICgi ZHJtL2k5MTUvdmJ0OiBzcGxpdCBvdXQgZGVmYXVsdHMKPiB0aGF0IGFyZSBzZXQgd2hlbiB0aGVy ZSBpcyBubyBWQlQiKSBzdG9wcGVkIGluaXRpYWxpemluZyB0aGUgZGVmYXVsdHMKPiB3aGVuZXZl ciBWQlQgaXMgcHJlc2VudCwgdGh1cyB0cnVzdGluZyB0aGUgVkJUIG1vcmUsIHdlIHN0b3BwZWQK PiBpbml0aWFsaXppbmcgcG9ydHMgd2hpY2ggd2VyZSByZWZlcmVuY2VkIGJ5IG1vcmUgdGhhbiBv bmUgY2hpbGQgZGV2aWNlLgo+IAo+IEFwcGFyZW50bHkgYXQgbGVhc3QgQXN1cyBVWDMwNVVBLCBV WDMwNVUsIGFuZCBVWDMwNlUgbGFwdG9wcyBoYXZlIFZCVAo+IGNoaWxkIGRldmljZSBibG9ja3Mg d2hpY2ggY2F1c2UgdGhpcyBiZWhhdmlvdXIuIEFyZ3VhYmx5IHRoZXkgd2VyZQo+IHNoaXBwZWQg d2l0aCBhIGJyb2tlbiBWQlQuCj4gCj4gUmVsYXggdGhlIHJ1bGVzIGZvciBtdWx0aXBsZSByZWZl cmVuY2VzIHRvIHRoZSBzYW1lIHBvcnQsIGFuZCB1c2UgdGhlCj4gZmlyc3QgY2hpbGQgZGV2aWNl IGluZm8gdG8gcmVmZXJlbmNlIGEgcG9ydC4gUmV0YWluIHRoZSBsb2dpYyB0byBkZWJ1Zwo+IGxv ZyBhYm91dCB0aGlzLCB0aG91Z2guCj4gCj4gQnVnemlsbGE6IGh0dHBzOi8vYnVncy5mcmVlZGVz a3RvcC5vcmcvc2hvd19idWcuY2dpP2lkPTEwMTc0NQo+IEJ1Z3ppbGxhOiBodHRwczovL2J1Z3pp bGxhLmtlcm5lbC5vcmcvc2hvd19idWcuY2dpP2lkPTE5NjIzMwo+IEZpeGVzOiBiYjFkMTMyOTM1 YzIgKCJkcm0vaTkxNS92YnQ6IHNwbGl0IG91dCBkZWZhdWx0cyB0aGF0IGFyZSBzZXQgd2hlbiB0 aGVyZSBpcyBubyBWQlQiKQo+IFRlc3RlZC1ieTogT2xpdmVyIFdlacOfYmFydGggPG1haWxAb3dl aXNzYmFydGguZGU+Cj4gUmVwb3J0ZWQtYnk6IE9saXZlciBXZWnDn2JhcnRoIDxtYWlsQG93ZWlz c2JhcnRoLmRlPgo+IFJlcG9ydGVkLWJ5OiBEaWRpZXIgRyA8ZGlkaWVyZy1kaXZlcnNAb3Jhbmdl LmZyPgo+IFJlcG9ydGVkLWJ5OiBHaWxlcyBBbmRlcnNvbiA8YWdhbmRlckBnbWFpbC5jb20+Cj4g Q2M6IE1hbmFzaSBOYXZhcmUgPG1hbmFzaS5kLm5hdmFyZUBpbnRlbC5jb20+Cj4gQ2M6IFZpbGxl IFN5cmrDpGzDpCA8dmlsbGUuc3lyamFsYUBsaW51eC5pbnRlbC5jb20+Cj4gQ2M6IFBhdWxvIFph bm9uaSA8cGF1bG8uci56YW5vbmlAaW50ZWwuY29tPgo+IENjOiA8c3RhYmxlQHZnZXIua2VybmVs Lm9yZz4gIyB2NC4xMisKPiBTaWduZWQtb2ZmLWJ5OiBKYW5pIE5pa3VsYSA8amFuaS5uaWt1bGFA aW50ZWwuY29tPgo+IC0tLQo+ICBkcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9iaW9zLmMgfCAx NSArKysrKysrKystLS0tLS0KPiAgMSBmaWxlIGNoYW5nZWQsIDkgaW5zZXJ0aW9ucygrKSwgNiBk ZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxf Ymlvcy5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfYmlvcy5jCj4gaW5kZXggODJiMTQ0 Y2RmYTFkLi4xODNlODdlOGVhMzEgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUv aW50ZWxfYmlvcy5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfYmlvcy5jCj4g QEAgLTExMjAsOCArMTEyMCw4IEBAIHN0YXRpYyB2b2lkIHBhcnNlX2RkaV9wb3J0KHN0cnVjdCBk cm1faTkxNV9wcml2YXRlICpkZXZfcHJpdiwgZW51bSBwb3J0IHBvcnQsCj4gIAlib29sIGlzX2R2 aSwgaXNfaGRtaSwgaXNfZHAsIGlzX2VkcCwgaXNfY3J0Owo+ICAJdWludDhfdCBhdXhfY2hhbm5l bCwgZGRjX3BpbjsKPiAgCS8qIEVhY2ggRERJIHBvcnQgY2FuIGhhdmUgbW9yZSB0aGFuIG9uZSB2 YWx1ZSBvbiB0aGUgIkRWTyBQb3J0IiBmaWVsZCwKPiAtCSAqIHNvIGxvb2sgZm9yIGFsbCB0aGUg cG9zc2libGUgdmFsdWVzIGZvciBlYWNoIHBvcnQgYW5kIGFib3J0IGlmIG1vcmUKPiAtCSAqIHRo YW4gb25lIGlzIGZvdW5kLiAqLwo+ICsJICogc28gbG9vayBmb3IgYWxsIHRoZSBwb3NzaWJsZSB2 YWx1ZXMgZm9yIGVhY2ggcG9ydC4KPiArCSAqLwo+ICAJaW50IGR2b19wb3J0c1tdWzNdID0gewo+ ICAJCXtEVk9fUE9SVF9IRE1JQSwgRFZPX1BPUlRfRFBBLCAtMX0sCgpTbyBhdCBsZWFzdCBvbmUg b2YgdGhlIG1hY2hpbmVzIGhhcyBkdm9fcG9ydD09SERNSS1BIGluIHRoZSBWQlQKYWxvbmdzaWRl IERQLUEuIEhETUktQSBpcyBub3QgcmVhbGx5IGxlZ2FsIG9uIGFueSBwbGF0Zm9ybSBJSVJDLCBz byB3ZQptaWdodCB3YW50IHRvIHJlamVjdCBpdCBvdXRyaWdodC4gQnV0IG9uIHRoZSBvdGhlciBo YW5kLCB3ZSd2ZSBzZWVuIGEKbG90IG9mIFZCVHMgdGhhdCBtYWtlIGEgcmVhbCBtZXNzIG9mIHRo ZSBkdm9fcG9ydCB0eXBlIHZzLiB0aGUgZGV2aWNlX3R5cGUKYml0cywgYW5kIHRodXMgd2Ugb2Z0 ZW4gY29uc2lkZXIgYm90aCBIRE1JIGFuZCBEUCBkdm9fcG9ydHMgdG8gYmUgdmFsaWQKZm9yIGVp dGhlciBwb3J0IHR5cGUuIFNvIEkgZ3Vlc3MgaXQgbWlnaHQgYmUgcG9zc2libGUgdGhhdCB0aGVy ZSBhcmUgVkJUcwpvdXQgdGhlcmUgdGhhdCByZWx5IG9uIEhETUktQSBiZWluZyBwaWNrZWQgdXAu CgpFaXRoZXIgd2F5IHRoZSBwYXRjaCBzZWVtcyBhYm91dCBhcyBzYW5lIGFzIGFueXRoaW5nIGVs c2UgcmVsYXRpbmcgdG8gVkJUIHNvClJldmlld2VkLWJ5OiBWaWxsZSBTeXJqw6Rsw6QgPHZpbGxl LnN5cmphbGFAbGludXguaW50ZWwuY29tPgoKPiAgCQl7RFZPX1BPUlRfSERNSUIsIERWT19QT1JU X0RQQiwgLTF9LAo+IEBAIC0xMTMwLDcgKzExMzAsMTAgQEAgc3RhdGljIHZvaWQgcGFyc2VfZGRp X3BvcnQoc3RydWN0IGRybV9pOTE1X3ByaXZhdGUgKmRldl9wcml2LCBlbnVtIHBvcnQgcG9ydCwK PiAgCQl7RFZPX1BPUlRfQ1JULCBEVk9fUE9SVF9IRE1JRSwgRFZPX1BPUlRfRFBFfSwKPiAgCX07 Cj4gIAo+IC0JLyogRmluZCB0aGUgY2hpbGQgZGV2aWNlIHRvIHVzZSwgYWJvcnQgaWYgbW9yZSB0 aGFuIG9uZSBmb3VuZC4gKi8KPiArCS8qCj4gKwkgKiBGaW5kIHRoZSBmaXJzdCBjaGlsZCBkZXZp Y2UgdG8gcmVmZXJlbmNlIHRoZSBwb3J0LCByZXBvcnQgaWYgbW9yZQo+ICsJICogdGhhbiBvbmUg Zm91bmQuCj4gKwkgKi8KPiAgCWZvciAoaSA9IDA7IGkgPCBkZXZfcHJpdi0+dmJ0LmNoaWxkX2Rl dl9udW07IGkrKykgewo+ICAJCWl0ID0gZGV2X3ByaXYtPnZidC5jaGlsZF9kZXYgKyBpOwo+ICAK PiBAQCAtMTE0MCwxMSArMTE0MywxMSBAQCBzdGF0aWMgdm9pZCBwYXJzZV9kZGlfcG9ydChzdHJ1 Y3QgZHJtX2k5MTVfcHJpdmF0ZSAqZGV2X3ByaXYsIGVudW0gcG9ydCBwb3J0LAo+ICAKPiAgCQkJ aWYgKGl0LT5jb21tb24uZHZvX3BvcnQgPT0gZHZvX3BvcnRzW3BvcnRdW2pdKSB7Cj4gIAkJCQlp ZiAoY2hpbGQpIHsKPiAtCQkJCQlEUk1fREVCVUdfS01TKCJNb3JlIHRoYW4gb25lIGNoaWxkIGRl dmljZSBmb3IgcG9ydCAlYyBpbiBWQlQuXG4iLAo+ICsJCQkJCURSTV9ERUJVR19LTVMoIk1vcmUg dGhhbiBvbmUgY2hpbGQgZGV2aWNlIGZvciBwb3J0ICVjIGluIFZCVCwgdXNpbmcgdGhlIGZpcnN0 LlxuIiwKPiAgCQkJCQkJICAgICAgcG9ydF9uYW1lKHBvcnQpKTsKPiAtCQkJCQlyZXR1cm47Cj4g KwkJCQl9IGVsc2Ugewo+ICsJCQkJCWNoaWxkID0gaXQ7Cj4gIAkJCQl9Cj4gLQkJCQljaGlsZCA9 IGl0Owo+ICAJCQl9Cj4gIAkJfQo+ICAJfQo+IC0tIAo+IDIuMTEuMAoKLS0gClZpbGxlIFN5cmrD pGzDpApJbnRlbCBPVEMKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Au b3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwt Z2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com ([134.134.136.100]:33585 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751370AbdHPOd0 (ORCPT ); Wed, 16 Aug 2017 10:33:26 -0400 Date: Wed, 16 Aug 2017 17:33:21 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org, Manasi Navare , Paulo Zanoni , "# v4 . 12+" Subject: Re: [PATCH] drm/i915/vbt: ignore extraneous child devices for a port Message-ID: <20170816143321.GA4914@intel.com> References: <20170811113907.6716-1-jani.nikula@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170811113907.6716-1-jani.nikula@intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Fri, Aug 11, 2017 at 02:39:07PM +0300, Jani Nikula wrote: > Ever since we've parsed VBT child devices, starting from 6acab15a7b0d > ("drm/i915: use the HDMI DDI buffer translations from VBT"), we've > ignored the child device information if more than one child device > references the same port. The rationale for this seems lost in time. > > Since commit 311a20949f04 ("drm/i915: don't init DP or HDMI when not > supported by DDI port") we started using this information more to skip > HDMI/DP init if the port wasn't there per VBT child devices. However, at > the same time it added port defaults without further explanation. > > Thus, if the child device info was skipped due to multiple child devices > referencing the same port, the device info would be retrieved from the > somewhat arbitrary defaults. > > Finally, when commit bb1d132935c2 ("drm/i915/vbt: split out defaults > that are set when there is no VBT") stopped initializing the defaults > whenever VBT is present, thus trusting the VBT more, we stopped > initializing ports which were referenced by more than one child device. > > Apparently at least Asus UX305UA, UX305U, and UX306U laptops have VBT > child device blocks which cause this behaviour. Arguably they were > shipped with a broken VBT. > > Relax the rules for multiple references to the same port, and use the > first child device info to reference a port. Retain the logic to debug > log about this, though. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101745 > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196233 > Fixes: bb1d132935c2 ("drm/i915/vbt: split out defaults that are set when there is no VBT") > Tested-by: Oliver Wei�barth > Reported-by: Oliver Wei�barth > Reported-by: Didier G > Reported-by: Giles Anderson > Cc: Manasi Navare > Cc: Ville Syrj�l� > Cc: Paulo Zanoni > Cc: # v4.12+ > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/i915/intel_bios.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 82b144cdfa1d..183e87e8ea31 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -1120,8 +1120,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > bool is_dvi, is_hdmi, is_dp, is_edp, is_crt; > uint8_t aux_channel, ddc_pin; > /* Each DDI port can have more than one value on the "DVO Port" field, > - * so look for all the possible values for each port and abort if more > - * than one is found. */ > + * so look for all the possible values for each port. > + */ > int dvo_ports[][3] = { > {DVO_PORT_HDMIA, DVO_PORT_DPA, -1}, So at least one of the machines has dvo_port==HDMI-A in the VBT alongside DP-A. HDMI-A is not really legal on any platform IIRC, so we might want to reject it outright. But on the other hand, we've seen a lot of VBTs that make a real mess of the dvo_port type vs. the device_type bits, and thus we often consider both HDMI and DP dvo_ports to be valid for either port type. So I guess it might be possible that there are VBTs out there that rely on HDMI-A being picked up. Either way the patch seems about as sane as anything else relating to VBT so Reviewed-by: Ville Syrj�l� > {DVO_PORT_HDMIB, DVO_PORT_DPB, -1}, > @@ -1130,7 +1130,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > {DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE}, > }; > > - /* Find the child device to use, abort if more than one found. */ > + /* > + * Find the first child device to reference the port, report if more > + * than one found. > + */ > for (i = 0; i < dev_priv->vbt.child_dev_num; i++) { > it = dev_priv->vbt.child_dev + i; > > @@ -1140,11 +1143,11 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > > if (it->common.dvo_port == dvo_ports[port][j]) { > if (child) { > - DRM_DEBUG_KMS("More than one child device for port %c in VBT.\n", > + DRM_DEBUG_KMS("More than one child device for port %c in VBT, using the first.\n", > port_name(port)); > - return; > + } else { > + child = it; > } > - child = it; > } > } > } > -- > 2.11.0 -- Ville Syrj�l� Intel OTC