From mboxrd@z Thu Jan  1 00:00:00 1970
From: "j.glisse" <j.glisse@gmail.com>
Subject: Re: [PATCH 07/10] drm/radeon: apply Murphy's law to the kms irq code
Date: Thu, 24 May 2012 11:53:54 -0400
Message-ID: <20120524155353.GA3467@gmail.com>
References: <1337845754-3718-1-git-send-email-deathsimple@vodafone.de>
	<1337845754-3718-7-git-send-email-deathsimple@vodafone.de>
	<CADnq5_PqgeHdSGCnZ+rWAZivcjBiq-Ye9OqX+AEetQWgJB5yzg@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Return-path: <dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org>
Received: from mail-vc0-f177.google.com (mail-vc0-f177.google.com
	[209.85.220.177])
	by gabe.freedesktop.org (Postfix) with ESMTP id 3ED469E763
	for <dri-devel@lists.freedesktop.org>;
	Thu, 24 May 2012 08:54:16 -0700 (PDT)
Received: by vcbf13 with SMTP id f13so1804343vcb.36
	for <dri-devel@lists.freedesktop.org>;
	Thu, 24 May 2012 08:54:15 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <CADnq5_PqgeHdSGCnZ+rWAZivcjBiq-Ye9OqX+AEetQWgJB5yzg@mail.gmail.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
To: Alex Deucher <alexdeucher@gmail.com>
Cc: Christian Koenig <christian.koenig@amd.com>, dri-devel@lists.freedesktop.org
List-Id: dri-devel@lists.freedesktop.org

On Thu, May 24, 2012 at 11:35:15AM -0400, Alex Deucher wrote:
> On Thu, May 24, 2012 at 3:49 AM, Christian K=F6nig
> <deathsimple@vodafone.de> wrote:
> > From: Christian Koenig <christian.koenig@amd.com>
> >
> > 1. It is really dangerous to have more than one
> > =A0 spinlock protecting the same information.
> >
> > 2. radeon_irq_set sometimes wasn't called with lock
> > =A0 protection, so it can happen that more than one
> > =A0 CPU would tamper with the irq regs at the same
> > =A0 time.
> >
> > 3. The pm.gui_idle variable was assuming that the 3D
> > =A0 engine wasn't becoming idle between testing the
> > =A0 register and setting the variable. So just remove
> > =A0 it and test the register directly.
> >
> > Signed-off-by: Christian Koenig <christian.koenig@amd.com>
> > ---
> > =A0drivers/gpu/drm/radeon/evergreen.c =A0 =A0 =A0| =A0 =A01 -
> > =A0drivers/gpu/drm/radeon/r100.c =A0 =A0 =A0 =A0 =A0 | =A0 =A01 -
> > =A0drivers/gpu/drm/radeon/r600.c =A0 =A0 =A0 =A0 =A0 | =A0 =A01 -
> > =A0drivers/gpu/drm/radeon/r600_hdmi.c =A0 =A0 =A0| =A0 =A06 +--
> > =A0drivers/gpu/drm/radeon/radeon.h =A0 =A0 =A0 =A0 | =A0 33 +++++++----=
---
> > =A0drivers/gpu/drm/radeon/radeon_irq_kms.c | =A0 72 +++++++++++++++++++=
++++++------
> > =A0drivers/gpu/drm/radeon/radeon_kms.c =A0 =A0 | =A0 12 ++++--
> > =A0drivers/gpu/drm/radeon/radeon_pm.c =A0 =A0 =A0| =A0 12 +-----
> > =A0drivers/gpu/drm/radeon/rs600.c =A0 =A0 =A0 =A0 =A0| =A0 =A01 -
> > =A0drivers/gpu/drm/radeon/si.c =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A01 -
> > =A010 files changed, 90 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeo=
n/evergreen.c
> > index bfcb39e..9e9b3bb 100644
> > --- a/drivers/gpu/drm/radeon/evergreen.c
> > +++ b/drivers/gpu/drm/radeon/evergreen.c
> > @@ -3254,7 +3254,6 @@ restart_ih:
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 233: /* GUI IDLE */
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0DRM_DEBUG("IH: GUI idle\=
n");
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rdev->pm.gui_idle =3D tru=
e;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0wake_up(&rdev->irq.idle_=
queue);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0default:
> > diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r10=
0.c
> > index 415b7d8..2587426 100644
> > --- a/drivers/gpu/drm/radeon/r100.c
> > +++ b/drivers/gpu/drm/radeon/r100.c
> > @@ -782,7 +782,6 @@ int r100_irq_process(struct radeon_device *rdev)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* gui idle interrupt */
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (status & RADEON_GUI_IDLE_STAT) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rdev->irq.gui_idle_acked=
 =3D true;
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rdev->pm.gui_idle =3D tru=
e;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0wake_up(&rdev->irq.idle_=
queue);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Vertical blank interrupts */
> > diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r60=
0.c
> > index eadbb06..90c6639 100644
> > --- a/drivers/gpu/drm/radeon/r600.c
> > +++ b/drivers/gpu/drm/radeon/r600.c
> > @@ -3542,7 +3542,6 @@ restart_ih:
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 233: /* GUI IDLE */
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0DRM_DEBUG("IH: GUI idle\=
n");
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rdev->pm.gui_idle =3D tru=
e;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0wake_up(&rdev->irq.idle_=
queue);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0default:
> > diff --git a/drivers/gpu/drm/radeon/r600_hdmi.c b/drivers/gpu/drm/radeo=
n/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)
> >
> > =A0 =A0 =A0 =A0if (rdev->irq.installed) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* if irq is available use it */
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 rdev->irq.afmt[dig->afmt->id] =3D true;
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 radeon_irq_set(rdev);
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 radeon_irq_kms_enable_afmt(rdev, dig->afm=
t->id);
> > =A0 =A0 =A0 =A0}
> >
> > =A0 =A0 =A0 =A0dig->afmt->enabled =3D true;
> > @@ -560,8 +559,7 @@ void r600_hdmi_disable(struct drm_encoder *encoder)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0offset, radeon_encoder->encoder_id);
> >
> > =A0 =A0 =A0 =A0/* disable irq */
> > - =A0 =A0 =A0 rdev->irq.afmt[dig->afmt->id] =3D false;
> > - =A0 =A0 =A0 radeon_irq_set(rdev);
> > + =A0 =A0 =A0 radeon_irq_kms_disable_afmt(rdev, dig->afmt->id);
> >
> > =A0 =A0 =A0 =A0/* Older chipsets not handled by AtomBIOS */
> > =A0 =A0 =A0 =A0if (rdev->family >=3D CHIP_R600 && !ASIC_IS_DCE3(rdev)) {
> > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/r=
adeon.h
> > index 8479096..23552b4 100644
> > --- a/drivers/gpu/drm/radeon/radeon.h
> > +++ b/drivers/gpu/drm/radeon/radeon.h
> > @@ -610,21 +610,20 @@ union radeon_irq_stat_regs {
> > =A0#define RADEON_MAX_AFMT_BLOCKS 6
> >
> > =A0struct radeon_irq {
> > - =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0installed;
> > - =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0sw_int[RADEON_NUM_RINGS];
> > - =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0crtc_vblank_int[RADEON_MAX_CR=
TCS];
> > - =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0pflip[RADEON_MAX_CRTCS];
> > - =A0 =A0 =A0 wait_queue_head_t =A0 =A0 =A0 vblank_queue;
> > - =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0hpd[RADEON_MAX_HPD_PINS];
> > - =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0gui_idle;
> > - =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0gui_idle_acked;
> > - =A0 =A0 =A0 wait_queue_head_t =A0 =A0 =A0 idle_queue;
> > - =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0afmt[RADEON_MAX_AFMT_BLOCKS];
> > - =A0 =A0 =A0 spinlock_t sw_lock;
> > - =A0 =A0 =A0 int sw_refcount[RADEON_NUM_RINGS];
> > - =A0 =A0 =A0 union radeon_irq_stat_regs stat_regs;
> > - =A0 =A0 =A0 spinlock_t pflip_lock[RADEON_MAX_CRTCS];
> > - =A0 =A0 =A0 int pflip_refcount[RADEON_MAX_CRTCS];
> > + =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0installed;
> > + =A0 =A0 =A0 spinlock_t =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0loc=
k;
> > + =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0sw_int[RADEON_NUM_RINGS];
> > + =A0 =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 sw_refcount[RADEON_NUM_RINGS];
> > + =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0crtc_vblank_int[RADEON_MAX_CRTCS];
> > + =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0pflip[RADEON_MAX_CRTCS];
> > + =A0 =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 pflip_refcount[RADEON_MAX_CRTCS];
> > + =A0 =A0 =A0 wait_queue_head_t =A0 =A0 =A0 =A0 =A0 =A0 =A0 vblank_queu=
e;
> > + =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0hpd[RADEON_MAX_HPD_PINS];
> > + =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0gui_idle;
> > + =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0gui_idle_acked;
> > + =A0 =A0 =A0 wait_queue_head_t =A0 =A0 =A0 =A0 =A0 =A0 =A0 idle_queue;
> > + =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0afmt[RADEON_MAX_AFMT_BLOCKS];
> > + =A0 =A0 =A0 union radeon_irq_stat_regs =A0 =A0 =A0stat_regs;
> > =A0};
> >
> > =A0int radeon_irq_kms_init(struct radeon_device *rdev);
> > @@ -633,6 +632,9 @@ void radeon_irq_kms_sw_irq_get(struct radeon_device=
 *rdev, int ring);
> > =A0void radeon_irq_kms_sw_irq_put(struct radeon_device *rdev, int ring);
> > =A0void radeon_irq_kms_pflip_irq_get(struct radeon_device *rdev, int cr=
tc);
> > =A0void radeon_irq_kms_pflip_irq_put(struct radeon_device *rdev, int cr=
tc);
> > +void radeon_irq_kms_enable_afmt(struct radeon_device *rdev, int block);
> > +void radeon_irq_kms_disable_afmt(struct radeon_device *rdev, int block=
);
> > +int radeon_irq_kms_wait_gui_idle(struct radeon_device *rdev);
> >
> > =A0/*
> > =A0* CP & rings.
> > @@ -1058,7 +1060,6 @@ struct radeon_pm {
> > =A0 =A0 =A0 =A0int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 active_crtc_=
count;
> > =A0 =A0 =A0 =A0int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 req_vblank;
> > =A0 =A0 =A0 =A0bool =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vblank_sync;
> > - =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0gui_idle;
> > =A0 =A0 =A0 =A0fixed20_12 =A0 =A0 =A0 =A0 =A0 =A0 =A0max_bandwidth;
> > =A0 =A0 =A0 =A0fixed20_12 =A0 =A0 =A0 =A0 =A0 =A0 =A0igp_sideport_mclk;
> > =A0 =A0 =A0 =A0fixed20_12 =A0 =A0 =A0 =A0 =A0 =A0 =A0igp_system_mclk;
> > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/=
radeon/radeon_irq_kms.c
> > index 5df58d1..11fc4f7 100644
> > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
> > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> > @@ -32,6 +32,8 @@
> > =A0#include "radeon.h"
> > =A0#include "atom.h"
> >
> > +#define RADEON_WAIT_IDLE_TIMEOUT 200
> > +
> > =A0irqreturn_t radeon_driver_irq_handler_kms(DRM_IRQ_ARGS)
> > =A0{
> > =A0 =A0 =A0 =A0struct drm_device *dev =3D (struct drm_device *) arg;
> > @@ -62,8 +64,10 @@ static void radeon_hotplug_work_func(struct work_str=
uct *work)
> > =A0void radeon_driver_irq_preinstall_kms(struct drm_device *dev)
> > =A0{
> > =A0 =A0 =A0 =A0struct radeon_device *rdev =3D dev->dev_private;
> > + =A0 =A0 =A0 unsigned long irqflags;
> > =A0 =A0 =A0 =A0unsigned i;
> >
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > =A0 =A0 =A0 =A0/* Disable *all* interrupts */
> > =A0 =A0 =A0 =A0for (i =3D 0; i < RADEON_NUM_RINGS; i++)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rdev->irq.sw_int[i] =3D false;
> > @@ -76,6 +80,7 @@ void radeon_driver_irq_preinstall_kms(struct drm_devi=
ce *dev)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rdev->irq.afmt[i] =3D false;
> > =A0 =A0 =A0 =A0}
> > =A0 =A0 =A0 =A0radeon_irq_set(rdev);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > =A0 =A0 =A0 =A0/* Clear bits */
> > =A0 =A0 =A0 =A0radeon_irq_process(rdev);
> > =A0}
> > @@ -83,23 +88,28 @@ void radeon_driver_irq_preinstall_kms(struct drm_de=
vice *dev)
> > =A0int radeon_driver_irq_postinstall_kms(struct drm_device *dev)
> > =A0{
> > =A0 =A0 =A0 =A0struct radeon_device *rdev =3D dev->dev_private;
> > + =A0 =A0 =A0 unsigned long irqflags;
> > =A0 =A0 =A0 =A0unsigned i;
> >
> > =A0 =A0 =A0 =A0dev->max_vblank_count =3D 0x001fffff;
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > =A0 =A0 =A0 =A0for (i =3D 0; i < RADEON_NUM_RINGS; i++)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rdev->irq.sw_int[i] =3D true;
> > =A0 =A0 =A0 =A0radeon_irq_set(rdev);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > =A0 =A0 =A0 =A0return 0;
> > =A0}
> >
> > =A0void radeon_driver_irq_uninstall_kms(struct drm_device *dev)
> > =A0{
> > =A0 =A0 =A0 =A0struct radeon_device *rdev =3D dev->dev_private;
> > + =A0 =A0 =A0 unsigned long irqflags;
> > =A0 =A0 =A0 =A0unsigned i;
> >
> > =A0 =A0 =A0 =A0if (rdev =3D=3D NULL) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return;
> > =A0 =A0 =A0 =A0}
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > =A0 =A0 =A0 =A0/* Disable *all* interrupts */
> > =A0 =A0 =A0 =A0for (i =3D 0; i < RADEON_NUM_RINGS; i++)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rdev->irq.sw_int[i] =3D false;
> > @@ -112,6 +122,7 @@ void radeon_driver_irq_uninstall_kms(struct drm_dev=
ice *dev)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rdev->irq.afmt[i] =3D false;
> > =A0 =A0 =A0 =A0}
> > =A0 =A0 =A0 =A0radeon_irq_set(rdev);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > =A0}
> >
> > =A0static bool radeon_msi_ok(struct radeon_device *rdev)
> > @@ -168,15 +179,12 @@ static bool radeon_msi_ok(struct radeon_device *r=
dev)
> >
> > =A0int radeon_irq_kms_init(struct radeon_device *rdev)
> > =A0{
> > - =A0 =A0 =A0 int i;
> > =A0 =A0 =A0 =A0int r =3D 0;
> >
> > =A0 =A0 =A0 =A0INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
> > =A0 =A0 =A0 =A0INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi);
> >
> > - =A0 =A0 =A0 spin_lock_init(&rdev->irq.sw_lock);
> > - =A0 =A0 =A0 for (i =3D 0; i < rdev->num_crtc; i++)
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock_init(&rdev->irq.pflip_lock[i]);
> > + =A0 =A0 =A0 spin_lock_init(&rdev->irq.lock);
> > =A0 =A0 =A0 =A0r =3D drm_vblank_init(rdev->ddev, rdev->num_crtc);
> > =A0 =A0 =A0 =A0if (r) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return r;
> > @@ -217,25 +225,25 @@ void radeon_irq_kms_sw_irq_get(struct radeon_devi=
ce *rdev, int ring)
> > =A0{
> > =A0 =A0 =A0 =A0unsigned long irqflags;
> >
> > - =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.sw_lock, irqflags);
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > =A0 =A0 =A0 =A0if (rdev->ddev->irq_enabled && (++rdev->irq.sw_refcount[=
ring] =3D=3D 1)) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rdev->irq.sw_int[ring] =3D true;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0radeon_irq_set(rdev);
> > =A0 =A0 =A0 =A0}
> > - =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.sw_lock, irqflags);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > =A0}
> >
> > =A0void radeon_irq_kms_sw_irq_put(struct radeon_device *rdev, int ring)
> > =A0{
> > =A0 =A0 =A0 =A0unsigned long irqflags;
> >
> > - =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.sw_lock, irqflags);
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > =A0 =A0 =A0 =A0BUG_ON(rdev->ddev->irq_enabled && rdev->irq.sw_refcount[=
ring] <=3D 0);
> > =A0 =A0 =A0 =A0if (rdev->ddev->irq_enabled && (--rdev->irq.sw_refcount[=
ring] =3D=3D 0)) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rdev->irq.sw_int[ring] =3D false;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0radeon_irq_set(rdev);
> > =A0 =A0 =A0 =A0}
> > - =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.sw_lock, irqflags);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > =A0}
> >
> > =A0void radeon_irq_kms_pflip_irq_get(struct radeon_device *rdev, int cr=
tc)
> > @@ -245,12 +253,12 @@ void radeon_irq_kms_pflip_irq_get(struct radeon_d=
evice *rdev, int crtc)
> > =A0 =A0 =A0 =A0if (crtc < 0 || crtc >=3D rdev->num_crtc)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return;
> >
> > - =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.pflip_lock[crtc], irqflags);
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > =A0 =A0 =A0 =A0if (rdev->ddev->irq_enabled && (++rdev->irq.pflip_refcou=
nt[crtc] =3D=3D 1)) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rdev->irq.pflip[crtc] =3D true;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0radeon_irq_set(rdev);
> > =A0 =A0 =A0 =A0}
> > - =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.pflip_lock[crtc], irqfl=
ags);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > =A0}
> >
> > =A0void radeon_irq_kms_pflip_irq_put(struct radeon_device *rdev, int cr=
tc)
> > @@ -260,12 +268,52 @@ void radeon_irq_kms_pflip_irq_put(struct radeon_d=
evice *rdev, int crtc)
> > =A0 =A0 =A0 =A0if (crtc < 0 || crtc >=3D rdev->num_crtc)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return;
> >
> > - =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.pflip_lock[crtc], irqflags);
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > =A0 =A0 =A0 =A0BUG_ON(rdev->ddev->irq_enabled && rdev->irq.pflip_refcou=
nt[crtc] <=3D 0);
> > =A0 =A0 =A0 =A0if (rdev->ddev->irq_enabled && (--rdev->irq.pflip_refcou=
nt[crtc] =3D=3D 0)) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rdev->irq.pflip[crtc] =3D false;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0radeon_irq_set(rdev);
> > =A0 =A0 =A0 =A0}
> > - =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.pflip_lock[crtc], irqfl=
ags);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > +}
> > +
> > +void radeon_irq_kms_enable_afmt(struct radeon_device *rdev, int block)
> > +{
> > + =A0 =A0 =A0 unsigned long irqflags;
> > +
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > + =A0 =A0 =A0 rdev->irq.afmt[block] =3D true;
> > + =A0 =A0 =A0 radeon_irq_set(rdev);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > +
> > +}
> > +
> > +void radeon_irq_kms_disable_afmt(struct radeon_device *rdev, int block)
> > +{
> > + =A0 =A0 =A0 unsigned long irqflags;
> > +
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > + =A0 =A0 =A0 rdev->irq.afmt[block] =3D false;
> > + =A0 =A0 =A0 radeon_irq_set(rdev);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > =A0}
> >
> =

> Should probably also add radeon_irq_kms_[en|dis]able_hpd() function
> and call replaced the calls to *_irq_set() in the *_hpd_init() and
> *_hpd_fini() callbacks for display hotplug.
> =

> Alex

Agree, otherwise:
Reviewed-by: Jerome Glisse <jglisse@redhat.com>

Cheers,
Jerome

> > +int radeon_irq_kms_wait_gui_idle(struct radeon_device *rdev)
> > +{
> > + =A0 =A0 =A0 unsigned long irqflags;
> > + =A0 =A0 =A0 int r;
> > +
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > + =A0 =A0 =A0 rdev->irq.gui_idle =3D true;
> > + =A0 =A0 =A0 radeon_irq_set(rdev);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > +
> > + =A0 =A0 =A0 r =3D wait_event_timeout(rdev->irq.idle_queue, radeon_gui=
_idle(rdev),
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0msecs_to_j=
iffies(RADEON_WAIT_IDLE_TIMEOUT));
> > +
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > + =A0 =A0 =A0 rdev->irq.gui_idle =3D false;
> > + =A0 =A0 =A0 radeon_irq_set(rdev);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > + =A0 =A0 =A0 return r;
> > +}
> > diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/rade=
on/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_devi=
ce *dev, int crtc)
> > =A0int radeon_enable_vblank_kms(struct drm_device *dev, int crtc)
> > =A0{
> > =A0 =A0 =A0 =A0struct radeon_device *rdev =3D dev->dev_private;
> > + =A0 =A0 =A0 unsigned long irqflags;
> > + =A0 =A0 =A0 int r;
> >
> > =A0 =A0 =A0 =A0if (crtc < 0 || crtc >=3D rdev->num_crtc) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0DRM_ERROR("Invalid crtc %d\n", crtc);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EINVAL;
> > =A0 =A0 =A0 =A0}
> >
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > =A0 =A0 =A0 =A0rdev->irq.crtc_vblank_int[crtc] =3D true;
> > -
> > - =A0 =A0 =A0 return radeon_irq_set(rdev);
> > + =A0 =A0 =A0 r =3D radeon_irq_set(rdev);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > + =A0 =A0 =A0 return r;
> > =A0}
> >
> > =A0void radeon_disable_vblank_kms(struct drm_device *dev, int crtc)
> > =A0{
> > =A0 =A0 =A0 =A0struct radeon_device *rdev =3D dev->dev_private;
> > + =A0 =A0 =A0 unsigned long irqflags;
> >
> > =A0 =A0 =A0 =A0if (crtc < 0 || crtc >=3D rdev->num_crtc) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0DRM_ERROR("Invalid crtc %d\n", crtc);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return;
> > =A0 =A0 =A0 =A0}
> >
> > + =A0 =A0 =A0 spin_lock_irqsave(&rdev->irq.lock, irqflags);
> > =A0 =A0 =A0 =A0rdev->irq.crtc_vblank_int[crtc] =3D false;
> > -
> > =A0 =A0 =A0 =A0radeon_irq_set(rdev);
> > + =A0 =A0 =A0 spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
> > =A0}
> >
> > =A0int 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/radeo=
n/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 @@
> > =A0#define RADEON_IDLE_LOOP_MS 100
> > =A0#define RADEON_RECLOCK_DELAY_MS 200
> > =A0#define RADEON_WAIT_VBLANK_TIMEOUT 200
> > -#define RADEON_WAIT_IDLE_TIMEOUT 200
> >
> > =A0static const char *radeon_pm_state_type_name[5] =3D {
> > =A0 =A0 =A0 =A0"Default",
> > @@ -257,15 +256,8 @@ static void radeon_pm_set_clocks(struct radeon_dev=
ice *rdev)
> > =A0 =A0 =A0 =A0/* gui idle int has issues on older chips it seems */
> > =A0 =A0 =A0 =A0if (rdev->family >=3D CHIP_R600) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (rdev->irq.installed) {
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* wait for GPU idle */
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rdev->pm.gui_idle =3D fal=
se;
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rdev->irq.gui_idle =3D tr=
ue;
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 radeon_irq_set(rdev);
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 wait_event_interruptible_=
timeout(
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rdev->irq=
.idle_queue, rdev->pm.gui_idle,
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 msecs_to_=
jiffies(RADEON_WAIT_IDLE_TIMEOUT));
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rdev->irq.gui_idle =3D fa=
lse;
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 radeon_irq_set(rdev);
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* wait for GPU to become=
 idle */
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 radeon_irq_kms_wait_gui_i=
dle(rdev);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> > =A0 =A0 =A0 =A0} else {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct radeon_ring *ring =3D &rdev->ring=
[RADEON_RING_TYPE_GFX_INDEX];
> > diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs=
600.c
> > index 25f9eef..4e9c41a 100644
> > --- a/drivers/gpu/drm/radeon/rs600.c
> > +++ b/drivers/gpu/drm/radeon/rs600.c
> > @@ -686,7 +686,6 @@ int rs600_irq_process(struct radeon_device *rdev)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* GUI idle */
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (G_000040_GUI_IDLE(status)) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rdev->irq.gui_idle_acked=
 =3D true;
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rdev->pm.gui_idle =3D tru=
e;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0wake_up(&rdev->irq.idle_=
queue);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Vertical blank interrupts */
> > diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
> > index f969487..23be691 100644
> > --- a/drivers/gpu/drm/radeon/si.c
> > +++ b/drivers/gpu/drm/radeon/si.c
> > @@ -3773,7 +3773,6 @@ restart_ih:
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0case 233: /* GUI IDLE */
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0DRM_DEBUG("IH: GUI idle\=
n");
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rdev->pm.gui_idle =3D tru=
e;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0wake_up(&rdev->irq.idle_=
queue);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0default:
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel