diff mbox

[2/2] drm/i915: transform INTEL_OUTPUT_* into an enum

Message ID 1414439272-1885-2-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni Oct. 27, 2014, 7:47 p.m. UTC 
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Because I got annoyed that I had to document what values "int
ddi_personality" is supposed to hold.

A good side-effect of this change is that now the compilers can do
some additional checks on our code, which may prevent some bugs in the
future. A bad side-effect of this change is that now the compilers do
some additional checks on our code and complain when a switch
statement doesn't check for all possible values, so we need to add
"default" cases to all those switches. Hopefully, this may help
preventing confusions against DRM_MODE_CONNECTOR_* and
DRM_MODE_ENCODER_*.

I guess that just by looking at the patch, some people will think this
change is not worth its benefits. In this case, I don't really mind
dropping the patch.

Also, there's probably still a few more places where we can
s/int/enum intel_output_type/, but we can change that later, when we
spot the places.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  2 ++
 drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
 drivers/gpu/drm/i915/intel_drv.h     | 31 ++++++++++++++++---------------
 3 files changed, 28 insertions(+), 15 deletions(-)

Comments

Daniel Vetter Oct. 28, 2014, 7:55 a.m. UTC | #1 
On Mon, Oct 27, 2014 at 05:47:52PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Because I got annoyed that I had to document what values "int
> ddi_personality" is supposed to hold.
> 
> A good side-effect of this change is that now the compilers can do
> some additional checks on our code, which may prevent some bugs in the
> future. A bad side-effect of this change is that now the compilers do
> some additional checks on our code and complain when a switch
> statement doesn't check for all possible values, so we need to add
> "default" cases to all those switches. Hopefully, this may help
> preventing confusions against DRM_MODE_CONNECTOR_* and
> DRM_MODE_ENCODER_*.
> 
> I guess that just by looking at the patch, some people will think this
> change is not worth its benefits. In this case, I don't really mind
> dropping the patch.
> 
> Also, there's probably still a few more places where we can
> s/int/enum intel_output_type/, but we can change that later, when we
> spot the places.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I like this, so went right ahead and applied it and resolved the little
rebase conflict.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  2 ++
>  drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     | 31 ++++++++++++++++---------------
>  3 files changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 61ea8da..a79f83c 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2971,6 +2971,8 @@ static int i9xx_pipe_crc_auto_source(struct drm_device *dev, enum pipe pipe,
>  				break;
>  			}
>  			break;
> +		default:
> +			break;
>  		}
>  	}
>  	drm_modeset_unlock_all(dev);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 16750c4..83b8771 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6269,6 +6269,8 @@ static int i9xx_crtc_mode_set(struct intel_crtc *crtc,
>  		case INTEL_OUTPUT_DSI:
>  			is_dsi = true;
>  			break;
> +		default:
> +			break;
>  		}
>  
>  		num_connectors++;
> @@ -6597,6 +6599,8 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
>  			if (enc_to_dig_port(&encoder->base)->port == PORT_A)
>  				has_cpu_edp = true;
>  			break;
> +		default:
> +			break;
>  		}
>  	}
>  
> @@ -6901,6 +6905,8 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
>  		case INTEL_OUTPUT_ANALOG:
>  			has_vga = true;
>  			break;
> +		default:
> +			break;
>  		}
>  	}
>  
> @@ -6934,6 +6940,8 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
>  		case INTEL_OUTPUT_LVDS:
>  			is_lvds = true;
>  			break;
> +		default:
> +			break;
>  		}
>  		num_connectors++;
>  	}
> @@ -7188,6 +7196,8 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  		case INTEL_OUTPUT_HDMI:
>  			is_sdvo = true;
>  			break;
> +		default:
> +			break;
>  		}
>  
>  		num_connectors++;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bf3267c..deb1bc6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -94,18 +94,20 @@
>  
>  /* these are outputs from the chip - integrated only
>     external chips are via DVO or SDVO output */
> -#define INTEL_OUTPUT_UNUSED 0
> -#define INTEL_OUTPUT_ANALOG 1
> -#define INTEL_OUTPUT_DVO 2
> -#define INTEL_OUTPUT_SDVO 3
> -#define INTEL_OUTPUT_LVDS 4
> -#define INTEL_OUTPUT_TVOUT 5
> -#define INTEL_OUTPUT_HDMI 6
> -#define INTEL_OUTPUT_DISPLAYPORT 7
> -#define INTEL_OUTPUT_EDP 8
> -#define INTEL_OUTPUT_DSI 9
> -#define INTEL_OUTPUT_UNKNOWN 10
> -#define INTEL_OUTPUT_DP_MST 11
> +enum intel_output_type {
> +	INTEL_OUTPUT_UNUSED = 0,
> +	INTEL_OUTPUT_ANALOG = 1,
> +	INTEL_OUTPUT_DVO = 2,
> +	INTEL_OUTPUT_SDVO = 3,
> +	INTEL_OUTPUT_LVDS = 4,
> +	INTEL_OUTPUT_TVOUT = 5,
> +	INTEL_OUTPUT_HDMI = 6,
> +	INTEL_OUTPUT_DISPLAYPORT = 7,
> +	INTEL_OUTPUT_EDP = 8,
> +	INTEL_OUTPUT_DSI = 9,
> +	INTEL_OUTPUT_UNKNOWN = 10,
> +	INTEL_OUTPUT_DP_MST = 11,
> +};
>  
>  #define INTEL_DVO_CHIP_NONE 0
>  #define INTEL_DVO_CHIP_LVDS 1
> @@ -136,7 +138,7 @@ struct intel_encoder {
>  	 */
>  	struct intel_crtc *new_crtc;
>  
> -	int type;
> +	enum intel_output_type type;
>  	unsigned int cloneable;
>  	bool connectors_active;
>  	void (*hot_plug)(struct intel_encoder *);
> @@ -387,8 +389,7 @@ struct intel_crtc_config {
>  	bool dp_encoder_is_mst;
>  	int pbn;
>  
> -	/* This should be INTEL_OUTPUT_*. */
> -	int ddi_personality;
> +	enum intel_output_type ddi_personality;
>  };
>  
>  struct intel_pipe_wm {
> -- 
> 2.1.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 61ea8da..a79f83c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2971,6 +2971,8 @@  static int i9xx_pipe_crc_auto_source(struct drm_device *dev, enum pipe pipe,
 				break;
 			}
 			break;
+		default:
+			break;
 		}
 	}
 	drm_modeset_unlock_all(dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 16750c4..83b8771 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6269,6 +6269,8 @@  static int i9xx_crtc_mode_set(struct intel_crtc *crtc,
 		case INTEL_OUTPUT_DSI:
 			is_dsi = true;
 			break;
+		default:
+			break;
 		}
 
 		num_connectors++;
@@ -6597,6 +6599,8 @@  static void ironlake_init_pch_refclk(struct drm_device *dev)
 			if (enc_to_dig_port(&encoder->base)->port == PORT_A)
 				has_cpu_edp = true;
 			break;
+		default:
+			break;
 		}
 	}
 
@@ -6901,6 +6905,8 @@  static void lpt_init_pch_refclk(struct drm_device *dev)
 		case INTEL_OUTPUT_ANALOG:
 			has_vga = true;
 			break;
+		default:
+			break;
 		}
 	}
 
@@ -6934,6 +6940,8 @@  static int ironlake_get_refclk(struct drm_crtc *crtc)
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
 			break;
+		default:
+			break;
 		}
 		num_connectors++;
 	}
@@ -7188,6 +7196,8 @@  static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 		case INTEL_OUTPUT_HDMI:
 			is_sdvo = true;
 			break;
+		default:
+			break;
 		}
 
 		num_connectors++;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bf3267c..deb1bc6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -94,18 +94,20 @@ 
 
 /* these are outputs from the chip - integrated only
    external chips are via DVO or SDVO output */
-#define INTEL_OUTPUT_UNUSED 0
-#define INTEL_OUTPUT_ANALOG 1
-#define INTEL_OUTPUT_DVO 2
-#define INTEL_OUTPUT_SDVO 3
-#define INTEL_OUTPUT_LVDS 4
-#define INTEL_OUTPUT_TVOUT 5
-#define INTEL_OUTPUT_HDMI 6
-#define INTEL_OUTPUT_DISPLAYPORT 7
-#define INTEL_OUTPUT_EDP 8
-#define INTEL_OUTPUT_DSI 9
-#define INTEL_OUTPUT_UNKNOWN 10
-#define INTEL_OUTPUT_DP_MST 11
+enum intel_output_type {
+	INTEL_OUTPUT_UNUSED = 0,
+	INTEL_OUTPUT_ANALOG = 1,
+	INTEL_OUTPUT_DVO = 2,
+	INTEL_OUTPUT_SDVO = 3,
+	INTEL_OUTPUT_LVDS = 4,
+	INTEL_OUTPUT_TVOUT = 5,
+	INTEL_OUTPUT_HDMI = 6,
+	INTEL_OUTPUT_DISPLAYPORT = 7,
+	INTEL_OUTPUT_EDP = 8,
+	INTEL_OUTPUT_DSI = 9,
+	INTEL_OUTPUT_UNKNOWN = 10,
+	INTEL_OUTPUT_DP_MST = 11,
+};
 
 #define INTEL_DVO_CHIP_NONE 0
 #define INTEL_DVO_CHIP_LVDS 1
@@ -136,7 +138,7 @@  struct intel_encoder {
 	 */
 	struct intel_crtc *new_crtc;
 
-	int type;
+	enum intel_output_type type;
 	unsigned int cloneable;
 	bool connectors_active;
 	void (*hot_plug)(struct intel_encoder *);
@@ -387,8 +389,7 @@  struct intel_crtc_config {
 	bool dp_encoder_is_mst;
 	int pbn;
 
-	/* This should be INTEL_OUTPUT_*. */
-	int ddi_personality;
+	enum intel_output_type ddi_personality;
 };
 
 struct intel_pipe_wm {