[PATCH 06/76] drm/i915: simplify dvo dpms interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 26 Jul 2012 20:48:31 +0200
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:

> All dvo drivers only support 2 dpms states, and our dvo driver
> even switches of the dvo port for anything else than DPMS_ON. Hence
> ditch this complexity and simply use bool enable.
> 
> While reading through this code I've noticed that the mode_set
> function of ch7017 is a bit peculiar - it disable the lvds again, even
> though the crtc helper code should have done that ... This might be to
> work around an issue at driver load, we pretty much ignore the hw
> state when taking over.
> 
> v2: Also do the conversion for the new ns2501 driver.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/dvo.h        |    9 ++++-----
>  drivers/gpu/drm/i915/dvo_ch7017.c |    8 ++++----
>  drivers/gpu/drm/i915/dvo_ch7xxx.c |    4 ++--
>  drivers/gpu/drm/i915/dvo_ivch.c   |    8 ++++----
>  drivers/gpu/drm/i915/dvo_ns2501.c |   14 ++++++--------
>  drivers/gpu/drm/i915/dvo_sil164.c |    4 ++--
>  drivers/gpu/drm/i915/dvo_tfp410.c |    4 ++--
>  drivers/gpu/drm/i915/intel_dvo.c  |    4 ++--
>  8 files changed, 26 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/dvo.h b/drivers/gpu/drm/i915/dvo.h
> index 0c8ac4d..0fa839e 100644
> --- a/drivers/gpu/drm/i915/dvo.h
> +++ b/drivers/gpu/drm/i915/dvo.h
> @@ -58,13 +58,12 @@ struct intel_dvo_dev_ops {
>  	void (*create_resources)(struct intel_dvo_device *dvo);
>  
>  	/*
> -	 * Turn on/off output or set intermediate power levels if
> available.
> +	 * Turn on/off output.
>  	 *
> -	 * Unsupported intermediate modes drop to the lower power
> setting.
> -	 * If the  mode is DPMSModeOff, the output must be disabled,
> -	 * as the DPLL may be disabled afterwards.
> +	 * Because none of our dvo drivers support an intermediate
> power levels,
> +	 * we don't expose this in the interfac.
>  	 */
> -	void (*dpms)(struct intel_dvo_device *dvo, int mode);
> +	void (*dpms)(struct intel_dvo_device *dvo, bool enable);
>  
>  	/*
>  	 * Callback for testing a video mode for a given output.
> diff --git a/drivers/gpu/drm/i915/dvo_ch7017.c
> b/drivers/gpu/drm/i915/dvo_ch7017.c index 1ca799a..71e7650 100644
> --- a/drivers/gpu/drm/i915/dvo_ch7017.c
> +++ b/drivers/gpu/drm/i915/dvo_ch7017.c
> @@ -163,7 +163,7 @@ struct ch7017_priv {
>  };
>  
>  static void ch7017_dump_regs(struct intel_dvo_device *dvo);
> -static void ch7017_dpms(struct intel_dvo_device *dvo, int mode);
> +static void ch7017_dpms(struct intel_dvo_device *dvo, bool enable);
>  
>  static bool ch7017_read(struct intel_dvo_device *dvo, u8 addr, u8
> *val) {
> @@ -309,7 +309,7 @@ static void ch7017_mode_set(struct
> intel_dvo_device *dvo, lvds_power_down =
> CH7017_LVDS_POWER_DOWN_DEFAULT_RESERVED | (mode->hdisplay & 0x0700)
> >> 8; 
> -	ch7017_dpms(dvo, DRM_MODE_DPMS_OFF);
> +	ch7017_dpms(dvo, false);
>  	ch7017_write(dvo, CH7017_HORIZONTAL_ACTIVE_PIXEL_INPUT,
>  			horizontal_active_pixel_input);
>  	ch7017_write(dvo, CH7017_HORIZONTAL_ACTIVE_PIXEL_OUTPUT,
> @@ -331,7 +331,7 @@ static void ch7017_mode_set(struct
> intel_dvo_device *dvo, }
>  
>  /* set the CH7017 power state */
> -static void ch7017_dpms(struct intel_dvo_device *dvo, int mode)
> +static void ch7017_dpms(struct intel_dvo_device *dvo, bool enable)
>  {
>  	uint8_t val;
>  
> @@ -345,7 +345,7 @@ static void ch7017_dpms(struct intel_dvo_device
> *dvo, int mode) CH7017_DAC3_POWER_DOWN |
>  			CH7017_TV_POWER_DOWN_EN);
>  
> -	if (mode == DRM_MODE_DPMS_ON) {
> +	if (enable) {
>  		/* Turn on the LVDS */
>  		ch7017_write(dvo, CH7017_LVDS_POWER_DOWN,
>  			     val & ~CH7017_LVDS_POWER_DOWN_EN);
> diff --git a/drivers/gpu/drm/i915/dvo_ch7xxx.c
> b/drivers/gpu/drm/i915/dvo_ch7xxx.c index 4a03660..c1dea5b 100644
> --- a/drivers/gpu/drm/i915/dvo_ch7xxx.c
> +++ b/drivers/gpu/drm/i915/dvo_ch7xxx.c
> @@ -289,9 +289,9 @@ static void ch7xxx_mode_set(struct
> intel_dvo_device *dvo, }
>  
>  /* set the CH7xxx power state */
> -static void ch7xxx_dpms(struct intel_dvo_device *dvo, int mode)
> +static void ch7xxx_dpms(struct intel_dvo_device *dvo, bool enable)
>  {
> -	if (mode == DRM_MODE_DPMS_ON)
> +	if (enable)
>  		ch7xxx_writeb(dvo, CH7xxx_PM, CH7xxx_PM_DVIL |
> CH7xxx_PM_DVIP); else
>  		ch7xxx_writeb(dvo, CH7xxx_PM, CH7xxx_PM_FPD);
> diff --git a/drivers/gpu/drm/i915/dvo_ivch.c
> b/drivers/gpu/drm/i915/dvo_ivch.c index 04f2893..fa8ff6b 100644
> --- a/drivers/gpu/drm/i915/dvo_ivch.c
> +++ b/drivers/gpu/drm/i915/dvo_ivch.c
> @@ -288,7 +288,7 @@ static enum drm_mode_status
> ivch_mode_valid(struct intel_dvo_device *dvo, }
>  
>  /** Sets the power state of the panel connected to the ivch */
> -static void ivch_dpms(struct intel_dvo_device *dvo, int mode)
> +static void ivch_dpms(struct intel_dvo_device *dvo, bool enable)
>  {
>  	int i;
>  	uint16_t vr01, vr30, backlight;
> @@ -297,13 +297,13 @@ static void ivch_dpms(struct intel_dvo_device
> *dvo, int mode) if (!ivch_read(dvo, VR01, &vr01))
>  		return;
>  
> -	if (mode == DRM_MODE_DPMS_ON)
> +	if (enable)
>  		backlight = 1;
>  	else
>  		backlight = 0;
>  	ivch_write(dvo, VR80, backlight);
>  
> -	if (mode == DRM_MODE_DPMS_ON)
> +	if (enable)
>  		vr01 |= VR01_LCD_ENABLE | VR01_DVO_ENABLE;
>  	else
>  		vr01 &= ~(VR01_LCD_ENABLE | VR01_DVO_ENABLE);
> @@ -315,7 +315,7 @@ static void ivch_dpms(struct intel_dvo_device
> *dvo, int mode) if (!ivch_read(dvo, VR30, &vr30))
>  			break;
>  
> -		if (((vr30 & VR30_PANEL_ON) != 0) == (mode ==
> DRM_MODE_DPMS_ON))
> +		if (((vr30 & VR30_PANEL_ON) != 0) == enable)
>  			break;
>  		udelay(1000);
>  	}
> diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c
> b/drivers/gpu/drm/i915/dvo_ns2501.c index 6bd383d..c4d9f2f 100644
> --- a/drivers/gpu/drm/i915/dvo_ns2501.c
> +++ b/drivers/gpu/drm/i915/dvo_ns2501.c
> @@ -493,19 +493,19 @@ static void ns2501_mode_set(struct
> intel_dvo_device *dvo, }
>  
>  /* set the NS2501 power state */
> -static void ns2501_dpms(struct intel_dvo_device *dvo, int mode)
> +static void ns2501_dpms(struct intel_dvo_device *dvo, bool enable)
>  {
>  	bool ok;
>  	bool restore = false;
>  	struct ns2501_priv *ns = (struct ns2501_priv
> *)(dvo->dev_priv); unsigned char ch;
>  
> -	DRM_DEBUG_KMS("%s: Trying set the dpms of the DVO to %d\n",
> -		      __FUNCTION__, mode);
> +	DRM_DEBUG_KMS("%s: Trying set the dpms of the DVO to %i\n",
> +		      __FUNCTION__, enable);
>  
>  	ch = ns->reg_8_shadow;
>  
> -	if (mode == DRM_MODE_DPMS_ON)
> +	if (enable)
>  		ch |= NS2501_8_PD;
>  	else
>  		ch &= ~NS2501_8_PD;
> @@ -519,12 +519,10 @@ static void ns2501_dpms(struct intel_dvo_device
> *dvo, int mode) ok &= ns2501_writeb(dvo, NS2501_REG8, ch);
>  			ok &=
>  			    ns2501_writeb(dvo, 0x34,
> -					  (mode ==
> -					   DRM_MODE_DPMS_ON) ?
> (0x03) : (0x00));
> +					  enable ? 0x03 : 0x00);
>  			ok &=
>  			    ns2501_writeb(dvo, 0x35,
> -					  (mode ==
> -					   DRM_MODE_DPMS_ON) ?
> (0xff) : (0x00));
> +					  enable ? 0xff : 0x00);
>  			if (!ok) {
>  				if (restore)
>  					restore_dvo(dvo);
> diff --git a/drivers/gpu/drm/i915/dvo_sil164.c
> b/drivers/gpu/drm/i915/dvo_sil164.c index a0b13a6..cc24c1c 100644
> --- a/drivers/gpu/drm/i915/dvo_sil164.c
> +++ b/drivers/gpu/drm/i915/dvo_sil164.c
> @@ -208,7 +208,7 @@ static void sil164_mode_set(struct
> intel_dvo_device *dvo, }
>  
>  /* set the SIL164 power state */
> -static void sil164_dpms(struct intel_dvo_device *dvo, int mode)
> +static void sil164_dpms(struct intel_dvo_device *dvo, bool enable)
>  {
>  	int ret;
>  	unsigned char ch;
> @@ -217,7 +217,7 @@ static void sil164_dpms(struct intel_dvo_device
> *dvo, int mode) if (ret == false)
>  		return;
>  
> -	if (mode == DRM_MODE_DPMS_ON)
> +	if (enable)
>  		ch |= SIL164_8_PD;
>  	else
>  		ch &= ~SIL164_8_PD;
> diff --git a/drivers/gpu/drm/i915/dvo_tfp410.c
> b/drivers/gpu/drm/i915/dvo_tfp410.c index aa2cd3e..097b3e8 100644
> --- a/drivers/gpu/drm/i915/dvo_tfp410.c
> +++ b/drivers/gpu/drm/i915/dvo_tfp410.c
> @@ -234,14 +234,14 @@ static void tfp410_mode_set(struct
> intel_dvo_device *dvo, }
>  
>  /* set the tfp410 power state */
> -static void tfp410_dpms(struct intel_dvo_device *dvo, int mode)
> +static void tfp410_dpms(struct intel_dvo_device *dvo, bool enable)
>  {
>  	uint8_t ctl1;
>  
>  	if (!tfp410_readb(dvo, TFP410_CTL_1, &ctl1))
>  		return;
>  
> -	if (mode == DRM_MODE_DPMS_ON)
> +	if (enable)
>  		ctl1 |= TFP410_CTL_1_PD;
>  	else
>  		ctl1 &= ~TFP410_CTL_1_PD;
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c
> b/drivers/gpu/drm/i915/intel_dvo.c index 03dfdff..227551f 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -115,9 +115,9 @@ static void intel_dvo_dpms(struct drm_encoder
> *encoder, int mode) if (mode == DRM_MODE_DPMS_ON) {
>  		I915_WRITE(dvo_reg, temp | DVO_ENABLE);
>  		I915_READ(dvo_reg);
> -		intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, mode);
> +		intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, true);
>  	} else {
> -		intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, mode);
> +		intel_dvo->dev.dev_ops->dpms(&intel_dvo->dev, false);
>  		I915_WRITE(dvo_reg, temp & ~DVO_ENABLE);
>  		I915_READ(dvo_reg);
>  	}

Looks reasonable.  The comment about lvds disable probably doesn't need
to be in the commit message, but it would be good to test that theory
(need to find a tester though; maybe krh has his old system lying
around somewhere).

Jesse


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux