[v4,01/16] drm/dsi: Add message to packet translator

Commit Message

Thierry Reding Nov. 3, 2014, 9:13 a.m. UTC

From: Thierry Reding <[e-mail]>

This commit introduces a new function, mipi_dsi_create_packet(), which
converts from a MIPI DSI message to a MIPI DSI packet. The MIPI DSI
packet is as close to the protocol described in the DSI specification as
possible and useful in drivers that need to write a DSI packet into a
FIFO to send a message off to the peripheral.

Suggested-by: Andrzej Hajda <[e-mail]>
Signed-off-by: Thierry Reding <[e-mail]>
---
 drivers/gpu/drm/drm_mipi_dsi.c | 45 ++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_mipi_dsi.h     | 18 +++++++++++++++++
 2 files changed, 63 insertions(+)

Comments

Andrzej Hajda Nov. 4, 2014, 11:43 a.m. UTC | #1

On 11/03/2014 10:13 AM, Thierry Reding wrote:
> From: Thierry Reding <[e-mail]>
>
> This commit introduces a new function, mipi_dsi_create_packet(), which
> converts from a MIPI DSI message to a MIPI DSI packet. The MIPI DSI
> packet is as close to the protocol described in the DSI specification as
> possible and useful in drivers that need to write a DSI packet into a
> FIFO to send a message off to the peripheral.
>
> Suggested-by: Andrzej Hajda <[e-mail]>
> Signed-off-by: Thierry Reding <[e-mail]>
> ---
>  drivers/gpu/drm/drm_mipi_dsi.c | 45 ++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_mipi_dsi.h     | 18 +++++++++++++++++
>  2 files changed, 63 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index eb6dfe52cab2..76e81aba8220 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -199,6 +199,51 @@ int mipi_dsi_detach(struct mipi_dsi_device *dsi)
>  EXPORT_SYMBOL(mipi_dsi_detach);
>  
>  /**
> + * mipi_dsi_create_packet - create a packet from a message according to the
> + *     DSI protocol
> + * @packet: pointer to a DSI packet structure
> + * @msg: message to translate into a packet
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int mipi_dsi_create_packet(struct mipi_dsi_packet *packet,
> +			   const struct mipi_dsi_msg *msg)
> +{
> +	const u8 *tx = msg->tx_buf;
> +
> +	if (!packet || !msg)
> +		return -EINVAL;
> +
> +	memset(packet, 0, sizeof(*packet));
> +	packet->header[0] = ((msg->channel & 0x3) << 6) | (msg->type & 0x3f);
> +
> +	/* TODO: compute ECC if hardware support is not available */
> +
> +	/*
> +	 * Long write packets contain the word count in header bytes 1 and 2.
> +	 * The payload follows the header and is word count bytes long.
> +	 *
> +	 * Short write packets encode up to two parameters in header bytes 1
> +	 * and 2.
> +	 */
> +	if (msg->tx_len > 2) {

This is incorrect, you can have long packet of payload length 0, look for
"zero-byte Data Payload" phrase. I think you should check msg->type here.

I have used:

static bool exynos_dsi_is_short_dsi_type(u8 type)
{
    return (type & 0x0f) <= 8;
}

quite ugly, but works :)

> +		packet->header[1] = (msg->tx_len >> 0) & 0xff;
> +		packet->header[2] = (msg->tx_len >> 8) & 0xff;
> +
> +		packet->payload_length = msg->tx_len;
> +		packet->payload = tx;
> +	} else {
> +		packet->header[1] = (msg->tx_len > 0) ? tx[0] : 0;
> +		packet->header[2] = (msg->tx_len > 1) ? tx[1] : 0;
> +	}
> +
> +	packet->size = sizeof(packet->header) + packet->payload_length;

size seems to be completely useless.

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(mipi_dsi_create_packet);
> +
> +/**
>   * mipi_dsi_dcs_write - send DCS write command
>   * @dsi: DSI device
>   * @data: pointer to the command followed by parameters
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 8569dc5a1026..663aa68826f4 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -44,6 +44,24 @@ struct mipi_dsi_msg {
>  };
>  
>  /**
> + * struct mipi_dsi_packet - represents a MIPI DSI packet in protocol format
> + * @size: size (in bytes) of the packet
> + * @header: the four bytes that make up the header (Data ID, Word Count or
> + *     Packet Data, and ECC)
> + * @payload_length: number of bytes in the payload
> + * @payload: a pointer to a buffer containing the payload, if any
> + */
> +struct mipi_dsi_packet {
> +	size_t size;
> +	u8 header[4];

I wonder if it wouldn't be good to make it u32 or at least anonymous union:
union {
    u8 header[4];
    u32 header32;
};

And of course we should document its endiannes.
This way we can use le16_to_cpu(pkt->header32) instead of constructing
u32 value from array.

Regards
Andrzej

> +	size_t payload_length;
> +	const u8 *payload;
> +};
> +
> +int mipi_dsi_create_packet(struct mipi_dsi_packet *packet,
> +			   const struct mipi_dsi_msg *msg);
> +
> +/**
>   * struct mipi_dsi_host_ops - DSI bus operations
>   * @attach: attach DSI device to DSI host
>   * @detach: detach DSI device from DSI host

Thierry Reding Nov. 4, 2014, 1:58 p.m. UTC | #2

On Tue, Nov 04, 2014 at 12:43:21PM +0100, Andrzej Hajda wrote:
> On 11/03/2014 10:13 AM, Thierry Reding wrote:
> > From: Thierry Reding <[e-mail]>
> >
> > This commit introduces a new function, mipi_dsi_create_packet(), which
> > converts from a MIPI DSI message to a MIPI DSI packet. The MIPI DSI
> > packet is as close to the protocol described in the DSI specification as
> > possible and useful in drivers that need to write a DSI packet into a
> > FIFO to send a message off to the peripheral.
> >
> > Suggested-by: Andrzej Hajda <[e-mail]>
> > Signed-off-by: Thierry Reding <[e-mail]>
> > ---
> >  drivers/gpu/drm/drm_mipi_dsi.c | 45 ++++++++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_mipi_dsi.h     | 18 +++++++++++++++++
> >  2 files changed, 63 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> > index eb6dfe52cab2..76e81aba8220 100644
> > --- a/drivers/gpu/drm/drm_mipi_dsi.c
> > +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> > @@ -199,6 +199,51 @@ int mipi_dsi_detach(struct mipi_dsi_device *dsi)
> >  EXPORT_SYMBOL(mipi_dsi_detach);
> >  
> >  /**
> > + * mipi_dsi_create_packet - create a packet from a message according to the
> > + *     DSI protocol
> > + * @packet: pointer to a DSI packet structure
> > + * @msg: message to translate into a packet
> > + *
> > + * Return: 0 on success or a negative error code on failure.
> > + */
> > +int mipi_dsi_create_packet(struct mipi_dsi_packet *packet,
> > +			   const struct mipi_dsi_msg *msg)
> > +{
> > +	const u8 *tx = msg->tx_buf;
> > +
> > +	if (!packet || !msg)
> > +		return -EINVAL;
> > +
> > +	memset(packet, 0, sizeof(*packet));
> > +	packet->header[0] = ((msg->channel & 0x3) << 6) | (msg->type & 0x3f);
> > +
> > +	/* TODO: compute ECC if hardware support is not available */
> > +
> > +	/*
> > +	 * Long write packets contain the word count in header bytes 1 and 2.
> > +	 * The payload follows the header and is word count bytes long.
> > +	 *
> > +	 * Short write packets encode up to two parameters in header bytes 1
> > +	 * and 2.
> > +	 */
> > +	if (msg->tx_len > 2) {
> 
> This is incorrect, you can have long packet of payload length 0, look for
> "zero-byte Data Payload" phrase. I think you should check msg->type here.
> 
> I have used:
> 
> static bool exynos_dsi_is_short_dsi_type(u8 type)
> {
>     return (type & 0x0f) <= 8;
> }
> 
> quite ugly, but works :)

That would falsely return true for unspecified data types, too. I'll go
with a variant that uses an explicit switch.

> > +		packet->header[1] = (msg->tx_len >> 0) & 0xff;
> > +		packet->header[2] = (msg->tx_len >> 8) & 0xff;
> > +
> > +		packet->payload_length = msg->tx_len;
> > +		packet->payload = tx;
> > +	} else {
> > +		packet->header[1] = (msg->tx_len > 0) ? tx[0] : 0;
> > +		packet->header[2] = (msg->tx_len > 1) ? tx[1] : 0;
> > +	}
> > +
> > +	packet->size = sizeof(packet->header) + packet->payload_length;
> 
> size seems to be completely useless.

It's not. Tegra has two FIFOs that can be selected depending on the size
of a transfer. I use this field to detect which FIFO needs to be
selected.

> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(mipi_dsi_create_packet);
> > +
> > +/**
> >   * mipi_dsi_dcs_write - send DCS write command
> >   * @dsi: DSI device
> >   * @data: pointer to the command followed by parameters
> > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> > index 8569dc5a1026..663aa68826f4 100644
> > --- a/include/drm/drm_mipi_dsi.h
> > +++ b/include/drm/drm_mipi_dsi.h
> > @@ -44,6 +44,24 @@ struct mipi_dsi_msg {
> >  };
> >  
> >  /**
> > + * struct mipi_dsi_packet - represents a MIPI DSI packet in protocol format
> > + * @size: size (in bytes) of the packet
> > + * @header: the four bytes that make up the header (Data ID, Word Count or
> > + *     Packet Data, and ECC)
> > + * @payload_length: number of bytes in the payload
> > + * @payload: a pointer to a buffer containing the payload, if any
> > + */
> > +struct mipi_dsi_packet {
> > +	size_t size;
> > +	u8 header[4];
> 
> I wonder if it wouldn't be good to make it u32 or at least anonymous union:
> union {
>     u8 header[4];
>     u32 header32;
> };

I'm not sure this is very useful. It's pretty trivial how you
concatenate the individual bytes and it actually remove any ambiguity
about the endianness.

> And of course we should document its endiannes.

The endianness is already documented in the kerneldoc, isn't it? Data ID
followed by Word Count (long packets) or Packet Data (short packets) and
finally the ECC byte. That's the ordering defined in the specification,
so I think it's fairly obvious.

Thierry

Andrzej Hajda Nov. 4, 2014, 2:21 p.m. UTC | #3

On 11/04/2014 02:58 PM, Thierry Reding wrote:
> On Tue, Nov 04, 2014 at 12:43:21PM +0100, Andrzej Hajda wrote:
>> On 11/03/2014 10:13 AM, Thierry Reding wrote:
>>> From: Thierry Reding <[e-mail]>
>>>
>>> This commit introduces a new function, mipi_dsi_create_packet(), which
>>> converts from a MIPI DSI message to a MIPI DSI packet. The MIPI DSI
>>> packet is as close to the protocol described in the DSI specification as
>>> possible and useful in drivers that need to write a DSI packet into a
>>> FIFO to send a message off to the peripheral.
>>>
>>> Suggested-by: Andrzej Hajda <[e-mail]>
>>> Signed-off-by: Thierry Reding <[e-mail]>
>>> ---
>>>  drivers/gpu/drm/drm_mipi_dsi.c | 45 ++++++++++++++++++++++++++++++++++++++++++
>>>  include/drm/drm_mipi_dsi.h     | 18 +++++++++++++++++
>>>  2 files changed, 63 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
>>> index eb6dfe52cab2..76e81aba8220 100644
>>> --- a/drivers/gpu/drm/drm_mipi_dsi.c
>>> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
>>> @@ -199,6 +199,51 @@ int mipi_dsi_detach(struct mipi_dsi_device *dsi)
>>>  EXPORT_SYMBOL(mipi_dsi_detach);
>>>  
>>>  /**
>>> + * mipi_dsi_create_packet - create a packet from a message according to the
>>> + *     DSI protocol
>>> + * @packet: pointer to a DSI packet structure
>>> + * @msg: message to translate into a packet
>>> + *
>>> + * Return: 0 on success or a negative error code on failure.
>>> + */
>>> +int mipi_dsi_create_packet(struct mipi_dsi_packet *packet,
>>> +			   const struct mipi_dsi_msg *msg)
>>> +{
>>> +	const u8 *tx = msg->tx_buf;
>>> +
>>> +	if (!packet || !msg)
>>> +		return -EINVAL;
>>> +
>>> +	memset(packet, 0, sizeof(*packet));
>>> +	packet->header[0] = ((msg->channel & 0x3) << 6) | (msg->type & 0x3f);
>>> +
>>> +	/* TODO: compute ECC if hardware support is not available */
>>> +
>>> +	/*
>>> +	 * Long write packets contain the word count in header bytes 1 and 2.
>>> +	 * The payload follows the header and is word count bytes long.
>>> +	 *
>>> +	 * Short write packets encode up to two parameters in header bytes 1
>>> +	 * and 2.
>>> +	 */
>>> +	if (msg->tx_len > 2) {
>> This is incorrect, you can have long packet of payload length 0, look for
>> "zero-byte Data Payload" phrase. I think you should check msg->type here.
>>
>> I have used:
>>
>> static bool exynos_dsi_is_short_dsi_type(u8 type)
>> {
>>     return (type & 0x0f) <= 8;
>> }
>>
>> quite ugly, but works :)
> That would falsely return true for unspecified data types, too. I'll go
> with a variant that uses an explicit switch.

Sounds better.

>
>>> +		packet->header[1] = (msg->tx_len >> 0) & 0xff;
>>> +		packet->header[2] = (msg->tx_len >> 8) & 0xff;
>>> +
>>> +		packet->payload_length = msg->tx_len;
>>> +		packet->payload = tx;
>>> +	} else {
>>> +		packet->header[1] = (msg->tx_len > 0) ? tx[0] : 0;
>>> +		packet->header[2] = (msg->tx_len > 1) ? tx[1] : 0;
>>> +	}
>>> +
>>> +	packet->size = sizeof(packet->header) + packet->payload_length;
>> size seems to be completely useless.
> It's not. Tegra has two FIFOs that can be selected depending on the size
> of a transfer. I use this field to detect which FIFO needs to be
> selected.

But size is always equal payload_length + 4, so it does not carry any
additional information.

>
>>> +
>>> +	return 0;
>>> +}
>>> +EXPORT_SYMBOL(mipi_dsi_create_packet);
>>> +
>>> +/**
>>>   * mipi_dsi_dcs_write - send DCS write command
>>>   * @dsi: DSI device
>>>   * @data: pointer to the command followed by parameters
>>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>>> index 8569dc5a1026..663aa68826f4 100644
>>> --- a/include/drm/drm_mipi_dsi.h
>>> +++ b/include/drm/drm_mipi_dsi.h
>>> @@ -44,6 +44,24 @@ struct mipi_dsi_msg {
>>>  };
>>>  
>>>  /**
>>> + * struct mipi_dsi_packet - represents a MIPI DSI packet in protocol format
>>> + * @size: size (in bytes) of the packet
>>> + * @header: the four bytes that make up the header (Data ID, Word Count or
>>> + *     Packet Data, and ECC)
>>> + * @payload_length: number of bytes in the payload
>>> + * @payload: a pointer to a buffer containing the payload, if any
>>> + */
>>> +struct mipi_dsi_packet {
>>> +	size_t size;
>>> +	u8 header[4];
>> I wonder if it wouldn't be good to make it u32 or at least anonymous union:
>> union {
>>     u8 header[4];
>>     u32 header32;
>> };
> I'm not sure this is very useful. It's pretty trivial how you
> concatenate the individual bytes and it actually remove any ambiguity
> about the endianness.

This looks better:

val = le16_to_cpu(pkt->header32);
writel(val, REG);

than this:

val = pkt->header[2] << 16 | pkt->header[1] << 8 | pkt->header[0];
writel(val, REG);

But it is just a nitpick.

Regards
Andrzej

>
>> And of course we should document its endiannes.
> The endianness is already documented in the kerneldoc, isn't it? Data ID
> followed by Word Count (long packets) or Packet Data (short packets) and
> finally the ECC byte. That's the ordering defined in the specification,
> so I think it's fairly obvious.
>
> Thierry

Thierry Reding Nov. 4, 2014, 2:39 p.m. UTC | #4

On Tue, Nov 04, 2014 at 03:21:14PM +0100, Andrzej Hajda wrote:
> On 11/04/2014 02:58 PM, Thierry Reding wrote:
> > On Tue, Nov 04, 2014 at 12:43:21PM +0100, Andrzej Hajda wrote:
> >> On 11/03/2014 10:13 AM, Thierry Reding wrote:
[...]
> >>> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
[...]
> >>> +		packet->header[1] = (msg->tx_len >> 0) & 0xff;
> >>> +		packet->header[2] = (msg->tx_len >> 8) & 0xff;
> >>> +
> >>> +		packet->payload_length = msg->tx_len;
> >>> +		packet->payload = tx;
> >>> +	} else {
> >>> +		packet->header[1] = (msg->tx_len > 0) ? tx[0] : 0;
> >>> +		packet->header[2] = (msg->tx_len > 1) ? tx[1] : 0;
> >>> +	}
> >>> +
> >>> +	packet->size = sizeof(packet->header) + packet->payload_length;
> >> size seems to be completely useless.
> > It's not. Tegra has two FIFOs that can be selected depending on the size
> > of a transfer. I use this field to detect which FIFO needs to be
> > selected.
> 
> But size is always equal payload_length + 4, so it does not carry any
> additional information.

Right, but it's out of convenience to prevent every driver from doing
this again. It's part of the help that the helper provides. =)

> >>> +
> >>> +	return 0;
> >>> +}
> >>> +EXPORT_SYMBOL(mipi_dsi_create_packet);
> >>> +
> >>> +/**
> >>>   * mipi_dsi_dcs_write - send DCS write command
> >>>   * @dsi: DSI device
> >>>   * @data: pointer to the command followed by parameters
> >>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> >>> index 8569dc5a1026..663aa68826f4 100644
> >>> --- a/include/drm/drm_mipi_dsi.h
> >>> +++ b/include/drm/drm_mipi_dsi.h
> >>> @@ -44,6 +44,24 @@ struct mipi_dsi_msg {
> >>>  };
> >>>  
> >>>  /**
> >>> + * struct mipi_dsi_packet - represents a MIPI DSI packet in protocol format
> >>> + * @size: size (in bytes) of the packet
> >>> + * @header: the four bytes that make up the header (Data ID, Word Count or
> >>> + *     Packet Data, and ECC)
> >>> + * @payload_length: number of bytes in the payload
> >>> + * @payload: a pointer to a buffer containing the payload, if any
> >>> + */
> >>> +struct mipi_dsi_packet {
> >>> +	size_t size;
> >>> +	u8 header[4];
> >> I wonder if it wouldn't be good to make it u32 or at least anonymous union:
> >> union {
> >>     u8 header[4];
> >>     u32 header32;
> >> };
> > I'm not sure this is very useful. It's pretty trivial how you
> > concatenate the individual bytes and it actually remove any ambiguity
> > about the endianness.
> 
> This looks better:
> 
> val = le16_to_cpu(pkt->header32);
> writel(val, REG);
> 
> than this:
> 
> val = pkt->header[2] << 16 | pkt->header[1] << 8 | pkt->header[0];
> writel(val, REG);

I disagree. =) Having the individual bytes makes their ordering very
explicit.

