From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915 Trouble with minimum brightness Date: Mon, 01 Dec 2014 11:34:55 +0200 Message-ID: <87k32bbtmo.fsf@intel.com> References: <5478C9AA.2010600@aurabindo.in> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <5478C9AA.2010600@aurabindo.in> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Jay Aurabind , Daniel Vetter , David Airlie , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCAyOCBOb3YgMjAxNCwgSmF5IEF1cmFiaW5kIDxtYWlsQGF1cmFiaW5kby5pbj4gd3Jv dGU6Cj4gSGVsbG8gYWxsLAo+Cj4gSSBub3RpY2UgdGhhdCBzb21lIGFjdGl2aXR5IGhhcyBiZWVu IGdvaW5nIG9uIHdpdGggdGhlIG1pbmltdW0gdmFsdWUKPiBvZiBkaXNwbGF5IGJyaWdodG5lc3Mg cmVjZW50bHkgKGUxYzQxMmU3NTc1KS4KPgo+IEJ1dCB0aGUgbWluaW11bSB2YWx1ZSB0aGF0cyBj dXJyZW50bHkgY2hvc2VuIGlzIG5vdCBhdCBhbGwgYWNjZXB0YWJsZQo+IGZvciBteSBleWVzLiBN eSBkaXNwbGF5IGlzIHdvcmtpbmcgcGVyZmVjdGx5IHdpdGhvdXQgdGhhdCByZXN0cmljdGlvbgo+ IG9uIG1pbmltdW0gaW50ZW5zaXR5LiAgSSB0ZW5kIHRvIHN0YXJlIGF0IHRoZSBzY3JlZW4gYSBs b3QgYW5kIHRoZQo+IGN1cnJlbnQgbWluaW11bSBzZXR0aW5ncyBpcyBzdHJhaW5pbmcgbXkgZXll cy4KPgo+IEV2ZW4gaWYgeW91IHNheSB0aGF0IGl0IG1heSBub3QgYmUgZ29vZCBmb3IgdGhlIGRl dmljZXMsIEkgc3RpbGwKPiBpbnNpc3QsIGJlY2F1c2UgSSB3YW50IG15ICpkaXNwbGF5KiB0byBm YWlsIGZpcnN0LCBub3QgbXkgZXllcy4gIFNvCj4gcGxlYXNlIHRyeSBwcm92aWRpbmcgYSB3YXkg Zm9yIHRoZSBuZWVkeSB1c2VycyB0byBvdmVycmlkZSB0aGlzCj4gbWluaW11bSBzZXR0aW5ncy4g SSBob3BlIGFkZGluZyBhIG1vZHVsZSBwYXJhbWV0ZXIgd291bGQgYmUgZWFzeSBmaXguCj4KPiBT b21ldGhpbmcgbGlrZSB0aGlzOiA/IChJIGRvbnQgY2FsbCBteXNlbGYgYSBrZXJuZWwgcHJvZ3Jh bW1lciB5ZXQsCj4ganVzdCBzY3JhdGNoaW5nIHRoZSBzdXJmYWNlKQo+Cj4KPiBQcm92aWRlIHBy b3Zpc2lvbiBmb3IgdXNlcnMgdG8gZGlzYWJsZSByZXN0cmljdGlvbiBvbiBtaW5pbXVtIGJyaWdo dG5lc3MKPiB2YWx1ZSBpbnRyb2R1Y2VkIGluOgo+Cj4gICAgIGNvbW1pdCBlMWM0MTJlNzU3NTRh YjdiNzAwMmYzZTE4YTI2NTJkOTk5YzQwZDRiCj4gICAgIEF1dGhvcjogSmFuaSBOaWt1bGEgPGph bmkubmlrdWxhQGludGVsLmNvbT4KPiAgICAgRGF0ZTogICBXZWQgTm92IDUgMTQ6NDY6MzEgMjAx NCArMDIwMAo+Cj4gICAgICAgICBkcm0vaTkxNTogc2FmZWd1YXJkIGFnYWluc3QgdG9vIGhpZ2gg bWluaW11bSBicmlnaHRuZXNzCj4KPiBUaGVyZSBhcmUgc3lzdGVtcyB3aGljaCB3b3JrIHJlbGlh Ymx5IHdpdGhvdXQgcmVzdHJpY3Rpb24gb24gbWluaW11bQo+IHZhbHVlIG9mIGRpc3BsYXkgYnJp Z2h0bmVzcy4gQWxzbyB0aGUgYXJiaXRyYXJ5IHZhbHVlIG1heSBiZSB0b28gaGlnaAo+IGZvciBt YW55IHVzZXJzIGFzIHdlbGwuCgpQbGVhc2UgZmlsZSBhIG5ldyBidWcgYXQgWzFdLCByZWZlcmVu Y2UgdGhpcyBtYWlsLCBhbmQgYXR0YWNoCi9zeXMva2VybmVsL2RlYnVnL2RyaS8wL2k5MTVfb3By ZWdpb24uCgpUaGUgbWluaW11bSB2YWx1ZSBpcyBjaG9zZW4gYW5kIHByb3ZpZGVkIGJ5IHRoZSBP RU0uIFRoZXJlJ3Mgc3RpbGwgc29tZQpvcGVuIHF1ZXN0aW9ucyBhYm91dCB0aGUgaW50ZXJwcmV0 YXRpb24gb2YgdGhlIHZhbHVlIHRob3VnaCAod2hpY2ggbGVhZAp0byB0aGUgY29tbWl0IHlvdSBy ZWZlcmVuY2VkKSwgc28gSSdtIGhlc2l0YW50IHRvIG1ha2UgY2hhbmdlcyBiZWZvcmUgd2UKaGF2 ZSB0aG9zZSBjbGVhcmVkIHVwLgoKSW4gZ2VuZXJhbCBJIGFtIG5vdCBmb25kIG9mIGFkZGluZyBu ZXcgbW9kdWxlIHBhcmFtZXRlcnMgZm9yIHR1bmluZwp0aGluZ3MuIEV2ZXJ5IG5ldyBrbm9iIGlz IGFub3RoZXIgcG9pbnQgb2YgZmFpbHVyZSB0aGF0IHdlIG5lZWQgdG8gdGVzdCwKYW5kIHRoZXkg YXJlIHByZXR0eSBtdWNoIHBhcnQgb2YgdGhlIEFCSSB3ZSBjYW4ndCBlYXNpbHkgZHJvcCBvciBj aGFuZ2UuCgpCUiwKSmFuaS4KClsxXSBodHRwczovL2J1Z3MuZnJlZWRlc2t0b3Aub3JnL2VudGVy X2J1Zy5jZ2k/cHJvZHVjdD1EUkkmY29tcG9uZW50PURSTS9JbnRlbAoKCj4KPiBTaWduZWQtb2Zm LWJ5OiBBdXJhYmluZG8gSiA8bWFpbEBhdXJhYmluZG8uaW4+Cj4gLS0tCj4gIGRyaXZlcnMvZ3B1 L2RybS9pOTE1L2k5MTVfZHJ2LmggICAgfCAgMSArCj4gIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2k5 MTVfcGFyYW1zLmMgfCAgNiArKysrKysKPiAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfcGFu ZWwuYyB8IDE0ICsrKysrKysrKystLS0tCj4gIDMgZmlsZXMgY2hhbmdlZCwgMTcgaW5zZXJ0aW9u cygrKSwgNCBkZWxldGlvbnMoLSkKPgo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkx NS9pOTE1X2Rydi5oIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kcnYuaAo+IGluZGV4IDE2 YTZmNmQuLjU1ZDJlYWQgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9k cnYuaAo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZHJ2LmgKPiBAQCAtMjIxMyw2 ICsyMjEzLDcgQEAgc3RydWN0IGk5MTVfcGFyYW1zIHsKPiAgCWludCBkaXNhYmxlX3Bvd2VyX3dl bGw7Cj4gIAlpbnQgZW5hYmxlX2lwczsKPiAgCWludCBpbnZlcnRfYnJpZ2h0bmVzczsKPiArCWlu dCByZXN0cmljdF9taW5fYnJpZ2h0bmVzczsKPiAgCWludCBlbmFibGVfY21kX3BhcnNlcjsKPiAg CS8qIGxlYXZlIGJvb2xzIGF0IHRoZSBlbmQgdG8gbm90IGNyZWF0ZSBob2xlcyAqLwo+ICAJYm9v bCBlbmFibGVfaGFuZ2NoZWNrOwo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9p OTE1X3BhcmFtcy5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9wYXJhbXMuYwo+IGluZGV4 IGM5MWNiMjAuLjA2MDFjMmEgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkx NV9wYXJhbXMuYwo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfcGFyYW1zLmMKPiBA QCAtNDYsNiArNDYsNyBAQCBzdHJ1Y3QgaTkxNV9wYXJhbXMgaTkxNSBfX3JlYWRfbW9zdGx5ID0g ewo+ICAJLnByZWZhdWx0X2Rpc2FibGUgPSAwLAo+ICAJLnJlc2V0ID0gdHJ1ZSwKPiAgCS5pbnZl cnRfYnJpZ2h0bmVzcyA9IDAsCj4gKwkucmVzdHJpY3RfbWluX2JyaWdodG5lc3MgPSAxLAo+ICAJ LmRpc2FibGVfZGlzcGxheSA9IDAsCj4gIAkuZW5hYmxlX2NtZF9wYXJzZXIgPSAxLAo+ICAJLmRp c2FibGVfdnRkX3dhID0gMCwKPiBAQCAtMTU1LDYgKzE1NiwxMSBAQCBNT0RVTEVfUEFSTV9ERVND KGludmVydF9icmlnaHRuZXNzLAo+ICAJInRvIGRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5v cmcsIGlmIHlvdXIgbWFjaGluZSBuZWVkcyBpdC4gIgo+ICAJIkl0IHdpbGwgdGhlbiBiZSBpbmNs dWRlZCBpbiBhbiB1cGNvbWluZyBtb2R1bGUgdmVyc2lvbi4iKTsKPgo+ICttb2R1bGVfcGFyYW1f bmFtZWQocmVzdHJpY3RfbWluX2JyaWdodG5lc3MsIGk5MTUucmVzdHJpY3RfbWluX2JyaWdodG5l c3MsIGludCwgMDYwMCk7Cj4gK01PRFVMRV9QQVJNX0RFU0MocmVzdHJpY3RfbWluX2JyaWdodG5l c3MsCj4gKwkiUmVzdHJpY3QgbWluaW11bSBicmlnaHRuZXNzICIKPiArCSIoLTEgZGlzYWJsZSBy ZXN0cmljdGlvbiwgMCB2YWx1ZSBmcm9tIFZCVCwgMSBhcmJpdHJhcnkgdmFsdWUgKSIpOwo+ICsK PiAgbW9kdWxlX3BhcmFtX25hbWVkKGRpc2FibGVfZGlzcGxheSwgaTkxNS5kaXNhYmxlX2Rpc3Bs YXksIGJvb2wsIDA2MDApOwo+ICBNT0RVTEVfUEFSTV9ERVNDKGRpc2FibGVfZGlzcGxheSwgIkRp c2FibGUgZGlzcGxheSAoZGVmYXVsdDogZmFsc2UpIik7Cj4KPiBkaWZmIC0tZ2l0IGEvZHJpdmVy cy9ncHUvZHJtL2k5MTUvaW50ZWxfcGFuZWwuYyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVs X3BhbmVsLmMKPiBpbmRleCA0MWIzYmUyLi5kZWY5ZjRlIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMv Z3B1L2RybS9pOTE1L2ludGVsX3BhbmVsLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9p bnRlbF9wYW5lbC5jCj4gQEAgLTExMDksMTAgKzExMDksMTYgQEAgc3RhdGljIHUzMiBnZXRfYmFj a2xpZ2h0X21pbl92YnQoc3RydWN0IGludGVsX2Nvbm5lY3RvciAqY29ubmVjdG9yKQo+ICAJICog YWdhaW5zdCB0aGlzIGJ5IGxldHRpbmcgdGhlIG1pbmltdW0gYmUgYXQgbW9zdCAoYXJiaXRyYXJp bHkgY2hvc2VuKQo+ICAJICogMjUlIG9mIHRoZSBtYXguCj4gIAkgKi8KPiAtCW1pbiA9IGNsYW1w X3QoaW50LCBkZXZfcHJpdi0+dmJ0LmJhY2tsaWdodC5taW5fYnJpZ2h0bmVzcywgMCwgNjQpOwo+ IC0JaWYgKG1pbiAhPSBkZXZfcHJpdi0+dmJ0LmJhY2tsaWdodC5taW5fYnJpZ2h0bmVzcykgewo+ IC0JCURSTV9ERUJVR19LTVMoImNsYW1waW5nIFZCVCBtaW4gYmFja2xpZ2h0ICVkLzI1NSB0byAl ZC8yNTVcbiIsCj4gLQkJCSAgICAgIGRldl9wcml2LT52YnQuYmFja2xpZ2h0Lm1pbl9icmlnaHRu ZXNzLCBtaW4pOwo+ICsJaWYgKGk5MTUucmVzdHJpY3RfbWluX2JyaWdodG5lc3MgPCAwKQo+ICsJ CW1pbiA9IDA7Cj4gKwllbHNlIGlmIChpOTE1LnJlc3RyaWN0X21pbl9icmlnaHRuZXNzID09IDAp Cj4gKwkJbWluID0gZGV2X3ByaXYtPnZidC5iYWNrbGlnaHQubWluX2JyaWdodG5lc3M7Cj4gKwll bHNlIHsKPiArCQltaW4gPSBjbGFtcF90KGludCwgZGV2X3ByaXYtPnZidC5iYWNrbGlnaHQubWlu X2JyaWdodG5lc3MsIDAsIDY0KTsKPiArCQlpZiAobWluICE9IGRldl9wcml2LT52YnQuYmFja2xp Z2h0Lm1pbl9icmlnaHRuZXNzKSB7Cj4gKwkJCURSTV9ERUJVR19LTVMoImNsYW1waW5nIFZCVCBt aW4gYmFja2xpZ2h0ICVkLzI1NSB0byAlZC8yNTVcbiIsCj4gKwkJCQkJZGV2X3ByaXYtPnZidC5i YWNrbGlnaHQubWluX2JyaWdodG5lc3MsIG1pbik7Cj4gKwkgICAgfQo+ICAJfQo+Cj4gIAkvKiB2 YnQgdmFsdWUgaXMgYSBjb2VmZmljaWVudCBpbiByYW5nZSBbMC4uMjU1XSAqLwo+IC0tIAo+IDIu MS4zCj4KPgo+Cj4KPgoKLS0gCkphbmkgTmlrdWxhLCBJbnRlbCBPcGVuIFNvdXJjZSBUZWNobm9s b2d5IENlbnRlcgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f XwpJbnRlbC1nZnggbWFpbGluZyBsaXN0CkludGVsLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcK aHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752874AbaLAJfR (ORCPT ); Mon, 1 Dec 2014 04:35:17 -0500 Received: from mga09.intel.com ([134.134.136.24]:14393 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752716AbaLAJfN (ORCPT ); Mon, 1 Dec 2014 04:35:13 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,492,1413270000"; d="scan'208";a="646103243" From: Jani Nikula To: Jay Aurabind , Daniel Vetter , David Airlie , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/i915 Trouble with minimum brightness In-Reply-To: <5478C9AA.2010600@aurabindo.in> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <5478C9AA.2010600@aurabindo.in> User-Agent: Notmuch/0.19~rc1+1~g08b4944 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu) Date: Mon, 01 Dec 2014 11:34:55 +0200 Message-ID: <87k32bbtmo.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 28 Nov 2014, Jay Aurabind 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 > 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. 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. 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. BR, Jani. [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel > > Signed-off-by: Aurabindo J > --- > 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@lists.freedesktop.org, 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 > > > > > -- Jani Nikula, Intel Open Source Technology Center