From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: Fix enc_to_dig_port() for MST encoders Date: Thu, 28 Apr 2016 15:23:27 +0300 Message-ID: <87vb31hqgw.fsf@intel.com> References: <1461778466-17747-1-git-send-email-cpaul@redhat.com> <87h9emjm03.fsf@intel.com> <20160428104009.GZ4329@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20160428104009.GZ4329@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 =?utf-8?B?U3lyasOkbMOk?= 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 T24gVGh1LCAyOCBBcHIgMjAxNiwgVmlsbGUgU3lyasOkbMOkIDx2aWxsZS5zeXJqYWxhQGxpbnV4 LmludGVsLmNvbT4gd3JvdGU6Cj4gT24gVGh1LCBBcHIgMjgsIDIwMTYgYXQgMDk6MTc6MDBBTSAr MDMwMCwgSmFuaSBOaWt1bGEgd3JvdGU6Cj4+IE9uIFdlZCwgMjcgQXByIDIwMTYsIEx5dWRlIDxj cGF1bEByZWRoYXQuY29tPiB3cm90ZToKPj4gPiBGb3IgTVNUIGVuY29kZXJzLCB0aGUgZW5jb2Rl ciBzdHJ1Y3QgaXMgc3RvcmVkIGluIHRoZSBpbnRlbF9kcF9tc3QKPj4gPiBzdHJ1Y3QsIG5vdCBh IGludGVsX2RpZ2l0YWxfcG9ydCBzdHJ1Y3QuCj4+ID4KPj4gPiBUaGlzIGZpeGVzIGlzc3VlcyB3 aXRoIGhvdHBsdWdnaW5nIE1TVCBkaXNwbGF5cyB0aGF0IHN1cHBvcnQgTVNUIGF1ZGlvLAo+PiA+ IHdoZXJlIGhvdHBsdWdnaW5nIGhhZCBhIHN1cnByaXNpbmdseSBnb29kIGNoYW5jZSBvZiBhY2Np ZGVudGFsbHkKPj4gPiBvdmVyd3JpdGluZyBvdGhlciBwYXJ0cyBvZiB0aGUga2VybmVsIGxlYWRp bmcgdG8gc2VlbWluZ2x5IHVucmVsYXRlZAo+PiA+IGJhY2t0cmFjZXMgaW4gc3lzZnMsIGV4dDQs IGV0Yy4KPj4gPgo+PiA+IENjOiBzdGFibGVAdmdlci5rZXJuZWwub3JnCj4+ID4gU2lnbmVkLW9m Zi1ieTogTHl1ZGUgPGNwYXVsQHJlZGhhdC5jb20+Cj4+IAo+PiBVZ2guIEp1c3QgdWdoLgo+PiAK Pj4gQXQgYSBnbGFuY2UsIGl0J3MgcHJvYmFibHkgdGhpcyBvbmUgdGhhdCBzdGFydHMgY2FsbGlu Zwo+PiBpbnRlbF9hdWRpb19jb2RlY19lbmFibGUoKSBhbmQgaW50ZWxfYXVkaW9fY29kZWNfZGlz YWJsZSgpIG9uIERQIE1TVDoKPj4gCj4+IGNvbW1pdCAzZDUyY2NmNTJmMmM1MWY2MTNlNDJlNjVi ZTBmMDZlNGU2Nzg4MDkzCj4+IEF1dGhvcjogTGliaW4gWWFuZyA8bGliaW4ueWFuZ0BsaW51eC5p bnRlbC5jb20+Cj4+IERhdGU6ICAgV2VkIERlYyAyIDE0OjA5OjQ0IDIwMTUgKzA4MDAKPj4gCj4+ ICAgICBkcm0vaTkxNTogc3RhcnQgYWRkaW5nIGRwIG1zdCBhdWRpbwo+PiAKPj4gd2hpY2ggaGFz IGJlZW4gdGhlcmUgc2luY2UgdjQuNS4gV291bGQgaXQgYmUgZmVhc2libGUgZm9yIHlvdSB0byBy ZXZlcnQKPj4gdGhlIGNvbW1pdCBvciBjaGFuZ2UgaXQgc28gdGhhdCBpdCBzdG9wcyBjYWxsaW5n IHRoZSBhdWRpbyBjb2RlYwo+PiBlbmFibGUvZGlzYWJsZSwgdG8gdmVyaWZ5IHRoaXMgaXMgdGhl IGN1bHByaXQ/IFNvIHdlIGNvdWxkIGFkZCBhIHByb3Blcgo+PiBGaXhlczogbGluZSB0b28uCj4K PiBJIGRvbid0IHBhcnRpY3VsYXJseSBsaWtlIG1ha2luZyBlbmNfdG9fZGlnX3BvcnQoKSB3b3Jr IG9uIE1TVCBlbmNvZGVycwo+IGJlY2F1c2UgaXQnbGwgbGlrZWx5IHJlc3VsdCBpbiB0aGUgd3Jv bmcgdGhpbmcgYW55d2F5LiBJIGFscmVhZHkgYXNrZWQKPiBiZWZvcmUgaG93IE1TVCBhdWRpbyBw b3J0IGhhbmRsaW5nIGlzIHN1cHBvc2VkIHRvIHdvcmssIGJ1dCBuZXZlciBnb3QKPiBhbnkgYW5z d2VyLiBSaWdodCBub3csIGlmIHlvdSBqdXN0IHVzZSB0aGUgLT5wcmltYXJ5IGRpZ19wb3J0IGlu IHRoZQo+IGF1ZGlvIHBhdGhzLCB0aGVyZSdzIG5vIHdheSBhdWRpbyBjYW4gd29yayBjb3JyZWN0 bHkgd2l0aCBNU1QsIGF0IGxlYXN0Cj4gaWYgeW91IGVuYWJsZSBtdWx0aXBsZSBzdHJlYW1zIG9u IHRoZSBzYW1lIHBvcnQuCj4KPiBTbyB3aGF0IEkgd291bGQgZG8gaXMganVzdCByZXZlcnQgdGhl IGF1ZGlvIGNhbGxzIGZyb20gdGhlIE1TVCBwYXRocyBhbmQKPiBnbyBiYWNrIHRvIHRoZSBkcmF3 aW5nIGJvYXJkLgoKSSBkb24ndCBtaW5kIHRoYXQgYXBwcm9hY2ggZWl0aGVyLgoKSSB3YXMgdGhp bmtpbmcgd2Ugc2hvdWxkIHByb2JhYmx5IGFkZCBzb21lIGR5bmFtaWMgdHlwZSBjaGVja2luZyB0 bwplbmNfdG9fZGlnX3BvcnQgdG8gY2F0Y2ggdGhpcyB0eXBlIG9mIGJ1Z3MuCgpCUiwKSmFuaS4K Cj4KPj4gCj4+IFRoZSBub24tTVNUIGF3YXJlIGVuY190b19kaWdfcG9ydCgpIGNhbGxzIG9uIHBs YXRmb3JtcyB0aGF0IG1pZ2h0IGhhdmUKPj4gTVNUIHdlcmUgYWRkZWQgaW46Cj4+IAo+PiBjb21t aXQgNTFlMWQ4M2NhYjk5ODg3MTZhZTY4ODAxYTcyMWY0ZGYwYWFhMzc0Ygo+PiBBdXRob3I6IERh dmlkIEhlbm5pbmdzc29uIDxkYXZpZC5oZW5uaW5nc3NvbkBjYW5vbmljYWwuY29tPgo+PiBEYXRl OiAgIFdlZCBBdWcgMTkgMTA6NDg6NTYgMjAxNSArMDIwMAo+PiAKPj4gICAgIGRybS9pOTE1OiBD YWxsIGF1ZGlvIHBpbi9FTEQgbm90aWZ5IGZ1bmN0aW9uCj4+IAo+PiBhbmQ6Cj4+IAo+PiBjb21t aXQgN2U4Mjc1YzJmMmJiYjM4NGUxOGFmMzcwNjZiOGIyZjMyYjdkMDkyZgo+PiBBdXRob3I6IExp YmluIFlhbmcgPGxpYmluLnlhbmdAaW50ZWwuY29tPgo+PiBEYXRlOiAgIEZyaSBTZXAgMjUgMDk6 MzY6MTIgMjAxNSArMDgwMAo+PiAKPj4gICAgIGRybS9pOTE1OiBzZXQgcHJvcGVyIE4vQ1RTIGlu IG1vZGVzZXQKPj4gCj4+IGJ1dCBJIGRvbid0IHRoaW5rIHRoZSBjb2RlYyBlbmFibGUvZGlzYWJs ZSB3ZXJlIGNhbGxlZCBvbiBNU1QgYXQgdGhhdAo+PiB0aW1lLgo+PiAKPj4gU29tZSBvZiB0aGUg cHJvYmxlbSB3YXMgcHJvYmFibHkgc2VlbiBieSBUYWthc2hpIGluCj4+IAo+PiBjb21taXQgMGJk ZjVhMDU2NDdhNjZkY2M2Mzk0OTg2ZTA2MWRhZWFjOWIxY2Y5Ngo+PiBBdXRob3I6IFRha2FzaGkg SXdhaSA8dGl3YWlAc3VzZS5kZT4KPj4gRGF0ZTogICBNb24gTm92IDMwIDE4OjE5OjM5IDIwMTUg KzAxMDAKPj4gCj4+ICAgICBkcm0vaTkxNTogQWRkIHJldmVyc2UgbWFwcGluZyBiZXR3ZWVuIHBv cnQgYW5kIGludGVsX2VuY29kZXIKPj4gCj4+IHdoaWNoIGhhcyBhIGNvbW1lbnQgLyogaW50ZWxf ZW5jb2RlciBtaWdodCBiZSBOVUxMIGZvciBEUCBNU1QgKi8uIE1heWJlCj4+IHRoYXQgbmVlZHMg dG8gYmUgdXBkYXRlZCB0byBEVFJUIHRvby4KPj4gCj4+IAo+PiBMeXVkZSwgdGhhbmtzIGZvciB5 b3VyIGVmZm9ydHMgaW4gRFAgTVNUIGFyZWEuIE11Y2ggYXBwcmVjaWF0ZWQuCj4+IAo+PiAKPj4g QlIsCj4+IEphbmkuCj4+IAo+PiAKPj4gCj4+ID4gLS0tCj4+ID4gIGRyaXZlcnMvZ3B1L2RybS9p OTE1L2ludGVsX2Rydi5oIHwgMTcgKysrKysrKysrKystLS0tLS0KPj4gPiAgMSBmaWxlIGNoYW5n ZWQsIDExIGluc2VydGlvbnMoKyksIDYgZGVsZXRpb25zKC0pCj4+ID4KPj4gPiBkaWZmIC0tZ2l0 IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHJ2LmggYi9kcml2ZXJzL2dwdS9kcm0vaTkx NS9pbnRlbF9kcnYuaAo+PiA+IGluZGV4IDRjMDI3ZDYuLjgxZjIyMTIgMTAwNjQ0Cj4+ID4gLS0t IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHJ2LmgKPj4gPiArKysgYi9kcml2ZXJzL2dw dS9kcm0vaTkxNS9pbnRlbF9kcnYuaAo+PiA+IEBAIC05MTgsMTggKzkxOCwyMyBAQCBpbnRlbF9h dHRhY2hlZF9lbmNvZGVyKHN0cnVjdCBkcm1fY29ubmVjdG9yICpjb25uZWN0b3IpCj4+ID4gIAly ZXR1cm4gdG9faW50ZWxfY29ubmVjdG9yKGNvbm5lY3RvciktPmVuY29kZXI7Cj4+ID4gIH0KPj4g PiAgCj4+ID4gLXN0YXRpYyBpbmxpbmUgc3RydWN0IGludGVsX2RpZ2l0YWxfcG9ydCAqCj4+ID4g LWVuY190b19kaWdfcG9ydChzdHJ1Y3QgZHJtX2VuY29kZXIgKmVuY29kZXIpCj4+ID4gLXsKPj4g PiAtCXJldHVybiBjb250YWluZXJfb2YoZW5jb2Rlciwgc3RydWN0IGludGVsX2RpZ2l0YWxfcG9y dCwgYmFzZS5iYXNlKTsKPj4gPiAtfQo+PiA+IC0KPj4gPiAgc3RhdGljIGlubGluZSBzdHJ1Y3Qg aW50ZWxfZHBfbXN0X2VuY29kZXIgKgo+PiA+ICBlbmNfdG9fbXN0KHN0cnVjdCBkcm1fZW5jb2Rl ciAqZW5jb2RlcikKPj4gPiAgewo+PiA+ICAJcmV0dXJuIGNvbnRhaW5lcl9vZihlbmNvZGVyLCBz dHJ1Y3QgaW50ZWxfZHBfbXN0X2VuY29kZXIsIGJhc2UuYmFzZSk7Cj4+ID4gIH0KPj4gPiAgCj4+ ID4gK3N0YXRpYyBpbmxpbmUgc3RydWN0IGludGVsX2RpZ2l0YWxfcG9ydCAqCj4+ID4gK2VuY190 b19kaWdfcG9ydChzdHJ1Y3QgZHJtX2VuY29kZXIgKmVuY29kZXIpCj4+ID4gK3sKPj4gPiArCWlm IChlbmNvZGVyLT5lbmNvZGVyX3R5cGUgPT0gRFJNX01PREVfRU5DT0RFUl9EUE1TVCkKPj4gPiAr CQlyZXR1cm4gZW5jX3RvX21zdChlbmNvZGVyKS0+cHJpbWFyeTsKPj4gPiArCWVsc2Ugewo+PiA+ ICsJCXJldHVybiBjb250YWluZXJfb2YoZW5jb2Rlciwgc3RydWN0IGludGVsX2RpZ2l0YWxfcG9y dCwKPj4gPiArCQkJCSAgICBiYXNlLmJhc2UpOwo+PiA+ICsJfQo+PiA+ICt9Cj4+ID4gKwo+PiA+ ICBzdGF0aWMgaW5saW5lIHN0cnVjdCBpbnRlbF9kcCAqZW5jX3RvX2ludGVsX2RwKHN0cnVjdCBk cm1fZW5jb2RlciAqZW5jb2RlcikKPj4gPiAgewo+PiA+ICAJcmV0dXJuICZlbmNfdG9fZGlnX3Bv cnQoZW5jb2RlciktPmRwOwo+PiAKPj4gLS0gCj4+IEphbmkgTmlrdWxhLCBJbnRlbCBPcGVuIFNv dXJjZSBUZWNobm9sb2d5IENlbnRlcgo+PiBfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fXwo+PiBJbnRlbC1nZnggbWFpbGluZyBsaXN0Cj4+IEludGVsLWdmeEBs aXN0cy5mcmVlZGVza3RvcC5vcmcKPj4gaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFp bG1hbi9saXN0aW5mby9pbnRlbC1nZngKCi0tIApKYW5pIE5pa3VsYSwgSW50ZWwgT3BlbiBTb3Vy Y2UgVGVjaG5vbG9neSBDZW50ZXIKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRl c2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8v aW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com ([192.55.52.115]:42697 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753153AbcD1MXa convert rfc822-to-8bit (ORCPT ); Thu, 28 Apr 2016 08:23:30 -0400 From: Jani Nikula To: Ville =?utf-8?B?U3lyasOkbMOk?= 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 In-Reply-To: <20160428104009.GZ4329@intel.com> References: <1461778466-17747-1-git-send-email-cpaul@redhat.com> <87h9emjm03.fsf@intel.com> <20160428104009.GZ4329@intel.com> Date: Thu, 28 Apr 2016 15:23:27 +0300 Message-ID: <87vb31hqgw.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: stable-owner@vger.kernel.org List-ID: On Thu, 28 Apr 2016, Ville Syrjälä wrote: > 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. I don't mind that approach either. I was thinking we should probably add some dynamic type checking to enc_to_dig_port to catch this type of bugs. BR, Jani. > >> >> 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 -- Jani Nikula, Intel Open Source Technology Center