lkml.org 
[lkml]   [2018]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 1/2] drm/bridge/synopsys: dsi: use common mipi_dsi_create_packet()
    From
    Date


    On 01/10/2018 02:02 AM, Brian Norris wrote:
    > This takes care of 2 TODOs in this driver, by using the common DSI
    > packet-marshalling code instead of our custom short/long write code.
    > This both saves us some duplicated code and gets us free support for
    > command types that weren't already part of our switch block (e.g.,
    > MIPI_DSI_GENERIC_LONG_WRITE).
    >
    > The code logic stays mostly intact, except that it becomes unnecessary
    > to split the short/long write functions, and we have to copy data a bit
    > more.
    >
    > Along the way, I noticed that loop bounds were a little odd:
    >
    > while (DIV_ROUND_UP(len, pld_data_bytes))
    >
    > This really was just supposed to be 'len != 0', so I made that more
    > clear.
    >
    > Tested on RK3399 with some pending refactoring patches by Nickey Yang,
    > to make the Rockchip DSI driver wrap this common driver.

    Queued to drm-misc-next. Philippe's point about the return value of
    mipi_dsi_device_transfer is addressed in a different patch.

    Thanks,
    Archit

    >
    > Signed-off-by: Brian Norris <briannorris@chromium.org>
    > Reviewed-by: Philippe Cornu <philippe.cornu@st.com>
    > Tested-by: Philippe Cornu <philippe.cornu@st.com>
    > ---
    > v2:
    > * remove "dcs" naming, since these commands handle generic DSI too, not
    > just DCS (thanks Philippe)
    > * add Philippe's {Tested,Reviewed}-by
    > ---
    > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 78 ++++++---------------------
    > 1 file changed, 16 insertions(+), 62 deletions(-)
    >
    > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
    > index d9cca4fd66ec..ed91e32ee43a 100644
    > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
    > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
    > @@ -136,10 +136,6 @@
    > GEN_SW_0P_TX_LP)
    >
    > #define DSI_GEN_HDR 0x6c
    > -/* TODO These 2 defines will be reworked thanks to mipi_dsi_create_packet() */
    > -#define GEN_HDATA(data) (((data) & 0xffff) << 8)
    > -#define GEN_HTYPE(type) (((type) & 0xff) << 0)
    > -
    > #define DSI_GEN_PLD_DATA 0x70
    >
    > #define DSI_CMD_PKT_STATUS 0x74
    > @@ -359,44 +355,15 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val)
    > return 0;
    > }
    >
    > -static int dw_mipi_dsi_dcs_short_write(struct dw_mipi_dsi *dsi,
    > - const struct mipi_dsi_msg *msg)
    > -{
    > - const u8 *tx_buf = msg->tx_buf;
    > - u16 data = 0;
    > - u32 val;
    > -
    > - if (msg->tx_len > 0)
    > - data |= tx_buf[0];
    > - if (msg->tx_len > 1)
    > - data |= tx_buf[1] << 8;
    > -
    > - if (msg->tx_len > 2) {
    > - dev_err(dsi->dev, "too long tx buf length %zu for short write\n",
    > - msg->tx_len);
    > - return -EINVAL;
    > - }
    > -
    > - val = GEN_HDATA(data) | GEN_HTYPE(msg->type);
    > - return dw_mipi_dsi_gen_pkt_hdr_write(dsi, val);
    > -}
    > -
    > -static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi *dsi,
    > - const struct mipi_dsi_msg *msg)
    > +static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi,
    > + const struct mipi_dsi_packet *packet)
    > {
    > - const u8 *tx_buf = msg->tx_buf;
    > - int len = msg->tx_len, pld_data_bytes = sizeof(u32), ret;
    > - u32 hdr_val = GEN_HDATA(msg->tx_len) | GEN_HTYPE(msg->type);
    > + const u8 *tx_buf = packet->payload;
    > + int len = packet->payload_length, pld_data_bytes = sizeof(u32), ret;
    > u32 remainder;
    > u32 val;
    >
    > - if (msg->tx_len < 3) {
    > - dev_err(dsi->dev, "wrong tx buf length %zu for long write\n",
    > - msg->tx_len);
    > - return -EINVAL;
    > - }
    > -
    > - while (DIV_ROUND_UP(len, pld_data_bytes)) {
    > + while (len) {
    > if (len < pld_data_bytes) {
    > remainder = 0;
    > memcpy(&remainder, tx_buf, len);
    > @@ -419,40 +386,27 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi *dsi,
    > }
    > }
    >
    > - return dw_mipi_dsi_gen_pkt_hdr_write(dsi, hdr_val);
    > + remainder = 0;
    > + memcpy(&remainder, packet->header, sizeof(packet->header));
    > + return dw_mipi_dsi_gen_pkt_hdr_write(dsi, remainder);
    > }
    >
    > static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
    > const struct mipi_dsi_msg *msg)
    > {
    > struct dw_mipi_dsi *dsi = host_to_dsi(host);
    > + struct mipi_dsi_packet packet;
    > int ret;
    >
    > - /*
    > - * TODO dw drv improvements
    > - * use mipi_dsi_create_packet() instead of all following
    > - * functions and code (no switch cases, no
    > - * dw_mipi_dsi_dcs_short_write(), only the loop in long_write...)
    > - * and use packet.header...
    > - */
    > - dw_mipi_message_config(dsi, msg);
    > -
    > - switch (msg->type) {
    > - case MIPI_DSI_DCS_SHORT_WRITE:
    > - case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
    > - case MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE:
    > - ret = dw_mipi_dsi_dcs_short_write(dsi, msg);
    > - break;
    > - case MIPI_DSI_DCS_LONG_WRITE:
    > - ret = dw_mipi_dsi_dcs_long_write(dsi, msg);
    > - break;
    > - default:
    > - dev_err(dsi->dev, "unsupported message type 0x%02x\n",
    > - msg->type);
    > - ret = -EINVAL;
    > + ret = mipi_dsi_create_packet(&packet, msg);
    > + if (ret) {
    > + dev_err(dsi->dev, "failed to create packet: %d\n", ret);
    > + return ret;
    > }
    >
    > - return ret;
    > + dw_mipi_message_config(dsi, msg);
    > +
    > + return dw_mipi_dsi_write(dsi, &packet);
    > }
    >
    > static const struct mipi_dsi_host_ops dw_mipi_dsi_host_ops = {
    >

    --
    Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
    a Linux Foundation Collaborative Project

    \
     
     \ /
      Last update: 2018-01-16 07:57    [W:4.228 / U:1.168 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site