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

[Dmitry Baryshkov]

On 6 December 2022 00:07:12 GMT+03:00, Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> wrote:
>
>
>On 5 December 2022 20:44:29 GMT+03:00, Bjorn Andersson <quic_bjorande@xxxxxxxxxxx> wrote:
>>From: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
>>
>>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@xxxxxxxxxx>
>>Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>>Signed-off-by: Bjorn Andersson <quic_bjorande@xxxxxxxxxxx>
>>---
>>
>>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
>

-- 
With best wishes
Dmitry