[PATCH 0/4] drm/mediatek: Small mtk-dpi cleanups

[PATCH 0/4] drm/mediatek: Small mtk-dpi cleanups

[AngeloGioacchino Del Regno]

This is a small cleanup of the mtk-dpi driver, switching it to devm
variants where possible and where it made sense, and reducing lines
while retaining and improving human readability.

AngeloGioacchino Del Regno (4):
  drm/mediatek: mtk_dpi: Simplify with devm_drm_bridge_add()
  drm/mediatek: mtk_dpi: Simplify with dev_err_probe()
  drm/mediatek: mtk_dpi: Switch to devm_drm_of_get_bridge()
  drm/mediatek: mtk_dpi: Use devm_platform_get_and_ioremap_resource()

 drivers/gpu/drm/mediatek/mtk_dpi.c | 59 +++++++++++-------------------
 1 file changed, 21 insertions(+), 38 deletions(-)

-- 
2.40.0

[PATCH 1/4] drm/mediatek: mtk_dpi: Simplify with devm_drm_bridge_add()

[AngeloGioacchino Del Regno]

Change drm_bridge_add() to its devm variant to slightly simplify the
probe function.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_dpi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 948a53f1f4b3..2d5f3fc34f61 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -1090,11 +1090,12 @@ static int mtk_dpi_probe(struct platform_device *pdev)
 	dpi->bridge.of_node = dev->of_node;
 	dpi->bridge.type = DRM_MODE_CONNECTOR_DPI;
 
-	drm_bridge_add(&dpi->bridge);
+	ret = devm_drm_bridge_add(dev, &dpi->bridge);
+	if (ret)
+		return ret;
 
 	ret = component_add(dev, &mtk_dpi_component_ops);
 	if (ret) {
-		drm_bridge_remove(&dpi->bridge);
 		dev_err(dev, "Failed to add component: %d\n", ret);
 		return ret;
 	}
-- 
2.40.0

Re: [PATCH 1/4] drm/mediatek: mtk_dpi: Simplify with devm_drm_bridge_add()

[Fei Shao]

Hi Angelo,

On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Change drm_bridge_add() to its devm variant to slightly simplify the
> probe function.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dpi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 948a53f1f4b3..2d5f3fc34f61 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -1090,11 +1090,12 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>         dpi->bridge.of_node = dev->of_node;
>         dpi->bridge.type = DRM_MODE_CONNECTOR_DPI;
>
> -       drm_bridge_add(&dpi->bridge);
> +       ret = devm_drm_bridge_add(dev, &dpi->bridge);
> +       if (ret)
> +               return ret;

And also drop the drm_bridge_remove() call in mtk_dpi_remove()?

Regards,
Fei

>
>         ret = component_add(dev, &mtk_dpi_component_ops);
>         if (ret) {
> -               drm_bridge_remove(&dpi->bridge);
>                 dev_err(dev, "Failed to add component: %d\n", ret);
>                 return ret;
>         }
> --
> 2.40.0
>
>

Re: [PATCH 1/4] drm/mediatek: mtk_dpi: Simplify with devm_drm_bridge_add()

[AngeloGioacchino Del Regno]

Il 13/07/23 09:55, Fei Shao ha scritto:
> Hi Angelo,
> 
> On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Change drm_bridge_add() to its devm variant to slightly simplify the
>> probe function.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/gpu/drm/mediatek/mtk_dpi.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> index 948a53f1f4b3..2d5f3fc34f61 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> @@ -1090,11 +1090,12 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>>          dpi->bridge.of_node = dev->of_node;
>>          dpi->bridge.type = DRM_MODE_CONNECTOR_DPI;
>>
>> -       drm_bridge_add(&dpi->bridge);
>> +       ret = devm_drm_bridge_add(dev, &dpi->bridge);
>> +       if (ret)
>> +               return ret;
> 
> And also drop the drm_bridge_remove() call in mtk_dpi_remove()?
> 

Yes. V2 will drop that.

Cheers,
Angelo

> Regards,
> Fei
> 
>>
>>          ret = component_add(dev, &mtk_dpi_component_ops);
>>          if (ret) {
>> -               drm_bridge_remove(&dpi->bridge);
>>                  dev_err(dev, "Failed to add component: %d\n", ret);
>>                  return ret;
>>          }
>> --
>> 2.40.0
>>
>>
> 

[PATCH 2/4] drm/mediatek: mtk_dpi: Simplify with dev_err_probe()

[AngeloGioacchino Del Regno]

Use dev_err_probe() across the entire probe function of this driver
to shrink the size.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_dpi.c | 44 ++++++++++--------------------
 1 file changed, 14 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 2d5f3fc34f61..6be65ea21f8f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -1040,38 +1040,24 @@ static int mtk_dpi_probe(struct platform_device *pdev)
 	}
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	dpi->regs = devm_ioremap_resource(dev, mem);
-	if (IS_ERR(dpi->regs)) {
-		ret = PTR_ERR(dpi->regs);
-		dev_err(dev, "Failed to ioremap mem resource: %d\n", ret);
-		return ret;
-	}
+	if (IS_ERR(dpi->regs))
+		return dev_err_probe(dev, PTR_ERR(dpi->regs),
+				     "Failed to ioremap mem resource\n");
 
 	dpi->engine_clk = devm_clk_get(dev, "engine");
-	if (IS_ERR(dpi->engine_clk)) {
-		ret = PTR_ERR(dpi->engine_clk);
-		if (ret != -EPROBE_DEFER)
-			dev_err(dev, "Failed to get engine clock: %d\n", ret);
-
-		return ret;
-	}
+	if (IS_ERR(dpi->engine_clk))
+		return dev_err_probe(dev, PTR_ERR(dpi->engine_clk),
+				     "Failed to get engine clock\n");
 
 	dpi->pixel_clk = devm_clk_get(dev, "pixel");
-	if (IS_ERR(dpi->pixel_clk)) {
-		ret = PTR_ERR(dpi->pixel_clk);
-		if (ret != -EPROBE_DEFER)
-			dev_err(dev, "Failed to get pixel clock: %d\n", ret);
-
-		return ret;
-	}
+	if (IS_ERR(dpi->pixel_clk))
+		return dev_err_probe(dev, PTR_ERR(dpi->pixel_clk),
+				     "Failed to get pixel clock\n");
 
 	dpi->tvd_clk = devm_clk_get(dev, "pll");
-	if (IS_ERR(dpi->tvd_clk)) {
-		ret = PTR_ERR(dpi->tvd_clk);
-		if (ret != -EPROBE_DEFER)
-			dev_err(dev, "Failed to get tvdpll clock: %d\n", ret);
-
-		return ret;
-	}
+	if (IS_ERR(dpi->tvd_clk))
+		return dev_err_probe(dev, PTR_ERR(dpi->tvd_clk),
+				     "Failed to get tvdpll clock\n");
 
 	dpi->irq = platform_get_irq(pdev, 0);
 	if (dpi->irq <= 0)
@@ -1095,10 +1081,8 @@ static int mtk_dpi_probe(struct platform_device *pdev)
 		return ret;
 
 	ret = component_add(dev, &mtk_dpi_component_ops);
-	if (ret) {
-		dev_err(dev, "Failed to add component: %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add component.\n");
 
 	return 0;
 }
-- 
2.40.0

Re: [PATCH 2/4] drm/mediatek: mtk_dpi: Simplify with dev_err_probe()

[Fei Shao]

On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Use dev_err_probe() across the entire probe function of this driver
> to shrink the size.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Reviewed-by: Fei Shao <fshao@chromium.org>

[PATCH 3/4] drm/mediatek: mtk_dpi: Switch to devm_drm_of_get_bridge()

[AngeloGioacchino Del Regno]

Function drm_of_find_panel_or_bridge() is marked as deprecated: since
the usage of that in this driver exactly corresponds to the new function
devm_drm_of_get_bridge(), switch to it.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_dpi.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 6be65ea21f8f..9025111013d3 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -1063,10 +1063,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
 	if (dpi->irq <= 0)
 		return -EINVAL;
 
-	ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
-					  NULL, &dpi->next_bridge);
-	if (ret)
-		return ret;
+	dpi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
+	if (IS_ERR(dpi->next_bridge))
+		return PTR_ERR(dpi->next_bridge);
 
 	dev_info(dev, "Found bridge node: %pOF\n", dpi->next_bridge->of_node);
 
-- 
2.40.0

Re: [PATCH 3/4] drm/mediatek: mtk_dpi: Switch to devm_drm_of_get_bridge()

[Fei Shao]

Hi Angelo,

On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Function drm_of_find_panel_or_bridge() is marked as deprecated: since
> the usage of that in this driver exactly corresponds to the new function
> devm_drm_of_get_bridge(), switch to it.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dpi.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 6be65ea21f8f..9025111013d3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -1063,10 +1063,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>         if (dpi->irq <= 0)
>                 return -EINVAL;
>
> -       ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
> -                                         NULL, &dpi->next_bridge);
> -       if (ret)
> -               return ret;
> +       dpi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
> +       if (IS_ERR(dpi->next_bridge))
> +               return PTR_ERR(dpi->next_bridge);

The original code doesn't print any log so it's probably fine, but
given you're already at it, perhaps you want to also make use of
dev_err_probe() here?
devm_drm_of_get_bridge() can also pass -EPROBE_DEFER from its wrapped
drm_of_find_panel_or_bridge(). Furthermore, that will make the code
visually align with your previous patch.

But that's just optional, and since this patch works anyway,

Reviewed-by: Fei Shao <fshao@chromium.org>

>
>         dev_info(dev, "Found bridge node: %pOF\n", dpi->next_bridge->of_node);
>
> --
> 2.40.0
>
>

Re: [PATCH 3/4] drm/mediatek: mtk_dpi: Switch to devm_drm_of_get_bridge()

[AngeloGioacchino Del Regno]

Il 13/07/23 10:24, Fei Shao ha scritto:
> Hi Angelo,
> 
> On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Function drm_of_find_panel_or_bridge() is marked as deprecated: since
>> the usage of that in this driver exactly corresponds to the new function
>> devm_drm_of_get_bridge(), switch to it.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/gpu/drm/mediatek/mtk_dpi.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> index 6be65ea21f8f..9025111013d3 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> @@ -1063,10 +1063,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>>          if (dpi->irq <= 0)
>>                  return -EINVAL;
>>
>> -       ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
>> -                                         NULL, &dpi->next_bridge);
>> -       if (ret)
>> -               return ret;
>> +       dpi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
>> +       if (IS_ERR(dpi->next_bridge))
>> +               return PTR_ERR(dpi->next_bridge);
> 
> The original code doesn't print any log so it's probably fine, but
> given you're already at it, perhaps you want to also make use of
> dev_err_probe() here?
> devm_drm_of_get_bridge() can also pass -EPROBE_DEFER from its wrapped
> drm_of_find_panel_or_bridge(). Furthermore, that will make the code
> visually align with your previous patch.
> 
> But that's just optional, and since this patch works anyway,
> 
> Reviewed-by: Fei Shao <fshao@chromium.org>
> 

Thanks. Yeah it's a good idea to add an error print... since I will have
to anyway send a v2, I'll add it.

Cheers,
Angelo

>>
>>          dev_info(dev, "Found bridge node: %pOF\n", dpi->next_bridge->of_node);
>>
>> --
>> 2.40.0
>>
>>
> 

[PATCH 4/4] drm/mediatek: mtk_dpi: Use devm_platform_get_and_ioremap_resource()

[AngeloGioacchino Del Regno]

