Re: [PATCH v2 3/3] drm/gma500: Fix (vblank) IRQs not working after suspend/resume

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 6, 2022 at 10:38 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> Fix gnome-shell (and other page-flip users) hanging after suspend/resume
> because of the gma500's IRQs not working.
>
> This fixes 2 problems with the IRQ handling:
>
> 1. gma_power_off() calls gma_irq_uninstall() which does a free_irq(), but
>    gma_power_on() called gma_irq_preinstall() + gma_irq_postinstall() which
>    do not call request_irq. Replace the pre- + post-install calls with
>    gma_irq_install() which does prep + request + post.

Hmm, I don't think we're supposed to do free_irq() during suspend in
the first place. request_irq() and free_irq() are normally only called
in the pci device probe/remove hooks. Same is true for msi
enable/disable.

I can take this patch as is since it improves on the current situation
but feel free to dig deeper if you like. On Poulsbo I can see
interrupts not getting handled during suspend/resume even with this
patch applied.

-Patrik

>
> 2. After fixing 1. IRQs still do not work on a Packard Bell Dot SC (Intel
>    Atom N2600, cedarview) netbook.
>
>    Cederview uses MSI interrupts and it seems that the BIOS re-configures
>    things back to normal APIC based interrupts during S3 suspend. There is
>    some MSI PCI-config registers save/restore code which tries to deal with
>    this, but on the Packard Bell Dot SC this is not sufficient to restore
>    MSI IRQ functionality after a suspend/resume.
>
>    Replace the PCI-config registers save/restore with pci_disable_msi() on
>    suspend + pci_enable_msi() on resume. Fixing e.g. gnome-shell hanging.
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/gma500/cdv_device.c      |  4 +---
>  drivers/gpu/drm/gma500/oaktrail_device.c |  5 +----
>  drivers/gpu/drm/gma500/power.c           |  8 +-------
>  drivers/gpu/drm/gma500/psb_drv.c         |  2 +-
>  drivers/gpu/drm/gma500/psb_drv.h         |  5 +----
>  drivers/gpu/drm/gma500/psb_irq.c         | 15 ++++++++++++---
>  drivers/gpu/drm/gma500/psb_irq.h         |  2 +-
>  7 files changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c
> index dd32b484dd82..ce96234f3df2 100644
> --- a/drivers/gpu/drm/gma500/cdv_device.c
> +++ b/drivers/gpu/drm/gma500/cdv_device.c
> @@ -581,11 +581,9 @@ static const struct psb_offset cdv_regmap[2] = {
>  static int cdv_chip_setup(struct drm_device *dev)
>  {
>         struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> -       struct pci_dev *pdev = to_pci_dev(dev->dev);
>         INIT_WORK(&dev_priv->hotplug_work, cdv_hotplug_work_func);
>
> -       if (pci_enable_msi(pdev))
> -               dev_warn(dev->dev, "Enabling MSI failed!\n");
> +       dev_priv->use_msi = true;
>         dev_priv->regmap = cdv_regmap;
>         gma_get_core_freq(dev);
>         psb_intel_opregion_init(dev);
> diff --git a/drivers/gpu/drm/gma500/oaktrail_device.c b/drivers/gpu/drm/gma500/oaktrail_device.c
> index 5923a9c89312..f90e628cb482 100644
> --- a/drivers/gpu/drm/gma500/oaktrail_device.c
> +++ b/drivers/gpu/drm/gma500/oaktrail_device.c
> @@ -501,12 +501,9 @@ static const struct psb_offset oaktrail_regmap[2] = {
>  static int oaktrail_chip_setup(struct drm_device *dev)
>  {
>         struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> -       struct pci_dev *pdev = to_pci_dev(dev->dev);
>         int ret;
>
> -       if (pci_enable_msi(pdev))
> -               dev_warn(dev->dev, "Enabling MSI failed!\n");
> -
> +       dev_priv->use_msi = true;
>         dev_priv->regmap = oaktrail_regmap;
>
>         ret = mid_chip_setup(dev);
> diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c
> index b91de6d36e41..66873085d450 100644
> --- a/drivers/gpu/drm/gma500/power.c
> +++ b/drivers/gpu/drm/gma500/power.c
> @@ -139,8 +139,6 @@ static void gma_suspend_pci(struct pci_dev *pdev)
>         dev_priv->regs.saveBSM = bsm;
>         pci_read_config_dword(pdev, 0xFC, &vbt);
>         dev_priv->regs.saveVBT = vbt;
> -       pci_read_config_dword(pdev, PSB_PCIx_MSI_ADDR_LOC, &dev_priv->msi_addr);
> -       pci_read_config_dword(pdev, PSB_PCIx_MSI_DATA_LOC, &dev_priv->msi_data);
>
>         pci_disable_device(pdev);
>         pci_set_power_state(pdev, PCI_D3hot);
> @@ -168,9 +166,6 @@ static bool gma_resume_pci(struct pci_dev *pdev)
>         pci_restore_state(pdev);
>         pci_write_config_dword(pdev, 0x5c, dev_priv->regs.saveBSM);
>         pci_write_config_dword(pdev, 0xFC, dev_priv->regs.saveVBT);
> -       /* restoring MSI address and data in PCIx space */
> -       pci_write_config_dword(pdev, PSB_PCIx_MSI_ADDR_LOC, dev_priv->msi_addr);
> -       pci_write_config_dword(pdev, PSB_PCIx_MSI_DATA_LOC, dev_priv->msi_data);
>         ret = pci_enable_device(pdev);
>
>         if (ret != 0)
> @@ -223,8 +218,7 @@ int gma_power_resume(struct device *_dev)
>         mutex_lock(&power_mutex);
>         gma_resume_pci(pdev);
>         gma_resume_display(pdev);
> -       gma_irq_preinstall(dev);
> -       gma_irq_postinstall(dev);
> +       gma_irq_install(dev);
>         mutex_unlock(&power_mutex);
>         return 0;
>  }
> diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
> index 1d8744f3e702..54e756b48606 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.c
> +++ b/drivers/gpu/drm/gma500/psb_drv.c
> @@ -383,7 +383,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
>         PSB_WVDC32(0xFFFFFFFF, PSB_INT_MASK_R);
>         spin_unlock_irqrestore(&dev_priv->irqmask_lock, irqflags);
>
> -       gma_irq_install(dev, pdev->irq);
> +       gma_irq_install(dev);
>
>         dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
>
> diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h
> index 0ea3d23575f3..731cc356c07a 100644
> --- a/drivers/gpu/drm/gma500/psb_drv.h
> +++ b/drivers/gpu/drm/gma500/psb_drv.h
> @@ -490,6 +490,7 @@ struct drm_psb_private {
>         int rpm_enabled;
>
>         /* MID specific */
> +       bool use_msi;
>         bool has_gct;
>         struct oaktrail_gct_data gct_data;
>
> @@ -499,10 +500,6 @@ struct drm_psb_private {
>         /* Register state */
>         struct psb_save_area regs;
>
> -       /* MSI reg save */
> -       uint32_t msi_addr;
> -       uint32_t msi_data;
> -
>         /* Hotplug handling */
>         struct work_struct hotplug_work;
>
> diff --git a/drivers/gpu/drm/gma500/psb_irq.c b/drivers/gpu/drm/gma500/psb_irq.c
> index e6e6d61bbeab..038f18ed0a95 100644
> --- a/drivers/gpu/drm/gma500/psb_irq.c
> +++ b/drivers/gpu/drm/gma500/psb_irq.c
> @@ -316,17 +316,24 @@ void gma_irq_postinstall(struct drm_device *dev)
>         spin_unlock_irqrestore(&dev_priv->irqmask_lock, irqflags);
>  }
>
> -int gma_irq_install(struct drm_device *dev, unsigned int irq)
> +int gma_irq_install(struct drm_device *dev)
>  {
> +       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +       struct pci_dev *pdev = to_pci_dev(dev->dev);
>         int ret;
>
> -       if (irq == IRQ_NOTCONNECTED)
> +       if (dev_priv->use_msi && pci_enable_msi(pdev)) {
> +               dev_warn(dev->dev, "Enabling MSI failed!\n");
> +               dev_priv->use_msi = false;
> +       }
> +
> +       if (pdev->irq == IRQ_NOTCONNECTED)
>                 return -ENOTCONN;
>
>         gma_irq_preinstall(dev);
>
>         /* PCI devices require shared interrupts. */
> -       ret = request_irq(irq, gma_irq_handler, IRQF_SHARED, dev->driver->name, dev);
> +       ret = request_irq(pdev->irq, gma_irq_handler, IRQF_SHARED, dev->driver->name, dev);
>         if (ret)
>                 return ret;
>
> @@ -369,6 +376,8 @@ void gma_irq_uninstall(struct drm_device *dev)
>         spin_unlock_irqrestore(&dev_priv->irqmask_lock, irqflags);
>
>         free_irq(pdev->irq, dev);
> +       if (dev_priv->use_msi)
> +               pci_disable_msi(pdev);
>  }
>
>  int gma_crtc_enable_vblank(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/gma500/psb_irq.h b/drivers/gpu/drm/gma500/psb_irq.h
> index b51e395194ff..7648f69824a5 100644
> --- a/drivers/gpu/drm/gma500/psb_irq.h
> +++ b/drivers/gpu/drm/gma500/psb_irq.h
> @@ -17,7 +17,7 @@ struct drm_device;
>
>  void gma_irq_preinstall(struct drm_device *dev);
>  void gma_irq_postinstall(struct drm_device *dev);
> -int  gma_irq_install(struct drm_device *dev, unsigned int irq);
> +int  gma_irq_install(struct drm_device *dev);
>  void gma_irq_uninstall(struct drm_device *dev);
>
>  int  gma_crtc_enable_vblank(struct drm_crtc *crtc);
> --
> 2.37.2
>



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux