[PATCH v2 0/2] add more multi functions to streamline error handling

[PATCH v2 0/2] add more multi functions to streamline error handling

[Tejas Vipin]

This series adds more multi style functions and uses them in the 
startek-kd070fhfid015 panel. Additionally it marks the older functions
as deprecated.
---
Changes in v2:
    - Improved formatting
    - Rewrote hex as lowercase
    - Marked old functions as deprecated
    - Added more functions to transition
---
Tejas Vipin (2):
  drm/mipi-dsi: add more multi functions for better error handling
  drm/panel: startek-kd070fhfid015: transition to mipi_dsi wrapped
    functions

 drivers/gpu/drm/drm_mipi_dsi.c                | 226 ++++++++++++++++++
 .../drm/panel/panel-startek-kd070fhfid015.c   | 123 +++-------
 include/drm/drm_mipi_dsi.h                    |  12 +
 3 files changed, 277 insertions(+), 84 deletions(-)

-- 
2.45.2

[PATCH v2 1/2] drm/mipi-dsi: add more multi functions for better error handling

[Tejas Vipin]

Add more functions that can benefit from being multi style and mark
older variants as deprecated to eventually convert all mipi_dsi functions
to multi style.

Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
---
 drivers/gpu/drm/drm_mipi_dsi.c | 226 +++++++++++++++++++++++++++++++++
 include/drm/drm_mipi_dsi.h     |  12 ++
 2 files changed, 238 insertions(+)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index a471c46f5ca6..05ea7df5dec1 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -603,6 +603,8 @@ EXPORT_SYMBOL(mipi_dsi_shutdown_peripheral);
  * mipi_dsi_turn_on_peripheral() - sends a Turn On Peripheral command
  * @dsi: DSI peripheral device
  *
+ * This function is deprecated. Use mipi_dsi_turn_on_peripheral_multi() instead.
+ *
  * Return: 0 on success or a negative error code on failure.
  */
 int mipi_dsi_turn_on_peripheral(struct mipi_dsi_device *dsi)
@@ -652,6 +654,7 @@ EXPORT_SYMBOL(mipi_dsi_set_maximum_return_packet_size);
  * @pps_selector: Select PPS from the table of pre-stored or uploaded PPS entries
  *
  * Enable or disable Display Stream Compression on the peripheral.
+ * This function is deprecated. Use mipi_dsi_compression_mode_ext_multi() instead.
  *
  * Return: 0 on success or a negative error code on failure.
  */
@@ -703,6 +706,7 @@ EXPORT_SYMBOL(mipi_dsi_compression_mode);
  * @pps: VESA DSC 1.1 Picture Parameter Set
  *
  * Transmit the VESA DSC 1.1 Picture Parameter Set to the peripheral.
+ * This function is deprecated. Use mipi_dsi_picture_parameter_set_multi() instead.
  *
  * Return: 0 on success or a negative error code on failure.
  */
@@ -1037,6 +1041,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_read);
  * mipi_dsi_dcs_nop() - send DCS nop packet
  * @dsi: DSI peripheral device
  *
+ * This function is deprecated. Use mipi_dsi_dcs_nop_multi() instead.
+ *
  * Return: 0 on success or a negative error code on failure.
  */
 int mipi_dsi_dcs_nop(struct mipi_dsi_device *dsi)
@@ -1055,6 +1061,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_nop);
  * mipi_dsi_dcs_soft_reset() - perform a software reset of the display module
  * @dsi: DSI peripheral device
  *
+ * This function is deprecated. Use mipi_dsi_dcs_soft_reset_multi() instead.
+ *
  * Return: 0 on success or a negative error code on failure.
  */
 int mipi_dsi_dcs_soft_reset(struct mipi_dsi_device *dsi)
@@ -1124,6 +1132,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_get_pixel_format);
  *    display module except interface communication
  * @dsi: DSI peripheral device
  *
+ * This function is deprecated. Use mipi_dsi_dcs_enter_sleep_mode_multi() instead.
+ *
  * Return: 0 on success or a negative error code on failure.
  */
 int mipi_dsi_dcs_enter_sleep_mode(struct mipi_dsi_device *dsi)
@@ -1143,6 +1153,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_enter_sleep_mode);
  *    module
  * @dsi: DSI peripheral device
  *
+ * This function is deprecated. Use mipi_dsi_dcs_exit_sleep_mode_multi() instead.
+ *
  * Return: 0 on success or a negative error code on failure.
  */
 int mipi_dsi_dcs_exit_sleep_mode(struct mipi_dsi_device *dsi)
@@ -1162,6 +1174,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_exit_sleep_mode);
  *    display device
  * @dsi: DSI peripheral device
  *
+ * This function is deprecated. Use mipi_dsi_dcs_set_display_off_multi() instead.
+ *
  * Return: 0 on success or a negative error code on failure.
  */
 int mipi_dsi_dcs_set_display_off(struct mipi_dsi_device *dsi)
@@ -1181,6 +1195,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_display_off);
  *    display device
  * @dsi: DSI peripheral device
  *
+ * This function is deprecated. Use mipi_dsi_dcs_set_display_on_multi() instead.
+ *
  * Return: 0 on success or a negative error code on failure
  */
 int mipi_dsi_dcs_set_display_on(struct mipi_dsi_device *dsi)
@@ -1202,6 +1218,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_display_on);
  * @start: first column of frame memory
  * @end: last column of frame memory
  *
+ * This function is deprecated. Use mipi_dsi_dcs_set_column_address_multi()
+ * instead.
+ *
  * Return: 0 on success or a negative error code on failure.
  */
 int mipi_dsi_dcs_set_column_address(struct mipi_dsi_device *dsi, u16 start,
@@ -1226,6 +1245,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_column_address);
  * @start: first page of frame memory
  * @end: last page of frame memory
  *
+ * This function is deprecated. Use mipi_dsi_dcs_set_page_address_multi()
+ * instead.
+ *
  * Return: 0 on success or a negative error code on failure.
  */
 int mipi_dsi_dcs_set_page_address(struct mipi_dsi_device *dsi, u16 start,
@@ -1268,6 +1290,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_off);
  * @dsi: DSI peripheral device
  * @mode: the Tearing Effect Output Line mode
  *
+ * This function is deprecated. Use mipi_dsi_dcs_set_tear_on_multi() instead.
+ *
  * Return: 0 on success or a negative error code on failure
  */
 int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi,
@@ -1291,6 +1315,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_on);
  * @dsi: DSI peripheral device
  * @format: pixel format
  *
+ * This function is deprecated. Use mipi_dsi_dcs_set_pixel_format_multi()
+ * instead.
+ *
  * Return: 0 on success or a negative error code on failure.
  */
 int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format)
@@ -1334,6 +1361,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_scanline);
  * @dsi: DSI peripheral device
  * @brightness: brightness value
  *
+ * This function is deprecated. Use mipi_dsi_dcs_set_display_brightness_multi()
+ * instead.
+ *
  * Return: 0 on success or a negative error code on failure.
  */
 int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi,
@@ -1357,6 +1387,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness);
  * @dsi: DSI peripheral device
  * @brightness: brightness value
  *
+ * This function is deprecated. Use mipi_dsi_dcs_get_display_brightness_multi()
+ * instead.
+ *
  * Return: 0 on success or a negative error code on failure.
  */
 int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi,
@@ -1639,6 +1672,199 @@ void mipi_dsi_dcs_set_tear_on_multi(struct mipi_dsi_multi_context *ctx,
 }
 EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_on_multi);
 
+/**
+ * mipi_dsi_turn_on_peripheral_multi() - sends a Turn On Peripheral command
+ * @ctx: Context for multiple DSI transactions
+ *
+ * Like mipi_dsi_turn_on_peripheral() but deals with errors in a way that
+ * makes it convenient to make several calls in a row.
+ */
+void mipi_dsi_turn_on_peripheral_multi(struct mipi_dsi_multi_context *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	if (ctx->accum_err)
+		return;
+
+	ret = mipi_dsi_turn_on_peripheral(dsi);
+	if (ret < 0) {
+		ctx->accum_err = ret;
+		dev_err(dev, "Failed to turn on peripheral: %d\n",
+			ctx->accum_err);
+	}
+}
+EXPORT_SYMBOL(mipi_dsi_turn_on_peripheral_multi);
+
+/**
+ * mipi_dsi_dcs_soft_reset_multi() - perform a software reset of the display module
+ * @ctx: Context for multiple DSI transactions
+ *
+ * Like mipi_dsi_dcs_soft_reset() but deals with errors in a way that
+ * makes it convenient to make several calls in a row.
+ */
+void mipi_dsi_dcs_soft_reset_multi(struct mipi_dsi_multi_context *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	if (ctx->accum_err)
+		return;
+
+	ret = mipi_dsi_dcs_soft_reset(dsi);
+	if (ret < 0) {
+		ctx->accum_err = ret;
+		dev_err(dev, "Failed to mipi_dsi_dcs_soft_reset: %d\n",
+			ctx->accum_err);
+	}
+}
+EXPORT_SYMBOL(mipi_dsi_dcs_soft_reset_multi);
+
+/**
+ * mipi_dsi_dcs_set_display_brightness_multi() - sets the brightness value of
+ *	the display
+ * @ctx: Context for multiple DSI transactions
+ * @brightness: brightness value
+ *
+ * Like mipi_dsi_dcs_set_display_brightness() but deals with errors in a way that
+ * makes it convenient to make several calls in a row.
+ */
+void mipi_dsi_dcs_set_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
+					       u16 brightness)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	if (ctx->accum_err)
+		return;
+
+	ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
+	if (ret < 0) {
+		ctx->accum_err = ret;
+		dev_err(dev, "Failed to write display brightness: %d\n",
+			ctx->accum_err);
+	}
+}
+EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness_multi);
+
+/**
+ * mipi_dsi_dcs_set_pixel_format_multi() - sets the pixel format for the RGB image
+ *	data used by the interface
+ * @ctx: Context for multiple DSI transactions
+ * @format: pixel format
+ *
+ * Like mipi_dsi_dcs_set_pixel_format() but deals with errors in a way that
+ * makes it convenient to make several calls in a row.
+ */
+void mipi_dsi_dcs_set_pixel_format_multi(struct mipi_dsi_multi_context *ctx,
+					 u8 format)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	if (ctx->accum_err)
+		return;
+
+	ret = mipi_dsi_dcs_set_pixel_format(dsi, format);
+	if (ret < 0) {
+		ctx->accum_err = ret;
+		dev_err(dev, "Failed to set pixel format: %d\n",
+			ctx->accum_err);
+	}
+}
+EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format_multi);
+
+/**
+ * mipi_dsi_dcs_set_column_address_multi() - define the column extent of the
+ *	frame memory accessed by the host processor
+ * @ctx: Context for multiple DSI transactions
+ * @start: first column of frame memory
+ * @end: last column of frame memory
+ *
+ * Like mipi_dsi_dcs_set_column_address() but deals with errors in a way that
+ * makes it convenient to make several calls in a row.
+ */
+void mipi_dsi_dcs_set_column_address_multi(struct mipi_dsi_multi_context *ctx,
+					   u16 start, u16 end)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	if (ctx->accum_err)
+		return;
+
+	ret = mipi_dsi_dcs_set_column_address(dsi, start, end);
+	if (ret < 0) {
+		ctx->accum_err = ret;
+		dev_err(dev, "Failed to set column address: %d\n",
+			ctx->accum_err);
+	}
+}
+EXPORT_SYMBOL(mipi_dsi_dcs_set_column_address_multi);
+
+/**
+ * mipi_dsi_dcs_set_page_address_multi() - define the page extent of the
+ *	frame memory accessed by the host processor
+ * @ctx: Context for multiple DSI transactions
+ * @start: first page of frame memory
+ * @end: last page of frame memory
+ *
+ * Like mipi_dsi_dcs_set_page_address() but deals with errors in a way that
+ * makes it convenient to make several calls in a row.
+ */
+void mipi_dsi_dcs_set_page_address_multi(struct mipi_dsi_multi_context *ctx,
+					 u16 start, u16 end)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	if (ctx->accum_err)
+		return;
+
+	ret = mipi_dsi_dcs_set_page_address(dsi, start, end);
+	if (ret < 0) {
+		ctx->accum_err = ret;
+		dev_err(dev, "Failed to set page address: %d\n",
+			ctx->accum_err);
+	}
+}
+EXPORT_SYMBOL(mipi_dsi_dcs_set_page_address_multi);
+
+/**
+ * mipi_dsi_dcs_get_display_brightness_multi() - gets the current brightness value
+ *    of the display
+ * @ctx: Context for multiple DSI transactions
+ * @brightness: brightness value
+ *
+ * Like mipi_dsi_dcs_get_display_brightness() but deals with errors in a way that
+ * makes it convenient to make several calls in a row.
+ */
+void mipi_dsi_dcs_get_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
+					       u16 *brightness)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	if (ctx->accum_err)
+		return;
+
+	ret = mipi_dsi_dcs_get_display_brightness(dsi, brightness);
+	if (ret < 0) {
+		ctx->accum_err = ret;
+		dev_err(dev, "Failed to get display brightness: %d\n",
+			ctx->accum_err);
+	}
+}
+EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness_multi);
+
+
 static int mipi_dsi_drv_probe(struct device *dev)
 {
 	struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver);
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 0f520eeeaa8e..7c6239d7b492 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -365,6 +365,18 @@ void mipi_dsi_dcs_set_display_off_multi(struct mipi_dsi_multi_context *ctx);
 void mipi_dsi_dcs_set_display_on_multi(struct mipi_dsi_multi_context *ctx);
 void mipi_dsi_dcs_set_tear_on_multi(struct mipi_dsi_multi_context *ctx,
 				    enum mipi_dsi_dcs_tear_mode mode);
+void mipi_dsi_turn_on_peripheral_multi(struct mipi_dsi_multi_context *ctx);
+void mipi_dsi_dcs_soft_reset_multi(struct mipi_dsi_multi_context *ctx);
+void mipi_dsi_dcs_set_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
+					       u16 brightness);
+void mipi_dsi_dcs_set_pixel_format_multi(struct mipi_dsi_multi_context *ctx,
+					 u8 format);
+void mipi_dsi_dcs_set_column_address_multi(struct mipi_dsi_multi_context *ctx,
+					   u16 start, u16 end);
+void mipi_dsi_dcs_set_page_address_multi(struct mipi_dsi_multi_context *ctx,
+					   u16 start, u16 end);
+void mipi_dsi_dcs_get_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
+					       u16 *brightness);
 
 /**
  * mipi_dsi_generic_write_seq - transmit data using a generic write packet
-- 
2.45.2

Re: [PATCH v2 1/2] drm/mipi-dsi: add more multi functions for better error handling

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

On Tue, Jul 30, 2024 at 11:36:58AM GMT, Tejas Vipin wrote:
> Add more functions that can benefit from being multi style and mark
> older variants as deprecated to eventually convert all mipi_dsi functions
> to multi style.
> 
> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>

Acked-by: Maxime Ripard <mripard@kernel.org>

We should also add a TODO note to convert existing drivers to the _multi
variant and get rid of the !multi one when it's done.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v2 1/2] drm/mipi-dsi: add more multi functions for better error handling

[Doug Anderson]

Hi,

On Mon, Jul 29, 2024 at 11:07 PM Tejas Vipin <tejasvipin76@gmail.com> wrote:
> +/**
> + * mipi_dsi_dcs_get_display_brightness_multi() - gets the current brightness value
> + *    of the display
> + * @ctx: Context for multiple DSI transactions
> + * @brightness: brightness value
> + *
> + * Like mipi_dsi_dcs_get_display_brightness() but deals with errors in a way that
> + * makes it convenient to make several calls in a row.
> + */
> +void mipi_dsi_dcs_get_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
> +                                              u16 *brightness)
> +{
> +       struct mipi_dsi_device *dsi = ctx->dsi;
> +       struct device *dev = &dsi->dev;
> +       int ret;
> +
> +       if (ctx->accum_err)
> +               return;
> +
> +       ret = mipi_dsi_dcs_get_display_brightness(dsi, brightness);
> +       if (ret < 0) {
> +               ctx->accum_err = ret;
> +               dev_err(dev, "Failed to get display brightness: %d\n",
> +                       ctx->accum_err);
> +       }
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness_multi);

I'd be interested in others' opinions, but this function strikes me as
one that *shouldn't* be converted to _multi.

Specifically the whole point of the _multi abstraction is that you can
fire off a whole pile of initialization commands without needing to
check for errors constantly. You can check for errors once at the end
of a sequence of commands and you can be sure that an error message
was printed for the command that failed and that all of the future
commands didn't do anything.

I have a hard time believing that _get_ brightness would be part of
this pile of initialization commands. ...and looking at how you use it
in the next patch I can see that, indeed, it's a bit awkward using the
_multi variant in the case you're using it.

The one advantage of the _multi functions is that they are also
"chatty" and we don't need to print the error everywhere. However, it
seems like we could just make the existing function print an error
message but still return the error directly. If this automatic
printing an error message is a problem for someone then I guess maybe
we've already reached the "tomorrow" [1] and need to figure out if we
need to keep two variants of the function around instead of marking
one as deprecated.

NOTE: If we don't convert this then the "set" function will still be
_multi but the "get" one won't be. I think that's fine since the "set"
function could plausibly be in a big sequence of commands but the
"get" function not so much...

[1] https://lore.kernel.org/r/CAD=FV=WbXdnM4or3Ae+nYoQW1Sce0jP6FWtCHShsALuEFNhiww@mail.gmail.com

Re: [PATCH v2 1/2] drm/mipi-dsi: add more multi functions for better error handling

[Tejas Vipin]

On 8/1/24 2:59 AM, Doug Anderson wrote:
> Hi,
> 
> On Mon, Jul 29, 2024 at 11:07 PM Tejas Vipin <tejasvipin76@gmail.com> wrote:
>> +/**
>> + * mipi_dsi_dcs_get_display_brightness_multi() - gets the current brightness value
>> + *    of the display
>> + * @ctx: Context for multiple DSI transactions
>> + * @brightness: brightness value
>> + *
>> + * Like mipi_dsi_dcs_get_display_brightness() but deals with errors in a way that
>> + * makes it convenient to make several calls in a row.
>> + */
>> +void mipi_dsi_dcs_get_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
>> +                                              u16 *brightness)
>> +{
>> +       struct mipi_dsi_device *dsi = ctx->dsi;
>> +       struct device *dev = &dsi->dev;
>> +       int ret;
>> +
>> +       if (ctx->accum_err)
>> +               return;
>> +
>> +       ret = mipi_dsi_dcs_get_display_brightness(dsi, brightness);
>> +       if (ret < 0) {
>> +               ctx->accum_err = ret;
>> +               dev_err(dev, "Failed to get display brightness: %d\n",
>> +                       ctx->accum_err);
>> +       }
>> +}
>> +EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness_multi);
> 
> I'd be interested in others' opinions, but this function strikes me as
> one that *shouldn't* be converted to _multi.
>

Only reason I converted the function at all was really for uniformity's
sake. But I don't think that's valid anymore seeing how there's already
other mipi_dsi funtions that I'm not converting and this function
probably wouldn't show up in the context of the other multi
functions.

> Specifically the whole point of the _multi abstraction is that you can
> fire off a whole pile of initialization commands without needing to
> check for errors constantly. You can check for errors once at the end
> of a sequence of commands and you can be sure that an error message
> was printed for the command that failed and that all of the future
> commands didn't do anything.
> 
> I have a hard time believing that _get_ brightness would be part of
> this pile of initialization commands. ...and looking at how you use it
> in the next patch I can see that, indeed, it's a bit awkward using the
> _multi variant in the case you're using it.
> 
> The one advantage of the _multi functions is that they are also
> "chatty" and we don't need to print the error everywhere. However, it
> seems like we could just make the existing function print an error
> message but still return the error directly. If this automatic
> printing an error message is a problem for someone then I guess maybe
> we've already reached the "tomorrow" [1] and need to figure out if we
> need to keep two variants of the function around instead of marking
> one as deprecated.
>

One thing that struck me as odd was that the callers of
mipi_dsi_dcs_get_display_brightness never bothered to print errors at
all? If we want to print errors for non-multi functions, then I think it
would be best to just modify the existing function. And in the case that
someone doesn't want those errors showing up, I agree with what Maxime
said [2] and let users handle it.

> NOTE: If we don't convert this then the "set" function will still be
> _multi but the "get" one won't be. I think that's fine since the "set"
> function could plausibly be in a big sequence of commands but the
> "get" function not so much...
> 
> [1] https://lore.kernel.org/r/CAD=FV=WbXdnM4or3Ae+nYoQW1Sce0jP6FWtCHShsALuEFNhiww@mail.gmail.com

[2] https://lore.kernel.org/all/20240726-cerise-civet-of-reverence-ebeb9d@houat/
-- 
Tejas Vipin

Re: [PATCH v2 1/2] drm/mipi-dsi: add more multi functions for better error handling

[Jani Nikula]

On Tue, 30 Jul 2024, Tejas Vipin <tejasvipin76@gmail.com> wrote:
> Add more functions that can benefit from being multi style and mark
> older variants as deprecated to eventually convert all mipi_dsi functions
> to multi style.

What?

Why would a lot of regular DSI commands that are not exclusively used
for one time setup need to be deprecated or converted to _multi()?

BR,
Jani.

>
> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> ---
>  drivers/gpu/drm/drm_mipi_dsi.c | 226 +++++++++++++++++++++++++++++++++
>  include/drm/drm_mipi_dsi.h     |  12 ++
>  2 files changed, 238 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index a471c46f5ca6..05ea7df5dec1 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -603,6 +603,8 @@ EXPORT_SYMBOL(mipi_dsi_shutdown_peripheral);
>   * mipi_dsi_turn_on_peripheral() - sends a Turn On Peripheral command
>   * @dsi: DSI peripheral device
>   *
> + * This function is deprecated. Use mipi_dsi_turn_on_peripheral_multi() instead.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int mipi_dsi_turn_on_peripheral(struct mipi_dsi_device *dsi)
> @@ -652,6 +654,7 @@ EXPORT_SYMBOL(mipi_dsi_set_maximum_return_packet_size);
>   * @pps_selector: Select PPS from the table of pre-stored or uploaded PPS entries
>   *
>   * Enable or disable Display Stream Compression on the peripheral.
> + * This function is deprecated. Use mipi_dsi_compression_mode_ext_multi() instead.
>   *
>   * Return: 0 on success or a negative error code on failure.
>   */
> @@ -703,6 +706,7 @@ EXPORT_SYMBOL(mipi_dsi_compression_mode);
>   * @pps: VESA DSC 1.1 Picture Parameter Set
>   *
>   * Transmit the VESA DSC 1.1 Picture Parameter Set to the peripheral.
> + * This function is deprecated. Use mipi_dsi_picture_parameter_set_multi() instead.
>   *
>   * Return: 0 on success or a negative error code on failure.
>   */
> @@ -1037,6 +1041,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_read);
>   * mipi_dsi_dcs_nop() - send DCS nop packet
>   * @dsi: DSI peripheral device
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_nop_multi() instead.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int mipi_dsi_dcs_nop(struct mipi_dsi_device *dsi)
> @@ -1055,6 +1061,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_nop);
>   * mipi_dsi_dcs_soft_reset() - perform a software reset of the display module
>   * @dsi: DSI peripheral device
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_soft_reset_multi() instead.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int mipi_dsi_dcs_soft_reset(struct mipi_dsi_device *dsi)
> @@ -1124,6 +1132,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_get_pixel_format);
>   *    display module except interface communication
>   * @dsi: DSI peripheral device
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_enter_sleep_mode_multi() instead.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int mipi_dsi_dcs_enter_sleep_mode(struct mipi_dsi_device *dsi)
> @@ -1143,6 +1153,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_enter_sleep_mode);
>   *    module
>   * @dsi: DSI peripheral device
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_exit_sleep_mode_multi() instead.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int mipi_dsi_dcs_exit_sleep_mode(struct mipi_dsi_device *dsi)
> @@ -1162,6 +1174,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_exit_sleep_mode);
>   *    display device
>   * @dsi: DSI peripheral device
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_set_display_off_multi() instead.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int mipi_dsi_dcs_set_display_off(struct mipi_dsi_device *dsi)
> @@ -1181,6 +1195,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_display_off);
>   *    display device
>   * @dsi: DSI peripheral device
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_set_display_on_multi() instead.
> + *
>   * Return: 0 on success or a negative error code on failure
>   */
>  int mipi_dsi_dcs_set_display_on(struct mipi_dsi_device *dsi)
> @@ -1202,6 +1218,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_display_on);
>   * @start: first column of frame memory
>   * @end: last column of frame memory
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_set_column_address_multi()
> + * instead.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int mipi_dsi_dcs_set_column_address(struct mipi_dsi_device *dsi, u16 start,
> @@ -1226,6 +1245,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_column_address);
>   * @start: first page of frame memory
>   * @end: last page of frame memory
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_set_page_address_multi()
> + * instead.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int mipi_dsi_dcs_set_page_address(struct mipi_dsi_device *dsi, u16 start,
> @@ -1268,6 +1290,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_off);
>   * @dsi: DSI peripheral device
>   * @mode: the Tearing Effect Output Line mode
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_set_tear_on_multi() instead.
> + *
>   * Return: 0 on success or a negative error code on failure
>   */
>  int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi,
> @@ -1291,6 +1315,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_on);
>   * @dsi: DSI peripheral device
>   * @format: pixel format
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_set_pixel_format_multi()
> + * instead.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format)
> @@ -1334,6 +1361,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_scanline);
>   * @dsi: DSI peripheral device
>   * @brightness: brightness value
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_set_display_brightness_multi()
> + * instead.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi,
> @@ -1357,6 +1387,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness);
>   * @dsi: DSI peripheral device
>   * @brightness: brightness value
>   *
> + * This function is deprecated. Use mipi_dsi_dcs_get_display_brightness_multi()
> + * instead.
> + *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi,
> @@ -1639,6 +1672,199 @@ void mipi_dsi_dcs_set_tear_on_multi(struct mipi_dsi_multi_context *ctx,
>  }
>  EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_on_multi);
>  
> +/**
> + * mipi_dsi_turn_on_peripheral_multi() - sends a Turn On Peripheral command
> + * @ctx: Context for multiple DSI transactions
> + *
> + * Like mipi_dsi_turn_on_peripheral() but deals with errors in a way that
> + * makes it convenient to make several calls in a row.
> + */
> +void mipi_dsi_turn_on_peripheral_multi(struct mipi_dsi_multi_context *ctx)
> +{
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct device *dev = &dsi->dev;
> +	int ret;
> +
> +	if (ctx->accum_err)
> +		return;
> +
> +	ret = mipi_dsi_turn_on_peripheral(dsi);
> +	if (ret < 0) {
> +		ctx->accum_err = ret;
> +		dev_err(dev, "Failed to turn on peripheral: %d\n",
> +			ctx->accum_err);
> +	}
> +}
> +EXPORT_SYMBOL(mipi_dsi_turn_on_peripheral_multi);
> +
> +/**
> + * mipi_dsi_dcs_soft_reset_multi() - perform a software reset of the display module
> + * @ctx: Context for multiple DSI transactions
> + *
> + * Like mipi_dsi_dcs_soft_reset() but deals with errors in a way that
> + * makes it convenient to make several calls in a row.
> + */
> +void mipi_dsi_dcs_soft_reset_multi(struct mipi_dsi_multi_context *ctx)
> +{
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct device *dev = &dsi->dev;
> +	int ret;
> +
> +	if (ctx->accum_err)
> +		return;
> +
> +	ret = mipi_dsi_dcs_soft_reset(dsi);
> +	if (ret < 0) {
> +		ctx->accum_err = ret;
> +		dev_err(dev, "Failed to mipi_dsi_dcs_soft_reset: %d\n",
> +			ctx->accum_err);
> +	}
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_soft_reset_multi);
> +
> +/**
> + * mipi_dsi_dcs_set_display_brightness_multi() - sets the brightness value of
> + *	the display
> + * @ctx: Context for multiple DSI transactions
> + * @brightness: brightness value
> + *
> + * Like mipi_dsi_dcs_set_display_brightness() but deals with errors in a way that
> + * makes it convenient to make several calls in a row.
> + */
> +void mipi_dsi_dcs_set_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
> +					       u16 brightness)
> +{
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct device *dev = &dsi->dev;
> +	int ret;
> +
> +	if (ctx->accum_err)
> +		return;
> +
> +	ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
> +	if (ret < 0) {
> +		ctx->accum_err = ret;
> +		dev_err(dev, "Failed to write display brightness: %d\n",
> +			ctx->accum_err);
> +	}
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness_multi);
> +
> +/**
> + * mipi_dsi_dcs_set_pixel_format_multi() - sets the pixel format for the RGB image
> + *	data used by the interface
> + * @ctx: Context for multiple DSI transactions
> + * @format: pixel format
> + *
> + * Like mipi_dsi_dcs_set_pixel_format() but deals with errors in a way that
> + * makes it convenient to make several calls in a row.
> + */
> +void mipi_dsi_dcs_set_pixel_format_multi(struct mipi_dsi_multi_context *ctx,
> +					 u8 format)
> +{
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct device *dev = &dsi->dev;
> +	int ret;
> +
> +	if (ctx->accum_err)
> +		return;
> +
> +	ret = mipi_dsi_dcs_set_pixel_format(dsi, format);
> +	if (ret < 0) {
> +		ctx->accum_err = ret;
> +		dev_err(dev, "Failed to set pixel format: %d\n",
> +			ctx->accum_err);
> +	}
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format_multi);
> +
> +/**
> + * mipi_dsi_dcs_set_column_address_multi() - define the column extent of the
> + *	frame memory accessed by the host processor
> + * @ctx: Context for multiple DSI transactions
> + * @start: first column of frame memory
> + * @end: last column of frame memory
> + *
> + * Like mipi_dsi_dcs_set_column_address() but deals with errors in a way that
> + * makes it convenient to make several calls in a row.
> + */
> +void mipi_dsi_dcs_set_column_address_multi(struct mipi_dsi_multi_context *ctx,
> +					   u16 start, u16 end)
> +{
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct device *dev = &dsi->dev;
> +	int ret;
> +
> +	if (ctx->accum_err)
> +		return;
> +
> +	ret = mipi_dsi_dcs_set_column_address(dsi, start, end);
> +	if (ret < 0) {
> +		ctx->accum_err = ret;
> +		dev_err(dev, "Failed to set column address: %d\n",
> +			ctx->accum_err);
> +	}
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_set_column_address_multi);
> +
> +/**
> + * mipi_dsi_dcs_set_page_address_multi() - define the page extent of the
> + *	frame memory accessed by the host processor
> + * @ctx: Context for multiple DSI transactions
> + * @start: first page of frame memory
> + * @end: last page of frame memory
> + *
> + * Like mipi_dsi_dcs_set_page_address() but deals with errors in a way that
> + * makes it convenient to make several calls in a row.
> + */
> +void mipi_dsi_dcs_set_page_address_multi(struct mipi_dsi_multi_context *ctx,
> +					 u16 start, u16 end)
> +{
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct device *dev = &dsi->dev;
> +	int ret;
> +
> +	if (ctx->accum_err)
> +		return;
> +
> +	ret = mipi_dsi_dcs_set_page_address(dsi, start, end);
> +	if (ret < 0) {
> +		ctx->accum_err = ret;
> +		dev_err(dev, "Failed to set page address: %d\n",
> +			ctx->accum_err);
> +	}
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_set_page_address_multi);
> +
> +/**
> + * mipi_dsi_dcs_get_display_brightness_multi() - gets the current brightness value
> + *    of the display
> + * @ctx: Context for multiple DSI transactions
> + * @brightness: brightness value
> + *
> + * Like mipi_dsi_dcs_get_display_brightness() but deals with errors in a way that
> + * makes it convenient to make several calls in a row.
> + */
> +void mipi_dsi_dcs_get_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
> +					       u16 *brightness)
> +{
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct device *dev = &dsi->dev;
> +	int ret;
> +
> +	if (ctx->accum_err)
> +		return;
> +
> +	ret = mipi_dsi_dcs_get_display_brightness(dsi, brightness);
> +	if (ret < 0) {
> +		ctx->accum_err = ret;
> +		dev_err(dev, "Failed to get display brightness: %d\n",
> +			ctx->accum_err);
> +	}
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness_multi);
> +
> +
>  static int mipi_dsi_drv_probe(struct device *dev)
>  {
>  	struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver);
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 0f520eeeaa8e..7c6239d7b492 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -365,6 +365,18 @@ void mipi_dsi_dcs_set_display_off_multi(struct mipi_dsi_multi_context *ctx);
>  void mipi_dsi_dcs_set_display_on_multi(struct mipi_dsi_multi_context *ctx);
>  void mipi_dsi_dcs_set_tear_on_multi(struct mipi_dsi_multi_context *ctx,
>  				    enum mipi_dsi_dcs_tear_mode mode);
> +void mipi_dsi_turn_on_peripheral_multi(struct mipi_dsi_multi_context *ctx);
> +void mipi_dsi_dcs_soft_reset_multi(struct mipi_dsi_multi_context *ctx);
> +void mipi_dsi_dcs_set_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
> +					       u16 brightness);
> +void mipi_dsi_dcs_set_pixel_format_multi(struct mipi_dsi_multi_context *ctx,
> +					 u8 format);
> +void mipi_dsi_dcs_set_column_address_multi(struct mipi_dsi_multi_context *ctx,
> +					   u16 start, u16 end);
> +void mipi_dsi_dcs_set_page_address_multi(struct mipi_dsi_multi_context *ctx,
> +					   u16 start, u16 end);
> +void mipi_dsi_dcs_get_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
> +					       u16 *brightness);
>  
>  /**
>   * mipi_dsi_generic_write_seq - transmit data using a generic write packet

-- 
Jani Nikula, Intel

Re: [PATCH v2 1/2] drm/mipi-dsi: add more multi functions for better error handling

[Tejas Vipin]

On 8/1/24 4:39 PM, Jani Nikula wrote:
> On Tue, 30 Jul 2024, Tejas Vipin <tejasvipin76@gmail.com> wrote:
>> Add more functions that can benefit from being multi style and mark
>> older variants as deprecated to eventually convert all mipi_dsi functions
>> to multi style.
> 
> What?
> 
> Why would a lot of regular DSI commands that are not exclusively used
> for one time setup need to be deprecated or converted to _multi()?
> 

All of the functions I've marked as deprecated here have a good amount
of their usage in conjunction with other mipi_dsi functions (an
exception being mipi_dsi_dcs_get_display_brightness which I have
realized is not suitable for this type of conversion). Them being
rewritten as multi style functions saves a lot of early returns and
errors being repeated over and over again across the codebase.

In the cases where they are just called by themselves, there is very little
overhead in replacing them with a multi variant. These functions would
be better off converted to multi variants, and the old versions removed
when all the function calls are replaced.

> BR,
> Jani.
> 
>>
>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
>> ---
>>  drivers/gpu/drm/drm_mipi_dsi.c | 226 +++++++++++++++++++++++++++++++++
>>  include/drm/drm_mipi_dsi.h     |  12 ++
>>  2 files changed, 238 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
>> index a471c46f5ca6..05ea7df5dec1 100644
>> --- a/drivers/gpu/drm/drm_mipi_dsi.c
>> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
>> @@ -603,6 +603,8 @@ EXPORT_SYMBOL(mipi_dsi_shutdown_peripheral);
>>   * mipi_dsi_turn_on_peripheral() - sends a Turn On Peripheral command
>>   * @dsi: DSI peripheral device
>>   *
>> + * This function is deprecated. Use mipi_dsi_turn_on_peripheral_multi() instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  int mipi_dsi_turn_on_peripheral(struct mipi_dsi_device *dsi)
>> @@ -652,6 +654,7 @@ EXPORT_SYMBOL(mipi_dsi_set_maximum_return_packet_size);
>>   * @pps_selector: Select PPS from the table of pre-stored or uploaded PPS entries
>>   *
>>   * Enable or disable Display Stream Compression on the peripheral.
>> + * This function is deprecated. Use mipi_dsi_compression_mode_ext_multi() instead.
>>   *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>> @@ -703,6 +706,7 @@ EXPORT_SYMBOL(mipi_dsi_compression_mode);
>>   * @pps: VESA DSC 1.1 Picture Parameter Set
>>   *
>>   * Transmit the VESA DSC 1.1 Picture Parameter Set to the peripheral.
>> + * This function is deprecated. Use mipi_dsi_picture_parameter_set_multi() instead.
>>   *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>> @@ -1037,6 +1041,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_read);
>>   * mipi_dsi_dcs_nop() - send DCS nop packet
>>   * @dsi: DSI peripheral device
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_nop_multi() instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  int mipi_dsi_dcs_nop(struct mipi_dsi_device *dsi)
>> @@ -1055,6 +1061,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_nop);
>>   * mipi_dsi_dcs_soft_reset() - perform a software reset of the display module
>>   * @dsi: DSI peripheral device
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_soft_reset_multi() instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  int mipi_dsi_dcs_soft_reset(struct mipi_dsi_device *dsi)
>> @@ -1124,6 +1132,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_get_pixel_format);
>>   *    display module except interface communication
>>   * @dsi: DSI peripheral device
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_enter_sleep_mode_multi() instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  int mipi_dsi_dcs_enter_sleep_mode(struct mipi_dsi_device *dsi)
>> @@ -1143,6 +1153,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_enter_sleep_mode);
>>   *    module
>>   * @dsi: DSI peripheral device
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_exit_sleep_mode_multi() instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  int mipi_dsi_dcs_exit_sleep_mode(struct mipi_dsi_device *dsi)
>> @@ -1162,6 +1174,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_exit_sleep_mode);
>>   *    display device
>>   * @dsi: DSI peripheral device
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_set_display_off_multi() instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  int mipi_dsi_dcs_set_display_off(struct mipi_dsi_device *dsi)
>> @@ -1181,6 +1195,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_display_off);
>>   *    display device
>>   * @dsi: DSI peripheral device
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_set_display_on_multi() instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure
>>   */
>>  int mipi_dsi_dcs_set_display_on(struct mipi_dsi_device *dsi)
>> @@ -1202,6 +1218,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_display_on);
>>   * @start: first column of frame memory
>>   * @end: last column of frame memory
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_set_column_address_multi()
>> + * instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  int mipi_dsi_dcs_set_column_address(struct mipi_dsi_device *dsi, u16 start,
>> @@ -1226,6 +1245,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_column_address);
>>   * @start: first page of frame memory
>>   * @end: last page of frame memory
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_set_page_address_multi()
>> + * instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  int mipi_dsi_dcs_set_page_address(struct mipi_dsi_device *dsi, u16 start,
>> @@ -1268,6 +1290,8 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_off);
>>   * @dsi: DSI peripheral device
>>   * @mode: the Tearing Effect Output Line mode
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_set_tear_on_multi() instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure
>>   */
>>  int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi,
>> @@ -1291,6 +1315,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_on);
>>   * @dsi: DSI peripheral device
>>   * @format: pixel format
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_set_pixel_format_multi()
>> + * instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format)
>> @@ -1334,6 +1361,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_scanline);
>>   * @dsi: DSI peripheral device
>>   * @brightness: brightness value
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_set_display_brightness_multi()
>> + * instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi,
>> @@ -1357,6 +1387,9 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness);
>>   * @dsi: DSI peripheral device
>>   * @brightness: brightness value
>>   *
>> + * This function is deprecated. Use mipi_dsi_dcs_get_display_brightness_multi()
>> + * instead.
>> + *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>>  int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi,
>> @@ -1639,6 +1672,199 @@ void mipi_dsi_dcs_set_tear_on_multi(struct mipi_dsi_multi_context *ctx,
>>  }
>>  EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_on_multi);
>>  
>> +/**
>> + * mipi_dsi_turn_on_peripheral_multi() - sends a Turn On Peripheral command
>> + * @ctx: Context for multiple DSI transactions
>> + *
>> + * Like mipi_dsi_turn_on_peripheral() but deals with errors in a way that
>> + * makes it convenient to make several calls in a row.
>> + */
>> +void mipi_dsi_turn_on_peripheral_multi(struct mipi_dsi_multi_context *ctx)
>> +{
>> +	struct mipi_dsi_device *dsi = ctx->dsi;
>> +	struct device *dev = &dsi->dev;
>> +	int ret;
>> +
>> +	if (ctx->accum_err)
>> +		return;
>> +
>> +	ret = mipi_dsi_turn_on_peripheral(dsi);
>> +	if (ret < 0) {
>> +		ctx->accum_err = ret;
>> +		dev_err(dev, "Failed to turn on peripheral: %d\n",
>> +			ctx->accum_err);
>> +	}
>> +}
>> +EXPORT_SYMBOL(mipi_dsi_turn_on_peripheral_multi);
>> +
>> +/**
>> + * mipi_dsi_dcs_soft_reset_multi() - perform a software reset of the display module
>> + * @ctx: Context for multiple DSI transactions
>> + *
>> + * Like mipi_dsi_dcs_soft_reset() but deals with errors in a way that
>> + * makes it convenient to make several calls in a row.
>> + */
>> +void mipi_dsi_dcs_soft_reset_multi(struct mipi_dsi_multi_context *ctx)
>> +{
>> +	struct mipi_dsi_device *dsi = ctx->dsi;
>> +	struct device *dev = &dsi->dev;
>> +	int ret;
>> +
>> +	if (ctx->accum_err)
>> +		return;
>> +
>> +	ret = mipi_dsi_dcs_soft_reset(dsi);
>> +	if (ret < 0) {
>> +		ctx->accum_err = ret;
>> +		dev_err(dev, "Failed to mipi_dsi_dcs_soft_reset: %d\n",
>> +			ctx->accum_err);
>> +	}
>> +}
>> +EXPORT_SYMBOL(mipi_dsi_dcs_soft_reset_multi);
>> +
>> +/**
>> + * mipi_dsi_dcs_set_display_brightness_multi() - sets the brightness value of
>> + *	the display
>> + * @ctx: Context for multiple DSI transactions
>> + * @brightness: brightness value
>> + *
>> + * Like mipi_dsi_dcs_set_display_brightness() but deals with errors in a way that
>> + * makes it convenient to make several calls in a row.
>> + */
>> +void mipi_dsi_dcs_set_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
>> +					       u16 brightness)
>> +{
>> +	struct mipi_dsi_device *dsi = ctx->dsi;
>> +	struct device *dev = &dsi->dev;
>> +	int ret;
>> +
>> +	if (ctx->accum_err)
>> +		return;
>> +
>> +	ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
>> +	if (ret < 0) {
>> +		ctx->accum_err = ret;
>> +		dev_err(dev, "Failed to write display brightness: %d\n",
>> +			ctx->accum_err);
>> +	}
>> +}
>> +EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness_multi);
>> +
>> +/**
>> + * mipi_dsi_dcs_set_pixel_format_multi() - sets the pixel format for the RGB image
>> + *	data used by the interface
>> + * @ctx: Context for multiple DSI transactions
>> + * @format: pixel format
>> + *
>> + * Like mipi_dsi_dcs_set_pixel_format() but deals with errors in a way that
>> + * makes it convenient to make several calls in a row.
>> + */
>> +void mipi_dsi_dcs_set_pixel_format_multi(struct mipi_dsi_multi_context *ctx,
>> +					 u8 format)
>> +{
>> +	struct mipi_dsi_device *dsi = ctx->dsi;
>> +	struct device *dev = &dsi->dev;
>> +	int ret;
>> +
>> +	if (ctx->accum_err)
>> +		return;
>> +
>> +	ret = mipi_dsi_dcs_set_pixel_format(dsi, format);
>> +	if (ret < 0) {
>> +		ctx->accum_err = ret;
>> +		dev_err(dev, "Failed to set pixel format: %d\n",
>> +			ctx->accum_err);
>> +	}
>> +}
>> +EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format_multi);
>> +
>> +/**
>> + * mipi_dsi_dcs_set_column_address_multi() - define the column extent of the
>> + *	frame memory accessed by the host processor
>> + * @ctx: Context for multiple DSI transactions
>> + * @start: first column of frame memory
>> + * @end: last column of frame memory
>> + *
>> + * Like mipi_dsi_dcs_set_column_address() but deals with errors in a way that
>> + * makes it convenient to make several calls in a row.
>> + */
>> +void mipi_dsi_dcs_set_column_address_multi(struct mipi_dsi_multi_context *ctx,
>> +					   u16 start, u16 end)
>> +{
>> +	struct mipi_dsi_device *dsi = ctx->dsi;
>> +	struct device *dev = &dsi->dev;
>> +	int ret;
>> +
>> +	if (ctx->accum_err)
>> +		return;
>> +
>> +	ret = mipi_dsi_dcs_set_column_address(dsi, start, end);
>> +	if (ret < 0) {
>> +		ctx->accum_err = ret;
>> +		dev_err(dev, "Failed to set column address: %d\n",
>> +			ctx->accum_err);
>> +	}
>> +}
>> +EXPORT_SYMBOL(mipi_dsi_dcs_set_column_address_multi);
>> +
>> +/**
>> + * mipi_dsi_dcs_set_page_address_multi() - define the page extent of the
>> + *	frame memory accessed by the host processor
>> + * @ctx: Context for multiple DSI transactions
>> + * @start: first page of frame memory
>> + * @end: last page of frame memory
>> + *
>> + * Like mipi_dsi_dcs_set_page_address() but deals with errors in a way that
>> + * makes it convenient to make several calls in a row.
>> + */
>> +void mipi_dsi_dcs_set_page_address_multi(struct mipi_dsi_multi_context *ctx,
>> +					 u16 start, u16 end)
>> +{
>> +	struct mipi_dsi_device *dsi = ctx->dsi;
>> +	struct device *dev = &dsi->dev;
>> +	int ret;
>> +
>> +	if (ctx->accum_err)
>> +		return;
>> +
>> +	ret = mipi_dsi_dcs_set_page_address(dsi, start, end);
>> +	if (ret < 0) {
>> +		ctx->accum_err = ret;
>> +		dev_err(dev, "Failed to set page address: %d\n",
>> +			ctx->accum_err);
>> +	}
>> +}
>> +EXPORT_SYMBOL(mipi_dsi_dcs_set_page_address_multi);
>> +
>> +/**
>> + * mipi_dsi_dcs_get_display_brightness_multi() - gets the current brightness value
>> + *    of the display
>> + * @ctx: Context for multiple DSI transactions
>> + * @brightness: brightness value
>> + *
>> + * Like mipi_dsi_dcs_get_display_brightness() but deals with errors in a way that
>> + * makes it convenient to make several calls in a row.
>> + */
>> +void mipi_dsi_dcs_get_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
>> +					       u16 *brightness)
>> +{
>> +	struct mipi_dsi_device *dsi = ctx->dsi;
>> +	struct device *dev = &dsi->dev;
>> +	int ret;
>> +
>> +	if (ctx->accum_err)
>> +		return;
>> +
>> +	ret = mipi_dsi_dcs_get_display_brightness(dsi, brightness);
>> +	if (ret < 0) {
>> +		ctx->accum_err = ret;
>> +		dev_err(dev, "Failed to get display brightness: %d\n",
>> +			ctx->accum_err);
>> +	}
>> +}
>> +EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness_multi);
>> +
>> +
>>  static int mipi_dsi_drv_probe(struct device *dev)
>>  {
>>  	struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver);
>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>> index 0f520eeeaa8e..7c6239d7b492 100644
>> --- a/include/drm/drm_mipi_dsi.h
>> +++ b/include/drm/drm_mipi_dsi.h
>> @@ -365,6 +365,18 @@ void mipi_dsi_dcs_set_display_off_multi(struct mipi_dsi_multi_context *ctx);
>>  void mipi_dsi_dcs_set_display_on_multi(struct mipi_dsi_multi_context *ctx);
>>  void mipi_dsi_dcs_set_tear_on_multi(struct mipi_dsi_multi_context *ctx,
>>  				    enum mipi_dsi_dcs_tear_mode mode);
>> +void mipi_dsi_turn_on_peripheral_multi(struct mipi_dsi_multi_context *ctx);
>> +void mipi_dsi_dcs_soft_reset_multi(struct mipi_dsi_multi_context *ctx);
>> +void mipi_dsi_dcs_set_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
>> +					       u16 brightness);
>> +void mipi_dsi_dcs_set_pixel_format_multi(struct mipi_dsi_multi_context *ctx,
>> +					 u8 format);
>> +void mipi_dsi_dcs_set_column_address_multi(struct mipi_dsi_multi_context *ctx,
>> +					   u16 start, u16 end);
>> +void mipi_dsi_dcs_set_page_address_multi(struct mipi_dsi_multi_context *ctx,
>> +					   u16 start, u16 end);
>> +void mipi_dsi_dcs_get_display_brightness_multi(struct mipi_dsi_multi_context *ctx,
>> +					       u16 *brightness);
>>  
>>  /**
>>   * mipi_dsi_generic_write_seq - transmit data using a generic write packet
> 

-- 
Tejas Vipin

[PATCH v2 2/2] drm/panel: startek-kd070fhfid015: transition to mipi_dsi wrapped functions

[Tejas Vipin]

Use multi style wrapped functions for mipi_dsi in the
startek-kd070fhfid015 panel.

Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
---
 .../drm/panel/panel-startek-kd070fhfid015.c   | 123 ++++++------------
 1 file changed, 39 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-startek-kd070fhfid015.c b/drivers/gpu/drm/panel/panel-startek-kd070fhfid015.c
index 0156689f41cd..f1df727b9183 100644
--- a/drivers/gpu/drm/panel/panel-startek-kd070fhfid015.c
+++ b/drivers/gpu/drm/panel/panel-startek-kd070fhfid015.c
@@ -24,10 +24,10 @@
 #include <drm/drm_modes.h>
 #include <drm/drm_panel.h>
 
-#define DSI_REG_MCAP	0xB0
-#define DSI_REG_IS	0xB3 /* Interface Setting */
-#define DSI_REG_IIS	0xB4 /* Interface ID Setting */
-#define DSI_REG_CTRL	0xB6
+#define DSI_REG_MCAP	0xb0
+#define DSI_REG_IS	0xb3 /* Interface Setting */
+#define DSI_REG_IIS	0xb4 /* Interface ID Setting */
+#define DSI_REG_CTRL	0xb6
 
 enum {
 	IOVCC = 0,
@@ -52,92 +52,55 @@ static inline struct stk_panel *to_stk_panel(struct drm_panel *panel)
 static int stk_panel_init(struct stk_panel *stk)
 {
 	struct mipi_dsi_device *dsi = stk->dsi;
-	struct device *dev = &stk->dsi->dev;
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = {.dsi = dsi};
 
-	ret = mipi_dsi_dcs_soft_reset(dsi);
-	if (ret < 0) {
-		dev_err(dev, "failed to mipi_dsi_dcs_soft_reset: %d\n", ret);
-		return ret;
-	}
-	mdelay(5);
+	mipi_dsi_dcs_soft_reset_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 5);
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 120);
 
-	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
-	if (ret < 0) {
-		dev_err(dev, "failed to set exit sleep mode: %d\n", ret);
-		return ret;
-	}
-	msleep(120);
-
-	mipi_dsi_generic_write_seq(dsi, DSI_REG_MCAP, 0x04);
+	mipi_dsi_generic_write_seq_multi(&dsi_ctx, DSI_REG_MCAP, 0x04);
 
 	/* Interface setting, video mode */
-	mipi_dsi_generic_write_seq(dsi, DSI_REG_IS, 0x14, 0x08, 0x00, 0x22, 0x00);
-	mipi_dsi_generic_write_seq(dsi, DSI_REG_IIS, 0x0C, 0x00);
-	mipi_dsi_generic_write_seq(dsi, DSI_REG_CTRL, 0x3A, 0xD3);
+	mipi_dsi_generic_write_seq_multi(&dsi_ctx, DSI_REG_IS, 0x14, 0x08, 0x00, 0x22, 0x00);
+	mipi_dsi_generic_write_seq_multi(&dsi_ctx, DSI_REG_IIS, 0x0c, 0x00);
+	mipi_dsi_generic_write_seq_multi(&dsi_ctx, DSI_REG_CTRL, 0x3a, 0xd3);
 
-	ret = mipi_dsi_dcs_set_display_brightness(dsi, 0x77);
-	if (ret < 0) {
-		dev_err(dev, "failed to write display brightness: %d\n", ret);
-		return ret;
-	}
+	mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx, 0x77);
 
