RE: [PATCH 1/2] drm/radeon: fix surface sync in fence on cayman

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

 



> -----Original Message-----
> From: Tom Stellard [mailto:tom@xxxxxxxxxxxx]
> Sent: Thursday, January 16, 2014 6:25 PM
> To: Alex Deucher
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander;
> stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/2] drm/radeon: fix surface sync in fence on cayman
> 
> On Thu, Jan 16, 2014 at 06:14:58PM -0500, Alex Deucher wrote:
> > We need to set the engine bit to select the ME and
> > also set the full cache bit.  Should help stability
> > on TN and cayman.
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
> > ---
> >  drivers/gpu/drm/radeon/ni.c  | 7 +++----
> >  drivers/gpu/drm/radeon/nid.h | 1 +
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
> > index 9f11a55..04e9516 100644
> > --- a/drivers/gpu/drm/radeon/ni.c
> > +++ b/drivers/gpu/drm/radeon/ni.c
> > @@ -1319,13 +1319,12 @@ void cayman_fence_ring_emit(struct
> radeon_device *rdev,
> >  {
> >  	struct radeon_ring *ring = &rdev->ring[fence->ring];
> >  	u64 addr = rdev->fence_drv[fence->ring].gpu_addr;
> > +	u32 cp_coher_cntl = PACKET3_FULL_CACHE_ENA |
> PACKET3_TC_ACTION_ENA |
> > +		PACKET3_SH_ACTION_ENA;
> >
> >  	/* flush read cache over gart for this vmid */
> > -	radeon_ring_write(ring, PACKET3(PACKET3_SET_CONFIG_REG, 1));
> > -	radeon_ring_write(ring, (CP_COHER_CNTL2 -
> PACKET3_SET_CONFIG_REG_START) >> 2);
> > -	radeon_ring_write(ring, 0);
> >  	radeon_ring_write(ring, PACKET3(PACKET3_SURFACE_SYNC, 3));
> > -	radeon_ring_write(ring, PACKET3_TC_ACTION_ENA |
> PACKET3_SH_ACTION_ENA);
> > +	radeon_ring_write(ring, PACKET3_ENGINE_ME | cp_coher_cntl);
> >  	radeon_ring_write(ring, 0xFFFFFFFF);
> >  	radeon_ring_write(ring, 0);
> >  	radeon_ring_write(ring, 10); /* poll interval */
> 
> Doesn't this also need to be changed in cayman_ring_ib_execute() ?

Oh, yeah.  That also emits a surface sync packet.  I'm not sure why we need one there, but I'll fix that up too.


> 
> > diff --git a/drivers/gpu/drm/radeon/nid.h
> b/drivers/gpu/drm/radeon/nid.h
> > index 22421bc..d996033 100644
> > --- a/drivers/gpu/drm/radeon/nid.h
> > +++ b/drivers/gpu/drm/radeon/nid.h
> > @@ -1154,6 +1154,7 @@
> >  #              define PACKET3_DB_ACTION_ENA        (1 << 26)
> >  #              define PACKET3_SH_ACTION_ENA        (1 << 27)
> >  #              define PACKET3_SX_ACTION_ENA        (1 << 28)
> > +#              define PACKET3_ENGINE_ME            (1 << 31)
> 
> Technically (1 << 31) is undefined.  It should be (1U << 31).

Hmmm... a lot of drivers do this pretty regularly.  I guess gcc does the right thing?

Alex

> 
> -Tom
> 
> >  #define	PACKET3_ME_INITIALIZE				0x44
> >  #define		PACKET3_ME_INITIALIZE_DEVICE_ID(x) ((x) << 16)
> >  #define	PACKET3_COND_WRITE				0x45
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel


--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]