* [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity @ 2022-10-04 21:35 Dmitry Torokhov 2022-10-04 21:35 ` [PATCH 2/4] ARM: dts: omap4-sdp: " Dmitry Torokhov ` (3 more replies) 0 siblings, 4 replies; 15+ messages in thread From: Dmitry Torokhov @ 2022-10-04 21:35 UTC (permalink / raw) To: Benoît Cousson, Tony Lindgren, Krzysztof Kozlowski, Rob Herring Cc: devicetree, linux-omap, linux-kernel The LCD driver (panel-sony-acx565akm), when probing, starts with line driven low, and then toggles it to high and keeps it there. Also, the line is driven low when powering off the device, and ls released when powering it back on. This means that the reset line should be described as "active low" in DTS. This will be important when the driver is converted to gpiod API which respects the polarity declared in DTS. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- arch/arm/boot/dts/omap3-n900.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts index dd7971556449..c2e5bde19452 100644 --- a/arch/arm/boot/dts/omap3-n900.dts +++ b/arch/arm/boot/dts/omap3-n900.dts @@ -1038,7 +1038,7 @@ lcd: acx565akm@2 { pinctrl-0 = <&acx565akm_pins>; label = "lcd"; - reset-gpios = <&gpio3 26 GPIO_ACTIVE_HIGH>; /* 90 */ + reset-gpios = <&gpio3 26 GPIO_ACTIVE_LOW>; /* 90 */ port { lcd_in: endpoint { -- 2.38.0.rc1.362.ged0d419d3c-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/4] ARM: dts: omap4-sdp: fix LCD reset line polarity 2022-10-04 21:35 [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity Dmitry Torokhov @ 2022-10-04 21:35 ` Dmitry Torokhov 2022-10-04 21:35 ` [PATCH 3/4] ARM: dts: omap3-n950: " Dmitry Torokhov ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ messages in thread From: Dmitry Torokhov @ 2022-10-04 21:35 UTC (permalink / raw) To: Benoît Cousson, Tony Lindgren, Krzysztof Kozlowski, Rob Herring Cc: devicetree, linux-omap, linux-kernel The LCD driver (panel-dsi-cm), when performing reset, starts with line set high, then drives it low, holds it there for a moment, and releases it back to high. This means that the reset line should be described as "active low" in DTS. This will be important when the driver is converted to gpiod API which respects the polarity declared in DTS. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- arch/arm/boot/dts/omap4-sdp.dts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts index 9e976140f34a..77e99b77a0c5 100644 --- a/arch/arm/boot/dts/omap4-sdp.dts +++ b/arch/arm/boot/dts/omap4-sdp.dts @@ -669,7 +669,7 @@ lcd0: panel@0 { reg = <0>; label = "lcd0"; - reset-gpios = <&gpio4 6 GPIO_ACTIVE_HIGH>; /* 102 */ + reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* 102 */ port { lcd0_in: endpoint { @@ -695,7 +695,7 @@ lcd1: panel@0 { reg = <0>; label = "lcd1"; - reset-gpios = <&gpio4 8 GPIO_ACTIVE_HIGH>; /* 104 */ + reset-gpios = <&gpio4 8 GPIO_ACTIVE_LOW>; /* 104 */ port { lcd1_in: endpoint { -- 2.38.0.rc1.362.ged0d419d3c-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/4] ARM: dts: omap3-n950: fix LCD reset line polarity 2022-10-04 21:35 [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity Dmitry Torokhov 2022-10-04 21:35 ` [PATCH 2/4] ARM: dts: omap4-sdp: " Dmitry Torokhov @ 2022-10-04 21:35 ` Dmitry Torokhov 2022-10-04 21:35 ` [PATCH 4/4] ARM: dts: motorola-mapphone: " Dmitry Torokhov 2022-10-11 5:45 ` [PATCH 1/4] ARM: dts: omap3-n900: " Tony Lindgren 3 siblings, 0 replies; 15+ messages in thread From: Dmitry Torokhov @ 2022-10-04 21:35 UTC (permalink / raw) To: Benoît Cousson, Tony Lindgren, Krzysztof Kozlowski, Rob Herring Cc: devicetree, linux-omap, linux-kernel The LCD driver (panel-dsi-cm), when performing reset, starts with line set high, then drives it low, holds it there for a moment, and releases it back to high. This means that the reset line should be described as "active low" in DTS. This will be important when the driver is converted to gpiod API which respects the polarity declared in DTS. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- arch/arm/boot/dts/omap3-n950.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/omap3-n950.dts b/arch/arm/boot/dts/omap3-n950.dts index b2f480022ff6..fa133612bcc2 100644 --- a/arch/arm/boot/dts/omap3-n950.dts +++ b/arch/arm/boot/dts/omap3-n950.dts @@ -235,7 +235,7 @@ lcd0: panel@0 { vpnl-supply = <&vmmc2>; vddi-supply = <&vio>; - reset-gpios = <&gpio3 23 GPIO_ACTIVE_HIGH>; /* 87 */ + reset-gpios = <&gpio3 23 GPIO_ACTIVE_LOW>; /* 87 */ te-gpios = <&gpio2 30 GPIO_ACTIVE_HIGH>; /* 62 */ width-mm = <49>; /* 48.960 mm */ -- 2.38.0.rc1.362.ged0d419d3c-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] ARM: dts: motorola-mapphone: fix LCD reset line polarity 2022-10-04 21:35 [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity Dmitry Torokhov 2022-10-04 21:35 ` [PATCH 2/4] ARM: dts: omap4-sdp: " Dmitry Torokhov 2022-10-04 21:35 ` [PATCH 3/4] ARM: dts: omap3-n950: " Dmitry Torokhov @ 2022-10-04 21:35 ` Dmitry Torokhov 2022-10-11 5:45 ` [PATCH 1/4] ARM: dts: omap3-n900: " Tony Lindgren 3 siblings, 0 replies; 15+ messages in thread From: Dmitry Torokhov @ 2022-10-04 21:35 UTC (permalink / raw) To: Benoît Cousson, Tony Lindgren, Krzysztof Kozlowski, Rob Herring Cc: devicetree, linux-omap, linux-kernel The LCD driver (panel-dsi-cm), when performing reset, starts with line set high, then drives it low, holds it there for a moment, and releases it back to high. This means that the reset line should be described as "active low" in DTS. This will be important when the driver is converted to gpiod API which respects the polarity declared in DTS. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- arch/arm/boot/dts/motorola-mapphone-common.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/motorola-mapphone-common.dtsi b/arch/arm/boot/dts/motorola-mapphone-common.dtsi index c7a1f3ffc48c..beffa1cf08b2 100644 --- a/arch/arm/boot/dts/motorola-mapphone-common.dtsi +++ b/arch/arm/boot/dts/motorola-mapphone-common.dtsi @@ -212,7 +212,7 @@ lcd0: panel@0 { reg = <0>; label = "lcd0"; vddi-supply = <&lcd_regulator>; - reset-gpios = <&gpio4 5 GPIO_ACTIVE_HIGH>; /* gpio101 */ + reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* gpio101 */ backlight = <&backlight>; -- 2.38.0.rc1.362.ged0d419d3c-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity 2022-10-04 21:35 [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity Dmitry Torokhov ` (2 preceding siblings ...) 2022-10-04 21:35 ` [PATCH 4/4] ARM: dts: motorola-mapphone: " Dmitry Torokhov @ 2022-10-11 5:45 ` Tony Lindgren 2022-10-11 12:37 ` Sebastian Reichel 3 siblings, 1 reply; 15+ messages in thread From: Tony Lindgren @ 2022-10-11 5:45 UTC (permalink / raw) To: Dmitry Torokhov Cc: Benoît Cousson, Krzysztof Kozlowski, Rob Herring, devicetree, linux-omap, linux-kernel Hi, * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221004 21:26]: > The LCD driver (panel-sony-acx565akm), when probing, starts with line > driven low, and then toggles it to high and keeps it there. Also, the > line is driven low when powering off the device, and ls released when > powering it back on. This means that the reset line should be described > as "active low" in DTS. This will be important when the driver is > converted to gpiod API which respects the polarity declared in DTS. We should ensure these patches get merged together with the driver change to avoid breaking LCD for booting. Probably no need to have the driver quirk handling for inverted polartity in this case. It's probably easiest to have an immutable branch for the driver changes I can base the dts changes on. Or I can ack the dts changes if they get merged with the driver. Regards, Tony ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity 2022-10-11 5:45 ` [PATCH 1/4] ARM: dts: omap3-n900: " Tony Lindgren @ 2022-10-11 12:37 ` Sebastian Reichel 2022-10-11 14:06 ` Dmitry Torokhov 0 siblings, 1 reply; 15+ messages in thread From: Sebastian Reichel @ 2022-10-11 12:37 UTC (permalink / raw) To: Tony Lindgren Cc: Dmitry Torokhov, Benoît Cousson, Krzysztof Kozlowski, Rob Herring, devicetree, linux-omap, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1152 bytes --] Hi, On Tue, Oct 11, 2022 at 08:45:54AM +0300, Tony Lindgren wrote: > * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221004 21:26]: > > The LCD driver (panel-sony-acx565akm), when probing, starts with line > > driven low, and then toggles it to high and keeps it there. Also, the > > line is driven low when powering off the device, and ls released when > > powering it back on. This means that the reset line should be described > > as "active low" in DTS. This will be important when the driver is > > converted to gpiod API which respects the polarity declared in DTS. > > We should ensure these patches get merged together with the driver > change to avoid breaking LCD for booting. Probably no need to have > the driver quirk handling for inverted polartity in this case. > > It's probably easiest to have an immutable branch for the driver > changes I can base the dts changes on. Or I can ack the dts changes > if they get merged with the driver. Both drivers are already using gpiod API: drivers/gpu/drm/panel/panel-sony-acx565akm.c drivers/gpu/drm/panel/panel-dsi-cm.c So this just breaks things. -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity 2022-10-11 12:37 ` Sebastian Reichel @ 2022-10-11 14:06 ` Dmitry Torokhov 2022-10-11 14:30 ` Tony Lindgren 0 siblings, 1 reply; 15+ messages in thread From: Dmitry Torokhov @ 2022-10-11 14:06 UTC (permalink / raw) To: Sebastian Reichel Cc: Tony Lindgren, Benoît Cousson, Krzysztof Kozlowski, Rob Herring, devicetree, linux-omap, linux-kernel Hi Sebastian, On Tue, Oct 11, 2022 at 02:37:26PM +0200, Sebastian Reichel wrote: > Hi, > > On Tue, Oct 11, 2022 at 08:45:54AM +0300, Tony Lindgren wrote: > > * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221004 21:26]: > > > The LCD driver (panel-sony-acx565akm), when probing, starts with line > > > driven low, and then toggles it to high and keeps it there. Also, the > > > line is driven low when powering off the device, and ls released when > > > powering it back on. This means that the reset line should be described > > > as "active low" in DTS. This will be important when the driver is > > > converted to gpiod API which respects the polarity declared in DTS. > > > > We should ensure these patches get merged together with the driver > > change to avoid breaking LCD for booting. Probably no need to have > > the driver quirk handling for inverted polartity in this case. > > > > It's probably easiest to have an immutable branch for the driver > > changes I can base the dts changes on. Or I can ack the dts changes > > if they get merged with the driver. > > Both drivers are already using gpiod API: > > drivers/gpu/drm/panel/panel-sony-acx565akm.c > drivers/gpu/drm/panel/panel-dsi-cm.c I was looking at drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c which are not using gpiod. Should they be retired? > > So this just breaks things. I missed the drivers in drivers/gpu/... and I see that they essentially abuse gpiod API as gpiod_set_value() operates on logical level (active/inactive) and not absolute (high/low). They should either use the gpiod_*_raw() variants, or they should be adjusted to do the proper thing together with the accompanying DTS change. What are your preferences? Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity 2022-10-11 14:06 ` Dmitry Torokhov @ 2022-10-11 14:30 ` Tony Lindgren 2022-10-11 14:38 ` Dmitry Torokhov 0 siblings, 1 reply; 15+ messages in thread From: Tony Lindgren @ 2022-10-11 14:30 UTC (permalink / raw) To: Dmitry Torokhov Cc: Sebastian Reichel, Benoît Cousson, Krzysztof Kozlowski, Rob Herring, devicetree, linux-omap, linux-kernel * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221011 13:57]: > Hi Sebastian, > > On Tue, Oct 11, 2022 at 02:37:26PM +0200, Sebastian Reichel wrote: > > Hi, > > > > On Tue, Oct 11, 2022 at 08:45:54AM +0300, Tony Lindgren wrote: > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221004 21:26]: > > > > The LCD driver (panel-sony-acx565akm), when probing, starts with line > > > > driven low, and then toggles it to high and keeps it there. Also, the > > > > line is driven low when powering off the device, and ls released when > > > > powering it back on. This means that the reset line should be described > > > > as "active low" in DTS. This will be important when the driver is > > > > converted to gpiod API which respects the polarity declared in DTS. > > > > > > We should ensure these patches get merged together with the driver > > > change to avoid breaking LCD for booting. Probably no need to have > > > the driver quirk handling for inverted polartity in this case. > > > > > > It's probably easiest to have an immutable branch for the driver > > > changes I can base the dts changes on. Or I can ack the dts changes > > > if they get merged with the driver. > > > > Both drivers are already using gpiod API: > > > > drivers/gpu/drm/panel/panel-sony-acx565akm.c > > drivers/gpu/drm/panel/panel-dsi-cm.c > > I was looking at > > drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c > drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c Ah OK that explains :) > which are not using gpiod. Should they be retired? Yes we should just get rid of them with omapdrm working just fine. > > So this just breaks things. > > I missed the drivers in drivers/gpu/... and I see that they essentially > abuse gpiod API as gpiod_set_value() operates on logical level > (active/inactive) and not absolute (high/low). They should either use > the gpiod_*_raw() variants, or they should be adjusted to do the proper > thing together with the accompanying DTS change. > > What are your preferences? Seems like high/low at the connected device end is what we should use, right? Otherwise things will misbehave if the panel is connected to some other SoC possibly. Regards, Tony ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity 2022-10-11 14:30 ` Tony Lindgren @ 2022-10-11 14:38 ` Dmitry Torokhov 2022-10-11 14:47 ` Tony Lindgren 0 siblings, 1 reply; 15+ messages in thread From: Dmitry Torokhov @ 2022-10-11 14:38 UTC (permalink / raw) To: Tony Lindgren Cc: Sebastian Reichel, Benoît Cousson, Krzysztof Kozlowski, Rob Herring, devicetree, linux-omap, linux-kernel On Tue, Oct 11, 2022 at 05:30:12PM +0300, Tony Lindgren wrote: > * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221011 13:57]: > > Hi Sebastian, > > > > On Tue, Oct 11, 2022 at 02:37:26PM +0200, Sebastian Reichel wrote: > > > Hi, > > > > > > On Tue, Oct 11, 2022 at 08:45:54AM +0300, Tony Lindgren wrote: > > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221004 21:26]: > > > > > The LCD driver (panel-sony-acx565akm), when probing, starts with line > > > > > driven low, and then toggles it to high and keeps it there. Also, the > > > > > line is driven low when powering off the device, and ls released when > > > > > powering it back on. This means that the reset line should be described > > > > > as "active low" in DTS. This will be important when the driver is > > > > > converted to gpiod API which respects the polarity declared in DTS. > > > > > > > > We should ensure these patches get merged together with the driver > > > > change to avoid breaking LCD for booting. Probably no need to have > > > > the driver quirk handling for inverted polartity in this case. > > > > > > > > It's probably easiest to have an immutable branch for the driver > > > > changes I can base the dts changes on. Or I can ack the dts changes > > > > if they get merged with the driver. > > > > > > Both drivers are already using gpiod API: > > > > > > drivers/gpu/drm/panel/panel-sony-acx565akm.c > > > drivers/gpu/drm/panel/panel-dsi-cm.c > > > > I was looking at > > > > drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c > > drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c > > Ah OK that explains :) > > > which are not using gpiod. Should they be retired? > > Yes we should just get rid of them with omapdrm working just fine. Will you be submitting such patches? I'd like to get rid of of_get_named_gpio() and friends if I can... > > > > So this just breaks things. > > > > I missed the drivers in drivers/gpu/... and I see that they essentially > > abuse gpiod API as gpiod_set_value() operates on logical level > > (active/inactive) and not absolute (high/low). They should either use > > the gpiod_*_raw() variants, or they should be adjusted to do the proper > > thing together with the accompanying DTS change. > > > > What are your preferences? > > Seems like high/low at the connected device end is what we should use, > right? Otherwise things will misbehave if the panel is connected to > some other SoC possibly. It is exactly because of this case the driver should use active/inactive and follow polarity described in DTS. If the driver does: gpiod_set_value_cansleep(d->reset, 1); then if DTS is saying that the reset line is active low, under the wraps the line will be driven to "0", but if DTS is saying that the line is active high, then the very same call will drive the line to "1". This allows accommodating different designs without having to change the driver code. -- Dmitry ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity 2022-10-11 14:38 ` Dmitry Torokhov @ 2022-10-11 14:47 ` Tony Lindgren 2022-10-12 10:58 ` Tomi Valkeinen 0 siblings, 1 reply; 15+ messages in thread From: Tony Lindgren @ 2022-10-11 14:47 UTC (permalink / raw) To: Dmitry Torokhov Cc: Sebastian Reichel, Benoît Cousson, Krzysztof Kozlowski, Rob Herring, devicetree, linux-omap, linux-kernel, Tomi Valkeinen * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221011 14:30]: > On Tue, Oct 11, 2022 at 05:30:12PM +0300, Tony Lindgren wrote: > > * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221011 13:57]: > > > Hi Sebastian, > > > > > > On Tue, Oct 11, 2022 at 02:37:26PM +0200, Sebastian Reichel wrote: > > > > Hi, > > > > > > > > On Tue, Oct 11, 2022 at 08:45:54AM +0300, Tony Lindgren wrote: > > > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221004 21:26]: > > > > > > The LCD driver (panel-sony-acx565akm), when probing, starts with line > > > > > > driven low, and then toggles it to high and keeps it there. Also, the > > > > > > line is driven low when powering off the device, and ls released when > > > > > > powering it back on. This means that the reset line should be described > > > > > > as "active low" in DTS. This will be important when the driver is > > > > > > converted to gpiod API which respects the polarity declared in DTS. > > > > > > > > > > We should ensure these patches get merged together with the driver > > > > > change to avoid breaking LCD for booting. Probably no need to have > > > > > the driver quirk handling for inverted polartity in this case. > > > > > > > > > > It's probably easiest to have an immutable branch for the driver > > > > > changes I can base the dts changes on. Or I can ack the dts changes > > > > > if they get merged with the driver. > > > > > > > > Both drivers are already using gpiod API: > > > > > > > > drivers/gpu/drm/panel/panel-sony-acx565akm.c > > > > drivers/gpu/drm/panel/panel-dsi-cm.c > > > > > > I was looking at > > > > > > drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c > > > drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c > > > > Ah OK that explains :) > > > > > which are not using gpiod. Should they be retired? > > > > Yes we should just get rid of them with omapdrm working just fine. > > Will you be submitting such patches? I'd like to get rid of > of_get_named_gpio() and friends if I can... Adding Tomi to Cc, my guess is he already has such patches and knows better which ones can go :) > > > > So this just breaks things. > > > > > > I missed the drivers in drivers/gpu/... and I see that they essentially > > > abuse gpiod API as gpiod_set_value() operates on logical level > > > (active/inactive) and not absolute (high/low). They should either use > > > the gpiod_*_raw() variants, or they should be adjusted to do the proper > > > thing together with the accompanying DTS change. > > > > > > What are your preferences? > > > > Seems like high/low at the connected device end is what we should use, > > right? Otherwise things will misbehave if the panel is connected to > > some other SoC possibly. > > It is exactly because of this case the driver should use active/inactive > and follow polarity described in DTS. If the driver does: > > gpiod_set_value_cansleep(d->reset, 1); > > then if DTS is saying that the reset line is active low, under the wraps > the line will be driven to "0", but if DTS is saying that the line is > active high, then the very same call will drive the line to "1". > > This allows accommodating different designs without having to change the > driver code. OK Thanks, Tony ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity 2022-10-11 14:47 ` Tony Lindgren @ 2022-10-12 10:58 ` Tomi Valkeinen 2022-10-12 19:30 ` Dmitry Torokhov 0 siblings, 1 reply; 15+ messages in thread From: Tomi Valkeinen @ 2022-10-12 10:58 UTC (permalink / raw) To: Tony Lindgren, Dmitry Torokhov Cc: Sebastian Reichel, Benoît Cousson, Krzysztof Kozlowski, Rob Herring, devicetree, linux-omap, linux-kernel Hi, On 11/10/2022 17:47, Tony Lindgren wrote: > * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221011 14:30]: >> On Tue, Oct 11, 2022 at 05:30:12PM +0300, Tony Lindgren wrote: >>> * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221011 13:57]: >>>> Hi Sebastian, >>>> >>>> On Tue, Oct 11, 2022 at 02:37:26PM +0200, Sebastian Reichel wrote: >>>>> Hi, >>>>> >>>>> On Tue, Oct 11, 2022 at 08:45:54AM +0300, Tony Lindgren wrote: >>>>>> * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221004 21:26]: >>>>>>> The LCD driver (panel-sony-acx565akm), when probing, starts with line >>>>>>> driven low, and then toggles it to high and keeps it there. Also, the >>>>>>> line is driven low when powering off the device, and ls released when >>>>>>> powering it back on. This means that the reset line should be described >>>>>>> as "active low" in DTS. This will be important when the driver is >>>>>>> converted to gpiod API which respects the polarity declared in DTS. >>>>>> >>>>>> We should ensure these patches get merged together with the driver >>>>>> change to avoid breaking LCD for booting. Probably no need to have >>>>>> the driver quirk handling for inverted polartity in this case. >>>>>> >>>>>> It's probably easiest to have an immutable branch for the driver >>>>>> changes I can base the dts changes on. Or I can ack the dts changes >>>>>> if they get merged with the driver. >>>>> >>>>> Both drivers are already using gpiod API: >>>>> >>>>> drivers/gpu/drm/panel/panel-sony-acx565akm.c >>>>> drivers/gpu/drm/panel/panel-dsi-cm.c >>>> >>>> I was looking at >>>> >>>> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c >>>> drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c >>> >>> Ah OK that explains :) >>> >>>> which are not using gpiod. Should they be retired? >>> >>> Yes we should just get rid of them with omapdrm working just fine. >> >> Will you be submitting such patches? I'd like to get rid of >> of_get_named_gpio() and friends if I can... > > Adding Tomi to Cc, my guess is he already has such patches and knows > better which ones can go :) To be honest, I haven't really even had a glance towards fbdev for a long time. There is one thing that omapdrm doesn't support, which is VRFB rotation. I cannot say if the users of those above-mentioned panels require VRFB. >>>>> So this just breaks things. >>>> >>>> I missed the drivers in drivers/gpu/... and I see that they essentially >>>> abuse gpiod API as gpiod_set_value() operates on logical level >>>> (active/inactive) and not absolute (high/low). They should either use >>>> the gpiod_*_raw() variants, or they should be adjusted to do the proper >>>> thing together with the accompanying DTS change. >>>> >>>> What are your preferences? >>> >>> Seems like high/low at the connected device end is what we should use, >>> right? Otherwise things will misbehave if the panel is connected to >>> some other SoC possibly. >> >> It is exactly because of this case the driver should use active/inactive >> and follow polarity described in DTS. If the driver does: >> >> gpiod_set_value_cansleep(d->reset, 1); >> >> then if DTS is saying that the reset line is active low, under the wraps >> the line will be driven to "0", but if DTS is saying that the line is >> active high, then the very same call will drive the line to "1". >> >> This allows accommodating different designs without having to change the >> driver code. Isn't breaking an old dts file quite a bad thing? Why not just add a comment to the .dts and to the driver about the situation. I don't quite see that the fixing the dts (And, if done properly, adding a boot time fixup for old dtbs) and changing the drivers is worth the hassle. Unless we see new users for these drivers, which would require the new users to write broken dts files. Tomi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity 2022-10-12 10:58 ` Tomi Valkeinen @ 2022-10-12 19:30 ` Dmitry Torokhov 2022-10-13 6:22 ` Tomi Valkeinen 0 siblings, 1 reply; 15+ messages in thread From: Dmitry Torokhov @ 2022-10-12 19:30 UTC (permalink / raw) To: Tomi Valkeinen Cc: Tony Lindgren, Sebastian Reichel, Benoît Cousson, Krzysztof Kozlowski, Rob Herring, devicetree, linux-omap, linux-kernel On Wed, Oct 12, 2022 at 01:58:15PM +0300, Tomi Valkeinen wrote: > Hi, > > On 11/10/2022 17:47, Tony Lindgren wrote: > > * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221011 14:30]: > > > On Tue, Oct 11, 2022 at 05:30:12PM +0300, Tony Lindgren wrote: > > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221011 13:57]: > > > > > Hi Sebastian, > > > > > > > > > > On Tue, Oct 11, 2022 at 02:37:26PM +0200, Sebastian Reichel wrote: > > > > > > Hi, > > > > > > > > > > > > On Tue, Oct 11, 2022 at 08:45:54AM +0300, Tony Lindgren wrote: > > > > > > > * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221004 21:26]: > > > > > > > > The LCD driver (panel-sony-acx565akm), when probing, starts with line > > > > > > > > driven low, and then toggles it to high and keeps it there. Also, the > > > > > > > > line is driven low when powering off the device, and ls released when > > > > > > > > powering it back on. This means that the reset line should be described > > > > > > > > as "active low" in DTS. This will be important when the driver is > > > > > > > > converted to gpiod API which respects the polarity declared in DTS. > > > > > > > > > > > > > > We should ensure these patches get merged together with the driver > > > > > > > change to avoid breaking LCD for booting. Probably no need to have > > > > > > > the driver quirk handling for inverted polartity in this case. > > > > > > > > > > > > > > It's probably easiest to have an immutable branch for the driver > > > > > > > changes I can base the dts changes on. Or I can ack the dts changes > > > > > > > if they get merged with the driver. > > > > > > > > > > > > Both drivers are already using gpiod API: > > > > > > > > > > > > drivers/gpu/drm/panel/panel-sony-acx565akm.c > > > > > > drivers/gpu/drm/panel/panel-dsi-cm.c > > > > > > > > > > I was looking at > > > > > > > > > > drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c > > > > > drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c > > > > > > > > Ah OK that explains :) > > > > > > > > > which are not using gpiod. Should they be retired? > > > > > > > > Yes we should just get rid of them with omapdrm working just fine. > > > > > > Will you be submitting such patches? I'd like to get rid of > > > of_get_named_gpio() and friends if I can... > > > > Adding Tomi to Cc, my guess is he already has such patches and knows > > better which ones can go :) > > To be honest, I haven't really even had a glance towards fbdev for a long > time. > > There is one thing that omapdrm doesn't support, which is VRFB rotation. I > cannot say if the users of those above-mentioned panels require VRFB. > > > > > > > So this just breaks things. > > > > > > > > > > I missed the drivers in drivers/gpu/... and I see that they essentially > > > > > abuse gpiod API as gpiod_set_value() operates on logical level > > > > > (active/inactive) and not absolute (high/low). They should either use > > > > > the gpiod_*_raw() variants, or they should be adjusted to do the proper > > > > > thing together with the accompanying DTS change. > > > > > > > > > > What are your preferences? > > > > > > > > Seems like high/low at the connected device end is what we should use, > > > > right? Otherwise things will misbehave if the panel is connected to > > > > some other SoC possibly. > > > > > > It is exactly because of this case the driver should use active/inactive > > > and follow polarity described in DTS. If the driver does: > > > > > > gpiod_set_value_cansleep(d->reset, 1); > > > > > > then if DTS is saying that the reset line is active low, under the wraps > > > the line will be driven to "0", but if DTS is saying that the line is > > > active high, then the very same call will drive the line to "1". > > > > > > This allows accommodating different designs without having to change the > > > driver code. > > Isn't breaking an old dts file quite a bad thing? Why not just add a comment > to the .dts and to the driver about the situation. I don't quite see that > the fixing the dts (And, if done properly, adding a boot time fixup for old > dtbs) and changing the drivers is worth the hassle. > > Unless we see new users for these drivers, which would require the new users > to write broken dts files. Or maybe there are devices with fixed DTSes and fixed up kernels but the fixes have not been contributed upstream. I don't know... My personal opinion is that we pay too much attention to DTS compatibility in cases when it is not totally clear if there are devices that use DTSes that are not bundled with the kernel and also have a chance to have their kernel updated (and be lucky enough for the upstream kernel to work on such device without extensive work). Anyway, my goal is to stop exposing of_get_named_gpio() and its derivatives, so please let me know your preference. Should I: - mirror in omapfb drivers what gpu drivers do and use inverted polarity - use gpiod_set_value_raw() and essentially ignore polarity described in DTS - continue pushing polarity fixes throughout - something else? Thanks! -- Dmitry ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity 2022-10-12 19:30 ` Dmitry Torokhov @ 2022-10-13 6:22 ` Tomi Valkeinen 2022-10-13 11:08 ` Tony Lindgren 0 siblings, 1 reply; 15+ messages in thread From: Tomi Valkeinen @ 2022-10-13 6:22 UTC (permalink / raw) To: Dmitry Torokhov Cc: Tony Lindgren, Sebastian Reichel, Benoît Cousson, Krzysztof Kozlowski, Rob Herring, devicetree, linux-omap, linux-kernel On 12/10/2022 22:30, Dmitry Torokhov wrote: > On Wed, Oct 12, 2022 at 01:58:15PM +0300, Tomi Valkeinen wrote: >> Hi, >> >> On 11/10/2022 17:47, Tony Lindgren wrote: >>> * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221011 14:30]: >>>> On Tue, Oct 11, 2022 at 05:30:12PM +0300, Tony Lindgren wrote: >>>>> * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221011 13:57]: >>>>>> Hi Sebastian, >>>>>> >>>>>> On Tue, Oct 11, 2022 at 02:37:26PM +0200, Sebastian Reichel wrote: >>>>>>> Hi, >>>>>>> >>>>>>> On Tue, Oct 11, 2022 at 08:45:54AM +0300, Tony Lindgren wrote: >>>>>>>> * Dmitry Torokhov <dmitry.torokhov@gmail.com> [221004 21:26]: >>>>>>>>> The LCD driver (panel-sony-acx565akm), when probing, starts with line >>>>>>>>> driven low, and then toggles it to high and keeps it there. Also, the >>>>>>>>> line is driven low when powering off the device, and ls released when >>>>>>>>> powering it back on. This means that the reset line should be described >>>>>>>>> as "active low" in DTS. This will be important when the driver is >>>>>>>>> converted to gpiod API which respects the polarity declared in DTS. >>>>>>>> >>>>>>>> We should ensure these patches get merged together with the driver >>>>>>>> change to avoid breaking LCD for booting. Probably no need to have >>>>>>>> the driver quirk handling for inverted polartity in this case. >>>>>>>> >>>>>>>> It's probably easiest to have an immutable branch for the driver >>>>>>>> changes I can base the dts changes on. Or I can ack the dts changes >>>>>>>> if they get merged with the driver. >>>>>>> >>>>>>> Both drivers are already using gpiod API: >>>>>>> >>>>>>> drivers/gpu/drm/panel/panel-sony-acx565akm.c >>>>>>> drivers/gpu/drm/panel/panel-dsi-cm.c >>>>>> >>>>>> I was looking at >>>>>> >>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c >>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c >>>>> >>>>> Ah OK that explains :) >>>>> >>>>>> which are not using gpiod. Should they be retired? >>>>> >>>>> Yes we should just get rid of them with omapdrm working just fine. >>>> >>>> Will you be submitting such patches? I'd like to get rid of >>>> of_get_named_gpio() and friends if I can... >>> >>> Adding Tomi to Cc, my guess is he already has such patches and knows >>> better which ones can go :) >> >> To be honest, I haven't really even had a glance towards fbdev for a long >> time. >> >> There is one thing that omapdrm doesn't support, which is VRFB rotation. I >> cannot say if the users of those above-mentioned panels require VRFB. >> >>>>>>> So this just breaks things. >>>>>> >>>>>> I missed the drivers in drivers/gpu/... and I see that they essentially >>>>>> abuse gpiod API as gpiod_set_value() operates on logical level >>>>>> (active/inactive) and not absolute (high/low). They should either use >>>>>> the gpiod_*_raw() variants, or they should be adjusted to do the proper >>>>>> thing together with the accompanying DTS change. >>>>>> >>>>>> What are your preferences? >>>>> >>>>> Seems like high/low at the connected device end is what we should use, >>>>> right? Otherwise things will misbehave if the panel is connected to >>>>> some other SoC possibly. >>>> >>>> It is exactly because of this case the driver should use active/inactive >>>> and follow polarity described in DTS. If the driver does: >>>> >>>> gpiod_set_value_cansleep(d->reset, 1); >>>> >>>> then if DTS is saying that the reset line is active low, under the wraps >>>> the line will be driven to "0", but if DTS is saying that the line is >>>> active high, then the very same call will drive the line to "1". >>>> >>>> This allows accommodating different designs without having to change the >>>> driver code. >> >> Isn't breaking an old dts file quite a bad thing? Why not just add a comment >> to the .dts and to the driver about the situation. I don't quite see that >> the fixing the dts (And, if done properly, adding a boot time fixup for old >> dtbs) and changing the drivers is worth the hassle. >> >> Unless we see new users for these drivers, which would require the new users >> to write broken dts files. > > Or maybe there are devices with fixed DTSes and fixed up kernels but the > fixes have not been contributed upstream. I don't know... > > My personal opinion is that we pay too much attention to DTS > compatibility in cases when it is not totally clear if there are devices > that use DTSes that are not bundled with the kernel and also have a > chance to have their kernel updated (and be lucky enough for the > upstream kernel to work on such device without extensive work). > > Anyway, my goal is to stop exposing of_get_named_gpio() and its > derivatives, so please let me know your preference. Should I: > > - mirror in omapfb drivers what gpu drivers do and use inverted > polarity I would just go with the above for the time being. It should be an easy change, and as these omapfb and drm panel drivers are kind of copies of each other, I think it makes sense to use the same code in both. That said, I personally don't mind fixing the dts files and the drivers, and even dropping the omapfb panel drivers. However, as I don't know if someone needs the omapfb drivers or has to use an old dtb, I don't want to step on that possible mine field. If someone else wants to go there (without my involvement), fine for me =). Tomi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity 2022-10-13 6:22 ` Tomi Valkeinen @ 2022-10-13 11:08 ` Tony Lindgren 2022-10-13 13:17 ` Sebastian Reichel 0 siblings, 1 reply; 15+ messages in thread From: Tony Lindgren @ 2022-10-13 11:08 UTC (permalink / raw) To: Tomi Valkeinen Cc: Dmitry Torokhov, Sebastian Reichel, Benoît Cousson, Krzysztof Kozlowski, Rob Herring, devicetree, linux-omap, linux-kernel, Ivaylo Dimitrov Hi, * Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> [221013 06:13]: > I would just go with the above for the time being. It should be an easy > change, and as these omapfb and drm panel drivers are kind of copies of each > other, I think it makes sense to use the same code in both. Maybe if a fix is needed, sure let's fix things first, then drop the unused panel drivers. We already have drivers/gpu/drm/panel driver for both of these two omapfb panels: drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c The compatible strings used translate to these dts files: arch/arm/boot/dts/motorola-mapphone-common.dtsi arch/arm/boot/dts/omap3-n900.dts arch/arm/boot/dts/omap3-n950.dts arch/arm/boot/dts/omap4-sdp.dts These devices work with omapdrm and there should not be any need to stick with the omapfb driver. We can just drop the omapfb panel drivers for panel-sony-acx565akm.c and panel-dsi-cm.c. Let's put the limited effort where there is activity instead :) The vrfb rotation work has been discussed on the lists, so seems like we will eventually have that for omapdrm. Meanwhile, software rotation is being used for postmarketos and leste with omapdrm AFAIK. > That said, I personally don't mind fixing the dts files and the drivers, and > even dropping the omapfb panel drivers. However, as I don't know if someone > needs the omapfb drivers or has to use an old dtb, I don't want to step on > that possible mine field. If someone else wants to go there (without my > involvement), fine for me =). I belive the only valid use case for omap2 omapfb is the n8x0 rfbi driver that has no omapdrm driver. Regards, Tony ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity 2022-10-13 11:08 ` Tony Lindgren @ 2022-10-13 13:17 ` Sebastian Reichel 0 siblings, 0 replies; 15+ messages in thread From: Sebastian Reichel @ 2022-10-13 13:17 UTC (permalink / raw) To: Tony Lindgren Cc: Tomi Valkeinen, Dmitry Torokhov, Benoît Cousson, Krzysztof Kozlowski, Rob Herring, devicetree, linux-omap, linux-kernel, Ivaylo Dimitrov [-- Attachment #1: Type: text/plain, Size: 2507 bytes --] Hi, On Thu, Oct 13, 2022 at 02:08:54PM +0300, Tony Lindgren wrote: > Hi, > > * Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> [221013 06:13]: > > I would just go with the above for the time being. It should be an easy > > change, and as these omapfb and drm panel drivers are kind of copies of each > > other, I think it makes sense to use the same code in both. > > Maybe if a fix is needed, sure let's fix things first, then drop > the unused panel drivers. > > We already have drivers/gpu/drm/panel driver for both of these two > omapfb panels: > > drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c > drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c > > The compatible strings used translate to these dts files: > > arch/arm/boot/dts/motorola-mapphone-common.dtsi > arch/arm/boot/dts/omap3-n900.dts > arch/arm/boot/dts/omap3-n950.dts > arch/arm/boot/dts/omap4-sdp.dts > > These devices work with omapdrm and there should not be any need to > stick with the omapfb driver. We can just drop the omapfb panel > drivers for panel-sony-acx565akm.c and panel-dsi-cm.c. Let's put > the limited effort where there is activity instead :) FWIW Acked-by: Sebastian Reichel <sre@kernel.org> for removal of those two omapfb panel drivers. > The vrfb rotation work has been discussed on the lists, so seems > like we will eventually have that for omapdrm. Meanwhile, software > rotation is being used for postmarketos and leste with omapdrm > AFAIK. > > > That said, I personally don't mind fixing the dts files and the drivers, and > > even dropping the omapfb panel drivers. However, as I don't know if someone > > needs the omapfb drivers or has to use an old dtb, I don't want to step on > > that possible mine field. If someone else wants to go there (without my > > involvement), fine for me =). > > I belive the only valid use case for omap2 omapfb is the n8x0 rfbi > driver that has no omapdrm driver. Is that upstream? omapfb (and omapdrm) both have this: /* * HACK * We don't have a working driver for rfbi, so skip it here always. * Otherwise dss will never get probed successfully, as it will wait * for rfbi to get probed. */ if (strstr(dev_name(dev), "rfbi")) return 0; I've seen a few old drivers being removed by marking them as BROKEN (and updating Kconfig help text to explain the situation). Then the code is dropped 1-2 cycles later assuming nobody complained. -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-10-13 13:18 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-04 21:35 [PATCH 1/4] ARM: dts: omap3-n900: fix LCD reset line polarity Dmitry Torokhov 2022-10-04 21:35 ` [PATCH 2/4] ARM: dts: omap4-sdp: " Dmitry Torokhov 2022-10-04 21:35 ` [PATCH 3/4] ARM: dts: omap3-n950: " Dmitry Torokhov 2022-10-04 21:35 ` [PATCH 4/4] ARM: dts: motorola-mapphone: " Dmitry Torokhov 2022-10-11 5:45 ` [PATCH 1/4] ARM: dts: omap3-n900: " Tony Lindgren 2022-10-11 12:37 ` Sebastian Reichel 2022-10-11 14:06 ` Dmitry Torokhov 2022-10-11 14:30 ` Tony Lindgren 2022-10-11 14:38 ` Dmitry Torokhov 2022-10-11 14:47 ` Tony Lindgren 2022-10-12 10:58 ` Tomi Valkeinen 2022-10-12 19:30 ` Dmitry Torokhov 2022-10-13 6:22 ` Tomi Valkeinen 2022-10-13 11:08 ` Tony Lindgren 2022-10-13 13:17 ` Sebastian Reichel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).