[v3,0/3] drm/msm: rework clock handling

msm_dss_clk_*() functions significantly duplicate clk_bulk_* family of
functions. Drop custom code and use bulk clocks directly. This also
removes dependency of DP driver on the DPU driver internals.

Prerequisites: [1]

Changes since v2:
 - Retain conditional code/prints in DP code to ease debugging
 - Rebase on top of msm-next and [1]
 - Split helper functions to msm_io_utils.c as suggested by Jessica

Changes since v1:
 - Rebase on top of current tree to fix conflicts

Dmitry Baryshkov (3):
  drm/msm: move utility functions from msm_drv.c
  drm/msm/dpu: simplify clocks handling
  drm/msm/dp: rewrite dss_module_power to use bulk clock functions

[1] https://patchwork.freedesktop.org/series/98521/

 drivers/gpu/drm/msm/Makefile                  |   2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c |  24 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |   6 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c   | 187 ------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h   |  40 ----
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |  46 ++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h       |   4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c      |  26 +--
 drivers/gpu/drm/msm/dp/dp_ctrl.c              |  19 +-
 drivers/gpu/drm/msm/dp/dp_parser.c            |  21 +-
 drivers/gpu/drm/msm/dp/dp_parser.h            |  17 +-
 drivers/gpu/drm/msm/dp/dp_power.c             |  82 +++++---
 drivers/gpu/drm/msm/msm_drv.c                 | 119 +----------
 drivers/gpu/drm/msm/msm_drv.h                 |   1 +
 drivers/gpu/drm/msm/msm_io_utils.c            | 176 +++++++++++++++++
 15 files changed, 305 insertions(+), 465 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h
 create mode 100644 drivers/gpu/drm/msm/msm_io_utils.c


base-commit: 6ed95285382d6f90a3c3a11d5806a5eb7db715c3
prerequisite-patch-id: b5572c353efa8ebefb32fc70e54bc537f78ff2bd
prerequisite-patch-id: 40a854aee69b6ddf5b1c33cac3647efae3e3ebf1
prerequisite-patch-id: 8b687a19047983e26262a1bb2feda8a9fd5bc97f

On 1/19/2022 2:16 PM, Dmitry Baryshkov wrote:
> Move clock/IO/hrtimer utility functions from msm_drv.c to new
> msm_io_utils.c file.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Tested on: Qualcomm RB3 (debian, sdm845), Qualcomm RB5 (debian, qrb5165)

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

> ---
>   drivers/gpu/drm/msm/Makefile       |   1 +
>   drivers/gpu/drm/msm/msm_drv.c      | 118 ---------------------------
>   drivers/gpu/drm/msm/msm_io_utils.c | 126 +++++++++++++++++++++++++++++
>   3 files changed, 127 insertions(+), 118 deletions(-)
>   create mode 100644 drivers/gpu/drm/msm/msm_io_utils.c
> 
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 03ab55c37beb..5b6e37477079 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -87,6 +87,7 @@ msm-y := \
>   	msm_gem_vma.o \
>   	msm_gpu.o \
>   	msm_gpu_devfreq.o \
> +	msm_io_utils.o \
>   	msm_iommu.o \
>   	msm_perf.o \
>   	msm_rd.o \
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index fd62a4da14a1..30c44c395a24 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -75,124 +75,6 @@ static bool modeset = true;
>   MODULE_PARM_DESC(modeset, "Use kernel modesetting [KMS] (1=on (default), 0=disable)");
>   module_param(modeset, bool, 0600);
>   
> -/*
> - * Util/helpers:
> - */
> -
> -struct clk *msm_clk_bulk_get_clock(struct clk_bulk_data *bulk, int count,
> -		const char *name)
> -{
> -	int i;
> -	char n[32];
> -
> -	snprintf(n, sizeof(n), "%s_clk", name);
> -
> -	for (i = 0; bulk && i < count; i++) {
> -		if (!strcmp(bulk[i].id, name) || !strcmp(bulk[i].id, n))
> -			return bulk[i].clk;
> -	}
> -
> -
> -	return NULL;
> -}
> -
> -struct clk *msm_clk_get(struct platform_device *pdev, const char *name)
> -{
> -	struct clk *clk;
> -	char name2[32];
> -
> -	clk = devm_clk_get(&pdev->dev, name);
> -	if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
> -		return clk;
> -
> -	snprintf(name2, sizeof(name2), "%s_clk", name);
> -
> -	clk = devm_clk_get(&pdev->dev, name2);
> -	if (!IS_ERR(clk))
> -		dev_warn(&pdev->dev, "Using legacy clk name binding.  Use "
> -				"\"%s\" instead of \"%s\"\n", name, name2);
> -
> -	return clk;
> -}
> -
> -static void __iomem *_msm_ioremap(struct platform_device *pdev, const char *name,
> -				  bool quiet, phys_addr_t *psize)
> -{
> -	struct resource *res;
> -	unsigned long size;
> -	void __iomem *ptr;
> -
> -	if (name)
> -		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
> -	else
> -		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -
> -	if (!res) {
> -		if (!quiet)
> -			DRM_DEV_ERROR(&pdev->dev, "failed to get memory resource: %s\n", name);
> -		return ERR_PTR(-EINVAL);
> -	}
> -
> -	size = resource_size(res);
> -
> -	ptr = devm_ioremap(&pdev->dev, res->start, size);
> -	if (!ptr) {
> -		if (!quiet)
> -			DRM_DEV_ERROR(&pdev->dev, "failed to ioremap: %s\n", name);
> -		return ERR_PTR(-ENOMEM);
> -	}
> -
> -	if (psize)
> -		*psize = size;
> -
> -	return ptr;
> -}
> -
> -void __iomem *msm_ioremap(struct platform_device *pdev, const char *name)
> -{
> -	return _msm_ioremap(pdev, name, false, NULL);
> -}
> -
> -void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name)
> -{
> -	return _msm_ioremap(pdev, name, true, NULL);
> -}
> -
> -void __iomem *msm_ioremap_size(struct platform_device *pdev, const char *name,
> -			  phys_addr_t *psize)
> -{
> -	return _msm_ioremap(pdev, name, false, psize);
> -}
> -
> -static enum hrtimer_restart msm_hrtimer_worktimer(struct hrtimer *t)
> -{
> -	struct msm_hrtimer_work *work = container_of(t,
> -			struct msm_hrtimer_work, timer);
> -
> -	kthread_queue_work(work->worker, &work->work);
> -
> -	return HRTIMER_NORESTART;
> -}
> -
> -void msm_hrtimer_queue_work(struct msm_hrtimer_work *work,
> -			    ktime_t wakeup_time,
> -			    enum hrtimer_mode mode)
> -{
> -	hrtimer_start(&work->timer, wakeup_time, mode);
> -}
> -
> -void msm_hrtimer_work_init(struct msm_hrtimer_work *work,
> -			   struct kthread_worker *worker,
> -			   kthread_work_func_t fn,
> -			   clockid_t clock_id,
> -			   enum hrtimer_mode mode)
> -{
> -	hrtimer_init(&work->timer, clock_id, mode);
> -	work->timer.function = msm_hrtimer_worktimer;
> -	work->worker = worker;
> -	kthread_init_work(&work->work, fn);
> -}
> -
>   static irqreturn_t msm_irq(int irq, void *arg)
>   {
>   	struct drm_device *dev = arg;
> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c
> new file mode 100644
> index 000000000000..7b504617833a
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/msm_io_utils.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2016-2018, 2020-2021 The Linux Foundation. All rights reserved.
> + * Copyright (C) 2013 Red Hat
> + * Author: Rob Clark <robdclark@gmail.com>
> + */
> +
> +#include "msm_drv.h"
> +
> +/*
> + * Util/helpers:
> + */
> +
> +struct clk *msm_clk_bulk_get_clock(struct clk_bulk_data *bulk, int count,
> +		const char *name)
> +{
> +	int i;
> +	char n[32];
> +
> +	snprintf(n, sizeof(n), "%s_clk", name);
> +
> +	for (i = 0; bulk && i < count; i++) {
> +		if (!strcmp(bulk[i].id, name) || !strcmp(bulk[i].id, n))
> +			return bulk[i].clk;
> +	}
> +
> +
> +	return NULL;
> +}
> +
> +struct clk *msm_clk_get(struct platform_device *pdev, const char *name)
> +{
> +	struct clk *clk;
> +	char name2[32];
> +
> +	clk = devm_clk_get(&pdev->dev, name);
> +	if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
> +		return clk;
> +
> +	snprintf(name2, sizeof(name2), "%s_clk", name);
> +
> +	clk = devm_clk_get(&pdev->dev, name2);
> +	if (!IS_ERR(clk))
> +		dev_warn(&pdev->dev, "Using legacy clk name binding.  Use "
> +				"\"%s\" instead of \"%s\"\n", name, name2);
> +
> +	return clk;
> +}
> +
> +static void __iomem *_msm_ioremap(struct platform_device *pdev, const char *name,
> +				  bool quiet, phys_addr_t *psize)
> +{
> +	struct resource *res;
> +	unsigned long size;
> +	void __iomem *ptr;
> +
> +	if (name)
> +		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
> +	else
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	if (!res) {
> +		if (!quiet)
> +			DRM_DEV_ERROR(&pdev->dev, "failed to get memory resource: %s\n", name);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	size = resource_size(res);
> +
> +	ptr = devm_ioremap(&pdev->dev, res->start, size);
> +	if (!ptr) {
> +		if (!quiet)
> +			DRM_DEV_ERROR(&pdev->dev, "failed to ioremap: %s\n", name);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	if (psize)
> +		*psize = size;
> +
> +	return ptr;
> +}
> +
> +void __iomem *msm_ioremap(struct platform_device *pdev, const char *name)
> +{
> +	return _msm_ioremap(pdev, name, false, NULL);
> +}
> +
> +void __iomem *msm_ioremap_quiet(struct platform_device *pdev, const char *name)
> +{
> +	return _msm_ioremap(pdev, name, true, NULL);
> +}
> +
> +void __iomem *msm_ioremap_size(struct platform_device *pdev, const char *name,
> +			  phys_addr_t *psize)
> +{
> +	return _msm_ioremap(pdev, name, false, psize);
> +}
> +
> +static enum hrtimer_restart msm_hrtimer_worktimer(struct hrtimer *t)
> +{
> +	struct msm_hrtimer_work *work = container_of(t,
> +			struct msm_hrtimer_work, timer);
> +
> +	kthread_queue_work(work->worker, &work->work);
> +
> +	return HRTIMER_NORESTART;
> +}
> +
> +void msm_hrtimer_queue_work(struct msm_hrtimer_work *work,
> +			    ktime_t wakeup_time,
> +			    enum hrtimer_mode mode)
> +{
> +	hrtimer_start(&work->timer, wakeup_time, mode);
> +}
> +
> +void msm_hrtimer_work_init(struct msm_hrtimer_work *work,
> +			   struct kthread_worker *worker,
> +			   kthread_work_func_t fn,
> +			   clockid_t clock_id,
> +			   enum hrtimer_mode mode)
> +{
> +	hrtimer_init(&work->timer, clock_id, mode);
> +	work->timer.function = msm_hrtimer_worktimer;
> +	work->worker = worker;
> +	kthread_init_work(&work->work, fn);
> +}
> -- 
> 2.34.1
>