Re: [Freedreno] [PATCH v2 19/24] drm/msm: dpu: Remove vblank_callback from encoder

2018-11-19 Thread Jeykumar Sankaran

On 2018-11-16 10:42, Sean Paul wrote:

From: Sean Paul 

The indirection of registering a callback and opaque pointer isn't 
reall

useful when there's only one callsite. So instead of having the
vblank_cb registration, just give encoder a crtc and let it directly
call the vblank handler.

In a later patch, we'll make use of this further.

Changes in v2:
- None

Cc: Jeykumar Sankaran 
Signed-off-by: Sean Paul 


Reviewed-by: Jeykumar Sankaran 


---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c|  8 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h|  6 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 10 -
 4 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 141ed1b0e90a..4d7f9ff1e9f4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -293,9 +293,8 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct
drm_crtc *crtc)
return INTF_MODE_NONE;
 }

-static void dpu_crtc_vblank_cb(void *data)
+void dpu_crtc_vblank_callback(struct drm_crtc *crtc)
 {
-   struct drm_crtc *crtc = (struct drm_crtc *)data;
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);

/* keep statistics on vblank callback - with auto reset via
debugfs */
@@ -771,8 +770,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
 DRMID(enc), enable,
 dpu_crtc);

-   dpu_encoder_register_vblank_callback(enc,
-   dpu_crtc_vblank_cb, (void *)crtc);
+   dpu_encoder_assign_crtc(enc, crtc);
}
} else {
list_for_each_entry(enc, >mode_config.encoder_list,
head) {
@@ -783,7 +781,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
 DRMID(enc), enable,
 dpu_crtc);

-   dpu_encoder_register_vblank_callback(enc, NULL,
NULL);
+   dpu_encoder_assign_crtc(enc, NULL);
}
}
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 93d21a61a040..54595cc29be5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -270,6 +270,12 @@ static inline int dpu_crtc_frame_pending(struct
drm_crtc *crtc)
  */
 int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);

+/**
+ * dpu_crtc_vblank_callback - called on vblank irq, issues completion
events
+ * @crtc: Pointer to drm crtc object
+ */
+void dpu_crtc_vblank_callback(struct drm_crtc *crtc);
+
 /**
  * dpu_crtc_commit_kickoff - trigger kickoff of the commit for this 
crtc

  * @crtc: Pointer to drm crtc object
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index d89ac520f7e6..fd6514f681ae 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -142,9 +142,11 @@ enum dpu_enc_rc_states {
  * @intfs_swapped  Whether or not the phys_enc interfaces have been
swapped
  * for partial update right-only cases, such as
pingpong
  * split where virtual pingpong does not generate
IRQs
- * @crtc_vblank_cb:Callback into the upper layer / CRTC for
- * notification of the VBLANK
- * @crtc_vblank_cb_data:   Data from upper layer for VBLANK
notification
+ * @crtc:  Pointer to the currently assigned crtc. Normally
you
+ * would use crtc->state->encoder_mask to determine
the
+ * link between encoder/crtc. However in this case we
need
+ * to track crtc in the disable() hook which is
called
+ * _after_ encoder_mask is cleared.
  * @crtc_kickoff_cb:   Callback into CRTC that will flush & start
  * all CTL paths
  * @crtc_kickoff_cb_data:  Opaque user data given to crtc_kickoff_cb
@@ -186,8 +188,7 @@ struct dpu_encoder_virt {

bool intfs_swapped;

-   void (*crtc_vblank_cb)(void *);
-   void *crtc_vblank_cb_data;
+   struct drm_crtc *crtc;

struct dentry *debugfs_root;
struct mutex enc_lock;
@@ -1241,8 +1242,8 @@ static void dpu_encoder_vblank_callback(struct
drm_encoder *drm_enc,
dpu_enc = to_dpu_encoder_virt(drm_enc);

spin_lock_irqsave(_enc->enc_spinlock, lock_flags);
-   if (dpu_enc->crtc_vblank_cb)
-   dpu_enc->crtc_vblank_cb(dpu_enc->crtc_vblank_cb_data);
+   if (dpu_enc->crtc)
+   dpu_crtc_vblank_callback(dpu_enc->crtc);
spin_unlock_irqrestore(_enc->enc_spinlock, lock_flags);

atomic_inc(_enc->vsync_cnt);
@@ 

[Freedreno] [PATCH v2 19/24] drm/msm: dpu: Remove vblank_callback from encoder

2018-11-16 Thread Sean Paul
From: Sean Paul 

The indirection of registering a callback and opaque pointer isn't reall
useful when there's only one callsite. So instead of having the
vblank_cb registration, just give encoder a crtc and let it directly
call the vblank handler.

In a later patch, we'll make use of this further.

Changes in v2:
- None

Cc: Jeykumar Sankaran 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c|  8 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h|  6 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 10 -
 4 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 141ed1b0e90a..4d7f9ff1e9f4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -293,9 +293,8 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct drm_crtc 
*crtc)
return INTF_MODE_NONE;
 }
 
-static void dpu_crtc_vblank_cb(void *data)
+void dpu_crtc_vblank_callback(struct drm_crtc *crtc)
 {
-   struct drm_crtc *crtc = (struct drm_crtc *)data;
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
 
/* keep statistics on vblank callback - with auto reset via debugfs */
@@ -771,8 +770,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
 DRMID(enc), enable,
 dpu_crtc);
 
-   dpu_encoder_register_vblank_callback(enc,
-   dpu_crtc_vblank_cb, (void *)crtc);
+   dpu_encoder_assign_crtc(enc, crtc);
}
} else {
list_for_each_entry(enc, >mode_config.encoder_list, head) {
@@ -783,7 +781,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
 DRMID(enc), enable,
 dpu_crtc);
 
-   dpu_encoder_register_vblank_callback(enc, NULL, NULL);
+   dpu_encoder_assign_crtc(enc, NULL);
}
}
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 93d21a61a040..54595cc29be5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -270,6 +270,12 @@ static inline int dpu_crtc_frame_pending(struct drm_crtc 
*crtc)
  */
 int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
 
+/**
+ * dpu_crtc_vblank_callback - called on vblank irq, issues completion events
+ * @crtc: Pointer to drm crtc object
+ */
+void dpu_crtc_vblank_callback(struct drm_crtc *crtc);
+
 /**
  * dpu_crtc_commit_kickoff - trigger kickoff of the commit for this crtc
  * @crtc: Pointer to drm crtc object
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index d89ac520f7e6..fd6514f681ae 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -142,9 +142,11 @@ enum dpu_enc_rc_states {
  * @intfs_swapped  Whether or not the phys_enc interfaces have been swapped
  * for partial update right-only cases, such as pingpong
  * split where virtual pingpong does not generate IRQs
- * @crtc_vblank_cb:Callback into the upper layer / CRTC for
- * notification of the VBLANK
- * @crtc_vblank_cb_data:   Data from upper layer for VBLANK notification
+ * @crtc:  Pointer to the currently assigned crtc. Normally you
+ * would use crtc->state->encoder_mask to determine the
+ * link between encoder/crtc. However in this case we need
+ * to track crtc in the disable() hook which is called
+ * _after_ encoder_mask is cleared.
  * @crtc_kickoff_cb:   Callback into CRTC that will flush & start
  * all CTL paths
  * @crtc_kickoff_cb_data:  Opaque user data given to crtc_kickoff_cb
@@ -186,8 +188,7 @@ struct dpu_encoder_virt {
 
bool intfs_swapped;
 
-   void (*crtc_vblank_cb)(void *);
-   void *crtc_vblank_cb_data;
+   struct drm_crtc *crtc;
 
struct dentry *debugfs_root;
struct mutex enc_lock;
@@ -1241,8 +1242,8 @@ static void dpu_encoder_vblank_callback(struct 
drm_encoder *drm_enc,
dpu_enc = to_dpu_encoder_virt(drm_enc);
 
spin_lock_irqsave(_enc->enc_spinlock, lock_flags);
-   if (dpu_enc->crtc_vblank_cb)
-   dpu_enc->crtc_vblank_cb(dpu_enc->crtc_vblank_cb_data);
+   if (dpu_enc->crtc)
+   dpu_crtc_vblank_callback(dpu_enc->crtc);
spin_unlock_irqrestore(_enc->enc_spinlock, lock_flags);
 
atomic_inc(_enc->vsync_cnt);
@@ -1262,15 +1263,14 @@ static void dpu_encoder_underrun_callback(struct