[PATCH 0/3] drm/omap: fix am4 evm lcd

[PATCH 0/3] drm/omap: fix am4 evm lcd

[Tomi Valkeinen]

Hi Tony, Thierry, Laurent,

After the recent change of moving from omapdrm specific panel-dpi driver
to the DRM simple panel, AM4 EVM/ePOS's panel is not working quite
right. This series has fixes for it, but I'm not sure if these are the
right ways to fix the issues, so comments welcome.

1) Panel driver is not probed. With omapdrm's panel-dpi, the match
happened with "panel-dpi" compatible string. Now with panel-simple, the
match should happen with the panel model compatible string, which is
"osddisplays,osd057T0559-34ts" in the DT file. However, no such
compatible exists in panel-simple.

Interestingly, the actual panel at least on my EVMs and ePOSes is not
osd057T0559-34ts, but osd070t1718-19ts. Also, I was unable to find any
information about osd057T0559-34ts. I don't know the history with this,
so it is possible that the early versions of the boards did have
osd057T0559-34ts, but was later changed to osd070t1718-19ts.

As osd070t1718-19ts is supported by panel-simple, changing the
compatible string to osd070t1718-19ts in the DT file solves this one.

2) Timings in DT file cause a kernel warning. Omapdrm's panel-dpi used
video timings from the DT file, so they are present in all the DT files.
panel-simple uses timings from a table in the panel-simple driver, but
gives a kernel warning if the DT file contains timings.

This can be solved by removing the timings from the DT file.

3) Sync drive edge is not right. This one might have been present also
with panel-dpi, I didn't verify. The problem is that the panel-simple
data for osddisplays_osd070t1718_19ts defines bus_flags for DE polarity
and pixdata edge, but not for sync edge.

The datasheet for the panel does not give any hint on what the edge
should be.  Omapdrm defaults to driving syncs on falling edge, which
caused the image to be shifted one pixel to the right.

Adding DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE bus_flag solves the problem. AM5
EVM also has the same panel with the same behavior.

====

The reason I'm not sure if the 1) and 2) fixes are correct is that
they're breaking DT compatibility. Should we instead make changes to
panel-simple to keep the same DT files working?

This would mean adding a new entry for the osd057T0559-34ts panel, but
as we don't have datasheet for it, I think we could just append the
compatible string to osd070t1718-19t's data.

It would also mean doing some change to the panel-simple code that gives
the warning about timings in DT data. This might make sense, as I think
we have other DT files with video timings too.

For 3), I think the patch is fine, but I'm not sure if the display
controller driver should be able to deduce the sync drive edge from the
pixdata drive edge. Are they usually the same? I have no idea...

 Tomi

Tomi Valkeinen (3):
  ARM: dts: am437x-gp/epos-evm: fix panel compatible
  ARM: dts: am437x-gp/epos-evm: drop unused panel timings
  drm/panel: simple: fix osd070t1718_19ts sync drive edge

 arch/arm/boot/dts/am437x-gp-evm.dts  | 18 +-----------------
 arch/arm/boot/dts/am43x-epos-evm.dts | 18 +-----------------
 drivers/gpu/drm/panel/panel-simple.c |  3 ++-
 3 files changed, 4 insertions(+), 35 deletions(-)

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 0/3] drm/omap: fix am4 evm lcd

[Tomi Valkeinen]

Hi Tony, Thierry, Laurent,

After the recent change of moving from omapdrm specific panel-dpi driver
to the DRM simple panel, AM4 EVM/ePOS's panel is not working quite
right. This series has fixes for it, but I'm not sure if these are the
right ways to fix the issues, so comments welcome.

1) Panel driver is not probed. With omapdrm's panel-dpi, the match
happened with "panel-dpi" compatible string. Now with panel-simple, the
match should happen with the panel model compatible string, which is
"osddisplays,osd057T0559-34ts" in the DT file. However, no such
compatible exists in panel-simple.

Interestingly, the actual panel at least on my EVMs and ePOSes is not
osd057T0559-34ts, but osd070t1718-19ts. Also, I was unable to find any
information about osd057T0559-34ts. I don't know the history with this,
so it is possible that the early versions of the boards did have
osd057T0559-34ts, but was later changed to osd070t1718-19ts.

As osd070t1718-19ts is supported by panel-simple, changing the
compatible string to osd070t1718-19ts in the DT file solves this one.

2) Timings in DT file cause a kernel warning. Omapdrm's panel-dpi used
video timings from the DT file, so they are present in all the DT files.
panel-simple uses timings from a table in the panel-simple driver, but
gives a kernel warning if the DT file contains timings.

This can be solved by removing the timings from the DT file.

3) Sync drive edge is not right. This one might have been present also
with panel-dpi, I didn't verify. The problem is that the panel-simple
data for osddisplays_osd070t1718_19ts defines bus_flags for DE polarity
and pixdata edge, but not for sync edge.

The datasheet for the panel does not give any hint on what the edge
should be.  Omapdrm defaults to driving syncs on falling edge, which
caused the image to be shifted one pixel to the right.

Adding DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE bus_flag solves the problem. AM5
EVM also has the same panel with the same behavior.

====

The reason I'm not sure if the 1) and 2) fixes are correct is that
they're breaking DT compatibility. Should we instead make changes to
panel-simple to keep the same DT files working?

This would mean adding a new entry for the osd057T0559-34ts panel, but
as we don't have datasheet for it, I think we could just append the
compatible string to osd070t1718-19t's data.

It would also mean doing some change to the panel-simple code that gives
the warning about timings in DT data. This might make sense, as I think
we have other DT files with video timings too.

For 3), I think the patch is fine, but I'm not sure if the display
controller driver should be able to deduce the sync drive edge from the
pixdata drive edge. Are they usually the same? I have no idea...

 Tomi

Tomi Valkeinen (3):
  ARM: dts: am437x-gp/epos-evm: fix panel compatible
  ARM: dts: am437x-gp/epos-evm: drop unused panel timings
  drm/panel: simple: fix osd070t1718_19ts sync drive edge

 arch/arm/boot/dts/am437x-gp-evm.dts  | 18 +-----------------
 arch/arm/boot/dts/am43x-epos-evm.dts | 18 +-----------------
 drivers/gpu/drm/panel/panel-simple.c |  3 ++-
 3 files changed, 4 insertions(+), 35 deletions(-)

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 1/3] ARM: dts: am437x-gp/epos-evm: fix panel compatible

[Tomi Valkeinen]

The LCD panel on AM4 GP EVMs and ePOS boards seems to be
osd070t1718-19ts. The current dts files say osd057T0559-34ts. Possibly
the panel has changed since the early EVMs, or there has been a mistake
with the panel type.

Update the DT files accordingly.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/boot/dts/am437x-gp-evm.dts  | 2 +-
 arch/arm/boot/dts/am43x-epos-evm.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
index cae4500194fe..811c8cae315b 100644
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -86,7 +86,7 @@
 		};
 
 	lcd0: display {
-		compatible = "osddisplays,osd057T0559-34ts", "panel-dpi";
+		compatible = "osddisplays,osd070t1718-19ts", "panel-dpi";
 		label = "lcd";
 
 		backlight = <&lcd_bl>;
diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts
index 95314121d111..078cb473fa7d 100644
--- a/arch/arm/boot/dts/am43x-epos-evm.dts
+++ b/arch/arm/boot/dts/am43x-epos-evm.dts
@@ -42,7 +42,7 @@
 	};
 
 	lcd0: display {
-		compatible = "osddisplays,osd057T0559-34ts", "panel-dpi";
+		compatible = "osddisplays,osd070t1718-19ts", "panel-dpi";
 		label = "lcd";
 
 		backlight = <&lcd_bl>;
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 1/3] ARM: dts: am437x-gp/epos-evm: fix panel compatible

[Tomi Valkeinen]

The LCD panel on AM4 GP EVMs and ePOS boards seems to be
osd070t1718-19ts. The current dts files say osd057T0559-34ts. Possibly
the panel has changed since the early EVMs, or there has been a mistake
with the panel type.

Update the DT files accordingly.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/boot/dts/am437x-gp-evm.dts  | 2 +-
 arch/arm/boot/dts/am43x-epos-evm.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
index cae4500194fe..811c8cae315b 100644
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -86,7 +86,7 @@
 		};
 
 	lcd0: display {
-		compatible = "osddisplays,osd057T0559-34ts", "panel-dpi";
+		compatible = "osddisplays,osd070t1718-19ts", "panel-dpi";
 		label = "lcd";
 
 		backlight = <&lcd_bl>;
diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts
index 95314121d111..078cb473fa7d 100644
--- a/arch/arm/boot/dts/am43x-epos-evm.dts
+++ b/arch/arm/boot/dts/am43x-epos-evm.dts
@@ -42,7 +42,7 @@
 	};
 
 	lcd0: display {
-		compatible = "osddisplays,osd057T0559-34ts", "panel-dpi";
+		compatible = "osddisplays,osd070t1718-19ts", "panel-dpi";
 		label = "lcd";
 
 		backlight = <&lcd_bl>;
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Tomi Valkeinen]

