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

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

[Hans de Goede]

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(+)

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;
-- 
2.30.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [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/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;
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rodrigo Vivi]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Ville Syrjälä]

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.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [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 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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Ville Syrjälä]

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?

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [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@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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Ville Syrjälä]

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.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [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@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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [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 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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel