Re: [PATCH] drm/radeon: drop register readback in cayman_cp_int_cntl_setup

["Christian König" <christian.koenig@amd.com>]

Am 24.10.2016 um 23:32 schrieb Lucas Stach:
> The read is taking a considerable amount of time (about 50us on this
> machine). The register does not ever hold anything other than the ring
> ID that is updated in this exact function, so there is no need for
> the read modify write cycle.
>
> This chops off a big chunk of the time spent in hardirq disabled
> context, as this function is called multiple times in the interrupt
> handler. With this change applied radeon won't show up in the list
> of the worst IRQ latency offenders anymore, where it was a regular
> before.
>
> Signed-off-by: Lucas Stach <dev@lynxeye.de>

Ups, and to make it even worse SRBM_GFX_CNTL is explicitly documented to 
be a write only register.

That it takes a considerable amount of time is probably because the SRBM 
runs into an error condition when you read it.

So patch is Reviewed-by: Christian König <christian.koenig@amd.com> and 
please also add a CC:stable on that.

Do we have other occasions where we try to use a read modify write cycle?

Regards,
Christian.

> ---
>   drivers/gpu/drm/radeon/ni.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
> index 103fc86..a0d4a05 100644
> --- a/drivers/gpu/drm/radeon/ni.c
> +++ b/drivers/gpu/drm/radeon/ni.c
> @@ -1396,9 +1396,7 @@ static void cayman_pcie_gart_fini(struct radeon_device *rdev)
>   void cayman_cp_int_cntl_setup(struct radeon_device *rdev,
>   			      int ring, u32 cp_int_cntl)
>   {
> -	u32 srbm_gfx_cntl = RREG32(SRBM_GFX_CNTL) & ~3;
> -
> -	WREG32(SRBM_GFX_CNTL, srbm_gfx_cntl | (ring & 3));
> +	WREG32(SRBM_GFX_CNTL, RINGID(ring));
>   	WREG32(CP_INT_CNTL, cp_int_cntl);
>   }
>   


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel