From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Deucher Subject: Re: [PATCH 4/6] drm/radeon: Add a rmb() in IH processing Date: Wed, 13 Jul 2011 10:48:20 -0400 Message-ID: References: <1310538499.4968.94.camel@pasglop> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-iy0-f177.google.com (mail-iy0-f177.google.com [209.85.210.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 767069F316 for ; Wed, 13 Jul 2011 07:48:21 -0700 (PDT) Received: by iyn15 with SMTP id 15so6625396iyn.36 for ; Wed, 13 Jul 2011 07:48:21 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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: Benjamin Herrenschmidt Cc: xorg-driver-ati@lists.x.org, dri-devel@lists.freedesktop.org, Cedric Cano List-Id: dri-devel@lists.freedesktop.org On Wed, Jul 13, 2011 at 10:43 AM, Alex Deucher wrot= e: > On Wed, Jul 13, 2011 at 2:28 AM, Benjamin Herrenschmidt > wrote: >> We should have a read memory barrier between reading the WPTR from >> memory and reading ring entries based on that value (ie, we need to >> ensure both loads are done in order by the CPU). >> >> It could be argued that the MMIO reads in r600_ack_irq() might be >> enough to get that barrier but I prefer keeping an explicit one just >> in case. >> >> Signed-off-by: Benjamin Herrenschmidt > > Reviewed-by: Alex Deucher evergreen.c will need a similar fix. Alex > >> --- >> >> (resent adding dri-devel to the CC list to hit patchwork) >> >> drivers/gpu/drm/radeon/r600.c | =A0 =A03 +++ >> =A01 files changed, 3 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600= .c >> index 3c86b15..7e5c801 100644 >> --- a/drivers/gpu/drm/radeon/r600.c >> +++ b/drivers/gpu/drm/radeon/r600.c >> @@ -3312,6 +3312,9 @@ int r600_irq_process(struct radeon_device *rdev) >> =A0 =A0 =A0 =A0} >> >> =A0restart_ih: >> + =A0 =A0 =A0 /* Order reading of wptr vs. reading of IH ring data */ >> + =A0 =A0 =A0 wmb(); >> + >> =A0 =A0 =A0 =A0/* display interrupts */ >> =A0 =A0 =A0 =A0r600_irq_ack(rdev); >> >> >> >> >> >