| Message ID | 20180712170256.13018-4-lyude@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 12, 2018 at 01:02:54PM -0400, Lyude Paul wrote: > This both uses the legacy modesetting structures in a racy manner, and > additionally also doesn't even check the right variable (enabled != the > CRTC is actually turned on for atomic). > > This fixes issues on my P50 regarding the dedicated GPU not entering > runtime suspend. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Cc: stable@vger.kernel.org On both patch 2&3: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> ->enable vs. ->active is probably the biggest source of pain in atomic, and beyond typing even more kerneldoc that will be ignored (there's another series doing exactly that on the list) I have no idea what to do. 90% rule is to look at ->enable in atomic_check code (since DPMS changes should always work) and ->active in atomic_commit code. Wrt the legacy state: For the legacy pointers we can set them to NULL for atomic, and Ville has done that. That's real effective at stopping drivers from looking at the wrong thing. But for the others like this one here I dunno what to do to effectively hide them from atomic drivers. </rant> Cheers, Daniel > --- > drivers/gpu/drm/nouveau/nouveau_drm.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c > index 0f668e275ee1..c7ec86d6c3c9 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > @@ -881,22 +881,11 @@ nouveau_pmops_runtime_resume(struct device *dev) > static int > nouveau_pmops_runtime_idle(struct device *dev) > { > - struct pci_dev *pdev = to_pci_dev(dev); > - struct drm_device *drm_dev = pci_get_drvdata(pdev); > - struct nouveau_drm *drm = nouveau_drm(drm_dev); > - struct drm_crtc *crtc; > - > if (!nouveau_pmops_runtime()) { > pm_runtime_forbid(dev); > return -EBUSY; > } > > - list_for_each_entry(crtc, &drm->dev->mode_config.crtc_list, head) { > - if (crtc->enabled) { > - DRM_DEBUG_DRIVER("failing to power off - crtc active\n"); > - return -EBUSY; > - } > - } > pm_runtime_mark_last_busy(dev); > pm_runtime_autosuspend(dev); > /* we don't want the main rpm_idle to call suspend - we want to autosuspend */ > -- > 2.17.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 0f668e275ee1..c7ec86d6c3c9 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -881,22 +881,11 @@ nouveau_pmops_runtime_resume(struct device *dev) static int nouveau_pmops_runtime_idle(struct device *dev) { - struct pci_dev *pdev = to_pci_dev(dev); - struct drm_device *drm_dev = pci_get_drvdata(pdev); - struct nouveau_drm *drm = nouveau_drm(drm_dev); - struct drm_crtc *crtc; - if (!nouveau_pmops_runtime()) { pm_runtime_forbid(dev); return -EBUSY; } - list_for_each_entry(crtc, &drm->dev->mode_config.crtc_list, head) { - if (crtc->enabled) { - DRM_DEBUG_DRIVER("failing to power off - crtc active\n"); - return -EBUSY; - } - } pm_runtime_mark_last_busy(dev); pm_runtime_autosuspend(dev); /* we don't want the main rpm_idle to call suspend - we want to autosuspend */
This both uses the legacy modesetting structures in a racy manner, and additionally also doesn't even check the right variable (enabled != the CRTC is actually turned on for atomic). This fixes issues on my P50 regarding the dedicated GPU not entering runtime suspend. Signed-off-by: Lyude Paul <lyude@redhat.com> Cc: stable@vger.kernel.org --- drivers/gpu/drm/nouveau/nouveau_drm.c | 11 ----------- 1 file changed, 11 deletions(-)