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

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

[Dmitry Baryshkov]

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 <dmitry.baryshkov@linaro.org>
---
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 <dmitry.baryshkov@linaro.org>

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

[Dmitry Baryshkov]

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 <dmitry.baryshkov@linaro.org>
---
 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

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

[Jessica Zhang]

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 <quic_jesszhan@quicinc.com>

Thanks,

Jessica Zhang

> 
> Fixes: c7f66d32dd43 ("drm/panel: add support for rm69299 visionox panel")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   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
> 

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

[Dmitry Baryshkov]

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 <dmitry.baryshkov@linaro.org>
---
 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

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

[Jessica Zhang]

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 <dmitry.baryshkov@linaro.org>

Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>

> ---
>   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
> 

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

[Dmitry Baryshkov]

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 <dmitry.baryshkov@linaro.org>
---
 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

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

[Neil Armstrong]

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 <dmitry.baryshkov@linaro.org>
> ---
>   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 <neil.armstrong@linaro.org>

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

[Dmitry Baryshkov]

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

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 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

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

[Jessica Zhang]

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 <quic_jesszhan@quicinc.com>

Thanks,

Jessica Zhang

> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   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
> 

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

[Dmitry Baryshkov]

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 <quic_jesszhan@quicinc.com>

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

-- 
With best wishes
Dmitry

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

[Dmitry Baryshkov]

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

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 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

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

[Neil Armstrong]

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 <dmitry.baryshkov@linaro.org>
> ---
>   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 <neil.armstrong@linaro.org>

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

[Doug Anderson]

Hi,

On Thu, Apr 4, 2024 at 3:08 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Use .init_load_uA part of the bulk regulator API instead of calling
> register_set_load() manually.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  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

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

[Dmitry Baryshkov]

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
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > Use .init_load_uA part of the bulk regulator API instead of calling
> > register_set_load() manually.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  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

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

[Dmitry Baryshkov]

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

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 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

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

[Neil Armstrong]

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 <dmitry.baryshkov@linaro.org>
> ---
>   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 <neil.armstrong@linaro.org>

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

[Dmitry Baryshkov]

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

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

[Neil Armstrong]

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