From patchwork Mon Nov 9 16:57:22 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jani Nikula X-Patchwork-Id: 7585581 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 19DF9C05C6 for ; Mon, 9 Nov 2015 16:57:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 302A2205F7 for ; Mon, 9 Nov 2015 16:57:58 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 26644205F4 for ; Mon, 9 Nov 2015 16:57:57 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 97DC96E4DF; Mon, 9 Nov 2015 08:57:56 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 869B06E4DF for ; Mon, 9 Nov 2015 08:57:54 -0800 (PST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga102.jf.intel.com with ESMTP; 09 Nov 2015 08:57:54 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,266,1444719600"; d="scan'208";a="846822350" Received: from bmurphy-mobl3.ger.corp.intel.com (HELO localhost) ([10.252.7.31]) by fmsmga002.fm.intel.com with ESMTP; 09 Nov 2015 08:57:51 -0800 From: Jani Nikula To: Paulo Zanoni In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <1447051331-11829-1-git-send-email-sylee@canonical.com> <87si4fjx33.fsf@intel.com> User-Agent: Notmuch/0.20.2+57~gff3a03d (http://notmuchmail.org) Emacs/24.4.1 (x86_64-pc-linux-gnu) Date: Mon, 09 Nov 2015 18:57:22 +0200 Message-ID: <874mgvrtzh.fsf@intel.com> MIME-Version: 1.0 Cc: "Shih-Yuan Lee \(FourDollars\)" , Intel Graphics Development Subject: Re: [Intel-gfx] [PATCH] drm/i915: A better maximum brightness for users. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, 09 Nov 2015, Paulo Zanoni wrote: > 2015-11-09 8:17 GMT-02:00 Jani Nikula : >> On Mon, 09 Nov 2015, "Shih-Yuan Lee (FourDollars)" wrote: >>> The PWM brightness level of Dell XPS 13 (2015) is from 10 to 937 however >>> the sysfs brightness level always starts from 0 so it is better to use >>> 927 as the sysfs maximum brightness level and it becomes easier to map >>> from the PWM brightness level to the sysfs brightness level. >> >> We've been thinking we should provide a fixed range to userspace >> instead. Say, 0-100. > > While not clearly stated, this reply and the further ones sound like a > rejection to his patch. I wanted to understand the motivation rather than bluntly rejecting. > I'm not really an expert on backlight, but his idea sounds simple and > an overall improvement to the codebase. I don't think it's fair to > block a simple one-line improvement based on the fact that we have an > idea (that may or may not be implemented) for a possibly better > implementation. Why not apply his patch (in case we conclude it > doesn't have any bugs), and then go for the 0-100 idea once someone > actually decides to implement it? The implementation is and "just" testing is required. > Besides, isn't there the possibility of having a panel that has a > 0-9999 range where the change from 0 to 9990 is negligible, and only > 9991-9999 matters, so when we scale to 0-100 it will become basically > a on/off switch? We all learned to never underestimate panel hardware. That's a somewhat more valid argument. The answer to that should be that we map the userspace range to the hardware range according to a curve rather than a linear mapping. See Ville's reply. BR, Jani. > >> >> BR, >> Jani. >> >> >> >> >>> >>> Signed-off-by: Shih-Yuan Lee (FourDollars) >>> --- >>> drivers/gpu/drm/i915/intel_panel.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c >>> index a24df35..697fd4d 100644 >>> --- a/drivers/gpu/drm/i915/intel_panel.c >>> +++ b/drivers/gpu/drm/i915/intel_panel.c >>> @@ -1211,7 +1211,7 @@ static int intel_backlight_device_register(struct intel_connector *connector) >>> * Note: Everything should work even if the backlight device max >>> * presented to the userspace is arbitrarily chosen. >>> */ >>> - props.max_brightness = panel->backlight.max; >>> + props.max_brightness = panel->backlight.max - panel->backlight.min; >>> props.brightness = scale_hw_to_user(connector, >>> panel->backlight.level, >>> props.max_brightness); >> >> -- >> Jani Nikula, Intel Open Source Technology Center >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index a24df35e11e7..d5d86601d411 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1211,7 +1211,7 @@ static int intel_backlight_device_register(struct intel_connector *connector) * Note: Everything should work even if the backlight device max * presented to the userspace is arbitrarily chosen. */ - props.max_brightness = panel->backlight.max; + props.max_brightness = 100; props.brightness = scale_hw_to_user(connector, panel->backlight.level, props.max_brightness);