[PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code v2

Alex Deucher alexdeucher at gmail.com
Thu May 31 13:33:56 PDT 2012


On Thu, May 31, 2012 at 4:16 PM, Christian König
<deathsimple at vodafone.de> wrote:
> From: Christian Koenig <christian.koenig at amd.com>
>
> 1. It is really dangerous to have more than one
>   spinlock protecting the same information.
>
> 2. radeon_irq_set sometimes wasn't called with lock
>   protection, so it can happen that more than one
>   CPU would tamper with the irq regs at the same
>   time.
>
> 3. The pm.gui_idle variable was assuming that the 3D
>   engine wasn't becoming idle between testing the
>   register and setting the variable. So just remove
>   it and test the register directly.
>
> v2: Also handle the hpd irq code the same way.

couple of comments below for clarity, other than that:

Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

>
> Signed-off-by: Christian Koenig <christian.koenig at amd.com>
> ---
>  drivers/gpu/drm/radeon/evergreen.c      |   21 ++-----
>  drivers/gpu/drm/radeon/r100.c           |   29 ++--------
>  drivers/gpu/drm/radeon/r600.c           |   38 ++++--------
>  drivers/gpu/drm/radeon/r600_hdmi.c      |    6 +-
>  drivers/gpu/drm/radeon/radeon.h         |   35 +++++------
>  drivers/gpu/drm/radeon/radeon_irq_kms.c |   96 +++++++++++++++++++++++++++----
>  drivers/gpu/drm/radeon/radeon_kms.c     |   12 +++-
>  drivers/gpu/drm/radeon/radeon_pm.c      |   12 +---
>  drivers/gpu/drm/radeon/rs600.c          |   13 ++---
>  drivers/gpu/drm/radeon/si.c             |    1 -
>  10 files changed, 144 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
> index e4c3321..48ec1a0 100644
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -428,6 +428,7 @@ void evergreen_hpd_init(struct radeon_device *rdev)
>  {
>        struct drm_device *dev = rdev->ddev;
>        struct drm_connector *connector;
> +       unsigned enabled = 0;
>        u32 tmp = DC_HPDx_CONNECTION_TIMER(0x9c4) |
>                DC_HPDx_RX_INT_TIMER(0xfa) | DC_HPDx_EN;
>
> @@ -436,73 +437,64 @@ void evergreen_hpd_init(struct radeon_device *rdev)
>                switch (radeon_connector->hpd.hpd) {
>                case RADEON_HPD_1:
>                        WREG32(DC_HPD1_CONTROL, tmp);
> -                       rdev->irq.hpd[0] = true;
>                        break;
>                case RADEON_HPD_2:
>                        WREG32(DC_HPD2_CONTROL, tmp);
> -                       rdev->irq.hpd[1] = true;
>                        break;
>                case RADEON_HPD_3:
>                        WREG32(DC_HPD3_CONTROL, tmp);
> -                       rdev->irq.hpd[2] = true;
>                        break;
>                case RADEON_HPD_4:
>                        WREG32(DC_HPD4_CONTROL, tmp);
> -                       rdev->irq.hpd[3] = true;
>                        break;
>                case RADEON_HPD_5:
>                        WREG32(DC_HPD5_CONTROL, tmp);
> -                       rdev->irq.hpd[4] = true;
>                        break;
>                case RADEON_HPD_6:
>                        WREG32(DC_HPD6_CONTROL, tmp);
> -                       rdev->irq.hpd[5] = true;
>                        break;
>                default:
>                        break;
>                }
>                radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd);
> +               enabled |= 1 << radeon_connector->hpd.hpd;
>        }
> -       if (rdev->irq.installed)
> -               evergreen_irq_set(rdev);
> +       radeon_irq_kms_enable_hpd(rdev, enabled);
>  }
>
>  void evergreen_hpd_fini(struct radeon_device *rdev)
>  {
>        struct drm_device *dev = rdev->ddev;
>        struct drm_connector *connector;
> +       unsigned disabled = 0;
>
>        list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>                struct radeon_connector *radeon_connector = to_radeon_connector(connector);
>                switch (radeon_connector->hpd.hpd) {
>                case RADEON_HPD_1:
>                        WREG32(DC_HPD1_CONTROL, 0);
> -                       rdev->irq.hpd[0] = false;
>                        break;
>                case RADEON_HPD_2:
>                        WREG32(DC_HPD2_CONTROL, 0);
> -                       rdev->irq.hpd[1] = false;
>                        break;
>                case RADEON_HPD_3:
>                        WREG32(DC_HPD3_CONTROL, 0);
> -                       rdev->irq.hpd[2] = false;
>                        break;
>                case RADEON_HPD_4:
>                        WREG32(DC_HPD4_CONTROL, 0);
> -                       rdev->irq.hpd[3] = false;
>                        break;
>                case RADEON_HPD_5:
>                        WREG32(DC_HPD5_CONTROL, 0);
> -                       rdev->irq.hpd[4] = false;
>                        break;
>                case RADEON_HPD_6:
>                        WREG32(DC_HPD6_CONTROL, 0);
> -                       rdev->irq.hpd[5] = false;
>                        break;
>                default:
>                        break;
>                }
> +               disabled |= 1 << radeon_connector->hpd.hpd;
>        }
> +       radeon_irq_kms_disable_hpd(rdev, disabled);
>  }
>
>  /* watermark setup */
> @@ -3252,7 +3244,6 @@ restart_ih:
>                        break;
>                case 233: /* GUI IDLE */
>                        DRM_DEBUG("IH: GUI idle\n");
> -                       rdev->pm.gui_idle = true;
>                        wake_up(&rdev->irq.idle_queue);
>                        break;
>                default:
> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
> index 415b7d8..e8fe4ae 100644
> --- a/drivers/gpu/drm/radeon/r100.c
> +++ b/drivers/gpu/drm/radeon/r100.c
> @@ -567,43 +567,27 @@ void r100_hpd_init(struct radeon_device *rdev)
>  {
>        struct drm_device *dev = rdev->ddev;
>        struct drm_connector *connector;
> +       unsigned enable = 0;
>
>        list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>                struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> -               switch (radeon_connector->hpd.hpd) {
> -               case RADEON_HPD_1:
> -                       rdev->irq.hpd[0] = true;
> -                       break;
> -               case RADEON_HPD_2:
> -                       rdev->irq.hpd[1] = true;
> -                       break;
> -               default:
> -                       break;
> -               }
> +               enable |= 1 << radeon_connector->hpd.hpd;
>                radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd);
>        }
> -       if (rdev->irq.installed)
> -               r100_irq_set(rdev);
> +       radeon_irq_kms_enable_hpd(rdev, enable);
>  }
>
>  void r100_hpd_fini(struct radeon_device *rdev)
>  {
>        struct drm_device *dev = rdev->ddev;
>        struct drm_connector *connector;
> +       unsigned disable = 0;
>
>        list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>                struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> -               switch (radeon_connector->hpd.hpd) {
> -               case RADEON_HPD_1:
> -                       rdev->irq.hpd[0] = false;
> -                       break;
> -               case RADEON_HPD_2:
> -                       rdev->irq.hpd[1] = false;
> -                       break;
> -               default:
> -                       break;
> -               }
> +               disable |= 1 << radeon_connector->hpd.hpd;
>        }
> +       radeon_irq_kms_disable_hpd(rdev, disable);
>  }
>
>  /*
> @@ -782,7 +766,6 @@ int r100_irq_process(struct radeon_device *rdev)
>                /* gui idle interrupt */
>                if (status & RADEON_GUI_IDLE_STAT) {
>                        rdev->irq.gui_idle_acked = true;
> -                       rdev->pm.gui_idle = true;
>                        wake_up(&rdev->irq.idle_queue);
>                }
>                /* Vertical blank interrupts */
> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
> index e1861ac..72b5084 100644
> --- a/drivers/gpu/drm/radeon/r600.c
> +++ b/drivers/gpu/drm/radeon/r600.c
> @@ -709,6 +709,7 @@ void r600_hpd_init(struct radeon_device *rdev)
>  {
>        struct drm_device *dev = rdev->ddev;
>        struct drm_connector *connector;
> +       unsigned enable = 0;
>
>        list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>                struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> @@ -729,28 +730,22 @@ void r600_hpd_init(struct radeon_device *rdev)
>                        switch (radeon_connector->hpd.hpd) {
>                        case RADEON_HPD_1:
>                                WREG32(DC_HPD1_CONTROL, tmp);
> -                               rdev->irq.hpd[0] = true;
>                                break;
>                        case RADEON_HPD_2:
>                                WREG32(DC_HPD2_CONTROL, tmp);
> -                               rdev->irq.hpd[1] = true;
>                                break;
>                        case RADEON_HPD_3:
>                                WREG32(DC_HPD3_CONTROL, tmp);
> -                               rdev->irq.hpd[2] = true;
>                                break;
>                        case RADEON_HPD_4:
>                                WREG32(DC_HPD4_CONTROL, tmp);
> -                               rdev->irq.hpd[3] = true;
>                                break;
>                                /* DCE 3.2 */
>                        case RADEON_HPD_5:
>                                WREG32(DC_HPD5_CONTROL, tmp);
> -                               rdev->irq.hpd[4] = true;
>                                break;
>                        case RADEON_HPD_6:
>                                WREG32(DC_HPD6_CONTROL, tmp);
> -                               rdev->irq.hpd[5] = true;
>                                break;
>                        default:
>                                break;
> @@ -759,85 +754,73 @@ void r600_hpd_init(struct radeon_device *rdev)
>                        switch (radeon_connector->hpd.hpd) {
>                        case RADEON_HPD_1:
>                                WREG32(DC_HOT_PLUG_DETECT1_CONTROL, DC_HOT_PLUG_DETECTx_EN);
> -                               rdev->irq.hpd[0] = true;
>                                break;
>                        case RADEON_HPD_2:
>                                WREG32(DC_HOT_PLUG_DETECT2_CONTROL, DC_HOT_PLUG_DETECTx_EN);
> -                               rdev->irq.hpd[1] = true;
>                                break;
>                        case RADEON_HPD_3:
>                                WREG32(DC_HOT_PLUG_DETECT3_CONTROL, DC_HOT_PLUG_DETECTx_EN);
> -                               rdev->irq.hpd[2] = true;
>                                break;
>                        default:
>                                break;
>                        }
>                }
> +               enable |= 1 << radeon_connector->hpd.hpd;
>                radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd);
>        }
> -       if (rdev->irq.installed)
> -               r600_irq_set(rdev);
> +       radeon_irq_kms_enable_hpd(rdev, enable);
>  }
>
>  void r600_hpd_fini(struct radeon_device *rdev)
>  {
>        struct drm_device *dev = rdev->ddev;
>        struct drm_connector *connector;
> +       unsigned disable = 0;
>
> -       if (ASIC_IS_DCE3(rdev)) {
> -               list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> -                       struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> +       list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> +               struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> +               if (ASIC_IS_DCE3(rdev)) {
>                        switch (radeon_connector->hpd.hpd) {
>                        case RADEON_HPD_1:
>                                WREG32(DC_HPD1_CONTROL, 0);
> -                               rdev->irq.hpd[0] = false;
>                                break;
>                        case RADEON_HPD_2:
>                                WREG32(DC_HPD2_CONTROL, 0);
> -                               rdev->irq.hpd[1] = false;
>                                break;
>                        case RADEON_HPD_3:
>                                WREG32(DC_HPD3_CONTROL, 0);
> -                               rdev->irq.hpd[2] = false;
>                                break;
>                        case RADEON_HPD_4:
>                                WREG32(DC_HPD4_CONTROL, 0);
> -                               rdev->irq.hpd[3] = false;
>                                break;
>                                /* DCE 3.2 */
>                        case RADEON_HPD_5:
>                                WREG32(DC_HPD5_CONTROL, 0);
> -                               rdev->irq.hpd[4] = false;
>                                break;
>                        case RADEON_HPD_6:
>                                WREG32(DC_HPD6_CONTROL, 0);
> -                               rdev->irq.hpd[5] = false;
>                                break;
>                        default:
>                                break;
>                        }
> -               }
> -       } else {
> -               list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> -                       struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> +               } else {
>                        switch (radeon_connector->hpd.hpd) {
>                        case RADEON_HPD_1:
>                                WREG32(DC_HOT_PLUG_DETECT1_CONTROL, 0);
> -                               rdev->irq.hpd[0] = false;
>                                break;
>                        case RADEON_HPD_2:
>                                WREG32(DC_HOT_PLUG_DETECT2_CONTROL, 0);
> -                               rdev->irq.hpd[1] = false;
>                                break;
>                        case RADEON_HPD_3:
>                                WREG32(DC_HOT_PLUG_DETECT3_CONTROL, 0);
> -                               rdev->irq.hpd[2] = false;
>                                break;
>                        default:
>                                break;
>                        }
>                }
> +               disable |= 1 << radeon_connector->hpd.hpd;
>        }
> +       radeon_irq_kms_disable_hpd(rdev, disable);
>  }
>
>  /*
> @@ -3541,7 +3524,6 @@ restart_ih:
>                        break;
>                case 233: /* GUI IDLE */
>                        DRM_DEBUG("IH: GUI idle\n");
> -                       rdev->pm.gui_idle = true;
>                        wake_up(&rdev->irq.idle_queue);
>                        break;
>                default:
> diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeon/r600_hdmi.c
> index 226379e..b76c0f2 100644
> --- a/drivers/gpu/drm/radeon/r600_hdmi.c
> +++ b/drivers/gpu/drm/radeon/r600_hdmi.c
> @@ -523,8 +523,7 @@ void r600_hdmi_enable(struct drm_encoder *encoder)
>
>        if (rdev->irq.installed) {
>                /* if irq is available use it */
> -               rdev->irq.afmt[dig->afmt->id] = true;
> -               radeon_irq_set(rdev);
> +               radeon_irq_kms_enable_afmt(rdev, dig->afmt->id);
>        }
>
>        dig->afmt->enabled = true;
> @@ -560,8 +559,7 @@ void r600_hdmi_disable(struct drm_encoder *encoder)
>                  offset, radeon_encoder->encoder_id);
>
>        /* disable irq */
> -       rdev->irq.afmt[dig->afmt->id] = false;
> -       radeon_irq_set(rdev);
> +       radeon_irq_kms_disable_afmt(rdev, dig->afmt->id);
>
>        /* Older chipsets not handled by AtomBIOS */
>        if (rdev->family >= CHIP_R600 && !ASIC_IS_DCE3(rdev)) {
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 378d43b..8a2a1f4 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -610,21 +610,20 @@ union radeon_irq_stat_regs {
>  #define RADEON_MAX_AFMT_BLOCKS 6
>
>  struct radeon_irq {
> -       bool            installed;
> -       bool            sw_int[RADEON_NUM_RINGS];
> -       bool            crtc_vblank_int[RADEON_MAX_CRTCS];
> -       bool            pflip[RADEON_MAX_CRTCS];
> -       wait_queue_head_t       vblank_queue;
> -       bool            hpd[RADEON_MAX_HPD_PINS];
> -       bool            gui_idle;
> -       bool            gui_idle_acked;
> -       wait_queue_head_t       idle_queue;
> -       bool            afmt[RADEON_MAX_AFMT_BLOCKS];
> -       spinlock_t sw_lock;
> -       int sw_refcount[RADEON_NUM_RINGS];
> -       union radeon_irq_stat_regs stat_regs;
> -       spinlock_t pflip_lock[RADEON_MAX_CRTCS];
> -       int pflip_refcount[RADEON_MAX_CRTCS];
> +       bool                            installed;
> +       spinlock_t                      lock;
> +       bool                            sw_int[RADEON_NUM_RINGS];
> +       int                             sw_refcount[RADEON_NUM_RINGS];
> +       bool                            crtc_vblank_int[RADEON_MAX_CRTCS];
> +       bool                            pflip[RADEON_MAX_CRTCS];
> +       int                             pflip_refcount[RADEON_MAX_CRTCS];
> +       wait_queue_head_t               vblank_queue;
> +       bool                            hpd[RADEON_MAX_HPD_PINS];
> +       bool                            gui_idle;
> +       bool                            gui_idle_acked;
> +       wait_queue_head_t               idle_queue;
> +       bool                            afmt[RADEON_MAX_AFMT_BLOCKS];
> +       union radeon_irq_stat_regs      stat_regs;
>  };
>
>  int radeon_irq_kms_init(struct radeon_device *rdev);
> @@ -633,6 +632,11 @@ void radeon_irq_kms_sw_irq_get(struct radeon_device *rdev, int ring);
>  void radeon_irq_kms_sw_irq_put(struct radeon_device *rdev, int ring);
>  void radeon_irq_kms_pflip_irq_get(struct radeon_device *rdev, int crtc);
>  void radeon_irq_kms_pflip_irq_put(struct radeon_device *rdev, int crtc);
> +void radeon_irq_kms_enable_afmt(struct radeon_device *rdev, int block);
> +void radeon_irq_kms_disable_afmt(struct radeon_device *rdev, int block);
> +void radeon_irq_kms_enable_hpd(struct radeon_device *rdev, unsigned crtcs);
> +void radeon_irq_kms_disable_hpd(struct radeon_device *rdev, unsigned crtcs);
> +int radeon_irq_kms_wait_gui_idle(struct radeon_device *rdev);
>
>  /*
>  * CP & rings.
> @@ -1058,7 +1062,6 @@ struct radeon_pm {
>        int                     active_crtc_count;
>        int                     req_vblank;
>        bool                    vblank_sync;
> -       bool                    gui_idle;
>        fixed20_12              max_bandwidth;
>        fixed20_12              igp_sideport_mclk;
>        fixed20_12              igp_system_mclk;
> diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> index 5df58d1..73cd0fd 100644
> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> @@ -32,6 +32,8 @@
>  #include "radeon.h"
>  #include "atom.h"
>
> +#define RADEON_WAIT_IDLE_TIMEOUT 200
> +
>  irqreturn_t radeon_driver_irq_handler_kms(DRM_IRQ_ARGS)
>  {
>        struct drm_device *dev = (struct drm_device *) arg;
> @@ -62,8 +64,10 @@ static void radeon_hotplug_work_func(struct work_struct *work)
>  void radeon_driver_irq_preinstall_kms(struct drm_device *dev)
>  {
>        struct radeon_device *rdev = dev->dev_private;
> +       unsigned long irqflags;
>        unsigned i;
>
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
>        /* Disable *all* interrupts */
>        for (i = 0; i < RADEON_NUM_RINGS; i++)
>                rdev->irq.sw_int[i] = false;
> @@ -76,6 +80,7 @@ void radeon_driver_irq_preinstall_kms(struct drm_device *dev)
>                rdev->irq.afmt[i] = false;
>        }
>        radeon_irq_set(rdev);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
>        /* Clear bits */
>        radeon_irq_process(rdev);
>  }
> @@ -83,23 +88,28 @@ void radeon_driver_irq_preinstall_kms(struct drm_device *dev)
>  int radeon_driver_irq_postinstall_kms(struct drm_device *dev)
>  {
>        struct radeon_device *rdev = dev->dev_private;
> +       unsigned long irqflags;
>        unsigned i;
>
>        dev->max_vblank_count = 0x001fffff;
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
>        for (i = 0; i < RADEON_NUM_RINGS; i++)
>                rdev->irq.sw_int[i] = true;
>        radeon_irq_set(rdev);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
>        return 0;
>  }
>
>  void radeon_driver_irq_uninstall_kms(struct drm_device *dev)
>  {
>        struct radeon_device *rdev = dev->dev_private;
> +       unsigned long irqflags;
>        unsigned i;
>
>        if (rdev == NULL) {
>                return;
>        }
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
>        /* Disable *all* interrupts */
>        for (i = 0; i < RADEON_NUM_RINGS; i++)
>                rdev->irq.sw_int[i] = false;
> @@ -112,6 +122,7 @@ void radeon_driver_irq_uninstall_kms(struct drm_device *dev)
>                rdev->irq.afmt[i] = false;
>        }
>        radeon_irq_set(rdev);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
>  }
>
>  static bool radeon_msi_ok(struct radeon_device *rdev)
> @@ -168,15 +179,12 @@ static bool radeon_msi_ok(struct radeon_device *rdev)
>
>  int radeon_irq_kms_init(struct radeon_device *rdev)
>  {
> -       int i;
>        int r = 0;
>
>        INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
>        INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi);
>
> -       spin_lock_init(&rdev->irq.sw_lock);
> -       for (i = 0; i < rdev->num_crtc; i++)
> -               spin_lock_init(&rdev->irq.pflip_lock[i]);
> +       spin_lock_init(&rdev->irq.lock);
>        r = drm_vblank_init(rdev->ddev, rdev->num_crtc);
>        if (r) {
>                return r;
> @@ -217,25 +225,25 @@ void radeon_irq_kms_sw_irq_get(struct radeon_device *rdev, int ring)
>  {
>        unsigned long irqflags;
>
> -       spin_lock_irqsave(&rdev->irq.sw_lock, irqflags);
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
>        if (rdev->ddev->irq_enabled && (++rdev->irq.sw_refcount[ring] == 1)) {
>                rdev->irq.sw_int[ring] = true;
>                radeon_irq_set(rdev);
>        }
> -       spin_unlock_irqrestore(&rdev->irq.sw_lock, irqflags);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
>  }
>
>  void radeon_irq_kms_sw_irq_put(struct radeon_device *rdev, int ring)
>  {
>        unsigned long irqflags;
>
> -       spin_lock_irqsave(&rdev->irq.sw_lock, irqflags);
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
>        BUG_ON(rdev->ddev->irq_enabled && rdev->irq.sw_refcount[ring] <= 0);
>        if (rdev->ddev->irq_enabled && (--rdev->irq.sw_refcount[ring] == 0)) {
>                rdev->irq.sw_int[ring] = false;
>                radeon_irq_set(rdev);
>        }
> -       spin_unlock_irqrestore(&rdev->irq.sw_lock, irqflags);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
>  }
>
>  void radeon_irq_kms_pflip_irq_get(struct radeon_device *rdev, int crtc)
> @@ -245,12 +253,12 @@ void radeon_irq_kms_pflip_irq_get(struct radeon_device *rdev, int crtc)
>        if (crtc < 0 || crtc >= rdev->num_crtc)
>                return;
>
> -       spin_lock_irqsave(&rdev->irq.pflip_lock[crtc], irqflags);
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
>        if (rdev->ddev->irq_enabled && (++rdev->irq.pflip_refcount[crtc] == 1)) {
>                rdev->irq.pflip[crtc] = true;
>                radeon_irq_set(rdev);
>        }
> -       spin_unlock_irqrestore(&rdev->irq.pflip_lock[crtc], irqflags);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
>  }
>
>  void radeon_irq_kms_pflip_irq_put(struct radeon_device *rdev, int crtc)
> @@ -260,12 +268,76 @@ void radeon_irq_kms_pflip_irq_put(struct radeon_device *rdev, int crtc)
>        if (crtc < 0 || crtc >= rdev->num_crtc)
>                return;
>
> -       spin_lock_irqsave(&rdev->irq.pflip_lock[crtc], irqflags);
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
>        BUG_ON(rdev->ddev->irq_enabled && rdev->irq.pflip_refcount[crtc] <= 0);
>        if (rdev->ddev->irq_enabled && (--rdev->irq.pflip_refcount[crtc] == 0)) {
>                rdev->irq.pflip[crtc] = false;
>                radeon_irq_set(rdev);
>        }
> -       spin_unlock_irqrestore(&rdev->irq.pflip_lock[crtc], irqflags);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
>  }
>
> +void radeon_irq_kms_enable_afmt(struct radeon_device *rdev, int block)
> +{
> +       unsigned long irqflags;
> +
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
> +       rdev->irq.afmt[block] = true;
> +       radeon_irq_set(rdev);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> +
> +}
> +
> +void radeon_irq_kms_disable_afmt(struct radeon_device *rdev, int block)
> +{
> +       unsigned long irqflags;
> +
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
> +       rdev->irq.afmt[block] = false;
> +       radeon_irq_set(rdev);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> +}
> +
> +void radeon_irq_kms_enable_hpd(struct radeon_device *rdev, unsigned crtcs)

