Create mipi_dsi_dual, mipi_dsi_dual_dcs_write_seq_multi, and
mipi_dsi_dual_generic_write_seq_multi macros for panels which are driven
by two parallel serial interfaces. This allows for the reduction of code
duplication in drivers for these panels.
Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
---
mipi_dsi_dual_dcs_write_seq_multi goes unused by jdi-lpm102a188a. It's
included in this patch for completeness and in anticipation of its use
in other drivers in the future.
drivers/gpu/drm/drm_mipi_dsi.c | 48 ++++++++++++++++++
include/drm/drm_mipi_dsi.h | 89 ++++++++++++++++++++++++++++++++++
2 files changed, 137 insertions(+)
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index a00d76443128..4a7ca1261105 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -827,6 +827,30 @@ void mipi_dsi_generic_write_multi(struct mipi_dsi_multi_context *ctx,
}
EXPORT_SYMBOL(mipi_dsi_generic_write_multi);
+/**
+ * mipi_dsi_dual_generic_write_multi() - mipi_dsi_generic_write_multi() for
+ * two dsi channels, one after the other
+ * @dsi1: First dsi channel to write buffer to
+ * @dsi2: Second dsi channel to write buffer to
+ * @ctx: Context for multiple DSI transactions
+ * @payload: Buffer containing the payload
+ * @size: Size of payload buffer
+ *
+ * A wrapper around mipi_dsi_generic_write_multi() that allows the user to
+ * conveniently write to two dsi channels, one after the other.
+ */
+void mpi_dsi_dual_generic_write_multi(struct mipi_dsi_device *dsi1,
+ struct mipi_dsi_device *dsi2,
+ struct mipi_dsi_multi_context *ctx,
+ const void *payload, size_t size)
+{
+ ctx->dsi = dsi1;
+ mipi_dsi_generic_write_multi(ctx, data, len);
+ ctx->dsi = dsi2;
+ mipi_dsi_generic_write_multi(ctx, data, len);
+}
+EXPORT_SYMBOL(mipi_dsi_dual_generic_write_multi);
+
/**
* mipi_dsi_generic_read() - receive data using a generic read packet
* @dsi: DSI peripheral device
@@ -1006,6 +1030,30 @@ void mipi_dsi_dcs_write_buffer_multi(struct mipi_dsi_multi_context *ctx,
}
EXPORT_SYMBOL(mipi_dsi_dcs_write_buffer_multi);
+/**
+ * mipi_dsi_dual_dcs_write_buffer_multi - mipi_dsi_dcs_write_buffer_multi() for
+ * two dsi channels, one after the other
+ * @dsi1: First dsi channel to write buffer to
+ * @dsi2: Second dsi channel to write buffer to
+ * @ctx: Context for multiple DSI transactions
+ * @data: Buffer containing data to be transmitted
+ * @len: Size of transmission buffer
+ *
+ * A wrapper around mipi_dsi_dcs_write_buffer_multi() that allows the user to
+ * conveniently write to two dsi channels, one after the other.
+ */
+void mipi_dsi_dual_dcs_write_buffer_multi(struct mipi_dsi_device *dsi1,
+ struct mipi_dsi_device *dsi2,
+ struct mipi_dsi_multi_context *ctx,
+ const void *data, size_t len)
+{
+ ctx->dsi = dsi1;
+ mipi_dsi_dcs_write_buffer_multi(ctx, data, len);
+ ctx->dsi = dsi2;
+ mipi_dsi_dcs_write_buffer_multi(ctx, data, len);
+}
+EXPORT_SYMBOL(mipi_dsi_dual_dcs_write_buffer_multi);
+
/**
* mipi_dsi_dcs_write() - send DCS write command
* @dsi: DSI peripheral device
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 369b0d8830c3..ffdfcb57cbd4 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -289,6 +289,10 @@ int mipi_dsi_generic_write_chatty(struct mipi_dsi_device *dsi,
const void *payload, size_t size);
void mipi_dsi_generic_write_multi(struct mipi_dsi_multi_context *ctx,
const void *payload, size_t size);
+void mipi_dsi_dual_generic_write_multi(struct mipi_dsi_device *dsi1,
+ struct mipi_dsi_device *dsi2,
+ struct mipi_dsi_multi_context *ctx,
+ const void *payload, size_t size);
ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi, const void *params,
size_t num_params, void *data, size_t size);
u32 drm_mipi_dsi_get_input_bus_fmt(enum mipi_dsi_pixel_format dsi_format);
@@ -329,6 +333,10 @@ int mipi_dsi_dcs_write_buffer_chatty(struct mipi_dsi_device *dsi,
const void *data, size_t len);
void mipi_dsi_dcs_write_buffer_multi(struct mipi_dsi_multi_context *ctx,
const void *data, size_t len);
+void mipi_dsi_dual_dcs_write_buffer_multi(struct mipi_dsi_device *dsi1,
+ struct mipi_dsi_device *dsi2,
+ struct mipi_dsi_multi_context *ctx,
+ const void *data, size_t len);
ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
const void *data, size_t len);
ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data,
@@ -431,6 +439,87 @@ void mipi_dsi_dcs_set_tear_off_multi(struct mipi_dsi_multi_context *ctx);
mipi_dsi_dcs_write_buffer_multi(ctx, d, ARRAY_SIZE(d)); \
} while (0)
+/**
+ * mipi_dsi_dual - send the same MIPI DSI command to two interfaces
+ *
+ * This macro will send the specified MIPI DSI command twice, once per each of
+ * the two interfaces supplied. This is useful for reducing duplication of code
+ * in panel drivers which use two parallel serial interfaces.
+ *
+ * WARNING: This macro reuses the _func argument and the optional trailing
+ * arguments twice each, which may cause unintended side effects. For example,
+ * adding the postfix increment ++ operator to one of the arguments to be
+ * passed to _func will cause the variable to be incremented twice instead of
+ * once and the variable will be its original value + 1 when sent to _dsi2.
+ *
+ * @_func: MIPI DSI function or macro to pass context and arguments into
+ * @_dsi1: First DSI interface to act as recipient of the MIPI DSI command
+ * @_dsi2: Second DSI interface to act as recipient of the MIPI DSI command
+ * @_ctx: Context for multiple DSI transactions
+ * @...: Arguments to pass to MIPI DSI function or macro
+ */
+#define mipi_dsi_dual(_func, _dsi1, _dsi2, _ctx, ...) \
+ do { \
+ struct mipi_dsi_multi_context *_ctxcpy = (_ctx); \
+ (_ctxcpy)->dsi = (_dsi1); \
+ (_func)((_ctxcpy), ##__VA_ARGS__); \
+ (_ctxcpy)->dsi = (_dsi2); \
+ (_func)((_ctxcpy), ##__VA_ARGS__); \
+ } while (0)
+
+/**
+ * mipi_dsi_dual_generic_write_seq_multi - transmit data using a generic write
+ * packet to two dsi interfaces, one after the other
+ *
+ * This macro will send the specified generic packet twice, once per each of
+ * the two interfaces supplied. This is useful for reducing duplication of code
+ * in panel drivers which use two parallel serial interfaces.
+ *
+ * Note that if an error occurs while transmitting the packet to the first DSI
+ * interface, the packet will not be sent to the second DSI interface.
+ *
+ * This macro will print errors for you and error handling is optimized for
+ * callers that call this multiple times in a row.
+ *
+ * @_dsi1: First DSI interface to act as recipient of packet
+ * @_dsi2: Second DSI interface to act as recipient of packet
+ * @_ctx: Context for multiple DSI transactions
+ * @_seq: buffer containing the payload
+ */
+#define mipi_dsi_dual_generic_write_seq_multi(_dsi1, _dsi2, _ctx, _seq...) \
+ do { \
+ static const u8 d[] = { _seq }; \
+ mipi_dsi_dual_generic_write_multi(_dsi1, _dsi2, _ctx, d, \
+ ARRAY_SIZE(d)); \
+ } while (0)
+
+/**
+ * mipi_dsi_dual_dcs_write_seq_multi - transmit a DCS command with payload to
+ * two dsi interfaces, one after the other
+ *
+ * This macro will send the specified DCS command with payload twice, once per
+ * each of the two interfaces supplied. This is useful for reducing duplication
+ * of code in panel drivers which use two parallel serial interfaces.
+ *
+ * Note that if an error occurs while transmitting the payload to the first DSI
+ * interface, the payload will not be sent to the second DSI interface.
+ *
+ * This macro will print errors for you and error handling is optimized for
+ * callers that call this multiple times in a row.
+ *
+ * @_dsi1: First DSI interface to act as recipient of packet
+ * @_dsi2: Second DSI interface to act as recipient of packet
+ * @_ctx: Context for multiple DSI transactions
+ * @_cmd: Command
+ * @_seq: buffer containing the payload
+ */
+#define mipi_dsi_dual_dcs_write_seq_multi(_dsi1, _dsi2, _ctx, _cmd, _seq) \
+ do { \
+ static const u8 d[] = { _cmd, _seq }; \
+ mipi_dsi_dual_dcs_write_buffer_multi(_dsi1, _dsi2, _ctx, d, \
+ ARRAY_SIZE(d)); \
+ } while (0)
+
/**
* struct mipi_dsi_driver - DSI driver
* @driver: device driver model driver
--
2.50.1Hi,
On Sat, Jul 19, 2025 at 1:27 AM Brigham Campbell <me@brighamcampbell.com> wrote:
>
> @@ -827,6 +827,30 @@ void mipi_dsi_generic_write_multi(struct mipi_dsi_multi_context *ctx,
> }
> EXPORT_SYMBOL(mipi_dsi_generic_write_multi);
>
> +/**
> + * mipi_dsi_dual_generic_write_multi() - mipi_dsi_generic_write_multi() for
> + * two dsi channels, one after the other
> + * @dsi1: First dsi channel to write buffer to
> + * @dsi2: Second dsi channel to write buffer to
> + * @ctx: Context for multiple DSI transactions
> + * @payload: Buffer containing the payload
> + * @size: Size of payload buffer
> + *
> + * A wrapper around mipi_dsi_generic_write_multi() that allows the user to
> + * conveniently write to two dsi channels, one after the other.
> + */
> +void mpi_dsi_dual_generic_write_multi(struct mipi_dsi_device *dsi1,
BUG: above should be "mipi", not "mpi"
> + struct mipi_dsi_device *dsi2,
> + struct mipi_dsi_multi_context *ctx,
> + const void *payload, size_t size)
> +{
> + ctx->dsi = dsi1;
> + mipi_dsi_generic_write_multi(ctx, data, len);
BUG: "data" and "len" are not valid local variables...
> @@ -431,6 +439,87 @@ void mipi_dsi_dcs_set_tear_off_multi(struct mipi_dsi_multi_context *ctx);
> mipi_dsi_dcs_write_buffer_multi(ctx, d, ARRAY_SIZE(d)); \
> } while (0)
>
> +/**
> + * mipi_dsi_dual - send the same MIPI DSI command to two interfaces
> + *
> + * This macro will send the specified MIPI DSI command twice, once per each of
> + * the two interfaces supplied. This is useful for reducing duplication of code
> + * in panel drivers which use two parallel serial interfaces.
> + *
> + * WARNING: This macro reuses the _func argument and the optional trailing
> + * arguments twice each, which may cause unintended side effects. For example,
> + * adding the postfix increment ++ operator to one of the arguments to be
> + * passed to _func will cause the variable to be incremented twice instead of
> + * once and the variable will be its original value + 1 when sent to _dsi2.
It could be worth also pointing people to
mipi_dsi_dual_generic_write_seq_multi() and
mipi_dsi_dual_dcs_write_seq_multi() below?
> + *
> + * @_func: MIPI DSI function or macro to pass context and arguments into
nit: remove "or macro".
> + * @_dsi1: First DSI interface to act as recipient of the MIPI DSI command
> + * @_dsi2: Second DSI interface to act as recipient of the MIPI DSI command
> + * @_ctx: Context for multiple DSI transactions
> + * @...: Arguments to pass to MIPI DSI function or macro
> + */
> +#define mipi_dsi_dual(_func, _dsi1, _dsi2, _ctx, ...) \
> + do { \
> + struct mipi_dsi_multi_context *_ctxcpy = (_ctx); \
> + (_ctxcpy)->dsi = (_dsi1); \
nit: now that "_ctxcpy" is a local variable you no longer need the
extra parenthesis around it.
> + (_func)((_ctxcpy), ##__VA_ARGS__); \
> + (_ctxcpy)->dsi = (_dsi2); \
> + (_func)((_ctxcpy), ##__VA_ARGS__); \
> + } while (0)
> +
> +/**
> + * mipi_dsi_dual_generic_write_seq_multi - transmit data using a generic write
> + * packet to two dsi interfaces, one after the other
> + *
> + * This macro will send the specified generic packet twice, once per each of
> + * the two interfaces supplied. This is useful for reducing duplication of code
> + * in panel drivers which use two parallel serial interfaces.
> + *
> + * Note that if an error occurs while transmitting the packet to the first DSI
> + * interface, the packet will not be sent to the second DSI interface.
> + *
> + * This macro will print errors for you and error handling is optimized for
> + * callers that call this multiple times in a row.
> + *
> + * @_dsi1: First DSI interface to act as recipient of packet
> + * @_dsi2: Second DSI interface to act as recipient of packet
> + * @_ctx: Context for multiple DSI transactions
> + * @_seq: buffer containing the payload
> + */
> +#define mipi_dsi_dual_generic_write_seq_multi(_dsi1, _dsi2, _ctx, _seq...) \
> + do { \
> + static const u8 d[] = { _seq }; \
> + mipi_dsi_dual_generic_write_multi(_dsi1, _dsi2, _ctx, d, \
> + ARRAY_SIZE(d)); \
nit: the indentation of ARRAY_SIZE() is slightly off.
> + } while (0)
> +
> +/**
> + * mipi_dsi_dual_dcs_write_seq_multi - transmit a DCS command with payload to
> + * two dsi interfaces, one after the other
> + *
> + * This macro will send the specified DCS command with payload twice, once per
> + * each of the two interfaces supplied. This is useful for reducing duplication
> + * of code in panel drivers which use two parallel serial interfaces.
> + *
> + * Note that if an error occurs while transmitting the payload to the first DSI
> + * interface, the payload will not be sent to the second DSI interface.
> + *
> + * This macro will print errors for you and error handling is optimized for
> + * callers that call this multiple times in a row.
> + *
> + * @_dsi1: First DSI interface to act as recipient of packet
> + * @_dsi2: Second DSI interface to act as recipient of packet
> + * @_ctx: Context for multiple DSI transactions
> + * @_cmd: Command
> + * @_seq: buffer containing the payload
> + */
> +#define mipi_dsi_dual_dcs_write_seq_multi(_dsi1, _dsi2, _ctx, _cmd, _seq) \
BUG: doesn't "_seq" need to be "_seq..." ?
BUG: You need to remove the definition of this macro from
`panel-novatek-nt36523.c` or else it won't compile anymore since the
name of your macro is the exact same as theirs and they include this
header file. It would be OK w/ me if you squashed that into the same
patch since otherwise rejiggering things would just be churn...
I guess we also chose different argument orders than they did (that's
probably my fault, sorry!). They had the "ctx" still first and this
patch consistently has "dsi1" and "dsi2" first. I don't think it
really matters, but we should be consistent which means either
adjusting your patch or theirs. It's probably worth confirming that
the novatek driver at least compiles before you submit v6.
-Doug
On Mon Jul 21, 2025 at 10:30 AM MDT, Doug Anderson wrote:
>> +void mpi_dsi_dual_generic_write_multi(struct mipi_dsi_device *dsi1,
>
> BUG: above should be "mipi", not "mpi"
>
>> + struct mipi_dsi_device *dsi2,
>> + struct mipi_dsi_multi_context *ctx,
>> + const void *payload, size_t size)
>> +{
>> + ctx->dsi = dsi1;
>> + mipi_dsi_generic_write_multi(ctx, data, len);
>
> BUG: "data" and "len" are not valid local variables...
>
>> + * mipi_dsi_dual - send the same MIPI DSI command to two interfaces
>
> It could be worth also pointing people to
> mipi_dsi_dual_generic_write_seq_multi() and
> mipi_dsi_dual_dcs_write_seq_multi() below?
>
>> + * @_func: MIPI DSI function or macro to pass context and arguments into
>
> nit: remove "or macro".
>
>> + struct mipi_dsi_multi_context *_ctxcpy = (_ctx); \
>> + (_ctxcpy)->dsi = (_dsi1); \
>
> nit: now that "_ctxcpy" is a local variable you no longer need the
> extra parenthesis around it.
>
>> + mipi_dsi_dual_generic_write_multi(_dsi1, _dsi2, _ctx, d, \
>> + ARRAY_SIZE(d)); \
>
> nit: the indentation of ARRAY_SIZE() is slightly off.
>
>> +#define mipi_dsi_dual_dcs_write_seq_multi(_dsi1, _dsi2, _ctx, _cmd, _seq) \
>
> BUG: doesn't "_seq" need to be "_seq..." ?
>
> BUG: You need to remove the definition of this macro from
> `panel-novatek-nt36523.c` or else it won't compile anymore since the
> name of your macro is the exact same as theirs and they include this
> header file. It would be OK w/ me if you squashed that into the same
> patch since otherwise rejiggering things would just be churn...
Sorry to have sent out such a poor quality patch, Doug! I always compile
changed files and test my changes when I can, but I think I must have
compiled just the lpm102a188a panel C source file itself by mistake when
I sent out this series revision. From now on, I'll simply enable the
relevant kernel config options and rebuild the entire kernel.
I'll address each of these items in v6.
> I guess we also chose different argument orders than they did (that's
> probably my fault, sorry!). They had the "ctx" still first and this
> patch consistently has "dsi1" and "dsi2" first. I don't think it
> really matters, but we should be consistent which means either
> adjusting your patch or theirs. It's probably worth confirming that
> the novatek driver at least compiles before you submit v6.
No, this was my fault. You had suggested the correct order. When I
implemented the change, I preferred to put `ctx` after `dsi1` and `dsi2`
because that's what I had done when I implemented the mipi_dsi_dual
macro. I'll swap up the order, remove the function definition from the
novatek driver, and compile both lpm102a188a and the novatek driver
before sending out v6.
By the way, we can discuss this further when I've sent out v6, but the
novatek driver appears to pass a mipi_dsi_context struct into the
write_seq_multi macro directly instead of a mipi_dsi_context struct
pointer. We opted to use a pointer in this patch series so that it can
be passed to a function in order to reduce the compiled size of drivers.
For now, I'll plan to solve this by changing calls to write_seq_multi in
the novatek driver to pass a pointer. I hope that the churn that this
will cause in the novatek driver isn't unacceptable.
Thanks for your patience,
Brigham
Hi,
On Mon, Jul 21, 2025 at 5:43 PM Brigham Campbell <me@brighamcampbell.com> wrote:
>
> On Mon Jul 21, 2025 at 10:30 AM MDT, Doug Anderson wrote:
> >> +void mpi_dsi_dual_generic_write_multi(struct mipi_dsi_device *dsi1,
> >
> > BUG: above should be "mipi", not "mpi"
> >
> >> + struct mipi_dsi_device *dsi2,
> >> + struct mipi_dsi_multi_context *ctx,
> >> + const void *payload, size_t size)
> >> +{
> >> + ctx->dsi = dsi1;
> >> + mipi_dsi_generic_write_multi(ctx, data, len);
> >
> > BUG: "data" and "len" are not valid local variables...
> >
> >> + * mipi_dsi_dual - send the same MIPI DSI command to two interfaces
> >
> > It could be worth also pointing people to
> > mipi_dsi_dual_generic_write_seq_multi() and
> > mipi_dsi_dual_dcs_write_seq_multi() below?
> >
> >> + * @_func: MIPI DSI function or macro to pass context and arguments into
> >
> > nit: remove "or macro".
> >
> >> + struct mipi_dsi_multi_context *_ctxcpy = (_ctx); \
> >> + (_ctxcpy)->dsi = (_dsi1); \
> >
> > nit: now that "_ctxcpy" is a local variable you no longer need the
> > extra parenthesis around it.
> >
> >> + mipi_dsi_dual_generic_write_multi(_dsi1, _dsi2, _ctx, d, \
> >> + ARRAY_SIZE(d)); \
> >
> > nit: the indentation of ARRAY_SIZE() is slightly off.
> >
> >> +#define mipi_dsi_dual_dcs_write_seq_multi(_dsi1, _dsi2, _ctx, _cmd, _seq) \
> >
> > BUG: doesn't "_seq" need to be "_seq..." ?
> >
> > BUG: You need to remove the definition of this macro from
> > `panel-novatek-nt36523.c` or else it won't compile anymore since the
> > name of your macro is the exact same as theirs and they include this
> > header file. It would be OK w/ me if you squashed that into the same
> > patch since otherwise rejiggering things would just be churn...
>
> Sorry to have sent out such a poor quality patch, Doug! I always compile
> changed files and test my changes when I can, but I think I must have
> compiled just the lpm102a188a panel C source file itself by mistake when
> I sent out this series revision. From now on, I'll simply enable the
> relevant kernel config options and rebuild the entire kernel.
>
> I'll address each of these items in v6.
Don't worry too much about it. While it's good to make sure you test
your patches, everyone makes mistakes! :-)
> > I guess we also chose different argument orders than they did (that's
> > probably my fault, sorry!). They had the "ctx" still first and this
> > patch consistently has "dsi1" and "dsi2" first. I don't think it
> > really matters, but we should be consistent which means either
> > adjusting your patch or theirs. It's probably worth confirming that
> > the novatek driver at least compiles before you submit v6.
>
> No, this was my fault. You had suggested the correct order. When I
> implemented the change, I preferred to put `ctx` after `dsi1` and `dsi2`
> because that's what I had done when I implemented the mipi_dsi_dual
> macro. I'll swap up the order, remove the function definition from the
> novatek driver, and compile both lpm102a188a and the novatek driver
> before sending out v6.
>
> By the way, we can discuss this further when I've sent out v6, but the
> novatek driver appears to pass a mipi_dsi_context struct into the
> write_seq_multi macro directly instead of a mipi_dsi_context struct
> pointer. We opted to use a pointer in this patch series so that it can
> be passed to a function in order to reduce the compiled size of drivers.
> For now, I'll plan to solve this by changing calls to write_seq_multi in
> the novatek driver to pass a pointer. I hope that the churn that this
> will cause in the novatek driver isn't unacceptable.
Looks fine. It probably should have been a pointer in the novatek
driver to begin with, but when it was a macro it didn't really matter.
-Doug
On Sat, Jul 19, 2025 at 02:26:35AM -0600, Brigham Campbell wrote: > Create mipi_dsi_dual, mipi_dsi_dual_dcs_write_seq_multi, and > mipi_dsi_dual_generic_write_seq_multi macros for panels which are driven > by two parallel serial interfaces. This allows for the reduction of code > duplication in drivers for these panels. > > Signed-off-by: Brigham Campbell <me@brighamcampbell.com> > --- > > mipi_dsi_dual_dcs_write_seq_multi goes unused by jdi-lpm102a188a. It's > included in this patch for completeness and in anticipation of its use > in other drivers in the future. > > drivers/gpu/drm/drm_mipi_dsi.c | 48 ++++++++++++++++++ > include/drm/drm_mipi_dsi.h | 89 ++++++++++++++++++++++++++++++++++ > 2 files changed, 137 insertions(+) > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> -- With best wishes Dmitry
© 2016 - 2025 Red Hat, Inc.