On 2021-05-14 00:03, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2021-05-12 16:29:59)
irq_hpd interrupt should be handled after dongle plugged in and
before dongle unplugged. Hence irq_hpd interrupt is enabled at
the end of the plugin handle and disabled at the beginning of
unplugged handle. Current irq_hpd with sink_count = 0 is wrongly
handled same as the dongle unplugged which tears down the mainlink
and disables the phy. This patch fixes this problem by only tearing
down the mainlink but keeping phy enabled at irq_hpd with
sink_count = 0 handle so that next irq_hpe with sink_count =1 can be
s/irq_hpe/irq/hpd/
handled by setup mainlink only.
Changes in v2:
-- add ctrl->phy_Power_count
Changes in v3:
-- del ctrl->phy_Power_count
-- add phy_power_off to dp_ctrl_off_link_stream()
Signed-off-by: Kuogee Hsieh <khsieh@xxxxxxxxxxxxxx>
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 5 ++--
drivers/gpu/drm/msm/dp/dp_ctrl.c | 54
+++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/msm/dp/dp_ctrl.h | 2 ++
drivers/gpu/drm/msm/dp/dp_display.c | 46
++++++++++++++++++++++++-------
4 files changed, 94 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index b1a9b1b..f4f53f2 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -582,10 +582,9 @@ void dp_catalog_ctrl_hpd_config(struct dp_catalog
*dp_catalog)
u32 reftimer = dp_read_aux(catalog, REG_DP_DP_HPD_REFTIMER);
- /* enable HPD interrupts */
+ /* enable HPD plug and unplug interrupts */
dp_catalog_hpd_config_intr(dp_catalog,
- DP_DP_HPD_PLUG_INT_MASK | DP_DP_IRQ_HPD_INT_MASK
- | DP_DP_HPD_UNPLUG_INT_MASK |
DP_DP_HPD_REPLUG_INT_MASK, true);
+ DP_DP_HPD_PLUG_INT_MASK | DP_DP_HPD_UNPLUG_INT_MASK,
true);
/* Configure REFTIMER and enable it */
reftimer |= DP_DP_HPD_REFTIMER_ENABLE;
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 8d59eb9..41c122c 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1811,6 +1811,60 @@ int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl)
return ret;
}
+int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl)
+{
+ struct dp_ctrl_private *ctrl;
+ struct dp_io *dp_io;
+ struct phy *phy;
+ int ret = 0;
Please don't assign ret and then reassign it without testing it first.
+
+ if (!dp_ctrl)
Does this ever happen?
+ return -EINVAL;
+
+ ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
+ dp_io = &ctrl->parser->io;
+ phy = dp_io->phy;
+
+ dp_catalog_ctrl_mainlink_ctrl(ctrl->catalog, false);
+
+ ret = dp_power_clk_enable(ctrl->power, DP_STREAM_PM, false);
+ if (ret)
+ DRM_ERROR("Failed to disable pixel clocks. ret=%d\n",
ret);
Why do we continue instead of returning with a failure?
+
+ ret = dp_power_clk_enable(ctrl->power, DP_CTRL_PM, false);
+ if (ret)
+ DRM_ERROR("Failed to disable link clocks. ret=%d\n",
ret);
Same, why do we continue?
+
+ phy_power_off(phy);
+ phy_exit(phy);
+
+ /* aux channel down, reinit phy */
So we need to call exit/init to reinitialize the phy? I wonder if there
could be some phy_reinit() function that combines those.
there is no phy_reinit() function existing currently.
we can add phy_reinit() to combine phy_exit/phy_init as long as ther is
no changed to phy->init_count.
+ phy_init(phy);
+
+ DRM_DEBUG_DP("DP off link/stream done\n");
+ return ret;
Except here we return with a failure.
+}
+
+void dp_ctrl_off_phy(struct dp_ctrl *dp_ctrl)
+{
+ struct dp_ctrl_private *ctrl;
+ struct dp_io *dp_io;
+ struct phy *phy;
+
+ if (!dp_ctrl)
Does this ever happen?
+ return;
+
+ ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
+ dp_io = &ctrl->parser->io;
+ phy = dp_io->phy;
+
+ dp_catalog_ctrl_reset(ctrl->catalog);
+
+ phy_exit(phy);
+
+ DRM_DEBUG_DP("DP off phy done\n");
+}
+
int dp_ctrl_off(struct dp_ctrl *dp_ctrl)
{
struct dp_ctrl_private *ctrl;
