| Message ID | 20240412072409.27650-1-patrik.r.jakobsson@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/gma500: Check power status before accessing lid data in opregion | expand |
Hi, the issue of hanging during boot is still resent. Best regards Thomas Am 12.04.24 um 09:24 schrieb Patrik Jakobsson: > Due to changes in the order of initialization the psb_lid_timer_func > could get called without the device being powered. Fix this by checking > the power status before accessing the opregion. > > 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/psb_lid.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/gma500/psb_lid.c b/drivers/gpu/drm/gma500/psb_lid.c > index 58a7fe392636..eeb91d11336e 100644 > --- a/drivers/gpu/drm/gma500/psb_lid.c > +++ b/drivers/gpu/drm/gma500/psb_lid.c > @@ -10,6 +10,7 @@ > #include "psb_drv.h" > #include "psb_intel_reg.h" > #include "psb_reg.h" > +#include "power.h" > > static void psb_lid_timer_func(struct timer_list *t) > { > @@ -20,9 +21,12 @@ static void psb_lid_timer_func(struct timer_list *t) > u32 __iomem *lid_state = dev_priv->opregion.lid_state; > u32 pp_status; > > - if (readl(lid_state) == dev_priv->lid_last_state) > + if (!gma_power_begin(dev, false)) > goto lid_timer_schedule; > > + if (readl(lid_state) == dev_priv->lid_last_state) > + goto power_end; > + > if ((readl(lid_state)) & 0x01) { > /*lid state is open*/ > REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) | POWER_TARGET_ON); > @@ -36,6 +40,7 @@ static void psb_lid_timer_func(struct timer_list *t) > psb_intel_lvds_set_brightness(dev, 100); > } else { > DRM_DEBUG("LVDS panel never powered up"); > + gma_power_end(dev); > return; > } > } else { > @@ -48,6 +53,9 @@ static void psb_lid_timer_func(struct timer_list *t) > } > dev_priv->lid_last_state = readl(lid_state); > > +power_end: > + gma_power_end(dev); > + > lid_timer_schedule: > spin_lock_irqsave(&dev_priv->lid_lock, irq_flags); > if (!timer_pending(lid_timer)) {
Am 12.04.24 um 10:02 schrieb Thomas Zimmermann: > Hi, > > the issue of hanging during boot is still resent. 'present' > > Best regards > Thomas > > Am 12.04.24 um 09:24 schrieb Patrik Jakobsson: >> Due to changes in the order of initialization the psb_lid_timer_func >> could get called without the device being powered. Fix this by checking >> the power status before accessing the opregion. >> >> 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/psb_lid.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/gma500/psb_lid.c >> b/drivers/gpu/drm/gma500/psb_lid.c >> index 58a7fe392636..eeb91d11336e 100644 >> --- a/drivers/gpu/drm/gma500/psb_lid.c >> +++ b/drivers/gpu/drm/gma500/psb_lid.c >> @@ -10,6 +10,7 @@ >> #include "psb_drv.h" >> #include "psb_intel_reg.h" >> #include "psb_reg.h" >> +#include "power.h" >> static void psb_lid_timer_func(struct timer_list *t) >> { >> @@ -20,9 +21,12 @@ static void psb_lid_timer_func(struct timer_list *t) >> u32 __iomem *lid_state = dev_priv->opregion.lid_state; >> u32 pp_status; >> - if (readl(lid_state) == dev_priv->lid_last_state) >> + if (!gma_power_begin(dev, false)) >> goto lid_timer_schedule; >> + if (readl(lid_state) == dev_priv->lid_last_state) >> + goto power_end; >> + >> if ((readl(lid_state)) & 0x01) { >> /*lid state is open*/ >> REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) | POWER_TARGET_ON); >> @@ -36,6 +40,7 @@ static void psb_lid_timer_func(struct timer_list *t) >> psb_intel_lvds_set_brightness(dev, 100); >> } else { >> DRM_DEBUG("LVDS panel never powered up"); >> + gma_power_end(dev); >> return; >> } >> } else { >> @@ -48,6 +53,9 @@ static void psb_lid_timer_func(struct timer_list *t) >> } >> dev_priv->lid_last_state = readl(lid_state); >> +power_end: >> + gma_power_end(dev); >> + >> lid_timer_schedule: >> spin_lock_irqsave(&dev_priv->lid_lock, irq_flags); >> if (!timer_pending(lid_timer)) { >
On Fri, Apr 12, 2024 at 10:02 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Hi, > > the issue of hanging during boot is still resent. Thanks for testing. Then it cannot be that psb_lid_timer_func runs before initialization. The BUG from Enrico hints that the set_brightness function is called before initialization. That might be another place to look. I'll see if I can do some testing on my own. > > Best regards > Thomas > > Am 12.04.24 um 09:24 schrieb Patrik Jakobsson: > > Due to changes in the order of initialization the psb_lid_timer_func > > could get called without the device being powered. Fix this by checking > > the power status before accessing the opregion. > > > > 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/psb_lid.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/gma500/psb_lid.c b/drivers/gpu/drm/gma500/psb_lid.c > > index 58a7fe392636..eeb91d11336e 100644 > > --- a/drivers/gpu/drm/gma500/psb_lid.c > > +++ b/drivers/gpu/drm/gma500/psb_lid.c > > @@ -10,6 +10,7 @@ > > #include "psb_drv.h" > > #include "psb_intel_reg.h" > > #include "psb_reg.h" > > +#include "power.h" > > > > static void psb_lid_timer_func(struct timer_list *t) > > { > > @@ -20,9 +21,12 @@ static void psb_lid_timer_func(struct timer_list *t) > > u32 __iomem *lid_state = dev_priv->opregion.lid_state; > > u32 pp_status; > > > > - if (readl(lid_state) == dev_priv->lid_last_state) > > + if (!gma_power_begin(dev, false)) > > goto lid_timer_schedule; > > > > + if (readl(lid_state) == dev_priv->lid_last_state) > > + goto power_end; > > + > > if ((readl(lid_state)) & 0x01) { > > /*lid state is open*/ > > REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) | POWER_TARGET_ON); > > @@ -36,6 +40,7 @@ static void psb_lid_timer_func(struct timer_list *t) > > psb_intel_lvds_set_brightness(dev, 100); > > } else { > > DRM_DEBUG("LVDS panel never powered up"); > > + gma_power_end(dev); > > return; > > } > > } else { > > @@ -48,6 +53,9 @@ static void psb_lid_timer_func(struct timer_list *t) > > } > > dev_priv->lid_last_state = readl(lid_state); > > > > +power_end: > > + gma_power_end(dev); > > + > > lid_timer_schedule: > > spin_lock_irqsave(&dev_priv->lid_lock, irq_flags); > > if (!timer_pending(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 Am 12.04.24 um 10:16 schrieb Patrik Jakobsson: > On Fri, Apr 12, 2024 at 10:02 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: >> Hi, >> >> the issue of hanging during boot is still resent. > Thanks for testing. Then it cannot be that psb_lid_timer_func runs > before initialization. The BUG from Enrico hints that the > set_brightness function is called before initialization. That might be > another place to look. set_briggtness happens within psb_lid_timer_func() and within psb_intel_lvds_encoder_dpms() I see this: [ 15.509688] gma500 0000:00:02.0: Backlight lvds set brightness 7a127a12 [ 15.509791] gma500 0000:00:02.0: [drm] Skipping psb-bl backlight registration [ 15.523883] acpi device:0b: registered as cooling_device2 [ 15.526252] [drm] Initialized gma500 1.0.0 20140314 for 0000:00:02.0 on minor 0 [ 15.613363] gma500 0000:00:02.0: Backlight lvds set brightness 7a127a12 before booting stops. Best regards Thomas > > I'll see if I can do some testing on my own. > >> Best regards >> Thomas >> >> Am 12.04.24 um 09:24 schrieb Patrik Jakobsson: >>> Due to changes in the order of initialization the psb_lid_timer_func >>> could get called without the device being powered. Fix this by checking >>> the power status before accessing the opregion. >>> >>> 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/psb_lid.c | 10 +++++++++- >>> 1 file changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/gma500/psb_lid.c b/drivers/gpu/drm/gma500/psb_lid.c >>> index 58a7fe392636..eeb91d11336e 100644 >>> --- a/drivers/gpu/drm/gma500/psb_lid.c >>> +++ b/drivers/gpu/drm/gma500/psb_lid.c >>> @@ -10,6 +10,7 @@ >>> #include "psb_drv.h" >>> #include "psb_intel_reg.h" >>> #include "psb_reg.h" >>> +#include "power.h" >>> >>> static void psb_lid_timer_func(struct timer_list *t) >>> { >>> @@ -20,9 +21,12 @@ static void psb_lid_timer_func(struct timer_list *t) >>> u32 __iomem *lid_state = dev_priv->opregion.lid_state; >>> u32 pp_status; >>> >>> - if (readl(lid_state) == dev_priv->lid_last_state) >>> + if (!gma_power_begin(dev, false)) >>> goto lid_timer_schedule; >>> >>> + if (readl(lid_state) == dev_priv->lid_last_state) >>> + goto power_end; >>> + >>> if ((readl(lid_state)) & 0x01) { >>> /*lid state is open*/ >>> REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) | POWER_TARGET_ON); >>> @@ -36,6 +40,7 @@ static void psb_lid_timer_func(struct timer_list *t) >>> psb_intel_lvds_set_brightness(dev, 100); >>> } else { >>> DRM_DEBUG("LVDS panel never powered up"); >>> + gma_power_end(dev); >>> return; >>> } >>> } else { >>> @@ -48,6 +53,9 @@ static void psb_lid_timer_func(struct timer_list *t) >>> } >>> dev_priv->lid_last_state = readl(lid_state); >>> >>> +power_end: >>> + gma_power_end(dev); >>> + >>> lid_timer_schedule: >>> spin_lock_irqsave(&dev_priv->lid_lock, irq_flags); >>> if (!timer_pending(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/psb_lid.c b/drivers/gpu/drm/gma500/psb_lid.c index 58a7fe392636..eeb91d11336e 100644 --- a/drivers/gpu/drm/gma500/psb_lid.c +++ b/drivers/gpu/drm/gma500/psb_lid.c @@ -10,6 +10,7 @@ #include "psb_drv.h" #include "psb_intel_reg.h" #include "psb_reg.h" +#include "power.h" static void psb_lid_timer_func(struct timer_list *t) { @@ -20,9 +21,12 @@ static void psb_lid_timer_func(struct timer_list *t) u32 __iomem *lid_state = dev_priv->opregion.lid_state; u32 pp_status; - if (readl(lid_state) == dev_priv->lid_last_state) + if (!gma_power_begin(dev, false)) goto lid_timer_schedule; + if (readl(lid_state) == dev_priv->lid_last_state) + goto power_end; + if ((readl(lid_state)) & 0x01) { /*lid state is open*/ REG_WRITE(PP_CONTROL, REG_READ(PP_CONTROL) | POWER_TARGET_ON); @@ -36,6 +40,7 @@ static void psb_lid_timer_func(struct timer_list *t) psb_intel_lvds_set_brightness(dev, 100); } else { DRM_DEBUG("LVDS panel never powered up"); + gma_power_end(dev); return; } } else { @@ -48,6 +53,9 @@ static void psb_lid_timer_func(struct timer_list *t) } dev_priv->lid_last_state = readl(lid_state); +power_end: + gma_power_end(dev); + lid_timer_schedule: spin_lock_irqsave(&dev_priv->lid_lock, irq_flags); if (!timer_pending(lid_timer)) {
Due to changes in the order of initialization the psb_lid_timer_func could get called without the device being powered. Fix this by checking the power status before accessing the opregion. 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/psb_lid.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)