Hi Chris, Am Montag, 30. Dezember 2024, 03:20:55 CET schrieb Chris Hofstaedtler: > On Tue, Dec 10, 2024 at 12:10:19AM +0100, Heiko Stuebner wrote: > > From: Heiko Stuebner <heiko.stuebner@xxxxxxxxx> > > > > Add a Synopsys Designware MIPI DSI host DRM bridge driver for their > > DSI2 host controller, based on the Rockchip version from the driver > > rockchip/dw-mipi-dsi2.c in their vendor-kernel with phy & bridge APIs. > > > > While the driver is heavily modelled after the previous IP, the register > > set of this DSI2 controller is completely different and there are also > > additional properties like the variable-width phy interface. > > > > Tested-by: Daniel Semkowicz <dse@xxxxxxxxxxxxx> > > Tested-by: Dmitry Yashin <dmt.yashin@xxxxxxxxx> > > Reviewed-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> > > Signed-off-by: Heiko Stuebner <heiko.stuebner@xxxxxxxxx> > [..] > > +static void dw_mipi_dsi2_set_vid_mode(struct dw_mipi_dsi2 *dsi2) > > +{ > > + u32 val = 0, mode; > > + int ret; > > + > > + if (dsi2->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP) > > + val |= BLK_HFP_HS_EN; > > + > > + if (dsi2->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP) > > + val |= BLK_HBP_HS_EN; > > + > > + if (dsi2->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA) > > + val |= BLK_HSA_HS_EN; > > For all three of these: is setting an ENable bit the right thing to > turn features *off*? first of all, thanks a lot for noticing this discrepancy :-) . Looking at the documentation, all 3 of those hw-bits are described as "Enables filling the H.. period with blanking packets. ..." where the MIPI_DSI_VIDEO_MODE_NO_* flags are described as "disable hfront-porch/... area" So yes, I _think_ "disable front-porch" would _should_ result in "don't fill the period with blanking packets", but am not fully sure. I've run the two boards I have with inverting the checks as sounds sensible right now, aka doing: if (!(dsi2->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP)) etc and both displays I have ran just fine. As the driver was originally part of a vendor-kernel based on 5.10, which I think was before the _NO addition from [0] it could be caused by a misread of the flags that were named differently back then. So yes, switching things around does sound like the right thing to do. Heiko [0] https://lore.kernel.org/all/20210629074703.v2.1.I629b2366a6591410359c7fcf6d385b474b705ca2@changeid/