2024-04-04 10:08:18

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH 0/6] drm/panel: small fixes for visionox and novatek panel drivers

While taking a glance at the novatek-nt36672e driver I stumbled upon a
call to unregister the DSI device for the panel, although it was not the
panel driver that registered the device.

Remove this call and a similar call from the visionox-rm69299 driver.
While we are at it, also optimize regulator API calls in these two
drivers and in the novatek-nt36672a driver.

Signed-off-by: Dmitry Baryshkov <[email protected]>
---
Dmitry Baryshkov (6):
drm/panel: visionox-rm69299: don't unregister DSI device
drm/panel: novatek-nt36682e: don't unregister DSI device
drm/panel: novatek-nt36672e: stop setting register load before disable
drm/panel: novatek-nt36672e: stop calling regulator_set_load manually
drm/panel: novatek-nt36672a: stop calling regulator_set_load manually
drm/panel: visionox-rm69299: stop calling regulator_set_load manually

drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 11 +++-----
drivers/gpu/drm/panel/panel-novatek-nt36672e.c | 35 +++-----------------------
drivers/gpu/drm/panel/panel-visionox-rm69299.c | 18 ++-----------
3 files changed, 9 insertions(+), 55 deletions(-)
---
base-commit: a6bd6c9333397f5a0e2667d4d82fef8c970108f2
change-id: 20240404-drop-panel-unregister-744a9fd41cc6

Best regards,
--
Dmitry Baryshkov <[email protected]>



2024-04-04 10:09:21

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH 3/6] drm/panel: novatek-nt36672e: stop setting register load before disable

It is pointless to set register load before disabling the register. This
vote is going to be dropped as soon as the register is disabled. Drop
these register_set_load calls.

Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/panel/panel-novatek-nt36672e.c | 17 -----------------
1 file changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
index c39fe0fc5d69..9a870b9b6765 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
@@ -25,12 +25,6 @@ static const unsigned long regulator_enable_loads[] = {
100000,
};

-static const unsigned long regulator_disable_loads[] = {
- 80,
- 100,
- 100,
-};
-
struct panel_desc {
const struct drm_display_mode *display_mode;
u32 width_mm;
@@ -385,20 +379,9 @@ static int nt36672e_power_off(struct nt36672e_panel *ctx)
{
struct mipi_dsi_device *dsi = ctx->dsi;
int ret = 0;
- int i;

gpiod_set_value(ctx->reset_gpio, 0);

- for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
- ret = regulator_set_load(ctx->supplies[i].consumer,
- regulator_disable_loads[i]);
- if (ret) {
- dev_err(&dsi->dev, "regulator set load failed for supply %s: %d\n",
- ctx->supplies[i].supply, ret);
- return ret;
- }
- }
-
ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
if (ret)
dev_err(&dsi->dev, "regulator bulk disable failed: %d\n", ret);

--
2.39.2


2024-04-04 10:09:23

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH 4/6] drm/panel: novatek-nt36672e: stop calling regulator_set_load manually

Use .init_load_uA part of the bulk regulator API instead of calling
register_set_load() manually.

Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/panel/panel-novatek-nt36672e.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
index 9a870b9b6765..20b7bfe4aa12 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
@@ -343,17 +343,7 @@ static int nt36672e_1080x2408_60hz_init(struct mipi_dsi_device *dsi)
static int nt36672e_power_on(struct nt36672e_panel *ctx)
{
struct mipi_dsi_device *dsi = ctx->dsi;
- int ret, i;
-
- for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
- ret = regulator_set_load(ctx->supplies[i].consumer,
- regulator_enable_loads[i]);
- if (ret) {
- dev_err(&dsi->dev, "regulator set load failed for supply %s: %d\n",
- ctx->supplies[i].supply, ret);
- return ret;
- }
- }
+ int ret;

ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
if (ret < 0) {
@@ -550,8 +540,10 @@ static int nt36672e_panel_probe(struct mipi_dsi_device *dsi)
return -ENODEV;
}

- for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++)
+ for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
ctx->supplies[i].supply = regulator_names[i];
+ ctx->supplies[i].init_load_uA = regulator_enable_loads[i];
+ }

ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
ctx->supplies);