These aren't crtcs.  I'd replace crtcs with hpd_mask for clarity.

> +{
> +       unsigned long irqflags;
> +       int i;
> +
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
> +       for (i = 0; i < RADEON_MAX_HPD_PINS; ++i)
> +               rdev->irq.hpd[i] |= !!(crtcs & (1 << i));
> +       radeon_irq_set(rdev);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> +}
> +
> +void radeon_irq_kms_disable_hpd(struct radeon_device *rdev, unsigned crtcs)

same here.

> +{
> +       unsigned long irqflags;
> +       int i;
> +
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
> +       for (i = 0; i < RADEON_MAX_HPD_PINS; ++i)
> +               rdev->irq.hpd[i] &= !(crtcs & (1 << i));
> +       radeon_irq_set(rdev);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> +}
> +
> +int radeon_irq_kms_wait_gui_idle(struct radeon_device *rdev)
> +{
> +       unsigned long irqflags;
> +       int r;
> +
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
> +       rdev->irq.gui_idle = true;
> +       radeon_irq_set(rdev);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> +
> +       r = wait_event_timeout(rdev->irq.idle_queue, radeon_gui_idle(rdev),
> +                              msecs_to_jiffies(RADEON_WAIT_IDLE_TIMEOUT));
> +
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
> +       rdev->irq.gui_idle = false;
> +       radeon_irq_set(rdev);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> +       return r;
> +}
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index f1016a5..abbb04d 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -382,29 +382,35 @@ u32 radeon_get_vblank_counter_kms(struct drm_device *dev, int crtc)
>  int radeon_enable_vblank_kms(struct drm_device *dev, int crtc)
>  {
>        struct radeon_device *rdev = dev->dev_private;
> +       unsigned long irqflags;
> +       int r;
>
>        if (crtc < 0 || crtc >= rdev->num_crtc) {
>                DRM_ERROR("Invalid crtc %d\n", crtc);
>                return -EINVAL;
>        }
>
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
>        rdev->irq.crtc_vblank_int[crtc] = true;
> -
> -       return radeon_irq_set(rdev);
> +       r = radeon_irq_set(rdev);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> +       return r;
>  }
>
>  void radeon_disable_vblank_kms(struct drm_device *dev, int crtc)
>  {
>        struct radeon_device *rdev = dev->dev_private;
> +       unsigned long irqflags;
>
>        if (crtc < 0 || crtc >= rdev->num_crtc) {
>                DRM_ERROR("Invalid crtc %d\n", crtc);
>                return;
>        }
>
> +       spin_lock_irqsave(&rdev->irq.lock, irqflags);
>        rdev->irq.crtc_vblank_int[crtc] = false;
> -
>        radeon_irq_set(rdev);
> +       spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
>  }
>
>  int radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,
> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
> index d13b6ae..79642cd 100644
> --- a/drivers/gpu/drm/radeon/radeon_pm.c
> +++ b/drivers/gpu/drm/radeon/radeon_pm.c
> @@ -34,7 +34,6 @@
>  #define RADEON_IDLE_LOOP_MS 100
>  #define RADEON_RECLOCK_DELAY_MS 200
>  #define RADEON_WAIT_VBLANK_TIMEOUT 200
> -#define RADEON_WAIT_IDLE_TIMEOUT 200
>
>  static const char *radeon_pm_state_type_name[5] = {
>        "Default",
> @@ -257,15 +256,8 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev)
>        /* gui idle int has issues on older chips it seems */
>        if (rdev->family >= CHIP_R600) {
>                if (rdev->irq.installed) {
> -                       /* wait for GPU idle */
> -                       rdev->pm.gui_idle = false;
> -                       rdev->irq.gui_idle = true;
> -                       radeon_irq_set(rdev);
> -                       wait_event_interruptible_timeout(
> -                               rdev->irq.idle_queue, rdev->pm.gui_idle,
> -                               msecs_to_jiffies(RADEON_WAIT_IDLE_TIMEOUT));
> -                       rdev->irq.gui_idle = false;
> -                       radeon_irq_set(rdev);
> +                       /* wait for GPU to become idle */
> +                       radeon_irq_kms_wait_gui_idle(rdev);
>                }
>        } else {
>                struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX];
> diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
> index 25f9eef..8387709 100644
> --- a/drivers/gpu/drm/radeon/rs600.c
> +++ b/drivers/gpu/drm/radeon/rs600.c
> @@ -294,6 +294,7 @@ void rs600_hpd_init(struct radeon_device *rdev)
>  {
>        struct drm_device *dev = rdev->ddev;
>        struct drm_connector *connector;
> +       unsigned enable = 0;
>
>        list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>                struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> @@ -301,26 +302,25 @@ void rs600_hpd_init(struct radeon_device *rdev)
>                case RADEON_HPD_1:
>                        WREG32(R_007D00_DC_HOT_PLUG_DETECT1_CONTROL,
>                               S_007D00_DC_HOT_PLUG_DETECT1_EN(1));
> -                       rdev->irq.hpd[0] = true;
>                        break;
>                case RADEON_HPD_2:
>                        WREG32(R_007D10_DC_HOT_PLUG_DETECT2_CONTROL,
>                               S_007D10_DC_HOT_PLUG_DETECT2_EN(1));
> -                       rdev->irq.hpd[1] = true;
>                        break;
>                default:
>                        break;
>                }
> +               enable |= 1 << radeon_connector->hpd.hpd;
>                radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd);
>        }
> -       if (rdev->irq.installed)
> -               rs600_irq_set(rdev);
> +       radeon_irq_kms_enable_hpd(rdev, enable);
>  }
>
>  void rs600_hpd_fini(struct radeon_device *rdev)
>  {
>        struct drm_device *dev = rdev->ddev;
>        struct drm_connector *connector;
> +       unsigned disable = 0;
>
>        list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>                struct radeon_connector *radeon_connector = to_radeon_connector(connector);
> @@ -328,17 +328,17 @@ void rs600_hpd_fini(struct radeon_device *rdev)
>                case RADEON_HPD_1:
>                        WREG32(R_007D00_DC_HOT_PLUG_DETECT1_CONTROL,
>                               S_007D00_DC_HOT_PLUG_DETECT1_EN(0));
> -                       rdev->irq.hpd[0] = false;
>                        break;
>                case RADEON_HPD_2:
>                        WREG32(R_007D10_DC_HOT_PLUG_DETECT2_CONTROL,
>                               S_007D10_DC_HOT_PLUG_DETECT2_EN(0));
> -                       rdev->irq.hpd[1] = false;
>                        break;
>                default:
>                        break;
>                }
> +               disable |= 1 << radeon_connector->hpd.hpd;
>        }
> +       radeon_irq_kms_disable_hpd(rdev, disable);
>  }
>
>  int rs600_asic_reset(struct radeon_device *rdev)
> @@ -686,7 +686,6 @@ int rs600_irq_process(struct radeon_device *rdev)
>                /* GUI idle */
>                if (G_000040_GUI_IDLE(status)) {
>                        rdev->irq.gui_idle_acked = true;
> -                       rdev->pm.gui_idle = true;
>                        wake_up(&rdev->irq.idle_queue);
>                }
>                /* Vertical blank interrupts */
> diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
> index 93da01c..9e691b5 100644
> --- a/drivers/gpu/drm/radeon/si.c
> +++ b/drivers/gpu/drm/radeon/si.c
> @@ -3771,7 +3771,6 @@ restart_ih:
>                        break;
>                case 233: /* GUI IDLE */
>                        DRM_DEBUG("IH: GUI idle\n");
> -                       rdev->pm.gui_idle = true;
>                        wake_up(&rdev->irq.idle_queue);
>                        break;
>                default:
> --
> 1.7.9.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


More information about the dri-devel mailing list