| Message ID | 20211024155010.126275-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Add NO_VLV_DISP_PW_DPIO_CMN_BC_INIT quirk | expand |
On Sun, 24 Oct 2021, Hans de Goede <hdegoede@redhat.com> 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. > > 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 > ------------[ cut here ]------------ > i915 0000:00:02.0: DPIO read pipe A reg 0x8170 == 0xffffffff > WARNING: CPU: 3 PID: 258 at drivers/gpu/drm/i915/intel_sideband.c:257 vlv_dpio_read+0x95/0xb0 [i915] > ... > Call Trace: > chv_dpio_cmn_power_well_enable+0xab/0x210 [i915] > __intel_display_power_get_domain.part.0+0xa0/0xc0 [i915] > intel_power_domains_init_hw+0x26d/0x760 [i915] > intel_modeset_init_noirq+0x5d/0x270 [i915] > i915_driver_probe+0x6b6/0xd10 [i915] > ... > > If I disable the WARN about the register being 0xffffffff, so that the > system can log some more dmesg output over the serial console before > freezing, the following errors are also logged: > > i915 0000:00:02.0: [drm] *ERROR* timeout setting power well state 00000000 (fcfff3ff) > i915 0000:00:02.0: [drm] *ERROR* Display PHY 1 is not power up > > With this patch to disable the force-enabling of the PHY 0 / dpio-common-bc > powerwell in place, this error for PHY 1 goes away. So it seems that trying > the force-enabling of the PHY 0 / dpio-common-bc powerwell freezes the > P-Unit, causing the subsequent enabling of PHY 1 to also fail (and causing > the entire system to freeze within seconds). > > With this patch the PHY 1 error disappears and the entire system works. > > Note this change also moves the intel_init_quirks() call a bit up inside > intel_modeset_init_noirq() this is necessary so that the quirk is set > before the intel_power_domains_init_hw() call. This is harmless, all that > intel_init_quirks() does is set some bits in drm_i915_private.quirks and > make some drm_info() log calls. > > Reported-by: Tsuchiya Yuto <kitakar@gmail.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 4 ++-- > .../gpu/drm/i915/display/intel_display_power.c | 16 ++++++++++++++-- > drivers/gpu/drm/i915/display/intel_quirks.c | 10 ++++++++++ > drivers/gpu/drm/i915/i915_drv.h | 1 + > 4 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 015854b5078c..1fb885cc86c9 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -12467,6 +12467,8 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915) > if (ret) > goto cleanup_bios; > > + intel_init_quirks(i915); > + > /* FIXME: completely on the wrong abstraction layer */ > intel_power_domains_init_hw(i915, false); > > @@ -12501,8 +12503,6 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915) > INIT_WORK(&i915->atomic_helper.free_work, > intel_atomic_helper_free_state_worker); > > - intel_init_quirks(i915); > - > intel_fbc_init(i915); > > return 0; > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index cce1a926fcc1..eeaba3dc064b 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -2090,8 +2090,14 @@ __intel_display_power_get_domain(struct drm_i915_private *dev_priv, > if (intel_display_power_grab_async_put_ref(dev_priv, domain)) > return; > > - for_each_power_domain_well(dev_priv, power_well, BIT_ULL(domain)) > + for_each_power_domain_well(dev_priv, power_well, BIT_ULL(domain)) { > + if (domain == POWER_DOMAIN_INIT && > + (dev_priv->quirks & QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT) && > + power_well->desc->id == VLV_DISP_PW_DPIO_CMN_BC) > + continue; > + > intel_power_well_get(dev_priv, power_well); > + } Cc: Imre There has got to be a way to hide this better. Having this here is unacceptable. BR, Jani. > > power_domains->domain_use_count[domain]++; > } > @@ -2184,8 +2190,14 @@ __intel_display_power_put_domain(struct drm_i915_private *dev_priv, > > power_domains->domain_use_count[domain]--; > > - for_each_power_domain_well_reverse(dev_priv, power_well, BIT_ULL(domain)) > + for_each_power_domain_well_reverse(dev_priv, power_well, BIT_ULL(domain)) { > + if (domain == POWER_DOMAIN_INIT && > + (dev_priv->quirks & QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT) && > + power_well->desc->id == VLV_DISP_PW_DPIO_CMN_BC) > + continue; > + > intel_power_well_put(dev_priv, power_well); > + } > } > > static void __intel_display_power_put(struct drm_i915_private *dev_priv, > diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c > index 8a52b7a16774..c377f515bbf4 100644 > --- a/drivers/gpu/drm/i915/display/intel_quirks.c > +++ b/drivers/gpu/drm/i915/display/intel_quirks.c > @@ -59,6 +59,13 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915) > drm_info(&i915->drm, "Applying no pps backlight power quirk\n"); > } > > +/* The Xiaomi Mi Pad 2 CHT tablet hangs on enabling the dpio-common-bc well */ > +static void quirk_no_vlv_disp_pw_dpio_cmn_bc_init(struct drm_i915_private *i915) > +{ > + i915->quirks |= QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT; > + drm_info(&i915->drm, "Applying no dpio-common-bc powerwell init quirk\n"); > +} > + > struct intel_quirk { > int device; > int subsystem_vendor; > @@ -190,6 +197,9 @@ static struct intel_quirk intel_quirks[] = { > /* ASRock ITX*/ > { 0x3185, 0x1849, 0x2212, quirk_increase_ddi_disabled_time }, > { 0x3184, 0x1849, 0x2212, quirk_increase_ddi_disabled_time }, > + > + /* Xiaomi Mi Pad 2 */ > + { 0x22b0, 0x1d72, 0x1502, quirk_no_vlv_disp_pw_dpio_cmn_bc_init }, > }; > > void intel_init_quirks(struct drm_i915_private *i915) > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 005b1cec7007..b907b49b4f0e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -524,6 +524,7 @@ struct i915_drrs { > #define QUIRK_INCREASE_T12_DELAY (1<<6) > #define QUIRK_INCREASE_DDI_DISABLED_TIME (1<<7) > #define QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK (1<<8) > +#define QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT (1<<9) > > struct intel_fbdev; > struct intel_fbc_work;
Hi, On 10/25/21 10:25, Jani Nikula wrote: > On Sun, 24 Oct 2021, Hans de Goede <hdegoede@redhat.com> 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. >> >> 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 >> ------------[ cut here ]------------ >> i915 0000:00:02.0: DPIO read pipe A reg 0x8170 == 0xffffffff >> WARNING: CPU: 3 PID: 258 at drivers/gpu/drm/i915/intel_sideband.c:257 vlv_dpio_read+0x95/0xb0 [i915] >> ... >> Call Trace: >> chv_dpio_cmn_power_well_enable+0xab/0x210 [i915] >> __intel_display_power_get_domain.part.0+0xa0/0xc0 [i915] >> intel_power_domains_init_hw+0x26d/0x760 [i915] >> intel_modeset_init_noirq+0x5d/0x270 [i915] >> i915_driver_probe+0x6b6/0xd10 [i915] >> ... >> >> If I disable the WARN about the register being 0xffffffff, so that the >> system can log some more dmesg output over the serial console before >> freezing, the following errors are also logged: >> >> i915 0000:00:02.0: [drm] *ERROR* timeout setting power well state 00000000 (fcfff3ff) >> i915 0000:00:02.0: [drm] *ERROR* Display PHY 1 is not power up >> >> With this patch to disable the force-enabling of the PHY 0 / dpio-common-bc >> powerwell in place, this error for PHY 1 goes away. So it seems that trying >> the force-enabling of the PHY 0 / dpio-common-bc powerwell freezes the >> P-Unit, causing the subsequent enabling of PHY 1 to also fail (and causing >> the entire system to freeze within seconds). >> >> With this patch the PHY 1 error disappears and the entire system works. >> >> Note this change also moves the intel_init_quirks() call a bit up inside >> intel_modeset_init_noirq() this is necessary so that the quirk is set >> before the intel_power_domains_init_hw() call. This is harmless, all that >> intel_init_quirks() does is set some bits in drm_i915_private.quirks and >> make some drm_info() log calls. >> >> Reported-by: Tsuchiya Yuto <kitakar@gmail.com> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 4 ++-- >> .../gpu/drm/i915/display/intel_display_power.c | 16 ++++++++++++++-- >> drivers/gpu/drm/i915/display/intel_quirks.c | 10 ++++++++++ >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> 4 files changed, 27 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> index 015854b5078c..1fb885cc86c9 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -12467,6 +12467,8 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915) >> if (ret) >> goto cleanup_bios; >> >> + intel_init_quirks(i915); >> + >> /* FIXME: completely on the wrong abstraction layer */ >> intel_power_domains_init_hw(i915, false); >> >> @@ -12501,8 +12503,6 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915) >> INIT_WORK(&i915->atomic_helper.free_work, >> intel_atomic_helper_free_state_worker); >> >> - intel_init_quirks(i915); >> - >> intel_fbc_init(i915); >> >> return 0; >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c >> index cce1a926fcc1..eeaba3dc064b 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_power.c >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c >> @@ -2090,8 +2090,14 @@ __intel_display_power_get_domain(struct drm_i915_private *dev_priv, >> if (intel_display_power_grab_async_put_ref(dev_priv, domain)) >> return; >> >> - for_each_power_domain_well(dev_priv, power_well, BIT_ULL(domain)) >> + for_each_power_domain_well(dev_priv, power_well, BIT_ULL(domain)) { >> + if (domain == POWER_DOMAIN_INIT && >> + (dev_priv->quirks & QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT) && >> + power_well->desc->id == VLV_DISP_PW_DPIO_CMN_BC) >> + continue; >> + >> intel_power_well_get(dev_priv, power_well); >> + } > > Cc: Imre > > There has got to be a way to hide this better. Having this here is > unacceptable. Thank you for your quick review. For a first quick hack I just removed POWER_DOMAIN_INIT from CHV_DPIO_CMN_BC_POWER_DOMAINS. Some alternative ideas: 1. Mask out the POWER_DOMAIN_INIT bit from desc->domains But that requires making the desc struct non const; Or storing a copy of the domains field in struct i915_power_well and masking it out in the copy (and make for_each_power_domain_well use the copy). 2. Have a separate desc without POWER_DOMAIN_INIT and patch up the power_well->desc pointer in intel_power_domains_init(). I believe that 1. will be slightly cleaner (with the domains mask copy added to struct i915_power_well). If this sounds like it might be acceptable I can give 1. a shot (or 2 if that is preferred). So does 1. sound acceptable ? Regards, Hans >> power_domains->domain_use_count[domain]++; >> } >> @@ -2184,8 +2190,14 @@ __intel_display_power_put_domain(struct drm_i915_private *dev_priv, >> >> power_domains->domain_use_count[domain]--; >> >> - for_each_power_domain_well_reverse(dev_priv, power_well, BIT_ULL(domain)) >> + for_each_power_domain_well_reverse(dev_priv, power_well, BIT_ULL(domain)) { >> + if (domain == POWER_DOMAIN_INIT && >> + (dev_priv->quirks & QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT) && >> + power_well->desc->id == VLV_DISP_PW_DPIO_CMN_BC) >> + continue; >> + >> intel_power_well_put(dev_priv, power_well); >> + } >> } >> >> static void __intel_display_power_put(struct drm_i915_private *dev_priv, >> diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c >> index 8a52b7a16774..c377f515bbf4 100644 >> --- a/drivers/gpu/drm/i915/display/intel_quirks.c >> +++ b/drivers/gpu/drm/i915/display/intel_quirks.c >> @@ -59,6 +59,13 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915) >> drm_info(&i915->drm, "Applying no pps backlight power quirk\n"); >> } >> >> +/* The Xiaomi Mi Pad 2 CHT tablet hangs on enabling the dpio-common-bc well */ >> +static void quirk_no_vlv_disp_pw_dpio_cmn_bc_init(struct drm_i915_private *i915) >> +{ >> + i915->quirks |= QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT; >> + drm_info(&i915->drm, "Applying no dpio-common-bc powerwell init quirk\n"); >> +} >> + >> struct intel_quirk { >> int device; >> int subsystem_vendor; >> @@ -190,6 +197,9 @@ static struct intel_quirk intel_quirks[] = { >> /* ASRock ITX*/ >> { 0x3185, 0x1849, 0x2212, quirk_increase_ddi_disabled_time }, >> { 0x3184, 0x1849, 0x2212, quirk_increase_ddi_disabled_time }, >> + >> + /* Xiaomi Mi Pad 2 */ >> + { 0x22b0, 0x1d72, 0x1502, quirk_no_vlv_disp_pw_dpio_cmn_bc_init }, >> }; >> >> void intel_init_quirks(struct drm_i915_private *i915) >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 005b1cec7007..b907b49b4f0e 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -524,6 +524,7 @@ struct i915_drrs { >> #define QUIRK_INCREASE_T12_DELAY (1<<6) >> #define QUIRK_INCREASE_DDI_DISABLED_TIME (1<<7) >> #define QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK (1<<8) >> +#define QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT (1<<9) >> >> struct intel_fbdev; >> struct intel_fbc_work; >
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. > > 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;
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
On Wed, Oct 27, 2021 at 08:39:57PM +0200, Hans de Goede wrote: > 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 ? The power wells are hierarchical. Also power wells != power domains.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 015854b5078c..1fb885cc86c9 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -12467,6 +12467,8 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915) if (ret) goto cleanup_bios; + intel_init_quirks(i915); + /* FIXME: completely on the wrong abstraction layer */ intel_power_domains_init_hw(i915, false); @@ -12501,8 +12503,6 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915) INIT_WORK(&i915->atomic_helper.free_work, intel_atomic_helper_free_state_worker); - intel_init_quirks(i915); - intel_fbc_init(i915); return 0; diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index cce1a926fcc1..eeaba3dc064b 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -2090,8 +2090,14 @@ __intel_display_power_get_domain(struct drm_i915_private *dev_priv, if (intel_display_power_grab_async_put_ref(dev_priv, domain)) return; - for_each_power_domain_well(dev_priv, power_well, BIT_ULL(domain)) + for_each_power_domain_well(dev_priv, power_well, BIT_ULL(domain)) { + if (domain == POWER_DOMAIN_INIT && + (dev_priv->quirks & QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT) && + power_well->desc->id == VLV_DISP_PW_DPIO_CMN_BC) + continue; + intel_power_well_get(dev_priv, power_well); + } power_domains->domain_use_count[domain]++; } @@ -2184,8 +2190,14 @@ __intel_display_power_put_domain(struct drm_i915_private *dev_priv, power_domains->domain_use_count[domain]--; - for_each_power_domain_well_reverse(dev_priv, power_well, BIT_ULL(domain)) + for_each_power_domain_well_reverse(dev_priv, power_well, BIT_ULL(domain)) { + if (domain == POWER_DOMAIN_INIT && + (dev_priv->quirks & QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT) && + power_well->desc->id == VLV_DISP_PW_DPIO_CMN_BC) + continue; + intel_power_well_put(dev_priv, power_well); + } } static void __intel_display_power_put(struct drm_i915_private *dev_priv, diff --git a/drivers/gpu/drm/i915/display/intel_quirks.c b/drivers/gpu/drm/i915/display/intel_quirks.c index 8a52b7a16774..c377f515bbf4 100644 --- a/drivers/gpu/drm/i915/display/intel_quirks.c +++ b/drivers/gpu/drm/i915/display/intel_quirks.c @@ -59,6 +59,13 @@ static void quirk_no_pps_backlight_power_hook(struct drm_i915_private *i915) drm_info(&i915->drm, "Applying no pps backlight power quirk\n"); } +/* The Xiaomi Mi Pad 2 CHT tablet hangs on enabling the dpio-common-bc well */ +static void quirk_no_vlv_disp_pw_dpio_cmn_bc_init(struct drm_i915_private *i915) +{ + i915->quirks |= QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT; + drm_info(&i915->drm, "Applying no dpio-common-bc powerwell init quirk\n"); +} + struct intel_quirk { int device; int subsystem_vendor; @@ -190,6 +197,9 @@ static struct intel_quirk intel_quirks[] = { /* ASRock ITX*/ { 0x3185, 0x1849, 0x2212, quirk_increase_ddi_disabled_time }, { 0x3184, 0x1849, 0x2212, quirk_increase_ddi_disabled_time }, + + /* Xiaomi Mi Pad 2 */ + { 0x22b0, 0x1d72, 0x1502, quirk_no_vlv_disp_pw_dpio_cmn_bc_init }, }; void intel_init_quirks(struct drm_i915_private *i915) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 005b1cec7007..b907b49b4f0e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -524,6 +524,7 @@ struct i915_drrs { #define QUIRK_INCREASE_T12_DELAY (1<<6) #define QUIRK_INCREASE_DDI_DISABLED_TIME (1<<7) #define QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK (1<<8) +#define QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT (1<<9) struct intel_fbdev; struct intel_fbc_work;
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. 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 ------------[ cut here ]------------ i915 0000:00:02.0: DPIO read pipe A reg 0x8170 == 0xffffffff WARNING: CPU: 3 PID: 258 at drivers/gpu/drm/i915/intel_sideband.c:257 vlv_dpio_read+0x95/0xb0 [i915] ... Call Trace: chv_dpio_cmn_power_well_enable+0xab/0x210 [i915] __intel_display_power_get_domain.part.0+0xa0/0xc0 [i915] intel_power_domains_init_hw+0x26d/0x760 [i915] intel_modeset_init_noirq+0x5d/0x270 [i915] i915_driver_probe+0x6b6/0xd10 [i915] ... If I disable the WARN about the register being 0xffffffff, so that the system can log some more dmesg output over the serial console before freezing, the following errors are also logged: i915 0000:00:02.0: [drm] *ERROR* timeout setting power well state 00000000 (fcfff3ff) i915 0000:00:02.0: [drm] *ERROR* Display PHY 1 is not power up With this patch to disable the force-enabling of the PHY 0 / dpio-common-bc powerwell in place, this error for PHY 1 goes away. So it seems that trying the force-enabling of the PHY 0 / dpio-common-bc powerwell freezes the P-Unit, causing the subsequent enabling of PHY 1 to also fail (and causing the entire system to freeze within seconds). With this patch the PHY 1 error disappears and the entire system works. Note this change also moves the intel_init_quirks() call a bit up inside intel_modeset_init_noirq() this is necessary so that the quirk is set before the intel_power_domains_init_hw() call. This is harmless, all that intel_init_quirks() does is set some bits in drm_i915_private.quirks and make some drm_info() log calls. Reported-by: Tsuchiya Yuto <kitakar@gmail.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/gpu/drm/i915/display/intel_display.c | 4 ++-- .../gpu/drm/i915/display/intel_display_power.c | 16 ++++++++++++++-- drivers/gpu/drm/i915/display/intel_quirks.c | 10 ++++++++++ drivers/gpu/drm/i915/i915_drv.h | 1 + 4 files changed, 27 insertions(+), 4 deletions(-)