mbox series

[0/3,SRU,U/H/I/OEM-5.10/OEM-5.14] Fix System hangs on black screen when reboot

Message ID 20211101041500.2467168-1-koba.ko@canonical.com
Headers show
Series Fix System hangs on black screen when reboot | expand

Message

Koba Ko Nov. 1, 2021, 4:14 a.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1949321

[Impact]
System hangs on black screen when reboot

[Fix]
Looks like our VBIOS/GOP generally fail to turn the DP dual mode adater
TMDS output buffers back on after a reboot. This leads to a black screen
after reboot if we turned the TMDS output buffers off prior to reboot.
And if i915 decides to do a fastboot the black screen will persist even
after i915 takes over.

Apparently this has been a problem ever since commit b2ccb822d376 ("drm/i915:Enable/disable TMDS output buffers in DP++ adaptor as needed") if one rebooted while the display was turned off. And things became worse with commit fe0f1e3bfdfe ("drm/i915: Shut down displays gracefully on reboot") since now we always turn the display off before a reboot.

This was reported on a RKL, but I confirmed the same behaviour on my
SNB as well. So looks pretty universal.

Let's fix this by explicitly turning the TMDS output buffers back on
in the encoder->shutdown() hook. Note that this gets called after irqs
have been disabled, so the i2c communication with the DP dual mode
adapter has to be performed via polling (which the gmbus code is
perfectly happy to do for us).

We also need a bit of care in handling DDI encoders which may or may
not be set up for HDMI output. Specifically ddc_pin will not be
populated for a DP only DDI encoder, in which case we don't want to
call intel_gmbus_get_adapter(). We can handle that by simply doing
the dual mode adapter type check before calling
intel_gmbus_get_adapter().
Ref: https://patchwork.freedesktop.org/series/96433/

[Test]
1. install kernel
2. reboot
3. check the monitor works well.

[Regression Potential]
Medium, patch has sent to patchwork but may have a upgraded version in the furtue.

Ville Syrjälä (2):
  drm/i915: Don't request GMBUS to generate irqs when called while irqs
    are off
  drm/i915/hdmi: Turn DP++ TMDS output buffers back on in
    encoder->shutdown()

 drivers/gpu/drm/i915/display/g4x_hdmi.c    |  1 +
 drivers/gpu/drm/i915/display/intel_ddi.c   |  1 +
 drivers/gpu/drm/i915/display/intel_gmbus.c | 13 +++++++++++--
 drivers/gpu/drm/i915/display/intel_hdmi.c  | 16 ++++++++++++++--
 drivers/gpu/drm/i915/display/intel_hdmi.h  |  1 +
 5 files changed, 28 insertions(+), 4 deletions(-)

Comments

Timo Aaltonen Nov. 1, 2021, 9:17 a.m. UTC | #1
On 1.11.2021 6.14, Koba Ko wrote:
> BugLink: https://bugs.launchpad.net/bugs/1949321
> 
> [Impact]
> System hangs on black screen when reboot
> 
> [Fix]
> Looks like our VBIOS/GOP generally fail to turn the DP dual mode adater
> TMDS output buffers back on after a reboot. This leads to a black screen
> after reboot if we turned the TMDS output buffers off prior to reboot.
> And if i915 decides to do a fastboot the black screen will persist even
> after i915 takes over.
> 
> Apparently this has been a problem ever since commit b2ccb822d376 ("drm/i915:Enable/disable TMDS output buffers in DP++ adaptor as needed") if one rebooted while the display was turned off. And things became worse with commit fe0f1e3bfdfe ("drm/i915: Shut down displays gracefully on reboot") since now we always turn the display off before a reboot.
> 
> This was reported on a RKL, but I confirmed the same behaviour on my
> SNB as well. So looks pretty universal.
> 
> Let's fix this by explicitly turning the TMDS output buffers back on
> in the encoder->shutdown() hook. Note that this gets called after irqs
> have been disabled, so the i2c communication with the DP dual mode
> adapter has to be performed via polling (which the gmbus code is
> perfectly happy to do for us).
> 
> We also need a bit of care in handling DDI encoders which may or may
> not be set up for HDMI output. Specifically ddc_pin will not be
> populated for a DP only DDI encoder, in which case we don't want to
> call intel_gmbus_get_adapter(). We can handle that by simply doing
> the dual mode adapter type check before calling
> intel_gmbus_get_adapter().
> Ref: https://patchwork.freedesktop.org/series/96433/
> 
> [Test]
> 1. install kernel
> 2. reboot
> 3. check the monitor works well.
> 
> [Regression Potential]
> Medium, patch has sent to patchwork but may have a upgraded version in the furtue.
> 
> Ville Syrjälä (2):
>    drm/i915: Don't request GMBUS to generate irqs when called while irqs
>      are off
>    drm/i915/hdmi: Turn DP++ TMDS output buffers back on in
>      encoder->shutdown()
> 
>   drivers/gpu/drm/i915/display/g4x_hdmi.c    |  1 +
>   drivers/gpu/drm/i915/display/intel_ddi.c   |  1 +
>   drivers/gpu/drm/i915/display/intel_gmbus.c | 13 +++++++++++--
>   drivers/gpu/drm/i915/display/intel_hdmi.c  | 16 ++++++++++++++--
>   drivers/gpu/drm/i915/display/intel_hdmi.h  |  1 +
>   5 files changed, 28 insertions(+), 4 deletions(-)
> 

applied to oem-5.14, thanks
Stefan Bader Nov. 3, 2021, 8:46 a.m. UTC | #2
On 01.11.21 05:14, Koba Ko wrote:
> BugLink: https://bugs.launchpad.net/bugs/1949321
> 
> [Impact]
> System hangs on black screen when reboot
> 
> [Fix]
> Looks like our VBIOS/GOP generally fail to turn the DP dual mode adater
> TMDS output buffers back on after a reboot. This leads to a black screen
> after reboot if we turned the TMDS output buffers off prior to reboot.
> And if i915 decides to do a fastboot the black screen will persist even
> after i915 takes over.
> 
> Apparently this has been a problem ever since commit b2ccb822d376 ("drm/i915:Enable/disable TMDS output buffers in DP++ adaptor as needed") if one rebooted while the display was turned off. And things became worse with commit fe0f1e3bfdfe ("drm/i915: Shut down displays gracefully on reboot") since now we always turn the display off before a reboot.
> 
> This was reported on a RKL, but I confirmed the same behaviour on my
> SNB as well. So looks pretty universal.
> 
> Let's fix this by explicitly turning the TMDS output buffers back on
> in the encoder->shutdown() hook. Note that this gets called after irqs
> have been disabled, so the i2c communication with the DP dual mode
> adapter has to be performed via polling (which the gmbus code is
> perfectly happy to do for us).
> 
> We also need a bit of care in handling DDI encoders which may or may
> not be set up for HDMI output. Specifically ddc_pin will not be
> populated for a DP only DDI encoder, in which case we don't want to
> call intel_gmbus_get_adapter(). We can handle that by simply doing
> the dual mode adapter type check before calling
> intel_gmbus_get_adapter().
> Ref: https://patchwork.freedesktop.org/series/96433/
> 
> [Test]
> 1. install kernel
> 2. reboot
> 3. check the monitor works well.
> 
> [Regression Potential]
> Medium, patch has sent to patchwork but may have a upgraded version in the furtue.

This is not SRU material. When this is final and no longer experimental (at 
least on linux-next), then it can be considered for stable releases.

-Stefan
> 
> Ville Syrjälä (2):
>    drm/i915: Don't request GMBUS to generate irqs when called while irqs
>      are off
>    drm/i915/hdmi: Turn DP++ TMDS output buffers back on in
>      encoder->shutdown()
> 
>   drivers/gpu/drm/i915/display/g4x_hdmi.c    |  1 +
>   drivers/gpu/drm/i915/display/intel_ddi.c   |  1 +
>   drivers/gpu/drm/i915/display/intel_gmbus.c | 13 +++++++++++--
>   drivers/gpu/drm/i915/display/intel_hdmi.c  | 16 ++++++++++++++--
>   drivers/gpu/drm/i915/display/intel_hdmi.h  |  1 +
>   5 files changed, 28 insertions(+), 4 deletions(-)
>
Koba Ko Nov. 8, 2021, 1:34 p.m. UTC | #3
On Wed, Nov 3, 2021, 4:47 PM Stefan Bader <stefan.bader@canonical.com>
wrote:

> On 01.11.21 05:14, Koba Ko wrote:
> > BugLink: https://bugs.launchpad.net/bugs/1949321
> >
> > [Impact]
> > System hangs on black screen when reboot
> >
> > [Fix]
> > Looks like our VBIOS/GOP generally fail to turn the DP dual mode adater
> > TMDS output buffers back on after a reboot. This leads to a black screen
> > after reboot if we turned the TMDS output buffers off prior to reboot.
> > And if i915 decides to do a fastboot the black screen will persist even
> > after i915 takes over.
> >
> > Apparently this has been a problem ever since commit b2ccb822d376
> ("drm/i915:Enable/disable TMDS output buffers in DP++ adaptor as needed")
> if one rebooted while the display was turned off. And things became worse
> with commit fe0f1e3bfdfe ("drm/i915: Shut down displays gracefully on
> reboot") since now we always turn the display off before a reboot.
> >
> > This was reported on a RKL, but I confirmed the same behaviour on my
> > SNB as well. So looks pretty universal.
> >
> > Let's fix this by explicitly turning the TMDS output buffers back on
> > in the encoder->shutdown() hook. Note that this gets called after irqs
> > have been disabled, so the i2c communication with the DP dual mode
> > adapter has to be performed via polling (which the gmbus code is
> > perfectly happy to do for us).
> >
> > We also need a bit of care in handling DDI encoders which may or may
> > not be set up for HDMI output. Specifically ddc_pin will not be
> > populated for a DP only DDI encoder, in which case we don't want to
> > call intel_gmbus_get_adapter(). We can handle that by simply doing
> > the dual mode adapter type check before calling
> > intel_gmbus_get_adapter().
> > Ref: https://patchwork.freedesktop.org/series/96433/
> >
> > [Test]
> > 1. install kernel
> > 2. reboot
> > 3. check the monitor works well.
> >
> > [Regression Potential]
> > Medium, patch has sent to patchwork but may have a upgraded version in
> the furtue.
>
> This is not SRU material. When this is final and no longer experimental
> (at
> least on linux-next), then it can be considered for stable releases.
>
Ok, I will update in the future

> -Stefan
> >
> > Ville Syrjälä (2):
> >    drm/i915: Don't request GMBUS to generate irqs when called while irqs
> >      are off
> >    drm/i915/hdmi: Turn DP++ TMDS output buffers back on in
> >      encoder->shutdown()
> >
> >   drivers/gpu/drm/i915/display/g4x_hdmi.c    |  1 +
> >   drivers/gpu/drm/i915/display/intel_ddi.c   |  1 +
> >   drivers/gpu/drm/i915/display/intel_gmbus.c | 13 +++++++++++--
> >   drivers/gpu/drm/i915/display/intel_hdmi.c  | 16 ++++++++++++++--
> >   drivers/gpu/drm/i915/display/intel_hdmi.h  |  1 +
> >   5 files changed, 28 insertions(+), 4 deletions(-)
> >
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
Paolo Pisati Dec. 14, 2021, 4 p.m. UTC | #4
On Mon, Nov 08, 2021 at 09:34:02PM +0800, Koba Ko wrote:
>    Ok, I will update in the future

Have you tried to upstream this code? This looks like stable material.

When ready, send a v2 for Jammy too.