drm/i915/display: Trigger Modeset at boot for audio codec init

Commit Message

Shankar, Uma March 18, 2020, 11:30 a.m. UTC

If external monitors are connected during boot up, driver
uses the same mode programmed by BIOS and avoids a full modeset.
This results in display audio codec left uninitialized and
display audio fails to work till user triggers a modeset.

This patch fixes the same by triggering a modeset at boot.

Cc: Ville Syrjälä <[e-mail]>
Cc: Maarten Lankhorst <[e-mail]>
Cc: Kai Vehmanen <[e-mail]>
Signed-off-by: Uma Shankar <[e-mail]>
Signed-off-by: SweeAun Khor <[e-mail]>
---
 drivers/gpu/drm/i915/display/intel_ddi.c     | 4 ++++
 drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++++
 drivers/gpu/drm/i915/i915_drv.h              | 3 +++
 3 files changed, 15 insertions(+)

Comments

Gupta, Anshuman March 18, 2020, 12:07 p.m. UTC | #1

On 2020-03-18 at 17:00:09 +0530, Uma Shankar wrote:
> If external monitors are connected during boot up, driver
> uses the same mode programmed by BIOS and avoids a full modeset.
> This results in display audio codec left uninitialized and
> display audio fails to work till user triggers a modeset.
> 
> This patch fixes the same by triggering a modeset at boot.
> 
> Cc: Ville Syrjälä <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Kai Vehmanen <[e-mail]>
> Signed-off-by: Uma Shankar <[e-mail]>
> Signed-off-by: SweeAun Khor <[e-mail]>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 4 ++++
>  drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++++
>  drivers/gpu/drm/i915/i915_drv.h              | 3 +++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 73d0f4648c06..ba380afa73a6 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3704,6 +3704,10 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
>  				  const struct intel_crtc_state *crtc_state,
>  				  const struct drm_connector_state *conn_state)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> +	/* Clear the bootflag */
> +	dev_priv->bootflag = false;
>  
>  	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>  		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8f23c4d51c33..a1487539495f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14751,6 +14751,10 @@ static int intel_atomic_check(struct drm_device *dev,
>  		if (new_crtc_state->hw.mode.private_flags !=
>  		    old_crtc_state->hw.mode.private_flags)
>  			new_crtc_state->uapi.mode_changed = true;
> +
> +		/* Set mode_change to init audio code once at boot */
> +		if (dev_priv->bootflag && new_crtc_state->hw.active)
> +			new_crtc_state->uapi.mode_changed = true;
I would prefer to compute crtc_state->has_audio in compute_config at boot up 
 and then force a full modeset in intel_pipe_config_compare(), this way atomically we can force the
full modeset on boot up.

Thanks,
Anshuman Gupta.
>  	}
>  
>  	ret = drm_atomic_helper_check_modeset(dev, &state->base);
> @@ -17655,11 +17659,15 @@ static void intel_update_fdi_pll_freq(struct drm_i915_private *dev_priv)
>  
>  static int intel_initial_commit(struct drm_device *dev)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct drm_atomic_state *state = NULL;
>  	struct drm_modeset_acquire_ctx ctx;
>  	struct intel_crtc *crtc;
>  	int ret = 0;
>  
> +	/* Set Flag to trigger modeset for audio codec init */
> +	dev_priv->bootflag = true;
> +
>  	state = drm_atomic_state_alloc(dev);
>  	if (!state)
>  		return -ENOMEM;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a7ea1d855359..207196f9632b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1210,6 +1210,9 @@ struct drm_i915_private {
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
>  	 */
> +
> +	/* Flag to trigger modeset for Audio codec init once during boot */
> +	bool bootflag;
>  };
>  
>  static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> [e-mail]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Maarten Lankhorst March 18, 2020, 12:15 p.m. UTC | #2

Op 18-03-2020 om 12:30 schreef Uma Shankar:
> If external monitors are connected during boot up, driver
> uses the same mode programmed by BIOS and avoids a full modeset.
> This results in display audio codec left uninitialized and
> display audio fails to work till user triggers a modeset.
>
> This patch fixes the same by triggering a modeset at boot.
>
> Cc: Ville Syrjälä <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Kai Vehmanen <[e-mail]>
> Signed-off-by: Uma Shankar <[e-mail]>
> Signed-off-by: SweeAun Khor <[e-mail]>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 4 ++++
>  drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++++
>  drivers/gpu/drm/i915/i915_drv.h              | 3 +++
>  3 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 73d0f4648c06..ba380afa73a6 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3704,6 +3704,10 @@ static void intel_ddi_update_pipe(struct intel_encoder *encoder,
>  				  const struct intel_crtc_state *crtc_state,
>  				  const struct drm_connector_state *conn_state)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> +	/* Clear the bootflag */
> +	dev_priv->bootflag = false;
>  
>  	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>  		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8f23c4d51c33..a1487539495f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14751,6 +14751,10 @@ static int intel_atomic_check(struct drm_device *dev,
>  		if (new_crtc_state->hw.mode.private_flags !=
>  		    old_crtc_state->hw.mode.private_flags)
>  			new_crtc_state->uapi.mode_changed = true;
> +
> +		/* Set mode_change to init audio code once at boot */
> +		if (dev_priv->bootflag && new_crtc_state->hw.active)
> +			new_crtc_state->uapi.mode_changed = true;
>  	}
>  
>  	ret = drm_atomic_helper_check_modeset(dev, &state->base);
> @@ -17655,11 +17659,15 @@ static void intel_update_fdi_pll_freq(struct drm_i915_private *dev_priv)
>  
>  static int intel_initial_commit(struct drm_device *dev)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct drm_atomic_state *state = NULL;
>  	struct drm_modeset_acquire_ctx ctx;
>  	struct intel_crtc *crtc;
>  	int ret = 0;
>  
> +	/* Set Flag to trigger modeset for audio codec init */
> +	dev_priv->bootflag = true;
> +
>  	state = drm_atomic_state_alloc(dev);
>  	if (!state)
>  		return -ENOMEM;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a7ea1d855359..207196f9632b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1210,6 +1210,9 @@ struct drm_i915_private {
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
>  	 */
> +
> +	/* Flag to trigger modeset for Audio codec init once during boot */
> +	bool bootflag;
>  };
>  
>  static inline struct drm_i915_private *to_i915(const struct drm_device *dev)

