| Message ID | 20201208122855.254819-7-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Convert DSI code to use drm_mipi_dsi and drm_panel (second half) | expand |
Hi, On Tue, Dec 08, 2020 at 02:28:32PM +0200, Tomi Valkeinen wrote: > The OMAP DSI command mode panel driver used to send page & column > address before each frame update, and this code was moved into the DSI > host driver when converting it to the DRM bridge model. > > However, it's not really required to send the page & column address > before each frame. It's also something that doesn't really belong to the > DSI host driver, so we should drop the code. > > That said, frame updates break if we don't send _something_ between the > frames. A NOP command does the trick. > > It is not clear if this behavior is as expected from a DSI command mode > frame transfer, or is it a feature/issue with OMAP DSI driver, or a > feature/issue in the command mode panel used. > > Most likely this is related to the following from the DSI spec: > > "To enable PHY synchronization the host processor should periodically > end HS transmission and drive the Data Lanes to the LP state. This > transition should take place at least once per frame." > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/gpu/drm/omapdrm/dss/dsi.c | 46 ++++++++++++------------------- > 1 file changed, 17 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index 7fee9cf8782d..c6e044f8bece 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -3884,35 +3884,19 @@ static int _dsi_update(struct dsi_data *dsi) > return 0; > } > > -static int _dsi_update_window(struct dsi_data *dsi, int channel, > - int x, int y, int w, int h) > -{ > - int x1 = x, x2 = (x + w - 1); > - int y1 = y, y2 = (y + h - 1); > - u8 payloadX[5] = { MIPI_DCS_SET_COLUMN_ADDRESS, > - x1 >> 8, x1 & 0xff, x2 >> 8, x2 & 0xff }; > - u8 payloadY[5] = { MIPI_DCS_SET_PAGE_ADDRESS, > - y1 >> 8, y1 & 0xff, y2 >> 8, y2 & 0xff }; > - struct mipi_dsi_msg msgX = { 0 }, msgY = { 0 }; > - int ret; > +static int _dsi_send_nop(struct dsi_data *dsi, int channel) > +{ > + const u8 payload[] = { MIPI_DCS_NOP }; > + const struct mipi_dsi_msg msg = { > + .channel = channel, > + .type = MIPI_DSI_DCS_SHORT_WRITE, > + .tx_len = 1, > + .tx_buf = payload, > + }; > > WARN_ON(!dsi_bus_is_locked(dsi)); > > - msgX.type = MIPI_DSI_DCS_LONG_WRITE; > - msgX.channel = channel; > - msgX.tx_buf = payloadX; > - msgX.tx_len = sizeof(payloadX); > - > - msgY.type = MIPI_DSI_DCS_LONG_WRITE; > - msgY.channel = channel; > - msgY.tx_buf = payloadY; > - msgY.tx_len = sizeof(payloadY); > - > - ret = _omap_dsi_host_transfer(dsi, &msgX); > - if (ret != 0) > - return ret; > - > - return _omap_dsi_host_transfer(dsi, &msgY); > + return _omap_dsi_host_transfer(dsi, &msg); > } > > static int dsi_update_channel(struct omap_dss_device *dssdev, int channel) > @@ -3944,10 +3928,14 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int channel) > > dsi_set_ulps_auto(dsi, false); > > - r = _dsi_update_window(dsi, channel, 0, 0, dsi->vm.hactive, > - dsi->vm.vactive); > + /* > + * Send NOP between the frames. If we don't send something here, the > + * updates stop working. This is probably related to DSI spec stating > + * that the DSI host should transition to LP at least once per frame. > + */ > + r = _dsi_send_nop(dsi, channel); > if (r < 0) { > - DSSWARN("window update error: %d\n", r); > + DSSWARN("failed to send nop between frames: %d\n", r); > goto err; > } > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >
diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c index 7fee9cf8782d..c6e044f8bece 100644 --- a/drivers/gpu/drm/omapdrm/dss/dsi.c +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c @@ -3884,35 +3884,19 @@ static int _dsi_update(struct dsi_data *dsi) return 0; } -static int _dsi_update_window(struct dsi_data *dsi, int channel, - int x, int y, int w, int h) -{ - int x1 = x, x2 = (x + w - 1); - int y1 = y, y2 = (y + h - 1); - u8 payloadX[5] = { MIPI_DCS_SET_COLUMN_ADDRESS, - x1 >> 8, x1 & 0xff, x2 >> 8, x2 & 0xff }; - u8 payloadY[5] = { MIPI_DCS_SET_PAGE_ADDRESS, - y1 >> 8, y1 & 0xff, y2 >> 8, y2 & 0xff }; - struct mipi_dsi_msg msgX = { 0 }, msgY = { 0 }; - int ret; +static int _dsi_send_nop(struct dsi_data *dsi, int channel) +{ + const u8 payload[] = { MIPI_DCS_NOP }; + const struct mipi_dsi_msg msg = { + .channel = channel, + .type = MIPI_DSI_DCS_SHORT_WRITE, + .tx_len = 1, + .tx_buf = payload, + }; WARN_ON(!dsi_bus_is_locked(dsi)); - msgX.type = MIPI_DSI_DCS_LONG_WRITE; - msgX.channel = channel; - msgX.tx_buf = payloadX; - msgX.tx_len = sizeof(payloadX); - - msgY.type = MIPI_DSI_DCS_LONG_WRITE; - msgY.channel = channel; - msgY.tx_buf = payloadY; - msgY.tx_len = sizeof(payloadY); - - ret = _omap_dsi_host_transfer(dsi, &msgX); - if (ret != 0) - return ret; - - return _omap_dsi_host_transfer(dsi, &msgY); + return _omap_dsi_host_transfer(dsi, &msg); } static int dsi_update_channel(struct omap_dss_device *dssdev, int channel) @@ -3944,10 +3928,14 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int channel) dsi_set_ulps_auto(dsi, false); - r = _dsi_update_window(dsi, channel, 0, 0, dsi->vm.hactive, - dsi->vm.vactive); + /* + * Send NOP between the frames. If we don't send something here, the + * updates stop working. This is probably related to DSI spec stating + * that the DSI host should transition to LP at least once per frame. + */ + r = _dsi_send_nop(dsi, channel); if (r < 0) { - DSSWARN("window update error: %d\n", r); + DSSWARN("failed to send nop between frames: %d\n", r); goto err; }