Hi Laurent,
On Tue, Feb 25, 2020 at 05:31:05PM +0200, Laurent Pinchart wrote:
> > + if (mipi_dsi_packet_format_is_short(msg->type)) {
> > + u16 data = packet.header[1] | (packet.header[2] << 8);
> > + r = dsi_vc_send_short(dsi, msg->channel, msg->type, data, 0);
>
> You use the packet for this case only, I think you could simply write
>
> u16 data = ((msg->tx_len > 0) ? tx[0] : 0)
> | (((msg->tx_len > 1) ? tx[1] : 0) << 8);
> r = dsi_vc_send_short(dsi, msg->channel, msg->type, data, 0);
That probably works with 's/tx[/((u8*) msg->tx_buf)[', which looks
really ugly :) This code is further simplified by a further patch,
which forwards the complete message into dsi_vc_send_short().
> > } else {
> > - r = dsi_vc_send_long(dsi, channel,
> > - type == DSS_DSI_CONTENT_GENERIC ?
> > - MIPI_DSI_GENERIC_LONG_WRITE :
> > - MIPI_DSI_DCS_LONG_WRITE, data, len, 0);
> > + r = dsi_vc_send_long(dsi, msg->channel, msg->type,
> > + msg->tx_buf, msg->tx_len, 0);
>
> Indentation.
Ok.
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Are you fine with keeping the mipi_dsi_packet, since it will be
removed in a further patch?
> > }
> >
> > - return r;
> > -}
> > -
> > -static int dsi_vc_dcs_write_nosync(struct omap_dss_device *dssdev, int channel,
> > - const u8 *data, int len)
> > -{
> > - struct dsi_data *dsi = to_dsi_data(dssdev);
> > -
> > - return dsi_vc_write_nosync_common(dsi, channel, data, len,
> > - DSS_DSI_CONTENT_DCS);
> > -}
> > -
> > -static int dsi_vc_generic_write_nosync(struct omap_dss_device *dssdev, int channel,
> > - const u8 *data, int len)
> > -{
> > - struct dsi_data *dsi = to_dsi_data(dssdev);
> > -
> > - return dsi_vc_write_nosync_common(dsi, channel, data, len,
> > - DSS_DSI_CONTENT_GENERIC);
> > -}
> > -
> > -static int dsi_vc_write_common(struct omap_dss_device *dssdev,
> > - int channel, const u8 *data, int len,
> > - enum dss_dsi_content_type type)
> > -{
> > - struct dsi_data *dsi = to_dsi_data(dssdev);
> > - int r;
> > + if (r < 0)
> > + return r;
> >
> > - r = dsi_vc_write_nosync_common(dsi, channel, data, len, type);
> > - if (r)
> > - goto err;
> > + /*
> > + * we do not always have to do the BTA sync, for example we can
> > + * improve performance by setting the update window information
> > + * without sending BTA sync between the commands. In that case
> > + * we can return earily.
>
> s/earily/early/
>
> Do I understand correctly that this isn't implemented yet ? You should
> make it clear in the comment that it's a candidate for a future
> optimization.
Yes. I forgot the TODO keyword for some reason. Has been quite some
time since I wrote this patch :) I fixed the earily and prefixed the
message with TODO.
-- Sebastian
Attachment:
signature.asc
Description: PGP signature
