From patchwork Tue May 14 17:20:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Anderson X-Patchwork-Id: 13664292 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 1F567C25B79 for ; Tue, 14 May 2024 17:22:07 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D24EC10E345; Tue, 14 May 2024 17:22:04 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="JjSOUUhC"; dkim-atps=neutral Received: from mail-pl1-f171.google.com (mail-pl1-f171.google.com [209.85.214.171]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0086D10E345 for ; Tue, 14 May 2024 17:22:02 +0000 (UTC) Received: by mail-pl1-f171.google.com with SMTP id d9443c01a7336-1ec486198b6so44827085ad.1 for ; Tue, 14 May 2024 10:22:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1715707322; x=1716312122; 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=IKR874ePRAyu7x/9s6Ii0LVvINBFCzhhReL02qJjlmI=; b=JjSOUUhCYPKvJSA4hT4x7f9HQhbu1K+KiDBKsDJlnRS7C3NR0RDBUO39BzC6tmxMxB NHHEzonOmku/0xJvp+071N1XuqL+JRZZrfKqZ12PB7PwIiXAchIZbbbcEP9/fKeO3GWH UNwVYo8XmdVlOpyKfhhUwPEbSZXFZ5z9caUNQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715707322; x=1716312122; 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=IKR874ePRAyu7x/9s6Ii0LVvINBFCzhhReL02qJjlmI=; b=lqsWd2M2oBI3LrqGqSki2pP5YjVkqBI+H2JQH9bLxRx0sGZ4A8JvDLLAeXw7WtznwU 6tHSVAlQFmGavdeuaGakgpcteqCLGfjo4rvirsUb12jSWs2LQE9bjvRqR/VRocNbqG0D EPjaqiRyALfj/2bswmEUqpLskbgjyyoghvbU1KsEDlJJceG/BhscVwk8LCXGB4mFJi7h d+gZoUeRx6chjsav3hNEc+kFzQMOP2oK1GDZUNjokWtUad0pO+8VvoIvexCMGc4BjXvc ZDHIMs8U9n0xHF1iFX9WEtIcWGk2Je2XL7sf91DaPQmKJx9cDCN5QlN3GC9bWsdz0enK EGbw== X-Gm-Message-State: AOJu0Yxxfl3U7X810cqlg4iIdsyCdUXLFwi4F88dqI8Bkd2PwQ4r5V9x OPalXe+qQv7bRSZvBCmX6o39NPNYCuIQa20FTto8u/7bde5YtFerTNn9epWWLjbvWjl0vEAms6E = X-Google-Smtp-Source: AGHT+IG08x+GfLQSsNNcgVmwrHZwZlQ4i+oR67/nqB6S90T6BeMOwMgalLiI9LR0SCKqKJJp5I8unQ== X-Received: by 2002:a17:902:eb91:b0:1e4:60d4:916b with SMTP id d9443c01a7336-1ef441a7ecdmr149994355ad.64.1715707321665; Tue, 14 May 2024 10:22:01 -0700 (PDT) Received: from dianders.sjc.corp.google.com ([2620:15c:9d:2:ef10:6fdf:5041:421f]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1ef0bf3101csm100147575ad.121.2024.05.14.10.21.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 May 2024 10:22:01 -0700 (PDT) From: Douglas Anderson To: dri-devel@lists.freedesktop.org Cc: Javier Martinez Canillas , lvzhaoxiong@huaqin.corp-partner.google.com, Jani Nikula , Neil Armstrong , Brian Norris , Sam Ravnborg , Dmitry Baryshkov , Maxime Ripard , Joel Selvaraj , Hsin-Yi Wang , Cong Yang , Linus Walleij , Douglas Anderson , Daniel Vetter , David Airlie , Maarten Lankhorst , Thomas Zimmermann , linux-kernel@vger.kernel.org Subject: [PATCH v5 4/9] drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq() Date: Tue, 14 May 2024 10:20:54 -0700 Message-ID: <20240514102056.v5.4.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid> X-Mailer: git-send-email 2.45.0.rc1.225.g2a3ae87e7f-goog In-Reply-To: <20240514172136.1578498-1-dianders@chromium.org> References: <20240514172136.1578498-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() 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 nice savings. Specifically the macro was inlined and thus the error report call was inlined into every call to mipi_dsi_dcs_write_seq() and mipi_dsi_generic_write_seq(). By using a call to a "chatty" function, the usage is reduced to one call in the chatty function and a function call at the invoking site. 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/0 grow/shrink: 0/1 up/down: 0/-4404 (-4404) Function old new delta nt36672e_1080x2408_60hz_init 10640 6236 -4404 Total: Before=15055, After=10651, chg -29.25% 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. Reviewed-by: Dmitry Baryshkov Reviewed-by: Neil Armstrong Reviewed-by: Linus Walleij Signed-off-by: Douglas Anderson --- Changes in v5: - Fix comment dev_err_ratelimited() => dev_err(). Changes in v4: - Update wording as per Linus W. Changes in v3: - Rebased upon patch to remove ratelimit of prints. Changes in v2: - Add some comments to the macros about printing and returning. - Change the way err value is handled in prep for next patch. - Modify commit message now that this is part of a series. - Rebased upon patches to avoid theoretical int overflow. drivers/gpu/drm/drm_mipi_dsi.c | 56 ++++++++++++++++++++++++++++++++++ include/drm/drm_mipi_dsi.h | 47 +++++++++++++++------------- 2 files changed, 82 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c index 795001bb7ff1..4d2685d5a6e0 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() + * 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(dev, "sending generic data %*ph failed: %zd\n", + (int)size, payload, 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() + * 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(dev, "sending dcs data %*ph failed: %zd\n", + (int)len, data, 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 67967be48dbd..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,38 +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(dev, "transmit data failed: %zd\n", ret); \ - return ret; \ - } \ +#define mipi_dsi_generic_write_seq(dsi, seq...) \ + do { \ + static const u8 d[] = { seq }; \ + 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(dev, "sending command %#02x failed: %zd\n", \ - cmd, 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) /**