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/
