diff mbox

[RESEND,2/6] drm/i915: Pass atomic state to intel_audio_codec_enable

Message ID 1476885339-2265-3-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maarten Lankhorst Oct. 19, 2016, 1:55 p.m. UTC
drm_select_eld requires mode_config.mutex and connection_mutex
because it looks at the connector list and at the legacy encoders.

This is not required, because when we call audio_codec_enable we know
which connector it was called for, so pass the state.

This also removes having to look at crtc->config.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 17 ++++++++++-------
 drivers/gpu/drm/i915/intel_ddi.c   |  2 +-
 drivers/gpu/drm/i915/intel_dp.c    | 11 ++++++-----
 drivers/gpu/drm/i915/intel_drv.h   |  4 +++-
 drivers/gpu/drm/i915/intel_hdmi.c  |  2 +-
 5 files changed, 21 insertions(+), 15 deletions(-)

Comments

Ville Syrjälä Oct. 21, 2016, 2:04 p.m. UTC | #1
On Wed, Oct 19, 2016 at 03:55:35PM +0200, Maarten Lankhorst wrote:
> drm_select_eld requires mode_config.mutex and connection_mutex
> because it looks at the connector list and at the legacy encoders.
> 
> This is not required, because when we call audio_codec_enable we know
> which connector it was called for, so pass the state.
> 
> This also removes having to look at crtc->config.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 17 ++++++++++-------
>  drivers/gpu/drm/i915/intel_ddi.c   |  2 +-
>  drivers/gpu/drm/i915/intel_dp.c    | 11 ++++++-----
>  drivers/gpu/drm/i915/intel_drv.h   |  4 +++-
>  drivers/gpu/drm/i915/intel_hdmi.c  |  2 +-
>  5 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 7093cfbb62b1..63ef25893c7e 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -485,23 +485,26 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>  /**
>   * intel_audio_codec_enable - Enable the audio codec for HD audio
>   * @intel_encoder: encoder on which to enable audio
> + * @crtc_state: pointer to the current crtc state.
> + * @conn_state: pointer to the current connector state.
>   *
>   * The enable sequences may only be performed after enabling the transcoder and
>   * port, and after completed link training.
>   */
> -void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
> +void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
> +			      const struct intel_crtc_state *crtc_state,
> +			      const struct drm_connector_state *conn_state)
>  {
>  	struct drm_encoder *encoder = &intel_encoder->base;
> -	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> -	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
> +	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
>  	struct drm_connector *connector;
>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>  	struct i915_audio_component *acomp = dev_priv->audio_component;
>  	enum port port = intel_encoder->port;
> -	enum pipe pipe = crtc->pipe;
> +	enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);

While we may require that to be true, I'm not sure I like this use.

>  
> -	connector = drm_select_eld(encoder);
> -	if (!connector)
> +	connector = conn_state->connector;
> +	if (!connector || !connector->eld[0])
>  		return;
>  
>  	DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
> @@ -512,7 +515,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
>  
>  	/* ELD Conn_Type */
>  	connector->eld[5] &= ~(3 << 2);
> -	if (intel_crtc_has_dp_encoder(crtc->config))
> +	if (intel_crtc_has_dp_encoder(crtc_state))
>  		connector->eld[5] |= (1 << 2);
>  
>  	connector->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 7f7741c1406d..6748279310a4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1824,7 +1824,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
>  
>  	if (intel_crtc->config->has_audio) {
>  		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> -		intel_audio_codec_enable(intel_encoder);
> +		intel_audio_codec_enable(intel_encoder, pipe_config, conn_state);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 88f3b745a326..6046f0f54cb3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2734,7 +2734,8 @@ static void intel_dp_enable_port(struct intel_dp *intel_dp,
>  }
>  
>  static void intel_enable_dp(struct intel_encoder *encoder,
> -			    struct intel_crtc_state *pipe_config)
> +			    struct intel_crtc_state *pipe_config,
> +			    struct drm_connector_state *conn_state)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>  	struct drm_device *dev = encoder->base.dev;
> @@ -2776,7 +2777,7 @@ static void intel_enable_dp(struct intel_encoder *encoder,
>  	if (pipe_config->has_audio) {
>  		DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
>  				 pipe_name(pipe));
> -		intel_audio_codec_enable(encoder);
> +		intel_audio_codec_enable(encoder, pipe_config, conn_state);
>  	}
>  }
>  
> @@ -2786,7 +2787,7 @@ static void g4x_enable_dp(struct intel_encoder *encoder,
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>  
> -	intel_enable_dp(encoder, pipe_config);
> +	intel_enable_dp(encoder, pipe_config, conn_state);
>  	intel_edp_backlight_on(intel_dp);
>  }
>  
> @@ -2923,7 +2924,7 @@ static void vlv_pre_enable_dp(struct intel_encoder *encoder,
>  {
>  	vlv_phy_pre_encoder_enable(encoder);
>  
> -	intel_enable_dp(encoder, pipe_config);
> +	intel_enable_dp(encoder, pipe_config, conn_state);
>  }
>  
>  static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder,
> @@ -2941,7 +2942,7 @@ static void chv_pre_enable_dp(struct intel_encoder *encoder,
>  {
>  	chv_phy_pre_encoder_enable(encoder);
>  
> -	intel_enable_dp(encoder, pipe_config);
> +	intel_enable_dp(encoder, pipe_config, conn_state);
>  
>  	/* Second common lane will stay alive on its own now */
>  	chv_phy_release_cl2_override(encoder);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5760420ace61..e3c426563589 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1192,7 +1192,9 @@ u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
>  
>  /* intel_audio.c */
>  void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
> -void intel_audio_codec_enable(struct intel_encoder *encoder);
> +void intel_audio_codec_enable(struct intel_encoder *encoder,
> +			      const struct intel_crtc_state *crtc_state,
> +			      const struct drm_connector_state *conn_state);
>  void intel_audio_codec_disable(struct intel_encoder *encoder);
>  void i915_audio_component_init(struct drm_i915_private *dev_priv);
>  void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index ef43244a884f..fd6439a2882b 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -984,7 +984,7 @@ static void intel_enable_hdmi_audio(struct intel_encoder *encoder,
>  	WARN_ON(!crtc->config->has_hdmi_sink);
>  	DRM_DEBUG_DRIVER("Enabling HDMI audio on pipe %c\n",
>  			 pipe_name(crtc->pipe));
> -	intel_audio_codec_enable(encoder);
> +	intel_audio_codec_enable(encoder, pipe_config, conn_state);
>  }
>  
>  static void g4x_enable_hdmi(struct intel_encoder *encoder,
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Oct. 21, 2016, 2:16 p.m. UTC | #2
On Fri, Oct 21, 2016 at 05:04:46PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 19, 2016 at 03:55:35PM +0200, Maarten Lankhorst wrote:
> > drm_select_eld requires mode_config.mutex and connection_mutex
> > because it looks at the connector list and at the legacy encoders.
> > 
> > This is not required, because when we call audio_codec_enable we know
> > which connector it was called for, so pass the state.
> > 
> > This also removes having to look at crtc->config.
> > 
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_audio.c | 17 ++++++++++-------
> >  drivers/gpu/drm/i915/intel_ddi.c   |  2 +-
> >  drivers/gpu/drm/i915/intel_dp.c    | 11 ++++++-----
> >  drivers/gpu/drm/i915/intel_drv.h   |  4 +++-
> >  drivers/gpu/drm/i915/intel_hdmi.c  |  2 +-
> >  5 files changed, 21 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> > index 7093cfbb62b1..63ef25893c7e 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -485,23 +485,26 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
> >  /**
> >   * intel_audio_codec_enable - Enable the audio codec for HD audio
> >   * @intel_encoder: encoder on which to enable audio
> > + * @crtc_state: pointer to the current crtc state.
> > + * @conn_state: pointer to the current connector state.
> >   *
> >   * The enable sequences may only be performed after enabling the transcoder and
> >   * port, and after completed link training.
> >   */
> > -void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
> > +void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
> > +			      const struct intel_crtc_state *crtc_state,
> > +			      const struct drm_connector_state *conn_state)
> >  {
> >  	struct drm_encoder *encoder = &intel_encoder->base;
> > -	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> > -	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
> > +	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
> >  	struct drm_connector *connector;
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
> >  	struct i915_audio_component *acomp = dev_priv->audio_component;
> >  	enum port port = intel_encoder->port;
> > -	enum pipe pipe = crtc->pipe;
> > +	enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);
> 
> While we may require that to be true, I'm not sure I like this use.

I should say that otherwise I like this.

> 
> >  
> > -	connector = drm_select_eld(encoder);
> > -	if (!connector)
> > +	connector = conn_state->connector;
> > +	if (!connector || !connector->eld[0])
> >  		return;
> >  
> >  	DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
> > @@ -512,7 +515,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
> >  
> >  	/* ELD Conn_Type */
> >  	connector->eld[5] &= ~(3 << 2);
> > -	if (intel_crtc_has_dp_encoder(crtc->config))
> > +	if (intel_crtc_has_dp_encoder(crtc_state))
> >  		connector->eld[5] |= (1 << 2);
> >  
> >  	connector->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 7f7741c1406d..6748279310a4 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1824,7 +1824,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder,
> >  
> >  	if (intel_crtc->config->has_audio) {
> >  		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> > -		intel_audio_codec_enable(intel_encoder);
> > +		intel_audio_codec_enable(intel_encoder, pipe_config, conn_state);
> >  	}
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 88f3b745a326..6046f0f54cb3 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2734,7 +2734,8 @@ static void intel_dp_enable_port(struct intel_dp *intel_dp,
> >  }
> >  
> >  static void intel_enable_dp(struct intel_encoder *encoder,
> > -			    struct intel_crtc_state *pipe_config)
> > +			    struct intel_crtc_state *pipe_config,
> > +			    struct drm_connector_state *conn_state)
> >  {
> >  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> >  	struct drm_device *dev = encoder->base.dev;
> > @@ -2776,7 +2777,7 @@ static void intel_enable_dp(struct intel_encoder *encoder,
> >  	if (pipe_config->has_audio) {
> >  		DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
> >  				 pipe_name(pipe));
> > -		intel_audio_codec_enable(encoder);
> > +		intel_audio_codec_enable(encoder, pipe_config, conn_state);
> >  	}
> >  }
> >  
> > @@ -2786,7 +2787,7 @@ static void g4x_enable_dp(struct intel_encoder *encoder,
> >  {
> >  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> >  
> > -	intel_enable_dp(encoder, pipe_config);
> > +	intel_enable_dp(encoder, pipe_config, conn_state);
> >  	intel_edp_backlight_on(intel_dp);
> >  }
> >  
> > @@ -2923,7 +2924,7 @@ static void vlv_pre_enable_dp(struct intel_encoder *encoder,
> >  {
> >  	vlv_phy_pre_encoder_enable(encoder);
> >  
> > -	intel_enable_dp(encoder, pipe_config);
> > +	intel_enable_dp(encoder, pipe_config, conn_state);
> >  }
> >  
> >  static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder,
> > @@ -2941,7 +2942,7 @@ static void chv_pre_enable_dp(struct intel_encoder *encoder,
> >  {
> >  	chv_phy_pre_encoder_enable(encoder);
> >  
> > -	intel_enable_dp(encoder, pipe_config);
> > +	intel_enable_dp(encoder, pipe_config, conn_state);
> >  
> >  	/* Second common lane will stay alive on its own now */
> >  	chv_phy_release_cl2_override(encoder);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 5760420ace61..e3c426563589 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1192,7 +1192,9 @@ u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
> >  
> >  /* intel_audio.c */
> >  void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
> > -void intel_audio_codec_enable(struct intel_encoder *encoder);
> > +void intel_audio_codec_enable(struct intel_encoder *encoder,
> > +			      const struct intel_crtc_state *crtc_state,
> > +			      const struct drm_connector_state *conn_state);
> >  void intel_audio_codec_disable(struct intel_encoder *encoder);
> >  void i915_audio_component_init(struct drm_i915_private *dev_priv);
> >  void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index ef43244a884f..fd6439a2882b 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -984,7 +984,7 @@ static void intel_enable_hdmi_audio(struct intel_encoder *encoder,
> >  	WARN_ON(!crtc->config->has_hdmi_sink);
> >  	DRM_DEBUG_DRIVER("Enabling HDMI audio on pipe %c\n",
> >  			 pipe_name(crtc->pipe));
> > -	intel_audio_codec_enable(encoder);
> > +	intel_audio_codec_enable(encoder, pipe_config, conn_state);
> >  }
> >  
> >  static void g4x_enable_hdmi(struct intel_encoder *encoder,
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
Maarten Lankhorst Oct. 24, 2016, 8:45 a.m. UTC | #3
Op 21-10-16 om 16:16 schreef Ville Syrjälä:
> On Fri, Oct 21, 2016 at 05:04:46PM +0300, Ville Syrjälä wrote:
>> On Wed, Oct 19, 2016 at 03:55:35PM +0200, Maarten Lankhorst wrote:
>>> drm_select_eld requires mode_config.mutex and connection_mutex
>>> because it looks at the connector list and at the legacy encoders.
>>>
>>> This is not required, because when we call audio_codec_enable we know
>>> which connector it was called for, so pass the state.
>>>
>>> This also removes having to look at crtc->config.
>>>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_audio.c | 17 ++++++++++-------
>>>  drivers/gpu/drm/i915/intel_ddi.c   |  2 +-
>>>  drivers/gpu/drm/i915/intel_dp.c    | 11 ++++++-----
>>>  drivers/gpu/drm/i915/intel_drv.h   |  4 +++-
>>>  drivers/gpu/drm/i915/intel_hdmi.c  |  2 +-
>>>  5 files changed, 21 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>>> index 7093cfbb62b1..63ef25893c7e 100644
>>> --- a/drivers/gpu/drm/i915/intel_audio.c
>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>>> @@ -485,23 +485,26 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>>>  /**
>>>   * intel_audio_codec_enable - Enable the audio codec for HD audio
>>>   * @intel_encoder: encoder on which to enable audio
>>> + * @crtc_state: pointer to the current crtc state.
>>> + * @conn_state: pointer to the current connector state.
>>>   *
>>>   * The enable sequences may only be performed after enabling the transcoder and
>>>   * port, and after completed link training.
>>>   */
>>> -void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
>>> +void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
>>> +			      const struct intel_crtc_state *crtc_state,
>>> +			      const struct drm_connector_state *conn_state)
>>>  {
>>>  	struct drm_encoder *encoder = &intel_encoder->base;
>>> -	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
>>> -	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
>>> +	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
>>>  	struct drm_connector *connector;
>>>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>>>  	struct i915_audio_component *acomp = dev_priv->audio_component;
>>>  	enum port port = intel_encoder->port;
>>> -	enum pipe pipe = crtc->pipe;
>>> +	enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);
>> While we may require that to be true, I'm not sure I like this use.
> I should say that otherwise I like this.
What do you mean with this comment?

~Maarten
Ville Syrjälä Oct. 24, 2016, 10:04 a.m. UTC | #4
On Mon, Oct 24, 2016 at 10:45:36AM +0200, Maarten Lankhorst wrote:
> Op 21-10-16 om 16:16 schreef Ville Syrjälä:
> > On Fri, Oct 21, 2016 at 05:04:46PM +0300, Ville Syrjälä wrote:
> >> On Wed, Oct 19, 2016 at 03:55:35PM +0200, Maarten Lankhorst wrote:
> >>> drm_select_eld requires mode_config.mutex and connection_mutex
> >>> because it looks at the connector list and at the legacy encoders.
> >>>
> >>> This is not required, because when we call audio_codec_enable we know
> >>> which connector it was called for, so pass the state.
> >>>
> >>> This also removes having to look at crtc->config.
> >>>
> >>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_audio.c | 17 ++++++++++-------
> >>>  drivers/gpu/drm/i915/intel_ddi.c   |  2 +-
> >>>  drivers/gpu/drm/i915/intel_dp.c    | 11 ++++++-----
> >>>  drivers/gpu/drm/i915/intel_drv.h   |  4 +++-
> >>>  drivers/gpu/drm/i915/intel_hdmi.c  |  2 +-
> >>>  5 files changed, 21 insertions(+), 15 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> >>> index 7093cfbb62b1..63ef25893c7e 100644
> >>> --- a/drivers/gpu/drm/i915/intel_audio.c
> >>> +++ b/drivers/gpu/drm/i915/intel_audio.c
> >>> @@ -485,23 +485,26 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
> >>>  /**
> >>>   * intel_audio_codec_enable - Enable the audio codec for HD audio
> >>>   * @intel_encoder: encoder on which to enable audio
> >>> + * @crtc_state: pointer to the current crtc state.
> >>> + * @conn_state: pointer to the current connector state.
> >>>   *
> >>>   * The enable sequences may only be performed after enabling the transcoder and
> >>>   * port, and after completed link training.
> >>>   */
> >>> -void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
> >>> +void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
> >>> +			      const struct intel_crtc_state *crtc_state,
> >>> +			      const struct drm_connector_state *conn_state)
> >>>  {
> >>>  	struct drm_encoder *encoder = &intel_encoder->base;
> >>> -	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> >>> -	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
> >>> +	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
> >>>  	struct drm_connector *connector;
> >>>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
> >>>  	struct i915_audio_component *acomp = dev_priv->audio_component;
> >>>  	enum port port = intel_encoder->port;
> >>> -	enum pipe pipe = crtc->pipe;
> >>> +	enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);
> >> While we may require that to be true, I'm not sure I like this use.
> > I should say that otherwise I like this.
> What do you mean with this comment?

That the rest of the patch makes sense.
Maarten Lankhorst Oct. 24, 2016, 10:12 a.m. UTC | #5
Op 24-10-16 om 12:04 schreef Ville Syrjälä:
> On Mon, Oct 24, 2016 at 10:45:36AM +0200, Maarten Lankhorst wrote:
>> Op 21-10-16 om 16:16 schreef Ville Syrjälä:
>>> On Fri, Oct 21, 2016 at 05:04:46PM +0300, Ville Syrjälä wrote:
>>>> On Wed, Oct 19, 2016 at 03:55:35PM +0200, Maarten Lankhorst wrote:
>>>>> drm_select_eld requires mode_config.mutex and connection_mutex
>>>>> because it looks at the connector list and at the legacy encoders.
>>>>>
>>>>> This is not required, because when we call audio_codec_enable we know
>>>>> which connector it was called for, so pass the state.
>>>>>
>>>>> This also removes having to look at crtc->config.
>>>>>
>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>> ---
>>>>>  drivers/gpu/drm/i915/intel_audio.c | 17 ++++++++++-------
>>>>>  drivers/gpu/drm/i915/intel_ddi.c   |  2 +-
>>>>>  drivers/gpu/drm/i915/intel_dp.c    | 11 ++++++-----
>>>>>  drivers/gpu/drm/i915/intel_drv.h   |  4 +++-
>>>>>  drivers/gpu/drm/i915/intel_hdmi.c  |  2 +-
>>>>>  5 files changed, 21 insertions(+), 15 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>>>>> index 7093cfbb62b1..63ef25893c7e 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_audio.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>>>>> @@ -485,23 +485,26 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>>>>>  /**
>>>>>   * intel_audio_codec_enable - Enable the audio codec for HD audio
>>>>>   * @intel_encoder: encoder on which to enable audio
>>>>> + * @crtc_state: pointer to the current crtc state.
>>>>> + * @conn_state: pointer to the current connector state.
>>>>>   *
>>>>>   * The enable sequences may only be performed after enabling the transcoder and
>>>>>   * port, and after completed link training.
>>>>>   */
>>>>> -void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
>>>>> +void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
>>>>> +			      const struct intel_crtc_state *crtc_state,
>>>>> +			      const struct drm_connector_state *conn_state)
>>>>>  {
>>>>>  	struct drm_encoder *encoder = &intel_encoder->base;
>>>>> -	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
>>>>> -	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
>>>>> +	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
>>>>>  	struct drm_connector *connector;
>>>>>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>>>>>  	struct i915_audio_component *acomp = dev_priv->audio_component;
>>>>>  	enum port port = intel_encoder->port;
>>>>> -	enum pipe pipe = crtc->pipe;
>>>>> +	enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);
>>>> While we may require that to be true, I'm not sure I like this use.
>>> I should say that otherwise I like this.
>> What do you mean with this comment?
> That the rest of the patch makes sense.
>
Sorry, I meant the first comment you wrote.


~Maarten
Ville Syrjälä Oct. 24, 2016, 10:17 a.m. UTC | #6
On Mon, Oct 24, 2016 at 12:12:59PM +0200, Maarten Lankhorst wrote:
> Op 24-10-16 om 12:04 schreef Ville Syrjälä:
> > On Mon, Oct 24, 2016 at 10:45:36AM +0200, Maarten Lankhorst wrote:
> >> Op 21-10-16 om 16:16 schreef Ville Syrjälä:
> >>> On Fri, Oct 21, 2016 at 05:04:46PM +0300, Ville Syrjälä wrote:
> >>>> On Wed, Oct 19, 2016 at 03:55:35PM +0200, Maarten Lankhorst wrote:
> >>>>> drm_select_eld requires mode_config.mutex and connection_mutex
> >>>>> because it looks at the connector list and at the legacy encoders.
> >>>>>
> >>>>> This is not required, because when we call audio_codec_enable we know
> >>>>> which connector it was called for, so pass the state.
> >>>>>
> >>>>> This also removes having to look at crtc->config.
> >>>>>
> >>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>>> ---
> >>>>>  drivers/gpu/drm/i915/intel_audio.c | 17 ++++++++++-------
> >>>>>  drivers/gpu/drm/i915/intel_ddi.c   |  2 +-
> >>>>>  drivers/gpu/drm/i915/intel_dp.c    | 11 ++++++-----
> >>>>>  drivers/gpu/drm/i915/intel_drv.h   |  4 +++-
> >>>>>  drivers/gpu/drm/i915/intel_hdmi.c  |  2 +-
> >>>>>  5 files changed, 21 insertions(+), 15 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> >>>>> index 7093cfbb62b1..63ef25893c7e 100644
> >>>>> --- a/drivers/gpu/drm/i915/intel_audio.c
> >>>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
> >>>>> @@ -485,23 +485,26 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
> >>>>>  /**
> >>>>>   * intel_audio_codec_enable - Enable the audio codec for HD audio
> >>>>>   * @intel_encoder: encoder on which to enable audio
> >>>>> + * @crtc_state: pointer to the current crtc state.
> >>>>> + * @conn_state: pointer to the current connector state.
> >>>>>   *
> >>>>>   * The enable sequences may only be performed after enabling the transcoder and
> >>>>>   * port, and after completed link training.
> >>>>>   */
> >>>>> -void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
> >>>>> +void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
> >>>>> +			      const struct intel_crtc_state *crtc_state,
> >>>>> +			      const struct drm_connector_state *conn_state)
> >>>>>  {
> >>>>>  	struct drm_encoder *encoder = &intel_encoder->base;
> >>>>> -	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> >>>>> -	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
> >>>>> +	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
> >>>>>  	struct drm_connector *connector;
> >>>>>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
> >>>>>  	struct i915_audio_component *acomp = dev_priv->audio_component;
> >>>>>  	enum port port = intel_encoder->port;
> >>>>> -	enum pipe pipe = crtc->pipe;
> >>>>> +	enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);
> >>>> While we may require that to be true, I'm not sure I like this use.
> >>> I should say that otherwise I like this.
> >> What do you mean with this comment?
> > That the rest of the patch makes sense.
> >
> Sorry, I meant the first comment you wrote.

I mean that 'enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);'
is not something that's done anywhere else. So it looks totally foreign.
Maarten Lankhorst Oct. 24, 2016, 10:47 a.m. UTC | #7
Op 24-10-16 om 12:17 schreef Ville Syrjälä:
> On Mon, Oct 24, 2016 at 12:12:59PM +0200, Maarten Lankhorst wrote:
>> Op 24-10-16 om 12:04 schreef Ville Syrjälä:
>>> On Mon, Oct 24, 2016 at 10:45:36AM +0200, Maarten Lankhorst wrote:
>>>> Op 21-10-16 om 16:16 schreef Ville Syrjälä:
>>>>> On Fri, Oct 21, 2016 at 05:04:46PM +0300, Ville Syrjälä wrote:
>>>>>> On Wed, Oct 19, 2016 at 03:55:35PM +0200, Maarten Lankhorst wrote:
>>>>>>> drm_select_eld requires mode_config.mutex and connection_mutex
>>>>>>> because it looks at the connector list and at the legacy encoders.
>>>>>>>
>>>>>>> This is not required, because when we call audio_codec_enable we know
>>>>>>> which connector it was called for, so pass the state.
>>>>>>>
>>>>>>> This also removes having to look at crtc->config.
>>>>>>>
>>>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>>>> ---
>>>>>>>  drivers/gpu/drm/i915/intel_audio.c | 17 ++++++++++-------
>>>>>>>  drivers/gpu/drm/i915/intel_ddi.c   |  2 +-
>>>>>>>  drivers/gpu/drm/i915/intel_dp.c    | 11 ++++++-----
>>>>>>>  drivers/gpu/drm/i915/intel_drv.h   |  4 +++-
>>>>>>>  drivers/gpu/drm/i915/intel_hdmi.c  |  2 +-
>>>>>>>  5 files changed, 21 insertions(+), 15 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>>>>>>> index 7093cfbb62b1..63ef25893c7e 100644
>>>>>>> --- a/drivers/gpu/drm/i915/intel_audio.c
>>>>>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>>>>>>> @@ -485,23 +485,26 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>>>>>>>  /**
>>>>>>>   * intel_audio_codec_enable - Enable the audio codec for HD audio
>>>>>>>   * @intel_encoder: encoder on which to enable audio
>>>>>>> + * @crtc_state: pointer to the current crtc state.
>>>>>>> + * @conn_state: pointer to the current connector state.
>>>>>>>   *
>>>>>>>   * The enable sequences may only be performed after enabling the transcoder and
>>>>>>>   * port, and after completed link training.
>>>>>>>   */
>>>>>>> -void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
>>>>>>> +void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
>>>>>>> +			      const struct intel_crtc_state *crtc_state,
>>>>>>> +			      const struct drm_connector_state *conn_state)
>>>>>>>  {
>>>>>>>  	struct drm_encoder *encoder = &intel_encoder->base;
>>>>>>> -	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
>>>>>>> -	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
>>>>>>> +	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
>>>>>>>  	struct drm_connector *connector;
>>>>>>>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>>>>>>>  	struct i915_audio_component *acomp = dev_priv->audio_component;
>>>>>>>  	enum port port = intel_encoder->port;
>>>>>>> -	enum pipe pipe = crtc->pipe;
>>>>>>> +	enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);
>>>>>> While we may require that to be true, I'm not sure I like this use.
>>>>> I should say that otherwise I like this.
>>>> What do you mean with this comment?
>>> That the rest of the patch makes sense.
>>>
>> Sorry, I meant the first comment you wrote.
> I mean that 'enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);'
> is not something that's done anywhere else. So it looks totally foreign.
>
Not directly I guess. Some places already assume that drm_crtc_index == pipe.
But it's ok when I change it to to_intel_crtc(crtc)->pipe?

~Maarten
Ville Syrjälä Oct. 24, 2016, 11:41 a.m. UTC | #8
On Mon, Oct 24, 2016 at 12:47:21PM +0200, Maarten Lankhorst wrote:
> Op 24-10-16 om 12:17 schreef Ville Syrjälä:
> > On Mon, Oct 24, 2016 at 12:12:59PM +0200, Maarten Lankhorst wrote:
> >> Op 24-10-16 om 12:04 schreef Ville Syrjälä:
> >>> On Mon, Oct 24, 2016 at 10:45:36AM +0200, Maarten Lankhorst wrote:
> >>>> Op 21-10-16 om 16:16 schreef Ville Syrjälä:
> >>>>> On Fri, Oct 21, 2016 at 05:04:46PM +0300, Ville Syrjälä wrote:
> >>>>>> On Wed, Oct 19, 2016 at 03:55:35PM +0200, Maarten Lankhorst wrote:
> >>>>>>> drm_select_eld requires mode_config.mutex and connection_mutex
> >>>>>>> because it looks at the connector list and at the legacy encoders.
> >>>>>>>
> >>>>>>> This is not required, because when we call audio_codec_enable we know
> >>>>>>> which connector it was called for, so pass the state.
> >>>>>>>
> >>>>>>> This also removes having to look at crtc->config.
> >>>>>>>
> >>>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>>>>> ---
> >>>>>>>  drivers/gpu/drm/i915/intel_audio.c | 17 ++++++++++-------
> >>>>>>>  drivers/gpu/drm/i915/intel_ddi.c   |  2 +-
> >>>>>>>  drivers/gpu/drm/i915/intel_dp.c    | 11 ++++++-----
> >>>>>>>  drivers/gpu/drm/i915/intel_drv.h   |  4 +++-
> >>>>>>>  drivers/gpu/drm/i915/intel_hdmi.c  |  2 +-
> >>>>>>>  5 files changed, 21 insertions(+), 15 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> >>>>>>> index 7093cfbb62b1..63ef25893c7e 100644
> >>>>>>> --- a/drivers/gpu/drm/i915/intel_audio.c
> >>>>>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
> >>>>>>> @@ -485,23 +485,26 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
> >>>>>>>  /**
> >>>>>>>   * intel_audio_codec_enable - Enable the audio codec for HD audio
> >>>>>>>   * @intel_encoder: encoder on which to enable audio
> >>>>>>> + * @crtc_state: pointer to the current crtc state.
> >>>>>>> + * @conn_state: pointer to the current connector state.
> >>>>>>>   *
> >>>>>>>   * The enable sequences may only be performed after enabling the transcoder and
> >>>>>>>   * port, and after completed link training.
> >>>>>>>   */
> >>>>>>> -void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
> >>>>>>> +void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
> >>>>>>> +			      const struct intel_crtc_state *crtc_state,
> >>>>>>> +			      const struct drm_connector_state *conn_state)
> >>>>>>>  {
> >>>>>>>  	struct drm_encoder *encoder = &intel_encoder->base;
> >>>>>>> -	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
> >>>>>>> -	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
> >>>>>>> +	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
> >>>>>>>  	struct drm_connector *connector;
> >>>>>>>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
> >>>>>>>  	struct i915_audio_component *acomp = dev_priv->audio_component;
> >>>>>>>  	enum port port = intel_encoder->port;
> >>>>>>> -	enum pipe pipe = crtc->pipe;
> >>>>>>> +	enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);
> >>>>>> While we may require that to be true, I'm not sure I like this use.
> >>>>> I should say that otherwise I like this.
> >>>> What do you mean with this comment?
> >>> That the rest of the patch makes sense.
> >>>
> >> Sorry, I meant the first comment you wrote.
> > I mean that 'enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);'
> > is not something that's done anywhere else. So it looks totally foreign.
> >
> Not directly I guess. Some places already assume that drm_crtc_index == pipe.
> But it's ok when I change it to to_intel_crtc(crtc)->pipe?

Yes.

If we wanted to, I guess we could just do

static inline enum pipe intel_crtc_pipe(crtc)
{
	return drm_crtc_index(&crtc->base);
}

and just nuke crtc->pipe entirely.

And then we get to hope that the hw people aren't going to allow fusing
off pipes in some random order (eg. just fuse off pipe B on a three pipe
platform). That would obviously break this scheme.
Maarten Lankhorst Oct. 25, 2016, 11 a.m. UTC | #9
Op 24-10-16 om 13:41 schreef Ville Syrjälä:
> On Mon, Oct 24, 2016 at 12:47:21PM +0200, Maarten Lankhorst wrote:
>> Op 24-10-16 om 12:17 schreef Ville Syrjälä:
>>> On Mon, Oct 24, 2016 at 12:12:59PM +0200, Maarten Lankhorst wrote:
>>>> Op 24-10-16 om 12:04 schreef Ville Syrjälä:
>>>>> On Mon, Oct 24, 2016 at 10:45:36AM +0200, Maarten Lankhorst wrote:
>>>>>> Op 21-10-16 om 16:16 schreef Ville Syrjälä:
>>>>>>> On Fri, Oct 21, 2016 at 05:04:46PM +0300, Ville Syrjälä wrote:
>>>>>>>> On Wed, Oct 19, 2016 at 03:55:35PM +0200, Maarten Lankhorst wrote:
>>>>>>>>> drm_select_eld requires mode_config.mutex and connection_mutex
>>>>>>>>> because it looks at the connector list and at the legacy encoders.
>>>>>>>>>
>>>>>>>>> This is not required, because when we call audio_codec_enable we know
>>>>>>>>> which connector it was called for, so pass the state.
>>>>>>>>>
>>>>>>>>> This also removes having to look at crtc->config.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>>>>>> ---
>>>>>>>>>  drivers/gpu/drm/i915/intel_audio.c | 17 ++++++++++-------
>>>>>>>>>  drivers/gpu/drm/i915/intel_ddi.c   |  2 +-
>>>>>>>>>  drivers/gpu/drm/i915/intel_dp.c    | 11 ++++++-----
>>>>>>>>>  drivers/gpu/drm/i915/intel_drv.h   |  4 +++-
>>>>>>>>>  drivers/gpu/drm/i915/intel_hdmi.c  |  2 +-
>>>>>>>>>  5 files changed, 21 insertions(+), 15 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>>>>>>>>> index 7093cfbb62b1..63ef25893c7e 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/intel_audio.c
>>>>>>>>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>>>>>>>>> @@ -485,23 +485,26 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>>>>>>>>>  /**
>>>>>>>>>   * intel_audio_codec_enable - Enable the audio codec for HD audio
>>>>>>>>>   * @intel_encoder: encoder on which to enable audio
>>>>>>>>> + * @crtc_state: pointer to the current crtc state.
>>>>>>>>> + * @conn_state: pointer to the current connector state.
>>>>>>>>>   *
>>>>>>>>>   * The enable sequences may only be performed after enabling the transcoder and
>>>>>>>>>   * port, and after completed link training.
>>>>>>>>>   */
>>>>>>>>> -void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
>>>>>>>>> +void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
>>>>>>>>> +			      const struct intel_crtc_state *crtc_state,
>>>>>>>>> +			      const struct drm_connector_state *conn_state)
>>>>>>>>>  {
>>>>>>>>>  	struct drm_encoder *encoder = &intel_encoder->base;
>>>>>>>>> -	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
>>>>>>>>> -	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
>>>>>>>>> +	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
>>>>>>>>>  	struct drm_connector *connector;
>>>>>>>>>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>>>>>>>>>  	struct i915_audio_component *acomp = dev_priv->audio_component;
>>>>>>>>>  	enum port port = intel_encoder->port;
>>>>>>>>> -	enum pipe pipe = crtc->pipe;
>>>>>>>>> +	enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);
>>>>>>>> While we may require that to be true, I'm not sure I like this use.
>>>>>>> I should say that otherwise I like this.
>>>>>> What do you mean with this comment?
>>>>> That the rest of the patch makes sense.
>>>>>
>>>> Sorry, I meant the first comment you wrote.
>>> I mean that 'enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);'
>>> is not something that's done anywhere else. So it looks totally foreign.
>>>
>> Not directly I guess. Some places already assume that drm_crtc_index == pipe.
>> But it's ok when I change it to to_intel_crtc(crtc)->pipe?
> Yes.
>
> If we wanted to, I guess we could just do
>
> static inline enum pipe intel_crtc_pipe(crtc)
> {
> 	return drm_crtc_index(&crtc->base);
> }
>
> and just nuke crtc->pipe entirely.
>
> And then we get to hope that the hw people aren't going to allow fusing
> off pipes in some random order (eg. just fuse off pipe B on a three pipe
> platform). That would obviously break this scheme.
>
That would already cause subtle bugs right now. Lets hope it never happens. :)

