Re: [Nouveau] [PATCH v2 22/27] drm/nouveau: Don't grab runtime PM refs for HPD IRQs

2019-09-25 Thread Sean Paul
On Tue, Sep 03, 2019 at 04:46:00PM -0400, Lyude Paul wrote:
> In order for suspend/resume reprobing to work, we need to be able to
> perform sideband communications during suspend/resume, along with
> runtime PM suspend/resume. In order to do so, we also need to make sure
> that nouveau doesn't bother grabbing a runtime PM reference to do so,
> since otherwise we'll start deadlocking runtime PM again.
> 
> Note that we weren't able to do this before, because of the DP MST
> helpers processing UP requests from topologies in the same context as
> drm_dp_mst_hpd_irq() which would have caused us to open ourselves up to
> receiving hotplug events and deadlocking with runtime suspend/resume.
> Now that those requests are handled asynchronously, this change should
> be completely safe.
> 
> Cc: Juston Li 
> Cc: Imre Deak 
> Cc: Ville Syrjälä 
> Cc: Harry Wentland 
> Cc: Daniel Vetter 
> Signed-off-by: Lyude Paul 

Seems reasonable to me, but would feel better if a nouveau person confirmed

Reviewed-by: Sean Paul 


> ---
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 33 +++--
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 56871d34e3fb..f276918d3f3b 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -1131,6 +1131,16 @@ nouveau_connector_hotplug(struct nvif_notify *notify)
>   const char *name = connector->name;
>   struct nouveau_encoder *nv_encoder;
>   int ret;
> + bool plugged = (rep->mask != NVIF_NOTIFY_CONN_V0_UNPLUG);
> +
> + if (rep->mask & NVIF_NOTIFY_CONN_V0_IRQ) {
> + NV_DEBUG(drm, "service %s\n", name);
> + drm_dp_cec_irq(_connector->aux);
> + if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP)))
> + nv50_mstm_service(nv_encoder->dp.mstm);
> +
> + return NVIF_NOTIFY_KEEP;
> + }
>  
>   ret = pm_runtime_get(drm->dev->dev);
>   if (ret == 0) {
> @@ -1151,25 +1161,16 @@ nouveau_connector_hotplug(struct nvif_notify *notify)
>   return NVIF_NOTIFY_DROP;
>   }
>  
> - if (rep->mask & NVIF_NOTIFY_CONN_V0_IRQ) {
> - NV_DEBUG(drm, "service %s\n", name);
> - drm_dp_cec_irq(_connector->aux);
> - if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP)))
> - nv50_mstm_service(nv_encoder->dp.mstm);
> - } else {
> - bool plugged = (rep->mask != NVIF_NOTIFY_CONN_V0_UNPLUG);
> -
> + if (!plugged)
> + drm_dp_cec_unset_edid(_connector->aux);
> + NV_DEBUG(drm, "%splugged %s\n", plugged ? "" : "un", name);
> + if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP))) {
>   if (!plugged)
> - drm_dp_cec_unset_edid(_connector->aux);
> - NV_DEBUG(drm, "%splugged %s\n", plugged ? "" : "un", name);
> - if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP))) {
> - if (!plugged)
> - nv50_mstm_remove(nv_encoder->dp.mstm);
> - }
> -
> - drm_helper_hpd_irq_event(connector->dev);
> + nv50_mstm_remove(nv_encoder->dp.mstm);
>   }
>  
> + drm_helper_hpd_irq_event(connector->dev);
> +
>   pm_runtime_mark_last_busy(drm->dev->dev);
>   pm_runtime_put_autosuspend(drm->dev->dev);
>   return NVIF_NOTIFY_KEEP;
> -- 
> 2.21.0
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

[Nouveau] [PATCH v2 22/27] drm/nouveau: Don't grab runtime PM refs for HPD IRQs

2019-09-03 Thread Lyude Paul
In order for suspend/resume reprobing to work, we need to be able to
perform sideband communications during suspend/resume, along with
runtime PM suspend/resume. In order to do so, we also need to make sure
that nouveau doesn't bother grabbing a runtime PM reference to do so,
since otherwise we'll start deadlocking runtime PM again.

Note that we weren't able to do this before, because of the DP MST
helpers processing UP requests from topologies in the same context as
drm_dp_mst_hpd_irq() which would have caused us to open ourselves up to
receiving hotplug events and deadlocking with runtime suspend/resume.
Now that those requests are handled asynchronously, this change should
be completely safe.

Cc: Juston Li 
Cc: Imre Deak 
Cc: Ville Syrjälä 
Cc: Harry Wentland 
Cc: Daniel Vetter 
Signed-off-by: Lyude Paul 
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 33 +++--
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 56871d34e3fb..f276918d3f3b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1131,6 +1131,16 @@ nouveau_connector_hotplug(struct nvif_notify *notify)
const char *name = connector->name;
struct nouveau_encoder *nv_encoder;
int ret;
+   bool plugged = (rep->mask != NVIF_NOTIFY_CONN_V0_UNPLUG);
+
+   if (rep->mask & NVIF_NOTIFY_CONN_V0_IRQ) {
+   NV_DEBUG(drm, "service %s\n", name);
+   drm_dp_cec_irq(_connector->aux);
+   if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP)))
+   nv50_mstm_service(nv_encoder->dp.mstm);
+
+   return NVIF_NOTIFY_KEEP;
+   }
 
ret = pm_runtime_get(drm->dev->dev);
if (ret == 0) {
@@ -1151,25 +1161,16 @@ nouveau_connector_hotplug(struct nvif_notify *notify)
return NVIF_NOTIFY_DROP;
}
 
-   if (rep->mask & NVIF_NOTIFY_CONN_V0_IRQ) {
-   NV_DEBUG(drm, "service %s\n", name);
-   drm_dp_cec_irq(_connector->aux);
-   if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP)))
-   nv50_mstm_service(nv_encoder->dp.mstm);
-   } else {
-   bool plugged = (rep->mask != NVIF_NOTIFY_CONN_V0_UNPLUG);
-
+   if (!plugged)
+   drm_dp_cec_unset_edid(_connector->aux);
+   NV_DEBUG(drm, "%splugged %s\n", plugged ? "" : "un", name);
+   if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP))) {
if (!plugged)
-   drm_dp_cec_unset_edid(_connector->aux);
-   NV_DEBUG(drm, "%splugged %s\n", plugged ? "" : "un", name);
-   if ((nv_encoder = find_encoder(connector, DCB_OUTPUT_DP))) {
-   if (!plugged)
-   nv50_mstm_remove(nv_encoder->dp.mstm);
-   }
-
-   drm_helper_hpd_irq_event(connector->dev);
+   nv50_mstm_remove(nv_encoder->dp.mstm);
}
 
+   drm_helper_hpd_irq_event(connector->dev);
+
pm_runtime_mark_last_busy(drm->dev->dev);
pm_runtime_put_autosuspend(drm->dev->dev);
return NVIF_NOTIFY_KEEP;
-- 
2.21.0

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau