[PATCH 0/3] Galaxy S2 (i9100) panel updates

[PATCH 0/3] Galaxy S2 (i9100) panel updates

[Paul Cercueil]

Hi,

Here's a small patchset that I created while playing with PostmarketOS
on my Samsung Galaxy S2.

The first two patches update the LD9040 panel driver so that it looks
much better, and supports setting the backlight.

The third patch fixes the size of the panel in the Device Tree. The
previous values were completely bogus and caused Phosh (PmOS' UI) to
display tiny icons and text as it thought the DPI was much lower.

Cheers,
-Paul

Paul Cercueil (3):
  drm/panel: ld9040: Use better magic values
  drm/panel: ld9040: Register a backlight device
  ARM: dts: exynos/i9100: Fix LCD screen's physical size

 arch/arm/boot/dts/exynos4210-i9100.dts       |  4 +-
 drivers/gpu/drm/panel/panel-samsung-ld9040.c | 51 ++++++++++++++++++--
 2 files changed, 48 insertions(+), 7 deletions(-)

-- 
2.40.1

[PATCH 0/3] Galaxy S2 (i9100) panel updates

[Paul Cercueil]

Hi,

Here's a small patchset that I created while playing with PostmarketOS
on my Samsung Galaxy S2.

The first two patches update the LD9040 panel driver so that it looks
much better, and supports setting the backlight.

The third patch fixes the size of the panel in the Device Tree. The
previous values were completely bogus and caused Phosh (PmOS' UI) to
display tiny icons and text as it thought the DPI was much lower.

Cheers,
-Paul

Paul Cercueil (3):
  drm/panel: ld9040: Use better magic values
  drm/panel: ld9040: Register a backlight device
  ARM: dts: exynos/i9100: Fix LCD screen's physical size

 arch/arm/boot/dts/exynos4210-i9100.dts       |  4 +-
 drivers/gpu/drm/panel/panel-samsung-ld9040.c | 51 ++++++++++++++++++--
 2 files changed, 48 insertions(+), 7 deletions(-)

-- 
2.40.1


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

[PATCH 0/3] Galaxy S2 (i9100) panel updates

[Paul Cercueil]

Hi,

Here's a small patchset that I created while playing with PostmarketOS
on my Samsung Galaxy S2.

The first two patches update the LD9040 panel driver so that it looks
much better, and supports setting the backlight.

The third patch fixes the size of the panel in the Device Tree. The
previous values were completely bogus and caused Phosh (PmOS' UI) to
display tiny icons and text as it thought the DPI was much lower.

Cheers,
-Paul

Paul Cercueil (3):
  drm/panel: ld9040: Use better magic values
  drm/panel: ld9040: Register a backlight device
  ARM: dts: exynos/i9100: Fix LCD screen's physical size

 arch/arm/boot/dts/exynos4210-i9100.dts       |  4 +-
 drivers/gpu/drm/panel/panel-samsung-ld9040.c | 51 ++++++++++++++++++--
 2 files changed, 48 insertions(+), 7 deletions(-)

-- 
2.40.1

[PATCH 1/3] drm/panel: ld9040: Use better magic values

[Paul Cercueil]

I have no idea what the prior magic values mean, and I have no idea
what my replacement (extracted from [1]) magic values mean.

What I do know, is that these new values result in a much better
picture, where the blacks are really black (as you would expect on an
AMOLED display) instead of grey-ish.

[1] https://github.com/dorimanx/Dorimanx-SG2-I9100-Kernel/blob/master-jelly-bean/arch/arm/mach-exynos/u1-panel.h

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/panel/panel-samsung-ld9040.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
index 01eb211f32f7..7fd9444b42c5 100644
--- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
+++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
@@ -180,17 +180,18 @@ static void ld9040_init(struct ld9040 *ctx)
 {
 	ld9040_dcs_write_seq_static(ctx, MCS_USER_SETTING, 0x5a, 0x5a);
 	ld9040_dcs_write_seq_static(ctx, MCS_PANEL_CONDITION,
-		0x05, 0x65, 0x96, 0x71, 0x7d, 0x19, 0x3b, 0x0d,
-		0x19, 0x7e, 0x0d, 0xe2, 0x00, 0x00, 0x7e, 0x7d,
-		0x07, 0x07, 0x20, 0x20, 0x20, 0x02, 0x02);
+		0x05, 0x5e, 0x96, 0x6b, 0x7d, 0x0d, 0x3f, 0x00,
+		0x00, 0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x07, 0x05, 0x1f, 0x1f, 0x1f, 0x00, 0x00);
 	ld9040_dcs_write_seq_static(ctx, MCS_DISPCTL,
-		0x02, 0x08, 0x08, 0x10, 0x10);
+		0x02, 0x06, 0x0a, 0x10, 0x10);
 	ld9040_dcs_write_seq_static(ctx, MCS_MANPWR, 0x04);
 	ld9040_dcs_write_seq_static(ctx, MCS_POWER_CTRL,
 		0x0a, 0x87, 0x25, 0x6a, 0x44, 0x02, 0x88);
-	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0d, 0x00, 0x16);
+	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0f, 0x00, 0x16);
 	ld9040_dcs_write_seq_static(ctx, MCS_GTCON, 0x09, 0x00, 0x00);
 	ld9040_brightness_set(ctx);
+
 	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
 	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_SET_DISPLAY_ON);
 }
-- 
2.40.1

[PATCH 1/3] drm/panel: ld9040: Use better magic values

[Paul Cercueil]

I have no idea what the prior magic values mean, and I have no idea
what my replacement (extracted from [1]) magic values mean.

What I do know, is that these new values result in a much better
picture, where the blacks are really black (as you would expect on an
AMOLED display) instead of grey-ish.

[1] https://github.com/dorimanx/Dorimanx-SG2-I9100-Kernel/blob/master-jelly-bean/arch/arm/mach-exynos/u1-panel.h

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/panel/panel-samsung-ld9040.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
index 01eb211f32f7..7fd9444b42c5 100644
--- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
+++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
@@ -180,17 +180,18 @@ static void ld9040_init(struct ld9040 *ctx)
 {
 	ld9040_dcs_write_seq_static(ctx, MCS_USER_SETTING, 0x5a, 0x5a);
 	ld9040_dcs_write_seq_static(ctx, MCS_PANEL_CONDITION,
-		0x05, 0x65, 0x96, 0x71, 0x7d, 0x19, 0x3b, 0x0d,
-		0x19, 0x7e, 0x0d, 0xe2, 0x00, 0x00, 0x7e, 0x7d,
-		0x07, 0x07, 0x20, 0x20, 0x20, 0x02, 0x02);
+		0x05, 0x5e, 0x96, 0x6b, 0x7d, 0x0d, 0x3f, 0x00,
+		0x00, 0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x07, 0x05, 0x1f, 0x1f, 0x1f, 0x00, 0x00);
 	ld9040_dcs_write_seq_static(ctx, MCS_DISPCTL,
-		0x02, 0x08, 0x08, 0x10, 0x10);
+		0x02, 0x06, 0x0a, 0x10, 0x10);
 	ld9040_dcs_write_seq_static(ctx, MCS_MANPWR, 0x04);
 	ld9040_dcs_write_seq_static(ctx, MCS_POWER_CTRL,
 		0x0a, 0x87, 0x25, 0x6a, 0x44, 0x02, 0x88);
-	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0d, 0x00, 0x16);
+	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0f, 0x00, 0x16);
 	ld9040_dcs_write_seq_static(ctx, MCS_GTCON, 0x09, 0x00, 0x00);
 	ld9040_brightness_set(ctx);
+
 	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
 	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_SET_DISPLAY_ON);
 }
-- 
2.40.1


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

[PATCH 1/3] drm/panel: ld9040: Use better magic values

[Paul Cercueil]

I have no idea what the prior magic values mean, and I have no idea
what my replacement (extracted from [1]) magic values mean.

What I do know, is that these new values result in a much better
picture, where the blacks are really black (as you would expect on an
AMOLED display) instead of grey-ish.

[1] https://github.com/dorimanx/Dorimanx-SG2-I9100-Kernel/blob/master-jelly-bean/arch/arm/mach-exynos/u1-panel.h

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/panel/panel-samsung-ld9040.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
index 01eb211f32f7..7fd9444b42c5 100644
--- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
+++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
@@ -180,17 +180,18 @@ static void ld9040_init(struct ld9040 *ctx)
 {
 	ld9040_dcs_write_seq_static(ctx, MCS_USER_SETTING, 0x5a, 0x5a);
 	ld9040_dcs_write_seq_static(ctx, MCS_PANEL_CONDITION,
-		0x05, 0x65, 0x96, 0x71, 0x7d, 0x19, 0x3b, 0x0d,
-		0x19, 0x7e, 0x0d, 0xe2, 0x00, 0x00, 0x7e, 0x7d,
-		0x07, 0x07, 0x20, 0x20, 0x20, 0x02, 0x02);
+		0x05, 0x5e, 0x96, 0x6b, 0x7d, 0x0d, 0x3f, 0x00,
+		0x00, 0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x07, 0x05, 0x1f, 0x1f, 0x1f, 0x00, 0x00);
 	ld9040_dcs_write_seq_static(ctx, MCS_DISPCTL,
-		0x02, 0x08, 0x08, 0x10, 0x10);
+		0x02, 0x06, 0x0a, 0x10, 0x10);
 	ld9040_dcs_write_seq_static(ctx, MCS_MANPWR, 0x04);
 	ld9040_dcs_write_seq_static(ctx, MCS_POWER_CTRL,
 		0x0a, 0x87, 0x25, 0x6a, 0x44, 0x02, 0x88);
-	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0d, 0x00, 0x16);
+	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0f, 0x00, 0x16);
 	ld9040_dcs_write_seq_static(ctx, MCS_GTCON, 0x09, 0x00, 0x00);
 	ld9040_brightness_set(ctx);
+
 	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
 	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_SET_DISPLAY_ON);
 }
-- 
2.40.1

[PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Paul Cercueil]

Register a backlight device to be able to switch between all the gamma
levels.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
index 7fd9444b42c5..b4f87d6244cb 100644
--- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
+++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
@@ -8,6 +8,7 @@
  * Andrzej Hajda <a.hajda@samsung.com>
 */
 
+#include <linux/backlight.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
@@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
 	return 0;
 }
 
+static int ld9040_bl_update_status(struct backlight_device *dev)
+{
+	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
+
+	ctx->brightness = dev->props.brightness;
+	ld9040_brightness_set(ctx);
+
+	return 0;
+}
+
+static int ld9040_bl_get_intensity(struct backlight_device *dev)
+{
+	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
+			dev->props.power == FB_BLANK_UNBLANK)
+		return dev->props.brightness;
+
+	return 0;
+}
+
+static const struct backlight_ops ld9040_bl_ops = {
+	.get_brightness = ld9040_bl_get_intensity,
+	.update_status  = ld9040_bl_update_status,
+};
+
+static const struct backlight_properties ld9040_bl_props = {
+	.type = BACKLIGHT_RAW,
+	.scale = BACKLIGHT_SCALE_NON_LINEAR,
+	.max_brightness = ARRAY_SIZE(ld9040_gammas) - 1,
+	.brightness = ARRAY_SIZE(ld9040_gammas) - 1,
+};
+
 static int ld9040_probe(struct spi_device *spi)
 {
+	struct backlight_device *bldev;
 	struct device *dev = &spi->dev;
 	struct ld9040 *ctx;
 	int ret;
@@ -354,6 +387,13 @@ static int ld9040_probe(struct spi_device *spi)
 	drm_panel_init(&ctx->panel, dev, &ld9040_drm_funcs,
 		       DRM_MODE_CONNECTOR_DPI);
 
+
+	bldev = devm_backlight_device_register(dev, dev_name(dev), dev,
+					       ctx, &ld9040_bl_ops,
+					       &ld9040_bl_props);
+	if (IS_ERR(bldev))
+		return PTR_ERR(bldev);
+
 	drm_panel_add(&ctx->panel);
 
 	return 0;
-- 
2.40.1

[PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Paul Cercueil]

Register a backlight device to be able to switch between all the gamma
levels.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
index 7fd9444b42c5..b4f87d6244cb 100644
--- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
+++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
@@ -8,6 +8,7 @@
  * Andrzej Hajda <a.hajda@samsung.com>
 */
 
+#include <linux/backlight.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
@@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
 	return 0;
 }
 
+static int ld9040_bl_update_status(struct backlight_device *dev)
+{
+	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
+
+	ctx->brightness = dev->props.brightness;
+	ld9040_brightness_set(ctx);
+
+	return 0;
+}
+
+static int ld9040_bl_get_intensity(struct backlight_device *dev)
+{
+	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
+			dev->props.power == FB_BLANK_UNBLANK)
+		return dev->props.brightness;
+
+	return 0;
+}
+
+static const struct backlight_ops ld9040_bl_ops = {
+	.get_brightness = ld9040_bl_get_intensity,
+	.update_status  = ld9040_bl_update_status,
+};
+
+static const struct backlight_properties ld9040_bl_props = {
+	.type = BACKLIGHT_RAW,
+	.scale = BACKLIGHT_SCALE_NON_LINEAR,
+	.max_brightness = ARRAY_SIZE(ld9040_gammas) - 1,
+	.brightness = ARRAY_SIZE(ld9040_gammas) - 1,
+};
+
 static int ld9040_probe(struct spi_device *spi)
 {
+	struct backlight_device *bldev;
 	struct device *dev = &spi->dev;
 	struct ld9040 *ctx;
 	int ret;
@@ -354,6 +387,13 @@ static int ld9040_probe(struct spi_device *spi)
 	drm_panel_init(&ctx->panel, dev, &ld9040_drm_funcs,
 		       DRM_MODE_CONNECTOR_DPI);
 
+
+	bldev = devm_backlight_device_register(dev, dev_name(dev), dev,
+					       ctx, &ld9040_bl_ops,
+					       &ld9040_bl_props);
+	if (IS_ERR(bldev))
+		return PTR_ERR(bldev);
+
 	drm_panel_add(&ctx->panel);
 
 	return 0;
-- 
2.40.1


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

[PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Paul Cercueil]

Register a backlight device to be able to switch between all the gamma
levels.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
index 7fd9444b42c5..b4f87d6244cb 100644
--- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
+++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
@@ -8,6 +8,7 @@
  * Andrzej Hajda <a.hajda@samsung.com>
 */
 
+#include <linux/backlight.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
@@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
 	return 0;
 }
 
+static int ld9040_bl_update_status(struct backlight_device *dev)
+{
+	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
+
+	ctx->brightness = dev->props.brightness;
+	ld9040_brightness_set(ctx);
+
+	return 0;
+}
+
+static int ld9040_bl_get_intensity(struct backlight_device *dev)
+{
+	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
+			dev->props.power == FB_BLANK_UNBLANK)
+		return dev->props.brightness;
+
+	return 0;
+}
+
+static const struct backlight_ops ld9040_bl_ops = {
+	.get_brightness = ld9040_bl_get_intensity,
+	.update_status  = ld9040_bl_update_status,
+};
+
+static const struct backlight_properties ld9040_bl_props = {
+	.type = BACKLIGHT_RAW,
+	.scale = BACKLIGHT_SCALE_NON_LINEAR,
+	.max_brightness = ARRAY_SIZE(ld9040_gammas) - 1,
+	.brightness = ARRAY_SIZE(ld9040_gammas) - 1,
+};
+
 static int ld9040_probe(struct spi_device *spi)
 {
+	struct backlight_device *bldev;
 	struct device *dev = &spi->dev;
 	struct ld9040 *ctx;
 	int ret;
@@ -354,6 +387,13 @@ static int ld9040_probe(struct spi_device *spi)
 	drm_panel_init(&ctx->panel, dev, &ld9040_drm_funcs,
 		       DRM_MODE_CONNECTOR_DPI);
 
+
+	bldev = devm_backlight_device_register(dev, dev_name(dev), dev,
+					       ctx, &ld9040_bl_ops,
+					       &ld9040_bl_props);
+	if (IS_ERR(bldev))
+		return PTR_ERR(bldev);
+
 	drm_panel_add(&ctx->panel);
 
 	return 0;
-- 
2.40.1

[PATCH 3/3] ARM: dts: exynos/i9100: Fix LCD screen's physical size

[Paul Cercueil]

The previous values were completely bogus, and resulted in the computed
DPI ratio being much lower than reality, causing applications and UIs to
misbehave.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Fixes: 8620cc2f99b7 ("ARM: dts: exynos: Add devicetree file for the Galaxy S2")
Cc: <stable@vger.kernel.org> # v5.8+
---
 arch/arm/boot/dts/exynos4210-i9100.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
index 37cd4dde53e4..a9ec1f6c1dea 100644
--- a/arch/arm/boot/dts/exynos4210-i9100.dts
+++ b/arch/arm/boot/dts/exynos4210-i9100.dts
@@ -207,8 +207,8 @@ lcd@0 {
 			power-on-delay = <10>;
 			reset-delay = <10>;
 
-			panel-width-mm = <90>;
-			panel-height-mm = <154>;
+			panel-width-mm = <56>;
+			panel-height-mm = <93>;
 
 			display-timings {
 				timing {
-- 
2.40.1

[PATCH 3/3] ARM: dts: exynos/i9100: Fix LCD screen's physical size

[Paul Cercueil]

The previous values were completely bogus, and resulted in the computed
DPI ratio being much lower than reality, causing applications and UIs to
misbehave.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Fixes: 8620cc2f99b7 ("ARM: dts: exynos: Add devicetree file for the Galaxy S2")
Cc: <stable@vger.kernel.org> # v5.8+
---
 arch/arm/boot/dts/exynos4210-i9100.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
index 37cd4dde53e4..a9ec1f6c1dea 100644
--- a/arch/arm/boot/dts/exynos4210-i9100.dts
+++ b/arch/arm/boot/dts/exynos4210-i9100.dts
@@ -207,8 +207,8 @@ lcd@0 {
 			power-on-delay = <10>;
 			reset-delay = <10>;
 
-			panel-width-mm = <90>;
-			panel-height-mm = <154>;
+			panel-width-mm = <56>;
+			panel-height-mm = <93>;
 
 			display-timings {
 				timing {
-- 
2.40.1


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

[PATCH 3/3] ARM: dts: exynos/i9100: Fix LCD screen's physical size

[Paul Cercueil]

The previous values were completely bogus, and resulted in the computed
DPI ratio being much lower than reality, causing applications and UIs to
misbehave.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Fixes: 8620cc2f99b7 ("ARM: dts: exynos: Add devicetree file for the Galaxy S2")
Cc: <stable@vger.kernel.org> # v5.8+
---
 arch/arm/boot/dts/exynos4210-i9100.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
index 37cd4dde53e4..a9ec1f6c1dea 100644
--- a/arch/arm/boot/dts/exynos4210-i9100.dts
+++ b/arch/arm/boot/dts/exynos4210-i9100.dts
@@ -207,8 +207,8 @@ lcd@0 {
 			power-on-delay = <10>;
 			reset-delay = <10>;
 
-			panel-width-mm = <90>;
-			panel-height-mm = <154>;
+			panel-width-mm = <56>;
+			panel-height-mm = <93>;
 
 			display-timings {
 				timing {
-- 
2.40.1

RE: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[대인기/Tizen Platform Lab(SR)/삼성전자]

Hi,

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> Paul Cercueil
> Sent: Tuesday, July 4, 2023 6:47 AM
> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Rob Herring
> <robh+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Alim Akhtar
> <alim.akhtar@samsung.com>; Neil Armstrong <neil.armstrong@linaro.org>; Sam
> Ravnborg <sam@ravnborg.org>
> Cc: devicetree@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; Paul Cercueil
> <paul@crapouillou.net>; linux-arm-kernel@lists.infradead.org
> Subject: [PATCH 2/3] drm/panel: ld9040: Register a backlight device
> 
> Register a backlight device to be able to switch between all the gamma
> levels.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 7fd9444b42c5..b4f87d6244cb 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -8,6 +8,7 @@
>   * Andrzej Hajda <a.hajda@samsung.com>
>  */
> 
> +#include <linux/backlight.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
> @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
>  	return 0;
>  }
> 
> +static int ld9040_bl_update_status(struct backlight_device *dev)
> +{
> +	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> +
> +	ctx->brightness = dev->props.brightness;
> +	ld9040_brightness_set(ctx);
> +
> +	return 0;
> +}
> +
> +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> +{
> +	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&

fb_blank member is deprecated according to the description of backlight.h
file so you could drop above condition I think.

Thanks,
Inki Dae

RE: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[대인기/Tizen Platform Lab(SR)/삼성전자]

Hi,

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> Paul Cercueil
> Sent: Tuesday, July 4, 2023 6:47 AM
> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Rob Herring
> <robh+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Alim Akhtar
> <alim.akhtar@samsung.com>; Neil Armstrong <neil.armstrong@linaro.org>; Sam
> Ravnborg <sam@ravnborg.org>
> Cc: devicetree@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; Paul Cercueil
> <paul@crapouillou.net>; linux-arm-kernel@lists.infradead.org
> Subject: [PATCH 2/3] drm/panel: ld9040: Register a backlight device
> 
> Register a backlight device to be able to switch between all the gamma
> levels.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 7fd9444b42c5..b4f87d6244cb 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -8,6 +8,7 @@
>   * Andrzej Hajda <a.hajda@samsung.com>
>  */
> 
> +#include <linux/backlight.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
> @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
>  	return 0;
>  }
> 
> +static int ld9040_bl_update_status(struct backlight_device *dev)
> +{
> +	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> +
> +	ctx->brightness = dev->props.brightness;
> +	ld9040_brightness_set(ctx);
> +
> +	return 0;
> +}
> +
> +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> +{
> +	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&

fb_blank member is deprecated according to the description of backlight.h
file so you could drop above condition I think.

Thanks,
Inki Dae

RE: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[대인기/Tizen Platform Lab(SR)/삼성전자]

Hi,

> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> Paul Cercueil
> Sent: Tuesday, July 4, 2023 6:47 AM
> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Rob Herring
> <robh+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Alim Akhtar
> <alim.akhtar@samsung.com>; Neil Armstrong <neil.armstrong@linaro.org>; Sam
> Ravnborg <sam@ravnborg.org>
> Cc: devicetree@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-
> kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; Paul Cercueil
> <paul@crapouillou.net>; linux-arm-kernel@lists.infradead.org
> Subject: [PATCH 2/3] drm/panel: ld9040: Register a backlight device
> 
> Register a backlight device to be able to switch between all the gamma
> levels.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 7fd9444b42c5..b4f87d6244cb 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -8,6 +8,7 @@
>   * Andrzej Hajda <a.hajda@samsung.com>
>  */
> 
> +#include <linux/backlight.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
> @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
>  	return 0;
>  }
> 
> +static int ld9040_bl_update_status(struct backlight_device *dev)
> +{
> +	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> +
> +	ctx->brightness = dev->props.brightness;
> +	ld9040_brightness_set(ctx);
> +
> +	return 0;
> +}
> +
> +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> +{
> +	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&

fb_blank member is deprecated according to the description of backlight.h
file so you could drop above condition I think.

Thanks,
Inki Dae


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

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Paul Cercueil]

Hi Inki,

Le mardi 04 juillet 2023 à 08:49 +0900, 대인기/Tizen Platform Lab(SR)/삼성전자
a écrit :
> Hi,
> 
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf
> > Of
> > Paul Cercueil
> > Sent: Tuesday, July 4, 2023 6:47 AM
> > To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Rob
> > Herring
> > <robh+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Alim
> > Akhtar
> > <alim.akhtar@samsung.com>; Neil Armstrong
> > <neil.armstrong@linaro.org>; Sam
> > Ravnborg <sam@ravnborg.org>
> > Cc: devicetree@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
> > linux-
> > kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; Paul
> > Cercueil
> > <paul@crapouillou.net>; linux-arm-kernel@lists.infradead.org
> > Subject: [PATCH 2/3] drm/panel: ld9040: Register a backlight device
> > 
> > Register a backlight device to be able to switch between all the
> > gamma
> > levels.
> > 
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > ---
> >  drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40
> > ++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > index 7fd9444b42c5..b4f87d6244cb 100644
> > --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > @@ -8,6 +8,7 @@
> >   * Andrzej Hajda <a.hajda@samsung.com>
> >  */
> > 
> > +#include <linux/backlight.h>
> >  #include <linux/delay.h>
> >  #include <linux/gpio/consumer.h>
> >  #include <linux/module.h>
> > @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
> >         return 0;
> >  }
> > 
> > +static int ld9040_bl_update_status(struct backlight_device *dev)
> > +{
> > +       struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> > +
> > +       ctx->brightness = dev->props.brightness;
> > +       ld9040_brightness_set(ctx);
> > +
> > +       return 0;
> > +}
> > +
> > +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> > +{
> > +       if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> 
> fb_blank member is deprecated according to the description of
> backlight.h
> file so you could drop above condition I think.

Thanks. I'll send a V2.

Cheers,
-Paul

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Paul Cercueil]

Hi Inki,

Le mardi 04 juillet 2023 à 08:49 +0900, 대인기/Tizen Platform Lab(SR)/삼성전자
a écrit :
> Hi,
> 
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf
> > Of
> > Paul Cercueil
> > Sent: Tuesday, July 4, 2023 6:47 AM
> > To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Rob
> > Herring
> > <robh+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Alim
> > Akhtar
> > <alim.akhtar@samsung.com>; Neil Armstrong
> > <neil.armstrong@linaro.org>; Sam
> > Ravnborg <sam@ravnborg.org>
> > Cc: devicetree@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
> > linux-
> > kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; Paul
> > Cercueil
> > <paul@crapouillou.net>; linux-arm-kernel@lists.infradead.org
> > Subject: [PATCH 2/3] drm/panel: ld9040: Register a backlight device
> > 
> > Register a backlight device to be able to switch between all the
> > gamma
> > levels.
> > 
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > ---
> >  drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40
> > ++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > index 7fd9444b42c5..b4f87d6244cb 100644
> > --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > @@ -8,6 +8,7 @@
> >   * Andrzej Hajda <a.hajda@samsung.com>
> >  */
> > 
> > +#include <linux/backlight.h>
> >  #include <linux/delay.h>
> >  #include <linux/gpio/consumer.h>
> >  #include <linux/module.h>
> > @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
> >         return 0;
> >  }
> > 
> > +static int ld9040_bl_update_status(struct backlight_device *dev)
> > +{
> > +       struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> > +
> > +       ctx->brightness = dev->props.brightness;
> > +       ld9040_brightness_set(ctx);
> > +
> > +       return 0;
> > +}
> > +
> > +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> > +{
> > +       if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> 
> fb_blank member is deprecated according to the description of
> backlight.h
> file so you could drop above condition I think.

Thanks. I'll send a V2.

Cheers,
-Paul

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

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Paul Cercueil]

Hi Inki,

Le mardi 04 juillet 2023 à 08:49 +0900, 대인기/Tizen Platform Lab(SR)/삼성전자
a écrit :
> Hi,
> 
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf
> > Of
> > Paul Cercueil
> > Sent: Tuesday, July 4, 2023 6:47 AM
> > To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Rob
> > Herring
> > <robh+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Alim
> > Akhtar
> > <alim.akhtar@samsung.com>; Neil Armstrong
> > <neil.armstrong@linaro.org>; Sam
> > Ravnborg <sam@ravnborg.org>
> > Cc: devicetree@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
> > linux-
> > kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; Paul
> > Cercueil
> > <paul@crapouillou.net>; linux-arm-kernel@lists.infradead.org
> > Subject: [PATCH 2/3] drm/panel: ld9040: Register a backlight device
> > 
> > Register a backlight device to be able to switch between all the
> > gamma
> > levels.
> > 
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > ---
> >  drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40
> > ++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > index 7fd9444b42c5..b4f87d6244cb 100644
> > --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > @@ -8,6 +8,7 @@
> >   * Andrzej Hajda <a.hajda@samsung.com>
> >  */
> > 
> > +#include <linux/backlight.h>
> >  #include <linux/delay.h>
> >  #include <linux/gpio/consumer.h>
> >  #include <linux/module.h>
> > @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
> >         return 0;
> >  }
> > 
> > +static int ld9040_bl_update_status(struct backlight_device *dev)
> > +{
> > +       struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> > +
> > +       ctx->brightness = dev->props.brightness;
> > +       ld9040_brightness_set(ctx);
> > +
> > +       return 0;
> > +}
> > +
> > +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> > +{
> > +       if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> 
> fb_blank member is deprecated according to the description of
> backlight.h
> file so you could drop above condition I think.

Thanks. I'll send a V2.

Cheers,
-Paul

Re: [PATCH 1/3] drm/panel: ld9040: Use better magic values

[Neil Armstrong]

Hi

On 03/07/2023 23:47, Paul Cercueil wrote:
> I have no idea what the prior magic values mean, and I have no idea
> what my replacement (extracted from [1]) magic values mean.
> 
> What I do know, is that these new values result in a much better
> picture, where the blacks are really black (as you would expect on an
> AMOLED display) instead of grey-ish.
> 
> [1] https://github.com/dorimanx/Dorimanx-SG2-I9100-Kernel/blob/master-jelly-bean/arch/arm/mach-exynos/u1-panel.h
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 01eb211f32f7..7fd9444b42c5 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -180,17 +180,18 @@ static void ld9040_init(struct ld9040 *ctx)
>   {
>   	ld9040_dcs_write_seq_static(ctx, MCS_USER_SETTING, 0x5a, 0x5a);
>   	ld9040_dcs_write_seq_static(ctx, MCS_PANEL_CONDITION,
> -		0x05, 0x65, 0x96, 0x71, 0x7d, 0x19, 0x3b, 0x0d,
> -		0x19, 0x7e, 0x0d, 0xe2, 0x00, 0x00, 0x7e, 0x7d,
> -		0x07, 0x07, 0x20, 0x20, 0x20, 0x02, 0x02);
> +		0x05, 0x5e, 0x96, 0x6b, 0x7d, 0x0d, 0x3f, 0x00,
> +		0x00, 0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +		0x07, 0x05, 0x1f, 0x1f, 0x1f, 0x00, 0x00);
>   	ld9040_dcs_write_seq_static(ctx, MCS_DISPCTL,
> -		0x02, 0x08, 0x08, 0x10, 0x10);
> +		0x02, 0x06, 0x0a, 0x10, 0x10);
>   	ld9040_dcs_write_seq_static(ctx, MCS_MANPWR, 0x04);
>   	ld9040_dcs_write_seq_static(ctx, MCS_POWER_CTRL,
>   		0x0a, 0x87, 0x25, 0x6a, 0x44, 0x02, 0x88);
> -	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0d, 0x00, 0x16);
> +	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0f, 0x00, 0x16);
>   	ld9040_dcs_write_seq_static(ctx, MCS_GTCON, 0x09, 0x00, 0x00);
>   	ld9040_brightness_set(ctx);
> +

You can drop this spurious new line for v2

>   	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
>   	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_SET_DISPLAY_ON);
>   }

And add

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Neil

Re: [PATCH 1/3] drm/panel: ld9040: Use better magic values

[Neil Armstrong]

Hi

On 03/07/2023 23:47, Paul Cercueil wrote:
> I have no idea what the prior magic values mean, and I have no idea
> what my replacement (extracted from [1]) magic values mean.
> 
> What I do know, is that these new values result in a much better
> picture, where the blacks are really black (as you would expect on an
> AMOLED display) instead of grey-ish.
> 
> [1] https://github.com/dorimanx/Dorimanx-SG2-I9100-Kernel/blob/master-jelly-bean/arch/arm/mach-exynos/u1-panel.h
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 01eb211f32f7..7fd9444b42c5 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -180,17 +180,18 @@ static void ld9040_init(struct ld9040 *ctx)
>   {
>   	ld9040_dcs_write_seq_static(ctx, MCS_USER_SETTING, 0x5a, 0x5a);
>   	ld9040_dcs_write_seq_static(ctx, MCS_PANEL_CONDITION,
> -		0x05, 0x65, 0x96, 0x71, 0x7d, 0x19, 0x3b, 0x0d,
> -		0x19, 0x7e, 0x0d, 0xe2, 0x00, 0x00, 0x7e, 0x7d,
> -		0x07, 0x07, 0x20, 0x20, 0x20, 0x02, 0x02);
> +		0x05, 0x5e, 0x96, 0x6b, 0x7d, 0x0d, 0x3f, 0x00,
> +		0x00, 0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +		0x07, 0x05, 0x1f, 0x1f, 0x1f, 0x00, 0x00);
>   	ld9040_dcs_write_seq_static(ctx, MCS_DISPCTL,
> -		0x02, 0x08, 0x08, 0x10, 0x10);
> +		0x02, 0x06, 0x0a, 0x10, 0x10);
>   	ld9040_dcs_write_seq_static(ctx, MCS_MANPWR, 0x04);
>   	ld9040_dcs_write_seq_static(ctx, MCS_POWER_CTRL,
>   		0x0a, 0x87, 0x25, 0x6a, 0x44, 0x02, 0x88);
> -	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0d, 0x00, 0x16);
> +	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0f, 0x00, 0x16);
>   	ld9040_dcs_write_seq_static(ctx, MCS_GTCON, 0x09, 0x00, 0x00);
>   	ld9040_brightness_set(ctx);
> +

