Re: [PATCH v8 01/14] drm: exynos: dsi: Fix MIPI_DSI*_NO_* mode flags

From: Nicolas Boichat <drinkcat@chromium.org>
To: Jagan Teki <jagan@amarulasolutions.com>
Cc: dri-devel@lists.freedesktop.org,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	Fancy Fang <chen.fang@nxp.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Marek Vasut <marex@denx.de>,
	linux-samsung-soc@vger.kernel.org,
	Joonyoung Shim <jy0922.shim@samsung.com>,
	Neil Armstrong <narmstrong@linaro.org>,
	Frieder Schrempf <frieder.schrempf@kontron.de>,
	Tommaso Merciai <tommaso.merciai@amarulasolutions.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Michael Nazzareno Trimarchi <michael@amarulasolutions.com>,
	Matteo Lisi <matteo.lisi@engicam.com>,
	Tim Harvey <tharvey@gateworks.com>,
	Inki Dae <inki.dae@samsung.com>, Adam Ford <aford173@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	Robert Foss <robert.foss@linaro.org>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	linux-amarula <linux-amarula@amarulasolutions.com>
Subject: Re: [PATCH v8 01/14] drm: exynos: dsi: Fix MIPI_DSI*_NO_* mode flags
Date: Fri, 11 Nov 2022 20:12:43 +0800	[thread overview]
Message-ID: <CANMq1KDmF+cMo8qZh7UyM1vyZ6BD_cFPc6j2MNMNPFiutqxL3w@mail.gmail.com> (raw)
In-Reply-To: <CAMty3ZBC3pbWf4yixjv4DZWFR+fYkPctiL3hzubgY8RF4ZGHmA@mail.gmail.com>

On Fri, Nov 11, 2022 at 4:49 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Fri, Nov 11, 2022 at 6:19 AM Nicolas Boichat <drinkcat@chromium.org> wrote:
> >
> > On Fri, Nov 11, 2022 at 2:40 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > HFP/HBP/HSA/EOT_PACKET modes in Exynos DSI host specifies
> > > 0 = Enable and 1 = Disable.
> >
> > Oh I see, that's confusing... IMHO you might want to change the
> > register macro name... (but if that's what the datasheet uses, it
> > might not be ideal either). At the _very_ least, I'd add a comment in
> > the code so the next person doesn't attempt to "fix" it again...
>
> 02/14 on the same series doing the name change.
> https://lore.kernel.org/all/20221110183853.3678209-3-jagan@amarulasolutions.com/

Oh thanks I wasn't cc'ed on that one, makes sense.

You can add my reviewed tag to this one, as my HSE comment doesn't change this.

Reviewed-by: Nicolas Boichat <drinkcat@chromium.org>

But please double check HSE.

>
> >
> > BTW, are you sure DSIM_HSE_MODE is correct now?
>
> Yes, we have tested in imx8m platforms as well. Sébastien Szymanski
> initially observed this issue on the imx8m platform.

I'll repeat, are you sure about HSE specifically? You invert the
polarity for HBP, HFP, and HSA, which makes sense given your patch
02/14.

I'm concerned about HSE. Is the bit really a disable bit?
MIPI_DSI_MODE_VIDEO_HSE is supposed to be an enable flag, so you
should not do `reg |= DSIM_HSE_DISABLE;`, probably.

Thanks,

>
> Jagan.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel