From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D68A2C433DF for ; Thu, 2 Jul 2020 08:01:13 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B4F1A20720 for ; Thu, 2 Jul 2020 08:01:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B4F1A20720 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ravnborg.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 38D106E13F; Thu, 2 Jul 2020 08:01:13 +0000 (UTC) Received: from asavdk4.altibox.net (asavdk4.altibox.net [109.247.116.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id 52DD96E13F for ; Thu, 2 Jul 2020 08:01:12 +0000 (UTC) Received: from ravnborg.org (unknown [188.228.123.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by asavdk4.altibox.net (Postfix) with ESMTPS id C4DC080575; Thu, 2 Jul 2020 10:01:06 +0200 (CEST) Date: Thu, 2 Jul 2020 10:01:05 +0200 From: Sam Ravnborg To: Emil Velikov Subject: Re: [PATCH v3 0/16] backlight updates Message-ID: <20200702080105.GA1277474@ravnborg.org> References: <20200601065207.492614-1-sam@ravnborg.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CMAE-Score: 0 X-CMAE-Analysis: v=2.3 cv=aP3eV41m c=1 sm=1 tr=0 a=S6zTFyMACwkrwXSdXUNehg==:117 a=S6zTFyMACwkrwXSdXUNehg==:17 a=kj9zAlcOel0A:10 a=7gkXJVJtAAAA:8 a=pGLkceISAAAA:8 a=e5mUnYsNAAAA:8 a=OR89HmRyg7TBtgxA9QYA:9 a=CjuIK1q_8ugA:10 a=E9Po1WZjFZOl8hwRPBS3:22 a=Vxmtnl_E_bksehYqCbjh:22 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-pwm@vger.kernel.org, Daniel Thompson , Support Opensource , Michael Hennerich , Jonathan Corbet , David Airlie , Daniel Vetter , Tomi Valkeinen , Bartlomiej Zolnierkiewicz , Andy Gross , ML dri-devel , Bjorn Andersson , Peter Ujfalusi , Thierry Reding , Thomas Zimmermann , Uwe Kleine-Konig , Jingoo Han , Lee Jones , patches@opensource.cirrus.com, linux-arm-msm Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Emil. Long overdue feedback, I did not find time to go back to this patch-set until now. On Tue, Jun 02, 2020 at 03:04:39PM +0100, Emil Velikov wrote: > Hi Sam, > > On Mon, 1 Jun 2020 at 07:52, Sam Ravnborg wrote: > > > > v3: > > - Dropped video patch that was reviewd and thus applied > > - Updated kernel-doc so all fields now have a short intro > > - Improved readability in a lot of places, thanks to review > > feedback from Daniel - thanks! > > - Added better intro to backlight > > - Added acks > > > > Several other smaller changes documented in the > > patches. > > I left out patches to make functions static as > > there are dependencies to drm-misc-next for these. > > When this is landed I have a pile of follow-up patches waiting, > > mostly introducing backlight_is_blank() all over. > > > What happened with my suggestion to use backlight_is_blank() in fbdev > core itself? Following your suggestion I implemented: +static inline int backlight_get_brightness(struct backlight_device *bd) +{ + if (backlight_is_blank(bd)) + return 0; + else + return bd->props.brightness; +} This results in code like this: static int adp8870_bl_update_status(struct backlight_device *bl) { return adp8870_bl_set(bl, backlight_get_brightness(bl)); } Compare that with the original code: static int adp8870_bl_update_status(struct backlight_device *bl) { int brightness = bl->props.brightness; if (bl->props.power != FB_BLANK_UNBLANK) brightness = 0; if (bl->props.fb_blank != FB_BLANK_UNBLANK) brightness = 0; return adp8870_bl_set(bl, brightness); } Much nicer! The old code reads the brightness property direct. I prefer the small helper so we do not hardcode too much of the internals in the drivers. Also the above is simpler than trying to maintain the correct value in props.brightness all the time. And can be introduced gradually. I will rework the series to use this helper. I will also fix so I can use a const backlight_device *. Thanks for the suggestion. Sam > It effectively makes 13/13 and the above mentioned follow-up series obsolete. > > That said, series look spot on. With the documentation fixed (pointer > by Daniel) patches 1-12 are: > Reviewed-by: Emil Velikov > > -Emil > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel