diff mbox

[11/29] drm/i915: Store DVO SRCDIM register offset under intel_dvo_device

Message ID 1446672017-24497-12-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Nov. 4, 2015, 9:19 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Store the DVO SRCDIM register offset alongside the DVO control register
offset in intel_dvo_device. This gets rid of the switch statement whose
case values are the DVO control register offsets. Such a construct would
cause problems for register type safety.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/dvo.h       |  1 +
 drivers/gpu/drm/i915/intel_dvo.c | 23 +++++++++--------------
 2 files changed, 10 insertions(+), 14 deletions(-)

Comments

Chris Wilson Nov. 5, 2015, 11:10 a.m. UTC | #1 
On Wed, Nov 04, 2015 at 11:19:59PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Store the DVO SRCDIM register offset alongside the DVO control register
> offset in intel_dvo_device. This gets rid of the switch statement whose
> case values are the DVO control register offsets. Such a construct would
> cause problems for register type safety.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> @@ -255,20 +262,8 @@ static void intel_dvo_pre_enable(struct intel_encoder *encoder)
>  	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
>  	int pipe = crtc->pipe;
>  	u32 dvo_val;
> -	u32 dvo_reg = intel_dvo->dev.dvo_reg, dvo_srcdim_reg;
> -
> -	switch (dvo_reg) {
> -	case DVOA:
> -	default:
> -		dvo_srcdim_reg = DVOA_SRCDIM;
> -		break;
> -	case DVOB:
> -		dvo_srcdim_reg = DVOB_SRCDIM;
> -		break;
> -	case DVOC:
> -		dvo_srcdim_reg = DVOC_SRCDIM;
> -		break;
> -	}
> +	u32 dvo_reg = intel_dvo->dev.dvo_reg;
> +	u32 dvo_srcdim_reg = intel_dvo->dev.dvo_srcdim_reg;

It doesn't look like we get much advantage here from the locals now.
Or perhaps struct intel_dvo_device *info = &intel_dvo->dev;

then

val = I915_READ(info->dvo_reg);
....
I915_WRITE(info->dvo_srcdim_reg, foo);
I915_WRITE(info->dvo_reg, val);
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/dvo.h b/drivers/gpu/drm/i915/dvo.h
index 0e2c1b9..20873d6 100644
--- a/drivers/gpu/drm/i915/dvo.h
+++ b/drivers/gpu/drm/i915/dvo.h
@@ -33,6 +33,7 @@  struct intel_dvo_device {
 	int type;
 	/* DVOA/B/C output register */
 	u32 dvo_reg;
+	u32 dvo_srcdim_reg;
 	/* GPIO register used for i2c bus to control this device */
 	u32 gpio;
 	int slave_addr;
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 8492053..3d31d84 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -44,6 +44,7 @@  static const struct intel_dvo_device intel_dvo_devices[] = {
 		.type = INTEL_DVO_CHIP_TMDS,
 		.name = "sil164",
 		.dvo_reg = DVOC,
+		.dvo_srcdim_reg = DVOC_SRCDIM,
 		.slave_addr = SIL164_ADDR,
 		.dev_ops = &sil164_ops,
 	},
@@ -51,6 +52,7 @@  static const struct intel_dvo_device intel_dvo_devices[] = {
 		.type = INTEL_DVO_CHIP_TMDS,
 		.name = "ch7xxx",
 		.dvo_reg = DVOC,
+		.dvo_srcdim_reg = DVOC_SRCDIM,
 		.slave_addr = CH7xxx_ADDR,
 		.dev_ops = &ch7xxx_ops,
 	},
@@ -58,6 +60,7 @@  static const struct intel_dvo_device intel_dvo_devices[] = {
 		.type = INTEL_DVO_CHIP_TMDS,
 		.name = "ch7xxx",
 		.dvo_reg = DVOC,
+		.dvo_srcdim_reg = DVOC_SRCDIM,
 		.slave_addr = 0x75, /* For some ch7010 */
 		.dev_ops = &ch7xxx_ops,
 	},
@@ -65,6 +68,7 @@  static const struct intel_dvo_device intel_dvo_devices[] = {
 		.type = INTEL_DVO_CHIP_LVDS,
 		.name = "ivch",
 		.dvo_reg = DVOA,
+		.dvo_srcdim_reg = DVOA_SRCDIM,
 		.slave_addr = 0x02, /* Might also be 0x44, 0x84, 0xc4 */
 		.dev_ops = &ivch_ops,
 	},
@@ -72,6 +76,7 @@  static const struct intel_dvo_device intel_dvo_devices[] = {
 		.type = INTEL_DVO_CHIP_TMDS,
 		.name = "tfp410",
 		.dvo_reg = DVOC,
+		.dvo_srcdim_reg = DVOC_SRCDIM,
 		.slave_addr = TFP410_ADDR,
 		.dev_ops = &tfp410_ops,
 	},
@@ -79,6 +84,7 @@  static const struct intel_dvo_device intel_dvo_devices[] = {
 		.type = INTEL_DVO_CHIP_LVDS,
 		.name = "ch7017",
 		.dvo_reg = DVOC,
+		.dvo_srcdim_reg = DVOC_SRCDIM,
 		.slave_addr = 0x75,
 		.gpio = GMBUS_PIN_DPB,
 		.dev_ops = &ch7017_ops,
@@ -87,6 +93,7 @@  static const struct intel_dvo_device intel_dvo_devices[] = {
 	        .type = INTEL_DVO_CHIP_TMDS,
 		.name = "ns2501",
 		.dvo_reg = DVOB,
+		.dvo_srcdim_reg = DVOB_SRCDIM,
 		.slave_addr = NS2501_ADDR,
 		.dev_ops = &ns2501_ops,
        }
@@ -255,20 +262,8 @@  static void intel_dvo_pre_enable(struct intel_encoder *encoder)
 	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
 	int pipe = crtc->pipe;
 	u32 dvo_val;
-	u32 dvo_reg = intel_dvo->dev.dvo_reg, dvo_srcdim_reg;
-
-	switch (dvo_reg) {
-	case DVOA:
-	default:
-		dvo_srcdim_reg = DVOA_SRCDIM;
-		break;
-	case DVOB:
-		dvo_srcdim_reg = DVOB_SRCDIM;
-		break;
-	case DVOC:
-		dvo_srcdim_reg = DVOC_SRCDIM;
-		break;
-	}
+	u32 dvo_reg = intel_dvo->dev.dvo_reg;
+	u32 dvo_srcdim_reg = intel_dvo->dev.dvo_srcdim_reg;
 
 	/* Save the data order, since I don't know what it should be set to. */
 	dvo_val = I915_READ(dvo_reg) &