From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A42ADC54E58 for ; Mon, 18 Mar 2024 17:17:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=rUVQDwy3xrleeFSF5QkpHZLxZnleQbAdzUdiGD0u430=; b=g9wIfu7zbxnsbh Th3c+jGiwzcRjjNZFUv1O/F/fCmPmcPloZiJ5c18o5+mcdbKYdOLwE2Tk2WHq9S1w05T053MEF1MK WMakWVzTnYtaUcORLKFRPGXRXNkFDJJHNCYDYNBIVd8AFJ8eqrp5+wONM815miuuW5LIgzbkhpwzT 4RIFBWL3lp8m5FMwaLOCxc1kSEFrxZFvQjdVUUmuQ1q8JU4BXFxaNnDC6sU/u4cB5xq4QA19M3/93 SvYbVVPLI//TBKFAHLYcQv7cREGQF/obMuQFiEpE/NOz3DOvd93oCXw9wVdBiE52jW0QtDTPNrcB4 mqFKK1zBIyukWxMGGAcA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rmGbc-00000009PSq-1Bh6; Mon, 18 Mar 2024 17:17:00 +0000 Received: from perceval.ideasonboard.com ([213.167.242.64]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rmGbY-00000009PPd-14B3 for linux-arm-kernel@lists.infradead.org; Mon, 18 Mar 2024 17:16:58 +0000 Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BAAF87E9; Mon, 18 Mar 2024 18:16:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710782187; bh=sSz3J6DdrpfFg+fiM5uwo3CSw8bryn65eUaO1X2q4CI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=freZd4F0xyzGCudVLWlDsfpirbzAWyc2efjwWyaxIcmfzrxU1TubtiI6tfnpN6I7D PnqeMQIMPtyCqV/2AsdLvCa5qbuhAZ2Yc2hyibzUREqODuCT3P22X4Iux6fBJ3vPA/ /6i5e46s3LU0vL+LockMBi0Hr1nATn24f2MRCjoA= Date: Mon, 18 Mar 2024 19:16:51 +0200 From: Laurent Pinchart To: Sean Anderson Cc: Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , dri-devel@lists.freedesktop.org, David Airlie , linux-kernel@vger.kernel.org, Michal Simek , linux-arm-kernel@lists.infradead.org, Daniel Vetter Subject: Re: [PATCH 3/6] drm: zynqmp_dp: Add locking Message-ID: <20240318171651.GJ13682@pendragon.ideasonboard.com> References: <20240315230916.1759060-1-sean.anderson@linux.dev> <20240315230916.1759060-4-sean.anderson@linux.dev> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240315230916.1759060-4-sean.anderson@linux.dev> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240318_101656_496299_121617D9 X-CRM114-Status: GOOD ( 28.33 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Sean, Thank you for the patch. On Fri, Mar 15, 2024 at 07:09:13PM -0400, Sean Anderson wrote: > Add some locking, since none is provided by the drm subsystem. This will That's not quite right, the DRM core doesn't call bridge operations concurrently. We may need locking to protect against race conditions between bridge operations and interrupts though. > prevent the IRQ/workers/bridge API calls from stepping on each other's > toes. > > Signed-off-by: Sean Anderson > --- > > drivers/gpu/drm/xlnx/zynqmp_dp.c | 59 +++++++++++++++++++++++--------- > 1 file changed, 42 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c > index 8635b5673386..d2dee58e7bf2 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c > @@ -279,6 +279,7 @@ struct zynqmp_dp_config { > * @dpsub: Display subsystem > * @iomem: device I/O memory for register access > * @reset: reset controller > + * @lock: Mutex protecting this struct and register access (but not AUX) This patch does two things at once, it defers link training from the IRQ handler to a work queue, and covers everything with a big lock. The scope is too large. Please restrict the lock scope and document the individual fields that need to be protected, and explain the locking design in the commit message (or comments in the code). > * @irq: irq > * @bridge: DRM bridge for the DP encoder > * @next_bridge: The downstream bridge > @@ -299,6 +300,7 @@ struct zynqmp_dp { > struct zynqmp_dpsub *dpsub; > void __iomem *iomem; > struct reset_control *reset; > + struct mutex lock; > int irq; > > struct drm_bridge bridge; > @@ -308,7 +310,7 @@ struct zynqmp_dp { > struct drm_dp_aux aux; > struct phy *phy[ZYNQMP_DP_MAX_LANES]; > u8 num_lanes; > - struct delayed_work hpd_work; > + struct delayed_work hpd_work, hpd_irq_work; One variable per line please. > enum drm_connector_status status; > bool enabled; > > @@ -1371,8 +1373,10 @@ zynqmp_dp_bridge_mode_valid(struct drm_bridge *bridge, > } > > /* Check with link rate and lane count */ > + mutex_lock(&dp->lock); > rate = zynqmp_dp_max_rate(dp->link_config.max_rate, > dp->link_config.max_lanes, dp->config.bpp); > + mutex_unlock(&dp->lock); > if (mode->clock > rate) { > dev_dbg(dp->dev, "filtered mode %s for high pixel rate\n", > mode->name); > @@ -1399,6 +1403,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge, > > pm_runtime_get_sync(dp->dev); > > + mutex_lock(&dp->lock); > zynqmp_dp_disp_enable(dp, old_bridge_state); > > /* > @@ -1459,6 +1464,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge, > zynqmp_dp_write(dp, ZYNQMP_DP_SOFTWARE_RESET, > ZYNQMP_DP_SOFTWARE_RESET_ALL); > zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 1); > + mutex_unlock(&dp->lock); > } > > static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge, > @@ -1466,6 +1472,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge, > { > struct zynqmp_dp *dp = bridge_to_dp(bridge); > > + mutex_lock(&dp->lock); > dp->enabled = false; > cancel_delayed_work(&dp->hpd_work); > zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 0); > @@ -1476,6 +1483,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge, > zynqmp_dp_write(dp, ZYNQMP_DP_TX_AUDIO_CONTROL, 0); > > zynqmp_dp_disp_disable(dp, old_bridge_state); > + mutex_unlock(&dp->lock); > > pm_runtime_put_sync(dp->dev); > } > @@ -1518,6 +1526,8 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid > u32 state, i; > int ret; > > + mutex_lock(&dp->lock); > + > /* > * This is from heuristic. It takes some delay (ex, 100 ~ 500 msec) to > * get the HPD signal with some monitors. > @@ -1545,11 +1555,13 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid > dp->num_lanes); > > dp->status = connector_status_connected; > + mutex_unlock(&dp->lock); > return connector_status_connected; > } > > disconnected: > dp->status = connector_status_disconnected; > + mutex_unlock(&dp->lock); > return connector_status_disconnected; > } > > @@ -1611,6 +1623,29 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work) > drm_bridge_hpd_notify(&dp->bridge, status); > } > > +static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work) > +{ > + struct zynqmp_dp *dp = container_of(work, struct zynqmp_dp, > + hpd_irq_work.work); > + u8 status[DP_LINK_STATUS_SIZE + 2]; > + int err; > + > + mutex_lock(&dp->lock); > + err = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status, > + DP_LINK_STATUS_SIZE + 2); > + if (err < 0) { > + dev_dbg_ratelimited(dp->dev, > + "could not read sink status: %d\n", err); > + } else { > + if (status[4] & DP_LINK_STATUS_UPDATED || > + !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) || > + !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) { > + zynqmp_dp_train_loop(dp); > + } > + } > + mutex_unlock(&dp->lock); > +} > + > static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data) > { > struct zynqmp_dp *dp = (struct zynqmp_dp *)data; > @@ -1635,23 +1670,9 @@ static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data) > if (status & ZYNQMP_DP_INT_HPD_EVENT) > schedule_delayed_work(&dp->hpd_work, 0); > > - if (status & ZYNQMP_DP_INT_HPD_IRQ) { > - int ret; > - u8 status[DP_LINK_STATUS_SIZE + 2]; > + if (status & ZYNQMP_DP_INT_HPD_IRQ) > + schedule_delayed_work(&dp->hpd_irq_work, 0); > > - ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status, > - DP_LINK_STATUS_SIZE + 2); > - if (ret < 0) > - goto handled; > - > - if (status[4] & DP_LINK_STATUS_UPDATED || > - !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) || > - !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) { > - zynqmp_dp_train_loop(dp); > - } > - } > - > -handled: > return IRQ_HANDLED; > } > > @@ -1674,8 +1695,10 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub) > dp->dev = &pdev->dev; > dp->dpsub = dpsub; > dp->status = connector_status_disconnected; > + mutex_init(&dp->lock); > > INIT_DELAYED_WORK(&dp->hpd_work, zynqmp_dp_hpd_work_func); > + INIT_DELAYED_WORK(&dp->hpd_irq_work, zynqmp_dp_hpd_irq_work_func); > > /* Acquire all resources (IOMEM, IRQ and PHYs). */ > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dp"); > @@ -1775,6 +1798,7 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub) > zynqmp_dp_write(dp, ZYNQMP_DP_INT_DS, ZYNQMP_DP_INT_ALL); > disable_irq(dp->irq); > > + cancel_delayed_work_sync(&dp->hpd_irq_work); > cancel_delayed_work_sync(&dp->hpd_work); > > zynqmp_dp_write(dp, ZYNQMP_DP_TRANSMITTER_ENABLE, 0); > @@ -1782,4 +1806,5 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub) > > zynqmp_dp_phy_exit(dp); > zynqmp_dp_reset(dp, true); > + mutex_destroy(&dp->lock); > } -- Regards, Laurent Pinchart _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EC8E5C54E5D for ; Mon, 18 Mar 2024 17:16:57 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 420BA10FCFA; Mon, 18 Mar 2024 17:16:57 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="freZd4F0"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1993610FCFA for ; Mon, 18 Mar 2024 17:16:55 +0000 (UTC) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BAAF87E9; Mon, 18 Mar 2024 18:16:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710782187; bh=sSz3J6DdrpfFg+fiM5uwo3CSw8bryn65eUaO1X2q4CI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=freZd4F0xyzGCudVLWlDsfpirbzAWyc2efjwWyaxIcmfzrxU1TubtiI6tfnpN6I7D PnqeMQIMPtyCqV/2AsdLvCa5qbuhAZ2Yc2hyibzUREqODuCT3P22X4Iux6fBJ3vPA/ /6i5e46s3LU0vL+LockMBi0Hr1nATn24f2MRCjoA= Date: Mon, 18 Mar 2024 19:16:51 +0200 From: Laurent Pinchart To: Sean Anderson Cc: Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , dri-devel@lists.freedesktop.org, David Airlie , linux-kernel@vger.kernel.org, Michal Simek , linux-arm-kernel@lists.infradead.org, Daniel Vetter Subject: Re: [PATCH 3/6] drm: zynqmp_dp: Add locking Message-ID: <20240318171651.GJ13682@pendragon.ideasonboard.com> References: <20240315230916.1759060-1-sean.anderson@linux.dev> <20240315230916.1759060-4-sean.anderson@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20240315230916.1759060-4-sean.anderson@linux.dev> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Sean, Thank you for the patch. On Fri, Mar 15, 2024 at 07:09:13PM -0400, Sean Anderson wrote: > Add some locking, since none is provided by the drm subsystem. This will That's not quite right, the DRM core doesn't call bridge operations concurrently. We may need locking to protect against race conditions between bridge operations and interrupts though. > prevent the IRQ/workers/bridge API calls from stepping on each other's > toes. > > Signed-off-by: Sean Anderson > --- > > drivers/gpu/drm/xlnx/zynqmp_dp.c | 59 +++++++++++++++++++++++--------- > 1 file changed, 42 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c > index 8635b5673386..d2dee58e7bf2 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c > @@ -279,6 +279,7 @@ struct zynqmp_dp_config { > * @dpsub: Display subsystem > * @iomem: device I/O memory for register access > * @reset: reset controller > + * @lock: Mutex protecting this struct and register access (but not AUX) This patch does two things at once, it defers link training from the IRQ handler to a work queue, and covers everything with a big lock. The scope is too large. Please restrict the lock scope and document the individual fields that need to be protected, and explain the locking design in the commit message (or comments in the code). > * @irq: irq > * @bridge: DRM bridge for the DP encoder > * @next_bridge: The downstream bridge > @@ -299,6 +300,7 @@ struct zynqmp_dp { > struct zynqmp_dpsub *dpsub; > void __iomem *iomem; > struct reset_control *reset; > + struct mutex lock; > int irq; > > struct drm_bridge bridge; > @@ -308,7 +310,7 @@ struct zynqmp_dp { > struct drm_dp_aux aux; > struct phy *phy[ZYNQMP_DP_MAX_LANES]; > u8 num_lanes; > - struct delayed_work hpd_work; > + struct delayed_work hpd_work, hpd_irq_work; One variable per line please. > enum drm_connector_status status; > bool enabled; > > @@ -1371,8 +1373,10 @@ zynqmp_dp_bridge_mode_valid(struct drm_bridge *bridge, > } > > /* Check with link rate and lane count */ > + mutex_lock(&dp->lock); > rate = zynqmp_dp_max_rate(dp->link_config.max_rate, > dp->link_config.max_lanes, dp->config.bpp); > + mutex_unlock(&dp->lock); > if (mode->clock > rate) { > dev_dbg(dp->dev, "filtered mode %s for high pixel rate\n", > mode->name); > @@ -1399,6 +1403,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge, > > pm_runtime_get_sync(dp->dev); > > + mutex_lock(&dp->lock); > zynqmp_dp_disp_enable(dp, old_bridge_state); > > /* > @@ -1459,6 +1464,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge, > zynqmp_dp_write(dp, ZYNQMP_DP_SOFTWARE_RESET, > ZYNQMP_DP_SOFTWARE_RESET_ALL); > zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 1); > + mutex_unlock(&dp->lock); > } > > static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge, > @@ -1466,6 +1472,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge, > { > struct zynqmp_dp *dp = bridge_to_dp(bridge); > > + mutex_lock(&dp->lock); > dp->enabled = false; > cancel_delayed_work(&dp->hpd_work); > zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 0); > @@ -1476,6 +1483,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge, > zynqmp_dp_write(dp, ZYNQMP_DP_TX_AUDIO_CONTROL, 0); > > zynqmp_dp_disp_disable(dp, old_bridge_state); > + mutex_unlock(&dp->lock); > > pm_runtime_put_sync(dp->dev); > } > @@ -1518,6 +1526,8 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid > u32 state, i; > int ret; > > + mutex_lock(&dp->lock); > + > /* > * This is from heuristic. It takes some delay (ex, 100 ~ 500 msec) to > * get the HPD signal with some monitors. > @@ -1545,11 +1555,13 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid > dp->num_lanes); > > dp->status = connector_status_connected; > + mutex_unlock(&dp->lock); > return connector_status_connected; > } > > disconnected: > dp->status = connector_status_disconnected; > + mutex_unlock(&dp->lock); > return connector_status_disconnected; > } > > @@ -1611,6 +1623,29 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work) > drm_bridge_hpd_notify(&dp->bridge, status); > } > > +static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work) > +{ > + struct zynqmp_dp *dp = container_of(work, struct zynqmp_dp, > + hpd_irq_work.work); > + u8 status[DP_LINK_STATUS_SIZE + 2]; > + int err; > + > + mutex_lock(&dp->lock); > + err = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status, > + DP_LINK_STATUS_SIZE + 2); > + if (err < 0) { > + dev_dbg_ratelimited(dp->dev, > + "could not read sink status: %d\n", err); > + } else { > + if (status[4] & DP_LINK_STATUS_UPDATED || > + !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) || > + !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) { > + zynqmp_dp_train_loop(dp); > + } > + } > + mutex_unlock(&dp->lock); > +} > + > static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data) > { > struct zynqmp_dp *dp = (struct zynqmp_dp *)data; > @@ -1635,23 +1670,9 @@ static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data) > if (status & ZYNQMP_DP_INT_HPD_EVENT) > schedule_delayed_work(&dp->hpd_work, 0); > > - if (status & ZYNQMP_DP_INT_HPD_IRQ) { > - int ret; > - u8 status[DP_LINK_STATUS_SIZE + 2]; > + if (status & ZYNQMP_DP_INT_HPD_IRQ) > + schedule_delayed_work(&dp->hpd_irq_work, 0); > > - ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status, > - DP_LINK_STATUS_SIZE + 2); > - if (ret < 0) > - goto handled; > - > - if (status[4] & DP_LINK_STATUS_UPDATED || > - !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) || > - !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) { > - zynqmp_dp_train_loop(dp); > - } > - } > - > -handled: > return IRQ_HANDLED; > } > > @@ -1674,8 +1695,10 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub) > dp->dev = &pdev->dev; > dp->dpsub = dpsub; > dp->status = connector_status_disconnected; > + mutex_init(&dp->lock); > > INIT_DELAYED_WORK(&dp->hpd_work, zynqmp_dp_hpd_work_func); > + INIT_DELAYED_WORK(&dp->hpd_irq_work, zynqmp_dp_hpd_irq_work_func); > > /* Acquire all resources (IOMEM, IRQ and PHYs). */ > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dp"); > @@ -1775,6 +1798,7 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub) > zynqmp_dp_write(dp, ZYNQMP_DP_INT_DS, ZYNQMP_DP_INT_ALL); > disable_irq(dp->irq); > > + cancel_delayed_work_sync(&dp->hpd_irq_work); > cancel_delayed_work_sync(&dp->hpd_work); > > zynqmp_dp_write(dp, ZYNQMP_DP_TRANSMITTER_ENABLE, 0); > @@ -1782,4 +1806,5 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub) > > zynqmp_dp_phy_exit(dp); > zynqmp_dp_reset(dp, true); > + mutex_destroy(&dp->lock); > } -- Regards, Laurent Pinchart