On Mon, 25 May 2015, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> On Fri, 22 May 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)
>>
>> 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 | 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
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> 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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
