| Message ID | 20210301154347.50052-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/display/vlv_dsi: Do no shut down displays on reboot if a DSI panel is used | expand |
Hi, On 3/1/21 4:43 PM, Hans de Goede wrote: > After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down > displays gracefully on reboot"), the DSI panel on a Cherry Trail based > Predia Basic tablet would no longer properly light up after reboot. > > The backlight still turns back on after reboot, but the LCD shows an > all black display. The display is also all black during the time that > EFI / the GOP is managing it, so e.g. the grub menu also is not visible. > > In this scenario the panel is initialized so that it appears to be working > and the fastboot code skips doing a modeset. Forcing a modeset by doing a > chvt to a text-console over ssh followed by echo-ing 1 and then 0 to > /sys/class/graphics/fb0/blank causes the panel to work again. > > Add a QUIRK_SKIP_SHUTDOWN quirk which turns i915_driver_shutdown() into > a no-op when set; and set this on vlv/chv devices when a DSI panel is > detected, to work around this. > > Admittedly this is a bit of a big hammer, but these platforms have been > around for quite some time now and they have always worked fine without > the new behavior to shutdown everything on shutdown/reboot. This approach > simply disables the recently introduced new shutdown behavior in this > specific case where it is known to cause problems. Which is a nice and > simple way to deal with this. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Ping? Since sending this patch I've been seeing the issue addressed by this on variour other CHT based devices too. So we have various devices suffering from a black screen after reboot now. This is pretty serious usability regression. As such it would be good to get this reviewed, or another fix proposed. Regards, Hans > --- > drivers/gpu/drm/i915/display/vlv_dsi.c | 3 +++ > drivers/gpu/drm/i915/i915_drv.c | 3 +++ > drivers/gpu/drm/i915/i915_drv.h | 1 + > 3 files changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c > index f94025ec603a..792ef868b6af 100644 > --- a/drivers/gpu/drm/i915/display/vlv_dsi.c > +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c > @@ -1949,6 +1949,9 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv) > > vlv_dsi_add_properties(intel_connector); > > + /* Some BIOS-es fail to re-init the DSI panel on reboot if we turn it off */ > + dev_priv->quirks |= QUIRK_SKIP_SHUTDOWN; > + > return; > > err_cleanup_connector: > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 8e9cb44e66e5..92f2af55af6d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1048,6 +1048,9 @@ static void intel_shutdown_encoders(struct drm_i915_private *dev_priv) > > void i915_driver_shutdown(struct drm_i915_private *i915) > { > + if (i915->quirks & QUIRK_SKIP_SHUTDOWN) > + return; > + > disable_rpm_wakeref_asserts(&i915->runtime_pm); > > i915_gem_suspend(i915); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 26d69d06aa6d..272cd7cb22d4 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -517,6 +517,7 @@ struct i915_psr { > #define QUIRK_PIN_SWIZZLED_PAGES (1<<5) > #define QUIRK_INCREASE_T12_DELAY (1<<6) > #define QUIRK_INCREASE_DDI_DISABLED_TIME (1<<7) > +#define QUIRK_SKIP_SHUTDOWN (1<<8) > > struct intel_fbdev; > struct intel_fbc_work; >
On Fri, Mar 19, 2021 at 04:45:32PM +0100, Hans de Goede wrote: > Hi, > > On 3/1/21 4:43 PM, Hans de Goede wrote: > > After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down > > displays gracefully on reboot"), the DSI panel on a Cherry Trail based > > Predia Basic tablet would no longer properly light up after reboot. > > > > The backlight still turns back on after reboot, but the LCD shows an > > all black display. The display is also all black during the time that > > EFI / the GOP is managing it, so e.g. the grub menu also is not visible. > > > > In this scenario the panel is initialized so that it appears to be working > > and the fastboot code skips doing a modeset. Forcing a modeset by doing a > > chvt to a text-console over ssh followed by echo-ing 1 and then 0 to > > /sys/class/graphics/fb0/blank causes the panel to work again. > > > > Add a QUIRK_SKIP_SHUTDOWN quirk which turns i915_driver_shutdown() into > > a no-op when set; and set this on vlv/chv devices when a DSI panel is > > detected, to work around this. > > > > Admittedly this is a bit of a big hammer, but these platforms have been > > around for quite some time now and they have always worked fine without > > the new behavior to shutdown everything on shutdown/reboot. This approach > > simply disables the recently introduced new shutdown behavior in this > > specific case where it is known to cause problems. Which is a nice and > > simple way to deal with this. > > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Ping? Since sending this patch I've been seeing the issue addressed by > this on variour other CHT based devices too. > > So we have various devices suffering from a black screen after reboot > now. This is pretty serious usability regression. > > As such it would be good to get this reviewed, or another fix proposed. For the quirks we try to limit them to very specific vendor and model ids, so I wonder if it would be possible to get this information in here instead to all the vlv with dsi... Or avoid the quirk "infra" and skip to all vlv with active dsi?! Jani? Ville? > > Regards, > > Hans > > > > > --- > > drivers/gpu/drm/i915/display/vlv_dsi.c | 3 +++ > > drivers/gpu/drm/i915/i915_drv.c | 3 +++ > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > 3 files changed, 7 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c > > index f94025ec603a..792ef868b6af 100644 > > --- a/drivers/gpu/drm/i915/display/vlv_dsi.c > > +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c > > @@ -1949,6 +1949,9 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv) > > > > vlv_dsi_add_properties(intel_connector); > > > > + /* Some BIOS-es fail to re-init the DSI panel on reboot if we turn it off */ > > + dev_priv->quirks |= QUIRK_SKIP_SHUTDOWN; > > + > > return; > > > > err_cleanup_connector: > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 8e9cb44e66e5..92f2af55af6d 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1048,6 +1048,9 @@ static void intel_shutdown_encoders(struct drm_i915_private *dev_priv) > > > > void i915_driver_shutdown(struct drm_i915_private *i915) > > { > > + if (i915->quirks & QUIRK_SKIP_SHUTDOWN) > > + return; > > + > > disable_rpm_wakeref_asserts(&i915->runtime_pm); > > > > i915_gem_suspend(i915); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 26d69d06aa6d..272cd7cb22d4 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -517,6 +517,7 @@ struct i915_psr { > > #define QUIRK_PIN_SWIZZLED_PAGES (1<<5) > > #define QUIRK_INCREASE_T12_DELAY (1<<6) > > #define QUIRK_INCREASE_DDI_DISABLED_TIME (1<<7) > > +#define QUIRK_SKIP_SHUTDOWN (1<<8) > > > > struct intel_fbdev; > > struct intel_fbc_work; > > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Mar 22, 2021 at 04:51:47PM -0400, Rodrigo Vivi wrote: > On Fri, Mar 19, 2021 at 04:45:32PM +0100, Hans de Goede wrote: > > Hi, > > > > On 3/1/21 4:43 PM, Hans de Goede wrote: > > > After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down > > > displays gracefully on reboot"), the DSI panel on a Cherry Trail based > > > Predia Basic tablet would no longer properly light up after reboot. > > > > > > The backlight still turns back on after reboot, but the LCD shows an > > > all black display. The display is also all black during the time that > > > EFI / the GOP is managing it, so e.g. the grub menu also is not visible. > > > > > > In this scenario the panel is initialized so that it appears to be working > > > and the fastboot code skips doing a modeset. Forcing a modeset by doing a > > > chvt to a text-console over ssh followed by echo-ing 1 and then 0 to > > > /sys/class/graphics/fb0/blank causes the panel to work again. > > > > > > Add a QUIRK_SKIP_SHUTDOWN quirk which turns i915_driver_shutdown() into > > > a no-op when set; and set this on vlv/chv devices when a DSI panel is > > > detected, to work around this. > > > > > > Admittedly this is a bit of a big hammer, but these platforms have been > > > around for quite some time now and they have always worked fine without > > > the new behavior to shutdown everything on shutdown/reboot. This approach > > > simply disables the recently introduced new shutdown behavior in this > > > specific case where it is known to cause problems. Which is a nice and > > > simple way to deal with this. > > > > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > > > Ping? Since sending this patch I've been seeing the issue addressed by > > this on variour other CHT based devices too. > > > > So we have various devices suffering from a black screen after reboot > > now. This is pretty serious usability regression. > > > > As such it would be good to get this reviewed, or another fix proposed. > > For the quirks we try to limit them to very specific vendor and model ids, > so I wonder if it would be possible to get this information in here instead > to all the vlv with dsi... > > Or avoid the quirk "infra" and skip to all vlv with active dsi?! > > Jani? > Ville? We need to figure out why the panel doesn't start up again. If it has problems with this then surely it also fails if we just happen to reboot with the panel already off? Oh a modeset fixes it? Then I guess it's just fastboot fail due to DSI code being crap? If no one is willing to fix it then I guess we just need to disable fastboot for DSI somehow.
Hi, On 3/22/21 9:59 PM, Ville Syrjälä wrote: > On Mon, Mar 22, 2021 at 04:51:47PM -0400, Rodrigo Vivi wrote: >> On Fri, Mar 19, 2021 at 04:45:32PM +0100, Hans de Goede wrote: >>> Hi, >>> >>> On 3/1/21 4:43 PM, Hans de Goede wrote: >>>> After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down >>>> displays gracefully on reboot"), the DSI panel on a Cherry Trail based >>>> Predia Basic tablet would no longer properly light up after reboot. >>>> >>>> The backlight still turns back on after reboot, but the LCD shows an >>>> all black display. The display is also all black during the time that >>>> EFI / the GOP is managing it, so e.g. the grub menu also is not visible. >>>> >>>> In this scenario the panel is initialized so that it appears to be working >>>> and the fastboot code skips doing a modeset. Forcing a modeset by doing a >>>> chvt to a text-console over ssh followed by echo-ing 1 and then 0 to >>>> /sys/class/graphics/fb0/blank causes the panel to work again. >>>> >>>> Add a QUIRK_SKIP_SHUTDOWN quirk which turns i915_driver_shutdown() into >>>> a no-op when set; and set this on vlv/chv devices when a DSI panel is >>>> detected, to work around this. >>>> >>>> Admittedly this is a bit of a big hammer, but these platforms have been >>>> around for quite some time now and they have always worked fine without >>>> the new behavior to shutdown everything on shutdown/reboot. This approach >>>> simply disables the recently introduced new shutdown behavior in this >>>> specific case where it is known to cause problems. Which is a nice and >>>> simple way to deal with this. >>>> >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> >>> Ping? Since sending this patch I've been seeing the issue addressed by >>> this on variour other CHT based devices too. >>> >>> So we have various devices suffering from a black screen after reboot >>> now. This is pretty serious usability regression. >>> >>> As such it would be good to get this reviewed, or another fix proposed. >> >> For the quirks we try to limit them to very specific vendor and model ids, >> so I wonder if it would be possible to get this information in here instead >> to all the vlv with dsi... >> >> Or avoid the quirk "infra" and skip to all vlv with active dsi?! >> >> Jani? >> Ville? > > We need to figure out why the panel doesn't start up again. Note it is the GOP which fails to light it up again. I think we turn something off, which after a power-on-reset is on, so the GOP expects it to be on. > If it has > problems with this then surely it also fails if we just happen to reboot > with the panel already off? I would assume so, yes, but that only happens on say a "reboot --force" over ssh, while the screen is off. Which are rather exceptional circumstances. Where as just a regular reboot is quite normal and now results in a black screen. And recovery of this condition by a normal user involves a power-cycle by pressing the power-button for 10 seconds (these are tablets so the force-power-off time is quite long), which most users don't even know they can do... > Oh a modeset fixes it? Then I guess it's just fastboot fail due to DSI > code being crap? This is not a fastboot issue, if I make the grub menu show every boot, the grub-menu is also all black, as the GOP fails to properly initialize the panel when this happens fastboot just inherits this condition. Assuming that we want to have the EFI gfx/console work on reboot (for say the grub menu), then disabling fastboot is not going to help. Also note that the Windows boot-splash with the circling dots uses the efifb, so rebooting into Windows also leads to a blackscreen at least until Windows has booted all the way. Note I have not tried Windows, so I don't know if Windows will recover from the black screen once its gfx driver loads, or if it stays black then too. > If no one is willing to fix it then I guess we just > need to disable fastboot for DSI somehow. See above, this is a GOP issue, so there is nothing for us to fix, what we need to do is stop leaving the hw in a state which the GOP cannot deal with. Which leads me to believe that we just need to disable the "graceful shutdown" on the combination of DSI + BYT/CHT. Regards, Hans
On Mon, Mar 22, 2021 at 10:28:06PM +0100, Hans de Goede wrote: > Hi, > > On 3/22/21 9:59 PM, Ville Syrjälä wrote: > > On Mon, Mar 22, 2021 at 04:51:47PM -0400, Rodrigo Vivi wrote: > >> On Fri, Mar 19, 2021 at 04:45:32PM +0100, Hans de Goede wrote: > >>> Hi, > >>> > >>> On 3/1/21 4:43 PM, Hans de Goede wrote: > >>>> After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down > >>>> displays gracefully on reboot"), the DSI panel on a Cherry Trail based > >>>> Predia Basic tablet would no longer properly light up after reboot. > >>>> > >>>> The backlight still turns back on after reboot, but the LCD shows an > >>>> all black display. The display is also all black during the time that > >>>> EFI / the GOP is managing it, so e.g. the grub menu also is not visible. > >>>> > >>>> In this scenario the panel is initialized so that it appears to be working > >>>> and the fastboot code skips doing a modeset. Forcing a modeset by doing a > >>>> chvt to a text-console over ssh followed by echo-ing 1 and then 0 to > >>>> /sys/class/graphics/fb0/blank causes the panel to work again. > >>>> > >>>> Add a QUIRK_SKIP_SHUTDOWN quirk which turns i915_driver_shutdown() into > >>>> a no-op when set; and set this on vlv/chv devices when a DSI panel is > >>>> detected, to work around this. > >>>> > >>>> Admittedly this is a bit of a big hammer, but these platforms have been > >>>> around for quite some time now and they have always worked fine without > >>>> the new behavior to shutdown everything on shutdown/reboot. This approach > >>>> simply disables the recently introduced new shutdown behavior in this > >>>> specific case where it is known to cause problems. Which is a nice and > >>>> simple way to deal with this. > >>>> > >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >>> > >>> Ping? Since sending this patch I've been seeing the issue addressed by > >>> this on variour other CHT based devices too. > >>> > >>> So we have various devices suffering from a black screen after reboot > >>> now. This is pretty serious usability regression. > >>> > >>> As such it would be good to get this reviewed, or another fix proposed. > >> > >> For the quirks we try to limit them to very specific vendor and model ids, > >> so I wonder if it would be possible to get this information in here instead > >> to all the vlv with dsi... > >> > >> Or avoid the quirk "infra" and skip to all vlv with active dsi?! > >> > >> Jani? > >> Ville? > > > > We need to figure out why the panel doesn't start up again. > > Note it is the GOP which fails to light it up again. I think we turn something > off, which after a power-on-reset is on, so the GOP expects it to be on. Hmm. Do any of the reboot=warm|cold|whatever knobs make a difference? Are there any fast vs. slow boot settings in the BIOS setup?
Hi, On 3/22/21 10:47 PM, Ville Syrjälä wrote: > On Mon, Mar 22, 2021 at 10:28:06PM +0100, Hans de Goede wrote: >> Hi, >> >> On 3/22/21 9:59 PM, Ville Syrjälä wrote: >>> On Mon, Mar 22, 2021 at 04:51:47PM -0400, Rodrigo Vivi wrote: >>>> On Fri, Mar 19, 2021 at 04:45:32PM +0100, Hans de Goede wrote: >>>>> Hi, >>>>> >>>>> On 3/1/21 4:43 PM, Hans de Goede wrote: >>>>>> After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down >>>>>> displays gracefully on reboot"), the DSI panel on a Cherry Trail based >>>>>> Predia Basic tablet would no longer properly light up after reboot. >>>>>> >>>>>> The backlight still turns back on after reboot, but the LCD shows an >>>>>> all black display. The display is also all black during the time that >>>>>> EFI / the GOP is managing it, so e.g. the grub menu also is not visible. >>>>>> >>>>>> In this scenario the panel is initialized so that it appears to be working >>>>>> and the fastboot code skips doing a modeset. Forcing a modeset by doing a >>>>>> chvt to a text-console over ssh followed by echo-ing 1 and then 0 to >>>>>> /sys/class/graphics/fb0/blank causes the panel to work again. >>>>>> >>>>>> Add a QUIRK_SKIP_SHUTDOWN quirk which turns i915_driver_shutdown() into >>>>>> a no-op when set; and set this on vlv/chv devices when a DSI panel is >>>>>> detected, to work around this. >>>>>> >>>>>> Admittedly this is a bit of a big hammer, but these platforms have been >>>>>> around for quite some time now and they have always worked fine without >>>>>> the new behavior to shutdown everything on shutdown/reboot. This approach >>>>>> simply disables the recently introduced new shutdown behavior in this >>>>>> specific case where it is known to cause problems. Which is a nice and >>>>>> simple way to deal with this. >>>>>> >>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>>> >>>>> Ping? Since sending this patch I've been seeing the issue addressed by >>>>> this on variour other CHT based devices too. >>>>> >>>>> So we have various devices suffering from a black screen after reboot >>>>> now. This is pretty serious usability regression. >>>>> >>>>> As such it would be good to get this reviewed, or another fix proposed. >>>> >>>> For the quirks we try to limit them to very specific vendor and model ids, >>>> so I wonder if it would be possible to get this information in here instead >>>> to all the vlv with dsi... >>>> >>>> Or avoid the quirk "infra" and skip to all vlv with active dsi?! >>>> >>>> Jani? >>>> Ville? >>> >>> We need to figure out why the panel doesn't start up again. >> >> Note it is the GOP which fails to light it up again. I think we turn something >> off, which after a power-on-reset is on, so the GOP expects it to be on. > > Hmm. Do any of the reboot=warm|cold|whatever knobs make a difference? > Are there any fast vs. slow boot settings in the BIOS setup? Ok, so I was running the tests which you requested and during this I managed to find the real problem. What happens on reboot is a really quick panel off/on cycle and that is causing the issue. I can reproduce this by doing: chvt 3; echo 1 > /sys/class/graphics/fb0/blank; echo 0 > /sys/class/graphics/fb0/blank The problem is that we're not honoring panel_pwr_cycle_delay because intel_dsi_msleep() is a no-op on devices with a MIPI-sequences version >= 3, because those sequences already contain the necessary delays, at least for most of the steps during the on/off sequences. It seems that the pwr-cycle delay is not handled by those v3+ sequences. So fixing this is as simple as switching to a regular msleep for the intel_dsi->panel_pwr_cycle_delay. Once we do that it would be good (for e.g. suspend/resume speed) to fix: /* * FIXME As we do with eDP, just make a note of the time here * and perform the wait before the next panel power on. */ Which sits right above that msleep. Since I have a reproducer now which shows when the sleep is too short, it should now be easy ti fix the FIXME and test that the fix works. I'll do this in a separate patch and send a patch-set with both patches replacing this patch. Regards, Hans
On Tue, Mar 23, 2021 at 06:29:53PM +0100, Hans de Goede wrote: > Hi, > > On 3/22/21 10:47 PM, Ville Syrjälä wrote: > > On Mon, Mar 22, 2021 at 10:28:06PM +0100, Hans de Goede wrote: > >> Hi, > >> > >> On 3/22/21 9:59 PM, Ville Syrjälä wrote: > >>> On Mon, Mar 22, 2021 at 04:51:47PM -0400, Rodrigo Vivi wrote: > >>>> On Fri, Mar 19, 2021 at 04:45:32PM +0100, Hans de Goede wrote: > >>>>> Hi, > >>>>> > >>>>> On 3/1/21 4:43 PM, Hans de Goede wrote: > >>>>>> After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down > >>>>>> displays gracefully on reboot"), the DSI panel on a Cherry Trail based > >>>>>> Predia Basic tablet would no longer properly light up after reboot. > >>>>>> > >>>>>> The backlight still turns back on after reboot, but the LCD shows an > >>>>>> all black display. The display is also all black during the time that > >>>>>> EFI / the GOP is managing it, so e.g. the grub menu also is not visible. > >>>>>> > >>>>>> In this scenario the panel is initialized so that it appears to be working > >>>>>> and the fastboot code skips doing a modeset. Forcing a modeset by doing a > >>>>>> chvt to a text-console over ssh followed by echo-ing 1 and then 0 to > >>>>>> /sys/class/graphics/fb0/blank causes the panel to work again. > >>>>>> > >>>>>> Add a QUIRK_SKIP_SHUTDOWN quirk which turns i915_driver_shutdown() into > >>>>>> a no-op when set; and set this on vlv/chv devices when a DSI panel is > >>>>>> detected, to work around this. > >>>>>> > >>>>>> Admittedly this is a bit of a big hammer, but these platforms have been > >>>>>> around for quite some time now and they have always worked fine without > >>>>>> the new behavior to shutdown everything on shutdown/reboot. This approach > >>>>>> simply disables the recently introduced new shutdown behavior in this > >>>>>> specific case where it is known to cause problems. Which is a nice and > >>>>>> simple way to deal with this. > >>>>>> > >>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >>>>> > >>>>> Ping? Since sending this patch I've been seeing the issue addressed by > >>>>> this on variour other CHT based devices too. > >>>>> > >>>>> So we have various devices suffering from a black screen after reboot > >>>>> now. This is pretty serious usability regression. > >>>>> > >>>>> As such it would be good to get this reviewed, or another fix proposed. > >>>> > >>>> For the quirks we try to limit them to very specific vendor and model ids, > >>>> so I wonder if it would be possible to get this information in here instead > >>>> to all the vlv with dsi... > >>>> > >>>> Or avoid the quirk "infra" and skip to all vlv with active dsi?! > >>>> > >>>> Jani? > >>>> Ville? > >>> > >>> We need to figure out why the panel doesn't start up again. > >> > >> Note it is the GOP which fails to light it up again. I think we turn something > >> off, which after a power-on-reset is on, so the GOP expects it to be on. > > > > Hmm. Do any of the reboot=warm|cold|whatever knobs make a difference? > > Are there any fast vs. slow boot settings in the BIOS setup? > > Ok, so I was running the tests which you requested and during this > I managed to find the real problem. > > What happens on reboot is a really quick panel off/on cycle and that is > causing the issue. > > I can reproduce this by doing: > > chvt 3; echo 1 > /sys/class/graphics/fb0/blank; echo 0 > /sys/class/graphics/fb0/blank > > The problem is that we're not honoring panel_pwr_cycle_delay because > intel_dsi_msleep() is a no-op on devices with a MIPI-sequences version >= 3, > because those sequences already contain the necessary delays, at least > for most of the steps during the on/off sequences. It seems that the > pwr-cycle delay is not handled by those v3+ sequences. > > So fixing this is as simple as switching to a regular msleep for the > intel_dsi->panel_pwr_cycle_delay. > > Once we do that it would be good (for e.g. suspend/resume speed) to fix: > > /* > * FIXME As we do with eDP, just make a note of the time here > * and perform the wait before the next panel power on. > */ > > Which sits right above that msleep. Since I have a reproducer now which > shows when the sleep is too short, it should now be easy ti fix the FIXME > and test that the fix works. I'll do this in a separate patch and send > a patch-set with both patches replacing this patch. Awesome. I'm really happy to avoid any quirks and whatnot since they always come back to bite you later. Thanks for digging into it. Speaking of DSI, you wouldn't happen to have one these machines: https://gitlab.freedesktop.org/drm/intel/-/issues/2698 ? Haven't gotten a response from the bug reporter so no idea if my quick patch helps or not.
Hi, On 3/23/21 6:51 PM, Ville Syrjälä wrote: > On Tue, Mar 23, 2021 at 06:29:53PM +0100, Hans de Goede wrote: >> Hi, >> >> On 3/22/21 10:47 PM, Ville Syrjälä wrote: >>> On Mon, Mar 22, 2021 at 10:28:06PM +0100, Hans de Goede wrote: >>>> Hi, >>>> >>>> On 3/22/21 9:59 PM, Ville Syrjälä wrote: >>>>> On Mon, Mar 22, 2021 at 04:51:47PM -0400, Rodrigo Vivi wrote: >>>>>> On Fri, Mar 19, 2021 at 04:45:32PM +0100, Hans de Goede wrote: >>>>>>> Hi, >>>>>>> >>>>>>> On 3/1/21 4:43 PM, Hans de Goede wrote: >>>>>>>> After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down >>>>>>>> displays gracefully on reboot"), the DSI panel on a Cherry Trail based >>>>>>>> Predia Basic tablet would no longer properly light up after reboot. >>>>>>>> >>>>>>>> The backlight still turns back on after reboot, but the LCD shows an >>>>>>>> all black display. The display is also all black during the time that >>>>>>>> EFI / the GOP is managing it, so e.g. the grub menu also is not visible. >>>>>>>> >>>>>>>> In this scenario the panel is initialized so that it appears to be working >>>>>>>> and the fastboot code skips doing a modeset. Forcing a modeset by doing a >>>>>>>> chvt to a text-console over ssh followed by echo-ing 1 and then 0 to >>>>>>>> /sys/class/graphics/fb0/blank causes the panel to work again. >>>>>>>> >>>>>>>> Add a QUIRK_SKIP_SHUTDOWN quirk which turns i915_driver_shutdown() into >>>>>>>> a no-op when set; and set this on vlv/chv devices when a DSI panel is >>>>>>>> detected, to work around this. >>>>>>>> >>>>>>>> Admittedly this is a bit of a big hammer, but these platforms have been >>>>>>>> around for quite some time now and they have always worked fine without >>>>>>>> the new behavior to shutdown everything on shutdown/reboot. This approach >>>>>>>> simply disables the recently introduced new shutdown behavior in this >>>>>>>> specific case where it is known to cause problems. Which is a nice and >>>>>>>> simple way to deal with this. >>>>>>>> >>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>>>>> >>>>>>> Ping? Since sending this patch I've been seeing the issue addressed by >>>>>>> this on variour other CHT based devices too. >>>>>>> >>>>>>> So we have various devices suffering from a black screen after reboot >>>>>>> now. This is pretty serious usability regression. >>>>>>> >>>>>>> As such it would be good to get this reviewed, or another fix proposed. >>>>>> >>>>>> For the quirks we try to limit them to very specific vendor and model ids, >>>>>> so I wonder if it would be possible to get this information in here instead >>>>>> to all the vlv with dsi... >>>>>> >>>>>> Or avoid the quirk "infra" and skip to all vlv with active dsi?! >>>>>> >>>>>> Jani? >>>>>> Ville? >>>>> >>>>> We need to figure out why the panel doesn't start up again. >>>> >>>> Note it is the GOP which fails to light it up again. I think we turn something >>>> off, which after a power-on-reset is on, so the GOP expects it to be on. >>> >>> Hmm. Do any of the reboot=warm|cold|whatever knobs make a difference? >>> Are there any fast vs. slow boot settings in the BIOS setup? >> >> Ok, so I was running the tests which you requested and during this >> I managed to find the real problem. >> >> What happens on reboot is a really quick panel off/on cycle and that is >> causing the issue. >> >> I can reproduce this by doing: >> >> chvt 3; echo 1 > /sys/class/graphics/fb0/blank; echo 0 > /sys/class/graphics/fb0/blank >> >> The problem is that we're not honoring panel_pwr_cycle_delay because >> intel_dsi_msleep() is a no-op on devices with a MIPI-sequences version >= 3, >> because those sequences already contain the necessary delays, at least >> for most of the steps during the on/off sequences. It seems that the >> pwr-cycle delay is not handled by those v3+ sequences. >> >> So fixing this is as simple as switching to a regular msleep for the >> intel_dsi->panel_pwr_cycle_delay. >> >> Once we do that it would be good (for e.g. suspend/resume speed) to fix: >> >> /* >> * FIXME As we do with eDP, just make a note of the time here >> * and perform the wait before the next panel power on. >> */ >> >> Which sits right above that msleep. Since I have a reproducer now which >> shows when the sleep is too short, it should now be easy ti fix the FIXME >> and test that the fix works. I'll do this in a separate patch and send >> a patch-set with both patches replacing this patch. > > Awesome. I'm really happy to avoid any quirks and whatnot since > they always come back to bite you later. Thanks for digging into it. > > Speaking of DSI, you wouldn't happen to have one these machines: > https://gitlab.freedesktop.org/drm/intel/-/issues/2698 ? Sorry I don't have any 10" Dell models in my collection. Regards, Hans
HI, On 3/23/21 8:13 PM, Hans de Goede wrote: > Hi, > > On 3/23/21 6:51 PM, Ville Syrjälä wrote: >> On Tue, Mar 23, 2021 at 06:29:53PM +0100, Hans de Goede wrote: >>> Hi, >>> >>> On 3/22/21 10:47 PM, Ville Syrjälä wrote: >>>> On Mon, Mar 22, 2021 at 10:28:06PM +0100, Hans de Goede wrote: >>>>> Hi, >>>>> >>>>> On 3/22/21 9:59 PM, Ville Syrjälä wrote: >>>>>> On Mon, Mar 22, 2021 at 04:51:47PM -0400, Rodrigo Vivi wrote: >>>>>>> On Fri, Mar 19, 2021 at 04:45:32PM +0100, Hans de Goede wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> On 3/1/21 4:43 PM, Hans de Goede wrote: >>>>>>>>> After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down >>>>>>>>> displays gracefully on reboot"), the DSI panel on a Cherry Trail based >>>>>>>>> Predia Basic tablet would no longer properly light up after reboot. >>>>>>>>> >>>>>>>>> The backlight still turns back on after reboot, but the LCD shows an >>>>>>>>> all black display. The display is also all black during the time that >>>>>>>>> EFI / the GOP is managing it, so e.g. the grub menu also is not visible. >>>>>>>>> >>>>>>>>> In this scenario the panel is initialized so that it appears to be working >>>>>>>>> and the fastboot code skips doing a modeset. Forcing a modeset by doing a >>>>>>>>> chvt to a text-console over ssh followed by echo-ing 1 and then 0 to >>>>>>>>> /sys/class/graphics/fb0/blank causes the panel to work again. >>>>>>>>> >>>>>>>>> Add a QUIRK_SKIP_SHUTDOWN quirk which turns i915_driver_shutdown() into >>>>>>>>> a no-op when set; and set this on vlv/chv devices when a DSI panel is >>>>>>>>> detected, to work around this. >>>>>>>>> >>>>>>>>> Admittedly this is a bit of a big hammer, but these platforms have been >>>>>>>>> around for quite some time now and they have always worked fine without >>>>>>>>> the new behavior to shutdown everything on shutdown/reboot. This approach >>>>>>>>> simply disables the recently introduced new shutdown behavior in this >>>>>>>>> specific case where it is known to cause problems. Which is a nice and >>>>>>>>> simple way to deal with this. >>>>>>>>> >>>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>>>>>> >>>>>>>> Ping? Since sending this patch I've been seeing the issue addressed by >>>>>>>> this on variour other CHT based devices too. >>>>>>>> >>>>>>>> So we have various devices suffering from a black screen after reboot >>>>>>>> now. This is pretty serious usability regression. >>>>>>>> >>>>>>>> As such it would be good to get this reviewed, or another fix proposed. >>>>>>> >>>>>>> For the quirks we try to limit them to very specific vendor and model ids, >>>>>>> so I wonder if it would be possible to get this information in here instead >>>>>>> to all the vlv with dsi... >>>>>>> >>>>>>> Or avoid the quirk "infra" and skip to all vlv with active dsi?! >>>>>>> >>>>>>> Jani? >>>>>>> Ville? >>>>>> >>>>>> We need to figure out why the panel doesn't start up again. >>>>> >>>>> Note it is the GOP which fails to light it up again. I think we turn something >>>>> off, which after a power-on-reset is on, so the GOP expects it to be on. >>>> >>>> Hmm. Do any of the reboot=warm|cold|whatever knobs make a difference? >>>> Are there any fast vs. slow boot settings in the BIOS setup? >>> >>> Ok, so I was running the tests which you requested and during this >>> I managed to find the real problem. >>> >>> What happens on reboot is a really quick panel off/on cycle and that is >>> causing the issue. >>> >>> I can reproduce this by doing: >>> >>> chvt 3; echo 1 > /sys/class/graphics/fb0/blank; echo 0 > /sys/class/graphics/fb0/blank >>> >>> The problem is that we're not honoring panel_pwr_cycle_delay because >>> intel_dsi_msleep() is a no-op on devices with a MIPI-sequences version >= 3, >>> because those sequences already contain the necessary delays, at least >>> for most of the steps during the on/off sequences. It seems that the >>> pwr-cycle delay is not handled by those v3+ sequences. >>> >>> So fixing this is as simple as switching to a regular msleep for the >>> intel_dsi->panel_pwr_cycle_delay. >>> >>> Once we do that it would be good (for e.g. suspend/resume speed) to fix: >>> >>> /* >>> * FIXME As we do with eDP, just make a note of the time here >>> * and perform the wait before the next panel power on. >>> */ >>> >>> Which sits right above that msleep. Since I have a reproducer now which >>> shows when the sleep is too short, it should now be easy ti fix the FIXME >>> and test that the fix works. I'll do this in a separate patch and send >>> a patch-set with both patches replacing this patch. >> >> Awesome. I'm really happy to avoid any quirks and whatnot since >> they always come back to bite you later. Thanks for digging into it. >> >> Speaking of DSI, you wouldn't happen to have one these machines: >> https://gitlab.freedesktop.org/drm/intel/-/issues/2698 ? > > Sorry I don't have any 10" Dell models in my collection. But I do see that the reporter is a Fedora user. So I can prep a test-kernel in rpm form for him with the patch applied, which should make it a lot easier for the reporter to test the patch. I'm building a test-kernel for this now: https://koji.fedoraproject.org/koji/taskinfo?taskID=64447613 I'll update the issue with a link to it when the build is done. Regards, Hans
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c index f94025ec603a..792ef868b6af 100644 --- a/drivers/gpu/drm/i915/display/vlv_dsi.c +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c @@ -1949,6 +1949,9 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv) vlv_dsi_add_properties(intel_connector); + /* Some BIOS-es fail to re-init the DSI panel on reboot if we turn it off */ + dev_priv->quirks |= QUIRK_SKIP_SHUTDOWN; + return; err_cleanup_connector: diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 8e9cb44e66e5..92f2af55af6d 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1048,6 +1048,9 @@ static void intel_shutdown_encoders(struct drm_i915_private *dev_priv) void i915_driver_shutdown(struct drm_i915_private *i915) { + if (i915->quirks & QUIRK_SKIP_SHUTDOWN) + return; + disable_rpm_wakeref_asserts(&i915->runtime_pm); i915_gem_suspend(i915); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 26d69d06aa6d..272cd7cb22d4 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -517,6 +517,7 @@ struct i915_psr { #define QUIRK_PIN_SWIZZLED_PAGES (1<<5) #define QUIRK_INCREASE_T12_DELAY (1<<6) #define QUIRK_INCREASE_DDI_DISABLED_TIME (1<<7) +#define QUIRK_SKIP_SHUTDOWN (1<<8) struct intel_fbdev; struct intel_fbc_work;
After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down displays gracefully on reboot"), the DSI panel on a Cherry Trail based Predia Basic tablet would no longer properly light up after reboot. The backlight still turns back on after reboot, but the LCD shows an all black display. The display is also all black during the time that EFI / the GOP is managing it, so e.g. the grub menu also is not visible. In this scenario the panel is initialized so that it appears to be working and the fastboot code skips doing a modeset. Forcing a modeset by doing a chvt to a text-console over ssh followed by echo-ing 1 and then 0 to /sys/class/graphics/fb0/blank causes the panel to work again. Add a QUIRK_SKIP_SHUTDOWN quirk which turns i915_driver_shutdown() into a no-op when set; and set this on vlv/chv devices when a DSI panel is detected, to work around this. Admittedly this is a bit of a big hammer, but these platforms have been around for quite some time now and they have always worked fine without the new behavior to shutdown everything on shutdown/reboot. This approach simply disables the recently introduced new shutdown behavior in this specific case where it is known to cause problems. Which is a nice and simple way to deal with this. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/gpu/drm/i915/display/vlv_dsi.c | 3 +++ drivers/gpu/drm/i915/i915_drv.c | 3 +++ drivers/gpu/drm/i915/i915_drv.h | 1 + 3 files changed, 7 insertions(+)