| Message ID | 20240415112731.31841-1-patrik.r.jakobsson@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/gma500: Remove lid code | expand |
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@gmail.com> The patch deserves a Reported-by: from Enrico. With this fixed: Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> Best regards Thomas > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > --- > 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); > -} > -
On Mon, Apr 15, 2024 at 1:45 PM Thomas Zimmermann <tzimmermann@suse.de> 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@gmail.com> > > 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@suse.de> > > Best regards > Thomas > > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > > --- > > 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) >
Hi, sorry for the delay. This patch fixes the crash during boot! (tested against linux 6.9-rc3) Greetings Am Mo., 15. Apr. 2024 um 13:57 Uhr schrieb Patrik Jakobsson < patrik.r.jakobsson@gmail.com>: > On Mon, Apr 15, 2024 at 1:45 PM Thomas Zimmermann <tzimmermann@suse.de> > 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@gmail.com> > > > > 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@suse.de> > > > > Best regards > > Thomas > > > > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > > > --- > > > 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) > > >
On Wed, Apr 17, 2024 at 11:47 AM Enrico Bartky <enrico.bartky@gmail.com> 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@gmail.com>: >> >> On Mon, Apr 15, 2024 at 1:45 PM Thomas Zimmermann <tzimmermann@suse.de> 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@gmail.com> >> > >> > 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@suse.de> >> > >> > Best regards >> > Thomas >> > >> > > Cc: Thomas Zimmermann <tzimmermann@suse.de> >> > > Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> >> > > --- >> > > 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) >> >
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); -} -
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@gmail.com> Cc: Thomas Zimmermann <tzimmermann@suse.de> Signed-off-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> --- 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