Re: [PATCH v2 1/3] drm/i915: use pch backlight override on hsw too

[Clint Taylor <clinton.a.taylor@intel.com>]

On 09/04/2015 06:55 AM, Jani Nikula wrote:
> Currently the difference between backlight control on HSW vs. BDW/SKL is
> that on HSW we modify the duty cycle on the CPU register, and have the
> hardware pass the changes on to the PCH registers. We still drive the
> PCH PWM on both. While HSW and BDW use the same LPT PCH, BDW does not
> pass these messages on to the PCH. Therefore on BDW we need to enable
> the PCH override bit, and program the PCH directly. (On SPT PCH, this
> mode is the default.) We could as well do this on HSW too, and in fact
> I've been told this is what a certain other operating system does. So
> use PCH backlight override on HSW too.
>
> This simplifies some follow-up code, but it does have the danger of
> breaking backlight on HSW machines. It should work, but mysterious are
> the ways of backlight.
>
> While at it, name the related backlight hooks according to the PCH
> rather than the CPU for clarity.
>
> Cc: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_panel.c | 34 +++++++++++++++++++++++-----------
>   1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index e2ab3f6ed022..681686e5d550 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -484,7 +484,7 @@ static u32 intel_panel_compute_brightness(struct intel_connector *connector,
>   	return val;
>   }
>
> -static u32 bdw_get_backlight(struct intel_connector *connector)
> +static u32 lpt_get_backlight(struct intel_connector *connector)
>   {
>   	struct drm_device *dev = connector->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -576,7 +576,7 @@ static u32 intel_panel_get_backlight(struct intel_connector *connector)
>   	return val;
>   }
>
> -static void bdw_set_backlight(struct intel_connector *connector, u32 level)
> +static void lpt_set_backlight(struct intel_connector *connector, u32 level)
>   {
>   	struct drm_device *dev = connector->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -729,6 +729,18 @@ void intel_panel_set_backlight_acpi(struct intel_connector *connector,
>   	mutex_unlock(&dev_priv->backlight_lock);
>   }
>
> +static void lpt_disable_backlight(struct intel_connector *connector)
> +{
> +	struct drm_device *dev = connector->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 tmp;
> +
> +	intel_panel_actually_set_backlight(connector, 0);
> +
> +	tmp = I915_READ(BLC_PWM_PCH_CTL1);
> +	I915_WRITE(BLC_PWM_PCH_CTL1, tmp & ~BLM_PCH_PWM_ENABLE);
> +}
> +
>   static void pch_disable_backlight(struct intel_connector *connector)
>   {
>   	struct drm_device *dev = connector->base.dev;
> @@ -829,7 +841,7 @@ void intel_panel_disable_backlight(struct intel_connector *connector)
>   	mutex_unlock(&dev_priv->backlight_lock);
>   }
>
> -static void bdw_enable_backlight(struct intel_connector *connector)
> +static void lpt_enable_backlight(struct intel_connector *connector)
>   {
>   	struct drm_device *dev = connector->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -1243,7 +1255,7 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector)
>   	return scale(min, 0, 255, 0, panel->backlight.max);
>   }
>
> -static int bdw_setup_backlight(struct intel_connector *connector, enum pipe unused)
> +static int lpt_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;
> @@ -1260,7 +1272,7 @@ static int bdw_setup_backlight(struct intel_connector *connector, enum pipe unus
>
>   	panel->backlight.min = get_backlight_min_vbt(connector);
>
> -	val = bdw_get_backlight(connector);
> +	val = lpt_get_backlight(connector);
>   	panel->backlight.level = intel_panel_compute_brightness(connector, val);
>
>   	panel->backlight.enabled = (pch_ctl1 & BLM_PCH_PWM_ENABLE) &&
> @@ -1519,12 +1531,12 @@ void intel_panel_init_backlight_funcs(struct drm_device *dev)
>   		dev_priv->display.disable_backlight = bxt_disable_backlight;
>   		dev_priv->display.set_backlight = bxt_set_backlight;
>   		dev_priv->display.get_backlight = bxt_get_backlight;
> -	} else if (IS_BROADWELL(dev) || IS_SKYLAKE(dev)) {
> -		dev_priv->display.setup_backlight = bdw_setup_backlight;
> -		dev_priv->display.enable_backlight = bdw_enable_backlight;
> -		dev_priv->display.disable_backlight = pch_disable_backlight;
> -		dev_priv->display.set_backlight = bdw_set_backlight;
> -		dev_priv->display.get_backlight = bdw_get_backlight;
> +	} else if (HAS_PCH_LPT(dev) || HAS_PCH_SPT(dev)) {
> +		dev_priv->display.setup_backlight = lpt_setup_backlight;
> +		dev_priv->display.enable_backlight = lpt_enable_backlight;
> +		dev_priv->display.disable_backlight = lpt_disable_backlight;
> +		dev_priv->display.set_backlight = lpt_set_backlight;
> +		dev_priv->display.get_backlight = lpt_get_backlight;
>   	} else if (HAS_PCH_SPLIT(dev)) {
>   		dev_priv->display.setup_backlight = pch_setup_backlight;
>   		dev_priv->display.enable_backlight = pch_enable_backlight;
>

Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>

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