diff mbox series

drm/mcde/panel: Inverse misunderstood flag

Message ID 20210304004138.1785057-1-linus.walleij@linaro.org
State Accepted
Commit d0c5ac04e7feedbc069f26f4dcbf35b521ae7fc5
Headers show
Series drm/mcde/panel: Inverse misunderstood flag | expand

Commit Message

Linus Walleij March 4, 2021, 12:41 a.m. UTC 
A recent patch renaming MIPI_DSI_MODE_EOT_PACKET to
MIPI_DSI_MODE_NO_EOT_PACKET brought to light the
misunderstanding in the current MCDE driver and all
its associated panel drivers that MIPI_DSI_MODE_EOT_PACKET
would mean "use EOT packet" when in fact it means the
reverse.

Fix it up by implementing the flag right in the MCDE
DSI driver and remove the flag from panels that actually
want the EOT packet.

Suggested-by: Nicolas Boichat <drinkcat@chromium.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 drivers/gpu/drm/mcde/mcde_dsi.c                   | 2 +-
 drivers/gpu/drm/panel/panel-novatek-nt35510.c     | 3 +--
 drivers/gpu/drm/panel/panel-samsung-s6d16d0.c     | 4 +---
 drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c | 1 -
 drivers/gpu/drm/panel/panel-sony-acx424akp.c      | 3 +--
 5 files changed, 4 insertions(+), 9 deletions(-)

-- 
2.29.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Comments

Nicolas Boichat March 4, 2021, 1:56 a.m. UTC | #1 
On Thu, Mar 4, 2021 at 8:41 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>

> A recent patch renaming MIPI_DSI_MODE_EOT_PACKET to

> MIPI_DSI_MODE_NO_EOT_PACKET brought to light the

> misunderstanding in the current MCDE driver and all

> its associated panel drivers that MIPI_DSI_MODE_EOT_PACKET

> would mean "use EOT packet" when in fact it means the

> reverse.

>

> Fix it up by implementing the flag right in the MCDE

> DSI driver and remove the flag from panels that actually

> want the EOT packet.

>

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

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>


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


I wonder if it's worth adding the fixes, should be:

Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")
Fixes: 899f24ed8d3a ("drm/panel: Add driver for Novatek NT35510-based panels")
Fixes: ac1d6d74884e ("drm/panel: Add driver for Samsung S6D16D0 panel")
Fixes: 435e06c06cb2 ("drm/panel: s6e63m0: Add DSI transport")
Fixes: 8152c2bfd780 ("drm/panel: Add driver for Sony ACX424AKP panel")

But then you'd almost need to separate the patches in multiple bits
(these patches landed in very different releases).

I'll leave that up to the maintainers to decide: this would only
matter if anybody tried to use these panels on LTS releases with a
non-MCDE driver (or MCDE with other panels).

> ---

>  drivers/gpu/drm/mcde/mcde_dsi.c                   | 2 +-

>  drivers/gpu/drm/panel/panel-novatek-nt35510.c     | 3 +--

>  drivers/gpu/drm/panel/panel-samsung-s6d16d0.c     | 4 +---

>  drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c | 1 -

>  drivers/gpu/drm/panel/panel-sony-acx424akp.c      | 3 +--

>  5 files changed, 4 insertions(+), 9 deletions(-)

>

> diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c

> index 2314c8122992..b3fd3501c412 100644

> --- a/drivers/gpu/drm/mcde/mcde_dsi.c

> +++ b/drivers/gpu/drm/mcde/mcde_dsi.c

> @@ -760,7 +760,7 @@ static void mcde_dsi_start(struct mcde_dsi *d)

>                 DSI_MCTL_MAIN_DATA_CTL_BTA_EN |

>                 DSI_MCTL_MAIN_DATA_CTL_READ_EN |

>                 DSI_MCTL_MAIN_DATA_CTL_REG_TE_EN;

> -       if (d->mdsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET)

> +       if (!(d->mdsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET))

>                 val |= DSI_MCTL_MAIN_DATA_CTL_HOST_EOT_GEN;

>         writel(val, d->regs + DSI_MCTL_MAIN_DATA_CTL);

>

> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c

> index b9a0e56f33e2..ef70140c5b09 100644

> --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c

> +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c

> @@ -898,8 +898,7 @@ static int nt35510_probe(struct mipi_dsi_device *dsi)

>          */

>         dsi->hs_rate = 349440000;

>         dsi->lp_rate = 9600000;

> -       dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS |

> -               MIPI_DSI_MODE_EOT_PACKET;

> +       dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS;

>

>         /*

>          * Every new incarnation of this display must have a unique

> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c

> index 4aac0d1573dd..70560cac53a9 100644

> --- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c

> +++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c

> @@ -184,9 +184,7 @@ static int s6d16d0_probe(struct mipi_dsi_device *dsi)

>          * As we only send commands we do not need to be continuously

>          * clocked.

>          */

> -       dsi->mode_flags =

> -               MIPI_DSI_CLOCK_NON_CONTINUOUS |

> -               MIPI_DSI_MODE_EOT_PACKET;

> +       dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS;

>

>         s6->supply = devm_regulator_get(dev, "vdd1");

>         if (IS_ERR(s6->supply))

> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c

> index eec74c10ddda..9c3563c61e8c 100644

> --- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c

> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c

> @@ -97,7 +97,6 @@ static int s6e63m0_dsi_probe(struct mipi_dsi_device *dsi)

>         dsi->hs_rate = 349440000;

>         dsi->lp_rate = 9600000;

>         dsi->mode_flags = MIPI_DSI_MODE_VIDEO |

> -               MIPI_DSI_MODE_EOT_PACKET |

>                 MIPI_DSI_MODE_VIDEO_BURST;

>

>         ret = s6e63m0_probe(dev, s6e63m0_dsi_dcs_read, s6e63m0_dsi_dcs_write,

> diff --git a/drivers/gpu/drm/panel/panel-sony-acx424akp.c b/drivers/gpu/drm/panel/panel-sony-acx424akp.c

> index 065efae213f5..95659a4d15e9 100644

> --- a/drivers/gpu/drm/panel/panel-sony-acx424akp.c

> +++ b/drivers/gpu/drm/panel/panel-sony-acx424akp.c

> @@ -449,8 +449,7 @@ static int acx424akp_probe(struct mipi_dsi_device *dsi)

>                         MIPI_DSI_MODE_VIDEO_BURST;

>         else

>                 dsi->mode_flags =

> -                       MIPI_DSI_CLOCK_NON_CONTINUOUS |

> -                       MIPI_DSI_MODE_EOT_PACKET;

> +                       MIPI_DSI_CLOCK_NON_CONTINUOUS;

>

>         acx->supply = devm_regulator_get(dev, "vddi");

>         if (IS_ERR(acx->supply))

> --

> 2.29.2

>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Linus Walleij March 4, 2021, 8:20 a.m. UTC | #2 
On Thu, Mar 4, 2021 at 2:57 AM Nicolas Boichat <drinkcat@chromium.org> wrote:
> On Thu, Mar 4, 2021 at 8:41 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> >

> > A recent patch renaming MIPI_DSI_MODE_EOT_PACKET to

> > MIPI_DSI_MODE_NO_EOT_PACKET brought to light the

> > misunderstanding in the current MCDE driver and all

> > its associated panel drivers that MIPI_DSI_MODE_EOT_PACKET

> > would mean "use EOT packet" when in fact it means the

> > reverse.

> >

> > Fix it up by implementing the flag right in the MCDE

> > DSI driver and remove the flag from panels that actually

> > want the EOT packet.

> >

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

> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

>

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

>

> I wonder if it's worth adding the fixes, should be:

>

> Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")

> Fixes: 899f24ed8d3a ("drm/panel: Add driver for Novatek NT35510-based panels")

> Fixes: ac1d6d74884e ("drm/panel: Add driver for Samsung S6D16D0 panel")

> Fixes: 435e06c06cb2 ("drm/panel: s6e63m0: Add DSI transport")

> Fixes: 8152c2bfd780 ("drm/panel: Add driver for Sony ACX424AKP panel")

>

> But then you'd almost need to separate the patches in multiple bits

> (these patches landed in very different releases).

>

> I'll leave that up to the maintainers to decide: this would only

> matter if anybody tried to use these panels on LTS releases with a

> non-MCDE driver (or MCDE with other panels).


It's not even a regression until someone else starts to use these panel
drivers (currently noone does), so I'd say let's just apply this and then
rebase and apply your cleanup on top.

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
index 2314c8122992..b3fd3501c412 100644
--- a/drivers/gpu/drm/mcde/mcde_dsi.c
+++ b/drivers/gpu/drm/mcde/mcde_dsi.c
@@ -760,7 +760,7 @@  static void mcde_dsi_start(struct mcde_dsi *d)
 		DSI_MCTL_MAIN_DATA_CTL_BTA_EN |
 		DSI_MCTL_MAIN_DATA_CTL_READ_EN |
 		DSI_MCTL_MAIN_DATA_CTL_REG_TE_EN;
-	if (d->mdsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET)
+	if (!(d->mdsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET))
 		val |= DSI_MCTL_MAIN_DATA_CTL_HOST_EOT_GEN;
 	writel(val, d->regs + DSI_MCTL_MAIN_DATA_CTL);
 
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
index b9a0e56f33e2..ef70140c5b09 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
@@ -898,8 +898,7 @@  static int nt35510_probe(struct mipi_dsi_device *dsi)
 	 */
 	dsi->hs_rate = 349440000;
 	dsi->lp_rate = 9600000;
-	dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS |
-		MIPI_DSI_MODE_EOT_PACKET;
+	dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS;
 
 	/*
 	 * Every new incarnation of this display must have a unique
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
index 4aac0d1573dd..70560cac53a9 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
@@ -184,9 +184,7 @@  static int s6d16d0_probe(struct mipi_dsi_device *dsi)
 	 * As we only send commands we do not need to be continuously
 	 * clocked.
 	 */
-	dsi->mode_flags =
-		MIPI_DSI_CLOCK_NON_CONTINUOUS |
-		MIPI_DSI_MODE_EOT_PACKET;
+	dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS;
 
 	s6->supply = devm_regulator_get(dev, "vdd1");
 	if (IS_ERR(s6->supply))
diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c
index eec74c10ddda..9c3563c61e8c 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c
@@ -97,7 +97,6 @@  static int s6e63m0_dsi_probe(struct mipi_dsi_device *dsi)
 	dsi->hs_rate = 349440000;
 	dsi->lp_rate = 9600000;
 	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
-		MIPI_DSI_MODE_EOT_PACKET |
 		MIPI_DSI_MODE_VIDEO_BURST;
 
 	ret = s6e63m0_probe(dev, s6e63m0_dsi_dcs_read, s6e63m0_dsi_dcs_write,
diff --git a/drivers/gpu/drm/panel/panel-sony-acx424akp.c b/drivers/gpu/drm/panel/panel-sony-acx424akp.c
index 065efae213f5..95659a4d15e9 100644
--- a/drivers/gpu/drm/panel/panel-sony-acx424akp.c
+++ b/drivers/gpu/drm/panel/panel-sony-acx424akp.c
@@ -449,8 +449,7 @@  static int acx424akp_probe(struct mipi_dsi_device *dsi)
 			MIPI_DSI_MODE_VIDEO_BURST;
 	else
 		dsi->mode_flags =
-			MIPI_DSI_CLOCK_NON_CONTINUOUS |
-			MIPI_DSI_MODE_EOT_PACKET;
+			MIPI_DSI_CLOCK_NON_CONTINUOUS;
 
 	acx->supply = devm_regulator_get(dev, "vddi");
 	if (IS_ERR(acx->supply))