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

[Jeykumar Sankaran <jsanka@codeaurora.org>]

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