On Tue, 01 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.
>
> 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 | 84 ++++++++++++++++++++++++++++--------
> 3 files changed, 89 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e43b053..8407b5c 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..9fcf86c 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);
Please start out with 0 val instead of modifying existing state. This is
the style across backlight enabling, apart from setup which gathers the
needed state.
With that fixed,
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> + 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,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
