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
