<html> <head> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix"> <blockquote type="cite"> We could merge those to reduce the # of LOC. </blockquote> Yes, please do so if you have time. BTW: Do you know any tool/hack/whatever to find unused structure members? E.g. members which are never accessed or only set but never read? We seem to have quite a number of those in amdgpu, radeon as well as TTM. Would be nice to clean such stuff up. Christian. Am 02.09.2016 um 13:54 schrieb StDenis, Tom: </div> <blockquote cite="mid:CY4PR12MB1768BF04DBC2C421B0FC969FF7E50@CY4PR12MB1768.namprd12.prod.outlook.com" type="cite"> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <style type="text/css" style="display:none;"></style> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;background-color:#FFFFFF;font-family:Calibri,Arial,Helvetica,sans-serif;"> Nope your eyes are fine. Note that the same pattern exists in gfx_v8_0.c as well. I guess different ring struct pointers are passed so they're not talking to the same ring. We could merge those to reduce the # of LOC. Tom <div style="color: rgb(0, 0, 0);"> <div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr">From: Christian König <a class="moz-txt-link-rfc2396E" href="mailto:deathsimple@vodafone.de"><deathsimple@vodafone.de></a> Sent: Thursday, September 1, 2016 13:59 To: Tom St Denis; <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Cc: StDenis, Tom Subject: Re: [PATCH 3/3] drm/amd/amdgpu: Various tidy ups for gfx6 <div> </div> </div> </div> <div class="PlainText">Am 01.09.2016 um 19:44 schrieb Tom St Denis: > Various whitespace and logical simplifications for gfx6. > > Signed-off-by: Tom St Denis <a class="moz-txt-link-rfc2396E" href="mailto:tom.stdenis@amd.com"><tom.stdenis@amd.com></a> > --- > drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 40 +++++------------------------------ > 1 file changed, 5 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c > index 5f508c96496f..63ca77937714 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c > @@ -1211,11 +1211,8 @@ static void gfx_v6_0_gpu_init(struct amdgpu_device *adev) > SC_EARLYZ_TILE_FIFO_SIZE(adev->gfx.config.sc_earlyz_tile_fifo_size))); > > WREG32(VGT_NUM_INSTANCES, 1); > - > WREG32(CP_PERFMON_CNTL, 0); > - > WREG32(SQ_CONFIG, 0); > - > WREG32(PA_SC_FORCE_EOV_MAX_CNTS, (FORCE_EOV_MAX_CLK_CNT(4095) | > FORCE_EOV_MAX_REZ_CNT(255))); > > @@ -1240,7 +1237,6 @@ static void gfx_v6_0_gpu_init(struct amdgpu_device *adev) > WREG32(PA_CL_ENHANCE, CLIP_VTX_REORDER_ENA | NUM_CLIP_SEQ(3)); > > udelay(50); > - > } > > > @@ -1661,21 +1657,14 @@ static int gfx_v6_0_cp_gfx_resume(struct amdgpu_device *adev) > > static u32 gfx_v6_0_ring_get_rptr_gfx(struct amdgpu_ring *ring) > { > - u32 rptr; > - > - rptr = ring->adev->wb.wb[ring->rptr_offs]; > - > - return rptr; > + return ring->adev->wb.wb[ring->rptr_offs]; > } > > static u32 gfx_v6_0_ring_get_wptr_gfx(struct amdgpu_ring *ring) > { > struct amdgpu_device *adev = ring->adev; > - u32 wptr; > > - wptr = RREG32(CP_RB0_WPTR); > - > - return wptr; > + return RREG32(CP_RB0_WPTR); > } > > static void gfx_v6_0_ring_set_wptr_gfx(struct amdgpu_ring *ring) > @@ -1688,9 +1677,7 @@ static void gfx_v6_0_ring_set_wptr_gfx(struct amdgpu_ring *ring) > > static u32 gfx_v6_0_ring_get_rptr_compute(struct amdgpu_ring *ring) > { > - u32 rptr = ring->adev->wb.wb[ring->rptr_offs]; > - > - return rptr; > + return ring->adev->wb.wb[ring->rptr_offs]; > } Am I blind or are the gfx_v6_0_ring_get_rptr_compute() and gfx_v6_0_ring_get_rptr_gfx() functions identical? If that's true might be a good idea to just use one function. Either way patch is Reviewed-by: Christian König <a class="moz-txt-link-rfc2396E" href="mailto:christian.koenig@amd.com"><christian.koenig@amd.com></a> Christian. > > static u32 gfx_v6_0_ring_get_wptr_compute(struct amdgpu_ring *ring) > @@ -1770,14 +1757,12 @@ static int gfx_v6_0_cp_compute_resume(struct amdgpu_device *adev) > ring->wptr = 0; > WREG32(CP_RB1_WPTR, ring->wptr); > > - > rptr_addr = adev->wb.gpu_addr + (ring->rptr_offs * 4); > WREG32(CP_RB1_RPTR_ADDR, lower_32_bits(rptr_addr)); > WREG32(CP_RB1_RPTR_ADDR_HI, upper_32_bits(rptr_addr) & 0xFF); > > mdelay(1); > WREG32(CP_RB1_CNTL, tmp); > - > WREG32(CP_RB1_BASE, ring->gpu_addr >> 8); > > ring = &adev->gfx.compute_ring[1]; > @@ -1797,7 +1782,6 @@ static int gfx_v6_0_cp_compute_resume(struct amdgpu_device *adev) > > mdelay(1); > WREG32(CP_RB2_CNTL, tmp); > - > WREG32(CP_RB2_BASE, ring->gpu_addr >> 8); > > adev->gfx.compute_ring[0].ready = true; > @@ -1825,12 +1809,7 @@ static void gfx_v6_0_cp_enable(struct amdgpu_device *adev, bool enable) > > static int gfx_v6_0_cp_load_microcode(struct amdgpu_device *adev) > { > - int r; > - > - r = gfx_v6_0_cp_gfx_load_microcode(adev); > - > - return r; > - > + return gfx_v6_0_cp_gfx_load_microcode(adev); > } > > static void gfx_v6_0_enable_gui_idle_interrupt(struct amdgpu_device *adev, > @@ -2172,7 +2151,6 @@ static void gfx_v6_0_rlc_stop(struct amdgpu_device *adev) > WREG32(RLC_CNTL, 0); > > gfx_v6_0_enable_gui_idle_interrupt(adev, false); > - > gfx_v6_0_wait_for_rlc_serdes(adev); > } > > @@ -2223,11 +2201,8 @@ static int gfx_v6_0_rlc_resume(struct amdgpu_device *adev) > return -EINVAL; > > gfx_v6_0_rlc_stop(adev); > - > gfx_v6_0_rlc_reset(adev); > - > gfx_v6_0_init_pg(adev); > - > gfx_v6_0_init_cg(adev); > > WREG32(RLC_RL_BASE, 0); > @@ -2254,7 +2229,6 @@ static int gfx_v6_0_rlc_resume(struct amdgpu_device *adev) > WREG32(RLC_UCODE_ADDR, 0); > > gfx_v6_0_enable_lbpw(adev, gfx_v6_0_lbpw_supported(adev)); > - > gfx_v6_0_rlc_start(adev); > > return 0; > @@ -2278,7 +2252,6 @@ static void gfx_v6_0_enable_cgcg(struct amdgpu_device *adev, bool enable) > WREG32(RLC_SERDES_WR_CTRL, 0x00b000ff); > > gfx_v6_0_wait_for_rlc_serdes(adev); > - > gfx_v6_0_update_rlc(adev, tmp); > > WREG32(RLC_SERDES_WR_CTRL, 0x007000ff); > @@ -2931,13 +2904,10 @@ static bool gfx_v6_0_is_idle(void *handle) > static int gfx_v6_0_wait_for_idle(void *handle) > { > unsigned i; > - u32 tmp; > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > for (i = 0; i < adev->usec_timeout; i++) { > - tmp = RREG32(GRBM_STATUS) & GRBM_STATUS__GUI_ACTIVE_MASK; > - > - if (!tmp) > + if (gfx_v6_0_is_idle(handle)) > return 0; > udelay(1); > } </div> </div> </div> </blockquote> </body> </html>