On Tue, Jul 16, 2024 at 07:01:17PM GMT, Tejas Vipin wrote:
> Introduce 2 new macros, DSI_CTX_NO_OP and MIPI_DSI_ADD_MULTI_VARIANT.
>
> DSI_CTX_NO_OP calls a function only if the context passed to it hasn't
> encountered any errors. It is a generic form of what mipi_dsi_msleep
> does.
>
> MIPI_DSI_ADD_MULTI_VARIANT defines a multi style function of any
> mipi_dsi function that follows a certain style. This allows us to
> greatly reduce the amount of redundant code written for each multi
> function. It reduces the overhead for a developer introducing new
> mipi_dsi_*_multi functions.
>
> Signed-off-by: Tejas Vipin <tejasvipi...@gmail.com>
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 286 ++++++++++-----------------------
> 1 file changed, 83 insertions(+), 203 deletions(-)
>
[...]
> -void mipi_dsi_dcs_set_tear_on_multi(struct mipi_dsi_multi_context *ctx,
> - enum mipi_dsi_dcs_tear_mode mode)
> -{
> - struct mipi_dsi_device *dsi = ctx->dsi;
> - struct device *dev = &dsi->dev;
> - ssize_t ret;
> -
> - if (ctx->accum_err)
> - return;
> -
> - ret = mipi_dsi_dcs_set_tear_on(dsi, mode);
> - if (ret < 0) {
> - ctx->accum_err = ret;
> - dev_err(dev, "sending DCS SET_TEAR_ON failed: %d\n",
> - ctx->accum_err);
> - }
> -}
> -EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_on_multi);
> +#define MIPI_DSI_ADD_MULTI_VARIANT(proto, err, inner_func, ...) \
> +proto { \
> + struct mipi_dsi_device *dsi = ctx->dsi; \
> + struct device *dev = &dsi->dev; \
> + int ret; \
> + \
> + if (ctx->accum_err) \
> + return; \
> + \
> + ret = inner_func(dsi, ##__VA_ARGS__); \
> + if (ret < 0) { \
> + ctx->accum_err = ret; \
> + dev_err(dev, err, ctx->accum_err); \
> + } \
> +} \
> +EXPORT_SYMBOL(inner_func##_multi);
> +
> +MIPI_DSI_ADD_MULTI_VARIANT(
> + void mipi_dsi_picture_parameter_set_multi(
> + struct mipi_dsi_multi_context *ctx,
> + const struct drm_dsc_picture_parameter_set *pps),
> + "sending PPS failed: %d\n",
> + mipi_dsi_picture_parameter_set, pps);
I'd say that having everything wrapped in the macro looks completely
unreadable.
If you really insist, it can become something like:
MIPI_DSI_ADD_MULTI_VARIANT(mipi_dsi_picture_parameter_set
MULTI_PROTO(const struct drm_dsc_picture_parameter_set *pps),
MULTI_ARGS(pps),
"sending PPS failed");
(note, I dropped the obvious parts: that the funciton is foo_multi, its
return type is void, first parameter is context, etc).
However it might be better to go other way arround.
Do we want for all the drivers to migrate to _multi()-kind of API? If
so, what about renaming the multi and non-multi functions accordingly
and making the old API a wrapper around the multi() function?
--
With best wishes
Dmitry
