From mboxrd@z Thu Jan 1 00:00:00 1970 From: Manasi Navare Subject: Re: [PATCH] drm/i915/vbt: ignore extraneous child devices for a port Date: Mon, 14 Aug 2017 16:02:04 -0700 Message-ID: <20170814230204.GA14362@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 mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id 31EDD6E24D for ; Mon, 14 Aug 2017 22:54:25 +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: "# v4 . 12+" , intel-gfx@lists.freedesktop.org, Paulo Zanoni 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+ ICAJCXtEVk9fUE9SVF9IRE1JQSwgRFZPX1BPUlRfRFBBLCAtMX0sCj4gIAkJe0RWT19QT1JUX0hE TUlCLCBEVk9fUE9SVF9EUEIsIC0xfSwKPiBAQCAtMTEzMCw3ICsxMTMwLDEwIEBAIHN0YXRpYyB2 b2lkIHBhcnNlX2RkaV9wb3J0KHN0cnVjdCBkcm1faTkxNV9wcml2YXRlICpkZXZfcHJpdiwgZW51 bSBwb3J0IHBvcnQsCj4gIAkJe0RWT19QT1JUX0NSVCwgRFZPX1BPUlRfSERNSUUsIERWT19QT1JU X0RQRX0sCj4gIAl9Owo+ICAKPiAtCS8qIEZpbmQgdGhlIGNoaWxkIGRldmljZSB0byB1c2UsIGFi b3J0IGlmIG1vcmUgdGhhbiBvbmUgZm91bmQuICovCj4gKwkvKgo+ICsJICogRmluZCB0aGUgZmly c3QgY2hpbGQgZGV2aWNlIHRvIHJlZmVyZW5jZSB0aGUgcG9ydCwgcmVwb3J0IGlmIG1vcmUKPiAr CSAqIHRoYW4gb25lIGZvdW5kLgo+ICsJICovCj4gIAlmb3IgKGkgPSAwOyBpIDwgZGV2X3ByaXYt PnZidC5jaGlsZF9kZXZfbnVtOyBpKyspIHsKPiAgCQlpdCA9IGRldl9wcml2LT52YnQuY2hpbGRf ZGV2ICsgaTsKPiAgCj4gQEAgLTExNDAsMTEgKzExNDMsMTEgQEAgc3RhdGljIHZvaWQgcGFyc2Vf ZGRpX3BvcnQoc3RydWN0IGRybV9pOTE1X3ByaXZhdGUgKmRldl9wcml2LCBlbnVtIHBvcnQgcG9y dCwKPiAgCj4gIAkJCWlmIChpdC0+Y29tbW9uLmR2b19wb3J0ID09IGR2b19wb3J0c1twb3J0XVtq XSkgewo+ICAJCQkJaWYgKGNoaWxkKSB7Cj4gLQkJCQkJRFJNX0RFQlVHX0tNUygiTW9yZSB0aGFu IG9uZSBjaGlsZCBkZXZpY2UgZm9yIHBvcnQgJWMgaW4gVkJULlxuIiwKPiArCQkJCQlEUk1fREVC VUdfS01TKCJNb3JlIHRoYW4gb25lIGNoaWxkIGRldmljZSBmb3IgcG9ydCAlYyBpbiBWQlQsIHVz aW5nIHRoZSBmaXJzdC5cbiIsCj4gIAkJCQkJCSAgICAgIHBvcnRfbmFtZShwb3J0KSk7Cj4gLQkJ CQkJcmV0dXJuOwoKU28gdGhlIGJ1ZyBoZXJlIHdhcyB0aGF0IGluIGNhc2Ugb2YgcG9ydCByZWZl cmVuY2VkIGJ5IG11bHRpcGxlIGNoaWxkIGRldmljZXMgYmVjYXVzZSBpdCB3b3VsZCByZXR1cm4g ZnJvbSB0aGUgZnVuY3Rpb24sCml0IHdvdWxkIHNraXAgdGhlIGluaXRpYWxpemF0aW9uIG9mIGZs YWdzIGxpa2UgaXNfZHAvaXNfaGRtaT8KSXQgYWxtb3N0IGZlZWxzIGxpa2UgdGhleSBtZWFudCB0 byBoYXZlIGEgYnJlYWs7IGhlcmUgaW5zdGVhZCBvZiByZXR1cm4uCk5vdyB0aGF0IHRoaXMgcGF0 Y2ggcmVtb3ZlcyByZXR1cm4gaXQgd2lsbCBzdGlsbCBpdGVyYXRlIHRocm91Z2ggYWxsIHRoZSBj aGlsZSBkZXZpY2VzIGV2ZW4KYWZ0ZXIgZmluZGluZyBzZWNvbmQgcmVmZXJuY2UgdG8gdGhlIHNh bWUgcG9ydCwgaXNudCB0aGF0IHVubmVjZXNzYXJ5PwpXb3VsZCBhZGRpbmcgYSBicmVhayB0aGVy ZSBpbnN0ZWFkIG9wdGltaXplIGl0PwoKUmVnYXJkcwpNYW5hc2kKCj4gKwkJCQl9IGVsc2Ugewo+ ICsJCQkJCWNoaWxkID0gaXQ7Cj4gIAkJCQl9Cj4gLQkJCQljaGlsZCA9IGl0Owo+ICAJCQl9Cj4g IAkJfQo+ICAJfQo+IC0tIAo+IDIuMTEuMAo+IApfX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGluZyBsaXN0CkludGVsLWdmeEBsaXN0 cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9s aXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com ([134.134.136.20]:42753 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752856AbdHNWyd (ORCPT ); Mon, 14 Aug 2017 18:54:33 -0400 Date: Mon, 14 Aug 2017 16:02:04 -0700 From: Manasi Navare To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org, Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , Paulo Zanoni , "# v4 . 12+" Subject: Re: [PATCH] drm/i915/vbt: ignore extraneous child devices for a port Message-ID: <20170814230204.GA14362@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}, > {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; So the bug here was that in case of port referenced by multiple child devices because it would return from the function, it would skip the initialization of flags like is_dp/is_hdmi? It almost feels like they meant to have a break; here instead of return. Now that this patch removes return it will still iterate through all the chile devices even after finding second refernce to the same port, isnt that unnecessary? Would adding a break there instead optimize it? Regards Manasi > + } else { > + child = it; > } > - child = it; > } > } > } > -- > 2.11.0 >