[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/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 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.
> 
> 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