>-----Original Message-----
>From: Nikula, Jani
>Sent: Friday, September 18, 2015 7:21 PM
>To: Shankar, Uma; intel-gfx@lists.freedesktop.org
>Cc: Kumar, Shobhit; Deak, Imre; Kamath, Sunil; Kannan, Vandana; Shankar, Uma
>Subject: Re: [BXT MIPI PATCH v3 11/14] drm/i915/bxt: Modify BXT BLC
>according to VBT changes
>
>On Tue, 01 Sep 2015, Uma Shankar <uma.shan...@intel.com> wrote:
>> From: Sunil Kamath <sunil.kam...@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.
>>
>> v3: Split the backight PWM frequency programming into separate patch,
>>     in cases BIOS doesn't initializes it.
>>
>> Signed-off-by: Vandana Kannan <vandana.kan...@intel.com>
>> Signed-off-by: Sunil Kamath <sunil.kam...@intel.com>
>> Signed-off-by: Uma Shankar <uma.shan...@intel.com>
>> ---
>>  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)
>>
>>  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.nik...@intel.com>
>

Thanks for the comment and spotting it out. Will update this in subsequent 
patchset.

Regards,
Uma Shankar

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to