Re: [PATCH] drm/i915: safeguard against too high minimum brightness

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Nov 05, 2014 at 02:46:31PM +0200, Jani Nikula wrote:
> Never trust (your interpretation of) the VBT. Regression from
> 
> commit 6dda730e55f412a6dfb181cae6784822ba463847
> Author: Jani Nikula <jani.nikula@intel.com>
> Date:   Tue Jun 24 18:27:40 2014 +0300
> 
>     drm/i915: respect the VBT minimum backlight brightness
> 
> causing div by zero if VBT minimum brightness equals maximum brightness.
> 
> Despite my attempts I've failed in my detective work to figure out what
> the root cause is. This is not the real fix, but we have to do
> something.
> 
> Reported-by: Mike Auty <mike.auty@gmail.com>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=86551
> Cc: stable@vger.kernel.org (v3.17+)
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_panel.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index e18b3f49074c..b001c90312e7 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1094,12 +1094,25 @@ static u32 get_backlight_min_vbt(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;
> +	int min;
>  
>  	WARN_ON(panel->backlight.max == 0);
>  
> +	/*
> +	 * XXX: If the vbt value is 255, it makes min equal to max, which leads
> +	 * to problems. There are such machines out there. Either our
> +	 * interpretation is wrong or the vbt has bogus data. Or both. Safeguard
> +	 * against this by letting the minimum be at most (arbitrarily chosen)
> +	 * 25% of the max.
> +	 */
> +	min = clamp_t(int, dev_priv->vbt.backlight.min_brightness, 0, 64);

min_brightness is unsigned everywhere I've look, so a min_t looks
sufficient here. Otherwise

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Also, more gin pls :(
-Daniel

> +	if (min != dev_priv->vbt.backlight.min_brightness) {
> +		DRM_DEBUG_KMS("clamping VBT min backlight %d/255 to %d/255\n",
> +			      dev_priv->vbt.backlight.min_brightness, min);
> +	}
> +
>  	/* vbt value is a coefficient in range [0..255] */
> -	return scale(dev_priv->vbt.backlight.min_brightness, 0, 255,
> -		     0, panel->backlight.max);
> +	return scale(min, 0, 255, 0, panel->backlight.max);
>  }
>  
>  static int bdw_setup_backlight(struct intel_connector *connector)
> -- 
> 2.1.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx