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

Jeykumar Sankaran
2018-11-19T19:41:28Z

On 2018-11-16 10: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.
>
> Changes in v2:
> - None
>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.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 ab8574ab8327..654ea5060e02 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1137,6 +1137,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;
>
> @@ -1154,6 +1155,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