On Mon, 25 May 2015, Jani Nikula <[email protected]> wrote:
> On Fri, 22 May 2015, Uma Shankar <[email protected]> wrote:
>> From: Sunil Kamath <[email protected]>
>>
>> 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)
>>
>> Signed-off-by: Vandana Kannan <[email protected]>
>> Signed-off-by: Sunil Kamath <[email protected]>
>> Signed-off-by: Uma Shankar <[email protected]>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h    |   28 +++++++---
>>  drivers/gpu/drm/i915/intel_drv.h   |    2 +
>>  drivers/gpu/drm/i915/intel_panel.c |  100 
>> ++++++++++++++++++++++++++++++------
>>  3 files changed, 106 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index f96f049..5e3958f 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3509,17 +3509,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 47bc729..ee9eb2b 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 7d83527..23847f2 100644
>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> @@ -32,7 +32,10 @@
>>  
>>  #include <linux/kernel.h>
>>  #include <linux/moduleparam.h>
>> +#include <drm/drm_panel.h>
>>  #include "intel_drv.h"
>> +#include "intel_dsi.h"
>> +#include "i915_drv.h"
>>  
>>  void
>>  intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>> @@ -539,9 +542,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 +632,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 +766,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)
>> @@ -980,16 +993,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;
>>  
>> -    pwm_ctl = I915_READ(BXT_BLC_PWM_CTL1);
>> +    /* 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_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);
>>  
>> @@ -997,9 +1033,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)
>> @@ -1008,6 +1045,9 @@ void intel_panel_enable_backlight(struct 
>> intel_connector *connector)
>>      struct drm_i915_private *dev_priv = dev->dev_private;
>>      struct intel_panel *panel = &connector->panel;
>>      enum pipe pipe = intel_get_pipe_from_connector(connector);
>> +    struct intel_dsi *intel_dsi = NULL;
>> +    struct drm_crtc *crtc = NULL;
>> +    struct intel_encoder *encoder = NULL;
>>  
>>      if (!panel->backlight.present)
>>              return;
>> @@ -1027,7 +1067,18 @@ void intel_panel_enable_backlight(struct 
>> intel_connector *connector)
>>                                               
>> panel->backlight.device->props.max_brightness);
>>      }
>>  
>> -    dev_priv->display.enable_backlight(connector);
>> +    list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>> +            for_each_encoder_on_crtc(dev, crtc, encoder) {
>> +                    if (encoder->type == INTEL_OUTPUT_DSI)
>> +                            intel_dsi = enc_to_intel_dsi(&encoder->base);
>> +            }
>> +    }
>> +
>> +    if (IS_BROXTON(dev) && intel_dsi->panel->funcs->enable)
>> +            intel_dsi->panel->funcs->enable(intel_dsi->panel);
>> +    else
>> +            dev_priv->display.enable_backlight(connector);
>> +
>
> This is backwards. The DSI code should call intel_panel_enable_backlight
> and intel_panel_disable_backlight, not the other way round.

The code should also call setup backlight. Presumably there won't (at
least initially) be a setup with both eDP and DSI.

BR,
Jani.

>
> BR,
> Jani.
>
>
>>      panel->backlight.enabled = true;
>>      if (panel->backlight.device)
>>              panel->backlight.device->props.power = FB_BLANK_UNBLANK;
>> @@ -1362,10 +1413,27 @@ 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;
>> +
>> +    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.max = I915_READ(BXT_BLC_PWM_FREQ1);
>> +    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)
>>              return -ENODEV;
>>  
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> [email protected]
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Jani Nikula, Intel Open Source Technology Center

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

Reply via email to