From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin Herrenschmidt Subject: Re: [PATCH 4/6] drm/radeon: Add a rmb() in IH processing Date: Thu, 14 Jul 2011 07:42:25 +1000 Message-ID: <1310593345.4968.265.camel@pasglop> References: <1310538499.4968.94.camel@pasglop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: xorg-driver-ati-bounces+gcfxda-xorg-driver-ati=m.gmane.org-go0+a7rfsptAfugRpC6u6w@public.gmane.org Errors-To: xorg-driver-ati-bounces+gcfxda-xorg-driver-ati=m.gmane.org-go0+a7rfsptAfugRpC6u6w@public.gmane.org To: Alex Deucher Cc: xorg-driver-ati-go0+a7rfsptAfugRpC6u6w@public.gmane.org, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, Cedric Cano List-Id: dri-devel@lists.freedesktop.org On Wed, 2011-07-13 at 10:48 -0400, Alex Deucher wrote: > On Wed, Jul 13, 2011 at 10:43 AM, Alex Deucher wrote: > > 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. Ok. I can do that. Cheers, Ben. > Alex > > > > >> --- > >> > >> (resent adding dri-devel to the CC list to hit patchwork) > >> > >> drivers/gpu/drm/radeon/r600.c | 3 +++ > >> 1 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) > >> } > >> > >> restart_ih: > >> + /* Order reading of wptr vs. reading of IH ring data */ > >> + wmb(); > >> + > >> /* display interrupts */ > >> r600_irq_ack(rdev); > >> > >> > >> > >> > >> > >