diff mbox series

[v4,09/13] drm/msm/dp: Don't enable HPD interrupts for edp

Message ID 20221205174433.16847-10-quic_bjorande@quicinc.com (mailing list archive)
State Superseded
Headers show
Series drm/msm: Add SC8280XP support | expand

Commit Message

Bjorn Andersson Dec. 5, 2022, 5:44 p.m. UTC 
From: Bjorn Andersson <bjorn.andersson@linaro.org>

Most instances where HPD interrupts are masked and unmasked are guareded
by the presence of an EDP panel being connected, but not all. Extend
this to cover the last few places, as HPD interrupt handling is not used
for the EDP case.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---

Changes since v3:
- None

 drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

Dmitry Baryshkov Dec. 5, 2022, 9:07 p.m. UTC | #1 
On 5 December 2022 20:44:29 GMT+03:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>From: Bjorn Andersson <bjorn.andersson@linaro.org>
>
>Most instances where HPD interrupts are masked and unmasked are guareded
>by the presence of an EDP panel being connected, but not all. Extend
>this to cover the last few places, as HPD interrupt handling is not used
>for the EDP case.

I don't remember whether I asked that or not. Would it be possible to move hpd irq enablement to bridge's hpd_enable() / hpd_disable() callbacks ? I think this would allow us to drop the is_edp checks.

>
>Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
>---
>
>Changes since v3:
>- None
>
> drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>index 17fcf8cd84cd..bb92c33beff8 100644
>--- a/drivers/gpu/drm/msm/dp/dp_display.c
>+++ b/drivers/gpu/drm/msm/dp/dp_display.c
>@@ -610,8 +610,10 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data)
> 	}
> 
> 	/* enable HDP irq_hpd/replug interrupt */
>-	dp_catalog_hpd_config_intr(dp->catalog,
>-		DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, true);
>+	if (!dp->dp_display.is_edp)
>+		dp_catalog_hpd_config_intr(dp->catalog,
>+					   DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK,
>+					   true);
> 
> 	drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n",
> 			dp->dp_display.connector_type, state);
>@@ -651,8 +653,10 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data)
> 			dp->dp_display.connector_type, state);
> 
> 	/* disable irq_hpd/replug interrupts */
>-	dp_catalog_hpd_config_intr(dp->catalog,
>-		DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, false);
>+	if (!dp->dp_display.is_edp)
>+		dp_catalog_hpd_config_intr(dp->catalog,
>+					   DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK,
>+					   false);
> 
> 	/* unplugged, no more irq_hpd handle */
> 	dp_del_event(dp, EV_IRQ_HPD_INT);
>@@ -678,7 +682,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data)
> 	}
> 
> 	/* disable HPD plug interrupts */
>-	dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false);
>+	if (!dp->dp_display.is_edp)
>+		dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false);
> 
> 	/*
> 	 * We don't need separate work for disconnect as
Dmitry Baryshkov Dec. 5, 2022, 9:11 p.m. UTC | #2 
On 6 December 2022 00:07:12 GMT+03:00, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:
>
>
>On 5 December 2022 20:44:29 GMT+03:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>>From: Bjorn Andersson <bjorn.andersson@linaro.org>
>>
>>Most instances where HPD interrupts are masked and unmasked are guareded
>>by the presence of an EDP panel being connected, but not all. Extend
>>this to cover the last few places, as HPD interrupt handling is not used
>>for the EDP case.
>
>I don't remember whether I asked that or not. Would it be possible to move hpd irq enablement to bridge's hpd_enable() / hpd_disable() callbacks ? I think this would allow us to drop the is_edp checks.

Ignore this. I should read the series carefully.

>
>>
>>Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
>>---
>>
>>Changes since v3:
>>- None
>>
>> drivers/gpu/drm/msm/dp/dp_display.c | 15 ++++++++++-----
>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>>index 17fcf8cd84cd..bb92c33beff8 100644
>>--- a/drivers/gpu/drm/msm/dp/dp_display.c
>>+++ b/drivers/gpu/drm/msm/dp/dp_display.c
>>@@ -610,8 +610,10 @@ static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data)
>> 	}
>> 
>> 	/* enable HDP irq_hpd/replug interrupt */
>>-	dp_catalog_hpd_config_intr(dp->catalog,
>>-		DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, true);
>>+	if (!dp->dp_display.is_edp)
>>+		dp_catalog_hpd_config_intr(dp->catalog,
>>+					   DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK,
>>+					   true);
>> 
>> 	drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n",
>> 			dp->dp_display.connector_type, state);
>>@@ -651,8 +653,10 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data)
>> 			dp->dp_display.connector_type, state);
>> 
>> 	/* disable irq_hpd/replug interrupts */
>>-	dp_catalog_hpd_config_intr(dp->catalog,
>>-		DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, false);
>>+	if (!dp->dp_display.is_edp)
>>+		dp_catalog_hpd_config_intr(dp->catalog,
>>+					   DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK,
>>+					   false);
>> 
>> 	/* unplugged, no more irq_hpd handle */
>> 	dp_del_event(dp, EV_IRQ_HPD_INT);
>>@@ -678,7 +682,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data)
>> 	}
>> 
>> 	/* disable HPD plug interrupts */
>>-	dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false);
>>+	if (!dp->dp_display.is_edp)
>>+		dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false);
>> 
>> 	/*
>> 	 * We don't need separate work for disconnect as
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 17fcf8cd84cd..bb92c33beff8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -610,8 +610,10 @@  static int dp_hpd_plug_handle(struct dp_display_private *dp, u32 data)
 	}
 
 	/* enable HDP irq_hpd/replug interrupt */
-	dp_catalog_hpd_config_intr(dp->catalog,
-		DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, true);
+	if (!dp->dp_display.is_edp)
+		dp_catalog_hpd_config_intr(dp->catalog,
+					   DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK,
+					   true);
 
 	drm_dbg_dp(dp->drm_dev, "After, type=%d hpd_state=%d\n",
 			dp->dp_display.connector_type, state);
@@ -651,8 +653,10 @@  static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data)
 			dp->dp_display.connector_type, state);
 
 	/* disable irq_hpd/replug interrupts */
-	dp_catalog_hpd_config_intr(dp->catalog,
-		DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK, false);
+	if (!dp->dp_display.is_edp)
+		dp_catalog_hpd_config_intr(dp->catalog,
+					   DP_DP_IRQ_HPD_INT_MASK | DP_DP_HPD_REPLUG_INT_MASK,
+					   false);
 
 	/* unplugged, no more irq_hpd handle */
 	dp_del_event(dp, EV_IRQ_HPD_INT);
@@ -678,7 +682,8 @@  static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data)
 	}
 
 	/* disable HPD plug interrupts */
-	dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false);
+	if (!dp->dp_display.is_edp)
+		dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, false);
 
 	/*
 	 * We don't need separate work for disconnect as