[4/7] drm: Move the legacy kms disable_all helper to crtc helpers

Submitted by Daniel Vetter on Dec. 10, 2018, 10:03 a.m.

Details

Message ID 20181210100359.22507-5-daniel.vetter@ffwll.ch
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Nouveau

Not browsing as part of any series.

Commit Message

Daniel Vetter Dec. 10, 2018, 10:03 a.m.
It's not a core function, and the matching atomic functions are also
not in the core. Plus the suspend/resume helper is also already there.

Needs a tiny bit of open-coding, but less midlayer beats that I think.

Cc: Sam Bobroff <sbobroff@linux.ibm.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: Rex Zhu <Rex.Zhu@amd.com>
Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
Cc: Huang Rui <ray.huang@amd.com>
Cc: Shaoyun Liu <Shaoyun.Liu@amd.com>
Cc: Monk Liu <Monk.Liu@amd.com>
Cc: nouveau@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
 drivers/gpu/drm/drm_crtc.c                 | 31 -------------------
 drivers/gpu/drm/drm_crtc_helper.c          | 35 ++++++++++++++++++++++
 drivers/gpu/drm/nouveau/nouveau_display.c  |  2 +-
 drivers/gpu/drm/radeon/radeon_display.c    |  2 +-
 include/drm/drm_crtc.h                     |  2 --
 include/drm/drm_crtc_helper.h              |  1 +
 7 files changed, 39 insertions(+), 36 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index c75badfa5c4c..e669297ffefb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2689,7 +2689,7 @@  void amdgpu_device_fini(struct amdgpu_device *adev)
 	amdgpu_irq_disable_all(adev);
 	if (adev->mode_info.mode_config_initialized){
 		if (!amdgpu_device_has_dc_support(adev))
-			drm_crtc_force_disable_all(adev->ddev);
+			drm_helper_force_disable_all(adev->ddev);
 		else
 			drm_atomic_helper_shutdown(adev->ddev);
 	}
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f660819d406e..7dabbaf033a1 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -104,37 +104,6 @@  int drm_crtc_force_disable(struct drm_crtc *crtc)
 	return drm_mode_set_config_internal(&set);
 }
 
-/**
- * drm_crtc_force_disable_all - Forcibly turn off all enabled CRTCs
- * @dev: DRM device whose CRTCs to turn off
- *
- * Drivers may want to call this on unload to ensure that all displays are
- * unlit and the GPU is in a consistent, low power state. Takes modeset locks.
- *
- * Note: This should only be used by non-atomic legacy drivers. For an atomic
- * version look at drm_atomic_helper_shutdown().
- *
- * Returns:
- * Zero on success, error code on failure.
- */
-int drm_crtc_force_disable_all(struct drm_device *dev)
-{
-	struct drm_crtc *crtc;
-	int ret = 0;
-
-	drm_modeset_lock_all(dev);
-	drm_for_each_crtc(crtc, dev)
-		if (crtc->enabled) {
-			ret = drm_crtc_force_disable(crtc);
-			if (ret)
-				goto out;
-		}
-out:
-	drm_modeset_unlock_all(dev);
-	return ret;
-}
-EXPORT_SYMBOL(drm_crtc_force_disable_all);
-
 static unsigned int drm_num_crtcs(struct drm_device *dev)
 {
 	unsigned int num = 0;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index a3c81850e755..23159eb494f1 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -984,3 +984,38 @@  void drm_helper_resume_force_mode(struct drm_device *dev)
 	drm_modeset_unlock_all(dev);
 }
 EXPORT_SYMBOL(drm_helper_resume_force_mode);