panel-simple now handled panel osd070t1718-19ts, and we no longer need
the panel timings in the DT file. So remove them.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/boot/dts/am437x-gp-evm.dts  | 16 ----------------
 arch/arm/boot/dts/am43x-epos-evm.dts | 16 ----------------
 2 files changed, 32 deletions(-)

diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
index 811c8cae315b..040403049cea 100644
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -91,22 +91,6 @@
 
 		backlight = <&lcd_bl>;
 
-		panel-timing {
-			clock-frequency = <33000000>;
-			hactive = <800>;
-			vactive = <480>;
-			hfront-porch = <210>;
-			hback-porch = <16>;
-			hsync-len = <30>;
-			vback-porch = <10>;
-			vfront-porch = <22>;
-			vsync-len = <13>;
-			hsync-active = <0>;
-			vsync-active = <0>;
-			de-active = <1>;
-			pixelclk-active = <1>;
-		};
-
 		port {
 			lcd_in: endpoint {
 				remote-endpoint = <&dpi_out>;
diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts
index 078cb473fa7d..330fce060e8b 100644
--- a/arch/arm/boot/dts/am43x-epos-evm.dts
+++ b/arch/arm/boot/dts/am43x-epos-evm.dts
@@ -47,22 +47,6 @@
 
 		backlight = <&lcd_bl>;
 
-		panel-timing {
-			clock-frequency = <33000000>;
-			hactive = <800>;
-			vactive = <480>;
-			hfront-porch = <210>;
-			hback-porch = <16>;
-			hsync-len = <30>;
-			vback-porch = <10>;
-			vfront-porch = <22>;
-			vsync-len = <13>;
-			hsync-active = <0>;
-			vsync-active = <0>;
-			de-active = <1>;
-			pixelclk-active = <1>;
-		};
-
 		port {
 			lcd_in: endpoint {
 				remote-endpoint = <&dpi_out>;
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Tomi Valkeinen]

panel-simple now handled panel osd070t1718-19ts, and we no longer need
the panel timings in the DT file. So remove them.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/boot/dts/am437x-gp-evm.dts  | 16 ----------------
 arch/arm/boot/dts/am43x-epos-evm.dts | 16 ----------------
 2 files changed, 32 deletions(-)

diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
index 811c8cae315b..040403049cea 100644
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -91,22 +91,6 @@
 
 		backlight = <&lcd_bl>;
 
-		panel-timing {
-			clock-frequency = <33000000>;
-			hactive = <800>;
-			vactive = <480>;
-			hfront-porch = <210>;
-			hback-porch = <16>;
-			hsync-len = <30>;
-			vback-porch = <10>;
-			vfront-porch = <22>;
-			vsync-len = <13>;
-			hsync-active = <0>;
-			vsync-active = <0>;
-			de-active = <1>;
-			pixelclk-active = <1>;
-		};
-
 		port {
 			lcd_in: endpoint {
 				remote-endpoint = <&dpi_out>;
diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts
index 078cb473fa7d..330fce060e8b 100644
--- a/arch/arm/boot/dts/am43x-epos-evm.dts
+++ b/arch/arm/boot/dts/am43x-epos-evm.dts
@@ -47,22 +47,6 @@
 
 		backlight = <&lcd_bl>;
 
-		panel-timing {
-			clock-frequency = <33000000>;
-			hactive = <800>;
-			vactive = <480>;
-			hfront-porch = <210>;
-			hback-porch = <16>;
-			hsync-len = <30>;
-			vback-porch = <10>;
-			vfront-porch = <22>;
-			vsync-len = <13>;
-			hsync-active = <0>;
-			vsync-active = <0>;
-			de-active = <1>;
-			pixelclk-active = <1>;
-		};
-
 		port {
 			lcd_in: endpoint {
 				remote-endpoint = <&dpi_out>;
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 3/3] drm/panel: simple: fix osd070t1718_19ts sync drive edge

[Tomi Valkeinen]

The panel datasheet says that the panel samples at falling edge, but
does not say anything about h/v sync signals. Testing shows that if the
sync signals are driven on falling edge, the picture on the panel will
be slightly shifted right.

Setting sync drive edge to the same as data drive edge fixes this issue.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 5d487686d25c..0784536ae6af 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2397,7 +2397,8 @@ static const struct panel_desc osddisplays_osd070t1718_19ts = {
 		.height = 91,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
-	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
+		DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
 	.connector_type = DRM_MODE_CONNECTOR_DPI,
 };
 
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 3/3] drm/panel: simple: fix osd070t1718_19ts sync drive edge

[Tomi Valkeinen]

The panel datasheet says that the panel samples at falling edge, but
does not say anything about h/v sync signals. Testing shows that if the
sync signals are driven on falling edge, the picture on the panel will
be slightly shifted right.

Setting sync drive edge to the same as data drive edge fixes this issue.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/panel/panel-simple.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 5d487686d25c..0784536ae6af 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2397,7 +2397,8 @@ static const struct panel_desc osddisplays_osd070t1718_19ts = {
 		.height = 91,
 	},
 	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
-	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
+	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
+		DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
 	.connector_type = DRM_MODE_CONNECTOR_DPI,
 };
 
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

Re: [PATCH 0/3] drm/omap: fix am4 evm lcd

[Tomi Valkeinen]

Hi Tony, Thierry, Laurent,

Any thoughts on the below points?

I think yet another option is to write some omap boot time quirks code, which looks at the DT data, 
and changes the panel compatible string (for 1), and removes the timings node (for 2).

  Tomi

On 14/11/2019 11:39, Tomi Valkeinen wrote:
> Hi Tony, Thierry, Laurent,
> 
> After the recent change of moving from omapdrm specific panel-dpi driver
> to the DRM simple panel, AM4 EVM/ePOS's panel is not working quite
> right. This series has fixes for it, but I'm not sure if these are the
> right ways to fix the issues, so comments welcome.
> 
> 1) Panel driver is not probed. With omapdrm's panel-dpi, the match
> happened with "panel-dpi" compatible string. Now with panel-simple, the
> match should happen with the panel model compatible string, which is
> "osddisplays,osd057T0559-34ts" in the DT file. However, no such
> compatible exists in panel-simple.
> 
> Interestingly, the actual panel at least on my EVMs and ePOSes is not
> osd057T0559-34ts, but osd070t1718-19ts. Also, I was unable to find any
> information about osd057T0559-34ts. I don't know the history with this,
> so it is possible that the early versions of the boards did have
> osd057T0559-34ts, but was later changed to osd070t1718-19ts.
> 
> As osd070t1718-19ts is supported by panel-simple, changing the
> compatible string to osd070t1718-19ts in the DT file solves this one.
> 
> 2) Timings in DT file cause a kernel warning. Omapdrm's panel-dpi used
> video timings from the DT file, so they are present in all the DT files.
> panel-simple uses timings from a table in the panel-simple driver, but
> gives a kernel warning if the DT file contains timings.
> 
> This can be solved by removing the timings from the DT file.
> 
> 3) Sync drive edge is not right. This one might have been present also
> with panel-dpi, I didn't verify. The problem is that the panel-simple
> data for osddisplays_osd070t1718_19ts defines bus_flags for DE polarity
> and pixdata edge, but not for sync edge.
> 
> The datasheet for the panel does not give any hint on what the edge
> should be.  Omapdrm defaults to driving syncs on falling edge, which
> caused the image to be shifted one pixel to the right.
> 
> Adding DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE bus_flag solves the problem. AM5
> EVM also has the same panel with the same behavior.
> 
> ====
> 
> The reason I'm not sure if the 1) and 2) fixes are correct is that
> they're breaking DT compatibility. Should we instead make changes to
> panel-simple to keep the same DT files working?
> 
> This would mean adding a new entry for the osd057T0559-34ts panel, but
> as we don't have datasheet for it, I think we could just append the
> compatible string to osd070t1718-19t's data.
> 
> It would also mean doing some change to the panel-simple code that gives
> the warning about timings in DT data. This might make sense, as I think
> we have other DT files with video timings too.
> 
> For 3), I think the patch is fine, but I'm not sure if the display
> controller driver should be able to deduce the sync drive edge from the
> pixdata drive edge. Are they usually the same? I have no idea...
> 
>   Tomi
> 
> Tomi Valkeinen (3):
>    ARM: dts: am437x-gp/epos-evm: fix panel compatible
>    ARM: dts: am437x-gp/epos-evm: drop unused panel timings
>    drm/panel: simple: fix osd070t1718_19ts sync drive edge
> 
>   arch/arm/boot/dts/am437x-gp-evm.dts  | 18 +-----------------
>   arch/arm/boot/dts/am43x-epos-evm.dts | 18 +-----------------
>   drivers/gpu/drm/panel/panel-simple.c |  3 ++-
>   3 files changed, 4 insertions(+), 35 deletions(-)
> 
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/omap: fix am4 evm lcd

[Tomi Valkeinen]

Hi Tony, Thierry, Laurent,

Any thoughts on the below points?

I think yet another option is to write some omap boot time quirks code, which looks at the DT data, 
and changes the panel compatible string (for 1), and removes the timings node (for 2).

  Tomi

On 14/11/2019 11:39, Tomi Valkeinen wrote:
> Hi Tony, Thierry, Laurent,
> 
> After the recent change of moving from omapdrm specific panel-dpi driver
> to the DRM simple panel, AM4 EVM/ePOS's panel is not working quite
> right. This series has fixes for it, but I'm not sure if these are the
> right ways to fix the issues, so comments welcome.
> 
> 1) Panel driver is not probed. With omapdrm's panel-dpi, the match
> happened with "panel-dpi" compatible string. Now with panel-simple, the
> match should happen with the panel model compatible string, which is
> "osddisplays,osd057T0559-34ts" in the DT file. However, no such
> compatible exists in panel-simple.
> 
> Interestingly, the actual panel at least on my EVMs and ePOSes is not
> osd057T0559-34ts, but osd070t1718-19ts. Also, I was unable to find any
> information about osd057T0559-34ts. I don't know the history with this,
> so it is possible that the early versions of the boards did have
> osd057T0559-34ts, but was later changed to osd070t1718-19ts.
> 
> As osd070t1718-19ts is supported by panel-simple, changing the
> compatible string to osd070t1718-19ts in the DT file solves this one.
> 
> 2) Timings in DT file cause a kernel warning. Omapdrm's panel-dpi used
> video timings from the DT file, so they are present in all the DT files.
> panel-simple uses timings from a table in the panel-simple driver, but
> gives a kernel warning if the DT file contains timings.
> 
> This can be solved by removing the timings from the DT file.
> 
> 3) Sync drive edge is not right. This one might have been present also
> with panel-dpi, I didn't verify. The problem is that the panel-simple
> data for osddisplays_osd070t1718_19ts defines bus_flags for DE polarity
> and pixdata edge, but not for sync edge.
> 
> The datasheet for the panel does not give any hint on what the edge
> should be.  Omapdrm defaults to driving syncs on falling edge, which
> caused the image to be shifted one pixel to the right.
> 
> Adding DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE bus_flag solves the problem. AM5
> EVM also has the same panel with the same behavior.
> 
> ====
> 
> The reason I'm not sure if the 1) and 2) fixes are correct is that
> they're breaking DT compatibility. Should we instead make changes to
> panel-simple to keep the same DT files working?
> 
> This would mean adding a new entry for the osd057T0559-34ts panel, but
> as we don't have datasheet for it, I think we could just append the
> compatible string to osd070t1718-19t's data.
> 
> It would also mean doing some change to the panel-simple code that gives
> the warning about timings in DT data. This might make sense, as I think
> we have other DT files with video timings too.
> 
> For 3), I think the patch is fine, but I'm not sure if the display
> controller driver should be able to deduce the sync drive edge from the
> pixdata drive edge. Are they usually the same? I have no idea...
> 
>   Tomi
> 
> Tomi Valkeinen (3):
>    ARM: dts: am437x-gp/epos-evm: fix panel compatible
>    ARM: dts: am437x-gp/epos-evm: drop unused panel timings
>    drm/panel: simple: fix osd070t1718_19ts sync drive edge
> 
>   arch/arm/boot/dts/am437x-gp-evm.dts  | 18 +-----------------
>   arch/arm/boot/dts/am43x-epos-evm.dts | 18 +-----------------
>   drivers/gpu/drm/panel/panel-simple.c |  3 ++-
>   3 files changed, 4 insertions(+), 35 deletions(-)
> 
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/omap: fix am4 evm lcd

[Tony Lindgren]

Hi

* Tomi Valkeinen <tomi.valkeinen@ti.com> [191127 12:59]:
> Hi Tony, Thierry, Laurent,
> 
> Any thoughts on the below points?

> I think yet another option is to write some omap boot time quirks code,
> which looks at the DT data, and changes the panel compatible string (for 1),
> and removes the timings node (for 2).

Nah, seems we can just update the compatible.

> On 14/11/2019 11:39, Tomi Valkeinen wrote:
> > 1) Panel driver is not probed. With omapdrm's panel-dpi, the match
> > happened with "panel-dpi" compatible string. Now with panel-simple, the
> > match should happen with the panel model compatible string, which is
> > "osddisplays,osd057T0559-34ts" in the DT file. However, no such
> > compatible exists in panel-simple.
> >
> > Interestingly, the actual panel at least on my EVMs and ePOSes is not
> > osd057T0559-34ts, but osd070t1718-19ts. Also, I was unable to find any
> > information about osd057T0559-34ts. I don't know the history with this,
> > so it is possible that the early versions of the boards did have
> > osd057T0559-34ts, but was later changed to osd070t1718-19ts.

I guess you could keep the old compatible there too if really needed.
But then again if the old compatible is known to be incorrect, it
should be just updated.

So it looks good to me for the dts changes. Do you want me to pick
them into fixes as it seems that the panel driver fix is a separate
issue?

Regards,

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

Re: [PATCH 0/3] drm/omap: fix am4 evm lcd

[Tomi Valkeinen]

Hi Tony,

On 27/11/2019 17:45, Tony Lindgren wrote:

>>> Interestingly, the actual panel at least on my EVMs and ePOSes is not
>>> osd057T0559-34ts, but osd070t1718-19ts. Also, I was unable to find any
>>> information about osd057T0559-34ts. I don't know the history with this,
>>> so it is possible that the early versions of the boards did have
>>> osd057T0559-34ts, but was later changed to osd070t1718-19ts.
> 
> I guess you could keep the old compatible there too if really needed.
> But then again if the old compatible is known to be incorrect, it
> should be just updated.
> 
> So it looks good to me for the dts changes. Do you want me to pick
> them into fixes as it seems that the panel driver fix is a separate
> issue?

Yes, the third patch can be handled separately, so please pick the first two ones.

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] ARM: dts: am437x-gp/epos-evm: fix panel compatible

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thu, Nov 14, 2019 at 11:39:48AM +0200, Tomi Valkeinen wrote:
> The LCD panel on AM4 GP EVMs and ePOS boards seems to be
> osd070t1718-19ts. The current dts files say osd057T0559-34ts. Possibly
> the panel has changed since the early EVMs, or there has been a mistake
> with the panel type.
> 
> Update the DT files accordingly.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  arch/arm/boot/dts/am437x-gp-evm.dts  | 2 +-
>  arch/arm/boot/dts/am43x-epos-evm.dts | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
> index cae4500194fe..811c8cae315b 100644
> --- a/arch/arm/boot/dts/am437x-gp-evm.dts
> +++ b/arch/arm/boot/dts/am437x-gp-evm.dts
> @@ -86,7 +86,7 @@
>  		};
>  
>  	lcd0: display {
> -		compatible = "osddisplays,osd057T0559-34ts", "panel-dpi";
> +		compatible = "osddisplays,osd070t1718-19ts", "panel-dpi";
>  		label = "lcd";
>  
>  		backlight = <&lcd_bl>;
> diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts
> index 95314121d111..078cb473fa7d 100644
> --- a/arch/arm/boot/dts/am43x-epos-evm.dts
> +++ b/arch/arm/boot/dts/am43x-epos-evm.dts
> @@ -42,7 +42,7 @@
>  	};
>  
>  	lcd0: display {
> -		compatible = "osddisplays,osd057T0559-34ts", "panel-dpi";
> +		compatible = "osddisplays,osd070t1718-19ts", "panel-dpi";
>  		label = "lcd";
>  
>  		backlight = <&lcd_bl>;

-- 
Regards,

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

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thu, Nov 14, 2019 at 11:39:49AM +0200, Tomi Valkeinen wrote:
> panel-simple now handled panel osd070t1718-19ts, and we no longer need
> the panel timings in the DT file. So remove them.

Should you in that case drop the panel-dpi compatible string too, as the
panel-dpi bindings require panel timings in DT ?

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  arch/arm/boot/dts/am437x-gp-evm.dts  | 16 ----------------
>  arch/arm/boot/dts/am43x-epos-evm.dts | 16 ----------------
>  2 files changed, 32 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
> index 811c8cae315b..040403049cea 100644
> --- a/arch/arm/boot/dts/am437x-gp-evm.dts
> +++ b/arch/arm/boot/dts/am437x-gp-evm.dts
> @@ -91,22 +91,6 @@
>  
>  		backlight = <&lcd_bl>;
>  
> -		panel-timing {
> -			clock-frequency = <33000000>;
> -			hactive = <800>;
> -			vactive = <480>;
> -			hfront-porch = <210>;
> -			hback-porch = <16>;
> -			hsync-len = <30>;
> -			vback-porch = <10>;
> -			vfront-porch = <22>;
> -			vsync-len = <13>;
> -			hsync-active = <0>;
> -			vsync-active = <0>;
> -			de-active = <1>;
> -			pixelclk-active = <1>;
> -		};
> -
>  		port {
>  			lcd_in: endpoint {
>  				remote-endpoint = <&dpi_out>;
> diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts
> index 078cb473fa7d..330fce060e8b 100644
> --- a/arch/arm/boot/dts/am43x-epos-evm.dts
> +++ b/arch/arm/boot/dts/am43x-epos-evm.dts
> @@ -47,22 +47,6 @@
>  
>  		backlight = <&lcd_bl>;
>  
> -		panel-timing {
> -			clock-frequency = <33000000>;
> -			hactive = <800>;
> -			vactive = <480>;
> -			hfront-porch = <210>;
> -			hback-porch = <16>;
> -			hsync-len = <30>;
> -			vback-porch = <10>;
> -			vfront-porch = <22>;
> -			vsync-len = <13>;
> -			hsync-active = <0>;
> -			vsync-active = <0>;
> -			de-active = <1>;
> -			pixelclk-active = <1>;
> -		};
> -
>  		port {
>  			lcd_in: endpoint {
>  				remote-endpoint = <&dpi_out>;

-- 
Regards,

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

Re: [PATCH 3/3] drm/panel: simple: fix osd070t1718_19ts sync drive edge

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Thu, Nov 14, 2019 at 11:39:50AM +0200, Tomi Valkeinen wrote:
> The panel datasheet says that the panel samples at falling edge, but
> does not say anything about h/v sync signals. Testing shows that if the
> sync signals are driven on falling edge, the picture on the panel will
> be slightly shifted right.
> 
> Setting sync drive edge to the same as data drive edge fixes this issue.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

I don't have access to the documentation, but this makes sense, so

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/panel/panel-simple.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index 5d487686d25c..0784536ae6af 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -2397,7 +2397,8 @@ static const struct panel_desc osddisplays_osd070t1718_19ts = {
>  		.height = 91,
>  	},
>  	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
> +		DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
>  	.connector_type = DRM_MODE_CONNECTOR_DPI,
>  };
>  

-- 
Regards,

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

Re: [PATCH 0/3] drm/omap: fix am4 evm lcd

[Laurent Pinchart]

Hi Tomi,

On Thu, Nov 14, 2019 at 11:39:47AM +0200, Tomi Valkeinen wrote:
> Hi Tony, Thierry, Laurent,
> 
> After the recent change of moving from omapdrm specific panel-dpi driver
> to the DRM simple panel, AM4 EVM/ePOS's panel is not working quite
> right. This series has fixes for it, but I'm not sure if these are the
> right ways to fix the issues, so comments welcome.
> 
> 1) Panel driver is not probed. With omapdrm's panel-dpi, the match
> happened with "panel-dpi" compatible string. Now with panel-simple, the
> match should happen with the panel model compatible string, which is
> "osddisplays,osd057T0559-34ts" in the DT file. However, no such
> compatible exists in panel-simple.

I've also noticed that we have a few other omap-based platforms that
got broken, for the same reason :-( We're missing driver support for
innolux,at070tn83, samsung,lte430wq-f0c and startek,startek-kd050c, and
we have a few nodes that use panel-dpi without any more precise
compatible string.

> Interestingly, the actual panel at least on my EVMs and ePOSes is not
> osd057T0559-34ts, but osd070t1718-19ts. Also, I was unable to find any
> information about osd057T0559-34ts. I don't know the history with this,
> so it is possible that the early versions of the boards did have
> osd057T0559-34ts, but was later changed to osd070t1718-19ts.
> 
> As osd070t1718-19ts is supported by panel-simple, changing the
> compatible string to osd070t1718-19ts in the DT file solves this one.
> 
> 2) Timings in DT file cause a kernel warning. Omapdrm's panel-dpi used
> video timings from the DT file, so they are present in all the DT files.
> panel-simple uses timings from a table in the panel-simple driver, but
> gives a kernel warning if the DT file contains timings.
> 
> This can be solved by removing the timings from the DT file.
> 
> 3) Sync drive edge is not right. This one might have been present also
> with panel-dpi, I didn't verify. The problem is that the panel-simple
> data for osddisplays_osd070t1718_19ts defines bus_flags for DE polarity
> and pixdata edge, but not for sync edge.
> 
> The datasheet for the panel does not give any hint on what the edge
> should be.  Omapdrm defaults to driving syncs on falling edge, which
> caused the image to be shifted one pixel to the right.
> 
> Adding DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE bus_flag solves the problem. AM5
> EVM also has the same panel with the same behavior.
> 
> ====
> 
> The reason I'm not sure if the 1) and 2) fixes are correct is that
> they're breaking DT compatibility. Should we instead make changes to
> panel-simple to keep the same DT files working?

That's tempting, as breaking DT is quite bad, but that would require
adding a match on panel-dpi, and parsing timings in the panel-simple
driver. Thierry has always been opposed to that as far as I can tell,
and even if I don't share his point of view, I don't want to move in
this direction without a consensus.

Your series is fine in my opinion, as even if we decide to handle
backward compatibility in this case, changing the DT files in mainline
is still the right way to go (if only to avoid giving wrong examples).

> This would mean adding a new entry for the osd057T0559-34ts panel, but
> as we don't have datasheet for it, I think we could just append the
> compatible string to osd070t1718-19t's data.
> 
> It would also mean doing some change to the panel-simple code that gives
> the warning about timings in DT data. This might make sense, as I think
> we have other DT files with video timings too.
> 
> For 3), I think the patch is fine, but I'm not sure if the display
> controller driver should be able to deduce the sync drive edge from the
> pixdata drive edge. Are they usually the same? I have no idea...
> 
> Tomi Valkeinen (3):
>   ARM: dts: am437x-gp/epos-evm: fix panel compatible
>   ARM: dts: am437x-gp/epos-evm: drop unused panel timings
>   drm/panel: simple: fix osd070t1718_19ts sync drive edge
> 
>  arch/arm/boot/dts/am437x-gp-evm.dts  | 18 +-----------------
>  arch/arm/boot/dts/am43x-epos-evm.dts | 18 +-----------------
>  drivers/gpu/drm/panel/panel-simple.c |  3 ++-
>  3 files changed, 4 insertions(+), 35 deletions(-)

-- 
Regards,

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

Re: [PATCH 0/3] drm/omap: fix am4 evm lcd

[Adam Ford]

On Mon, Dec 2, 2019 at 7:13 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Tomi,
>
> On Thu, Nov 14, 2019 at 11:39:47AM +0200, Tomi Valkeinen wrote:
> > Hi Tony, Thierry, Laurent,
> >
> > After the recent change of moving from omapdrm specific panel-dpi driver
> > to the DRM simple panel, AM4 EVM/ePOS's panel is not working quite
> > right. This series has fixes for it, but I'm not sure if these are the
> > right ways to fix the issues, so comments welcome.
> >
> > 1) Panel driver is not probed. With omapdrm's panel-dpi, the match
> > happened with "panel-dpi" compatible string. Now with panel-simple, the
> > match should happen with the panel model compatible string, which is
> > "osddisplays,osd057T0559-34ts" in the DT file. However, no such
> > compatible exists in panel-simple.
>
> I've also noticed that we have a few other omap-based platforms that
> got broken, for the same reason :-( We're missing driver support for
> innolux,at070tn83, samsung,lte430wq-f0c and startek,startek-kd050c, and
> we have a few nodes that use panel-dpi without any more precise
> compatible string.
>
> > Interestingly, the actual panel at least on my EVMs and ePOSes is not
> > osd057T0559-34ts, but osd070t1718-19ts. Also, I was unable to find any
> > information about osd057T0559-34ts. I don't know the history with this,
> > so it is possible that the early versions of the boards did have
> > osd057T0559-34ts, but was later changed to osd070t1718-19ts.
> >
> > As osd070t1718-19ts is supported by panel-simple, changing the
> > compatible string to osd070t1718-19ts in the DT file solves this one.
> >
> > 2) Timings in DT file cause a kernel warning. Omapdrm's panel-dpi used
> > video timings from the DT file, so they are present in all the DT files.
> > panel-simple uses timings from a table in the panel-simple driver, but
> > gives a kernel warning if the DT file contains timings.
> >
> > This can be solved by removing the timings from the DT file.
> >
> > 3) Sync drive edge is not right. This one might have been present also
> > with panel-dpi, I didn't verify. The problem is that the panel-simple
> > data for osddisplays_osd070t1718_19ts defines bus_flags for DE polarity
> > and pixdata edge, but not for sync edge.
> >
> > The datasheet for the panel does not give any hint on what the edge
> > should be.  Omapdrm defaults to driving syncs on falling edge, which
> > caused the image to be shifted one pixel to the right.
> >
> > Adding DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE bus_flag solves the problem. AM5
> > EVM also has the same panel with the same behavior.
> >
> > ====
> >
> > The reason I'm not sure if the 1) and 2) fixes are correct is that
> > they're breaking DT compatibility. Should we instead make changes to
> > panel-simple to keep the same DT files working?
>
> That's tempting, as breaking DT is quite bad, but that would require
> adding a match on panel-dpi, and parsing timings in the panel-simple
> driver. Thierry has always been opposed to that as far as I can tell,
> and even if I don't share his point of view, I don't want to move in
> this direction without a consensus.

What about a generic driver separate from the simple panel driver that
does just the panel-dpi parsing?

If we exported the necessary functions from simple-panel, we could
call them from the panel-dpi parser and not have to re-invent the
functions to enable power, gpio or back light and/or fix them each
time they get updated.


adam
>
> Your series is fine in my opinion, as even if we decide to handle
> backward compatibility in this case, changing the DT files in mainline
> is still the right way to go (if only to avoid giving wrong examples).
>
> > This would mean adding a new entry for the osd057T0559-34ts panel, but
> > as we don't have datasheet for it, I think we could just append the
> > compatible string to osd070t1718-19t's data.
> >
> > It would also mean doing some change to the panel-simple code that gives
> > the warning about timings in DT data. This might make sense, as I think
> > we have other DT files with video timings too.
> >
> > For 3), I think the patch is fine, but I'm not sure if the display
> > controller driver should be able to deduce the sync drive edge from the
> > pixdata drive edge. Are they usually the same? I have no idea...
> >
> > Tomi Valkeinen (3):
> >   ARM: dts: am437x-gp/epos-evm: fix panel compatible
> >   ARM: dts: am437x-gp/epos-evm: drop unused panel timings
> >   drm/panel: simple: fix osd070t1718_19ts sync drive edge
> >
> >  arch/arm/boot/dts/am437x-gp-evm.dts  | 18 +-----------------
> >  arch/arm/boot/dts/am43x-epos-evm.dts | 18 +-----------------
> >  drivers/gpu/drm/panel/panel-simple.c |  3 ++-
> >  3 files changed, 4 insertions(+), 35 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] drm/omap: fix am4 evm lcd

[Laurent Pinchart]

Hi Adam,

On Mon, Dec 02, 2019 at 07:24:09AM -0600, Adam Ford wrote:
> On Mon, Dec 2, 2019 at 7:13 AM Laurent Pinchart wrote:
> > On Thu, Nov 14, 2019 at 11:39:47AM +0200, Tomi Valkeinen wrote:
> > > Hi Tony, Thierry, Laurent,
> > >
> > > After the recent change of moving from omapdrm specific panel-dpi driver
> > > to the DRM simple panel, AM4 EVM/ePOS's panel is not working quite
> > > right. This series has fixes for it, but I'm not sure if these are the
> > > right ways to fix the issues, so comments welcome.
> > >
> > > 1) Panel driver is not probed. With omapdrm's panel-dpi, the match
> > > happened with "panel-dpi" compatible string. Now with panel-simple, the
> > > match should happen with the panel model compatible string, which is
> > > "osddisplays,osd057T0559-34ts" in the DT file. However, no such
> > > compatible exists in panel-simple.
> >
> > I've also noticed that we have a few other omap-based platforms that
> > got broken, for the same reason :-( We're missing driver support for
> > innolux,at070tn83, samsung,lte430wq-f0c and startek,startek-kd050c, and
> > we have a few nodes that use panel-dpi without any more precise
> > compatible string.
> >
> > > Interestingly, the actual panel at least on my EVMs and ePOSes is not
> > > osd057T0559-34ts, but osd070t1718-19ts. Also, I was unable to find any
> > > information about osd057T0559-34ts. I don't know the history with this,
> > > so it is possible that the early versions of the boards did have
> > > osd057T0559-34ts, but was later changed to osd070t1718-19ts.
> > >
> > > As osd070t1718-19ts is supported by panel-simple, changing the
> > > compatible string to osd070t1718-19ts in the DT file solves this one.
> > >
> > > 2) Timings in DT file cause a kernel warning. Omapdrm's panel-dpi used
> > > video timings from the DT file, so they are present in all the DT files.
> > > panel-simple uses timings from a table in the panel-simple driver, but
> > > gives a kernel warning if the DT file contains timings.
> > >
> > > This can be solved by removing the timings from the DT file.
> > >
> > > 3) Sync drive edge is not right. This one might have been present also
> > > with panel-dpi, I didn't verify. The problem is that the panel-simple
> > > data for osddisplays_osd070t1718_19ts defines bus_flags for DE polarity
> > > and pixdata edge, but not for sync edge.
> > >
> > > The datasheet for the panel does not give any hint on what the edge
> > > should be.  Omapdrm defaults to driving syncs on falling edge, which
> > > caused the image to be shifted one pixel to the right.
> > >
> > > Adding DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE bus_flag solves the problem. AM5
> > > EVM also has the same panel with the same behavior.
> > >
> > > ====
> > >
> > > The reason I'm not sure if the 1) and 2) fixes are correct is that
> > > they're breaking DT compatibility. Should we instead make changes to
> > > panel-simple to keep the same DT files working?
> >
> > That's tempting, as breaking DT is quite bad, but that would require
> > adding a match on panel-dpi, and parsing timings in the panel-simple
> > driver. Thierry has always been opposed to that as far as I can tell,
> > and even if I don't share his point of view, I don't want to move in
> > this direction without a consensus.
> 
> What about a generic driver separate from the simple panel driver that
> does just the panel-dpi parsing?

A separate driver wouldn't make too much of a difference. Thierry's
point was that drivers should hardcode the timings, as the compatible
string gives enough information and encoding timings in DT would
duplicate that information. I have a different point of view, and there
are pros and cons for both options, so we haven't been able to reach an
agreement. That's why I haven't pushed too hard for timings parsing from
DT, regardless of whether it would be in panel-simple or in a separate
driver.

> If we exported the necessary functions from simple-panel, we could
> call them from the panel-dpi parser and not have to re-invent the
> functions to enable power, gpio or back light and/or fix them each
> time they get updated.
> 
> > Your series is fine in my opinion, as even if we decide to handle
> > backward compatibility in this case, changing the DT files in mainline
> > is still the right way to go (if only to avoid giving wrong examples).
> >
> > > This would mean adding a new entry for the osd057T0559-34ts panel, but
> > > as we don't have datasheet for it, I think we could just append the
> > > compatible string to osd070t1718-19t's data.
> > >
> > > It would also mean doing some change to the panel-simple code that gives
> > > the warning about timings in DT data. This might make sense, as I think
> > > we have other DT files with video timings too.
> > >
> > > For 3), I think the patch is fine, but I'm not sure if the display
> > > controller driver should be able to deduce the sync drive edge from the
> > > pixdata drive edge. Are they usually the same? I have no idea...
> > >
> > > Tomi Valkeinen (3):
> > >   ARM: dts: am437x-gp/epos-evm: fix panel compatible
> > >   ARM: dts: am437x-gp/epos-evm: drop unused panel timings
> > >   drm/panel: simple: fix osd070t1718_19ts sync drive edge
> > >
> > >  arch/arm/boot/dts/am437x-gp-evm.dts  | 18 +-----------------
> > >  arch/arm/boot/dts/am43x-epos-evm.dts | 18 +-----------------
> > >  drivers/gpu/drm/panel/panel-simple.c |  3 ++-
> > >  3 files changed, 4 insertions(+), 35 deletions(-)

-- 
Regards,

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

Re: [PATCH 1/3] ARM: dts: am437x-gp/epos-evm: fix panel compatible

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [191202 13:02]:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Thu, Nov 14, 2019 at 11:39:48AM +0200, Tomi Valkeinen wrote:
> > The LCD panel on AM4 GP EVMs and ePOS boards seems to be
> > osd070t1718-19ts. The current dts files say osd057T0559-34ts. Possibly
> > the panel has changed since the early EVMs, or there has been a mistake
> > with the panel type.
> > 
> > Update the DT files accordingly.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Applying this one into fixes thanks.

Regards,

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

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [191202 13:05]:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Thu, Nov 14, 2019 at 11:39:49AM +0200, Tomi Valkeinen wrote:
> > panel-simple now handled panel osd070t1718-19ts, and we no longer need
> > the panel timings in the DT file. So remove them.
> 
> Should you in that case drop the panel-dpi compatible string too, as the
> panel-dpi bindings require panel timings in DT ?

Yeah sounds like if panel-dpi is no longer usable for this device it
should be dropped from the compatible list.

Regards,

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

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Tomi Valkeinen]

On 11/12/2019 18:53, Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [191202 13:05]:
>> Hi Tomi,
>>
>> Thank you for the patch.
>>
>> On Thu, Nov 14, 2019 at 11:39:49AM +0200, Tomi Valkeinen wrote:
>>> panel-simple now handled panel osd070t1718-19ts, and we no longer need
>>> the panel timings in the DT file. So remove them.
>>
>> Should you in that case drop the panel-dpi compatible string too, as the
>> panel-dpi bindings require panel timings in DT ?
> 
> Yeah sounds like if panel-dpi is no longer usable for this device it
> should be dropped from the compatible list.

Ok, I agree.

Looking at the dts files, panel-dpi is used in a bunch of boards. But we even have 3 dts files with 
panel-dpi, without the detailed panel model in compatible...

Fixing those will break the compatibility with old dtbs and new kernel, unless we add 
timings-from-dt to a panel driver that handles panel-dpi.

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Laurent Pinchart]

Hi Tomi,

On Thu, Dec 12, 2019 at 11:37:51AM +0200, Tomi Valkeinen wrote:
> On 11/12/2019 18:53, Tony Lindgren wrote:
> > * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [191202 13:05]:
> >> Hi Tomi,
> >>
> >> Thank you for the patch.
> >>
> >> On Thu, Nov 14, 2019 at 11:39:49AM +0200, Tomi Valkeinen wrote:
> >>> panel-simple now handled panel osd070t1718-19ts, and we no longer need
> >>> the panel timings in the DT file. So remove them.
> >>
> >> Should you in that case drop the panel-dpi compatible string too, as the
> >> panel-dpi bindings require panel timings in DT ?
> > 
> > Yeah sounds like if panel-dpi is no longer usable for this device it
> > should be dropped from the compatible list.
> 
> Ok, I agree.
> 
> Looking at the dts files, panel-dpi is used in a bunch of boards. But
> we even have 3 dts files with panel-dpi, without the detailed panel
> model in compatible...
> 
> Fixing those will break the compatibility with old dtbs and new
> kernel, unless we add timings-from-dt to a panel driver that handles
> panel-dpi.

I know, and I don't have a perfect answer for this :-( I don't see a
third option, it's either breaking DT backward compatibility or adding
timings parsing to a panel driver (either a new panel-dpi driver or to
panel-simple). What's your preferred option ?

-- 
Regards,

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

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Tomi Valkeinen]

On 12/12/2019 22:35, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Thu, Dec 12, 2019 at 11:37:51AM +0200, Tomi Valkeinen wrote:
>> On 11/12/2019 18:53, Tony Lindgren wrote:
>>> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [191202 13:05]:
>>>> Hi Tomi,
>>>>
>>>> Thank you for the patch.
>>>>
>>>> On Thu, Nov 14, 2019 at 11:39:49AM +0200, Tomi Valkeinen wrote:
>>>>> panel-simple now handled panel osd070t1718-19ts, and we no longer need
>>>>> the panel timings in the DT file. So remove them.
>>>>
>>>> Should you in that case drop the panel-dpi compatible string too, as the
>>>> panel-dpi bindings require panel timings in DT ?
>>>
>>> Yeah sounds like if panel-dpi is no longer usable for this device it
>>> should be dropped from the compatible list.
>>
>> Ok, I agree.
>>
>> Looking at the dts files, panel-dpi is used in a bunch of boards. But
>> we even have 3 dts files with panel-dpi, without the detailed panel
>> model in compatible...
>>
>> Fixing those will break the compatibility with old dtbs and new
>> kernel, unless we add timings-from-dt to a panel driver that handles
>> panel-dpi.
> 
> I know, and I don't have a perfect answer for this :-( I don't see a
> third option, it's either breaking DT backward compatibility or adding
> timings parsing to a panel driver (either a new panel-dpi driver or to
> panel-simple). What's your preferred option ?

Hmm, I just realized that changing these will break omapfb. It relies on panel-dpi and timings from 
DT...

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/panel: simple: fix osd070t1718_19ts sync drive edge

[Tomi Valkeinen]

Hi Thierry,

On 02/12/2019 15:07, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Thu, Nov 14, 2019 at 11:39:50AM +0200, Tomi Valkeinen wrote:
>> The panel datasheet says that the panel samples at falling edge, but
>> does not say anything about h/v sync signals. Testing shows that if the
>> sync signals are driven on falling edge, the picture on the panel will
>> be slightly shifted right.
>>
>> Setting sync drive edge to the same as data drive edge fixes this issue.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> I don't have access to the documentation, but this makes sense, so
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
>> ---
>>   drivers/gpu/drm/panel/panel-simple.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
>> index 5d487686d25c..0784536ae6af 100644
>> --- a/drivers/gpu/drm/panel/panel-simple.c
>> +++ b/drivers/gpu/drm/panel/panel-simple.c
>> @@ -2397,7 +2397,8 @@ static const struct panel_desc osddisplays_osd070t1718_19ts = {
>>   		.height = 91,
>>   	},
>>   	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
>> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
>> +		DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
>>   	.connector_type = DRM_MODE_CONNECTOR_DPI,
>>   };
>>   
> 

Can you pick this one up?

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/panel: simple: fix osd070t1718_19ts sync drive edge

[Tomi Valkeinen]

Hi Thierry,

On 02/12/2019 15:07, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Thu, Nov 14, 2019 at 11:39:50AM +0200, Tomi Valkeinen wrote:
>> The panel datasheet says that the panel samples at falling edge, but
>> does not say anything about h/v sync signals. Testing shows that if the
>> sync signals are driven on falling edge, the picture on the panel will
>> be slightly shifted right.
>>
>> Setting sync drive edge to the same as data drive edge fixes this issue.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> I don't have access to the documentation, but this makes sense, so
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
>> ---
>>   drivers/gpu/drm/panel/panel-simple.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
>> index 5d487686d25c..0784536ae6af 100644
>> --- a/drivers/gpu/drm/panel/panel-simple.c
>> +++ b/drivers/gpu/drm/panel/panel-simple.c
>> @@ -2397,7 +2397,8 @@ static const struct panel_desc osddisplays_osd070t1718_19ts = {
>>   		.height = 91,
>>   	},
>>   	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
>> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
>> +		DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
>>   	.connector_type = DRM_MODE_CONNECTOR_DPI,
>>   };

Can this be merged?

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Tomi Valkeinen]

On 13/01/2020 14:01, Tomi Valkeinen wrote:
> On 12/12/2019 22:35, Laurent Pinchart wrote:
>> Hi Tomi,
>>
>> On Thu, Dec 12, 2019 at 11:37:51AM +0200, Tomi Valkeinen wrote:
>>> On 11/12/2019 18:53, Tony Lindgren wrote:
>>>> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [191202 13:05]:
>>>>> Hi Tomi,
>>>>>
>>>>> Thank you for the patch.
>>>>>
>>>>> On Thu, Nov 14, 2019 at 11:39:49AM +0200, Tomi Valkeinen wrote:
>>>>>> panel-simple now handled panel osd070t1718-19ts, and we no longer need
>>>>>> the panel timings in the DT file. So remove them.
>>>>>
>>>>> Should you in that case drop the panel-dpi compatible string too, as the
>>>>> panel-dpi bindings require panel timings in DT ?
>>>>
>>>> Yeah sounds like if panel-dpi is no longer usable for this device it
>>>> should be dropped from the compatible list.
>>>
>>> Ok, I agree.
>>>
>>> Looking at the dts files, panel-dpi is used in a bunch of boards. But
>>> we even have 3 dts files with panel-dpi, without the detailed panel
>>> model in compatible...
>>>
>>> Fixing those will break the compatibility with old dtbs and new
>>> kernel, unless we add timings-from-dt to a panel driver that handles
>>> panel-dpi.
>>
>> I know, and I don't have a perfect answer for this :-( I don't see a
>> third option, it's either breaking DT backward compatibility or adding
>> timings parsing to a panel driver (either a new panel-dpi driver or to
>> panel-simple). What's your preferred option ?
> 
> Hmm, I just realized that changing these will break omapfb. It relies on panel-dpi and timings from 
> DT...

If no one objects, I think we should just drop the timings from the .dts, and say that these boards 
are no longer supported with omapfb. I don't think there's much point in trying to keep omapfb 
working fine for boards that are fully supported by omapdrm.

Hopefully soon (in five years? =) we can say that omapdrm supports all the boards, and we can 
deprecate omapfb.

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Laurent Pinchart]

Hi Tomi,

On Tue, Feb 11, 2020 at 12:01:31PM +0200, Tomi Valkeinen wrote:
> On 13/01/2020 14:01, Tomi Valkeinen wrote:
> > On 12/12/2019 22:35, Laurent Pinchart wrote:
> >> On Thu, Dec 12, 2019 at 11:37:51AM +0200, Tomi Valkeinen wrote:
> >>> On 11/12/2019 18:53, Tony Lindgren wrote:
> >>>> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [191202 13:05]:
> >>>>> Hi Tomi,
> >>>>>
> >>>>> Thank you for the patch.
> >>>>>
> >>>>> On Thu, Nov 14, 2019 at 11:39:49AM +0200, Tomi Valkeinen wrote:
> >>>>>> panel-simple now handled panel osd070t1718-19ts, and we no longer need
> >>>>>> the panel timings in the DT file. So remove them.
> >>>>>
> >>>>> Should you in that case drop the panel-dpi compatible string too, as the
> >>>>> panel-dpi bindings require panel timings in DT ?
> >>>>
> >>>> Yeah sounds like if panel-dpi is no longer usable for this device it
> >>>> should be dropped from the compatible list.
> >>>
> >>> Ok, I agree.
> >>>
> >>> Looking at the dts files, panel-dpi is used in a bunch of boards. But
> >>> we even have 3 dts files with panel-dpi, without the detailed panel
> >>> model in compatible...
> >>>
> >>> Fixing those will break the compatibility with old dtbs and new
> >>> kernel, unless we add timings-from-dt to a panel driver that handles
> >>> panel-dpi.
> >>
> >> I know, and I don't have a perfect answer for this :-( I don't see a
> >> third option, it's either breaking DT backward compatibility or adding
> >> timings parsing to a panel driver (either a new panel-dpi driver or to
> >> panel-simple). What's your preferred option ?
> > 
> > Hmm, I just realized that changing these will break omapfb. It
> > relies on panel-dpi and timings from DT...
> 
> If no one objects, I think we should just drop the timings from the
> .dts, and say that these boards are no longer supported with omapfb. I
> don't think there's much point in trying to keep omapfb working fine
> for boards that are fully supported by omapdrm.

No objection from me.

> Hopefully soon (in five years? =) we can say that omapdrm supports all
> the boards, and we can deprecate omapfb.

I'd love to send a patch to remove omapfb, but I'll let you do the
honours :-)

-- 
Regards,

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

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Tomi Valkeinen]

On 11/02/2020 13:07, Laurent Pinchart wrote:

>> Hopefully soon (in five years? =) we can say that omapdrm supports all
>> the boards, and we can deprecate omapfb.
> 
> I'd love to send a patch to remove omapfb, but I'll let you do the
> honours :-)

Not before we add DSI support to omapdrm...

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Laurent Pinchart]

On Tue, Feb 11, 2020 at 01:08:12PM +0200, Tomi Valkeinen wrote:
> On 11/02/2020 13:07, Laurent Pinchart wrote:
> 
> >> Hopefully soon (in five years? =) we can say that omapdrm supports all
> >> the boards, and we can deprecate omapfb.
> > 
> > I'd love to send a patch to remove omapfb, but I'll let you do the
> > honours :-)
> 
> Not before we add DSI support to omapdrm...

Details, details ;-)

Seriously speaking, Sebastian's patches are on my todo list.

-- 
Regards,

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

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Merlijn Wajer]

[-- Attachment #1.1.1: Type: text/plain, Size: 802 bytes --]

Hi,

On 11/02/2020 12:08, Tomi Valkeinen wrote:
> On 11/02/2020 13:07, Laurent Pinchart wrote:
> 
>>> Hopefully soon (in five years? =) we can say that omapdrm supports all
>>> the boards, and we can deprecate omapfb.
>>
>> I'd love to send a patch to remove omapfb, but I'll let you do the
>> honours :-)
> 
> Not before we add DSI support to omapdrm...

This is probably known, but for devices that would like use PowerVR SGX,
there sometimes is only userspace available that works with omap(l)fb,
and not with DRM. The Nokia N900 is such an example.

There might be a newer release of (closed) userspace coming for the
aging device(s), but as it stands, I don't think it's possible to do 3D
with PowerVR SGX on omapdrm currently.

But I might be wrong...

Cheers,
Merlijn


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Tony Lindgren]

* Merlijn Wajer <merlijn@wizzup.org> [200211 12:54]:
> Hi,
> 
> On 11/02/2020 12:08, Tomi Valkeinen wrote:
> > On 11/02/2020 13:07, Laurent Pinchart wrote:
> > 
> >>> Hopefully soon (in five years? =) we can say that omapdrm supports all
> >>> the boards, and we can deprecate omapfb.
> >>
> >> I'd love to send a patch to remove omapfb, but I'll let you do the
> >> honours :-)
> > 
> > Not before we add DSI support to omapdrm...

Hmm do your mean RFBI? The DSI support we already have :)

> This is probably known, but for devices that would like use PowerVR SGX,
> there sometimes is only userspace available that works with omap(l)fb,
> and not with DRM. The Nokia N900 is such an example.
> 
> There might be a newer release of (closed) userspace coming for the
> aging device(s), but as it stands, I don't think it's possible to do 3D
> with PowerVR SGX on omapdrm currently.
> 
> But I might be wrong...

Yes SGX is a bottleneck currently for omap3 users. And I think
RFBI is blocking n8x0 from moving to omapdrm and finally leaving
drivers/video/fbdev/omap2 behind.

With those blockers fixed, I'd also be happy to just get rid of
drivers/video/fbdev/omap2 as there should be no reason to
keep using it.

Regards,

Tony


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

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Tomi Valkeinen]

On 11/02/2020 18:05, Tony Lindgren wrote:
> * Merlijn Wajer <merlijn@wizzup.org> [200211 12:54]:
>> Hi,
>>
>> On 11/02/2020 12:08, Tomi Valkeinen wrote:
>>> On 11/02/2020 13:07, Laurent Pinchart wrote:
>>>
>>>>> Hopefully soon (in five years? =) we can say that omapdrm supports all
>>>>> the boards, and we can deprecate omapfb.
>>>>
>>>> I'd love to send a patch to remove omapfb, but I'll let you do the
>>>> honours :-)
>>>
>>> Not before we add DSI support to omapdrm...
> 
> Hmm do your mean RFBI? The DSI support we already have :)

Oh, I didn't even remember RFBI... But it is not supported in omapfb either, so it's not blocking 
removal of omapfb.

We are still missing DSI command mode support, and moving it to the common DRM model.

>> This is probably known, but for devices that would like use PowerVR SGX,
>> there sometimes is only userspace available that works with omap(l)fb,
>> and not with DRM. The Nokia N900 is such an example.
>>
>> There might be a newer release of (closed) userspace coming for the
>> aging device(s), but as it stands, I don't think it's possible to do 3D
>> with PowerVR SGX on omapdrm currently.
>>
>> But I might be wrong...
> 
> Yes SGX is a bottleneck currently for omap3 users. And I think
> RFBI is blocking n8x0 from moving to omapdrm and finally leaving
> drivers/video/fbdev/omap2 behind.
> 
> With those blockers fixed, I'd also be happy to just get rid of
> drivers/video/fbdev/omap2 as there should be no reason to
> keep using it.

I don't know what to do about SGX, but if someone sends patches to omapdrm to allow using SGX, I'll 
review =).

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Tony Lindgren]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [200211 16:14]:
> On 11/02/2020 18:05, Tony Lindgren wrote:
> > * Merlijn Wajer <merlijn@wizzup.org> [200211 12:54]:
> > > Hi,
> > > 
> > > On 11/02/2020 12:08, Tomi Valkeinen wrote:
> > > > On 11/02/2020 13:07, Laurent Pinchart wrote:
> > > > 
> > > > > > Hopefully soon (in five years? =) we can say that omapdrm supports all
> > > > > > the boards, and we can deprecate omapfb.
> > > > > 
> > > > > I'd love to send a patch to remove omapfb, but I'll let you do the
> > > > > honours :-)
> > > > 
> > > > Not before we add DSI support to omapdrm...
> > 
> > Hmm do your mean RFBI? The DSI support we already have :)
> 
> Oh, I didn't even remember RFBI... But it is not supported in omapfb either,
> so it's not blocking removal of omapfb.

Oh OK.

> We are still missing DSI command mode support, and moving it to the common DRM model.

Nope, DSI command mode support has been working just fine for
a while now :) And Sebastian has a WIP git tree of the common DRM
model changes for it. I don't think we have devices with DSI
command mode working for omapfb but not for omapdrm?

What got missed for v5.6-rc1 is the LCD backlight patch though,
I think the only issue there is default-brightness vs more common
default-brightness-value usage if you have any input to that.

> > > This is probably known, but for devices that would like use PowerVR SGX,
> > > there sometimes is only userspace available that works with omap(l)fb,
> > > and not with DRM. The Nokia N900 is such an example.
> > > 
> > > There might be a newer release of (closed) userspace coming for the
> > > aging device(s), but as it stands, I don't think it's possible to do 3D
> > > with PowerVR SGX on omapdrm currently.
> > > 
> > > But I might be wrong...
> > 
> > Yes SGX is a bottleneck currently for omap3 users. And I think
> > RFBI is blocking n8x0 from moving to omapdrm and finally leaving
> > drivers/video/fbdev/omap2 behind.
> > 
> > With those blockers fixed, I'd also be happy to just get rid of
> > drivers/video/fbdev/omap2 as there should be no reason to
> > keep using it.
> 
> I don't know what to do about SGX, but if someone sends patches to omapdrm
> to allow using SGX, I'll review =).