~Maarten
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 7093cfbb62b1..63ef25893c7e 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -485,23 +485,26 @@  static void ilk_audio_codec_enable(struct drm_connector *connector,
 /**
  * intel_audio_codec_enable - Enable the audio codec for HD audio
  * @intel_encoder: encoder on which to enable audio
+ * @crtc_state: pointer to the current crtc state.
+ * @conn_state: pointer to the current connector state.
  *
  * The enable sequences may only be performed after enabling the transcoder and
  * port, and after completed link training.
  */
-void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
+void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
+			      const struct intel_crtc_state *crtc_state,
+			      const struct drm_connector_state *conn_state)
 {
 	struct drm_encoder *encoder = &intel_encoder->base;
-	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
-	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
+	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
 	struct drm_connector *connector;
 	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
 	struct i915_audio_component *acomp = dev_priv->audio_component;
 	enum port port = intel_encoder->port;
-	enum pipe pipe = crtc->pipe;
+	enum pipe pipe = drm_crtc_index(crtc_state->base.crtc);
 
-	connector = drm_select_eld(encoder);
-	if (!connector)
+	connector = conn_state->connector;
+	if (!connector || !connector->eld[0])
 		return;
 
 	DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
@@ -512,7 +515,7 @@  void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
 
 	/* ELD Conn_Type */
 	connector->eld[5] &= ~(3 << 2);
-	if (intel_crtc_has_dp_encoder(crtc->config))
+	if (intel_crtc_has_dp_encoder(crtc_state))
 		connector->eld[5] |= (1 << 2);
 
 	connector->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 7f7741c1406d..6748279310a4 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1824,7 +1824,7 @@  static void intel_enable_ddi(struct intel_encoder *intel_encoder,
 
 	if (intel_crtc->config->has_audio) {
 		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
-		intel_audio_codec_enable(intel_encoder);
+		intel_audio_codec_enable(intel_encoder, pipe_config, conn_state);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 88f3b745a326..6046f0f54cb3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2734,7 +2734,8 @@  static void intel_dp_enable_port(struct intel_dp *intel_dp,
 }
 
 static void intel_enable_dp(struct intel_encoder *encoder,
-			    struct intel_crtc_state *pipe_config)
+			    struct intel_crtc_state *pipe_config,
+			    struct drm_connector_state *conn_state)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	struct drm_device *dev = encoder->base.dev;
@@ -2776,7 +2777,7 @@  static void intel_enable_dp(struct intel_encoder *encoder,
 	if (pipe_config->has_audio) {
 		DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
 				 pipe_name(pipe));
-		intel_audio_codec_enable(encoder);
+		intel_audio_codec_enable(encoder, pipe_config, conn_state);
 	}
 }
 
@@ -2786,7 +2787,7 @@  static void g4x_enable_dp(struct intel_encoder *encoder,
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
-	intel_enable_dp(encoder, pipe_config);
+	intel_enable_dp(encoder, pipe_config, conn_state);
 	intel_edp_backlight_on(intel_dp);
 }
 
@@ -2923,7 +2924,7 @@  static void vlv_pre_enable_dp(struct intel_encoder *encoder,
 {
 	vlv_phy_pre_encoder_enable(encoder);
 
-	intel_enable_dp(encoder, pipe_config);
+	intel_enable_dp(encoder, pipe_config, conn_state);
 }
 
 static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder,
@@ -2941,7 +2942,7 @@  static void chv_pre_enable_dp(struct intel_encoder *encoder,
 {
 	chv_phy_pre_encoder_enable(encoder);
 
-	intel_enable_dp(encoder, pipe_config);
+	intel_enable_dp(encoder, pipe_config, conn_state);
 
 	/* Second common lane will stay alive on its own now */
 	chv_phy_release_cl2_override(encoder);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5760420ace61..e3c426563589 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1192,7 +1192,9 @@  u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
 
 /* intel_audio.c */
 void intel_init_audio_hooks(struct drm_i915_private *dev_priv);
-void intel_audio_codec_enable(struct intel_encoder *encoder);
+void intel_audio_codec_enable(struct intel_encoder *encoder,
+			      const struct intel_crtc_state *crtc_state,
+			      const struct drm_connector_state *conn_state);
 void intel_audio_codec_disable(struct intel_encoder *encoder);
 void i915_audio_component_init(struct drm_i915_private *dev_priv);
 void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ef43244a884f..fd6439a2882b 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -984,7 +984,7 @@  static void intel_enable_hdmi_audio(struct intel_encoder *encoder,
 	WARN_ON(!crtc->config->has_hdmi_sink);
 	DRM_DEBUG_DRIVER("Enabling HDMI audio on pipe %c\n",
 			 pipe_name(crtc->pipe));
-	intel_audio_codec_enable(encoder);
+	intel_audio_codec_enable(encoder, pipe_config, conn_state);
 }
 
 static void g4x_enable_hdmi(struct intel_encoder *encoder,