Thierry

Andrzej Hajda Nov. 5, 2014, 1:35 p.m. UTC | #5

On 11/04/2014 03:39 PM, Thierry Reding wrote:
> On Tue, Nov 04, 2014 at 03:21:14PM +0100, Andrzej Hajda wrote:
>> On 11/04/2014 02:58 PM, Thierry Reding wrote:
>>> On Tue, Nov 04, 2014 at 12:43:21PM +0100, Andrzej Hajda wrote:
>>>> On 11/03/2014 10:13 AM, Thierry Reding wrote:
> [...]
>>>>> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> [...]
>>>>> +		packet->header[1] = (msg->tx_len >> 0) & 0xff;
>>>>> +		packet->header[2] = (msg->tx_len >> 8) & 0xff;
>>>>> +
>>>>> +		packet->payload_length = msg->tx_len;
>>>>> +		packet->payload = tx;
>>>>> +	} else {
>>>>> +		packet->header[1] = (msg->tx_len > 0) ? tx[0] : 0;
>>>>> +		packet->header[2] = (msg->tx_len > 1) ? tx[1] : 0;
>>>>> +	}
>>>>> +
>>>>> +	packet->size = sizeof(packet->header) + packet->payload_length;
>>>> size seems to be completely useless.
>>> It's not. Tegra has two FIFOs that can be selected depending on the size
>>> of a transfer. I use this field to detect which FIFO needs to be
>>> selected.
>> But size is always equal payload_length + 4, so it does not carry any
>> additional information.
> Right, but it's out of convenience to prevent every driver from doing
> this again. It's part of the help that the helper provides. =)
>
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +EXPORT_SYMBOL(mipi_dsi_create_packet);
>>>>> +
>>>>> +/**
>>>>>   * mipi_dsi_dcs_write - send DCS write command
>>>>>   * @dsi: DSI device
>>>>>   * @data: pointer to the command followed by parameters
>>>>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>>>>> index 8569dc5a1026..663aa68826f4 100644
>>>>> --- a/include/drm/drm_mipi_dsi.h
>>>>> +++ b/include/drm/drm_mipi_dsi.h
>>>>> @@ -44,6 +44,24 @@ struct mipi_dsi_msg {
>>>>>  };
>>>>>  
>>>>>  /**
>>>>> + * struct mipi_dsi_packet - represents a MIPI DSI packet in protocol format
>>>>> + * @size: size (in bytes) of the packet
>>>>> + * @header: the four bytes that make up the header (Data ID, Word Count or
>>>>> + *     Packet Data, and ECC)
>>>>> + * @payload_length: number of bytes in the payload
>>>>> + * @payload: a pointer to a buffer containing the payload, if any
>>>>> + */
>>>>> +struct mipi_dsi_packet {
>>>>> +	size_t size;
>>>>> +	u8 header[4];
>>>> I wonder if it wouldn't be good to make it u32 or at least anonymous union:
>>>> union {
>>>>     u8 header[4];
>>>>     u32 header32;
>>>> };
>>> I'm not sure this is very useful. It's pretty trivial how you
>>> concatenate the individual bytes and it actually remove any ambiguity
>>> about the endianness.
>> This looks better:
>>
>> val = le16_to_cpu(pkt->header32);
>> writel(val, REG);
>>
>> than this:
>>
>> val = pkt->header[2] << 16 | pkt->header[1] << 8 | pkt->header[0];
>> writel(val, REG);
> I disagree. =) Having the individual bytes makes their ordering very
> explicit.
>

Wow, you want to keep size field to prevent drivers from adding
sometimes 4 to payload,
but you do not want to simplify header calculation that is much more
complicated :)

Regards
Andrzej

Patch

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index eb6dfe52cab2..76e81aba8220 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -199,6 +199,51 @@  int mipi_dsi_detach(struct mipi_dsi_device *dsi)
 EXPORT_SYMBOL(mipi_dsi_detach);
 
 /**
+ * mipi_dsi_create_packet - create a packet from a message according to the
+ *     DSI protocol
+ * @packet: pointer to a DSI packet structure
+ * @msg: message to translate into a packet
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int mipi_dsi_create_packet(struct mipi_dsi_packet *packet,
+			   const struct mipi_dsi_msg *msg)
+{
+	const u8 *tx = msg->tx_buf;
+
+	if (!packet || !msg)
+		return -EINVAL;
+
+	memset(packet, 0, sizeof(*packet));
+	packet->header[0] = ((msg->channel & 0x3) << 6) | (msg->type & 0x3f);
+
+	/* TODO: compute ECC if hardware support is not available */
+
+	/*
+	 * Long write packets contain the word count in header bytes 1 and 2.
+	 * The payload follows the header and is word count bytes long.
+	 *
+	 * Short write packets encode up to two parameters in header bytes 1
+	 * and 2.
+	 */
+	if (msg->tx_len > 2) {
+		packet->header[1] = (msg->tx_len >> 0) & 0xff;
+		packet->header[2] = (msg->tx_len >> 8) & 0xff;
+
+		packet->payload_length = msg->tx_len;
+		packet->payload = tx;
+	} else {
+		packet->header[1] = (msg->tx_len > 0) ? tx[0] : 0;
+		packet->header[2] = (msg->tx_len > 1) ? tx[1] : 0;
+	}
+
+	packet->size = sizeof(packet->header) + packet->payload_length;
+
+	return 0;
+}
+EXPORT_SYMBOL(mipi_dsi_create_packet);
+
+/**
  * mipi_dsi_dcs_write - send DCS write command
  * @dsi: DSI device
  * @data: pointer to the command followed by parameters
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 8569dc5a1026..663aa68826f4 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -44,6 +44,24 @@  struct mipi_dsi_msg {
 };
 
 /**
+ * struct mipi_dsi_packet - represents a MIPI DSI packet in protocol format
+ * @size: size (in bytes) of the packet
+ * @header: the four bytes that make up the header (Data ID, Word Count or
+ *     Packet Data, and ECC)
+ * @payload_length: number of bytes in the payload
+ * @payload: a pointer to a buffer containing the payload, if any
+ */
+struct mipi_dsi_packet {
+	size_t size;
+	u8 header[4];
+	size_t payload_length;
+	const u8 *payload;
+};
+
+int mipi_dsi_create_packet(struct mipi_dsi_packet *packet,
+			   const struct mipi_dsi_msg *msg);
+
+/**
  * struct mipi_dsi_host_ops - DSI bus operations
  * @attach: attach DSI device to DSI host
  * @detach: detach DSI device from DSI host