You can drop this spurious new line for v2

>   	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
>   	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_SET_DISPLAY_ON);
>   }

And add

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Neil

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

Re: [PATCH 1/3] drm/panel: ld9040: Use better magic values

[Neil Armstrong]

Hi

On 03/07/2023 23:47, Paul Cercueil wrote:
> I have no idea what the prior magic values mean, and I have no idea
> what my replacement (extracted from [1]) magic values mean.
> 
> What I do know, is that these new values result in a much better
> picture, where the blacks are really black (as you would expect on an
> AMOLED display) instead of grey-ish.
> 
> [1] https://github.com/dorimanx/Dorimanx-SG2-I9100-Kernel/blob/master-jelly-bean/arch/arm/mach-exynos/u1-panel.h
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 01eb211f32f7..7fd9444b42c5 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -180,17 +180,18 @@ static void ld9040_init(struct ld9040 *ctx)
>   {
>   	ld9040_dcs_write_seq_static(ctx, MCS_USER_SETTING, 0x5a, 0x5a);
>   	ld9040_dcs_write_seq_static(ctx, MCS_PANEL_CONDITION,
> -		0x05, 0x65, 0x96, 0x71, 0x7d, 0x19, 0x3b, 0x0d,
> -		0x19, 0x7e, 0x0d, 0xe2, 0x00, 0x00, 0x7e, 0x7d,
> -		0x07, 0x07, 0x20, 0x20, 0x20, 0x02, 0x02);
> +		0x05, 0x5e, 0x96, 0x6b, 0x7d, 0x0d, 0x3f, 0x00,
> +		0x00, 0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +		0x07, 0x05, 0x1f, 0x1f, 0x1f, 0x00, 0x00);
>   	ld9040_dcs_write_seq_static(ctx, MCS_DISPCTL,
> -		0x02, 0x08, 0x08, 0x10, 0x10);
> +		0x02, 0x06, 0x0a, 0x10, 0x10);
>   	ld9040_dcs_write_seq_static(ctx, MCS_MANPWR, 0x04);
>   	ld9040_dcs_write_seq_static(ctx, MCS_POWER_CTRL,
>   		0x0a, 0x87, 0x25, 0x6a, 0x44, 0x02, 0x88);
> -	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0d, 0x00, 0x16);
> +	ld9040_dcs_write_seq_static(ctx, MCS_ELVSS_ON, 0x0f, 0x00, 0x16);
>   	ld9040_dcs_write_seq_static(ctx, MCS_GTCON, 0x09, 0x00, 0x00);
>   	ld9040_brightness_set(ctx);
> +

You can drop this spurious new line for v2

>   	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
>   	ld9040_dcs_write_seq_static(ctx, MIPI_DCS_SET_DISPLAY_ON);
>   }

And add

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Neil

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Neil Armstrong]

On 03/07/2023 23:47, Paul Cercueil wrote:
> Register a backlight device to be able to switch between all the gamma
> levels.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
>   1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 7fd9444b42c5..b4f87d6244cb 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -8,6 +8,7 @@
>    * Andrzej Hajda <a.hajda@samsung.com>
>   */
>   
> +#include <linux/backlight.h>
>   #include <linux/delay.h>
>   #include <linux/gpio/consumer.h>
>   #include <linux/module.h>
> @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
>   	return 0;
>   }
>   
> +static int ld9040_bl_update_status(struct backlight_device *dev)
> +{
> +	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> +
> +	ctx->brightness = dev->props.brightness;
> +	ld9040_brightness_set(ctx);
> +
> +	return 0;
> +}
> +
> +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> +{
> +	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> +			dev->props.power == FB_BLANK_UNBLANK)
> +		return dev->props.brightness;
> +
> +	return 0;
> +}

You can totally drop the _get_brightness.

Neil

> +
> +static const struct backlight_ops ld9040_bl_ops = {
> +	.get_brightness = ld9040_bl_get_intensity,
> +	.update_status  = ld9040_bl_update_status,
> +};
> +
> +static const struct backlight_properties ld9040_bl_props = {
> +	.type = BACKLIGHT_RAW,
> +	.scale = BACKLIGHT_SCALE_NON_LINEAR,
> +	.max_brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> +	.brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> +};
> +
>   static int ld9040_probe(struct spi_device *spi)
>   {
> +	struct backlight_device *bldev;
>   	struct device *dev = &spi->dev;
>   	struct ld9040 *ctx;
>   	int ret;
> @@ -354,6 +387,13 @@ static int ld9040_probe(struct spi_device *spi)
>   	drm_panel_init(&ctx->panel, dev, &ld9040_drm_funcs,
>   		       DRM_MODE_CONNECTOR_DPI);
>   
> +
> +	bldev = devm_backlight_device_register(dev, dev_name(dev), dev,
> +					       ctx, &ld9040_bl_ops,
> +					       &ld9040_bl_props);
> +	if (IS_ERR(bldev))
> +		return PTR_ERR(bldev);
> +
>   	drm_panel_add(&ctx->panel);
>   
>   	return 0;

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Neil Armstrong]

On 03/07/2023 23:47, Paul Cercueil wrote:
> Register a backlight device to be able to switch between all the gamma
> levels.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
>   1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 7fd9444b42c5..b4f87d6244cb 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -8,6 +8,7 @@
>    * Andrzej Hajda <a.hajda@samsung.com>
>   */
>   
> +#include <linux/backlight.h>
>   #include <linux/delay.h>
>   #include <linux/gpio/consumer.h>
>   #include <linux/module.h>
> @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
>   	return 0;
>   }
>   
> +static int ld9040_bl_update_status(struct backlight_device *dev)
> +{
> +	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> +
> +	ctx->brightness = dev->props.brightness;
> +	ld9040_brightness_set(ctx);
> +
> +	return 0;
> +}
> +
> +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> +{
> +	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> +			dev->props.power == FB_BLANK_UNBLANK)
> +		return dev->props.brightness;
> +
> +	return 0;
> +}

You can totally drop the _get_brightness.

Neil

> +
> +static const struct backlight_ops ld9040_bl_ops = {
> +	.get_brightness = ld9040_bl_get_intensity,
> +	.update_status  = ld9040_bl_update_status,
> +};
> +
> +static const struct backlight_properties ld9040_bl_props = {
> +	.type = BACKLIGHT_RAW,
> +	.scale = BACKLIGHT_SCALE_NON_LINEAR,
> +	.max_brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> +	.brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> +};
> +
>   static int ld9040_probe(struct spi_device *spi)
>   {
> +	struct backlight_device *bldev;
>   	struct device *dev = &spi->dev;
>   	struct ld9040 *ctx;
>   	int ret;
> @@ -354,6 +387,13 @@ static int ld9040_probe(struct spi_device *spi)
>   	drm_panel_init(&ctx->panel, dev, &ld9040_drm_funcs,
>   		       DRM_MODE_CONNECTOR_DPI);
>   
> +
> +	bldev = devm_backlight_device_register(dev, dev_name(dev), dev,
> +					       ctx, &ld9040_bl_ops,
> +					       &ld9040_bl_props);
> +	if (IS_ERR(bldev))
> +		return PTR_ERR(bldev);
> +
>   	drm_panel_add(&ctx->panel);
>   
>   	return 0;


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

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Neil Armstrong]

On 03/07/2023 23:47, Paul Cercueil wrote:
> Register a backlight device to be able to switch between all the gamma
> levels.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
>   1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 7fd9444b42c5..b4f87d6244cb 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -8,6 +8,7 @@
>    * Andrzej Hajda <a.hajda@samsung.com>
>   */
>   
> +#include <linux/backlight.h>
>   #include <linux/delay.h>
>   #include <linux/gpio/consumer.h>
>   #include <linux/module.h>
> @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
>   	return 0;
>   }
>   
> +static int ld9040_bl_update_status(struct backlight_device *dev)
> +{
> +	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> +
> +	ctx->brightness = dev->props.brightness;
> +	ld9040_brightness_set(ctx);
> +
> +	return 0;
> +}
> +
> +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> +{
> +	if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> +			dev->props.power == FB_BLANK_UNBLANK)
> +		return dev->props.brightness;
> +
> +	return 0;
> +}

You can totally drop the _get_brightness.

Neil

> +
> +static const struct backlight_ops ld9040_bl_ops = {
> +	.get_brightness = ld9040_bl_get_intensity,
> +	.update_status  = ld9040_bl_update_status,
> +};
> +
> +static const struct backlight_properties ld9040_bl_props = {
> +	.type = BACKLIGHT_RAW,
> +	.scale = BACKLIGHT_SCALE_NON_LINEAR,
> +	.max_brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> +	.brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> +};
> +
>   static int ld9040_probe(struct spi_device *spi)
>   {
> +	struct backlight_device *bldev;
>   	struct device *dev = &spi->dev;
>   	struct ld9040 *ctx;
>   	int ret;
> @@ -354,6 +387,13 @@ static int ld9040_probe(struct spi_device *spi)
>   	drm_panel_init(&ctx->panel, dev, &ld9040_drm_funcs,
>   		       DRM_MODE_CONNECTOR_DPI);
>   
> +
> +	bldev = devm_backlight_device_register(dev, dev_name(dev), dev,
> +					       ctx, &ld9040_bl_ops,
> +					       &ld9040_bl_props);
> +	if (IS_ERR(bldev))
> +		return PTR_ERR(bldev);
> +
>   	drm_panel_add(&ctx->panel);
>   
>   	return 0;

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Paul Cercueil]

Hi Neil,

Le mercredi 05 juillet 2023 à 15:57 +0200, Neil Armstrong a écrit :
> On 03/07/2023 23:47, Paul Cercueil wrote:
> > Register a backlight device to be able to switch between all the
> > gamma
> > levels.
> > 
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > ---
> >   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40
> > ++++++++++++++++++++
> >   1 file changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > index 7fd9444b42c5..b4f87d6244cb 100644
> > --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > @@ -8,6 +8,7 @@
> >    * Andrzej Hajda <a.hajda@samsung.com>
> >   */
> >   
> > +#include <linux/backlight.h>
> >   #include <linux/delay.h>
> >   #include <linux/gpio/consumer.h>
> >   #include <linux/module.h>
> > @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
> >         return 0;
> >   }
> >   
> > +static int ld9040_bl_update_status(struct backlight_device *dev)
> > +{
> > +       struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> > +
> > +       ctx->brightness = dev->props.brightness;
> > +       ld9040_brightness_set(ctx);
> > +
> > +       return 0;
> > +}
> > +
> > +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> > +{
> > +       if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> > +                       dev->props.power == FB_BLANK_UNBLANK)
> > +               return dev->props.brightness;
> > +
> > +       return 0;
> > +}
> 
> You can totally drop the _get_brightness.

The current behaviour is to return 0 when the framebuffer is blanked. A
few drivers do that so I thought it was the norm; and the backlight
core doesn't do that by default (and just uses dev->props.brightness).

It is not clear to me if that's the preferred behaviour. The
"backlight_get_brightness" function in backlight.h seems to suggest
that the current behaviour is correct, unless it is not supposed to be
used in the backlight_ops.get_brightness() callback. Then in that case
some other drivers get it wrong too.

Cheers,
-Paul

> Neil
> 
> > +
> > +static const struct backlight_ops ld9040_bl_ops = {
> > +       .get_brightness = ld9040_bl_get_intensity,
> > +       .update_status  = ld9040_bl_update_status,
> > +};
> > +
> > +static const struct backlight_properties ld9040_bl_props = {
> > +       .type = BACKLIGHT_RAW,
> > +       .scale = BACKLIGHT_SCALE_NON_LINEAR,
> > +       .max_brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> > +       .brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> > +};
> > +
> >   static int ld9040_probe(struct spi_device *spi)
> >   {
> > +       struct backlight_device *bldev;
> >         struct device *dev = &spi->dev;
> >         struct ld9040 *ctx;
> >         int ret;
> > @@ -354,6 +387,13 @@ static int ld9040_probe(struct spi_device
> > *spi)
> >         drm_panel_init(&ctx->panel, dev, &ld9040_drm_funcs,
> >                        DRM_MODE_CONNECTOR_DPI);
> >   
> > +
> > +       bldev = devm_backlight_device_register(dev, dev_name(dev),
> > dev,
> > +                                              ctx, &ld9040_bl_ops,
> > +                                              &ld9040_bl_props);
> > +       if (IS_ERR(bldev))
> > +               return PTR_ERR(bldev);
> > +
> >         drm_panel_add(&ctx->panel);
> >   
> >         return 0;
> 

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Paul Cercueil]

