On Wed, Apr 17, 2024 at 11:47 AM Enrico Bartky <enrico.bartky@xxxxxxxxx> wrote:
>
> Hi,
>
> sorry for the delay. This patch fixes the crash during boot! (tested against linux 6.9-rc3)
>
> Greetings
Thanks for testing. Then I'll push this to drm-next-fixes.
-Patrik
>
> Am Mo., 15. Apr. 2024 um 13:57 Uhr schrieb Patrik Jakobsson <patrik.r.jakobsson@xxxxxxxxx>:
>>
>> On Mon, Apr 15, 2024 at 1:45 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
>> >
>> > Hi
>> >
>> > Am 15.04.24 um 13:27 schrieb Patrik Jakobsson:
>> > > Due to a change in the order of initialization, the lid timer got
>> > > started before proper setup was made. This resulted in a crash during
>> > > boot.
>> > >
>> > > The lid switch is handled by gma500 through a timer that periodically
>> > > polls the opregion for changes. These types of ACPI events shouldn't be
>> > > handled by the graphics driver so let's get rid of the lid code. This
>> > > fixes the crash during boot.
>> > >
>> > > Fixes: 8f1aaccb04b7 ("drm/gma500: Implement client-based fbdev emulation")
>> > > Cc: Enrico Bartky <enrico.bartky@xxxxxxxxx>
>> >
>> > The patch deserves a Reported-by: from Enrico.
>>
>> Enrico, can you test this patch to make sure it works for you as well?
>>
>> Thanks
>> Patrik
>>
>> >
>> > With this fixed:
>> >
>> > Reviewed-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
>> >
>> > Best regards
>> > Thomas
>> >
>> > > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
>> > > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@xxxxxxxxx>
>> > > ---
>> > > drivers/gpu/drm/gma500/Makefile | 1 -
>> > > drivers/gpu/drm/gma500/psb_device.c | 5 +-
>> > > drivers/gpu/drm/gma500/psb_drv.h | 9 ----
>> > > drivers/gpu/drm/gma500/psb_lid.c | 80 -----------------------------
>> > > 4 files changed, 1 insertion(+), 94 deletions(-)
>> > > delete mode 100644 drivers/gpu/drm/gma500/psb_lid.c
>> > >
>> > > diff --git a/drivers/gpu/drm/gma500/Makefile b/drivers/gpu/drm/gma500/Makefile
>> > > index 4f302cd5e1a6..58fed80c7392 100644
>> > > --- a/drivers/gpu/drm/gma500/Makefile
>> > > +++ b/drivers/gpu/drm/gma500/Makefile
>> > > @@ -34,7 +34,6 @@ gma500_gfx-y += \
>> > > psb_intel_lvds.o \
>> > > psb_intel_modes.o \
>> > > psb_intel_sdvo.o \
>> > > - psb_lid.o \
>> > > psb_irq.o
>> > >
>> > > gma500_gfx-$(CONFIG_ACPI) += opregion.o
>> > > diff --git a/drivers/gpu/drm/gma500/psb_device.c b/drivers/gpu/drm/gma500/psb_device.c
>> > > index dcfcd7b89d4a..6dece8f0e380 100644
>> > > --- a/drivers/gpu/drm/gma500/psb_device.c
>> > > +++ b/drivers/gpu/drm/gma500/psb_device.c
>> > > @@ -73,8 +73,7 @@ static int psb_backlight_setup(struct drm_device *dev)
>> > > }
>> > >
>> > > psb_intel_lvds_set_brightness(dev, PSB_MAX_BRIGHTNESS);
>> > > - /* This must occur after the backlight is properly initialised */
>> > > - psb_lid_timer_init(dev_priv);
>> > > +
>> > > return 0;
>> > > }
>> > >
>> > > @@ -259,8 +258,6 @@ static int psb_chip_setup(struct drm_device *dev)
>> > >
>> > > static void psb_chip_teardown(struct drm_device *dev)
>> > > {
>> > > - struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>> > > - psb_lid_timer_takedown(dev_priv);
>> > > gma_intel_teardown_gmbus(dev);
>> > > }
>> > >
>> > > diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h
>> > > index c5edfa4aa4cc..83c17689c454 100644
>> > > --- a/drivers/gpu/drm/gma500/psb_drv.h
>> > > +++ b/drivers/gpu/drm/gma500/psb_drv.h
>> > > @@ -162,7 +162,6 @@
>> > > #define PSB_NUM_VBLANKS 2
>> > >
>> > > #define PSB_WATCHDOG_DELAY (HZ * 2)
>> > > -#define PSB_LID_DELAY (HZ / 10)
>> > >
>> > > #define PSB_MAX_BRIGHTNESS 100
>> > >
>> > > @@ -491,11 +490,7 @@ struct drm_psb_private {
>> > > /* Hotplug handling */
>> > > struct work_struct hotplug_work;
>> > >
>> > > - /* LID-Switch */
>> > > - spinlock_t lid_lock;
>> > > - struct timer_list lid_timer;
>> > > struct psb_intel_opregion opregion;
>> > > - u32 lid_last_state;
>> > >
>> > > /* Watchdog */
>> > > uint32_t apm_reg;
>> > > @@ -591,10 +586,6 @@ struct psb_ops {
>> > > int i2c_bus; /* I2C bus identifier for Moorestown */
>> > > };
>> > >
>> > > -/* psb_lid.c */
>> > > -extern void psb_lid_timer_init(struct drm_psb_private *dev_priv);
>> > > -extern void psb_lid_timer_takedown(struct drm_psb_private *dev_priv);
>> > > -
>> > > /* modesetting */
>> > > extern void psb_modeset_init(struct drm_device *dev);
>> > > extern void psb_modeset_cleanup(struct drm_device *dev);
>> > > diff --git a/drivers/gpu/drm/gma500/psb_lid.c b/drivers/gpu/drm/gma500/psb_lid.c
>> > > deleted file mode 100644
>> > > index 58a7fe392636..000000000000
>> > > --- a/drivers/gpu/drm/gma500/psb_lid.c
>> > > +++ /dev/null
>> > > @@ -1,80 +0,0 @@
>> > > -// SPDX-License-Identifier: GPL-2.0-only
>> > > -/**************************************************************************
>> > > - * Copyright (c) 2007, Intel Corporation.
>> > > - *
>> > > - * Authors: Thomas Hellstrom <thomas-at-tungstengraphics-dot-com>
>> > > - **************************************************************************/
>> > > -
>> > > -#include <linux/spinlock.h>
>> > > -
>> > > -#include "psb_drv.h"
>> > > -#include "psb_intel_reg.h"
>> > > -#include "psb_reg.h"
>> > > -
>> > > -static void psb_lid_timer_func(struct timer_list *t)
>> > > -{
>> > > - struct drm_psb_private *dev_priv = from_timer(dev_priv, t, lid_timer);
>> > > - struct drm_device *dev = (struct drm_device *)&dev_priv->dev;
>> > > - struct timer_list *lid_timer = &dev_priv->lid_timer;
>> > > - unsigned long irq_flags;
>> > > - u32 __iomem *lid_state = dev_priv->opregion.lid_state;
>> > > - u32 pp_status;
>> > > -
>> > > - if (readl(lid_state) == dev_priv->lid_last_state)
>> > > - goto lid_timer_schedule;
>> > > -
>> > > - if ((readl(lid_state)) & 0x01) {
>> > > - /*lid state is open*/
>> > > - REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) | POWER_TARGET_ON);
>> > > - do {
>> > > - pp_status = REG_READ(PP_STATUS);
>> > > - } while ((pp_status & PP_ON) == 0 &&
>> > > - (pp_status & PP_SEQUENCE_MASK) != 0);
>> > > -
>> > > - if (REG_READ(PP_STATUS) & PP_ON) {
>> > > - /*FIXME: should be backlight level before*/
>> > > - psb_intel_lvds_set_brightness(dev, 100);
>> > > - } else {
>> > > - DRM_DEBUG("LVDS panel never powered up");
>> > > - return;
>> > > - }
>> > > - } else {
>> > > - psb_intel_lvds_set_brightness(dev, 0);
>> > > -
>> > > - REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) & ~POWER_TARGET_ON);
>> > > - do {
>> > > - pp_status = REG_READ(PP_STATUS);
>> > > - } while ((pp_status & PP_ON) == 0);
>> > > - }
>> > > - dev_priv->lid_last_state = readl(lid_state);
>> > > -
>> > > -lid_timer_schedule:
>> > > - spin_lock_irqsave(&dev_priv->lid_lock, irq_flags);
>> > > - if (!timer_pending(lid_timer)) {
>> > > - lid_timer->expires = jiffies + PSB_LID_DELAY;
>> > > - add_timer(lid_timer);
>> > > - }
>> > > - spin_unlock_irqrestore(&dev_priv->lid_lock, irq_flags);
>> > > -}
>> > > -
>> > > -void psb_lid_timer_init(struct drm_psb_private *dev_priv)
>> > > -{
>> > > - struct timer_list *lid_timer = &dev_priv->lid_timer;
>> > > - unsigned long irq_flags;
>> > > -
>> > > - spin_lock_init(&dev_priv->lid_lock);
>> > > - spin_lock_irqsave(&dev_priv->lid_lock, irq_flags);
>> > > -
>> > > - timer_setup(lid_timer, psb_lid_timer_func, 0);
>> > > -
>> > > - lid_timer->expires = jiffies + PSB_LID_DELAY;
>> > > -
>> > > - add_timer(lid_timer);
>> > > - spin_unlock_irqrestore(&dev_priv->lid_lock, irq_flags);
>> > > -}
>> > > -
>> > > -void psb_lid_timer_takedown(struct drm_psb_private *dev_priv)
>> > > -{
>> > > - del_timer_sync(&dev_priv->lid_timer);
>> > > -}
>> > > -
>> >
>> > --
>> > --
>> > Thomas Zimmermann
>> > Graphics Driver Developer
>> > SUSE Software Solutions Germany GmbH
>> > Frankenstrasse 146, 90461 Nuernberg, Germany
>> > GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> > HRB 36809 (AG Nuernberg)
>> >
