On Thu, 31 Mar 2022 at 15:42, Marek Vasut <marex@xxxxxxx> wrote: > > On 3/31/22 14:02, Robert Foss wrote: > > Hey Marek, > > Hi, > > > On Fri, 18 Mar 2022 at 19:48, Marek Vasut <marex@xxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx> > >> Cc: Maxime Ripard <maxime@xxxxxxxxxx> > >> Cc: Robert Foss <robert.foss@xxxxxxxxxx> > >> Cc: Sam Ravnborg <sam@xxxxxxxxxxxx> > >> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> > >> To: dri-devel@xxxxxxxxxxxxxxxxxxxxx > >> > >> 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.
