Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret'
On Sun, 23 Jun 2019, Dan Carpenter wrote: > On Sat, Jun 22, 2019 at 01:43:19PM +0300, Dan Carpenter wrote: > > On Sat, Jun 22, 2019 at 11:03:14AM +0800, Mao Wenan wrote: > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > > index 0e6dba9..0bf4dd9 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > > @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device > > > *adev, > > > /* init amdgpu_pmu */ > > > int amdgpu_pmu_init(struct amdgpu_device *adev) > > > { > > > - int ret = 0; > > > - > > > switch (adev->asic_type) { > > > case CHIP_VEGA20: > > > /* init df */ > > > - ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > > > + init_pmu_by_type(adev, df_v3_6_attr_groups, > > > "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > > > DF_V3_6_MAX_COUNTERS); > > > > > > You're resending this for other reasons, but don't forget to update the > > indenting on the arguments so they still line up with the '('. > > > > Sorry, I was unclear. If you pull the init_pmu_by_type( back 6 > characters then you also need to pull the "DF" back 6 characters. > > init_pmu_by_type(adev, df_v3_6_attr_groups, "DF", "amdgpu_df", >PERF_TYPE_AMDGPU_DF, DF_V3_6_MAX_COUNTERS); > > You can actually fit it into two lines afterwards. My suggestion was to keep the ret = instead of discarding the indication of failure, so I think that this is not relevant. julia
Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret'
On Sat, Jun 22, 2019 at 01:43:19PM +0300, Dan Carpenter wrote: > On Sat, Jun 22, 2019 at 11:03:14AM +0800, Mao Wenan wrote: > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > index 0e6dba9..0bf4dd9 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device > > *adev, > > /* init amdgpu_pmu */ > > int amdgpu_pmu_init(struct amdgpu_device *adev) > > { > > - int ret = 0; > > - > > switch (adev->asic_type) { > > case CHIP_VEGA20: > > /* init df */ > > - ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > > + init_pmu_by_type(adev, df_v3_6_attr_groups, > >"DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > >DF_V3_6_MAX_COUNTERS); > > > You're resending this for other reasons, but don't forget to update the > indenting on the arguments so they still line up with the '('. > Sorry, I was unclear. If you pull the init_pmu_by_type( back 6 characters then you also need to pull the "DF" back 6 characters. init_pmu_by_type(adev, df_v3_6_attr_groups, "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, DF_V3_6_MAX_COUNTERS); You can actually fit it into two lines afterwards. regards, dan carpenter ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[pull] amdgpu, amdkfd, radeon drm-next-5.3
Hi Dave, Daniel, Last round of updates for 5.3. The big one here is navi10 support. The rest is just a few other odds and ends. My first shot at annotated tags. The following changes since commit 561564bea3248293398dc32ec36da40fb71faed0: Merge tag 'omapdrm-5.3' of git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux into drm-next (2019-06-11 13:29:33 +0200) are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux tags/drm-next-5.3-2019-06-22 for you to fetch changes up to f3f48d7331cf5ad9a6b3a6beff38f3dad1871b49: drm/amdgpu: drop unused df init callback (2019-06-22 09:34:14 -0500) drm-next-5.3-2019-06-22: amdgpu: - SR-IOV L1 policy fixes - Removed no longer needed vram_page_split module parameter - Add module parameter to override default ABM level - Gamma fixes - No need to check return values for debugfs - Improve HMM error handling - Avoid possible OOM situations when lots of thread are submitting with memory contention - Improve hw i2c access abritration - DSC (Display Stream Compression) support in DC - Initial navi10 support * DC support * GFX/Compute support * SDMA support * Power Management support * VCN support amdkfd: - Implement priority controls for gfx9 - Enable VEGAM - Rework mqd allocation and init - Circular locking fix - Fix SDMA queue allocation race condition - No need to check return values for debugfs - Add proc style process information - Initial navi10 support radeon: - No need to check return values for debugfs UAPI changes: - GDDR6 added to vram type query - New Navi10 details added gpu info query - Navi family added to asic family query Aidan Wood (2): drm/amd/display: Properly set DCF clock drm/amd/display: Properly set u clock Alex Deucher (17): drm/amdgpu: return 0 by default in amdgpu_pm_load_smu_firmware drm/amdgpu: wait to fetch the vbios until after common init Revert "drm/amd/display: make clk_mgr call enable_pme_wa" Revert "drm/amd/display: Add Underflow Asserts to dc" Revert "drm/amd/display: move vmid determination logic out of dc" Revert "drm/amd/display: Rework CRTC color management" Revert "drm/amd/display: Use macro for invalid OPP ID" Revert "drm/amd/display: Copy stream updates onto streams" drm/amdgpu: add Navi10 pci ids drm/amd/powerplay/smu11: remove smu_update_table_with_arg drm/amdgpu/powerplay: add license to smu11 header drm/amdgpu/powerplay/vega20: use correct table index drm/amdgpu/gfx10: update to latest golden setting drm/amd/display: add fast_validate parameter to dcn20_validate_bandwidth drm/amd/display: updates for dcn20_update_bandwidth drm/amd/display: update dcn2 dc_plane_cap drm/amdgpu: drop unused df init callback Anthony Koo (2): drm/amd/display: fix issue with eDP not detected on driver load drm/amd/display: do not power on eDP power rail early Aric Cyr (4): drm/amd/display: 3.2.33 drm/amd/display: 3.2.34 drm/amd/display: 3.2.35 drm/amd/display: Intermittent DCN2 pipe hang on mode change Arnd Bergmann (1): drm/amdgpu: fix error handling in df_v3_6_pmc_start Bob Yang (1): drm/amd/display: fixed DCC corruption Charlene Liu (15): drm/amd/display: add some math functions for dcn_calc_math drm/amd/display: add audio related regs drm/amd/display: dcn2 dmcu wait_for_loop update with dispclk. drm/amd/display: fix can not turn on two displays due to DSC_RESOURCE failed. drm/amd/display: Add hubp_init entry to hubp vtable drm/amd/display: add SW_USE_I2C_REG request. drm/amd/display: Create DWB resource for DCN2 drm/amd/display: [backport] dwb dm + efc support drm/amd/display: used optimum VSTARTUP instead of MaxVStartup drm/amd/display: Return UPDATE_TYPE_FULL on writeback update drm/amd/display: add some parameters to validate bandwidth functions drm/amd/display: add dwb stere caps and version drm/amd/display: add p010 and ayuv plane caps drm/amd/display: dcn2 use fixed clocks. drm/amd/display: expose dentist_get_did_from_divider Chengming Gui (1): drm/amd/powerplay: add set_power_profile_mode for raven1_refresh Chris Park (2): drm/amd/display: Clean up scdc_test_data struct drm/amd/display: Move link functions from dc to dc_link Christian König (4): drm/amdgpu: drop some validation failure messages drm/amdgpu: create GDS, GWS and OA in system domain drm/amdgpu: stop removing BOs from the LRU v3 drm/amdgpu: disable concurrent flushes for Navi10 v2 Dan Carpenter (1): drm/amdgpu: Fix bounds checking in amdgpu_ras_is_supported() Derek Lai (1): drm/amd/display: add i2c_hw_Status check to make sure as HW I2c in use Dmytro Laktyushkin (10): drm/amd
Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init
On Sat, 2019-06-22 at 21:05 +0800, Mao Wenan wrote: > There is one warning: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set > but not used [-Wunused-but-set-variable] > int ret = 0; [] > v1->v2: change the subject for this patch; change the indenting when it > calls init_pmu_by_type; use the value 'ret' in > amdgpu_pmu_init(). [] > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c [] > @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > case CHIP_VEGA20: > /* init df */ > ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > -"DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > -DF_V3_6_MAX_COUNTERS); > +"DF", "amdgpu_df", > PERF_TYPE_AMDGPU_DF, > + > DF_V3_6_MAX_COUNTERS); trivia: The indentation change seems superfluous and appears to make the code harder to read. You could also cc Jonathan Kim who wrote all of this.
Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init
On Sat, 22 Jun 2019, maowenan wrote: > > > On 2019/6/22 21:06, Julia Lawall wrote: > > > > > > On Sat, 22 Jun 2019, Mao Wenan wrote: > > > >> There is one warning: > >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: > >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set > >> but not used [-Wunused-but-set-variable] > >> int ret = 0; > >> ^ > >> amdgpu_pmu_init() is called by amdgpu_device_init() in > >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, > >> which will use the return value. So it returns 'ret' for caller. > >> amdgpu_device_init() > >>r = amdgpu_pmu_init(adev); > >> > >> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") > >> > >> Signed-off-by: Mao Wenan > >> --- > >> v1->v2: change the subject for this patch; change the indenting when it > >> calls init_pmu_by_type; use the value 'ret' in > >> amdgpu_pmu_init(). > >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > >> index 0e6dba9..145e720 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > >> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > >>case CHIP_VEGA20: > >>/* init df */ > >>ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > >> - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > >> - DF_V3_6_MAX_COUNTERS); > >> + "DF", "amdgpu_df", > >> PERF_TYPE_AMDGPU_DF, > >> + > >> DF_V3_6_MAX_COUNTERS); > >> > >>/* other pmu types go here*/ > > > > I don't know what is the impact of the other pmu types that are planned > > for the future. Perhaps it would be better to abort the function > > immediately in the case of a failure. > > I guess it would be better to use new function or new switch case clause to > process different PMU types > in future. I don't know. But normally when an error may occur it is checked for immediately, rather than just letting it go until the end of the function. But maybe the developer know what is planned for the future for this function. julia > > > > > julia > > > >>break; > >> @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > >>return 0; > >>} > >> > >> - return 0; > >> + return ret; > >> } > >> > >> > >> -- > >> 2.7.4 > >> > >> > > > >
Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init
On 2019/6/22 21:06, Julia Lawall wrote: > > > On Sat, 22 Jun 2019, Mao Wenan wrote: > >> There is one warning: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set >> but not used [-Wunused-but-set-variable] >> int ret = 0; >> ^ >> amdgpu_pmu_init() is called by amdgpu_device_init() in >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, >> which will use the return value. So it returns 'ret' for caller. >> amdgpu_device_init() >> r = amdgpu_pmu_init(adev); >> >> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") >> >> Signed-off-by: Mao Wenan >> --- >> v1->v2: change the subject for this patch; change the indenting when it >> calls init_pmu_by_type; use the value 'ret' in >> amdgpu_pmu_init(). >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> index 0e6dba9..145e720 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) >> case CHIP_VEGA20: >> /* init df */ >> ret = init_pmu_by_type(adev, df_v3_6_attr_groups, >> - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, >> - DF_V3_6_MAX_COUNTERS); >> + "DF", "amdgpu_df", >> PERF_TYPE_AMDGPU_DF, >> + >> DF_V3_6_MAX_COUNTERS); >> >> /* other pmu types go here*/ > > I don't know what is the impact of the other pmu types that are planned > for the future. Perhaps it would be better to abort the function > immediately in the case of a failure. I guess it would be better to use new function or new switch case clause to process different PMU types in future. > > julia > >> break; >> @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) >> return 0; >> } >> >> -return 0; >> +return ret; >> } >> >> >> -- >> 2.7.4 >> >> >
Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init
On Sat, 22 Jun 2019, Mao Wenan wrote: > There is one warning: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set > but not used [-Wunused-but-set-variable] > int ret = 0; > ^ > amdgpu_pmu_init() is called by amdgpu_device_init() in > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, > which will use the return value. So it returns 'ret' for caller. > amdgpu_device_init() > r = amdgpu_pmu_init(adev); > > Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") > > Signed-off-by: Mao Wenan > --- > v1->v2: change the subject for this patch; change the indenting when it > calls init_pmu_by_type; use the value 'ret' in > amdgpu_pmu_init(). > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > index 0e6dba9..145e720 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > case CHIP_VEGA20: > /* init df */ > ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > -"DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > -DF_V3_6_MAX_COUNTERS); > +"DF", "amdgpu_df", > PERF_TYPE_AMDGPU_DF, > + > DF_V3_6_MAX_COUNTERS); > > /* other pmu types go here*/ I don't know what is the impact of the other pmu types that are planned for the future. Perhaps it would be better to abort the function immediately in the case of a failure. julia > break; > @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > return 0; > } > > - return 0; > + return ret; > } > > > -- > 2.7.4 > >
[PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init
There is one warning: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] int ret = 0; ^ amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, which will use the return value. So it returns 'ret' for caller. amdgpu_device_init() r = amdgpu_pmu_init(adev); Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") Signed-off-by: Mao Wenan --- v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in amdgpu_pmu_init(). drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c index 0e6dba9..145e720 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) case CHIP_VEGA20: /* init df */ ret = init_pmu_by_type(adev, df_v3_6_attr_groups, - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, - DF_V3_6_MAX_COUNTERS); + "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, + DF_V3_6_MAX_COUNTERS); /* other pmu types go here*/ break; @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) return 0; } - return 0; + return ret; } -- 2.7.4
Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret'
On Sat, Jun 22, 2019 at 11:03:14AM +0800, Mao Wenan wrote: > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > index 0e6dba9..0bf4dd9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev, > /* init amdgpu_pmu */ > int amdgpu_pmu_init(struct amdgpu_device *adev) > { > - int ret = 0; > - > switch (adev->asic_type) { > case CHIP_VEGA20: > /* init df */ > - ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > + init_pmu_by_type(adev, df_v3_6_attr_groups, > "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > DF_V3_6_MAX_COUNTERS); You're resending this for other reasons, but don't forget to update the indenting on the arguments so they still line up with the '('. regards, dan carpenter
Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret'
On 2019/6/22 14:02, Julia Lawall wrote: > > > On Sat, 22 Jun 2019, Mao Wenan wrote: > >> Fixes gcc '-Wunused-but-set-variable' warning: >> >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set >> but not used [-Wunused-but-set-variable] >> int ret = 0; >> ^ >> >> It is never used since introduction in 9c7c85f7ea1f ("drm/amdgpu: add pmu >> counters") >> >> Signed-off-by: Mao Wenan >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> index 0e6dba9..0bf4dd9 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev, >> /* init amdgpu_pmu */ >> int amdgpu_pmu_init(struct amdgpu_device *adev) >> { >> -int ret = 0; >> - >> switch (adev->asic_type) { >> case CHIP_VEGA20: >> /* init df */ >> -ret = init_pmu_by_type(adev, df_v3_6_attr_groups, >> +init_pmu_by_type(adev, df_v3_6_attr_groups, >> "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, >> DF_V3_6_MAX_COUNTERS); > > Maybe it would be better to use ret? > > If knowing whether the call has failed is really useless, then maybe the > return type should be void? right. amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, which will use the return value. amdgpu_device_init() r = amdgpu_pmu_init(adev); thanks, I will send v2. > > julia > > >> >> -- >> 2.7.4 >> >> >