[PATCH RFC] drm/radeon/ci add comment to document intentionally unreachable code

Nicholas Mc Guire hofrat at osadl.org
Tue Jul 19 19:54:06 UTC 2016


 commit d967be9b80a5 ("drm/radeon/ci: disable needless sclk changes")
 introduces an unreachable if(C != C) conditional code section
 flagged by coccinelle script bad_conditional.cocci:

 Add a comment to make it clear that this is intentional.
 
 Fixes: d967be9b80a5 ("drm/radeon/ci: disable needless sclk changes")

Signed-off-by: Nicholas Mc Guire <hofrat at osadl.org>
---
        ./drivers/gpu/drm/radeon/ci_dpm.c:3847 bad/useless conditional
<snip>
3845     } else {
3846          /* XXX check display min clock requirements */
3847          if (CISLAND_MINIMUM_ENGINE_CLOCK !=
CISLAND_MINIMUM_ENGINE_CLOCK)
3848               pi->need_update_smu7_dpm_table |= DPMTABLE_UPDATE_SCLK;
3849     }
<snip>

 which is unreachable given the condition - as this seems to be an
 intentional disabling of that code section as stated in the commit
 message of d967be9b80a5 ("drm/radeon/ci: disable needless sclk changes")
 that introduced that change it should be at least commented

  in drivers/gpu/drm/amd/amdgpu/ci_dpm.c at line 3988
3986    } else {
3987        /* XXX check display min clock requirements */
3988        if (CISLAND_MINIMUM_ENGINE_CLOCK !=
CISLAND_MINIMUM_ENGINE_CLOCK)
3989             pi->need_update_smu7_dpm_table |= DPMTABLE_UPDATE_SCLK;
3990    }

 The below patch adds a possibly suitable comment (taken directly from
 commit d967be9b80a5 ("drm/radeon/ci: disable needless sclk changes")
 But I´m not sure if such constructs really should be in the upstream
 kernel at all....

Patch was compile tested with x86_64_defconfig + CONFIG_DRM_RADEON=m

Patch is against 4.7.0-rc7 (localversion-next is -next-20160719)

 drivers/gpu/drm/radeon/ci_dpm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/ci_dpm.c b/drivers/gpu/drm/radeon/ci_dpm.c
index 35e0fc3..7ba4508 100644
--- a/drivers/gpu/drm/radeon/ci_dpm.c
+++ b/drivers/gpu/drm/radeon/ci_dpm.c
@@ -3843,7 +3843,10 @@ static void ci_find_dpm_states_clocks_in_dpm_table(struct radeon_device *rdev,
 	if (i >= sclk_table->count) {
 		pi->need_update_smu7_dpm_table |= DPMTABLE_OD_UPDATE_SCLK;
 	} else {
-		/* XXX check display min clock requirements */
+		/* XXX The current code always reprogrammed the sclk levels,
+		 * but we don't currently handle disp sclk requirements
+		 * so just skip it.
+		 */
 		if (CISLAND_MINIMUM_ENGINE_CLOCK != CISLAND_MINIMUM_ENGINE_CLOCK)
 			pi->need_update_smu7_dpm_table |= DPMTABLE_UPDATE_SCLK;
 	}
-- 
2.1.4



More information about the dri-devel mailing list