Hi Tomi,
Thank you for the patch.
On Tue, Dec 08, 2020 at 02:28:37PM +0200, Tomi Valkeinen wrote:
> To start fixing the issues related to channels and vcs described in the
> previous commit, pass vc to various functions which will need it do
> properly handle different DSI channels and VCs.
This is a bit hard to review as you add the OMAP DSI VC as a parameter
(named vc) to some functions , and the MIPI DSI virtual channel as a
parameter (named channel) to other functions. Only the former matches
the commit message. Splitting this in two would make the changes
clearer. Still, I believe the patch is correct, so
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> No functional changes.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
> ---
> drivers/gpu/drm/omapdrm/dss/dsi.c | 54 ++++++++++++++++---------------
> 1 file changed, 28 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index 273159e8f992..8d8412199693 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -214,9 +214,9 @@ static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable);
> static int dsi_display_init_dispc(struct dsi_data *dsi);
> static void dsi_display_uninit_dispc(struct dsi_data *dsi);
>
> -static int dsi_vc_send_null(struct dsi_data *dsi, int vc);
> +static int dsi_vc_send_null(struct dsi_data *dsi, int vc, int channel);
>
> -static ssize_t _omap_dsi_host_transfer(struct dsi_data *dsi,
> +static ssize_t _omap_dsi_host_transfer(struct dsi_data *dsi, int vc,
> const struct mipi_dsi_msg *msg);
>
> static void dsi_display_disable(struct omap_dss_device *dssdev);
> @@ -2455,7 +2455,7 @@ static void dsi_vc_enable_hs(struct omap_dss_device *dssdev, int vc,
>
> /* start the DDR clock by sending a NULL packet */
> if (dsi->vm_timings.ddr_clk_always_on && enable)
> - dsi_vc_send_null(dsi, vc);
> + dsi_vc_send_null(dsi, vc, dsi->dsidev->channel);
>
> dsi->in_lp_mode = !enable;
> }
> @@ -2605,7 +2605,8 @@ static int dsi_vc_send_bta_sync(struct omap_dss_device *dssdev, int vc)
> }
>
> static inline void dsi_vc_write_long_header(struct dsi_data *dsi, int vc,
> - u8 data_type, u16 len, u8 ecc)
> + int channel, u8 data_type, u16 len,
> + u8 ecc)
> {
> u32 val;
> u8 data_id;
> @@ -2633,7 +2634,7 @@ static inline void dsi_vc_write_long_payload(struct dsi_data *dsi, int vc,
> dsi_write_reg(dsi, DSI_VC_LONG_PACKET_PAYLOAD(vc), val);
> }
>
> -static int dsi_vc_send_long(struct dsi_data *dsi,
> +static int dsi_vc_send_long(struct dsi_data *dsi, int vc,
> const struct mipi_dsi_msg *msg)
> {
> /*u32 val; */
> @@ -2653,7 +2654,7 @@ static int dsi_vc_send_long(struct dsi_data *dsi,
>
> dsi_vc_config_source(dsi, msg->channel, DSI_VC_SOURCE_L4);
>
> - dsi_vc_write_long_header(dsi, msg->channel, msg->type, msg->tx_len, 0);
> + dsi_vc_write_long_header(dsi, vc, msg->channel, msg->type, msg->tx_len, 0);
>
> p = msg->tx_buf;
> for (i = 0; i < msg->tx_len >> 2; i++) {
> @@ -2696,7 +2697,7 @@ static int dsi_vc_send_long(struct dsi_data *dsi,
> return r;
> }
>
> -static int dsi_vc_send_short(struct dsi_data *dsi,
> +static int dsi_vc_send_short(struct dsi_data *dsi, int vc,
> const struct mipi_dsi_msg *msg)
> {
> struct mipi_dsi_packet pkt;
> @@ -2727,26 +2728,26 @@ static int dsi_vc_send_short(struct dsi_data *dsi,
> return 0;
> }
>
> -static int dsi_vc_send_null(struct dsi_data *dsi, int vc)
> +static int dsi_vc_send_null(struct dsi_data *dsi, int vc, int channel)
> {
> const struct mipi_dsi_msg msg = {
> .channel = vc,
> .type = MIPI_DSI_NULL_PACKET,
> };
>
> - return dsi_vc_send_long(dsi, &msg);
> + return dsi_vc_send_long(dsi, vc, &msg);
> }
>
> -static int dsi_vc_write_common(struct omap_dss_device *dssdev,
> +static int dsi_vc_write_common(struct omap_dss_device *dssdev, int vc,
> const struct mipi_dsi_msg *msg)
> {
> struct dsi_data *dsi = to_dsi_data(dssdev);
> int r;
>
> if (mipi_dsi_packet_format_is_short(msg->type))
> - r = dsi_vc_send_short(dsi, msg);
> + r = dsi_vc_send_short(dsi, vc, msg);
> else
> - r = dsi_vc_send_long(dsi, msg);
> + r = dsi_vc_send_long(dsi, vc, msg);
>
> if (r < 0)
> return r;
> @@ -2882,7 +2883,7 @@ static int dsi_vc_read_rx_fifo(struct dsi_data *dsi, int vc, u8 *buf,
> return r;
> }
>
> -static int dsi_vc_dcs_read(struct omap_dss_device *dssdev,
> +static int dsi_vc_dcs_read(struct omap_dss_device *dssdev, int vc,
> const struct mipi_dsi_msg *msg)
> {
> struct dsi_data *dsi = to_dsi_data(dssdev);
> @@ -2893,7 +2894,7 @@ static int dsi_vc_dcs_read(struct omap_dss_device *dssdev,
> if (dsi->debug_read)
> DSSDBG("%s(ch %d, cmd %x)\n", __func__, channel, cmd);
>
> - r = dsi_vc_send_short(dsi, msg);
> + r = dsi_vc_send_short(dsi, vc, msg);
> if (r)
> goto err;
>
> @@ -2917,13 +2918,13 @@ static int dsi_vc_dcs_read(struct omap_dss_device *dssdev,
> return r;
> }
>
> -static int dsi_vc_generic_read(struct omap_dss_device *dssdev,
> +static int dsi_vc_generic_read(struct omap_dss_device *dssdev, int vc,
> const struct mipi_dsi_msg *msg)
> {
> struct dsi_data *dsi = to_dsi_data(dssdev);
> int r;
>
> - r = dsi_vc_send_short(dsi, msg);
> + r = dsi_vc_send_short(dsi, vc, msg);
> if (r)
> goto err;
>
> @@ -3672,7 +3673,7 @@ static void dsi_enable_video_output(struct omap_dss_device *dssdev, int vc)
>
> word_count = DIV_ROUND_UP(dsi->vm.hactive * bpp, 8);
>
> - dsi_vc_write_long_header(dsi, vc, data_type,
> + dsi_vc_write_long_header(dsi, vc, dsi->dsidev->channel, data_type,
> word_count, 0);
>
> dsi_vc_enable(dsi, vc, true);
> @@ -3770,7 +3771,7 @@ static void dsi_update_screen_dispc(struct dsi_data *dsi)
> l = FLD_VAL(total_len, 23, 0); /* TE_SIZE */
> dsi_write_reg(dsi, DSI_VC_TE(vc), l);
>
> - dsi_vc_write_long_header(dsi, vc, MIPI_DSI_DCS_LONG_WRITE,
> + dsi_vc_write_long_header(dsi, vc, dsi->dsidev->channel, MIPI_DSI_DCS_LONG_WRITE,
> packet_len, 0);
>
> if (dsi->te_enabled)
> @@ -3882,7 +3883,7 @@ static int _dsi_update(struct dsi_data *dsi)
> return 0;
> }
>
> -static int _dsi_send_nop(struct dsi_data *dsi, int channel)
> +static int _dsi_send_nop(struct dsi_data *dsi, int vc, int channel)
> {
> const u8 payload[] = { MIPI_DCS_NOP };
> const struct mipi_dsi_msg msg = {
> @@ -3894,7 +3895,7 @@ static int _dsi_send_nop(struct dsi_data *dsi, int channel)
>
> WARN_ON(!dsi_bus_is_locked(dsi));
>
> - return _omap_dsi_host_transfer(dsi, &msg);
> + return _omap_dsi_host_transfer(dsi, vc, &msg);
> }
>
> static int dsi_update_channel(struct omap_dss_device *dssdev, int vc)
> @@ -3923,7 +3924,7 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int vc)
> * 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, vc);
> + r = _dsi_send_nop(dsi, vc, dsi->dsidev->channel);
> if (r < 0) {
> DSSWARN("failed to send nop between frames: %d\n", r);
> goto err;
> @@ -4885,7 +4886,7 @@ static enum omap_channel dsi_get_dispc_channel(struct dsi_data *dsi)
> }
> }
>
> -static ssize_t _omap_dsi_host_transfer(struct dsi_data *dsi,
> +static ssize_t _omap_dsi_host_transfer(struct dsi_data *dsi, int vc,
> const struct mipi_dsi_msg *msg)
> {
> struct omap_dss_device *dssdev = &dsi->output;
> @@ -4905,15 +4906,15 @@ static ssize_t _omap_dsi_host_transfer(struct dsi_data *dsi,
> case MIPI_DSI_DCS_LONG_WRITE:
> case MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE:
> case MIPI_DSI_NULL_PACKET:
> - r = dsi_vc_write_common(dssdev, msg);
> + r = dsi_vc_write_common(dssdev, vc, msg);
> break;
> case MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM:
> case MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM:
> case MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM:
> - r = dsi_vc_generic_read(dssdev, msg);
> + r = dsi_vc_generic_read(dssdev, vc, msg);
> break;
> case MIPI_DSI_DCS_READ:
> - r = dsi_vc_dcs_read(dssdev, msg);
> + r = dsi_vc_dcs_read(dssdev, vc, msg);
> break;
> default:
> r = -EINVAL;
> @@ -4941,12 +4942,13 @@ static ssize_t omap_dsi_host_transfer(struct mipi_dsi_host *host,
> {
> struct dsi_data *dsi = host_to_omap(host);
> int r;
> + int vc = VC_DEFAULT;
>
> dsi_bus_lock(dsi);
>
> if (dsi->video_enabled) {
> dsi_set_ulps_auto(dsi, false);
> - r = _omap_dsi_host_transfer(dsi, msg);
> + r = _omap_dsi_host_transfer(dsi, vc, msg);
> dsi_set_ulps_auto(dsi, true);
> } else {
> r = -EIO;
--
Regards,
Laurent Pinchart
