Hi Sebastian,
Thank you for the patch.
On Tue, Feb 25, 2020 at 12:20:38AM +0100, Sebastian Reichel wrote:
This converts the panel-dsi-cm driver to use the transfer API instead of specific functions, so that the specific functions can be unexported and squashed into the generic transfer function.
Signed-off-by: Sebastian Reichel sebastian.reichel@collabora.com
.../gpu/drm/omapdrm/displays/panel-dsi-cm.c | 133 +++++++++++++----- 1 file changed, 96 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c index e6ebfc35243e..92f510a771fe 100644 --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c @@ -140,45 +140,61 @@ static void hw_guard_wait(struct panel_drv_data *ddata) static int dsicm_dcs_read_1(struct panel_drv_data *ddata, u8 dcs_cmd, u8 *data) { struct omap_dss_device *src = ddata->src;
- int r;
- u8 buf[1];
- r = src->ops->dsi.dcs_read(src, ddata->channel, dcs_cmd, buf, 1);
- if (r < 0)
return r;- *data = buf[0];
- const struct mipi_dsi_msg msg = {
.channel = ddata->channel,.type = MIPI_DSI_DCS_READ,.tx_len = 1,.tx_buf = &dcs_cmd,.rx_len = 1,.rx_buf = data- };
- return 0;
- return src->ops->dsi.transfer(src, &msg);
}
static int dsicm_dcs_write_0(struct panel_drv_data *ddata, u8 dcs_cmd) { struct omap_dss_device *src = ddata->src;
- const struct mipi_dsi_msg msg = {
.channel = ddata->channel,.type = MIPI_DSI_DCS_SHORT_WRITE,.tx_buf = &dcs_cmd,.tx_len = 1,- };
- return src->ops->dsi.dcs_write(src, ddata->channel, &dcs_cmd, 1);
- return src->ops->dsi.transfer(src, &msg);
}
static int dsicm_dcs_write_1(struct panel_drv_data *ddata, u8 dcs_cmd, u8 param) { struct omap_dss_device *src = ddata->src;
- u8 buf[2] = { dcs_cmd, param };
- const u8 buf[] = { dcs_cmd, param };
- const struct mipi_dsi_msg msg = {
.channel = ddata->channel,.type = MIPI_DSI_DCS_SHORT_WRITE_PARAM,.tx_buf = &buf,.tx_len = 2,- };
- return src->ops->dsi.dcs_write(src, ddata->channel, buf, 2);
- return src->ops->dsi.transfer(src, &msg);
}
static int dsicm_sleep_in(struct panel_drv_data *ddata)
{ struct omap_dss_device *src = ddata->src;
- u8 cmd; int r;
const u8 cmd = MIPI_DCS_ENTER_SLEEP_MODE;
const struct mipi_dsi_msg msg = {
.channel = ddata->channel,.type = MIPI_DSI_DCS_SHORT_WRITE,.tx_buf = &cmd,.tx_len = 1,};
hw_guard_wait(ddata);
- cmd = MIPI_DCS_ENTER_SLEEP_MODE;
- r = src->ops->dsi.dcs_write_nosync(src, ddata->channel, &cmd, 1);
- r = src->ops->dsi.transfer(src, &msg);
Should you call dsicm_dcs_write_0(ddata, MIPI_DCS_ENTER_SLEEP_MODE) instead ? This uses the _nosync variant though, is it an issue ?
if (r) return r;
@@ -233,28 +249,44 @@ static int dsicm_set_update_window(struct panel_drv_data *ddata, u16 y1 = y; u16 y2 = y + h - 1;
- u8 buf[5];
- buf[0] = MIPI_DCS_SET_COLUMN_ADDRESS;
- buf[1] = (x1 >> 8) & 0xff;
- buf[2] = (x1 >> 0) & 0xff;
- buf[3] = (x2 >> 8) & 0xff;
- buf[4] = (x2 >> 0) & 0xff;
- const u8 paramX[] = {
MIPI_DCS_SET_COLUMN_ADDRESS,(x1 >> 8) & 0xff,(x1 >> 0) & 0xff,(x2 >> 8) & 0xff,(x2 >> 0) & 0xff,- };
- r = src->ops->dsi.dcs_write_nosync(src, ddata->channel, buf, sizeof(buf));
- if (r)
return r;
- const struct mipi_dsi_msg msgX = {
.channel = ddata->channel,.type = MIPI_DSI_GENERIC_LONG_WRITE,.tx_buf = paramX,.tx_len = 5,- };
- buf[0] = MIPI_DCS_SET_PAGE_ADDRESS;
- buf[1] = (y1 >> 8) & 0xff;
- buf[2] = (y1 >> 0) & 0xff;
- buf[3] = (y2 >> 8) & 0xff;
- buf[4] = (y2 >> 0) & 0xff;
- const u8 paramY[] = {
MIPI_DCS_SET_PAGE_ADDRESS,(y1 >> 8) & 0xff,(y1 >> 0) & 0xff,(y2 >> 8) & 0xff,(y2 >> 0) & 0xff,- };
- r = src->ops->dsi.dcs_write_nosync(src, ddata->channel, buf, sizeof(buf));
Also replacing a _nosync variant here.
- const struct mipi_dsi_msg msgY = {
.channel = ddata->channel,.type = MIPI_DSI_GENERIC_LONG_WRITE,.tx_buf = paramY,.tx_len = 5,- };
A single blank line is enough.
- r = src->ops->dsi.transfer(src, &msgX); if (r) return r;
- src->ops->dsi.bta_sync(src, ddata->channel);
- r = src->ops->dsi.transfer(src, &msgY);
And here, you're replacing bta_sync. If I understand the code correctly, you're essentially removing an optimization, as each write will sync, right ? I'm fine with this change if we add the functionality back later in this series.
if (r)
return r;return r;
} @@ -991,6 +1023,27 @@ static int dsicm_get_te(struct omap_dss_device *dssdev) return r; }
+static int dsicm_set_max_rx_packet_size(struct omap_dss_device *dssdev,
u16 size)
Please use tabs instead of spaces for indentation.
+{
- struct panel_drv_data *ddata = to_panel_data(dssdev);
- struct omap_dss_device *src = ddata->src;
- const u8 buf[] = {
size & 0xff,size >> 8 & 0xff,- };
- const struct mipi_dsi_msg msg = {
.channel = ddata->channel,.type = MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE,.tx_buf = buf,.tx_len = 2,- };
- return src->ops->dsi.transfer(src, &msg);
+}
static int dsicm_memory_read(struct omap_dss_device *dssdev, void *buf, size_t size, u16 x, u16 y, u16 w, u16 h) @@ -1031,17 +1084,23 @@ static int dsicm_memory_read(struct omap_dss_device *dssdev,
dsicm_set_update_window(ddata, x, y, w, h);
- r = src->ops->dsi.set_max_rx_packet_size(src, ddata->channel, plen);
r = dsicm_set_max_rx_packet_size(dssdev, plen); if (r) goto err2;
while (buf_used < size) { u8 dcs_cmd = first ? 0x2e : 0x3e;
const struct mipi_dsi_msg msg = {.channel = ddata->channel,.type = MIPI_DSI_DCS_READ,.tx_buf = &dcs_cmd,.tx_len = 1,.rx_buf = buf + buf_used,.rx_len = size - buf_used,};first = 0;
r = src->ops->dsi.dcs_read(src, ddata->channel, dcs_cmd,buf + buf_used, size - buf_used);
r = src->ops->dsi.transfer(src, &msg);@@ -1065,7 +1124,7 @@ static int dsicm_memory_read(struct omap_dss_device *dssdev, r = buf_used;
err3:
- src->ops->dsi.set_max_rx_packet_size(src, ddata->channel, 1);
- dsicm_set_max_rx_packet_size(dssdev, 1);
err2: src->ops->dsi.bus_unlock(src); err1: