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

[Sean Paul <sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>]

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