linux-kernel - Re: [PATCH v4 1/3] drm/bridge/synopsys: Add MIPI DSI2 host controller bridge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <3556284.QJadu78ljV@diego>
Date: Tue, 14 Jan 2025 18:30:30 +0100
From: Heiko Stübner <heiko@...ech.de>
To: Chris Hofstaedtler <zeha@...ian.org>, andy.yan@...k-chips.com,
 Guochun Huang <hero.huang@...k-chips.com>
Cc: maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
 tzimmermann@...e.de, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, andrzej.hajda@...el.com, neil.armstrong@...aro.org,
 rfoss@...nel.org, Laurent.pinchart@...asonboard.com, jonas@...boo.se,
 jernej.skrabec@...il.com, dri-devel@...ts.freedesktop.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
 quentin.schulz@...rry.de, Heiko Stuebner <heiko.stuebner@...rry.de>,
 Daniel Semkowicz <dse@...umatec.com>, Dmitry Yashin <dmt.yashin@...il.com>
Subject:
 Re: [PATCH v4 1/3] drm/bridge/synopsys: Add MIPI DSI2 host controller bridge

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@...rry.de>
> > 
> > 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@...umatec.com>
> > Tested-by: Dmitry Yashin <dmt.yashin@...il.com>
> > Reviewed-by: Neil Armstrong <neil.armstrong@...aro.org>
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@...rry.de>
> [..]
> > +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/