Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

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

On Fri, Jan 27, 2017 at 08:47:50AM -0600, Pierre-Louis Bossart wrote:
> Thanks Jani and Ville for the comments. Couple of precisions needed below:
> >>
> >>>   #define GEN6_BSD_RNCID			_MMIO(0x12198)
> >>>   
> >>>   #define GEN7_FF_THREAD_MODE		_MMIO(0x20a0)
> >>> diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> >>> index 245523e..b3134ef 100644
> >>> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> >>> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> >>> @@ -248,6 +248,15 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv)
> >>>   		goto err_free_irq;
> >>>   	}
> >>>   
> >>> +	/* Enable DPAudio debug bits by default */
> >>> +	if (IS_CHERRYVIEW(dev_priv)) {
> > VLV too. And like I said we might need this in the powerwell code as
> > well. You should make a test to see if the register value is retained
> > across the display power well being turned off. Eg. simply disable all
> > displays, check the log to make sure it really did turn off the display
> > power well, then re-enable some displays, and finally check if the
> > register value was retained or not).
> VLV has DisplayPort?

Yes. And as I said earlier the docs don't make any distinciton between
HDMI and DP in the audio programming sequence. So based on the docs we
should always unmute the amp manually.

>I thought this was an addition on CHT, and I really 
> don't know of any devices with this combination of HDaudio disabled+DP. 
> I'd rather keep this CHT-only until we find a device where this can be 
> tested.
> 
> On the powerwell, I could use more guidance. i tried this first solution 
> to see if streaming worked (and it did :-)). I don't mind moving the 
> code somewhere else but I have no idea where.

1. boot with drm.debug=0xe
1. 'intel_reg read 0x62f38' just after boot, to see that the
   chicken bit is as it should
2. disable all displays (eg. xset dpms force off)
3. grep 'power_well.*display' (make sure the last thing it says is
   "disabling"
4. turn on the displays (eg. xset dpms force on)
5. 'intel_reg read 0x62f38' to check the chicken bit again

If it didn't survive then we need to set it in either in
vlv_display_power_well_init(), or we could just set it whenever
we set the AMP_MUTE bit for any of the ports.

>
> >>> +		u32 chicken_bit;
> >>> +
> >>> +		chicken_bit = I915_READ(VLV_AUD_CHICKEN_BIT_REG);
> >>> +		I915_WRITE(VLV_AUD_CHICKEN_BIT_REG,
> >>> +			   chicken_bit | CHICKEN_BIT_DBG_ENABLE);
> >>> +	}
> >>> +
> >>>   	return 0;
> >>>   err_free_irq:
> >>>   	irq_free_desc(dev_priv->lpe_audio.irq);
> >>> @@ -357,6 +366,24 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
> >>>   			pdata->tmds_clock_speed = tmds_clk_speed;
> >>>   		if (link_rate)
> >>>   			pdata->link_rate = link_rate;
> >>> +
> >>> +		if (dp_output) { /* unmute the amp */
> > The spec doesn't distinquish DP vs. HDMI here. So I presume we should be
> > able to do this always.
> 
> I'll try to see if HDMI still works with this. We could tentatively add 
> unmute in all cases but I'll need to add a test for Baytrail (no PORT_D) 
> so in the end it's the same number of tests.
> 
> >
> > And I think we might want to mute things again when disabling audio.
> I was wondering if there would be side effects of writing to a register 
> controlling a port if that port is not connected any longer. I'll give 
> it a try.

Nothing apart from HPD circuitry really cares if there's anything
connectect or not.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx