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

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

[Jani Nikula]

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);
+	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

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

[Daniel Vetter]

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

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

[Jani Nikula]

On Wed, 05 Nov 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> 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>

Pushed to drm-intel-fixes, thanks for the review.

BR,
Jani.

>
> 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

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