Re: [PATCH 05/15] drm/dsi: Implement generic read and write commands

On 10/13/2014 12:16 PM, Thierry Reding wrote:

From: Thierry Reding treding@nvidia.com

Implement generic read and write commands. Selection of the proper data type for packets is done automatically based on the number of parameters or payload length.

Signed-off-by: Thierry Reding treding@nvidia.com

drivers/gpu/drm/drm_mipi_dsi.c | 115 +++++++++++++++++++++++++++++++++++++++++ include/drm/drm_mipi_dsi.h | 6 +++ 2 files changed, 121 insertions(+)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c index 27fc6dac5e4a..7cd69273dbad 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -229,6 +229,121 @@ int mipi_dsi_set_maximum_return_packet_size(struct mipi_dsi_device *dsi, EXPORT_SYMBOL(mipi_dsi_set_maximum_return_packet_size);

/**

• • mipi_dsi_generic_write() - transmit data using a generic write packet
• • @dsi: DSI peripheral device
• • @payload: buffer containing the payload
• • @size: size of payload buffer
• • This function will automatically choose the right data type depending on
• • the payload length.
• • Return: The number of bytes transmitted on success or a negative error code
• • on failure.
• */

+ssize_t mipi_dsi_generic_write(struct mipi_dsi_device *dsi, const void *payload,

• 	       size_t size)
  

+{

• struct mipi_dsi_msg msg;
• ssize_t err;
• u8 *tx;
• memset(&msg, 0, sizeof(msg));
• msg.channel = dsi->channel;

Again, maybe initializer would be better.

• msg.flags = MIPI_DSI_MSG_USE_LPM | MIPI_DSI_MSG_REQ_ACK;

You should not hardcode flags here, these flags have nothing to do with dsi generic write. But there is general problem of encoding flags in helpers. Possible solutions I see: 1. Translate DSI flags to msg flags as in case of MIPI_DSI_MODE_LPM -> MIPI_DSI_MSG_USE_LPM. 2. Copy dsi.mode_flags to msg.flags, or to dedicated field of msg. 3. Call transfer callback with dsi pointer instead of host, this way it will have access to mode_flags.

• if (size > 2) {

• tx = kmalloc(2 + size, GFP_KERNEL);
  

• if (!tx)
  

• 	return -ENOMEM;
  

• tx[0] = (size >> 0) & 0xff;
  

• tx[1] = (size >> 8) & 0xff;
  

• memcpy(tx + 2, payload, size);
  

• msg.tx_len = 2 + size;
  

• msg.tx_buf = tx;
  

• } else {

• msg.tx_buf = payload;
  

• msg.tx_len = size;
  

• }

Another API breakage, commented already in response to 1st patch.

• switch (size) {
• case 0:

• msg.type = MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM;
  

• break;
  

• case 1:

• msg.type = MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM;
  

• break;
  

• case 2:

• msg.type = MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM;
  

• break;
  

• default:

• msg.type = MIPI_DSI_GENERIC_LONG_WRITE;
  

• break;
  

• }
• err = dsi->host->ops->transfer(dsi->host, &msg);
• if (size > 2)

• kfree(tx);
  

• return err;

+} +EXPORT_SYMBOL(mipi_dsi_generic_write);

+/**

• • mipi_dsi_generic_read() - receive data using a generic read packet
• • @dsi: DSI peripheral device
• • @params: buffer containing the request parameters
• • @num_params: number of request parameters
• • @data: buffer in which to return the received data
• • @size: size of receive buffer
• • This function will automatically choose the right data type depending on
• • the number of parameters passed in.
• • Return: The number of bytes successfully read or a negative error code on
• • failure.
• */

+ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi, const void *params,

• 	      size_t num_params, void *data, size_t size)
  

+{

• struct mipi_dsi_msg msg;
• memset(&msg, 0, sizeof(msg));
• msg.channel = dsi->channel;
• msg.flags = MIPI_DSI_MSG_USE_LPM;
• msg.tx_len = num_params;
• msg.tx_buf = params;
• msg.rx_len = size;
• msg.rx_buf = data;
• switch (num_params) {
• case 0:

• msg.type = MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM;
  

• break;
  

• case 1:

• msg.type = MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM;
  

• break;
  

• case 2:

• msg.type = MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM;
  

• break;
  

• default:

• return -EINVAL;
  

• }
• return dsi->host->ops->transfer(dsi->host, &msg);

+} +EXPORT_SYMBOL(mipi_dsi_generic_read);

+/**

• mipi_dsi_dcs_write_buffer() - transmit a DCS command with payload
• @dsi: DSI peripheral device
• @data: buffer containing data to be transmitted

diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h index ef50b5d0de57..92ca66306702 100644 --- a/include/drm/drm_mipi_dsi.h +++ b/include/drm/drm_mipi_dsi.h @@ -134,6 +134,12 @@ int mipi_dsi_attach(struct mipi_dsi_device *dsi); int mipi_dsi_detach(struct mipi_dsi_device *dsi); int mipi_dsi_set_maximum_return_packet_size(struct mipi_dsi_device *dsi, u16 value);

+ssize_t mipi_dsi_generic_write(struct mipi_dsi_device *dsi, 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);
  

ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device *dsi, const void *data, size_t len); ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,