* [PATCH v2 0/2] Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_*
@ 2021-04-19 15:03 ` Fabio M. De Francesco
0 siblings, 0 replies; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-04-19 15:03 UTC (permalink / raw)
To: outreachy-kernel, dri-devel, Alex Deucher, Christian König,
David Airlie, Daniel Vetter, Melissa Wen
Cc: Fabio M. De Francesco
Replace the deprecated API with new helpers, according to the TODO list
of the DRM subsystem. The new API has been introduced with commit
b7ea04d299c7: DRM_MODESET_LOCK_ALL_BEGIN() simplifies grabbing all modeset
locks using a local context. This has the advantage of reducing boilerplate.
Changes from v1: Added further information to the commit message.
Fabio M. De Francesco (2):
drm/amd/amdgpu/amdgpu_device.c: Replace drm_modeset_*_all with
DRM_MODESET_LOCK_ALL_*
drm/amd/amdgpu/amdgpu_drv.c: Replace drm_modeset_*_all with
DRM_MODESET_LOCK_ALL_*
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++------
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ++++--
2 files changed, 16 insertions(+), 8 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 0/2] Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* @ 2021-04-19 15:03 ` Fabio M. De Francesco 0 siblings, 0 replies; 10+ messages in thread From: Fabio M. De Francesco @ 2021-04-19 15:03 UTC (permalink / raw) To: outreachy-kernel, dri-devel, Alex Deucher, Christian König, David Airlie, Daniel Vetter, Melissa Wen Cc: Fabio M. De Francesco Replace the deprecated API with new helpers, according to the TODO list of the DRM subsystem. The new API has been introduced with commit b7ea04d299c7: DRM_MODESET_LOCK_ALL_BEGIN() simplifies grabbing all modeset locks using a local context. This has the advantage of reducing boilerplate. Changes from v1: Added further information to the commit message. Fabio M. De Francesco (2): drm/amd/amdgpu/amdgpu_device.c: Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* drm/amd/amdgpu/amdgpu_drv.c: Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++------ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ++++-- 2 files changed, 16 insertions(+), 8 deletions(-) -- 2.31.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] drm/amd/amdgpu/amdgpu_device.c: Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* 2021-04-19 15:03 ` Fabio M. De Francesco @ 2021-04-19 15:03 ` Fabio M. De Francesco -1 siblings, 0 replies; 10+ messages in thread From: Fabio M. De Francesco @ 2021-04-19 15:03 UTC (permalink / raw) To: outreachy-kernel, dri-devel, Alex Deucher, Christian König, David Airlie, Daniel Vetter, Melissa Wen Cc: Fabio M. De Francesco Replace the deprecated API with new helpers, according to the TODO list of the DRM subsystem. The new API has been introduced with commit b7ea04d299c7: DRM_MODESET_LOCK_ALL_BEGIN() simplifies grabbing all modeset locks using a local context and has the advantage of reducing boilerplate. Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- Changes from v1: Added further information in the commit message. drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 6447cd6ca5a8..e1a71579f8e6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -32,6 +32,7 @@ #include <linux/slab.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_drv.h> #include <drm/drm_probe_helper.h> #include <drm/amdgpu_drm.h> #include <linux/vgaarb.h> @@ -3694,14 +3695,17 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon) if (!amdgpu_device_has_dc_support(adev)) { /* turn off display hw */ - drm_modeset_lock_all(dev); + struct drm_modeset_acquire_ctx ctx; + int ret; + + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); drm_connector_list_iter_begin(dev, &iter); drm_for_each_connector_iter(connector, &iter) drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF); drm_connector_list_iter_end(&iter); - drm_modeset_unlock_all(dev); - /* unpin the front buffers and cursors */ + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); + /* unpin the front buffers and cursors */ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc); struct drm_framebuffer *fb = crtc->primary->fb; @@ -3830,19 +3834,21 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon) /* blat the mode back in */ if (fbcon) { if (!amdgpu_device_has_dc_support(adev)) { + struct drm_modeset_acquire_ctx ctx; + int ret; + /* pre DCE11 */ drm_helper_resume_force_mode(dev); /* turn on display hw */ - drm_modeset_lock_all(dev); + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); drm_connector_list_iter_begin(dev, &iter); drm_for_each_connector_iter(connector, &iter) drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON); drm_connector_list_iter_end(&iter); - - drm_modeset_unlock_all(dev); + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); } amdgpu_fbdev_set_suspend(adev, 0); } -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] drm/amd/amdgpu/amdgpu_device.c: Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* @ 2021-04-19 15:03 ` Fabio M. De Francesco 0 siblings, 0 replies; 10+ messages in thread From: Fabio M. De Francesco @ 2021-04-19 15:03 UTC (permalink / raw) To: outreachy-kernel, dri-devel, Alex Deucher, Christian König, David Airlie, Daniel Vetter, Melissa Wen Cc: Fabio M. De Francesco Replace the deprecated API with new helpers, according to the TODO list of the DRM subsystem. The new API has been introduced with commit b7ea04d299c7: DRM_MODESET_LOCK_ALL_BEGIN() simplifies grabbing all modeset locks using a local context and has the advantage of reducing boilerplate. Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- Changes from v1: Added further information in the commit message. drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 6447cd6ca5a8..e1a71579f8e6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -32,6 +32,7 @@ #include <linux/slab.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_drv.h> #include <drm/drm_probe_helper.h> #include <drm/amdgpu_drm.h> #include <linux/vgaarb.h> @@ -3694,14 +3695,17 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon) if (!amdgpu_device_has_dc_support(adev)) { /* turn off display hw */ - drm_modeset_lock_all(dev); + struct drm_modeset_acquire_ctx ctx; + int ret; + + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); drm_connector_list_iter_begin(dev, &iter); drm_for_each_connector_iter(connector, &iter) drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF); drm_connector_list_iter_end(&iter); - drm_modeset_unlock_all(dev); - /* unpin the front buffers and cursors */ + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); + /* unpin the front buffers and cursors */ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc); struct drm_framebuffer *fb = crtc->primary->fb; @@ -3830,19 +3834,21 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon) /* blat the mode back in */ if (fbcon) { if (!amdgpu_device_has_dc_support(adev)) { + struct drm_modeset_acquire_ctx ctx; + int ret; + /* pre DCE11 */ drm_helper_resume_force_mode(dev); /* turn on display hw */ - drm_modeset_lock_all(dev); + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); drm_connector_list_iter_begin(dev, &iter); drm_for_each_connector_iter(connector, &iter) drm_helper_connector_dpms(connector, DRM_MODE_DPMS_ON); drm_connector_list_iter_end(&iter); - - drm_modeset_unlock_all(dev); + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); } amdgpu_fbdev_set_suspend(adev, 0); } -- 2.31.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm/amd/amdgpu/amdgpu_device.c: Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* 2021-04-19 15:03 ` Fabio M. De Francesco @ 2021-04-20 17:49 ` Daniel Vetter -1 siblings, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2021-04-20 17:49 UTC (permalink / raw) To: Fabio M. De Francesco Cc: outreachy-kernel, dri-devel, Alex Deucher, Christian König, David Airlie, Daniel Vetter, Melissa Wen On Mon, Apr 19, 2021 at 05:03:40PM +0200, Fabio M. De Francesco wrote: > Replace the deprecated API with new helpers, according to the TODO list > of the DRM subsystem. The new API has been introduced with commit > b7ea04d299c7: DRM_MODESET_LOCK_ALL_BEGIN() simplifies grabbing all modeset > locks using a local context and has the advantage of reducing boilerplate. So this is only half of the story, because the boilerplate only exists when you're using drm_modeset_lock_all_ctx. Which should be used for modern atomic display drivers everywhere. But drm_modeset_lock_all_ctx isn't exactly the same as drm_modeset_lock_all, so this needs to be explained. Now the problem with drm_modeset_lock_all is that it hides a memory allocation, and if that allocation fails then we just die. Which isn't great really, but in practice the kernel tries really hard to never fail this allocation. That's why we move the drm_modeset_acquire_ctx onto the stack. I think for understanding what's going on here you'd first need to convert the code to the full open-code boilerplate using drm_modeset_lock_all_ctx, with explanations of why the changes are ok. Then replace it with the convenient macro. Once it's clear what's going on under the hood it should then be easier to explain the situation in subsequent conversions with just one patch. -Daniel > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > --- > > Changes from v1: Added further information in the commit message. > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 6447cd6ca5a8..e1a71579f8e6 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -32,6 +32,7 @@ > #include <linux/slab.h> > > #include <drm/drm_atomic_helper.h> > +#include <drm/drm_drv.h> > #include <drm/drm_probe_helper.h> > #include <drm/amdgpu_drm.h> > #include <linux/vgaarb.h> > @@ -3694,14 +3695,17 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon) > > if (!amdgpu_device_has_dc_support(adev)) { > /* turn off display hw */ > - drm_modeset_lock_all(dev); > + struct drm_modeset_acquire_ctx ctx; > + int ret; > + > + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); > drm_connector_list_iter_begin(dev, &iter); > drm_for_each_connector_iter(connector, &iter) > drm_helper_connector_dpms(connector, > DRM_MODE_DPMS_OFF); > drm_connector_list_iter_end(&iter); > - drm_modeset_unlock_all(dev); > - /* unpin the front buffers and cursors */ > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > + /* unpin the front buffers and cursors */ > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc); > struct drm_framebuffer *fb = crtc->primary->fb; > @@ -3830,19 +3834,21 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon) > /* blat the mode back in */ > if (fbcon) { > if (!amdgpu_device_has_dc_support(adev)) { > + struct drm_modeset_acquire_ctx ctx; > + int ret; > + > /* pre DCE11 */ > drm_helper_resume_force_mode(dev); > > /* turn on display hw */ > - drm_modeset_lock_all(dev); > + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); > > drm_connector_list_iter_begin(dev, &iter); > drm_for_each_connector_iter(connector, &iter) > drm_helper_connector_dpms(connector, > DRM_MODE_DPMS_ON); > drm_connector_list_iter_end(&iter); > - > - drm_modeset_unlock_all(dev); > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > } > amdgpu_fbdev_set_suspend(adev, 0); > } > -- > 2.31.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm/amd/amdgpu/amdgpu_device.c: Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* @ 2021-04-20 17:49 ` Daniel Vetter 0 siblings, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2021-04-20 17:49 UTC (permalink / raw) To: Fabio M. De Francesco Cc: David Airlie, dri-devel, Melissa Wen, outreachy-kernel, Alex Deucher, Christian König On Mon, Apr 19, 2021 at 05:03:40PM +0200, Fabio M. De Francesco wrote: > Replace the deprecated API with new helpers, according to the TODO list > of the DRM subsystem. The new API has been introduced with commit > b7ea04d299c7: DRM_MODESET_LOCK_ALL_BEGIN() simplifies grabbing all modeset > locks using a local context and has the advantage of reducing boilerplate. So this is only half of the story, because the boilerplate only exists when you're using drm_modeset_lock_all_ctx. Which should be used for modern atomic display drivers everywhere. But drm_modeset_lock_all_ctx isn't exactly the same as drm_modeset_lock_all, so this needs to be explained. Now the problem with drm_modeset_lock_all is that it hides a memory allocation, and if that allocation fails then we just die. Which isn't great really, but in practice the kernel tries really hard to never fail this allocation. That's why we move the drm_modeset_acquire_ctx onto the stack. I think for understanding what's going on here you'd first need to convert the code to the full open-code boilerplate using drm_modeset_lock_all_ctx, with explanations of why the changes are ok. Then replace it with the convenient macro. Once it's clear what's going on under the hood it should then be easier to explain the situation in subsequent conversions with just one patch. -Daniel > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > --- > > Changes from v1: Added further information in the commit message. > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 6447cd6ca5a8..e1a71579f8e6 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -32,6 +32,7 @@ > #include <linux/slab.h> > > #include <drm/drm_atomic_helper.h> > +#include <drm/drm_drv.h> > #include <drm/drm_probe_helper.h> > #include <drm/amdgpu_drm.h> > #include <linux/vgaarb.h> > @@ -3694,14 +3695,17 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon) > > if (!amdgpu_device_has_dc_support(adev)) { > /* turn off display hw */ > - drm_modeset_lock_all(dev); > + struct drm_modeset_acquire_ctx ctx; > + int ret; > + > + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); > drm_connector_list_iter_begin(dev, &iter); > drm_for_each_connector_iter(connector, &iter) > drm_helper_connector_dpms(connector, > DRM_MODE_DPMS_OFF); > drm_connector_list_iter_end(&iter); > - drm_modeset_unlock_all(dev); > - /* unpin the front buffers and cursors */ > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > + /* unpin the front buffers and cursors */ > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc); > struct drm_framebuffer *fb = crtc->primary->fb; > @@ -3830,19 +3834,21 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon) > /* blat the mode back in */ > if (fbcon) { > if (!amdgpu_device_has_dc_support(adev)) { > + struct drm_modeset_acquire_ctx ctx; > + int ret; > + > /* pre DCE11 */ > drm_helper_resume_force_mode(dev); > > /* turn on display hw */ > - drm_modeset_lock_all(dev); > + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); > > drm_connector_list_iter_begin(dev, &iter); > drm_for_each_connector_iter(connector, &iter) > drm_helper_connector_dpms(connector, > DRM_MODE_DPMS_ON); > drm_connector_list_iter_end(&iter); > - > - drm_modeset_unlock_all(dev); > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > } > amdgpu_fbdev_set_suspend(adev, 0); > } > -- > 2.31.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm/amd/amdgpu/amdgpu_device.c: Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* 2021-04-20 17:49 ` Daniel Vetter @ 2021-04-20 19:38 ` Fabio M. De Francesco -1 siblings, 0 replies; 10+ messages in thread From: Fabio M. De Francesco @ 2021-04-20 19:38 UTC (permalink / raw) To: Daniel Vetter Cc: outreachy-kernel, dri-devel, Alex Deucher, Christian König, David Airlie, Daniel Vetter, Melissa Wen On Tuesday, April 20, 2021 7:49:35 PM CEST Daniel Vetter wrote: > On Mon, Apr 19, 2021 at 05:03:40PM +0200, Fabio M. De Francesco wrote: > > Replace the deprecated API with new helpers, according to the TODO list > > of the DRM subsystem. The new API has been introduced with commit > > b7ea04d299c7: DRM_MODESET_LOCK_ALL_BEGIN() simplifies grabbing all modeset > > locks using a local context and has the advantage of reducing boilerplate. > > So this is only half of the story, because the boilerplate only exists > when you're using drm_modeset_lock_all_ctx. Which should be used for > modern atomic display drivers everywhere. > > But drm_modeset_lock_all_ctx isn't exactly the same as > drm_modeset_lock_all, so this needs to be explained. > > Now the problem with drm_modeset_lock_all is that it hides a memory > allocation, and if that allocation fails then we just die. Which isn't > great really, but in practice the kernel tries really hard to never fail > this allocation. That's why we move the drm_modeset_acquire_ctx onto the > stack. > > I think for understanding what's going on here you'd first need to convert > the code to the full open-code boilerplate using drm_modeset_lock_all_ctx, > with explanations of why the changes are ok. Then replace it with the > convenient macro. Once it's clear what's going on under the hood it should > then be easier to explain the situation in subsequent conversions with > just one patch. > -Daniel > Thanks for the clarification. I'll go through this code again using the path you showed. Eventually I will send out another patch. Fabio > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > > --- > > > > Changes from v1: Added further information in the commit message. > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++------ > > 1 file changed, 12 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 6447cd6ca5a8..e1a71579f8e6 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > @@ -32,6 +32,7 @@ > > > > #include <linux/slab.h> > > > > #include <drm/drm_atomic_helper.h> > > > > +#include <drm/drm_drv.h> > > > > #include <drm/drm_probe_helper.h> > > #include <drm/amdgpu_drm.h> > > #include <linux/vgaarb.h> > > > > @@ -3694,14 +3695,17 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon) > > > > if (!amdgpu_device_has_dc_support(adev)) { > > > > /* turn off display hw */ > > > > - drm_modeset_lock_all(dev); > > + struct drm_modeset_acquire_ctx ctx; > > + int ret; > > + > > + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); > > > > drm_connector_list_iter_begin(dev, &iter); > > drm_for_each_connector_iter(connector, &iter) > > > > drm_helper_connector_dpms(connector, > > > > DRM_MODE_DPMS_OFF); > > > > drm_connector_list_iter_end(&iter); > > > > - drm_modeset_unlock_all(dev); > > - /* unpin the front buffers and cursors */ > > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > > + /* unpin the front buffers and cursors */ > > > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > > > > struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc); > > struct drm_framebuffer *fb = crtc->primary- >fb; > > > > @@ -3830,19 +3834,21 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon) > > > > /* blat the mode back in */ > > if (fbcon) { > > > > if (!amdgpu_device_has_dc_support(adev)) { > > > > + struct drm_modeset_acquire_ctx ctx; > > + int ret; > > + > > > > /* pre DCE11 */ > > drm_helper_resume_force_mode(dev); > > > > /* turn on display hw */ > > > > - drm_modeset_lock_all(dev); > > + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); > > > > drm_connector_list_iter_begin(dev, &iter); > > drm_for_each_connector_iter(connector, &iter) > > > > drm_helper_connector_dpms(connector, > > > > DRM_MODE_DPMS_ON); > > > > drm_connector_list_iter_end(&iter); > > > > - > > - drm_modeset_unlock_all(dev); > > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > > > > } > > amdgpu_fbdev_set_suspend(adev, 0); > > > > } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm/amd/amdgpu/amdgpu_device.c: Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* @ 2021-04-20 19:38 ` Fabio M. De Francesco 0 siblings, 0 replies; 10+ messages in thread From: Fabio M. De Francesco @ 2021-04-20 19:38 UTC (permalink / raw) To: Daniel Vetter Cc: David Airlie, dri-devel, Melissa Wen, outreachy-kernel, Alex Deucher, Christian König On Tuesday, April 20, 2021 7:49:35 PM CEST Daniel Vetter wrote: > On Mon, Apr 19, 2021 at 05:03:40PM +0200, Fabio M. De Francesco wrote: > > Replace the deprecated API with new helpers, according to the TODO list > > of the DRM subsystem. The new API has been introduced with commit > > b7ea04d299c7: DRM_MODESET_LOCK_ALL_BEGIN() simplifies grabbing all modeset > > locks using a local context and has the advantage of reducing boilerplate. > > So this is only half of the story, because the boilerplate only exists > when you're using drm_modeset_lock_all_ctx. Which should be used for > modern atomic display drivers everywhere. > > But drm_modeset_lock_all_ctx isn't exactly the same as > drm_modeset_lock_all, so this needs to be explained. > > Now the problem with drm_modeset_lock_all is that it hides a memory > allocation, and if that allocation fails then we just die. Which isn't > great really, but in practice the kernel tries really hard to never fail > this allocation. That's why we move the drm_modeset_acquire_ctx onto the > stack. > > I think for understanding what's going on here you'd first need to convert > the code to the full open-code boilerplate using drm_modeset_lock_all_ctx, > with explanations of why the changes are ok. Then replace it with the > convenient macro. Once it's clear what's going on under the hood it should > then be easier to explain the situation in subsequent conversions with > just one patch. > -Daniel > Thanks for the clarification. I'll go through this code again using the path you showed. Eventually I will send out another patch. Fabio > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > > --- > > > > Changes from v1: Added further information in the commit message. > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++------ > > 1 file changed, 12 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 6447cd6ca5a8..e1a71579f8e6 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > @@ -32,6 +32,7 @@ > > > > #include <linux/slab.h> > > > > #include <drm/drm_atomic_helper.h> > > > > +#include <drm/drm_drv.h> > > > > #include <drm/drm_probe_helper.h> > > #include <drm/amdgpu_drm.h> > > #include <linux/vgaarb.h> > > > > @@ -3694,14 +3695,17 @@ int amdgpu_device_suspend(struct drm_device *dev, bool fbcon) > > > > if (!amdgpu_device_has_dc_support(adev)) { > > > > /* turn off display hw */ > > > > - drm_modeset_lock_all(dev); > > + struct drm_modeset_acquire_ctx ctx; > > + int ret; > > + > > + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); > > > > drm_connector_list_iter_begin(dev, &iter); > > drm_for_each_connector_iter(connector, &iter) > > > > drm_helper_connector_dpms(connector, > > > > DRM_MODE_DPMS_OFF); > > > > drm_connector_list_iter_end(&iter); > > > > - drm_modeset_unlock_all(dev); > > - /* unpin the front buffers and cursors */ > > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > > + /* unpin the front buffers and cursors */ > > > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > > > > struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc); > > struct drm_framebuffer *fb = crtc->primary- >fb; > > > > @@ -3830,19 +3834,21 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon) > > > > /* blat the mode back in */ > > if (fbcon) { > > > > if (!amdgpu_device_has_dc_support(adev)) { > > > > + struct drm_modeset_acquire_ctx ctx; > > + int ret; > > + > > > > /* pre DCE11 */ > > drm_helper_resume_force_mode(dev); > > > > /* turn on display hw */ > > > > - drm_modeset_lock_all(dev); > > + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret); > > > > drm_connector_list_iter_begin(dev, &iter); > > drm_for_each_connector_iter(connector, &iter) > > > > drm_helper_connector_dpms(connector, > > > > DRM_MODE_DPMS_ON); > > > > drm_connector_list_iter_end(&iter); > > > > - > > - drm_modeset_unlock_all(dev); > > + DRM_MODESET_LOCK_ALL_END(dev, ctx, ret); > > > > } > > amdgpu_fbdev_set_suspend(adev, 0); > > > > } _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] drm/amd/amdgpu/amdgpu_drv.c: Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* 2021-04-19 15:03 ` Fabio M. De Francesco @ 2021-04-19 15:03 ` Fabio M. De Francesco -1 siblings, 0 replies; 10+ messages in thread From: Fabio M. De Francesco @ 2021-04-19 15:03 UTC (permalink / raw) To: outreachy-kernel, dri-devel, Alex Deucher, Christian König, David Airlie, Daniel Vetter, Melissa Wen Cc: Fabio M. De Francesco Replace the deprecated API with new helpers, according to the TODO list of the DRM subsystem. The new API has been introduced with commit b7ea04d299c7: DRM_MODESET_LOCK_ALL_BEGIN() simplifies grabbing all modeset locks using a local context and has the advantage of reducing boilerplate. Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- Changes from v1: Added further information to the commit message. drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 671ec1002230..0e9b7a180ee7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -1438,8 +1438,10 @@ static int amdgpu_pmops_runtime_idle(struct device *dev) if (amdgpu_device_has_dc_support(adev)) { struct drm_crtc *crtc; + struct drm_modeset_acquire_ctx ctx; + int ret_lock; - drm_modeset_lock_all(drm_dev); + DRM_MODESET_LOCK_ALL_BEGIN(drm_dev, ctx, 0, ret_lock); drm_for_each_crtc(crtc, drm_dev) { if (crtc->state->active) { @@ -1448,7 +1450,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev) } } - drm_modeset_unlock_all(drm_dev); + DRM_MODESET_LOCK_ALL_END(drm_dev, ctx, ret_lock); } else { struct drm_connector *list_connector; -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] drm/amd/amdgpu/amdgpu_drv.c: Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* @ 2021-04-19 15:03 ` Fabio M. De Francesco 0 siblings, 0 replies; 10+ messages in thread From: Fabio M. De Francesco @ 2021-04-19 15:03 UTC (permalink / raw) To: outreachy-kernel, dri-devel, Alex Deucher, Christian König, David Airlie, Daniel Vetter, Melissa Wen Cc: Fabio M. De Francesco Replace the deprecated API with new helpers, according to the TODO list of the DRM subsystem. The new API has been introduced with commit b7ea04d299c7: DRM_MODESET_LOCK_ALL_BEGIN() simplifies grabbing all modeset locks using a local context and has the advantage of reducing boilerplate. Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- Changes from v1: Added further information to the commit message. drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 671ec1002230..0e9b7a180ee7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -1438,8 +1438,10 @@ static int amdgpu_pmops_runtime_idle(struct device *dev) if (amdgpu_device_has_dc_support(adev)) { struct drm_crtc *crtc; + struct drm_modeset_acquire_ctx ctx; + int ret_lock; - drm_modeset_lock_all(drm_dev); + DRM_MODESET_LOCK_ALL_BEGIN(drm_dev, ctx, 0, ret_lock); drm_for_each_crtc(crtc, drm_dev) { if (crtc->state->active) { @@ -1448,7 +1450,7 @@ static int amdgpu_pmops_runtime_idle(struct device *dev) } } - drm_modeset_unlock_all(drm_dev); + DRM_MODESET_LOCK_ALL_END(drm_dev, ctx, ret_lock); } else { struct drm_connector *list_connector; -- 2.31.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-04-20 19:39 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-04-19 15:03 [PATCH v2 0/2] Replace drm_modeset_*_all with DRM_MODESET_LOCK_ALL_* Fabio M. De Francesco 2021-04-19 15:03 ` Fabio M. De Francesco 2021-04-19 15:03 ` [PATCH v2 1/2] drm/amd/amdgpu/amdgpu_device.c: " Fabio M. De Francesco 2021-04-19 15:03 ` Fabio M. De Francesco 2021-04-20 17:49 ` Daniel Vetter 2021-04-20 17:49 ` Daniel Vetter 2021-04-20 19:38 ` Fabio M. De Francesco 2021-04-20 19:38 ` Fabio M. De Francesco 2021-04-19 15:03 ` [PATCH v2 2/2] drm/amd/amdgpu/amdgpu_drv.c: " Fabio M. De Francesco 2021-04-19 15:03 ` Fabio M. De Francesco
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.