On 7/16/24 10:31 PM, Dmitry Baryshkov wrote:
> 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 <tejasvipin76@xxxxxxxxx>
>> ---
>> 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?
>
I think this would be good. For the wrapper to make a multi() function
non-multi, what do you think about a macro that would just pass a
default dsi_ctx to the multi() func and return on error? In this case
it would also be good to let the code fill inline instead of generating
a whole new function imo.
So in this scenario all the mipi dsi functions would be multi functions,
and a function could be called non-multi like MACRO_NAME(func) at the
call site.
I also think there is merit in keeping DSI_CTX_NO_OP.
What do you think?
--
Tejas Vipin
