Received-SPF: pass (google.com: domain of linux-kernel+bounces-160866-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240426235857.3870424-1-dianders@chromium.org> <20240426165839.v2.3.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid>
In-Reply-To: <20240426165839.v2.3.Id15fae80582bc74a0d4f1338987fa375738f45b9@changeid>
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date: Sat, 27 Apr 2024 05:05:14 +0300
Message-ID: <CAA8EJpon4wAT8=U5t2pyzdDWDjY6651QyDSp0hJGn9WGks6-ug@mail.gmail.com>
Subject: Re: [PATCH v2 3/8] drm/mipi-dsi: Reduce driver bloat of mipi_dsi_*_write_seq()
To: Douglas Anderson <dianders@chromium.org>
Cc: dri-devel@lists.freedesktop.org, Linus Walleij <linus.walleij@linaro.org>, 
	lvzhaoxiong@huaqin.corp-partner.google.com, 
	Jani Nikula <jani.nikula@linux.intel.com>, Hsin-Yi Wang <hsinyi@google.com>, 
	Javier Martinez Canillas <javierm@redhat.com>, Neil Armstrong <neil.armstrong@linaro.org>, 
	Joel Selvaraj <jo@jsfamily.in>, Cong Yang <yangcong5@huaqin.corp-partner.google.com>, 
	Sam Ravnborg <sam@ravnborg.org>, Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@gmail.com>, 
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, 
	Thomas Zimmermann <tzimmermann@suse.de>, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"

On Sat, 27 Apr 2024 at 02:59, Douglas Anderson <dianders@chromium.org> wrote:
>
> 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 <dianders@chromium.org>
> ---
>
> 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.
>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry