[Intel-gfx] [PATCH] drm/i915: Add NO_VLV_DISP_PW_DPIO_CMN_BC_INIT quirk

[Hans de Goede]

Hi,

On 10/27/21 15:38, Ville Syrjälä wrote:
> On Sun, Oct 24, 2021 at 05:50:10PM +0200, Hans de Goede wrote:
>> Add a NO_VLV_DISP_PW_DPIO_CMN_BC_INIT quirk to fix i915 not working on
>> the Xiaomi Mi Pad 2 (with CHT x5-Z8500 SoC).
>>
>> The Xiaomi Mi Pad 2 uses quite an unusual hardware-design for a Cherry
>> Trail tablet. It deviates from the typical reference design based tablets
>> in many ways.
>>
>> The Mi Pad 2 does not have any DisplayPort or HDMI outouts. I suspect that
>> as part of its unusual design it also has some supply rail which is only
>> used for DisplayPort or HDMI not connected.
> 
> Do we have the VBT somewhere (preferable attached to a bug report)?
> Maybe we can avoid an ugly quirk.

I agree that solving this in a way where we can avoid the quirk would be great.

I've filed an issue for this here now:

https://gitlab.freedesktop.org/drm/intel/-/issues/4385

This has a dump of /sys/kernel/debug/dri/0/i915_vbt as well as
dmesg output from a boot with drm.debug=0x1e attached (from a boot
with this patch, since otherwise the system hangs).

>>
>> Force-enabling the dpio-common-bc powerwell as the i915 normal does at boot
>> appears to cause the P-Unit to hang. When booting with a serial-usb console
>> the following errors are logged before the system freezes:
>>
>>  i915 0000:00:02.0: [drm] *ERROR* timeout setting power well state 00000000 (fffff3ff)
>>  i915 0000:00:02.0: [drm] *ERROR* Display PHY 0 is not power up
> 
> Hmm. I wonder if we're missing a clock or something...
> 
> Either of these do anything different?
> 
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1419,6 +1419,10 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
>  	for_each_pipe(dev_priv, pipe) {
>  		u32 val = intel_de_read(dev_priv, DPLL(pipe));
>  
> +		val |= DPLL_SSC_REF_CLK_CHV;
> 		or
> +		val &= ~DPLL_SSC_REF_CLK_CHV;
> 
>  		val |= DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
>  		if (pipe != PIPE_A)
>  			val |= DPLL_INTEGRATED_CRI_CLK_VLV;
> 

The hang gets triggered from chv_dpio_cmn_power_well_enable() which does not
call vlv_display_power_well_init() at all, it directly calls vlv_set_power_well()
without first calling vlv_display_power_well_init() .

Note the same goes for vlv_dpio_cmn_power_well_enable(). Only the
vlv_display_power_well_enable() / chv_pipe_power_well_enable() call
vlv_display_power_well_init().

Note I can still give the suggested change a try if you want,
the "display" powerwell is listed first and has DOMAIN_INIT set,
so assuming for_each_power_domain_well() goes through the domains in
the order they are listed, then vlv_display_power_well_init() will
still run first. But it would seem to be wrong if enabling one domain
depends on things setup by another domain ?

Regards,

Hans