[Intel-gfx] [PATCH] drm/i915/display/vlv_dsi: Do no shut down displays on reboot if a DSI panel is used

[Hans de Goede]

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 at 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