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@xxxxxxxxx>
> > ---
> >
> > 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