This is not going to help. We read out the has_audio flag now, the only thing we miss is to enable the audio codec. This should be done after the first detect(), in the manner which I described to get the correct eld values.

~Maarten

Shankar, Uma March 18, 2020, 2:02 p.m. UTC | #3

> -----Original Message-----
> From: Gupta, Anshuman <[e-mail]>
> Sent: Wednesday, March 18, 2020 5:37 PM
> To: Shankar, Uma <[e-mail]>
> Cc: [e-mail]; Khor, Swee Aun <[e-mail]>
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio
> codec init
> 
> On 2020-03-18 at 17:00:09 +0530, Uma Shankar wrote:
> > If external monitors are connected during boot up, driver uses the
> > same mode programmed by BIOS and avoids a full modeset.
> > This results in display audio codec left uninitialized and display
> > audio fails to work till user triggers a modeset.
> >
> > This patch fixes the same by triggering a modeset at boot.
> >
> > Cc: Ville Syrjälä <[e-mail]>
> > Cc: Maarten Lankhorst <[e-mail]>
> > Cc: Kai Vehmanen <[e-mail]>
> > Signed-off-by: Uma Shankar <[e-mail]>
> > Signed-off-by: SweeAun Khor <[e-mail]>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c     | 4 ++++
> >  drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++++
> >  drivers/gpu/drm/i915/i915_drv.h              | 3 +++
> >  3 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 73d0f4648c06..ba380afa73a6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3704,6 +3704,10 @@ static void intel_ddi_update_pipe(struct intel_encoder
> *encoder,
> >  				  const struct intel_crtc_state *crtc_state,
> >  				  const struct drm_connector_state *conn_state)  {
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +
> > +	/* Clear the bootflag */
> > +	dev_priv->bootflag = false;
> >
> >  	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> >  		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state); diff
> > --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 8f23c4d51c33..a1487539495f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -14751,6 +14751,10 @@ static int intel_atomic_check(struct drm_device
> *dev,
> >  		if (new_crtc_state->hw.mode.private_flags !=
> >  		    old_crtc_state->hw.mode.private_flags)
> >  			new_crtc_state->uapi.mode_changed = true;
> > +
> > +		/* Set mode_change to init audio code once at boot */
> > +		if (dev_priv->bootflag && new_crtc_state->hw.active)
> > +			new_crtc_state->uapi.mode_changed = true;
> I would prefer to compute crtc_state->has_audio in compute_config at boot up  and
> then force a full modeset in intel_pipe_config_compare(), this way atomically we
> can force the full modeset on boot up.

Unfortunately the audio state is not yet detected here at bootup (read of EDID not yet done),
hence EDID data is not available. We tried to explore this path but this didn't worked out. We
forced modeset at intel_initial_commit but due to the above reason, things didn't worked out.

Regards,
Uma Shankar

> Thanks,
> Anshuman Gupta.
> >  	}
> >
> >  	ret = drm_atomic_helper_check_modeset(dev, &state->base); @@
> > -17655,11 +17659,15 @@ static void intel_update_fdi_pll_freq(struct
> > drm_i915_private *dev_priv)
> >
> >  static int intel_initial_commit(struct drm_device *dev)  {
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct drm_atomic_state *state = NULL;
> >  	struct drm_modeset_acquire_ctx ctx;
> >  	struct intel_crtc *crtc;
> >  	int ret = 0;
> >
> > +	/* Set Flag to trigger modeset for audio codec init */
> > +	dev_priv->bootflag = true;
> > +
> >  	state = drm_atomic_state_alloc(dev);
> >  	if (!state)
> >  		return -ENOMEM;
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index a7ea1d855359..207196f9632b
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1210,6 +1210,9 @@ struct drm_i915_private {
> >  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> >  	 * will be rejected. Instead look for a better place.
> >  	 */
> > +
> > +	/* Flag to trigger modeset for Audio codec init once during boot */
> > +	bool bootflag;
> >  };
> >
> >  static inline struct drm_i915_private *to_i915(const struct
> > drm_device *dev)
> > --
> > 2.22.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > [e-mail]
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Shankar, Uma March 18, 2020, 2:37 p.m. UTC | #4

> -----Original Message-----
> From: Maarten Lankhorst <[e-mail]>
> Sent: Wednesday, March 18, 2020 5:46 PM
> To: Shankar, Uma <[e-mail]>; [e-mail]
> Cc: Ville Syrjälä <[e-mail]>; Kai Vehmanen
> <[e-mail]>; Khor, Swee Aun <[e-mail]>
> Subject: Re: [PATCH] drm/i915/display: Trigger Modeset at boot for audio codec init
> 
> Op 18-03-2020 om 12:30 schreef Uma Shankar:
> > If external monitors are connected during boot up, driver uses the
> > same mode programmed by BIOS and avoids a full modeset.
> > This results in display audio codec left uninitialized and display
> > audio fails to work till user triggers a modeset.
> >
> > This patch fixes the same by triggering a modeset at boot.
> >
> > Cc: Ville Syrjälä <[e-mail]>
> > Cc: Maarten Lankhorst <[e-mail]>
> > Cc: Kai Vehmanen <[e-mail]>
> > Signed-off-by: Uma Shankar <[e-mail]>
> > Signed-off-by: SweeAun Khor <[e-mail]>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c     | 4 ++++
> >  drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++++
> >  drivers/gpu/drm/i915/i915_drv.h              | 3 +++
> >  3 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 73d0f4648c06..ba380afa73a6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3704,6 +3704,10 @@ static void intel_ddi_update_pipe(struct intel_encoder
> *encoder,
> >  				  const struct intel_crtc_state *crtc_state,
> >  				  const struct drm_connector_state *conn_state)  {
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +
> > +	/* Clear the bootflag */
> > +	dev_priv->bootflag = false;
> >
> >  	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> >  		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state); diff
> > --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 8f23c4d51c33..a1487539495f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -14751,6 +14751,10 @@ static int intel_atomic_check(struct drm_device
> *dev,
> >  		if (new_crtc_state->hw.mode.private_flags !=
> >  		    old_crtc_state->hw.mode.private_flags)
> >  			new_crtc_state->uapi.mode_changed = true;
> > +
> > +		/* Set mode_change to init audio code once at boot */
> > +		if (dev_priv->bootflag && new_crtc_state->hw.active)
> > +			new_crtc_state->uapi.mode_changed = true;
> >  	}
> >
> >  	ret = drm_atomic_helper_check_modeset(dev, &state->base); @@
> > -17655,11 +17659,15 @@ static void intel_update_fdi_pll_freq(struct
> > drm_i915_private *dev_priv)
> >
> >  static int intel_initial_commit(struct drm_device *dev)  {
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct drm_atomic_state *state = NULL;
> >  	struct drm_modeset_acquire_ctx ctx;
> >  	struct intel_crtc *crtc;
> >  	int ret = 0;
> >
> > +	/* Set Flag to trigger modeset for audio codec init */
> > +	dev_priv->bootflag = true;
> > +
> >  	state = drm_atomic_state_alloc(dev);
> >  	if (!state)
> >  		return -ENOMEM;
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index a7ea1d855359..207196f9632b
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1210,6 +1210,9 @@ struct drm_i915_private {
> >  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> >  	 * will be rejected. Instead look for a better place.
> >  	 */
> > +
> > +	/* Flag to trigger modeset for Audio codec init once during boot */
> > +	bool bootflag;
> >  };
> >
> >  static inline struct drm_i915_private *to_i915(const struct
> > drm_device *dev)
> 
> This is not going to help. We read out the has_audio flag now, the only thing we miss
> is to enable the audio codec. This should be done after the first detect(), in the
> manner which I described to get the correct eld values.

Hi Maarten,
At the time of sanitize, we don't have edid so has_audio flag will not be set. Also codec_enable
needs encoder, crtc_state, conn_state. All these are not there as part of detect callbacks.

With current approach, we force a modeset just during boot and then reset the flag so that normal
operations don't get affected.

Regards,
Uma Shankar

> ~Maarten

Maarten Lankhorst March 19, 2020, 12:11 p.m. UTC | #5

Op 18-03-2020 om 15:37 schreef Shankar, Uma:
>
>> -----Original Message-----
>> From: Maarten Lankhorst <[e-mail]>
>> Sent: Wednesday, March 18, 2020 5:46 PM
>> To: Shankar, Uma <[e-mail]>; [e-mail]
>> Cc: Ville Syrjälä <[e-mail]>; Kai Vehmanen
>> <[e-mail]>; Khor, Swee Aun <[e-mail]>
>> Subject: Re: [PATCH] drm/i915/display: Trigger Modeset at boot for audio codec init
>>
>> Op 18-03-2020 om 12:30 schreef Uma Shankar:
>>> If external monitors are connected during boot up, driver uses the
>>> same mode programmed by BIOS and avoids a full modeset.
>>> This results in display audio codec left uninitialized and display
>>> audio fails to work till user triggers a modeset.
>>>
>>> This patch fixes the same by triggering a modeset at boot.
>>>
>>> Cc: Ville Syrjälä <[e-mail]>
>>> Cc: Maarten Lankhorst <[e-mail]>
>>> Cc: Kai Vehmanen <[e-mail]>
>>> Signed-off-by: Uma Shankar <[e-mail]>
>>> Signed-off-by: SweeAun Khor <[e-mail]>
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_ddi.c     | 4 ++++
>>>  drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++++
>>>  drivers/gpu/drm/i915/i915_drv.h              | 3 +++
>>>  3 files changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
>>> b/drivers/gpu/drm/i915/display/intel_ddi.c
>>> index 73d0f4648c06..ba380afa73a6 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>>> @@ -3704,6 +3704,10 @@ static void intel_ddi_update_pipe(struct intel_encoder
>> *encoder,
>>>  				  const struct intel_crtc_state *crtc_state,
>>>  				  const struct drm_connector_state *conn_state)  {
>>> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>> +
>>> +	/* Clear the bootflag */
>>> +	dev_priv->bootflag = false;
>>>
>>>  	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>>>  		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state); diff
>>> --git a/drivers/gpu/drm/i915/display/intel_display.c
>>> b/drivers/gpu/drm/i915/display/intel_display.c
>>> index 8f23c4d51c33..a1487539495f 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>> @@ -14751,6 +14751,10 @@ static int intel_atomic_check(struct drm_device
>> *dev,
>>>  		if (new_crtc_state->hw.mode.private_flags !=
>>>  		    old_crtc_state->hw.mode.private_flags)
>>>  			new_crtc_state->uapi.mode_changed = true;
>>> +
>>> +		/* Set mode_change to init audio code once at boot */
>>> +		if (dev_priv->bootflag && new_crtc_state->hw.active)
>>> +			new_crtc_state->uapi.mode_changed = true;
>>>  	}
>>>
>>>  	ret = drm_atomic_helper_check_modeset(dev, &state->base); @@
>>> -17655,11 +17659,15 @@ static void intel_update_fdi_pll_freq(struct
>>> drm_i915_private *dev_priv)
>>>
>>>  static int intel_initial_commit(struct drm_device *dev)  {
>>> +	struct drm_i915_private *dev_priv = to_i915(dev);
>>>  	struct drm_atomic_state *state = NULL;
>>>  	struct drm_modeset_acquire_ctx ctx;
>>>  	struct intel_crtc *crtc;
>>>  	int ret = 0;
>>>
>>> +	/* Set Flag to trigger modeset for audio codec init */
>>> +	dev_priv->bootflag = true;
>>> +
>>>  	state = drm_atomic_state_alloc(dev);
>>>  	if (!state)
>>>  		return -ENOMEM;
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>> b/drivers/gpu/drm/i915/i915_drv.h index a7ea1d855359..207196f9632b
>>> 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -1210,6 +1210,9 @@ struct drm_i915_private {
>>>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>>>  	 * will be rejected. Instead look for a better place.
>>>  	 */
>>> +
>>> +	/* Flag to trigger modeset for Audio codec init once during boot */
>>> +	bool bootflag;
>>>  };
>>>
>>>  static inline struct drm_i915_private *to_i915(const struct
>>> drm_device *dev)
>> This is not going to help. We read out the has_audio flag now, the only thing we miss
>> is to enable the audio codec. This should be done after the first detect(), in the
>> manner which I described to get the correct eld values.
> Hi Maarten,
> At the time of sanitize, we don't have edid so has_audio flag will not be set. Also codec_enable
> needs encoder, crtc_state, conn_state. All these are not there as part of detect callbacks.

They can be used. You have connection_mutex, so you can look at connector->state,

if you then lock conn_state->crtc, you can inspect conn_state->crtc->state as well.

conn_state->encoder is available as well.

It needs some casting to get the intel_XX_state but you can do it safely.

~Maarten

> With current approach, we force a modeset just during boot and then reset the flag so that normal
> operations don't get affected.
>
> Regards,
> Uma Shankar
>
>> ~Maarten

Souza, Jose March 19, 2020, 7:05 p.m. UTC | #6

On Wed, 2020-03-18 at 17:00 +0530, Uma Shankar wrote:
> If external monitors are connected during boot up, driver
> uses the same mode programmed by BIOS and avoids a full modeset.
> This results in display audio codec left uninitialized and
> display audio fails to work till user triggers a modeset.
> 
> This patch fixes the same by triggering a modeset at boot.

We had the same issue for PSR, take a look to the fix:
commit 33e059a2e4df454359f642f2235af39de9d3e914
drm/i915/psr: Force PSR probe only after full initialization

Maybe make this even more generic.

> 
> Cc: Ville Syrjälä <[e-mail]>
> Cc: Maarten Lankhorst <[e-mail]>
> Cc: Kai Vehmanen <[e-mail]>
> Signed-off-by: Uma Shankar <[e-mail]>
> Signed-off-by: SweeAun Khor <[e-mail]>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 4 ++++
>  drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++++
>  drivers/gpu/drm/i915/i915_drv.h              | 3 +++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 73d0f4648c06..ba380afa73a6 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3704,6 +3704,10 @@ static void intel_ddi_update_pipe(struct
> intel_encoder *encoder,
>  				  const struct intel_crtc_state
> *crtc_state,
>  				  const struct drm_connector_state
> *conn_state)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> +	/* Clear the bootflag */
> +	dev_priv->bootflag = false;
>  
>  	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>  		intel_ddi_update_pipe_dp(encoder, crtc_state,
> conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 8f23c4d51c33..a1487539495f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14751,6 +14751,10 @@ static int intel_atomic_check(struct
> drm_device *dev,
>  		if (new_crtc_state->hw.mode.private_flags !=
>  		    old_crtc_state->hw.mode.private_flags)
>  			new_crtc_state->uapi.mode_changed = true;
> +
> +		/* Set mode_change to init audio code once at boot */
> +		if (dev_priv->bootflag && new_crtc_state->hw.active)
> +			new_crtc_state->uapi.mode_changed = true;
>  	}
>  
>  	ret = drm_atomic_helper_check_modeset(dev, &state->base);
> @@ -17655,11 +17659,15 @@ static void
> intel_update_fdi_pll_freq(struct drm_i915_private *dev_priv)
>  
>  static int intel_initial_commit(struct drm_device *dev)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct drm_atomic_state *state = NULL;
>  	struct drm_modeset_acquire_ctx ctx;
>  	struct intel_crtc *crtc;
>  	int ret = 0;
>  
> +	/* Set Flag to trigger modeset for audio codec init */
> +	dev_priv->bootflag = true;
> +
>  	state = drm_atomic_state_alloc(dev);
>  	if (!state)
>  		return -ENOMEM;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index a7ea1d855359..207196f9632b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1210,6 +1210,9 @@ struct drm_i915_private {
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here.
> Your patch
>  	 * will be rejected. Instead look for a better place.
>  	 */
> +
> +	/* Flag to trigger modeset for Audio codec init once during
> boot */
> +	bool bootflag;
>  };
>  
>  static inline struct drm_i915_private *to_i915(const struct
> drm_device *dev)

Shankar, Uma March 20, 2020, 6:19 a.m. UTC | #7

> -----Original Message-----
> From: Souza, Jose <[e-mail]>
> Sent: Friday, March 20, 2020 12:36 AM
> To: Shankar, Uma <[e-mail]>; [e-mail]
> Cc: Khor, Swee Aun <[e-mail]>
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio
> codec init
> 
> On Wed, 2020-03-18 at 17:00 +0530, Uma Shankar wrote:
> > If external monitors are connected during boot up, driver uses the
> > same mode programmed by BIOS and avoids a full modeset.
> > This results in display audio codec left uninitialized and display
> > audio fails to work till user triggers a modeset.
> >
> > This patch fixes the same by triggering a modeset at boot.
> 
> We had the same issue for PSR, take a look to the fix:
> commit 33e059a2e4df454359f642f2235af39de9d3e914
> drm/i915/psr: Force PSR probe only after full initialization
> 
> Maybe make this even more generic.

Yeah this looks to dealing with almost a similar need. Thanks for pointing this out,
will try to come up with a generalized solution.

Regards,
Uma Shankar

> >
> > Cc: Ville Syrjälä <[e-mail]>
> > Cc: Maarten Lankhorst <[e-mail]>
> > Cc: Kai Vehmanen <[e-mail]>
> > Signed-off-by: Uma Shankar <[e-mail]>
> > Signed-off-by: SweeAun Khor <[e-mail]>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c     | 4 ++++
> >  drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++++
> >  drivers/gpu/drm/i915/i915_drv.h              | 3 +++
> >  3 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 73d0f4648c06..ba380afa73a6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3704,6 +3704,10 @@ static void intel_ddi_update_pipe(struct
> > intel_encoder *encoder,
> >  				  const struct intel_crtc_state
> > *crtc_state,
> >  				  const struct drm_connector_state
> > *conn_state)
> >  {
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +
> > +	/* Clear the bootflag */
> > +	dev_priv->bootflag = false;
> >
> >  	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> >  		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state); diff
> > --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 8f23c4d51c33..a1487539495f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -14751,6 +14751,10 @@ static int intel_atomic_check(struct
> > drm_device *dev,
> >  		if (new_crtc_state->hw.mode.private_flags !=
> >  		    old_crtc_state->hw.mode.private_flags)
> >  			new_crtc_state->uapi.mode_changed = true;
> > +
> > +		/* Set mode_change to init audio code once at boot */
> > +		if (dev_priv->bootflag && new_crtc_state->hw.active)
> > +			new_crtc_state->uapi.mode_changed = true;
> >  	}
> >
> >  	ret = drm_atomic_helper_check_modeset(dev, &state->base); @@
> > -17655,11 +17659,15 @@ static void intel_update_fdi_pll_freq(struct
> > drm_i915_private *dev_priv)
> >
> >  static int intel_initial_commit(struct drm_device *dev)  {
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct drm_atomic_state *state = NULL;
> >  	struct drm_modeset_acquire_ctx ctx;
> >  	struct intel_crtc *crtc;
> >  	int ret = 0;
> >
> > +	/* Set Flag to trigger modeset for audio codec init */
> > +	dev_priv->bootflag = true;
> > +
> >  	state = drm_atomic_state_alloc(dev);
> >  	if (!state)
> >  		return -ENOMEM;
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index a7ea1d855359..207196f9632b
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1210,6 +1210,9 @@ struct drm_i915_private {
> >  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here.
> > Your patch
> >  	 * will be rejected. Instead look for a better place.
> >  	 */
> > +
> > +	/* Flag to trigger modeset for Audio codec init once during
> > boot */
> > +	bool bootflag;
> >  };
> >
> >  static inline struct drm_i915_private *to_i915(const struct
> > drm_device *dev)

Ville Syrjälä March 20, 2020, 3:24 p.m. UTC | #8

On Fri, Mar 20, 2020 at 06:19:37AM +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Souza, Jose <[e-mail]>
> > Sent: Friday, March 20, 2020 12:36 AM
> > To: Shankar, Uma <[e-mail]>; [e-mail]
> > Cc: Khor, Swee Aun <[e-mail]>
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio
> > codec init
> > 
> > On Wed, 2020-03-18 at 17:00 +0530, Uma Shankar wrote:
> > > If external monitors are connected during boot up, driver uses the
> > > same mode programmed by BIOS and avoids a full modeset.
> > > This results in display audio codec left uninitialized and display
> > > audio fails to work till user triggers a modeset.
> > >
> > > This patch fixes the same by triggering a modeset at boot.
> > 
> > We had the same issue for PSR, take a look to the fix:
> > commit 33e059a2e4df454359f642f2235af39de9d3e914
> > drm/i915/psr: Force PSR probe only after full initialization
> > 
> > Maybe make this even more generic.
> 
> Yeah this looks to dealing with almost a similar need. Thanks for pointing this out,
> will try to come up with a generalized solution.

How about just fixing the uapi vs. hw fail I showed instead of adding
even more hacks?

Khor, Swee Aun March 20, 2020, 4:54 p.m. UTC | #9

Hi Ville,
You means this change right? Sure. Will try your suggestion as well. 
By the way, what is different between hw.mode and uapi.mode and how we know which to be used? It used to only base.mode before hw/uapi split patches. 

> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14671,8 +14671,8 @@ static int intel_atomic_check(struct drm_device *dev,
>         /* Catch I915_MODE_FLAG_INHERITED */
>         for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>                                             new_crtc_state, i) {
> -               if (new_crtc_state->hw.mode.private_flags !=
> -                   old_crtc_state->hw.mode.private_flags)
> +               if (new_crtc_state->uapi.mode.private_flags !=
> +                   old_crtc_state->uapi.mode.private_flags)
>                         new_crtc_state->uapi.mode_changed = true;
>         }
> 
> ?

Regards,
SweeAun

-----Original Message-----
From: Ville Syrjälä <[e-mail]> 
Sent: Friday, March 20, 2020 11:24 PM
To: Shankar, Uma <[e-mail]>
Cc: Souza, Jose <[e-mail]>; [e-mail]; Khor, Swee Aun <[e-mail]>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio codec init

On Fri, Mar 20, 2020 at 06:19:37AM +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Souza, Jose <[e-mail]>
> > Sent: Friday, March 20, 2020 12:36 AM
> > To: Shankar, Uma <[e-mail]>; 
> > [e-mail]
> > Cc: Khor, Swee Aun <[e-mail]>
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset 
> > at boot for audio codec init
> > 
> > On Wed, 2020-03-18 at 17:00 +0530, Uma Shankar wrote:
> > > If external monitors are connected during boot up, driver uses the 
> > > same mode programmed by BIOS and avoids a full modeset.
> > > This results in display audio codec left uninitialized and display 
> > > audio fails to work till user triggers a modeset.
> > >
> > > This patch fixes the same by triggering a modeset at boot.
> > 
> > We had the same issue for PSR, take a look to the fix:
> > commit 33e059a2e4df454359f642f2235af39de9d3e914
> > drm/i915/psr: Force PSR probe only after full initialization
> > 
> > Maybe make this even more generic.
> 
> Yeah this looks to dealing with almost a similar need. Thanks for 
> pointing this out, will try to come up with a generalized solution.

How about just fixing the uapi vs. hw fail I showed instead of adding even more hacks?

--
Ville Syrjälä
Intel

Khor, Swee Aun March 23, 2020, 2:29 p.m. UTC | #10

Hi Ville, 

You are right, your suggestion will fix this issue.

#Based on dmesg log, uapi mode private flags change is captured
...
[   11.404578] fbcon: i915drmfb (fb0) is primary device
[   11.404743] [drm] SA: intel_atomic_check: uapi change 
[   11.404744] [drm] SA2: intel_atomic_check: new_crtc_state->uapi.mode.private_flags= 0, old_crtc_state->uapi.mode.private_flags= 1  
[   11.404744] [drm] SA2: intel_atomic_check: new_crtc_state->uapi.mode.private_flags= 0, old_crtc_state->uapi.mode.private_flags= 0  
[   11.404745] [drm] SA2: intel_atomic_check: new_crtc_state->uapi.mode.private_flags= 0, old_crtc_state->uapi.mode.private_flags= 0  
[   11.404799] [drm:intel_atomic_check [i915]] [CONNECTOR:110:HDMI-A-2] Limiting display bpp to 24 instead of EDID bpp 24, requested bpp 36, max platform bpp 36
[   11.404855] [drm:intel_hdmi_compute_config [i915]] picking 8 bpc for HDMI output (pipe bpp: 24)
[   11.404898] [drm:intel_atomic_check [i915]] hw max bpp: 24, pipe bpp: 24, dithering: 0
...

#Here is the git diff 
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 4d1634ed6a1b..b5c56cd513d9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14108,11 +14108,15 @@ static int intel_atomic_check(struct drm_device *dev,
 	int ret, i;
 	bool any_ms = false;
 
+	DRM_INFO("SA: intel_atomic_check: uapi change \n");
+
 	/* Catch I915_MODE_FLAG_INHERITED */
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-		if (new_crtc_state->hw.mode.private_flags !=
-		    old_crtc_state->hw.mode.private_flags)
+
+		DRM_INFO("SA2: intel_atomic_check: new_crtc_state->uapi.mode.private_flags= %d, old_crtc_state->uapi.mode.private_flags= %d  \n", new_crtc_state->uapi.mode.private_flags, old_crtc_state->uapi.mode.private_flags );
+		if (new_crtc_state->uapi.mode.private_flags !=
+		    old_crtc_state->uapi.mode.private_flags)
 			new_crtc_state->uapi.mode_changed = true;
 	}

Regards,
SweeAun

-----Original Message-----
From: Khor, Swee Aun 
Sent: Saturday, March 21, 2020 12:55 AM
To: Ville Syrjälä <[e-mail]>; Shankar, Uma <[e-mail]>
Cc: Souza, Jose <[e-mail]>; [e-mail]
Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio codec init

Hi Ville,
You means this change right? Sure. Will try your suggestion as well. 
By the way, what is different between hw.mode and uapi.mode and how we know which to be used? It used to only base.mode before hw/uapi split patches. 

> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14671,8 +14671,8 @@ static int intel_atomic_check(struct drm_device *dev,
>         /* Catch I915_MODE_FLAG_INHERITED */
>         for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>                                             new_crtc_state, i) {
> -               if (new_crtc_state->hw.mode.private_flags !=
> -                   old_crtc_state->hw.mode.private_flags)
> +               if (new_crtc_state->uapi.mode.private_flags !=
> +                   old_crtc_state->uapi.mode.private_flags)
>                         new_crtc_state->uapi.mode_changed = true;
>         }
> 
> ?

Regards,
SweeAun

-----Original Message-----
From: Ville Syrjälä <[e-mail]>
Sent: Friday, March 20, 2020 11:24 PM
To: Shankar, Uma <[e-mail]>
Cc: Souza, Jose <[e-mail]>; [e-mail]; Khor, Swee Aun <[e-mail]>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio codec init

On Fri, Mar 20, 2020 at 06:19:37AM +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Souza, Jose <[e-mail]>
> > Sent: Friday, March 20, 2020 12:36 AM
> > To: Shankar, Uma <[e-mail]>; 
> > [e-mail]
> > Cc: Khor, Swee Aun <[e-mail]>
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset 
> > at boot for audio codec init
> > 
> > On Wed, 2020-03-18 at 17:00 +0530, Uma Shankar wrote:
> > > If external monitors are connected during boot up, driver uses the 
> > > same mode programmed by BIOS and avoids a full modeset.
> > > This results in display audio codec left uninitialized and display 
> > > audio fails to work till user triggers a modeset.
> > >
> > > This patch fixes the same by triggering a modeset at boot.
> > 
> > We had the same issue for PSR, take a look to the fix:
> > commit 33e059a2e4df454359f642f2235af39de9d3e914
> > drm/i915/psr: Force PSR probe only after full initialization
> > 
> > Maybe make this even more generic.
> 
> Yeah this looks to dealing with almost a similar need. Thanks for 
> pointing this out, will try to come up with a generalized solution.

How about just fixing the uapi vs. hw fail I showed instead of adding even more hacks?

--
Ville Syrjälä
Intel

Khor, Swee Aun March 24, 2020, 5:56 a.m. UTC | #11

Git diff without debug print. Please review. Thanks. 

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 4d1634ed6a1b..806cf622fb39 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14108,11 +14108,13 @@ static int intel_atomic_check(struct drm_device *dev,
        int ret, i;
        bool any_ms = false;
 
+
        /* Catch I915_MODE_FLAG_INHERITED */
        for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
                                            new_crtc_state, i) {
-               if (new_crtc_state->hw.mode.private_flags !=
-                   old_crtc_state->hw.mode.private_flags)
+
+               if (new_crtc_state->uapi.mode.private_flags !=
+                   old_crtc_state->uapi.mode.private_flags)
                        new_crtc_state->uapi.mode_changed = true;
        }

Regards,
SweeAun

-----Original Message-----
From: Khor, Swee Aun 
Sent: Monday, March 23, 2020 10:29 PM
To: 'Ville Syrjälä' <[e-mail]>; Shankar, Uma <[e-mail]>
Cc: Souza, Jose <[e-mail]>; '[e-mail]' <[e-mail]>
Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio codec init

Hi Ville, 

You are right, your suggestion will fix this issue.

#Based on dmesg log, uapi mode private flags change is captured ...
[   11.404578] fbcon: i915drmfb (fb0) is primary device
[   11.404743] [drm] SA: intel_atomic_check: uapi change 
[   11.404744] [drm] SA2: intel_atomic_check: new_crtc_state->uapi.mode.private_flags= 0, old_crtc_state->uapi.mode.private_flags= 1  
[   11.404744] [drm] SA2: intel_atomic_check: new_crtc_state->uapi.mode.private_flags= 0, old_crtc_state->uapi.mode.private_flags= 0  
[   11.404745] [drm] SA2: intel_atomic_check: new_crtc_state->uapi.mode.private_flags= 0, old_crtc_state->uapi.mode.private_flags= 0  
[   11.404799] [drm:intel_atomic_check [i915]] [CONNECTOR:110:HDMI-A-2] Limiting display bpp to 24 instead of EDID bpp 24, requested bpp 36, max platform bpp 36
[   11.404855] [drm:intel_hdmi_compute_config [i915]] picking 8 bpc for HDMI output (pipe bpp: 24)
[   11.404898] [drm:intel_atomic_check [i915]] hw max bpp: 24, pipe bpp: 24, dithering: 0
...

#Here is the git diff
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 4d1634ed6a1b..b5c56cd513d9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14108,11 +14108,15 @@ static int intel_atomic_check(struct drm_device *dev,
 	int ret, i;
 	bool any_ms = false;
 
+	DRM_INFO("SA: intel_atomic_check: uapi change \n");
+
 	/* Catch I915_MODE_FLAG_INHERITED */
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-		if (new_crtc_state->hw.mode.private_flags !=
-		    old_crtc_state->hw.mode.private_flags)
+
+		DRM_INFO("SA2: intel_atomic_check: new_crtc_state->uapi.mode.private_flags= %d, old_crtc_state->uapi.mode.private_flags= %d  \n", new_crtc_state->uapi.mode.private_flags, old_crtc_state->uapi.mode.private_flags );
+		if (new_crtc_state->uapi.mode.private_flags !=
+		    old_crtc_state->uapi.mode.private_flags)
 			new_crtc_state->uapi.mode_changed = true;
 	}

Regards,
SweeAun

-----Original Message-----
From: Khor, Swee Aun
Sent: Saturday, March 21, 2020 12:55 AM
To: Ville Syrjälä <[e-mail]>; Shankar, Uma <[e-mail]>
Cc: Souza, Jose <[e-mail]>; [e-mail]
Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio codec init

Hi Ville,
You means this change right? Sure. Will try your suggestion as well. 
By the way, what is different between hw.mode and uapi.mode and how we know which to be used? It used to only base.mode before hw/uapi split patches. 

> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14671,8 +14671,8 @@ static int intel_atomic_check(struct drm_device *dev,
>         /* Catch I915_MODE_FLAG_INHERITED */
>         for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>                                             new_crtc_state, i) {
> -               if (new_crtc_state->hw.mode.private_flags !=
> -                   old_crtc_state->hw.mode.private_flags)
> +               if (new_crtc_state->uapi.mode.private_flags !=
> +                   old_crtc_state->uapi.mode.private_flags)
>                         new_crtc_state->uapi.mode_changed = true;
>         }
> 
> ?

Regards,
SweeAun

-----Original Message-----
From: Ville Syrjälä <[e-mail]>
Sent: Friday, March 20, 2020 11:24 PM
To: Shankar, Uma <[e-mail]>
Cc: Souza, Jose <[e-mail]>; [e-mail]; Khor, Swee Aun <[e-mail]>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio codec init

On Fri, Mar 20, 2020 at 06:19:37AM +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Souza, Jose <[e-mail]>
> > Sent: Friday, March 20, 2020 12:36 AM
> > To: Shankar, Uma <[e-mail]>; 
> > [e-mail]
> > Cc: Khor, Swee Aun <[e-mail]>
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset 
> > at boot for audio codec init
> > 
> > On Wed, 2020-03-18 at 17:00 +0530, Uma Shankar wrote:
> > > If external monitors are connected during boot up, driver uses the 
> > > same mode programmed by BIOS and avoids a full modeset.
> > > This results in display audio codec left uninitialized and display 
> > > audio fails to work till user triggers a modeset.
> > >
> > > This patch fixes the same by triggering a modeset at boot.
> > 
> > We had the same issue for PSR, take a look to the fix:
> > commit 33e059a2e4df454359f642f2235af39de9d3e914
> > drm/i915/psr: Force PSR probe only after full initialization
> > 
> > Maybe make this even more generic.
> 
> Yeah this looks to dealing with almost a similar need. Thanks for 
> pointing this out, will try to come up with a generalized solution.

How about just fixing the uapi vs. hw fail I showed instead of adding even more hacks?

--
Ville Syrjälä
Intel

Kai Vehmanen March 24, 2020, 6:38 p.m. UTC | #12

Hey folks,

On Fri, 20 Mar 2020, Shankar, Uma wrote:
> Souza, Jose <[e-mail]> wrote:
> > On Wed, 2020-03-18 at 17:00 +0530, Uma Shankar wrote:
> > > This patch fixes the same by triggering a modeset at boot.
> > 
> > We had the same issue for PSR, take a look to the fix:
> > commit 33e059a2e4df454359f642f2235af39de9d3e914
> > drm/i915/psr: Force PSR probe only after full initialization
> > 
> > Maybe make this even more generic.
> 
> Yeah this looks to dealing with almost a similar need. Thanks for pointing this out,
> will try to come up with a generalized solution.

btw, there's an additional regression in the posted patch, reported 
by Michael Marley in:

"HDMI/DisplayPort audio does not work initially on boot with Linux 5.6 
(Bisected)"
https://gitlab.freedesktop.org/drm/intel/issues/1363

Br, Kai

Shankar, Uma March 26, 2020, 7:25 a.m. UTC | #13

> -----Original Message-----
> From: Khor, Swee Aun <[e-mail]>
> Sent: Tuesday, March 24, 2020 11:26 AM
> To: Ville Syrjälä <[e-mail]>; Shankar, Uma
> <[e-mail]>
> Cc: Souza, Jose <[e-mail]>; [e-mail]
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio
> codec init
> 
> Git diff without debug print. Please review. Thanks.

Will send this as a separate patch as this commit header doesn't hold good now.
Please review the change here: https://patchwork.freedesktop.org/series/75106/

Thanks Ville, Maarten and Jose for all your inputs. Thanks SweeAun for trying this at your end
and confirming the change suggested by Ville works to resolve audio codec issues.

Regards,
Uma Shankar

> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 4d1634ed6a1b..806cf622fb39 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14108,11 +14108,13 @@ static int intel_atomic_check(struct drm_device
> *dev,
>         int ret, i;
>         bool any_ms = false;
> 
> +
>         /* Catch I915_MODE_FLAG_INHERITED */
>         for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>                                             new_crtc_state, i) {
> -               if (new_crtc_state->hw.mode.private_flags !=
> -                   old_crtc_state->hw.mode.private_flags)
> +
> +               if (new_crtc_state->uapi.mode.private_flags !=
> +                   old_crtc_state->uapi.mode.private_flags)
>                         new_crtc_state->uapi.mode_changed = true;
>         }
> 
> Regards,
> SweeAun
> 
> -----Original Message-----
> From: Khor, Swee Aun
> Sent: Monday, March 23, 2020 10:29 PM
> To: 'Ville Syrjälä' <[e-mail]>; Shankar, Uma
> <[e-mail]>
> Cc: Souza, Jose <[e-mail]>; '[e-mail]' <intel-
> [e-mail]>
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio
> codec init
> 
> Hi Ville,
> 
> You are right, your suggestion will fix this issue.
> 
> #Based on dmesg log, uapi mode private flags change is captured ...
> [   11.404578] fbcon: i915drmfb (fb0) is primary device
> [   11.404743] [drm] SA: intel_atomic_check: uapi change
> [   11.404744] [drm] SA2: intel_atomic_check: new_crtc_state-
> >uapi.mode.private_flags= 0, old_crtc_state->uapi.mode.private_flags= 1
> [   11.404744] [drm] SA2: intel_atomic_check: new_crtc_state-
> >uapi.mode.private_flags= 0, old_crtc_state->uapi.mode.private_flags= 0
> [   11.404745] [drm] SA2: intel_atomic_check: new_crtc_state-
> >uapi.mode.private_flags= 0, old_crtc_state->uapi.mode.private_flags= 0
> [   11.404799] [drm:intel_atomic_check [i915]] [CONNECTOR:110:HDMI-A-2]
> Limiting display bpp to 24 instead of EDID bpp 24, requested bpp 36, max platform
> bpp 36
> [   11.404855] [drm:intel_hdmi_compute_config [i915]] picking 8 bpc for HDMI
> output (pipe bpp: 24)
> [   11.404898] [drm:intel_atomic_check [i915]] hw max bpp: 24, pipe bpp: 24,
> dithering: 0
> ...
> 
> #Here is the git diff
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 4d1634ed6a1b..b5c56cd513d9 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14108,11 +14108,15 @@ static int intel_atomic_check(struct drm_device
> *dev,  int ret, i;  bool any_ms = false;
> 
> +DRM_INFO("SA: intel_atomic_check: uapi change \n");
> +
>  /* Catch I915_MODE_FLAG_INHERITED */
>  for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>      new_crtc_state, i) {
> -if (new_crtc_state->hw.mode.private_flags !=
> -    old_crtc_state->hw.mode.private_flags)
> +
> +DRM_INFO("SA2: intel_atomic_check:
> +new_crtc_state->uapi.mode.private_flags= %d, old_crtc_state-
> >uapi.mode.private_flags= %d  \n", new_crtc_state->uapi.mode.private_flags,
> old_crtc_state->uapi.mode.private_flags ); if (new_crtc_state-
> >uapi.mode.private_flags !=
> +    old_crtc_state->uapi.mode.private_flags)
>  new_crtc_state->uapi.mode_changed = true;  }
> 
> Regards,
> SweeAun
> 
> -----Original Message-----
> From: Khor, Swee Aun
> Sent: Saturday, March 21, 2020 12:55 AM
> To: Ville Syrjälä <[e-mail]>; Shankar, Uma
> <[e-mail]>
> Cc: Souza, Jose <[e-mail]>; [e-mail]
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio
> codec init
> 
> Hi Ville,
> You means this change right? Sure. Will try your suggestion as well.
> By the way, what is different between hw.mode and uapi.mode and how we know
> which to be used? It used to only base.mode before hw/uapi split patches.
> 
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -14671,8 +14671,8 @@ static int intel_atomic_check(struct drm_device *dev,
> >         /* Catch I915_MODE_FLAG_INHERITED */
> >         for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> >                                             new_crtc_state, i) {
> > -               if (new_crtc_state->hw.mode.private_flags !=
> > -                   old_crtc_state->hw.mode.private_flags)
> > +               if (new_crtc_state->uapi.mode.private_flags !=
> > +                   old_crtc_state->uapi.mode.private_flags)
> >                         new_crtc_state->uapi.mode_changed = true;
> >         }
> >
> > ?
> 
> Regards,
> SweeAun
> 
> -----Original Message-----
> From: Ville Syrjälä <[e-mail]>
> Sent: Friday, March 20, 2020 11:24 PM
> To: Shankar, Uma <[e-mail]>
> Cc: Souza, Jose <[e-mail]>; [e-mail]; Khor,
> Swee Aun <[e-mail]>
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset at boot for audio
> codec init
> 
> On Fri, Mar 20, 2020 at 06:19:37AM +0000, Shankar, Uma wrote:
> >
> >
> > > -----Original Message-----
> > > From: Souza, Jose <[e-mail]>
> > > Sent: Friday, March 20, 2020 12:36 AM
> > > To: Shankar, Uma <[e-mail]>;
> > > [e-mail]
> > > Cc: Khor, Swee Aun <[e-mail]>
> > > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Trigger Modeset
> > > at boot for audio codec init
> > >
> > > On Wed, 2020-03-18 at 17:00 +0530, Uma Shankar wrote:
> > > > If external monitors are connected during boot up, driver uses the
> > > > same mode programmed by BIOS and avoids a full modeset.
> > > > This results in display audio codec left uninitialized and display
> > > > audio fails to work till user triggers a modeset.
> > > >
> > > > This patch fixes the same by triggering a modeset at boot.
> > >
> > > We had the same issue for PSR, take a look to the fix:
> > > commit 33e059a2e4df454359f642f2235af39de9d3e914
> > > drm/i915/psr: Force PSR probe only after full initialization
> > >
> > > Maybe make this even more generic.
> >
> > Yeah this looks to dealing with almost a similar need. Thanks for
> > pointing this out, will try to come up with a generalized solution.
> 
> How about just fixing the uapi vs. hw fail I showed instead of adding even more
> hacks?
> 
> --
> Ville Syrjälä
> Intel

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 73d0f4648c06..ba380afa73a6 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3704,6 +3704,10 @@  static void intel_ddi_update_pipe(struct intel_encoder *encoder,
 				  const struct intel_crtc_state *crtc_state,
 				  const struct drm_connector_state *conn_state)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
+	/* Clear the bootflag */
+	dev_priv->bootflag = false;
 
 	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		intel_ddi_update_pipe_dp(encoder, crtc_state, conn_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 8f23c4d51c33..a1487539495f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14751,6 +14751,10 @@  static int intel_atomic_check(struct drm_device *dev,
 		if (new_crtc_state->hw.mode.private_flags !=
 		    old_crtc_state->hw.mode.private_flags)
 			new_crtc_state->uapi.mode_changed = true;
+
+		/* Set mode_change to init audio code once at boot */
+		if (dev_priv->bootflag && new_crtc_state->hw.active)
+			new_crtc_state->uapi.mode_changed = true;
 	}
 
 	ret = drm_atomic_helper_check_modeset(dev, &state->base);
@@ -17655,11 +17659,15 @@  static void intel_update_fdi_pll_freq(struct drm_i915_private *dev_priv)
 
 static int intel_initial_commit(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_atomic_state *state = NULL;
 	struct drm_modeset_acquire_ctx ctx;
 	struct intel_crtc *crtc;
 	int ret = 0;
 
+	/* Set Flag to trigger modeset for audio codec init */
+	dev_priv->bootflag = true;
+
 	state = drm_atomic_state_alloc(dev);
 	if (!state)
 		return -ENOMEM;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a7ea1d855359..207196f9632b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1210,6 +1210,9 @@  struct drm_i915_private {
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
 	 */
+
+	/* Flag to trigger modeset for Audio codec init once during boot */
+	bool bootflag;
 };
 
 static inline struct drm_i915_private *to_i915(const struct drm_device *dev)