+
+/**
+ * drm_helper_force_disable_all - Forcibly turn off all enabled CRTCs
+ * @dev: DRM device whose CRTCs to turn off
+ *
+ * Drivers may want to call this on unload to ensure that all displays are
+ * unlit and the GPU is in a consistent, low power state. Takes modeset locks.
+ *
+ * Note: This should only be used by non-atomic legacy drivers. For an atomic
+ * version look at drm_atomic_helper_shutdown().
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_helper_force_disable_all(struct drm_device *dev)
+{
+	struct drm_crtc *crtc;
+	int ret = 0;
+
+	drm_modeset_lock_all(dev);
+	drm_for_each_crtc(crtc, dev)
+		if (crtc->enabled) {
+			struct drm_mode_set set = {
+				.crtc = crtc,
+			};
+
+			ret = drm_mode_set_config_internal(&set);
+			if (ret)
+				goto out;
+		}
+out:
+	drm_modeset_unlock_all(dev);
+	return ret;
+}
+EXPORT_SYMBOL(drm_helper_force_disable_all);
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index f326ffd86766..5d273a655479 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -453,7 +453,7 @@  nouveau_display_fini(struct drm_device *dev, bool suspend, bool runtime)
 		if (drm_drv_uses_atomic_modeset(dev))
 			drm_atomic_helper_shutdown(dev);
 		else
-			drm_crtc_force_disable_all(dev);
+			drm_helper_force_disable_all(dev);
 	}
 
 	/* disable flip completion events */
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index e6912eb99b42..92332226e5cf 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -1643,7 +1643,7 @@  void radeon_modeset_fini(struct radeon_device *rdev)
 	if (rdev->mode_info.mode_config_initialized) {
 		drm_kms_helper_poll_fini(rdev->ddev);
 		radeon_hpd_fini(rdev);
-		drm_crtc_force_disable_all(rdev->ddev);
+		drm_helper_force_disable_all(rdev->ddev);
 		radeon_fbdev_fini(rdev);
 		radeon_afmt_fini(rdev);
 		drm_mode_config_cleanup(rdev->ddev);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index b45bec0b7a9c..85abd3fe9e83 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1149,8 +1149,6 @@  static inline uint32_t drm_crtc_mask(const struct drm_crtc *crtc)
 	return 1 << drm_crtc_index(crtc);
 }
 
-int drm_crtc_force_disable_all(struct drm_device *dev);
-
 int drm_mode_set_config_internal(struct drm_mode_set *set);
 struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx);
 
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index d65f034843ce..0ee9a96b70da 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -56,6 +56,7 @@  bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
 int drm_helper_connector_dpms(struct drm_connector *connector, int mode);
 
 void drm_helper_resume_force_mode(struct drm_device *dev);
