| Message ID | 20240716133117.483514-1-tejasvipin76@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/mipi-dsi: Introduce macros to create mipi_dsi_*_multi functions | expand |
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@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?
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@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? > 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?
On Wed, 17 Jul 2024 at 12:58, Tejas Vipin <tejasvipin76@gmail.com> wrote: > > > > 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@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? > > > > 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. Sounds good to me. I'd suggest to wait for a day or two for further feedback / comments from other developers. > > I also think there is merit in keeping DSI_CTX_NO_OP. > > What do you think? > > -- > Tejas Vipin
Hi, On Wed, Jul 17, 2024 at 3:07 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > > > 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. > > Sounds good to me. I'd suggest to wait for a day or two for further > feedback / comments from other developers. I don't totally hate the idea of going full-multi and just having macros to wrap anyone who hasn't converted, but I'd be curious how much driver bloat this will cause for drivers that aren't converted. Would the compiler do a good job optimizing here? Maybe we don't care if we just want everyone to switch over? If nothing else, it might make sense to at least keep both versions for the very generic functions (mipi_dsi_generic_write_multi and mipi_dsi_dcs_write_buffer_multi) ...oh, but wait. We probably have the non-multi versions wrap the _multi ones. One of the things about the _multi functions is that they are also "chatty" and print errors. They're designed for the use case of a pile of init code where any error is fatal and needs to be printed. I suspect that somewhere along the way someone will want to be able to call these functions and not have them print errors... Maybe going with Dmitry's suggested syntax is the best option here? Depending on how others feel, we could potentially even get rid of the special error message and just stringify the function name for the error message? -Doug
On 7/19/24 10:29 PM, Doug Anderson wrote: > Hi, > > On Wed, Jul 17, 2024 at 3:07 AM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: >> >>>> 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. >> >> Sounds good to me. I'd suggest to wait for a day or two for further >> feedback / comments from other developers. > > I don't totally hate the idea of going full-multi and just having > macros to wrap anyone who hasn't converted, but I'd be curious how > much driver bloat this will cause for drivers that aren't converted. > Would the compiler do a good job optimizing here? Maybe we don't care > if we just want everyone to switch over? If nothing else, it might > make sense to at least keep both versions for the very generic > functions (mipi_dsi_generic_write_multi and > mipi_dsi_dcs_write_buffer_multi) > > ...oh, but wait. We probably have the non-multi versions wrap the > _multi ones. One of the things about the _multi functions is that they > are also "chatty" and print errors. They're designed for the use case > of a pile of init code where any error is fatal and needs to be > printed. I suspect that somewhere along the way someone will want to > be able to call these functions and not have them print errors... > I think what would be interesting is if we had "chatty" member as a part of mipi_dsi_multi_context as a check for if dev_err should run. That way, we could make any function not chatty. If we did this, is there any reason for a driver to not switch over to using the multi functions? > Maybe going with Dmitry's suggested syntax is the best option here? > Depending on how others feel, we could potentially even get rid of the > special error message and just stringify the function name for the > error message? > The problem with any macro solution that defines new multi functions is that it creates a lot of bloat. Defining the function through macros might be smaller than defining them manually, but there are still twice as many function declarations as there would be if we went all multi. The generated code is still just as big as if we manually defined everything. I think a macro that defines functions should be more of a last resort if we don't have any other options. > -Doug
Hi, On Fri, Jul 19, 2024 at 10:19 PM Tejas Vipin <tejasvipin76@gmail.com> wrote: > > On 7/19/24 10:29 PM, Doug Anderson wrote: > > Hi, > > > > On Wed, Jul 17, 2024 at 3:07 AM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > >> > >>>> 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. > >> > >> Sounds good to me. I'd suggest to wait for a day or two for further > >> feedback / comments from other developers. > > > > I don't totally hate the idea of going full-multi and just having > > macros to wrap anyone who hasn't converted, but I'd be curious how > > much driver bloat this will cause for drivers that aren't converted. > > Would the compiler do a good job optimizing here? Maybe we don't care > > if we just want everyone to switch over? If nothing else, it might > > make sense to at least keep both versions for the very generic > > functions (mipi_dsi_generic_write_multi and > > mipi_dsi_dcs_write_buffer_multi) > > > > ...oh, but wait. We probably have the non-multi versions wrap the > > _multi ones. One of the things about the _multi functions is that they > > are also "chatty" and print errors. They're designed for the use case > > of a pile of init code where any error is fatal and needs to be > > printed. I suspect that somewhere along the way someone will want to > > be able to call these functions and not have them print errors... > > > > I think what would be interesting is if we had "chatty" member as a > part of mipi_dsi_multi_context as a check for if dev_err should run. > That way, we could make any function not chatty. If we did this, is > there any reason for a driver to not switch over to using the multi > functions? I'd be OK with that. My preference would be that "chatty" would be the zero-initialized value just to make that slightly more efficient and preserve existing behavior. So I guess the member would be something like "quiet" and when 0 (false) it wouldn't be quiet. > > Maybe going with Dmitry's suggested syntax is the best option here? > > Depending on how others feel, we could potentially even get rid of the > > special error message and just stringify the function name for the > > error message? > > > The problem with any macro solution that defines new multi functions is > that it creates a lot of bloat. Defining the function through macros > might be smaller than defining them manually, but there are still twice > as many function declarations as there would be if we went all multi. > The generated code is still just as big as if we manually defined > everything. I think a macro that defines functions should be more of a > last resort if we don't have any other options. Ah, somehow I was thinking that Dmitry's solution was conflated with switching back to a macro. I haven't prototyped it, but I thought Dmitry's primary complaint was that the syntax for declaring the "_multi" function was a bit dodgy and I'd expect that using a macro would solve that. In any case, I'm good with keeping away from macros / bloating things. -Doug
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c index a471c46f5ca6..53880b486f22 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -1430,214 +1430,94 @@ int mipi_dsi_dcs_get_display_brightness_large(struct mipi_dsi_device *dsi, EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness_large); /** - * mipi_dsi_picture_parameter_set_multi() - transmit the DSC PPS to the peripheral - * @ctx: Context for multiple DSI transactions - * @pps: VESA DSC 1.1 Picture Parameter Set - * - * Like mipi_dsi_picture_parameter_set() but deals with errors in a way that - * makes it convenient to make several calls in a row. - */ -void mipi_dsi_picture_parameter_set_multi(struct mipi_dsi_multi_context *ctx, - const struct drm_dsc_picture_parameter_set *pps) -{ - struct mipi_dsi_device *dsi = ctx->dsi; - struct device *dev = &dsi->dev; - ssize_t ret; - - if (ctx->accum_err) - return; - - ret = mipi_dsi_picture_parameter_set(dsi, pps); - if (ret < 0) { - ctx->accum_err = ret; - dev_err(dev, "sending PPS failed: %d\n", - ctx->accum_err); - } -} -EXPORT_SYMBOL(mipi_dsi_picture_parameter_set_multi); - -/** - * mipi_dsi_compression_mode_ext_multi() - enable/disable DSC on the peripheral - * @ctx: Context for multiple DSI transactions - * @enable: Whether to enable or disable the DSC - * @algo: Selected compression algorithm - * @pps_selector: Select PPS from the table of pre-stored or uploaded PPS entries - * - * Like mipi_dsi_compression_mode_ext() but deals with errors in a way that - * makes it convenient to make several calls in a row. - */ -void mipi_dsi_compression_mode_ext_multi(struct mipi_dsi_multi_context *ctx, - bool enable, - enum mipi_dsi_compression_algo algo, - unsigned int pps_selector) -{ - struct mipi_dsi_device *dsi = ctx->dsi; - struct device *dev = &dsi->dev; - ssize_t ret; - - if (ctx->accum_err) - return; - - ret = mipi_dsi_compression_mode_ext(dsi, enable, algo, pps_selector); - if (ret < 0) { - ctx->accum_err = ret; - dev_err(dev, "sending COMPRESSION_MODE failed: %d\n", - ctx->accum_err); - } -} -EXPORT_SYMBOL(mipi_dsi_compression_mode_ext_multi); - -/** - * mipi_dsi_dcs_nop_multi() - send DCS NOP packet - * @ctx: Context for multiple DSI transactions - * - * Like mipi_dsi_dcs_nop() but deals with errors in a way that - * makes it convenient to make several calls in a row. + * DSI_CTX_NO_OP() - Calls a function only if no previous errors have + * occurred. + * @func: The function call that needs to happen. + * @ctx: Context whose accum_err is checked to decide if the function + * should run. */ -void mipi_dsi_dcs_nop_multi(struct mipi_dsi_multi_context *ctx) -{ - struct mipi_dsi_device *dsi = ctx->dsi; - struct device *dev = &dsi->dev; - ssize_t ret; - - if (ctx->accum_err) - return; - - ret = mipi_dsi_dcs_nop(dsi); - if (ret < 0) { - ctx->accum_err = ret; - dev_err(dev, "sending DCS NOP failed: %d\n", - ctx->accum_err); - } -} -EXPORT_SYMBOL(mipi_dsi_dcs_nop_multi); +#define DSI_CTX_NO_OP(func, ctx) \ + do { \ + if (!(ctx)->accum_err) \ + func; \ + } while (0) /** - * mipi_dsi_dcs_enter_sleep_mode_multi() - send DCS ENTER_SLEEP_MODE packet - * @ctx: Context for multiple DSI transactions + * MIPI_DSI_ADD_MULTI_VARIANT() - Define the multi variant of + * an existing MIPI DSI function. + * @proto: The prototype of the desired multi variant + * @err: The error string used by dev_err on an error occurring. + * @inner_func: Identifier of the function being wrapped + * @...: Any arguments that need to be passed to inner_func * - * Like mipi_dsi_dcs_enter_sleep_mode() but deals with errors in a way that - * makes it convenient to make several calls in a row. */ -void mipi_dsi_dcs_enter_sleep_mode_multi(struct mipi_dsi_multi_context *ctx) -{ - struct mipi_dsi_device *dsi = ctx->dsi; - struct device *dev = &dsi->dev; - ssize_t ret; - - if (ctx->accum_err) - return; - - ret = mipi_dsi_dcs_enter_sleep_mode(dsi); - if (ret < 0) { - ctx->accum_err = ret; - dev_err(dev, "sending DCS ENTER_SLEEP_MODE failed: %d\n", - ctx->accum_err); - } -} -EXPORT_SYMBOL(mipi_dsi_dcs_enter_sleep_mode_multi); - -/** - * mipi_dsi_dcs_exit_sleep_mode_multi() - send DCS EXIT_SLEEP_MODE packet - * @ctx: Context for multiple DSI transactions - * - * Like mipi_dsi_dcs_exit_sleep_mode() but deals with errors in a way that - * makes it convenient to make several calls in a row. - */ -void mipi_dsi_dcs_exit_sleep_mode_multi(struct mipi_dsi_multi_context *ctx) -{ - struct mipi_dsi_device *dsi = ctx->dsi; - struct device *dev = &dsi->dev; - ssize_t ret; - - if (ctx->accum_err) - return; - - ret = mipi_dsi_dcs_exit_sleep_mode(dsi); - if (ret < 0) { - ctx->accum_err = ret; - dev_err(dev, "sending DCS EXIT_SLEEP_MODE failed: %d\n", - ctx->accum_err); - } -} -EXPORT_SYMBOL(mipi_dsi_dcs_exit_sleep_mode_multi); - -/** - * mipi_dsi_dcs_set_display_off_multi() - send DCS SET_DISPLAY_OFF packet - * @ctx: Context for multiple DSI transactions - * - * Like mipi_dsi_dcs_set_display_off() but deals with errors in a way that - * makes it convenient to make several calls in a row. - */ -void mipi_dsi_dcs_set_display_off_multi(struct mipi_dsi_multi_context *ctx) -{ - 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_display_off(dsi); - if (ret < 0) { - ctx->accum_err = ret; - dev_err(dev, "sending DCS SET_DISPLAY_OFF failed: %d\n", - ctx->accum_err); - } -} -EXPORT_SYMBOL(mipi_dsi_dcs_set_display_off_multi); - -/** - * mipi_dsi_dcs_set_display_on_multi() - send DCS SET_DISPLAY_ON packet - * @ctx: Context for multiple DSI transactions - * - * Like mipi_dsi_dcs_set_display_on() but deals with errors in a way that - * makes it convenient to make several calls in a row. - */ -void mipi_dsi_dcs_set_display_on_multi(struct mipi_dsi_multi_context *ctx) -{ - 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_display_on(dsi); - if (ret < 0) { - ctx->accum_err = ret; - dev_err(dev, "sending DCS SET_DISPLAY_ON failed: %d\n", - ctx->accum_err); - } -} -EXPORT_SYMBOL(mipi_dsi_dcs_set_display_on_multi); - -/** - * mipi_dsi_dcs_set_tear_on_multi() - send DCS SET_TEAR_ON packet - * @ctx: Context for multiple DSI transactions - * @mode: the Tearing Effect Output Line mode - * - * Like mipi_dsi_dcs_set_tear_on() but deals with errors in a way that - * makes it convenient to make several calls in a row. - */ -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); + +MIPI_DSI_ADD_MULTI_VARIANT( + void mipi_dsi_compression_mode_ext_multi( + struct mipi_dsi_multi_context *ctx, bool enable, + enum mipi_dsi_compression_algo algo, unsigned int pps_selector), + "sending COMPRESSION_MODE failed: %d\n", + mipi_dsi_compression_mode_ext, enable, algo, pps_selector); + +MIPI_DSI_ADD_MULTI_VARIANT( + void mipi_dsi_dcs_nop_multi( + struct mipi_dsi_multi_context *ctx), + "sending DCS NOP failed: %d\n", + mipi_dsi_dcs_nop); + +MIPI_DSI_ADD_MULTI_VARIANT( + void mipi_dsi_dcs_enter_sleep_mode_multi( + struct mipi_dsi_multi_context *ctx), + "sending DCS ENTER_SLEEP_MODE failed: %d\n", + mipi_dsi_dcs_enter_sleep_mode); + +MIPI_DSI_ADD_MULTI_VARIANT( + void mipi_dsi_dcs_exit_sleep_mode_multi( + struct mipi_dsi_multi_context *ctx), + "sending DCS EXIT_SLEEP_MODE failed: %d\n", + mipi_dsi_dcs_exit_sleep_mode); + +MIPI_DSI_ADD_MULTI_VARIANT( + void mipi_dsi_dcs_set_display_off_multi( + struct mipi_dsi_multi_context *ctx), + "sending DCS SET_DISPLAY_OFF failed: %d\n", + mipi_dsi_dcs_set_display_off); + +MIPI_DSI_ADD_MULTI_VARIANT( + void mipi_dsi_dcs_set_display_on_multi( + struct mipi_dsi_multi_context *ctx), + "sending DCS SET_DISPLAY_ON failed: %d\n", + mipi_dsi_dcs_set_display_on); + +MIPI_DSI_ADD_MULTI_VARIANT( + void mipi_dsi_dcs_set_tear_on_multi( + struct mipi_dsi_multi_context *ctx, + enum mipi_dsi_dcs_tear_mode mode), + "sending DCS SET_TEAR_ON failed: %d\n", + mipi_dsi_dcs_set_tear_on, mode); static int mipi_dsi_drv_probe(struct device *dev) {
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@gmail.com> --- drivers/gpu/drm/drm_mipi_dsi.c | 286 ++++++++++----------------------- 1 file changed, 83 insertions(+), 203 deletions(-)