[PATCH 00/12] drm/msm: dpu: Clean up runtime power handling

[PATCH 00/12] drm/msm: dpu: Clean up runtime power handling

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

I started pulling a thread last week when looking at dpu locking. It led
me into the power_handle code and eventually runtime suspend/resume.

This set removes the power_handle stuff entirely. I'm sure it's more
useful when there are multiple clients/handles/events, but for our case
with one handle/client/event, it's just overcomplicating things.

Once the power_handle code is out of the way, I added a few patches on
the end that move the runtime resume code out of crtc and into encoder.
The benefit of this is to avoid inspecting crtc->state without holding
the modeset lock.

There's also a few trivial changes sprinkled through the series.

PTAL,

Sean


Sean Paul (12):
  drm/msm: dpu: Remove dpu_power_handle_get_dbus_name()
  drm/msm: dpu: Remove unused trace_dpu_perf_update_bus()
  drm/msm: dpu: Remove dpu_power_client
  drm/msm: dpu: Don't use power_event for vbif_init_memtypes
  drm/msm: dpu: Handle crtc pm_runtime_resume() directly
  drm/msm: dpu: Remove power_handle from core_perf
  drm/msm: dpu: Include dpu_io_util.h directly in dpu_kms.h
  drm/msm: dpu: Move DPU_POWER_HANDLE_DBUS_ID to core_perf
  drm/msm: dpu: Remove dpu_power_handle
  drm/msm: dpu: Fix typo in dpu_encoder
  drm/msm: dpu: Add ->enabled to dpu_encoder_virt
  drm/msm: dpu: Move crtc runtime resume to encoder

 drivers/gpu/drm/msm/Makefile                  |   1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c |  37 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  22 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      |  45 +---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h      |   4 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   |  53 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h   |   4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |  53 +---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h       |   6 +-
 .../gpu/drm/msm/disp/dpu1/dpu_power_handle.c  | 240 ------------------
 .../gpu/drm/msm/disp/dpu1/dpu_power_handle.h  | 217 ----------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h     |  29 +--
 12 files changed, 82 insertions(+), 629 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h

-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 02/12] drm/msm: dpu: Remove unused trace_dpu_perf_update_bus()

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 0c122e173892..7ab0ba8224f6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -99,27 +99,6 @@ TRACE_EVENT(dpu_perf_set_ot,
 			__entry->vbif_idx)
 )
 
-TRACE_EVENT(dpu_perf_update_bus,
-	TP_PROTO(int client, unsigned long long ab_quota,
-	unsigned long long ib_quota),
-	TP_ARGS(client, ab_quota, ib_quota),
-	TP_STRUCT__entry(
-			__field(int, client)
-			__field(u64, ab_quota)
-			__field(u64, ib_quota)
-	),
-	TP_fast_assign(
-			__entry->client = client;
-			__entry->ab_quota = ab_quota;
-			__entry->ib_quota = ib_quota;
-	),
-	TP_printk("Request client:%d ab=%llu ib=%llu",
-			__entry->client,
-			__entry->ab_quota,
-			__entry->ib_quota)
-)
-
-
 TRACE_EVENT(dpu_cmd_release_bw,
 	TP_PROTO(u32 crtc_id),
 	TP_ARGS(crtc_id),
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/12] drm/msm: dpu: Remove unused trace_dpu_perf_update_bus()

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 21 ---------------------
>  1 file changed, 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> index 0c122e173892..7ab0ba8224f6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> @@ -99,27 +99,6 @@ TRACE_EVENT(dpu_perf_set_ot,
>  			__entry->vbif_idx)
>  )
> 
> -TRACE_EVENT(dpu_perf_update_bus,
> -	TP_PROTO(int client, unsigned long long ab_quota,
> -	unsigned long long ib_quota),
> -	TP_ARGS(client, ab_quota, ib_quota),
> -	TP_STRUCT__entry(
> -			__field(int, client)
> -			__field(u64, ab_quota)
> -			__field(u64, ib_quota)
> -	),
> -	TP_fast_assign(
> -			__entry->client = client;
> -			__entry->ab_quota = ab_quota;
> -			__entry->ib_quota = ib_quota;
> -	),
> -	TP_printk("Request client:%d ab=%llu ib=%llu",
> -			__entry->client,
> -			__entry->ab_quota,
> -			__entry->ib_quota)
> -)
> -
> -
>  TRACE_EVENT(dpu_cmd_release_bw,
>  	TP_PROTO(u32 crtc_id),
>  	TP_ARGS(crtc_id),

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 04/12] drm/msm: dpu: Don't use power_event for vbif_init_memtypes

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

power_events are only used for pm_runtime, and that's all handled in
dpu_kms. So just call vbif_init_memtypes at the correct times.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 +++------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  1 -
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 23094d108e81..ae2bbaae923d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -651,10 +651,6 @@ static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms)
 		dpu_hw_intr_destroy(dpu_kms->hw_intr);
 	dpu_kms->hw_intr = NULL;
 
-	if (dpu_kms->power_event)
-		dpu_power_handle_unregister_event(
-				&dpu_kms->phandle, dpu_kms->power_event);
-
 	/* safe to call these more than once during shutdown */
 	_dpu_debugfs_destroy(dpu_kms);
 	_dpu_kms_mmu_destroy(dpu_kms);
@@ -832,16 +828,6 @@ u64 dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms, char *clock_name)
 	return clk_get_rate(clk->clk);
 }
 
-static void dpu_kms_handle_power_event(u32 event_type, void *usr)
-{
-	struct dpu_kms *dpu_kms = usr;
-
-	if (!dpu_kms)
-		return;
-
-	dpu_vbif_init_memtypes(dpu_kms);
-}
-
 static int dpu_kms_hw_init(struct msm_kms *kms)
 {
 	struct dpu_kms *dpu_kms;
@@ -1015,10 +1001,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
 	/*
 	 * Handle (re)initializations during power enable
 	 */
-	dpu_kms_handle_power_event(DPU_POWER_EVENT_ENABLE, dpu_kms);
-	dpu_kms->power_event = dpu_power_handle_register_event(
-			&dpu_kms->phandle, DPU_POWER_EVENT_ENABLE,
-			dpu_kms_handle_power_event, dpu_kms, "kms");
+	dpu_vbif_init_memtypes(dpu_kms);
 
 	pm_runtime_put_sync(&dpu_kms->pdev->dev);
 
@@ -1172,6 +1155,8 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
 		return rc;
 	}
 
+	dpu_vbif_init_memtypes(dpu_kms);
+
 	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
 	if (rc)
 		DPU_ERROR("resource enable failed: %d\n", rc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index f2c78deb0854..5f08be187c86 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -114,7 +114,6 @@ struct dpu_kms {
 	struct dpu_mdss_cfg *catalog;
 
 	struct dpu_power_handle phandle;
-	struct dpu_power_event *power_event;
 
 	/* directory entry for debugfs */
 	struct dentry *debugfs_root;
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/12] drm/msm: dpu: Don't use power_event for vbif_init_memtypes

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> power_events are only used for pm_runtime, and that's all handled in
> dpu_kms. So just call vbif_init_memtypes at the correct times.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 +++------------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  1 -
>  2 files changed, 3 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 23094d108e81..ae2bbaae923d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -651,10 +651,6 @@ static void _dpu_kms_hw_destroy(struct dpu_kms
> *dpu_kms)
>  		dpu_hw_intr_destroy(dpu_kms->hw_intr);
>  	dpu_kms->hw_intr = NULL;
> 
> -	if (dpu_kms->power_event)
> -		dpu_power_handle_unregister_event(
> -				&dpu_kms->phandle, dpu_kms->power_event);
> -
>  	/* safe to call these more than once during shutdown */
>  	_dpu_debugfs_destroy(dpu_kms);
>  	_dpu_kms_mmu_destroy(dpu_kms);
> @@ -832,16 +828,6 @@ u64 dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms,
> char *clock_name)
>  	return clk_get_rate(clk->clk);
>  }
> 
> -static void dpu_kms_handle_power_event(u32 event_type, void *usr)
> -{
> -	struct dpu_kms *dpu_kms = usr;
> -
> -	if (!dpu_kms)
> -		return;
> -
> -	dpu_vbif_init_memtypes(dpu_kms);
> -}
> -
>  static int dpu_kms_hw_init(struct msm_kms *kms)
>  {
>  	struct dpu_kms *dpu_kms;
> @@ -1015,10 +1001,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  	/*
>  	 * Handle (re)initializations during power enable
>  	 */

Is this comment valid anymore?

> -	dpu_kms_handle_power_event(DPU_POWER_EVENT_ENABLE, dpu_kms);
> -	dpu_kms->power_event = dpu_power_handle_register_event(
> -			&dpu_kms->phandle, DPU_POWER_EVENT_ENABLE,
> -			dpu_kms_handle_power_event, dpu_kms, "kms");
> +	dpu_vbif_init_memtypes(dpu_kms);
> 
>  	pm_runtime_put_sync(&dpu_kms->pdev->dev);
> 
> @@ -1172,6 +1155,8 @@ static int __maybe_unused 
> dpu_runtime_resume(struct
> device *dev)
>  		return rc;
>  	}
> 
> +	dpu_vbif_init_memtypes(dpu_kms);
> +
>  	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
>  	if (rc)
>  		DPU_ERROR("resource enable failed: %d\n", rc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index f2c78deb0854..5f08be187c86 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -114,7 +114,6 @@ struct dpu_kms {
>  	struct dpu_mdss_cfg *catalog;
> 
>  	struct dpu_power_handle phandle;
> -	struct dpu_power_event *power_event;
> 
>  	/* directory entry for debugfs */
>  	struct dentry *debugfs_root;

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 04/12] drm/msm: dpu: Don't use power_event for vbif_init_memtypes

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

power_events are only used for pm_runtime, and that's all handled in
dpu_kms. So just call vbif_init_memtypes at the correct times.

Changes in v2:
- Removed obsolete comment (Jeykumar)

Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 24 +++---------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  1 -
 2 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 23094d108e81..ab8574ab8327 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -651,10 +651,6 @@ static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms)
 		dpu_hw_intr_destroy(dpu_kms->hw_intr);
 	dpu_kms->hw_intr = NULL;
 
-	if (dpu_kms->power_event)
-		dpu_power_handle_unregister_event(
-				&dpu_kms->phandle, dpu_kms->power_event);
-
 	/* safe to call these more than once during shutdown */
 	_dpu_debugfs_destroy(dpu_kms);
 	_dpu_kms_mmu_destroy(dpu_kms);
@@ -832,16 +828,6 @@ u64 dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms, char *clock_name)
 	return clk_get_rate(clk->clk);
 }
 
-static void dpu_kms_handle_power_event(u32 event_type, void *usr)
-{
-	struct dpu_kms *dpu_kms = usr;
-
-	if (!dpu_kms)
-		return;
-
-	dpu_vbif_init_memtypes(dpu_kms);
-}
-
 static int dpu_kms_hw_init(struct msm_kms *kms)
 {
 	struct dpu_kms *dpu_kms;
@@ -1012,13 +998,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
 	 */
 	dev->mode_config.allow_fb_modifiers = true;
 
-	/*
-	 * Handle (re)initializations during power enable
-	 */
-	dpu_kms_handle_power_event(DPU_POWER_EVENT_ENABLE, dpu_kms);
-	dpu_kms->power_event = dpu_power_handle_register_event(
-			&dpu_kms->phandle, DPU_POWER_EVENT_ENABLE,
-			dpu_kms_handle_power_event, dpu_kms, "kms");
+	dpu_vbif_init_memtypes(dpu_kms);
 
 	pm_runtime_put_sync(&dpu_kms->pdev->dev);
 
@@ -1172,6 +1152,8 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
 		return rc;
 	}
 
+	dpu_vbif_init_memtypes(dpu_kms);
+
 	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
 	if (rc)
 		DPU_ERROR("resource enable failed: %d\n", rc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index f2c78deb0854..5f08be187c86 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -114,7 +114,6 @@ struct dpu_kms {
 	struct dpu_mdss_cfg *catalog;
 
 	struct dpu_power_handle phandle;
-	struct dpu_power_event *power_event;
 
 	/* directory entry for debugfs */
 	struct dentry *debugfs_root;
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 04/12] drm/msm: dpu: Don't use power_event for vbif_init_memtypes

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> power_events are only used for pm_runtime, and that's all handled in
> dpu_kms. So just call vbif_init_memtypes at the correct times.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 +++------------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  1 -
>  2 files changed, 3 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 23094d108e81..ae2bbaae923d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -651,10 +651,6 @@ static void _dpu_kms_hw_destroy(struct dpu_kms
> *dpu_kms)
>  		dpu_hw_intr_destroy(dpu_kms->hw_intr);
>  	dpu_kms->hw_intr = NULL;
> 
> -	if (dpu_kms->power_event)
> -		dpu_power_handle_unregister_event(
> -				&dpu_kms->phandle, dpu_kms->power_event);
> -
>  	/* safe to call these more than once during shutdown */
>  	_dpu_debugfs_destroy(dpu_kms);
>  	_dpu_kms_mmu_destroy(dpu_kms);
> @@ -832,16 +828,6 @@ u64 dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms,
> char *clock_name)
>  	return clk_get_rate(clk->clk);
>  }
> 
> -static void dpu_kms_handle_power_event(u32 event_type, void *usr)
> -{
> -	struct dpu_kms *dpu_kms = usr;
> -
> -	if (!dpu_kms)
> -		return;
> -
> -	dpu_vbif_init_memtypes(dpu_kms);
> -}
> -
>  static int dpu_kms_hw_init(struct msm_kms *kms)
>  {
>  	struct dpu_kms *dpu_kms;
> @@ -1015,10 +1001,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  	/*
>  	 * Handle (re)initializations during power enable
>  	 */
> -	dpu_kms_handle_power_event(DPU_POWER_EVENT_ENABLE, dpu_kms);
> -	dpu_kms->power_event = dpu_power_handle_register_event(
> -			&dpu_kms->phandle, DPU_POWER_EVENT_ENABLE,
> -			dpu_kms_handle_power_event, dpu_kms, "kms");
> +	dpu_vbif_init_memtypes(dpu_kms);
> 
>  	pm_runtime_put_sync(&dpu_kms->pdev->dev);
> 
> @@ -1172,6 +1155,8 @@ static int __maybe_unused 
> dpu_runtime_resume(struct
> device *dev)
>  		return rc;
>  	}
> 
> +	dpu_vbif_init_memtypes(dpu_kms);
> +
>  	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
>  	if (rc)
>  		DPU_ERROR("resource enable failed: %d\n", rc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index f2c78deb0854..5f08be187c86 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -114,7 +114,6 @@ struct dpu_kms {
>  	struct dpu_mdss_cfg *catalog;
> 
>  	struct dpu_power_handle phandle;

You can get rid of the handle and header inclusions here itself
to clean up KMS from power_handle stuff!

> -	struct dpu_power_event *power_event;
> 
>  	/* directory entry for debugfs */
>  	struct dentry *debugfs_root;

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 04/12] drm/msm: dpu: Don't use power_event for vbif_init_memtypes

[Jeykumar Sankaran]

On 2018-11-12 17:06, Jeykumar Sankaran wrote:
> On 2018-11-12 11:42, Sean Paul wrote:
>> From: Sean Paul <seanpaul@chromium.org>
>> 
>> power_events are only used for pm_runtime, and that's all handled in
>> dpu_kms. So just call vbif_init_memtypes at the correct times.
>> 
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 +++------------------
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h |  1 -
>>  2 files changed, 3 insertions(+), 19 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> index 23094d108e81..ae2bbaae923d 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> @@ -651,10 +651,6 @@ static void _dpu_kms_hw_destroy(struct dpu_kms
>> *dpu_kms)
>>  		dpu_hw_intr_destroy(dpu_kms->hw_intr);
>>  	dpu_kms->hw_intr = NULL;
>> 
>> -	if (dpu_kms->power_event)
>> -		dpu_power_handle_unregister_event(
>> -				&dpu_kms->phandle, dpu_kms->power_event);
>> -
>>  	/* safe to call these more than once during shutdown */
>>  	_dpu_debugfs_destroy(dpu_kms);
>>  	_dpu_kms_mmu_destroy(dpu_kms);
>> @@ -832,16 +828,6 @@ u64 dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms,
>> char *clock_name)
>>  	return clk_get_rate(clk->clk);
>>  }
>> 
>> -static void dpu_kms_handle_power_event(u32 event_type, void *usr)
>> -{
>> -	struct dpu_kms *dpu_kms = usr;
>> -
>> -	if (!dpu_kms)
>> -		return;
>> -
>> -	dpu_vbif_init_memtypes(dpu_kms);
>> -}
>> -
>>  static int dpu_kms_hw_init(struct msm_kms *kms)
>>  {
>>  	struct dpu_kms *dpu_kms;
>> @@ -1015,10 +1001,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>>  	/*
>>  	 * Handle (re)initializations during power enable
>>  	 */
>> -	dpu_kms_handle_power_event(DPU_POWER_EVENT_ENABLE, dpu_kms);
>> -	dpu_kms->power_event = dpu_power_handle_register_event(
>> -			&dpu_kms->phandle, DPU_POWER_EVENT_ENABLE,
>> -			dpu_kms_handle_power_event, dpu_kms, "kms");
>> +	dpu_vbif_init_memtypes(dpu_kms);
>> 
>>  	pm_runtime_put_sync(&dpu_kms->pdev->dev);
>> 
>> @@ -1172,6 +1155,8 @@ static int __maybe_unused 
>> dpu_runtime_resume(struct
>> device *dev)
>>  		return rc;
>>  	}
>> 
>> +	dpu_vbif_init_memtypes(dpu_kms);
>> +
>>  	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
>>  	if (rc)
>>  		DPU_ERROR("resource enable failed: %d\n", rc);
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
>> index f2c78deb0854..5f08be187c86 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
>> @@ -114,7 +114,6 @@ struct dpu_kms {
>>  	struct dpu_mdss_cfg *catalog;
>> 
>>  	struct dpu_power_handle phandle;
> 
> You can get rid of the handle and header inclusions here itself
> to clean up KMS from power_handle stuff!

nvm. I see you are taking care of this in patch 9/12.
> 
>> -	struct dpu_power_event *power_event;
>> 
>>  	/* directory entry for debugfs */
>>  	struct dentry *debugfs_root;

-- 
Jeykumar S
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/12] drm/msm: dpu: Handle crtc pm_runtime_resume() directly

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Instead of registering through dpu_power_handle just to get a call on
runtime_resume, call the crtc function directly.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 23 ++++++-----------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h  | 10 ++++++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   |  4 ++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h |  8 ++++----
 4 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index e09209d6c469..c55cb751e2b4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -33,7 +33,6 @@
 #include "dpu_plane.h"
 #include "dpu_encoder.h"
 #include "dpu_vbif.h"
-#include "dpu_power_handle.h"
 #include "dpu_core_perf.h"
 #include "dpu_trace.h"
 
@@ -69,8 +68,6 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc)
 	if (!crtc)
 		return;
 
-	dpu_crtc->phandle = NULL;
-
 	drm_crtc_cleanup(crtc);
 	mutex_destroy(&dpu_crtc->crtc_lock);
 	kfree(dpu_crtc);
@@ -844,15 +841,17 @@ static struct drm_crtc_state *dpu_crtc_duplicate_state(struct drm_crtc *crtc)
 	return &cstate->base;
 }
 
-static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
+void dpu_crtc_runtime_resume(struct drm_crtc *crtc)
 {
-	struct drm_crtc *crtc = arg;
 	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
 	struct drm_encoder *encoder;
 
 	mutex_lock(&dpu_crtc->crtc_lock);
 
-	trace_dpu_crtc_handle_power_event(DRMID(crtc), event_type);
+	if (!dpu_crtc->enabled)
+		goto end;
+
+	trace_dpu_crtc_runtime_resume(DRMID(crtc));
 
 	/* restore encoder; crtc will be programmed during commit */
 	drm_for_each_encoder(encoder, crtc->dev) {
@@ -862,6 +861,7 @@ static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
 		dpu_encoder_virt_restore(encoder);
 	}
 
+end:
 	mutex_unlock(&dpu_crtc->crtc_lock);
 }
 
@@ -917,10 +917,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
 		dpu_encoder_register_frame_event_callback(encoder, NULL, NULL);
 	}
 
-	if (dpu_crtc->power_event)
-		dpu_power_handle_unregister_event(dpu_crtc->phandle,
-				dpu_crtc->power_event);
-
 	memset(cstate->mixers, 0, sizeof(cstate->mixers));
 	cstate->num_mixers = 0;
 
@@ -972,11 +968,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
 
 	/* Enable/restore vblank irq handling */
 	drm_crtc_vblank_on(crtc);
-
-	dpu_crtc->power_event = dpu_power_handle_register_event(
-		dpu_crtc->phandle, DPU_POWER_EVENT_ENABLE,
-		dpu_crtc_handle_power_event, crtc, dpu_crtc->name);
-
 }
 
 struct plane_state {
@@ -1522,8 +1513,6 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev, struct drm_plane *plane,
 	/* initialize event handling */
 	spin_lock_init(&dpu_crtc->event_lock);
 
-	dpu_crtc->phandle = &kms->phandle;
-
 	DPU_DEBUG("%s: successfully initialized crtc\n", dpu_crtc->name);
 	return crtc;
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 4822602402f9..1dca91d1210f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -151,7 +151,6 @@ struct dpu_crtc_frame_event {
  * @event_worker  : Event worker queue
  * @event_lock    : Spinlock around event handling code
  * @phandle: Pointer to power handler
- * @power_event   : registered power event handle
  * @cur_perf      : current performance committed to clock/bandwidth driver
  */
 struct dpu_crtc {
@@ -187,9 +186,6 @@ struct dpu_crtc {
 	/* for handling internal event thread */
 	spinlock_t event_lock;
 
-	struct dpu_power_handle *phandle;
-	struct dpu_power_event *power_event;
-
 	struct dpu_core_perf_params cur_perf;
 
 	struct dpu_crtc_smmu_state_data smmu_state;
@@ -333,4 +329,10 @@ static inline bool dpu_crtc_is_enabled(struct drm_crtc *crtc)
 	return crtc ? crtc->enabled : false;
 }
 
+/**
+ * dpu_crtc_runtime_resume - called by the top-level on pm_runtime_resume
+ * @crtc: CRTC to resume
+ */
+void dpu_crtc_runtime_resume(struct drm_crtc *crtc);
+
 #endif /* _DPU_CRTC_H_ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index ae2bbaae923d..62a02c606811 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1140,6 +1140,7 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
 	int rc = -1;
 	struct platform_device *pdev = to_platform_device(dev);
 	struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
+	struct drm_crtc *crtc;
 	struct drm_device *ddev;
 	struct dss_module_power *mp = &dpu_kms->mp;
 
@@ -1157,6 +1158,9 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
 
 	dpu_vbif_init_memtypes(dpu_kms);
 
+	drm_for_each_crtc(crtc, ddev)
+		dpu_crtc_runtime_resume(crtc);
+
 	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
 	if (rc)
 		DPU_ERROR("resource enable failed: %d\n", rc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 7ab0ba8224f6..328df37d7580 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -298,6 +298,10 @@ DEFINE_EVENT(dpu_drm_obj_template, dpu_kms_wait_for_commit_done,
 	TP_PROTO(uint32_t drm_id),
 	TP_ARGS(drm_id)
 );
+DEFINE_EVENT(dpu_drm_obj_template, dpu_crtc_runtime_resume,
+	TP_PROTO(uint32_t drm_id),
+	TP_ARGS(drm_id)
+);
 
 TRACE_EVENT(dpu_enc_enable,
 	TP_PROTO(uint32_t drm_id, int hdisplay, int vdisplay),
@@ -518,10 +522,6 @@ DEFINE_EVENT(dpu_id_event_template, dpu_crtc_frame_event_cb,
 	TP_PROTO(uint32_t drm_id, u32 event),
 	TP_ARGS(drm_id, event)
 );
-DEFINE_EVENT(dpu_id_event_template, dpu_crtc_handle_power_event,
-	TP_PROTO(uint32_t drm_id, u32 event),
-	TP_ARGS(drm_id, event)
-);
 DEFINE_EVENT(dpu_id_event_template, dpu_crtc_frame_event_done,
 	TP_PROTO(uint32_t drm_id, u32 event),
 	TP_ARGS(drm_id, event)
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/12] drm/msm: dpu: Handle crtc pm_runtime_resume() directly

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Instead of registering through dpu_power_handle just to get a call on
> runtime_resume, call the crtc function directly.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 23 ++++++-----------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h  | 10 ++++++----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   |  4 ++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h |  8 ++++----
>  4 files changed, 20 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index e09209d6c469..c55cb751e2b4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -33,7 +33,6 @@
>  #include "dpu_plane.h"
>  #include "dpu_encoder.h"
>  #include "dpu_vbif.h"
> -#include "dpu_power_handle.h"
>  #include "dpu_core_perf.h"
>  #include "dpu_trace.h"
> 
> @@ -69,8 +68,6 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc)
>  	if (!crtc)
>  		return;
> 
> -	dpu_crtc->phandle = NULL;
> -
>  	drm_crtc_cleanup(crtc);
>  	mutex_destroy(&dpu_crtc->crtc_lock);
>  	kfree(dpu_crtc);
> @@ -844,15 +841,17 @@ static struct drm_crtc_state
> *dpu_crtc_duplicate_state(struct drm_crtc *crtc)
>  	return &cstate->base;
>  }
> 
> -static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
> +void dpu_crtc_runtime_resume(struct drm_crtc *crtc)
>  {
> -	struct drm_crtc *crtc = arg;
>  	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
>  	struct drm_encoder *encoder;
> 
>  	mutex_lock(&dpu_crtc->crtc_lock);
> 
> -	trace_dpu_crtc_handle_power_event(DRMID(crtc), event_type);
> +	if (!dpu_crtc->enabled)
> +		goto end;
> +
> +	trace_dpu_crtc_runtime_resume(DRMID(crtc));
> 
>  	/* restore encoder; crtc will be programmed during commit */
>  	drm_for_each_encoder(encoder, crtc->dev) {
> @@ -862,6 +861,7 @@ static void dpu_crtc_handle_power_event(u32
> event_type, void *arg)
>  		dpu_encoder_virt_restore(encoder);
>  	}
> 
> +end:
>  	mutex_unlock(&dpu_crtc->crtc_lock);
>  }
> 
> @@ -917,10 +917,6 @@ static void dpu_crtc_disable(struct drm_crtc 
> *crtc)
>  		dpu_encoder_register_frame_event_callback(encoder, NULL,
> NULL);
>  	}
> 
> -	if (dpu_crtc->power_event)
> -		dpu_power_handle_unregister_event(dpu_crtc->phandle,
> -				dpu_crtc->power_event);
> -
>  	memset(cstate->mixers, 0, sizeof(cstate->mixers));
>  	cstate->num_mixers = 0;
> 
> @@ -972,11 +968,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
> 
>  	/* Enable/restore vblank irq handling */
>  	drm_crtc_vblank_on(crtc);
> -
> -	dpu_crtc->power_event = dpu_power_handle_register_event(
> -		dpu_crtc->phandle, DPU_POWER_EVENT_ENABLE,
> -		dpu_crtc_handle_power_event, crtc, dpu_crtc->name);
> -
>  }
> 
>  struct plane_state {
> @@ -1522,8 +1513,6 @@ struct drm_crtc *dpu_crtc_init(struct drm_device
> *dev, struct drm_plane *plane,
>  	/* initialize event handling */
>  	spin_lock_init(&dpu_crtc->event_lock);
If this is for synchronizing power events, we can get rid of this too.

Thanks,
Jeykumar S.
> 
> -	dpu_crtc->phandle = &kms->phandle;
> -
>  	DPU_DEBUG("%s: successfully initialized crtc\n", dpu_crtc->name);
>  	return crtc;
>  }
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index 4822602402f9..1dca91d1210f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -151,7 +151,6 @@ struct dpu_crtc_frame_event {
>   * @event_worker  : Event worker queue
>   * @event_lock    : Spinlock around event handling code
>   * @phandle: Pointer to power handler
> - * @power_event   : registered power event handle
>   * @cur_perf      : current performance committed to clock/bandwidth
> driver
>   */
>  struct dpu_crtc {
> @@ -187,9 +186,6 @@ struct dpu_crtc {
>  	/* for handling internal event thread */
>  	spinlock_t event_lock;
> 
> -	struct dpu_power_handle *phandle;
> -	struct dpu_power_event *power_event;
> -
>  	struct dpu_core_perf_params cur_perf;
> 
>  	struct dpu_crtc_smmu_state_data smmu_state;
> @@ -333,4 +329,10 @@ static inline bool dpu_crtc_is_enabled(struct
> drm_crtc *crtc)
>  	return crtc ? crtc->enabled : false;
>  }
> 
> +/**
> + * dpu_crtc_runtime_resume - called by the top-level on 
> pm_runtime_resume
> + * @crtc: CRTC to resume
> + */
> +void dpu_crtc_runtime_resume(struct drm_crtc *crtc);
> +
>  #endif /* _DPU_CRTC_H_ */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index ae2bbaae923d..62a02c606811 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1140,6 +1140,7 @@ static int __maybe_unused 
> dpu_runtime_resume(struct
> device *dev)
>  	int rc = -1;
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
> +	struct drm_crtc *crtc;
>  	struct drm_device *ddev;
>  	struct dss_module_power *mp = &dpu_kms->mp;
> 
> @@ -1157,6 +1158,9 @@ static int __maybe_unused 
> dpu_runtime_resume(struct
> device *dev)
> 
>  	dpu_vbif_init_memtypes(dpu_kms);
> 
> +	drm_for_each_crtc(crtc, ddev)
> +		dpu_crtc_runtime_resume(crtc);
> +
>  	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
>  	if (rc)
>  		DPU_ERROR("resource enable failed: %d\n", rc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> index 7ab0ba8224f6..328df37d7580 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> @@ -298,6 +298,10 @@ DEFINE_EVENT(dpu_drm_obj_template,
> dpu_kms_wait_for_commit_done,
>  	TP_PROTO(uint32_t drm_id),
>  	TP_ARGS(drm_id)
>  );
> +DEFINE_EVENT(dpu_drm_obj_template, dpu_crtc_runtime_resume,
> +	TP_PROTO(uint32_t drm_id),
> +	TP_ARGS(drm_id)
> +);
> 
>  TRACE_EVENT(dpu_enc_enable,
>  	TP_PROTO(uint32_t drm_id, int hdisplay, int vdisplay),
> @@ -518,10 +522,6 @@ DEFINE_EVENT(dpu_id_event_template,
> dpu_crtc_frame_event_cb,
>  	TP_PROTO(uint32_t drm_id, u32 event),
>  	TP_ARGS(drm_id, event)
>  );
> -DEFINE_EVENT(dpu_id_event_template, dpu_crtc_handle_power_event,
> -	TP_PROTO(uint32_t drm_id, u32 event),
> -	TP_ARGS(drm_id, event)
> -);
>  DEFINE_EVENT(dpu_id_event_template, dpu_crtc_frame_event_done,
>  	TP_PROTO(uint32_t drm_id, u32 event),
>  	TP_ARGS(drm_id, event)

-- 
Jeykumar S
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/12] drm/msm: dpu: Handle crtc pm_runtime_resume() directly

[Sean Paul]

On Mon, Nov 12, 2018 at 05:20:28PM -0800, Jeykumar Sankaran wrote:
> On 2018-11-12 11:42, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > Instead of registering through dpu_power_handle just to get a call on
> > runtime_resume, call the crtc function directly.
> > 
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 23 ++++++-----------------
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h  | 10 ++++++----
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   |  4 ++++
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h |  8 ++++----
> >  4 files changed, 20 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index e09209d6c469..c55cb751e2b4 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -33,7 +33,6 @@
> >  #include "dpu_plane.h"
> >  #include "dpu_encoder.h"
> >  #include "dpu_vbif.h"
> > -#include "dpu_power_handle.h"
> >  #include "dpu_core_perf.h"
> >  #include "dpu_trace.h"
> > 
> > @@ -69,8 +68,6 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc)
> >  	if (!crtc)
> >  		return;
> > 
> > -	dpu_crtc->phandle = NULL;
> > -
> >  	drm_crtc_cleanup(crtc);
> >  	mutex_destroy(&dpu_crtc->crtc_lock);
> >  	kfree(dpu_crtc);
> > @@ -844,15 +841,17 @@ static struct drm_crtc_state
> > *dpu_crtc_duplicate_state(struct drm_crtc *crtc)
> >  	return &cstate->base;
> >  }
> > 
> > -static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
> > +void dpu_crtc_runtime_resume(struct drm_crtc *crtc)
> >  {
> > -	struct drm_crtc *crtc = arg;
> >  	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> >  	struct drm_encoder *encoder;
> > 
> >  	mutex_lock(&dpu_crtc->crtc_lock);
> > 
> > -	trace_dpu_crtc_handle_power_event(DRMID(crtc), event_type);
> > +	if (!dpu_crtc->enabled)
> > +		goto end;
> > +
> > +	trace_dpu_crtc_runtime_resume(DRMID(crtc));
> > 
> >  	/* restore encoder; crtc will be programmed during commit */
> >  	drm_for_each_encoder(encoder, crtc->dev) {
> > @@ -862,6 +861,7 @@ static void dpu_crtc_handle_power_event(u32
> > event_type, void *arg)
> >  		dpu_encoder_virt_restore(encoder);
> >  	}
> > 
> > +end:
> >  	mutex_unlock(&dpu_crtc->crtc_lock);
> >  }
> > 
> > @@ -917,10 +917,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
> >  		dpu_encoder_register_frame_event_callback(encoder, NULL,
> > NULL);
> >  	}
> > 
> > -	if (dpu_crtc->power_event)
> > -		dpu_power_handle_unregister_event(dpu_crtc->phandle,
> > -				dpu_crtc->power_event);
> > -
> >  	memset(cstate->mixers, 0, sizeof(cstate->mixers));
> >  	cstate->num_mixers = 0;
> > 
> > @@ -972,11 +968,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
> > 
> >  	/* Enable/restore vblank irq handling */
> >  	drm_crtc_vblank_on(crtc);
> > -
> > -	dpu_crtc->power_event = dpu_power_handle_register_event(
> > -		dpu_crtc->phandle, DPU_POWER_EVENT_ENABLE,
> > -		dpu_crtc_handle_power_event, crtc, dpu_crtc->name);
> > -
> >  }
> > 
> >  struct plane_state {
> > @@ -1522,8 +1513,6 @@ struct drm_crtc *dpu_crtc_init(struct drm_device
> > *dev, struct drm_plane *plane,
> >  	/* initialize event handling */
> >  	spin_lock_init(&dpu_crtc->event_lock);
> If this is for synchronizing power events, we can get rid of this too.

In this case, "event" refers to u/s vblank events, not power events.

Sean

> 
> Thanks,
> Jeykumar S.
> > 
> > -	dpu_crtc->phandle = &kms->phandle;
> > -
> >  	DPU_DEBUG("%s: successfully initialized crtc\n", dpu_crtc->name);
> >  	return crtc;
> >  }
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > index 4822602402f9..1dca91d1210f 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > @@ -151,7 +151,6 @@ struct dpu_crtc_frame_event {
> >   * @event_worker  : Event worker queue
> >   * @event_lock    : Spinlock around event handling code
> >   * @phandle: Pointer to power handler
> > - * @power_event   : registered power event handle
> >   * @cur_perf      : current performance committed to clock/bandwidth
> > driver
> >   */
> >  struct dpu_crtc {
> > @@ -187,9 +186,6 @@ struct dpu_crtc {
> >  	/* for handling internal event thread */
> >  	spinlock_t event_lock;
> > 
> > -	struct dpu_power_handle *phandle;
> > -	struct dpu_power_event *power_event;
> > -
> >  	struct dpu_core_perf_params cur_perf;
> > 
> >  	struct dpu_crtc_smmu_state_data smmu_state;
> > @@ -333,4 +329,10 @@ static inline bool dpu_crtc_is_enabled(struct
> > drm_crtc *crtc)
> >  	return crtc ? crtc->enabled : false;
> >  }
> > 
> > +/**
> > + * dpu_crtc_runtime_resume - called by the top-level on
> > pm_runtime_resume
> > + * @crtc: CRTC to resume
> > + */
> > +void dpu_crtc_runtime_resume(struct drm_crtc *crtc);
> > +
> >  #endif /* _DPU_CRTC_H_ */
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index ae2bbaae923d..62a02c606811 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -1140,6 +1140,7 @@ static int __maybe_unused
> > dpu_runtime_resume(struct
> > device *dev)
> >  	int rc = -1;
> >  	struct platform_device *pdev = to_platform_device(dev);
> >  	struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
> > +	struct drm_crtc *crtc;
> >  	struct drm_device *ddev;
> >  	struct dss_module_power *mp = &dpu_kms->mp;
> > 
> > @@ -1157,6 +1158,9 @@ static int __maybe_unused
> > dpu_runtime_resume(struct
> > device *dev)
> > 
> >  	dpu_vbif_init_memtypes(dpu_kms);
> > 
> > +	drm_for_each_crtc(crtc, ddev)
> > +		dpu_crtc_runtime_resume(crtc);
> > +
> >  	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
> >  	if (rc)
> >  		DPU_ERROR("resource enable failed: %d\n", rc);
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> > index 7ab0ba8224f6..328df37d7580 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> > @@ -298,6 +298,10 @@ DEFINE_EVENT(dpu_drm_obj_template,
> > dpu_kms_wait_for_commit_done,
> >  	TP_PROTO(uint32_t drm_id),
> >  	TP_ARGS(drm_id)
> >  );
> > +DEFINE_EVENT(dpu_drm_obj_template, dpu_crtc_runtime_resume,
> > +	TP_PROTO(uint32_t drm_id),
> > +	TP_ARGS(drm_id)
> > +);
> > 
> >  TRACE_EVENT(dpu_enc_enable,
> >  	TP_PROTO(uint32_t drm_id, int hdisplay, int vdisplay),
> > @@ -518,10 +522,6 @@ DEFINE_EVENT(dpu_id_event_template,
> > dpu_crtc_frame_event_cb,
> >  	TP_PROTO(uint32_t drm_id, u32 event),
> >  	TP_ARGS(drm_id, event)
> >  );
> > -DEFINE_EVENT(dpu_id_event_template, dpu_crtc_handle_power_event,
> > -	TP_PROTO(uint32_t drm_id, u32 event),
> > -	TP_ARGS(drm_id, event)
> > -);
> >  DEFINE_EVENT(dpu_id_event_template, dpu_crtc_frame_event_done,
> >  	TP_PROTO(uint32_t drm_id, u32 event),
> >  	TP_ARGS(drm_id, event)
> 
> -- 
> Jeykumar S

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 06/12] drm/msm: dpu: Remove power_handle from core_perf

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

It's unused

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 5 -----
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       | 1 -
 3 files changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 22e84b3d7f98..ef6dd43f8bec 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -605,7 +605,6 @@ void dpu_core_perf_destroy(struct dpu_core_perf *perf)
 	dpu_core_perf_debugfs_destroy(perf);
 	perf->max_core_clk_rate = 0;
 	perf->core_clk = NULL;
-	perf->phandle = NULL;
 	perf->catalog = NULL;
 	perf->dev = NULL;
 }
@@ -613,12 +612,10 @@ void dpu_core_perf_destroy(struct dpu_core_perf *perf)
 int dpu_core_perf_init(struct dpu_core_perf *perf,
 		struct drm_device *dev,
 		struct dpu_mdss_cfg *catalog,
-		struct dpu_power_handle *phandle,
 		struct dss_clk *core_clk)
 {
 	perf->dev = dev;
 	perf->catalog = catalog;
-	perf->phandle = phandle;
 	perf->core_clk = core_clk;
 
 	perf->max_core_clk_rate = core_clk->max_rate;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
index fbcbe0c7527a..68b84d85eb8f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
@@ -19,7 +19,6 @@
 #include <drm/drm_crtc.h>
 
 #include "dpu_hw_catalog.h"
-#include "dpu_power_handle.h"
 
 #define	DPU_PERF_DEFAULT_MAX_CORE_CLK_RATE	412500000
 
@@ -52,7 +51,6 @@ struct dpu_core_perf_tune {
  * @dev: Pointer to drm device
  * @debugfs_root: top level debug folder
  * @catalog: Pointer to catalog configuration
- * @phandle: Pointer to power handler
  * @core_clk: Pointer to core clock structure
  * @core_clk_rate: current core clock rate
  * @max_core_clk_rate: maximum allowable core clock rate
@@ -66,7 +64,6 @@ struct dpu_core_perf {
 	struct drm_device *dev;
 	struct dentry *debugfs_root;
 	struct dpu_mdss_cfg *catalog;
-	struct dpu_power_handle *phandle;
 	struct dss_clk *core_clk;
 	u64 core_clk_rate;
 	u64 max_core_clk_rate;
@@ -113,13 +110,11 @@ void dpu_core_perf_destroy(struct dpu_core_perf *perf);
  * @perf: Pointer to core performance context
  * @dev: Pointer to drm device
  * @catalog: Pointer to catalog
- * @phandle: Pointer to power handle
  * @core_clk: pointer to core clock
  */
 int dpu_core_perf_init(struct dpu_core_perf *perf,
 		struct drm_device *dev,
 		struct dpu_mdss_cfg *catalog,
-		struct dpu_power_handle *phandle,
 		struct dss_clk *core_clk);
 
 /**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 62a02c606811..bae7e86b2913 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -957,7 +957,6 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
 	}
 
 	rc = dpu_core_perf_init(&dpu_kms->perf, dev, dpu_kms->catalog,
-			&dpu_kms->phandle,
 			_dpu_kms_get_clk(dpu_kms, "core"));
 	if (rc) {
 		DPU_ERROR("failed to init perf %d\n", rc);
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/12] drm/msm: dpu: Remove power_handle from core_perf

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> It's unused
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 5 -----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       | 1 -
>  3 files changed, 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index 22e84b3d7f98..ef6dd43f8bec 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -605,7 +605,6 @@ void dpu_core_perf_destroy(struct dpu_core_perf 
> *perf)
>  	dpu_core_perf_debugfs_destroy(perf);
>  	perf->max_core_clk_rate = 0;
>  	perf->core_clk = NULL;
> -	perf->phandle = NULL;
>  	perf->catalog = NULL;
>  	perf->dev = NULL;
>  }
> @@ -613,12 +612,10 @@ void dpu_core_perf_destroy(struct dpu_core_perf
> *perf)
>  int dpu_core_perf_init(struct dpu_core_perf *perf,
>  		struct drm_device *dev,
>  		struct dpu_mdss_cfg *catalog,
> -		struct dpu_power_handle *phandle,
>  		struct dss_clk *core_clk)
>  {
>  	perf->dev = dev;
>  	perf->catalog = catalog;
> -	perf->phandle = phandle;
>  	perf->core_clk = core_clk;
> 
>  	perf->max_core_clk_rate = core_clk->max_rate;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> index fbcbe0c7527a..68b84d85eb8f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> @@ -19,7 +19,6 @@
>  #include <drm/drm_crtc.h>
> 
>  #include "dpu_hw_catalog.h"
> -#include "dpu_power_handle.h"
> 
>  #define	DPU_PERF_DEFAULT_MAX_CORE_CLK_RATE	412500000
> 
> @@ -52,7 +51,6 @@ struct dpu_core_perf_tune {
>   * @dev: Pointer to drm device
>   * @debugfs_root: top level debug folder
>   * @catalog: Pointer to catalog configuration
> - * @phandle: Pointer to power handler
>   * @core_clk: Pointer to core clock structure
>   * @core_clk_rate: current core clock rate
>   * @max_core_clk_rate: maximum allowable core clock rate
> @@ -66,7 +64,6 @@ struct dpu_core_perf {
>  	struct drm_device *dev;
>  	struct dentry *debugfs_root;
>  	struct dpu_mdss_cfg *catalog;
> -	struct dpu_power_handle *phandle;
>  	struct dss_clk *core_clk;
>  	u64 core_clk_rate;
>  	u64 max_core_clk_rate;
> @@ -113,13 +110,11 @@ void dpu_core_perf_destroy(struct dpu_core_perf
> *perf);
>   * @perf: Pointer to core performance context
>   * @dev: Pointer to drm device
>   * @catalog: Pointer to catalog
> - * @phandle: Pointer to power handle
>   * @core_clk: pointer to core clock
>   */
>  int dpu_core_perf_init(struct dpu_core_perf *perf,
>  		struct drm_device *dev,
>  		struct dpu_mdss_cfg *catalog,
> -		struct dpu_power_handle *phandle,
>  		struct dss_clk *core_clk);
> 
>  /**
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 62a02c606811..bae7e86b2913 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -957,7 +957,6 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  	}
> 
>  	rc = dpu_core_perf_init(&dpu_kms->perf, dev, dpu_kms->catalog,
> -			&dpu_kms->phandle,
>  			_dpu_kms_get_clk(dpu_kms, "core"));
>  	if (rc) {
>  		DPU_ERROR("failed to init perf %d\n", rc);

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 01/12] drm/msm: dpu: Remove dpu_power_handle_get_dbus_name()

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

It's only used for debugfs, so just output the enum value instead.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c         |  6 ++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c | 14 --------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h |  7 -------
 3 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index ed84cf44a222..e09209d6c469 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1381,11 +1381,9 @@ static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v)
 			dpu_crtc->cur_perf.core_clk_rate);
 	for (i = DPU_POWER_HANDLE_DBUS_ID_MNOC;
 			i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
-		seq_printf(s, "bw_ctl[%s]: %llu\n",
-				dpu_power_handle_get_dbus_name(i),
+		seq_printf(s, "bw_ctl[%d]: %llu\n", i,
 				dpu_crtc->cur_perf.bw_ctl[i]);
-		seq_printf(s, "max_per_pipe_ib[%s]: %llu\n",
-				dpu_power_handle_get_dbus_name(i),
+		seq_printf(s, "max_per_pipe_ib[%d]: %llu\n", i,
 				dpu_crtc->cur_perf.max_per_pipe_ib[i]);
 	}
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
index fc14116789f2..8c6f92aaaf87 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
@@ -24,20 +24,6 @@
 #include "dpu_power_handle.h"
 #include "dpu_trace.h"
 
-static const char *data_bus_name[DPU_POWER_HANDLE_DBUS_ID_MAX] = {
-	[DPU_POWER_HANDLE_DBUS_ID_MNOC] = "qcom,dpu-data-bus",
-	[DPU_POWER_HANDLE_DBUS_ID_LLCC] = "qcom,dpu-llcc-bus",
-	[DPU_POWER_HANDLE_DBUS_ID_EBI] = "qcom,dpu-ebi-bus",
-};
-
-const char *dpu_power_handle_get_dbus_name(u32 bus_id)
-{
-	if (bus_id < DPU_POWER_HANDLE_DBUS_ID_MAX)
-		return data_bus_name[bus_id];
-
-	return NULL;
-}
-
 static void dpu_power_event_trigger_locked(struct dpu_power_handle *phandle,
 		u32 event_type)
 {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
index a65b7a297f21..f627ae28ec68 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
@@ -207,11 +207,4 @@ struct dpu_power_event *dpu_power_handle_register_event(
 void dpu_power_handle_unregister_event(struct dpu_power_handle *phandle,
 		struct dpu_power_event *event);
 
-/**
- * dpu_power_handle_get_dbus_name - get name of given data bus identifier
- * @bus_id:	data bus identifier
- * Return:	Pointer to name string if success; NULL otherwise
- */
-const char *dpu_power_handle_get_dbus_name(u32 bus_id);
-
 #endif /* _DPU_POWER_HANDLE_H_ */
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 01/12] drm/msm: dpu: Remove dpu_power_handle_get_dbus_name()

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> It's only used for debugfs, so just output the enum value instead.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c         |  6 ++----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c | 14 --------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h |  7 -------
>  3 files changed, 2 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index ed84cf44a222..e09209d6c469 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1381,11 +1381,9 @@ static int dpu_crtc_debugfs_state_show(struct
> seq_file *s, void *v)
>  			dpu_crtc->cur_perf.core_clk_rate);
>  	for (i = DPU_POWER_HANDLE_DBUS_ID_MNOC;
>  			i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
> -		seq_printf(s, "bw_ctl[%s]: %llu\n",
> -				dpu_power_handle_get_dbus_name(i),
> +		seq_printf(s, "bw_ctl[%d]: %llu\n", i,
>  				dpu_crtc->cur_perf.bw_ctl[i]);
> -		seq_printf(s, "max_per_pipe_ib[%s]: %llu\n",
> -				dpu_power_handle_get_dbus_name(i),
> +		seq_printf(s, "max_per_pipe_ib[%d]: %llu\n", i,
>  				dpu_crtc->cur_perf.max_per_pipe_ib[i]);
>  	}
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> index fc14116789f2..8c6f92aaaf87 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> @@ -24,20 +24,6 @@
>  #include "dpu_power_handle.h"
>  #include "dpu_trace.h"
> 
> -static const char *data_bus_name[DPU_POWER_HANDLE_DBUS_ID_MAX] = {
> -	[DPU_POWER_HANDLE_DBUS_ID_MNOC] = "qcom,dpu-data-bus",
> -	[DPU_POWER_HANDLE_DBUS_ID_LLCC] = "qcom,dpu-llcc-bus",
> -	[DPU_POWER_HANDLE_DBUS_ID_EBI] = "qcom,dpu-ebi-bus",
> -};
> -
> -const char *dpu_power_handle_get_dbus_name(u32 bus_id)
> -{
> -	if (bus_id < DPU_POWER_HANDLE_DBUS_ID_MAX)
> -		return data_bus_name[bus_id];
> -
> -	return NULL;
> -}
> -
>  static void dpu_power_event_trigger_locked(struct dpu_power_handle
> *phandle,
>  		u32 event_type)
>  {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> index a65b7a297f21..f627ae28ec68 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> @@ -207,11 +207,4 @@ struct dpu_power_event
> *dpu_power_handle_register_event(
>  void dpu_power_handle_unregister_event(struct dpu_power_handle 
> *phandle,
>  		struct dpu_power_event *event);
> 
> -/**
> - * dpu_power_handle_get_dbus_name - get name of given data bus 
> identifier
> - * @bus_id:	data bus identifier
> - * Return:	Pointer to name string if success; NULL otherwise
> - */
> -const char *dpu_power_handle_get_dbus_name(u32 bus_id);
> -
>  #endif /* _DPU_POWER_HANDLE_H_ */

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 03/12] drm/msm: dpu: Remove dpu_power_client

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

There's only one client -- core, and it's only used for runtime pm which
is already refcounted.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       | 22 +----
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h       |  1 -
 .../gpu/drm/msm/disp/dpu1/dpu_power_handle.c  | 96 +------------------
 .../gpu/drm/msm/disp/dpu1/dpu_power_handle.h  | 86 +----------------
 4 files changed, 6 insertions(+), 199 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 985c855796ae..23094d108e81 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -676,11 +676,6 @@ static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms)
 		dpu_hw_catalog_deinit(dpu_kms->catalog);
 	dpu_kms->catalog = NULL;
 
-	if (dpu_kms->core_client)
-		dpu_power_client_destroy(&dpu_kms->phandle,
-			dpu_kms->core_client);
-	dpu_kms->core_client = NULL;
-
 	if (dpu_kms->vbif[VBIF_NRT])
 		devm_iounmap(&dpu_kms->pdev->dev, dpu_kms->vbif[VBIF_NRT]);
 	dpu_kms->vbif[VBIF_NRT] = NULL;
@@ -913,17 +908,6 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
 		dpu_kms->reg_dma_len = dpu_iomap_size(dpu_kms->pdev, "regdma");
 	}
 
-	dpu_kms->core_client = dpu_power_client_create(&dpu_kms->phandle,
-					"core");
-	if (IS_ERR_OR_NULL(dpu_kms->core_client)) {
-		rc = PTR_ERR(dpu_kms->core_client);
-		if (!dpu_kms->core_client)
-			rc = -EINVAL;
-		DPU_ERROR("dpu power client create failed: %d\n", rc);
-		dpu_kms->core_client = NULL;
-		goto error;
-	}
-
 	pm_runtime_get_sync(&dpu_kms->pdev->dev);
 
 	_dpu_kms_core_hw_rev_init(dpu_kms);
@@ -1157,8 +1141,7 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev)
 		return rc;
 	}
 
-	rc = dpu_power_resource_enable(&dpu_kms->phandle,
-			dpu_kms->core_client, false);
+	rc = dpu_power_resource_enable(&dpu_kms->phandle, false);
 	if (rc)
 		DPU_ERROR("resource disable failed: %d\n", rc);
 
@@ -1189,8 +1172,7 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
 		return rc;
 	}
 
-	rc = dpu_power_resource_enable(&dpu_kms->phandle,
-			dpu_kms->core_client, true);
+	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
 	if (rc)
 		DPU_ERROR("resource enable failed: %d\n", rc);
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 2a3625eef6d3..f2c78deb0854 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -114,7 +114,6 @@ struct dpu_kms {
 	struct dpu_mdss_cfg *catalog;
 
 	struct dpu_power_handle phandle;
-	struct dpu_power_client *core_client;
 	struct dpu_power_event *power_event;
 
 	/* directory entry for debugfs */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
index 8c6f92aaaf87..8e64f0a52147 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
@@ -35,59 +35,11 @@ static void dpu_power_event_trigger_locked(struct dpu_power_handle *phandle,
 	}
 }
 
-struct dpu_power_client *dpu_power_client_create(
-	struct dpu_power_handle *phandle, char *client_name)
-{
-	struct dpu_power_client *client;
-	static u32 id;
-
-	if (!client_name || !phandle) {
-		pr_err("client name is null or invalid power data\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	client = kzalloc(sizeof(struct dpu_power_client), GFP_KERNEL);
-	if (!client)
-		return ERR_PTR(-ENOMEM);
-
-	mutex_lock(&phandle->phandle_lock);
-	strlcpy(client->name, client_name, MAX_CLIENT_NAME_LEN);
-	client->usecase_ndx = VOTE_INDEX_DISABLE;
-	client->id = id;
-	client->active = true;
-	pr_debug("client %s created:%pK id :%d\n", client_name,
-		client, id);
-	id++;
-	list_add(&client->list, &phandle->power_client_clist);
-	mutex_unlock(&phandle->phandle_lock);
-
-	return client;
-}
-
-void dpu_power_client_destroy(struct dpu_power_handle *phandle,
-	struct dpu_power_client *client)
-{
-	if (!client  || !phandle) {
-		pr_err("reg bus vote: invalid client handle\n");
-	} else if (!client->active) {
-		pr_err("dpu power deinit already done\n");
-		kfree(client);
-	} else {
-		pr_debug("bus vote client %s destroyed:%pK id:%u\n",
-			client->name, client, client->id);
-		mutex_lock(&phandle->phandle_lock);
-		list_del_init(&client->list);
-		mutex_unlock(&phandle->phandle_lock);
-		kfree(client);
-	}
-}
-
 void dpu_power_resource_init(struct platform_device *pdev,
 	struct dpu_power_handle *phandle)
 {
 	phandle->dev = &pdev->dev;
 
-	INIT_LIST_HEAD(&phandle->power_client_clist);
 	INIT_LIST_HEAD(&phandle->event_list);
 
 	mutex_init(&phandle->phandle_lock);
@@ -96,7 +48,6 @@ void dpu_power_resource_init(struct platform_device *pdev,
 void dpu_power_resource_deinit(struct platform_device *pdev,
 	struct dpu_power_handle *phandle)
 {
-	struct dpu_power_client *curr_client, *next_client;
 	struct dpu_power_event *curr_event, *next_event;
 
 	if (!phandle || !pdev) {
@@ -105,15 +56,6 @@ void dpu_power_resource_deinit(struct platform_device *pdev,
 	}
 
 	mutex_lock(&phandle->phandle_lock);
-	list_for_each_entry_safe(curr_client, next_client,
-			&phandle->power_client_clist, list) {
-		pr_err("client:%s-%d still registered with refcount:%d\n",
-				curr_client->name, curr_client->id,
-				curr_client->refcount);
-		curr_client->active = false;
-		list_del(&curr_client->list);
-	}
-
 	list_for_each_entry_safe(curr_event, next_event,
 			&phandle->event_list, list) {
 		pr_err("event:%d, client:%s still registered\n",
@@ -125,53 +67,21 @@ void dpu_power_resource_deinit(struct platform_device *pdev,
 	mutex_unlock(&phandle->phandle_lock);
 }
 
-int dpu_power_resource_enable(struct dpu_power_handle *phandle,
-	struct dpu_power_client *pclient, bool enable)
+int dpu_power_resource_enable(struct dpu_power_handle *phandle, bool enable)
 {
-	bool changed = false;
-	u32 max_usecase_ndx = VOTE_INDEX_DISABLE, prev_usecase_ndx;
-	struct dpu_power_client *client;
 	u32 event_type;
 
-	if (!phandle || !pclient) {
+	if (!phandle) {
 		pr_err("invalid input argument\n");
 		return -EINVAL;
 	}
 
 	mutex_lock(&phandle->phandle_lock);
-	if (enable)
-		pclient->refcount++;
-	else if (pclient->refcount)
-		pclient->refcount--;
-
-	if (pclient->refcount)
-		pclient->usecase_ndx = VOTE_INDEX_LOW;
-	else
-		pclient->usecase_ndx = VOTE_INDEX_DISABLE;
-
-	list_for_each_entry(client, &phandle->power_client_clist, list) {
-		if (client->usecase_ndx < VOTE_INDEX_MAX &&
-		    client->usecase_ndx > max_usecase_ndx)
-			max_usecase_ndx = client->usecase_ndx;
-	}
-
-	if (phandle->current_usecase_ndx != max_usecase_ndx) {
-		changed = true;
-		prev_usecase_ndx = phandle->current_usecase_ndx;
-		phandle->current_usecase_ndx = max_usecase_ndx;
-	}
-
-	pr_debug("%pS: changed=%d current idx=%d request client %s id:%u enable:%d refcount:%d\n",
-		__builtin_return_address(0), changed, max_usecase_ndx,
-		pclient->name, pclient->id, enable, pclient->refcount);
-
-	if (!changed)
-		goto end;
 
 	event_type = enable ? DPU_POWER_EVENT_ENABLE : DPU_POWER_EVENT_DISABLE;
 
 	dpu_power_event_trigger_locked(phandle,	event_type);
-end:
+
 	mutex_unlock(&phandle->phandle_lock);
 	return 0;
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
index f627ae28ec68..124ebc93c877 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
@@ -27,30 +27,6 @@
 #define DPU_POWER_EVENT_DISABLE	BIT(0)
 #define DPU_POWER_EVENT_ENABLE	BIT(1)
 
-/**
- * mdss_bus_vote_type: register bus vote type
- * VOTE_INDEX_DISABLE: removes the client vote
- * VOTE_INDEX_LOW: keeps the lowest vote for register bus
- * VOTE_INDEX_MAX: invalid
- */
-enum mdss_bus_vote_type {
-	VOTE_INDEX_DISABLE,
-	VOTE_INDEX_LOW,
-	VOTE_INDEX_MAX,
-};
-
-/**
- * enum dpu_power_handle_data_bus_client - type of axi bus clients
- * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_RT: core real-time bus client
- * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_NRT: core non-real-time bus client
- * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX: maximum number of bus client type
- */
-enum dpu_power_handle_data_bus_client {
-	DPU_POWER_HANDLE_DATA_BUS_CLIENT_RT,
-	DPU_POWER_HANDLE_DATA_BUS_CLIENT_NRT,
-	DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX
-};
-
 /**
  * enum DPU_POWER_HANDLE_DBUS_ID - data bus identifier
  * @DPU_POWER_HANDLE_DBUS_ID_MNOC: DPU/MNOC data bus
@@ -64,31 +40,6 @@ enum DPU_POWER_HANDLE_DBUS_ID {
 	DPU_POWER_HANDLE_DBUS_ID_MAX,
 };
 
-/**
- * struct dpu_power_client: stores the power client for dpu driver
- * @name:	name of the client
- * @usecase_ndx: current regs bus vote type
- * @refcount:	current refcount if multiple modules are using same
- *              same client for enable/disable. Power module will
- *              aggregate the refcount and vote accordingly for this
- *              client.
- * @id:		assigned during create. helps for debugging.
- * @list:	list to attach power handle master list
- * @ab:         arbitrated bandwidth for each bus client
- * @ib:         instantaneous bandwidth for each bus client
- * @active:	inidcates the state of dpu power handle
- */
-struct dpu_power_client {
-	char name[MAX_CLIENT_NAME_LEN];
-	short usecase_ndx;
-	short refcount;
-	u32 id;
-	struct list_head list;
-	u64 ab[DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX];
-	u64 ib[DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX];
-	bool active;
-};
-
 /*
  * struct dpu_power_event - local event registration structure
  * @client_name: name of the client registering
@@ -109,14 +60,12 @@ struct dpu_power_event {
 
 /**
  * struct dpu_power_handle: power handle main struct
- * @client_clist: master list to store all clients
  * @phandle_lock: lock to synchronize the enable/disable
  * @dev: pointer to device structure
  * @usecase_ndx: current usecase index
  * @event_list: current power handle event list
  */
 struct dpu_power_handle {
-	struct list_head power_client_clist;
 	struct mutex phandle_lock;
 	struct device *dev;
 	u32 current_usecase_ndx;
@@ -141,47 +90,14 @@ void dpu_power_resource_init(struct platform_device *pdev,
 void dpu_power_resource_deinit(struct platform_device *pdev,
 	struct dpu_power_handle *pdata);
 
-/**
- * dpu_power_client_create() - create the client on power handle
- * @pdata:  power handle containing the resources
- * @client_name: new client name for registration
- *
- * Return: error code.
- */
-struct dpu_power_client *dpu_power_client_create(struct dpu_power_handle *pdata,
-	char *client_name);
-
-/**
- * dpu_power_client_destroy() - destroy the client on power handle
- * @pdata:  power handle containing the resources
- * @client_name: new client name for registration
- *
- * Return: none
- */
-void dpu_power_client_destroy(struct dpu_power_handle *phandle,
-	struct dpu_power_client *client);
-
 /**
  * dpu_power_resource_enable() - enable/disable the power resources
  * @pdata:  power handle containing the resources
- * @client: client information to enable/disable its vote
  * @enable: boolean request for enable/disable
  *
  * Return: error code.
  */
-int dpu_power_resource_enable(struct dpu_power_handle *pdata,
-	struct dpu_power_client *pclient, bool enable);
-
-/**
- * dpu_power_data_bus_bandwidth_ctrl() - control data bus bandwidth enable
- * @phandle:  power handle containing the resources
- * @client: client information to bandwidth control
- * @enable: true to enable bandwidth for data base
- *
- * Return: none
- */
-void dpu_power_data_bus_bandwidth_ctrl(struct dpu_power_handle *phandle,
-		struct dpu_power_client *pclient, int enable);
+int dpu_power_resource_enable(struct dpu_power_handle *pdata, bool enable);
 
 /**
  * dpu_power_handle_register_event - register a callback function for an event.
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 03/12] drm/msm: dpu: Remove dpu_power_client

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> There's only one client -- core, and it's only used for runtime pm 
> which
> is already refcounted.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       | 22 +----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h       |  1 -
>  .../gpu/drm/msm/disp/dpu1/dpu_power_handle.c  | 96 +------------------
>  .../gpu/drm/msm/disp/dpu1/dpu_power_handle.h  | 86 +----------------
>  4 files changed, 6 insertions(+), 199 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 985c855796ae..23094d108e81 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -676,11 +676,6 @@ static void _dpu_kms_hw_destroy(struct dpu_kms
> *dpu_kms)
>  		dpu_hw_catalog_deinit(dpu_kms->catalog);
>  	dpu_kms->catalog = NULL;
> 
> -	if (dpu_kms->core_client)
> -		dpu_power_client_destroy(&dpu_kms->phandle,
> -			dpu_kms->core_client);
> -	dpu_kms->core_client = NULL;
> -
>  	if (dpu_kms->vbif[VBIF_NRT])
>  		devm_iounmap(&dpu_kms->pdev->dev,
> dpu_kms->vbif[VBIF_NRT]);
>  	dpu_kms->vbif[VBIF_NRT] = NULL;
> @@ -913,17 +908,6 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>  		dpu_kms->reg_dma_len = dpu_iomap_size(dpu_kms->pdev,
> "regdma");
>  	}
> 
> -	dpu_kms->core_client = dpu_power_client_create(&dpu_kms->phandle,
> -					"core");
> -	if (IS_ERR_OR_NULL(dpu_kms->core_client)) {
> -		rc = PTR_ERR(dpu_kms->core_client);
> -		if (!dpu_kms->core_client)
> -			rc = -EINVAL;
> -		DPU_ERROR("dpu power client create failed: %d\n", rc);
> -		dpu_kms->core_client = NULL;
> -		goto error;
> -	}
> -
>  	pm_runtime_get_sync(&dpu_kms->pdev->dev);
> 
>  	_dpu_kms_core_hw_rev_init(dpu_kms);
> @@ -1157,8 +1141,7 @@ static int __maybe_unused 
> dpu_runtime_suspend(struct
> device *dev)
>  		return rc;
>  	}
> 
> -	rc = dpu_power_resource_enable(&dpu_kms->phandle,
> -			dpu_kms->core_client, false);
> +	rc = dpu_power_resource_enable(&dpu_kms->phandle, false);
>  	if (rc)
>  		DPU_ERROR("resource disable failed: %d\n", rc);
> 
> @@ -1189,8 +1172,7 @@ static int __maybe_unused 
> dpu_runtime_resume(struct
> device *dev)
>  		return rc;
>  	}
> 
> -	rc = dpu_power_resource_enable(&dpu_kms->phandle,
> -			dpu_kms->core_client, true);
> +	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
>  	if (rc)
>  		DPU_ERROR("resource enable failed: %d\n", rc);
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 2a3625eef6d3..f2c78deb0854 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -114,7 +114,6 @@ struct dpu_kms {
>  	struct dpu_mdss_cfg *catalog;
> 
>  	struct dpu_power_handle phandle;
> -	struct dpu_power_client *core_client;
>  	struct dpu_power_event *power_event;
> 
>  	/* directory entry for debugfs */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> index 8c6f92aaaf87..8e64f0a52147 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> @@ -35,59 +35,11 @@ static void dpu_power_event_trigger_locked(struct
> dpu_power_handle *phandle,
>  	}
>  }
> 
> -struct dpu_power_client *dpu_power_client_create(
> -	struct dpu_power_handle *phandle, char *client_name)
> -{
> -	struct dpu_power_client *client;
> -	static u32 id;
> -
> -	if (!client_name || !phandle) {
> -		pr_err("client name is null or invalid power data\n");
> -		return ERR_PTR(-EINVAL);
> -	}
> -
> -	client = kzalloc(sizeof(struct dpu_power_client), GFP_KERNEL);
> -	if (!client)
> -		return ERR_PTR(-ENOMEM);
> -
> -	mutex_lock(&phandle->phandle_lock);
> -	strlcpy(client->name, client_name, MAX_CLIENT_NAME_LEN);
> -	client->usecase_ndx = VOTE_INDEX_DISABLE;
> -	client->id = id;
> -	client->active = true;
> -	pr_debug("client %s created:%pK id :%d\n", client_name,
> -		client, id);
> -	id++;
> -	list_add(&client->list, &phandle->power_client_clist);
> -	mutex_unlock(&phandle->phandle_lock);
> -
> -	return client;
> -}
> -
> -void dpu_power_client_destroy(struct dpu_power_handle *phandle,
> -	struct dpu_power_client *client)
> -{
> -	if (!client  || !phandle) {
> -		pr_err("reg bus vote: invalid client handle\n");
> -	} else if (!client->active) {
> -		pr_err("dpu power deinit already done\n");
> -		kfree(client);
> -	} else {
> -		pr_debug("bus vote client %s destroyed:%pK id:%u\n",
> -			client->name, client, client->id);
> -		mutex_lock(&phandle->phandle_lock);
> -		list_del_init(&client->list);
> -		mutex_unlock(&phandle->phandle_lock);
> -		kfree(client);
> -	}
> -}
> -
>  void dpu_power_resource_init(struct platform_device *pdev,
>  	struct dpu_power_handle *phandle)
>  {
>  	phandle->dev = &pdev->dev;
> 
> -	INIT_LIST_HEAD(&phandle->power_client_clist);
>  	INIT_LIST_HEAD(&phandle->event_list);
> 
>  	mutex_init(&phandle->phandle_lock);
> @@ -96,7 +48,6 @@ void dpu_power_resource_init(struct platform_device
> *pdev,
>  void dpu_power_resource_deinit(struct platform_device *pdev,
>  	struct dpu_power_handle *phandle)
>  {
> -	struct dpu_power_client *curr_client, *next_client;
>  	struct dpu_power_event *curr_event, *next_event;
> 
>  	if (!phandle || !pdev) {
> @@ -105,15 +56,6 @@ void dpu_power_resource_deinit(struct 
> platform_device
> *pdev,
>  	}
> 
>  	mutex_lock(&phandle->phandle_lock);
> -	list_for_each_entry_safe(curr_client, next_client,
> -			&phandle->power_client_clist, list) {
> -		pr_err("client:%s-%d still registered with refcount:%d\n",
> -				curr_client->name, curr_client->id,
> -				curr_client->refcount);
> -		curr_client->active = false;
> -		list_del(&curr_client->list);
> -	}
> -
>  	list_for_each_entry_safe(curr_event, next_event,
>  			&phandle->event_list, list) {
>  		pr_err("event:%d, client:%s still registered\n",
> @@ -125,53 +67,21 @@ void dpu_power_resource_deinit(struct 
> platform_device
> *pdev,
>  	mutex_unlock(&phandle->phandle_lock);
>  }
> 
> -int dpu_power_resource_enable(struct dpu_power_handle *phandle,
> -	struct dpu_power_client *pclient, bool enable)
> +int dpu_power_resource_enable(struct dpu_power_handle *phandle, bool
> enable)
>  {
> -	bool changed = false;
> -	u32 max_usecase_ndx = VOTE_INDEX_DISABLE, prev_usecase_ndx;
> -	struct dpu_power_client *client;
>  	u32 event_type;
> 
> -	if (!phandle || !pclient) {
> +	if (!phandle) {
>  		pr_err("invalid input argument\n");
>  		return -EINVAL;
>  	}
> 
>  	mutex_lock(&phandle->phandle_lock);
> -	if (enable)
> -		pclient->refcount++;
> -	else if (pclient->refcount)
> -		pclient->refcount--;
> -
> -	if (pclient->refcount)
> -		pclient->usecase_ndx = VOTE_INDEX_LOW;
> -	else
> -		pclient->usecase_ndx = VOTE_INDEX_DISABLE;
> -
> -	list_for_each_entry(client, &phandle->power_client_clist, list) {
> -		if (client->usecase_ndx < VOTE_INDEX_MAX &&
> -		    client->usecase_ndx > max_usecase_ndx)
> -			max_usecase_ndx = client->usecase_ndx;
> -	}
> -
> -	if (phandle->current_usecase_ndx != max_usecase_ndx) {
> -		changed = true;
> -		prev_usecase_ndx = phandle->current_usecase_ndx;
> -		phandle->current_usecase_ndx = max_usecase_ndx;
> -	}
> -
> -	pr_debug("%pS: changed=%d current idx=%d request client %s id:%u
> enable:%d refcount:%d\n",
> -		__builtin_return_address(0), changed, max_usecase_ndx,
> -		pclient->name, pclient->id, enable, pclient->refcount);
> -
> -	if (!changed)
> -		goto end;
> 
>  	event_type = enable ? DPU_POWER_EVENT_ENABLE :
> DPU_POWER_EVENT_DISABLE;
> 
>  	dpu_power_event_trigger_locked(phandle,	event_type);
> -end:
> +
>  	mutex_unlock(&phandle->phandle_lock);
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> index f627ae28ec68..124ebc93c877 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> @@ -27,30 +27,6 @@
>  #define DPU_POWER_EVENT_DISABLE	BIT(0)
>  #define DPU_POWER_EVENT_ENABLE	BIT(1)
> 
> -/**
> - * mdss_bus_vote_type: register bus vote type
> - * VOTE_INDEX_DISABLE: removes the client vote
> - * VOTE_INDEX_LOW: keeps the lowest vote for register bus
> - * VOTE_INDEX_MAX: invalid
> - */
> -enum mdss_bus_vote_type {
> -	VOTE_INDEX_DISABLE,
> -	VOTE_INDEX_LOW,
> -	VOTE_INDEX_MAX,
> -};
> -
> -/**
> - * enum dpu_power_handle_data_bus_client - type of axi bus clients
> - * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_RT: core real-time bus client
> - * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_NRT: core non-real-time bus 
> client
> - * @DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX: maximum number of bus client
> type
> - */
> -enum dpu_power_handle_data_bus_client {
> -	DPU_POWER_HANDLE_DATA_BUS_CLIENT_RT,
> -	DPU_POWER_HANDLE_DATA_BUS_CLIENT_NRT,
> -	DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX
> -};
> -
>  /**
>   * enum DPU_POWER_HANDLE_DBUS_ID - data bus identifier
>   * @DPU_POWER_HANDLE_DBUS_ID_MNOC: DPU/MNOC data bus
> @@ -64,31 +40,6 @@ enum DPU_POWER_HANDLE_DBUS_ID {
>  	DPU_POWER_HANDLE_DBUS_ID_MAX,
>  };
> 
> -/**
> - * struct dpu_power_client: stores the power client for dpu driver
> - * @name:	name of the client
> - * @usecase_ndx: current regs bus vote type
> - * @refcount:	current refcount if multiple modules are using same
> - *              same client for enable/disable. Power module will
> - *              aggregate the refcount and vote accordingly for this
> - *              client.
> - * @id:		assigned during create. helps for debugging.
> - * @list:	list to attach power handle master list
> - * @ab:         arbitrated bandwidth for each bus client
> - * @ib:         instantaneous bandwidth for each bus client
> - * @active:	inidcates the state of dpu power handle
> - */
> -struct dpu_power_client {
> -	char name[MAX_CLIENT_NAME_LEN];
> -	short usecase_ndx;
> -	short refcount;
> -	u32 id;
> -	struct list_head list;
> -	u64 ab[DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX];
> -	u64 ib[DPU_POWER_HANDLE_DATA_BUS_CLIENT_MAX];
> -	bool active;
> -};
> -
>  /*
>   * struct dpu_power_event - local event registration structure
>   * @client_name: name of the client registering
> @@ -109,14 +60,12 @@ struct dpu_power_event {
> 
>  /**
>   * struct dpu_power_handle: power handle main struct
> - * @client_clist: master list to store all clients
>   * @phandle_lock: lock to synchronize the enable/disable
>   * @dev: pointer to device structure
>   * @usecase_ndx: current usecase index
>   * @event_list: current power handle event list
>   */
>  struct dpu_power_handle {
> -	struct list_head power_client_clist;
>  	struct mutex phandle_lock;
>  	struct device *dev;
>  	u32 current_usecase_ndx;
> @@ -141,47 +90,14 @@ void dpu_power_resource_init(struct 
> platform_device
> *pdev,
>  void dpu_power_resource_deinit(struct platform_device *pdev,
>  	struct dpu_power_handle *pdata);
> 
> -/**
> - * dpu_power_client_create() - create the client on power handle
> - * @pdata:  power handle containing the resources
> - * @client_name: new client name for registration
> - *
> - * Return: error code.
> - */
> -struct dpu_power_client *dpu_power_client_create(struct 
> dpu_power_handle
> *pdata,
> -	char *client_name);
> -
> -/**
> - * dpu_power_client_destroy() - destroy the client on power handle
> - * @pdata:  power handle containing the resources
> - * @client_name: new client name for registration
> - *
> - * Return: none
> - */
> -void dpu_power_client_destroy(struct dpu_power_handle *phandle,
> -	struct dpu_power_client *client);
> -
>  /**
>   * dpu_power_resource_enable() - enable/disable the power resources
>   * @pdata:  power handle containing the resources
> - * @client: client information to enable/disable its vote
>   * @enable: boolean request for enable/disable
>   *
>   * Return: error code.
>   */
> -int dpu_power_resource_enable(struct dpu_power_handle *pdata,
> -	struct dpu_power_client *pclient, bool enable);
> -
> -/**
> - * dpu_power_data_bus_bandwidth_ctrl() - control data bus bandwidth
> enable
> - * @phandle:  power handle containing the resources
> - * @client: client information to bandwidth control
> - * @enable: true to enable bandwidth for data base
> - *
> - * Return: none
> - */
> -void dpu_power_data_bus_bandwidth_ctrl(struct dpu_power_handle 
> *phandle,
> -		struct dpu_power_client *pclient, int enable);
> +int dpu_power_resource_enable(struct dpu_power_handle *pdata, bool
> enable);
> 
>  /**
>   * dpu_power_handle_register_event - register a callback function for 
> an
> event.

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 07/12] drm/msm: dpu: Include dpu_io_util.h directly in dpu_kms.h

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

It's needed for struct dss_module_power, and is currently being pulled
in by dpu_power_handle.h

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 5f08be187c86..4e5acacb3065 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -29,6 +29,7 @@
 #include "dpu_hw_lm.h"
 #include "dpu_hw_interrupts.h"
 #include "dpu_hw_top.h"
+#include "dpu_io_util.h"
 #include "dpu_rm.h"
 #include "dpu_power_handle.h"
 #include "dpu_irq.h"
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 07/12] drm/msm: dpu: Include dpu_io_util.h directly in dpu_kms.h

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> It's needed for struct dss_module_power, and is currently being pulled
> in by dpu_power_handle.h
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 5f08be187c86..4e5acacb3065 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -29,6 +29,7 @@
>  #include "dpu_hw_lm.h"
>  #include "dpu_hw_interrupts.h"
>  #include "dpu_hw_top.h"
> +#include "dpu_io_util.h"
>  #include "dpu_rm.h"
>  #include "dpu_power_handle.h"
>  #include "dpu_irq.h"

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 10/12] drm/msm: dpu: Fix typo in dpu_encoder

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

enc_spinlock instead of enc_spin_lock.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 82c55efb500f..10a0676d1dcf 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -130,7 +130,7 @@ enum dpu_enc_rc_states {
  *	Virtual encoder defers as much as possible to the physical encoders.
  *	Virtual encoder registers itself with the DRM Framework as the encoder.
  * @base:		drm_encoder base class for registration with DRM
- * @enc_spin_lock:	Virtual-Encoder-Wide Spin Lock for IRQ purposes
+ * @enc_spinlock:	Virtual-Encoder-Wide Spin Lock for IRQ purposes
  * @bus_scaling_client:	Client handle to the bus scaling interface
  * @num_phys_encs:	Actual number of physical encoders contained.
  * @phys_encs:		Container of physical encoders managed.
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 10/12] drm/msm: dpu: Fix typo in dpu_encoder

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> enc_spinlock instead of enc_spin_lock.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 82c55efb500f..10a0676d1dcf 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -130,7 +130,7 @@ enum dpu_enc_rc_states {
>   *	Virtual encoder defers as much as possible to the physical
> encoders.
>   *	Virtual encoder registers itself with the DRM Framework as the
> encoder.
>   * @base:		drm_encoder base class for registration with DRM
> - * @enc_spin_lock:	Virtual-Encoder-Wide Spin Lock for IRQ purposes
> + * @enc_spinlock:	Virtual-Encoder-Wide Spin Lock for IRQ purposes
>   * @bus_scaling_client:	Client handle to the bus scaling interface
>   * @num_phys_encs:	Actual number of physical encoders contained.
>   * @phys_encs:		Container of physical encoders managed.

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 12/12] drm/msm: dpu: Move crtc runtime resume to encoder

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

The crtc runtime resume doesn't actually operate on the crtc, but rather
its encoders. The problem with this is that we need to inspect the crtc
state to get the currently connected encoders. Since runtime resume
isn't guaranteed to be called while holding the modeset locks (although
it sometimes is), this presents a race condition.

Now that we have ->enabled on the virtual encoders, and a lock to
protect it, just call resume on each encoder and only restore the ones
that are enabled.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 24 ---------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  6 ------
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 +++++++++------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  4 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     |  6 +++---
 5 files changed, 15 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index d8f58caf2772..9be24907f8c1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -841,30 +841,6 @@ static struct drm_crtc_state *dpu_crtc_duplicate_state(struct drm_crtc *crtc)
 	return &cstate->base;
 }
 
-void dpu_crtc_runtime_resume(struct drm_crtc *crtc)
-{
-	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
-	struct drm_encoder *encoder;
-
-	mutex_lock(&dpu_crtc->crtc_lock);
-
-	if (!dpu_crtc->enabled)
-		goto end;
-
-	trace_dpu_crtc_runtime_resume(DRMID(crtc));
-
-	/* restore encoder; crtc will be programmed during commit */
-	drm_for_each_encoder(encoder, crtc->dev) {
-		if (encoder->crtc != crtc)
-			continue;
-
-		dpu_encoder_virt_restore(encoder);
-	}
-
-end:
-	mutex_unlock(&dpu_crtc->crtc_lock);
-}
-
 static void dpu_crtc_disable(struct drm_crtc *crtc)
 {
 	struct dpu_crtc *dpu_crtc;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 1dca91d1210f..93d21a61a040 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -329,10 +329,4 @@ static inline bool dpu_crtc_is_enabled(struct drm_crtc *crtc)
 	return crtc ? crtc->enabled : false;
 }
 
-/**
- * dpu_crtc_runtime_resume - called by the top-level on pm_runtime_resume
- * @crtc: CRTC to resume
- */
-void dpu_crtc_runtime_resume(struct drm_crtc *crtc);
-
 #endif /* _DPU_CRTC_H_ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3daa86220d47..d89ac520f7e6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1089,28 +1089,24 @@ static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
 	_dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info);
 }
 
-void dpu_encoder_virt_restore(struct drm_encoder *drm_enc)
+void dpu_encoder_virt_runtime_resume(struct drm_encoder *drm_enc)
 {
-	struct dpu_encoder_virt *dpu_enc = NULL;
-	int i;
-
-	if (!drm_enc) {
-		DPU_ERROR("invalid encoder\n");
-		return;
-	}
-	dpu_enc = to_dpu_encoder_virt(drm_enc);
+	struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
 
-	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
-		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
+	mutex_lock(&dpu_enc->enc_lock);
 
-		if (phys && (phys != dpu_enc->cur_master) && phys->ops.restore)
-			phys->ops.restore(phys);
-	}
+	if (!dpu_enc->enabled)
+		goto out;
 
+	if (dpu_enc->cur_slave && dpu_enc->cur_slave->ops.restore)
+		dpu_enc->cur_slave->ops.restore(dpu_enc->cur_slave);
 	if (dpu_enc->cur_master && dpu_enc->cur_master->ops.restore)
 		dpu_enc->cur_master->ops.restore(dpu_enc->cur_master);
 
 	_dpu_encoder_virt_enable_helper(drm_enc);
+
+out:
+	mutex_unlock(&dpu_enc->enc_lock);
 }
 
 static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 9dbf38f446d9..aa4f135218fa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -126,10 +126,10 @@ int dpu_encoder_wait_for_event(struct drm_encoder *drm_encoder,
 enum dpu_intf_mode dpu_encoder_get_intf_mode(struct drm_encoder *encoder);
 
 /**
- * dpu_encoder_virt_restore - restore the encoder configs
+ * dpu_encoder_virt_runtime_resume - pm runtime resume the encoder configs
  * @encoder:	encoder pointer
  */
-void dpu_encoder_virt_restore(struct drm_encoder *encoder);
+void dpu_encoder_virt_runtime_resume(struct drm_encoder *encoder);
 
 /**
  * dpu_encoder_init - initialize virtual encoder object
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index e42685a1d928..c385eab7e441 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1132,7 +1132,7 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
 	int rc = -1;
 	struct platform_device *pdev = to_platform_device(dev);
 	struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
-	struct drm_crtc *crtc;
+	struct drm_encoder *encoder;
 	struct drm_device *ddev;
 	struct dss_module_power *mp = &dpu_kms->mp;
 
@@ -1150,8 +1150,8 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
 
 	dpu_vbif_init_memtypes(dpu_kms);
 
-	drm_for_each_crtc(crtc, ddev)
-		dpu_crtc_runtime_resume(crtc);
+	drm_for_each_encoder(encoder, ddev)
+		dpu_encoder_virt_runtime_resume(encoder);
 
 	return rc;
 }
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 12/12] drm/msm: dpu: Move crtc runtime resume to encoder

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> The crtc runtime resume doesn't actually operate on the crtc, but 
> rather
> its encoders. The problem with this is that we need to inspect the crtc
> state to get the currently connected encoders. Since runtime resume
> isn't guaranteed to be called while holding the modeset locks (although
> it sometimes is), this presents a race condition.
> 
> Now that we have ->enabled on the virtual encoders, and a lock to
> protect it, just call resume on each encoder and only restore the ones
> that are enabled.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 24 ---------------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  6 ------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 +++++++++------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  4 ++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     |  6 +++---
>  5 files changed, 15 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d8f58caf2772..9be24907f8c1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -841,30 +841,6 @@ static struct drm_crtc_state
> *dpu_crtc_duplicate_state(struct drm_crtc *crtc)
>  	return &cstate->base;
>  }
> 
> -void dpu_crtc_runtime_resume(struct drm_crtc *crtc)
> -{
> -	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> -	struct drm_encoder *encoder;
> -
> -	mutex_lock(&dpu_crtc->crtc_lock);
> -
> -	if (!dpu_crtc->enabled)
> -		goto end;
> -
> -	trace_dpu_crtc_runtime_resume(DRMID(crtc));
> -
> -	/* restore encoder; crtc will be programmed during commit */
> -	drm_for_each_encoder(encoder, crtc->dev) {
> -		if (encoder->crtc != crtc)
> -			continue;
> -
> -		dpu_encoder_virt_restore(encoder);
> -	}
I agree the patch provides a cleaner solution. Just for the 
understanding,
won't the race condition be addressed if we acquire the
modeset lock here and iterate through crtc->state->encoder_mask?

Thanks and Regards,
Jeykumar S.
> -
> -end:
> -	mutex_unlock(&dpu_crtc->crtc_lock);
> -}
> -
>  static void dpu_crtc_disable(struct drm_crtc *crtc)
>  {
>  	struct dpu_crtc *dpu_crtc;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index 1dca91d1210f..93d21a61a040 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -329,10 +329,4 @@ static inline bool dpu_crtc_is_enabled(struct
> drm_crtc *crtc)
>  	return crtc ? crtc->enabled : false;
>  }
> 
> -/**
> - * dpu_crtc_runtime_resume - called by the top-level on 
> pm_runtime_resume
> - * @crtc: CRTC to resume
> - */
> -void dpu_crtc_runtime_resume(struct drm_crtc *crtc);
> -
>  #endif /* _DPU_CRTC_H_ */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 3daa86220d47..d89ac520f7e6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1089,28 +1089,24 @@ static void 
> _dpu_encoder_virt_enable_helper(struct
> drm_encoder *drm_enc)
>  	_dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info);
>  }
> 
> -void dpu_encoder_virt_restore(struct drm_encoder *drm_enc)
> +void dpu_encoder_virt_runtime_resume(struct drm_encoder *drm_enc)
>  {
> -	struct dpu_encoder_virt *dpu_enc = NULL;
> -	int i;
> -
> -	if (!drm_enc) {
> -		DPU_ERROR("invalid encoder\n");
> -		return;
> -	}
> -	dpu_enc = to_dpu_encoder_virt(drm_enc);
> +	struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
> 
> -	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> -		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
> +	mutex_lock(&dpu_enc->enc_lock);
> 
> -		if (phys && (phys != dpu_enc->cur_master) &&
> phys->ops.restore)
> -			phys->ops.restore(phys);
> -	}
> +	if (!dpu_enc->enabled)
> +		goto out;
> 
> +	if (dpu_enc->cur_slave && dpu_enc->cur_slave->ops.restore)
> +		dpu_enc->cur_slave->ops.restore(dpu_enc->cur_slave);
>  	if (dpu_enc->cur_master && dpu_enc->cur_master->ops.restore)
>  		dpu_enc->cur_master->ops.restore(dpu_enc->cur_master);
> 
>  	_dpu_encoder_virt_enable_helper(drm_enc);
> +
> +out:
> +	mutex_unlock(&dpu_enc->enc_lock);
>  }
> 
>  static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index 9dbf38f446d9..aa4f135218fa 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -126,10 +126,10 @@ int dpu_encoder_wait_for_event(struct drm_encoder
> *drm_encoder,
>  enum dpu_intf_mode dpu_encoder_get_intf_mode(struct drm_encoder
> *encoder);
> 
>  /**
> - * dpu_encoder_virt_restore - restore the encoder configs
> + * dpu_encoder_virt_runtime_resume - pm runtime resume the encoder
> configs
>   * @encoder:	encoder pointer
>   */
> -void dpu_encoder_virt_restore(struct drm_encoder *encoder);
> +void dpu_encoder_virt_runtime_resume(struct drm_encoder *encoder);
> 
>  /**
>   * dpu_encoder_init - initialize virtual encoder object
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index e42685a1d928..c385eab7e441 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1132,7 +1132,7 @@ static int __maybe_unused 
> dpu_runtime_resume(struct
> device *dev)
>  	int rc = -1;
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
> -	struct drm_crtc *crtc;
> +	struct drm_encoder *encoder;
>  	struct drm_device *ddev;
>  	struct dss_module_power *mp = &dpu_kms->mp;
> 
> @@ -1150,8 +1150,8 @@ static int __maybe_unused 
> dpu_runtime_resume(struct
> device *dev)
> 
>  	dpu_vbif_init_memtypes(dpu_kms);
> 
> -	drm_for_each_crtc(crtc, ddev)
> -		dpu_crtc_runtime_resume(crtc);
> +	drm_for_each_encoder(encoder, ddev)
> +		dpu_encoder_virt_runtime_resume(encoder);
> 
>  	return rc;
>  }

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 12/12] drm/msm: dpu: Move crtc runtime resume to encoder

[Sean Paul]

On Mon, Nov 12, 2018 at 05:47:58PM -0800, Jeykumar Sankaran wrote:
> On 2018-11-12 11:42, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > The crtc runtime resume doesn't actually operate on the crtc, but rather
> > its encoders. The problem with this is that we need to inspect the crtc
> > state to get the currently connected encoders. Since runtime resume
> > isn't guaranteed to be called while holding the modeset locks (although
> > it sometimes is), this presents a race condition.
> > 
> > Now that we have ->enabled on the virtual encoders, and a lock to
> > protect it, just call resume on each encoder and only restore the ones
> > that are enabled.
> > 
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 24 ---------------------
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  6 ------
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 +++++++++------------
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  4 ++--
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     |  6 +++---
> >  5 files changed, 15 insertions(+), 49 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index d8f58caf2772..9be24907f8c1 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -841,30 +841,6 @@ static struct drm_crtc_state
> > *dpu_crtc_duplicate_state(struct drm_crtc *crtc)
> >  	return &cstate->base;
> >  }
> > 
> > -void dpu_crtc_runtime_resume(struct drm_crtc *crtc)
> > -{
> > -	struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> > -	struct drm_encoder *encoder;
> > -
> > -	mutex_lock(&dpu_crtc->crtc_lock);
> > -
> > -	if (!dpu_crtc->enabled)
> > -		goto end;
> > -
> > -	trace_dpu_crtc_runtime_resume(DRMID(crtc));
> > -
> > -	/* restore encoder; crtc will be programmed during commit */
> > -	drm_for_each_encoder(encoder, crtc->dev) {
> > -		if (encoder->crtc != crtc)
> > -			continue;
> > -
> > -		dpu_encoder_virt_restore(encoder);
> > -	}
> I agree the patch provides a cleaner solution. Just for the understanding,
> won't the race condition be addressed if we acquire the
> modeset lock here and iterate through crtc->state->encoder_mask?

You can't do that since we do synchronous pm_runtime_get_sync while holding the
modeset locks. So you'll have lock recursion.

Unfortunately it's not guaranteed that this will be called while holding modeset
locks, so that's why we need to use enc_lock.

Sean

> 
> Thanks and Regards,
> Jeykumar S.
> > -
> > -end:
> > -	mutex_unlock(&dpu_crtc->crtc_lock);
> > -}
> > -
> >  static void dpu_crtc_disable(struct drm_crtc *crtc)
> >  {
> >  	struct dpu_crtc *dpu_crtc;
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > index 1dca91d1210f..93d21a61a040 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > @@ -329,10 +329,4 @@ static inline bool dpu_crtc_is_enabled(struct
> > drm_crtc *crtc)
> >  	return crtc ? crtc->enabled : false;
> >  }
> > 
> > -/**
> > - * dpu_crtc_runtime_resume - called by the top-level on
> > pm_runtime_resume
> > - * @crtc: CRTC to resume
> > - */
> > -void dpu_crtc_runtime_resume(struct drm_crtc *crtc);
> > -
> >  #endif /* _DPU_CRTC_H_ */
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 3daa86220d47..d89ac520f7e6 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -1089,28 +1089,24 @@ static void
> > _dpu_encoder_virt_enable_helper(struct
> > drm_encoder *drm_enc)
> >  	_dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info);
> >  }
> > 
> > -void dpu_encoder_virt_restore(struct drm_encoder *drm_enc)
> > +void dpu_encoder_virt_runtime_resume(struct drm_encoder *drm_enc)
> >  {
> > -	struct dpu_encoder_virt *dpu_enc = NULL;
> > -	int i;
> > -
> > -	if (!drm_enc) {
> > -		DPU_ERROR("invalid encoder\n");
> > -		return;
> > -	}
> > -	dpu_enc = to_dpu_encoder_virt(drm_enc);
> > +	struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
> > 
> > -	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> > -		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
> > +	mutex_lock(&dpu_enc->enc_lock);
> > 
> > -		if (phys && (phys != dpu_enc->cur_master) &&
> > phys->ops.restore)
> > -			phys->ops.restore(phys);
> > -	}
> > +	if (!dpu_enc->enabled)
> > +		goto out;
> > 
> > +	if (dpu_enc->cur_slave && dpu_enc->cur_slave->ops.restore)
> > +		dpu_enc->cur_slave->ops.restore(dpu_enc->cur_slave);
> >  	if (dpu_enc->cur_master && dpu_enc->cur_master->ops.restore)
> >  		dpu_enc->cur_master->ops.restore(dpu_enc->cur_master);
> > 
> >  	_dpu_encoder_virt_enable_helper(drm_enc);
> > +
> > +out:
> > +	mutex_unlock(&dpu_enc->enc_lock);
> >  }
> > 
> >  static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > index 9dbf38f446d9..aa4f135218fa 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > @@ -126,10 +126,10 @@ int dpu_encoder_wait_for_event(struct drm_encoder
> > *drm_encoder,
> >  enum dpu_intf_mode dpu_encoder_get_intf_mode(struct drm_encoder
> > *encoder);
> > 
> >  /**
> > - * dpu_encoder_virt_restore - restore the encoder configs
> > + * dpu_encoder_virt_runtime_resume - pm runtime resume the encoder
> > configs
> >   * @encoder:	encoder pointer
> >   */
> > -void dpu_encoder_virt_restore(struct drm_encoder *encoder);
> > +void dpu_encoder_virt_runtime_resume(struct drm_encoder *encoder);
> > 
> >  /**
> >   * dpu_encoder_init - initialize virtual encoder object
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index e42685a1d928..c385eab7e441 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -1132,7 +1132,7 @@ static int __maybe_unused
> > dpu_runtime_resume(struct
> > device *dev)
> >  	int rc = -1;
> >  	struct platform_device *pdev = to_platform_device(dev);
> >  	struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
> > -	struct drm_crtc *crtc;
> > +	struct drm_encoder *encoder;
> >  	struct drm_device *ddev;
> >  	struct dss_module_power *mp = &dpu_kms->mp;
> > 
> > @@ -1150,8 +1150,8 @@ static int __maybe_unused
> > dpu_runtime_resume(struct
> > device *dev)
> > 
> >  	dpu_vbif_init_memtypes(dpu_kms);
> > 
> > -	drm_for_each_crtc(crtc, ddev)
> > -		dpu_crtc_runtime_resume(crtc);
> > +	drm_for_each_encoder(encoder, ddev)
> > +		dpu_encoder_virt_runtime_resume(encoder);
> > 
> >  	return rc;
> >  }
> 
> -- 
> Jeykumar S

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 13/12] drm/msm: dpu: Don't drop locks in crtc_vblank_enable

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Now that runtime resume is handled in encoder, we don't need to worry
about crtc_lock recursion when calling pm_runtime_(get|put). So drop the
lock drops in _dpu_crtc_vblank_enable_no_lock().

Changes in v2:
- Added to patch series

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 9be24907f8c1..80de5289ada3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -777,10 +777,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
 	struct drm_encoder *enc;
 
 	if (enable) {
-		/* drop lock since power crtc cb may try to re-acquire lock */
-		mutex_unlock(&dpu_crtc->crtc_lock);
 		pm_runtime_get_sync(dev->dev);
-		mutex_lock(&dpu_crtc->crtc_lock);
 
 		list_for_each_entry(enc, &dev->mode_config.encoder_list, head) {
 			if (enc->crtc != crtc)
@@ -805,10 +802,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
 			dpu_encoder_register_vblank_callback(enc, NULL, NULL);
 		}
 
-		/* drop lock since power crtc cb may try to re-acquire lock */
-		mutex_unlock(&dpu_crtc->crtc_lock);
 		pm_runtime_put_sync(dev->dev);
-		mutex_lock(&dpu_crtc->crtc_lock);
 	}
 }
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 08/12] drm/msm: dpu: Move DPU_POWER_HANDLE_DBUS_ID to core_perf

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

It's only used in core_perf, so stick it there (and change the name to
reflect that).

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 34 +++++++++----------
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 17 ++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      |  4 +--
 .../gpu/drm/msm/disp/dpu1/dpu_power_handle.h  | 13 -------
 4 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index ef6dd43f8bec..bffc51e496e7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -95,20 +95,20 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
 	memset(perf, 0, sizeof(struct dpu_core_perf_params));
 
 	if (!dpu_cstate->bw_control) {
-		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
 			perf->bw_ctl[i] = kms->catalog->perf.max_bw_high *
 					1000ULL;
 			perf->max_per_pipe_ib[i] = perf->bw_ctl[i];
 		}
 		perf->core_clk_rate = kms->perf.max_core_clk_rate;
 	} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
-		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
 			perf->bw_ctl[i] = 0;
 			perf->max_per_pipe_ib[i] = 0;
 		}
 		perf->core_clk_rate = 0;
 	} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED) {
-		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
 			perf->bw_ctl[i] = kms->perf.fix_core_ab_vote;
 			perf->max_per_pipe_ib[i] = kms->perf.fix_core_ib_vote;
 		}
@@ -118,12 +118,12 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
 	DPU_DEBUG(
 		"crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu llcc_ib=%llu llcc_ab=%llu mem_ib=%llu mem_ab=%llu\n",
 			crtc->base.id, perf->core_clk_rate,
-			perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_MNOC],
-			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MNOC],
-			perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_LLCC],
-			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
-			perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_EBI],
-			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI]);
+			perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
+			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
+			perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
+			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
+			perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_EBI],
+			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI]);
 }
 
 int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
@@ -158,8 +158,8 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
 	/* obtain new values */
 	_dpu_core_perf_calc_crtc(kms, crtc, state, &dpu_cstate->new_perf);
 
-	for (i = DPU_POWER_HANDLE_DBUS_ID_MNOC;
-			i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+	for (i = DPU_CORE_PERF_DATA_BUS_ID_MNOC;
+			i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
 		bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl[i];
 		curr_client_type = dpu_crtc_get_client_type(crtc);
 
@@ -290,7 +290,7 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)
 	if (kms->perf.enable_bw_release) {
 		trace_dpu_cmd_release_bw(crtc->base.id);
 		DPU_DEBUG("Release BW crtc=%d\n", crtc->base.id);
-		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
 			dpu_crtc->cur_perf.bw_ctl[i] = 0;
 			_dpu_core_perf_crtc_update_bus(kms, crtc, i);
 		}
@@ -367,7 +367,7 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
 	new = &dpu_cstate->new_perf;
 
 	if (_dpu_core_perf_crtc_is_power_on(crtc) && !stop_req) {
-		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
 			/*
 			 * cases for bus bandwidth update.
 			 * 1. new bandwidth vote - "ab or ib vote" is higher
@@ -409,13 +409,13 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
 		update_clk = 1;
 	}
 	trace_dpu_perf_crtc_update(crtc->base.id,
-				new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MNOC],
-				new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
-				new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI],
+				new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
+				new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
+				new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI],
 				new->core_clk_rate, stop_req,
 				update_bus, update_clk);
 
-	for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+	for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
 		if (update_bus & BIT(i)) {
 			ret = _dpu_core_perf_crtc_update_bus(kms, crtc, i);
 			if (ret) {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
index 68b84d85eb8f..c708451a94a1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
@@ -22,6 +22,19 @@
 
 #define	DPU_PERF_DEFAULT_MAX_CORE_CLK_RATE	412500000
 
+/**
+ * enum dpu_core_perf_data_bus_id - data bus identifier
+ * @DPU_CORE_PERF_DATA_BUS_ID_MNOC: DPU/MNOC data bus
+ * @DPU_CORE_PERF_DATA_BUS_ID_LLCC: MNOC/LLCC data bus
+ * @DPU_CORE_PERF_DATA_BUS_ID_EBI: LLCC/EBI data bus
+ */
+enum dpu_core_perf_data_bus_id {
+	DPU_CORE_PERF_DATA_BUS_ID_MNOC,
+	DPU_CORE_PERF_DATA_BUS_ID_LLCC,
+	DPU_CORE_PERF_DATA_BUS_ID_EBI,
+	DPU_CORE_PERF_DATA_BUS_ID_MAX,
+};
+
 /**
  * struct dpu_core_perf_params - definition of performance parameters
  * @max_per_pipe_ib: maximum instantaneous bandwidth request
@@ -29,8 +42,8 @@
  * @core_clk_rate: core clock rate request
  */
 struct dpu_core_perf_params {
-	u64 max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_MAX];
-	u64 bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MAX];
+	u64 max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MAX];
+	u64 bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MAX];
 	u64 core_clk_rate;
 };
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index c55cb751e2b4..d8f58caf2772 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1370,8 +1370,8 @@ static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v)
 	seq_printf(s, "intf_mode: %d\n", dpu_crtc_get_intf_mode(crtc));
 	seq_printf(s, "core_clk_rate: %llu\n",
 			dpu_crtc->cur_perf.core_clk_rate);
-	for (i = DPU_POWER_HANDLE_DBUS_ID_MNOC;
-			i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+	for (i = DPU_CORE_PERF_DATA_BUS_ID_MNOC;
+			i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
 		seq_printf(s, "bw_ctl[%d]: %llu\n", i,
 				dpu_crtc->cur_perf.bw_ctl[i]);
 		seq_printf(s, "max_per_pipe_ib[%d]: %llu\n", i,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
index 124ebc93c877..7536624c8b20 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
@@ -27,19 +27,6 @@
 #define DPU_POWER_EVENT_DISABLE	BIT(0)
 #define DPU_POWER_EVENT_ENABLE	BIT(1)
 
-/**
- * enum DPU_POWER_HANDLE_DBUS_ID - data bus identifier
- * @DPU_POWER_HANDLE_DBUS_ID_MNOC: DPU/MNOC data bus
- * @DPU_POWER_HANDLE_DBUS_ID_LLCC: MNOC/LLCC data bus
- * @DPU_POWER_HANDLE_DBUS_ID_EBI: LLCC/EBI data bus
- */
-enum DPU_POWER_HANDLE_DBUS_ID {
-	DPU_POWER_HANDLE_DBUS_ID_MNOC,
-	DPU_POWER_HANDLE_DBUS_ID_LLCC,
-	DPU_POWER_HANDLE_DBUS_ID_EBI,
-	DPU_POWER_HANDLE_DBUS_ID_MAX,
-};
-
 /*
  * struct dpu_power_event - local event registration structure
  * @client_name: name of the client registering
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/12] drm/msm: dpu: Move DPU_POWER_HANDLE_DBUS_ID to core_perf

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> It's only used in core_perf, so stick it there (and change the name to
> reflect that).
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 34 +++++++++----------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 17 ++++++++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      |  4 +--
>  .../gpu/drm/msm/disp/dpu1/dpu_power_handle.h  | 13 -------
>  4 files changed, 34 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index ef6dd43f8bec..bffc51e496e7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -95,20 +95,20 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms
> *kms,
>  	memset(perf, 0, sizeof(struct dpu_core_perf_params));
> 
>  	if (!dpu_cstate->bw_control) {
> -		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
> +		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
>  			perf->bw_ctl[i] = kms->catalog->perf.max_bw_high *
>  					1000ULL;
>  			perf->max_per_pipe_ib[i] = perf->bw_ctl[i];
>  		}
>  		perf->core_clk_rate = kms->perf.max_core_clk_rate;
>  	} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
> -		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
> +		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
>  			perf->bw_ctl[i] = 0;
>  			perf->max_per_pipe_ib[i] = 0;
>  		}
>  		perf->core_clk_rate = 0;
>  	} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED) {
> -		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
> +		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
>  			perf->bw_ctl[i] = kms->perf.fix_core_ab_vote;
>  			perf->max_per_pipe_ib[i] =
> kms->perf.fix_core_ib_vote;
>  		}
> @@ -118,12 +118,12 @@ static void _dpu_core_perf_calc_crtc(struct 
> dpu_kms
> *kms,
>  	DPU_DEBUG(
>  		"crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu
> llcc_ib=%llu llcc_ab=%llu mem_ib=%llu mem_ab=%llu\n",
>  			crtc->base.id, perf->core_clk_rate,
> -
> perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_MNOC],
> -			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MNOC],
> -
> perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_LLCC],
> -			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
> -
> perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_EBI],
> -			perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI]);
> +
> perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
> +			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
> +
> perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
> +			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
> +
> perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_EBI],
> +			perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI]);
>  }
> 
>  int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
> @@ -158,8 +158,8 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
>  	/* obtain new values */
>  	_dpu_core_perf_calc_crtc(kms, crtc, state, &dpu_cstate->new_perf);
> 
> -	for (i = DPU_POWER_HANDLE_DBUS_ID_MNOC;
> -			i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
> +	for (i = DPU_CORE_PERF_DATA_BUS_ID_MNOC;
> +			i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
>  		bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl[i];
>  		curr_client_type = dpu_crtc_get_client_type(crtc);
> 
> @@ -290,7 +290,7 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc
> *crtc)
>  	if (kms->perf.enable_bw_release) {
>  		trace_dpu_cmd_release_bw(crtc->base.id);
>  		DPU_DEBUG("Release BW crtc=%d\n", crtc->base.id);
> -		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
> +		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
>  			dpu_crtc->cur_perf.bw_ctl[i] = 0;
>  			_dpu_core_perf_crtc_update_bus(kms, crtc, i);
>  		}
> @@ -367,7 +367,7 @@ int dpu_core_perf_crtc_update(struct drm_crtc 
> *crtc,
>  	new = &dpu_cstate->new_perf;
> 
>  	if (_dpu_core_perf_crtc_is_power_on(crtc) && !stop_req) {
> -		for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
> +		for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
>  			/*
>  			 * cases for bus bandwidth update.
>  			 * 1. new bandwidth vote - "ab or ib vote" is
> higher
> @@ -409,13 +409,13 @@ int dpu_core_perf_crtc_update(struct drm_crtc 
> *crtc,
>  		update_clk = 1;
>  	}
>  	trace_dpu_perf_crtc_update(crtc->base.id,
> -
> new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MNOC],
> -
> new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
> -				new->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI],
> +
> new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
> +
> new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
> +
> new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI],
>  				new->core_clk_rate, stop_req,
>  				update_bus, update_clk);
> 
> -	for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
> +	for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
>  		if (update_bus & BIT(i)) {
>  			ret = _dpu_core_perf_crtc_update_bus(kms, crtc,
> i);
>  			if (ret) {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> index 68b84d85eb8f..c708451a94a1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> @@ -22,6 +22,19 @@
> 
>  #define	DPU_PERF_DEFAULT_MAX_CORE_CLK_RATE	412500000
> 
> +/**
> + * enum dpu_core_perf_data_bus_id - data bus identifier
> + * @DPU_CORE_PERF_DATA_BUS_ID_MNOC: DPU/MNOC data bus
> + * @DPU_CORE_PERF_DATA_BUS_ID_LLCC: MNOC/LLCC data bus
> + * @DPU_CORE_PERF_DATA_BUS_ID_EBI: LLCC/EBI data bus
> + */
> +enum dpu_core_perf_data_bus_id {
> +	DPU_CORE_PERF_DATA_BUS_ID_MNOC,
> +	DPU_CORE_PERF_DATA_BUS_ID_LLCC,
> +	DPU_CORE_PERF_DATA_BUS_ID_EBI,
> +	DPU_CORE_PERF_DATA_BUS_ID_MAX,
> +};
> +
>  /**
>   * struct dpu_core_perf_params - definition of performance parameters
>   * @max_per_pipe_ib: maximum instantaneous bandwidth request
> @@ -29,8 +42,8 @@
>   * @core_clk_rate: core clock rate request
>   */
>  struct dpu_core_perf_params {
> -	u64 max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_MAX];
> -	u64 bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MAX];
> +	u64 max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MAX];
> +	u64 bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MAX];
>  	u64 core_clk_rate;
>  };
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index c55cb751e2b4..d8f58caf2772 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1370,8 +1370,8 @@ static int dpu_crtc_debugfs_state_show(struct
> seq_file *s, void *v)
>  	seq_printf(s, "intf_mode: %d\n", dpu_crtc_get_intf_mode(crtc));
>  	seq_printf(s, "core_clk_rate: %llu\n",
>  			dpu_crtc->cur_perf.core_clk_rate);
> -	for (i = DPU_POWER_HANDLE_DBUS_ID_MNOC;
> -			i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
> +	for (i = DPU_CORE_PERF_DATA_BUS_ID_MNOC;
> +			i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
>  		seq_printf(s, "bw_ctl[%d]: %llu\n", i,
>  				dpu_crtc->cur_perf.bw_ctl[i]);
>  		seq_printf(s, "max_per_pipe_ib[%d]: %llu\n", i,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> index 124ebc93c877..7536624c8b20 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> @@ -27,19 +27,6 @@
>  #define DPU_POWER_EVENT_DISABLE	BIT(0)
>  #define DPU_POWER_EVENT_ENABLE	BIT(1)
> 
> -/**
> - * enum DPU_POWER_HANDLE_DBUS_ID - data bus identifier
> - * @DPU_POWER_HANDLE_DBUS_ID_MNOC: DPU/MNOC data bus
> - * @DPU_POWER_HANDLE_DBUS_ID_LLCC: MNOC/LLCC data bus
> - * @DPU_POWER_HANDLE_DBUS_ID_EBI: LLCC/EBI data bus
> - */
> -enum DPU_POWER_HANDLE_DBUS_ID {
> -	DPU_POWER_HANDLE_DBUS_ID_MNOC,
> -	DPU_POWER_HANDLE_DBUS_ID_LLCC,
> -	DPU_POWER_HANDLE_DBUS_ID_EBI,
> -	DPU_POWER_HANDLE_DBUS_ID_MAX,
> -};
> -
>  /*
>   * struct dpu_power_event - local event registration structure
>   * @client_name: name of the client registering

-- 
Jeykumar S
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/12] drm/msm: dpu: Remove dpu_power_handle

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Now that we don't have any event handlers, remove dpu_power_handle!

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/Makefile                  |   1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |  11 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h       |   3 -
 .../gpu/drm/msm/disp/dpu1/dpu_power_handle.c  | 136 ------------------
 .../gpu/drm/msm/disp/dpu1/dpu_power_handle.h  | 113 ---------------
 5 files changed, 264 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 19ab521d4c3a..7d02ef3655b5 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -72,7 +72,6 @@ msm-y := \
 	disp/dpu1/dpu_kms.o \
 	disp/dpu1/dpu_mdss.o \
 	disp/dpu1/dpu_plane.o \
-	disp/dpu1/dpu_power_handle.o \
 	disp/dpu1/dpu_rm.o \
 	disp/dpu1/dpu_vbif.o \
 	msm_atomic.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index bae7e86b2913..e42685a1d928 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1063,8 +1063,6 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)
 		return ret;
 	}
 
-	dpu_power_resource_init(pdev, &dpu_kms->phandle);
-
 	platform_set_drvdata(pdev, dpu_kms);
 
 	msm_kms_init(&dpu_kms->base, &kms_funcs);
@@ -1084,7 +1082,6 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data)
 	struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
 	struct dss_module_power *mp = &dpu_kms->mp;
 
-	dpu_power_resource_deinit(pdev, &dpu_kms->phandle);
 	msm_dss_put_clk(mp->clk_config, mp->num_clk);
 	devm_kfree(&pdev->dev, mp->clk_config);
 	mp->num_clk = 0;
@@ -1123,10 +1120,6 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev)
 		return rc;
 	}
 
-	rc = dpu_power_resource_enable(&dpu_kms->phandle, false);
-	if (rc)
-		DPU_ERROR("resource disable failed: %d\n", rc);
-
 	rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false);
 	if (rc)
 		DPU_ERROR("clock disable failed rc:%d\n", rc);
@@ -1160,10 +1153,6 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev)
 	drm_for_each_crtc(crtc, ddev)
 		dpu_crtc_runtime_resume(crtc);
 
-	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
-	if (rc)
-		DPU_ERROR("resource enable failed: %d\n", rc);
-
 	return rc;
 }
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 4e5acacb3065..59e18e2d3c59 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -31,7 +31,6 @@
 #include "dpu_hw_top.h"
 #include "dpu_io_util.h"
 #include "dpu_rm.h"
-#include "dpu_power_handle.h"
 #include "dpu_irq.h"
 #include "dpu_core_perf.h"
 
@@ -114,8 +113,6 @@ struct dpu_kms {
 	int core_rev;
 	struct dpu_mdss_cfg *catalog;
 
-	struct dpu_power_handle phandle;
-
 	/* directory entry for debugfs */
 	struct dentry *debugfs_root;
 	struct dentry *debugfs_danger;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
deleted file mode 100644
index 8e64f0a52147..000000000000
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
+++ /dev/null
@@ -1,136 +0,0 @@
-/* Copyright (c) 2014-2018, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-
-#define pr_fmt(fmt)	"[drm:%s:%d]: " fmt, __func__, __LINE__
-
-#include <linux/kernel.h>
-#include <linux/of.h>
-#include <linux/string.h>
-#include <linux/of_address.h>
-#include <linux/slab.h>
-#include <linux/mutex.h>
-#include <linux/of_platform.h>
-
-#include "dpu_power_handle.h"
-#include "dpu_trace.h"
-
-static void dpu_power_event_trigger_locked(struct dpu_power_handle *phandle,
-		u32 event_type)
-{
-	struct dpu_power_event *event;
-
-	list_for_each_entry(event, &phandle->event_list, list) {
-		if (event->event_type & event_type)
-			event->cb_fnc(event_type, event->usr);
-	}
-}
-
-void dpu_power_resource_init(struct platform_device *pdev,
-	struct dpu_power_handle *phandle)
-{
-	phandle->dev = &pdev->dev;
-
-	INIT_LIST_HEAD(&phandle->event_list);
-
-	mutex_init(&phandle->phandle_lock);
-}
-
-void dpu_power_resource_deinit(struct platform_device *pdev,
-	struct dpu_power_handle *phandle)
-{
-	struct dpu_power_event *curr_event, *next_event;
-
-	if (!phandle || !pdev) {
-		pr_err("invalid input param\n");
-		return;
-	}
-
-	mutex_lock(&phandle->phandle_lock);
-	list_for_each_entry_safe(curr_event, next_event,
-			&phandle->event_list, list) {
-		pr_err("event:%d, client:%s still registered\n",
-				curr_event->event_type,
-				curr_event->client_name);
-		curr_event->active = false;
-		list_del(&curr_event->list);
-	}
-	mutex_unlock(&phandle->phandle_lock);
-}
-
-int dpu_power_resource_enable(struct dpu_power_handle *phandle, bool enable)
-{
-	u32 event_type;
-
-	if (!phandle) {
-		pr_err("invalid input argument\n");
-		return -EINVAL;
-	}
-
-	mutex_lock(&phandle->phandle_lock);
-
-	event_type = enable ? DPU_POWER_EVENT_ENABLE : DPU_POWER_EVENT_DISABLE;
-
-	dpu_power_event_trigger_locked(phandle,	event_type);
-
-	mutex_unlock(&phandle->phandle_lock);
-	return 0;
-}
-
-struct dpu_power_event *dpu_power_handle_register_event(
-		struct dpu_power_handle *phandle,
-		u32 event_type, void (*cb_fnc)(u32 event_type, void *usr),
-		void *usr, char *client_name)
-{
-	struct dpu_power_event *event;
-
-	if (!phandle) {
-		pr_err("invalid power handle\n");
-		return ERR_PTR(-EINVAL);
-	} else if (!cb_fnc || !event_type) {
-		pr_err("no callback fnc or event type\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	event = kzalloc(sizeof(struct dpu_power_event), GFP_KERNEL);
-	if (!event)
-		return ERR_PTR(-ENOMEM);
-
-	event->event_type = event_type;
-	event->cb_fnc = cb_fnc;
-	event->usr = usr;
-	strlcpy(event->client_name, client_name, MAX_CLIENT_NAME_LEN);
-	event->active = true;
-
-	mutex_lock(&phandle->phandle_lock);
-	list_add(&event->list, &phandle->event_list);
-	mutex_unlock(&phandle->phandle_lock);
-
-	return event;
-}
-
-void dpu_power_handle_unregister_event(
-		struct dpu_power_handle *phandle,
-		struct dpu_power_event *event)
-{
-	if (!phandle || !event) {
-		pr_err("invalid phandle or event\n");
-	} else if (!event->active) {
-		pr_err("power handle deinit already done\n");
-		kfree(event);
-	} else {
-		mutex_lock(&phandle->phandle_lock);
-		list_del_init(&event->list);
-		mutex_unlock(&phandle->phandle_lock);
-		kfree(event);
-	}
-}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
deleted file mode 100644
index 7536624c8b20..000000000000
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
+++ /dev/null
@@ -1,113 +0,0 @@
-/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-
-#ifndef _DPU_POWER_HANDLE_H_
-#define _DPU_POWER_HANDLE_H_
-
-#define MAX_CLIENT_NAME_LEN 128
-
-#define DPU_POWER_HANDLE_ENABLE_BUS_AB_QUOTA	0
-#define DPU_POWER_HANDLE_DISABLE_BUS_AB_QUOTA	0
-#define DPU_POWER_HANDLE_ENABLE_BUS_IB_QUOTA	1600000000
-#define DPU_POWER_HANDLE_DISABLE_BUS_IB_QUOTA	0
-
-#include "dpu_io_util.h"
-
-/* events will be triggered on power handler enable/disable */
-#define DPU_POWER_EVENT_DISABLE	BIT(0)
-#define DPU_POWER_EVENT_ENABLE	BIT(1)
-
-/*
- * struct dpu_power_event - local event registration structure
- * @client_name: name of the client registering
- * @cb_fnc: pointer to desired callback function
- * @usr: user pointer to pass to callback event trigger
- * @event: refer to DPU_POWER_HANDLE_EVENT_*
- * @list: list to attach event master list
- * @active: indicates the state of dpu power handle
- */
-struct dpu_power_event {
-	char client_name[MAX_CLIENT_NAME_LEN];
-	void (*cb_fnc)(u32 event_type, void *usr);
-	void *usr;
-	u32 event_type;
-	struct list_head list;
-	bool active;
-};
-
-/**
- * struct dpu_power_handle: power handle main struct
- * @phandle_lock: lock to synchronize the enable/disable
- * @dev: pointer to device structure
- * @usecase_ndx: current usecase index
- * @event_list: current power handle event list
- */
-struct dpu_power_handle {
-	struct mutex phandle_lock;
-	struct device *dev;
-	u32 current_usecase_ndx;
-	struct list_head event_list;
-};
-
-/**
- * dpu_power_resource_init() - initializes the dpu power handle
- * @pdev:   platform device to search the power resources
- * @pdata:  power handle to store the power resources
- */
-void dpu_power_resource_init(struct platform_device *pdev,
-	struct dpu_power_handle *pdata);
-
-/**
- * dpu_power_resource_deinit() - release the dpu power handle
- * @pdev:   platform device for power resources
- * @pdata:  power handle containing the resources
- *
- * Return: error code.
- */
-void dpu_power_resource_deinit(struct platform_device *pdev,
-	struct dpu_power_handle *pdata);
-
-/**
- * dpu_power_resource_enable() - enable/disable the power resources
- * @pdata:  power handle containing the resources
- * @enable: boolean request for enable/disable
- *
- * Return: error code.
- */
-int dpu_power_resource_enable(struct dpu_power_handle *pdata, bool enable);
-
-/**
- * dpu_power_handle_register_event - register a callback function for an event.
- *	Clients can register for multiple events with a single register.
- *	Any block with access to phandle can register for the event
- *	notification.
- * @phandle:	power handle containing the resources
- * @event_type:	event type to register; refer DPU_POWER_HANDLE_EVENT_*
- * @cb_fnc:	pointer to desired callback function
- * @usr:	user pointer to pass to callback on event trigger
- *
- * Return:	event pointer if success, or error code otherwise
- */
-struct dpu_power_event *dpu_power_handle_register_event(
-		struct dpu_power_handle *phandle,
-		u32 event_type, void (*cb_fnc)(u32 event_type, void *usr),
-		void *usr, char *client_name);
-/**
- * dpu_power_handle_unregister_event - unregister callback for event(s)
- * @phandle:	power handle containing the resources
- * @event:	event pointer returned after power handle register
- */
-void dpu_power_handle_unregister_event(struct dpu_power_handle *phandle,
-		struct dpu_power_event *event);
-
-#endif /* _DPU_POWER_HANDLE_H_ */
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/12] drm/msm: dpu: Remove dpu_power_handle

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Now that we don't have any event handlers, remove dpu_power_handle!
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

> ---
>  drivers/gpu/drm/msm/Makefile                  |   1 -
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |  11 --
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h       |   3 -
>  .../gpu/drm/msm/disp/dpu1/dpu_power_handle.c  | 136 ------------------
>  .../gpu/drm/msm/disp/dpu1/dpu_power_handle.h  | 113 ---------------
>  5 files changed, 264 deletions(-)
>  delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
>  delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> 
> diff --git a/drivers/gpu/drm/msm/Makefile 
> b/drivers/gpu/drm/msm/Makefile
> index 19ab521d4c3a..7d02ef3655b5 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -72,7 +72,6 @@ msm-y := \
>  	disp/dpu1/dpu_kms.o \
>  	disp/dpu1/dpu_mdss.o \
>  	disp/dpu1/dpu_plane.o \
> -	disp/dpu1/dpu_power_handle.o \
>  	disp/dpu1/dpu_rm.o \
>  	disp/dpu1/dpu_vbif.o \
>  	msm_atomic.o \
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index bae7e86b2913..e42685a1d928 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1063,8 +1063,6 @@ static int dpu_bind(struct device *dev, struct
> device *master, void *data)
>  		return ret;
>  	}
> 
> -	dpu_power_resource_init(pdev, &dpu_kms->phandle);
> -
>  	platform_set_drvdata(pdev, dpu_kms);
> 
>  	msm_kms_init(&dpu_kms->base, &kms_funcs);
> @@ -1084,7 +1082,6 @@ static void dpu_unbind(struct device *dev, struct
> device *master, void *data)
>  	struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
>  	struct dss_module_power *mp = &dpu_kms->mp;
> 
> -	dpu_power_resource_deinit(pdev, &dpu_kms->phandle);
>  	msm_dss_put_clk(mp->clk_config, mp->num_clk);
>  	devm_kfree(&pdev->dev, mp->clk_config);
>  	mp->num_clk = 0;
> @@ -1123,10 +1120,6 @@ static int __maybe_unused
> dpu_runtime_suspend(struct device *dev)
>  		return rc;
>  	}
> 
> -	rc = dpu_power_resource_enable(&dpu_kms->phandle, false);
> -	if (rc)
> -		DPU_ERROR("resource disable failed: %d\n", rc);
> -
>  	rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false);
>  	if (rc)
>  		DPU_ERROR("clock disable failed rc:%d\n", rc);
> @@ -1160,10 +1153,6 @@ static int __maybe_unused 
> dpu_runtime_resume(struct
> device *dev)
>  	drm_for_each_crtc(crtc, ddev)
>  		dpu_crtc_runtime_resume(crtc);
> 
> -	rc = dpu_power_resource_enable(&dpu_kms->phandle, true);
> -	if (rc)
> -		DPU_ERROR("resource enable failed: %d\n", rc);
> -
>  	return rc;
>  }
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 4e5acacb3065..59e18e2d3c59 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -31,7 +31,6 @@
>  #include "dpu_hw_top.h"
>  #include "dpu_io_util.h"
>  #include "dpu_rm.h"
> -#include "dpu_power_handle.h"
>  #include "dpu_irq.h"
>  #include "dpu_core_perf.h"
> 
> @@ -114,8 +113,6 @@ struct dpu_kms {
>  	int core_rev;
>  	struct dpu_mdss_cfg *catalog;
> 
> -	struct dpu_power_handle phandle;
> -
>  	/* directory entry for debugfs */
>  	struct dentry *debugfs_root;
>  	struct dentry *debugfs_danger;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> deleted file mode 100644
> index 8e64f0a52147..000000000000
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
> +++ /dev/null
> @@ -1,136 +0,0 @@
> -/* Copyright (c) 2014-2018, The Linux Foundation. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or 
> modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - *
> - */
> -
> -#define pr_fmt(fmt)	"[drm:%s:%d]: " fmt, __func__, __LINE__
> -
> -#include <linux/kernel.h>
> -#include <linux/of.h>
> -#include <linux/string.h>
> -#include <linux/of_address.h>
> -#include <linux/slab.h>
> -#include <linux/mutex.h>
> -#include <linux/of_platform.h>
> -
> -#include "dpu_power_handle.h"
> -#include "dpu_trace.h"
> -
> -static void dpu_power_event_trigger_locked(struct dpu_power_handle
> *phandle,
> -		u32 event_type)
> -{
> -	struct dpu_power_event *event;
> -
> -	list_for_each_entry(event, &phandle->event_list, list) {
> -		if (event->event_type & event_type)
> -			event->cb_fnc(event_type, event->usr);
> -	}
> -}
> -
> -void dpu_power_resource_init(struct platform_device *pdev,
> -	struct dpu_power_handle *phandle)
> -{
> -	phandle->dev = &pdev->dev;
> -
> -	INIT_LIST_HEAD(&phandle->event_list);
> -
> -	mutex_init(&phandle->phandle_lock);
> -}
> -
> -void dpu_power_resource_deinit(struct platform_device *pdev,
> -	struct dpu_power_handle *phandle)
> -{
> -	struct dpu_power_event *curr_event, *next_event;
> -
> -	if (!phandle || !pdev) {
> -		pr_err("invalid input param\n");
> -		return;
> -	}
> -
> -	mutex_lock(&phandle->phandle_lock);
> -	list_for_each_entry_safe(curr_event, next_event,
> -			&phandle->event_list, list) {
> -		pr_err("event:%d, client:%s still registered\n",
> -				curr_event->event_type,
> -				curr_event->client_name);
> -		curr_event->active = false;
> -		list_del(&curr_event->list);
> -	}
> -	mutex_unlock(&phandle->phandle_lock);
> -}
> -
> -int dpu_power_resource_enable(struct dpu_power_handle *phandle, bool
> enable)
> -{
> -	u32 event_type;
> -
> -	if (!phandle) {
> -		pr_err("invalid input argument\n");
> -		return -EINVAL;
> -	}
> -
> -	mutex_lock(&phandle->phandle_lock);
> -
> -	event_type = enable ? DPU_POWER_EVENT_ENABLE :
> DPU_POWER_EVENT_DISABLE;
> -
> -	dpu_power_event_trigger_locked(phandle,	event_type);
> -
> -	mutex_unlock(&phandle->phandle_lock);
> -	return 0;
> -}
> -
> -struct dpu_power_event *dpu_power_handle_register_event(
> -		struct dpu_power_handle *phandle,
> -		u32 event_type, void (*cb_fnc)(u32 event_type, void *usr),
> -		void *usr, char *client_name)
> -{
> -	struct dpu_power_event *event;
> -
> -	if (!phandle) {
> -		pr_err("invalid power handle\n");
> -		return ERR_PTR(-EINVAL);
> -	} else if (!cb_fnc || !event_type) {
> -		pr_err("no callback fnc or event type\n");
> -		return ERR_PTR(-EINVAL);
> -	}
> -
> -	event = kzalloc(sizeof(struct dpu_power_event), GFP_KERNEL);
> -	if (!event)
> -		return ERR_PTR(-ENOMEM);
> -
> -	event->event_type = event_type;
> -	event->cb_fnc = cb_fnc;
> -	event->usr = usr;
> -	strlcpy(event->client_name, client_name, MAX_CLIENT_NAME_LEN);
> -	event->active = true;
> -
> -	mutex_lock(&phandle->phandle_lock);
> -	list_add(&event->list, &phandle->event_list);
> -	mutex_unlock(&phandle->phandle_lock);
> -
> -	return event;
> -}
> -
> -void dpu_power_handle_unregister_event(
> -		struct dpu_power_handle *phandle,
> -		struct dpu_power_event *event)
> -{
> -	if (!phandle || !event) {
> -		pr_err("invalid phandle or event\n");
> -	} else if (!event->active) {
> -		pr_err("power handle deinit already done\n");
> -		kfree(event);
> -	} else {
> -		mutex_lock(&phandle->phandle_lock);
> -		list_del_init(&event->list);
> -		mutex_unlock(&phandle->phandle_lock);
> -		kfree(event);
> -	}
> -}
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> deleted file mode 100644
> index 7536624c8b20..000000000000
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
> +++ /dev/null
> @@ -1,113 +0,0 @@
> -/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or 
> modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - *
> - */
> -
> -#ifndef _DPU_POWER_HANDLE_H_
> -#define _DPU_POWER_HANDLE_H_
> -
> -#define MAX_CLIENT_NAME_LEN 128
> -
> -#define DPU_POWER_HANDLE_ENABLE_BUS_AB_QUOTA	0
> -#define DPU_POWER_HANDLE_DISABLE_BUS_AB_QUOTA	0
> -#define DPU_POWER_HANDLE_ENABLE_BUS_IB_QUOTA	1600000000
> -#define DPU_POWER_HANDLE_DISABLE_BUS_IB_QUOTA	0
> -
> -#include "dpu_io_util.h"
> -
> -/* events will be triggered on power handler enable/disable */
> -#define DPU_POWER_EVENT_DISABLE	BIT(0)
> -#define DPU_POWER_EVENT_ENABLE	BIT(1)
> -
> -/*
> - * struct dpu_power_event - local event registration structure
> - * @client_name: name of the client registering
> - * @cb_fnc: pointer to desired callback function
> - * @usr: user pointer to pass to callback event trigger
> - * @event: refer to DPU_POWER_HANDLE_EVENT_*
> - * @list: list to attach event master list
> - * @active: indicates the state of dpu power handle
> - */
> -struct dpu_power_event {
> -	char client_name[MAX_CLIENT_NAME_LEN];
> -	void (*cb_fnc)(u32 event_type, void *usr);
> -	void *usr;
> -	u32 event_type;
> -	struct list_head list;
> -	bool active;
> -};
> -
> -/**
> - * struct dpu_power_handle: power handle main struct
> - * @phandle_lock: lock to synchronize the enable/disable
> - * @dev: pointer to device structure
> - * @usecase_ndx: current usecase index
> - * @event_list: current power handle event list
> - */
> -struct dpu_power_handle {
> -	struct mutex phandle_lock;
> -	struct device *dev;
> -	u32 current_usecase_ndx;
> -	struct list_head event_list;
> -};
> -
> -/**
> - * dpu_power_resource_init() - initializes the dpu power handle
> - * @pdev:   platform device to search the power resources
> - * @pdata:  power handle to store the power resources
> - */
> -void dpu_power_resource_init(struct platform_device *pdev,
> -	struct dpu_power_handle *pdata);
> -
> -/**
> - * dpu_power_resource_deinit() - release the dpu power handle
> - * @pdev:   platform device for power resources
> - * @pdata:  power handle containing the resources
> - *
> - * Return: error code.
> - */
> -void dpu_power_resource_deinit(struct platform_device *pdev,
> -	struct dpu_power_handle *pdata);
> -
> -/**
> - * dpu_power_resource_enable() - enable/disable the power resources
> - * @pdata:  power handle containing the resources
> - * @enable: boolean request for enable/disable
> - *
> - * Return: error code.
> - */
> -int dpu_power_resource_enable(struct dpu_power_handle *pdata, bool
> enable);
> -
> -/**
> - * dpu_power_handle_register_event - register a callback function for 
> an
> event.
> - *	Clients can register for multiple events with a single register.
> - *	Any block with access to phandle can register for the event
> - *	notification.
> - * @phandle:	power handle containing the resources
> - * @event_type:	event type to register; refer
> DPU_POWER_HANDLE_EVENT_*
> - * @cb_fnc:	pointer to desired callback function
> - * @usr:	user pointer to pass to callback on event trigger
> - *
> - * Return:	event pointer if success, or error code otherwise
> - */
> -struct dpu_power_event *dpu_power_handle_register_event(
> -		struct dpu_power_handle *phandle,
> -		u32 event_type, void (*cb_fnc)(u32 event_type, void *usr),
> -		void *usr, char *client_name);
> -/**
> - * dpu_power_handle_unregister_event - unregister callback for 
> event(s)
> - * @phandle:	power handle containing the resources
> - * @event:	event pointer returned after power handle register
> - */
> -void dpu_power_handle_unregister_event(struct dpu_power_handle 
> *phandle,
> -		struct dpu_power_event *event);
> -
> -#endif /* _DPU_POWER_HANDLE_H_ */

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 11/12] drm/msm: dpu: Add ->enabled to dpu_encoder_virt

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Add a bool to dpu_encoder_virt to track whether the encoder is enabled
or not. Repurpose the enc_lock mutex to ensure that it is consistent
with the hw state.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 +++++++++++++++++----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 10a0676d1dcf..3daa86220d47 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -132,6 +132,7 @@ enum dpu_enc_rc_states {
  * @base:		drm_encoder base class for registration with DRM
  * @enc_spinlock:	Virtual-Encoder-Wide Spin Lock for IRQ purposes
  * @bus_scaling_client:	Client handle to the bus scaling interface
+ * @enabled:		True if the encoder is active, protected by enc_lock
  * @num_phys_encs:	Actual number of physical encoders contained.
  * @phys_encs:		Container of physical encoders managed.
  * @cur_master:		Pointer to the current master in this mode. Optimization
@@ -148,8 +149,8 @@ enum dpu_enc_rc_states {
  *				all CTL paths
  * @crtc_kickoff_cb_data:	Opaque user data given to crtc_kickoff_cb
  * @debugfs_root:		Debug file system root file node
- * @enc_lock:			Lock around physical encoder create/destroy and
-				access.
+ * @enc_lock:			Lock around physical encoder
+ *				create/destroy/enable/disable
  * @frame_busy_mask:		Bitmask tracking which phys_enc we are still
  *				busy processing current command.
  *				Bit0 = phys_encs[0] etc.
@@ -175,6 +176,8 @@ struct dpu_encoder_virt {
 	spinlock_t enc_spinlock;
 	uint32_t bus_scaling_client;
 
+	bool enabled;
+
 	unsigned int num_phys_encs;
 	struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL];
 	struct dpu_encoder_phys *cur_master;
@@ -1121,6 +1124,8 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
 		return;
 	}
 	dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+	mutex_lock(&dpu_enc->enc_lock);
 	cur_mode = &dpu_enc->base.crtc->state->adjusted_mode;
 
 	trace_dpu_enc_enable(DRMID(drm_enc), cur_mode->hdisplay,
@@ -1137,10 +1142,15 @@ static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
 	if (ret) {
 		DPU_ERROR_ENC(dpu_enc, "dpu resource control failed: %d\n",
 				ret);
-		return;
+		goto out;
 	}
 
 	_dpu_encoder_virt_enable_helper(drm_enc);
+
+	dpu_enc->enabled = true;
+
+out:
+	mutex_unlock(&dpu_enc->enc_lock);
 }
 
 static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
@@ -1162,11 +1172,14 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
 		return;
 	}
 
-	mode = &drm_enc->crtc->state->adjusted_mode;
-
 	dpu_enc = to_dpu_encoder_virt(drm_enc);
 	DPU_DEBUG_ENC(dpu_enc, "\n");
 
+	mutex_lock(&dpu_enc->enc_lock);
+	dpu_enc->enabled = false;
+
+	mode = &drm_enc->crtc->state->adjusted_mode;
+
 	priv = drm_enc->dev->dev_private;
 	dpu_kms = to_dpu_kms(priv->kms);
 
@@ -1200,6 +1213,8 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
 	DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n");
 
 	dpu_rm_release(&dpu_kms->rm, drm_enc);
+
+	mutex_unlock(&dpu_enc->enc_lock);
 }
 
 static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg *catalog,
@@ -2233,6 +2248,8 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
 
 	drm_encoder_helper_add(&dpu_enc->base, &dpu_encoder_helper_funcs);
 
+	dpu_enc->enabled = false;
+
 	return &dpu_enc->base;
 }
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/12] drm/msm: dpu: Add ->enabled to dpu_encoder_virt

[Jeykumar Sankaran]

On 2018-11-12 11:42, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Add a bool to dpu_encoder_virt to track whether the encoder is enabled
> or not. Repurpose the enc_lock mutex to ensure that it is consistent
> with the hw state.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 +++++++++++++++++----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 10a0676d1dcf..3daa86220d47 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -132,6 +132,7 @@ enum dpu_enc_rc_states {
>   * @base:		drm_encoder base class for registration with DRM
>   * @enc_spinlock:	Virtual-Encoder-Wide Spin Lock for IRQ purposes
>   * @bus_scaling_client:	Client handle to the bus scaling interface
> + * @enabled:		True if the encoder is active, protected by
> enc_lock
>   * @num_phys_encs:	Actual number of physical encoders contained.
>   * @phys_encs:		Container of physical encoders managed.
>   * @cur_master:		Pointer to the current master in this
> mode. Optimization
> @@ -148,8 +149,8 @@ enum dpu_enc_rc_states {
>   *				all CTL paths
>   * @crtc_kickoff_cb_data:	Opaque user data given to crtc_kickoff_cb
>   * @debugfs_root:		Debug file system root file node
> - * @enc_lock:			Lock around physical encoder
> create/destroy and
> -				access.
> + * @enc_lock:			Lock around physical encoder
> + *				create/destroy/enable/disable
>   * @frame_busy_mask:		Bitmask tracking which phys_enc we are
> still
>   *				busy processing current command.
>   *				Bit0 = phys_encs[0] etc.
> @@ -175,6 +176,8 @@ struct dpu_encoder_virt {
>  	spinlock_t enc_spinlock;
>  	uint32_t bus_scaling_client;
> 
> +	bool enabled;
> +
>  	unsigned int num_phys_encs;
>  	struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL];
>  	struct dpu_encoder_phys *cur_master;
> @@ -1121,6 +1124,8 @@ static void dpu_encoder_virt_enable(struct
> drm_encoder *drm_enc)
>  		return;
>  	}
>  	dpu_enc = to_dpu_encoder_virt(drm_enc);
> +
> +	mutex_lock(&dpu_enc->enc_lock);
>  	cur_mode = &dpu_enc->base.crtc->state->adjusted_mode;
> 
>  	trace_dpu_enc_enable(DRMID(drm_enc), cur_mode->hdisplay,
> @@ -1137,10 +1142,15 @@ static void dpu_encoder_virt_enable(struct
> drm_encoder *drm_enc)
>  	if (ret) {
>  		DPU_ERROR_ENC(dpu_enc, "dpu resource control failed:
> %d\n",
>  				ret);
> -		return;
> +		goto out;
>  	}
> 
>  	_dpu_encoder_virt_enable_helper(drm_enc);
> +
> +	dpu_enc->enabled = true;
> +
> +out:
> +	mutex_unlock(&dpu_enc->enc_lock);
>  }
> 
>  static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
> @@ -1162,11 +1172,14 @@ static void dpu_encoder_virt_disable(struct
> drm_encoder *drm_enc)
>  		return;
>  	}
> 
> -	mode = &drm_enc->crtc->state->adjusted_mode;
> -
>  	dpu_enc = to_dpu_encoder_virt(drm_enc);
>  	DPU_DEBUG_ENC(dpu_enc, "\n");
> 
> +	mutex_lock(&dpu_enc->enc_lock);
Where do you expect it to go wrong if enable/disable
is not protected using enc_lock?

Thanks and Regards,
Jeykumar S.
> +	dpu_enc->enabled = false;
> +
> +	mode = &drm_enc->crtc->state->adjusted_mode;
> +
>  	priv = drm_enc->dev->dev_private;
>  	dpu_kms = to_dpu_kms(priv->kms);
> 
> @@ -1200,6 +1213,8 @@ static void dpu_encoder_virt_disable(struct
> drm_encoder *drm_enc)
>  	DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n");
> 
>  	dpu_rm_release(&dpu_kms->rm, drm_enc);
> +
> +	mutex_unlock(&dpu_enc->enc_lock);
>  }
> 
>  static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg 
> *catalog,
> @@ -2233,6 +2248,8 @@ struct drm_encoder *dpu_encoder_init(struct
> drm_device *dev,
> 
>  	drm_encoder_helper_add(&dpu_enc->base, &dpu_encoder_helper_funcs);
> 
> +	dpu_enc->enabled = false;
> +
>  	return &dpu_enc->base;
>  }

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 11/12] drm/msm: dpu: Add ->enabled to dpu_encoder_virt

[Sean Paul]

On Mon, Nov 12, 2018 at 05:43:17PM -0800, Jeykumar Sankaran wrote:
> On 2018-11-12 11:42, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > Add a bool to dpu_encoder_virt to track whether the encoder is enabled
> > or not. Repurpose the enc_lock mutex to ensure that it is consistent
> > with the hw state.
> > 
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 +++++++++++++++++----
> >  1 file changed, 22 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 10a0676d1dcf..3daa86220d47 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -132,6 +132,7 @@ enum dpu_enc_rc_states {
> >   * @base:		drm_encoder base class for registration with DRM
> >   * @enc_spinlock:	Virtual-Encoder-Wide Spin Lock for IRQ purposes
> >   * @bus_scaling_client:	Client handle to the bus scaling interface
> > + * @enabled:		True if the encoder is active, protected by
> > enc_lock
> >   * @num_phys_encs:	Actual number of physical encoders contained.
> >   * @phys_encs:		Container of physical encoders managed.
> >   * @cur_master:		Pointer to the current master in this
> > mode. Optimization
> > @@ -148,8 +149,8 @@ enum dpu_enc_rc_states {
> >   *				all CTL paths
> >   * @crtc_kickoff_cb_data:	Opaque user data given to crtc_kickoff_cb
> >   * @debugfs_root:		Debug file system root file node
> > - * @enc_lock:			Lock around physical encoder
> > create/destroy and
> > -				access.
> > + * @enc_lock:			Lock around physical encoder
> > + *				create/destroy/enable/disable
> >   * @frame_busy_mask:		Bitmask tracking which phys_enc we are
> > still
> >   *				busy processing current command.
> >   *				Bit0 = phys_encs[0] etc.
> > @@ -175,6 +176,8 @@ struct dpu_encoder_virt {
> >  	spinlock_t enc_spinlock;
> >  	uint32_t bus_scaling_client;
> > 
> > +	bool enabled;
> > +
> >  	unsigned int num_phys_encs;
> >  	struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL];
> >  	struct dpu_encoder_phys *cur_master;
> > @@ -1121,6 +1124,8 @@ static void dpu_encoder_virt_enable(struct
> > drm_encoder *drm_enc)
> >  		return;
> >  	}
> >  	dpu_enc = to_dpu_encoder_virt(drm_enc);
> > +
> > +	mutex_lock(&dpu_enc->enc_lock);
> >  	cur_mode = &dpu_enc->base.crtc->state->adjusted_mode;
> > 
> >  	trace_dpu_enc_enable(DRMID(drm_enc), cur_mode->hdisplay,
> > @@ -1137,10 +1142,15 @@ static void dpu_encoder_virt_enable(struct
> > drm_encoder *drm_enc)
> >  	if (ret) {
> >  		DPU_ERROR_ENC(dpu_enc, "dpu resource control failed:
> > %d\n",
> >  				ret);
> > -		return;
> > +		goto out;
> >  	}
> > 
> >  	_dpu_encoder_virt_enable_helper(drm_enc);
> > +
> > +	dpu_enc->enabled = true;
> > +
> > +out:
> > +	mutex_unlock(&dpu_enc->enc_lock);
> >  }
> > 
> >  static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
> > @@ -1162,11 +1172,14 @@ static void dpu_encoder_virt_disable(struct
> > drm_encoder *drm_enc)
> >  		return;
> >  	}
> > 
> > -	mode = &drm_enc->crtc->state->adjusted_mode;
> > -
> >  	dpu_enc = to_dpu_encoder_virt(drm_enc);
> >  	DPU_DEBUG_ENC(dpu_enc, "\n");
> > 
> > +	mutex_lock(&dpu_enc->enc_lock);
> Where do you expect it to go wrong if enable/disable
> is not protected using enc_lock?

runtime_resume can run concurrently with either enable or disable. We need the
enc_lock to ensure that enable/disable/runtime_resume are all relatively atomic.

Sean


> 
> Thanks and Regards,
> Jeykumar S.
> > +	dpu_enc->enabled = false;
> > +
> > +	mode = &drm_enc->crtc->state->adjusted_mode;
> > +
> >  	priv = drm_enc->dev->dev_private;
> >  	dpu_kms = to_dpu_kms(priv->kms);
> > 
> > @@ -1200,6 +1213,8 @@ static void dpu_encoder_virt_disable(struct
> > drm_encoder *drm_enc)
> >  	DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n");
> > 
> >  	dpu_rm_release(&dpu_kms->rm, drm_enc);
> > +
> > +	mutex_unlock(&dpu_enc->enc_lock);
> >  }
> > 
> >  static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg *catalog,
> > @@ -2233,6 +2248,8 @@ struct drm_encoder *dpu_encoder_init(struct
> > drm_device *dev,
> > 
> >  	drm_encoder_helper_add(&dpu_enc->base, &dpu_encoder_helper_funcs);
> > 
> > +	dpu_enc->enabled = false;
> > +
> >  	return &dpu_enc->base;
> >  }
> 
> -- 
> Jeykumar S

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno