Re: [PATCH 05/12] drm/msm: dpu: Handle crtc pm_runtime_resume() directly
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
> >
> > 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
> > ---
> > 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(_crtc->crtc_lock);
> > kfree(dpu_crtc);
> > @@ -844,15 +841,17 @@ static struct drm_crtc_state
> > *dpu_crtc_duplicate_state(struct drm_crtc *crtc)
> > return >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(_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(_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(_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 = >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 /*
Re: [PATCH 05/12] drm/msm: dpu: Handle crtc pm_runtime_resume() directly
On 2018-11-12 11:42, Sean Paul wrote:
From: Sean Paul
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
---
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(_crtc->crtc_lock);
kfree(dpu_crtc);
@@ -844,15 +841,17 @@ static struct drm_crtc_state
*dpu_crtc_duplicate_state(struct drm_crtc *crtc)
return >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(_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(_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(_crtc->event_lock);
If this is for synchronizing power events, we can get rid of this too.
Thanks,
Jeykumar S.
- dpu_crtc->phandle = >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;
[PATCH 05/12] drm/msm: dpu: Handle crtc pm_runtime_resume() directly
From: Sean Paul
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
---
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(_crtc->crtc_lock);
kfree(dpu_crtc);
@@ -844,15 +841,17 @@ static struct drm_crtc_state
*dpu_crtc_duplicate_state(struct drm_crtc *crtc)
return >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(_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(_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(_crtc->event_lock);
- dpu_crtc->phandle = >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 = _kms->mp;
@@ -1157,6 +1158,9 @@ static int
