[PATCH v6 2/2] drm/msm/dp: enhance dp controller isr

From: Kuogee Hsieh
Date: Tue Dec 27 2022 - 21:17:01 EST

Next message: Kuogee Hsieh: "[PATCH v6 0/2] do not complete dp_aux_cmd_fifo_tx() if irq is not for aux transfer"
Previous message: SeongJae Park: "Re: [PATCH v2] mm/page_reporting: replace rcu_access_pointer() with rcu_dereference_protected()"
In reply to: Kuogee Hsieh: "[PATCH v6 0/2] do not complete dp_aux_cmd_fifo_tx() if irq is not for aux transfer"
Next in thread: Kuogee Hsieh: "[PATCH v6 1/2] drm/msm/dp: do not complete dp_aux_cmd_fifo_tx() if irq is not for aux transfer"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

dp_display_irq_handler() is the main isr handler with the helps
of two sub isr, dp_aux_isr and dp_ctrl_isr, to service all DP
interrupts on every irq triggered. Current all three isr does
not return IRQ_HANDLED if there are any interrupts it had
serviced. This patch fix this ambiguity by having all isr
return IRQ_HANDLED if there are interrupts had been serviced
or IRQ_NONE otherwise.

Changes in v5:
-- move complete into dp_aux_native_handler()
-- move complete into dp_aux_i2c_handler()
-- restore null ctrl check at isr
-- return IRQ_NODE directly

Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
Suggested-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
---
drivers/gpu/drm/msm/dp/dp_aux.c | 95 ++++++++++++++++++++++++++-----------
drivers/gpu/drm/msm/dp/dp_aux.h | 2 +-
drivers/gpu/drm/msm/dp/dp_ctrl.c | 12 ++++-
drivers/gpu/drm/msm/dp/dp_ctrl.h | 2 +-
drivers/gpu/drm/msm/dp/dp_display.c | 16 +++++--
5 files changed, 89 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
index cc3efed..d01ff45 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.c
+++ b/drivers/gpu/drm/msm/dp/dp_aux.c
@@ -162,45 +162,84 @@ static ssize_t dp_aux_cmd_fifo_rx(struct dp_aux_private *aux,
return i;
}

-static void dp_aux_native_handler(struct dp_aux_private *aux, u32 isr)
+static irqreturn_t dp_aux_native_handler(struct dp_aux_private *aux, u32 isr)
{
- if (isr & DP_INTR_AUX_I2C_DONE)
+ irqreturn_t ret = IRQ_NONE;
+
+ if (isr & DP_INTR_AUX_I2C_DONE) {
aux->aux_error_num = DP_AUX_ERR_NONE;
- else if (isr & DP_INTR_WRONG_ADDR)
+ ret = IRQ_HANDLED;
+ } else if (isr & DP_INTR_WRONG_ADDR) {
aux->aux_error_num = DP_AUX_ERR_ADDR;
- else if (isr & DP_INTR_TIMEOUT)
+ ret = IRQ_HANDLED;
+ } else if (isr & DP_INTR_TIMEOUT) {
aux->aux_error_num = DP_AUX_ERR_TOUT;
- if (isr & DP_INTR_NACK_DEFER)
+ ret = IRQ_HANDLED;
+ }
+
+ if (isr & DP_INTR_NACK_DEFER) {
aux->aux_error_num = DP_AUX_ERR_NACK;
+ ret = IRQ_HANDLED;
+ }
+
if (isr & DP_INTR_AUX_ERROR) {
aux->aux_error_num = DP_AUX_ERR_PHY;
dp_catalog_aux_clear_hw_interrupts(aux->catalog);
+ ret = IRQ_HANDLED;
}
+
+ if (ret == IRQ_HANDLED)
+ complete(&aux->comp);
+
+ return ret;
}

-static void dp_aux_i2c_handler(struct dp_aux_private *aux, u32 isr)
+static irqreturn_t dp_aux_i2c_handler(struct dp_aux_private *aux, u32 isr)
{
+ irqreturn_t ret = IRQ_NONE;
+
if (isr & DP_INTR_AUX_I2C_DONE) {
if (isr & (DP_INTR_I2C_NACK | DP_INTR_I2C_DEFER))
aux->aux_error_num = DP_AUX_ERR_NACK;
else
aux->aux_error_num = DP_AUX_ERR_NONE;
- } else {
- if (isr & DP_INTR_WRONG_ADDR)
- aux->aux_error_num = DP_AUX_ERR_ADDR;
- else if (isr & DP_INTR_TIMEOUT)
- aux->aux_error_num = DP_AUX_ERR_TOUT;
- if (isr & DP_INTR_NACK_DEFER)
- aux->aux_error_num = DP_AUX_ERR_NACK_DEFER;
- if (isr & DP_INTR_I2C_NACK)
- aux->aux_error_num = DP_AUX_ERR_NACK;
- if (isr & DP_INTR_I2C_DEFER)
- aux->aux_error_num = DP_AUX_ERR_DEFER;
- if (isr & DP_INTR_AUX_ERROR) {
- aux->aux_error_num = DP_AUX_ERR_PHY;
- dp_catalog_aux_clear_hw_interrupts(aux->catalog);
- }
+
+ return IRQ_HANDLED;
}
+
+ if (isr & DP_INTR_WRONG_ADDR) {
+ aux->aux_error_num = DP_AUX_ERR_ADDR;
+ ret = IRQ_HANDLED;
+ } else if (isr & DP_INTR_TIMEOUT) {
+ aux->aux_error_num = DP_AUX_ERR_TOUT;
+ ret = IRQ_HANDLED;
+ }
+
+ if (isr & DP_INTR_NACK_DEFER) {
+ aux->aux_error_num = DP_AUX_ERR_NACK_DEFER;
+ ret = IRQ_HANDLED;
+ }
+
+ if (isr & DP_INTR_I2C_NACK) {
+ aux->aux_error_num = DP_AUX_ERR_NACK;
+ ret = IRQ_HANDLED;
+ }
+
+ if (isr & DP_INTR_I2C_DEFER) {
+ aux->aux_error_num = DP_AUX_ERR_DEFER;
+ ret = IRQ_HANDLED;
+ }
+
+ if (isr & DP_INTR_AUX_ERROR) {
+ aux->aux_error_num = DP_AUX_ERR_PHY;
+ dp_catalog_aux_clear_hw_interrupts(aux->catalog);
+ ret = IRQ_HANDLED;
+ }
+
+ if (ret == IRQ_HANDLED)
+ complete(&aux->comp);
+
+ return ret;
}

static void dp_aux_update_offset_and_segment(struct dp_aux_private *aux,
@@ -409,14 +448,14 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
return ret;
}

-void dp_aux_isr(struct drm_dp_aux *dp_aux)
+irqreturn_t dp_aux_isr(struct drm_dp_aux *dp_aux)
{
u32 isr;
struct dp_aux_private *aux;

if (!dp_aux) {
DRM_ERROR("invalid input\n");
- return;
+ return IRQ_NONE;
}

aux = container_of(dp_aux, struct dp_aux_private, dp_aux);
@@ -425,17 +464,15 @@ void dp_aux_isr(struct drm_dp_aux *dp_aux)

/* no interrupts pending, return immediately */
if (!isr)
- return;
+ return IRQ_NONE;

if (!aux->cmd_busy)
- return;
+ return IRQ_NONE;

if (aux->native)
- dp_aux_native_handler(aux, isr);
+ return dp_aux_native_handler(aux, isr);
else
- dp_aux_i2c_handler(aux, isr);
-
- complete(&aux->comp);
+ return dp_aux_i2c_handler(aux, isr);
}

void dp_aux_reconfig(struct drm_dp_aux *dp_aux)
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.h b/drivers/gpu/drm/msm/dp/dp_aux.h
index e930974..511305d 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.h
+++ b/drivers/gpu/drm/msm/dp/dp_aux.h
@@ -11,7 +11,7 @@

int dp_aux_register(struct drm_dp_aux *dp_aux);
void dp_aux_unregister(struct drm_dp_aux *dp_aux);
-void dp_aux_isr(struct drm_dp_aux *dp_aux);
+irqreturn_t dp_aux_isr(struct drm_dp_aux *dp_aux);
void dp_aux_init(struct drm_dp_aux *dp_aux);
void dp_aux_deinit(struct drm_dp_aux *dp_aux);
void dp_aux_reconfig(struct drm_dp_aux *dp_aux);
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 3854c9f..cb0acb1 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1982,27 +1982,35 @@ int dp_ctrl_off(struct dp_ctrl *dp_ctrl)
return ret;
}

-void dp_ctrl_isr(struct dp_ctrl *dp_ctrl)
+irqreturn_t dp_ctrl_isr(struct dp_ctrl *dp_ctrl)
{
struct dp_ctrl_private *ctrl;
u32 isr;
+ irqreturn_t ret = IRQ_NONE;

if (!dp_ctrl)
- return;
+ return IRQ_NONE;

ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);

isr = dp_catalog_ctrl_get_interrupt(ctrl->catalog);
+ /* no interrupts pending, return immediately */
+ if (!isr)
+ return IRQ_NONE;

if (isr & DP_CTRL_INTR_READY_FOR_VIDEO) {
drm_dbg_dp(ctrl->drm_dev, "dp_video_ready\n");
complete(&ctrl->video_comp);
+ ret = IRQ_HANDLED;
}

if (isr & DP_CTRL_INTR_IDLE_PATTERN_SENT) {
drm_dbg_dp(ctrl->drm_dev, "idle_patterns_sent\n");
complete(&ctrl->idle_comp);
+ ret = IRQ_HANDLED;
}
+
+ return ret;
}

struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 9f29734..c3af06d 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -25,7 +25,7 @@ int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl);
int dp_ctrl_off_link(struct dp_ctrl *dp_ctrl);
int dp_ctrl_off(struct dp_ctrl *dp_ctrl);
void dp_ctrl_push_idle(struct dp_ctrl *dp_ctrl);
-void dp_ctrl_isr(struct dp_ctrl *dp_ctrl);
+irqreturn_t dp_ctrl_isr(struct dp_ctrl *dp_ctrl);
void dp_ctrl_handle_sink_request(struct dp_ctrl *dp_ctrl);
struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
struct dp_panel *panel, struct drm_dp_aux *aux,
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index bfd0aef..d40bfbd 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1192,7 +1192,7 @@ static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
{
struct dp_display_private *dp = dev_id;
- irqreturn_t ret = IRQ_HANDLED;
+ irqreturn_t ret = IRQ_NONE;
u32 hpd_isr_status;

if (!dp) {
@@ -1206,27 +1206,33 @@ static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
drm_dbg_dp(dp->drm_dev, "type=%d isr=0x%x\n",
dp->dp_display.connector_type, hpd_isr_status);
/* hpd related interrupts */
- if (hpd_isr_status & DP_DP_HPD_PLUG_INT_MASK)
+ if (hpd_isr_status & DP_DP_HPD_PLUG_INT_MASK) {
dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
+ ret = IRQ_HANDLED;
+ }

if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK) {
dp_add_event(dp, EV_IRQ_HPD_INT, 0, 0);
+ ret = IRQ_HANDLED;
}

if (hpd_isr_status & DP_DP_HPD_REPLUG_INT_MASK) {
dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
dp_add_event(dp, EV_HPD_PLUG_INT, 0, 3);
+ ret = IRQ_HANDLED;
}

- if (hpd_isr_status & DP_DP_HPD_UNPLUG_INT_MASK)
+ if (hpd_isr_status & DP_DP_HPD_UNPLUG_INT_MASK) {
dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+ ret = IRQ_HANDLED;
+ }
}

/* DP controller isr */
- dp_ctrl_isr(dp->ctrl);
+ ret |= dp_ctrl_isr(dp->ctrl);

/* DP aux isr */
- dp_aux_isr(dp->aux);
+ ret |= dp_aux_isr(dp->aux);

return ret;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Next message: Kuogee Hsieh: "[PATCH v6 0/2] do not complete dp_aux_cmd_fifo_tx() if irq is not for aux transfer"
Previous message: SeongJae Park: "Re: [PATCH v2] mm/page_reporting: replace rcu_access_pointer() with rcu_dereference_protected()"
In reply to: Kuogee Hsieh: "[PATCH v6 0/2] do not complete dp_aux_cmd_fifo_tx() if irq is not for aux transfer"
Next in thread: Kuogee Hsieh: "[PATCH v6 1/2] drm/msm/dp: do not complete dp_aux_cmd_fifo_tx() if irq is not for aux transfer"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]