Hi Neil,

Le mercredi 05 juillet 2023 à 15:57 +0200, Neil Armstrong a écrit :
> On 03/07/2023 23:47, Paul Cercueil wrote:
> > Register a backlight device to be able to switch between all the
> > gamma
> > levels.
> > 
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > ---
> >   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40
> > ++++++++++++++++++++
> >   1 file changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > index 7fd9444b42c5..b4f87d6244cb 100644
> > --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > @@ -8,6 +8,7 @@
> >    * Andrzej Hajda <a.hajda@samsung.com>
> >   */
> >   
> > +#include <linux/backlight.h>
> >   #include <linux/delay.h>
> >   #include <linux/gpio/consumer.h>
> >   #include <linux/module.h>
> > @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
> >         return 0;
> >   }
> >   
> > +static int ld9040_bl_update_status(struct backlight_device *dev)
> > +{
> > +       struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> > +
> > +       ctx->brightness = dev->props.brightness;
> > +       ld9040_brightness_set(ctx);
> > +
> > +       return 0;
> > +}
> > +
> > +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> > +{
> > +       if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> > +                       dev->props.power == FB_BLANK_UNBLANK)
> > +               return dev->props.brightness;
> > +
> > +       return 0;
> > +}
> 
> You can totally drop the _get_brightness.

The current behaviour is to return 0 when the framebuffer is blanked. A
few drivers do that so I thought it was the norm; and the backlight
core doesn't do that by default (and just uses dev->props.brightness).

It is not clear to me if that's the preferred behaviour. The
"backlight_get_brightness" function in backlight.h seems to suggest
that the current behaviour is correct, unless it is not supposed to be
used in the backlight_ops.get_brightness() callback. Then in that case
some other drivers get it wrong too.

Cheers,
-Paul

> Neil
> 
> > +
> > +static const struct backlight_ops ld9040_bl_ops = {
> > +       .get_brightness = ld9040_bl_get_intensity,
> > +       .update_status  = ld9040_bl_update_status,
> > +};
> > +
> > +static const struct backlight_properties ld9040_bl_props = {
> > +       .type = BACKLIGHT_RAW,
> > +       .scale = BACKLIGHT_SCALE_NON_LINEAR,
> > +       .max_brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> > +       .brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> > +};
> > +
> >   static int ld9040_probe(struct spi_device *spi)
> >   {
> > +       struct backlight_device *bldev;
> >         struct device *dev = &spi->dev;
> >         struct ld9040 *ctx;
> >         int ret;
> > @@ -354,6 +387,13 @@ static int ld9040_probe(struct spi_device
> > *spi)
> >         drm_panel_init(&ctx->panel, dev, &ld9040_drm_funcs,
> >                        DRM_MODE_CONNECTOR_DPI);
> >   
> > +
> > +       bldev = devm_backlight_device_register(dev, dev_name(dev),
> > dev,
> > +                                              ctx, &ld9040_bl_ops,
> > +                                              &ld9040_bl_props);
> > +       if (IS_ERR(bldev))
> > +               return PTR_ERR(bldev);
> > +
> >         drm_panel_add(&ctx->panel);
> >   
> >         return 0;
> 

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

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Paul Cercueil]

Hi Neil,

Le mercredi 05 juillet 2023 à 15:57 +0200, Neil Armstrong a écrit :
> On 03/07/2023 23:47, Paul Cercueil wrote:
> > Register a backlight device to be able to switch between all the
> > gamma
> > levels.
> > 
> > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > ---
> >   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40
> > ++++++++++++++++++++
> >   1 file changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > index 7fd9444b42c5..b4f87d6244cb 100644
> > --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > @@ -8,6 +8,7 @@
> >    * Andrzej Hajda <a.hajda@samsung.com>
> >   */
> >   
> > +#include <linux/backlight.h>
> >   #include <linux/delay.h>
> >   #include <linux/gpio/consumer.h>
> >   #include <linux/module.h>
> > @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
> >         return 0;
> >   }
> >   
> > +static int ld9040_bl_update_status(struct backlight_device *dev)
> > +{
> > +       struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> > +
> > +       ctx->brightness = dev->props.brightness;
> > +       ld9040_brightness_set(ctx);
> > +
> > +       return 0;
> > +}
> > +
> > +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> > +{
> > +       if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> > +                       dev->props.power == FB_BLANK_UNBLANK)
> > +               return dev->props.brightness;
> > +
> > +       return 0;
> > +}
> 
> You can totally drop the _get_brightness.

The current behaviour is to return 0 when the framebuffer is blanked. A
few drivers do that so I thought it was the norm; and the backlight
core doesn't do that by default (and just uses dev->props.brightness).

It is not clear to me if that's the preferred behaviour. The
"backlight_get_brightness" function in backlight.h seems to suggest
that the current behaviour is correct, unless it is not supposed to be
used in the backlight_ops.get_brightness() callback. Then in that case
some other drivers get it wrong too.

Cheers,
-Paul

> Neil
> 
> > +
> > +static const struct backlight_ops ld9040_bl_ops = {
> > +       .get_brightness = ld9040_bl_get_intensity,
> > +       .update_status  = ld9040_bl_update_status,
> > +};
> > +
> > +static const struct backlight_properties ld9040_bl_props = {
> > +       .type = BACKLIGHT_RAW,
> > +       .scale = BACKLIGHT_SCALE_NON_LINEAR,
> > +       .max_brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> > +       .brightness = ARRAY_SIZE(ld9040_gammas) - 1,
> > +};
> > +
> >   static int ld9040_probe(struct spi_device *spi)
> >   {
> > +       struct backlight_device *bldev;
> >         struct device *dev = &spi->dev;
> >         struct ld9040 *ctx;
> >         int ret;
> > @@ -354,6 +387,13 @@ static int ld9040_probe(struct spi_device
> > *spi)
> >         drm_panel_init(&ctx->panel, dev, &ld9040_drm_funcs,
> >                        DRM_MODE_CONNECTOR_DPI);
> >   
> > +
> > +       bldev = devm_backlight_device_register(dev, dev_name(dev),
> > dev,
> > +                                              ctx, &ld9040_bl_ops,
> > +                                              &ld9040_bl_props);
> > +       if (IS_ERR(bldev))
> > +               return PTR_ERR(bldev);
> > +
> >         drm_panel_add(&ctx->panel);
> >   
> >         return 0;
> 

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Sam Ravnborg]

Hi Paul,

On Wed, Jul 05, 2023 at 04:38:05PM +0200, Paul Cercueil wrote:
> Hi Neil,
> 
> Le mercredi 05 juillet 2023 à 15:57 +0200, Neil Armstrong a écrit :
> > On 03/07/2023 23:47, Paul Cercueil wrote:
> > > Register a backlight device to be able to switch between all the
> > > gamma
> > > levels.
> > > 
> > > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > > ---
> > >   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40
> > > ++++++++++++++++++++
> > >   1 file changed, 40 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > index 7fd9444b42c5..b4f87d6244cb 100644
> > > --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > @@ -8,6 +8,7 @@
> > >    * Andrzej Hajda <a.hajda@samsung.com>
> > >   */
> > >   
> > > +#include <linux/backlight.h>
> > >   #include <linux/delay.h>
> > >   #include <linux/gpio/consumer.h>
> > >   #include <linux/module.h>
> > > @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
> > >         return 0;
> > >   }
> > >   
> > > +static int ld9040_bl_update_status(struct backlight_device *dev)
> > > +{
> > > +       struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> > > +
> > > +       ctx->brightness = dev->props.brightness;
Use backlight_get_brightness(dev);

> > > +       ld9040_brightness_set(ctx);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> > > +{
> > > +       if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> > > +                       dev->props.power == FB_BLANK_UNBLANK)
> > > +               return dev->props.brightness;
> > > +
> > > +       return 0;
> > > +}
> > 
> > You can totally drop the _get_brightness.
> 
> The current behaviour is to return 0 when the framebuffer is blanked. A
> few drivers do that so I thought it was the norm; and the backlight
> core doesn't do that by default (and just uses dev->props.brightness).
> 
> It is not clear to me if that's the preferred behaviour. The
> "backlight_get_brightness" function in backlight.h seems to suggest
> that the current behaviour is correct, unless it is not supposed to be
> used in the backlight_ops.get_brightness() callback. Then in that case
> some other drivers get it wrong too.
Several drivers get it wrong.
You are supposed to provide get_brightness only when you read back a
value from the HW, which is not the case here so just drop it is the
right choice.

	Sam

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Sam Ravnborg]

