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

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

 



On Wed, 23 Sep 2015, Uma Shankar <uma.shankar@xxxxxxxxx> wrote:
> From: Sunil Kamath <sunil.kamath@xxxxxxxxx>
>
> 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.
>
> v3: Split the backight PWM frequency programming into separate patch,
>     in cases BIOS doesn't initializes it.
>
> v4: Starting afresh and not modifying existing state for backlight, as
>     per Jani's recommendation.
>
> Signed-off-by: Vandana Kannan <vandana.kannan@xxxxxxxxx>
> Signed-off-by: Sunil Kamath <sunil.kamath@xxxxxxxxx>
> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |   28 +++++++++----
>  drivers/gpu/drm/i915/intel_drv.h   |    2 +
>  drivers/gpu/drm/i915/intel_panel.c |   76 ++++++++++++++++++++++++++++--------
>  3 files changed, 81 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 88a16e2..519f764 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3512,17 +3512,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_CTL2			0xC8350
> -#define BXT_BLC_PWM_FREQ2			0xC8354
> -#define BXT_BLC_PWM_DUTY2			0xC8358
> -
> +#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_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)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1059283..d8ca075 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -182,7 +182,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..fafbccc 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,31 @@ 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) {

Please put back this part from the previous patch:

+		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);
+		}

And then continue like below.

With that fixes, this is

Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>


> +		val = 0;
> +		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 +1030,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 +1396,28 @@ 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));
>  
>  	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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




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