[PATCH v5 00/11] drm: bridge: icn6211: Fix hard-coded panel settings and add I2C support

[Robert Foss]

On Thu, 31 Mar 2022 at 15:42, Marek Vasut <marex at denx.de> wrote:
>
> On 3/31/22 14:02, Robert Foss wrote:
> > Hey Marek,
>
> Hi,
>
> > On Fri, 18 Mar 2022 at 19:48, Marek Vasut <marex at denx.de> wrote:
> >>
> >> This series fixes multiple problems with the ICN6211 driver and adds
> >> support for configuration of the chip via I2C bus.
> >>
> >> First, in the current state, the ICN6211 driver hard-codes DPI timing
> >> and clock settings specific to some unknown panel. The settings provided
> >> by panel driver are ignored. Using any other panel than the one for which
> >> this driver is currently hard-coded can lead to permanent damage of the
> >> panel (per display supplier warning, and it sure did in my case. The
> >> damage looks like multiple rows of dead pixels at the bottom of the
> >> panel, and this is not going away even after long power off time).
> >>
> >> Much of this series thus fixes incorrect register layout, DPI timing
> >> programming, clock generation by adding actual PLL configuration code.
> >> This series no longer adds lane count decoding and retains current
> >> hard-coded lane count 4 due to disagreement over lane count parsing
> >> from DT. The lane count support will come later. The series also fills
> >> in a couple of registers with likely correct default values.
> >>
> >> Second, this series adds support for I2C configuration of the ICN6211.
> >> The device can be configured either via DSI command mode or via I2C,
> >> the register layout is the same in both cases.
> >>
> >> Since the datasheet for this device is very hard to come by, a lot of
> >> information has been salvaged from [1] and [2].
> >>
> >> [1] https://github.com/rockchip-linux/kernel/blob/develop-4.19/drivers/gpu/drm/bridge/icn6211.c
> >> [2] https://github.com/tdjastrzebski/ICN6211-Configurator
> >>
> >> Cc: Jagan Teki <jagan at amarulasolutions.com>
> >> Cc: Maxime Ripard <maxime at cerno.tech>
> >> Cc: Robert Foss <robert.foss at linaro.org>
> >> Cc: Sam Ravnborg <sam at ravnborg.org>
> >> Cc: Thomas Zimmermann <tzimmermann at suse.de>
> >> To: dri-devel at lists.freedesktop.org
> >>
> >> Marek Vasut (11):
> >>    drm: bridge: icn6211: Fix register layout
> >>    drm: bridge: icn6211: Fix HFP_HSW_HBP_HI and HFP_MIN handling
> >>    drm: bridge: icn6211: Add HS/VS/DE polarity handling
> >>    drm: bridge: icn6211: Add generic DSI-to-DPI PLL configuration
> >>    drm: bridge: icn6211: Use DSI burst mode without EoT and with LP
> >>      command mode
> >>    drm: bridge: icn6211: Disable DPI color swap
> >>    drm: bridge: icn6211: Set SYS_CTRL_1 to value used in examples
> >>    drm: bridge: icn6211: Implement atomic_get_input_bus_fmts
> >>    drm: bridge: icn6211: Add I2C configuration support
> >>    drm: bridge: icn6211: Rework ICN6211_DSI to chipone_writeb()
> >>    drm: bridge: icn6211: Read and validate chip IDs before configuration
> >>
> >>   drivers/gpu/drm/bridge/chipone-icn6211.c | 486 ++++++++++++++++++++---
> >>   1 file changed, 434 insertions(+), 52 deletions(-)
> >>
> >> --
> >> 2.35.1
> >>
> >
> > This series looks ready to be merged
>
> I was waiting for 5.18-rc1 to be out and MW closed before picking it
> into drm-misc-next . Maybe I can pick it up now already ?
>
> > , could you fix the remaining
> > 'checkpatch --strict' warnings that are applicable?
>
> There are only these left, which I think is OK:
> WARNING: Possible unwrapped commit description (prefer a maximum 75
> chars per line)
>
> And then this one strict CHECK, but if I change that the formatting
> looks even uglier:
> 0010-drm-bridge-icn6211-Rework-ICN6211_DSI-to-chipone_wri.patch
>
> CHECK: Alignment should match open parenthesis
> #68: FILE: drivers/gpu/drm/bridge/chipone-icn6211.c:238:
> +       chipone_writeb(icn, PLL_REF_DIV,
>                      (best_p ? PLL_REF_DIV_Pe : 0) | /* Prefer /2
> pre-divider */
>
> CHECK: Alignment should match open parenthesis
> #97: FILE: drivers/gpu/drm/bridge/chipone-icn6211.c:272:
> +       chipone_writeb(icn, VACTIVE_HACTIVE_HI,
>                      ((mode->hdisplay >> 8) & 0xf) |
>
> CHECK: Alignment should match open parenthesis
> #113: FILE: drivers/gpu/drm/bridge/chipone-icn6211.c:284:
> +       chipone_writeb(icn, HFP_HSW_HBP_HI,
>                      HFP_HSW_HBP_HI_HFP(hfp) |
>

I'd like to at least strive for uniformity, so checkpatch is king, and
whatever formatting it preferes is what should be used.

> > Ideally the line
> > removal suggested by Maxime would be included too.
>
> This line removal comment has nothing to do with changes in this series,
> it is about the following patch, which is no longer part of this series
> because there is ongoing disagreement about that part and OF graph, so
> that patch will be resubmitted separately later:

Ack, thanks for explaining.

>
> [PATCH v4 05/13] drm: bridge: icn6211: Add DSI lane count DT property
> parsing
>
> The continuation of that discussion is already in:
>
> [PATCH] dt-bindings: display: bridge: Drop requirement on input port for
> DSI devices
>
> So this series itself has no outstanding changes pending, unless you
> really want the uglier formatting above.

Yes, please resend with this fixed, and I'll merge it.