On Mon, May 02, 2016 at 11:11:31AM -0400, Lyude Paul wrote:
> I gave a short try at fixing MST audio, but it definitely looks like it's going
> to require quite a bit of troubleshooting and a few more patches :(.
>
> Since I can't find an immediate fix to actually make MST audio work I'm totally
> in favor of reverting the MST audio support for the time being
Can you please send in that revert?
Thanks, Daniel
>
> On Thu, 2016-04-28 at 15:23 +0300, Jani Nikula wrote:
> > On Thu, 28 Apr 2016, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> > >
> > > On Thu, Apr 28, 2016 at 09:17:00AM +0300, Jani Nikula wrote:
> > > >
> > > > On Wed, 27 Apr 2016, Lyude <cpaul@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> > > > > Signed-off-by: Lyude <cpaul@xxxxxxxxxx>
> > > > 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 <libin.yang@xxxxxxxxxxxxxxx>
> > > > 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 <david.henningsson@xxxxxxxxxxxxx>
> > > > Date: Wed Aug 19 10:48:56 2015 +0200
> > > >
> > > > drm/i915: Call audio pin/ELD notify function
> > > >
> > > > and:
> > > >
> > > > commit 7e8275c2f2bbb384e18af37066b8b2f32b7d092f
> > > > Author: Libin Yang <libin.yang@xxxxxxxxx>
> > > > 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 <tiwai@xxxxxxx>
> > > > 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@xxxxxxxxxxxxxxxxxxxxx
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
