Re: [Intel-gfx] [PATCH] drm/i915: Fix enc_to_dig_port() for MST encoders

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Apr 28, 2016 at 09:17:00AM +0300, Jani Nikula wrote:
> On Wed, 27 Apr 2016, Lyude <cpaul@redhat.com> 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 <cpaul@redhat.com>
> 
> 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@linux.intel.com>
> 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 <david.henningsson@canonical.com>
> 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@intel.com>
> 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@suse.de>
> 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