Re: [BXT MIPI PATCH v2 11/13] drm/i915/bxt: Modify BXT BLC according to VBT changes

[Jani Nikula <jani.nikula@linux.intel.com>]

On Sun, 26 Jul 2015, Uma Shankar <uma.shankar@intel.com> wrote:
> From: Sunil Kamath <sunil.kamath@intel.com>
>
> Latest VBT mentions which set of registers will be used for BLC,
> as controller number field. Making use of this field in BXT
> BLC implementation. Also, the registers are used in case control
> pin indicates display DDI. Adding a check for this.
> According to Bspec, BLC_PWM_*_2 uses the display utility pin for output.
> To use backlight 2, enable the utility pin with mode = PWM
>    v2: Jani's review comments
>    addressed
>        - Add a prefix _ to BXT BLC registers definitions.
>        - Add "bxt only" comment for u8 controller
>        - Remove control_pin check for DDI controller
>        - Check for valid controller values
>        - Set pipe bits in UTIL_PIN_CTL
>        - Enable/Disable UTIL_PIN_CTL in enable/disable_backlight()
>        - If BLC 2 is used, read active_low_pwm from UTIL_PIN polarity
>    Satheesh's review comment addressed
>        - If UTIL PIN is already enabled, BIOS would have programmed it. No
>        need to disable and enable again.
>    v3: Jani's review comments
>        - add UTIL_PIN_PIPE_MASK and UTIL_PIN_MODE_MASK
>        - Disable UTIL_PIN if controller 1 is used
>        - Mask out UTIL_PIN_PIPE_MASK and UTIL_PIN_MODE_MASK before enabling
>        UTIL_PIN
>        - check valid controller value in intel_bios.c
>        - add backlight.util_pin_active_low
>        - disable util pin before enabling
>    v4: Change for BXT-PO branch:
>    Stubbed unwanted definition which was existing before
>    because of DC6 patch.
>    UTIL_PIN_MODE_PWM     (0x1b << 24)
>
> v2: Fixed Jani's review comment.
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |   27 ++++++++---
>  drivers/gpu/drm/i915/intel_drv.h   |    2 +
>  drivers/gpu/drm/i915/intel_panel.c |   94 +++++++++++++++++++++++++++++-------
>  3 files changed, 100 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 310afd4..0b1d7ff 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3553,17 +3553,29 @@ enum skl_disp_power_wells {
>  #define UTIL_PIN_CTL		0x48400
>  #define   UTIL_PIN_ENABLE	(1 << 31)
>  
> +#define   UTIL_PIN_PIPE(x)     ((x) << 29)
> +#define   UTIL_PIN_PIPE_MASK   (3 << 29)
> +#define   UTIL_PIN_MODE_PWM    (1 << 24)
> +#define   UTIL_PIN_MODE_MASK   (0xf << 24)
> +#define   UTIL_PIN_POLARITY    (1 << 22)
> +
>  /* BXT backlight register definition. */
> -#define BXT_BLC_PWM_CTL1			0xC8250
> +#define _BXT_BLC_PWM_CTL1			0xC8250
>  #define   BXT_BLC_PWM_ENABLE			(1 << 31)
>  #define   BXT_BLC_PWM_POLARITY			(1 << 29)
> -#define BXT_BLC_PWM_FREQ1			0xC8254
> -#define BXT_BLC_PWM_DUTY1			0xC8258
> +#define _BXT_BLC_PWM_FREQ1			0xC8254
> +#define _BXT_BLC_PWM_DUTY1			0xC8258
>  
> -#define BXT_BLC_PWM_CTL2			0xC8350
> -#define BXT_BLC_PWM_FREQ2			0xC8354
> -#define BXT_BLC_PWM_DUTY2			0xC8358
> +#define _BXT_BLC_PWM_CTL2			0xC8350
> +#define _BXT_BLC_PWM_FREQ2			0xC8354
> +#define _BXT_BLC_PWM_DUTY2			0xC8358
>  
> +#define BXT_BLC_PWM_CTL(controller)    _PIPE(controller, \
> +					_BXT_BLC_PWM_CTL1, _BXT_BLC_PWM_CTL2)
> +#define BXT_BLC_PWM_FREQ(controller)   _PIPE(controller, \
> +					_BXT_BLC_PWM_FREQ1, _BXT_BLC_PWM_FREQ2)
> +#define BXT_BLC_PWM_DUTY(controller)   _PIPE(controller, \
> +					_BXT_BLC_PWM_DUTY1, _BXT_BLC_PWM_DUTY2)
>  
>  #define PCH_GTC_CTL		0xe7000
>  #define   PCH_GTC_ENABLE	(1 << 31)
> @@ -7280,6 +7292,9 @@ enum skl_disp_power_wells {
>  #define  BXT_CDCLK_CD2X_DIV_SEL_4	(3<<22)
>  #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1<<16)
>  
> +/* Max CDCLK freq for BXT in HZ */
> +#define BXT_CDCLK_MAX			624000000
> +
>  /* LCPLL_CTL */
>  #define LCPLL1_CTL		0x46010
>  #define LCPLL2_CTL		0x46014
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3b00d00..8111a21 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -177,7 +177,9 @@ struct intel_panel {
>  		bool enabled;
>  		bool combination_mode;	/* gen 2/4 only */
>  		bool active_low_pwm;
> +		bool util_pin_active_low;	/* bxt+ */
>  		struct backlight_device *device;
> +		u8 controller;		/* bxt+ only */
>  	} backlight;
>  
>  	void (*backlight_power)(struct intel_connector *, bool enable);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 55aad23..2a97e5c 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -539,9 +539,10 @@ static u32 vlv_get_backlight(struct intel_connector *connector)
>  static u32 bxt_get_backlight(struct intel_connector *connector)
>  {
>  	struct drm_device *dev = connector->base.dev;
> +	struct intel_panel *panel = &connector->panel;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	return I915_READ(BXT_BLC_PWM_DUTY1);
> +	return I915_READ(BXT_BLC_PWM_DUTY(panel->backlight.controller));
>  }
>  
>  static u32 intel_panel_get_backlight(struct intel_connector *connector)
> @@ -628,8 +629,9 @@ static void bxt_set_backlight(struct intel_connector *connector, u32 level)
>  {
>  	struct drm_device *dev = connector->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_panel *panel = &connector->panel;
>  
> -	I915_WRITE(BXT_BLC_PWM_DUTY1, level);
> +	I915_WRITE(BXT_BLC_PWM_DUTY(panel->backlight.controller), level);
>  }
>  
>  static void
> @@ -761,12 +763,20 @@ static void bxt_disable_backlight(struct intel_connector *connector)
>  {
>  	struct drm_device *dev = connector->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 tmp;
> +	struct intel_panel *panel = &connector->panel;
> +	u32 tmp, val;
>  
>  	intel_panel_actually_set_backlight(connector, 0);
>  
> -	tmp = I915_READ(BXT_BLC_PWM_CTL1);
> -	I915_WRITE(BXT_BLC_PWM_CTL1, tmp & ~BXT_BLC_PWM_ENABLE);
> +	tmp = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
> +	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
> +			tmp & ~BXT_BLC_PWM_ENABLE);
> +
> +	if (panel->backlight.controller == 1) {
> +		val = I915_READ(UTIL_PIN_CTL);
> +		val &= ~UTIL_PIN_ENABLE;
> +		I915_WRITE(UTIL_PIN_CTL, val);
> +	}
>  }
>  
>  void intel_panel_disable_backlight(struct intel_connector *connector)
> @@ -988,16 +998,39 @@ static void bxt_enable_backlight(struct intel_connector *connector)
>  	struct drm_device *dev = connector->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_panel *panel = &connector->panel;
> -	u32 pwm_ctl;
> +	enum pipe pipe = intel_get_pipe_from_connector(connector);
> +	u32 pwm_ctl, val;
> +
> +	/* To use 2nd set of backlight registers, utility pin has to be
> +	 * enabled with PWM mode.
> +	 * The field should only be changed when the utility pin is disabled
> +	 */
> +	if (panel->backlight.controller == 1) {
> +		val = I915_READ(UTIL_PIN_CTL);
> +		if (val & UTIL_PIN_ENABLE) {
> +			DRM_DEBUG_KMS("util pin already enabled\n");
> +			val &= ~UTIL_PIN_ENABLE;
> +			I915_WRITE(UTIL_PIN_CTL, val);
> +		}
> +		/* mask out UTIL_PIN_PIPE and UTIL_PIN_MODE */
> +		val &= ~(UTIL_PIN_PIPE_MASK | UTIL_PIN_MODE_MASK);
> +		I915_WRITE(UTIL_PIN_CTL, val);
> +		if (panel->backlight.util_pin_active_low)
> +			val |= UTIL_PIN_POLARITY;
> +		I915_WRITE(UTIL_PIN_CTL, val | UTIL_PIN_PIPE(pipe) |
> +				UTIL_PIN_MODE_PWM | UTIL_PIN_ENABLE);
> +	}
>  
> -	pwm_ctl = I915_READ(BXT_BLC_PWM_CTL1);
> +	pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>  	if (pwm_ctl & BXT_BLC_PWM_ENABLE) {
>  		DRM_DEBUG_KMS("backlight already enabled\n");
>  		pwm_ctl &= ~BXT_BLC_PWM_ENABLE;
> -		I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl);
> +		I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
> +				pwm_ctl);
>  	}
>  
> -	I915_WRITE(BXT_BLC_PWM_FREQ1, panel->backlight.max);
> +	I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
> +			panel->backlight.max);
>  
>  	intel_panel_actually_set_backlight(connector, panel->backlight.level);
>  
> @@ -1005,9 +1038,10 @@ static void bxt_enable_backlight(struct intel_connector *connector)
>  	if (panel->backlight.active_low_pwm)
>  		pwm_ctl |= BXT_BLC_PWM_POLARITY;
>  
> -	I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl);
> -	POSTING_READ(BXT_BLC_PWM_CTL1);
> -	I915_WRITE(BXT_BLC_PWM_CTL1, pwm_ctl | BXT_BLC_PWM_ENABLE);
> +	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller), pwm_ctl);
> +	POSTING_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
> +	I915_WRITE(BXT_BLC_PWM_CTL(panel->backlight.controller),
> +			pwm_ctl | BXT_BLC_PWM_ENABLE);
>  }
>  
>  void intel_panel_enable_backlight(struct intel_connector *connector)
> @@ -1370,12 +1404,38 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused)
>  	struct intel_panel *panel = &connector->panel;
>  	u32 pwm_ctl, val;
>  
> -	pwm_ctl = I915_READ(BXT_BLC_PWM_CTL1);
> -	panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY;
> +	/* For BXT hard coding the Backlight controller to 0.
> +	 * TODO : Read the controller value from VBT and generalize
> +	 */
> +	panel->backlight.controller = 0;
>  
> -	panel->backlight.max = I915_READ(BXT_BLC_PWM_FREQ1);
> -	if (!panel->backlight.max)
> -		return -ENODEV;
> +	pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
> +
> +	/* Keeping the check if controller 1 is to be programmed.
> +	 * This will come into affect once the VBT parsing
> +	 * is fixed for controller selection, and controller 1 is used
> +	 * for a prticular display configuration.
> +	 */
> +	if (panel->backlight.controller == 1) {
> +		val = I915_READ(UTIL_PIN_CTL);
> +		panel->backlight.util_pin_active_low =
> +					val & UTIL_PIN_POLARITY;
> +	}
> +
> +	panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY;
> +	panel->backlight.max = I915_READ(
> +			BXT_BLC_PWM_FREQ(panel->backlight.controller));
> +
> +	if (!panel->backlight.max) {
> +		DRM_DEBUG_KMS("PWM freq not programmed by BIOS\n");
> +		DRM_DEBUG_KMS("Programming PWM freq\n");
> +
> +		/* Max Backlight = Max CD Clock / pwm freq) */
> +		panel->backlight.max = (BXT_CDCLK_MAX /
> +				dev_priv->vbt.backlight.pwm_freq_hz);
> +		I915_WRITE(BXT_BLC_PWM_FREQ(panel->backlight.controller),
> +				panel->backlight.max);
> +	}

Handling (!panel->backlight.max) should probably be a separate patch.

>  
>  	val = bxt_get_backlight(connector);
>  	panel->backlight.level = intel_panel_compute_brightness(connector, val);
> -- 
> 1.7.9.5
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx