diff mbox series

[v1] drm/msm: use compatible string to find mdp node

Message ID 1633427071-19523-1-git-send-email-mkrishn@codeaurora.org (mailing list archive)
State Not Applicable
Headers show
Series [v1] drm/msm: use compatible string to find mdp node | expand

Commit Message

Krishna Manikandan Oct. 5, 2021, 9:44 a.m. UTC
In the current implementation, substring comparison
using device node name is used to find mdp node
during driver probe. Use compatible string instead
of node name to get mdp node from the parent mdss node.

Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
---
 drivers/gpu/drm/msm/msm_drv.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Stephen Boyd Oct. 5, 2021, 6:06 p.m. UTC | #1
Quoting Krishna Manikandan (2021-10-05 02:44:31)
> In the current implementation, substring comparison
> using device node name is used to find mdp node
> during driver probe. Use compatible string instead
> of node name to get mdp node from the parent mdss node.
>
> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/msm_drv.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 2e6fc18..50a23cf 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -1241,9 +1241,16 @@ static int add_components_mdp(struct device *mdp_dev,
>         return 0;
>  }
>
> -static int compare_name_mdp(struct device *dev, void *data)
> +static int find_mdp_node(struct device *dev, void *data)
>  {
> -       return (strstr(dev_name(dev), "mdp") != NULL);
> +       return of_device_is_compatible(dev->of_node, "qcom,mdp4") ||

Why do we care about mdp4? It looks like this function is only called if
get_mdp_ver() returns 5 or DPU, in which case 4 isn't relevant?

> +               of_device_is_compatible(dev->of_node, "qcom,mdp5") ||
> +               of_device_is_compatible(dev->of_node, "qcom,mdss_mdp") ||
> +               of_device_is_compatible(dev->of_node, "qcom,sdm845-dpu") ||
> +               of_device_is_compatible(dev->of_node, "qcom,sm8150-dpu") ||
> +               of_device_is_compatible(dev->of_node, "qcom,sm8250-dpu") ||
> +               of_device_is_compatible(dev->of_node, "qcom,sc7180-dpu") ||
> +               of_device_is_compatible(dev->of_node, "qcom,sc7280-dpu");

Instead of this duplicate string check why not use canonical compatible
lists?

	return of_match_node(dpu_dt_match, dev->of_node) ||
	       of_match_node(mdp5_dt_match, dev->of_node);


This way we're not constantly updating this list of compatibles in two
places.

>  }
>
>  static int add_display_components(struct platform_device *pdev,
> @@ -1268,7 +1275,7 @@ static int add_display_components(struct platform_device *pdev,
>                         return ret;
>                 }
>
> -               mdp_dev = device_find_child(dev, NULL, compare_name_mdp);
> +               mdp_dev = device_find_child(dev, NULL, find_mdp_node);
>                 if (!mdp_dev) {
>                         DRM_DEV_ERROR(dev, "failed to find MDSS MDP node\n");
>                         of_platform_depopulate(dev);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 2e6fc18..50a23cf 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1241,9 +1241,16 @@  static int add_components_mdp(struct device *mdp_dev,
 	return 0;
 }
 
-static int compare_name_mdp(struct device *dev, void *data)
+static int find_mdp_node(struct device *dev, void *data)
 {
-	return (strstr(dev_name(dev), "mdp") != NULL);
+	return of_device_is_compatible(dev->of_node, "qcom,mdp4") ||
+		of_device_is_compatible(dev->of_node, "qcom,mdp5") ||
+		of_device_is_compatible(dev->of_node, "qcom,mdss_mdp") ||
+		of_device_is_compatible(dev->of_node, "qcom,sdm845-dpu") ||
+		of_device_is_compatible(dev->of_node, "qcom,sm8150-dpu") ||
+		of_device_is_compatible(dev->of_node, "qcom,sm8250-dpu") ||
+		of_device_is_compatible(dev->of_node, "qcom,sc7180-dpu") ||
+		of_device_is_compatible(dev->of_node, "qcom,sc7280-dpu");
 }
 
 static int add_display_components(struct platform_device *pdev,
@@ -1268,7 +1275,7 @@  static int add_display_components(struct platform_device *pdev,
 			return ret;
 		}
 
-		mdp_dev = device_find_child(dev, NULL, compare_name_mdp);
+		mdp_dev = device_find_child(dev, NULL, find_mdp_node);
 		if (!mdp_dev) {
 			DRM_DEV_ERROR(dev, "failed to find MDSS MDP node\n");
 			of_platform_depopulate(dev);