* [PATCH v3 1/6] drm/msm/dpu: remove extra wrappers around dpu_core_irq
2022-02-17 4:31 [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers Dmitry Baryshkov
@ 2022-02-17 4:31 ` Dmitry Baryshkov
2022-02-17 4:31 ` [PATCH v3 2/6] drm/msm/dpu: remove always-true argument of dpu_core_irq_read() Dmitry Baryshkov
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-02-17 4:31 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Sean Paul, Abhinav Kumar
Cc: Stephen Boyd, David Airlie, Daniel Vetter, linux-arm-msm,
dri-devel, freedreno
Remove extra dpu_irq_* wrappers from dpu_kms.c, merge them directly into
dpu_core_irq_* functions.
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 12 ++++-----
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 9 ++++---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 27 +++----------------
3 files changed, 15 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
index 90ae6c9ccc95..7023ccb79814 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
@@ -10,24 +10,24 @@
/**
* dpu_core_irq_preinstall - perform pre-installation of core IRQ handler
- * @dpu_kms: DPU handle
+ * @kms: MSM KMS handle
* @return: none
*/
-void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms);
+void dpu_core_irq_preinstall(struct msm_kms *kms);
/**
* dpu_core_irq_uninstall - uninstall core IRQ handler
- * @dpu_kms: DPU handle
+ * @kms: MSM KMS handle
* @return: none
*/
-void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms);
+void dpu_core_irq_uninstall(struct msm_kms *kms);
/**
* dpu_core_irq - core IRQ handler
- * @dpu_kms: DPU handle
+ * @kms: MSM KMS handle
* @return: interrupt handling status
*/
-irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms);
+irqreturn_t dpu_core_irq(struct msm_kms *kms);
/**
* dpu_core_irq_read - IRQ helper function for reading IRQ status
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index a77a5eaa78ad..71882d3fe705 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -162,8 +162,9 @@ static void dpu_core_irq_callback_handler(struct dpu_kms *dpu_kms, int irq_idx)
cb->func(cb->arg, irq_idx);
}
-irqreturn_t dpu_core_irq(struct dpu_kms *dpu_kms)
+irqreturn_t dpu_core_irq(struct msm_kms *kms)
{
+ struct dpu_kms *dpu_kms = to_dpu_kms(kms);
struct dpu_hw_intr *intr = dpu_kms->hw_intr;
int reg_idx;
int irq_idx;
@@ -569,8 +570,9 @@ void dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,
}
#endif
-void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms)
+void dpu_core_irq_preinstall(struct msm_kms *kms)
{
+ struct dpu_kms *dpu_kms = to_dpu_kms(kms);
int i;
pm_runtime_get_sync(&dpu_kms->pdev->dev);
@@ -589,8 +591,9 @@ void dpu_core_irq_preinstall(struct dpu_kms *dpu_kms)
}
}
-void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms)
+void dpu_core_irq_uninstall(struct msm_kms *kms)
{
+ struct dpu_kms *dpu_kms = to_dpu_kms(kms);
int i;
pm_runtime_get_sync(&dpu_kms->pdev->dev);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 47fe11a84a77..364bd4e60fba 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -841,20 +841,6 @@ static void dpu_kms_destroy(struct msm_kms *kms)
msm_kms_destroy(&dpu_kms->base);
}
-static irqreturn_t dpu_irq(struct msm_kms *kms)
-{
- struct dpu_kms *dpu_kms = to_dpu_kms(kms);
-
- return dpu_core_irq(dpu_kms);
-}
-
-static void dpu_irq_preinstall(struct msm_kms *kms)
-{
- struct dpu_kms *dpu_kms = to_dpu_kms(kms);
-
- dpu_core_irq_preinstall(dpu_kms);
-}
-
static int dpu_irq_postinstall(struct msm_kms *kms)
{
struct msm_drm_private *priv;
@@ -874,13 +860,6 @@ static int dpu_irq_postinstall(struct msm_kms *kms)
return 0;
}
-static void dpu_irq_uninstall(struct msm_kms *kms)
-{
- struct dpu_kms *dpu_kms = to_dpu_kms(kms);
-
- dpu_core_irq_uninstall(dpu_kms);
-}
-
static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_kms *kms)
{
int i;
@@ -933,10 +912,10 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k
static const struct msm_kms_funcs kms_funcs = {
.hw_init = dpu_kms_hw_init,
- .irq_preinstall = dpu_irq_preinstall,
+ .irq_preinstall = dpu_core_irq_preinstall,
.irq_postinstall = dpu_irq_postinstall,
- .irq_uninstall = dpu_irq_uninstall,
- .irq = dpu_irq,
+ .irq_uninstall = dpu_core_irq_uninstall,
+ .irq = dpu_core_irq,
.enable_commit = dpu_kms_enable_commit,
.disable_commit = dpu_kms_disable_commit,
.vsync_time = dpu_kms_vsync_time,
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 2/6] drm/msm/dpu: remove always-true argument of dpu_core_irq_read()
2022-02-17 4:31 [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers Dmitry Baryshkov
2022-02-17 4:31 ` [PATCH v3 1/6] drm/msm/dpu: remove extra wrappers around dpu_core_irq Dmitry Baryshkov
@ 2022-02-17 4:31 ` Dmitry Baryshkov
2022-02-17 4:31 ` [PATCH v3 3/6] drm/msm/dpu: allow just single IRQ callback Dmitry Baryshkov
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-02-17 4:31 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Sean Paul, Abhinav Kumar
Cc: Stephen Boyd, David Airlie, Daniel Vetter, linux-arm-msm,
dri-devel, freedreno
The argument clear of the function dpu_core_irq_read() is always true.
Remove it.
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 4 +---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 4 ++--
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
index 7023ccb79814..6dce5d89f817 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
@@ -33,13 +33,11 @@ irqreturn_t dpu_core_irq(struct msm_kms *kms);
* dpu_core_irq_read - IRQ helper function for reading IRQ status
* @dpu_kms: DPU handle
* @irq_idx: irq index
- * @clear: True to clear the irq after read
* @return: non-zero if irq detected; otherwise no irq detected
*/
u32 dpu_core_irq_read(
struct dpu_kms *dpu_kms,
- int irq_idx,
- bool clear);
+ int irq_idx);
/**
* dpu_core_irq_register_callback - For registering callback function on IRQ
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1e648db439f9..5576b8a3e6ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -301,8 +301,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
wait_info);
if (ret <= 0) {
- irq_status = dpu_core_irq_read(phys_enc->dpu_kms,
- irq->irq_idx, true);
+ irq_status = dpu_core_irq_read(phys_enc->dpu_kms, irq->irq_idx);
if (irq_status) {
unsigned long flags;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index 71882d3fe705..85404c9ab4e1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -357,7 +357,7 @@ static void dpu_disable_all_irqs(struct dpu_kms *dpu_kms)
wmb();
}
-u32 dpu_core_irq_read(struct dpu_kms *dpu_kms, int irq_idx, bool clear)
+u32 dpu_core_irq_read(struct dpu_kms *dpu_kms, int irq_idx)
{
struct dpu_hw_intr *intr = dpu_kms->hw_intr;
int reg_idx;
@@ -384,7 +384,7 @@ u32 dpu_core_irq_read(struct dpu_kms *dpu_kms, int irq_idx, bool clear)
intr_status = DPU_REG_READ(&intr->hw,
dpu_intr_set[reg_idx].status_off) &
DPU_IRQ_MASK(irq_idx);
- if (intr_status && clear)
+ if (intr_status)
DPU_REG_WRITE(&intr->hw, dpu_intr_set[reg_idx].clr_off,
intr_status);
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 3/6] drm/msm/dpu: allow just single IRQ callback
2022-02-17 4:31 [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers Dmitry Baryshkov
2022-02-17 4:31 ` [PATCH v3 1/6] drm/msm/dpu: remove extra wrappers around dpu_core_irq Dmitry Baryshkov
2022-02-17 4:31 ` [PATCH v3 2/6] drm/msm/dpu: remove always-true argument of dpu_core_irq_read() Dmitry Baryshkov
@ 2022-02-17 4:31 ` Dmitry Baryshkov
2022-03-03 21:20 ` Stephen Boyd
2022-02-17 4:31 ` [PATCH v3 4/6] drm/msm/dpu: get rid of dpu_encoder_helper_(un)register_irq Dmitry Baryshkov
` (3 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-02-17 4:31 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Sean Paul, Abhinav Kumar
Cc: Stephen Boyd, David Airlie, Daniel Vetter, linux-arm-msm,
dri-devel, freedreno, kernel test robot
DPU interrupts code allows multiple callbacks per interrut. In reality
none of the interrupts is shared between blocks (and will probably never
be). Drop support for registering multiple callbacks per interrupt to
simplify interrupt handling code.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 16 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 +-
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 10 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 6 +-
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 140 ++++++------------
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 12 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 12 --
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 26 ++--
9 files changed, 84 insertions(+), 147 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
index 6dce5d89f817..b5b6e7031fb9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
@@ -44,10 +44,8 @@ u32 dpu_core_irq_read(
* interrupt
* @dpu_kms: DPU handle
* @irq_idx: irq index
- * @irq_cb: IRQ callback structure, containing callback function
- * and argument. Passing NULL for irq_cb will unregister
- * the callback for the given irq_idx
- * This must exist until un-registration.
+ * @irq_cb: IRQ callback funcion.
+ * @irq_arg: IRQ callback argument.
* @return: 0 for success registering callback, otherwise failure
*
* This function supports registration of multiple callbacks for each interrupt.
@@ -55,25 +53,21 @@ u32 dpu_core_irq_read(
int dpu_core_irq_register_callback(
struct dpu_kms *dpu_kms,
int irq_idx,
- struct dpu_irq_callback *irq_cb);
+ void (*irq_cb)(void *arg, int irq_idx),
+ void *irq_arg);
/**
* dpu_core_irq_unregister_callback - For unregistering callback function on IRQ
* interrupt
* @dpu_kms: DPU handle
* @irq_idx: irq index
- * @irq_cb: IRQ callback structure, containing callback function
- * and argument. Passing NULL for irq_cb will unregister
- * the callback for the given irq_idx
- * This must match with registration.
* @return: 0 for success registering callback, otherwise failure
*
* This function supports registration of multiple callbacks for each interrupt.
*/
int dpu_core_irq_unregister_callback(
struct dpu_kms *dpu_kms,
- int irq_idx,
- struct dpu_irq_callback *irq_cb);
+ int irq_idx);
/**
* dpu_debugfs_core_irq_init - register core irq debugfs
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 5576b8a3e6ee..17ca149e7dcd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -311,7 +311,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
local_irq_save(flags);
- irq->cb.func(phys_enc, irq->irq_idx);
+ irq->func(phys_enc, irq->irq_idx);
local_irq_restore(flags);
ret = 0;
} else {
@@ -352,7 +352,7 @@ int dpu_encoder_helper_register_irq(struct dpu_encoder_phys *phys_enc,
}
ret = dpu_core_irq_register_callback(phys_enc->dpu_kms, irq->irq_idx,
- &irq->cb);
+ irq->func, phys_enc);
if (ret) {
DPU_ERROR_PHYS(phys_enc,
"failed to register IRQ callback for %s\n",
@@ -383,8 +383,7 @@ int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys *phys_enc,
return 0;
}
- ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms, irq->irq_idx,
- &irq->cb);
+ ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms, irq->irq_idx);
if (ret) {
DRM_ERROR("unreg cb fail id=%u, intr=%d, irq=%d ret=%d",
DRMID(phys_enc->parent), intr_idx,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index e7270eb6b84b..80d87871fd94 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -174,7 +174,7 @@ struct dpu_encoder_irq {
const char *name;
enum dpu_intr_idx intr_idx;
int irq_idx;
- struct dpu_irq_callback cb;
+ void (*func)(void *arg, int irq_idx);
};
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 34a6940d12c5..a0652bdcf59b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -782,30 +782,28 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
phys_enc->enable_state = DPU_ENC_DISABLED;
for (i = 0; i < INTR_IDX_MAX; i++) {
irq = &phys_enc->irq[i];
- INIT_LIST_HEAD(&irq->cb.list);
irq->irq_idx = -EINVAL;
- irq->cb.arg = phys_enc;
}
irq = &phys_enc->irq[INTR_IDX_CTL_START];
irq->name = "ctl_start";
irq->intr_idx = INTR_IDX_CTL_START;
- irq->cb.func = dpu_encoder_phys_cmd_ctl_start_irq;
+ irq->func = dpu_encoder_phys_cmd_ctl_start_irq;
irq = &phys_enc->irq[INTR_IDX_PINGPONG];
irq->name = "pp_done";
irq->intr_idx = INTR_IDX_PINGPONG;
- irq->cb.func = dpu_encoder_phys_cmd_pp_tx_done_irq;
+ irq->func = dpu_encoder_phys_cmd_pp_tx_done_irq;
irq = &phys_enc->irq[INTR_IDX_RDPTR];
irq->name = "pp_rd_ptr";
irq->intr_idx = INTR_IDX_RDPTR;
- irq->cb.func = dpu_encoder_phys_cmd_pp_rd_ptr_irq;
+ irq->func = dpu_encoder_phys_cmd_pp_rd_ptr_irq;
irq = &phys_enc->irq[INTR_IDX_UNDERRUN];
irq->name = "underrun";
irq->intr_idx = INTR_IDX_UNDERRUN;
- irq->cb.func = dpu_encoder_phys_cmd_underrun_irq;
+ irq->func = dpu_encoder_phys_cmd_underrun_irq;
atomic_set(&phys_enc->vblank_refcount, 0);
atomic_set(&phys_enc->pending_kickoff_cnt, 0);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index ddd9d89cd456..407e45e2010b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -725,20 +725,18 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
phys_enc->enc_spinlock = p->enc_spinlock;
for (i = 0; i < INTR_IDX_MAX; i++) {
irq = &phys_enc->irq[i];
- INIT_LIST_HEAD(&irq->cb.list);
irq->irq_idx = -EINVAL;
- irq->cb.arg = phys_enc;
}
irq = &phys_enc->irq[INTR_IDX_VSYNC];
irq->name = "vsync_irq";
irq->intr_idx = INTR_IDX_VSYNC;
- irq->cb.func = dpu_encoder_phys_vid_vblank_irq;
+ irq->func = dpu_encoder_phys_vid_vblank_irq;
irq = &phys_enc->irq[INTR_IDX_UNDERRUN];
irq->name = "underrun";
irq->intr_idx = INTR_IDX_UNDERRUN;
- irq->cb.func = dpu_encoder_phys_vid_underrun_irq;
+ irq->func = dpu_encoder_phys_vid_underrun_irq;
atomic_set(&phys_enc->vblank_refcount, 0);
atomic_set(&phys_enc->pending_kickoff_cnt, 0);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index 85404c9ab4e1..8297498f2279 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -145,21 +145,17 @@ static const struct dpu_intr_reg dpu_intr_set[] = {
*/
static void dpu_core_irq_callback_handler(struct dpu_kms *dpu_kms, int irq_idx)
{
- struct dpu_irq_callback *cb;
-
VERB("irq_idx=%d\n", irq_idx);
- if (list_empty(&dpu_kms->hw_intr->irq_cb_tbl[irq_idx]))
+ if (!dpu_kms->hw_intr->irq_tbl[irq_idx].cb)
DRM_ERROR("no registered cb, idx:%d\n", irq_idx);
- atomic_inc(&dpu_kms->hw_intr->irq_counts[irq_idx]);
+ atomic_inc(&dpu_kms->hw_intr->irq_tbl[irq_idx].count);
/*
* Perform registered function callback
*/
- list_for_each_entry(cb, &dpu_kms->hw_intr->irq_cb_tbl[irq_idx], list)
- if (cb->func)
- cb->func(cb->arg, irq_idx);
+ dpu_kms->hw_intr->irq_tbl[irq_idx].cb(dpu_kms->hw_intr->irq_tbl[irq_idx].arg, irq_idx);
}
irqreturn_t dpu_core_irq(struct msm_kms *kms)
@@ -408,24 +404,18 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr,
struct dpu_mdss_cfg *m)
{
struct dpu_hw_intr *intr;
+ int nirq = MDP_INTR_MAX * 32;
if (!addr || !m)
return ERR_PTR(-EINVAL);
- intr = kzalloc(sizeof(*intr), GFP_KERNEL);
+ intr = kzalloc(struct_size(intr, irq_tbl, nirq), GFP_KERNEL);
if (!intr)
return ERR_PTR(-ENOMEM);
__intr_offset(m, addr, &intr->hw);
- intr->total_irqs = ARRAY_SIZE(dpu_intr_set) * 32;
-
- intr->cache_irq_mask = kcalloc(ARRAY_SIZE(dpu_intr_set), sizeof(u32),
- GFP_KERNEL);
- if (intr->cache_irq_mask == NULL) {
- kfree(intr);
- return ERR_PTR(-ENOMEM);
- }
+ intr->total_irqs = nirq;
intr->irq_mask = m->mdss_irqs;
@@ -436,31 +426,19 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr,
void dpu_hw_intr_destroy(struct dpu_hw_intr *intr)
{
- if (intr) {
- kfree(intr->cache_irq_mask);
-
- kfree(intr->irq_cb_tbl);
- kfree(intr->irq_counts);
-
+ if (intr)
kfree(intr);
- }
}
int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx,
- struct dpu_irq_callback *register_irq_cb)
+ void (*irq_cb)(void *arg, int irq_idx),
+ void *irq_arg)
{
unsigned long irq_flags;
+ int ret;
- if (!dpu_kms->hw_intr->irq_cb_tbl) {
- DPU_ERROR("invalid params\n");
- return -EINVAL;
- }
-
- if (!register_irq_cb || !register_irq_cb->func) {
- DPU_ERROR("invalid irq_cb:%d func:%d\n",
- register_irq_cb != NULL,
- register_irq_cb ?
- register_irq_cb->func != NULL : -1);
+ if (!irq_cb) {
+ DPU_ERROR("invalid ird_idx:%d irq_cb:%ps\n", irq_idx, irq_cb);
return -EINVAL;
}
@@ -472,41 +450,32 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx,
VERB("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);
spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags);
- trace_dpu_core_irq_register_callback(irq_idx, register_irq_cb);
- list_del_init(®ister_irq_cb->list);
- list_add_tail(®ister_irq_cb->list,
- &dpu_kms->hw_intr->irq_cb_tbl[irq_idx]);
- if (list_is_first(®ister_irq_cb->list,
- &dpu_kms->hw_intr->irq_cb_tbl[irq_idx])) {
- int ret = dpu_hw_intr_enable_irq_locked(
+
+ if (unlikely(WARN_ON(dpu_kms->hw_intr->irq_tbl[irq_idx].cb))) {
+ spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
+
+ return -EBUSY;
+ }
+
+ trace_dpu_core_irq_register_callback(irq_idx, irq_cb);
+ dpu_kms->hw_intr->irq_tbl[irq_idx].arg = irq_arg;
+ dpu_kms->hw_intr->irq_tbl[irq_idx].cb = irq_cb;
+
+ ret = dpu_hw_intr_enable_irq_locked(
dpu_kms->hw_intr,
irq_idx);
- if (ret)
- DPU_ERROR("Fail to enable IRQ for irq_idx:%d\n",
+ if (ret)
+ DPU_ERROR("Fail to enable IRQ for irq_idx:%d\n",
irq_idx);
- }
spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
return 0;
}
-int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx,
- struct dpu_irq_callback *register_irq_cb)
+int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx)
{
unsigned long irq_flags;
-
- if (!dpu_kms->hw_intr->irq_cb_tbl) {
- DPU_ERROR("invalid params\n");
- return -EINVAL;
- }
-
- if (!register_irq_cb || !register_irq_cb->func) {
- DPU_ERROR("invalid irq_cb:%d func:%d\n",
- register_irq_cb != NULL,
- register_irq_cb ?
- register_irq_cb->func != NULL : -1);
- return -EINVAL;
- }
+ int ret;
if (irq_idx < 0 || irq_idx >= dpu_kms->hw_intr->total_irqs) {
DPU_ERROR("invalid IRQ index: [%d]\n", irq_idx);
@@ -516,18 +485,16 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx,
VERB("[%pS] irq_idx=%d\n", __builtin_return_address(0), irq_idx);
spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags);
- trace_dpu_core_irq_unregister_callback(irq_idx, register_irq_cb);
- list_del_init(®ister_irq_cb->list);
- /* empty callback list but interrupt is still enabled */
- if (list_empty(&dpu_kms->hw_intr->irq_cb_tbl[irq_idx])) {
- int ret = dpu_hw_intr_disable_irq_locked(
- dpu_kms->hw_intr,
- irq_idx);
- if (ret)
- DPU_ERROR("Fail to disable IRQ for irq_idx:%d\n",
- irq_idx);
- VERB("irq_idx=%d ret=%d\n", irq_idx, ret);
- }
+ trace_dpu_core_irq_unregister_callback(irq_idx);
+
+ ret = dpu_hw_intr_disable_irq_locked(dpu_kms->hw_intr, irq_idx);
+ if (ret)
+ DPU_ERROR("Fail to disable IRQ for irq_idx:%d: %d\n",
+ irq_idx, ret);
+
+ dpu_kms->hw_intr->irq_tbl[irq_idx].cb = NULL;
+ dpu_kms->hw_intr->irq_tbl[irq_idx].arg = NULL;
+
spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
return 0;
@@ -537,24 +504,18 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx,
static int dpu_debugfs_core_irq_show(struct seq_file *s, void *v)
{
struct dpu_kms *dpu_kms = s->private;
- struct dpu_irq_callback *cb;
unsigned long irq_flags;
- int i, irq_count, cb_count;
-
- if (WARN_ON(!dpu_kms->hw_intr->irq_cb_tbl))
- return 0;
+ int i, irq_count;
+ void *cb;
for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) {
spin_lock_irqsave(&dpu_kms->hw_intr->irq_lock, irq_flags);
- cb_count = 0;
- irq_count = atomic_read(&dpu_kms->hw_intr->irq_counts[i]);
- list_for_each_entry(cb, &dpu_kms->hw_intr->irq_cb_tbl[i], list)
- cb_count++;
+ irq_count = atomic_read(&dpu_kms->hw_intr->irq_tbl[i].count);
+ cb = dpu_kms->hw_intr->irq_tbl[i].cb;
spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
- if (irq_count || cb_count)
- seq_printf(s, "idx:%d irq:%d cb:%d\n",
- i, irq_count, cb_count);
+ if (irq_count || cb)
+ seq_printf(s, "idx:%d irq:%d cb:%ps\n", i, irq_count, cb);
}
return 0;
@@ -580,15 +541,8 @@ void dpu_core_irq_preinstall(struct msm_kms *kms)
dpu_disable_all_irqs(dpu_kms);
pm_runtime_put_sync(&dpu_kms->pdev->dev);
- /* Create irq callbacks for all possible irq_idx */
- dpu_kms->hw_intr->irq_cb_tbl = kcalloc(dpu_kms->hw_intr->total_irqs,
- sizeof(struct list_head), GFP_KERNEL);
- dpu_kms->hw_intr->irq_counts = kcalloc(dpu_kms->hw_intr->total_irqs,
- sizeof(atomic_t), GFP_KERNEL);
- for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) {
- INIT_LIST_HEAD(&dpu_kms->hw_intr->irq_cb_tbl[i]);
- atomic_set(&dpu_kms->hw_intr->irq_counts[i], 0);
- }
+ for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++)
+ atomic_set(&dpu_kms->hw_intr->irq_tbl[i].count, 0);
}
void dpu_core_irq_uninstall(struct msm_kms *kms)
@@ -598,7 +552,7 @@ void dpu_core_irq_uninstall(struct msm_kms *kms)
pm_runtime_get_sync(&dpu_kms->pdev->dev);
for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++)
- if (!list_empty(&dpu_kms->hw_intr->irq_cb_tbl[i]))
+ if (dpu_kms->hw_intr->irq_tbl[i].cb)
DPU_ERROR("irq_idx=%d still enabled/registered\n", i);
dpu_clear_irqs(dpu_kms);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
index 1ab75cccd145..3136c1d260f8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h
@@ -43,19 +43,21 @@ enum dpu_hw_intr_reg {
* @save_irq_status: array of IRQ status reg storage created during init
* @total_irqs: total number of irq_idx mapped in the hw_interrupts
* @irq_lock: spinlock for accessing IRQ resources
- * @irq_cb_tbl: array of IRQ callbacks lists
- * @irq_counts: array of IRQ counts
+ * @irq_cb_tbl: array of IRQ callbacks
*/
struct dpu_hw_intr {
struct dpu_hw_blk_reg_map hw;
- u32 *cache_irq_mask;
+ u32 cache_irq_mask[MDP_INTR_MAX];
u32 *save_irq_status;
u32 total_irqs;
spinlock_t irq_lock;
unsigned long irq_mask;
- struct list_head *irq_cb_tbl;
- atomic_t *irq_counts;
+ struct {
+ void (*cb)(void *arg, int irq_idx);
+ void *arg;
+ atomic_t count;
+ } irq_tbl[];
};
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 2d385b4b7f5e..b6371f1d554c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -66,18 +66,6 @@
#define DPU_NAME_SIZE 12
-/*
- * struct dpu_irq_callback - IRQ callback handlers
- * @list: list to callback
- * @func: intr handler
- * @arg: argument for the handler
- */
-struct dpu_irq_callback {
- struct list_head list;
- void (*func)(void *arg, int irq_idx);
- void *arg;
-};
-
struct dpu_kms {
struct msm_kms base;
struct drm_device *dev;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 54d74341e690..2d492b03a8b1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -871,27 +871,31 @@ TRACE_EVENT(dpu_pp_connect_ext_te,
TP_printk("pp:%d cfg:%u", __entry->pp, __entry->cfg)
);
-DECLARE_EVENT_CLASS(dpu_core_irq_callback_template,
- TP_PROTO(int irq_idx, struct dpu_irq_callback *callback),
+TRACE_EVENT(dpu_core_irq_register_callback,
+ TP_PROTO(int irq_idx, void *callback),
TP_ARGS(irq_idx, callback),
TP_STRUCT__entry(
__field( int, irq_idx )
- __field( struct dpu_irq_callback *, callback)
+ __field( void *, callback)
),
TP_fast_assign(
__entry->irq_idx = irq_idx;
__entry->callback = callback;
),
- TP_printk("irq_idx:%d callback:%pK", __entry->irq_idx,
+ TP_printk("irq_idx:%d callback:%ps", __entry->irq_idx,
__entry->callback)
);
-DEFINE_EVENT(dpu_core_irq_callback_template, dpu_core_irq_register_callback,
- TP_PROTO(int irq_idx, struct dpu_irq_callback *callback),
- TP_ARGS(irq_idx, callback)
-);
-DEFINE_EVENT(dpu_core_irq_callback_template, dpu_core_irq_unregister_callback,
- TP_PROTO(int irq_idx, struct dpu_irq_callback *callback),
- TP_ARGS(irq_idx, callback)
+
+TRACE_EVENT(dpu_core_irq_unregister_callback,
+ TP_PROTO(int irq_idx),
+ TP_ARGS(irq_idx),
+ TP_STRUCT__entry(
+ __field( int, irq_idx )
+ ),
+ TP_fast_assign(
+ __entry->irq_idx = irq_idx;
+ ),
+ TP_printk("irq_idx:%d", __entry->irq_idx)
);
TRACE_EVENT(dpu_core_perf_update_clk,
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 3/6] drm/msm/dpu: allow just single IRQ callback
2022-02-17 4:31 ` [PATCH v3 3/6] drm/msm/dpu: allow just single IRQ callback Dmitry Baryshkov
@ 2022-03-03 21:20 ` Stephen Boyd
0 siblings, 0 replies; 12+ messages in thread
From: Stephen Boyd @ 2022-03-03 21:20 UTC (permalink / raw)
To: Abhinav Kumar, Bjorn Andersson, Dmitry Baryshkov, Rob Clark,
Sean Paul
Cc: David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno,
kernel test robot
Quoting Dmitry Baryshkov (2022-02-16 20:31:45)
> DPU interrupts code allows multiple callbacks per interrut. In reality
> none of the interrupts is shared between blocks (and will probably never
> be). Drop support for registering multiple callbacks per interrupt to
> simplify interrupt handling code.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 4/6] drm/msm/dpu: get rid of dpu_encoder_helper_(un)register_irq
2022-02-17 4:31 [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers Dmitry Baryshkov
` (2 preceding siblings ...)
2022-02-17 4:31 ` [PATCH v3 3/6] drm/msm/dpu: allow just single IRQ callback Dmitry Baryshkov
@ 2022-02-17 4:31 ` Dmitry Baryshkov
2022-02-17 4:31 ` [PATCH v3 5/6] drm/msm/dpu: remove struct dpu_encoder_irq Dmitry Baryshkov
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-02-17 4:31 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Sean Paul, Abhinav Kumar
Cc: Stephen Boyd, David Airlie, Daniel Vetter, linux-arm-msm,
dri-devel, freedreno, Abhinav Kumar
Get rid of dpu_encoder_helper_register_irq/unregister_irq helpers, call
dpu_core_register/unregister_callback directly, without surrounding them
with helpers.
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 63 -------------------
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 18 ------
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 34 ++++++----
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 18 ++++--
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 4 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 29 +++------
6 files changed, 48 insertions(+), 118 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 17ca149e7dcd..dbcbf96cf8eb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -333,69 +333,6 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
return ret;
}
-int dpu_encoder_helper_register_irq(struct dpu_encoder_phys *phys_enc,
- enum dpu_intr_idx intr_idx)
-{
- struct dpu_encoder_irq *irq;
- int ret = 0;
-
- if (intr_idx >= INTR_IDX_MAX) {
- DPU_ERROR("invalid params\n");
- return -EINVAL;
- }
- irq = &phys_enc->irq[intr_idx];
-
- if (irq->irq_idx < 0) {
- DPU_ERROR_PHYS(phys_enc,
- "invalid IRQ index:%d\n", irq->irq_idx);
- return -EINVAL;
- }
-
- ret = dpu_core_irq_register_callback(phys_enc->dpu_kms, irq->irq_idx,
- irq->func, phys_enc);
- if (ret) {
- DPU_ERROR_PHYS(phys_enc,
- "failed to register IRQ callback for %s\n",
- irq->name);
- irq->irq_idx = -EINVAL;
- return ret;
- }
-
- trace_dpu_enc_irq_register_success(DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx);
-
- return ret;
-}
-
-int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys *phys_enc,
- enum dpu_intr_idx intr_idx)
-{
- struct dpu_encoder_irq *irq;
- int ret;
-
- irq = &phys_enc->irq[intr_idx];
-
- /* silently skip irqs that weren't registered */
- if (irq->irq_idx < 0) {
- DRM_ERROR("duplicate unregister id=%u, intr=%d, irq=%d",
- DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx);
- return 0;
- }
-
- ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms, irq->irq_idx);
- if (ret) {
- DRM_ERROR("unreg cb fail id=%u, intr=%d, irq=%d ret=%d",
- DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx, ret);
- }
-
- trace_dpu_enc_irq_unregister_success(DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx);
-
- return 0;
-}
-
int dpu_encoder_get_vsync_count(struct drm_encoder *drm_enc)
{
struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 80d87871fd94..ff2218155b44 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -364,22 +364,4 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
enum dpu_intr_idx intr_idx,
struct dpu_encoder_wait_info *wait_info);
-/**
- * dpu_encoder_helper_register_irq - register and enable an irq
- * @phys_enc: Pointer to physical encoder structure
- * @intr_idx: encoder interrupt index
- * @Return: 0 or -ERROR
- */
-int dpu_encoder_helper_register_irq(struct dpu_encoder_phys *phys_enc,
- enum dpu_intr_idx intr_idx);
-
-/**
- * dpu_encoder_helper_unregister_irq - unregister and disable an irq
- * @phys_enc: Pointer to physical encoder structure
- * @intr_idx: encoder interrupt index
- * @Return: 0 or -ERROR
- */
-int dpu_encoder_helper_unregister_irq(struct dpu_encoder_phys *phys_enc,
- enum dpu_intr_idx intr_idx);
-
#endif /* __dpu_encoder_phys_H__ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index a0652bdcf59b..ca7d557312ff 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -211,7 +211,8 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout(
cmd_enc->pp_timeout_report_cnt,
atomic_read(&phys_enc->pending_kickoff_cnt));
msm_disp_snapshot_state(drm_enc->dev);
- dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_RDPTR);
+ dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_RDPTR].irq_idx);
}
atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0);
@@ -277,10 +278,12 @@ static int dpu_encoder_phys_cmd_control_vblank_irq(
enable ? "true" : "false", refcount);
if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1)
- ret = dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_RDPTR);
+ ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_RDPTR].irq_idx,
+ phys_enc->irq[INTR_IDX_RDPTR].func, phys_enc);
else if (!enable && atomic_dec_return(&phys_enc->vblank_refcount) == 0)
- ret = dpu_encoder_helper_unregister_irq(phys_enc,
- INTR_IDX_RDPTR);
+ ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_RDPTR].irq_idx);
end:
if (ret) {
@@ -301,21 +304,28 @@ static void dpu_encoder_phys_cmd_irq_control(struct dpu_encoder_phys *phys_enc,
enable, atomic_read(&phys_enc->vblank_refcount));
if (enable) {
- dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_PINGPONG);
- dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_UNDERRUN);
+ dpu_core_irq_register_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_PINGPONG].irq_idx,
+ phys_enc->irq[INTR_IDX_PINGPONG].func, phys_enc);
+ dpu_core_irq_register_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
+ phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
dpu_encoder_phys_cmd_control_vblank_irq(phys_enc, true);
if (dpu_encoder_phys_cmd_is_master(phys_enc))
- dpu_encoder_helper_register_irq(phys_enc,
- INTR_IDX_CTL_START);
+ dpu_core_irq_register_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_CTL_START].irq_idx,
+ phys_enc->irq[INTR_IDX_CTL_START].func, phys_enc);
} else {
if (dpu_encoder_phys_cmd_is_master(phys_enc))
- dpu_encoder_helper_unregister_irq(phys_enc,
- INTR_IDX_CTL_START);
+ dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_CTL_START].irq_idx);
- dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_UNDERRUN);
+ dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx);
dpu_encoder_phys_cmd_control_vblank_irq(phys_enc, false);
- dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_PINGPONG);
+ dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_PINGPONG].irq_idx);
}
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 407e45e2010b..50c063de1509 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -404,10 +404,12 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
atomic_read(&phys_enc->vblank_refcount));
if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1)
- ret = dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_VSYNC);
+ ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_VSYNC].irq_idx,
+ phys_enc->irq[INTR_IDX_VSYNC].func, phys_enc);
else if (!enable && atomic_dec_return(&phys_enc->vblank_refcount) == 0)
- ret = dpu_encoder_helper_unregister_irq(phys_enc,
- INTR_IDX_VSYNC);
+ ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_VSYNC].irq_idx);
end:
if (ret) {
@@ -539,7 +541,8 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff(
DPU_ERROR_VIDENC(phys_enc, "ctl %d reset failure: %d\n",
ctl->idx, rc);
msm_disp_snapshot_state(drm_enc->dev);
- dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_VSYNC);
+ dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_VSYNC].irq_idx);
}
}
@@ -628,10 +631,13 @@ static void dpu_encoder_phys_vid_irq_control(struct dpu_encoder_phys *phys_enc,
if (WARN_ON(ret))
return;
- dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_UNDERRUN);
+ dpu_core_irq_register_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
+ phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
} else {
dpu_encoder_phys_vid_control_vblank_irq(phys_enc, false);
- dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_UNDERRUN);
+ dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
+ phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx);
}
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index 8297498f2279..78e1eca6aa70 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -469,6 +469,8 @@ int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx,
irq_idx);
spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
+ trace_dpu_irq_register_success(irq_idx);
+
return 0;
}
@@ -497,6 +499,8 @@ int dpu_core_irq_unregister_callback(struct dpu_kms *dpu_kms, int irq_idx)
spin_unlock_irqrestore(&dpu_kms->hw_intr->irq_lock, irq_flags);
+ trace_dpu_irq_unregister_success(irq_idx);
+
return 0;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 2d492b03a8b1..60f63a15a894 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -167,33 +167,24 @@ TRACE_EVENT(dpu_perf_crtc_update,
__entry->update_clk)
);
-DECLARE_EVENT_CLASS(dpu_enc_irq_template,
- TP_PROTO(uint32_t drm_id, enum dpu_intr_idx intr_idx,
- int irq_idx),
- TP_ARGS(drm_id, intr_idx, irq_idx),
+DECLARE_EVENT_CLASS(dpu_irq_template,
+ TP_PROTO(int irq_idx),
+ TP_ARGS(irq_idx),
TP_STRUCT__entry(
- __field( uint32_t, drm_id )
- __field( enum dpu_intr_idx, intr_idx )
__field( int, irq_idx )
),
TP_fast_assign(
- __entry->drm_id = drm_id;
- __entry->intr_idx = intr_idx;
__entry->irq_idx = irq_idx;
),
- TP_printk("id=%u, intr=%d, irq=%d",
- __entry->drm_id, __entry->intr_idx,
- __entry->irq_idx)
+ TP_printk("irq=%d", __entry->irq_idx)
);
-DEFINE_EVENT(dpu_enc_irq_template, dpu_enc_irq_register_success,
- TP_PROTO(uint32_t drm_id, enum dpu_intr_idx intr_idx,
- int irq_idx),
- TP_ARGS(drm_id, intr_idx, irq_idx)
+DEFINE_EVENT(dpu_irq_template, dpu_irq_register_success,
+ TP_PROTO(int irq_idx),
+ TP_ARGS(irq_idx)
);
-DEFINE_EVENT(dpu_enc_irq_template, dpu_enc_irq_unregister_success,
- TP_PROTO(uint32_t drm_id, enum dpu_intr_idx intr_idx,
- int irq_idx),
- TP_ARGS(drm_id, intr_idx, irq_idx)
+DEFINE_EVENT(dpu_irq_template, dpu_irq_unregister_success,
+ TP_PROTO(int irq_idx),
+ TP_ARGS(irq_idx)
);
TRACE_EVENT(dpu_enc_irq_wait_success,
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 5/6] drm/msm/dpu: remove struct dpu_encoder_irq
2022-02-17 4:31 [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers Dmitry Baryshkov
` (3 preceding siblings ...)
2022-02-17 4:31 ` [PATCH v3 4/6] drm/msm/dpu: get rid of dpu_encoder_helper_(un)register_irq Dmitry Baryshkov
@ 2022-02-17 4:31 ` Dmitry Baryshkov
2022-03-03 21:21 ` Stephen Boyd
2022-02-17 4:31 ` [PATCH v3 6/6] drm/msm/dpu: pass irq to dpu_encoder_helper_wait_for_irq() Dmitry Baryshkov
2022-03-03 21:22 ` [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers Stephen Boyd
6 siblings, 1 reply; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-02-17 4:31 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Sean Paul, Abhinav Kumar
Cc: Stephen Boyd, David Airlie, Daniel Vetter, linux-arm-msm,
dri-devel, freedreno
Remove additional indirection: specify IRQ callbacks and IRQ indices
directly rather than through the pointer in the irq structure. For each
IRQ we have a constant IRQ callback. This change simplifies code review
as the reader no longer needs to remember which function is called.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 28 +++----
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 21 +-----
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 73 +++++++------------
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 42 ++++-------
4 files changed, 58 insertions(+), 106 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index dbcbf96cf8eb..83b6715820fa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -261,9 +261,10 @@ static int dpu_encoder_helper_wait_event_timeout(int32_t drm_id,
int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
enum dpu_intr_idx intr_idx,
+ void (*func)(void *arg, int irq_idx),
struct dpu_encoder_wait_info *wait_info)
{
- struct dpu_encoder_irq *irq;
+ int irq;
u32 irq_status;
int ret;
@@ -271,7 +272,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
DPU_ERROR("invalid params\n");
return -EINVAL;
}
- irq = &phys_enc->irq[intr_idx];
+ irq = phys_enc->irq[intr_idx];
/* note: do master / slave checking outside */
@@ -279,53 +280,52 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
if (phys_enc->enable_state == DPU_ENC_DISABLED) {
DRM_ERROR("encoder is disabled id=%u, intr=%d, irq=%d\n",
DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx);
+ irq);
return -EWOULDBLOCK;
}
- if (irq->irq_idx < 0) {
- DRM_DEBUG_KMS("skip irq wait id=%u, intr=%d, irq=%s\n",
- DRMID(phys_enc->parent), intr_idx,
- irq->name);
+ if (irq < 0) {
+ DRM_DEBUG_KMS("skip irq wait id=%u, intr=%d\n",
+ DRMID(phys_enc->parent), intr_idx);
return 0;
}
DRM_DEBUG_KMS("id=%u, intr=%d, irq=%d, pp=%d, pending_cnt=%d\n",
DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx, phys_enc->hw_pp->idx - PINGPONG_0,
+ irq, phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
ret = dpu_encoder_helper_wait_event_timeout(
DRMID(phys_enc->parent),
- irq->irq_idx,
+ irq,
wait_info);
if (ret <= 0) {
- irq_status = dpu_core_irq_read(phys_enc->dpu_kms, irq->irq_idx);
+ irq_status = dpu_core_irq_read(phys_enc->dpu_kms, irq);
if (irq_status) {
unsigned long flags;
DRM_DEBUG_KMS("irq not triggered id=%u, intr=%d, irq=%d, pp=%d, atomic_cnt=%d\n",
DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx,
+ irq,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
local_irq_save(flags);
- irq->func(phys_enc, irq->irq_idx);
+ func(phys_enc, irq);
local_irq_restore(flags);
ret = 0;
} else {
ret = -ETIMEDOUT;
DRM_DEBUG_KMS("irq timeout id=%u, intr=%d, irq=%d, pp=%d, atomic_cnt=%d\n",
DRMID(phys_enc->parent), intr_idx,
- irq->irq_idx,
+ irq,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
}
} else {
ret = 0;
trace_dpu_enc_irq_wait_success(DRMID(phys_enc->parent),
- intr_idx, irq->irq_idx,
+ intr_idx, irq,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index ff2218155b44..803fd6f25da1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -162,21 +162,6 @@ enum dpu_intr_idx {
INTR_IDX_MAX,
};
-/**
- * dpu_encoder_irq - tracking structure for interrupts
- * @name: string name of interrupt
- * @intr_idx: Encoder interrupt enumeration
- * @irq_idx: IRQ interface lookup index from DPU IRQ framework
- * will be -EINVAL if IRQ is not registered
- * @irq_cb: interrupt callback
- */
-struct dpu_encoder_irq {
- const char *name;
- enum dpu_intr_idx intr_idx;
- int irq_idx;
- void (*func)(void *arg, int irq_idx);
-};
-
/**
* struct dpu_encoder_phys - physical encoder that drives a single INTF block
* tied to a specific panel / sub-panel. Abstract type, sub-classed by
@@ -207,7 +192,7 @@ struct dpu_encoder_irq {
* @pending_ctlstart_cnt: Atomic counter tracking the number of ctl start
* pending.
* @pending_kickoff_wq: Wait queue for blocking until kickoff completes
- * @irq: IRQ tracking structures
+ * @irq: IRQ indices
*/
struct dpu_encoder_phys {
struct drm_encoder *parent;
@@ -231,7 +216,7 @@ struct dpu_encoder_phys {
atomic_t pending_ctlstart_cnt;
atomic_t pending_kickoff_cnt;
wait_queue_head_t pending_kickoff_wq;
- struct dpu_encoder_irq irq[INTR_IDX_MAX];
+ int irq[INTR_IDX_MAX];
};
static inline int dpu_encoder_phys_inc_pending(struct dpu_encoder_phys *phys)
@@ -357,11 +342,13 @@ void dpu_encoder_helper_report_irq_timeout(struct dpu_encoder_phys *phys_enc,
* note: will call dpu_encoder_helper_wait_for_irq on timeout
* @phys_enc: Pointer to physical encoder structure
* @intr_idx: encoder interrupt index
+ * @func: IRQ callback to be called in case of timeout
* @wait_info: wait info struct
* @Return: 0 or -ERROR
*/
int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
enum dpu_intr_idx intr_idx,
+ void (*func)(void *arg, int irq_idx),
struct dpu_encoder_wait_info *wait_info);
#endif /* __dpu_encoder_phys_H__ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index ca7d557312ff..7f5b7f0fff2e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -151,7 +151,6 @@ static void dpu_encoder_phys_cmd_mode_set(
{
struct dpu_encoder_phys_cmd *cmd_enc =
to_dpu_encoder_phys_cmd(phys_enc);
- struct dpu_encoder_irq *irq;
if (!mode || !adj_mode) {
DPU_ERROR("invalid args\n");
@@ -161,17 +160,13 @@ static void dpu_encoder_phys_cmd_mode_set(
DPU_DEBUG_CMDENC(cmd_enc, "caching mode:\n");
drm_mode_debug_printmodeline(adj_mode);
- irq = &phys_enc->irq[INTR_IDX_CTL_START];
- irq->irq_idx = phys_enc->hw_ctl->caps->intr_start;
+ phys_enc->irq[INTR_IDX_CTL_START] = phys_enc->hw_ctl->caps->intr_start;
- irq = &phys_enc->irq[INTR_IDX_PINGPONG];
- irq->irq_idx = phys_enc->hw_pp->caps->intr_done;
+ phys_enc->irq[INTR_IDX_PINGPONG] = phys_enc->hw_pp->caps->intr_done;
- irq = &phys_enc->irq[INTR_IDX_RDPTR];
- irq->irq_idx = phys_enc->hw_pp->caps->intr_rdptr;
+ phys_enc->irq[INTR_IDX_RDPTR] = phys_enc->hw_pp->caps->intr_rdptr;
- irq = &phys_enc->irq[INTR_IDX_UNDERRUN];
- irq->irq_idx = phys_enc->hw_intf->cap->intr_underrun;
+ phys_enc->irq[INTR_IDX_UNDERRUN] = phys_enc->hw_intf->cap->intr_underrun;
}
static int _dpu_encoder_phys_cmd_handle_ppdone_timeout(
@@ -212,7 +207,7 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout(
atomic_read(&phys_enc->pending_kickoff_cnt));
msm_disp_snapshot_state(drm_enc->dev);
dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_RDPTR].irq_idx);
+ phys_enc->irq[INTR_IDX_RDPTR]);
}
atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0);
@@ -240,6 +235,7 @@ static int _dpu_encoder_phys_cmd_wait_for_idle(
wait_info.timeout_ms = KICKOFF_TIMEOUT_MS;
ret = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_PINGPONG,
+ dpu_encoder_phys_cmd_pp_tx_done_irq,
&wait_info);
if (ret == -ETIMEDOUT)
_dpu_encoder_phys_cmd_handle_ppdone_timeout(phys_enc);
@@ -279,11 +275,12 @@ static int dpu_encoder_phys_cmd_control_vblank_irq(
if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1)
ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_RDPTR].irq_idx,
- phys_enc->irq[INTR_IDX_RDPTR].func, phys_enc);
+ phys_enc->irq[INTR_IDX_RDPTR],
+ dpu_encoder_phys_cmd_pp_rd_ptr_irq,
+ phys_enc);
else if (!enable && atomic_dec_return(&phys_enc->vblank_refcount) == 0)
ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_RDPTR].irq_idx);
+ phys_enc->irq[INTR_IDX_RDPTR]);
end:
if (ret) {
@@ -305,27 +302,30 @@ static void dpu_encoder_phys_cmd_irq_control(struct dpu_encoder_phys *phys_enc,
if (enable) {
dpu_core_irq_register_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_PINGPONG].irq_idx,
- phys_enc->irq[INTR_IDX_PINGPONG].func, phys_enc);
+ phys_enc->irq[INTR_IDX_PINGPONG],
+ dpu_encoder_phys_cmd_pp_tx_done_irq,
+ phys_enc);
dpu_core_irq_register_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
- phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
+ phys_enc->irq[INTR_IDX_UNDERRUN],
+ dpu_encoder_phys_cmd_underrun_irq,
+ phys_enc);
dpu_encoder_phys_cmd_control_vblank_irq(phys_enc, true);
if (dpu_encoder_phys_cmd_is_master(phys_enc))
dpu_core_irq_register_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_CTL_START].irq_idx,
- phys_enc->irq[INTR_IDX_CTL_START].func, phys_enc);
+ phys_enc->irq[INTR_IDX_CTL_START],
+ dpu_encoder_phys_cmd_ctl_start_irq,
+ phys_enc);
} else {
if (dpu_encoder_phys_cmd_is_master(phys_enc))
dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_CTL_START].irq_idx);
+ phys_enc->irq[INTR_IDX_CTL_START]);
dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx);
+ phys_enc->irq[INTR_IDX_UNDERRUN]);
dpu_encoder_phys_cmd_control_vblank_irq(phys_enc, false);
dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_PINGPONG].irq_idx);
+ phys_enc->irq[INTR_IDX_PINGPONG]);
}
}
@@ -660,6 +660,7 @@ static int _dpu_encoder_phys_cmd_wait_for_ctl_start(
wait_info.timeout_ms = KICKOFF_TIMEOUT_MS;
ret = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_CTL_START,
+ dpu_encoder_phys_cmd_ctl_start_irq,
&wait_info);
if (ret == -ETIMEDOUT) {
DPU_ERROR_CMDENC(cmd_enc, "ctl start interrupt wait failed\n");
@@ -715,6 +716,7 @@ static int dpu_encoder_phys_cmd_wait_for_vblank(
atomic_inc(&cmd_enc->pending_vblank_cnt);
rc = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_RDPTR,
+ dpu_encoder_phys_cmd_pp_rd_ptr_irq,
&wait_info);
return rc;
@@ -766,7 +768,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
{
struct dpu_encoder_phys *phys_enc = NULL;
struct dpu_encoder_phys_cmd *cmd_enc = NULL;
- struct dpu_encoder_irq *irq;
int i, ret = 0;
DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0);
@@ -790,30 +791,8 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
phys_enc->enc_spinlock = p->enc_spinlock;
cmd_enc->stream_sel = 0;
phys_enc->enable_state = DPU_ENC_DISABLED;
- for (i = 0; i < INTR_IDX_MAX; i++) {
- irq = &phys_enc->irq[i];
- irq->irq_idx = -EINVAL;
- }
-
- irq = &phys_enc->irq[INTR_IDX_CTL_START];
- irq->name = "ctl_start";
- irq->intr_idx = INTR_IDX_CTL_START;
- irq->func = dpu_encoder_phys_cmd_ctl_start_irq;
-
- irq = &phys_enc->irq[INTR_IDX_PINGPONG];
- irq->name = "pp_done";
- irq->intr_idx = INTR_IDX_PINGPONG;
- irq->func = dpu_encoder_phys_cmd_pp_tx_done_irq;
-
- irq = &phys_enc->irq[INTR_IDX_RDPTR];
- irq->name = "pp_rd_ptr";
- irq->intr_idx = INTR_IDX_RDPTR;
- irq->func = dpu_encoder_phys_cmd_pp_rd_ptr_irq;
-
- irq = &phys_enc->irq[INTR_IDX_UNDERRUN];
- irq->name = "underrun";
- irq->intr_idx = INTR_IDX_UNDERRUN;
- irq->func = dpu_encoder_phys_cmd_underrun_irq;
+ for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
+ phys_enc->irq[i] = -EINVAL;
atomic_set(&phys_enc->vblank_refcount, 0);
atomic_set(&phys_enc->pending_kickoff_cnt, 0);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 50c063de1509..1a319d56c879 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -366,19 +366,15 @@ static void dpu_encoder_phys_vid_mode_set(
struct drm_display_mode *mode,
struct drm_display_mode *adj_mode)
{
- struct dpu_encoder_irq *irq;
-
if (adj_mode) {
phys_enc->cached_mode = *adj_mode;
drm_mode_debug_printmodeline(adj_mode);
DPU_DEBUG_VIDENC(phys_enc, "caching mode:\n");
}
- irq = &phys_enc->irq[INTR_IDX_VSYNC];
- irq->irq_idx = phys_enc->hw_intf->cap->intr_vsync;
+ phys_enc->irq[INTR_IDX_VSYNC] = phys_enc->hw_intf->cap->intr_vsync;
- irq = &phys_enc->irq[INTR_IDX_UNDERRUN];
- irq->irq_idx = phys_enc->hw_intf->cap->intr_underrun;
+ phys_enc->irq[INTR_IDX_UNDERRUN] = phys_enc->hw_intf->cap->intr_underrun;
}
static int dpu_encoder_phys_vid_control_vblank_irq(
@@ -405,11 +401,12 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1)
ret = dpu_core_irq_register_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_VSYNC].irq_idx,
- phys_enc->irq[INTR_IDX_VSYNC].func, phys_enc);
+ phys_enc->irq[INTR_IDX_VSYNC],
+ dpu_encoder_phys_vid_vblank_irq,
+ phys_enc);
else if (!enable && atomic_dec_return(&phys_enc->vblank_refcount) == 0)
ret = dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_VSYNC].irq_idx);
+ phys_enc->irq[INTR_IDX_VSYNC]);
end:
if (ret) {
@@ -490,6 +487,7 @@ static int dpu_encoder_phys_vid_wait_for_vblank(
/* Wait for kickoff to complete */
ret = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_VSYNC,
+ dpu_encoder_phys_vid_vblank_irq,
&wait_info);
if (ret == -ETIMEDOUT) {
@@ -542,7 +540,7 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff(
ctl->idx, rc);
msm_disp_snapshot_state(drm_enc->dev);
dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_VSYNC].irq_idx);
+ phys_enc->irq[INTR_IDX_VSYNC]);
}
}
@@ -632,12 +630,13 @@ static void dpu_encoder_phys_vid_irq_control(struct dpu_encoder_phys *phys_enc,
return;
dpu_core_irq_register_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx,
- phys_enc->irq[INTR_IDX_UNDERRUN].func, phys_enc);
+ phys_enc->irq[INTR_IDX_UNDERRUN],
+ dpu_encoder_phys_vid_underrun_irq,
+ phys_enc);
} else {
dpu_encoder_phys_vid_control_vblank_irq(phys_enc, false);
dpu_core_irq_unregister_callback(phys_enc->dpu_kms,
- phys_enc->irq[INTR_IDX_UNDERRUN].irq_idx);
+ phys_enc->irq[INTR_IDX_UNDERRUN]);
}
}
@@ -703,7 +702,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
struct dpu_enc_phys_init_params *p)
{
struct dpu_encoder_phys *phys_enc = NULL;
- struct dpu_encoder_irq *irq;
int i;
if (!p) {
@@ -729,20 +727,8 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
phys_enc->split_role = p->split_role;
phys_enc->intf_mode = INTF_MODE_VIDEO;
phys_enc->enc_spinlock = p->enc_spinlock;
- for (i = 0; i < INTR_IDX_MAX; i++) {
- irq = &phys_enc->irq[i];
- irq->irq_idx = -EINVAL;
- }
-
- irq = &phys_enc->irq[INTR_IDX_VSYNC];
- irq->name = "vsync_irq";
- irq->intr_idx = INTR_IDX_VSYNC;
- irq->func = dpu_encoder_phys_vid_vblank_irq;
-
- irq = &phys_enc->irq[INTR_IDX_UNDERRUN];
- irq->name = "underrun";
- irq->intr_idx = INTR_IDX_UNDERRUN;
- irq->func = dpu_encoder_phys_vid_underrun_irq;
+ for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
+ phys_enc->irq[i] = -EINVAL;
atomic_set(&phys_enc->vblank_refcount, 0);
atomic_set(&phys_enc->pending_kickoff_cnt, 0);
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 5/6] drm/msm/dpu: remove struct dpu_encoder_irq
2022-02-17 4:31 ` [PATCH v3 5/6] drm/msm/dpu: remove struct dpu_encoder_irq Dmitry Baryshkov
@ 2022-03-03 21:21 ` Stephen Boyd
0 siblings, 0 replies; 12+ messages in thread
From: Stephen Boyd @ 2022-03-03 21:21 UTC (permalink / raw)
To: Abhinav Kumar, Bjorn Andersson, Dmitry Baryshkov, Rob Clark,
Sean Paul
Cc: David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno
Quoting Dmitry Baryshkov (2022-02-16 20:31:47)
> Remove additional indirection: specify IRQ callbacks and IRQ indices
> directly rather than through the pointer in the irq structure. For each
> IRQ we have a constant IRQ callback. This change simplifies code review
> as the reader no longer needs to remember which function is called.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 6/6] drm/msm/dpu: pass irq to dpu_encoder_helper_wait_for_irq()
2022-02-17 4:31 [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers Dmitry Baryshkov
` (4 preceding siblings ...)
2022-02-17 4:31 ` [PATCH v3 5/6] drm/msm/dpu: remove struct dpu_encoder_irq Dmitry Baryshkov
@ 2022-02-17 4:31 ` Dmitry Baryshkov
2022-03-03 21:22 ` [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers Stephen Boyd
6 siblings, 0 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-02-17 4:31 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Sean Paul, Abhinav Kumar
Cc: Stephen Boyd, David Airlie, Daniel Vetter, linux-arm-msm,
dri-devel, freedreno
Pass IRQ number directly rather than passing an index in the dpu_encoder's irq table.
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 29 +++++++++----------
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 4 +--
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 9 ++++--
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 3 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 12 ++++----
5 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 83b6715820fa..4c9e7c4fa14b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -260,38 +260,35 @@ static int dpu_encoder_helper_wait_event_timeout(int32_t drm_id,
u32 irq_idx, struct dpu_encoder_wait_info *info);
int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
- enum dpu_intr_idx intr_idx,
+ int irq,
void (*func)(void *arg, int irq_idx),
struct dpu_encoder_wait_info *wait_info)
{
- int irq;
u32 irq_status;
int ret;
- if (!wait_info || intr_idx >= INTR_IDX_MAX) {
+ if (!wait_info) {
DPU_ERROR("invalid params\n");
return -EINVAL;
}
- irq = phys_enc->irq[intr_idx];
-
/* note: do master / slave checking outside */
/* return EWOULDBLOCK since we know the wait isn't necessary */
if (phys_enc->enable_state == DPU_ENC_DISABLED) {
- DRM_ERROR("encoder is disabled id=%u, intr=%d, irq=%d\n",
- DRMID(phys_enc->parent), intr_idx,
+ DRM_ERROR("encoder is disabled id=%u, callback=%ps, irq=%d\n",
+ DRMID(phys_enc->parent), func,
irq);
return -EWOULDBLOCK;
}
if (irq < 0) {
- DRM_DEBUG_KMS("skip irq wait id=%u, intr=%d\n",
- DRMID(phys_enc->parent), intr_idx);
+ DRM_DEBUG_KMS("skip irq wait id=%u, callback=%ps\n",
+ DRMID(phys_enc->parent), func);
return 0;
}
- DRM_DEBUG_KMS("id=%u, intr=%d, irq=%d, pp=%d, pending_cnt=%d\n",
- DRMID(phys_enc->parent), intr_idx,
+ DRM_DEBUG_KMS("id=%u, callback=%ps, irq=%d, pp=%d, pending_cnt=%d\n",
+ DRMID(phys_enc->parent), func,
irq, phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
@@ -305,8 +302,8 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
if (irq_status) {
unsigned long flags;
- DRM_DEBUG_KMS("irq not triggered id=%u, intr=%d, irq=%d, pp=%d, atomic_cnt=%d\n",
- DRMID(phys_enc->parent), intr_idx,
+ DRM_DEBUG_KMS("irq not triggered id=%u, callback=%ps, irq=%d, pp=%d, atomic_cnt=%d\n",
+ DRMID(phys_enc->parent), func,
irq,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
@@ -316,8 +313,8 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
ret = 0;
} else {
ret = -ETIMEDOUT;
- DRM_DEBUG_KMS("irq timeout id=%u, intr=%d, irq=%d, pp=%d, atomic_cnt=%d\n",
- DRMID(phys_enc->parent), intr_idx,
+ DRM_DEBUG_KMS("irq timeout id=%u, callback=%ps, irq=%d, pp=%d, atomic_cnt=%d\n",
+ DRMID(phys_enc->parent), func,
irq,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
@@ -325,7 +322,7 @@ int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
} else {
ret = 0;
trace_dpu_enc_irq_wait_success(DRMID(phys_enc->parent),
- intr_idx, irq,
+ func, irq,
phys_enc->hw_pp->idx - PINGPONG_0,
atomic_read(wait_info->atomic_cnt));
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 803fd6f25da1..9843acdc33bd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -341,13 +341,13 @@ void dpu_encoder_helper_report_irq_timeout(struct dpu_encoder_phys *phys_enc,
* dpu_encoder_helper_wait_for_irq - utility to wait on an irq.
* note: will call dpu_encoder_helper_wait_for_irq on timeout
* @phys_enc: Pointer to physical encoder structure
- * @intr_idx: encoder interrupt index
+ * @irq: IRQ index
* @func: IRQ callback to be called in case of timeout
* @wait_info: wait info struct
* @Return: 0 or -ERROR
*/
int dpu_encoder_helper_wait_for_irq(struct dpu_encoder_phys *phys_enc,
- enum dpu_intr_idx intr_idx,
+ int irq,
void (*func)(void *arg, int irq_idx),
struct dpu_encoder_wait_info *wait_info);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 7f5b7f0fff2e..8bc3fdbe4821 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -234,7 +234,8 @@ static int _dpu_encoder_phys_cmd_wait_for_idle(
wait_info.atomic_cnt = &phys_enc->pending_kickoff_cnt;
wait_info.timeout_ms = KICKOFF_TIMEOUT_MS;
- ret = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_PINGPONG,
+ ret = dpu_encoder_helper_wait_for_irq(phys_enc,
+ phys_enc->irq[INTR_IDX_PINGPONG],
dpu_encoder_phys_cmd_pp_tx_done_irq,
&wait_info);
if (ret == -ETIMEDOUT)
@@ -659,7 +660,8 @@ static int _dpu_encoder_phys_cmd_wait_for_ctl_start(
wait_info.atomic_cnt = &phys_enc->pending_ctlstart_cnt;
wait_info.timeout_ms = KICKOFF_TIMEOUT_MS;
- ret = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_CTL_START,
+ ret = dpu_encoder_helper_wait_for_irq(phys_enc,
+ phys_enc->irq[INTR_IDX_CTL_START],
dpu_encoder_phys_cmd_ctl_start_irq,
&wait_info);
if (ret == -ETIMEDOUT) {
@@ -715,7 +717,8 @@ static int dpu_encoder_phys_cmd_wait_for_vblank(
atomic_inc(&cmd_enc->pending_vblank_cnt);
- rc = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_RDPTR,
+ rc = dpu_encoder_helper_wait_for_irq(phys_enc,
+ phys_enc->irq[INTR_IDX_RDPTR],
dpu_encoder_phys_cmd_pp_rd_ptr_irq,
&wait_info);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 1a319d56c879..f262e59d5f5c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -486,7 +486,8 @@ static int dpu_encoder_phys_vid_wait_for_vblank(
}
/* Wait for kickoff to complete */
- ret = dpu_encoder_helper_wait_for_irq(phys_enc, INTR_IDX_VSYNC,
+ ret = dpu_encoder_helper_wait_for_irq(phys_enc,
+ phys_enc->irq[INTR_IDX_VSYNC],
dpu_encoder_phys_vid_vblank_irq,
&wait_info);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 60f63a15a894..58b411fea7d7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -188,25 +188,25 @@ DEFINE_EVENT(dpu_irq_template, dpu_irq_unregister_success,
);
TRACE_EVENT(dpu_enc_irq_wait_success,
- TP_PROTO(uint32_t drm_id, enum dpu_intr_idx intr_idx,
+ TP_PROTO(uint32_t drm_id, void *func,
int irq_idx, enum dpu_pingpong pp_idx, int atomic_cnt),
- TP_ARGS(drm_id, intr_idx, irq_idx, pp_idx, atomic_cnt),
+ TP_ARGS(drm_id, func, irq_idx, pp_idx, atomic_cnt),
TP_STRUCT__entry(
__field( uint32_t, drm_id )
- __field( enum dpu_intr_idx, intr_idx )
+ __field( void *, func )
__field( int, irq_idx )
__field( enum dpu_pingpong, pp_idx )
__field( int, atomic_cnt )
),
TP_fast_assign(
__entry->drm_id = drm_id;
- __entry->intr_idx = intr_idx;
+ __entry->func = func;
__entry->irq_idx = irq_idx;
__entry->pp_idx = pp_idx;
__entry->atomic_cnt = atomic_cnt;
),
- TP_printk("id=%u, intr=%d, irq=%d, pp=%d, atomic_cnt=%d",
- __entry->drm_id, __entry->intr_idx,
+ TP_printk("id=%u, callback=%ps, irq=%d, pp=%d, atomic_cnt=%d",
+ __entry->drm_id, __entry->func,
__entry->irq_idx, __entry->pp_idx, __entry->atomic_cnt)
);
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers
2022-02-17 4:31 [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers Dmitry Baryshkov
` (5 preceding siblings ...)
2022-02-17 4:31 ` [PATCH v3 6/6] drm/msm/dpu: pass irq to dpu_encoder_helper_wait_for_irq() Dmitry Baryshkov
@ 2022-03-03 21:22 ` Stephen Boyd
2022-03-03 21:47 ` Bjorn Andersson
2022-03-03 21:52 ` Dmitry Baryshkov
6 siblings, 2 replies; 12+ messages in thread
From: Stephen Boyd @ 2022-03-03 21:22 UTC (permalink / raw)
To: Abhinav Kumar, Bjorn Andersson, Dmitry Baryshkov, Rob Clark,
Sean Paul
Cc: David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno
Quoting Dmitry Baryshkov (2022-02-16 20:31:42)
> This is the second part of
> https://patchwork.freedesktop.org/series/91631/ reworked and cleaned up.
>
> Changes since v1:
v2?
> - Fix warning ins dpu_trace.h related to
> dpu_core_irq_unregister_callback event
Any plans to migrate to hierarchical irqchip in here?
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers
2022-03-03 21:22 ` [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers Stephen Boyd
@ 2022-03-03 21:47 ` Bjorn Andersson
2022-03-03 21:52 ` Dmitry Baryshkov
1 sibling, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2022-03-03 21:47 UTC (permalink / raw)
To: Stephen Boyd
Cc: Abhinav Kumar, Dmitry Baryshkov, Rob Clark, Sean Paul,
David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno
On Thu 03 Mar 13:22 PST 2022, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2022-02-16 20:31:42)
> > This is the second part of
> > https://patchwork.freedesktop.org/series/91631/ reworked and cleaned up.
> >
> > Changes since v1:
>
> v2?
>
> > - Fix warning ins dpu_trace.h related to
> > dpu_core_irq_unregister_callback event
>
> Any plans to migrate to hierarchical irqchip in here?
I like this idea, since it would give us some more visibility, both in
form of /proc/interrupts and the standard irq tracepoints.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers
2022-03-03 21:22 ` [PATCH v3 0/6] drm/msm/dpu: simplify IRQ helpers Stephen Boyd
2022-03-03 21:47 ` Bjorn Andersson
@ 2022-03-03 21:52 ` Dmitry Baryshkov
1 sibling, 0 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2022-03-03 21:52 UTC (permalink / raw)
To: Stephen Boyd
Cc: Abhinav Kumar, Bjorn Andersson, Rob Clark, Sean Paul,
David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno
On Fri, 4 Mar 2022 at 00:23, Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Dmitry Baryshkov (2022-02-16 20:31:42)
> > This is the second part of
> > https://patchwork.freedesktop.org/series/91631/ reworked and cleaned up.
> >
> > Changes since v1:
>
> v2?
And the series should have been v3. Thanks for pointing it out!
>
> > - Fix warning ins dpu_trace.h related to
> > dpu_core_irq_unregister_callback event
>
> Any plans to migrate to hierarchical irqchip in here?
I haven't thought about it. I think that using Linux irq framework for
the DPU driver internals might be an overkill.
I'd say that it's not at the top of my todo list. However the point is noted.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread