diff mbox series

[3/3] drm/i915/display: stop using BUG()

Message ID 20220504183716.987793-3-jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/3] drm/i915: remove unused GEM_DEBUG_DECL() and GEM_DEBUG_BUG_ON() | expand

Commit Message

Jani Nikula May 4, 2022, 6:37 p.m. UTC
Avoid bringing the entire machine down even if there's a bug that
shouldn't happen, but won't corrupt the system either. Log them loudly
and limp on.

Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      | 11 ++++++-----
 drivers/gpu/drm/i915/display/intel_display.c  | 19 +++++++++++--------
 .../drm/i915/display/intel_display_types.h    | 15 +++++++++------
 3 files changed, 26 insertions(+), 19 deletions(-)

Comments

Tvrtko Ursulin May 5, 2022, 11:07 a.m. UTC | #1
On 04/05/2022 19:37, Jani Nikula wrote:
> Avoid bringing the entire machine down even if there's a bug that
> shouldn't happen, but won't corrupt the system either. Log them loudly
> and limp on.
> 
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_ddi.c      | 11 ++++++-----
>   drivers/gpu/drm/i915/display/intel_display.c  | 19 +++++++++++--------
>   .../drm/i915/display/intel_display_types.h    | 15 +++++++++------
>   3 files changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 9e6fa59eabba..52cf6fb9994a 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -455,6 +455,9 @@ intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
>   		temp |= TRANS_DDI_SELECT_PORT(port);
>   
>   	switch (crtc_state->pipe_bpp) {
> +	default:
> +		MISSING_CASE(crtc_state->pipe_bpp);
> +		fallthrough;
>   	case 18:
>   		temp |= TRANS_DDI_BPC_6;
>   		break;

No idea of how this "limp" would look in practice and if it is safe. 
Same approach taken elsewhere in the patch so I'm afraid someone with 
display know how will have to look at it.

Regards,

Tvrtko

> @@ -467,8 +470,6 @@ intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
>   	case 36:
>   		temp |= TRANS_DDI_BPC_12;
>   		break;
> -	default:
> -		BUG();
>   	}
>   
>   	if (crtc_state->hw.adjusted_mode.flags & DRM_MODE_FLAG_PVSYNC)
> @@ -478,6 +479,9 @@ intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
>   
>   	if (cpu_transcoder == TRANSCODER_EDP) {
>   		switch (pipe) {
> +		default:
> +			MISSING_CASE(pipe);
> +			fallthrough;
>   		case PIPE_A:
>   			/* On Haswell, can only use the always-on power well for
>   			 * eDP when not using the panel fitter, and when not
> @@ -494,9 +498,6 @@ intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
>   		case PIPE_C:
>   			temp |= TRANS_DDI_EDP_INPUT_C_ONOFF;
>   			break;
> -		default:
> -			BUG();
> -			break;
>   		}
>   	}
>   
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 0decf3d24237..1a421dda36d7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -500,6 +500,9 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
>   	i915_reg_t dpll_reg;
>   
>   	switch (dig_port->base.port) {
> +	default:
> +		MISSING_CASE(dig_port->base.port);
> +		fallthrough;
>   	case PORT_B:
>   		port_mask = DPLL_PORTB_READY_MASK;
>   		dpll_reg = DPLL(0);
> @@ -513,8 +516,6 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
>   		port_mask = DPLL_PORTD_READY_MASK;
>   		dpll_reg = DPIO_PHY_STATUS;
>   		break;
> -	default:
> -		BUG();
>   	}
>   
>   	if (intel_de_wait_for_register(dev_priv, dpll_reg,
> @@ -3157,6 +3158,10 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
>   				    PIPECONF_DITHER_TYPE_SP;
>   
>   		switch (crtc_state->pipe_bpp) {
> +		default:
> +			/* Case prevented by intel_choose_pipe_bpp_dither. */
> +			MISSING_CASE(crtc_state->pipe_bpp);
> +			fallthrough;
>   		case 18:
>   			pipeconf |= PIPECONF_BPC_6;
>   			break;
> @@ -3166,9 +3171,6 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
>   		case 30:
>   			pipeconf |= PIPECONF_BPC_10;
>   			break;
> -		default:
> -			/* Case prevented by intel_choose_pipe_bpp_dither. */
> -			BUG();
>   		}
>   	}
>   
> @@ -3464,6 +3466,10 @@ static void ilk_set_pipeconf(const struct intel_crtc_state *crtc_state)
>   	val = 0;
>   
>   	switch (crtc_state->pipe_bpp) {
> +	default:
> +		/* Case prevented by intel_choose_pipe_bpp_dither. */
> +		MISSING_CASE(crtc_state->pipe_bpp);
> +		fallthrough;
>   	case 18:
>   		val |= PIPECONF_BPC_6;
>   		break;
> @@ -3476,9 +3482,6 @@ static void ilk_set_pipeconf(const struct intel_crtc_state *crtc_state)
>   	case 36:
>   		val |= PIPECONF_BPC_12;
>   		break;
> -	default:
> -		/* Case prevented by intel_choose_pipe_bpp_dither. */
> -		BUG();
>   	}
>   
>   	if (crtc_state->dither)
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 408152f9f46a..8b10ef5153f2 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1727,13 +1727,14 @@ static inline enum dpio_channel
>   vlv_dig_port_to_channel(struct intel_digital_port *dig_port)
>   {
>   	switch (dig_port->base.port) {
> +	default:
> +		MISSING_CASE(dig_port->base.port);
> +		fallthrough;
>   	case PORT_B:
>   	case PORT_D:
>   		return DPIO_CH0;
>   	case PORT_C:
>   		return DPIO_CH1;
> -	default:
> -		BUG();
>   	}
>   }
>   
> @@ -1741,13 +1742,14 @@ static inline enum dpio_phy
>   vlv_dig_port_to_phy(struct intel_digital_port *dig_port)
>   {
>   	switch (dig_port->base.port) {
> +	default:
> +		MISSING_CASE(dig_port->base.port);
> +		fallthrough;
>   	case PORT_B:
>   	case PORT_C:
>   		return DPIO_PHY0;
>   	case PORT_D:
>   		return DPIO_PHY1;
> -	default:
> -		BUG();
>   	}
>   }
>   
> @@ -1755,13 +1757,14 @@ static inline enum dpio_channel
>   vlv_pipe_to_channel(enum pipe pipe)
>   {
>   	switch (pipe) {
> +	default:
> +		MISSING_CASE(pipe);
> +		fallthrough;
>   	case PIPE_A:
>   	case PIPE_C:
>   		return DPIO_CH0;
>   	case PIPE_B:
>   		return DPIO_CH1;
> -	default:
> -		BUG();
>   	}
>   }
>
Jani Nikula May 5, 2022, 11:27 a.m. UTC | #2
On Thu, 05 May 2022, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> On 04/05/2022 19:37, Jani Nikula wrote:
>> Avoid bringing the entire machine down even if there's a bug that
>> shouldn't happen, but won't corrupt the system either. Log them loudly
>> and limp on.
>> 
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_ddi.c      | 11 ++++++-----
>>   drivers/gpu/drm/i915/display/intel_display.c  | 19 +++++++++++--------
>>   .../drm/i915/display/intel_display_types.h    | 15 +++++++++------
>>   3 files changed, 26 insertions(+), 19 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index 9e6fa59eabba..52cf6fb9994a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -455,6 +455,9 @@ intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
>>   		temp |= TRANS_DDI_SELECT_PORT(port);
>>   
>>   	switch (crtc_state->pipe_bpp) {
>> +	default:
>> +		MISSING_CASE(crtc_state->pipe_bpp);
>> +		fallthrough;
>>   	case 18:
>>   		temp |= TRANS_DDI_BPC_6;
>>   		break;
>
> No idea of how this "limp" would look in practice and if it is safe. 

Worst case it's a black screen I think, but that's infinitely better
than bringing the entire box down.

> Same approach taken elsewhere in the patch so I'm afraid someone with 
> display know how will have to look at it.

Cc: Ville.

BR,
Jani.


>
> Regards,
>
> Tvrtko
>
>> @@ -467,8 +470,6 @@ intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
>>   	case 36:
>>   		temp |= TRANS_DDI_BPC_12;
>>   		break;
>> -	default:
>> -		BUG();
>>   	}
>>   
>>   	if (crtc_state->hw.adjusted_mode.flags & DRM_MODE_FLAG_PVSYNC)
>> @@ -478,6 +479,9 @@ intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
>>   
>>   	if (cpu_transcoder == TRANSCODER_EDP) {
>>   		switch (pipe) {
>> +		default:
>> +			MISSING_CASE(pipe);
>> +			fallthrough;
>>   		case PIPE_A:
>>   			/* On Haswell, can only use the always-on power well for
>>   			 * eDP when not using the panel fitter, and when not
>> @@ -494,9 +498,6 @@ intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
>>   		case PIPE_C:
>>   			temp |= TRANS_DDI_EDP_INPUT_C_ONOFF;
>>   			break;
>> -		default:
>> -			BUG();
>> -			break;
>>   		}
>>   	}
>>   
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 0decf3d24237..1a421dda36d7 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -500,6 +500,9 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
>>   	i915_reg_t dpll_reg;
>>   
>>   	switch (dig_port->base.port) {
>> +	default:
>> +		MISSING_CASE(dig_port->base.port);
>> +		fallthrough;
>>   	case PORT_B:
>>   		port_mask = DPLL_PORTB_READY_MASK;
>>   		dpll_reg = DPLL(0);
>> @@ -513,8 +516,6 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
>>   		port_mask = DPLL_PORTD_READY_MASK;
>>   		dpll_reg = DPIO_PHY_STATUS;
>>   		break;
>> -	default:
>> -		BUG();
>>   	}
>>   
>>   	if (intel_de_wait_for_register(dev_priv, dpll_reg,
>> @@ -3157,6 +3158,10 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
>>   				    PIPECONF_DITHER_TYPE_SP;
>>   
>>   		switch (crtc_state->pipe_bpp) {
>> +		default:
>> +			/* Case prevented by intel_choose_pipe_bpp_dither. */
>> +			MISSING_CASE(crtc_state->pipe_bpp);
>> +			fallthrough;
>>   		case 18:
>>   			pipeconf |= PIPECONF_BPC_6;
>>   			break;
>> @@ -3166,9 +3171,6 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
>>   		case 30:
>>   			pipeconf |= PIPECONF_BPC_10;
>>   			break;
>> -		default:
>> -			/* Case prevented by intel_choose_pipe_bpp_dither. */
>> -			BUG();
>>   		}
>>   	}
>>   
>> @@ -3464,6 +3466,10 @@ static void ilk_set_pipeconf(const struct intel_crtc_state *crtc_state)
>>   	val = 0;
>>   
>>   	switch (crtc_state->pipe_bpp) {
>> +	default:
>> +		/* Case prevented by intel_choose_pipe_bpp_dither. */
>> +		MISSING_CASE(crtc_state->pipe_bpp);
>> +		fallthrough;
>>   	case 18:
>>   		val |= PIPECONF_BPC_6;
>>   		break;
>> @@ -3476,9 +3482,6 @@ static void ilk_set_pipeconf(const struct intel_crtc_state *crtc_state)
>>   	case 36:
>>   		val |= PIPECONF_BPC_12;
>>   		break;
>> -	default:
>> -		/* Case prevented by intel_choose_pipe_bpp_dither. */
>> -		BUG();
>>   	}
>>   
>>   	if (crtc_state->dither)
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 408152f9f46a..8b10ef5153f2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1727,13 +1727,14 @@ static inline enum dpio_channel
>>   vlv_dig_port_to_channel(struct intel_digital_port *dig_port)
>>   {
>>   	switch (dig_port->base.port) {
>> +	default:
>> +		MISSING_CASE(dig_port->base.port);
>> +		fallthrough;
>>   	case PORT_B:
>>   	case PORT_D:
>>   		return DPIO_CH0;
>>   	case PORT_C:
>>   		return DPIO_CH1;
>> -	default:
>> -		BUG();
>>   	}
>>   }
>>   
>> @@ -1741,13 +1742,14 @@ static inline enum dpio_phy
>>   vlv_dig_port_to_phy(struct intel_digital_port *dig_port)
>>   {
>>   	switch (dig_port->base.port) {
>> +	default:
>> +		MISSING_CASE(dig_port->base.port);
>> +		fallthrough;
>>   	case PORT_B:
>>   	case PORT_C:
>>   		return DPIO_PHY0;
>>   	case PORT_D:
>>   		return DPIO_PHY1;
>> -	default:
>> -		BUG();
>>   	}
>>   }
>>   
>> @@ -1755,13 +1757,14 @@ static inline enum dpio_channel
>>   vlv_pipe_to_channel(enum pipe pipe)
>>   {
>>   	switch (pipe) {
>> +	default:
>> +		MISSING_CASE(pipe);
>> +		fallthrough;
>>   	case PIPE_A:
>>   	case PIPE_C:
>>   		return DPIO_CH0;
>>   	case PIPE_B:
>>   		return DPIO_CH1;
>> -	default:
>> -		BUG();
>>   	}
>>   }
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 9e6fa59eabba..52cf6fb9994a 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -455,6 +455,9 @@  intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
 		temp |= TRANS_DDI_SELECT_PORT(port);
 
 	switch (crtc_state->pipe_bpp) {
+	default:
+		MISSING_CASE(crtc_state->pipe_bpp);
+		fallthrough;
 	case 18:
 		temp |= TRANS_DDI_BPC_6;
 		break;
@@ -467,8 +470,6 @@  intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
 	case 36:
 		temp |= TRANS_DDI_BPC_12;
 		break;
-	default:
-		BUG();
 	}
 
 	if (crtc_state->hw.adjusted_mode.flags & DRM_MODE_FLAG_PVSYNC)
