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@xxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxx>
> > ---
> > 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