Instead of the open-coded platform_get_resource, devm_ioremap_resource
switch to devm_platform_get_and_ioremap_resource(), doing exactly the
same.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_dpi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 9025111013d3..45535dc7970f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -1038,8 +1038,7 @@ static int mtk_dpi_probe(struct platform_device *pdev)
 			dev_dbg(&pdev->dev, "Cannot find pinctrl active!\n");
 		}
 	}
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	dpi->regs = devm_ioremap_resource(dev, mem);
+	dpi->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &mem);
 	if (IS_ERR(dpi->regs))
 		return dev_err_probe(dev, PTR_ERR(dpi->regs),
 				     "Failed to ioremap mem resource\n");
-- 
2.40.0

Re: [PATCH 4/4] drm/mediatek: mtk_dpi: Use devm_platform_get_and_ioremap_resource()

[Fei Shao]

Hi Angelo,

On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Instead of the open-coded platform_get_resource, devm_ioremap_resource
> switch to devm_platform_get_and_ioremap_resource(), doing exactly the
> same.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dpi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 9025111013d3..45535dc7970f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -1038,8 +1038,7 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>                         dev_dbg(&pdev->dev, "Cannot find pinctrl active!\n");
>                 }
>         }
> -       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -       dpi->regs = devm_ioremap_resource(dev, mem);
> +       dpi->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &mem);

"mem" appears to be unused elsewhere, so I assume
devm_platform_ioremap_resource() would be a better fit.

Regards,
Fei

>         if (IS_ERR(dpi->regs))
>                 return dev_err_probe(dev, PTR_ERR(dpi->regs),
>                                      "Failed to ioremap mem resource\n");
> --
> 2.40.0
>
>

Re: [PATCH 4/4] drm/mediatek: mtk_dpi: Use devm_platform_get_and_ioremap_resource()

[AngeloGioacchino Del Regno]

Il 13/07/23 10:34, Fei Shao ha scritto:
> Hi Angelo,
> 
> On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Instead of the open-coded platform_get_resource, devm_ioremap_resource
>> switch to devm_platform_get_and_ioremap_resource(), doing exactly the
>> same.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/gpu/drm/mediatek/mtk_dpi.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> index 9025111013d3..45535dc7970f 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> @@ -1038,8 +1038,7 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>>                          dev_dbg(&pdev->dev, "Cannot find pinctrl active!\n");
>>                  }
>>          }
>> -       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -       dpi->regs = devm_ioremap_resource(dev, mem);
>> +       dpi->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &mem);
> 
> "mem" appears to be unused elsewhere, so I assume
> devm_platform_ioremap_resource() would be a better fit.
> 

That's right. I'll switch to devm_platform_ioremap_resource() in v2.

Cheers,
Angelo

> Regards,
> Fei
> 
>>          if (IS_ERR(dpi->regs))
>>                  return dev_err_probe(dev, PTR_ERR(dpi->regs),
>>                                       "Failed to ioremap mem resource\n");
>> --
>> 2.40.0
>>
>>
> 

Re: [PATCH 0/4] drm/mediatek: Small mtk-dpi cleanups

[AngeloGioacchino Del Regno]

Il 12/04/23 13:52, AngeloGioacchino Del Regno ha scritto:
> This is a small cleanup of the mtk-dpi driver, switching it to devm
> variants where possible and where it made sense, and reducing lines
> while retaining and improving human readability.
> 
> AngeloGioacchino Del Regno (4):
>    drm/mediatek: mtk_dpi: Simplify with devm_drm_bridge_add()
>    drm/mediatek: mtk_dpi: Simplify with dev_err_probe()
>    drm/mediatek: mtk_dpi: Switch to devm_drm_of_get_bridge()
>    drm/mediatek: mtk_dpi: Use devm_platform_get_and_ioremap_resource()
> 
>   drivers/gpu/drm/mediatek/mtk_dpi.c | 59 +++++++++++-------------------
>   1 file changed, 21 insertions(+), 38 deletions(-)
> 

Ping for a fully ready and well tested series that I've sent *3 months* ago.

Thanks,
Angelo