-	mipi_dsi_dcs_write_seq(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY,
-			       MIPI_DCS_WRITE_MEMORY_START);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY,
+				     MIPI_DCS_WRITE_MEMORY_START);
 
-	ret = mipi_dsi_dcs_set_pixel_format(dsi, 0x77);
-	if (ret < 0) {
-		dev_err(dev, "failed to set pixel format: %d\n", ret);
-		return ret;
-	}
+	mipi_dsi_dcs_set_pixel_format_multi(&dsi_ctx, 0x77);
+	mipi_dsi_dcs_set_column_address_multi(&dsi_ctx, 0, stk->mode->hdisplay - 1);
+	mipi_dsi_dcs_set_page_address_multi(&dsi_ctx, 0, stk->mode->vdisplay - 1);
 
-	ret = mipi_dsi_dcs_set_column_address(dsi, 0, stk->mode->hdisplay - 1);
-	if (ret < 0) {
-		dev_err(dev, "failed to set column address: %d\n", ret);
-		return ret;
-	}
-
-	ret = mipi_dsi_dcs_set_page_address(dsi, 0, stk->mode->vdisplay - 1);
-	if (ret < 0) {
-		dev_err(dev, "failed to set page address: %d\n", ret);
-		return ret;
-	}
-
-	return 0;
+	return dsi_ctx.accum_err;
 }
 
 static int stk_panel_on(struct stk_panel *stk)
 {
 	struct mipi_dsi_device *dsi = stk->dsi;
-	struct device *dev = &stk->dsi->dev;
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = {.dsi = dsi};
 
-	ret = mipi_dsi_dcs_set_display_on(dsi);
-	if (ret < 0)
-		dev_err(dev, "failed to set display on: %d\n", ret);
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
 
-	mdelay(20);
+	mipi_dsi_msleep(&dsi_ctx, 20);
 
-	return ret;
+	return dsi_ctx.accum_err;
 }
 
 static void stk_panel_off(struct stk_panel *stk)
 {
 	struct mipi_dsi_device *dsi = stk->dsi;
-	struct device *dev = &stk->dsi->dev;
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = {.dsi = dsi};
 
 	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
 
-	ret = mipi_dsi_dcs_set_display_off(dsi);
-	if (ret < 0)
-		dev_err(dev, "failed to set display off: %d\n", ret);
-
-	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
-	if (ret < 0)
-		dev_err(dev, "failed to enter sleep mode: %d\n", ret);
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
 
-	msleep(100);
+	mipi_dsi_msleep(&dsi_ctx, 100);
 }
 
 static int stk_panel_unprepare(struct drm_panel *panel)
@@ -155,7 +118,6 @@ static int stk_panel_unprepare(struct drm_panel *panel)
 static int stk_panel_prepare(struct drm_panel *panel)
 {
 	struct stk_panel *stk = to_stk_panel(panel);
-	struct device *dev = &stk->dsi->dev;
 	int ret;
 
 	gpiod_set_value(stk->reset_gpio, 0);
@@ -175,16 +137,12 @@ static int stk_panel_prepare(struct drm_panel *panel)
 	gpiod_set_value(stk->reset_gpio, 1);
 	mdelay(10);
 	ret = stk_panel_init(stk);
-	if (ret < 0) {
-		dev_err(dev, "failed to init panel: %d\n", ret);
+	if (ret < 0)
 		goto poweroff;
-	}
 
 	ret = stk_panel_on(stk);
-	if (ret < 0) {
-		dev_err(dev, "failed to set panel on: %d\n", ret);
+	if (ret < 0)
 		goto poweroff;
-	}
 
 	return 0;
 
@@ -235,13 +193,13 @@ static int stk_panel_get_modes(struct drm_panel *panel,
 static int dsi_dcs_bl_get_brightness(struct backlight_device *bl)
 {
 	struct mipi_dsi_device *dsi = bl_get_data(bl);
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
 	u16 brightness;
 
 	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
-	ret = mipi_dsi_dcs_get_display_brightness(dsi, &brightness);
-	if (ret < 0)
-		return ret;
+	mipi_dsi_dcs_get_display_brightness_multi(&dsi_ctx, &brightness);
+	if (dsi_ctx.accum_err)
+		return dsi_ctx.accum_err;
 
 	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
 	return brightness & 0xff;
@@ -250,18 +208,15 @@ static int dsi_dcs_bl_get_brightness(struct backlight_device *bl)
 static int dsi_dcs_bl_update_status(struct backlight_device *bl)
 {
 	struct mipi_dsi_device *dsi = bl_get_data(bl);
-	struct device *dev = &dsi->dev;
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = {.dsi = dsi};
 
 	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
-	ret = mipi_dsi_dcs_set_display_brightness(dsi, bl->props.brightness);
-	if (ret < 0) {
-		dev_err(dev, "failed to set DSI control: %d\n", ret);
-		return ret;
-	}
+	mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx, bl->props.brightness);
+	if (dsi_ctx.accum_err)
+		return dsi_ctx.accum_err;
 
 	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
-	return 0;
+	return dsi_ctx.accum_err;
 }
 
 static const struct backlight_ops dsi_bl_ops = {
-- 
2.45.2