@@ -478,6 +479,9 @@  intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
 
 	if (cpu_transcoder == TRANSCODER_EDP) {
 		switch (pipe) {
+		default:
+			MISSING_CASE(pipe);
+			fallthrough;
 		case PIPE_A:
 			/* On Haswell, can only use the always-on power well for
 			 * eDP when not using the panel fitter, and when not
@@ -494,9 +498,6 @@  intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
 		case PIPE_C:
 			temp |= TRANS_DDI_EDP_INPUT_C_ONOFF;
 			break;
-		default:
-			BUG();
-			break;
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 0decf3d24237..1a421dda36d7 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -500,6 +500,9 @@  void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
 	i915_reg_t dpll_reg;
 
 	switch (dig_port->base.port) {
+	default:
+		MISSING_CASE(dig_port->base.port);
+		fallthrough;
 	case PORT_B:
 		port_mask = DPLL_PORTB_READY_MASK;
 		dpll_reg = DPLL(0);
@@ -513,8 +516,6 @@  void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
 		port_mask = DPLL_PORTD_READY_MASK;
 		dpll_reg = DPIO_PHY_STATUS;
 		break;
-	default:
-		BUG();
 	}
 
 	if (intel_de_wait_for_register(dev_priv, dpll_reg,
@@ -3157,6 +3158,10 @@  static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
 				    PIPECONF_DITHER_TYPE_SP;
 
 		switch (crtc_state->pipe_bpp) {
+		default:
+			/* Case prevented by intel_choose_pipe_bpp_dither. */
+			MISSING_CASE(crtc_state->pipe_bpp);
+			fallthrough;
 		case 18:
 			pipeconf |= PIPECONF_BPC_6;
 			break;
@@ -3166,9 +3171,6 @@  static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
 		case 30:
 			pipeconf |= PIPECONF_BPC_10;
 			break;
-		default:
-			/* Case prevented by intel_choose_pipe_bpp_dither. */
-			BUG();
 		}
 	}
 
@@ -3464,6 +3466,10 @@  static void ilk_set_pipeconf(const struct intel_crtc_state *crtc_state)
 	val = 0;
 
 	switch (crtc_state->pipe_bpp) {
+	default:
+		/* Case prevented by intel_choose_pipe_bpp_dither. */
+		MISSING_CASE(crtc_state->pipe_bpp);
+		fallthrough;
 	case 18:
 		val |= PIPECONF_BPC_6;
 		break;
@@ -3476,9 +3482,6 @@  static void ilk_set_pipeconf(const struct intel_crtc_state *crtc_state)
 	case 36:
 		val |= PIPECONF_BPC_12;
 		break;
-	default:
-		/* Case prevented by intel_choose_pipe_bpp_dither. */
-		BUG();
 	}
 
 	if (crtc_state->dither)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 408152f9f46a..8b10ef5153f2 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1727,13 +1727,14 @@  static inline enum dpio_channel
 vlv_dig_port_to_channel(struct intel_digital_port *dig_port)
 {
 	switch (dig_port->base.port) {
+	default:
+		MISSING_CASE(dig_port->base.port);
+		fallthrough;
 	case PORT_B:
 	case PORT_D:
 		return DPIO_CH0;
 	case PORT_C:
 		return DPIO_CH1;
-	default:
-		BUG();
 	}
 }
 
@@ -1741,13 +1742,14 @@  static inline enum dpio_phy
 vlv_dig_port_to_phy(struct intel_digital_port *dig_port)
 {
 	switch (dig_port->base.port) {
+	default:
+		MISSING_CASE(dig_port->base.port);
+		fallthrough;
 	case PORT_B:
 	case PORT_C:
 		return DPIO_PHY0;
 	case PORT_D:
 		return DPIO_PHY1;
-	default:
-		BUG();
 	}
 }
 
@@ -1755,13 +1757,14 @@  static inline enum dpio_channel
 vlv_pipe_to_channel(enum pipe pipe)
 {
 	switch (pipe) {
+	default:
+		MISSING_CASE(pipe);
+		fallthrough;
 	case PIPE_A:
 	case PIPE_C:
 		return DPIO_CH0;
 	case PIPE_B:
 		return DPIO_CH1;
-	default:
-		BUG();
 	}
 }