Re: [PATCH V6 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/8/21 11:42 AM, Robert Foss wrote:
On Tue, 8 Jun 2021 at 11:34, Marek Vasut <marex@xxxxxxx> wrote:

On 6/8/21 10:51 AM, Robert Foss wrote:
Hey Neil,

On Tue, 8 Jun 2021 at 10:10, Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote:

Hi,

On 07/06/2021 19:42, Marek Vasut wrote:
Add driver for TI SN65DSI83 Single-link DSI to Single-link LVDS bridge
and TI SN65DSI84 Single-link DSI to Dual-link or 2x Single-link LVDS
bridge. TI SN65DSI85 is unsupported due to lack of hardware to test on,
but easy to add.

The driver operates the chip via I2C bus. Currently the LVDS clock are
always derived from DSI clock lane, which is the usual mode of operation.
Support for clock from external oscillator is not implemented, but it is
easy to add if ever needed. Only RGB888 pixel format is implemented, the
LVDS666 is not supported, but could be added if needed.

Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
Reviewed-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>
Tested-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>
Tested-by: Adam Ford <aford173@xxxxxxxxx>
Signed-off-by: Marek Vasut <marex@xxxxxxx>
Cc: Douglas Anderson <dianders@xxxxxxxxxxxx>
Cc: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
Cc: Loic Poulain <loic.poulain@xxxxxxxxxx>
Cc: Philippe Schenker <philippe.schenker@xxxxxxxxxxx>
Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
Cc: Stephen Boyd <swboyd@xxxxxxxxxxxx>
Cc: Valentin Raevsky <valentin@xxxxxxxxxxxxxx>
To: dri-devel@xxxxxxxxxxxxxxxxxxxxx
---
V2: - Use dev_err_probe()
      - Set REG_RC_RESET as volatile
      - Wait for PLL stabilization by polling REG_RC_LVDS_PLL
      - Use ctx->mode = *adj instead of *mode in sn65dsi83_mode_set
      - Add tested DSI84 support in dual-link mode
      - Correctly set VCOM
      - Fill in missing DSI CHB and LVDS CHB bits from DSI84 and DSI85
        datasheets, with that all the reserved bits make far more sense
        as the DSI83 and DSI84 seems to be reduced version of DSI85
V3: - Handle the dual-link LVDS with two port panel or bridge
V4: - Add RB from Linus Walleij
      - Rename REG_DSI_LANE_LVDS_LINK_CFG_DUAL to
        REG_DSI_LANE_DSI_CHANNEL_MODE_SINGLE and fill in the remaining
        DSI link options from DSI85 datasheet. DSI85 can do dual and 2x
        single DSI mode, but DSI85 is currently unsupported by the
        driver. Add a comment about DSI85, so that all the places which
        need to be adjusted for DSI85 are marked accordingly.
      - Add REG_DSI_LANE_LEFT_RIGHT_PIXELS bit for DSI
      - Add handling for JEIDA18/JEIDA24/SPWG24 LVDS formats. Use SPWG24
        as fallback on output bridges until they are all fixed.
      - Patch DSI bus format to fixed RGB888_1X24 instead of passing
        through the LVDS bus format.
V5: - Move bus format patching to mode_fixup
      - Use cpu_to_le16() to guarantee endianness in regmap_bulk_write()
V6: - Cast of_device_get_match_data() output to uintptr_t first
---
   drivers/gpu/drm/bridge/Kconfig        |  10 +
   drivers/gpu/drm/bridge/Makefile       |   1 +
   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 709 ++++++++++++++++++++++++++
   3 files changed, 720 insertions(+)
   create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi83.c


Looks complete & well reviewed, LGTM

Rob, Laurent ? is it ok for you ?

This does look like it is ready. I was just about to merge it, but
found some checkpatch --strict formatting warnings. Do you mind fixing
those in a real quick v7?

We already discussed this thing with open parenthesis alignment before
in one of the previous versions of this series, and the readability of
the driver was worse with that change in place.

Is this change now a blocker ?

Ah, sorry I missed that part of the conversation. In that case no.
Let's go ahead.

Nevermind, I sent V7. Apparently line over 80 is no longer an issue and line over 100 is, so the result doesn't look so bad. Pick the V7 please.



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux