From patchwork Fri Apr 26 23:58:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Anderson X-Patchwork-Id: 13645487 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 000F5C19F53 for ; Fri, 26 Apr 2024 23:59:34 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 15D0011AB48; Fri, 26 Apr 2024 23:59:34 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="fIhljW9G"; dkim-atps=neutral Received: from mail-pj1-f45.google.com (mail-pj1-f45.google.com [209.85.216.45]) by gabe.freedesktop.org (Postfix) with ESMTPS id 631311125BE for ; Fri, 26 Apr 2024 23:59:12 +0000 (UTC) Received: by mail-pj1-f45.google.com with SMTP id 98e67ed59e1d1-2b07f6b38daso762480a91.1 for ; Fri, 26 Apr 2024 16:59:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1714175951; x=1714780751; darn=lists.freedesktop.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=olX2nqFQ/HIiDMzF6dlzKwUQwqcWWwyQPZ/a2dqsVIU=; b=fIhljW9G/5GkjPHSeO1Q3pKSrjG7T5fCLqHudZHV4DLzYe8hihGgwhQYQe3h/B3fKo P9Dlg9Xt8NpA9gZYvwgjlFA4r5PGT4IGC41JieLlzd7QqFwMD8wBJBWCTcuoHbzg2MoL BYZCnoAgbg8pHs3afk4iqe5jyorEFSvYQTxfc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714175951; x=1714780751; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=olX2nqFQ/HIiDMzF6dlzKwUQwqcWWwyQPZ/a2dqsVIU=; b=YAucVNl83J2lgXjoB1QLfcIPcC9yAAV4oKlczeUEUvX70XGa0/4Ii8Jatw1zfSc/em YRKyhGqHOjsqKMZPAPUoRxcqDd4+yNFBywknxpmbGM2tmpINmffiCp4rTeTQ2SksclBL ZnHl23WD1zMG8/4Te+3NLSMIJDUELjV4l1uuqBEvslO+7jlguxINwpYCEyLgXbDmoEWX tw0UqBT3dZhAhjF9s824sz2ZBGZ19M8+NzlOAaIqnxXK9Fq1hvapew8YzRLDOLi921MZ Sd7gpvrw9md7F5nzwQgBgBoQy24Q/BUM3Dypo/PV7uTVeVsgqp/pAxs6Q0QpkTyhrgvp If9Q== X-Gm-Message-State: AOJu0YyBHvlVtio060Zohfe05ID/Y4KLnT23n3M52rHi9Tb8WILkkKcx k/gq2hWc4d9lgYktokbOmacvq164kj3YlPmuXq5lI+YK7NCDXuihPOHZLDDu0O79AhWDYS8qtV8 = X-Google-Smtp-Source: AGHT+IERSyX1MgdxL1cw3vNSzu1k3ve74uWuziWR/EytMBtjf7Aw/D5Xwy6rDaVwR+TQjtAuEB5fNw== X-Received: by 2002:a17:90a:d916:b0:2af:3b61:318a with SMTP id c22-20020a17090ad91600b002af3b61318amr6231601pjv.9.1714175951177; Fri, 26 Apr 2024 16:59:11 -0700 (PDT) Received: from dianders.sjc.corp.google.com ([2620:15c:9d:2:fca:3ae2:20e6:82bb]) by smtp.gmail.com with ESMTPSA id n4-20020a17090aab8400b002af41bd563esm5111939pjq.33.2024.04.26.16.59.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 26 Apr 2024 16:59:10 -0700 (PDT) From: Douglas Anderson To: dri-devel@lists.freedesktop.org Cc: Linus Walleij , lvzhaoxiong@huaqin.corp-partner.google.com, Jani Nikula , Hsin-Yi Wang , Javier Martinez Canillas , Neil Armstrong , Joel Selvaraj , Dmitry Baryshkov , Cong Yang , Sam Ravnborg , Douglas Anderson , Daniel Vetter , David Airlie , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , linux-kernel@vger.kernel.org Subject: [PATCH v2 3/8] drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq() Date: Fri, 26 Apr 2024 16:58:36 -0700 Message-ID: <20240426165839.v2.3.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid> X-Mailer: git-send-email 2.44.0.769.g3c40516874-goog In-Reply-To: <20240426235857.3870424-1-dianders@chromium.org> References: <20240426235857.3870424-1-dianders@chromium.org> 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: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Through a cooperative effort between Hsin-Yi Wang and Dmitry Baryshkov, we have realized the dev_err_ratelimited() in the mipi_dsi_*_write_seq() macros was causing quite a bit of bloat to the kernel. Let's hoist this call into drm_mipi_dsi.c by adding a "chatty" version of the functions that includes the print. While doing this, add a bit more comments to these macros making it clear that they print errors and also that they return out of _the caller's_ function. Without any changes to clients this gives a dramatic savings. Building with my build system shows one example: $ scripts/bloat-o-meter \ .../before/panel-novatek-nt36672e.ko \ .../after/panel-novatek-nt36672e.ko add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-21496 (-21496) Function old new delta nt36672e_1080x2408_60hz_init._rs 10320 - -10320 nt36672e_1080x2408_60hz_init 17412 6236 -11176 Total: Before=32147, After=10651, chg -66.87% Note that given the change in location of the print it's harder to include the "cmd" in the printout for mipi_dsi_dcs_write_seq() since, theoretically, someone could call the new chatty function with a zero-size array and it would be illegal to dereference data[0]. There's a printk format to print the whole buffer and this is probably more useful for debugging anyway. Given that we're doing this for mipi_dsi_dcs_write_seq(), let's also print the buffer for mipi_dsi_generic_write_seq() in the error case. It should be noted that the current consensus of DRM folks is that the mipi_dsi_*_write_seq() should be deprecated due to the non-intuitive return behavior. A future patch will formally mark them as deprecated and provide an alternative. Signed-off-by: Douglas Anderson Reviewed-by: Dmitry Baryshkov --- Changes in v2: - Add some comments to the macros about printing and returning. - Modify commit message now that this is part of a series. - Change the way err value is handled in prep for next patch. - Rebased upon patches to avoid theoretical int overflow. drivers/gpu/drm/drm_mipi_dsi.c | 56 ++++++++++++++++++++++++++++++++++ include/drm/drm_mipi_dsi.h | 41 +++++++++++++------------ 2 files changed, 78 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c index 795001bb7ff1..1e062eda3b88 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -764,6 +764,34 @@ ssize_t mipi_dsi_generic_write(struct mipi_dsi_device *dsi, const void *payload, } EXPORT_SYMBOL(mipi_dsi_generic_write); +/** + * mipi_dsi_generic_write_chatty() - mipi_dsi_generic_write() w/ an error log + * @dsi: DSI peripheral device + * @payload: buffer containing the payload + * @size: size of payload buffer + * + * Like mipi_dsi_generic_write() but includes a dev_err_ratelimited() + * call for you and returns 0 upon success, not the number of bytes sent. + * + * Return: 0 on success or a negative error code on failure. + */ +int mipi_dsi_generic_write_chatty(struct mipi_dsi_device *dsi, + const void *payload, size_t size) +{ + struct device *dev = &dsi->dev; + ssize_t ret; + + ret = mipi_dsi_generic_write(dsi, payload, size); + if (ret < 0) { + dev_err_ratelimited(dev, "sending generic data %*ph failed: %d\n", + (int)size, payload, (int)ret); + return ret; + } + + return 0; +} +EXPORT_SYMBOL(mipi_dsi_generic_write_chatty); + /** * mipi_dsi_generic_read() - receive data using a generic read packet * @dsi: DSI peripheral device @@ -852,6 +880,34 @@ ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device *dsi, } EXPORT_SYMBOL(mipi_dsi_dcs_write_buffer); +/** + * mipi_dsi_dcs_write_buffer_chatty - mipi_dsi_dcs_write_buffer() w/ an error log + * @dsi: DSI peripheral device + * @data: buffer containing data to be transmitted + * @len: size of transmission buffer + * + * Like mipi_dsi_dcs_write_buffer() but includes a dev_err_ratelimited() + * call for you and returns 0 upon success, not the number of bytes sent. + * + * Return: 0 on success or a negative error code on failure. + */ +int mipi_dsi_dcs_write_buffer_chatty(struct mipi_dsi_device *dsi, + const void *data, size_t len) +{ + struct device *dev = &dsi->dev; + ssize_t ret; + + ret = mipi_dsi_dcs_write_buffer(dsi, data, len); + if (ret < 0) { + dev_err_ratelimited(dev, "sending dcs data %*ph failed: %d\n", + (int)len, data, (int)ret); + return ret; + } + + return 0; +} +EXPORT_SYMBOL(mipi_dsi_dcs_write_buffer_chatty); + /** * mipi_dsi_dcs_write() - send DCS write command * @dsi: DSI peripheral device diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h index 5de2bd62448b..6d68d9927f46 100644 --- a/include/drm/drm_mipi_dsi.h +++ b/include/drm/drm_mipi_dsi.h @@ -256,6 +256,8 @@ int mipi_dsi_picture_parameter_set(struct mipi_dsi_device *dsi, ssize_t mipi_dsi_generic_write(struct mipi_dsi_device *dsi, const void *payload, size_t size); +int mipi_dsi_generic_write_chatty(struct mipi_dsi_device *dsi, + const void *payload, size_t size); ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi, const void *params, size_t num_params, void *data, size_t size); @@ -279,6 +281,8 @@ enum mipi_dsi_dcs_tear_mode { ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device *dsi, const void *data, size_t len); +int mipi_dsi_dcs_write_buffer_chatty(struct mipi_dsi_device *dsi, + const void *data, size_t len); ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd, const void *data, size_t len); ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data, @@ -311,40 +315,39 @@ int mipi_dsi_dcs_get_display_brightness_large(struct mipi_dsi_device *dsi, /** * mipi_dsi_generic_write_seq - transmit data using a generic write packet + * + * This macro will print errors for you and will RETURN FROM THE CALLING + * FUNCTION (yes this is non-intuitive) upon error. + * * @dsi: DSI peripheral device * @seq: buffer containing the payload */ #define mipi_dsi_generic_write_seq(dsi, seq...) \ do { \ static const u8 d[] = { seq }; \ - struct device *dev = &dsi->dev; \ - ssize_t ret; \ - ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d)); \ - if (ret < 0) { \ - dev_err_ratelimited(dev, "transmit data failed: %d\n", \ - (int)ret); \ + int ret; \ + ret = mipi_dsi_generic_write_chatty(dsi, d, ARRAY_SIZE(d)); \ + if (ret < 0) \ return ret; \ - } \ } while (0) /** * mipi_dsi_dcs_write_seq - transmit a DCS command with payload + * + * This macro will print errors for you and will RETURN FROM THE CALLING + * FUNCTION (yes this is non-intuitive) upon error. + * * @dsi: DSI peripheral device * @cmd: Command * @seq: buffer containing data to be transmitted */ -#define mipi_dsi_dcs_write_seq(dsi, cmd, seq...) \ - do { \ - static const u8 d[] = { cmd, seq }; \ - struct device *dev = &dsi->dev; \ - ssize_t ret; \ - ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \ - if (ret < 0) { \ - dev_err_ratelimited( \ - dev, "sending command %#02x failed: %d\n", \ - cmd, (int)ret); \ - return ret; \ - } \ +#define mipi_dsi_dcs_write_seq(dsi, cmd, seq...) \ + do { \ + static const u8 d[] = { cmd, seq }; \ + int ret; \ + ret = mipi_dsi_dcs_write_buffer_chatty(dsi, d, ARRAY_SIZE(d)); \ + if (ret < 0) \ + return ret; \ } while (0) /**