From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: Fix enc_to_dig_port() for MST encoders Date: Thu, 28 Apr 2016 13:40:09 +0300 Message-ID: <20160428104009.GZ4329@intel.com> References: <1461778466-17747-1-git-send-email-cpaul@redhat.com> <87h9emjm03.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <87h9emjm03.fsf@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: Rob Clark , David Airlie , intel-gfx@lists.freedesktop.org, "open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...), linux-kernel@vger.kernel.org (open list)" , stable@vger.kernel.org, Daniel Vetter List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBBcHIgMjgsIDIwMTYgYXQgMDk6MTc6MDBBTSArMDMwMCwgSmFuaSBOaWt1bGEgd3Jv dGU6Cj4gT24gV2VkLCAyNyBBcHIgMjAxNiwgTHl1ZGUgPGNwYXVsQHJlZGhhdC5jb20+IHdyb3Rl Ogo+ID4gRm9yIE1TVCBlbmNvZGVycywgdGhlIGVuY29kZXIgc3RydWN0IGlzIHN0b3JlZCBpbiB0 aGUgaW50ZWxfZHBfbXN0Cj4gPiBzdHJ1Y3QsIG5vdCBhIGludGVsX2RpZ2l0YWxfcG9ydCBzdHJ1 Y3QuCj4gPgo+ID4gVGhpcyBmaXhlcyBpc3N1ZXMgd2l0aCBob3RwbHVnZ2luZyBNU1QgZGlzcGxh eXMgdGhhdCBzdXBwb3J0IE1TVCBhdWRpbywKPiA+IHdoZXJlIGhvdHBsdWdnaW5nIGhhZCBhIHN1 cnByaXNpbmdseSBnb29kIGNoYW5jZSBvZiBhY2NpZGVudGFsbHkKPiA+IG92ZXJ3cml0aW5nIG90 aGVyIHBhcnRzIG9mIHRoZSBrZXJuZWwgbGVhZGluZyB0byBzZWVtaW5nbHkgdW5yZWxhdGVkCj4g PiBiYWNrdHJhY2VzIGluIHN5c2ZzLCBleHQ0LCBldGMuCj4gPgo+ID4gQ2M6IHN0YWJsZUB2Z2Vy Lmtlcm5lbC5vcmcKPiA+IFNpZ25lZC1vZmYtYnk6IEx5dWRlIDxjcGF1bEByZWRoYXQuY29tPgo+ IAo+IFVnaC4gSnVzdCB1Z2guCj4gCj4gQXQgYSBnbGFuY2UsIGl0J3MgcHJvYmFibHkgdGhpcyBv bmUgdGhhdCBzdGFydHMgY2FsbGluZwo+IGludGVsX2F1ZGlvX2NvZGVjX2VuYWJsZSgpIGFuZCBp bnRlbF9hdWRpb19jb2RlY19kaXNhYmxlKCkgb24gRFAgTVNUOgo+IAo+IGNvbW1pdCAzZDUyY2Nm NTJmMmM1MWY2MTNlNDJlNjViZTBmMDZlNGU2Nzg4MDkzCj4gQXV0aG9yOiBMaWJpbiBZYW5nIDxs aWJpbi55YW5nQGxpbnV4LmludGVsLmNvbT4KPiBEYXRlOiAgIFdlZCBEZWMgMiAxNDowOTo0NCAy MDE1ICswODAwCj4gCj4gICAgIGRybS9pOTE1OiBzdGFydCBhZGRpbmcgZHAgbXN0IGF1ZGlvCj4g Cj4gd2hpY2ggaGFzIGJlZW4gdGhlcmUgc2luY2UgdjQuNS4gV291bGQgaXQgYmUgZmVhc2libGUg Zm9yIHlvdSB0byByZXZlcnQKPiB0aGUgY29tbWl0IG9yIGNoYW5nZSBpdCBzbyB0aGF0IGl0IHN0 b3BzIGNhbGxpbmcgdGhlIGF1ZGlvIGNvZGVjCj4gZW5hYmxlL2Rpc2FibGUsIHRvIHZlcmlmeSB0 aGlzIGlzIHRoZSBjdWxwcml0PyBTbyB3ZSBjb3VsZCBhZGQgYSBwcm9wZXIKPiBGaXhlczogbGlu ZSB0b28uCgpJIGRvbid0IHBhcnRpY3VsYXJseSBsaWtlIG1ha2luZyBlbmNfdG9fZGlnX3BvcnQo KSB3b3JrIG9uIE1TVCBlbmNvZGVycwpiZWNhdXNlIGl0J2xsIGxpa2VseSByZXN1bHQgaW4gdGhl IHdyb25nIHRoaW5nIGFueXdheS4gSSBhbHJlYWR5IGFza2VkCmJlZm9yZSBob3cgTVNUIGF1ZGlv IHBvcnQgaGFuZGxpbmcgaXMgc3VwcG9zZWQgdG8gd29yaywgYnV0IG5ldmVyIGdvdAphbnkgYW5z d2VyLiBSaWdodCBub3csIGlmIHlvdSBqdXN0IHVzZSB0aGUgLT5wcmltYXJ5IGRpZ19wb3J0IGlu IHRoZQphdWRpbyBwYXRocywgdGhlcmUncyBubyB3YXkgYXVkaW8gY2FuIHdvcmsgY29ycmVjdGx5 IHdpdGggTVNULCBhdCBsZWFzdAppZiB5b3UgZW5hYmxlIG11bHRpcGxlIHN0cmVhbXMgb24gdGhl IHNhbWUgcG9ydC4KClNvIHdoYXQgSSB3b3VsZCBkbyBpcyBqdXN0IHJldmVydCB0aGUgYXVkaW8g Y2FsbHMgZnJvbSB0aGUgTVNUIHBhdGhzIGFuZApnbyBiYWNrIHRvIHRoZSBkcmF3aW5nIGJvYXJk LgoKPiAKPiBUaGUgbm9uLU1TVCBhd2FyZSBlbmNfdG9fZGlnX3BvcnQoKSBjYWxscyBvbiBwbGF0 Zm9ybXMgdGhhdCBtaWdodCBoYXZlCj4gTVNUIHdlcmUgYWRkZWQgaW46Cj4gCj4gY29tbWl0IDUx ZTFkODNjYWI5OTg4NzE2YWU2ODgwMWE3MjFmNGRmMGFhYTM3NGIKPiBBdXRob3I6IERhdmlkIEhl bm5pbmdzc29uIDxkYXZpZC5oZW5uaW5nc3NvbkBjYW5vbmljYWwuY29tPgo+IERhdGU6ICAgV2Vk IEF1ZyAxOSAxMDo0ODo1NiAyMDE1ICswMjAwCj4gCj4gICAgIGRybS9pOTE1OiBDYWxsIGF1ZGlv IHBpbi9FTEQgbm90aWZ5IGZ1bmN0aW9uCj4gCj4gYW5kOgo+IAo+IGNvbW1pdCA3ZTgyNzVjMmYy YmJiMzg0ZTE4YWYzNzA2NmI4YjJmMzJiN2QwOTJmCj4gQXV0aG9yOiBMaWJpbiBZYW5nIDxsaWJp bi55YW5nQGludGVsLmNvbT4KPiBEYXRlOiAgIEZyaSBTZXAgMjUgMDk6MzY6MTIgMjAxNSArMDgw MAo+IAo+ICAgICBkcm0vaTkxNTogc2V0IHByb3BlciBOL0NUUyBpbiBtb2Rlc2V0Cj4gCj4gYnV0 IEkgZG9uJ3QgdGhpbmsgdGhlIGNvZGVjIGVuYWJsZS9kaXNhYmxlIHdlcmUgY2FsbGVkIG9uIE1T VCBhdCB0aGF0Cj4gdGltZS4KPiAKPiBTb21lIG9mIHRoZSBwcm9ibGVtIHdhcyBwcm9iYWJseSBz ZWVuIGJ5IFRha2FzaGkgaW4KPiAKPiBjb21taXQgMGJkZjVhMDU2NDdhNjZkY2M2Mzk0OTg2ZTA2 MWRhZWFjOWIxY2Y5Ngo+IEF1dGhvcjogVGFrYXNoaSBJd2FpIDx0aXdhaUBzdXNlLmRlPgo+IERh dGU6ICAgTW9uIE5vdiAzMCAxODoxOTozOSAyMDE1ICswMTAwCj4gCj4gICAgIGRybS9pOTE1OiBB ZGQgcmV2ZXJzZSBtYXBwaW5nIGJldHdlZW4gcG9ydCBhbmQgaW50ZWxfZW5jb2Rlcgo+IAo+IHdo aWNoIGhhcyBhIGNvbW1lbnQgLyogaW50ZWxfZW5jb2RlciBtaWdodCBiZSBOVUxMIGZvciBEUCBN U1QgKi8uIE1heWJlCj4gdGhhdCBuZWVkcyB0byBiZSB1cGRhdGVkIHRvIERUUlQgdG9vLgo+IAo+ IAo+IEx5dWRlLCB0aGFua3MgZm9yIHlvdXIgZWZmb3J0cyBpbiBEUCBNU1QgYXJlYS4gTXVjaCBh cHByZWNpYXRlZC4KPiAKPiAKPiBCUiwKPiBKYW5pLgo+IAo+IAo+IAo+ID4gLS0tCj4gPiAgZHJp dmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHJ2LmggfCAxNyArKysrKysrKysrKy0tLS0tLQo+ID4g IDEgZmlsZSBjaGFuZ2VkLCAxMSBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9ucygtKQo+ID4KPiA+ IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kcnYuaCBiL2RyaXZlcnMv Z3B1L2RybS9pOTE1L2ludGVsX2Rydi5oCj4gPiBpbmRleCA0YzAyN2Q2Li44MWYyMjEyIDEwMDY0 NAo+ID4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHJ2LmgKPiA+ICsrKyBiL2Ry aXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rydi5oCj4gPiBAQCAtOTE4LDE4ICs5MTgsMjMgQEAg aW50ZWxfYXR0YWNoZWRfZW5jb2RlcihzdHJ1Y3QgZHJtX2Nvbm5lY3RvciAqY29ubmVjdG9yKQo+ ID4gIAlyZXR1cm4gdG9faW50ZWxfY29ubmVjdG9yKGNvbm5lY3RvciktPmVuY29kZXI7Cj4gPiAg fQo+ID4gIAo+ID4gLXN0YXRpYyBpbmxpbmUgc3RydWN0IGludGVsX2RpZ2l0YWxfcG9ydCAqCj4g PiAtZW5jX3RvX2RpZ19wb3J0KHN0cnVjdCBkcm1fZW5jb2RlciAqZW5jb2RlcikKPiA+IC17Cj4g PiAtCXJldHVybiBjb250YWluZXJfb2YoZW5jb2Rlciwgc3RydWN0IGludGVsX2RpZ2l0YWxfcG9y dCwgYmFzZS5iYXNlKTsKPiA+IC19Cj4gPiAtCj4gPiAgc3RhdGljIGlubGluZSBzdHJ1Y3QgaW50 ZWxfZHBfbXN0X2VuY29kZXIgKgo+ID4gIGVuY190b19tc3Qoc3RydWN0IGRybV9lbmNvZGVyICpl bmNvZGVyKQo+ID4gIHsKPiA+ICAJcmV0dXJuIGNvbnRhaW5lcl9vZihlbmNvZGVyLCBzdHJ1Y3Qg aW50ZWxfZHBfbXN0X2VuY29kZXIsIGJhc2UuYmFzZSk7Cj4gPiAgfQo+ID4gIAo+ID4gK3N0YXRp YyBpbmxpbmUgc3RydWN0IGludGVsX2RpZ2l0YWxfcG9ydCAqCj4gPiArZW5jX3RvX2RpZ19wb3J0 KHN0cnVjdCBkcm1fZW5jb2RlciAqZW5jb2RlcikKPiA+ICt7Cj4gPiArCWlmIChlbmNvZGVyLT5l bmNvZGVyX3R5cGUgPT0gRFJNX01PREVfRU5DT0RFUl9EUE1TVCkKPiA+ICsJCXJldHVybiBlbmNf dG9fbXN0KGVuY29kZXIpLT5wcmltYXJ5Owo+ID4gKwllbHNlIHsKPiA+ICsJCXJldHVybiBjb250 YWluZXJfb2YoZW5jb2Rlciwgc3RydWN0IGludGVsX2RpZ2l0YWxfcG9ydCwKPiA+ICsJCQkJICAg IGJhc2UuYmFzZSk7Cj4gPiArCX0KPiA+ICt9Cj4gPiArCj4gPiAgc3RhdGljIGlubGluZSBzdHJ1 Y3QgaW50ZWxfZHAgKmVuY190b19pbnRlbF9kcChzdHJ1Y3QgZHJtX2VuY29kZXIgKmVuY29kZXIp Cj4gPiAgewo+ID4gIAlyZXR1cm4gJmVuY190b19kaWdfcG9ydChlbmNvZGVyKS0+ZHA7Cj4gCj4g LS0gCj4gSmFuaSBOaWt1bGEsIEludGVsIE9wZW4gU291cmNlIFRlY2hub2xvZ3kgQ2VudGVyCj4g X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KPiBJbnRlbC1n ZnggbWFpbGluZyBsaXN0Cj4gSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwo+IGh0dHBz Oi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4CgotLSAK VmlsbGUgU3lyasOkbMOkCkludGVsIE9UQwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGluZyBsaXN0CkludGVsLWdmeEBsaXN0cy5m cmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0 aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com ([192.55.52.115]:24572 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752159AbcD1KkO (ORCPT ); Thu, 28 Apr 2016 06:40:14 -0400 Date: Thu, 28 Apr 2016 13:40:09 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Jani Nikula Cc: Lyude , intel-gfx@lists.freedesktop.org, Rob Clark , David Airlie , Daniel Vetter , "open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...), linux-kernel@vger.kernel.org (open list)" , stable@vger.kernel.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix enc_to_dig_port() for MST encoders Message-ID: <20160428104009.GZ4329@intel.com> References: <1461778466-17747-1-git-send-email-cpaul@redhat.com> <87h9emjm03.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87h9emjm03.fsf@intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Apr 28, 2016 at 09:17:00AM +0300, Jani Nikula wrote: > On Wed, 27 Apr 2016, Lyude wrote: > > For MST encoders, the encoder struct is stored in the intel_dp_mst > > struct, not a intel_digital_port struct. > > > > This fixes issues with hotplugging MST displays that support MST audio, > > where hotplugging had a surprisingly good chance of accidentally > > overwriting other parts of the kernel leading to seemingly unrelated > > backtraces in sysfs, ext4, etc. > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Lyude > > Ugh. Just ugh. > > At a glance, it's probably this one that starts calling > intel_audio_codec_enable() and intel_audio_codec_disable() on DP MST: > > commit 3d52ccf52f2c51f613e42e65be0f06e4e6788093 > Author: Libin Yang > Date: Wed Dec 2 14:09:44 2015 +0800 > > drm/i915: start adding dp mst audio > > which has been there since v4.5. Would it be feasible for you to revert > the commit or change it so that it stops calling the audio codec > enable/disable, to verify this is the culprit? So we could add a proper > Fixes: line too. I don't particularly like making enc_to_dig_port() work on MST encoders because it'll likely result in the wrong thing anyway. I already asked before how MST audio port handling is supposed to work, but never got any answer. Right now, if you just use the ->primary dig_port in the audio paths, there's no way audio can work correctly with MST, at least if you enable multiple streams on the same port. So what I would do is just revert the audio calls from the MST paths and go back to the drawing board. > > The non-MST aware enc_to_dig_port() calls on platforms that might have > MST were added in: > > commit 51e1d83cab9988716ae68801a721f4df0aaa374b > Author: David Henningsson > Date: Wed Aug 19 10:48:56 2015 +0200 > > drm/i915: Call audio pin/ELD notify function > > and: > > commit 7e8275c2f2bbb384e18af37066b8b2f32b7d092f > Author: Libin Yang > Date: Fri Sep 25 09:36:12 2015 +0800 > > drm/i915: set proper N/CTS in modeset > > but I don't think the codec enable/disable were called on MST at that > time. > > Some of the problem was probably seen by Takashi in > > commit 0bdf5a05647a66dcc6394986e061daeac9b1cf96 > Author: Takashi Iwai > Date: Mon Nov 30 18:19:39 2015 +0100 > > drm/i915: Add reverse mapping between port and intel_encoder > > which has a comment /* intel_encoder might be NULL for DP MST */. Maybe > that needs to be updated to DTRT too. > > > Lyude, thanks for your efforts in DP MST area. Much appreciated. > > > BR, > Jani. > > > > > --- > > drivers/gpu/drm/i915/intel_drv.h | 17 +++++++++++------ > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 4c027d6..81f2212 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -918,18 +918,23 @@ intel_attached_encoder(struct drm_connector *connector) > > return to_intel_connector(connector)->encoder; > > } > > > > -static inline struct intel_digital_port * > > -enc_to_dig_port(struct drm_encoder *encoder) > > -{ > > - return container_of(encoder, struct intel_digital_port, base.base); > > -} > > - > > static inline struct intel_dp_mst_encoder * > > enc_to_mst(struct drm_encoder *encoder) > > { > > return container_of(encoder, struct intel_dp_mst_encoder, base.base); > > } > > > > +static inline struct intel_digital_port * > > +enc_to_dig_port(struct drm_encoder *encoder) > > +{ > > + if (encoder->encoder_type == DRM_MODE_ENCODER_DPMST) > > + return enc_to_mst(encoder)->primary; > > + else { > > + return container_of(encoder, struct intel_digital_port, > > + base.base); > > + } > > +} > > + > > static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder) > > { > > return &enc_to_dig_port(encoder)->dp; > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrj�l� Intel OTC