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.nikula@xxxxxxxxxxxxxxx] Sent: Tuesday, January 16, 2018 6:03 PM To: Mustaffa, Mustamin B <mustamin.b.mustaffa@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: Mustaffa, Mustamin B <mustamin.b.mustaffa@xxxxxxxxx> Subject: Re: [PATCH] drm/i915/bxt: Enable VBT based BL control for DP (v2) On Wed, 10 Jan 2018, Mustamin B Mustaffa <mustamin.b.mustaffa@xxxxxxxxx> 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.mustaffa@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
