mbox series

[v6,0/9] MediaTek DRM - DSI driver cleanups

Message ID 20240215085316.56835-1-angelogioacchino.delregno@collabora.com (mailing list archive)
Headers show
Series MediaTek DRM - DSI driver cleanups | expand

Message

AngeloGioacchino Del Regno Feb. 15, 2024, 8:53 a.m. UTC
Changes in v6:
 - Moved default case from patch [4/9] to patch [3/9]

Changes in v5:
 - Changed patch 1 to not fix the DSI_PS_SEL mask for newer SoCs

Changes in v4:
 - Added a fix for RGB666 formats setting and for wrong register
   definitions for packed vs loosely packed formats
 - Added a commit to make use of mipi_dsi_pixel_format_to_bpp() helper
   instead of open coding the same

Changes in v3:
 - Rebased over next-20240131
 - Added bitfield.h inclusion in patch 3
 - Added three more cleanup commits to the mix to simplify
   the probe function and remove gotos.

Changes in v2:
 - Rebased over next-20231213

This series performs some cleanups for mtk_dsi, enhancing human
readability, using kernel provided macros where possible and
also reducing code size.

Tested on MT8173 and MT8192 Chromebooks (using a DSI<->DP bridge)
and on MT6795 Sony Xperia M5 (DSI video mode panel).


AngeloGioacchino Del Regno (9):
  drm/mediatek: dsi: Use GENMASK() for register mask definitions
  drm/mediatek: dsi: Fix DSI RGB666 formats and definitions
  drm/mediatek: dsi: Cleanup functions mtk_dsi_ps_control{_vact}()
  drm/mediatek: dsi: Use bitfield macros where useful
  drm/mediatek: dsi: Replace open-coded instance of HZ_PER_MHZ
  drm/mediatek: dsi: Register DSI host after acquiring clocks and PHY
  drm/mediatek: dsi: Simplify with dev_err_probe and remove gotos
  drm/mediatek: dsi: Compress of_device_id entries and add sentinel
  drm/mediatek: dsi: Use mipi_dsi_pixel_format_to_bpp() helper function

 drivers/gpu/drm/mediatek/mtk_dsi.c | 310 ++++++++++++-----------------
 1 file changed, 126 insertions(+), 184 deletions(-)

Comments

CK Hu (胡俊光) Feb. 15, 2024, 9:17 a.m. UTC | #1
Hi, Angelo:

On Thu, 2024-02-15 at 09:53 +0100, AngeloGioacchino Del Regno wrote:
> Most of the functions that are called in the probe callback are
> devm managed, or all but mipi_dsi_host_register(): simplify the probe
> function's error paths with dev_err_probe() and remove the lonely
> instance of `goto err_unregister_host` by just directly calling the
> mipi_dsi_host_unregister() function in the devm_request_irq() error
> path, allowing to also remove the same label.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 60 +++++++++-------------------
> --
>  1 file changed, 18 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
> b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index b3dd6251d611..195ff4dfc3a3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1117,64 +1117,44 @@ static int mtk_dsi_probe(struct
> platform_device *pdev)
>  	dsi->driver_data = of_device_get_match_data(dev);
>  
>  	dsi->engine_clk = devm_clk_get(dev, "engine");
> -	if (IS_ERR(dsi->engine_clk)) {
> -		ret = PTR_ERR(dsi->engine_clk);
> +	if (IS_ERR(dsi->engine_clk))
> +		return dev_err_probe(dev, PTR_ERR(dsi->engine_clk),
> +				     "Failed to get engine clock\n");
>  
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(dev, "Failed to get engine clock:
> %d\n", ret);
> -		return ret;
> -	}
>  
>  	dsi->digital_clk = devm_clk_get(dev, "digital");
> -	if (IS_ERR(dsi->digital_clk)) {
> -		ret = PTR_ERR(dsi->digital_clk);
> -
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(dev, "Failed to get digital clock:
> %d\n", ret);
> -		return ret;
> -	}
> +	if (IS_ERR(dsi->digital_clk))
> +		return dev_err_probe(dev, PTR_ERR(dsi->digital_clk),
> +				     "Failed to get digital clock\n");
>  
>  	dsi->hs_clk = devm_clk_get(dev, "hs");
> -	if (IS_ERR(dsi->hs_clk)) {
> -		ret = PTR_ERR(dsi->hs_clk);
> -		dev_err(dev, "Failed to get hs clock: %d\n", ret);
> -		return ret;
> -	}
> +	if (IS_ERR(dsi->hs_clk))
> +		return dev_err_probe(dev, PTR_ERR(dsi->hs_clk), "Failed
> to get hs clock\n");
>  
>  	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	dsi->regs = devm_ioremap_resource(dev, regs);
> -	if (IS_ERR(dsi->regs)) {
> -		ret = PTR_ERR(dsi->regs);
> -		dev_err(dev, "Failed to ioremap memory: %d\n", ret);
> -		return ret;
> -	}
> +	if (IS_ERR(dsi->regs))
> +		return dev_err_probe(dev, PTR_ERR(dsi->regs), "Failed
> to ioremap memory\n");
>  
>  	dsi->phy = devm_phy_get(dev, "dphy");
> -	if (IS_ERR(dsi->phy)) {
> -		ret = PTR_ERR(dsi->phy);
> -		dev_err(dev, "Failed to get MIPI-DPHY: %d\n", ret);
> -		return ret;
> -	}
> +	if (IS_ERR(dsi->phy))
> +		return dev_err_probe(dev, PTR_ERR(dsi->phy), "Failed to
> get MIPI-DPHY\n");
>  
>  	irq_num = platform_get_irq(pdev, 0);
> -	if (irq_num < 0) {
> -		ret = irq_num;
> -		return ret;
> -	}
> +	if (irq_num < 0)
> +		return irq_num;
>  
>  	dsi->host.ops = &mtk_dsi_ops;
>  	dsi->host.dev = dev;
>  	ret = mipi_dsi_host_register(&dsi->host);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to register DSI host: %d\n", ret);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to register DSI
> host\n");
>  
>  	ret = devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq,
>  			       IRQF_TRIGGER_NONE, dev_name(&pdev->dev), 
> dsi);
>  	if (ret) {
> -		dev_err(&pdev->dev, "failed to request mediatek dsi
> irq\n");
> -		goto err_unregister_host;
> +		mipi_dsi_host_unregister(&dsi->host);
> +		return dev_err_probe(&pdev->dev, ret, "Failed to
> request DSI irq\n");
>  	}
>  
>  	init_waitqueue_head(&dsi->irq_wait_queue);
> @@ -1186,10 +1166,6 @@ static int mtk_dsi_probe(struct
> platform_device *pdev)
>  	dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
>  
>  	return 0;
> -
> -err_unregister_host:
> -	mipi_dsi_host_unregister(&dsi->host);
> -	return ret;
>  }
>  
>  static void mtk_dsi_remove(struct platform_device *pdev)