Hi Paul,

On Wed, Jul 05, 2023 at 04:38:05PM +0200, Paul Cercueil wrote:
> Hi Neil,
> 
> Le mercredi 05 juillet 2023 à 15:57 +0200, Neil Armstrong a écrit :
> > On 03/07/2023 23:47, Paul Cercueil wrote:
> > > Register a backlight device to be able to switch between all the
> > > gamma
> > > levels.
> > > 
> > > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > > ---
> > >   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40
> > > ++++++++++++++++++++
> > >   1 file changed, 40 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > index 7fd9444b42c5..b4f87d6244cb 100644
> > > --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > @@ -8,6 +8,7 @@
> > >    * Andrzej Hajda <a.hajda@samsung.com>
> > >   */
> > >   
> > > +#include <linux/backlight.h>
> > >   #include <linux/delay.h>
> > >   #include <linux/gpio/consumer.h>
> > >   #include <linux/module.h>
> > > @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
> > >         return 0;
> > >   }
> > >   
> > > +static int ld9040_bl_update_status(struct backlight_device *dev)
> > > +{
> > > +       struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> > > +
> > > +       ctx->brightness = dev->props.brightness;
Use backlight_get_brightness(dev);

> > > +       ld9040_brightness_set(ctx);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> > > +{
> > > +       if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> > > +                       dev->props.power == FB_BLANK_UNBLANK)
> > > +               return dev->props.brightness;
> > > +
> > > +       return 0;
> > > +}
> > 
> > You can totally drop the _get_brightness.
> 
> The current behaviour is to return 0 when the framebuffer is blanked. A
> few drivers do that so I thought it was the norm; and the backlight
> core doesn't do that by default (and just uses dev->props.brightness).
> 
> It is not clear to me if that's the preferred behaviour. The
> "backlight_get_brightness" function in backlight.h seems to suggest
> that the current behaviour is correct, unless it is not supposed to be
> used in the backlight_ops.get_brightness() callback. Then in that case
> some other drivers get it wrong too.
Several drivers get it wrong.
You are supposed to provide get_brightness only when you read back a
value from the HW, which is not the case here so just drop it is the
right choice.

	Sam

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

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Sam Ravnborg]

Hi Paul,

On Wed, Jul 05, 2023 at 04:38:05PM +0200, Paul Cercueil wrote:
> Hi Neil,
> 
> Le mercredi 05 juillet 2023 à 15:57 +0200, Neil Armstrong a écrit :
> > On 03/07/2023 23:47, Paul Cercueil wrote:
> > > Register a backlight device to be able to switch between all the
> > > gamma
> > > levels.
> > > 
> > > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > > ---
> > >   drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40
> > > ++++++++++++++++++++
> > >   1 file changed, 40 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > index 7fd9444b42c5..b4f87d6244cb 100644
> > > --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> > > @@ -8,6 +8,7 @@
> > >    * Andrzej Hajda <a.hajda@samsung.com>
> > >   */
> > >   
> > > +#include <linux/backlight.h>
> > >   #include <linux/delay.h>
> > >   #include <linux/gpio/consumer.h>
> > >   #include <linux/module.h>
> > > @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
> > >         return 0;
> > >   }
> > >   
> > > +static int ld9040_bl_update_status(struct backlight_device *dev)
> > > +{
> > > +       struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
> > > +
> > > +       ctx->brightness = dev->props.brightness;
Use backlight_get_brightness(dev);

> > > +       ld9040_brightness_set(ctx);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int ld9040_bl_get_intensity(struct backlight_device *dev)
> > > +{
> > > +       if (dev->props.fb_blank == FB_BLANK_UNBLANK &&
> > > +                       dev->props.power == FB_BLANK_UNBLANK)
> > > +               return dev->props.brightness;
> > > +
> > > +       return 0;
> > > +}
> > 
> > You can totally drop the _get_brightness.
> 
> The current behaviour is to return 0 when the framebuffer is blanked. A
> few drivers do that so I thought it was the norm; and the backlight
> core doesn't do that by default (and just uses dev->props.brightness).
> 
> It is not clear to me if that's the preferred behaviour. The
> "backlight_get_brightness" function in backlight.h seems to suggest
> that the current behaviour is correct, unless it is not supposed to be
> used in the backlight_ops.get_brightness() callback. Then in that case
> some other drivers get it wrong too.
Several drivers get it wrong.
You are supposed to provide get_brightness only when you read back a
value from the HW, which is not the case here so just drop it is the
right choice.

	Sam

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Sam Ravnborg]

Hi Paul,

On Mon, Jul 03, 2023 at 11:47:14PM +0200, Paul Cercueil wrote:
> Register a backlight device to be able to switch between all the gamma
> levels.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 7fd9444b42c5..b4f87d6244cb 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -8,6 +8,7 @@
>   * Andrzej Hajda <a.hajda@samsung.com>
>  */
>  
> +#include <linux/backlight.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
> @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
>  	return 0;
>  }
>  
> +static int ld9040_bl_update_status(struct backlight_device *dev)
> +{
> +	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
There is also the helper bl_get_data() - that do the same.

	Sam

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Sam Ravnborg]

Hi Paul,

On Mon, Jul 03, 2023 at 11:47:14PM +0200, Paul Cercueil wrote:
> Register a backlight device to be able to switch between all the gamma
> levels.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 7fd9444b42c5..b4f87d6244cb 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -8,6 +8,7 @@
>   * Andrzej Hajda <a.hajda@samsung.com>
>  */
>  
> +#include <linux/backlight.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
> @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
>  	return 0;
>  }
>  
> +static int ld9040_bl_update_status(struct backlight_device *dev)
> +{
> +	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
There is also the helper bl_get_data() - that do the same.

	Sam

Re: [PATCH 2/3] drm/panel: ld9040: Register a backlight device

[Sam Ravnborg]

Hi Paul,

On Mon, Jul 03, 2023 at 11:47:14PM +0200, Paul Cercueil wrote:
> Register a backlight device to be able to switch between all the gamma
> levels.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/gpu/drm/panel/panel-samsung-ld9040.c | 40 ++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-ld9040.c b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> index 7fd9444b42c5..b4f87d6244cb 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-ld9040.c
> @@ -8,6 +8,7 @@
>   * Andrzej Hajda <a.hajda@samsung.com>
>  */
>  
> +#include <linux/backlight.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
> @@ -311,8 +312,40 @@ static int ld9040_parse_dt(struct ld9040 *ctx)
>  	return 0;
>  }
>  
> +static int ld9040_bl_update_status(struct backlight_device *dev)
> +{
> +	struct ld9040 *ctx = dev_get_drvdata(&dev->dev);
There is also the helper bl_get_data() - that do the same.

	Sam

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