| Message ID | 20210422155951.6314-1-fmdefrancesco@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/amdgpu/amdgpu_drv.c: Replace drm_modeset_lock_all with drm_modeset_lock | expand |
On Thu, Apr 22, 2021 at 05:59:51PM +0200, Fabio M. De Francesco wrote: > - drm_modeset_lock_all(drm_dev); > - > drm_for_each_crtc(crtc, drm_dev) { > + drm_modeset_lock(&crtc->mutex, NULL); > if (crtc->state->active) { > ret = -EBUSY; > - break; > } > + drm_modeset_unlock(&crtc->mutex); > + if (ret < 0) > + break; > } > > - drm_modeset_unlock_all(drm_dev); > - I might remove the {} around ret = -EBUSY, but this is good. Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
On Thu, Apr 22, 2021 at 05:50:34PM +0100, Matthew Wilcox wrote: > On Thu, Apr 22, 2021 at 05:59:51PM +0200, Fabio M. De Francesco wrote: > > - drm_modeset_lock_all(drm_dev); > > - > > drm_for_each_crtc(crtc, drm_dev) { > > + drm_modeset_lock(&crtc->mutex, NULL); > > if (crtc->state->active) { > > ret = -EBUSY; > > - break; > > } > > + drm_modeset_unlock(&crtc->mutex); > > + if (ret < 0) > > + break; > > } > > > > - drm_modeset_unlock_all(drm_dev); > > - > > I might remove the {} around ret = -EBUSY, but this is good. > > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> Yup patch looks good, but it's not cc'ed to drm/amdgpu maintainers/m-l, so likely won't get picked up. Can you pls check scripts/get_maintainers for anything you've missed, add those and resend with Willy's r-b tag included? Then Alex can pick it up for merging. Thanks, Daniel
On Monday, April 26, 2021 6:11:11 PM CEST Daniel Vetter wrote: > On Thu, Apr 22, 2021 at 05:50:34PM +0100, Matthew Wilcox wrote: > > On Thu, Apr 22, 2021 at 05:59:51PM +0200, Fabio M. De Francesco wrote: > > > - drm_modeset_lock_all(drm_dev); > > > - > > > > > > drm_for_each_crtc(crtc, drm_dev) { > > > > > > + drm_modeset_lock(&crtc->mutex, NULL); > > > > > > if (crtc->state->active) { > > > > > > ret = -EBUSY; > > > > > > - break; > > > > > > } > > > > > > + drm_modeset_unlock(&crtc->mutex); > > > + if (ret < 0) > > > + break; > > > > > > } > > > > > > - drm_modeset_unlock_all(drm_dev); > > > - > > > > I might remove the {} around ret = -EBUSY, but this is good. > > > > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > Yup patch looks good, but it's not cc'ed to drm/amdgpu maintainers/m-l, so > likely won't get picked up. Can you pls check scripts/get_maintainers for > anything you've missed, add those and resend with Willy's r-b tag > included? > > Then Alex can pick it up for merging. > > Thanks, Daniel > I had already submitted a v2 of this patch with an added 'Review-by' Matthew Wilcox under my name. It removed the unnecessary braces that willy pointed out. However I see that not all maintainers had been cc'ed, so I'm going to resend it as v3. Thanks, Fabio
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 671ec1002230..bce8f6793d8e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -1439,17 +1439,16 @@ static int amdgpu_pmops_runtime_idle(struct device *dev) if (amdgpu_device_has_dc_support(adev)) { struct drm_crtc *crtc; - drm_modeset_lock_all(drm_dev); - drm_for_each_crtc(crtc, drm_dev) { + drm_modeset_lock(&crtc->mutex, NULL); if (crtc->state->active) { ret = -EBUSY; - break; } + drm_modeset_unlock(&crtc->mutex); + if (ret < 0) + break; } - drm_modeset_unlock_all(drm_dev); - } else { struct drm_connector *list_connector; struct drm_connector_list_iter iter;
drm_modeset_lock_all() is not needed here, so it is replaced with drm_modeset_lock(). The crtc list around which we are looping never changes, therefore the only lock we need is to protect access to crtc->state. Suggested-by: Daniel Vetter <daniel@ffwll.ch> Suggested-by: Matthew Wilcox <willy@infradead.org> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)