Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1446706 > unrolled thread
| Started by | Nicholas Mc Guire <hofrat@osadl.org> |
|---|---|
| First post | 2016-07-19 22:20 +0200 |
| Last post | 2016-07-19 22:20 +0200 |
| Articles | 1 — 1 participant |
Back to article view | Back to linux.kernel
[PATCH RFC] drm/radeon/ci add comment to document intentionally unreachable code Nicholas Mc Guire <hofrat@osadl.org> - 2016-07-19 22:20 +0200
| From | Nicholas Mc Guire <hofrat@osadl.org> |
|---|---|
| Date | 2016-07-19 22:20 +0200 |
| Subject | [PATCH RFC] drm/radeon/ci add comment to document intentionally unreachable code |
| Message-ID | <rWJEJ-8P-13@gated-at.bofh.it> |
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@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
Back to top | Article view | linux.kernel
csiph-web