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

[Marek Vasut]

On 6/8/21 10:51 AM, Robert Foss wrote:
> Hey Neil,
> 
> On Tue, 8 Jun 2021 at 10:10, Neil Armstrong <narmstrong at baylibre.com> 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 at linaro.org>
>>> Reviewed-by: Frieder Schrempf <frieder.schrempf at kontron.de>
>>> Tested-by: Frieder Schrempf <frieder.schrempf at kontron.de>
>>> Tested-by: Adam Ford <aford173 at gmail.com>
>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>> Cc: Douglas Anderson <dianders at chromium.org>
>>> Cc: Jagan Teki <jagan at amarulasolutions.com>
>>> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>> Cc: Linus Walleij <linus.walleij at linaro.org>
>>> Cc: Loic Poulain <loic.poulain at linaro.org>
>>> Cc: Philippe Schenker <philippe.schenker at toradex.com>
>>> Cc: Sam Ravnborg <sam at ravnborg.org>
>>> Cc: Stephen Boyd <swboyd at chromium.org>
>>> Cc: Valentin Raevsky <valentin at compulab.co.il>
>>> To: dri-devel at lists.freedesktop.org
>>> ---
>>> 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 ?