--
2.39.2


2024-04-04 10:09:49

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH 6/6] drm/panel: visionox-rm69299: stop calling regulator_set_load manually

Use .init_load_uA part of the bulk regulator API instead of calling
register_set_load() manually.

Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/panel/panel-visionox-rm69299.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-visionox-rm69299.c b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
index b15ca56a09a7..272490b9565b 100644
--- a/drivers/gpu/drm/panel/panel-visionox-rm69299.c
+++ b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
@@ -197,7 +197,9 @@ static int visionox_rm69299_probe(struct mipi_dsi_device *dsi)
ctx->dsi = dsi;

ctx->supplies[0].supply = "vdda";
+ ctx->supplies[0].init_load_uA = 32000;
ctx->supplies[1].supply = "vdd3p3";
+ ctx->supplies[1].init_load_uA = 13200;

ret = devm_regulator_bulk_get(ctx->panel.dev, ARRAY_SIZE(ctx->supplies),
ctx->supplies);
@@ -227,22 +229,8 @@ static int visionox_rm69299_probe(struct mipi_dsi_device *dsi)
goto err_dsi_attach;
}

- ret = regulator_set_load(ctx->supplies[0].consumer, 32000);
- if (ret) {
- dev_err(dev, "regulator set load failed for vdda supply ret = %d\n", ret);
- goto err_set_load;
- }
-
- ret = regulator_set_load(ctx->supplies[1].consumer, 13200);
- if (ret) {
- dev_err(dev, "regulator set load failed for vdd3p3 supply ret = %d\n", ret);
- goto err_set_load;
- }
-
return 0;

-err_set_load:
- mipi_dsi_detach(dsi);
err_dsi_attach:
drm_panel_remove(&ctx->panel);
return ret;

--
2.39.2


2024-04-04 10:09:50

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH 5/6] drm/panel: novatek-nt36672a: stop calling regulator_set_load manually

Use .init_load_uA part of the bulk regulator API instead of calling
register_set_load() manually.

Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
index 33fb3d715e54..3886372415c2 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
@@ -605,21 +605,16 @@ static int nt36672a_panel_add(struct nt36672a_panel *pinfo)
struct device *dev = &pinfo->link->dev;
int i, ret;

- for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++)
+ for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++) {
pinfo->supplies[i].supply = nt36672a_regulator_names[i];
+ pinfo->supplies[i].init_load_uA = nt36672a_regulator_enable_loads[i];
+ }

ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(pinfo->supplies),
pinfo->supplies);
if (ret < 0)
return dev_err_probe(dev, ret, "failed to get regulators\n");

- for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++) {
- ret = regulator_set_load(pinfo->supplies[i].consumer,
- nt36672a_regulator_enable_loads[i]);
- if (ret)
- return dev_err_probe(dev, ret, "failed to set regulator enable loads\n");
- }
-
pinfo->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(pinfo->reset_gpio))
return dev_err_probe(dev, PTR_ERR(pinfo->reset_gpio),

--
2.39.2


2024-04-04 10:20:23

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH 1/6] drm/panel: visionox-rm69299: don't unregister DSI device

The DSI device for the panel was registered by the DSI host, so it is an
error to unregister it from the panel driver. Drop the call to
mipi_dsi_device_unregister().

Fixes: c7f66d32dd43 ("drm/panel: add support for rm69299 visionox panel")
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/panel/panel-visionox-rm69299.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-visionox-rm69299.c b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
index 775144695283..b15ca56a09a7 100644
--- a/drivers/gpu/drm/panel/panel-visionox-rm69299.c
+++ b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
@@ -253,8 +253,6 @@ static void visionox_rm69299_remove(struct mipi_dsi_device *dsi)
struct visionox_rm69299 *ctx = mipi_dsi_get_drvdata(dsi);

mipi_dsi_detach(ctx->dsi);
- mipi_dsi_device_unregister(ctx->dsi);
-
drm_panel_remove(&ctx->panel);
}


--
2.39.2


2024-04-04 10:20:25

by Dmitry Baryshkov

[permalink] [raw]
Subject: [PATCH 2/6] drm/panel: novatek-nt36682e: don't unregister DSI device

The DSI device for the panel was registered by the DSI host, so it is an
error to unregister it from the panel driver. Drop the call to
mipi_dsi_device_unregister().

Fixes: ea4f9975625a ("drm/panel: Add support for Novatek NT36672E panel driver")
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/panel/panel-novatek-nt36672e.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
index cb7406d74466..c39fe0fc5d69 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
@@ -614,8 +614,6 @@ static void nt36672e_panel_remove(struct mipi_dsi_device *dsi)
struct nt36672e_panel *ctx = mipi_dsi_get_drvdata(dsi);

mipi_dsi_detach(ctx->dsi);
- mipi_dsi_device_unregister(ctx->dsi);
-
drm_panel_remove(&ctx->panel);
}


--
2.39.2


2024-04-05 21:43:23

by Jessica Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/6] drm/panel: visionox-rm69299: don't unregister DSI device



On 4/4/2024 3:07 AM, Dmitry Baryshkov wrote:
> The DSI device for the panel was registered by the DSI host, so it is an
> error to unregister it from the panel driver. Drop the call to
> mipi_dsi_device_unregister().

Hi Dmitry,

Reviewed-by: Jessica Zhang <[email protected]>

Thanks,

Jessica Zhang

>
> Fixes: c7f66d32dd43 ("drm/panel: add support for rm69299 visionox panel")
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/gpu/drm/panel/panel-visionox-rm69299.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-visionox-rm69299.c b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
> index 775144695283..b15ca56a09a7 100644
> --- a/drivers/gpu/drm/panel/panel-visionox-rm69299.c
> +++ b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
> @@ -253,8 +253,6 @@ static void visionox_rm69299_remove(struct mipi_dsi_device *dsi)
> struct visionox_rm69299 *ctx = mipi_dsi_get_drvdata(dsi);
>
> mipi_dsi_detach(ctx->dsi);
> - mipi_dsi_device_unregister(ctx->dsi);
> -
> drm_panel_remove(&ctx->panel);
> }
>
>
> --
> 2.39.2
>

2024-04-05 21:44:34

by Jessica Zhang

[permalink] [raw]
Subject: Re: [PATCH 2/6] drm/panel: novatek-nt36682e: don't unregister DSI device



On 4/4/2024 3:08 AM, Dmitry Baryshkov wrote:
> The DSI device for the panel was registered by the DSI host, so it is an
> error to unregister it from the panel driver. Drop the call to
> mipi_dsi_device_unregister().
>
> Fixes: ea4f9975625a ("drm/panel: Add support for Novatek NT36672E panel driver")
> Signed-off-by: Dmitry Baryshkov <[email protected]>

Reviewed-by: Jessica Zhang <[email protected]>

> ---
> drivers/gpu/drm/panel/panel-novatek-nt36672e.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
> index cb7406d74466..c39fe0fc5d69 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
> @@ -614,8 +614,6 @@ static void nt36672e_panel_remove(struct mipi_dsi_device *dsi)
> struct nt36672e_panel *ctx = mipi_dsi_get_drvdata(dsi);
>
> mipi_dsi_detach(ctx->dsi);
> - mipi_dsi_device_unregister(ctx->dsi);
> -
> drm_panel_remove(&ctx->panel);
> }
>
>
> --
> 2.39.2
>

2024-04-16 20:33:41

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: (subset) [PATCH 0/6] drm/panel: small fixes for visionox and novatek panel drivers

On Thu, 04 Apr 2024 13:07:58 +0300, Dmitry Baryshkov wrote:
> While taking a glance at the novatek-nt36672e driver I stumbled upon a
> call to unregister the DSI device for the panel, although it was not the
> panel driver that registered the device.
>
> Remove this call and a similar call from the visionox-rm69299 driver.
> While we are at it, also optimize regulator API calls in these two
> drivers and in the novatek-nt36672a driver.
>
> [...]

Applied to drm-misc-fixes, thanks!

[1/6] drm/panel: visionox-rm69299: don't unregister DSI device
commit: 9e4d3f4f34455abbaa9930bf6b7575a5cd081496
[2/6] drm/panel: novatek-nt36682e: don't unregister DSI device
commit: 941c0bdbc176df825adf77052263b2d63db6fef7

Other four patches were not reviewed and they are not fixes, so they are still
pending.

Best regards,
--
With best wishes
Dmitry


2024-04-17 21:21:20

by Jessica Zhang

[permalink] [raw]
Subject: Re: [PATCH 4/6] drm/panel: novatek-nt36672e: stop calling regulator_set_load manually



On 4/4/2024 3:08 AM, Dmitry Baryshkov wrote:
> Use .init_load_uA part of the bulk regulator API instead of calling
> register_set_load() manually.

Hi Dmitry,

Reviewed-by: Jessica Zhang <[email protected]>

Thanks,

Jessica Zhang

>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/gpu/drm/panel/panel-novatek-nt36672e.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
> index 9a870b9b6765..20b7bfe4aa12 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
> @@ -343,17 +343,7 @@ static int nt36672e_1080x2408_60hz_init(struct mipi_dsi_device *dsi)
> static int nt36672e_power_on(struct nt36672e_panel *ctx)
> {
> struct mipi_dsi_device *dsi = ctx->dsi;
> - int ret, i;
> -
> - for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> - ret = regulator_set_load(ctx->supplies[i].consumer,
> - regulator_enable_loads[i]);
> - if (ret) {
> - dev_err(&dsi->dev, "regulator set load failed for supply %s: %d\n",
> - ctx->supplies[i].supply, ret);
> - return ret;
> - }
> - }
> + int ret;
>
> ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> if (ret < 0) {
> @@ -550,8 +540,10 @@ static int nt36672e_panel_probe(struct mipi_dsi_device *dsi)
> return -ENODEV;
> }
>
> - for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++)
> + for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> ctx->supplies[i].supply = regulator_names[i];
> + ctx->supplies[i].init_load_uA = regulator_enable_loads[i];
> + }
>
> ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
> ctx->supplies);
>
> --
> 2.39.2
>

2024-04-18 00:04:47

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 4/6] drm/panel: novatek-nt36672e: stop calling regulator_set_load manually

On Wed, Apr 17, 2024 at 02:20:31PM -0700, Jessica Zhang wrote:
>
>
> On 4/4/2024 3:08 AM, Dmitry Baryshkov wrote:
> > Use .init_load_uA part of the bulk regulator API instead of calling
> > register_set_load() manually.
>
> Hi Dmitry,
>
> Reviewed-by: Jessica Zhang <[email protected]>

I wonder why patch 4 was reviewed, but patch 3 was not.

--
With best wishes
Dmitry

2024-04-24 06:52:43

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH 3/6] drm/panel: novatek-nt36672e: stop setting register load before disable

On 04/04/2024 12:08, Dmitry Baryshkov wrote:
> It is pointless to set register load before disabling the register. This
> vote is going to be dropped as soon as the register is disabled. Drop
> these register_set_load calls.
>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/gpu/drm/panel/panel-novatek-nt36672e.c | 17 -----------------
> 1 file changed, 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
> index c39fe0fc5d69..9a870b9b6765 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
> @@ -25,12 +25,6 @@ static const unsigned long regulator_enable_loads[] = {
> 100000,
> };
>
> -static const unsigned long regulator_disable_loads[] = {
> - 80,
> - 100,
> - 100,
> -};
> -
> struct panel_desc {
> const struct drm_display_mode *display_mode;
> u32 width_mm;
> @@ -385,20 +379,9 @@ static int nt36672e_power_off(struct nt36672e_panel *ctx)
> {
> struct mipi_dsi_device *dsi = ctx->dsi;
> int ret = 0;
> - int i;
>
> gpiod_set_value(ctx->reset_gpio, 0);
>
> - for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> - ret = regulator_set_load(ctx->supplies[i].consumer,
> - regulator_disable_loads[i]);
> - if (ret) {
> - dev_err(&dsi->dev, "regulator set load failed for supply %s: %d\n",
> - ctx->supplies[i].supply, ret);
> - return ret;
> - }
> - }
> -
> ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> if (ret)
> dev_err(&dsi->dev, "regulator bulk disable failed: %d\n", ret);
>

Reviewed-by: Neil Armstrong <[email protected]>

2024-04-24 06:53:36

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH 5/6] drm/panel: novatek-nt36672a: stop calling regulator_set_load manually

On 04/04/2024 12:08, Dmitry Baryshkov wrote:
> Use .init_load_uA part of the bulk regulator API instead of calling
> register_set_load() manually.
>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> index 33fb3d715e54..3886372415c2 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> @@ -605,21 +605,16 @@ static int nt36672a_panel_add(struct nt36672a_panel *pinfo)
> struct device *dev = &pinfo->link->dev;
> int i, ret;
>
> - for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++)
> + for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++) {
> pinfo->supplies[i].supply = nt36672a_regulator_names[i];
> + pinfo->supplies[i].init_load_uA = nt36672a_regulator_enable_loads[i];
> + }
>
> ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(pinfo->supplies),
> pinfo->supplies);
> if (ret < 0)
> return dev_err_probe(dev, ret, "failed to get regulators\n");
>
> - for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++) {
> - ret = regulator_set_load(pinfo->supplies[i].consumer,
> - nt36672a_regulator_enable_loads[i]);
> - if (ret)
> - return dev_err_probe(dev, ret, "failed to set regulator enable loads\n");
> - }
> -
> pinfo->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> if (IS_ERR(pinfo->reset_gpio))
> return dev_err_probe(dev, PTR_ERR(pinfo->reset_gpio),
>

Reviewed-by: Neil Armstrong <[email protected]>

2024-04-24 06:54:22

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH 6/6] drm/panel: visionox-rm69299: stop calling regulator_set_load manually

On 04/04/2024 12:08, Dmitry Baryshkov wrote:
> Use .init_load_uA part of the bulk regulator API instead of calling
> register_set_load() manually.
>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/gpu/drm/panel/panel-visionox-rm69299.c | 16 ++--------------
> 1 file changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-visionox-rm69299.c b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
> index b15ca56a09a7..272490b9565b 100644
> --- a/drivers/gpu/drm/panel/panel-visionox-rm69299.c
> +++ b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
> @@ -197,7 +197,9 @@ static int visionox_rm69299_probe(struct mipi_dsi_device *dsi)
> ctx->dsi = dsi;
>
> ctx->supplies[0].supply = "vdda";
> + ctx->supplies[0].init_load_uA = 32000;
> ctx->supplies[1].supply = "vdd3p3";
> + ctx->supplies[1].init_load_uA = 13200;
>
> ret = devm_regulator_bulk_get(ctx->panel.dev, ARRAY_SIZE(ctx->supplies),
> ctx->supplies);
> @@ -227,22 +229,8 @@ static int visionox_rm69299_probe(struct mipi_dsi_device *dsi)
> goto err_dsi_attach;
> }
>
> - ret = regulator_set_load(ctx->supplies[0].consumer, 32000);
> - if (ret) {
> - dev_err(dev, "regulator set load failed for vdda supply ret = %d\n", ret);
> - goto err_set_load;
> - }
> -
> - ret = regulator_set_load(ctx->supplies[1].consumer, 13200);
> - if (ret) {
> - dev_err(dev, "regulator set load failed for vdd3p3 supply ret = %d\n", ret);
> - goto err_set_load;
> - }
> -
> return 0;
>
> -err_set_load:
> - mipi_dsi_detach(dsi);
> err_dsi_attach:
> drm_panel_remove(&ctx->panel);
> return ret;
>

Reviewed-by: Neil Armstrong <[email protected]>

2024-04-24 07:01:08

by Neil Armstrong

[permalink] [raw]
Subject: Re: (subset) [PATCH 0/6] drm/panel: small fixes for visionox and novatek panel drivers

Hi,

On Thu, 04 Apr 2024 13:07:58 +0300, Dmitry Baryshkov wrote:
> While taking a glance at the novatek-nt36672e driver I stumbled upon a
> call to unregister the DSI device for the panel, although it was not the
> panel driver that registered the device.
>
> Remove this call and a similar call from the visionox-rm69299 driver.
> While we are at it, also optimize regulator API calls in these two
> drivers and in the novatek-nt36672a driver.
>
> [...]

Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-next)

