On Mon, Dec 12, 2022 at 2:28 PM Marek Szyprowski
<m.szyprowski@xxxxxxxxxxx> wrote:
>
> Hi Jagan,
>
> On 09.12.2022 16:23, Jagan Teki wrote:
> > HFP/HBP/HSA/EOT_PACKET modes in Exynos DSI host specifies
> > 0 = Enable and 1 = Disable.
> >
> > The logic for checking these mode flags was correct before
> > the MIPI_DSI*_NO_* mode flag conversion.
> >
> > Fix the MIPI_DSI*_NO_* mode flags handling.
> >
> > Fixes: <0f3b68b66a6d> ("drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling
> > features")
> > Reviewed-by: Nicolas Boichat <drinkcat@xxxxxxxxxxxx>
> > Reported-by: Sébastien Szymanski <sebastien.szymanski@xxxxxxxxxxxx>
> > Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> > ---
> > Changes for v9:
> > - none
> >
> > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > index e5b1540c4ae4..50a2a9ca88a9 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > @@ -805,15 +805,15 @@ static int exynos_dsi_init_link(struct exynos_dsi *dsi)
> > reg |= DSIM_AUTO_MODE;
> > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_HSE)
> > reg |= DSIM_HSE_MODE;
> > - if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP))
> > + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP)
> > reg |= DSIM_HFP_MODE;
> > - if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP))
> > + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP)
> > reg |= DSIM_HBP_MODE;
> > - if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA))
> > + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA)
> > reg |= DSIM_HSA_MODE;
> > }
> >
> > - if (!(dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET))
> > + if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)
> > reg |= DSIM_EOT_DISABLE;
> >
> > switch (dsi->format) {
>
>
> Huh, this changes the logic in the driver! I've spent another half of
> the night trying to figure out why v8 and v9 doesn't work on all my
> Exynos boards with DSI panels again...
>
> Please drop this patch from this series. If you want to get the Exynos
> DSI -> Samsung DSIM conversion merged, please focus on the core patches
> and don't add more random 'fixes' to each new version.
>
> This change has to be discussed separately. The values written by the
> Exynos DSI driver to the registers ARE CORRECT and DSI panels work fine
> with such configuration. So either everything is correct, or these flags
> are reversed both in the Exynos DSI driver AND at least tested DSI
> panels (s6e8aa0, s6e3ha2, s6e63j0x03). I would need to check this in
> panel datasheets if I manage to get them.
This issue was reproduced as part of the series in v7 in i.MX8m
platform and reported by Sébastien Szymanski [1] and TRM matches the
fix as well.
[1] https://lore.kernel.org/all/4c9475d0-f76f-0c59-1208-6e5395496c9e@xxxxxxxxxxxx/
Jagan.
