By enabling -Wunreachable-code-aggressive on Clang the following code paths are unreachable.
Commit ce22c320b8ca ("drm/i915/sdvo: convert to encoder disable/enable") Commit 19f1f627b333 ("drm/i915/gt: Move ivb GT workarounds from init_clock_gating to workarounds") Commit 0a97015d45ee ("drm/i915: Compress GPU objects in error state")
By removing the unreachable code at drivers/gpu/drm/i915/display/intel_sdvo.c the function intel_sdvo_set_encoder_power_state becomes unused.
Commit ea5b213ad4b1 ("drm/i915: Subclass intel_encoder.")
Clang warns unreachable:
drivers/gpu/drm/i915/display/intel_sdvo.c:1768:3: warning: code will never be executed [-Wunreachable-code] intel_sdvo_set_encoder_power_state(intel_sdvo, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_sdvo.c:1767:6: note: silence by adding parentheses to mark code as explicitly dead if (0) ^ /* DISABLES CODE */ ( ) drivers/gpu/drm/i915/display/intel_sdvo.c:1852:3: warning: code will never be executed [-Wunreachable-code] intel_sdvo_set_encoder_power_state(intel_sdvo, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_sdvo.c:1851:6: note: silence by adding parentheses to mark code as explicitly dead if (0) ^ /* DISABLES CODE */ ( )
drivers/gpu/drm/i915/gt/intel_workarounds.c:884:3: warning: code will never be executed [-Wunreachable-code] wa_masked_dis(wal, CACHE_MODE_0_GEN7, HIZ_RAW_STALL_OPT_DISABLE); ^~~~~~~~~~~~~ drivers/gpu/drm/i915/gt/intel_workarounds.c:882:6: note: silence by adding parentheses to mark code as explicitly dead if (0) { /* causes HiZ corruption on ivb:gt1 */ ^ /* DISABLES CODE */ ( )
drivers/gpu/drm/i915/i915_gpu_error.c:319:11: warning: code will never be executed [-Wunreachable-code] if (0 && zstream->total_out > zstream->total_in) ^~~~~~~ drivers/gpu/drm/i915/i915_gpu_error.c:319:6: note: silence by adding parentheses to mark code as explicitly dead if (0 && zstream->total_out > zstream->total_in) ^ /* DISABLES CODE */ ( )
Clang warns unused after removing unreachable:
drivers/gpu/drm/i915/display/intel_sdvo.c:696:13: warning: unused function 'intel_sdvo_set_encoder_power_state' [-Wunused-function] static bool intel_sdvo_set_encoder_power_state(struct intel_sdvo *intel_sdvo, ^
Signed-off-by: Vinicius Tinti viniciustinti@gmail.com --- drivers/gpu/drm/i915/display/intel_sdvo.c | 30 --------------------- drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 ---- drivers/gpu/drm/i915/i915_gpu_error.c | 4 --- 3 files changed, 39 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index 4eaa4aa86ecd..45d03b09f8f0 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -693,30 +693,6 @@ static bool intel_sdvo_get_active_outputs(struct intel_sdvo *intel_sdvo, outputs, sizeof(*outputs)); }
-static bool intel_sdvo_set_encoder_power_state(struct intel_sdvo *intel_sdvo, - int mode) -{ - u8 state = SDVO_ENCODER_STATE_ON; - - switch (mode) { - case DRM_MODE_DPMS_ON: - state = SDVO_ENCODER_STATE_ON; - break; - case DRM_MODE_DPMS_STANDBY: - state = SDVO_ENCODER_STATE_STANDBY; - break; - case DRM_MODE_DPMS_SUSPEND: - state = SDVO_ENCODER_STATE_SUSPEND; - break; - case DRM_MODE_DPMS_OFF: - state = SDVO_ENCODER_STATE_OFF; - break; - } - - return intel_sdvo_set_value(intel_sdvo, - SDVO_CMD_SET_ENCODER_POWER_STATE, &state, sizeof(state)); -} - static bool intel_sdvo_get_input_pixel_clock_range(struct intel_sdvo *intel_sdvo, int *clock_min, int *clock_max) @@ -1764,9 +1740,6 @@ static void intel_disable_sdvo(struct intel_atomic_state *state, intel_sdvo_disable_audio(intel_sdvo);
intel_sdvo_set_active_outputs(intel_sdvo, 0); - if (0) - intel_sdvo_set_encoder_power_state(intel_sdvo, - DRM_MODE_DPMS_OFF);
temp = intel_de_read(dev_priv, intel_sdvo->sdvo_reg);
@@ -1848,9 +1821,6 @@ static void intel_enable_sdvo(struct intel_atomic_state *state, "sync\n", SDVO_NAME(intel_sdvo)); }
- if (0) - intel_sdvo_set_encoder_power_state(intel_sdvo, - DRM_MODE_DPMS_ON); intel_sdvo_set_active_outputs(intel_sdvo, intel_sdvo->attached_output);
if (pipe_config->has_audio) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index adc9a8ea410a..d60ff2c67138 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -879,11 +879,6 @@ ivb_gt_workarounds_init(struct drm_i915_private *i915, struct i915_wa_list *wal) GEN7_FF_VS_SCHED_HW | GEN7_FF_DS_SCHED_HW);
- if (0) { /* causes HiZ corruption on ivb:gt1 */ - /* enable HiZ Raw Stall Optimization */ - wa_masked_dis(wal, CACHE_MODE_0_GEN7, HIZ_RAW_STALL_OPT_DISABLE); - } - /* WaDisable4x2SubspanOptimization:ivb */ wa_masked_en(wal, CACHE_MODE_1, PIXEL_SUBSPAN_COLLECT_OPT_DISABLE);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index d8cac4c5881f..6ec699da1dc2 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -315,10 +315,6 @@ static int compress_page(struct i915_vma_compress *c, cond_resched(); } while (zstream->avail_in);
- /* Fallback to uncompressed if we increase size? */ - if (0 && zstream->total_out > zstream->total_in) - return -E2BIG; - return 0; }
Quoting Vinicius Tinti (2021-01-29 18:15:19)
By enabling -Wunreachable-code-aggressive on Clang the following code paths are unreachable.
That code exists as commentary and, especially for sdvo, library functions that we may need in future.
The ivb-gt1 case => as we now set the gt level for ivb, should we not enable the optimisation for ivb unaffected by the w/a? Just no one has taken the time to see if it causes a regression.
For error state, the question remains whether we should revert to uncompressed data if the compressed stream is larger than the original. -Chris
On Fri, Jan 29, 2021 at 08:55:54PM +0000, Chris Wilson wrote:
Quoting Vinicius Tinti (2021-01-29 18:15:19)
By enabling -Wunreachable-code-aggressive on Clang the following code paths are unreachable.
That code exists as commentary and, especially for sdvo, library functions that we may need in future.
I would argue that this code could be removed since it is in git history. It can be restored when needed.
This will make the code cleaner.
The ivb-gt1 case => as we now set the gt level for ivb, should we not enable the optimisation for ivb unaffected by the w/a? Just no one has taken the time to see if it causes a regression.
I don't know. I just found out that the code is unreachable.
For error state, the question remains whether we should revert to uncompressed data if the compressed stream is larger than the original.
I don't know too.
In this last two cases the code could be commented and the decisions and problems explained in the comment section.
-Chris
Quoting Vinicius Tinti (2021-01-30 12:34:11)
On Fri, Jan 29, 2021 at 08:55:54PM +0000, Chris Wilson wrote:
Quoting Vinicius Tinti (2021-01-29 18:15:19)
By enabling -Wunreachable-code-aggressive on Clang the following code paths are unreachable.
That code exists as commentary and, especially for sdvo, library functions that we may need in future.
I would argue that this code could be removed since it is in git history. It can be restored when needed.
This will make the code cleaner.
It doesn't change the control flow, so no complexity argument. It removes documentation from the code, so I have the opposite opinion.
The ivb-gt1 case => as we now set the gt level for ivb, should we not enable the optimisation for ivb unaffected by the w/a? Just no one has taken the time to see if it causes a regression.
I don't know. I just found out that the code is unreachable.
For error state, the question remains whether we should revert to uncompressed data if the compressed stream is larger than the original.
I don't know too.
In this last two cases the code could be commented and the decisions and problems explained in the comment section.
They already are, that is the point. -Chris
On Sat, Jan 30, 2021 at 9:45 AM Chris Wilson chris@chris-wilson.co.uk wrote:
Quoting Vinicius Tinti (2021-01-30 12:34:11)
On Fri, Jan 29, 2021 at 08:55:54PM +0000, Chris Wilson wrote:
Quoting Vinicius Tinti (2021-01-29 18:15:19)
By enabling -Wunreachable-code-aggressive on Clang the following code paths are unreachable.
That code exists as commentary and, especially for sdvo, library functions that we may need in future.
I would argue that this code could be removed since it is in git history. It can be restored when needed.
This will make the code cleaner.
It doesn't change the control flow, so no complexity argument. It removes documentation from the code, so I have the opposite opinion.
The last change in sdvo related to this function is from commit ce22c320b8ca ("drm/i915/sdvo: convert to encoder disable/enable"), which dates from 2012.
It has not been used or changed for a long time. I think it could be converted to a block comment. This will preserve the documentation purpose. What do you think?
All this will avoid having "if (0)".
The ivb-gt1 case => as we now set the gt level for ivb, should we not enable the optimisation for ivb unaffected by the w/a? Just no one has taken the time to see if it causes a regression.
I don't know. I just found out that the code is unreachable.
For error state, the question remains whether we should revert to uncompressed data if the compressed stream is larger than the original.
I don't know too.
In this last two cases the code could be commented and the decisions and problems explained in the comment section.
They already are, that is the point.
I meant making them a block comment. For example:
/* * Enabling HiZ Raw Stall Optimization, at this point, causes corruption. * * Calling wa_masked_dis with the arguments wal, CACHE_MODE_0_GEN7, * HIZ_RAW_STALL_OPT_DISABLE will cause an HiZ corruption on ivb:g1. */
/* * Should fallback to uncompressed if we increase size * (zstream->total_out > zstream->total_in) by returning -E2BIG? */
-Chris
dri-devel@lists.freedesktop.org
-
Chris Wilson -
Vinicius Tinti