[3/6] drm/panel: novatek-nt36672e: stop setting register load before disable
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/302aeb946731923c4ff7cca093868e4148ebc701
[4/6] drm/panel: novatek-nt36672e: stop calling regulator_set_load manually
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/9dab1af1e7592d9317bf3857e8cf019120973053
[5/6] drm/panel: novatek-nt36672a: stop calling regulator_set_load manually
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/a8ee5f50a9d87f35ca80d6ea74ac07ae97d5a21b
[6/6] drm/panel: visionox-rm69299: stop calling regulator_set_load manually
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/251e3c1fe15cb8bf71a834f863f6225b8164f7b8

--
Neil


2024-05-03 17:04:05

by Douglas Anderson

[permalink] [raw]
Subject: Re: [PATCH 5/6] drm/panel: novatek-nt36672a: stop calling regulator_set_load manually

Hi,

On Thu, Apr 4, 2024 at 3:08 AM Dmitry Baryshkov
<[email protected]> wrote:
>
> Use .init_load_uA part of the bulk regulator API instead of calling
> register_set_load() manually.
>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> index 33fb3d715e54..3886372415c2 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> @@ -605,21 +605,16 @@ static int nt36672a_panel_add(struct nt36672a_panel *pinfo)
> struct device *dev = &pinfo->link->dev;
> int i, ret;
>
> - for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++)
> + for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++) {
> pinfo->supplies[i].supply = nt36672a_regulator_names[i];
> + pinfo->supplies[i].init_load_uA = nt36672a_regulator_enable_loads[i];
> + }
>
> ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(pinfo->supplies),
> pinfo->supplies);
> if (ret < 0)
> return dev_err_probe(dev, ret, "failed to get regulators\n");
>
> - for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++) {
> - ret = regulator_set_load(pinfo->supplies[i].consumer,
> - nt36672a_regulator_enable_loads[i]);
> - if (ret)
> - return dev_err_probe(dev, ret, "failed to set regulator enable loads\n");
> - }

Thanks for the cleanup! I happened to notice this commit and I'm
curious why you didn't take the next step and move to
devm_regulator_bulk_get_const(). That would have allowed you to avoid
the "for" loop above that copies the data ;-)

-Doug

2024-05-03 20:53:34

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 5/6] drm/panel: novatek-nt36672a: stop calling regulator_set_load manually

On Fri, May 03, 2024 at 10:03:35AM -0700, Doug Anderson wrote:
> Hi,
>
> On Thu, Apr 4, 2024 at 3:08 AM Dmitry Baryshkov
> <[email protected]> wrote:
> >
> > Use .init_load_uA part of the bulk regulator API instead of calling
> > register_set_load() manually.
> >
> > Signed-off-by: Dmitry Baryshkov <[email protected]>
> > ---
> > drivers/gpu/drm/panel/panel-novatek-nt36672a.c | 11 +++--------
> > 1 file changed, 3 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> > index 33fb3d715e54..3886372415c2 100644
> > --- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> > +++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
> > @@ -605,21 +605,16 @@ static int nt36672a_panel_add(struct nt36672a_panel *pinfo)
> > struct device *dev = &pinfo->link->dev;
> > int i, ret;
> >
> > - for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++)
> > + for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++) {
> > pinfo->supplies[i].supply = nt36672a_regulator_names[i];
> > + pinfo->supplies[i].init_load_uA = nt36672a_regulator_enable_loads[i];
> > + }
> >
> > ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(pinfo->supplies),
> > pinfo->supplies);
> > if (ret < 0)
> > return dev_err_probe(dev, ret, "failed to get regulators\n");
> >
> > - for (i = 0; i < ARRAY_SIZE(pinfo->supplies); i++) {
> > - ret = regulator_set_load(pinfo->supplies[i].consumer,
> > - nt36672a_regulator_enable_loads[i]);
> > - if (ret)
> > - return dev_err_probe(dev, ret, "failed to set regulator enable loads\n");
> > - }
>
> Thanks for the cleanup! I happened to notice this commit and I'm
> curious why you didn't take the next step and move to
> devm_regulator_bulk_get_const(). That would have allowed you to avoid
> the "for" loop above that copies the data ;-)

I haven't noticed this function beforehand. Thanks for the pointer!

>
> -Doug

--
With best wishes
Dmitry