OK so then the only blocker seems to be the lack of SGX blobs
for omap3 devices which seems to be in process of getting fixed.

Regards,

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

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Tomi Valkeinen]

On 11/02/2020 18:27, Tony Lindgren wrote:

>> We are still missing DSI command mode support, and moving it to the common DRM model.
> 
> Nope, DSI command mode support has been working just fine for
> a while now :) And Sebastian has a WIP git tree of the common DRM

Indeed... It had been going on for so long that now my mind is stuck at dsi-command-mode-not-yet-in =).

> model changes for it. I don't think we have devices with DSI
> command mode working for omapfb but not for omapdrm?

Yes, I think that is true.

> What got missed for v5.6-rc1 is the LCD backlight patch though,
> I think the only issue there is default-brightness vs more common
> default-brightness-value usage if you have any input to that.

At least for some boards a power supply is needed, and I think there was no conclusion on who should 
enable that. It didn't seem to fit in anywhere...

But need to check on the latest status. I wasn't following that work closely, as JJ was working on it.

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Sebastian Reichel]

[-- Attachment #1.1: Type: text/plain, Size: 1463 bytes --]

Hi,

On Tue, Feb 11, 2020 at 07:22:14PM +0200, Tomi Valkeinen wrote:
> On 11/02/2020 18:27, Tony Lindgren wrote:
> > > We are still missing DSI command mode support, and moving it
> > > to the common DRM model.
> > 
> > Nope, DSI command mode support has been working just fine for
> > a while now :) And Sebastian has a WIP git tree of the common DRM
> 
> Indeed... It had been going on for so long that now my mind is
> stuck at dsi-command-mode-not-yet-in =).

Welcome in the future :)

> > model changes for it. I don't think we have devices with DSI
> > command mode working for omapfb but not for omapdrm?
> 
> Yes, I think that is true.

Note, that OMAP3 quirk is missing (IDK if its supported in omapfb,
haven't used it for ages). I planned to have a look at OMAP3 once
the patchset moving omapdrm DSI to common DRM is merged, which
needs a non-trivial rebase.

> > What got missed for v5.6-rc1 is the LCD backlight patch though,
> > I think the only issue there is default-brightness vs more common
> > default-brightness-value usage if you have any input to that.
> 
> At least for some boards a power supply is needed, and I think
> there was no conclusion on who should enable that. It didn't seem
> to fit in anywhere...
> 
> But need to check on the latest status. I wasn't following that
> work closely, as JJ was working on it.

FWIW omapdrm's DSI driver is ready for that and omapfb is not :P

-- Sebastian

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings

[Sebastian Reichel]

[-- Attachment #1.1: Type: text/plain, Size: 1040 bytes --]

Hi,

On Tue, Feb 11, 2020 at 01:10:07PM +0200, Laurent Pinchart wrote:
> On Tue, Feb 11, 2020 at 01:08:12PM +0200, Tomi Valkeinen wrote:
> > On 11/02/2020 13:07, Laurent Pinchart wrote:
> > 
> > >> Hopefully soon (in five years? =) we can say that omapdrm supports all
> > >> the boards, and we can deprecate omapfb.
> > > 
> > > I'd love to send a patch to remove omapfb, but I'll let you do the
> > > honours :-)
> > 
> > Not before we add DSI support to omapdrm...
> 
> Details, details ;-)
> 
> Seriously speaking, Sebastian's patches are on my todo list.

The patches need to be rebased. My hack for supporting panel
un/rebind stole connector and drm_dev pointer from drm_panel,
which is no longer possible. I hoped this could be cleaned up
once your and my omapdrm patches landed...

I do have a compile-tested-only WIP branch here, but I doubt
I got this working on the first try:

https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-n900.git/log/?h=omapdrm-dsi-drm-panel-5.6-rc1-wip

-- Sebastian

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 3/3] drm/panel: simple: fix osd070t1718_19ts sync drive edge

[Sam Ravnborg]

Hi Tomi.

On Mon, Feb 10, 2020 at 10:15:33AM +0200, Tomi Valkeinen wrote:
> Hi Thierry,
> 
> On 02/12/2019 15:07, Laurent Pinchart wrote:
> > Hi Tomi,
> > 
> > Thank you for the patch.
> > 
> > On Thu, Nov 14, 2019 at 11:39:50AM +0200, Tomi Valkeinen wrote:
> > > The panel datasheet says that the panel samples at falling edge, but
> > > does not say anything about h/v sync signals. Testing shows that if the
> > > sync signals are driven on falling edge, the picture on the panel will
> > > be slightly shifted right.
> > > 
> > > Setting sync drive edge to the same as data drive edge fixes this issue.
> > > 
> > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > 
> > I don't have access to the documentation, but this makes sense, so
> > 
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > > ---
> > >   drivers/gpu/drm/panel/panel-simple.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > > index 5d487686d25c..0784536ae6af 100644
> > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > @@ -2397,7 +2397,8 @@ static const struct panel_desc osddisplays_osd070t1718_19ts = {
> > >   		.height = 91,
> > >   	},
> > >   	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> > > -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> > > +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
> > > +		DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
> > >   	.connector_type = DRM_MODE_CONNECTOR_DPI,
> > >   };
> 
> Can this be merged?

I have lost the original mail.
Can you re-send or provide a patchwork pointer or similar.
Then I will apply.

PS. Mail had been stuck in my spam quarantine so did not get it until
now.

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

Re: [PATCH 3/3] drm/panel: simple: fix osd070t1718_19ts sync drive edge

[Tomi Valkeinen]

Hi Sam,

On 14/02/2020 23:39, Sam Ravnborg wrote:
> Hi Tomi.
> 
> On Mon, Feb 10, 2020 at 10:15:33AM +0200, Tomi Valkeinen wrote:
>> Hi Thierry,
>>
>> On 02/12/2019 15:07, Laurent Pinchart wrote:
>>> Hi Tomi,
>>>
>>> Thank you for the patch.
>>>
>>> On Thu, Nov 14, 2019 at 11:39:50AM +0200, Tomi Valkeinen wrote:
>>>> The panel datasheet says that the panel samples at falling edge, but
>>>> does not say anything about h/v sync signals. Testing shows that if the
>>>> sync signals are driven on falling edge, the picture on the panel will
>>>> be slightly shifted right.
>>>>
>>>> Setting sync drive edge to the same as data drive edge fixes this issue.
>>>>
>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>>
>>> I don't have access to the documentation, but this makes sense, so
>>>
>>> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>
>>>> ---
>>>>    drivers/gpu/drm/panel/panel-simple.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
>>>> index 5d487686d25c..0784536ae6af 100644
>>>> --- a/drivers/gpu/drm/panel/panel-simple.c
>>>> +++ b/drivers/gpu/drm/panel/panel-simple.c
>>>> @@ -2397,7 +2397,8 @@ static const struct panel_desc osddisplays_osd070t1718_19ts = {
>>>>    		.height = 91,
>>>>    	},
>>>>    	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
>>>> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
>>>> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
>>>> +		DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
>>>>    	.connector_type = DRM_MODE_CONNECTOR_DPI,
>>>>    };
>>
>> Can this be merged?
> 
> I have lost the original mail.
> Can you re-send or provide a patchwork pointer or similar.
> Then I will apply.

Here are lore and patchwork links. Note that the first two patches in the series have already been handled.

https://lore.kernel.org/dri-devel/20191114093950.rT5m86Xh0-a9FZGq7JipQ3O3vldctJYcvRdhxUMGBVE@z/#t

https://patchwork.freedesktop.org/series/69459/

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/panel: simple: fix osd070t1718_19ts sync drive edge

[Sam Ravnborg]

Hi Tomi.

> >>>> +++ b/drivers/gpu/drm/panel/panel-simple.c
> >>>> @@ -2397,7 +2397,8 @@ static const struct panel_desc osddisplays_osd070t1718_19ts = {
> >>>>    		.height = 91,
> >>>>    	},
> >>>>    	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> >>>> -	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
> >>>> +	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
> >>>> +		DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
> >>>>    	.connector_type = DRM_MODE_CONNECTOR_DPI,
> >>>>    };
> >>
> >> Can this be merged?
> > 
> > I have lost the original mail.
> > Can you re-send or provide a patchwork pointer or similar.
> > Then I will apply.
> 
> Here are lore and patchwork links. Note that the first two patches in the series have already been handled.
> 
> https://lore.kernel.org/dri-devel/20191114093950.rT5m86Xh0-a9FZGq7JipQ3O3vldctJYcvRdhxUMGBVE@z/#t
> 
> https://patchwork.freedesktop.org/series/69459/

Thanks for the pointers. It is now applied to drm-misc-next.

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