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

Tue Mar 23 19:34:05 UTC 2021

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

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