On Fri, 07 Nov 2014, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> On VLV/CHV both pipes A and B have their own backlight control
> registers. In order to correctly read out the current hardware state at
> init we need to know which pipe is driving the eDP port. Pass that
> information down from the eDP init code into the backlight code.
>
> To determine the correct pipe we first look at which pipe is currently
> configured in the port control register, if that look invalid we look
> at which pipe's PPS is currently controlling the port, and if that
> too looks invalid we just assume pipe A.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_dp.c | 22 +++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_lvds.c | 2 +-
> drivers/gpu/drm/i915/intel_panel.c | 31 +++++++++++++++++--------------
> 5 files changed, 41 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0f00e58..65e4e29 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -510,7 +510,7 @@ struct drm_i915_display_funcs {
> /* display clock increase/decrease */
> /* pll clock increase/decrease */
>
> - int (*setup_backlight)(struct intel_connector *connector);
> + int (*setup_backlight)(struct intel_connector *connector, enum pipe pipe);
> uint32_t (*get_backlight)(struct intel_connector *connector);
> void (*set_backlight)(struct intel_connector *connector,
> uint32_t level);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ceb528f..45b53ff 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5232,6 +5232,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> bool has_dpcd;
> struct drm_display_mode *scan;
> struct edid *edid;
> + enum pipe pipe = INVALID_PIPE;
>
> intel_dp->drrs_state.type = DRRS_NOT_SUPPORTED;
>
> @@ -5300,11 +5301,30 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> if (IS_VALLEYVIEW(dev)) {
> intel_dp->edp_notifier.notifier_call = edp_notify_handler;
> register_reboot_notifier(&intel_dp->edp_notifier);
> +
> + /*
> + * Figure out the current pipe for the initial backlight setup.
> + * If the current pipe isn't valid, try the PPS pipe, and if that
> + * fails just assume pipe A.
> + */
> + if (IS_CHERRYVIEW(dev))
> + pipe = DP_PORT_TO_PIPE_CHV(intel_dp->DP);
> + else
> + pipe = PORT_TO_PIPE(intel_dp->DP);
> +
> + if (pipe != PIPE_A && pipe != PIPE_B)
> + pipe = intel_dp->pps_pipe;
> +
> + if (pipe != PIPE_A && pipe != PIPE_B)
> + pipe = PIPE_A;
> +
> + DRM_DEBUG_KMS("using pipe %c for initial backlight setup\n",
> + pipe_name(pipe));
> }
>
> intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> intel_connector->panel.backlight_power = intel_edp_backlight_power;
> - intel_panel_setup_backlight(connector);
> + intel_panel_setup_backlight(connector, pipe);
>
> return true;
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cb0e9db..0ff011e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1103,7 +1103,7 @@ void intel_gmch_panel_fitting(struct intel_crtc *crtc,
> int fitting_mode);
> void intel_panel_set_backlight_acpi(struct intel_connector *connector,
> u32 level, u32 max);
> -int intel_panel_setup_backlight(struct drm_connector *connector);
> +int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe);
> void intel_panel_enable_backlight(struct intel_connector *connector);
> void intel_panel_disable_backlight(struct intel_connector *connector);
> void intel_panel_destroy_backlight(struct drm_connector *connector);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 2b50c98..c03d457 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -1116,7 +1116,7 @@ out:
> drm_connector_register(connector);
>
> intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> - intel_panel_setup_backlight(connector);
> + intel_panel_setup_backlight(connector, INVALID_PIPE);
>
> return;
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 847d00f9..f6e8d23 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1117,7 +1117,7 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector)
> 0, panel->backlight.max);
> }
>
> -static int bdw_setup_backlight(struct intel_connector *connector)
> +static int bdw_setup_backlight(struct intel_connector *connector, enum pipe unused)
> {
> struct drm_device *dev = connector->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1143,7 +1143,7 @@ static int bdw_setup_backlight(struct intel_connector *connector)
> return 0;
> }
>
> -static int pch_setup_backlight(struct intel_connector *connector)
> +static int pch_setup_backlight(struct intel_connector *connector, enum pipe unused)
> {
> struct drm_device *dev = connector->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1170,7 +1170,7 @@ static int pch_setup_backlight(struct intel_connector *connector)
> return 0;
> }
>
> -static int i9xx_setup_backlight(struct intel_connector *connector)
> +static int i9xx_setup_backlight(struct intel_connector *connector, enum pipe unused)
> {
> struct drm_device *dev = connector->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1202,7 +1202,7 @@ static int i9xx_setup_backlight(struct intel_connector *connector)
> return 0;
> }
>
> -static int i965_setup_backlight(struct intel_connector *connector)
> +static int i965_setup_backlight(struct intel_connector *connector, enum pipe unused)
> {
> struct drm_device *dev = connector->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1232,37 +1232,40 @@ static int i965_setup_backlight(struct intel_connector *connector)
> return 0;
> }
>
> -static int vlv_setup_backlight(struct intel_connector *connector)
> +static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe)
> {
> struct drm_device *dev = connector->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_panel *panel = &connector->panel;
> - enum pipe pipe;
> + enum pipe p;
> u32 ctl, ctl2, val;
>
> - for_each_pipe(dev_priv, pipe) {
> - u32 cur_val = I915_READ(VLV_BLC_PWM_CTL(pipe));
> + for_each_pipe(dev_priv, p) {
> + u32 cur_val = I915_READ(VLV_BLC_PWM_CTL(p));
>
> /* Skip if the modulation freq is already set */
> if (cur_val & ~BACKLIGHT_DUTY_CYCLE_MASK)
> continue;
>
> cur_val &= BACKLIGHT_DUTY_CYCLE_MASK;
> - I915_WRITE(VLV_BLC_PWM_CTL(pipe), (0xf42 << 16) |
> + I915_WRITE(VLV_BLC_PWM_CTL(p), (0xf42 << 16) |
> cur_val);
> }
I think we could try a follow-up patch doing the above only for the pipe
passed in. That's the one we're using, and the regs will be written in
backlight enable if the pipe changes.
Anyway, I like this approach. I had a patch trying to figure all this
out harder in the backlight code, but really anyone calling setup
backlight is in a much better position to do it.
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>
> - ctl2 = I915_READ(VLV_BLC_PWM_CTL2(PIPE_A));
> + if (WARN_ON(pipe != PIPE_A && pipe != PIPE_B))
> + return -ENODEV;
> +
> + ctl2 = I915_READ(VLV_BLC_PWM_CTL2(pipe));
> panel->backlight.active_low_pwm = ctl2 & BLM_POLARITY_I965;
>
> - ctl = I915_READ(VLV_BLC_PWM_CTL(PIPE_A));
> + ctl = I915_READ(VLV_BLC_PWM_CTL(pipe));
> panel->backlight.max = ctl >> 16;
> if (!panel->backlight.max)
> return -ENODEV;
>
> panel->backlight.min = get_backlight_min_vbt(connector);
>
> - val = _vlv_get_backlight(dev, PIPE_A);
> + val = _vlv_get_backlight(dev, pipe);
> panel->backlight.level = intel_panel_compute_brightness(connector, val);
>
> panel->backlight.enabled = (ctl2 & BLM_PWM_ENABLE) &&
> @@ -1271,7 +1274,7 @@ static int vlv_setup_backlight(struct intel_connector *connector)
> return 0;
> }
>
> -int intel_panel_setup_backlight(struct drm_connector *connector)
> +int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
> {
> struct drm_device *dev = connector->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1290,7 +1293,7 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
>
> /* set level and max in panel struct */
> mutex_lock(&dev_priv->backlight_lock);
> - ret = dev_priv->display.setup_backlight(intel_connector);
> + ret = dev_priv->display.setup_backlight(intel_connector, pipe);
> mutex_unlock(&dev_priv->backlight_lock);
>
> if (ret) {
> --
> 2.0.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