+int drm_helper_force_disable_all(struct drm_device *dev);
 
 /* drm_probe_helper.c */
 int drm_helper_probe_single_connector_modes(struct drm_connector

Comments

On Mon, Dec 10, 2018 at 5:04 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> It's not a core function, and the matching atomic functions are also
> not in the core. Plus the suspend/resume helper is also already there.
>
> Needs a tiny bit of open-coding, but less midlayer beats that I think.
>
> Cc: Sam Bobroff <sbobroff@linux.ibm.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: Rex Zhu <Rex.Zhu@amd.com>
> Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> Cc: Huang Rui <ray.huang@amd.com>
> Cc: Shaoyun Liu <Shaoyun.Liu@amd.com>
> Cc: Monk Liu <Monk.Liu@amd.com>
> Cc: nouveau@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
>  drivers/gpu/drm/drm_crtc.c                 | 31 -------------------
>  drivers/gpu/drm/drm_crtc_helper.c          | 35 ++++++++++++++++++++++
>  drivers/gpu/drm/nouveau/nouveau_display.c  |  2 +-
>  drivers/gpu/drm/radeon/radeon_display.c    |  2 +-
>  include/drm/drm_crtc.h                     |  2 --
>  include/drm/drm_crtc_helper.h              |  1 +
>  7 files changed, 39 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index c75badfa5c4c..e669297ffefb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2689,7 +2689,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
>         amdgpu_irq_disable_all(adev);
>         if (adev->mode_info.mode_config_initialized){
>                 if (!amdgpu_device_has_dc_support(adev))
> -                       drm_crtc_force_disable_all(adev->ddev);
> +                       drm_helper_force_disable_all(adev->ddev);
>                 else
>                         drm_atomic_helper_shutdown(adev->ddev);
>         }
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f660819d406e..7dabbaf033a1 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -104,37 +104,6 @@ int drm_crtc_force_disable(struct drm_crtc *crtc)
>         return drm_mode_set_config_internal(&set);
>  }
>
> -/**
> - * drm_crtc_force_disable_all - Forcibly turn off all enabled CRTCs
> - * @dev: DRM device whose CRTCs to turn off
> - *
> - * Drivers may want to call this on unload to ensure that all displays are
> - * unlit and the GPU is in a consistent, low power state. Takes modeset locks.
> - *
> - * Note: This should only be used by non-atomic legacy drivers. For an atomic
> - * version look at drm_atomic_helper_shutdown().
> - *
> - * Returns:
> - * Zero on success, error code on failure.
> - */
> -int drm_crtc_force_disable_all(struct drm_device *dev)
> -{
> -       struct drm_crtc *crtc;
> -       int ret = 0;
> -
> -       drm_modeset_lock_all(dev);
> -       drm_for_each_crtc(crtc, dev)
> -               if (crtc->enabled) {
> -                       ret = drm_crtc_force_disable(crtc);
> -                       if (ret)
> -                               goto out;
> -               }
> -out:
> -       drm_modeset_unlock_all(dev);
> -       return ret;
> -}
> -EXPORT_SYMBOL(drm_crtc_force_disable_all);
> -
>  static unsigned int drm_num_crtcs(struct drm_device *dev)
>  {
>         unsigned int num = 0;
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index a3c81850e755..23159eb494f1 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -984,3 +984,38 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
>         drm_modeset_unlock_all(dev);
>  }
>  EXPORT_SYMBOL(drm_helper_resume_force_mode);
> +
> +/**
> + * drm_helper_force_disable_all - Forcibly turn off all enabled CRTCs
> + * @dev: DRM device whose CRTCs to turn off
> + *
> + * Drivers may want to call this on unload to ensure that all displays are
> + * unlit and the GPU is in a consistent, low power state. Takes modeset locks.
> + *
> + * Note: This should only be used by non-atomic legacy drivers. For an atomic
> + * version look at drm_atomic_helper_shutdown().
> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drm_helper_force_disable_all(struct drm_device *dev)

Maybe put crtc somewhere in the function name so it's clear what we
are disabling.  With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> +{
> +       struct drm_crtc *crtc;
> +       int ret = 0;
> +
> +       drm_modeset_lock_all(dev);
> +       drm_for_each_crtc(crtc, dev)
> +               if (crtc->enabled) {
> +                       struct drm_mode_set set = {
> +                               .crtc = crtc,
> +                       };
> +
> +                       ret = drm_mode_set_config_internal(&set);
> +                       if (ret)
> +                               goto out;
> +               }
> +out:
> +       drm_modeset_unlock_all(dev);
> +       return ret;
> +}
> +EXPORT_SYMBOL(drm_helper_force_disable_all);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index f326ffd86766..5d273a655479 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -453,7 +453,7 @@ nouveau_display_fini(struct drm_device *dev, bool suspend, bool runtime)
>                 if (drm_drv_uses_atomic_modeset(dev))
>                         drm_atomic_helper_shutdown(dev);
>                 else
> -                       drm_crtc_force_disable_all(dev);
> +                       drm_helper_force_disable_all(dev);
>         }
>
>         /* disable flip completion events */
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> index e6912eb99b42..92332226e5cf 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -1643,7 +1643,7 @@ void radeon_modeset_fini(struct radeon_device *rdev)
>         if (rdev->mode_info.mode_config_initialized) {
>                 drm_kms_helper_poll_fini(rdev->ddev);
>                 radeon_hpd_fini(rdev);
> -               drm_crtc_force_disable_all(rdev->ddev);
> +               drm_helper_force_disable_all(rdev->ddev);
>                 radeon_fbdev_fini(rdev);
>                 radeon_afmt_fini(rdev);
>                 drm_mode_config_cleanup(rdev->ddev);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index b45bec0b7a9c..85abd3fe9e83 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1149,8 +1149,6 @@ static inline uint32_t drm_crtc_mask(const struct drm_crtc *crtc)
>         return 1 << drm_crtc_index(crtc);
>  }
>
> -int drm_crtc_force_disable_all(struct drm_device *dev);
> -
>  int drm_mode_set_config_internal(struct drm_mode_set *set);
>  struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx);
>
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index d65f034843ce..0ee9a96b70da 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -56,6 +56,7 @@ bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
>  int drm_helper_connector_dpms(struct drm_connector *connector, int mode);
>
>  void drm_helper_resume_force_mode(struct drm_device *dev);
> +int drm_helper_force_disable_all(struct drm_device *dev);
>
>  /* drm_probe_helper.c */
>  int drm_helper_probe_single_connector_modes(struct drm_connector
> --
> 2.20.0.rc1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Mon, Dec 10, 2018 at 10:58:20AM -0500, Alex Deucher wrote:
> On Mon, Dec 10, 2018 at 5:04 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > It's not a core function, and the matching atomic functions are also
> > not in the core. Plus the suspend/resume helper is also already there.
> >
> > Needs a tiny bit of open-coding, but less midlayer beats that I think.
> >
> > Cc: Sam Bobroff <sbobroff@linux.ibm.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > Cc: Rex Zhu <Rex.Zhu@amd.com>
> > Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> > Cc: Huang Rui <ray.huang@amd.com>
> > Cc: Shaoyun Liu <Shaoyun.Liu@amd.com>
> > Cc: Monk Liu <Monk.Liu@amd.com>
> > Cc: nouveau@lists.freedesktop.org
> > Cc: amd-gfx@lists.freedesktop.org
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
> >  drivers/gpu/drm/drm_crtc.c                 | 31 -------------------
> >  drivers/gpu/drm/drm_crtc_helper.c          | 35 ++++++++++++++++++++++
> >  drivers/gpu/drm/nouveau/nouveau_display.c  |  2 +-
> >  drivers/gpu/drm/radeon/radeon_display.c    |  2 +-
> >  include/drm/drm_crtc.h                     |  2 --
> >  include/drm/drm_crtc_helper.h              |  1 +
> >  7 files changed, 39 insertions(+), 36 deletions(-)
> >

/snip

> > diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> > index a3c81850e755..23159eb494f1 100644
> > --- a/drivers/gpu/drm/drm_crtc_helper.c
> > +++ b/drivers/gpu/drm/drm_crtc_helper.c
> > @@ -984,3 +984,38 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
> >         drm_modeset_unlock_all(dev);
> >  }
> >  EXPORT_SYMBOL(drm_helper_resume_force_mode);
> > +
> > +/**
> > + * drm_helper_force_disable_all - Forcibly turn off all enabled CRTCs
> > + * @dev: DRM device whose CRTCs to turn off
> > + *
> > + * Drivers may want to call this on unload to ensure that all displays are
> > + * unlit and the GPU is in a consistent, low power state. Takes modeset locks.
> > + *
> > + * Note: This should only be used by non-atomic legacy drivers. For an atomic
> > + * version look at drm_atomic_helper_shutdown().
> > + *
> > + * Returns:
> > + * Zero on success, error code on failure.
> > + */
> > +int drm_helper_force_disable_all(struct drm_device *dev)
> 
> Maybe put crtc somewhere in the function name so it's clear what we
> are disabling.

