Hi Jani, 

May I know why the need to use connector as connector wasn't initialized in 
parent function ' intel_pps_get_registers'? 

While ' dev_priv' already initialized which also already initialized to the VBT 
value. So it make sense to me to use 'dev_priv' structure to read the VBT value 
instead of connector. 

Or did I missed something? Please let me know. 

Best regard 

Mustamin


-----Original Message-----
From: Jani Nikula [mailto:jani.nik...@linux.intel.com] 
Sent: Tuesday, January 16, 2018 6:03 PM
To: Mustaffa, Mustamin B <mustamin.b.musta...@intel.com>; 
intel-gfx@lists.freedesktop.org
Cc: Mustaffa, Mustamin B <mustamin.b.musta...@intel.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/bxt: Enable VBT based BL control for 
DP (v2)

On Wed, 10 Jan 2018, Mustamin B Mustaffa <mustamin.b.musta...@intel.com> wrote:
> Currently, BXT_PP is hardcoded with value '0'.
> It practically disabled eDP backlight on MRB (BXT) platform.
>
> This patch will tell which BXT_PP registers (there are two set of 
> PP_CONTROL in the spec) to be used as defined in VBT (Video Bios 
> Timing table) and this will enabled eDP backlight controller on MRB (BXT) 
> platform.
>
> v2:
>  - Remove unnecessary information in commit message
>  - Assign vbt.backlight.controller to a backlight_controller variable and 
> return
>    the variable value.
>
> Signed-off-by: Mustamin B Mustaffa <mustamin.b.musta...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> b/drivers/gpu/drm/i915/intel_dp.c index 09f2744..e00569f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -640,19 +640,15 @@ static enum pipe vlv_find_free_pps(struct 
> drm_i915_private *dev_priv)
>       struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>       struct drm_device *dev = intel_dig_port->base.base.dev;
>       struct drm_i915_private *dev_priv = to_i915(dev);
> +     int backlight_controller = dev_priv->vbt.backlight.controller;

You need to look at connector->panel->backlight->controller
instead. This gets initialized to the vbt value, but we only use the vbt 
information in bxt_setup_backlight().

BR,
Jani.

>  
>       lockdep_assert_held(&dev_priv->pps_mutex);
>  
>       /* We should never land here with regular DP ports */
>       WARN_ON(!is_edp(intel_dp));
>  
> -     /*
> -      * TODO: BXT has 2 PPS instances. The correct port->PPS instance
> -      * mapping needs to be retrieved from VBT, for now just hard-code to
> -      * use instance #0 always.
> -      */
>       if (!intel_dp->pps_reset)
> -             return 0;
> +             return backlight_controller;
>  
>       intel_dp->pps_reset = false;
>  
> @@ -662,7 +658,7 @@ static enum pipe vlv_find_free_pps(struct 
> drm_i915_private *dev_priv)
>        */
>       intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false);
>  
> -     return 0;
> +     return backlight_controller;
>  }
>  
>  typedef bool (*vlv_pipe_check)(struct drm_i915_private *dev_priv,

--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to