Message ID | 20170621082850.13224-12-daniel.vetter@ffwll.ch (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Wed, Jun 21, 2017 at 10:28:48AM +0200, Daniel Vetter wrote: > Again stopping the vblank before uninstalling the irq handler is kinda > the wrong way round, but the fb_off stuff should take care of > disabling the dsiplay at least in most cases. So drop the > drm_vblank_cleanup code since it's not really doing anything, it looks > all cargo-culted. > > v2: Appease gcc better. > > Cc: Sinclair Yeh <syeh@vmware.com> > Cc: Thomas Hellstrom <thellstrom@vmware.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 +++------ > drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 -- > drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 ---- > drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 9 --------- > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 27 +-------------------------- > 5 files changed, 4 insertions(+), 47 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > index 3d94ea67a825..a93c0bb73452 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > @@ -1658,7 +1658,7 @@ int vmw_kms_init(struct vmw_private *dev_priv) > > int vmw_kms_close(struct vmw_private *dev_priv) > { > - int ret; > + int ret = 0; > > /* > * Docs says we should take the lock before calling this function > @@ -1666,11 +1666,8 @@ int vmw_kms_close(struct vmw_private *dev_priv) > * drm_encoder_cleanup which takes the lock we deadlock. > */ > drm_mode_config_cleanup(dev_priv->dev); > - if (dev_priv->active_display_unit == vmw_du_screen_object) > - ret = vmw_kms_sou_close_display(dev_priv); > - else if (dev_priv->active_display_unit == vmw_du_screen_target) > - ret = vmw_kms_stdu_close_display(dev_priv); > - else > + if (dev_priv->active_display_unit != vmw_du_screen_object && > + dev_priv->active_display_unit != vmw_du_screen_target) I think this is equivalent to: if (dev_priv->active_display_unit == vmw_du_legacy) > ret = vmw_kms_ldu_close_display(dev_priv); > > return ret; > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h > index 5f8d678ae675..ff9c8389ff21 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h > @@ -390,7 +390,6 @@ int vmw_kms_update_proxy(struct vmw_resource *res, > * Screen Objects display functions - vmwgfx_scrn.c > */ > int vmw_kms_sou_init_display(struct vmw_private *dev_priv); > -int vmw_kms_sou_close_display(struct vmw_private *dev_priv); > int vmw_kms_sou_do_surface_dirty(struct vmw_private *dev_priv, > struct vmw_framebuffer *framebuffer, > struct drm_clip_rect *clips, > @@ -418,7 +417,6 @@ int vmw_kms_sou_readback(struct vmw_private *dev_priv, > * Screen Target Display Unit functions - vmwgfx_stdu.c > */ > int vmw_kms_stdu_init_display(struct vmw_private *dev_priv); > -int vmw_kms_stdu_close_display(struct vmw_private *dev_priv); > int vmw_kms_stdu_surface_dirty(struct vmw_private *dev_priv, > struct vmw_framebuffer *framebuffer, > struct drm_clip_rect *clips, > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c > index d3987bcf53f8..449ed4fba0f2 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c > @@ -582,13 +582,9 @@ int vmw_kms_ldu_init_display(struct vmw_private *dev_priv) > > int vmw_kms_ldu_close_display(struct vmw_private *dev_priv) > { > - struct drm_device *dev = dev_priv->dev; > - > if (!dev_priv->ldu_priv) > return -ENOSYS; > > - drm_vblank_cleanup(dev); > - > BUG_ON(!list_empty(&dev_priv->ldu_priv->active)); > > kfree(dev_priv->ldu_priv); > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > index 8d7dc9def7c2..3b917c9b0c21 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > @@ -746,15 +746,6 @@ int vmw_kms_sou_init_display(struct vmw_private *dev_priv) > return 0; > } > > -int vmw_kms_sou_close_display(struct vmw_private *dev_priv) > -{ > - struct drm_device *dev = dev_priv->dev; > - > - drm_vblank_cleanup(dev); > - > - return 0; > -} > - > static int do_dmabuf_define_gmrfb(struct vmw_private *dev_priv, > struct vmw_framebuffer *framebuffer) > { > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > index 50be1f034f9e..6aecba6cd5e2 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > @@ -1651,36 +1651,11 @@ int vmw_kms_stdu_init_display(struct vmw_private *dev_priv) > > if (unlikely(ret != 0)) { > DRM_ERROR("Failed to initialize STDU %d", i); > - goto err_vblank_cleanup; > + return ret; > } > } > > DRM_INFO("Screen Target Display device initialized\n"); > > return 0; > - > -err_vblank_cleanup: > - drm_vblank_cleanup(dev); > - return ret; > -} > - > - > - > -/** > - * vmw_kms_stdu_close_display - Cleans up after vmw_kms_stdu_init_display > - * > - * @dev_priv: VMW DRM device > - * > - * Frees up any resources allocated by vmw_kms_stdu_init_display > - * > - * RETURNS: > - * 0 on success > - */ > -int vmw_kms_stdu_close_display(struct vmw_private *dev_priv) > -{ > - struct drm_device *dev = dev_priv->dev; > - > - drm_vblank_cleanup(dev); > - > - return 0; > } > -- > 2.11.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
With the below fixed, Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com> On 06/23/2017 04:10 PM, Sean Paul wrote: > On Wed, Jun 21, 2017 at 10:28:48AM +0200, Daniel Vetter wrote: >> Again stopping the vblank before uninstalling the irq handler is kinda >> the wrong way round, but the fb_off stuff should take care of >> disabling the dsiplay at least in most cases. So drop the >> drm_vblank_cleanup code since it's not really doing anything, it looks >> all cargo-culted. >> >> v2: Appease gcc better. >> >> Cc: Sinclair Yeh <syeh@vmware.com> >> Cc: Thomas Hellstrom <thellstrom@vmware.com> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> --- >> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 +++------ >> drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 -- >> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 ---- >> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 9 --------- >> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 27 +-------------------------- >> 5 files changed, 4 insertions(+), 47 deletions(-) >> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> index 3d94ea67a825..a93c0bb73452 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c >> @@ -1658,7 +1658,7 @@ int vmw_kms_init(struct vmw_private *dev_priv) >> >> int vmw_kms_close(struct vmw_private *dev_priv) >> { >> - int ret; >> + int ret = 0; >> >> /* >> * Docs says we should take the lock before calling this function >> @@ -1666,11 +1666,8 @@ int vmw_kms_close(struct vmw_private *dev_priv) >> * drm_encoder_cleanup which takes the lock we deadlock. >> */ >> drm_mode_config_cleanup(dev_priv->dev); >> - if (dev_priv->active_display_unit == vmw_du_screen_object) >> - ret = vmw_kms_sou_close_display(dev_priv); >> - else if (dev_priv->active_display_unit == vmw_du_screen_target) >> - ret = vmw_kms_stdu_close_display(dev_priv); >> - else >> + if (dev_priv->active_display_unit != vmw_du_screen_object && >> + dev_priv->active_display_unit != vmw_du_screen_target) > I think this is equivalent to: > > if (dev_priv->active_display_unit == vmw_du_legacy) > >> ret = vmw_kms_ldu_close_display(dev_priv); >> >> return ret; >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h >> index 5f8d678ae675..ff9c8389ff21 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h >> @@ -390,7 +390,6 @@ int vmw_kms_update_proxy(struct vmw_resource *res, >> * Screen Objects display functions - vmwgfx_scrn.c >> */ >> int vmw_kms_sou_init_display(struct vmw_private *dev_priv); >> -int vmw_kms_sou_close_display(struct vmw_private *dev_priv); >> int vmw_kms_sou_do_surface_dirty(struct vmw_private *dev_priv, >> struct vmw_framebuffer *framebuffer, >> struct drm_clip_rect *clips, >> @@ -418,7 +417,6 @@ int vmw_kms_sou_readback(struct vmw_private *dev_priv, >> * Screen Target Display Unit functions - vmwgfx_stdu.c >> */ >> int vmw_kms_stdu_init_display(struct vmw_private *dev_priv); >> -int vmw_kms_stdu_close_display(struct vmw_private *dev_priv); >> int vmw_kms_stdu_surface_dirty(struct vmw_private *dev_priv, >> struct vmw_framebuffer *framebuffer, >> struct drm_clip_rect *clips, >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c >> index d3987bcf53f8..449ed4fba0f2 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c >> @@ -582,13 +582,9 @@ int vmw_kms_ldu_init_display(struct vmw_private *dev_priv) >> >> int vmw_kms_ldu_close_display(struct vmw_private *dev_priv) >> { >> - struct drm_device *dev = dev_priv->dev; >> - >> if (!dev_priv->ldu_priv) >> return -ENOSYS; >> >> - drm_vblank_cleanup(dev); >> - >> BUG_ON(!list_empty(&dev_priv->ldu_priv->active)); >> >> kfree(dev_priv->ldu_priv); >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c >> index 8d7dc9def7c2..3b917c9b0c21 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c >> @@ -746,15 +746,6 @@ int vmw_kms_sou_init_display(struct vmw_private *dev_priv) >> return 0; >> } >> >> -int vmw_kms_sou_close_display(struct vmw_private *dev_priv) >> -{ >> - struct drm_device *dev = dev_priv->dev; >> - >> - drm_vblank_cleanup(dev); >> - >> - return 0; >> -} >> - >> static int do_dmabuf_define_gmrfb(struct vmw_private *dev_priv, >> struct vmw_framebuffer *framebuffer) >> { >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c >> index 50be1f034f9e..6aecba6cd5e2 100644 >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c >> @@ -1651,36 +1651,11 @@ int vmw_kms_stdu_init_display(struct vmw_private *dev_priv) >> >> if (unlikely(ret != 0)) { >> DRM_ERROR("Failed to initialize STDU %d", i); >> - goto err_vblank_cleanup; >> + return ret; >> } >> } >> >> DRM_INFO("Screen Target Display device initialized\n"); >> >> return 0; >> - >> -err_vblank_cleanup: >> - drm_vblank_cleanup(dev); >> - return ret; >> -} >> - >> - >> - >> -/** >> - * vmw_kms_stdu_close_display - Cleans up after vmw_kms_stdu_init_display >> - * >> - * @dev_priv: VMW DRM device >> - * >> - * Frees up any resources allocated by vmw_kms_stdu_init_display >> - * >> - * RETURNS: >> - * 0 on success >> - */ >> -int vmw_kms_stdu_close_display(struct vmw_private *dev_priv) >> -{ >> - struct drm_device *dev = dev_priv->dev; >> - >> - drm_vblank_cleanup(dev); >> - >> - return 0; >> } >> -- >> 2.11.0 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=DwIBAg&c=uilaK90D4TOVoH58JNXRgQ&r=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ&m=SJKJc5bNyeaTecQ5ygKQsN-9XVZSCWUtM90cLY6uCwg&s=x2_NlUiPI4px-JC4k-ZDsMw_PPTJv8lq-J7AnjxbuVQ&e=
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 3d94ea67a825..a93c0bb73452 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -1658,7 +1658,7 @@ int vmw_kms_init(struct vmw_private *dev_priv) int vmw_kms_close(struct vmw_private *dev_priv) { - int ret; + int ret = 0; /* * Docs says we should take the lock before calling this function @@ -1666,11 +1666,8 @@ int vmw_kms_close(struct vmw_private *dev_priv) * drm_encoder_cleanup which takes the lock we deadlock. */ drm_mode_config_cleanup(dev_priv->dev); - if (dev_priv->active_display_unit == vmw_du_screen_object) - ret = vmw_kms_sou_close_display(dev_priv); - else if (dev_priv->active_display_unit == vmw_du_screen_target) - ret = vmw_kms_stdu_close_display(dev_priv); - else + if (dev_priv->active_display_unit != vmw_du_screen_object && + dev_priv->active_display_unit != vmw_du_screen_target) ret = vmw_kms_ldu_close_display(dev_priv); return ret; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h index 5f8d678ae675..ff9c8389ff21 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h @@ -390,7 +390,6 @@ int vmw_kms_update_proxy(struct vmw_resource *res, * Screen Objects display functions - vmwgfx_scrn.c */ int vmw_kms_sou_init_display(struct vmw_private *dev_priv); -int vmw_kms_sou_close_display(struct vmw_private *dev_priv); int vmw_kms_sou_do_surface_dirty(struct vmw_private *dev_priv, struct vmw_framebuffer *framebuffer, struct drm_clip_rect *clips, @@ -418,7 +417,6 @@ int vmw_kms_sou_readback(struct vmw_private *dev_priv, * Screen Target Display Unit functions - vmwgfx_stdu.c */ int vmw_kms_stdu_init_display(struct vmw_private *dev_priv); -int vmw_kms_stdu_close_display(struct vmw_private *dev_priv); int vmw_kms_stdu_surface_dirty(struct vmw_private *dev_priv, struct vmw_framebuffer *framebuffer, struct drm_clip_rect *clips, diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c index d3987bcf53f8..449ed4fba0f2 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c @@ -582,13 +582,9 @@ int vmw_kms_ldu_init_display(struct vmw_private *dev_priv) int vmw_kms_ldu_close_display(struct vmw_private *dev_priv) { - struct drm_device *dev = dev_priv->dev; - if (!dev_priv->ldu_priv) return -ENOSYS; - drm_vblank_cleanup(dev); - BUG_ON(!list_empty(&dev_priv->ldu_priv->active)); kfree(dev_priv->ldu_priv); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c index 8d7dc9def7c2..3b917c9b0c21 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c @@ -746,15 +746,6 @@ int vmw_kms_sou_init_display(struct vmw_private *dev_priv) return 0; } -int vmw_kms_sou_close_display(struct vmw_private *dev_priv) -{ - struct drm_device *dev = dev_priv->dev; - - drm_vblank_cleanup(dev); - - return 0; -} - static int do_dmabuf_define_gmrfb(struct vmw_private *dev_priv, struct vmw_framebuffer *framebuffer) { diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c index 50be1f034f9e..6aecba6cd5e2 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c @@ -1651,36 +1651,11 @@ int vmw_kms_stdu_init_display(struct vmw_private *dev_priv) if (unlikely(ret != 0)) { DRM_ERROR("Failed to initialize STDU %d", i); - goto err_vblank_cleanup; + return ret; } } DRM_INFO("Screen Target Display device initialized\n"); return 0; - -err_vblank_cleanup: - drm_vblank_cleanup(dev); - return ret; -} - - - -/** - * vmw_kms_stdu_close_display - Cleans up after vmw_kms_stdu_init_display - * - * @dev_priv: VMW DRM device - * - * Frees up any resources allocated by vmw_kms_stdu_init_display - * - * RETURNS: - * 0 on success - */ -int vmw_kms_stdu_close_display(struct vmw_private *dev_priv) -{ - struct drm_device *dev = dev_priv->dev; - - drm_vblank_cleanup(dev); - - return 0; }
Again stopping the vblank before uninstalling the irq handler is kinda the wrong way round, but the fb_off stuff should take care of disabling the dsiplay at least in most cases. So drop the drm_vblank_cleanup code since it's not really doing anything, it looks all cargo-culted. v2: Appease gcc better. Cc: Sinclair Yeh <syeh@vmware.com> Cc: Thomas Hellstrom <thellstrom@vmware.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 +++------ drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 -- drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 ---- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 9 --------- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 27 +-------------------------- 5 files changed, 4 insertions(+), 47 deletions(-)