FWIW, I think it's more clear this way. set_config_internal will turn off
everything attached to the crtc, so _everything_ will be disabled in this case.

Either way,

Reviewed-by: Sean Paul <sean@poorly.run>

Sean

> With that fixed:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> 
> > +{
> > +       struct drm_crtc *crtc;
> > +       int ret = 0;
> > +
> > +       drm_modeset_lock_all(dev);
> > +       drm_for_each_crtc(crtc, dev)
> > +               if (crtc->enabled) {
> > +                       struct drm_mode_set set = {
> > +                               .crtc = crtc,
> > +                       };
> > +
> > +                       ret = drm_mode_set_config_internal(&set);
> > +                       if (ret)
> > +                               goto out;
> > +               }
> > +out:
> > +       drm_modeset_unlock_all(dev);
> > +       return ret;
> > +}
> > +EXPORT_SYMBOL(drm_helper_force_disable_all);
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> > index f326ffd86766..5d273a655479 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> > @@ -453,7 +453,7 @@ nouveau_display_fini(struct drm_device *dev, bool suspend, bool runtime)
> >                 if (drm_drv_uses_atomic_modeset(dev))
> >                         drm_atomic_helper_shutdown(dev);
> >                 else
> > -                       drm_crtc_force_disable_all(dev);
> > +                       drm_helper_force_disable_all(dev);
> >         }
> >
> >         /* disable flip completion events */
> > diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> > index e6912eb99b42..92332226e5cf 100644
> > --- a/drivers/gpu/drm/radeon/radeon_display.c
> > +++ b/drivers/gpu/drm/radeon/radeon_display.c
> > @@ -1643,7 +1643,7 @@ void radeon_modeset_fini(struct radeon_device *rdev)
> >         if (rdev->mode_info.mode_config_initialized) {
> >                 drm_kms_helper_poll_fini(rdev->ddev);
> >                 radeon_hpd_fini(rdev);
> > -               drm_crtc_force_disable_all(rdev->ddev);
> > +               drm_helper_force_disable_all(rdev->ddev);
> >                 radeon_fbdev_fini(rdev);
> >                 radeon_afmt_fini(rdev);
> >                 drm_mode_config_cleanup(rdev->ddev);
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index b45bec0b7a9c..85abd3fe9e83 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -1149,8 +1149,6 @@ static inline uint32_t drm_crtc_mask(const struct drm_crtc *crtc)
> >         return 1 << drm_crtc_index(crtc);
> >  }
> >
> > -int drm_crtc_force_disable_all(struct drm_device *dev);
> > -
> >  int drm_mode_set_config_internal(struct drm_mode_set *set);
> >  struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx);
> >
> > diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> > index d65f034843ce..0ee9a96b70da 100644
> > --- a/include/drm/drm_crtc_helper.h
> > +++ b/include/drm/drm_crtc_helper.h
> > @@ -56,6 +56,7 @@ bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
> >  int drm_helper_connector_dpms(struct drm_connector *connector, int mode);
> >
> >  void drm_helper_resume_force_mode(struct drm_device *dev);
> > +int drm_helper_force_disable_all(struct drm_device *dev);
> >
> >  /* drm_probe_helper.c */
> >  int drm_helper_probe_single_connector_modes(struct drm_connector
> > --
> > 2.20.0.rc1
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Tue, Dec 11, 2018 at 10:53 AM Sean Paul <sean@poorly.run> wrote:
>
> On Mon, Dec 10, 2018 at 10:58:20AM -0500, Alex Deucher wrote:
> > On Mon, Dec 10, 2018 at 5:04 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > It's not a core function, and the matching atomic functions are also
> > > not in the core. Plus the suspend/resume helper is also already there.
> > >
> > > Needs a tiny bit of open-coding, but less midlayer beats that I think.
> > >
> > > Cc: Sam Bobroff <sbobroff@linux.ibm.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> > > Cc: Sean Paul <sean@poorly.run>
> > > Cc: David Airlie <airlied@linux.ie>
> > > Cc: Ben Skeggs <bskeggs@redhat.com>
> > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > Cc: "Christian König" <christian.koenig@amd.com>
> > > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > > Cc: Rex Zhu <Rex.Zhu@amd.com>
> > > Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> > > Cc: Huang Rui <ray.huang@amd.com>
> > > Cc: Shaoyun Liu <Shaoyun.Liu@amd.com>
> > > Cc: Monk Liu <Monk.Liu@amd.com>
> > > Cc: nouveau@lists.freedesktop.org
> > > Cc: amd-gfx@lists.freedesktop.org
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
> > >  drivers/gpu/drm/drm_crtc.c                 | 31 -------------------
> > >  drivers/gpu/drm/drm_crtc_helper.c          | 35 ++++++++++++++++++++++
> > >  drivers/gpu/drm/nouveau/nouveau_display.c  |  2 +-
> > >  drivers/gpu/drm/radeon/radeon_display.c    |  2 +-
> > >  include/drm/drm_crtc.h                     |  2 --
> > >  include/drm/drm_crtc_helper.h              |  1 +
> > >  7 files changed, 39 insertions(+), 36 deletions(-)
> > >
>
> /snip
>
> > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> > > index a3c81850e755..23159eb494f1 100644
> > > --- a/drivers/gpu/drm/drm_crtc_helper.c
> > > +++ b/drivers/gpu/drm/drm_crtc_helper.c
> > > @@ -984,3 +984,38 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
> > >         drm_modeset_unlock_all(dev);
> > >  }
> > >  EXPORT_SYMBOL(drm_helper_resume_force_mode);
> > > +
> > > +/**
> > > + * drm_helper_force_disable_all - Forcibly turn off all enabled CRTCs
> > > + * @dev: DRM device whose CRTCs to turn off
> > > + *
> > > + * Drivers may want to call this on unload to ensure that all displays are
> > > + * unlit and the GPU is in a consistent, low power state. Takes modeset locks.
> > > + *
> > > + * Note: This should only be used by non-atomic legacy drivers. For an atomic
> > > + * version look at drm_atomic_helper_shutdown().
> > > + *
> > > + * Returns:
> > > + * Zero on success, error code on failure.
> > > + */
> > > +int drm_helper_force_disable_all(struct drm_device *dev)
> >
> > Maybe put crtc somewhere in the function name so it's clear what we
> > are disabling.
>
> FWIW, I think it's more clear this way. set_config_internal will turn off
> everything attached to the crtc, so _everything_ will be disabled in this case.

I'm not pressed.  RB either way for me as well.

Alex

>
> Either way,
>
> Reviewed-by: Sean Paul <sean@poorly.run>
>
> Sean
>
> > With that fixed:
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> >
> > > +{
> > > +       struct drm_crtc *crtc;
> > > +       int ret = 0;
> > > +
> > > +       drm_modeset_lock_all(dev);
> > > +       drm_for_each_crtc(crtc, dev)
> > > +               if (crtc->enabled) {
> > > +                       struct drm_mode_set set = {
> > > +                               .crtc = crtc,
> > > +                       };
> > > +
> > > +                       ret = drm_mode_set_config_internal(&set);
> > > +                       if (ret)
> > > +                               goto out;
> > > +               }
> > > +out:
> > > +       drm_modeset_unlock_all(dev);
> > > +       return ret;
> > > +}
> > > +EXPORT_SYMBOL(drm_helper_force_disable_all);
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> > > index f326ffd86766..5d273a655479 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> > > @@ -453,7 +453,7 @@ nouveau_display_fini(struct drm_device *dev, bool suspend, bool runtime)
> > >                 if (drm_drv_uses_atomic_modeset(dev))
> > >                         drm_atomic_helper_shutdown(dev);
> > >                 else
> > > -                       drm_crtc_force_disable_all(dev);
> > > +                       drm_helper_force_disable_all(dev);
> > >         }
> > >
> > >         /* disable flip completion events */
> > > diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> > > index e6912eb99b42..92332226e5cf 100644
> > > --- a/drivers/gpu/drm/radeon/radeon_display.c
> > > +++ b/drivers/gpu/drm/radeon/radeon_display.c
> > > @@ -1643,7 +1643,7 @@ void radeon_modeset_fini(struct radeon_device *rdev)
> > >         if (rdev->mode_info.mode_config_initialized) {
> > >                 drm_kms_helper_poll_fini(rdev->ddev);
> > >                 radeon_hpd_fini(rdev);
> > > -               drm_crtc_force_disable_all(rdev->ddev);
> > > +               drm_helper_force_disable_all(rdev->ddev);
> > >                 radeon_fbdev_fini(rdev);
> > >                 radeon_afmt_fini(rdev);
> > >                 drm_mode_config_cleanup(rdev->ddev);
> > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > > index b45bec0b7a9c..85abd3fe9e83 100644
> > > --- a/include/drm/drm_crtc.h
> > > +++ b/include/drm/drm_crtc.h
> > > @@ -1149,8 +1149,6 @@ static inline uint32_t drm_crtc_mask(const struct drm_crtc *crtc)
> > >         return 1 << drm_crtc_index(crtc);
> > >  }
> > >
> > > -int drm_crtc_force_disable_all(struct drm_device *dev);
> > > -
> > >  int drm_mode_set_config_internal(struct drm_mode_set *set);
> > >  struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx);
> > >
> > > diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> > > index d65f034843ce..0ee9a96b70da 100644
> > > --- a/include/drm/drm_crtc_helper.h
> > > +++ b/include/drm/drm_crtc_helper.h
> > > @@ -56,6 +56,7 @@ bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
> > >  int drm_helper_connector_dpms(struct drm_connector *connector, int mode);
> > >
> > >  void drm_helper_resume_force_mode(struct drm_device *dev);
> > > +int drm_helper_force_disable_all(struct drm_device *dev);
> > >
> > >  /* drm_probe_helper.c */
> > >  int drm_helper_probe_single_connector_modes(struct drm_connector
> > > --
> > > 2.20.0.rc1
> > >
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS