On 12/01/2014 03:04 PM, Jani Nikula wrote: > On Fri, 28 Nov 2014, Jay Aurabind <mail@xxxxxxxxxxxx> wrote: >> Hello all, >> >> I notice that some activity has been going on with the minimum value >> of display brightness recently (e1c412e7575). >> >> But the minimum value thats currently chosen is not at all acceptable >> for my eyes. My display is working perfectly without that restriction >> on minimum intensity. I tend to stare at the screen a lot and the >> current minimum settings is straining my eyes. >> >> Even if you say that it may not be good for the devices, I still >> insist, because I want my *display* to fail first, not my eyes. So >> please try providing a way for the needy users to override this >> minimum settings. I hope adding a module parameter would be easy fix. >> >> Something like this: ? (I dont call myself a kernel programmer yet, >> just scratching the surface) >> >> >> Provide provision for users to disable restriction on minimum brightness >> value introduced in: >> >> commit e1c412e75754ab7b7002f3e18a2652d999c40d4b >> Author: Jani Nikula <jani.nikula@xxxxxxxxx> >> Date: Wed Nov 5 14:46:31 2014 +0200 >> >> drm/i915: safeguard against too high minimum brightness >> >> There are systems which work reliably without restriction on minimum >> value of display brightness. Also the arbitrary value may be too high >> for many users as well. > > Please file a new bug at [1], reference this mail, and attach > /sys/kernel/debug/dri/0/i915_opregion. Thank you for the response. But the file you mentioned to attach seems to be a binary file, because I'm getting lot of junk characters. Is this normal ? > > The minimum value is chosen and provided by the OEM. There's still some > open questions about the interpretation of the value though (which lead > to the commit you referenced), so I'm hesitant to make changes before we > have those cleared up. From a user's point of view, the reason many people and myself stick to linux is the flexibility it provides and the power the user has, and when you introduce a new policy without an easy "roll back" to get back the previous "mechanism", I would like to let you know that it matters. > In general I am not fond of adding new module parameters for tuning > things. Every new knob is another point of failure that we need to test, > and they are pretty much part of the ABI we can't easily drop or change. Would it be difficult to remove a parameter, if it is marked "experimental" ? Is there a fix this without changing ABI ? I mean can you make this policy apply only on those hardware which seems to have a problem with the value of minimum backlight? Since they are the minority, why should a policy for fixing them affect the rest of us ? -- Thanks and Regards, Aurabindo J > > BR, > Jani. > > [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel > > >> >> Signed-off-by: Aurabindo J <mail@xxxxxxxxxxxx> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> drivers/gpu/drm/i915/i915_params.c | 6 ++++++ >> drivers/gpu/drm/i915/intel_panel.c | 14 ++++++++++---- >> 3 files changed, 17 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 16a6f6d..55d2ead 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2213,6 +2213,7 @@ struct i915_params { >> int disable_power_well; >> int enable_ips; >> int invert_brightness; >> + int restrict_min_brightness; >> int enable_cmd_parser; >> /* leave bools at the end to not create holes */ >> bool enable_hangcheck; >> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c >> index c91cb20..0601c2a 100644 >> --- a/drivers/gpu/drm/i915/i915_params.c >> +++ b/drivers/gpu/drm/i915/i915_params.c >> @@ -46,6 +46,7 @@ struct i915_params i915 __read_mostly = { >> .prefault_disable = 0, >> .reset = true, >> .invert_brightness = 0, >> + .restrict_min_brightness = 1, >> .disable_display = 0, >> .enable_cmd_parser = 1, >> .disable_vtd_wa = 0, >> @@ -155,6 +156,11 @@ MODULE_PARM_DESC(invert_brightness, >> "to dri-devel@xxxxxxxxxxxxxxxxxxxxx, if your machine needs it. " >> "It will then be included in an upcoming module version."); >> >> +module_param_named(restrict_min_brightness, i915.restrict_min_brightness, int, 0600); >> +MODULE_PARM_DESC(restrict_min_brightness, >> + "Restrict minimum brightness " >> + "(-1 disable restriction, 0 value from VBT, 1 arbitrary value )"); >> + >> module_param_named(disable_display, i915.disable_display, bool, 0600); >> MODULE_PARM_DESC(disable_display, "Disable display (default: false)"); >> >> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c >> index 41b3be2..def9f4e 100644 >> --- a/drivers/gpu/drm/i915/intel_panel.c >> +++ b/drivers/gpu/drm/i915/intel_panel.c >> @@ -1109,10 +1109,16 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector) >> * 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); >> + if (i915.restrict_min_brightness < 0) >> + min = 0; >> + else if (i915.restrict_min_brightness == 0) >> + min = dev_priv->vbt.backlight.min_brightness; >> + else { >> + 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] */ >> -- >> 2.1.3 >> >> >> >> >> >
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
