[PATCH 4/7] drm: Move the legacy kms disable_all helper to crtc helpers
Alex Deucher
alexdeucher at gmail.com
Tue Dec 11 15:55:30 UTC 2018
On Tue, Dec 11, 2018 at 10:53 AM Sean Paul <sean at 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 at 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 at linux.ibm.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > > Cc: Maxime Ripard <maxime.ripard at bootlin.com>
> > > Cc: Sean Paul <sean at poorly.run>
> > > Cc: David Airlie <airlied at linux.ie>
> > > Cc: Ben Skeggs <bskeggs at redhat.com>
> > > Cc: Alex Deucher <alexander.deucher at amd.com>
> > > Cc: "Christian König" <christian.koenig at amd.com>
> > > Cc: "David (ChunMing) Zhou" <David1.Zhou at amd.com>
> > > Cc: Rex Zhu <Rex.Zhu at amd.com>
> > > Cc: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
> > > Cc: Huang Rui <ray.huang at amd.com>
> > > Cc: Shaoyun Liu <Shaoyun.Liu at amd.com>
> > > Cc: Monk Liu <Monk.Liu at amd.com>
> > > Cc: nouveau at lists.freedesktop.org
> > > Cc: amd-gfx at 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 at poorly.run>
>
> Sean
>
> > With that fixed:
> > Reviewed-by: Alex Deucher <alexander.deucher at 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 at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
More information about the amd-gfx
mailing list