From patchwork Thu Jun 3 22:43:48 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 453341 Delivered-To: patch@linaro.org Received: by 2002:a02:c735:0:0:0:0:0 with SMTP id h21csp675556jao; Thu, 3 Jun 2021 15:46:41 -0700 (PDT) X-Google-Smtp-Source: ABdhPJywX8QlNvEI5NVxmkOHbTNBptBpyVXlN7HFSpJWnJn2n30/MlejS+NLqJCk603IxQRpLgJl X-Received: by 2002:a63:d45:: with SMTP id 5mr1750940pgn.72.1622760401620; Thu, 03 Jun 2021 15:46:41 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1622760401; cv=none; d=google.com; s=arc-20160816; b=fAJrPYT7+jhShLsoWA9nsOBoMv75e92uBLAkA7vxbk44hsxDTJ4ZZSFksKpiwbfb4l yOrC48/x03JE5WNES21atGvYEY4QlP428E94BhV4+YwXXMSD2Ja/mSFNlXoY2Zr9YCfP H7GaxyQEg8dc8rbq9DLvCT/3T7MXnfq3JSzDrwLzzOO3nCfLsj4VkvrFU9FeK4IFDONk ee3oNwIQGMPaVePguJQptsnBOfZtwET4hsX0CA0bI9/NHl4YcPMRHi7WVLhWU08GSkJJ Z6QbF6l5Nec42GJ4JLlgUiVHZqpzk9Feofyny77dUQ314RFYvLV8YznnelxvMsCq+nWs qPsg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:message-id:date:subject:to:from:dkim-signature :delivered-to; bh=DfhY/QNJwoAcR0w/J0pS6U38bj1S77B4Ht7nL6DWsLo=; b=aP37vnQziXVmYGMlqtLg5Hbge0vpFuAH29h9ycIsjYmDd98NjFQZlOVX3gkTRT2jJJ yISpqZ5zWe0Ml4vZkJAfLG62MTf7KuncNbrRlbs1Fflvs2sKMNVlpfNEjyUgNc1LDr92 ESTkX3hJztrEqC507/J95S04W9P4Aa04CYXMrAhO4Nxu5WXk5t/bx4fIDOgBXiS21fgo GV4xKWfN5dmzNF+zdVE26bWqK1REXPB9nBXY6XPi/hZM12s1ArJh0VwEjxqT+qrfhDbP 8b6VvlKuV0crdQMjmhN0K3l1cCQupltCRHIfsFf6qq2jKTVUo0aTj0k4MTsAmRuMUv1r 7FQQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=E8AGxvpR; spf=pass (google.com: best guess record for domain of dri-devel-bounces@lists.freedesktop.org designates 131.252.210.177 as permitted sender) smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from gabe.freedesktop.org (gabe.freedesktop.org. [131.252.210.177]) by mx.google.com with ESMTPS id c30si277916pgb.183.2021.06.03.15.46.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Jun 2021 15:46:41 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of dri-devel-bounces@lists.freedesktop.org designates 131.252.210.177 as permitted sender) client-ip=131.252.210.177; Authentication-Results: mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=E8AGxvpR; spf=pass (google.com: best guess record for domain of dri-devel-bounces@lists.freedesktop.org designates 131.252.210.177 as permitted sender) smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AFB4B6F53F; Thu, 3 Jun 2021 22:46:40 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-lj1-x232.google.com (mail-lj1-x232.google.com [IPv6:2a00:1450:4864:20::232]) by gabe.freedesktop.org (Postfix) with ESMTPS id 432C06F53F for ; Thu, 3 Jun 2021 22:46:39 +0000 (UTC) Received: by mail-lj1-x232.google.com with SMTP id c11so9122083ljd.6 for ; Thu, 03 Jun 2021 15:46:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=DfhY/QNJwoAcR0w/J0pS6U38bj1S77B4Ht7nL6DWsLo=; b=E8AGxvpRnNYLKhu04jhQNO2Mt//q2iJkK3GvTiCnPc7lfDRJWS+zxS4YLKRBZb3di3 hEdguPkdo2wPV112Ng8DWjJOjeybRfUJOmGTP3bqDX920ED+tG4mzVu2wsW6ufWrJElM jptsycj3jLvj4BI/RPnSqtU5Sw6HHjAY65nu1eZfphhDoEcpvwzG6L3Ond2HrMdH9mc7 BdjjxFcG/nSVz402bqmWvRVjp7QQ03dIbP7EmCp0mAybnTKEBdXKhE+kixYh9SvLVl7c ob1tag+CDB2LMOD81Ec8K028+ldxTlEtaYToHcMNyDLUlIfPCflemr7WnBjPFx9cYhdc 9JBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=DfhY/QNJwoAcR0w/J0pS6U38bj1S77B4Ht7nL6DWsLo=; b=S4PrZMHq9DvdHwAK+XK20sTqLAXowNBsWhjvIaKJuv4LKrnv1skDxqHoyzzJqJsRfJ KX3wTdB7Fdy1WxV0nFAifopk0VW2FQWDyxxkM2Q18Ci57o5oNDQT1+unyOVOgegyGdPN adfUPoWKgkZVOdW3AaEEeyh9TCpWUZmqH5PPUAwK0AQyMFvkOG76Pllxc0S7h/bZUHpj 7qCb20nhp7zla8ie2jy6OiHBfw4PW6G3d86uwowi8JaCa8KqrV93RI4lHi75umIkn9bJ KXYYaM1RZGg/QqvWNOvA/23oZ293hGF1smrd5i2o2blQsHFS/hSP0Hkerl0HltQo5zTx 6uMw== X-Gm-Message-State: AOAM533wmJwiUVTESFA0qsDqnz1Bn8ruKyBBqkGGV0v4Z43b561rQ2+b JvABtQmZr+1YCjAXcpkyU6ivmg== X-Received: by 2002:a2e:9ec4:: with SMTP id h4mr1104318ljk.442.1622760397541; Thu, 03 Jun 2021 15:46:37 -0700 (PDT) Received: from localhost.localdomain (c-fdcc225c.014-348-6c756e10.bbcust.telenor.se. [92.34.204.253]) by smtp.gmail.com with ESMTPSA id z13sm502099lji.115.2021.06.03.15.46.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Jun 2021 15:46:37 -0700 (PDT) From: Linus Walleij To: Lee Jones , Daniel Thompson , Jingoo Han , dri-devel@lists.freedesktop.org Subject: [PATCH v2] backlight: ktd253: Stabilize backlight Date: Fri, 4 Jun 2021 00:43:48 +0200 Message-Id: <20210603224348.3165584-1-linus.walleij@linaro.org> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 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: newbyte@disroot.org, Stephan Gerhold Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Remove interrupt disablement during backlight setting. It is way to dangerous and makes platforms instable by having it miss vblank IRQs leading to the graphics derailing. The code is using ndelay() which is not available on platforms such as ARM and will result in 32 * udelay(1) which is substantial. Add some code to detect if an interrupt occurs during the tight loop and in that case just redo it from the top. Fixes: 5317f37e48b9 ("backlight: Add Kinetic KTD253 backlight driver") Cc: Stephan Gerhold Reported-by: newbyte@disroot.org Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Alter the dimming code to check for how many ns the pulse is low, and if it gets to ~100 us then redo from start. This is to account for the advent that an IRQ arrives while setting backlight and hits the low pulse making it way too long. --- drivers/video/backlight/ktd253-backlight.c | 70 ++++++++++++++++------ 1 file changed, 53 insertions(+), 17 deletions(-) -- 2.31.1 diff --git a/drivers/video/backlight/ktd253-backlight.c b/drivers/video/backlight/ktd253-backlight.c index a7df5bcca9da..dca19769846e 100644 --- a/drivers/video/backlight/ktd253-backlight.c +++ b/drivers/video/backlight/ktd253-backlight.c @@ -25,6 +25,7 @@ #define KTD253_T_LOW_NS (200 + 10) /* Additional 10ns as safety factor */ #define KTD253_T_HIGH_NS (200 + 10) /* Additional 10ns as safety factor */ +#define KTD253_T_OFF_CRIT_NS 100000 /* 100 us, now it doesn't look good */ #define KTD253_T_OFF_MS 3 struct ktd253_backlight { @@ -34,13 +35,50 @@ struct ktd253_backlight { u16 ratio; }; +static void ktd253_backlight_set_max_ratio(struct ktd253_backlight *ktd253) +{ + gpiod_set_value_cansleep(ktd253->gpiod, 1); + ndelay(KTD253_T_HIGH_NS); + /* We always fall back to this when we power on */ +} + +static int ktd253_backlight_stepdown(struct ktd253_backlight *ktd253) +{ + /* + * These GPIO operations absolutely can NOT sleep so no _cansleep + * suffixes, and no using GPIO expanders on slow buses for this! + * + * The maximum number of cycles of the loop is 32 so the time taken + * should nominally be: + * (T_LOW_NS + T_HIGH_NS + loop_time) * 32 + * + * Architectures do not always support ndelay() and we will get a few us + * instead. If we get to a critical time limit an interrupt has likely + * occured in the low part of the loop and we need to restart from the + * top so we have the backlight in a known state. + */ + u64 ns; + + ns = ktime_get_ns(); + gpiod_set_value(ktd253->gpiod, 0); + ndelay(KTD253_T_LOW_NS); + gpiod_set_value(ktd253->gpiod, 1); + ns = ktime_get_ns() - ns; + if (ns >= KTD253_T_OFF_CRIT_NS) { + dev_err(ktd253->dev, "PCM on backlight took too long (%llu ns)\n", ns); + return -EAGAIN; + } + ndelay(KTD253_T_HIGH_NS); + return 0; +} + static int ktd253_backlight_update_status(struct backlight_device *bl) { struct ktd253_backlight *ktd253 = bl_get_data(bl); int brightness = backlight_get_brightness(bl); u16 target_ratio; u16 current_ratio = ktd253->ratio; - unsigned long flags; + int ret; dev_dbg(ktd253->dev, "new brightness/ratio: %d/32\n", brightness); @@ -62,37 +100,35 @@ static int ktd253_backlight_update_status(struct backlight_device *bl) } if (current_ratio == 0) { - gpiod_set_value_cansleep(ktd253->gpiod, 1); - ndelay(KTD253_T_HIGH_NS); - /* We always fall back to this when we power on */ + ktd253_backlight_set_max_ratio(ktd253); current_ratio = KTD253_MAX_RATIO; } - /* - * WARNING: - * The loop to set the correct current level is performed - * with interrupts disabled as it is timing critical. - * The maximum number of cycles of the loop is 32 - * so the time taken will be (T_LOW_NS + T_HIGH_NS + loop_time) * 32, - */ - local_irq_save(flags); while (current_ratio != target_ratio) { /* * These GPIO operations absolutely can NOT sleep so no * _cansleep suffixes, and no using GPIO expanders on * slow buses for this! */ - gpiod_set_value(ktd253->gpiod, 0); - ndelay(KTD253_T_LOW_NS); - gpiod_set_value(ktd253->gpiod, 1); - ndelay(KTD253_T_HIGH_NS); + ret = ktd253_backlight_stepdown(ktd253); + if (ret == -EAGAIN) { + /* + * Something disturbed the backlight setting code when + * running so we need to bring the PWM back to a known + * state. This shouldn't happen too much. + */ + gpiod_set_value_cansleep(ktd253->gpiod, 0); + msleep(KTD253_T_OFF_MS); + ktd253_backlight_set_max_ratio(ktd253); + current_ratio = KTD253_MAX_RATIO; + } + /* After 1/32 we loop back to 32/32 */ if (current_ratio == KTD253_MIN_RATIO) current_ratio = KTD253_MAX_RATIO; else current_ratio--; } - local_irq_restore(flags); ktd253->ratio = current_ratio; dev_dbg(ktd253->dev, "new ratio set to %d/32\n", target_ratio);